diff --git a/GITLAB_SHELL_VERSION b/GITLAB_SHELL_VERSION index 090ea9dad19..9b9a244206f 100644 --- a/GITLAB_SHELL_VERSION +++ b/GITLAB_SHELL_VERSION @@ -1 +1 @@ -6.0.3 +6.0.2 diff --git a/lib/gitlab/git_access.rb b/lib/gitlab/git_access.rb index bc1e83f77b2..8ec3386184a 100644 --- a/lib/gitlab/git_access.rb +++ b/lib/gitlab/git_access.rb @@ -12,8 +12,9 @@ module Gitlab ERROR_MESSAGES = { upload: 'You are not allowed to upload code for this project.', download: 'You are not allowed to download code from this project.', - deploy_key_upload: - 'This deploy key does not have write access to this project.', + auth_upload: 'You are not allowed to upload code.', + 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.', project_not_found: 'The project you were looking for could not be found.', account_blocked: 'Your account has been blocked.', @@ -44,6 +45,7 @@ module Gitlab check_protocol! check_valid_actor! check_active_user! + check_authentication_abilities!(cmd) check_command_disabled!(cmd) check_command_existence!(cmd) check_db_accessibility!(cmd) @@ -104,6 +106,19 @@ module Gitlab 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! if project.blank? || !can_read_project? raise NotFoundError, ERROR_MESSAGES[:project_not_found] @@ -205,31 +220,21 @@ module Gitlab end 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 - check_user_push_access! + # User access is verified in check_change_access! else raise UnauthorizedError, ERROR_MESSAGES[:upload] end + return if changes.blank? # Allow access this is needed for EE. + 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) - return if changes.blank? # Allow access. - changes_list = Gitlab::ChangesList.new(changes) # Iterate over all changes to find if user allowed all of them to be applied diff --git a/spec/lib/gitlab/git_access_spec.rb b/spec/lib/gitlab/git_access_spec.rb index cc48373a0c0..3c3697e7aa9 100644 --- a/spec/lib/gitlab/git_access_spec.rb +++ b/spec/lib/gitlab/git_access_spec.rb @@ -119,7 +119,7 @@ describe Gitlab::GitAccess do end 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 @@ -327,7 +327,7 @@ describe Gitlab::GitAccess do let(:authentication_abilities) { [] } 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 context 'when authentication abilities include download code' do @@ -351,7 +351,7 @@ describe Gitlab::GitAccess do let(:authentication_abilities) { [] } 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 context 'when authentication abilities include push code' do @@ -852,26 +852,26 @@ describe Gitlab::GitAccess do project.add_reporter(user) 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 context 'when unauthorized' do context 'to public project' do 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 context 'to internal project' do 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 context 'to private project' do 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 diff --git a/spec/requests/git_http_spec.rb b/spec/requests/git_http_spec.rb index 8be94459c2e..2e2dccdafad 100644 --- a/spec/requests/git_http_spec.rb +++ b/spec/requests/git_http_spec.rb @@ -620,7 +620,7 @@ describe 'Git HTTP requests' do push_get(path, env) 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 # We are "authenticated" as CI using a valid token here. But we are @@ -660,7 +660,7 @@ describe 'Git HTTP requests' do push_get path, env 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