From 208cf512e4f6534a7d9bc97b08678788d9f5a922 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Jonas=20W=C3=A4lter?= <waelter@outlook.de>
Date: Wed, 28 Dec 2022 14:30:20 +0100
Subject: [PATCH 1/3] Add `admin_mode` scope for PATs

1) Add background migration to add the `admin_mode` scope to active
personal access tokens of administrators
2) Automatically add the `admin_mode` scope if a personal access token
for an administrator is created

Changelog: added
---
 app/models/personal_access_token.rb           | 19 ++++++-
 ...n_mode_scope_for_personal_access_tokens.rb | 21 ++++++++
 db/schema_migrations/20221228103133           |  1 +
 lib/gitlab/auth.rb                            |  7 ++-
 ...n_mode_scope_for_personal_access_tokens.rb | 28 ++++++++++
 spec/lib/gitlab/auth_spec.rb                  |  8 +--
 ...e_scope_for_personal_access_tokens_spec.rb | 53 +++++++++++++++++++
 ...e_scope_for_personal_access_tokens_spec.rb | 18 +++++++
 spec/models/personal_access_token_spec.rb     | 23 ++++++++
 9 files changed, 172 insertions(+), 6 deletions(-)
 create mode 100644 db/post_migrate/20221228103133_queue_backfill_admin_mode_scope_for_personal_access_tokens.rb
 create mode 100644 db/schema_migrations/20221228103133
 create mode 100644 lib/gitlab/background_migration/backfill_admin_mode_scope_for_personal_access_tokens.rb
 create mode 100644 spec/lib/gitlab/background_migration/backfill_admin_mode_scope_for_personal_access_tokens_spec.rb
 create mode 100644 spec/migrations/queue_backfill_admin_mode_scope_for_personal_access_tokens_spec.rb

diff --git a/app/models/personal_access_token.rb b/app/models/personal_access_token.rb
index 887ef36cc17fcd..89614a2571896e 100644
--- a/app/models/personal_access_token.rb
+++ b/app/models/personal_access_token.rb
@@ -21,6 +21,11 @@ class PersonalAccessToken < ApplicationRecord
   after_initialize :set_default_scopes, if: :persisted?
   before_save :ensure_token
 
+  # During the implementation of Admin Mode for API, tokens of
+  # administrators should automatically get the `admin_mode` scope as well
+  # See https://gitlab.com/gitlab-org/gitlab/-/issues/42692
+  before_create :add_admin_mode_scope, if: :user_admin?
+
   scope :active, -> { not_revoked.not_expired }
   scope :expiring_and_not_notified, ->(date) { where(["revoked = false AND expire_notification_delivered = false AND expires_at >= CURRENT_DATE AND expires_at <= ?", date]) }
   scope :expired_today_and_not_notified, -> { where(["revoked = false AND expires_at = CURRENT_DATE AND after_expiry_notification_delivered = false"]) }
@@ -79,7 +84,11 @@ def project_access_token?
   protected
 
   def validate_scopes
-    unless revoked || scopes.all? { |scope| Gitlab::Auth.all_available_scopes.include?(scope.to_sym) }
+    # During the implementation of Admin Mode for API, this scope shall not yet be available for selection on creation.
+    # See https://gitlab.com/gitlab-org/gitlab/-/issues/42692
+    valid_scopes = Gitlab::Auth.all_available_scopes - [Gitlab::Auth::ADMIN_MODE_SCOPE]
+
+    unless revoked || scopes.all? { |scope| valid_scopes.include?(scope.to_sym) }
       errors.add :scopes, "can only contain available scopes"
     end
   end
@@ -91,6 +100,14 @@ def set_default_scopes
 
     self.scopes = Gitlab::Auth::DEFAULT_SCOPES if self.scopes.empty?
   end
