diff options
18 files changed, 494 insertions, 45 deletions
diff --git a/app/graphql/mutations/packages/protection/rule/update.rb b/app/graphql/mutations/packages/protection/rule/update.rb new file mode 100644 index 00000000000..dc1f78e6822 --- /dev/null +++ b/app/graphql/mutations/packages/protection/rule/update.rb @@ -0,0 +1,62 @@ +# frozen_string_literal: true + +module Mutations + module Packages + module Protection + module Rule + class Update < ::Mutations::BaseMutation + graphql_name 'UpdatePackagesProtectionRule' + description 'Updates a package protection rule to restrict access to project packages. ' \ + 'You can prevent users without certain permissions from altering packages. ' \ + 'Available only when feature flag `packages_protected_packages` is enabled.' + + authorize :admin_package + + argument :id, + ::Types::GlobalIDType[::Packages::Protection::Rule], + required: true, + description: 'Global ID of the package protection rule to be updated.' + + argument :package_name_pattern, + GraphQL::Types::String, + required: false, + validates: { allow_blank: false }, + description: + 'Package name protected by the protection rule. For example, `@my-scope/my-package-*`. ' \ + 'Wildcard character `*` allowed.' + + argument :package_type, + Types::Packages::Protection::RulePackageTypeEnum, + required: false, + validates: { allow_blank: false }, + description: 'Package type protected by the protection rule. For example, `NPM`.' + + argument :push_protected_up_to_access_level, + Types::Packages::Protection::RuleAccessLevelEnum, + required: false, + validates: { allow_blank: false }, + description: + 'Maximum GitLab access level unable to push a package. For example, `DEVELOPER`, `MAINTAINER`, `OWNER`.' + + field :package_protection_rule, + Types::Packages::Protection::RuleType, + null: true, + description: 'Packages protection rule after mutation.' + + def resolve(id:, **kwargs) + package_protection_rule = authorized_find!(id: id) + + if Feature.disabled?(:packages_protected_packages, package_protection_rule.project) + raise_resource_not_available_error!("'packages_protected_packages' feature flag is disabled") + end + + response = ::Packages::Protection::UpdateRuleService.new(package_protection_rule, + current_user: current_user, params: kwargs).execute + + { package_protection_rule: response.payload[:package_protection_rule], errors: response.errors } + end + end + end + end + end +end diff --git a/app/graphql/types/mutation_type.rb b/app/graphql/types/mutation_type.rb index 8055247fb8a..d0a9ea11a27 100644 --- a/app/graphql/types/mutation_type.rb +++ b/app/graphql/types/mutation_type.rb @@ -177,6 +177,7 @@ module Types mount_mutation Mutations::Packages::DestroyFile mount_mutation Mutations::Packages::Protection::Rule::Create, alpha: { milestone: '16.5' } mount_mutation Mutations::Packages::Protection::Rule::Delete, alpha: { milestone: '16.6' } + mount_mutation Mutations::Packages::Protection::Rule::Update, alpha: { milestone: '16.6' } mount_mutation Mutations::Packages::DestroyFiles mount_mutation Mutations::Packages::Cleanup::Policy::Update mount_mutation Mutations::Echo diff --git a/app/models/concerns/transitionable.rb b/app/models/concerns/transitionable.rb index 70e1fc8b78a..b64b09336bb 100644 --- a/app/models/concerns/transitionable.rb +++ b/app/models/concerns/transitionable.rb @@ -6,9 +6,7 @@ module Transitionable attr_accessor :transitioning def transitioning? - return false unless transitioning && Feature.enabled?(:skip_validations_during_transitions, project) - - true + transitioning end def enable_transitioning diff --git a/app/services/packages/protection/update_rule_service.rb b/app/services/packages/protection/update_rule_service.rb new file mode 100644 index 00000000000..0dc7eb6a7b9 --- /dev/null +++ b/app/services/packages/protection/update_rule_service.rb @@ -0,0 +1,54 @@ +# frozen_string_literal: true + +module Packages + module Protection + class UpdateRuleService + include Gitlab::Allowable + + ALLOWED_ATTRIBUTES = %i[ + package_name_pattern + package_type + push_protected_up_to_access_level + ].freeze + + def initialize(package_protection_rule, current_user:, params:) + if package_protection_rule.blank? || current_user.blank? + raise ArgumentError, + 'package_protection_rule and current_user must be set' + end + + @package_protection_rule = package_protection_rule + @current_user = current_user + @params = params || {} + end + + def execute + unless can?(current_user, :admin_package, package_protection_rule.project) + error_message = _('Unauthorized to update a package protection rule') + return service_response_error(message: error_message) + end + + package_protection_rule.update(params.slice(*ALLOWED_ATTRIBUTES)) + + if package_protection_rule.errors.present? + return service_response_error(message: package_protection_rule.errors.full_messages) + end + + ServiceResponse.success(payload: { package_protection_rule: package_protection_rule }) + rescue StandardError => e + service_response_error(message: e.message) + end + + private + + attr_reader :package_protection_rule, :current_user, :params + + def service_response_error(message:) + ServiceResponse.error( + message: message, + payload: { package_protection_rule: nil } + ) + end + end + end +end diff --git a/config/feature_flags/development/skip_validations_during_transitions.yml b/config/feature_flags/development/skip_validations_during_transitions.yml deleted file mode 100644 index 53cf5f5ee71..00000000000 --- a/config/feature_flags/development/skip_validations_during_transitions.yml +++ /dev/null @@ -1,8 +0,0 @@ ---- -name: skip_validations_during_transitions -introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/129848 -rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/423092 -milestone: '16.4' -type: development -group: group::code review -default_enabled: false diff --git a/db/docs/approval_merge_request_rules_users.yml b/db/docs/approval_merge_request_rules_users.yml index 746aa70ebd2..d685eef8770 100644 --- a/db/docs/approval_merge_request_rules_users.yml +++ b/db/docs/approval_merge_request_rules_users.yml @@ -1,6 +1,7 @@ --- table_name: approval_merge_request_rules_users -classes: [] +classes: +- ApprovalMergeRequestRulesUser feature_categories: - source_code_management description: Keeps connection between user and a merge request approval rule diff --git a/doc/api/graphql/reference/index.md b/doc/api/graphql/reference/index.md index 6edf78aeec2..ad97272efff 100644 --- a/doc/api/graphql/reference/index.md +++ b/doc/api/graphql/reference/index.md @@ -7528,6 +7528,34 @@ Input type: `UpdatePackagesCleanupPolicyInput` | <a id="mutationupdatepackagescleanuppolicyerrors"></a>`errors` | [`[String!]!`](#string) | Errors encountered during execution of the mutation. | | <a id="mutationupdatepackagescleanuppolicypackagescleanuppolicy"></a>`packagesCleanupPolicy` | [`PackagesCleanupPolicy`](#packagescleanuppolicy) | Packages cleanup policy after mutation. | +### `Mutation.updatePackagesProtectionRule` + +Updates a package protection rule to restrict access to project packages. You can prevent users without certain permissions from altering packages. Available only when feature flag `packages_protected_packages` is enabled. + +WARNING: +**Introduced** in 16.6. +This feature is an Experiment. It can be changed or removed at any time. + +Input type: `UpdatePackagesProtectionRuleInput` + +#### Arguments + +| Name | Type | Description | +| ---- | ---- | ----------- | +| <a id="mutationupdatepackagesprotectionruleclientmutationid"></a>`clientMutationId` | [`String`](#string) | A unique identifier for the client performing the mutation. | +| <a id="mutationupdatepackagesprotectionruleid"></a>`id` | [`PackagesProtectionRuleID!`](#packagesprotectionruleid) | Global ID of the package protection rule to be updated. | +| <a id="mutationupdatepackagesprotectionrulepackagenamepattern"></a>`packageNamePattern` | [`String`](#string) | Package name protected by the protection rule. For example, `@my-scope/my-package-*`. Wildcard character `*` allowed. | +| <a id="mutationupdatepackagesprotectionrulepackagetype"></a>`packageType` | [`PackagesProtectionRulePackageType`](#packagesprotectionrulepackagetype) | Package type protected by the protection rule. For example, `NPM`. | +| <a id="mutationupdatepackagesprotectionrulepushprotecteduptoaccesslevel"></a>`pushProtectedUpToAccessLevel` | [`PackagesProtectionRuleAccessLevel`](#packagesprotectionruleaccesslevel) | Maximum GitLab access level unable to push a package. For example, `DEVELOPER`, `MAINTAINER`, `OWNER`. | + +#### Fields + +| Name | Type | Description | +| ---- | ---- | ----------- | +| <a id="mutationupdatepackagesprotectionruleclientmutationid"></a>`clientMutationId` | [`String`](#string) | A unique identifier for the client performing the mutation. | +| <a id="mutationupdatepackagesprotectionruleerrors"></a>`errors` | [`[String!]!`](#string) | Errors encountered during execution of the mutation. | +| <a id="mutationupdatepackagesprotectionrulepackageprotectionrule"></a>`packageProtectionRule` | [`PackagesProtectionRule`](#packagesprotectionrule) | Packages protection rule after mutation. | + ### `Mutation.updateRequirement` Input type: `UpdateRequirementInput` diff --git a/doc/api/merge_request_approvals.md b/doc/api/merge_request_approvals.md index 628f274c38f..fd8026d3077 100644 --- a/doc/api/merge_request_approvals.md +++ b/doc/api/merge_request_approvals.md @@ -1039,7 +1039,7 @@ Supported attributes: | Attribute | Type | Required | Description | |---------------------|-------------------|------------------------|--------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------| | `id` | integer or string | Yes | The ID or [URL-encoded path of a project](rest/index.md#namespaced-path-encoding). | -| `approval_password` | string | No | Current user's password. Required if [**Require user re-authentication to approve**](../user/project/merge_requests/approvals/settings.md#require-user-re-authentication-to-approve) is enabled in the project settings. | +| `approval_password` | string | No | Current user's password. Required if [**Require user password to approve**](../user/project/merge_requests/approvals/settings.md#require-user-password-to-approve) is enabled in the project settings. | | `merge_request_iid` | integer | Yes | The IID of the merge request. | | `sha` | string | No | The `HEAD` of the merge request. | diff --git a/doc/user/project/merge_requests/approvals/settings.md b/doc/user/project/merge_requests/approvals/settings.md index 9a510ed1582..bbc07f5e641 100644 --- a/doc/user/project/merge_requests/approvals/settings.md +++ b/doc/user/project/merge_requests/approvals/settings.md @@ -29,8 +29,8 @@ These settings limit who can approve merge requests: Prevents users who add commits to a merge request from also approving it. - [**Prevent editing approval rules in merge requests**](#prevent-editing-approval-rules-in-merge-requests): Prevents users from overriding project level approval rules on merge requests. -- [**Require user re-authentication (password or SAML) to approve**](#require-user-re-authentication-to-approve): - Force potential approvers to first authenticate with either a password or with SAML. +- [**Require user password to approve**](#require-user-password-to-approve): + Force potential approvers to first authenticate with a password. - Code Owner approval removals: Define what happens to existing approvals when commits are added to the merge request. - **Keep approvals**: Do not remove any approvals. @@ -104,29 +104,20 @@ on merge requests, you can disable this setting: This change affects all open merge requests. -## Require user re-authentication to approve +## Require user password to approve > - [Introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/5981) in GitLab 12.0. > - Moved to GitLab Premium in 13.9. -> - SAML authentication for GitLab.com groups [introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/5981) in GitLab 16.6. -You can force potential approvers to first authenticate with either: - -- A password. -- SAML. Available on GitLab.com groups only. - -This +You can force potential approvers to first authenticate with a password. This permission enables an electronic signature for approvals, such as the one defined by [Code of Federal Regulations (CFR) Part 11](https://www.accessdata.fda.gov/scripts/cdrh/cfdocs/cfcfr/CFRSearch.cfm?CFRPart=11&showFR=1&subpartNode=21:1.0.1.1.8.3)): -1. Enable password authentication and SAML authentication (available only on GitLab.com groups). For more information on: - - Password authentication, see - [sign-in restrictions documentation](../../../../administration/settings/sign_in_restrictions.md#password-authentication-enabled). - - SAML authentication for GitLab.com groups, see - [SAML SSO for GitLab.com groups documentation](../../../../user/group/saml_sso). +1. Enable password authentication for the web interface, as described in the + [sign-in restrictions documentation](../../../../administration/settings/sign_in_restrictions.md#password-authentication-enabled). 1. On the left sidebar, select **Settings > Merge requests**. 1. In the **Merge request approvals** section, scroll to **Approval settings** and - select **Require user re-authentication (password or SAML) to approve**. + select **Require user password to approve**. 1. Select **Save changes**. ## Remove all approvals when commits are added to the source branch diff --git a/lib/gitlab/bitbucket_import/ref_converter.rb b/lib/gitlab/bitbucket_import/ref_converter.rb index 1159159a76d..1763bd26d61 100644 --- a/lib/gitlab/bitbucket_import/ref_converter.rb +++ b/lib/gitlab/bitbucket_import/ref_converter.rb @@ -4,7 +4,7 @@ module Gitlab module BitbucketImport class RefConverter REPO_MATCHER = 'https://bitbucket.org/%s' - PR_NOTE_ISSUE_NAME_REGEX = '(?<=/)[^/\)]+(?=\)[^/]*$)' + PR_NOTE_ISSUE_NAME_REGEX = "(issues\/.*\/(.*)\\))" UNWANTED_NOTE_REF_HTML = "{: data-inline-card='' }" attr_reader :project @@ -24,7 +24,7 @@ module Gitlab if note.match?('issues') note.gsub!('issues', '-/issues') - note.gsub!(issue_name(note), '') + note.gsub!("/#{issue_name(note)}", '') if issue_name(note) else note.gsub!('pull-requests', '-/merge_requests') note.gsub!('src', '-/blob') @@ -41,7 +41,11 @@ module Gitlab end def issue_name(note) - note.match(PR_NOTE_ISSUE_NAME_REGEX)[0] + match_data = note.match(PR_NOTE_ISSUE_NAME_REGEX) + + return unless match_data + + match_data[2] end end end diff --git a/locale/gitlab.pot b/locale/gitlab.pot index 89c8453e677..31a0709854e 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -51329,6 +51329,9 @@ msgstr "" msgid "Unauthorized to delete a package protection rule" msgstr "" +msgid "Unauthorized to update a package protection rule" +msgstr "" + msgid "Unauthorized to update the environment" msgstr "" diff --git a/qa/gdk/Dockerfile.gdk b/qa/gdk/Dockerfile.gdk index eec86d8ca18..6b2e2e3315c 100644 --- a/qa/gdk/Dockerfile.gdk +++ b/qa/gdk/Dockerfile.gdk @@ -5,7 +5,7 @@ ENV GITLAB_LICENSE_MODE=test \ # Clone GDK at specific sha and bootstrap packages # -ARG GDK_SHA=af2d2124c594432dba364422c462135dcf3640f9 +ARG GDK_SHA=7f64f8fe4cc8615a35eee102968c991ba7ad58ca RUN set -eux; \ git clone --depth 1 https://gitlab.com/gitlab-org/gitlab-development-kit.git && cd gitlab-development-kit; \ git fetch --depth 1 origin ${GDK_SHA} && git -c advice.detachedHead=false checkout ${GDK_SHA}; \ diff --git a/qa/spec/runtime/path_spec.rb b/qa/spec/runtime/path_spec.rb new file mode 100644 index 00000000000..276522b3276 --- /dev/null +++ b/qa/spec/runtime/path_spec.rb @@ -0,0 +1,27 @@ +# frozen_string_literal: true + +RSpec.describe QA::Runtime::Path do + describe '.qa_root' do + it 'returns the fully-qualified path to the QA directory' do + expect(described_class.qa_root).to eq(File.expand_path('../../', __dir__)) + end + end + + describe '.fixtures_path' do + it 'returns the fully-qualified path to the fixtures directory' do + expect(described_class.fixtures_path).to eq(File.expand_path('../../qa/fixtures', __dir__)) + end + end + + describe '.fixture' do + it 'returns the fully-qualified path to a fixture file' do + expect(described_class.fixture('foo', 'bar')).to eq(File.expand_path('../../qa/fixtures/foo/bar', __dir__)) + end + end + + describe '.qa_tmp' do + it 'returns the fully-qualified path to the qa tmp directory' do + expect(described_class.qa_tmp('foo', 'bar')).to eq(File.expand_path('../../tmp/foo/bar', __dir__)) + end + end +end diff --git a/spec/lib/gitlab/bitbucket_import/ref_converter_spec.rb b/spec/lib/gitlab/bitbucket_import/ref_converter_spec.rb index 578b661d86b..c458214e794 100644 --- a/spec/lib/gitlab/bitbucket_import/ref_converter_spec.rb +++ b/spec/lib/gitlab/bitbucket_import/ref_converter_spec.rb @@ -19,7 +19,14 @@ RSpec.describe Gitlab::BitbucketImport::RefConverter, feature_category: :importe context 'when the note has an issue ref' do let(:note) { "[https://bitbucket.org/namespace/repo/issues/1/first-issue](https://bitbucket.org/namespace/repo/issues/1/first-issue){: data-inline-card='' } " } - let(:expected) { "[http://localhost/#{path}/-/issues/1/](http://localhost/#{path}/-/issues/1/)" } + let(:expected) { "[http://localhost/#{path}/-/issues/1](http://localhost/#{path}/-/issues/1)" } + + it_behaves_like 'converts the ref correctly' + end + + context 'when the note references issues without an issue name' do + let(:note) { "[https://bitbucket.org/namespace/repo/issues](https://bitbucket.org/namespace/repo/issues){: data-inline-card='' } " } + let(:expected) { "[http://localhost/#{path}/-/issues](http://localhost/#{path}/-/issues)" } it_behaves_like 'converts the ref correctly' end diff --git a/spec/models/concerns/transitionable_spec.rb b/spec/models/concerns/transitionable_spec.rb index b80d363ef78..c8cba1ae226 100644 --- a/spec/models/concerns/transitionable_spec.rb +++ b/spec/models/concerns/transitionable_spec.rb @@ -22,19 +22,16 @@ RSpec.describe Transitionable, feature_category: :code_review_workflow do let(:object) { klass.new(transitioning) } describe '#transitioning?' do - where(:transitioning, :feature_flag, :result) do - true | true | true - false | false | false - true | false | false - false | true | false + context "when trasnitioning" do + let(:transitioning) { true } + + it { expect(object.transitioning?).to eq(true) } end - with_them do - before do - stub_feature_flags(skip_validations_during_transitions: feature_flag) - end + context "when not trasnitioning" do + let(:transitioning) { false } - it { expect(object.transitioning?).to eq(result) } + it { expect(object.transitioning?).to eq(false) } end end end diff --git a/spec/models/merge_request_spec.rb b/spec/models/merge_request_spec.rb index d6efdc954e8..faea166c77d 100644 --- a/spec/models/merge_request_spec.rb +++ b/spec/models/merge_request_spec.rb @@ -404,7 +404,7 @@ RSpec.describe MergeRequest, factory_default: :keep, feature_category: :code_rev end end - context "with the skip_validations_during_transition_feature_flag" do + context "when transitioning between states" do let(:merge_request) { build(:merge_request, transitioning: transitioning) } where(:transitioning, :to_or_not_to) do 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 new file mode 100644 index 00000000000..cb4f9240311 --- /dev/null +++ b/spec/requests/api/graphql/mutations/packages/protection/rule/update_spec.rb @@ -0,0 +1,134 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe 'Updating the packages protection rule', :aggregate_failures, feature_category: :package_registry do + include GraphqlHelpers + + let_it_be(:project) { create(:project) } + let_it_be_with_reload(:package_protection_rule) do + create(:package_protection_rule, project: project, push_protected_up_to_access_level: :developer) + end + + let_it_be(:current_user) { create(:user, maintainer_projects: [project]) } + + let(:package_protection_rule_attributes) { build_stubbed(:package_protection_rule, project: project) } + + let(:mutation) do + graphql_mutation(:update_packages_protection_rule, input, + <<~QUERY + packageProtectionRule { + packageNamePattern + pushProtectedUpToAccessLevel + } + clientMutationId + errors + QUERY + ) + end + + let(:input) do + { + id: package_protection_rule.to_global_id, + package_name_pattern: "#{package_protection_rule.package_name_pattern}-updated", + push_protected_up_to_access_level: 'MAINTAINER' + } + end + + let(:mutation_response) { graphql_mutation_response(:update_packages_protection_rule) } + + subject { post_graphql_mutation(mutation, current_user: current_user) } + + shared_examples 'a successful response' do + it { subject.tap { expect_graphql_errors_to_be_empty } } + + it 'returns the updated package protection rule' do + subject + + expect(mutation_response).to include( + 'packageProtectionRule' => { + 'packageNamePattern' => input[:package_name_pattern], + 'pushProtectedUpToAccessLevel' => input[:push_protected_up_to_access_level] + } + ) + end + + it do + subject.tap do + expect(package_protection_rule.reload).to have_attributes( + package_name_pattern: input[:package_name_pattern], + push_protected_up_to_access_level: input[:push_protected_up_to_access_level].downcase + ) + end + end + end + + shared_examples 'an erroneous reponse' do + it { subject.tap { expect(mutation_response).to be_blank } } + it { expect { subject }.not_to change { package_protection_rule.reload.updated_at } } + end + + it_behaves_like 'a successful response' + + context 'with other existing package protection rule with same package_name_pattern' do + let_it_be_with_reload(:other_existing_package_protection_rule) do + create(:package_protection_rule, project: project, + package_name_pattern: "#{package_protection_rule.package_name_pattern}-other") + end + + let(:input) { super().merge(package_name_pattern: other_existing_package_protection_rule.package_name_pattern) } + + it { is_expected.tap { expect_graphql_errors_to_be_empty } } + + it 'returns a blank package protection rule' do + is_expected.tap { expect(mutation_response['packageProtectionRule']).to be_blank } + end + + it 'includes error message in response' do + is_expected.tap { expect(mutation_response['errors']).to eq ['Package name pattern has already been taken'] } + end + end + + context 'with invalid input param `pushProtectedUpToAccessLevel`' do + let(:input) { super().merge(push_protected_up_to_access_level: nil) } + + it_behaves_like 'an erroneous reponse' + + it { is_expected.tap { expect_graphql_errors_to_include(/pushProtectedUpToAccessLevel can't be blank/) } } + end + + context 'with invalid input param `packageNamePattern`' do + let(:input) { super().merge(package_name_pattern: '') } + + it_behaves_like 'an erroneous reponse' + + it { is_expected.tap { expect_graphql_errors_to_include(/packageNamePattern can't be blank/) } } + end + + context 'when current_user does not have permission' do + let_it_be(:developer) { create(:user).tap { |u| project.add_developer(u) } } + let_it_be(:reporter) { create(:user).tap { |u| project.add_reporter(u) } } + let_it_be(:guest) { create(:user).tap { |u| project.add_guest(u) } } + let_it_be(:anonymous) { create(:user) } + + where(:current_user) do + [ref(:developer), ref(:reporter), ref(:guest), ref(:anonymous)] + end + + with_them do + it { is_expected.tap { expect_graphql_errors_to_include(/you don't have permission to perform this action/) } } + end + end + + context "when feature flag ':packages_protected_packages' disabled" do + before do + stub_feature_flags(packages_protected_packages: false) + end + + it_behaves_like 'an erroneous reponse' + + it 'returns error of disabled feature flag' do + is_expected.tap { expect_graphql_errors_to_include(/'packages_protected_packages' feature flag is disabled/) } + end + end +end diff --git a/spec/services/packages/protection/update_rule_service_spec.rb b/spec/services/packages/protection/update_rule_service_spec.rb new file mode 100644 index 00000000000..70619a1caa3 --- /dev/null +++ b/spec/services/packages/protection/update_rule_service_spec.rb @@ -0,0 +1,150 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Packages::Protection::UpdateRuleService, '#execute', feature_category: :environment_management do + let_it_be(:project) { create(:project, :repository) } + let_it_be(:current_user) { create(:user, maintainer_projects: [project]) } + let_it_be_with_reload(:package_protection_rule) { create(:package_protection_rule, project: project) } + + let(:service) { described_class.new(package_protection_rule, current_user: current_user, params: params) } + + let(:params) do + attributes_for( + :package_protection_rule, + package_name_pattern: "#{package_protection_rule.package_name_pattern}-updated", + package_type: 'npm', + push_protected_up_to_access_level: 'owner' + ) + end + + subject(:service_execute) { service.execute } + + shared_examples 'a successful service response' do + let(:expected_attributes) { params } + + it { is_expected.to be_success } + + it do + is_expected.to have_attributes( + errors: be_blank, + message: be_blank, + payload: { package_protection_rule: be_a(Packages::Protection::Rule).and(have_attributes(expected_attributes)) } + ) + end + + it { expect { subject }.not_to change { Packages::Protection::Rule.count } } + + it { subject.tap { expect(package_protection_rule.reload).to have_attributes expected_attributes } } + end + + shared_examples 'an erroneous service response' do + it { is_expected.to be_error } + + it do + is_expected.to have_attributes( + errors: be_present, + message: be_present, + payload: { package_protection_rule: nil } + ) + end + + it { expect { subject }.not_to change { Packages::Protection::Rule.count } } + it { expect { subject }.not_to change { package_protection_rule.reload.updated_at } } + end + + it_behaves_like 'a successful service response' + + context 'with disallowed params' do + let(:params) { super().merge!(project_id: 1, unsupported_param: 'unsupported_param_value') } + + it_behaves_like 'a successful service response' do + let(:expected_attributes) { params.except(:project_id, :unsupported_param) } + end + end + + context 'when fields are invalid' do + let(:params) do + { package_name_pattern: '', package_type: 'unknown_package_type', + push_protected_up_to_access_level: 1000 } + end + + it_behaves_like 'an erroneous service response' + + it { is_expected.to have_attributes message: /'unknown_package_type' is not a valid package_type/ } + end + + context 'with empty params' do + let(:params) { {} } + + it_behaves_like 'a successful service response' do + let(:expected_attributes) { package_protection_rule.attributes } + end + + it { expect { service_execute }.not_to change { package_protection_rule.reload.updated_at } } + end + + context 'with nil params' do + let(:params) { nil } + + it_behaves_like 'a successful service response' do + let(:expected_attributes) { package_protection_rule.attributes } + end + + it { expect { service_execute }.not_to change { package_protection_rule.reload.updated_at } } + end + + context 'when updated field `package_name_pattern` is already taken' do + let_it_be_with_reload(:other_existing_package_protection_rule) do + create(:package_protection_rule, project: project, + package_name_pattern: "#{package_protection_rule.package_name_pattern}-other") + end + + let(:params) { { package_name_pattern: other_existing_package_protection_rule.package_name_pattern } } + + it_behaves_like 'an erroneous service response' + + it do + expect { service_execute }.not_to( + change { other_existing_package_protection_rule.reload.package_name_pattern } + ) + end + + it do + is_expected.to have_attributes( + errors: match_array([/Package name pattern has already been taken/]), + message: match_array([/Package name pattern has already been taken/]) + ) + end + end + + context 'when current_user does not have permission' do + let_it_be(:developer) { create(:user).tap { |u| project.add_developer(u) } } + let_it_be(:reporter) { create(:user).tap { |u| project.add_reporter(u) } } + let_it_be(:guest) { create(:user).tap { |u| project.add_guest(u) } } + let_it_be(:anonymous) { create(:user) } + + where(:current_user) do + [ref(:developer), ref(:reporter), ref(:guest), ref(:anonymous)] + end + + with_them do + it_behaves_like 'an erroneous service response' + + it { is_expected.to have_attributes errors: match_array(/Unauthorized/), message: /Unauthorized/ } + end + end + + context 'without package protection rule' do + let(:package_protection_rule) { nil } + let(:params) { {} } + + it { expect { service_execute }.to raise_error(ArgumentError) } + end + + context 'without current_user' do + let(:current_user) { nil } + + it { expect { service_execute }.to raise_error(ArgumentError) } + end +end |