Merge branch '21765-group-token-architecture' into 'master'

Add group deploy tokens

See merge request gitlab-org/gitlab!23460

app/models/deploy_token.rb

@@ -15,6 +15,11 @@ class DeployToken < ApplicationRecord
   has_many :project_deploy_tokens, inverse_of: :deploy_token
   has_many :projects, through: :project_deploy_tokens
 
+  has_many :group_deploy_tokens, inverse_of: :deploy_token
+  has_many :groups, through: :group_deploy_tokens
+
+  validate :no_groups, unless: :group_type?
+  validate :no_projects, unless: :project_type?
   validate :ensure_at_least_one_scope
   validates :username,
     length: { maximum: 255 },
@@ -24,6 +29,7 @@ class DeployToken < ApplicationRecord
       message: "can contain only letters, digits, '_', '-', '+', and '.'"
     }
 
+  validates :deploy_token_type, presence: true
   enum deploy_token_type: {
     group_type: 1,
     project_type: 2
@@ -56,18 +62,31 @@ class DeployToken < ApplicationRecord
   end
 
   def has_access_to?(requested_project)
-    active? && project == requested_project
+    return false unless active?
+    return false unless holder
+
+    holder.has_access_to?(requested_project)
   end
 
   # This is temporal. Currently we limit DeployToken
-  # to a single project, later we're going to extend
-  # that to be for multiple projects and namespaces.
+  # to a single project or group, later we're going to
+  # extend that to be for multiple projects and namespaces.
   def project
     strong_memoize(:project) do
       projects.first
     end
   end
 
+  def holder
+    strong_memoize(:holder) do
+      if project_type?
+        project_deploy_tokens.first
+      elsif group_type?
+        group_deploy_tokens.first
+      end
+    end
+  end
+
   def expires_at
     expires_at = read_attribute(:expires_at)
     expires_at != Forever.date ? expires_at : nil
@@ -92,4 +111,12 @@ class DeployToken < ApplicationRecord
   def default_username
     "gitlab+deploy-token-#{id}" if persisted?
   end
+
+  def no_groups
+    errors.add(:deploy_token, 'cannot have groups assigned') if group_deploy_tokens.any?
+  end
+
+  def no_projects
+    errors.add(:deploy_token, 'cannot have projects assigned') if project_deploy_tokens.any?
+  end
 end

app/models/group_deploy_token.rb

+# frozen_string_literal: true
+
+class GroupDeployToken < ApplicationRecord
+  belongs_to :group, class_name: '::Group'
+  belongs_to :deploy_token, inverse_of: :group_deploy_tokens
+
+  validates :deploy_token, presence: true
+  validates :group, presence: true
+  validates :deploy_token_id, uniqueness: { scope: [:group_id] }
+
+  def has_access_to?(requested_project)
+    return false unless Feature.enabled?(:allow_group_deploy_token, default: true)
+
+    requested_project_group = requested_project&.group
+    return false unless requested_project_group
+    return true if requested_project_group.id == group_id
+
+    requested_project_group
+      .ancestors
+      .where(id: group_id)
+      .exists?
+  end
+end

app/models/project_deploy_token.rb

@@ -7,4 +7,8 @@ class ProjectDeployToken < ApplicationRecord
   validates :deploy_token, presence: true
   validates :project, presence: true
   validates :deploy_token_id, uniqueness: { scope: [:project_id] }
+
+  def has_access_to?(requested_project)
+    requested_project == project
+  end
 end

changelogs/unreleased/21765-group-token-architecture.yml

+---
+title: Update deploy token architecture to introduce group-level deploy tokens.
+merge_request: 23460
+author:
+type: added

db/migrate/20200121200203_create_group_deploy_tokens.rb

+# frozen_string_literal: true
+
+class CreateGroupDeployTokens < ActiveRecord::Migration[5.2]
+  DOWNTIME = false
+
+  def change
+    create_table :group_deploy_tokens do |t|
+      t.timestamps_with_timezone null: false
+
+      t.references :group, index: false, null: false, foreign_key: { to_table: :namespaces, on_delete: :cascade }
+      t.references :deploy_token, null: false, foreign_key: { on_delete: :cascade }
+
+      t.index [:group_id, :deploy_token_id], unique: true, name: 'index_group_deploy_tokens_on_group_and_deploy_token_ids'
+    end
+  end
+end

db/schema.rb

@@ -1979,6 +1979,15 @@ ActiveRecord::Schema.define(version: 2020_02_04_131054) do
     t.index ["user_id"], name: "index_group_deletion_schedules_on_user_id"
   end
 
+  create_table "group_deploy_tokens", force: :cascade do |t|
+    t.datetime_with_timezone "created_at", null: false
+    t.datetime_with_timezone "updated_at", null: false
+    t.bigint "group_id", null: false
+    t.bigint "deploy_token_id", null: false
+    t.index ["deploy_token_id"], name: "index_group_deploy_tokens_on_deploy_token_id"
+    t.index ["group_id", "deploy_token_id"], name: "index_group_deploy_tokens_on_group_and_deploy_token_ids", unique: true
+  end
+
   create_table "group_group_links", force: :cascade do |t|
     t.datetime_with_timezone "created_at", null: false
     t.datetime_with_timezone "updated_at", null: false
@@ -4691,6 +4700,8 @@ ActiveRecord::Schema.define(version: 2020_02_04_131054) do
   add_foreign_key "group_custom_attributes", "namespaces", column: "group_id", on_delete: :cascade
   add_foreign_key "group_deletion_schedules", "namespaces", column: "group_id", on_delete: :cascade
   add_foreign_key "group_deletion_schedules", "users", name: "fk_11e3ebfcdd", on_delete: :cascade
+  add_foreign_key "group_deploy_tokens", "deploy_tokens", on_delete: :cascade
+  add_foreign_key "group_deploy_tokens", "namespaces", column: "group_id", on_delete: :cascade
   add_foreign_key "group_group_links", "namespaces", column: "shared_group_id", on_delete: :cascade
   add_foreign_key "group_group_links", "namespaces", column: "shared_with_group_id", on_delete: :cascade
   add_foreign_key "identities", "saml_providers", name: "fk_aade90f0fc", on_delete: :cascade

lib/gitlab/auth.rb

@@ -49,7 +49,7 @@ module Gitlab
           lfs_token_check(login, password, project) ||
           oauth_access_token_check(login, password) ||
           personal_access_token_check(password) ||
-          deploy_token_check(login, password) ||
+          deploy_token_check(login, password, project) ||
           user_with_password_for_git(login, password) ||
           Gitlab::Auth::Result.new
 
@@ -208,7 +208,7 @@ module Gitlab
         end.uniq
       end
 
-      def deploy_token_check(login, password)
+      def deploy_token_check(login, password, project)
         return unless password.present?
 
         token = DeployToken.active.find_by_token(password)
@@ -219,7 +219,7 @@ module Gitlab
         scopes = abilities_for_scopes(token.scopes)
 
         if valid_scoped_token?(token, all_available_scopes)
-          Gitlab::Auth::Result.new(token, token.project, :deploy_token, scopes)
+          Gitlab::Auth::Result.new(token, project, :deploy_token, scopes)
         end
       end
 

spec/factories/deploy_tokens.rb

@@ -9,6 +9,7 @@ FactoryBot.define do
     read_registry { true }
     revoked { false }
     expires_at { 5.days.from_now }
+    deploy_token_type { DeployToken.deploy_token_types[:project_type] }
 
     trait :revoked do
       revoked { true }
@@ -21,5 +22,13 @@ FactoryBot.define do
     trait :expired do
       expires_at { Date.today - 1.month }
     end
+
+    trait :group do
+      deploy_token_type { DeployToken.deploy_token_types[:group_type] }
+    end
+
+    trait :project do
+      deploy_token_type { DeployToken.deploy_token_types[:project_type] }
+    end
   end
 end

spec/factories/group_deploy_tokens.rb

+# frozen_string_literal: true
+
+FactoryBot.define do
+  factory :group_deploy_token do
+    group
+    deploy_token
+  end
+end

spec/lib/gitlab/auth_spec.rb

@@ -460,6 +460,20 @@ describe Gitlab::Auth, :use_clean_rails_memory_store_caching do
         end
       end
 
+      context 'when the deploy token is of group type' do
+        let(:project_with_group) { create(:project, group: create(:group)) }
+        let(:deploy_token) { create(:deploy_token, :group, read_repository: true, groups: [project_with_group.group]) }
+        let(:login) { deploy_token.username }
+
+        subject { gl_auth.find_for_git_client(login, deploy_token.token, project: project_with_group, ip: 'ip') }
+
+        it 'succeeds when login and a group deploy token are valid' do
+          auth_success = Gitlab::Auth::Result.new(deploy_token, project_with_group, :deploy_token, [:download_code, :read_container_image])
+
+          expect(subject).to eq(auth_success)
+        end
+      end
+
       context 'when the deploy token has read_registry as a scope' do
         let(:deploy_token) { create(:deploy_token, read_repository: false, projects: [project]) }
         let(:login) { deploy_token.username }
@@ -469,10 +483,10 @@ describe Gitlab::Auth, :use_clean_rails_memory_store_caching do
             stub_container_registry_config(enabled: true)
           end
 
-          it 'succeeds when login and token are valid' do
+          it 'succeeds when login and a project token are valid' do
             auth_success = Gitlab::Auth::Result.new(deploy_token, project, :deploy_token, [:read_container_image])
 
-            expect(gl_auth.find_for_git_client(login, deploy_token.token, project: nil, ip: 'ip'))
+            expect(gl_auth.find_for_git_client(login, deploy_token.token, project: project, ip: 'ip'))
               .to eq(auth_success)
           end
 

spec/models/deploy_token_spec.rb

@@ -7,6 +7,8 @@ describe DeployToken do
 
   it { is_expected.to have_many :project_deploy_tokens }
   it { is_expected.to have_many(:projects).through(:project_deploy_tokens) }
+  it { is_expected.to have_many :group_deploy_tokens }
+  it { is_expected.to have_many(:groups).through(:group_deploy_tokens) }
 
   it_behaves_like 'having unique enum values'
 
@@ -17,6 +19,29 @@ describe DeployToken do
     it { is_expected.to allow_value('GitLab+deploy_token-3.14').for(:username) }
     it { is_expected.not_to allow_value('<script>').for(:username).with_message(username_format_message) }
     it { is_expected.not_to allow_value('').for(:username).with_message(username_format_message) }
+    it { is_expected.to validate_presence_of(:deploy_token_type) }
+  end
+
+  describe 'deploy_token_type validations' do
+    context 'when a deploy token is associated to a group' do
+      it 'does not allow setting a project to it' do
+        group_token = create(:deploy_token, :group)
+        group_token.projects << build(:project)
+
+        expect(group_token).not_to be_valid
+        expect(group_token.errors.full_messages).to include('Deploy token cannot have projects assigned')
+      end
+    end
+
+    context 'when a deploy token is associated to a project' do
+      it 'does not allow setting a group to it' do
+        project_token = create(:deploy_token)
+        project_token.groups << build(:group)
+
+        expect(project_token).not_to be_valid
+        expect(project_token.errors.full_messages).to include('Deploy token cannot have groups assigned')
+      end
+    end
   end
 
   describe '#ensure_token' do
@@ -125,33 +150,148 @@ describe DeployToken do
     end
   end
 
+  describe '#holder' do
+    subject { deploy_token.holder }
+
+    context 'when the token is of project type' do
+      it 'returns the relevant holder token' do
+        expect(subject).to eq(deploy_token.project_deploy_tokens.first)
+      end
+    end
+
+    context 'when the token is of group type' do
+      let(:group) { create(:group) }
+      let(:deploy_token) { create(:deploy_token, :group) }
+
+      it 'returns the relevant holder token' do
+        expect(subject).to eq(deploy_token.group_deploy_tokens.first)
+      end
+    end
+  end
+
   describe '#has_access_to?' do
     let(:project) { create(:project) }
 
     subject { deploy_token.has_access_to?(project) }
 
-    context 'when deploy token is active and related to project' do
-      let(:deploy_token) { create(:deploy_token, projects: [project]) }
+    context 'when a project is not passed in' do
+      let(:project) { nil }
 
-      it { is_expected.to be_truthy }
+      it { is_expected.to be_falsy }
     end
 
-    context 'when deploy token is active but not related to project' do
-      let(:deploy_token) { create(:deploy_token) }
+    context 'when a project is passed in' do
+      context 'when deploy token is active and related to project' do
+        let(:deploy_token) { create(:deploy_token, projects: [project]) }
 
-      it { is_expected.to be_falsy }
-    end
+        it { is_expected.to be_truthy }
+      end
 
-    context 'when deploy token is revoked and related to project' do
-      let(:deploy_token) { create(:deploy_token, :revoked, projects: [project]) }
+      context 'when deploy token is active but not related to project' do
+        let(:deploy_token) { create(:deploy_token) }
 
-      it { is_expected.to be_falsy }
-    end
+        it { is_expected.to be_falsy }
+      end
 
-    context 'when deploy token is revoked and not related to the project' do
-      let(:deploy_token) { create(:deploy_token, :revoked) }
+      context 'when deploy token is revoked and related to project' do
+        let(:deploy_token) { create(:deploy_token, :revoked, projects: [project]) }
 
-      it { is_expected.to be_falsy }
+        it { is_expected.to be_falsy }
+      end
+
+      context 'when deploy token is revoked and not related to the project' do
+        let(:deploy_token) { create(:deploy_token, :revoked) }
+
+        it { is_expected.to be_falsy }
+      end
+
+      context 'and when the token is of group type' do
+        let_it_be(:group) { create(:group) }
+        let(:deploy_token) { create(:deploy_token, :group) }
+
+        before do
+          deploy_token.groups << group
+        end
+
+        context 'and the allow_group_deploy_token feature flag is turned off' do
+          it 'is false' do
+            stub_feature_flags(allow_group_deploy_token: false)
+
+            is_expected.to be_falsy
+          end
+        end
+
+        context 'and the allow_group_deploy_token feature flag is turned on' do
+          before do
+            stub_feature_flags(allow_group_deploy_token: true)
+          end
+
+          context 'and the passed-in project does not belong to any group' do
+            it { is_expected.to be_falsy }
+          end
+
+          context 'and the passed-in project belongs to the token group' do
+            it 'is true' do
+              group.projects << project
+
+              is_expected.to be_truthy
+            end
+          end
+
+          context 'and the passed-in project belongs to a subgroup' do
+            let(:child_group) { create(:group, parent_id: group.id) }
+            let(:grandchild_group) { create(:group, parent_id: child_group.id) }
+
+            before do
+              grandchild_group.projects << project
+            end
+
+            context 'and the token group is an ancestor (grand-parent) of this group' do
+              it { is_expected.to be_truthy }
+            end
+
+            context 'and the token group is not ancestor of this group' do
+              let(:child2_group) { create(:group, parent_id: group.id) }
+
+              it 'is false' do
+                deploy_token.groups = [child2_group]
+
+                is_expected.to be_falsey
+              end
+            end
+          end
+
+          context 'and the passed-in project does not belong to the token group' do
+            it { is_expected.to be_falsy }
+          end
+
+          context 'and the project belongs to a group that is parent of the token group' do
+            let(:super_group) { create(:group) }
+            let(:deploy_token) { create(:deploy_token, :group) }
+            let(:group) { create(:group, parent_id: super_group.id) }
+
+            it 'is false' do
+              super_group.projects << project
+
+              is_expected.to be_falsey
+            end
+          end
+        end
+      end
+
+      context 'and the token is of project type' do
+        let(:deploy_token) { create(:deploy_token, projects: [project]) }
+
+        context 'and the passed-in project is the same as the token project' do
+          it { is_expected.to be_truthy }
+        end
+
+        context 'and the passed-in project is not the same as the token project' do
+          subject { deploy_token.has_access_to?(create(:project)) }
+
+          it { is_expected.to be_falsey }
+        end
+      end
     end
   end
 
@@ -183,7 +323,7 @@ describe DeployToken do
       end
     end
 
-    context 'when passign a value' do
+    context 'when passing a value' do
       let(:expires_at) { Date.today + 5.months }
       let(:deploy_token) { create(:deploy_token, expires_at: expires_at) }
 

spec/models/group_deploy_token_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe GroupDeployToken, type: :model do
+  let(:group) { create(:group) }
+  let(:deploy_token) { create(:deploy_token) }
+
+  subject(:group_deploy_token) { create(:group_deploy_token, group: group, deploy_token: deploy_token) }
+
+  it { is_expected.to belong_to :group }
+  it { is_expected.to belong_to :deploy_token }
+
+  it { is_expected.to validate_presence_of :deploy_token }
+  it { is_expected.to validate_presence_of :group }
+  it { is_expected.to validate_uniqueness_of(:deploy_token_id).scoped_to(:group_id) }
+end