From 370fc05da7f95bf6621867a71d51493cf3899e25 Mon Sep 17 00:00:00 2001 From: Mayra Cabrera Date: Thu, 29 Mar 2018 16:56:35 -0600 Subject: [PATCH] Implement 'read_repo' for DeployTokens This will allow to download a repo using the token from the DeployToken --- app/models/deploy_token.rb | 8 +++ lib/gitlab/auth.rb | 16 +++++- lib/gitlab/git_access.rb | 11 ++++ .../settings/repository_settings_spec.rb | 38 +++++++++++++ spec/lib/gitlab/auth_spec.rb | 57 ++++++++++++++++++- spec/lib/gitlab/git_access_spec.rb | 27 +++++++++ spec/models/deploy_token_spec.rb | 46 ++++++++++----- 7 files changed, 184 insertions(+), 19 deletions(-) diff --git a/app/models/deploy_token.rb b/app/models/deploy_token.rb index 185bd806b18..089b4343f41 100644 --- a/app/models/deploy_token.rb +++ b/app/models/deploy_token.rb @@ -22,4 +22,12 @@ class DeployToken < ActiveRecord::Base def self.redis_shared_state_key(user_id) "gitlab:personal_access_token:#{user_id}" end + + def active? + !revoked + end + + def username + User.ghost.username + end end diff --git a/lib/gitlab/auth.rb b/lib/gitlab/auth.rb index 6af763faf10..77fef7d8cac 100644 --- a/lib/gitlab/auth.rb +++ b/lib/gitlab/auth.rb @@ -5,7 +5,7 @@ module Gitlab REGISTRY_SCOPES = [:read_registry].freeze # Scopes used for GitLab API access - API_SCOPES = [:api, :read_user, :sudo].freeze + API_SCOPES = [:api, :read_user, :sudo, :read_repo].freeze # Scopes used for OpenID Connect OPENID_SCOPES = [:openid].freeze @@ -26,6 +26,7 @@ module Gitlab lfs_token_check(login, password, project) || oauth_access_token_check(login, password) || personal_access_token_check(password) || + deploy_token_check(project, password) || user_with_password_for_git(login, password) || Gitlab::Auth::Result.new @@ -163,7 +164,8 @@ module Gitlab def abilities_for_scopes(scopes) abilities_by_scope = { api: full_authentication_abilities, - read_registry: [:read_container_image] + read_registry: [:read_container_image], + read_repo: read_authentication_abilities - [:read_container_image] } scopes.flat_map do |scope| @@ -171,6 +173,16 @@ module Gitlab end.uniq end + def deploy_token_check(project, password) + return unless project.present? && password.present? + + token = DeployToken.active.find_by(project: project, token: password) + + if token && valid_scoped_token?(token, available_scopes) + Gitlab::Auth::Result.new(token, project, :deploy_token, abilities_for_scopes(token.scopes)) + end + end + def lfs_token_check(login, password, project) deploy_key_matches = login.match(/\Alfs\+deploy-key-(\d+)\z/) diff --git a/lib/gitlab/git_access.rb b/lib/gitlab/git_access.rb index 01f8b22b2b6..e3c723ab274 100644 --- a/lib/gitlab/git_access.rb +++ b/lib/gitlab/git_access.rb @@ -208,6 +208,7 @@ module Gitlab def check_download_access! passed = deploy_key? || + deploy_token? || user_can_download_code? || build_can_download_code? || guest_can_download_code? @@ -274,6 +275,14 @@ module Gitlab actor.is_a?(DeployKey) end + def deploy_token + actor if deploy_token? + end + + def deploy_token? + actor.is_a?(DeployToken) + end + def ci? actor == :ci end @@ -283,6 +292,8 @@ module Gitlab deploy_key.has_access_to?(project) elsif user user.can?(:read_project, project) + elsif deploy_token? + deploy_token.active? && deploy_token.project == project elsif ci? true # allow CI (build without a user) for backwards compatibility end || Guest.can?(:read_project, project) diff --git a/spec/features/projects/settings/repository_settings_spec.rb b/spec/features/projects/settings/repository_settings_spec.rb index 14670e91006..1106a31d444 100644 --- a/spec/features/projects/settings/repository_settings_spec.rb +++ b/spec/features/projects/settings/repository_settings_spec.rb @@ -88,5 +88,43 @@ feature 'Repository settings' do expect(page).not_to have_content(private_deploy_key.title) end end + + context 'Deploy tokens' do + let(:deploy_token) { create(:deploy_token, project: project, expires_at: Date.today + 2.days) } + + before do + project.deploy_tokens << deploy_token + visit project_settings_repository_path(project) + end + + scenario 'view deploy tokens' do + within('.deploy-tokens') do + expect(page).to have_content(deploy_token.name) + expect(page).to have_content('In 1 day') + expect(page).to have_content(deploy_token.scopes.join(", ")) + end + end + + scenario 'add a new deploy token' do + fill_in 'deploy_token_name', with: 'new_deploy_key' + fill_in 'deploy_token_expires_at', with: (Date.today + 1.month).to_s + check 'deploy_token_scopes_read_repo' + check 'deploy_token_scopes_read_registry' + click_button 'Create deploy token' + + expect(page).to have_content('Your new project deploy token has been created') + end + + scenario 'revoke a deploy token', :js do + within('.deploy-tokens') do + click_link 'Revoke' + click_link "Revoke #{deploy_token.name}" + + expect(page).not_to have_content(deploy_token.name) + expect(page).not_to have_content('In 1 day') + expect(page).not_to have_content(deploy_token.scopes.join(", ")) + end + end + end end end diff --git a/spec/lib/gitlab/auth_spec.rb b/spec/lib/gitlab/auth_spec.rb index 18cef8ec996..685a0bb54be 100644 --- a/spec/lib/gitlab/auth_spec.rb +++ b/spec/lib/gitlab/auth_spec.rb @@ -5,7 +5,7 @@ describe Gitlab::Auth do describe 'constants' do it 'API_SCOPES contains all scopes for API access' do - expect(subject::API_SCOPES).to eq %i[api read_user sudo] + expect(subject::API_SCOPES).to eq %i[api read_user sudo read_repo] end it 'OPENID_SCOPES contains all scopes for OpenID Connect' do @@ -19,7 +19,7 @@ describe Gitlab::Auth do it 'optional_scopes contains all non-default scopes' do stub_container_registry_config(enabled: true) - expect(subject.optional_scopes).to eq %i[read_user sudo read_registry openid] + expect(subject.optional_scopes).to eq %i[read_user sudo read_repo read_registry openid] end context 'registry_scopes' do @@ -231,7 +231,7 @@ describe Gitlab::Auth do .to eq(Gitlab::Auth::Result.new(user, nil, :gitlab_or_ldap, full_authentication_abilities)) end - it 'falls through oauth authentication when the username is oauth2' do + it 'fails through oauth authentication when the username is oauth2' do user = create( :user, username: 'oauth2', @@ -255,6 +255,57 @@ describe Gitlab::Auth do expect { gl_auth.find_for_git_client('foo', 'bar', project: nil, ip: 'ip') }.to raise_error(Gitlab::Auth::MissingPersonalAccessTokenError) end + + context 'while using deploy tokens' do + let(:project) { create(:project) } + let(:deploy_token) { create(:deploy_token, :read_repo, project: project) } + let(:auth_failure) { Gitlab::Auth::Result.new(nil, nil) } + let(:abilities) { %i(read_project download_code) } + + it 'succeeds when project is present, token is valid and has read_repo as scope' do + auth_success = Gitlab::Auth::Result.new(deploy_token, project, :deploy_token, abilities) + + expect(gl_auth).to receive(:rate_limit!).with('ip', success: true, login: '') + expect(gl_auth.find_for_git_client('', deploy_token.token, project: project, ip: 'ip')) + .to eq(auth_success) + end + + it 'fails if deploy token does not have read_repo as scope' do + deploy_token = create(:deploy_token, :read_registry, project: project) + + expect(gl_auth).to receive(:rate_limit!).with('ip', success: false, login: '') + expect(gl_auth.find_for_git_client('', deploy_token.token, project: project, ip: 'ip')) + .to eq(auth_failure) + end + + it 'fails if token is nil' do + expect(gl_auth).to receive(:rate_limit!).with('ip', success: false, login: '') + expect(gl_auth.find_for_git_client('', nil, project: project, ip: 'ip')) + .to eq(auth_failure) + end + + it 'fails if token is not related to project' do + expect(gl_auth).to receive(:rate_limit!).with('ip', success: false, login: '') + expect(gl_auth.find_for_git_client('', 'abcdef', project: project, ip: 'ip')) + .to eq(auth_failure) + end + + it 'fails for any other project' do + another_project = create(:project) + expect(gl_auth).to receive(:rate_limit!).with('ip', success: false, login: '') + expect(gl_auth.find_for_git_client('', deploy_token.token, project: another_project, ip: 'ip')) + .to eq(auth_failure) + end + + it 'fails if token has been revoked' do + deploy_token.revoke! + + expect(deploy_token.revoked?).to be_truthy + expect(gl_auth).to receive(:rate_limit!).with('ip', success: false, login: 'deploy-token') + expect(gl_auth.find_for_git_client('deploy-token', deploy_token.token, project: project, ip: 'ip')) + .to eq(auth_failure) + end + end end describe 'find_with_user_password' do diff --git a/spec/lib/gitlab/git_access_spec.rb b/spec/lib/gitlab/git_access_spec.rb index c870997e274..928825c21fa 100644 --- a/spec/lib/gitlab/git_access_spec.rb +++ b/spec/lib/gitlab/git_access_spec.rb @@ -145,6 +145,33 @@ describe Gitlab::GitAccess do expect { push_access_check }.to raise_unauthorized(described_class::ERROR_MESSAGES[:auth_upload]) end end + + context 'when actor is DeployToken' do + context 'when DeployToken is active and belongs to project' do + let(:actor) { create(:deploy_token, :read_repo, project: project) } + + it 'allows pull access' do + expect { pull_access_check }.not_to raise_error + end + end + + context 'when DeployToken has been revoked' do + let(:actor) { create(:deploy_token, :read_repo, project: project) } + + it 'blocks pull access' do + actor.revoke! + expect { pull_access_check }.to raise_not_found + end + end + + context 'when DeployToken does not belong to project' do + let(:actor) { create(:deploy_token, :read_repo) } + + it 'blocks pull access' do + expect { pull_access_check }.to raise_not_found + end + end + end end context 'when actor is nil' do diff --git a/spec/models/deploy_token_spec.rb b/spec/models/deploy_token_spec.rb index bd27da63dfe..26d846ac6c8 100644 --- a/spec/models/deploy_token_spec.rb +++ b/spec/models/deploy_token_spec.rb @@ -1,38 +1,56 @@ require 'spec_helper' describe DeployToken do + let(:deploy_token) { create(:deploy_token) } + it { is_expected.to belong_to :project } describe 'validations' do - let(:project_deploy_token) { build(:deploy_token) } - context 'with no scopes defined' do it 'should not be valid' do - project_deploy_token.scopes = [] + deploy_token.scopes = [] - expect(project_deploy_token).not_to be_valid - expect(project_deploy_token.errors[:scopes].first).to eq("can't be blank") + expect(deploy_token).not_to be_valid + expect(deploy_token.errors[:scopes].first).to eq("can't be blank") end end end describe '#ensure_token' do - let(:project_deploy_token) { build(:deploy_token) } - it 'should ensure a token' do - project_deploy_token.token = nil - project_deploy_token.save + deploy_token.token = nil + deploy_token.save - expect(project_deploy_token.token).not_to be_empty + expect(deploy_token.token).not_to be_empty end end describe '#revoke!' do - subject { create(:deploy_token) } - it 'should update revoke attribute' do - subject.revoke! - expect(subject.revoked?).to be_truthy + deploy_token.revoke! + expect(deploy_token.revoked?).to be_truthy + end + end + + describe "#active?" do + context "when it has been revoked" do + it 'should return false' do + deploy_token.revoke! + expect(deploy_token.active?).to be_falsy + end + end + + context "when it hasn't been revoked" do + it 'should return true' do + expect(deploy_token.active?).to be_truthy + end + end + end + + describe '#username' do + it 'returns Ghost username' do + ghost = User.ghost + expect(deploy_token.username).to eq(ghost.username) end end end