diff --git a/app/models/packages/protection/rule.rb b/app/models/packages/protection/rule.rb index fafcbd025d7114542489fcc0e469f628122bd91b..0913bc875ec414dba158c195b5896791460078fb 100644 --- a/app/models/packages/protection/rule.rb +++ b/app/models/packages/protection/rule.rb @@ -4,6 +4,9 @@ module Packages module Protection class Rule < ApplicationRecord enum package_type: Packages::Package.package_types.slice(:conan, :maven, :npm, :pypi) + enum minimum_access_level_for_delete: + Gitlab::Access.sym_options_with_admin.slice(:owner, :admin), + _prefix: :minimum_access_level_for_delete enum minimum_access_level_for_push: Gitlab::Access.sym_options_with_admin.slice(:maintainer, :owner, :admin), _prefix: :minimum_access_level_for_push @@ -25,7 +28,8 @@ class Rule < ApplicationRecord }, if: :pypi? validates :package_type, presence: true - validates :minimum_access_level_for_push, presence: true + + validate :at_least_one_minimum_access_level_must_be_present scope :for_package_name, ->(package_name) do return none if package_name.blank? @@ -38,11 +42,13 @@ class Rule < ApplicationRecord scope :for_package_type, ->(package_type) { where(package_type: package_type) } - def self.for_push_exists?(access_level:, package_name:, package_type:) + def self.for_action_exists?(action:, access_level:, package_name:, package_type:) return false if [access_level, package_name, package_type].any?(&:blank?) + minimum_access_level_column = "minimum_access_level_for_#{action}" + for_package_type(package_type) - .where(':access_level < minimum_access_level_for_push', access_level: access_level) + .where(":access_level < #{minimum_access_level_column}", access_level: access_level) .for_package_name(package_name) .exists? end @@ -113,6 +119,12 @@ def self.for_push_exists_for_projects_and_packages(projects_and_packages) connection.exec_query(query.with(cte.to_arel).to_sql) end + + def at_least_one_minimum_access_level_must_be_present + return unless minimum_access_level_for_delete.blank? && minimum_access_level_for_push.blank? + + errors.add(:base, _('A rule must have at least a minimum access role for push or delete.')) + end end end end diff --git a/app/services/packages/protection/check_rule_existence_service.rb b/app/services/packages/protection/check_rule_existence_service.rb index 1af5ec0621e1410e929e13a9e77e4c5e59648ce7..8a705e3bda0c3631f3a789ffba71c5c724c21008 100644 --- a/app/services/packages/protection/check_rule_existence_service.rb +++ b/app/services/packages/protection/check_rule_existence_service.rb @@ -35,7 +35,8 @@ def check_rule_exists_for_user user_project_authorization_access_level = current_user.max_member_access_for_project(project.id) project.package_protection_rules - .for_push_exists?( + .for_action_exists?( + action: :push, access_level: user_project_authorization_access_level, package_name: params[:package_name], package_type: params[:package_type] diff --git a/db/migrate/20250130161000_remove_not_null_constraint_from_mininum_access_level_for_push.rb b/db/migrate/20250130161000_remove_not_null_constraint_from_mininum_access_level_for_push.rb new file mode 100644 index 0000000000000000000000000000000000000000..7dadc63fcc2f4e8cd0bc09b1c9242ca906ae2425 --- /dev/null +++ b/db/migrate/20250130161000_remove_not_null_constraint_from_mininum_access_level_for_push.rb @@ -0,0 +1,15 @@ +# frozen_string_literal: true + +class RemoveNotNullConstraintFromMininumAccessLevelForPush < Gitlab::Database::Migration[2.2] + milestone '17.9' + + disable_ddl_transaction! + + def up + remove_not_null_constraint :packages_protection_rules, :minimum_access_level_for_push + end + + def down + add_not_null_constraint :packages_protection_rules, :minimum_access_level_for_push + end +end diff --git a/db/migrate/20250130162000_remove_package_protection_rules_on_down_migration.rb b/db/migrate/20250130162000_remove_package_protection_rules_on_down_migration.rb new file mode 100644 index 0000000000000000000000000000000000000000..e5ac915e1e189220fe7c4a2a726290eac80c830e --- /dev/null +++ b/db/migrate/20250130162000_remove_package_protection_rules_on_down_migration.rb @@ -0,0 +1,28 @@ +# frozen_string_literal: true + +# This migration is only relevant for the down migration case. +# It ensures that package protection rules with nil minimum_access_level_for_push are removed +# before the NOT NULL constraint is added in the previous down migration, +# i.e. 20250130161000_remove_not_null_constraint_from_mininum_access_level_for_push.rb . +# See the related discussion thread https://gitlab.com/gitlab-org/gitlab/-/merge_requests/179739#note_2334019898 +class RemovePackageProtectionRulesOnDownMigration < Gitlab::Database::Migration[2.2] + milestone '17.9' + + disable_ddl_transaction! + + restrict_gitlab_migration gitlab_schema: :gitlab_main + + class PackagesProtectionRule < MigrationRecord + self.table_name = "packages_protection_rules" + end + + def up + # no-op + end + + def down + PackagesProtectionRule.where(minimum_access_level_for_push: nil).find_in_batches(batch_size: 100) do |ids| + PackagesProtectionRule.where(id: ids).delete_all + end + end +end diff --git a/db/migrate/20250130163000_add_minimum_access_level_for_delete_to_packages_protection_rules.rb b/db/migrate/20250130163000_add_minimum_access_level_for_delete_to_packages_protection_rules.rb new file mode 100644 index 0000000000000000000000000000000000000000..abf9739e36d342ca075e206446e50ff3fb390fd9 --- /dev/null +++ b/db/migrate/20250130163000_add_minimum_access_level_for_delete_to_packages_protection_rules.rb @@ -0,0 +1,9 @@ +# frozen_string_literal: true + +class AddMinimumAccessLevelForDeleteToPackagesProtectionRules < Gitlab::Database::Migration[2.2] + milestone '17.9' + + def change + add_column :packages_protection_rules, :minimum_access_level_for_delete, :smallint, null: true, if_not_exists: true + end +end diff --git a/db/migrate/20250130164000_add_multi_not_null_constraint_on_packages_protection_rules_minimum_access_levels.rb b/db/migrate/20250130164000_add_multi_not_null_constraint_on_packages_protection_rules_minimum_access_levels.rb new file mode 100644 index 0000000000000000000000000000000000000000..75e80b2cf3ab04fcaa538e618f42ce96f9329514 --- /dev/null +++ b/db/migrate/20250130164000_add_multi_not_null_constraint_on_packages_protection_rules_minimum_access_levels.rb @@ -0,0 +1,25 @@ +# frozen_string_literal: true + +class AddMultiNotNullConstraintOnPackagesProtectionRulesMinimumAccessLevels < Gitlab::Database::Migration[2.2] + milestone '17.9' + + disable_ddl_transaction! + + TABLE_NAME = :packages_protection_rules + + def up + add_multi_column_not_null_constraint(TABLE_NAME, + :minimum_access_level_for_push, + :minimum_access_level_for_delete, + operator: '>', + limit: 0 + ) + end + + def down + remove_multi_column_not_null_constraint(TABLE_NAME, + :minimum_access_level_for_push, + :minimum_access_level_for_delete + ) + end +end diff --git a/db/schema_migrations/20250130161000 b/db/schema_migrations/20250130161000 new file mode 100644 index 0000000000000000000000000000000000000000..506450296778841dd135b39c35c4bfedcc044d42 --- /dev/null +++ b/db/schema_migrations/20250130161000 @@ -0,0 +1 @@ +d7540cd5185033e059ef5cd1fec7534cd72d6817d38515cf5ad09494ca53adca \ No newline at end of file diff --git a/db/schema_migrations/20250130162000 b/db/schema_migrations/20250130162000 new file mode 100644 index 0000000000000000000000000000000000000000..6d08b504f4effb39f934d24fdf2a6c4450ef9938 --- /dev/null +++ b/db/schema_migrations/20250130162000 @@ -0,0 +1 @@ +bec94a6bc401509b2c95bcc81c30410bd278618be2de21ae5b437f9ad2324283 \ No newline at end of file diff --git a/db/schema_migrations/20250130163000 b/db/schema_migrations/20250130163000 new file mode 100644 index 0000000000000000000000000000000000000000..304887054dd7aa4c3e17421d0e3ce01ef1b81ea6 --- /dev/null +++ b/db/schema_migrations/20250130163000 @@ -0,0 +1 @@ +2228d1a19ee7f088c24212cba45511a8e8f5ae67cdfd5b70506b20494e8d10e8 \ No newline at end of file diff --git a/db/schema_migrations/20250130164000 b/db/schema_migrations/20250130164000 new file mode 100644 index 0000000000000000000000000000000000000000..e0ef7fcb98462d6626454e7a22946d58909ca3d2 --- /dev/null +++ b/db/schema_migrations/20250130164000 @@ -0,0 +1 @@ +c35f7df366f60a6593cde2450ee35072f579cdc19eb2368d8f1a74504a329c9a \ No newline at end of file diff --git a/db/structure.sql b/db/structure.sql index 595cdcbbc25edc5be56604a14628f83c2a9efdb9..dfbbeabd0a3f4cb3ebda673c3728f8761612a10a 100644 --- a/db/structure.sql +++ b/db/structure.sql @@ -18346,7 +18346,8 @@ CREATE TABLE packages_protection_rules ( package_type smallint NOT NULL, package_name_pattern text NOT NULL, minimum_access_level_for_push smallint, - CONSTRAINT check_12e13c202a CHECK ((minimum_access_level_for_push IS NOT NULL)), + minimum_access_level_for_delete smallint, + CONSTRAINT check_520a0596a3 CHECK ((num_nonnulls(minimum_access_level_for_delete, minimum_access_level_for_push) > 0)), CONSTRAINT check_d2d75d206d CHECK ((char_length(package_name_pattern) <= 255)) ); diff --git a/spec/factories/packages/protection/rules.rb b/spec/factories/packages/protection/rules.rb index feccee72a8d0adc8e6f5c55fedfe194a3990015b..ee46c7532146cfdf3327b063fe38600abfff2923 100644 --- a/spec/factories/packages/protection/rules.rb +++ b/spec/factories/packages/protection/rules.rb @@ -5,6 +5,7 @@ project package_name_pattern { '@my_scope/my_package' } package_type { :npm } + minimum_access_level_for_delete { :owner } minimum_access_level_for_push { :maintainer } end end diff --git a/spec/migrations/20250130162000_remove_package_protection_rules_on_down_migration_spec.rb b/spec/migrations/20250130162000_remove_package_protection_rules_on_down_migration_spec.rb new file mode 100644 index 0000000000000000000000000000000000000000..73eb936c5a2b94b14748e86448b090cb365d00ff --- /dev/null +++ b/spec/migrations/20250130162000_remove_package_protection_rules_on_down_migration_spec.rb @@ -0,0 +1,56 @@ +# frozen_string_literal: true + +require 'spec_helper' +require_migration! + +RSpec.describe RemovePackageProtectionRulesOnDownMigration, migration: :gitlab_main, feature_category: :package_registry do + let(:migration) { described_class.new } + + let(:organization) { table(:organizations).create!(name: 'organization', path: 'organization') } + let(:namespace) do + table(:namespaces).create!(name: 'group', path: 'group', type: 'Group', organization_id: organization.id) + end + + let(:project) do + table(:projects).create!(namespace_id: namespace.id, project_namespace_id: namespace.id, + organization_id: organization.id) + end + + let(:packages_protection_rules) { table(:packages_protection_rules) } + + describe '#down' do + before do + packages_protection_rules.create!(project_id: project.id, package_type: 2, + package_name_pattern: '@group/package-1', minimum_access_level_for_push: nil) + + packages_protection_rules.create!(project_id: project.id, package_type: 2, + package_name_pattern: '@group/package-2', minimum_access_level_for_push: nil) + + packages_protection_rules.create!(project_id: project.id, package_type: 2, + package_name_pattern: '@group/package-3', minimum_access_level_for_push: 30) + + packages_protection_rules.create!(project_id: project.id, package_type: 2, + package_name_pattern: '@group/package-4', minimum_access_level_for_push: 40) + end + + it 'removes records with nil minimum_access_level_for_push' do + expect { migration.down } + .to change { packages_protection_rules.count }.from(4).to(2) + .and change { packages_protection_rules.where(minimum_access_level_for_push: nil).count }.from(2).to(0) + .and not_change { packages_protection_rules.where.not(minimum_access_level_for_push: nil).count } + end + + it 'preserves records with non-nil minimum_access_level_for_push' do + migration.down + + expect(packages_protection_rules.where( + package_name_pattern: '@group/package-3', + minimum_access_level_for_push: 30 + )).to exist + expect(packages_protection_rules.where( + package_name_pattern: '@group/package-4', + minimum_access_level_for_push: 40 + )).to exist + end + end +end diff --git a/spec/models/packages/protection/rule_spec.rb b/spec/models/packages/protection/rule_spec.rb index d0610de18c43a58e6b1385157a9c6ce94788ac32..7ab7ab9c4a41c9573d2612f14cce6592c37d54f7 100644 --- a/spec/models/packages/protection/rule_spec.rb +++ b/spec/models/packages/protection/rule_spec.rb @@ -32,6 +32,14 @@ .with_prefix(:minimum_access_level_for_push) ) } + + it { + is_expected.to( + define_enum_for(:minimum_access_level_for_delete) + .with_values(owner: Gitlab::Access::OWNER, admin: Gitlab::Access::ADMIN) + .with_prefix(:minimum_access_level_for_delete) + ) + } end describe 'validations' do @@ -93,8 +101,32 @@ it { is_expected.to validate_presence_of(:package_type) } end - describe '#minimum_access_level_for_push' do - it { is_expected.to validate_presence_of(:minimum_access_level_for_push) } + describe '#at_least_one_minimum_access_level_must_be_present' do + where(:minimum_access_level_for_delete, :minimum_access_level_for_push, :valid) do + :owner | :maintainer | true + :owner | nil | true + nil | :maintainer | true + nil | nil | false + end + + with_them do + subject(:package_protection_rule) do + build(:package_protection_rule, + minimum_access_level_for_delete: minimum_access_level_for_delete, + minimum_access_level_for_push: minimum_access_level_for_push + ) + end + + if params[:valid] + it { is_expected.to be_valid } + else + it 'is invalid' do + expect(package_protection_rule).not_to be_valid + expect(package_protection_rule.errors[:base]) + .to include('A rule must have at least a minimum access role for push or delete.') + end + end + end end end @@ -209,7 +241,7 @@ end end - describe '.for_push_exists?' do + describe '.for_action_exists?' do let_it_be(:project_with_ppr) { create(:project) } let_it_be(:project_without_ppr) { create(:project) } @@ -218,6 +250,19 @@ package_name_pattern: '@my-scope/my-package-stage*', project: project_with_ppr, package_type: :npm, + minimum_access_level_for_delete: :owner, + minimum_access_level_for_push: :maintainer + ) + end + + # Creating an identical package protection rule for the same project + # to ensure that overlapping rules are considered properly + let_it_be(:ppr_2_for_developer) do + create(:package_protection_rule, + package_name_pattern: '@my-scope/my-package-*', + project: project_with_ppr, + package_type: :npm, + minimum_access_level_for_delete: :owner, minimum_access_level_for_push: :maintainer ) end @@ -227,85 +272,130 @@ package_name_pattern: '@my-scope/my-package-prod*', project: project_with_ppr, package_type: :npm, + minimum_access_level_for_delete: :owner, minimum_access_level_for_push: :owner ) end - let_it_be(:ppr_owner) do + let_it_be(:ppr_for_owner) do create(:package_protection_rule, package_name_pattern: '@my-scope/my-package-release*', project: project_with_ppr, package_type: :npm, + minimum_access_level_for_delete: :admin, minimum_access_level_for_push: :admin ) end - let_it_be(:ppr_2_for_developer) do + let_it_be(:ppr_only_deletion_protection) do create(:package_protection_rule, - package_name_pattern: '@my-scope/my-package-*', + package_name_pattern: '@my-scope/only_delete-protected-package*', project: project_with_ppr, package_type: :npm, - minimum_access_level_for_push: :maintainer + minimum_access_level_for_delete: :admin, + minimum_access_level_for_push: nil + ) + end + + let_it_be(:ppr_only_push_protection) do + create(:package_protection_rule, + package_name_pattern: '@my-scope/only-push-protected-package*', + project: project_with_ppr, + package_type: :npm, + minimum_access_level_for_delete: nil, + minimum_access_level_for_push: :admin ) end subject do project .package_protection_rules - .for_push_exists?( + .for_action_exists?( + action: action, access_level: access_level, package_name: package_name, package_type: package_type ) end - describe 'with different users and protection levels' do - where(:project, :access_level, :package_name, :package_type, :push_protected) do - ref(:project_with_ppr) | Gitlab::Access::REPORTER | '@my-scope/my-package-stage-sha-1234' | :npm | true - ref(:project_with_ppr) | Gitlab::Access::DEVELOPER | '@my-scope/my-package-stage-sha-1234' | :npm | true - ref(:project_with_ppr) | Gitlab::Access::MAINTAINER | '@my-scope/my-package-stage-sha-1234' | :npm | false - ref(:project_with_ppr) | Gitlab::Access::MAINTAINER | '@my-scope/my-package-stage-sha-1234' | :npm | false - ref(:project_with_ppr) | Gitlab::Access::OWNER | '@my-scope/my-package-stage-sha-1234' | :npm | false - ref(:project_with_ppr) | Gitlab::Access::ADMIN | '@my-scope/my-package-stage-sha-1234' | :npm | false - - ref(:project_with_ppr) | Gitlab::Access::DEVELOPER | '@my-scope/my-package-prod-sha-1234' | :npm | true - ref(:project_with_ppr) | Gitlab::Access::MAINTAINER | '@my-scope/my-package-prod-sha-1234' | :npm | true - ref(:project_with_ppr) | Gitlab::Access::OWNER | '@my-scope/my-package-prod-sha-1234' | :npm | false - ref(:project_with_ppr) | Gitlab::Access::ADMIN | '@my-scope/my-package-prod-sha-1234' | :npm | false - - ref(:project_with_ppr) | Gitlab::Access::DEVELOPER | '@my-scope/my-package-release-v1' | :npm | true - ref(:project_with_ppr) | Gitlab::Access::OWNER | '@my-scope/my-package-release-v1' | :npm | true - ref(:project_with_ppr) | Gitlab::Access::ADMIN | '@my-scope/my-package-release-v1' | :npm | false - - ref(:project_with_ppr) | Gitlab::Access::DEVELOPER | '@my-scope/my-package-any-suffix' | :npm | true - ref(:project_with_ppr) | Gitlab::Access::MAINTAINER | '@my-scope/my-package-any-suffix' | :npm | false - ref(:project_with_ppr) | Gitlab::Access::OWNER | '@my-scope/my-package-any-suffix' | :npm | false - - # For non-matching package_name - ref(:project_with_ppr) | Gitlab::Access::DEVELOPER | '@my-scope/non-matching-package' | :npm | false - - # For non-matching package_type - ref(:project_with_ppr) | Gitlab::Access::DEVELOPER | '@my-scope/my-package-any-suffix' | :conan | false - - # For no access level - ref(:project_with_ppr) | Gitlab::Access::NO_ACCESS | '@my-scope/my-package-prod' | :npm | true - - # Edge cases - ref(:project_with_ppr) | nil | '@my-scope/my-package-stage-sha-1234' | :npm | false - ref(:project_with_ppr) | Gitlab::Access::DEVELOPER | nil | :npm | false - ref(:project_with_ppr) | Gitlab::Access::DEVELOPER | '' | :npm | false - ref(:project_with_ppr) | Gitlab::Access::DEVELOPER | '@my-scope/my-package-stage-sha-1234' | nil | false - ref(:project_with_ppr) | nil | nil | nil | false - - # For projects that have no package protection rules - ref(:project_without_ppr) | Gitlab::Access::DEVELOPER | '@my-scope/my-package-prod' | :npm | false - ref(:project_without_ppr) | Gitlab::Access::MAINTAINER | '@my-scope/my-package-prod' | :npm | false - ref(:project_without_ppr) | Gitlab::Access::OWNER | '@my-scope/my-package-prod' | :npm | false - end + # rubocop:disable Layout/LineLength -- Avoid formatting to ensure one-line table syntax + where(:project, :action, :access_level, :package_name, :package_type, :protected?) do + ref(:project_with_ppr) | :push | Gitlab::Access::REPORTER | '@my-scope/my-package-stage-sha-1234' | :npm | true + ref(:project_with_ppr) | :push | Gitlab::Access::DEVELOPER | '@my-scope/my-package-stage-sha-1234' | :npm | true + ref(:project_with_ppr) | :push | Gitlab::Access::MAINTAINER | '@my-scope/my-package-stage-sha-1234' | :npm | false + ref(:project_with_ppr) | :push | Gitlab::Access::OWNER | '@my-scope/my-package-stage-sha-1234' | :npm | false + ref(:project_with_ppr) | :push | Gitlab::Access::ADMIN | '@my-scope/my-package-stage-sha-1234' | :npm | false + ref(:project_with_ppr) | :delete | Gitlab::Access::REPORTER | '@my-scope/my-package-stage-sha-1234' | :npm | true + ref(:project_with_ppr) | :delete | Gitlab::Access::DEVELOPER | '@my-scope/my-package-stage-sha-1234' | :npm | true + ref(:project_with_ppr) | :delete | Gitlab::Access::MAINTAINER | '@my-scope/my-package-stage-sha-1234' | :npm | true + ref(:project_with_ppr) | :delete | Gitlab::Access::OWNER | '@my-scope/my-package-stage-sha-1234' | :npm | false + ref(:project_with_ppr) | :delete | Gitlab::Access::ADMIN | '@my-scope/my-package-stage-sha-1234' | :npm | false + + ref(:project_with_ppr) | :push | Gitlab::Access::MAINTAINER | '@my-scope/my-package-prod-sha-1234' | :npm | true + ref(:project_with_ppr) | :push | Gitlab::Access::OWNER | '@my-scope/my-package-prod-sha-1234' | :npm | false + ref(:project_with_ppr) | :push | Gitlab::Access::ADMIN | '@my-scope/my-package-prod-sha-1234' | :npm | false + ref(:project_with_ppr) | :delete | Gitlab::Access::MAINTAINER | '@my-scope/my-package-prod-sha-1234' | :npm | true + ref(:project_with_ppr) | :delete | Gitlab::Access::OWNER | '@my-scope/my-package-prod-sha-1234' | :npm | false + ref(:project_with_ppr) | :delete | Gitlab::Access::ADMIN | '@my-scope/my-package-prod-sha-1234' | :npm | false + + ref(:project_with_ppr) | :push | Gitlab::Access::MAINTAINER | '@my-scope/my-package-release-v1' | :npm | true + ref(:project_with_ppr) | :push | Gitlab::Access::OWNER | '@my-scope/my-package-release-v1' | :npm | true + ref(:project_with_ppr) | :push | Gitlab::Access::ADMIN | '@my-scope/my-package-release-v1' | :npm | false + ref(:project_with_ppr) | :delete | Gitlab::Access::MAINTAINER | '@my-scope/my-package-release-v1' | :npm | true + ref(:project_with_ppr) | :delete | Gitlab::Access::OWNER | '@my-scope/my-package-release-v1' | :npm | true + ref(:project_with_ppr) | :delete | Gitlab::Access::ADMIN | '@my-scope/my-package-release-v1' | :npm | false + + ref(:project_with_ppr) | :push | Gitlab::Access::DEVELOPER | '@my-scope/my-package-any-suffix' | :npm | true + ref(:project_with_ppr) | :push | Gitlab::Access::MAINTAINER | '@my-scope/my-package-any-suffix' | :npm | false + ref(:project_with_ppr) | :push | Gitlab::Access::OWNER | '@my-scope/my-package-any-suffix' | :npm | false + ref(:project_with_ppr) | :push | Gitlab::Access::ADMIN | '@my-scope/my-package-any-suffix' | :npm | false + ref(:project_with_ppr) | :delete | Gitlab::Access::DEVELOPER | '@my-scope/my-package-any-suffix' | :npm | true + ref(:project_with_ppr) | :delete | Gitlab::Access::MAINTAINER | '@my-scope/my-package-any-suffix' | :npm | true + ref(:project_with_ppr) | :delete | Gitlab::Access::OWNER | '@my-scope/my-package-any-suffix' | :npm | false + ref(:project_with_ppr) | :delete | Gitlab::Access::ADMIN | '@my-scope/my-package-any-suffix' | :npm | false + + ref(:project_with_ppr) | :push | Gitlab::Access::DEVELOPER | '@my-scope/only_delete-protected-package' | :npm | false + ref(:project_with_ppr) | :push | Gitlab::Access::MAINTAINER | '@my-scope/only_delete-protected-package' | :npm | false + ref(:project_with_ppr) | :push | Gitlab::Access::OWNER | '@my-scope/only_delete-protected-package' | :npm | false + ref(:project_with_ppr) | :push | Gitlab::Access::ADMIN | '@my-scope/only_delete-protected-package' | :npm | false + ref(:project_with_ppr) | :push | Gitlab::Access::DEVELOPER | '@my-scope/only-push-protected-package' | :npm | true + ref(:project_with_ppr) | :push | Gitlab::Access::MAINTAINER | '@my-scope/only-push-protected-package' | :npm | true + ref(:project_with_ppr) | :push | Gitlab::Access::OWNER | '@my-scope/only-push-protected-package' | :npm | true + ref(:project_with_ppr) | :push | Gitlab::Access::ADMIN | '@my-scope/only-push-protected-package' | :npm | false + ref(:project_with_ppr) | :delete | Gitlab::Access::DEVELOPER | '@my-scope/only_delete-protected-package' | :npm | true + ref(:project_with_ppr) | :delete | Gitlab::Access::MAINTAINER | '@my-scope/only_delete-protected-package' | :npm | true + ref(:project_with_ppr) | :delete | Gitlab::Access::OWNER | '@my-scope/only_delete-protected-package' | :npm | true + ref(:project_with_ppr) | :delete | Gitlab::Access::ADMIN | '@my-scope/only_delete-protected-package' | :npm | false + ref(:project_with_ppr) | :delete | Gitlab::Access::DEVELOPER | '@my-scope/only-push-protected-package' | :npm | false + ref(:project_with_ppr) | :delete | Gitlab::Access::MAINTAINER | '@my-scope/only-push-protected-package' | :npm | false + ref(:project_with_ppr) | :delete | Gitlab::Access::OWNER | '@my-scope/only-push-protected-package' | :npm | false + ref(:project_with_ppr) | :delete | Gitlab::Access::ADMIN | '@my-scope/only-push-protected-package' | :npm | false + + # For non-matching packages + ref(:project_with_ppr) | :push | Gitlab::Access::DEVELOPER | '@my-scope/non-matching-package' | :npm | false + ref(:project_with_ppr) | :delete | Gitlab::Access::DEVELOPER | '@my-scope/non-matching-package' | :npm | false + ref(:project_with_ppr) | :push | Gitlab::Access::DEVELOPER | '@my-scope/my-package-any-suffix' | :conan | false + ref(:project_with_ppr) | :delete | Gitlab::Access::DEVELOPER | '@my-scope/my-package-any-suffix' | :conan | false + ref(:project_with_ppr) | :push | Gitlab::Access::NO_ACCESS | '@my-scope/my-package-prod' | :npm | true + ref(:project_with_ppr) | :delete | Gitlab::Access::NO_ACCESS | '@my-scope/my-package-prod' | :npm | true + + # Edge cases + ref(:project_with_ppr) | :push | nil | '@my-scope/my-package-stage-sha-1234' | :npm | false + ref(:project_with_ppr) | :push | Gitlab::Access::DEVELOPER | nil | :npm | false + ref(:project_with_ppr) | :push | Gitlab::Access::DEVELOPER | '' | :npm | false + ref(:project_with_ppr) | :push | Gitlab::Access::DEVELOPER | '@my-scope/my-package-stage-sha-1234' | nil | false + ref(:project_with_ppr) | :push | nil | nil | nil | false + + # For projects that have no package protection rules + ref(:project_without_ppr) | :push | Gitlab::Access::DEVELOPER | '@my-scope/my-package-prod' | :npm | false + ref(:project_without_ppr) | :push | Gitlab::Access::OWNER | '@my-scope/my-package-prod' | :npm | false + ref(:project_without_ppr) | :delete | Gitlab::Access::DEVELOPER | '@my-scope/my-package-prod' | :npm | false + ref(:project_without_ppr) | :delete | Gitlab::Access::OWNER | '@my-scope/my-package-prod' | :npm | false + end + # rubocop:enable Layout/LineLength - with_them do - it { is_expected.to eq push_protected } - end + with_them do + it { is_expected.to eq protected? } end end diff --git a/spec/services/packages/protection/update_rule_service_spec.rb b/spec/services/packages/protection/update_rule_service_spec.rb index 3d4b913e0ce2e750566030200f1817482c68a8a0..2d8f98aa6cda11451e546fa532d78eb72038b168 100644 --- a/spec/services/packages/protection/update_rule_service_spec.rb +++ b/spec/services/packages/protection/update_rule_service_spec.rb @@ -14,7 +14,8 @@ :package_protection_rule, package_name_pattern: "#{package_protection_rule.package_name_pattern}-updated", package_type: 'npm', - minimum_access_level_for_push: 'owner' + minimum_access_level_for_push: 'owner', + minimum_access_level_for_delete: 'owner' ) end