Commit 1050f523 authored by Dmitriy Zaporozhets's avatar Dmitriy Zaporozhets

Merge branch 'protected_branches' into 'master'

Developer can push to protected branch if allowed

#1876

See merge request !1418
parents 148740cc ab7a79bf
...@@ -233,13 +233,7 @@ class Projects::MergeRequestsController < Projects::ApplicationController ...@@ -233,13 +233,7 @@ class Projects::MergeRequestsController < Projects::ApplicationController
end end
def allowed_to_push_code?(project, branch) def allowed_to_push_code?(project, branch)
action = if project.protected_branch?(branch) ::Gitlab::GitAccess.can_push_to_branch?(current_user, project, branch)
:push_code_to_protected_branches
else
:push_code
end
can?(current_user, action, project)
end end
def merge_request_params def merge_request_params
......
...@@ -11,12 +11,7 @@ module BranchesHelper ...@@ -11,12 +11,7 @@ module BranchesHelper
def can_push_branch?(project, branch_name) def can_push_branch?(project, branch_name)
return false unless project.repository.branch_names.include?(branch_name) return false unless project.repository.branch_names.include?(branch_name)
action = if project.protected_branch?(branch_name)
:push_code_to_protected_branches
else
:push_code
end
current_user.can?(action, project) ::Gitlab::GitAccess.can_push_to_branch?(current_user, project, branch_name)
end end
end end
...@@ -58,11 +58,7 @@ module TreeHelper ...@@ -58,11 +58,7 @@ module TreeHelper
ref ||= @ref ref ||= @ref
return false unless project.repository.branch_names.include?(ref) return false unless project.repository.branch_names.include?(ref)
if project.protected_branch? ref ::Gitlab::GitAccess.can_push_to_branch?(current_user, project, ref)
can?(current_user, :push_code_to_protected_branches, project)
else
can?(current_user, :push_code, project)
end
end end
def edit_blob_link(project, ref, path, options = {}) def edit_blob_link(project, ref, path, options = {})
......
...@@ -3,11 +3,7 @@ require_relative "base_service" ...@@ -3,11 +3,7 @@ require_relative "base_service"
module Files module Files
class CreateService < BaseService class CreateService < BaseService
def execute def execute
allowed = if project.protected_branch?(ref) allowed = Gitlab::GitAccess.can_push_to_branch?(current_user, project, ref)
can?(current_user, :push_code_to_protected_branches, project)
else
can?(current_user, :push_code, project)
end
unless allowed unless allowed
return error("You are not allowed to create file in this branch") return error("You are not allowed to create file in this branch")
......
...@@ -3,11 +3,7 @@ require_relative "base_service" ...@@ -3,11 +3,7 @@ require_relative "base_service"
module Files module Files
class DeleteService < BaseService class DeleteService < BaseService
def execute def execute
allowed = if project.protected_branch?(ref) allowed = ::Gitlab::GitAccess.can_push_to_branch?(current_user, project, ref)
can?(current_user, :push_code_to_protected_branches, project)
else
can?(current_user, :push_code, project)
end
unless allowed unless allowed
return error("You are not allowed to push into this branch") return error("You are not allowed to push into this branch")
......
...@@ -3,11 +3,7 @@ require_relative "base_service" ...@@ -3,11 +3,7 @@ require_relative "base_service"
module Files module Files
class UpdateService < BaseService class UpdateService < BaseService
def execute def execute
allowed = if project.protected_branch?(ref) allowed = ::Gitlab::GitAccess.can_push_to_branch?(current_user, project, ref)
can?(current_user, :push_code_to_protected_branches, project)
else
can?(current_user, :push_code, project)
end
unless allowed unless allowed
return error("You are not allowed to push into this branch") return error("You are not allowed to push into this branch")
......
...@@ -167,13 +167,9 @@ module API ...@@ -167,13 +167,9 @@ module API
put ":id/merge_request/:merge_request_id/merge" do put ":id/merge_request/:merge_request_id/merge" do
merge_request = user_project.merge_requests.find(params[:merge_request_id]) merge_request = user_project.merge_requests.find(params[:merge_request_id])
action = if user_project.protected_branch?(merge_request.target_branch) allowed = ::Gitlab::GitAccess.can_push_to_branch?(current_user, user_project, merge_request.target_branch)
:push_code_to_protected_branches
else
:push_code
end
if can?(current_user, action, user_project) if allowed
if merge_request.unchecked? if merge_request.unchecked?
merge_request.check_if_can_be_merged merge_request.check_if_can_be_merged
end end
......
...@@ -5,6 +5,15 @@ module Gitlab ...@@ -5,6 +5,15 @@ module Gitlab
attr_reader :params, :project, :git_cmd, :user attr_reader :params, :project, :git_cmd, :user
def self.can_push_to_branch?(user, project, ref)
if project.protected_branch?(ref) &&
!(project.developers_can_push_to_protected_branch?(ref) && project.team.developer?(user))
user.can?(:push_code_to_protected_branches, project)
else
user.can?(:push_code, project)
end
end
def check(actor, cmd, project, changes = nil) def check(actor, cmd, project, changes = nil)
case cmd case cmd
when *DOWNLOAD_COMMANDS when *DOWNLOAD_COMMANDS
......
...@@ -5,6 +5,68 @@ describe Gitlab::GitAccess do ...@@ -5,6 +5,68 @@ describe Gitlab::GitAccess do
let(:project) { create(:project) } let(:project) { create(:project) }
let(:user) { create(:user) } let(:user) { create(:user) }
describe 'can_push_to_branch?' do
describe 'push to none protected branch' do
it "returns true if user is a master" do
project.team << [user, :master]
Gitlab::GitAccess.can_push_to_branch?(user, project, "random_branch").should be_true
end
it "returns true if user is a developer" do
project.team << [user, :developer]
Gitlab::GitAccess.can_push_to_branch?(user, project, "random_branch").should be_true
end
it "returns false if user is a reporter" do
project.team << [user, :reporter]
Gitlab::GitAccess.can_push_to_branch?(user, project, "random_branch").should be_false
end
end
describe 'push to protected branch' do
before do
@branch = create :protected_branch, project: project
end
it "returns true if user is a master" do
project.team << [user, :master]
Gitlab::GitAccess.can_push_to_branch?(user, project, @branch.name).should be_true
end
it "returns false if user is a developer" do
project.team << [user, :developer]
Gitlab::GitAccess.can_push_to_branch?(user, project, @branch.name).should be_false
end
it "returns false if user is a reporter" do
project.team << [user, :reporter]
Gitlab::GitAccess.can_push_to_branch?(user, project, @branch.name).should be_false
end
end
describe 'push to protected branch if allowed for developers' do
before do
@branch = create :protected_branch, project: project, developers_can_push: true
end
it "returns true if user is a master" do
project.team << [user, :master]
Gitlab::GitAccess.can_push_to_branch?(user, project, @branch.name).should be_true
end
it "returns true if user is a developer" do
project.team << [user, :developer]
Gitlab::GitAccess.can_push_to_branch?(user, project, @branch.name).should be_true
end
it "returns false if user is a reporter" do
project.team << [user, :reporter]
Gitlab::GitAccess.can_push_to_branch?(user, project, @branch.name).should be_false
end
end
end
describe 'download_access_check' do describe 'download_access_check' do
describe 'master permissions' do describe 'master permissions' do
before { project.team << [user, :master] } before { project.team << [user, :master] }
......
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