Include ProjectDeployTokens

Also:
- Changes scopes from serializer to use boolean columns
- Fixes broken specs

app/controllers/projects/deploy_tokens_controller.rb

@@ -21,6 +21,6 @@ class Projects::DeployTokensController < Projects::ApplicationController
   private
 
   def deploy_token_params
-    params.require(:deploy_token).permit(:name, :expires_at, scopes: [])
+    params.require(:deploy_token).permit(:name, :expires_at, :read_repository, :read_registry)
   end
 end

app/controllers/projects/settings/repository_controller.rb

@@ -56,7 +56,7 @@ module Projects
 
       def define_deploy_token
         attributes = @deploy_tokens.attributes_deploy_token
-        @deploy_token = @project.deploy_tokens.build(attributes)
+        @deploy_token = DeployToken.new(attributes)
         @deploy_token.valid? unless attributes.empty?
       end
     end

app/models/deploy_token.rb

@@ -3,36 +3,51 @@ class DeployToken < ActiveRecord::Base
   include TokenAuthenticatable
   add_authentication_token_field :token
 
-  AVAILABLE_SCOPES = %w(read_repository read_registry).freeze
+  AVAILABLE_SCOPES = %i(read_repository read_registry).freeze
 
-  serialize :scopes, Array # rubocop:disable Cop/ActiveRecordSerialize
-
-  validates :scopes, presence: true
-  validates :project, presence: true
-
-  belongs_to :project
+  has_many :project_deploy_tokens, inverse_of: :deploy_token
+  has_many :projects, through: :project_deploy_tokens
 
+  validate :ensure_at_least_one_scope
   before_save :ensure_token
 
+  accepts_nested_attributes_for :project_deploy_tokens
+
   scope :active, -> { where("revoked = false AND (expires_at >= NOW() OR expires_at IS NULL)") }
+  scope :read_repository, -> { where(read_repository: true) }
+  scope :read_registry, -> { where(read_registry: true) }
 
-  def revoke!
-    update!(revoked: true)
+  def self.redis_shared_state_key(user_id)
+    "gitlab:deploy_token:user_#{user_id}"
   end
 
-  def redis_shared_state_key(user_id)
-    "gitlab:deploy_token:#{project_id}:#{user_id}"
+  def revoke!
+    update!(revoked: true)
   end
 
   def active?
     !revoked
   end
 
+  def scopes
+    AVAILABLE_SCOPES.select { |token_scope| send("#{token_scope}") }  # rubocop:disable GitlabSecurity/PublicSend
+  end
+
   def username
     "gitlab+deploy-token-#{id}"
   end
 
-  def has_access_to?(project)
-    self.project == project
+  def has_access_to?(requested_project)
+    self.projects.first == requested_project
+  end
+
+  def project
+    projects.first
+  end
+
+  private
+
+  def ensure_at_least_one_scope
+    errors.add(:base, "Scopes can't be blank") unless read_repository || read_registry
   end
 end

app/models/project.rb

@@ -222,7 +222,8 @@ class Project < ActiveRecord::Base
   has_many :environments
   has_many :deployments
   has_many :pipeline_schedules, class_name: 'Ci::PipelineSchedule'
-  has_many :deploy_tokens
+  has_many :project_deploy_tokens
+  has_many :deploy_tokens, through: :project_deploy_tokens
 
   has_many :active_runners, -> { active }, through: :runner_projects, source: :runner, class_name: 'Ci::Runner'
 

app/models/project_deploy_token.rb

+class ProjectDeployToken < ActiveRecord::Base
+  belongs_to :project
+  belongs_to :deploy_token, inverse_of: :project_deploy_tokens
+
+  validates :deploy_token, presence: true
+  validates :project, presence: true
+  validates :deploy_token_id, uniqueness: { scope: [:project_id] }
+
+  accepts_nested_attributes_for :deploy_token
+
+  def redis_shared_state_key(user_id)
+    "gitlab:deploy_token:#{project_id}:#{user_id}"
+  end
+end

app/presenters/projects/settings/deploy_tokens_presenter.rb

@@ -5,18 +5,10 @@ module Projects
 
       presents :deploy_tokens
 
-      def available_scopes
-        DeployToken::AVAILABLE_SCOPES
-      end
-
       def length
         deploy_tokens.length
       end
 
-      def scope_description(scope)
-        scope_descriptions[scope]
-      end
-
       def each
         deploy_tokens.each do |deploy_token|
           yield deploy_token
@@ -42,15 +34,8 @@ module Projects
 
       private
 
