Commit 884f57c9 authored by Lin Jen-Shin's avatar Lin Jen-Shin

Use consistent names and move checks to the method,

and move those checks to be private. Feedback:

https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/7383#note_20285012

https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/7383#note_20285279
parent 0f0738e7
...@@ -29,16 +29,16 @@ module Gitlab ...@@ -29,16 +29,16 @@ module Gitlab
def check(cmd, changes) def check(cmd, changes)
check_protocol! check_protocol!
check_active_user! unless deploy_key? check_active_user!
check_project_accessibility! check_project_accessibility!
check_command_existence!(cmd) check_command_existence!(cmd)
check_repository_existence! check_repository_existence!
case cmd case cmd
when *DOWNLOAD_COMMANDS when *DOWNLOAD_COMMANDS
download_access_check unless deploy_key? check_download_access!
when *PUSH_COMMANDS when *PUSH_COMMANDS
push_access_check(changes) check_push_access!(changes)
end end
build_status_object(true) build_status_object(true)
...@@ -46,30 +46,6 @@ module Gitlab ...@@ -46,30 +46,6 @@ module Gitlab
build_status_object(false, ex.message) build_status_object(false, ex.message)
end end
def download_access_check
passed = user_can_download_code? ||
build_can_download_code? ||
guest_can_download_code?
unless passed
raise UnauthorizedError, ERROR_MESSAGES[:download]
end
end
def push_access_check(changes)
if deploy_key
deploy_key_push_access_check
elsif user
user_push_access_check
else
raise UnauthorizedError, ERROR_MESSAGES[:upload]
end
return if changes.blank? # Allow access.
check_change_access!(changes)
end
def guest_can_download_code? def guest_can_download_code?
Guest.can?(:download_code, project) Guest.can?(:download_code, project)
end end
...@@ -82,18 +58,6 @@ module Gitlab ...@@ -82,18 +58,6 @@ module Gitlab
authentication_abilities.include?(:build_download_code) && user_access.can_do_action?(:build_download_code) authentication_abilities.include?(:build_download_code) && user_access.can_do_action?(:build_download_code)
end end
def user_push_access_check
unless authentication_abilities.include?(:push_code)
raise UnauthorizedError, ERROR_MESSAGES[:upload]
end
end
def deploy_key_push_access_check
unless deploy_key.can_push_to?(project)
raise UnauthorizedError, ERROR_MESSAGES[:deploy_key_upload]
end
end
def protocol_allowed? def protocol_allowed?
Gitlab::ProtocolAccess.allowed?(protocol) Gitlab::ProtocolAccess.allowed?(protocol)
end end
...@@ -107,6 +71,8 @@ module Gitlab ...@@ -107,6 +71,8 @@ module Gitlab
end end
def check_active_user! def check_active_user!
return if deploy_key?
if user && !user_access.allowed? if user && !user_access.allowed?
raise UnauthorizedError, "Your account has been blocked." raise UnauthorizedError, "Your account has been blocked."
end end
...@@ -130,6 +96,44 @@ module Gitlab ...@@ -130,6 +96,44 @@ module Gitlab
end end
end end
def check_download_access!
return if deploy_key?
passed = user_can_download_code? ||
build_can_download_code? ||
guest_can_download_code?
unless passed
raise UnauthorizedError, ERROR_MESSAGES[:download]
end
end
def check_push_access!(changes)
if deploy_key
check_deploy_key_push_access!
elsif user
check_user_push_access!
else
raise UnauthorizedError, ERROR_MESSAGES[:upload]
end
return if changes.blank? # Allow access.
check_change_access!(changes)
end
def check_user_push_access!
unless authentication_abilities.include?(:push_code)
raise UnauthorizedError, ERROR_MESSAGES[:upload]
end
end
def check_deploy_key_push_access!
unless deploy_key.can_push_to?(project)
raise UnauthorizedError, ERROR_MESSAGES[:deploy_key_upload]
end
end
def check_change_access!(changes) def check_change_access!(changes)
changes_list = Gitlab::ChangesList.new(changes) changes_list = Gitlab::ChangesList.new(changes)
......
...@@ -50,7 +50,7 @@ describe Gitlab::GitAccess, lib: true do ...@@ -50,7 +50,7 @@ describe Gitlab::GitAccess, lib: true do
end end
end end
describe 'download_access_check' do describe '#check_download_access!' do
subject { access.check('git-upload-pack', '_any') } subject { access.check('git-upload-pack', '_any') }
describe 'master permissions' do describe 'master permissions' do
...@@ -183,7 +183,7 @@ describe Gitlab::GitAccess, lib: true do ...@@ -183,7 +183,7 @@ describe Gitlab::GitAccess, lib: true do
end end
end end
describe 'push_access_check' do describe '#check_push_access!' do
before { merge_into_protected_branch } before { merge_into_protected_branch }
let(:unprotected_branch) { FFaker::Internet.user_name } let(:unprotected_branch) { FFaker::Internet.user_name }
...@@ -231,7 +231,7 @@ describe Gitlab::GitAccess, lib: true do ...@@ -231,7 +231,7 @@ describe Gitlab::GitAccess, lib: true do
permissions_matrix[role].each do |action, allowed| permissions_matrix[role].each do |action, allowed|
context action do context action do
subject { access.push_access_check(changes[action]) } subject { access.send(:check_push_access!, changes[action]) }
it { expect(subject.allowed?).to allowed ? be_truthy : be_falsey } it { expect(subject.allowed?).to allowed ? be_truthy : be_falsey }
end end
end end
......
...@@ -27,7 +27,7 @@ describe Gitlab::GitAccessWiki, lib: true do ...@@ -27,7 +27,7 @@ describe Gitlab::GitAccessWiki, lib: true do
['6f6d7e7ed 570e7b2ab refs/heads/master'] ['6f6d7e7ed 570e7b2ab refs/heads/master']
end end
describe '#download_access_check' do describe '#access_check_download!' do
subject { access.check('git-upload-pack', '_any') } subject { access.check('git-upload-pack', '_any') }
before do before do
......
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