Addreses backend review suggestions

- Remove extra method for authorize_admin_project
- Ensure project presence
- Rename 'read_repo' to 'read_repository' to be more verbose

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
index 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