-      def scope_descriptions
-        {
-          'read_repository' => s_('DeployTokens|Allows read-only access to the repository'),
-          'read_registry' => s_('DeployTokens|Allows read-only access to the registry images')
-        }
-      end
-
       def deploy_token_key
-        @deploy_token_key ||= project.deploy_tokens.new.redis_shared_state_key(current_user.id)
+        @deploy_token_key ||= DeployToken.redis_shared_state_key(current_user.id)
       end
     end
   end

app/services/auth/container_registry_authentication_service.rb

@@ -165,7 +165,7 @@ module Auth
 
     def user_can_push?(requested_project)
       has_authentication_ability?(:create_container_image) &&
-        can_user?(current_user, :create_container_image, requested_project)
+        can_user?(:create_container_image, requested_project)
     end
 
     def error(code, status:, message: '')

app/services/deploy_tokens/create_service.rb

 module DeployTokens
   class CreateService < BaseService
-    REDIS_EXPIRY_TIME = 3.minutes
-
     def execute
       @project.deploy_tokens.build.tap do |deploy_token|
         deploy_token.attributes = params
@@ -13,7 +11,7 @@ module DeployTokens
     private
 
     def store_deploy_token_info_in_redis(deploy_token)
-      deploy_token_key = deploy_token.redis_shared_state_key(current_user.id)
+      deploy_token_key = DeployToken.redis_shared_state_key(current_user.id)
 
       if deploy_token.persisted?
         store_in_redis(deploy_token_key, deploy_token.token)
@@ -31,7 +29,7 @@ module DeployTokens
 
     def store_in_redis(key, value)
       Gitlab::Redis::SharedState.with do |redis|
-        redis.set(key, value, ex: REDIS_EXPIRY_TIME)
+        redis.set(key, value, ex: 3.minutes)
       end
     end
   end

app/views/projects/deploy_tokens/_form.html.haml

@@ -14,8 +14,15 @@
 
   .form-group
     = f.label :scopes, class: 'label-light'
-    - presenter.available_scopes.each do |scope|
-      = render 'projects/deploy_tokens/scope_form', token: token, scope: scope, presenter: presenter
+    %fieldset
+      = f.check_box :read_repository
+      = label_tag ("deploy_token_read_repository"), 'read_repository'
+      %span= s_('DeployTokens|Allows read-only access to the repository')
+
+    %fieldset
+      = f.check_box :read_registry
+      = label_tag ("deploy_token_read_registry"), 'read_registry'
+      %span= s_('DeployTokens|Allows read-only access to the registry images')
 
   .prepend-top-default
     = f.submit s_('DeployTokens|Create deploy token'), class: 'btn btn-success'

app/views/projects/deploy_tokens/_scope_form.html.haml

-%fieldset
-  = check_box_tag "deploy_token[scopes][]", scope, token.scopes.include?(scope), id: "deploy_token_scopes_#{scope}"
-  = label_tag ("deploy_token_scopes_#{scope}"), scope
-  %span= presenter.scope_description(scope)

db/migrate/20180319190020_create_deploy_tokens.rb

@@ -3,10 +3,10 @@ class CreateDeployTokens < ActiveRecord::Migration
 
   def change
     create_table :deploy_tokens do |t|
-      t.references :project, index: true, foreign_key: true, null: false
       t.string :name, null: false
       t.string :token, index: { unique: true }, null: false
-      t.string :scopes
+      t.boolean :read_repository, default: false
+      t.boolean :read_registry, default: false
       t.boolean :revoked, default: false
       t.datetime :expires_at
 

db/migrate/20180405142733_create_project_deploy_tokens.rb

+class CreateProjectDeployTokens < ActiveRecord::Migration
+  include Gitlab::Database::MigrationHelpers
+
+  DOWNTIME = false
+
+  disable_ddl_transaction!
+
+  def up
+    create_table :project_deploy_tokens do |t|
+      t.integer :project_id, null: false
+      t.integer :deploy_token_id, null: false
+
+      t.timestamps null: false
+    end
+
+    add_concurrent_index :project_deploy_tokens, [:project_id, :deploy_token_id]
+  end
+
+  def down
+    drop_table :project_deploy_tokens
+
+    remove_index :project_deploy_tokens, column: [:project_id, :deploy_token_id] if index_exists?(:project_deploy_tokens, [:project_id, :deploy_token_id])
+  end
+end

db/schema.rb

@@ -11,7 +11,7 @@
 #
 # It's strongly recommended that you check this file into your version control system.
 
