From ab7a79bf3bb47fd1c9d82da0bb29a3cdf0246cdc Mon Sep 17 00:00:00 2001
From: Valery Sizov <valery@gitlab.com>
Date: Tue, 20 Jan 2015 15:23:37 -0800
Subject: [PATCH] developer can push to protected branches
---
.../projects/merge_requests_controller.rb | 8 +--
app/helpers/branches_helper.rb | 9 +--
app/helpers/tree_helper.rb | 6 +-
app/services/files/create_service.rb | 6 +-
app/services/files/delete_service.rb | 6 +-
app/services/files/update_service.rb | 6 +-
lib/api/merge_requests.rb | 8 +--
lib/gitlab/git_access.rb | 9 +++
spec/lib/gitlab/git_access_spec.rb | 62 +++++++++++++++++++
9 files changed, 80 insertions(+), 40 deletions(-)
diff --git a/app/controllers/projects/merge_requests_controller.rb b/app/controllers/projects/merge_requests_controller.rb
index 3f702b0af97..912f9eb5b6b 100644
--- a/app/controllers/projects/merge_requests_controller.rb
+++ b/app/controllers/projects/merge_requests_controller.rb
@@ -233,13 +233,7 @@ class Projects::MergeRequestsController < Projects::ApplicationController
end
def allowed_to_push_code?(project, branch)
- action = if project.protected_branch?(branch)
- :push_code_to_protected_branches
- else
- :push_code
- end
-
- can?(current_user, action, project)
+ ::Gitlab::GitAccess.can_push_to_branch?(current_user, project, branch)
end
def merge_request_params
diff --git a/app/helpers/branches_helper.rb b/app/helpers/branches_helper.rb
index 2ec2cc96157..4a5edf6d101 100644
--- a/app/helpers/branches_helper.rb
+++ b/app/helpers/branches_helper.rb
@@ -11,12 +11,7 @@ module BranchesHelper
def can_push_branch?(project, 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
diff --git a/app/helpers/tree_helper.rb b/app/helpers/tree_helper.rb
index d316213b1fd..133b0dfae9b 100644
--- a/app/helpers/tree_helper.rb
+++ b/app/helpers/tree_helper.rb
@@ -58,11 +58,7 @@ module TreeHelper
ref ||= @ref
return false unless project.repository.branch_names.include?(ref)
- if project.protected_branch? ref
- can?(current_user, :push_code_to_protected_branches, project)
- else
- can?(current_user, :push_code, project)
- end
+ ::Gitlab::GitAccess.can_push_to_branch?(current_user, project, ref)
end
def edit_blob_link(project, ref, path, options = {})
diff --git a/app/services/files/create_service.rb b/app/services/files/create_service.rb
index 82e4d7b684f..b90adeef00a 100644
--- a/app/services/files/create_service.rb
+++ b/app/services/files/create_service.rb
@@ -3,11 +3,7 @@ require_relative "base_service"
module Files
class CreateService < BaseService
def execute
- allowed = if project.protected_branch?(ref)
- can?(current_user, :push_code_to_protected_branches, project)
- else
- can?(current_user, :push_code, project)
- end
+ allowed = Gitlab::GitAccess.can_push_to_branch?(current_user, project, ref)
unless allowed
return error("You are not allowed to create file in this branch")
diff --git a/app/services/files/delete_service.rb b/app/services/files/delete_service.rb
index ff5dc6ef34c..8e73c2e2727 100644
--- a/app/services/files/delete_service.rb
+++ b/app/services/files/delete_service.rb
@@ -3,11 +3,7 @@ require_relative "base_service"
module Files
class DeleteService < BaseService
def execute
- allowed = if project.protected_branch?(ref)
- can?(current_user, :push_code_to_protected_branches, project)
- else
- can?(current_user, :push_code, project)
- end
+ allowed = ::Gitlab::GitAccess.can_push_to_branch?(current_user, project, ref)
unless allowed
return error("You are not allowed to push into this branch")
diff --git a/app/services/files/update_service.rb b/app/services/files/update_service.rb
index a0f40154db0..b4986e1c5c6 100644
--- a/app/services/files/update_service.rb
+++ b/app/services/files/update_service.rb
@@ -3,11 +3,7 @@ require_relative "base_service"
module Files
class UpdateService < BaseService
def execute
- allowed = if project.protected_branch?(ref)
- can?(current_user, :push_code_to_protected_branches, project)
- else
- can?(current_user, :push_code, project)
- end
+ allowed = ::Gitlab::GitAccess.can_push_to_branch?(current_user, project, ref)
unless allowed
return error("You are not allowed to push into this branch")
diff --git a/lib/api/merge_requests.rb b/lib/api/merge_requests.rb
index 81038d05f12..2a5b10c6f52 100644
--- a/lib/api/merge_requests.rb
+++ b/lib/api/merge_requests.rb
@@ -167,13 +167,9 @@ module API
put ":id/merge_request/:merge_request_id/merge" do
merge_request = user_project.merge_requests.find(params[:merge_request_id])
- action = if user_project.protected_branch?(merge_request.target_branch)
- :push_code_to_protected_branches
- else
- :push_code
- end
+ allowed = ::Gitlab::GitAccess.can_push_to_branch?(current_user, user_project, merge_request.target_branch)
- if can?(current_user, action, user_project)
+ if allowed
if merge_request.unchecked?
merge_request.check_if_can_be_merged
end
diff --git a/lib/gitlab/git_access.rb b/lib/gitlab/git_access.rb
index d47ef61fd11..c7bf2efc628 100644
--- a/lib/gitlab/git_access.rb
+++ b/lib/gitlab/git_access.rb
@@ -5,6 +5,15 @@ module Gitlab
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)
case cmd
when *DOWNLOAD_COMMANDS
diff --git a/spec/lib/gitlab/git_access_spec.rb b/spec/lib/gitlab/git_access_spec.rb
index 8561fd89ba7..fbcaa405f8d 100644
--- a/spec/lib/gitlab/git_access_spec.rb
+++ b/spec/lib/gitlab/git_access_spec.rb
@@ -5,6 +5,68 @@ describe Gitlab::GitAccess do
let(:project) { create(:project) }
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 'master permissions' do
before { project.team << [user, :master] }
--
GitLab