summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
Diffstat
-rw-r--r--lib/gitlab/git_access.rb19
-rw-r--r--spec/lib/gitlab/git_access_spec.rb249
2 files changed, 126 insertions, 142 deletions
diff --git a/lib/gitlab/git_access.rb b/lib/gitlab/git_access.rb
index 0b62911958d..3e8b83c0f90 100644
--- a/lib/gitlab/git_access.rb
+++ b/lib/gitlab/git_access.rb
@@ -4,6 +4,7 @@ module Gitlab
class GitAccess
UnauthorizedError = Class.new(StandardError)
NotFoundError = Class.new(StandardError)
+ ProjectMovedError = Class.new(NotFoundError)
ERROR_MESSAGES = {
upload: 'You are not allowed to upload code for this project.',
@@ -90,18 +91,18 @@ module Gitlab
end
def check_project_moved!
- if redirected_path
- url = protocol == 'ssh' ? project.ssh_url_to_repo : project.http_url_to_repo
- message = <<-MESSAGE.strip_heredoc
- Project '#{redirected_path}' was moved to '#{project.full_path}'.
+ return unless redirected_path
- Please update your Git remote and try again:
+ url = protocol == 'ssh' ? project.ssh_url_to_repo : project.http_url_to_repo
+ message = <<-MESSAGE.strip_heredoc
+ Project '#{redirected_path}' was moved to '#{project.full_path}'.
- git remote set-url origin #{url}
- MESSAGE
+ Please update your Git remote and try again:
- raise NotFoundError, message
- end
+ git remote set-url origin #{url}
+ MESSAGE
+
+ raise ProjectMovedError, message
end
def check_command_disabled!(cmd)
diff --git a/spec/lib/gitlab/git_access_spec.rb b/spec/lib/gitlab/git_access_spec.rb
index 2d6ea37d0ac..80dc49e99cb 100644
--- a/spec/lib/gitlab/git_access_spec.rb
+++ b/spec/lib/gitlab/git_access_spec.rb
@@ -1,21 +1,17 @@
require 'spec_helper'
describe Gitlab::GitAccess do
- let(:pull_access_check) { access.check('git-upload-pack', '_any') }
- let(:push_access_check) { access.check('git-receive-pack', '_any') }
- let(:access) { described_class.new(actor, project, protocol, authentication_abilities: authentication_abilities, redirected_path: redirected_path) }
- let(:project) { create(:project, :repository) }
- let(:user) { create(:user) }
+ set(:user) { create(:user) }
+
let(:actor) { user }
+ let(:project) { create(:project, :repository) }
let(:protocol) { 'ssh' }
+ let(:authentication_abilities) { %i[read_project download_code push_code] }
let(:redirected_path) { nil }
- let(:authentication_abilities) do
- [
- :read_project,
- :download_code,
- :push_code
- ]
- end
+
+ let(:access) { described_class.new(actor, project, protocol, authentication_abilities: authentication_abilities, redirected_path: redirected_path) }
+ let(:push_access_check) { access.check('git-receive-pack', '_any') }
+ let(:pull_access_check) { access.check('git-upload-pack', '_any') }
describe '#check with single protocols allowed' do
def disable_protocol(protocol)
@@ -27,12 +23,11 @@ describe Gitlab::GitAccess do
disable_protocol('ssh')
end
- it 'blocks ssh git push' do
- expect { push_access_check }.to raise_unauthorized('Git access over SSH is not allowed')
- end
-
- it 'blocks ssh git pull' do
- expect { pull_access_check }.to raise_unauthorized('Git access over SSH is not allowed')
+ it 'blocks ssh git push and pull' do
+ aggregate_failures do
+ expect { push_access_check }.to raise_unauthorized('Git access over SSH is not allowed')
+ expect { pull_access_check }.to raise_unauthorized('Git access over SSH is not allowed')
+ end
end
end
@@ -43,12 +38,11 @@ describe Gitlab::GitAccess do
disable_protocol('http')
end
- it 'blocks http push' do
- expect { push_access_check }.to raise_unauthorized('Git access over HTTP is not allowed')
- end
-
- it 'blocks http git pull' do
- expect { pull_access_check }.to raise_unauthorized('Git access over HTTP is not allowed')
+ it 'blocks http push and pull' do
+ aggregate_failures do
+ expect { push_access_check }.to raise_unauthorized('Git access over HTTP is not allowed')
+ expect { pull_access_check }.to raise_unauthorized('Git access over HTTP is not allowed')
+ end
end
end
end
@@ -65,22 +59,20 @@ describe Gitlab::GitAccess do
deploy_key.projects << project
end
- it 'allows pull access' do
- expect { pull_access_check }.not_to raise_error
- end
-
- it 'allows push access' do
- expect { push_access_check }.not_to raise_error
+ it 'allows push and pull access' do
+ aggregate_failures do
+ expect { push_access_check }.not_to raise_error
+ expect { pull_access_check }.not_to raise_error
+ end
end
end
context 'when the Deploykey does not have access to the project' do
- it 'blocks pulls with "not found"' do
- expect { pull_access_check }.to raise_not_found('The project you were looking for could not be found.')
- end
-
- it 'blocks pushes with "not found"' do
- expect { push_access_check }.to raise_not_found('The project you were looking for could not be found.')
+ it 'blocks push and pull with "not found"' do
+ aggregate_failures do
+ expect { push_access_check }.to raise_not_found
+ expect { pull_access_check }.to raise_not_found
+ end
end
end
end
@@ -88,25 +80,23 @@ describe Gitlab::GitAccess do
context 'when actor is a User' do
context 'when the User can read the project' do
before do
- project.team << [user, :master]
+ project.add_master(user)
end
- it 'allows pull access' do
- expect { pull_access_check }.not_to raise_error
- end
-
- it 'allows push access' do
- expect { push_access_check }.not_to raise_error
+ it 'allows push and pull access' do
+ aggregate_failures do
+ expect { pull_access_check }.not_to raise_error
+ expect { push_access_check }.not_to raise_error
+ end
end
end
context 'when the User cannot read the project' do
- it 'blocks pulls with "not found"' do
- expect { pull_access_check }.to raise_not_found('The project you were looking for could not be found.')
- end
-
- it 'blocks pushes with "not found"' do
- expect { push_access_check }.to raise_not_found('The project you were looking for could not be found.')
+ it 'blocks push and pull with "not found"' do
+ aggregate_failures do
+ expect { push_access_check }.to raise_not_found
+ expect { pull_access_check }.to raise_not_found
+ end
end
end
end
@@ -121,7 +111,7 @@ describe Gitlab::GitAccess do
end
it 'does not block pushes with "not found"' do
- expect { push_access_check }.to raise_unauthorized('You are not allowed to upload code for this project.')
+ expect { push_access_check }.to raise_unauthorized(described_class::ERROR_MESSAGES[:upload])
end
end
end
@@ -137,17 +127,17 @@ describe Gitlab::GitAccess do
end
it 'does not block pushes with "not found"' do
- expect { push_access_check }.to raise_unauthorized('You are not allowed to upload code for this project.')
+ expect { push_access_check }.to raise_unauthorized(described_class::ERROR_MESSAGES[:upload])
end
end
context 'when guests cannot read the project' do
it 'blocks pulls with "not found"' do
- expect { pull_access_check }.to raise_not_found('The project you were looking for could not be found.')
+ expect { pull_access_check }.to raise_not_found
end
it 'blocks pushes with "not found"' do
- expect { push_access_check }.to raise_not_found('The project you were looking for could not be found.')
+ expect { push_access_check }.to raise_not_found
end
end
end
@@ -156,48 +146,50 @@ describe Gitlab::GitAccess do
context 'when the project is nil' do
let(:project) { nil }
- it 'blocks any command with "not found"' do
- expect { pull_access_check }.to raise_not_found('The project you were looking for could not be found.')
- expect { push_access_check }.to raise_not_found('The project you were looking for could not be found.')
+ it 'blocks push and pull with "not found"' do
+ aggregate_failures do
+ expect { pull_access_check }.to raise_not_found
+ expect { push_access_check }.to raise_not_found
+ end
end
end
end
describe '#check_project_moved!' do
before do
- project.team << [user, :master]
+ project.add_master(user)
end
context 'when a redirect was not followed to find the project' do
- context 'pull code' do
- it { expect { pull_access_check }.not_to raise_error }
- end
-
- context 'push code' do
- it { expect { push_access_check }.not_to raise_error }
+ it 'allows push and pull access' do
+ aggregate_failures do
+ expect { push_access_check }.not_to raise_error
+ expect { pull_access_check }.not_to raise_error
+ end
end
end
context 'when a redirect was followed to find the project' do
let(:redirected_path) { 'some/other-path' }
- context 'pull code' do
- it { expect { pull_access_check }.to raise_not_found(/Project '#{redirected_path}' was moved to '#{project.full_path}'/) }
- it { expect { pull_access_check }.to raise_not_found(/git remote set-url origin #{project.ssh_url_to_repo}/) }
+ it 'blocks push and pull access' do
+ aggregate_failures do
+ expect { push_access_check }.to raise_error(described_class::ProjectMovedError, /Project '#{redirected_path}' was moved to '#{project.full_path}'/)
+ expect { push_access_check }.to raise_error(described_class::ProjectMovedError, /git remote set-url origin #{project.ssh_url_to_repo}/)
- context 'http protocol' do
- let(:protocol) { 'http' }
- it { expect { pull_access_check }.to raise_not_found(/git remote set-url origin #{project.http_url_to_repo}/) }
+ expect { pull_access_check }.to raise_error(described_class::ProjectMovedError, /Project '#{redirected_path}' was moved to '#{project.full_path}'/)
+ expect { pull_access_check }.to raise_error(described_class::ProjectMovedError, /git remote set-url origin #{project.ssh_url_to_repo}/)
end
end
- context 'push code' do
- it { expect { push_access_check }.to raise_not_found(/Project '#{redirected_path}' was moved to '#{project.full_path}'/) }
- it { expect { push_access_check }.to raise_not_found(/git remote set-url origin #{project.ssh_url_to_repo}/) }
+ context 'http protocol' do
+ let(:protocol) { 'http' }
- context 'http protocol' do
- let(:protocol) { 'http' }
- it { expect { push_access_check }.to raise_not_found(/git remote set-url origin #{project.http_url_to_repo}/) }
+ it 'includes the path to the project using HTTP' do
+ aggregate_failures do
+ expect { push_access_check }.to raise_error(described_class::ProjectMovedError, /git remote set-url origin #{project.http_url_to_repo}/)
+ expect { pull_access_check }.to raise_error(described_class::ProjectMovedError, /git remote set-url origin #{project.http_url_to_repo}/)
+ end
end
end
end
@@ -242,40 +234,28 @@ describe Gitlab::GitAccess do
end
describe '#check_download_access!' do
- describe 'master permissions' do
- before do
- project.team << [user, :master]
- end
+ it 'allows masters to pull' do
+ project.add_master(user)
- context 'pull code' do
- it { expect { pull_access_check }.not_to raise_error }
- end
+ expect { pull_access_check }.not_to raise_error
end
- describe 'guest permissions' do
- before do
- project.team << [user, :guest]
- end
+ it 'disallows guests to pull' do
+ project.add_guest(user)
- context 'pull code' do
- it { expect { pull_access_check }.to raise_unauthorized('You are not allowed to download code from this project.') }
- end
+ expect { pull_access_check }.to raise_unauthorized(described_class::ERROR_MESSAGES[:download])
end
- describe 'blocked user' do
- before do
- project.team << [user, :master]
- user.block
- end
+ it 'disallows blocked users to pull' do
+ project.add_master(user)
+ user.block
- context 'pull code' do
- it { expect { pull_access_check }.to raise_unauthorized('Your account has been blocked.') }
- end
+ expect { pull_access_check }.to raise_unauthorized('Your account has been blocked.')
end
describe 'without access to project' do
context 'pull code' do
- it { expect { pull_access_check }.to raise_not_found('The project you were looking for could not be found.') }
+ it { expect { pull_access_check }.to raise_not_found }
end
context 'when project is public' do
@@ -292,7 +272,7 @@ describe Gitlab::GitAccess do
it 'does not give access to download code' do
public_project.project_feature.update_attribute(:repository_access_level, ProjectFeature::DISABLED)
- expect { pull_access_check }.to raise_unauthorized('You are not allowed to download code from this project.')
+ expect { pull_access_check }.to raise_unauthorized(described_class::ERROR_MESSAGES[:download])
end
end
end
@@ -321,13 +301,13 @@ describe Gitlab::GitAccess do
context 'from internal project' do
let(:project) { create(:project, :internal, :repository) }
- it { expect { pull_access_check }.to raise_not_found('The project you were looking for could not be found.') }
+ it { expect { pull_access_check }.to raise_not_found }
end
context 'from private project' do
let(:project) { create(:project, :private, :repository) }
- it { expect { pull_access_check }.to raise_not_found('The project you were looking for could not be found.') }
+ it { expect { pull_access_check }.to raise_not_found }
end
end
end
@@ -369,7 +349,7 @@ describe Gitlab::GitAccess do
context 'when is not member of the project' do
context 'pull code' do
- it { expect { pull_access_check }.to raise_unauthorized('You are not allowed to download code from this project.') }
+ it { expect { pull_access_check }.to raise_unauthorized(described_class::ERROR_MESSAGES[:download]) }
end
end
end
@@ -428,28 +408,30 @@ describe Gitlab::GitAccess do
end
end
- # Run permission checks for a user
def self.run_permission_checks(permissions_matrix)
- permissions_matrix.keys.each do |role|
- describe "#{role} access" do
- before do
- if role == :admin
- user.update_attribute(:admin, true)
- else
- project.team << [user, role]
- end
+ permissions_matrix.each_pair do |role, matrix|
+ # Run through the entire matrix for this role in one test to avoid
+ # repeated setup.
+ #
+ # Expectations are given a custom failure message proc so that it's
+ # easier to identify which check(s) failed.
+ it "has the correct permissions for #{role}s" do
+ if role == :admin
+ user.update_attribute(:admin, true)
+ else
+ project.team << [user, role]
end
- permissions_matrix[role].each do |action, allowed|
- context action.to_s do
- subject { access.send(:check_push_access!, changes[action]) }
+ aggregate_failures do
+ matrix.each do |action, allowed|
+ check = -> { access.send(:check_push_access!, changes[action]) }
- it do
- if allowed
- expect { subject }.not_to raise_error
- else
- expect { subject }.to raise_error(Gitlab::GitAccess::UnauthorizedError)
- end
+ if allowed
+ expect(&check).not_to raise_error,
+ -> { "expected #{action} to be allowed" }
+ else
+ expect(&check).to raise_error(Gitlab::GitAccess::UnauthorizedError),
+ -> { "expected #{action} to be disallowed" }
end
end
end
@@ -588,26 +570,26 @@ describe Gitlab::GitAccess do
project.team << [user, :reporter]
end
- it { expect { push_access_check }.to raise_unauthorized('You are not allowed to upload code for this project.') }
+ it { expect { push_access_check }.to raise_unauthorized(described_class::ERROR_MESSAGES[: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('You are not allowed to upload code for this project.') }
+ it { expect { push_access_check }.to raise_unauthorized(described_class::ERROR_MESSAGES[:upload]) }
end
context 'to internal project' do
let(:project) { create(:project, :internal, :repository) }
- it { expect { push_access_check }.to raise_unauthorized('You are not allowed to upload code for this project.') }
+ it { expect { push_access_check }.to raise_unauthorized(described_class::ERROR_MESSAGES[:upload]) }
end
context 'to private project' do
let(:project) { create(:project, :private, :repository) }
- it { expect { push_access_check }.to raise_not_found('The project you were looking for could not be found.') }
+ it { expect { push_access_check }.to raise_not_found }
end
end
end
@@ -631,19 +613,19 @@ describe Gitlab::GitAccess do
context 'to public project' do
let(:project) { create(:project, :public, :repository) }
- it { expect { push_access_check }.to raise_unauthorized('This deploy key does not have write access to this project.') }
+ it { expect { push_access_check }.to raise_unauthorized(described_class::ERROR_MESSAGES[:deploy_key_upload]) }
end
context 'to internal project' do
let(:project) { create(:project, :internal, :repository) }
- it { expect { push_access_check }.to raise_not_found('The project you were looking for could not be found.') }
+ it { expect { push_access_check }.to raise_not_found }
end
context 'to private project' do
let(:project) { create(:project, :private, :repository) }
- it { expect { push_access_check }.to raise_not_found('The project you were looking for could not be found.') }
+ it { expect { push_access_check }.to raise_not_found }
end
end
end
@@ -656,26 +638,26 @@ describe Gitlab::GitAccess do
key.projects << project
end
- it { expect { push_access_check }.to raise_unauthorized('This deploy key does not have write access to this project.') }
+ it { expect { push_access_check }.to raise_unauthorized(described_class::ERROR_MESSAGES[:deploy_key_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('This deploy key does not have write access to this project.') }
+ it { expect { push_access_check }.to raise_unauthorized(described_class::ERROR_MESSAGES[:deploy_key_upload]) }
end
context 'to internal project' do
let(:project) { create(:project, :internal, :repository) }
- it { expect { push_access_check }.to raise_not_found('The project you were looking for could not be found.') }
+ it { expect { push_access_check }.to raise_not_found }
end
context 'to private project' do
let(:project) { create(:project, :private, :repository) }
- it { expect { push_access_check }.to raise_not_found('The project you were looking for could not be found.') }
+ it { expect { push_access_check }.to raise_not_found }
end
end
end
@@ -687,8 +669,9 @@ describe Gitlab::GitAccess do
raise_error(Gitlab::GitAccess::UnauthorizedError, message)
end
- def raise_not_found(message)
- raise_error(Gitlab::GitAccess::NotFoundError, message)
+ def raise_not_found
+ raise_error(Gitlab::GitAccess::NotFoundError,
+ Gitlab::GitAccess::ERROR_MESSAGES[:project_not_found])
end
def build_authentication_abilities
View Lorry Controller Status