-ActiveRecord::Schema.define(version: 20180405101928) do
+ActiveRecord::Schema.define(version: 20180405142733) do
 
   # These are extensions that must be enabled in order to support this database
   enable_extension "plpgsql"
@@ -684,17 +684,16 @@ ActiveRecord::Schema.define(version: 20180405101928) do
   add_index "deploy_keys_projects", ["project_id"], name: "index_deploy_keys_projects_on_project_id", using: :btree
 
   create_table "deploy_tokens", force: :cascade do |t|
-    t.integer "project_id", null: false
     t.string "name", null: false
     t.string "token", null: false
-    t.string "scopes"
+    t.boolean "read_repository", default: false
+    t.boolean "read_registry", default: false
     t.boolean "revoked", default: false
     t.datetime "expires_at"
     t.datetime "created_at", null: false
     t.datetime "updated_at", null: false
   end
 
-  add_index "deploy_tokens", ["project_id"], name: "index_deploy_tokens_on_project_id", using: :btree
   add_index "deploy_tokens", ["token"], name: "index_deploy_tokens_on_token", unique: true, using: :btree
 
   create_table "deployments", force: :cascade do |t|
@@ -1444,6 +1443,15 @@ ActiveRecord::Schema.define(version: 20180405101928) do
   add_index "project_custom_attributes", ["key", "value"], name: "index_project_custom_attributes_on_key_and_value", using: :btree
   add_index "project_custom_attributes", ["project_id", "key"], name: "index_project_custom_attributes_on_project_id_and_key", unique: true, using: :btree
 
+  create_table "project_deploy_tokens", force: :cascade do |t|
+    t.integer "project_id", null: false
+    t.integer "deploy_token_id", null: false
+    t.datetime "created_at", null: false
+    t.datetime "updated_at", null: false
+  end
+
+  add_index "project_deploy_tokens", ["project_id", "deploy_token_id"], name: "index_project_deploy_tokens_on_project_id_and_deploy_token_id", using: :btree
+
   create_table "project_features", force: :cascade do |t|
     t.integer "project_id"
     t.integer "merge_requests_access_level"

lib/gitlab/auth.rb

@@ -186,7 +186,7 @@ module Gitlab
         return unless login != "gitlab+deploy-token-#{token.id}"
 
         scopes = abilities_for_scopes(token.scopes)
-        if valid_scoped_token?(token, scopes)
+        if valid_scoped_token?(token, available_scopes)
           Gitlab::Auth::Result.new(token, token.project, :deploy_token, scopes)
         end
       end

spec/factories/deploy_tokens.rb

 FactoryBot.define do
   factory :deploy_token do
-    project
     token { SecureRandom.hex(50) }
     sequence(:name) { |n| "PDT #{n}" }
+    read_repository true
+    read_registry true
     revoked false
     expires_at { 5.days.from_now }
-    scopes %w(read_repository read_registry)
 
     trait :revoked do
       revoked true
     end
-
-    trait :read_repository do
-      scopes ['read_repository']
-    end
-
-    trait :read_registry do
-      scopes ['read_registry']
-    end
   end
 end

spec/factories/project_deploy_tokens.rb

+FactoryBot.define do
+  factory :project_deploy_token do
+    project
+    deploy_token
+  end
+end

spec/features/projects/settings/repository_settings_spec.rb

@@ -90,25 +90,26 @@ feature 'Repository settings' do
     end
 
     context 'Deploy tokens' do
-      let(:deploy_token) { create(:deploy_token, project: project) }
+      let(:deploy_token_project) { create(:project_deploy_token, project: project) }
+      let!(:deploy_token) { deploy_token_project.deploy_token }
 
       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(deploy_token.scopes.join(", "))
+          expect(page).to have_content('read_repository')
+          expect(page).to have_content('read_registry')
         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'
+        check 'deploy_token_read_repository'
+        check 'deploy_token_read_registry'
         click_button 'Create deploy token'
 
         expect(page).to have_content('Your new project deploy token has been created')
@@ -120,7 +121,8 @@ feature 'Repository settings' do
           click_link "Revoke #{deploy_token.name}"
 
           expect(page).not_to have_content(deploy_token.name)
-          expect(page).not_to have_content(deploy_token.scopes.join(", "))
+          expect(page).not_to have_content('read_repository')
+          expect(page).not_to have_content('read_registry')
         end
       end
     end

spec/lib/gitlab/auth_spec.rb

@@ -261,7 +261,7 @@ describe Gitlab::Auth do
       let(:auth_failure) { Gitlab::Auth::Result.new(nil, nil) }
 
       context 'when the deploy token has read_repository as scope' do