+
+  def user_admin?
+    user.admin? # rubocop: disable Cop/UserAdmin
+  end
+
+  def add_admin_mode_scope
+    self.scopes += [Gitlab::Auth::ADMIN_MODE_SCOPE.to_s]
+  end
 end
 
 PersonalAccessToken.prepend_mod_with('PersonalAccessToken')
diff --git a/db/post_migrate/20221228103133_queue_backfill_admin_mode_scope_for_personal_access_tokens.rb b/db/post_migrate/20221228103133_queue_backfill_admin_mode_scope_for_personal_access_tokens.rb
new file mode 100644
index 00000000000000..c111d5090e1c79
--- /dev/null
+++ b/db/post_migrate/20221228103133_queue_backfill_admin_mode_scope_for_personal_access_tokens.rb
@@ -0,0 +1,21 @@
+# frozen_string_literal: true
+
+class QueueBackfillAdminModeScopeForPersonalAccessTokens < Gitlab::Database::Migration[2.1]
+  MIGRATION = 'BackfillAdminModeScopeForPersonalAccessTokens'
+  DELAY_INTERVAL = 2.minutes
+
+  restrict_gitlab_migration gitlab_schema: :gitlab_main
+
+  def up
+    queue_batched_background_migration(
+      MIGRATION,
+      :personal_access_tokens,
+      :id,
+      job_interval: DELAY_INTERVAL
+    )
+  end
+
+  def down
+    delete_batched_background_migration(MIGRATION, :personal_access_tokens, :id, [])
+  end
+end
diff --git a/db/schema_migrations/20221228103133 b/db/schema_migrations/20221228103133
new file mode 100644
index 00000000000000..757c1d9169d8bd
--- /dev/null
+++ b/db/schema_migrations/20221228103133
@@ -0,0 +1 @@
+59e19291b3f8bb08dd63c1b1993af8f75e06d56ca776c3e8711adcc8c5c26e86
\ No newline at end of file
diff --git a/lib/gitlab/auth.rb b/lib/gitlab/auth.rb
index 7e8f9c76dea645..d3407c1488feb5 100644
--- a/lib/gitlab/auth.rb
+++ b/lib/gitlab/auth.rb
@@ -31,7 +31,8 @@ module Auth
 
     # Scopes used for GitLab as admin
     SUDO_SCOPE = :sudo
-    ADMIN_SCOPES = [SUDO_SCOPE].freeze
+    ADMIN_MODE_SCOPE = :admin_mode
+    ADMIN_SCOPES = [SUDO_SCOPE, ADMIN_MODE_SCOPE].freeze
 
     # Default scopes for OAuth applications that don't define their own
     DEFAULT_SCOPES = [API_SCOPE].freeze
@@ -366,6 +367,10 @@ def full_authentication_abilities
       def available_scopes_for(current_user)
         scopes = non_admin_available_scopes
         scopes += ADMIN_SCOPES if current_user.admin?
+
+        # During the implementation of Admin Mode for API, this scope shall not yet be available for selection by user.
+        # See https://gitlab.com/gitlab-org/gitlab/-/issues/42692
+        scopes.delete(ADMIN_MODE_SCOPE)
         scopes
       end
 
