Commit 8b4280cb authored by Tiago Botelho's avatar Tiago Botelho

Check ability ability before proceeding with project specific checks

parent 1e56b3f4
...@@ -12,8 +12,9 @@ module Gitlab ...@@ -12,8 +12,9 @@ module Gitlab
ERROR_MESSAGES = { ERROR_MESSAGES = {
upload: 'You are not allowed to upload code for this project.', upload: 'You are not allowed to upload code for this project.',
download: 'You are not allowed to download code from this project.', download: 'You are not allowed to download code from this project.',
deploy_key_upload: auth_upload: 'You are not allowed to upload code.',
'This deploy key does not have write access to this project.', auth_download: 'You are not allowed to download code.',
deploy_key_upload: 'This deploy key does not have write access to this project.',
no_repo: 'A repository for this project does not exist yet.', no_repo: 'A repository for this project does not exist yet.',
project_not_found: 'The project you were looking for could not be found.', project_not_found: 'The project you were looking for could not be found.',
account_blocked: 'Your account has been blocked.', account_blocked: 'Your account has been blocked.',
...@@ -44,6 +45,7 @@ module Gitlab ...@@ -44,6 +45,7 @@ module Gitlab
check_protocol! check_protocol!
check_valid_actor! check_valid_actor!
check_active_user! check_active_user!
check_authentication_abilities!(cmd)
check_command_disabled!(cmd) check_command_disabled!(cmd)
check_command_existence!(cmd) check_command_existence!(cmd)
check_db_accessibility!(cmd) check_db_accessibility!(cmd)
...@@ -104,6 +106,19 @@ module Gitlab ...@@ -104,6 +106,19 @@ module Gitlab
end end
end end
def check_authentication_abilities!(cmd)
case cmd
when *DOWNLOAD_COMMANDS
unless authentication_abilities.include?(:download_code) || authentication_abilities.include?(:build_download_code)
raise UnauthorizedError, ERROR_MESSAGES[:auth_download]
end
when *PUSH_COMMANDS
unless authentication_abilities.include?(:push_code)
raise UnauthorizedError, ERROR_MESSAGES[:auth_upload]
end
end
end
def check_project_accessibility! def check_project_accessibility!
if project.blank? || !can_read_project? if project.blank? || !can_read_project?
raise NotFoundError, ERROR_MESSAGES[:project_not_found] raise NotFoundError, ERROR_MESSAGES[:project_not_found]
...@@ -205,31 +220,21 @@ module Gitlab ...@@ -205,31 +220,21 @@ module Gitlab
end end
if deploy_key if deploy_key
check_deploy_key_push_access! unless deploy_key.can_push_to?(project)
raise UnauthorizedError, ERROR_MESSAGES[:deploy_key_upload]
end
elsif user elsif user
check_user_push_access! # User access is verified in check_change_access!
else else
raise UnauthorizedError, ERROR_MESSAGES[:upload] raise UnauthorizedError, ERROR_MESSAGES[:upload]
end end
check_change_access!(changes) return if changes.blank? # Allow access this is needed for EE.
end
def check_user_push_access! check_change_access!(changes)
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 end
def check_change_access!(changes) def check_change_access!(changes)
return if changes.blank? # Allow access.
changes_list = Gitlab::ChangesList.new(changes) changes_list = Gitlab::ChangesList.new(changes)
# Iterate over all changes to find if user allowed all of them to be applied # Iterate over all changes to find if user allowed all of them to be applied
......
...@@ -119,7 +119,7 @@ describe Gitlab::GitAccess do ...@@ -119,7 +119,7 @@ describe Gitlab::GitAccess do
end end
it 'does not block pushes with "not found"' do it 'does not block pushes with "not found"' do
expect { push_access_check }.to raise_unauthorized(described_class::ERROR_MESSAGES[:upload]) expect { push_access_check }.to raise_unauthorized(described_class::ERROR_MESSAGES[:auth_upload])
end end
end end
end end
...@@ -327,7 +327,7 @@ describe Gitlab::GitAccess do ...@@ -327,7 +327,7 @@ describe Gitlab::GitAccess do
let(:authentication_abilities) { [] } let(:authentication_abilities) { [] }
it 'raises unauthorized with download error' do it 'raises unauthorized with download error' do
expect { pull_access_check }.to raise_unauthorized(described_class::ERROR_MESSAGES[:download]) expect { pull_access_check }.to raise_unauthorized(described_class::ERROR_MESSAGES[:auth_download])
end end
context 'when authentication abilities include download code' do context 'when authentication abilities include download code' do
...@@ -351,7 +351,7 @@ describe Gitlab::GitAccess do ...@@ -351,7 +351,7 @@ describe Gitlab::GitAccess do
let(:authentication_abilities) { [] } let(:authentication_abilities) { [] }
it 'raises unauthorized with push error' do it 'raises unauthorized with push error' do
expect { push_access_check }.to raise_unauthorized(described_class::ERROR_MESSAGES[:upload]) expect { push_access_check }.to raise_unauthorized(described_class::ERROR_MESSAGES[:auth_upload])
end end
context 'when authentication abilities include push code' do context 'when authentication abilities include push code' do
...@@ -852,26 +852,26 @@ describe Gitlab::GitAccess do ...@@ -852,26 +852,26 @@ describe Gitlab::GitAccess do
project.add_reporter(user) project.add_reporter(user)
end end
it { expect { push_access_check }.to raise_unauthorized(described_class::ERROR_MESSAGES[:upload]) } it { expect { push_access_check }.to raise_unauthorized(described_class::ERROR_MESSAGES[:auth_upload]) }
end end
context 'when unauthorized' do context 'when unauthorized' do
context 'to public project' do context 'to public project' do
let(:project) { create(:project, :public, :repository) } let(:project) { create(:project, :public, :repository) }
it { expect { push_access_check }.to raise_unauthorized(described_class::ERROR_MESSAGES[:upload]) } it { expect { push_access_check }.to raise_unauthorized(described_class::ERROR_MESSAGES[:auth_upload]) }
end end
context 'to internal project' do context 'to internal project' do
let(:project) { create(:project, :internal, :repository) } let(:project) { create(:project, :internal, :repository) }
it { expect { push_access_check }.to raise_unauthorized(described_class::ERROR_MESSAGES[:upload]) } it { expect { push_access_check }.to raise_unauthorized(described_class::ERROR_MESSAGES[:auth_upload]) }
end end
context 'to private project' do context 'to private project' do
let(:project) { create(:project, :private, :repository) } let(:project) { create(:project, :private, :repository) }
it { expect { push_access_check }.to raise_not_found } it { expect { push_access_check }.to raise_unauthorized(described_class::ERROR_MESSAGES[:auth_upload]) }
end end
end end
end end
......
...@@ -620,7 +620,7 @@ describe 'Git HTTP requests' do ...@@ -620,7 +620,7 @@ describe 'Git HTTP requests' do
push_get(path, env) push_get(path, env)
expect(response).to have_gitlab_http_status(:forbidden) expect(response).to have_gitlab_http_status(:forbidden)
expect(response.body).to eq(git_access_error(:upload)) expect(response.body).to eq(git_access_error(:auth_upload))
end end
# We are "authenticated" as CI using a valid token here. But we are # We are "authenticated" as CI using a valid token here. But we are
...@@ -660,7 +660,7 @@ describe 'Git HTTP requests' do ...@@ -660,7 +660,7 @@ describe 'Git HTTP requests' do
push_get path, env push_get path, env
expect(response).to have_gitlab_http_status(:forbidden) expect(response).to have_gitlab_http_status(:forbidden)
expect(response.body).to eq(git_access_error(:upload)) expect(response.body).to eq(git_access_error(:auth_upload))
end end
end end
......
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