-        let(:deploy_token) { create(:deploy_token, :read_repository, project: project) }
+        let(:deploy_token) { create(:deploy_token, read_registry: false, projects: [project]) }
 
         it 'succeeds when project is present, token is valid and has read_repository as scope' do
           abilities = %i(read_project download_code)
@@ -284,13 +284,6 @@ describe Gitlab::Auth do
             .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!
 
@@ -302,7 +295,7 @@ describe Gitlab::Auth do
       end
 
       context 'when the deploy token has read_registry as a scope' do
-        let(:deploy_token) { create(:deploy_token, :read_registry, project: project) }
+        let(:deploy_token) { create(:deploy_token, read_repository: false, projects: [project]) }
 
         context 'when registry enabled' do
           before do

spec/lib/gitlab/git_access_spec.rb

@@ -147,25 +147,17 @@ describe Gitlab::GitAccess do
         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) }
+          let(:project_deploy_token) { create(:project_deploy_token, project: project) }
+          let(:actor) { project_deploy_token.deploy_token }
 
+          context 'when DeployToken is active and belongs to project' do
             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) }
+            let(:actor) { create(:deploy_token) }
 
             it 'blocks pull access' do
               expect { pull_access_check }.to raise_not_found

spec/models/deploy_token_spec.rb

 require 'spec_helper'
 
 describe DeployToken do
-  let(:deploy_token) { create(:deploy_token) }
+  subject(:deploy_token) { create(:deploy_token) }
 
-  it { is_expected.to belong_to :project }
-  it { is_expected.to validate_presence_of :project }
+  it { is_expected.to have_many :project_deploy_tokens }
+  it { is_expected.to have_many(:projects).through(:project_deploy_tokens) }
 
-  describe 'validations' do
-    context 'with no scopes defined' do
-      it 'should not be valid' do
-        deploy_token.scopes = []
+  describe '#ensure_token' do
+    it 'should ensure a token' do
+      deploy_token.token = nil
+      deploy_token.save
+
+      expect(deploy_token.token).not_to be_empty
+    end
+  end
+
+  describe '#ensure_at_least_one_scope' do
+    context 'with at least one scope' do
+      it 'should be valid' do
+        is_expected.to be_valid
+      end
+    end
+
+    context 'with no scopes' do
+      it 'should be invalid' do
+        deploy_token = build(:deploy_token, read_repository: false, read_registry: false)
 
         expect(deploy_token).not_to be_valid
-        expect(deploy_token.errors[:scopes].first).to eq("can't be blank")
+        expect(deploy_token.errors[:base].first).to eq("Scopes can't be blank")
       end
     end
   end
 
-  describe '#ensure_token' do
-    it 'should ensure a token' do
-      deploy_token.token = nil
-      deploy_token.save
+  describe '#scopes' do
+    context 'with all the scopes' do
+      it 'should return scopes assigned to DeployToken' do
+        expect(deploy_token.scopes).to eq([:read_repository, :read_registry])
+      end
+    end
 
-      expect(deploy_token.token).not_to be_empty
+    context 'with only one scope' do
+      it 'should return scopes assigned to DeployToken' do
+        deploy_token = create(:deploy_token, read_registry: false)
+        expect(deploy_token.scopes).to eq([:read_repository])
+      end
     end
   end
 
@@ -50,8 +71,7 @@ describe DeployToken do
 
   describe '#username' do
     it 'returns Ghost username' do
-      ghost = User.ghost
-      expect(deploy_token.username).to eq(ghost.username)
+      expect(deploy_token.username).to eq("gitlab+deploy-token-#{deploy_token.id}")
     end
   end
 end

spec/models/project_deploy_token_spec.rb

+require 'rails_helper'
+
+RSpec.describe ProjectDeployToken, type: :model do
+  let(:project) { create(:project) }
+  let(:deploy_token) { create(:deploy_token) }
+  subject(:project_deploy_token) { create(:project_deploy_token, project: project, deploy_token: deploy_token) }
+
+  it { is_expected.to belong_to :project }
+  it { is_expected.to belong_to :deploy_token }
+  it { is_expected.to accept_nested_attributes_for :deploy_token }
+
+  it { is_expected.to validate_presence_of :deploy_token }
+  it { is_expected.to validate_presence_of :project }
+  it { is_expected.to validate_uniqueness_of(:deploy_token_id).scoped_to(:project_id) }
+end

spec/models/project_spec.rb

@@ -84,6 +84,8 @@ describe Project do
     it { is_expected.to have_many(:custom_attributes).class_name('ProjectCustomAttribute') }
     it { is_expected.to have_many(:project_badges).class_name('ProjectBadge') }
     it { is_expected.to have_many(:lfs_file_locks) }