diff --git a/lib/gitlab/background_migration/backfill_admin_mode_scope_for_personal_access_tokens.rb b/lib/gitlab/background_migration/backfill_admin_mode_scope_for_personal_access_tokens.rb
new file mode 100644
index 00000000000000..82e607ac7a77e7
--- /dev/null
+++ b/lib/gitlab/background_migration/backfill_admin_mode_scope_for_personal_access_tokens.rb
@@ -0,0 +1,28 @@
+# frozen_string_literal: true
+
+module Gitlab
+  module BackgroundMigration
+    # Backfill `admin_mode` scope for a range of personal access tokens
+    class BackfillAdminModeScopeForPersonalAccessTokens < ::Gitlab::BackgroundMigration::BatchedMigrationJob
+      scope_to ->(relation) do
+        relation.joins('INNER JOIN users ON personal_access_tokens.user_id = users.id')
+                .where(users: { admin: true })
+                .where(revoked: [false, nil])
+                .where.not('expires_at IS NOT NULL AND expires_at <= ?', Time.current)
+      end
+
+      operation_name :update_all
+      feature_category :authentication_and_authorization
+
+      ADMIN_MODE_SCOPE = ['admin_mode'].freeze
+
+      def perform
+        each_sub_batch do |sub_batch|
+          sub_batch.each do |token|
+            token.update!(scopes: (YAML.safe_load(token.scopes) + ADMIN_MODE_SCOPE).uniq.to_yaml)
+          end
+        end
+      end
+    end
+  end
+end
diff --git a/spec/lib/gitlab/auth_spec.rb b/spec/lib/gitlab/auth_spec.rb
index 5a6fa7c416ba48..f379b3d6bb5c3c 100644
--- a/spec/lib/gitlab/auth_spec.rb
+++ b/spec/lib/gitlab/auth_spec.rb
@@ -2,7 +2,7 @@
 
 require 'spec_helper'
 
-RSpec.describe Gitlab::Auth, :use_clean_rails_memory_store_caching do
+RSpec.describe Gitlab::Auth, :use_clean_rails_memory_store_caching, feature_category: :authentication_and_authorization do
   let_it_be(:project) { create(:project) }
 
   let(:auth_failure) { { actor: nil, project: nil, type: nil, authentication_abilities: nil } }
@@ -14,7 +14,7 @@
     end
 
     it 'ADMIN_SCOPES contains all scopes for ADMIN access' do
-      expect(subject::ADMIN_SCOPES).to match_array %i[sudo]
+      expect(subject::ADMIN_SCOPES).to match_array %i[sudo admin_mode]
     end
 
     it 'REPOSITORY_SCOPES contains all scopes for REPOSITORY access' do
@@ -32,7 +32,7 @@
     it 'optional_scopes contains all non-default scopes' do
       stub_container_registry_config(enabled: true)
 
-      expect(subject.optional_scopes).to match_array %i[read_user read_api read_repository write_repository read_registry write_registry sudo openid profile email]
+      expect(subject.optional_scopes).to match_array %i[read_user read_api read_repository write_repository read_registry write_registry sudo admin_mode openid profile email]
     end
   end
 
@@ -40,7 +40,7 @@
     it 'contains all non-default scopes' do
       stub_container_registry_config(enabled: true)
 
-      expect(subject.all_available_scopes).to match_array %i[api read_user read_api read_repository write_repository read_registry write_registry sudo]
+      expect(subject.all_available_scopes).to match_array %i[api read_user read_api read_repository write_repository read_registry write_registry sudo admin_mode]
     end
 
     it 'contains for non-admin user all non-default scopes without ADMIN access' do
