diff --git a/app/graphql/mutations/packages/protection/rule/create.rb b/app/graphql/mutations/packages/protection/rule/create.rb index 2e5fe790b4826b9f68b3224faf1b789818f030c1..2105569f2970f1dd0bba9d90d0ac930b90dac821 100644 --- a/app/graphql/mutations/packages/protection/rule/create.rb +++ b/app/graphql/mutations/packages/protection/rule/create.rb @@ -27,9 +27,15 @@ class Create < ::Mutations::BaseMutation required: true, description: copy_field_description(Types::Packages::Protection::RuleType, :package_type) + argument :minimum_access_level_for_delete, + Types::Packages::Protection::RuleAccessLevelForDeleteEnum, + required: false, + experiment: { milestone: '17.10' }, + description: copy_field_description(Types::Packages::Protection::RuleType, :minimum_access_level_for_delete) + argument :minimum_access_level_for_push, Types::Packages::Protection::RuleAccessLevelEnum, - required: true, + required: false, description: copy_field_description(Types::Packages::Protection::RuleType, :minimum_access_level_for_push) field :package_protection_rule, @@ -40,8 +46,13 @@ class Create < ::Mutations::BaseMutation def resolve(project_path:, **kwargs) project = authorized_find!(project_path) - response = ::Packages::Protection::CreateRuleService.new(project: project, current_user: current_user, - params: kwargs).execute + kwargs.except!(:minimum_access_level_for_delete) if Feature.disabled?(:packages_protected_packages_delete, + project) + + response = + ::Packages::Protection::CreateRuleService + .new(project: project, current_user: current_user, params: kwargs) + .execute { package_protection_rule: response.payload[:package_protection_rule], errors: response.errors } end diff --git a/app/graphql/mutations/packages/protection/rule/update.rb b/app/graphql/mutations/packages/protection/rule/update.rb index 5255f0b24304cfc7e3b58e20ac9f03d9b2c9d867..1deda1d5ca063d56c90cd7daec1b8132af583a28 100644 --- a/app/graphql/mutations/packages/protection/rule/update.rb +++ b/app/graphql/mutations/packages/protection/rule/update.rb @@ -28,10 +28,15 @@ class Update < ::Mutations::BaseMutation validates: { allow_blank: false }, description: copy_field_description(Types::Packages::Protection::RuleType, :package_type) + argument :minimum_access_level_for_delete, + Types::Packages::Protection::RuleAccessLevelForDeleteEnum, + required: false, + experiment: { milestone: '17.10' }, + description: copy_field_description(Types::Packages::Protection::RuleType, :minimum_access_level_for_delete) + argument :minimum_access_level_for_push, Types::Packages::Protection::RuleAccessLevelEnum, required: false, - validates: { allow_blank: false }, description: copy_field_description(Types::Packages::Protection::RuleType, :minimum_access_level_for_push) field :package_protection_rule, @@ -42,6 +47,9 @@ class Update < ::Mutations::BaseMutation def resolve(id:, **kwargs) package_protection_rule = authorized_find!(id: id) + kwargs.except!(:minimum_access_level_for_delete) if Feature.disabled?(:packages_protected_packages_delete, + package_protection_rule.project) + response = ::Packages::Protection::UpdateRuleService.new(package_protection_rule, current_user: current_user, params: kwargs).execute diff --git a/app/graphql/types/packages/protection/rule_access_level_for_delete_enum.rb b/app/graphql/types/packages/protection/rule_access_level_for_delete_enum.rb new file mode 100644 index 0000000000000000000000000000000000000000..646d0bef56d64542f879a242d83ceb24fff1d265 --- /dev/null +++ b/app/graphql/types/packages/protection/rule_access_level_for_delete_enum.rb @@ -0,0 +1,19 @@ +# frozen_string_literal: true + +module Types + module Packages + module Protection + class RuleAccessLevelForDeleteEnum < BaseEnum + graphql_name 'PackagesProtectionRuleAccessLevelForDelete' + description 'Access level for the deletion of a package protection rule resource.' + + ::Packages::Protection::Rule.minimum_access_level_for_deletes.each_key do |access_level_key| + value access_level_key.upcase, + value: access_level_key.to_s, + description: "#{access_level_key.capitalize} access. " \ + 'Available only when feature flag `packages_protected_packages_delete` is enabled.' + end + end + end + end +end diff --git a/app/graphql/types/packages/protection/rule_type.rb b/app/graphql/types/packages/protection/rule_type.rb index fd369f25be45a5d1987057fba8766a1c57fac694..62bfcd46d56787bc0fb3e19bbce08f7461c5e03a 100644 --- a/app/graphql/types/packages/protection/rule_type.rb +++ b/app/graphql/types/packages/protection/rule_type.rb @@ -27,13 +27,29 @@ class RuleType < ::Types::BaseObject null: false, description: 'Package type protected by the protection rule. For example, `NPM`, `PYPI`.' + field :minimum_access_level_for_delete, + Types::Packages::Protection::RuleAccessLevelForDeleteEnum, + null: true, + experiment: { milestone: '17.10' }, + description: + 'Minimum GitLab access required to delete packages from the package registry. ' \ + 'Valid values include `OWNER` or `ADMIN`. ' \ + 'If the value is `nil`, the default minimum access level is `MAINTAINER`. ' \ + 'Available only when feature flag `packages_protected_packages_delete` is enabled.' + field :minimum_access_level_for_push, Types::Packages::Protection::RuleAccessLevelEnum, - null: false, + null: true, description: 'Minimum GitLab access required to push packages to the package registry. ' \ 'Valid values include `MAINTAINER`, `OWNER`, or `ADMIN`. ' \ 'If the value is `nil`, the default minimum access level is `DEVELOPER`.' + + def minimum_access_level_for_delete + return unless Feature.enabled?(:packages_protected_packages_delete, object&.project) + + object.minimum_access_level_for_delete + end end end end diff --git a/app/services/packages/protection/create_rule_service.rb b/app/services/packages/protection/create_rule_service.rb index 22149b2be1c0fad10281bf4777e8ac5d2222c856..c9958db8c13324edf017c7e53366465f50557a70 100644 --- a/app/services/packages/protection/create_rule_service.rb +++ b/app/services/packages/protection/create_rule_service.rb @@ -6,6 +6,7 @@ class CreateRuleService < BaseProjectService ALLOWED_ATTRIBUTES = %i[ package_name_pattern package_type + minimum_access_level_for_delete minimum_access_level_for_push ].freeze diff --git a/app/services/packages/protection/delete_rule_service.rb b/app/services/packages/protection/delete_rule_service.rb index a1fa111b57bee180057da98f38b21ba0215a8aad..c25b6ae106a54c40af3fd4db9178eaba1a92402d 100644 --- a/app/services/packages/protection/delete_rule_service.rb +++ b/app/services/packages/protection/delete_rule_service.rb @@ -7,8 +7,7 @@ class DeleteRuleService def initialize(package_protection_rule, current_user:) if package_protection_rule.blank? || current_user.blank? - raise ArgumentError, - 'package_protection_rule and current_user must be set' + raise ArgumentError, 'package_protection_rule and current_user must be set' end @package_protection_rule = package_protection_rule diff --git a/app/services/packages/protection/update_rule_service.rb b/app/services/packages/protection/update_rule_service.rb index 9bfa91dc902b7356426fecfaecca42468021380d..b39dbfac39678d9d28d86c04214bbd9d83416a7d 100644 --- a/app/services/packages/protection/update_rule_service.rb +++ b/app/services/packages/protection/update_rule_service.rb @@ -8,6 +8,7 @@ class UpdateRuleService ALLOWED_ATTRIBUTES = %i[ package_name_pattern package_type + minimum_access_level_for_delete minimum_access_level_for_push ].freeze diff --git a/doc/api/graphql/reference/_index.md b/doc/api/graphql/reference/_index.md index 0d23c0d04783f793ed67f15e1e0f349421f73e57..45ec7b27141c552430b6c1d3097d0d3b52184bd9 100644 --- a/doc/api/graphql/reference/_index.md +++ b/doc/api/graphql/reference/_index.md @@ -4367,7 +4367,8 @@ Input type: `CreatePackagesProtectionRuleInput` | Name | Type | Description | | ---- | ---- | ----------- | | `clientMutationId` | [`String`](#string) | A unique identifier for the client performing the mutation. | -| `minimumAccessLevelForPush` | [`PackagesProtectionRuleAccessLevel!`](#packagesprotectionruleaccesslevel) | Minimum GitLab access required to push packages to the package registry. Valid values include `MAINTAINER`, `OWNER`, or `ADMIN`. If the value is `nil`, the default minimum access level is `DEVELOPER`. | +| `minimumAccessLevelForDelete` {{< icon name="warning-solid" >}} | [`PackagesProtectionRuleAccessLevelForDelete`](#packagesprotectionruleaccesslevelfordelete) | **Deprecated:** **Status**: Experiment. Introduced in GitLab 17.10. | +| `minimumAccessLevelForPush` | [`PackagesProtectionRuleAccessLevel`](#packagesprotectionruleaccesslevel) | Minimum GitLab access required to push packages to the package registry. Valid values include `MAINTAINER`, `OWNER`, or `ADMIN`. If the value is `nil`, the default minimum access level is `DEVELOPER`. | | `packageNamePattern` | [`String!`](#string) | Package name protected by the protection rule. For example, `@my-scope/my-package-*`. Wildcard character `*` allowed. | | `packageType` | [`PackagesProtectionRulePackageType!`](#packagesprotectionrulepackagetype) | Package type protected by the protection rule. For example, `NPM`, `PYPI`. | | `projectPath` | [`ID!`](#id) | Full path of the project where a protection rule is located. | @@ -11514,6 +11515,7 @@ Input type: `UpdatePackagesProtectionRuleInput` | ---- | ---- | ----------- | | `clientMutationId` | [`String`](#string) | A unique identifier for the client performing the mutation. | | `id` | [`PackagesProtectionRuleID!`](#packagesprotectionruleid) | Global ID of the package protection rule to be updated. | +| `minimumAccessLevelForDelete` {{< icon name="warning-solid" >}} | [`PackagesProtectionRuleAccessLevelForDelete`](#packagesprotectionruleaccesslevelfordelete) | **Deprecated:** **Status**: Experiment. Introduced in GitLab 17.10. | | `minimumAccessLevelForPush` | [`PackagesProtectionRuleAccessLevel`](#packagesprotectionruleaccesslevel) | Minimum GitLab access required to push packages to the package registry. Valid values include `MAINTAINER`, `OWNER`, or `ADMIN`. If the value is `nil`, the default minimum access level is `DEVELOPER`. | | `packageNamePattern` | [`String`](#string) | Package name protected by the protection rule. For example, `@my-scope/my-package-*`. Wildcard character `*` allowed. | | `packageType` | [`PackagesProtectionRulePackageType`](#packagesprotectionrulepackagetype) | Package type protected by the protection rule. For example, `NPM`, `PYPI`. | @@ -33076,7 +33078,8 @@ A packages protection rule designed to protect packages from being pushed by use | Name | Type | Description | | ---- | ---- | ----------- | | `id` | [`PackagesProtectionRuleID!`](#packagesprotectionruleid) | Global ID of the package protection rule. | -| `minimumAccessLevelForPush` | [`PackagesProtectionRuleAccessLevel!`](#packagesprotectionruleaccesslevel) | Minimum GitLab access required to push packages to the package registry. Valid values include `MAINTAINER`, `OWNER`, or `ADMIN`. If the value is `nil`, the default minimum access level is `DEVELOPER`. | +| `minimumAccessLevelForDelete` {{< icon name="warning-solid" >}} | [`PackagesProtectionRuleAccessLevelForDelete`](#packagesprotectionruleaccesslevelfordelete) | **Introduced** in GitLab 17.10. **Status**: Experiment. Minimum GitLab access required to delete packages from the package registry. Valid values include `OWNER` or `ADMIN`. If the value is `nil`, the default minimum access level is `MAINTAINER`. Available only when feature flag `packages_protected_packages_delete` is enabled. | +| `minimumAccessLevelForPush` | [`PackagesProtectionRuleAccessLevel`](#packagesprotectionruleaccesslevel) | Minimum GitLab access required to push packages to the package registry. Valid values include `MAINTAINER`, `OWNER`, or `ADMIN`. If the value is `nil`, the default minimum access level is `DEVELOPER`. | | `packageNamePattern` | [`String!`](#string) | Package name protected by the protection rule. For example, `@my-scope/my-package-*`. Wildcard character `*` allowed. | | `packageType` | [`PackagesProtectionRulePackageType!`](#packagesprotectionrulepackagetype) | Package type protected by the protection rule. For example, `NPM`, `PYPI`. | @@ -43357,6 +43360,15 @@ Access level of a package protection rule resource. | `MAINTAINER` | Maintainer access. | | `OWNER` | Owner access. | +### `PackagesProtectionRuleAccessLevelForDelete` + +Access level for the deletion of a package protection rule resource. + +| Value | Description | +| ----- | ----------- | +| `ADMIN` | Admin access. Available only when feature flag `packages_protected_packages_delete` is enabled. | +| `OWNER` | Owner access. Available only when feature flag `packages_protected_packages_delete` is enabled. | + ### `PackagesProtectionRulePackageType` Package type of a package protection rule resource. diff --git a/spec/graphql/types/packages/protection/rule_access_level_for_delete_enum_spec.rb b/spec/graphql/types/packages/protection/rule_access_level_for_delete_enum_spec.rb new file mode 100644 index 0000000000000000000000000000000000000000..c716715a4363381269f05f18c2e7d6a54d00155d --- /dev/null +++ b/spec/graphql/types/packages/protection/rule_access_level_for_delete_enum_spec.rb @@ -0,0 +1,9 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe GitlabSchema.types['PackagesProtectionRuleAccessLevelForDelete'], feature_category: :package_registry do + it 'exposes all options' do + expect(described_class.values.keys).to match_array(%w[OWNER ADMIN]) + end +end diff --git a/spec/graphql/types/packages/protection/rule_type_spec.rb b/spec/graphql/types/packages/protection/rule_type_spec.rb index 479fc324265d53079288a8d3bcc17649b0c36ee1..39250282b48fd92f009b051aa944c5ad9d087d82 100644 --- a/spec/graphql/types/packages/protection/rule_type_spec.rb +++ b/spec/graphql/types/packages/protection/rule_type_spec.rb @@ -3,6 +3,8 @@ require 'spec_helper' RSpec.describe GitlabSchema.types['PackagesProtectionRule'], feature_category: :package_registry do + include GraphqlHelpers + specify { expect(described_class.graphql_name).to eq('PackagesProtectionRule') } specify { expect(described_class.description).to be_present } @@ -27,9 +29,35 @@ it { is_expected.to have_non_null_graphql_type(Types::Packages::Protection::RulePackageTypeEnum) } end + describe 'minimum_access_level_for_delete' do + subject { described_class.fields['minimumAccessLevelForDelete'] } + + it { is_expected.to have_nullable_graphql_type(Types::Packages::Protection::RuleAccessLevelForDeleteEnum) } + + describe 'resolve field' do + let_it_be(:package_protection_rule) { create(:package_protection_rule) } + let(:user) { package_protection_rule.project.owner } + + subject do + resolve_field(:minimum_access_level_for_delete, package_protection_rule, current_user: user, + object_type: described_class) + end + + it { is_expected.to eq 'owner' } + + context 'when the feature flag `packages_protected_packages_delete` is disabled' do + before do + stub_feature_flags(packages_protected_packages_delete: false) + end + + it { is_expected.to be_nil } + end + end + end + describe 'minimum_access_level_for_push' do subject { described_class.fields['minimumAccessLevelForPush'] } - it { is_expected.to have_non_null_graphql_type(Types::Packages::Protection::RuleAccessLevelEnum) } + it { is_expected.to have_nullable_graphql_type(Types::Packages::Protection::RuleAccessLevelEnum) } end end diff --git a/spec/requests/api/graphql/mutations/packages/protection/rule/create_spec.rb b/spec/requests/api/graphql/mutations/packages/protection/rule/create_spec.rb index ffa4ba6197e358b18e2438d3cdf880e5cc421991..baa3ee81756016605ca02724c746db57d2850ba5 100644 --- a/spec/requests/api/graphql/mutations/packages/protection/rule/create_spec.rb +++ b/spec/requests/api/graphql/mutations/packages/protection/rule/create_spec.rb @@ -15,6 +15,7 @@ project_path: project.full_path, package_name_pattern: package_protection_rule_attributes.package_name_pattern, package_type: 'NPM', + minimum_access_level_for_delete: 'OWNER', minimum_access_level_for_push: 'MAINTAINER' } end @@ -26,6 +27,7 @@ id packageNamePattern packageType + minimumAccessLevelForDelete minimumAccessLevelForPush } errors @@ -54,9 +56,10 @@ expect(mutation_response_package_protection_rule).to include( 'id' => be_present, - 'packageNamePattern' => kwargs[:package_name_pattern], - 'packageType' => kwargs[:package_type], - 'minimumAccessLevelForPush' => kwargs[:minimum_access_level_for_push] + 'packageNamePattern' => expected_attributes[:package_name_pattern], + 'packageType' => expected_attributes[:package_type].upcase, + 'minimumAccessLevelForDelete' => expected_attributes[:minimum_access_level_for_delete]&.upcase, + 'minimumAccessLevelForPush' => expected_attributes[:minimum_access_level_for_push]&.upcase ) end @@ -65,9 +68,10 @@ expect(Packages::Protection::Rule.last).to have_attributes( project: project, - package_name_pattern: kwargs[:package_name_pattern], - package_type: kwargs[:package_type].downcase, - minimum_access_level_for_push: kwargs[:minimum_access_level_for_push].downcase + package_name_pattern: expected_attributes[:package_name_pattern], + package_type: expected_attributes[:package_type]&.downcase, + minimum_access_level_for_delete: expected_attributes[:minimum_access_level_for_delete]&.downcase, + minimum_access_level_for_push: expected_attributes[:minimum_access_level_for_push]&.downcase ) end end @@ -78,7 +82,31 @@ end end - it_behaves_like 'a successful response' + it_behaves_like 'a successful response' do + let(:expected_attributes) { kwargs } + end + + context 'when feature flag `packages_protected_packages_delete` is disabled' do + before do + stub_feature_flags(packages_protected_packages_delete: false) + end + + it_behaves_like 'a successful response' do + let(:expected_attributes) { kwargs.merge(minimum_access_level_for_delete: nil) } + end + + context 'when minimum_access_level_for_push is nil' do + let(:kwargs) { super().merge(minimum_access_level_for_push: nil) } + + it_behaves_like 'an erroneous response' + + it 'returns an error' do + subject + + expect(mutation_response_errors).to include(/must have at least a minimum access role for push or delete./) + end + end + end context 'with invalid kwargs leading to error from graphql' do let(:kwargs) do @@ -109,6 +137,35 @@ end end + context 'with invalid input fields `minimumAccessLevelForPush` and `minimumAccessLevelForDelete`' do + let(:kwargs) do + super().merge!( + minimum_access_level_for_push: 'INVALID_ACCESS_LEVEL', + minimum_access_level_for_delete: 'INVALID_ACCESS_LEVEL' + ) + end + + it_behaves_like 'an erroneous response' + + it 'returns an error' do + subject + + expect_graphql_errors_to_include([/minimumAccessLevelForPush/, /minimumAccessLevelForDelete/]) + end + end + + context 'with blank input fields `minimumAccessLevelForPush` and `minimumAccessLevelForDelete`' do + let(:kwargs) { super().merge(minimum_access_level_for_push: nil, minimum_access_level_for_delete: nil) } + + it_behaves_like 'an erroneous response' + + it 'returns an error' do + subject + + expect(mutation_response_errors).to include('A rule must have at least a minimum access role for push or delete.') + end + end + context 'with existing packages protection rule' do let_it_be(:existing_package_protection_rule) do create(:package_protection_rule, project: project, minimum_access_level_for_push: :maintainer) @@ -128,14 +185,15 @@ super().merge!(package_name_pattern: "#{existing_package_protection_rule.package_name_pattern}-unique") end - it_behaves_like 'a successful response' + it_behaves_like 'a successful response' do + let(:expected_attributes) { kwargs } + end end context 'when field `minimum_access_level_for_push` is different than existing one' do let(:kwargs) { super().merge!(minimum_access_level_for_push: 'MAINTAINER') } it_behaves_like 'an erroneous response' - it 'returns an error' do subject.tap { expect(mutation_response_errors).to include(/Package name pattern has already been taken/) } end diff --git a/spec/requests/api/graphql/mutations/packages/protection/rule/delete_spec.rb b/spec/requests/api/graphql/mutations/packages/protection/rule/delete_spec.rb index f4acdc6dbabe35765b7f3988a4dfb6be99a87ff6..bef8fe0c697419e565bf77f3d72a8ad6a2ab15f8 100644 --- a/spec/requests/api/graphql/mutations/packages/protection/rule/delete_spec.rb +++ b/spec/requests/api/graphql/mutations/packages/protection/rule/delete_spec.rb @@ -31,6 +31,7 @@ 'id' => package_protection_rule.to_global_id.to_s, 'packageNamePattern' => package_protection_rule.package_name_pattern, 'packageType' => package_protection_rule.package_type.upcase, + 'minimumAccessLevelForDelete' => package_protection_rule.minimum_access_level_for_delete.upcase, 'minimumAccessLevelForPush' => package_protection_rule.minimum_access_level_for_push.upcase } ) diff --git a/spec/requests/api/graphql/mutations/packages/protection/rule/update_spec.rb b/spec/requests/api/graphql/mutations/packages/protection/rule/update_spec.rb index 9b0e88c2cb7c4c270de37a130515b3e110e27000..5d263a7c31a854b471bd2fda405e72d204b488a9 100644 --- a/spec/requests/api/graphql/mutations/packages/protection/rule/update_spec.rb +++ b/spec/requests/api/graphql/mutations/packages/protection/rule/update_spec.rb @@ -19,6 +19,7 @@ <<~QUERY packageProtectionRule { packageNamePattern + minimumAccessLevelForDelete minimumAccessLevelForPush } clientMutationId @@ -31,11 +32,13 @@ { id: package_protection_rule.to_global_id, package_name_pattern: "#{package_protection_rule.package_name_pattern}-updated", + minimum_access_level_for_delete: 'ADMIN', minimum_access_level_for_push: 'MAINTAINER' } end let(:mutation_response) { graphql_mutation_response(:update_packages_protection_rule) } + let(:mutation_response_errors) { mutation_response['errors'] } subject { post_graphql_mutation(mutation, current_user: current_user) } @@ -47,19 +50,21 @@ expect(mutation_response).to include( 'packageProtectionRule' => { - 'packageNamePattern' => input[:package_name_pattern], - 'minimumAccessLevelForPush' => input[:minimum_access_level_for_push] + 'packageNamePattern' => expected_attributes[:package_name_pattern], + 'minimumAccessLevelForDelete' => expected_attributes[:minimum_access_level_for_delete]&.upcase, + 'minimumAccessLevelForPush' => expected_attributes[:minimum_access_level_for_push]&.upcase } ) end - it do - subject.tap do - expect(package_protection_rule.reload).to have_attributes( - package_name_pattern: input[:package_name_pattern], - minimum_access_level_for_push: input[:minimum_access_level_for_push].downcase - ) - end + it 'updates attributes of existing package protection rule' do + expect { subject }.not_to change { ::Packages::Protection::Rule.count } + + expect(package_protection_rule.reload).to have_attributes( + package_name_pattern: expected_attributes[:package_name_pattern], + minimum_access_level_for_delete: expected_attributes[:minimum_access_level_for_delete]&.downcase, + minimum_access_level_for_push: expected_attributes[:minimum_access_level_for_push]&.downcase + ) end end @@ -68,7 +73,33 @@ it { expect { subject }.not_to change { package_protection_rule.reload.updated_at } } end - it_behaves_like 'a successful response' + it_behaves_like 'a successful response' do + let(:expected_attributes) { input } + end + + context 'when feature flag `packages_protected_packages_delete` is disabled' do + before do + package_protection_rule.update!(minimum_access_level_for_delete: nil) + + stub_feature_flags(packages_protected_packages_delete: false) + end + + it_behaves_like 'a successful response' do + let(:expected_attributes) do + input.merge(minimum_access_level_for_delete: package_protection_rule.minimum_access_level_for_delete) + end + end + + context 'when minimum_access_level_for_push is nil' do + let(:input) { super().merge(minimum_access_level_for_push: nil) } + + it 'includes error message in response' do + is_expected.tap do + expect(mutation_response_errors).to include(/at least a minimum access role for push or delete/) + end + end + end + end context 'with other existing package protection rule with same package_name_pattern' do let_it_be_with_reload(:other_existing_package_protection_rule) do @@ -85,16 +116,20 @@ end it 'includes error message in response' do - is_expected.tap { expect(mutation_response['errors']).to eq ['Package name pattern has already been taken'] } + is_expected.tap { expect(mutation_response_errors).to eq ['Package name pattern has already been taken'] } end end context 'with invalid input param `minimumAccessLevelForPush`' do - let(:input) { super().merge(minimum_access_level_for_push: nil) } + let(:input) { super().merge(minimum_access_level_for_push: 'INVALID_ACCESS_LEVEL') } - it_behaves_like 'an erroneous response' + it { is_expected.tap { expect_graphql_errors_to_include(/invalid value for minimumAccessLevelForPush/) } } + end + + context 'with invalid input param `minimumAccessLevelForDelete`' do + let(:input) { super().merge(minimum_access_level_for_delete: 'INVALID_ACCESS_LEVEL') } - it { is_expected.tap { expect_graphql_errors_to_include(/minimumAccessLevelForPush can't be blank/) } } + it { is_expected.tap { expect_graphql_errors_to_include(/invalid value for minimumAccessLevelForDelete/) } } end context 'with invalid input param `packageNamePattern`' do @@ -105,6 +140,16 @@ it { is_expected.tap { expect_graphql_errors_to_include(/packageNamePattern can't be blank/) } } end + context 'with blank input fields `minimumAccessLevelForPush` and `minimumAccessLevelForDelete`' do + let(:input) { super().merge(minimum_access_level_for_push: nil, minimum_access_level_for_delete: nil) } + + it 'includes error message in response' do + is_expected.tap do + expect(mutation_response_errors).to include(/at least a minimum access role for push or delete/) + end + end + end + context 'when current_user does not have permission' do let_it_be(:developer) { create(:user, developer_of: project) } let_it_be(:reporter) { create(:user, reporter_of: project) } diff --git a/spec/requests/api/graphql/project/packages_protection_rules_spec.rb b/spec/requests/api/graphql/project/packages_protection_rules_spec.rb index 5b60a1458c3777c3278f0aa287896f0f463c2b20..ac716f5e3e128df2302e8ceb2bce5ca9f4d2ca69 100644 --- a/spec/requests/api/graphql/project/packages_protection_rules_spec.rb +++ b/spec/requests/api/graphql/project/packages_protection_rules_spec.rb @@ -37,6 +37,7 @@ hash_including( 'packageNamePattern' => package_protection_rule.package_name_pattern, 'packageType' => 'NPM', + 'minimumAccessLevelForDelete' => 'OWNER', 'minimumAccessLevelForPush' => 'MAINTAINER' ) ) diff --git a/spec/services/packages/protection/create_rule_service_spec.rb b/spec/services/packages/protection/create_rule_service_spec.rb index 1e8d8985cf5e1849d72022cbe43b2e63f85aab9a..52f8a096643f41d5183147302038751b4c80aa71 100644 --- a/spec/services/packages/protection/create_rule_service_spec.rb +++ b/spec/services/packages/protection/create_rule_service_spec.rb @@ -61,6 +61,13 @@ it_behaves_like 'an erroneous service response with side effect', message: "'unknown_package_type' is not a valid package_type" end + + context 'when minimum_access_level_for_delete and minimum_access_level_for_push are blank' do + let(:params) { super().merge(minimum_access_level_for_delete: nil, minimum_access_level_for_push: nil) } + + it_behaves_like 'an erroneous service response with side effect', + message: ['A rule must have at least a minimum access role for push or delete.'] + end end context 'with existing PackageProtectionRule' do diff --git a/spec/services/packages/protection/update_rule_service_spec.rb b/spec/services/packages/protection/update_rule_service_spec.rb index 2d8f98aa6cda11451e546fa532d78eb72038b168..3f38c0ee6c96badae5880a28fd93de47015e0c4e 100644 --- a/spec/services/packages/protection/update_rule_service_spec.rb +++ b/spec/services/packages/protection/update_rule_service_spec.rb @@ -14,8 +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_delete: 'owner' + minimum_access_level_for_delete: 'admin', + minimum_access_level_for_push: 'owner' ) end @@ -72,6 +72,13 @@ message: "'unknown_package_type' is not a valid package_type" end + context 'when minimum_access_level_for_delete and minimum_access_level_for_push are blank' do + let(:params) { super().merge(minimum_access_level_for_delete: nil, minimum_access_level_for_push: nil) } + + it_behaves_like 'an erroneous service response with side effect', + message: ['A rule must have at least a minimum access role for push or delete.'] + end + context 'with empty params' do let(:params) { {} }