diff --git a/app/models/user.rb b/app/models/user.rb index 40130b8b25c..5a2b232c4ed 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -501,10 +501,6 @@ class User < ActiveRecord::Base several_namespaces? || admin end - def has_access_to?(project) - can?(:read_project, project) - end - def can?(action, subject) Ability.allowed?(self, action, subject) end diff --git a/lib/gitlab/checks/change_access.rb b/lib/gitlab/checks/change_access.rb index cb1065223d4..6b6a86ffde9 100644 --- a/lib/gitlab/checks/change_access.rb +++ b/lib/gitlab/checks/change_access.rb @@ -1,13 +1,15 @@ module Gitlab module Checks class ChangeAccess - attr_reader :user_access, :project + attr_reader :user_access, :project, :skip_authorization - def initialize(change, user_access:, project:) + def initialize( + change, user_access:, project:, skip_authorization: false) @oldrev, @newrev, @ref = change.values_at(:oldrev, :newrev, :ref) @branch_name = Gitlab::Git.branch_name(@ref) @user_access = user_access @project = project + @skip_authorization = skip_authorization end def exec @@ -23,6 +25,7 @@ module Gitlab protected def protected_branch_checks + return if skip_authorization return unless @branch_name return unless project.protected_branch?(@branch_name) @@ -48,6 +51,8 @@ module Gitlab end def tag_checks + return if skip_authorization + tag_ref = Gitlab::Git.tag_name(@ref) if tag_ref && protected_tag?(tag_ref) && user_access.cannot_do_action?(:admin_project) @@ -56,6 +61,8 @@ module Gitlab end def push_checks + return if skip_authorization + if user_access.cannot_do_action?(:push_code) "You are not allowed to push code to this project." end diff --git a/lib/gitlab/git_access.rb b/lib/gitlab/git_access.rb index a7ad944e79e..6be0ab08a1f 100644 --- a/lib/gitlab/git_access.rb +++ b/lib/gitlab/git_access.rb @@ -27,7 +27,7 @@ module Gitlab def check(cmd, changes) check_protocol! - check_active_user! + check_active_user! unless deploy_key? check_project_accessibility! check_command_existence!(cmd) @@ -44,9 +44,13 @@ module Gitlab end def download_access_check - if user + if deploy_key + true + elsif user user_download_access_check - elsif deploy_key.nil? && !Guest.can?(:download_code, project) + elsif Guest.can?(:download_code, project) + true + else raise UnauthorizedError, ERROR_MESSAGES[:download] end end @@ -148,7 +152,10 @@ module Gitlab def check_single_change_access(change) Checks::ChangeAccess.new( - change, user_access: user_access, project: project).exec + change, + user_access: user_access, + project: project, + skip_authorization: deploy_key?).exec end def matching_merge_request?(newrev, branch_name) @@ -156,17 +163,19 @@ module Gitlab end def deploy_key - actor if actor.is_a?(DeployKey) + actor if deploy_key? + end + + def deploy_key? + actor.is_a?(DeployKey) end def can_read_project? if deploy_key - project.public? || deploy_key.has_access_to?(project) + deploy_key.has_access_to?(project) elsif user - user_access.can_read_project? - else - Guest.can?(:read_project, project) - end + user.can?(:read_project, project) + end || Guest.can?(:read_project, project) end protected diff --git a/spec/lib/gitlab/git_access_spec.rb b/spec/lib/gitlab/git_access_spec.rb index 9c19ea2d862..9810d79f8b4 100644 --- a/spec/lib/gitlab/git_access_spec.rb +++ b/spec/lib/gitlab/git_access_spec.rb @@ -115,10 +115,6 @@ describe Gitlab::GitAccess, lib: true do let(:key) { create(:deploy_key, user: user) } let(:actor) { key } - before do - project.team << [user, :master] - end - context 'pull code' do context 'when project is authorized' do before { key.projects << project } @@ -387,16 +383,6 @@ describe Gitlab::GitAccess, lib: true do end end - describe 'full authentication abilities' do - let(:authentication_abilities) { full_authentication_abilities } - - it_behaves_like 'pushing code', :to do - def authorize - project.team << [user, :developer] - end - end - end - describe 'build authentication abilities' do let(:authentication_abilities) { build_authentication_abilities } @@ -411,10 +397,6 @@ describe Gitlab::GitAccess, lib: true do let(:key) { create(:deploy_key, user: user, can_push: can_push) } let(:actor) { key } - before do - project.team << [user, :master] - end - context 'when deploy_key can push' do let(:can_push) { true }