diff --git a/spec/lib/gitlab/background_migration/backfill_admin_mode_scope_for_personal_access_tokens_spec.rb b/spec/lib/gitlab/background_migration/backfill_admin_mode_scope_for_personal_access_tokens_spec.rb
new file mode 100644
index 00000000000000..7075d4694aecdc
--- /dev/null
+++ b/spec/lib/gitlab/background_migration/backfill_admin_mode_scope_for_personal_access_tokens_spec.rb
@@ -0,0 +1,53 @@
+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe Gitlab::BackgroundMigration::BackfillAdminModeScopeForPersonalAccessTokens,
+  :migration, schema: 20221228103133, feature_category: :authentication_and_authorization do
+  let(:users) { table(:users) }
+  let(:personal_access_tokens) { table(:personal_access_tokens) }
+
+  let(:admin) { users.create!(name: 'admin', email: 'admin@example.com', projects_limit: 1, admin: true) }
+  let(:user) { users.create!(name: 'user', email: 'user@example.com', projects_limit: 1) }
+
+  let!(:pat_admin_1) { personal_access_tokens.create!(name: 'admin 1', user_id: admin.id, scopes: "---\n- api\n") }
+  let!(:pat_user) { personal_access_tokens.create!(name: 'user 1', user_id: user.id, scopes: "---\n- api\n") }
+  let!(:pat_revoked) do
+    personal_access_tokens.create!(name: 'admin 2', user_id: admin.id, scopes: "---\n- api\n", revoked: true)
+  end
+
+  let!(:pat_expired) do
+    personal_access_tokens.create!(name: 'admin 3', user_id: admin.id, scopes: "---\n- api\n", expires_at: 1.day.ago)
+  end
+
+  let!(:pat_admin_mode) do
+    personal_access_tokens.create!(name: 'admin 4', user_id: admin.id, scopes: "---\n- admin_mode\n")
+  end
+
+  let!(:pat_admin_2) { personal_access_tokens.create!(name: 'admin 5', user_id: admin.id, scopes: "---\n- read_api\n") }
+  let!(:pat_not_in_range) { personal_access_tokens.create!(name: 'admin 6', user_id: admin.id, scopes: "---\n- api\n") }
+
+  subject do
+    described_class.new(
+      start_id: pat_admin_1.id,
+      end_id: pat_admin_2.id,
+      batch_table: :personal_access_tokens,
+      batch_column: :id,
+      sub_batch_size: 1,
+      pause_ms: 0,
+      connection: ApplicationRecord.connection
+    )
+  end
+
+  it "adds `admin_mode` scope to active personal access tokens of administrators" do
+    subject.perform
+
+    expect(pat_admin_1.reload.scopes).to eq("---\n- api\n- admin_mode\n")
+    expect(pat_user.reload.scopes).to eq("---\n- api\n")
+    expect(pat_revoked.reload.scopes).to eq("---\n- api\n")
+    expect(pat_expired.reload.scopes).to eq("---\n- api\n")
+    expect(pat_admin_mode.reload.scopes).to eq("---\n- admin_mode\n")
+    expect(pat_admin_2.reload.scopes).to eq("---\n- read_api\n- admin_mode\n")
+    expect(pat_not_in_range.reload.scopes).to eq("---\n- api\n")
+  end
+end
diff --git a/spec/migrations/queue_backfill_admin_mode_scope_for_personal_access_tokens_spec.rb b/spec/migrations/queue_backfill_admin_mode_scope_for_personal_access_tokens_spec.rb
new file mode 100644
index 00000000000000..8209f3175501bb
--- /dev/null
+++ b/spec/migrations/queue_backfill_admin_mode_scope_for_personal_access_tokens_spec.rb
@@ -0,0 +1,18 @@
+# frozen_string_literal: true
+
+require 'spec_helper'
+require_migration!
+
+RSpec.describe QueueBackfillAdminModeScopeForPersonalAccessTokens,
+  feature_category: :authentication_and_authorization do
+  describe '#up' do
+    it 'schedules background migration' do
+      migrate!
+
+      expect(described_class::MIGRATION).to have_scheduled_batched_migration(
+        table_name: :personal_access_tokens,
+        column_name: :id,
+        interval: described_class::DELAY_INTERVAL)
+    end
+  end
+end
diff --git a/spec/models/personal_access_token_spec.rb b/spec/models/personal_access_token_spec.rb
index 9d4c53f8d55518..3c43086cb2bf3b 100644
--- a/spec/models/personal_access_token_spec.rb
+++ b/spec/models/personal_access_token_spec.rb
@@ -340,4 +340,27 @@
       end
     end
   end
