summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
Diffstat
-rw-r--r--app/helpers/branches_helper.rb2
-rw-r--r--app/models/merge_request.rb2
-rw-r--r--app/services/commits/change_service.rb4
-rw-r--r--app/services/files/base_service.rb2
-rw-r--r--lib/api/helpers.rb2
-rw-r--r--lib/gitlab/git_access.rb159
-rw-r--r--lib/gitlab/git_access/change_access_check.rb96
-rw-r--r--lib/gitlab/git_access_wiki.rb2
-rw-r--r--lib/gitlab/user_access.rb48
-rw-r--r--spec/lib/gitlab/git_access_spec.rb82
-rw-r--r--spec/lib/gitlab/user_access_spec.rb90
-rw-r--r--spec/requests/api/api_helpers_spec.rb4
12 files changed, 270 insertions, 223 deletions
diff --git a/app/helpers/branches_helper.rb b/app/helpers/branches_helper.rb
index 601df5c18df..bfd23aa4e04 100644
--- a/app/helpers/branches_helper.rb
+++ b/app/helpers/branches_helper.rb
@@ -12,7 +12,7 @@ module BranchesHelper
def can_push_branch?(project, branch_name)
return false unless project.repository.branch_exists?(branch_name)
- ::Gitlab::GitAccess.new(current_user, project, 'web').can_push_to_branch?(branch_name)
+ ::Gitlab::UserAccess.new(current_user, project: project).can_push_to_branch?(branch_name)
end
def project_branches
diff --git a/app/models/merge_request.rb b/app/models/merge_request.rb
index 23d68706527..e5853bdd589 100644
--- a/app/models/merge_request.rb
+++ b/app/models/merge_request.rb
@@ -552,7 +552,7 @@ class MergeRequest < ActiveRecord::Base
end
def can_be_merged_by?(user)
- access = ::Gitlab::GitAccess.new(user, project, 'web')
+ access = ::Gitlab::UserAccess.new(user, project: project)
access.can_push_to_branch?(target_branch) || access.can_merge_to_branch?(target_branch)
end
diff --git a/app/services/commits/change_service.rb b/app/services/commits/change_service.rb
index c578097376a..ed73d8cb8c2 100644
--- a/app/services/commits/change_service.rb
+++ b/app/services/commits/change_service.rb
@@ -23,7 +23,7 @@ module Commits
private
def check_push_permissions
- allowed = ::Gitlab::GitAccess.new(current_user, project, 'web').can_push_to_branch?(@target_branch)
+ allowed = ::Gitlab::UserAccess.new(current_user, project: project).can_push_to_branch?(@target_branch)
unless allowed
raise ValidationError.new('You are not allowed to push into this branch')
@@ -31,7 +31,7 @@ module Commits
true
end
-
+
def create_target_branch(new_branch)
# Temporary branch exists and contains the change commit
return success if repository.find_branch(new_branch)
diff --git a/app/services/files/base_service.rb b/app/services/files/base_service.rb
index 37c5e321b39..55da949f56a 100644
--- a/app/services/files/base_service.rb
+++ b/app/services/files/base_service.rb
@@ -42,7 +42,7 @@ module Files
end
def validate
- allowed = ::Gitlab::GitAccess.new(current_user, project, 'web').can_push_to_branch?(@target_branch)
+ allowed = ::Gitlab::UserAccess.new(current_user, project: project).can_push_to_branch?(@target_branch)
unless allowed
raise_error("You are not allowed to push into this branch")
diff --git a/lib/api/helpers.rb b/lib/api/helpers.rb
index 77e407b54c5..73557cf7db6 100644
--- a/lib/api/helpers.rb
+++ b/lib/api/helpers.rb
@@ -17,7 +17,7 @@ module API
def current_user
@current_user ||= (find_user_by_private_token || doorkeeper_guard)
- unless @current_user && Gitlab::UserAccess.allowed?(@current_user)
+ unless @current_user && Gitlab::UserAccess.new(@current_user).allowed?
return nil
end
diff --git a/lib/gitlab/git_access.rb b/lib/gitlab/git_access.rb
index feaf845209e..cfb48ac2382 100644
--- a/lib/gitlab/git_access.rb
+++ b/lib/gitlab/git_access.rb
@@ -1,62 +1,17 @@
+# Check a user's access to perform a git action. All public methods in this
+# class return an instance of `GitlabAccessStatus`
module Gitlab
class GitAccess
DOWNLOAD_COMMANDS = %w{ git-upload-pack git-upload-archive }
PUSH_COMMANDS = %w{ git-receive-pack }
- attr_reader :actor, :project, :protocol
+ attr_reader :actor, :project, :protocol, :user_access
def initialize(actor, project, protocol)
@actor = actor
@project = project
@protocol = protocol
- end
-
- def user
- return @user if defined?(@user)
-
- @user =
- case actor
- when User
- actor
- when DeployKey
- nil
- when Key
- actor.user
- end
- end
-
- def deploy_key
- actor if actor.is_a?(DeployKey)
- end
-
- def can_push_to_branch?(ref)
- return false unless user
-
- if project.protected_branch?(ref) && !project.developers_can_push_to_protected_branch?(ref)
- user.can?(:push_code_to_protected_branches, project)
- else
- user.can?(:push_code, project)
- end
- end
-
- def can_merge_to_branch?(ref)
- return false unless user
-
- if project.protected_branch?(ref) && !project.developers_can_merge_to_protected_branch?(ref)
- user.can?(:push_code_to_protected_branches, project)
- else
- user.can?(:push_code, project)
- end
- end
-
- def can_read_project?
- if user
- user.can?(:read_project, project)
- elsif deploy_key
- deploy_key.projects.include?(project)
- else
- false
- end
+ @user_access = UserAccess.new(user, project: project)
end
def check(cmd, changes = nil)
@@ -66,11 +21,11 @@ module Gitlab
return build_status_object(false, "No user or key was provided.")
end
- if user && !user_allowed?
+ if user && !user_access.allowed?
return build_status_object(false, "Your account has been blocked.")
end
- unless project && can_read_project?
+ unless project && (user_access.can_read_project? || deploy_key_can_read_project?)
return build_status_object(false, 'The project you were looking for could not be found.')
end
@@ -105,7 +60,7 @@ module Gitlab
end
def user_download_access_check
- unless user.can?(:download_code, project)
+ unless user_access.can_do_action?(:download_code)
return build_status_object(false, "You are not allowed to download code from this project.")
end
@@ -135,103 +90,49 @@ module Gitlab
build_status_object(true)
end
- def can_user_do_action?(action)
- @permission_cache ||= {}
- @permission_cache[action] ||= user.can?(action, project)
- end
-
def change_access_check(change)
- oldrev, newrev, ref = change.split(' ')
-
- action =
- if project.protected_branch?(branch_name(ref))
- protected_branch_action(oldrev, newrev, branch_name(ref))
- elsif (tag_ref = tag_name(ref)) && protected_tag?(tag_ref)
- # Prevent any changes to existing git tag unless user has permissions
- :admin_project
- else
- :push_code
- end
-
- unless can_user_do_action?(action)
- status =
- case action
- when :force_push_code_to_protected_branches
- build_status_object(false, "You are not allowed to force push code to a protected branch on this project.")
- when :remove_protected_branches
- build_status_object(false, "You are not allowed to deleted protected branches from this project.")
- when :push_code_to_protected_branches
- build_status_object(false, "You are not allowed to push code to protected branches on this project.")
- when :admin_project
- build_status_object(false, "You are not allowed to change existing tags on this project.")
- else # :push_code
- build_status_object(false, "You are not allowed to push code to this project.")
- end
- return status
- end
-
- build_status_object(true)
- end
-
- def forced_push?(oldrev, newrev)
- Gitlab::ForcePushCheck.force_push?(project, oldrev, newrev)
+ ChangeAccessCheck.new(change, user_access: user_access, project: project).exec
end
-
def protocol_allowed?
Gitlab::ProtocolAccess.allowed?(protocol)
end
- def matching_merge_request?(newrev, branch_name)
- Checks::MatchingMergeRequest.new(newrev, branch_name, project).match?
- end
-
private
- def protected_branch_action(oldrev, newrev, branch_name)
- # we dont allow force push to protected branch
- if forced_push?(oldrev, newrev)
- :force_push_code_to_protected_branches
- elsif Gitlab::Git.blank_ref?(newrev)
- # and we dont allow remove of protected branch
- :remove_protected_branches
- elsif matching_merge_request?(newrev, branch_name) && project.developers_can_merge_to_protected_branch?(branch_name)
- :push_code
- elsif project.developers_can_push_to_protected_branch?(branch_name)
- :push_code
- else
- :push_code_to_protected_branches
- end
- end
-
- def protected_tag?(tag_name)
- project.repository.tag_exists?(tag_name)
+ def matching_merge_request?(newrev, branch_name)
+ Checks::MatchingMergeRequest.new(newrev, branch_name, project).match?
end
- def user_allowed?
- Gitlab::UserAccess.allowed?(user)
+ def deploy_key
+ actor if actor.is_a?(DeployKey)
end
- def branch_name(ref)
- ref = ref.to_s
- if Gitlab::Git.branch_ref?(ref)
- Gitlab::Git.ref_name(ref)
- else
- nil
- end
- end
- def tag_name(ref)
- ref = ref.to_s
- if Gitlab::Git.tag_ref?(ref)
- Gitlab::Git.ref_name(ref)
+ def deploy_key_can_read_project?
+ if deploy_key
+ deploy_key.projects.include?(project)
else
- nil
+ false
end
end
protected
+ def user
+ return @user if defined?(@user)
+
+ @user =
+ case actor
+ when User
+ actor
+ when DeployKey
+ nil
+ when Key
+ actor.user
+ end
+ end
+
def build_status_object(status, message = '')
GitAccessStatus.new(status, message)
end
diff --git a/lib/gitlab/git_access/change_access_check.rb b/lib/gitlab/git_access/change_access_check.rb
new file mode 100644
index 00000000000..9268da4ec6e
--- /dev/null
+++ b/lib/gitlab/git_access/change_access_check.rb
@@ -0,0 +1,96 @@
+module Gitlab
+ class GitAccess
+ class ChangeAccessCheck
+ attr_reader :user_access, :project
+
+ def initialize(change, user_access:, project:)
+ @oldrev, @newrev, @ref = change.split(' ')
+ @branch_name = branch_name(@ref)
+ @user_access = user_access
+ @project = project
+ end
+
+ def exec
+ error = protected_branch_checks || tag_checks || push_checks
+
+ if error
+ GitAccessStatus.new(false, error)
+ else
+ GitAccessStatus.new(true)
+ end
+ end
+
+ protected
+
+ def protected_branch_checks
+ return unless project.protected_branch?(@branch_name)
+
+ return unless project.protected_branch?(@branch_name)
+
+ if forced_push? && user_access.cannot_do_action?(:force_push_code_to_protected_branches)
+ return "You are not allowed to force push code to a protected branch on this project."
+ elsif Gitlab::Git.blank_ref?(@newrev) && user_access.cannot_do_action?(:remove_protected_branches)
+ return "You are not allowed to deleted protected branches from this project."
+ end
+
+ if matching_merge_request?
+ if user_access.can_merge_to_branch?(@branch_name) || user_access.can_push_to_branch?(@branch_name)
+ return
+ else
+ "You are not allowed to merge code into protected branches on this project."
+ end
+ else
+ if user_access.can_push_to_branch?(@branch_name)
+ return
+ else
+ "You are not allowed to push code to protected branches on this project."
+ end
+ end
+ end
+
+ def tag_checks
+ if (tag_ref = tag_name(@ref)) && protected_tag?(tag_ref) && user_access.cannot_do_action?(:admin_project)
+ "You are not allowed to change existing tags on this project."
+ end
+ end
+
+ def push_checks
+ if user_access.cannot_do_action?(:push_code)
+ "You are not allowed to push code to this project."
+ end
+ end
+
+ private
+
+ def protected_tag?(tag_name)
+ project.repository.tag_exists?(tag_name)
+ end
+
+ def forced_push?
+ Gitlab::ForcePushCheck.force_push?(@project, @oldrev, @newrev)
+ end
+
+ def matching_merge_request?
+ Checks::MatchingMergeRequest.new(@newrev, @branch_name, @project).match?
+ end
+
+ def branch_name(ref)
+ ref = @ref.to_s
+ if Gitlab::Git.branch_ref?(ref)
+ Gitlab::Git.ref_name(ref)
+ else
+ nil
+ end
+ end
+
+ def tag_name(ref)
+ ref = @ref.to_s
+ if Gitlab::Git.tag_ref?(ref)
+ Gitlab::Git.ref_name(ref)
+ else
+ nil
+ end
+ end
+ end
+ end
+end
diff --git a/lib/gitlab/git_access_wiki.rb b/lib/gitlab/git_access_wiki.rb
index 8672cbc0ec4..f71d3575909 100644
--- a/lib/gitlab/git_access_wiki.rb
+++ b/lib/gitlab/git_access_wiki.rb
@@ -1,7 +1,7 @@
module Gitlab
class GitAccessWiki < GitAccess
def change_access_check(change)
- if user.can?(:create_wiki, project)
+ if user_access.can_do_action?(:create_wiki)
build_status_object(true)
else
build_status_object(false, "You are not allowed to write to this project's wiki.")
diff --git a/lib/gitlab/user_access.rb b/lib/gitlab/user_access.rb
index d1b42c1f9b9..c0f85e9b3a8 100644
--- a/lib/gitlab/user_access.rb
+++ b/lib/gitlab/user_access.rb
@@ -1,7 +1,23 @@
module Gitlab
- module UserAccess
- def self.allowed?(user)
- return false if user.blocked?
+ class UserAccess
+ attr_reader :user, :project
+
+ def initialize(user, project: nil)
+ @user = user
+ @project = project
+ end
+
+ def can_do_action?(action)
+ @permission_cache ||= {}
+ @permission_cache[action] ||= user.can?(action, project)
+ end
+
+ def cannot_do_action?(action)
+ !can_do_action?(action)
+ end
+
+ def allowed?
+ return false if user.blank? || user.blocked?
if user.requires_ldap_check? && user.try_obtain_ldap_lease
return false unless Gitlab::LDAP::Access.allowed?(user)
@@ -9,5 +25,31 @@ module Gitlab
true
end
+
+ def can_push_to_branch?(ref)
+ return false unless user
+
+ if project.protected_branch?(ref) && !project.developers_can_push_to_protected_branch?(ref)
+ user.can?(:push_code_to_protected_branches, project)
+ else
+ user.can?(:push_code, project)
+ end
+ end
+
+ def can_merge_to_branch?(ref)
+ return false unless user
+
+ if project.protected_branch?(ref) && !project.developers_can_merge_to_protected_branch?(ref)
+ user.can?(:push_code_to_protected_branches, project)
+ else
+ user.can?(:push_code, project)
+ end
+ end
+
+ def can_read_project?
+ return false unless user
+
+ user.can?(:read_project, project)
+ end
end
end
diff --git a/spec/lib/gitlab/git_access_spec.rb b/spec/lib/gitlab/git_access_spec.rb
index 50cf2f1e93f..059d8a2c355 100644
--- a/spec/lib/gitlab/git_access_spec.rb
+++ b/spec/lib/gitlab/git_access_spec.rb
@@ -6,88 +6,6 @@ describe Gitlab::GitAccess, lib: true do
let(:user) { create(:user) }
let(:actor) { 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]
- expect(access.can_push_to_branch?("random_branch")).to be_truthy
- end
-
- it "returns true if user is a developer" do
- project.team << [user, :developer]
- expect(access.can_push_to_branch?("random_branch")).to be_truthy
- end
-
- it "returns false if user is a reporter" do
- project.team << [user, :reporter]
- expect(access.can_push_to_branch?("random_branch")).to be_falsey
- 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]
- expect(access.can_push_to_branch?(@branch.name)).to be_truthy
- end
-
- it "returns false if user is a developer" do
- project.team << [user, :developer]
- expect(access.can_push_to_branch?(@branch.name)).to be_falsey
- end
-
- it "returns false if user is a reporter" do
- project.team << [user, :reporter]
- expect(access.can_push_to_branch?(@branch.name)).to be_falsey
- 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]
- expect(access.can_push_to_branch?(@branch.name)).to be_truthy
- end
-
- it "returns true if user is a developer" do
- project.team << [user, :developer]
- expect(access.can_push_to_branch?(@branch.name)).to be_truthy
- end
-
- it "returns false if user is a reporter" do
- project.team << [user, :reporter]
- expect(access.can_push_to_branch?(@branch.name)).to be_falsey
- end
- end
-
- describe 'merge to protected branch if allowed for developers' do
- before do
- @branch = create :protected_branch, project: project, developers_can_merge: true
- end
-
- it "returns true if user is a master" do
- project.team << [user, :master]
- expect(access.can_merge_to_branch?(@branch.name)).to be_truthy
- end
-
- it "returns true if user is a developer" do
- project.team << [user, :developer]
- expect(access.can_merge_to_branch?(@branch.name)).to be_truthy
- end
-
- it "returns false if user is a reporter" do
- project.team << [user, :reporter]
- expect(access.can_merge_to_branch?(@branch.name)).to be_falsey
- end
- end
- end
-
describe '#check with single protocols allowed' do
def disable_protocol(protocol)
settings = ::ApplicationSetting.create_from_defaults
diff --git a/spec/lib/gitlab/user_access_spec.rb b/spec/lib/gitlab/user_access_spec.rb
new file mode 100644
index 00000000000..c77b746cc88
--- /dev/null
+++ b/spec/lib/gitlab/user_access_spec.rb
@@ -0,0 +1,90 @@
+require 'spec_helper'
+
+describe Gitlab::UserAccess, lib: true do
+ let(:access) { Gitlab::UserAccess.new(user, project: project) }
+ 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]
+ expect(access.can_push_to_branch?("random_branch")).to be_truthy
+ end
+
+ it "returns true if user is a developer" do
+ project.team << [user, :developer]
+ expect(access.can_push_to_branch?("random_branch")).to be_truthy
+ end
+
+ it "returns false if user is a reporter" do
+ project.team << [user, :reporter]
+ expect(access.can_push_to_branch?("random_branch")).to be_falsey
+ 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]
+ expect(access.can_push_to_branch?(@branch.name)).to be_truthy
+ end
+
+ it "returns false if user is a developer" do
+ project.team << [user, :developer]
+ expect(access.can_push_to_branch?(@branch.name)).to be_falsey
+ end
+
+ it "returns false if user is a reporter" do
+ project.team << [user, :reporter]
+ expect(access.can_push_to_branch?(@branch.name)).to be_falsey
+ 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]
+ expect(access.can_push_to_branch?(@branch.name)).to be_truthy
+ end
+
+ it "returns true if user is a developer" do
+ project.team << [user, :developer]
+ expect(access.can_push_to_branch?(@branch.name)).to be_truthy
+ end
+
+ it "returns false if user is a reporter" do
+ project.team << [user, :reporter]
+ expect(access.can_push_to_branch?(@branch.name)).to be_falsey
+ end
+ end
+
+ describe 'merge to protected branch if allowed for developers' do
+ before do
+ @branch = create :protected_branch, project: project, developers_can_merge: true
+ end
+
+ it "returns true if user is a master" do
+ project.team << [user, :master]
+ expect(access.can_merge_to_branch?(@branch.name)).to be_truthy
+ end
+
+ it "returns true if user is a developer" do
+ project.team << [user, :developer]
+ expect(access.can_merge_to_branch?(@branch.name)).to be_truthy
+ end
+
+ it "returns false if user is a reporter" do
+ project.team << [user, :reporter]
+ expect(access.can_merge_to_branch?(@branch.name)).to be_falsey
+ end
+ end
+
+ end
+end
diff --git a/spec/requests/api/api_helpers_spec.rb b/spec/requests/api/api_helpers_spec.rb
index 83025953889..3d5c19aeff3 100644
--- a/spec/requests/api/api_helpers_spec.rb
+++ b/spec/requests/api/api_helpers_spec.rb
@@ -49,7 +49,7 @@ describe API::Helpers, api: true do
it "should return nil for a user without access" do
env[API::Helpers::PRIVATE_TOKEN_HEADER] = user.private_token
- allow(Gitlab::UserAccess).to receive(:allowed?).and_return(false)
+ allow_any_instance_of(Gitlab::UserAccess).to receive(:allowed?).and_return(false)
expect(current_user).to be_nil
end
@@ -73,7 +73,7 @@ describe API::Helpers, api: true do
it "should return nil for a user without access" do
env[API::Helpers::PRIVATE_TOKEN_HEADER] = personal_access_token.token
- allow(Gitlab::UserAccess).to receive(:allowed?).and_return(false)
+ allow_any_instance_of(Gitlab::UserAccess).to receive(:allowed?).and_return(false)
expect(current_user).to be_nil
end
View Lorry Controller Status