diff --git a/config/authz/permissions/merge_request_approval/approve.yml b/config/authz/permissions/merge_request_approval/approve.yml new file mode 100644 index 0000000000000000000000000000000000000000..fcef14886cb024140d2943327129dd1d298eb3a6 --- /dev/null +++ b/config/authz/permissions/merge_request_approval/approve.yml @@ -0,0 +1,6 @@ +--- +name: approve_merge_request +description: Grants the ability to approve merge requests +feature_category: source_code_management +boundaries: + - project diff --git a/config/authz/permissions/merge_request_approval/read.yml b/config/authz/permissions/merge_request_approval/read.yml new file mode 100644 index 0000000000000000000000000000000000000000..3adddd7c290956a937d69a8c11ab6161b10dce41 --- /dev/null +++ b/config/authz/permissions/merge_request_approval/read.yml @@ -0,0 +1,6 @@ +--- +name: read_merge_request_approval +description: Grants the ability to read merge request approvals +feature_category: source_code_management +boundaries: + - project diff --git a/ee/lib/ee/api/merge_request_approvals.rb b/ee/lib/ee/api/merge_request_approvals.rb index 55805f34b4fc83e4edc611de97684088dcd3343d..901ef2b2c7ce12b5ee12063aabd4e887fa323431 100644 --- a/ee/lib/ee/api/merge_request_approvals.rb +++ b/ee/lib/ee/api/merge_request_approvals.rb @@ -51,6 +51,7 @@ def present_merge_request_approval_state(presenter:, target_branch: nil) desc 'Get approval state of merge request' do success ::API::Entities::MergeRequestApprovalState end + route_setting :authorization, permissions: :read_merge_request_approval, boundary_type: :project get 'approval_state' do present_merge_request_approval_state(presenter: ::API::Entities::MergeRequestApprovalState) end diff --git a/ee/spec/requests/api/merge_request_approvals_spec.rb b/ee/spec/requests/api/merge_request_approvals_spec.rb index bfbcde447ff03104b9f30df24411496d04a3248d..2fc58eb6e65ee4e5452da8112f2d7196c5244346 100644 --- a/ee/spec/requests/api/merge_request_approvals_spec.rb +++ b/ee/spec/requests/api/merge_request_approvals_spec.rb @@ -249,6 +249,14 @@ expect(rule_response['approved_by'][0]['username']).to eq(approver.username) expect(rule_response['source_rule']).to eq(nil) end + + it_behaves_like 'authorizing granular token permissions', :read_merge_request_approval do + let(:boundary_object) { project } + let(:user) { user } + let(:request) do + get api("/projects/#{project.id}/merge_requests/#{merge_request.iid}/approval_state", personal_access_token: pat) + end + end end describe 'POST :id/merge_requests/:merge_request_iid/approvals' do diff --git a/lib/api/merge_request_approvals.rb b/lib/api/merge_request_approvals.rb index 5c0ac409bda852a1e48baf1ad7cacc9f00268965..bb282976953267bb8a8e1b119fb62b1a2c8e5d68 100644 --- a/lib/api/merge_request_approvals.rb +++ b/lib/api/merge_request_approvals.rb @@ -29,6 +29,7 @@ def present_approval(merge_request) { code: 404, message: 'Not found' } ] end + route_setting :authorization, permissions: :read_merge_request_approval, boundary_type: :project get 'approvals', urgency: :low do merge_request = find_merge_request_with_access(params[:merge_request_iid]) @@ -56,6 +57,7 @@ def present_approval(merge_request) use :ee_approval_params end + route_setting :authorization, permissions: :approve_merge_request, boundary_type: :project post 'approve', urgency: :low do merge_request = find_merge_request_with_access(params[:merge_request_iid], :approve_merge_request) @@ -84,6 +86,7 @@ def present_approval(merge_request) { code: 401, message: 'Unauthorized' } ] end + route_setting :authorization, permissions: :approve_merge_request, boundary_type: :project post 'unapprove', urgency: :low do merge_request = find_merge_request_with_access(params[:merge_request_iid], :approve_merge_request) @@ -104,6 +107,7 @@ def present_approval(merge_request) ] tags %w[merge_requests] end + route_setting :authorization, permissions: :approve_merge_request, boundary_type: :project put 'reset_approvals', urgency: :low do merge_request = find_project_merge_request(params[:merge_request_iid]) diff --git a/spec/requests/api/merge_request_approvals_spec.rb b/spec/requests/api/merge_request_approvals_spec.rb index d1738780d9308b0a5d9de27fb7bc6147594fdc0b..6317e57e496415c5cf8cb6860b0415c9b44b2103 100644 --- a/spec/requests/api/merge_request_approvals_spec.rb +++ b/spec/requests/api/merge_request_approvals_spec.rb @@ -22,6 +22,14 @@ expect(response).to have_gitlab_http_status(:ok) end + it_behaves_like 'authorizing granular token permissions', :read_merge_request_approval do + let(:boundary_object) { project } + let(:user) { user } + let(:request) do + get api("/projects/#{project.id}/merge_requests/#{merge_request.iid}/approvals", personal_access_token: pat) + end + end + context 'when merge request author has only guest access' do it_behaves_like 'rejects user from accessing merge request info' do let(:url) { "/projects/#{project.id}/merge_requests/#{merge_request.iid}/approvals" } @@ -50,6 +58,14 @@ def approve(extra_params = {}) expect(response).to have_gitlab_http_status(:created) end + + it_behaves_like 'authorizing granular token permissions', :approve_merge_request do + let(:boundary_object) { project } + let(:user) { approver } + let(:request) do + post api("/projects/#{project.id}/merge_requests/#{merge_request.iid}/approve", personal_access_token: pat) + end + end end context 'when the publish_review param is false' do @@ -130,6 +146,17 @@ def approve(extra_params = {}) expect(response).to have_gitlab_http_status(:created) end + it_behaves_like 'authorizing granular token permissions', :approve_merge_request do + let(:boundary_object) { project } + let(:unapprover) { create(:user) } + let(:user) { unapprover } + let(:request) do + project.add_developer(unapprover) + create(:approval, user: unapprover, merge_request: merge_request) + post api("/projects/#{project.id}/merge_requests/#{merge_request.iid}/unapprove", personal_access_token: pat) + end + end + it 'calls MergeRequests::UpdateReviewerStateService' do unapprover = create(:user) @@ -181,6 +208,14 @@ def approve(extra_params = {}) expect(merge_request.approvals).to be_empty end + it_behaves_like 'authorizing granular token permissions', :approve_merge_request do + let(:boundary_object) { project } + let(:user) { bot } + let(:request) do + put api("/projects/#{project.id}/merge_requests/#{merge_request.iid}/reset_approvals", personal_access_token: pat) + end + end + context 'when bot user approved the merge request' do before do merge_request.approvals.create!(user: bot)