Commit bcb340c2 authored by Patrick Bajao's avatar Patrick Bajao

Block creation/update/deletion of non-regular rule

Other types of rules (report_approver and code_owner) can be
generated by the system when features are enabled at the project
level.

Since these are system generated, we shouldn't allow users to
create/update/delete them.
parent 556e77c1
......@@ -4,7 +4,7 @@ class ApprovalMergeRequestRulePolicy < BasePolicy
delegate { @subject.merge_request }
condition(:editable) do
can?(:update_merge_request, @subject.merge_request)
can?(:update_merge_request, @subject.merge_request) && @subject.regular?
end
rule { editable }.enable :edit_approval_rule
......
......@@ -6,6 +6,14 @@ module API
ARRAY_COERCION_LAMBDA = ->(val) { val.empty? ? [] : Array.wrap(val) }
helpers do
def find_merge_request_approval_rule_with_access(merge_request, id, access_level = :edit_approval_rule)
approval_rule = merge_request.approval_rules.find_by_id!(id)
authorize! access_level, approval_rule
approval_rule
end
end
params do
requires :id, type: String, desc: 'The ID of a project'
requires :merge_request_iid, type: Integer, desc: 'The IID of a merge request'
......@@ -57,7 +65,7 @@ module API
put do
merge_request = find_merge_request_with_access(params[:merge_request_iid], :update_approvers)
params = declared_params(include_missing: false)
approval_rule = merge_request.approval_rules.find(params.delete(:approval_rule_id))
approval_rule = find_merge_request_approval_rule_with_access(merge_request, params.delete(:approval_rule_id))
result = ::ApprovalRules::UpdateService.new(approval_rule, current_user, params).execute
if result[:status] == :success
......@@ -73,7 +81,7 @@ module API
end
delete do
merge_request = find_merge_request_with_access(params[:merge_request_iid], :update_approvers)
approval_rule = merge_request.approval_rules.find(params[:approval_rule_id])
approval_rule = find_merge_request_approval_rule_with_access(merge_request, params[:approval_rule_id])
destroy_conditionally!(approval_rule) do |rule|
approval_rule.destroy
......
......@@ -4,7 +4,7 @@ require 'spec_helper'
describe ApprovalMergeRequestRulePolicy do
let(:merge_request) { create(:merge_request) }
let!(:approval_rule) { create(:approval_merge_request_rule, merge_request: merge_request) }
let(:approval_rule) { create(:approval_merge_request_rule, merge_request: merge_request) }
def permissions(user, approval_rule)
described_class.new(user, approval_rule)
......@@ -14,10 +14,18 @@ describe ApprovalMergeRequestRulePolicy do
it 'allows updating approval rule' do
expect(permissions(merge_request.author, approval_rule)).to be_allowed(:edit_approval_rule)
end
context 'when rule is not regular type' do
let(:approval_rule) { create(:code_owner_rule, merge_request: merge_request) }
it 'disallows updating approval rule' do
expect(permissions(merge_request.author, approval_rule)).to be_disallowed(:edit_approval_rule)
end
end
end
context 'when user cannot update merge request' do
it 'disallow updating approval rule' do
it 'disallows updating approval rule' do
expect(permissions(create(:user), approval_rule)).to be_disallowed(:edit_approval_rule)
end
end
......
......@@ -38,6 +38,16 @@ describe API::MergeRequestApprovalRules do
end
end
shared_examples_for 'a protected API endpoint that only allows action on regular merge request approval rule' do
context 'approval rule is not a regular type' do
let(:approval_rule) { create(:code_owner_rule, merge_request: merge_request) }
it 'responds with 403' do
expect(response).to have_gitlab_http_status(403)
end
end
end
describe 'GET /projects/:id/merge_requests/:merge_request_iid/approval_rules' do
let(:current_user) { other_user }
let(:url) { "/projects/#{project.id}/merge_requests/#{merge_request.iid}/approval_rules" }
......@@ -273,6 +283,8 @@ describe API::MergeRequestApprovalRules do
action
end
it_behaves_like 'a protected API endpoint that only allows action on regular merge request approval rule'
it 'matches the response schema' do
expect(response).to have_gitlab_http_status(200)
expect(response).to match_response_schema('public_api/v4/merge_request_approval_rule', dir: 'ee')
......@@ -348,6 +360,8 @@ describe API::MergeRequestApprovalRules do
action
end
it_behaves_like 'a protected API endpoint that only allows action on regular merge request approval rule'
it 'responds with 204' do
expect(response).to have_gitlab_http_status(204)
end
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment