diff --git a/app/policies/merge_request_policy.rb b/app/policies/merge_request_policy.rb index 8da2af506c7298c2ba3bd0c66cb9bc016ea820bd..49f9225a1d36cacbac2152f7b5cae70cb3291d5c 100644 --- a/app/policies/merge_request_policy.rb +++ b/app/policies/merge_request_policy.rb @@ -1,6 +1,8 @@ # frozen_string_literal: true class MergeRequestPolicy < IssuablePolicy + condition(:can_approve) { can_approve? } + rule { locked }.policy do prevent :reopen_merge_request end @@ -14,7 +16,7 @@ class MergeRequestPolicy < IssuablePolicy prevent :accept_merge_request end - rule { can?(:update_merge_request) & is_project_member }.policy do + rule { can_approve }.policy do enable :approve_merge_request end @@ -40,6 +42,12 @@ class MergeRequestPolicy < IssuablePolicy rule { can?(:admin_merge_request) }.policy do enable :set_merge_request_metadata end + + private + + def can_approve? + can?(:update_merge_request) && is_project_member? + end end MergeRequestPolicy.prepend_mod_with('MergeRequestPolicy') diff --git a/ee/app/policies/ee/merge_request_policy.rb b/ee/app/policies/ee/merge_request_policy.rb index baee58788b11a917c16d23761f170c109fff4c6a..88ac3dfff86f1919fca4b6e11004611d22c5f28d 100644 --- a/ee/app/policies/ee/merge_request_policy.rb +++ b/ee/app/policies/ee/merge_request_policy.rb @@ -3,6 +3,7 @@ module EE module MergeRequestPolicy extend ActiveSupport::Concern + extend ::Gitlab::Utils::Override prepended do with_scope :subject @@ -15,7 +16,7 @@ module MergeRequestPolicy can?(:developer_access, @subject.target_project) end - condition(:read_only, scope: :subject) { @subject.target_project&.namespace&.read_only? } + condition(:read_only, scope: :subject) { read_only? } condition(:merge_request_group_approver, score: 140) do project = @subject.target_project @@ -31,6 +32,10 @@ module MergeRequestPolicy @subject.target_project.licensed_feature_available?(:report_approver_rules) end + def read_only? + @subject.target_project&.namespace&.read_only? + end + def group_access?(protected_branch) protected_branch.approval_project_rules.for_groups(@user.group_members.reporters.select(:source_id)).exists? end @@ -47,14 +52,19 @@ def group_access?(protected_branch) rule { external_status_checks_enabled }.enable :provide_status_check_response rule { read_only }.policy do - prevent :approve_merge_request prevent :update_merge_request - prevent :reopen_merge_request - prevent :create_note - prevent :resolve_note end rule { approval_rules_licence_enabled }.enable :create_merge_request_approval_rules end + + private + + override :can_approve? + def can_approve? + return can?(:developer_access) if read_only? + + super + end end end diff --git a/ee/app/policies/ee/readonly_abilities.rb b/ee/app/policies/ee/readonly_abilities.rb index 6d478bdc5f9f7a51641cf6f4512bb0227f3f4060..52cc0704cf43bb9f1eea8a5bcc857bade1241881 100644 --- a/ee/app/policies/ee/readonly_abilities.rb +++ b/ee/app/policies/ee/readonly_abilities.rb @@ -7,7 +7,6 @@ module ReadonlyAbilities READONLY_ABILITIES = %i[ admin_tag push_to_delete_protected_branch - resolve_note create_merge_request_from create_merge_request_in admin_software_license_policy diff --git a/ee/spec/policies/merge_request_policy_spec.rb b/ee/spec/policies/merge_request_policy_spec.rb index f47f985a6d1e9fbd9f11f4521ebf640376cd27d8..08e85d38ca94823e3c411a121e6c44be9df264c2 100644 --- a/ee/spec/policies/merge_request_policy_spec.rb +++ b/ee/spec/policies/merge_request_policy_spec.rb @@ -233,12 +233,12 @@ def policy_for(user) allow(merge_request.target_project.namespace).to receive(:read_only?).and_return(true) end - it 'does not allow few policies for all users including maintainer' do - expect(policy_for(maintainer)).to be_disallowed(:approve_merge_request, - :update_merge_request, - :reopen_merge_request, - :create_note, - :resolve_note) + it 'does not allow update_merge_request for all users including maintainer' do + expect(policy_for(maintainer)).to be_disallowed(:update_merge_request) + end + + it 'does allow approval of the merge request' do + expect(policy_for(developer)).to be_allowed(:approve_merge_request) end end