Commit 973bd462 authored by James Edwards-Jones's avatar James Edwards-Jones

ProtectedBranchPolicy used from Controller for destroy/update

parent e7061396
...@@ -6,10 +6,14 @@ class ProtectedBranchPolicy < BasePolicy ...@@ -6,10 +6,14 @@ class ProtectedBranchPolicy < BasePolicy
end end
rule { can?(:admin_project) }.policy do rule { can?(:admin_project) }.policy do
enable :create_protected_branch
enable :update_protected_branch enable :update_protected_branch
enable :destroy_protected_branch
end end
rule { requires_admin_to_unprotect? & ~admin }.policy do rule { requires_admin_to_unprotect? & ~admin }.policy do
prevent :create_protected_branch
prevent :update_protected_branch prevent :update_protected_branch
prevent :destroy_protected_branch
end end
end end
module ProtectedBranches module ProtectedBranches
class CreateService < BaseService class CreateService < BaseService
attr_reader :protected_branch
def execute(skip_authorization: false) def execute(skip_authorization: false)
raise Gitlab::Access::AccessDeniedError unless skip_authorization || can?(current_user, :admin_project, project) raise Gitlab::Access::AccessDeniedError unless skip_authorization || authorized?
protected_branch.save
protected_branch
end
def authorized?
can?(current_user, :create_protected_branch, protected_branch)
end
private
project.protected_branches.create(params) def protected_branch
@protected_branch ||= project.protected_branches.new(params)
end end
end end
end end
module ProtectedBranches module ProtectedBranches
class DestroyService < BaseService class DestroyService < BaseService
def execute(protected_branch) def execute(protected_branch)
raise Gitlab::Access::AccessDeniedError unless can?(current_user, :destroy_protected_branch, protected_branch)
protected_branch.destroy protected_branch.destroy
end end
end end
......
...@@ -36,6 +36,19 @@ describe Projects::ProtectedBranchesController do ...@@ -36,6 +36,19 @@ describe Projects::ProtectedBranchesController do
post(:create, project_params.merge(protected_branch: create_params)) post(:create, project_params.merge(protected_branch: create_params))
end.to change(ProtectedBranch, :count).by(1) end.to change(ProtectedBranch, :count).by(1)
end end
context 'when a policy restricts rule deletion' do
before do
policy = instance_double(ProtectedBranchPolicy, can?: false)
allow(ProtectedBranchPolicy).to receive(:new).and_return(policy)
end
it "prevents creation of the protected branch rule" do
post(:create, project_params.merge(protected_branch: create_params))
expect(ProtectedBranch.count).to eq 0
end
end
end end
describe "PUT #update" do describe "PUT #update" do
...@@ -51,6 +64,21 @@ describe Projects::ProtectedBranchesController do ...@@ -51,6 +64,21 @@ describe Projects::ProtectedBranchesController do
expect(protected_branch.reload.name).to eq('new_name') expect(protected_branch.reload.name).to eq('new_name')
expect(json_response["name"]).to eq('new_name') expect(json_response["name"]).to eq('new_name')
end end
context 'when a policy restricts rule deletion' do
before do
policy = instance_double(ProtectedBranchPolicy, can?: false)
allow(ProtectedBranchPolicy).to receive(:new).and_return(policy)
end
it "prevents update of the protected branch rule" do
old_name = protected_branch.name
put(:update, base_params.merge(protected_branch: update_params))
expect(protected_branch.reload.name).to eq(old_name)
end
end
end end
describe "DELETE #destroy" do describe "DELETE #destroy" do
...@@ -63,5 +91,18 @@ describe Projects::ProtectedBranchesController do ...@@ -63,5 +91,18 @@ describe Projects::ProtectedBranchesController do
expect { ProtectedBranch.find(protected_branch.id) }.to raise_error(ActiveRecord::RecordNotFound) expect { ProtectedBranch.find(protected_branch.id) }.to raise_error(ActiveRecord::RecordNotFound)
end end
context 'when a policy restricts rule deletion' do
before do
policy = instance_double(ProtectedBranchPolicy, can?: false)
allow(ProtectedBranchPolicy).to receive(:new).and_return(policy)
end
it "prevents deletion of the protected branch rule" do
delete(:destroy, base_params)
expect(response.status).to eq(403)
end
end
end end
end end
...@@ -35,5 +35,18 @@ describe ProtectedBranches::CreateService do ...@@ -35,5 +35,18 @@ describe ProtectedBranches::CreateService do
expect { service.execute }.to raise_error(Gitlab::Access::AccessDeniedError) expect { service.execute }.to raise_error(Gitlab::Access::AccessDeniedError)
end end
end end
context 'when a policy restricts rule creation' do
before do
policy = instance_double(ProtectedBranchPolicy, can?: false)
expect(ProtectedBranchPolicy).to receive(:new).and_return(policy)
end
it "prevents creation of the protected branch rule" do
expect do
service.execute
end.to raise_error(Gitlab::Access::AccessDeniedError)
end
end
end end
end end
...@@ -13,5 +13,18 @@ describe ProtectedBranches::DestroyService do ...@@ -13,5 +13,18 @@ describe ProtectedBranches::DestroyService do
expect(protected_branch).to be_destroyed expect(protected_branch).to be_destroyed
end end
context 'when a policy restricts rule deletion' do
before do
policy = instance_double(ProtectedBranchPolicy, can?: false)
expect(ProtectedBranchPolicy).to receive(:new).and_return(policy)
end
it "prevents deletion of the protected branch rule" do
expect do
service.execute(protected_branch)
end.to raise_error(Gitlab::Access::AccessDeniedError)
end
end
end end
end end
...@@ -22,5 +22,16 @@ describe ProtectedBranches::UpdateService do ...@@ -22,5 +22,16 @@ describe ProtectedBranches::UpdateService do
expect { service.execute(protected_branch) }.to raise_error(Gitlab::Access::AccessDeniedError) expect { service.execute(protected_branch) }.to raise_error(Gitlab::Access::AccessDeniedError)
end end
end end
context 'when a policy restricts rule creation' do
before do
policy = instance_double(ProtectedBranchPolicy, can?: false)
expect(ProtectedBranchPolicy).to receive(:new).and_return(policy)
end
it "prevents creation of the protected branch rule" do
expect { service.execute(protected_branch) }.to raise_error(Gitlab::Access::AccessDeniedError)
end
end
end end
end 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