diff options
author | Mayra Cabrera <mcabrera@gitlab.com> | 2018-04-04 18:43:41 -0500 |
---|---|---|
committer | Mayra Cabrera <mcabrera@gitlab.com> | 2018-04-06 21:20:16 -0500 |
commit | 171b2625b128e5954ce0a150a4fc923a22164e4e (patch) | |
tree | 834586c27477a404e71fe2fac9d17ecf3e495e58 | |
parent | 7deab3172257bef7818ce834c1e0709432ddd5e0 (diff) | |
download | gitlab-ce-171b2625b128e5954ce0a150a4fc923a22164e4e.tar.gz |
Addreses backend review suggestions
- Remove extra method for authorize_admin_project
- Ensure project presence
- Rename 'read_repo' to 'read_repository' to be more verbose
-rw-r--r-- | app/controllers/projects/deploy_tokens_controller.rb | 4 | ||||
-rw-r--r-- | app/models/deploy_token.rb | 3 | ||||
-rw-r--r-- | app/policies/deploy_token_policy.rb | 11 | ||||
-rw-r--r-- | app/presenters/projects/settings/deploy_tokens_presenter.rb | 2 | ||||
-rw-r--r-- | changelogs/unreleased/31591-project-deploy-tokens-to-allow-permanent-access.yml | 2 | ||||
-rw-r--r-- | doc/user/project/deploy_tokens/img/deploy_tokens.png | bin | 73260 -> 75650 bytes | |||
-rw-r--r-- | doc/user/project/deploy_tokens/index.md | 4 | ||||
-rw-r--r-- | lib/gitlab/auth.rb | 4 | ||||
-rw-r--r-- | spec/factories/deploy_tokens.rb | 6 | ||||
-rw-r--r-- | spec/lib/gitlab/auth_spec.rb | 10 | ||||
-rw-r--r-- | spec/models/deploy_token_spec.rb | 1 | ||||
-rw-r--r-- | spec/policies/deploy_token_policy_spec.rb | 45 | ||||
-rw-r--r-- | spec/presenters/projects/settings/deploy_tokens_presenter_spec.rb | 2 |
13 files changed, 74 insertions, 20 deletions
diff --git a/app/controllers/projects/deploy_tokens_controller.rb b/app/controllers/projects/deploy_tokens_controller.rb index 1b1bd461b27..a7d9590ba19 100644 --- a/app/controllers/projects/deploy_tokens_controller.rb +++ b/app/controllers/projects/deploy_tokens_controller.rb @@ -23,8 +23,4 @@ class Projects::DeployTokensController < Projects::ApplicationController def deploy_token_params params.require(:deploy_token).permit(:name, :expires_at, scopes: []) end - - def authorize_admin_project! - return render_404 unless can?(current_user, :admin_project, @project) - end end diff --git a/app/models/deploy_token.rb b/app/models/deploy_token.rb index 475ad06906a..b4df44d295a 100644 --- a/app/models/deploy_token.rb +++ b/app/models/deploy_token.rb @@ -3,11 +3,12 @@ class DeployToken < ActiveRecord::Base include TokenAuthenticatable add_authentication_token_field :token - AVAILABLE_SCOPES = %w(read_repo read_registry).freeze + AVAILABLE_SCOPES = %w(read_repository read_registry).freeze serialize :scopes, Array # rubocop:disable Cop/ActiveRecordSerialize validates :scopes, presence: true + validates :project, presence: true belongs_to :project diff --git a/app/policies/deploy_token_policy.rb b/app/policies/deploy_token_policy.rb new file mode 100644 index 00000000000..7aa9106e8b1 --- /dev/null +++ b/app/policies/deploy_token_policy.rb @@ -0,0 +1,11 @@ +class DeployTokenPolicy < BasePolicy + with_options scope: :subject, score: 0 + condition(:master) { @subject.project.team.master?(@user) } + + rule { anonymous }.prevent_all + + rule { master }.policy do + enable :create_deploy_token + enable :update_deploy_token + end +end diff --git a/app/presenters/projects/settings/deploy_tokens_presenter.rb b/app/presenters/projects/settings/deploy_tokens_presenter.rb index e2aca2d273a..26bb42e9e7e 100644 --- a/app/presenters/projects/settings/deploy_tokens_presenter.rb +++ b/app/presenters/projects/settings/deploy_tokens_presenter.rb @@ -44,7 +44,7 @@ module Projects def scope_descriptions { - 'read_repo' => s_('DeployTokens|Allows read-only access to the repository'), + 'read_repository' => s_('DeployTokens|Allows read-only access to the repository'), 'read_registry' => s_('DeployTokens|Allows read-only access to the registry images') } end diff --git a/changelogs/unreleased/31591-project-deploy-tokens-to-allow-permanent-access.yml b/changelogs/unreleased/31591-project-deploy-tokens-to-allow-permanent-access.yml index b9b19eb20ad..5546d26d0fb 100644 --- a/changelogs/unreleased/31591-project-deploy-tokens-to-allow-permanent-access.yml +++ b/changelogs/unreleased/31591-project-deploy-tokens-to-allow-permanent-access.yml @@ -1,5 +1,5 @@ --- -title: Creates Deploy Tokens to allow permanent access to repo and registry +title: Create Deploy Tokens to allow permanent access to repository and registry merge_request: 17894 author: type: added diff --git a/doc/user/project/deploy_tokens/img/deploy_tokens.png b/doc/user/project/deploy_tokens/img/deploy_tokens.png Binary files differindex 4cf2e5ca612..7e2d67a3120 100644 --- a/doc/user/project/deploy_tokens/img/deploy_tokens.png +++ b/doc/user/project/deploy_tokens/img/deploy_tokens.png diff --git a/doc/user/project/deploy_tokens/index.md b/doc/user/project/deploy_tokens/index.md index 15bf6170b12..44e95bbcda0 100644 --- a/doc/user/project/deploy_tokens/index.md +++ b/doc/user/project/deploy_tokens/index.md @@ -36,7 +36,7 @@ the following table. | Scope | Description | | ----- | ----------- | -|`read_repo` | Allows read-access to the repository through `git clone` | +|`read_repository` | Allows read-access to the repository through `git clone` | | `read_registry` | Allows read-access to [container registry] images if a project is private and authorization is required. | ## Usage @@ -45,7 +45,7 @@ the following table. To download a repository using a Deploy Token, you just need to: -1. Create a Deploy Token with `read_repo` as a scope. +1. Create a Deploy Token with `read_repository` as a scope. 2. `git clone` the project using the Deploy Token: diff --git a/lib/gitlab/auth.rb b/lib/gitlab/auth.rb index 3ef2f7f2967..35458f607c6 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, :read_repo].freeze + API_SCOPES = [:api, :read_user, :sudo, :read_repository].freeze # Scopes used for OpenID Connect OPENID_SCOPES = [:openid].freeze @@ -165,7 +165,7 @@ module Gitlab abilities_by_scope = { api: full_authentication_abilities, read_registry: build_authentication_abilities - [:build_create_container_image], - read_repo: read_authentication_abilities - [:read_container_image] + read_repository: read_authentication_abilities - [:read_container_image] } scopes.flat_map do |scope| diff --git a/spec/factories/deploy_tokens.rb b/spec/factories/deploy_tokens.rb index fa349e10ddc..7cce55a3e14 100644 --- a/spec/factories/deploy_tokens.rb +++ b/spec/factories/deploy_tokens.rb @@ -5,14 +5,14 @@ FactoryBot.define do sequence(:name) { |n| "PDT #{n}" } revoked false expires_at { 5.days.from_now } - scopes %w(read_repo read_registry) + scopes %w(read_repository read_registry) trait :revoked do revoked true end - trait :read_repo do - scopes ['read_repo'] + trait :read_repository do + scopes ['read_repository'] end trait :read_registry do diff --git a/spec/lib/gitlab/auth_spec.rb b/spec/lib/gitlab/auth_spec.rb index 79984787e2a..f704c20f598 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 read_repo] + expect(subject::API_SCOPES).to eq %i[api read_user sudo read_repository] 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_repo read_registry openid] + expect(subject.optional_scopes).to eq %i[read_user sudo read_repository read_registry openid] end context 'registry_scopes' do @@ -260,10 +260,10 @@ describe Gitlab::Auth do let(:project) { create(:project) } let(:auth_failure) { Gitlab::Auth::Result.new(nil, nil) } - context 'when the deploy token has read_repo as scope' do - let(:deploy_token) { create(:deploy_token, :read_repo, project: project) } + context 'when the deploy token has read_repository as scope' do + let(:deploy_token) { create(:deploy_token, :read_repository, project: project) } - it 'succeeds when project is present, token is valid and has read_repo as scope' do + it 'succeeds when project is present, token is valid and has read_repository as scope' do abilities = %i(read_project download_code) auth_success = Gitlab::Auth::Result.new(deploy_token, project, :deploy_token, abilities) diff --git a/spec/models/deploy_token_spec.rb b/spec/models/deploy_token_spec.rb index 26d846ac6c8..50f6f441a58 100644 --- a/spec/models/deploy_token_spec.rb +++ b/spec/models/deploy_token_spec.rb @@ -4,6 +4,7 @@ describe DeployToken do let(:deploy_token) { create(:deploy_token) } it { is_expected.to belong_to :project } + it { is_expected.to validate_presence_of :project } describe 'validations' do context 'with no scopes defined' do diff --git a/spec/policies/deploy_token_policy_spec.rb b/spec/policies/deploy_token_policy_spec.rb new file mode 100644 index 00000000000..cbb5fb815a1 --- /dev/null +++ b/spec/policies/deploy_token_policy_spec.rb @@ -0,0 +1,45 @@ +require 'spec_helper' + +describe DeployTokenPolicy do + let(:current_user) { create(:user) } + let(:project) { create(:project) } + let(:deploy_token) { create(:deploy_token, project: project) } + + subject { described_class.new(current_user, deploy_token) } + + describe 'creating a deploy key' do + context 'when user is master' do + before do + project.add_master(current_user) + end + + it { is_expected.to be_allowed(:create_deploy_token) } + end + + context 'when user is not master' do + before do + project.add_developer(current_user) + end + + it { is_expected.to be_disallowed(:create_deploy_token) } + end + end + + describe 'updating a deploy key' do + context 'when user is master' do + before do + project.add_master(current_user) + end + + it { is_expected.to be_allowed(:update_deploy_token) } + end + + context 'when user is not master' do + before do + project.add_developer(current_user) + end + + it { is_expected.to be_disallowed(:update_deploy_token) } + end + end +end diff --git a/spec/presenters/projects/settings/deploy_tokens_presenter_spec.rb b/spec/presenters/projects/settings/deploy_tokens_presenter_spec.rb index ca734019727..7bfe074ad30 100644 --- a/spec/presenters/projects/settings/deploy_tokens_presenter_spec.rb +++ b/spec/presenters/projects/settings/deploy_tokens_presenter_spec.rb @@ -9,7 +9,7 @@ describe Projects::Settings::DeployTokensPresenter do describe '#available_scopes' do it 'returns the all the deploy token scopes' do - expect(presenter.available_scopes).to match_array(%w(read_repo read_registry)) + expect(presenter.available_scopes).to match_array(%w(read_repository read_registry)) end end |