diff options
author | Michael Kozono <mkozono@gmail.com> | 2017-05-16 22:58:46 +0300 |
---|---|---|
committer | Michael Kozono <mkozono@gmail.com> | 2017-06-05 15:32:26 +0300 |
commit | bad08fbea2a32655a6d87f2140840c317cea6c80 (patch) | |
tree | dc690059a2ce53d055c1eb2738a832c36f1ee89c /spec | |
parent | b387429458f77a3608e077dfe2d50b0a313f8832 (diff) |
Move CI access logic into GitAccess
Diffstat (limited to 'spec')
-rw-r--r-- | spec/lib/gitlab/git_access_spec.rb | 128 | ||||
-rw-r--r-- | spec/requests/git_http_spec.rb | 58 | ||||
-rw-r--r-- | spec/support/git_http_helpers.rb | 9 |
3 files changed, 166 insertions, 29 deletions
diff --git a/spec/lib/gitlab/git_access_spec.rb b/spec/lib/gitlab/git_access_spec.rb index 1033fef9684..0efe15856fc 100644 --- a/spec/lib/gitlab/git_access_spec.rb +++ b/spec/lib/gitlab/git_access_spec.rb @@ -1,6 +1,8 @@ require 'spec_helper' describe Gitlab::GitAccess, lib: true do + let(:pull_access_check) { access.check('git-upload-pack', '_any') } + let(:push_access_check) { access.check('git-receive-pack', '_any') } let(:access) { Gitlab::GitAccess.new(actor, project, protocol, authentication_abilities: authentication_abilities) } let(:project) { create(:project, :repository) } let(:user) { create(:user) } @@ -51,7 +53,123 @@ describe Gitlab::GitAccess, lib: true do end end - describe '#check with commands disabled' do + describe '#check_project_accessibility!' do + context 'when the project exists' do + context 'when actor exists' do + context 'when actor is a DeployKey' do + let(:deploy_key) { create(:deploy_key, user: user, can_push: true) } + let(:actor) { deploy_key } + + context 'when the DeployKey has access to the project' do + before { deploy_key.projects << project } + + it 'allows pull access' do + expect(pull_access_check.allowed?).to be_truthy + end + + it 'allows push access' do + expect(push_access_check.allowed?).to be_truthy + 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.allowed?).to be_falsey + expect(pull_access_check.message).to eq('The project you were looking for could not be found.') + end + + it 'blocks pushes with "not found"' do + expect(push_access_check.allowed?).to be_falsey + expect(push_access_check.message).to eq('The project you were looking for could not be found.') + end + end + end + + context 'when actor is a User' do + context 'when the User can read the project' do + before { project.team << [user, :master] } + + it 'allows pull access' do + expect(pull_access_check.allowed?).to be_truthy + end + + it 'allows push access' do + expect(push_access_check.allowed?).to be_truthy + end + end + + context 'when the User cannot read the project' do + it 'blocks pulls with "not found"' do + expect(pull_access_check.allowed?).to be_falsey + expect(pull_access_check.message).to eq('The project you were looking for could not be found.') + end + + it 'blocks pushes with "not found"' do + expect(push_access_check.allowed?).to be_falsey + expect(push_access_check.message).to eq('The project you were looking for could not be found.') + end + end + end + + # For backwards compatibility + context 'when actor is :ci' do + let(:actor) { :ci } + let(:authentication_abilities) { build_authentication_abilities } + + it 'allows pull access' do + expect(pull_access_check.allowed?).to be_truthy + end + + it 'does not block pushes with "not found"' do + expect(push_access_check.allowed?).to be_falsey + expect(push_access_check.message).to eq('You are not allowed to upload code for this project.') + end + end + end + + context 'when actor is nil' do + let(:actor) { nil } + + context 'when guests can read the project' do + let(:project) { create(:project, :repository, :public) } + + it 'allows pull access' do + expect(pull_access_check.allowed?).to be_truthy + end + + it 'does not block pushes with "not found"' do + expect(push_access_check.allowed?).to be_falsey + expect(push_access_check.message).to eq('You are not allowed to upload code for this project.') + end + end + + context 'when guests cannot read the project' do + it 'blocks pulls with "not found"' do + expect(pull_access_check.allowed?).to be_falsey + expect(pull_access_check.message).to eq('The project you were looking for could not be found.') + end + + it 'blocks pushes with "not found"' do + expect(push_access_check.allowed?).to be_falsey + expect(push_access_check.message).to eq('The project you were looking for could not be found.') + end + end + end + end + + context 'when the project is nil' do + let(:project) { nil } + + it 'blocks any command with "not found"' do + expect(pull_access_check.allowed?).to be_falsey + expect(pull_access_check.message).to eq('The project you were looking for could not be found.') + expect(push_access_check.allowed?).to be_falsey + expect(push_access_check.message).to eq('The project you were looking for could not be found.') + end + end + end + + describe '#check_command_disabled!' do before { project.team << [user, :master] } context 'over http' do @@ -219,6 +337,14 @@ describe Gitlab::GitAccess, lib: true do end end end + + describe 'generic CI (build without a user)' do + let(:actor) { :ci } + + context 'pull code' do + it { expect(subject).to be_allowed } + end + end end end diff --git a/spec/requests/git_http_spec.rb b/spec/requests/git_http_spec.rb index 080e2f12cd7..ab7c56fcdf0 100644 --- a/spec/requests/git_http_spec.rb +++ b/spec/requests/git_http_spec.rb @@ -489,29 +489,41 @@ describe 'Git HTTP requests', lib: true do end context "when a gitlab ci token is provided" do + let(:project) { create(:project, :repository) } let(:build) { create(:ci_build, :running) } - let(:project) { build.project } let(:other_project) { create(:empty_project) } + before do + build.update!(project: project) # can't associate it on factory create + end + context 'when build created by system is authenticated' do let(:path) { "#{project.path_with_namespace}.git" } let(:env) { { user: 'gitlab-ci-token', password: build.token } } it_behaves_like 'pulls are allowed' - # TODO Verify this is desired behavior - it "rejects pushes with 401 Unauthorized (no project existence information leak)" do + # A non-401 here is not an information leak since the system is + # "authenticated" as CI using the correct token. It does not have + # push access, so pushes should be rejected as forbidden, and giving + # a reason is fine. + # + # We know for sure it is not an information leak since pulls using + # the build token must be allowed. + it "rejects pushes with 403 Forbidden" do push_get(path, env) - expect(response).to have_http_status(:unauthorized) + expect(response).to have_http_status(:forbidden) + expect(response.body).to eq(git_access_error(:upload)) end - # TODO Verify this is desired behavior. Should be 403 Forbidden? + # We are "authenticated" as CI using a valid token here. But we are + # not authorized to see any other project, so return "not found". it "rejects pulls for other project with 404 Not Found" do clone_get("#{other_project.path_with_namespace}.git", env) expect(response).to have_http_status(:not_found) - expect(response.body).to eq('TODO: What should this be?') + expect(response.body).to eq(git_access_error(:project_not_found)) end end @@ -522,31 +534,27 @@ describe 'Git HTTP requests', lib: true do end shared_examples 'can download code only' do - it 'downloads get status 200' do - allow_any_instance_of(Repository). - to receive(:exists?).and_return(true) - - clone_get "#{project.path_with_namespace}.git", - user: 'gitlab-ci-token', password: build.token + let(:path) { "#{project.path_with_namespace}.git" } + let(:env) { { user: 'gitlab-ci-token', password: build.token } } - expect(response).to have_http_status(:ok) - expect(response.content_type.to_s).to eq(Gitlab::Workhorse::INTERNAL_API_CONTENT_TYPE) - end - - it 'downloads from non-existing repository and gets 403' do - allow_any_instance_of(Repository). - to receive(:exists?).and_return(false) + it_behaves_like 'pulls are allowed' - clone_get "#{project.path_with_namespace}.git", - user: 'gitlab-ci-token', password: build.token + context 'when the repo does not exist' do + let(:project) { create(:empty_project) } + + it 'rejects pulls with 403 Forbidden' do + clone_get path, env - expect(response).to have_http_status(:forbidden) + expect(response).to have_http_status(:forbidden) + expect(response.body).to eq(git_access_error(:no_repo)) + end end - it 'uploads get status 403' do - push_get "#{project.path_with_namespace}.git", user: 'gitlab-ci-token', password: build.token + it 'rejects pushes with 403 Forbidden' do + push_get path, env - expect(response).to have_http_status(:unauthorized) + expect(response).to have_http_status(:forbidden) + expect(response.body).to eq(git_access_error(:upload)) end end diff --git a/spec/support/git_http_helpers.rb b/spec/support/git_http_helpers.rb index d3d51560a9d..b8289e6c5f1 100644 --- a/spec/support/git_http_helpers.rb +++ b/spec/support/git_http_helpers.rb @@ -52,14 +52,17 @@ module GitHttpHelpers end def git_access_error(error_key) - Gitlab::GitAccess::ERROR_MESSAGES[error_key] + message = Gitlab::GitAccess::ERROR_MESSAGES[error_key] + message || raise("GitAccess error message key '#{error_key}' not found") end def git_access_wiki_error(error_key) - Gitlab::GitAccessWiki::ERROR_MESSAGES[error_key] + message = Gitlab::GitAccessWiki::ERROR_MESSAGES[error_key] + message || raise("GitAccessWiki error message key '#{error_key}' not found") end def change_access_error(error_key) - Gitlab::Checks::ChangeAccess::ERROR_MESSAGES[error_key] + message = Gitlab::Checks::ChangeAccess::ERROR_MESSAGES[error_key] + message || raise("ChangeAccess error message key '#{error_key}' not found") end end |