+    it { is_expected.to have_many(:project_deploy_tokens) }
+    it { is_expected.to have_many(:deploy_tokens).through(:project_deploy_tokens) }
 
     context 'after initialized' do
       it "has a project_feature" do

spec/presenters/projects/settings/deploy_tokens_presenter_spec.rb

@@ -3,25 +3,11 @@ require 'spec_helper'
 describe Projects::Settings::DeployTokensPresenter do
   let(:user) { create(:user) }
   let(:project) { create(:project) }
-  let(:deploy_tokens) { create_list(:deploy_token, 3, project: project) }
+  let!(:project_deploy_tokens) { create_list(:project_deploy_token, 3, project: project) }
+  let(:deploy_tokens) { project.deploy_tokens }
 
   subject(:presenter) { described_class.new(deploy_tokens, current_user: user, project: project) }
 
-  describe '#available_scopes' do
-    it 'returns the all the deploy token scopes' do
-      expect(presenter.available_scopes).to match_array(%w(read_repository read_registry))
-    end
-  end
-
-  describe '#scope_description' do
-    let(:deploy_token) { create(:deploy_token, project: project, scopes: [:read_registry]) }
-
-    it 'returns the description for a given scope' do
-      description = 'Allows read-only access to the registry images'
-      expect(presenter.scope_description('read_registry')).to eq(description)
-    end
-  end
-
   describe '#length' do
     it 'returns the size of deploy tokens presented' do
       expect(presenter.length).to eq(3)

spec/services/auth/container_registry_authentication_service_spec.rb

@@ -558,6 +558,7 @@ describe Auth::ContainerRegistryAuthenticationService do
       let(:project) { create(:project, :public) }
 
       context 'when pulling and pushing' do
+        let(:current_user) { create(:deploy_token, projects: [project]) }
         let(:current_params) do
           { scope: "repository:#{project.full_path}:pull,push" }
         end

spec/services/deploy_tokens/create_service_spec.rb

@@ -13,42 +13,50 @@ describe DeployTokens::CreateService, :clean_gitlab_redis_shared_state do
         expect { subject }.to change { DeployToken.count }.by(1)
       end
 
-      it 'returns a DeployToken' do
-        expect(subject).to be_an_instance_of DeployToken
+      it 'should create a new ProjectDeployToken' do
+        expect { subject }.to change { ProjectDeployToken.count }.by(1)
       end
 
-      it 'should assign the DeployToken to the project' do
-        expect(subject.project).to eq(project)
+      it 'returns a DeployToken' do
+        expect(subject).to be_an_instance_of DeployToken
       end
 
       it 'should store the token on redis' do
-        redis_key = subject.redis_shared_state_key(user.id)
+        redis_key = DeployToken.redis_shared_state_key(user.id)
+        subject
 
         expect(Gitlab::Redis::SharedState.with { |redis| redis.get(redis_key) }).not_to be_nil
       end
 
       it 'should not store deploy token attributes on redis' do
-        redis_key = subject.redis_shared_state_key(user.id) + ":attributes"
+        redis_key = DeployToken.redis_shared_state_key(user.id) + ":attributes"
+        subject
 
         expect(Gitlab::Redis::SharedState.with { |redis| redis.get(redis_key) }).to be_nil
       end
     end
 
     context 'when the deploy token is invalid' do
-      let(:deploy_token_params) { attributes_for(:deploy_token, scopes: []) }
+      let(:deploy_token_params) { attributes_for(:deploy_token, read_repository: false, read_registry: false) }
 
-      it 'it should not create a new DeployToken' do
+      it 'should not create a new DeployToken' do
         expect { subject }.not_to change { DeployToken.count }
       end
 
+      it 'should not create a new ProjectDeployToken' do
+        expect { subject }.not_to change { ProjectDeployToken.count }
+      end
+
       it 'should not store the token on redis' do
-        redis_key = subject.redis_shared_state_key(user.id)
+        redis_key = DeployToken.redis_shared_state_key(user.id)
+        subject
 
         expect(Gitlab::Redis::SharedState.with { |redis| redis.get(redis_key) }).to be_nil
       end
 
       it 'should store deploy token attributes on redis' do
-        redis_key = subject.redis_shared_state_key(user.id) + ":attributes"
+        redis_key = DeployToken.redis_shared_state_key(user.id) + ":attributes"
+        subject
 
         expect(Gitlab::Redis::SharedState.with { |redis| redis.get(redis_key) }).not_to be_nil
       end