+
+  # During the implementation of Admin Mode for API, tokens of
+  # administrators should automatically get the `admin_mode` scope as well
+  # See https://gitlab.com/gitlab-org/gitlab/-/issues/42692
+  describe '`admin_mode scope' do
+    subject { create(:personal_access_token, user: user, scopes: ['api']) }
+
+    context 'with administrator user' do
+      let_it_be(:user) { create(:user, :admin) }
+
+      it 'adds `admin_mode` scope before created' do
+        expect(subject.scopes).to contain_exactly('api', 'admin_mode')
+      end
+    end
+
+    context 'with normal user' do
+      let_it_be(:user) { create(:user) }
+
+      it 'does not add `admin_mode` scope before created' do
+        expect(subject.scopes).to contain_exactly('api')
+      end
+    end
+  end
 end
-- 
GitLab


From db90bc485c4850a4945270b71c7239cf2e622690 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Jonas=20W=C3=A4lter?= <waelter@outlook.de>
Date: Wed, 28 Dec 2022 16:55:25 +0100
Subject: [PATCH 2/3] Do not add `admin_mode` to all_available_scopes yet

---
 app/models/personal_access_token.rb | 6 +-----
 lib/gitlab/auth.rb                  | 5 +----
 spec/lib/gitlab/auth_spec.rb        | 8 ++++----
 3 files changed, 6 insertions(+), 13 deletions(-)

diff --git a/app/models/personal_access_token.rb b/app/models/personal_access_token.rb
index 89614a2571896e..e19eb7a4d46aef 100644
--- a/app/models/personal_access_token.rb
+++ b/app/models/personal_access_token.rb
@@ -84,11 +84,7 @@ def project_access_token?
   protected
 
   def validate_scopes
-    # During the implementation of Admin Mode for API, this scope shall not yet be available for selection on creation.
-    # See https://gitlab.com/gitlab-org/gitlab/-/issues/42692
-    valid_scopes = Gitlab::Auth.all_available_scopes - [Gitlab::Auth::ADMIN_MODE_SCOPE]
-
-    unless revoked || scopes.all? { |scope| valid_scopes.include?(scope.to_sym) }
+    unless revoked || scopes.all? { |scope| Gitlab::Auth.all_available_scopes.include?(scope.to_sym) }
       errors.add :scopes, "can only contain available scopes"
     end
   end
diff --git a/lib/gitlab/auth.rb b/lib/gitlab/auth.rb
index d3407c1488feb5..c97ef5a10ef0b4 100644
--- a/lib/gitlab/auth.rb
+++ b/lib/gitlab/auth.rb
@@ -32,7 +32,7 @@ module Auth
     # Scopes used for GitLab as admin
     SUDO_SCOPE = :sudo
     ADMIN_MODE_SCOPE = :admin_mode
-    ADMIN_SCOPES = [SUDO_SCOPE, ADMIN_MODE_SCOPE].freeze
+    ADMIN_SCOPES = [SUDO_SCOPE].freeze
 
     # Default scopes for OAuth applications that don't define their own
     DEFAULT_SCOPES = [API_SCOPE].freeze
@@ -368,9 +368,6 @@ def available_scopes_for(current_user)
         scopes = non_admin_available_scopes
         scopes += ADMIN_SCOPES if current_user.admin?
 
-        # During the implementation of Admin Mode for API, this scope shall not yet be available for selection by user.
-        # See https://gitlab.com/gitlab-org/gitlab/-/issues/42692
-        scopes.delete(ADMIN_MODE_SCOPE)
         scopes
       end
 
diff --git a/spec/lib/gitlab/auth_spec.rb b/spec/lib/gitlab/auth_spec.rb
index f379b3d6bb5c3c..5a6fa7c416ba48 100644
--- a/spec/lib/gitlab/auth_spec.rb
+++ b/spec/lib/gitlab/auth_spec.rb
@@ -2,7 +2,7 @@
 
 require 'spec_helper'
 
-RSpec.describe Gitlab::Auth, :use_clean_rails_memory_store_caching, feature_category: :authentication_and_authorization do
+RSpec.describe Gitlab::Auth, :use_clean_rails_memory_store_caching do
   let_it_be(:project) { create(:project) }
 
   let(:auth_failure) { { actor: nil, project: nil, type: nil, authentication_abilities: nil } }
