diff options
| -rw-r--r-- | app/controllers/projects/merge_requests_controller.rb | 8 | ||||
| -rw-r--r-- | app/helpers/branches_helper.rb | 9 | ||||
| -rw-r--r-- | app/helpers/tree_helper.rb | 6 | ||||
| -rw-r--r-- | app/services/files/create_service.rb | 6 | ||||
| -rw-r--r-- | app/services/files/delete_service.rb | 6 | ||||
| -rw-r--r-- | app/services/files/update_service.rb | 6 | ||||
| -rw-r--r-- | lib/api/merge_requests.rb | 8 | ||||
| -rw-r--r-- | lib/gitlab/git_access.rb | 9 | ||||
| -rw-r--r-- | 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] } |