Commit bb40f4ab authored by Robert Speicher's avatar Robert Speicher

Greatly reduce test duration for git_access_spec

- Combine multiple `it` blocks into one where it made sense
- Use `set` for the top-level User record
- Refactor permission matrix tests
parent 11661c2a
require 'spec_helper' require 'spec_helper'
describe Gitlab::GitAccess do describe Gitlab::GitAccess do
let(:pull_access_check) { access.check('git-upload-pack', '_any') } set(:user) { create(:user) }
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) }
let(:actor) { user } let(:actor) { user }
let(:project) { create(:project, :repository) }
let(:protocol) { 'ssh' } let(:protocol) { 'ssh' }
let(:authentication_abilities) { %i[read_project download_code push_code] }
let(:redirected_path) { nil } let(:redirected_path) { nil }
let(:authentication_abilities) do
[ let(:access) { described_class.new(actor, project, protocol, authentication_abilities: authentication_abilities, redirected_path: redirected_path) }
:read_project, let(:push_access_check) { access.check('git-receive-pack', '_any') }
:download_code, let(:pull_access_check) { access.check('git-upload-pack', '_any') }
:push_code
]
end
describe '#check with single protocols allowed' do describe '#check with single protocols allowed' do
def disable_protocol(protocol) def disable_protocol(protocol)
...@@ -27,12 +23,11 @@ describe Gitlab::GitAccess do ...@@ -27,12 +23,11 @@ describe Gitlab::GitAccess do
disable_protocol('ssh') disable_protocol('ssh')
end end
it 'blocks ssh git push' do it 'blocks ssh git push and pull' do
expect { push_access_check }.to raise_unauthorized('Git access over SSH is not allowed') aggregate_failures do
end 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')
it 'blocks ssh git pull' do end
expect { pull_access_check }.to raise_unauthorized('Git access over SSH is not allowed')
end end
end end
...@@ -43,12 +38,11 @@ describe Gitlab::GitAccess do ...@@ -43,12 +38,11 @@ describe Gitlab::GitAccess do
disable_protocol('http') disable_protocol('http')
end end
it 'blocks http push' do it 'blocks http push and pull' do
expect { push_access_check }.to raise_unauthorized('Git access over HTTP is not allowed') aggregate_failures do
end 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')
it 'blocks http git pull' do end
expect { pull_access_check }.to raise_unauthorized('Git access over HTTP is not allowed')
end end
end end
end end
...@@ -65,22 +59,20 @@ describe Gitlab::GitAccess do ...@@ -65,22 +59,20 @@ describe Gitlab::GitAccess do
deploy_key.projects << project deploy_key.projects << project
end end
it 'allows pull access' do it 'allows push and pull access' do
expect { pull_access_check }.not_to raise_error aggregate_failures do
end expect { push_access_check }.not_to raise_error
expect { pull_access_check }.not_to raise_error
it 'allows push access' do end
expect { push_access_check }.not_to raise_error
end end
end end
context 'when the Deploykey does not have access to the project' do context 'when the Deploykey does not have access to the project' do
it 'blocks pulls with "not found"' do it 'blocks push and pull with "not found"' do
expect { pull_access_check }.to raise_not_found('The project you were looking for could not be found.') aggregate_failures do
end expect { push_access_check }.to raise_not_found('The project you were looking for could not be found.')
expect { pull_access_check }.to raise_not_found('The project you were looking for could not be found.')
it 'blocks pushes with "not found"' do end
expect { push_access_check }.to raise_not_found('The project you were looking for could not be found.')
end end
end end
end end
...@@ -88,25 +80,23 @@ describe Gitlab::GitAccess do ...@@ -88,25 +80,23 @@ describe Gitlab::GitAccess do
context 'when actor is a User' do context 'when actor is a User' do
context 'when the User can read the project' do context 'when the User can read the project' do
before do before do
project.team << [user, :master] project.add_master(user)
end end
it 'allows pull access' do it 'allows push and pull access' do
expect { pull_access_check }.not_to raise_error aggregate_failures do
end expect { pull_access_check }.not_to raise_error
expect { push_access_check }.not_to raise_error
it 'allows push access' do end
expect { push_access_check }.not_to raise_error
end end
end end
context 'when the User cannot read the project' do context 'when the User cannot read the project' do
it 'blocks pulls with "not found"' do it 'blocks push and pull with "not found"' do
expect { pull_access_check }.to raise_not_found('The project you were looking for could not be found.') aggregate_failures do
end expect { push_access_check }.to raise_not_found('The project you were looking for could not be found.')
expect { pull_access_check }.to raise_not_found('The project you were looking for could not be found.')
it 'blocks pushes with "not found"' do end
expect { push_access_check }.to raise_not_found('The project you were looking for could not be found.')
end end
end end
end end
...@@ -156,48 +146,50 @@ describe Gitlab::GitAccess do ...@@ -156,48 +146,50 @@ describe Gitlab::GitAccess do
context 'when the project is nil' do context 'when the project is nil' do
let(:project) { nil } let(:project) { nil }
it 'blocks any command with "not found"' do it 'blocks push and pull with "not found"' do
expect { pull_access_check }.to raise_not_found('The project you were looking for could not be found.') aggregate_failures do
expect { push_access_check }.to raise_not_found('The project you were looking for could not be found.') 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.')
end
end end
end end
end end
describe '#check_project_moved!' do describe '#check_project_moved!' do
before do before do
project.team << [user, :master] project.add_master(user)
end end
context 'when a redirect was not followed to find the project' do context 'when a redirect was not followed to find the project' do
context 'pull code' do it 'allows push and pull access' do
it { expect { pull_access_check }.not_to raise_error } aggregate_failures do
end expect { push_access_check }.not_to raise_error
expect { pull_access_check }.not_to raise_error
context 'push code' do end
it { expect { push_access_check }.not_to raise_error }
end end
end end
context 'when a redirect was followed to find the project' do context 'when a redirect was followed to find the project' do
let(:redirected_path) { 'some/other-path' } let(:redirected_path) { 'some/other-path' }
context 'pull code' do it 'blocks push and pull access' do
it { expect { pull_access_check }.to raise_not_found(/Project '#{redirected_path}' was moved to '#{project.full_path}'/) } aggregate_failures do
it { expect { pull_access_check }.to raise_not_found(/git remote set-url origin #{project.ssh_url_to_repo}/) } expect { push_access_check }.to raise_not_found(/Project '#{redirected_path}' was moved to '#{project.full_path}'/)
expect { push_access_check }.to raise_not_found(/git remote set-url origin #{project.ssh_url_to_repo}/)
context 'http protocol' do expect { pull_access_check }.to raise_not_found(/Project '#{redirected_path}' was moved to '#{project.full_path}'/)
let(:protocol) { 'http' } expect { pull_access_check }.to raise_not_found(/git remote set-url origin #{project.ssh_url_to_repo}/)
it { expect { pull_access_check }.to raise_not_found(/git remote set-url origin #{project.http_url_to_repo}/) }
end end
end end
context 'push code' do context 'http protocol' do
it { expect { push_access_check }.to raise_not_found(/Project '#{redirected_path}' was moved to '#{project.full_path}'/) } let(:protocol) { 'http' }
it { expect { push_access_check }.to raise_not_found(/git remote set-url origin #{project.ssh_url_to_repo}/) }
context 'http protocol' do it 'includes the path to the project using HTTP' do
let(:protocol) { 'http' } aggregate_failures do
it { expect { push_access_check }.to raise_not_found(/git remote set-url origin #{project.http_url_to_repo}/) } expect { push_access_check }.to raise_not_found(/git remote set-url origin #{project.http_url_to_repo}/)
expect { pull_access_check }.to raise_not_found(/git remote set-url origin #{project.http_url_to_repo}/)
end
end end
end end
end end
...@@ -242,35 +234,23 @@ describe Gitlab::GitAccess do ...@@ -242,35 +234,23 @@ describe Gitlab::GitAccess do
end end
describe '#check_download_access!' do describe '#check_download_access!' do
describe 'master permissions' do it 'allows masters to pull' do
before do project.add_master(user)
project.team << [user, :master]
end
context 'pull code' do expect { pull_access_check }.not_to raise_error
it { expect { pull_access_check }.not_to raise_error }
end
end end
describe 'guest permissions' do it 'disallows guests to pull' do
before do project.add_guest(user)
project.team << [user, :guest]
end
context 'pull code' do 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('You are not allowed to download code from this project.') }
end
end end
describe 'blocked user' do it 'disallows blocked users to pull' do
before do project.add_master(user)
project.team << [user, :master] user.block
user.block
end
context 'pull code' do expect { pull_access_check }.to raise_unauthorized('Your account has been blocked.')
it { expect { pull_access_check }.to raise_unauthorized('Your account has been blocked.') }
end
end end
describe 'without access to project' do describe 'without access to project' do
...@@ -428,28 +408,30 @@ describe Gitlab::GitAccess do ...@@ -428,28 +408,30 @@ describe Gitlab::GitAccess do
end end
end end
# Run permission checks for a user
def self.run_permission_checks(permissions_matrix) def self.run_permission_checks(permissions_matrix)
permissions_matrix.keys.each do |role| permissions_matrix.each_pair do |role, matrix|
describe "#{role} access" do # Run through the entire matrix for this role in one test to avoid
before do # repeated setup.
if role == :admin #
user.update_attribute(:admin, true) # Expectations are given a custom failure message proc so that it's
else # easier to identify which check(s) failed.
project.team << [user, role] it "has the correct permissions for #{role}s" do
end if role == :admin
user.update_attribute(:admin, true)
else
project.team << [user, role]
end end
permissions_matrix[role].each do |action, allowed| aggregate_failures do
context action.to_s do matrix.each do |action, allowed|
subject { access.send(:check_push_access!, changes[action]) } check = -> { access.send(:check_push_access!, changes[action]) }
it do if allowed
if allowed expect(&check).not_to raise_error,
expect { subject }.not_to raise_error -> { "expected #{action} to be allowed" }
else else
expect { subject }.to raise_error(Gitlab::GitAccess::UnauthorizedError) expect(&check).to raise_error(Gitlab::GitAccess::UnauthorizedError),
end -> { "expected #{action} to be disallowed" }
end end
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