@@ -14,7 +14,7 @@
     end
 
     it 'ADMIN_SCOPES contains all scopes for ADMIN access' do
-      expect(subject::ADMIN_SCOPES).to match_array %i[sudo admin_mode]
+      expect(subject::ADMIN_SCOPES).to match_array %i[sudo]
     end
 
     it 'REPOSITORY_SCOPES contains all scopes for REPOSITORY access' do
@@ -32,7 +32,7 @@
     it 'optional_scopes contains all non-default scopes' do
       stub_container_registry_config(enabled: true)
 
-      expect(subject.optional_scopes).to match_array %i[read_user read_api read_repository write_repository read_registry write_registry sudo admin_mode openid profile email]
+      expect(subject.optional_scopes).to match_array %i[read_user read_api read_repository write_repository read_registry write_registry sudo openid profile email]
     end
   end
 
@@ -40,7 +40,7 @@
     it 'contains all non-default scopes' do
       stub_container_registry_config(enabled: true)
 
-      expect(subject.all_available_scopes).to match_array %i[api read_user read_api read_repository write_repository read_registry write_registry sudo admin_mode]
+      expect(subject.all_available_scopes).to match_array %i[api read_user read_api read_repository write_repository read_registry write_registry sudo]
     end
 
     it 'contains for non-admin user all non-default scopes without ADMIN access' do
-- 
GitLab


From c692263491f8c87f6dbc8ab289f49a91788309cd Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Jonas=20W=C3=A4lter?= <waelter@outlook.de>
Date: Tue, 10 Jan 2023 10:37:12 +0100
Subject: [PATCH 3/3] Make PATs with `admin_mode` scope valid

---
 app/models/personal_access_token.rb       | 7 ++++++-
 spec/models/personal_access_token_spec.rb | 8 +++++++-
 2 files changed, 13 insertions(+), 2 deletions(-)

diff --git a/app/models/personal_access_token.rb b/app/models/personal_access_token.rb
index e19eb7a4d46aef..0da205f86a53b2 100644
--- a/app/models/personal_access_token.rb
+++ b/app/models/personal_access_token.rb
@@ -84,7 +84,12 @@ def project_access_token?
   protected
 
   def validate_scopes
-    unless revoked || scopes.all? { |scope| Gitlab::Auth.all_available_scopes.include?(scope.to_sym) }
+    # During the implementation of Admin Mode for API,
+    # the `admin_mode` scope is not yet part of `all_available_scopes` but still valid.
+    # See https://gitlab.com/gitlab-org/gitlab/-/issues/42692
+    valid_scopes = Gitlab::Auth.all_available_scopes + [Gitlab::Auth::ADMIN_MODE_SCOPE]
+
+    unless revoked || scopes.all? { |scope| valid_scopes.include?(scope.to_sym) }
       errors.add :scopes, "can only contain available scopes"
     end
   end
diff --git a/spec/models/personal_access_token_spec.rb b/spec/models/personal_access_token_spec.rb
index 3c43086cb2bf3b..f65b5ff824b8b0 100644
--- a/spec/models/personal_access_token_spec.rb
+++ b/spec/models/personal_access_token_spec.rb
@@ -2,7 +2,7 @@
 
 require 'spec_helper'
 
-RSpec.describe PersonalAccessToken do
+RSpec.describe PersonalAccessToken, feature_category: :authentication_and_authorization do
   subject { described_class }
 
   describe '.build' do
@@ -210,6 +210,12 @@
       expect(personal_access_token).to be_valid
     end
 
+    it "allows creating a token with `admin_mode` scope" do
+      personal_access_token.scopes = [:api, :admin_mode]
+
+      expect(personal_access_token).to be_valid
+    end
+
     context 'when registry is disabled' do
       before do
         stub_container_registry_config(enabled: false)
-- 
GitLab