Commit 171b2625 authored by Mayra Cabrera's avatar Mayra Cabrera

Addreses backend review suggestions

- Remove extra method for authorize_admin_project
- Ensure project presence
- Rename 'read_repo' to 'read_repository' to be more verbose
parent 7deab317
...@@ -23,8 +23,4 @@ class Projects::DeployTokensController < Projects::ApplicationController ...@@ -23,8 +23,4 @@ class Projects::DeployTokensController < Projects::ApplicationController
def deploy_token_params def deploy_token_params
params.require(:deploy_token).permit(:name, :expires_at, scopes: []) params.require(:deploy_token).permit(:name, :expires_at, scopes: [])
end end
def authorize_admin_project!
return render_404 unless can?(current_user, :admin_project, @project)
end
end end
...@@ -3,11 +3,12 @@ class DeployToken < ActiveRecord::Base ...@@ -3,11 +3,12 @@ class DeployToken < ActiveRecord::Base
include TokenAuthenticatable include TokenAuthenticatable
add_authentication_token_field :token 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 serialize :scopes, Array # rubocop:disable Cop/ActiveRecordSerialize
validates :scopes, presence: true validates :scopes, presence: true
validates :project, presence: true
belongs_to :project belongs_to :project
......
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
...@@ -44,7 +44,7 @@ module Projects ...@@ -44,7 +44,7 @@ module Projects
def scope_descriptions 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') 'read_registry' => s_('DeployTokens|Allows read-only access to the registry images')
} }
end end
......
--- ---
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 merge_request: 17894
author: author:
type: added type: added
...@@ -36,7 +36,7 @@ the following table. ...@@ -36,7 +36,7 @@ the following table.
| Scope | Description | | 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. | | `read_registry` | Allows read-access to [container registry] images if a project is private and authorization is required. |
## Usage ## Usage
...@@ -45,7 +45,7 @@ the following table. ...@@ -45,7 +45,7 @@ the following table.
To download a repository using a Deploy Token, you just need to: 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: 2. `git clone` the project using the Deploy Token:
......
...@@ -5,7 +5,7 @@ module Gitlab ...@@ -5,7 +5,7 @@ module Gitlab
REGISTRY_SCOPES = [:read_registry].freeze REGISTRY_SCOPES = [:read_registry].freeze
# Scopes used for GitLab API access # 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 # Scopes used for OpenID Connect
OPENID_SCOPES = [:openid].freeze OPENID_SCOPES = [:openid].freeze
...@@ -165,7 +165,7 @@ module Gitlab ...@@ -165,7 +165,7 @@ module Gitlab
abilities_by_scope = { abilities_by_scope = {
api: full_authentication_abilities, api: full_authentication_abilities,
read_registry: build_authentication_abilities - [:build_create_container_image], 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| scopes.flat_map do |scope|
......
...@@ -5,14 +5,14 @@ FactoryBot.define do ...@@ -5,14 +5,14 @@ FactoryBot.define do
sequence(:name) { |n| "PDT #{n}" } sequence(:name) { |n| "PDT #{n}" }
revoked false revoked false
expires_at { 5.days.from_now } expires_at { 5.days.from_now }
scopes %w(read_repo read_registry) scopes %w(read_repository read_registry)
trait :revoked do trait :revoked do
revoked true revoked true
end end
trait :read_repo do trait :read_repository do
scopes ['read_repo'] scopes ['read_repository']
end end
trait :read_registry do trait :read_registry do
......
...@@ -5,7 +5,7 @@ describe Gitlab::Auth do ...@@ -5,7 +5,7 @@ describe Gitlab::Auth do
describe 'constants' do describe 'constants' do
it 'API_SCOPES contains all scopes for API access' 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 end
it 'OPENID_SCOPES contains all scopes for OpenID Connect' do it 'OPENID_SCOPES contains all scopes for OpenID Connect' do
...@@ -19,7 +19,7 @@ describe Gitlab::Auth do ...@@ -19,7 +19,7 @@ describe Gitlab::Auth do
it 'optional_scopes contains all non-default scopes' do it 'optional_scopes contains all non-default scopes' do
stub_container_registry_config(enabled: true) 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 end
context 'registry_scopes' do context 'registry_scopes' do
...@@ -260,10 +260,10 @@ describe Gitlab::Auth do ...@@ -260,10 +260,10 @@ describe Gitlab::Auth do
let(:project) { create(:project) } let(:project) { create(:project) }
let(:auth_failure) { Gitlab::Auth::Result.new(nil, nil) } let(:auth_failure) { Gitlab::Auth::Result.new(nil, nil) }
context 'when the deploy token has read_repo as scope' do context 'when the deploy token has read_repository as scope' do
let(:deploy_token) { create(:deploy_token, :read_repo, project: project) } 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) abilities = %i(read_project download_code)
auth_success = Gitlab::Auth::Result.new(deploy_token, project, :deploy_token, abilities) auth_success = Gitlab::Auth::Result.new(deploy_token, project, :deploy_token, abilities)
......
...@@ -4,6 +4,7 @@ describe DeployToken do ...@@ -4,6 +4,7 @@ describe DeployToken do
let(:deploy_token) { create(:deploy_token) } let(:deploy_token) { create(:deploy_token) }
it { is_expected.to belong_to :project } it { is_expected.to belong_to :project }
it { is_expected.to validate_presence_of :project }
describe 'validations' do describe 'validations' do
context 'with no scopes defined' do context 'with no scopes defined' do
......
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
...@@ -9,7 +9,7 @@ describe Projects::Settings::DeployTokensPresenter do ...@@ -9,7 +9,7 @@ describe Projects::Settings::DeployTokensPresenter do
describe '#available_scopes' do describe '#available_scopes' do
it 'returns the all the deploy token 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
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