From 3d0843c6de65e5faa3a8635d41f2d6f84c27d17d Mon Sep 17 00:00:00 2001
From: Nicholas Wittstruck <1494491-nwittstruck@users.noreply.gitlab.com>
Date: Sat, 19 Oct 2024 13:46:06 +0200
Subject: [PATCH 1/6] Refactor Token Identification

This commit merges the token identification of the
AgnosticTokenRevocationService and the Token Information API.

Issue: https://gitlab.com/gitlab-org/gitlab/-/issues/499247

Changelog: other
---
 .../agnostic_token_revocation_service.rb      | 42 +++++++++----------
 lib/api/admin/token.rb                        | 27 ++++++------
 lib/authn/agnostic_token_handler.rb           | 34 +++++++++++++++
 spec/lib/authn/agnostic_token_handler_spec.rb | 34 +++++++++++++++
 4 files changed, 100 insertions(+), 37 deletions(-)
 create mode 100644 lib/authn/agnostic_token_handler.rb
 create mode 100644 spec/lib/authn/agnostic_token_handler_spec.rb

diff --git a/app/services/groups/agnostic_token_revocation_service.rb b/app/services/groups/agnostic_token_revocation_service.rb
index db481c5419e496..1cf642425c7950 100644
--- a/app/services/groups/agnostic_token_revocation_service.rb
+++ b/app/services/groups/agnostic_token_revocation_service.rb
@@ -17,6 +17,8 @@
 # This Service returns a ServiceResponse object.
 module Groups # rubocop:disable Gitlab/BoundedContexts -- This service is strictly related to groups
   class AgnosticTokenRevocationService < Groups::BaseService
+    include ::Authn::AgnosticTokenHandler
+
     AUDIT_SOURCE = :group_token_revocation_service
 
     attr_reader :revocable
@@ -32,26 +34,7 @@ def execute
       return error("Group cannot be a subgroup") if group.subgroup?
       return error("Unauthorized") unless can?(current_user, :admin_group, group)
 
-      # Determine the type of token
-      if plaintext.start_with?(Gitlab::CurrentSettings.current_application_settings.personal_access_token_prefix,
-        ApplicationSetting.defaults[:personal_access_token_prefix])
-        @revocable = PersonalAccessToken.find_by_token(plaintext)
-        return error('PAT not found') unless revocable
-
-        handle_personal_access_token
-      elsif plaintext.start_with?(DeployToken::DEPLOY_TOKEN_PREFIX)
-        @revocable = DeployToken.find_by_token(plaintext)
-        return error('DeployToken not found') unless revocable && revocable.group_type?
-
-        handle_deploy_token
-      elsif plaintext.start_with?(User::FEED_TOKEN_PREFIX)
-        @revocable = User.find_by_feed_token(plaintext)
-        return error('Feed Token not found') unless revocable
-
-        handle_feed_token
-      else
-        error('Unsupported token type')
-      end
+      handle_plaintext(plaintext)
     end
 
     private
@@ -74,7 +57,10 @@ def error(message)
       ServiceResponse.error(message: message)
     end
 
-    def handle_personal_access_token
+    def handle_personal_access_token(token)
+      @revocable = PersonalAccessToken.find_by_token(token)
+      return error('PAT not found') unless revocable
+
       if user_has_group_membership?(revocable.user)
         # Only revoke active tokens. (Ignore expired tokens)
         if revocable.active?
@@ -112,7 +98,10 @@ def user_has_group_membership?(user)
         .any?
     end
 
-    def handle_deploy_token
+    def handle_deploy_token(token)
+      @revocable = DeployToken.find_by_token(token)
+      return error('DeployToken not found') unless revocable && revocable.group_type?
+
       if group.self_and_descendants.include?(revocable.group)
         if revocable.active?
           service = ::Groups::DeployTokens::RevokeService.new(
@@ -131,7 +120,10 @@ def handle_deploy_token
       error('DeployToken revocation failed')
     end
 
-    def handle_feed_token
+    def handle_feed_token(token)
+      @revocable = User.find_by_feed_token(token)
+      return error('Feed Token not found') unless revocable
+
       if user_has_group_membership?(revocable)
         current_token = revocable.feed_token
 
@@ -154,5 +146,9 @@ def handle_feed_token
       #  - rotation failed for some reason
       error('Feed token revocation failed')
     end
+
+    def handle_token_type_not_supported
+      error('Unsupported token type')
+    end
   end
 end
diff --git a/lib/api/admin/token.rb b/lib/api/admin/token.rb
index 9c521397114e92..427d155a7c6919 100644
--- a/lib/api/admin/token.rb
+++ b/lib/api/admin/token.rb
@@ -6,23 +6,22 @@ class Token < ::API::Base
       feature_category :system_access
 
       helpers do
-        def identify_token(token)
-          if token.start_with?(Gitlab::CurrentSettings.current_application_settings.personal_access_token_prefix,
-            ApplicationSetting.defaults[:personal_access_token_prefix])
-            handle_personal_access_token(token)
-          elsif token.start_with?(DeployToken::DEPLOY_TOKEN_PREFIX)
-            handle_deploy_token(token)
-          else
-            raise ArgumentError, 'Token type not supported.'
-          end
+        include ::Authn::AgnosticTokenHandler
+
+        def handle_personal_access_token(plaintext)
+          PersonalAccessToken.find_by_token(plaintext)
+        end
+
+        def handle_deploy_token(plaintext)
+          DeployToken.find_by_token(plaintext)
         end
 
-        def handle_personal_access_token(token)
-          PersonalAccessToken.find_by_token(token)
+        def handle_feed_token(_plaintext)
+          handle_token_type_not_supported
         end
 
-        def handle_deploy_token(token)
-          DeployToken.find_by_token(token)
+        def handle_token_type_not_supported
+          raise ArgumentError, 'Token type not supported.'
         end
       end
 
@@ -55,7 +54,7 @@ def handle_deploy_token(token)
           requires :token, type: String, desc: 'The token that information is requested about.'
         end
         post 'token' do
-          identified_token = identify_token(params[:token])
+          identified_token = handle_plaintext(params[:token])
 
           render_api_error!({ error: 'Not found' }, :not_found) if identified_token.nil?
 
diff --git a/lib/authn/agnostic_token_handler.rb b/lib/authn/agnostic_token_handler.rb
new file mode 100644
index 00000000000000..88b4003bf94ebe
--- /dev/null
+++ b/lib/authn/agnostic_token_handler.rb
@@ -0,0 +1,34 @@
+# frozen_string_literal:true
+
+module Authn
+  module AgnosticTokenHandler
+    def handle_plaintext(plaintext)
+      if plaintext.start_with?(Gitlab::CurrentSettings.current_application_settings.personal_access_token_prefix,
+        ApplicationSetting.defaults[:personal_access_token_prefix])
+        handle_personal_access_token(plaintext)
+      elsif plaintext.start_with?(DeployToken::DEPLOY_TOKEN_PREFIX)
+        handle_deploy_token(plaintext)
+      elsif plaintext.start_with?(User::FEED_TOKEN_PREFIX)
+        handle_feed_token(plaintext)
+      else
+        handle_token_type_not_supported
+      end
+    end
+
+    def handle_personal_access_token(plaintext)
+      raise NotImplementedError
+    end
+
+    def handle_deploy_token(plaintext)
+      raise NotImplementedError
+    end
+
+    def handle_feed_token(plaintext)
+      raise NotImplementedError
+    end
+
+    def handle_token_type_not_supported
+      raise NotImplementedError
+    end
+  end
+end
diff --git a/spec/lib/authn/agnostic_token_handler_spec.rb b/spec/lib/authn/agnostic_token_handler_spec.rb
new file mode 100644
index 00000000000000..d310c9b94c5eae
--- /dev/null
+++ b/spec/lib/authn/agnostic_token_handler_spec.rb
@@ -0,0 +1,34 @@
+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe Authn::AgnosticTokenHandler, feature_category: :system_access do
+  using RSpec::Parameterized::TableSyntax
+
+  let_it_be(:user) { create(:user) }
+  let_it_be(:deploy_token) { create(:deploy_token).token }
+  let_it_be(:feed_token) { user.feed_token }
+  let_it_be(:personal_access_token) { create(:personal_access_token, user: user).token }
+
+  context 'when module is included' do
+    subject(:instance) { Class.new.include(described_class).new }
+
+    where(:plaintext, :method) do
+      ref(:personal_access_token) | :handle_personal_access_token
+      ref(:feed_token) | :handle_feed_token
+      ref(:deploy_token) | :handle_deploy_token
+      'unsupported' | :handle_token_type_not_supported
+    end
+
+    with_them do
+      it 'calls the correct method' do
+        expect(instance).to receive(method)
+        instance.handle_plaintext(plaintext)
+      end
+
+      it 'returns NotImplementedError' do
+        expect { instance.handle_plaintext(plaintext) }.to raise_error NotImplementedError
+      end
+    end
+  end
+end
-- 
GitLab


From 8352c48d2efc7211bdbd45129ca0e95666f2ed37 Mon Sep 17 00:00:00 2001
From: Nicholas Wittstruck <1494491-nwittstruck@users.noreply.gitlab.com>
Date: Sun, 20 Oct 2024 14:44:41 +0200
Subject: [PATCH 2/6] Refactor Token Identification

This commit merges the token identification of the
AgnosticTokenRevocationService and the Token Information API.

It is based on https://gitlab.com/gitlab-org/gitlab/-/merge_requests/169827
by https://gitlab.com/mokhax

Issue: https://gitlab.com/gitlab-org/gitlab/-/issues/499247

Changelog: other
---
 .../agnostic_token_revocation_service.rb      | 55 ++++++++-----------
 lib/api/admin/token.rb                        | 21 ++-----
 lib/authn/agnostic_token_handler.rb           | 34 ------------
 lib/authn/token.rb                            | 28 ++++++++++
 lib/authn/tokens/deploy_token.rb              | 27 +++++++++
 lib/authn/tokens/feed_token.rb                | 25 +++++++++
 lib/authn/tokens/personal_access_token.rb     | 28 ++++++++++
 spec/lib/authn/agnostic_token_handler_spec.rb | 34 ------------
 8 files changed, 136 insertions(+), 116 deletions(-)
 delete mode 100644 lib/authn/agnostic_token_handler.rb
 create mode 100644 lib/authn/token.rb
 create mode 100644 lib/authn/tokens/deploy_token.rb
 create mode 100644 lib/authn/tokens/feed_token.rb
 create mode 100644 lib/authn/tokens/personal_access_token.rb
 delete mode 100644 spec/lib/authn/agnostic_token_handler_spec.rb

diff --git a/app/services/groups/agnostic_token_revocation_service.rb b/app/services/groups/agnostic_token_revocation_service.rb
index 1cf642425c7950..d46580b9d91b9b 100644
--- a/app/services/groups/agnostic_token_revocation_service.rb
+++ b/app/services/groups/agnostic_token_revocation_service.rb
@@ -17,11 +17,9 @@
 # This Service returns a ServiceResponse object.
 module Groups # rubocop:disable Gitlab/BoundedContexts -- This service is strictly related to groups
   class AgnosticTokenRevocationService < Groups::BaseService
-    include ::Authn::AgnosticTokenHandler
-
     AUDIT_SOURCE = :group_token_revocation_service
 
-    attr_reader :revocable
+    attr_reader :token
 
     def initialize(group, current_user, plaintext)
       @group = group
@@ -34,7 +32,19 @@ def execute
       return error("Group cannot be a subgroup") if group.subgroup?
       return error("Unauthorized") unless can?(current_user, :admin_group, group)
 
-      handle_plaintext(plaintext)
+      @token = ::Authn::Token.new(plaintext)
+
+      # Perform checks based on token type and group scope:
+      case token.revocable
+      when ::Authn::Tokens::PersonalAccessToken
+        handle_personal_access_token
+      when ::Authn::Tokens::DeployToken
+        handle_deploy_token
+      when ::Authn::Tokens::FeedToken
+        handle_feed_token
+      else
+        error('Unsupported token type')
+      end
     end
 
     private
@@ -57,19 +67,13 @@ def error(message)
       ServiceResponse.error(message: message)
     end
 
-    def handle_personal_access_token(token)
-      @revocable = PersonalAccessToken.find_by_token(token)
+    def handle_personal_access_token
+      revocable = token.revocable.target
       return error('PAT not found') unless revocable
 
       if user_has_group_membership?(revocable.user)
         # Only revoke active tokens. (Ignore expired tokens)
-        if revocable.active?
-          ::PersonalAccessTokens::RevokeService.new(
-            current_user,
-            token: revocable,
-            source: AUDIT_SOURCE
-          ).execute
-        end
+        token.revoke!(current_user) if revocable.active?
 
         # Always validate that, if we're returning token info, it
         # has been successfully revoked
@@ -98,21 +102,12 @@ def user_has_group_membership?(user)
         .any?
     end
 
-    def handle_deploy_token(token)
-      @revocable = DeployToken.find_by_token(token)
+    def handle_deploy_token
+      revocable = token.revocable.target
       return error('DeployToken not found') unless revocable && revocable.group_type?
 
       if group.self_and_descendants.include?(revocable.group)
-        if revocable.active?
-          service = ::Groups::DeployTokens::RevokeService.new(
-            revocable.group,
-            current_user,
-            { id: revocable.id }
-          )
-
-          service.source = AUDIT_SOURCE
-          service.execute
-        end
+        token.revoke!(current_user) if revocable.active?
 
         return success(revocable, 'DeployToken') if revocable.reset.revoked?
       end
@@ -120,18 +115,14 @@ def handle_deploy_token(token)
       error('DeployToken revocation failed')
     end
 
-    def handle_feed_token(token)
-      @revocable = User.find_by_feed_token(token)
+    def handle_feed_token
+      revocable = token.revocable.target
       return error('Feed Token not found') unless revocable
 
       if user_has_group_membership?(revocable)
         current_token = revocable.feed_token
 
-        response = Users::ResetFeedTokenService.new(
-          current_user,
-          user: revocable,
-          source: AUDIT_SOURCE
-        ).execute
+        response = token.revoke!(current_user)
 
         # Always validate that, if we're returning token info, it
         # has been successfully revoked. Feed tokens can only be rotated
diff --git a/lib/api/admin/token.rb b/lib/api/admin/token.rb
index 427d155a7c6919..71cce5402f5c7b 100644
--- a/lib/api/admin/token.rb
+++ b/lib/api/admin/token.rb
@@ -6,22 +6,11 @@ class Token < ::API::Base
       feature_category :system_access
 
       helpers do
-        include ::Authn::AgnosticTokenHandler
+        def identify_token(plaintext)
+          token = ::Authn::Token.new(plaintext)
+          raise ArgumentError, 'Token type not supported.' if token.revocable.blank?
 
-        def handle_personal_access_token(plaintext)
-          PersonalAccessToken.find_by_token(plaintext)
-        end
-
-        def handle_deploy_token(plaintext)
-          DeployToken.find_by_token(plaintext)
-        end
-
-        def handle_feed_token(_plaintext)
-          handle_token_type_not_supported
-        end
-
-        def handle_token_type_not_supported
-          raise ArgumentError, 'Token type not supported.'
+          token.target
         end
       end
 
@@ -54,7 +43,7 @@ def handle_token_type_not_supported
           requires :token, type: String, desc: 'The token that information is requested about.'
         end
         post 'token' do
-          identified_token = handle_plaintext(params[:token])
+          identified_token = identify_token(params[:token])
 
           render_api_error!({ error: 'Not found' }, :not_found) if identified_token.nil?
 
diff --git a/lib/authn/agnostic_token_handler.rb b/lib/authn/agnostic_token_handler.rb
deleted file mode 100644
index 88b4003bf94ebe..00000000000000
--- a/lib/authn/agnostic_token_handler.rb
+++ /dev/null
@@ -1,34 +0,0 @@
-# frozen_string_literal:true
-
-module Authn
-  module AgnosticTokenHandler
-    def handle_plaintext(plaintext)
-      if plaintext.start_with?(Gitlab::CurrentSettings.current_application_settings.personal_access_token_prefix,
-        ApplicationSetting.defaults[:personal_access_token_prefix])
-        handle_personal_access_token(plaintext)
-      elsif plaintext.start_with?(DeployToken::DEPLOY_TOKEN_PREFIX)
-        handle_deploy_token(plaintext)
-      elsif plaintext.start_with?(User::FEED_TOKEN_PREFIX)
-        handle_feed_token(plaintext)
-      else
-        handle_token_type_not_supported
-      end
-    end
-
-    def handle_personal_access_token(plaintext)
-      raise NotImplementedError
-    end
-
-    def handle_deploy_token(plaintext)
-      raise NotImplementedError
-    end
-
-    def handle_feed_token(plaintext)
-      raise NotImplementedError
-    end
-
-    def handle_token_type_not_supported
-      raise NotImplementedError
-    end
-  end
-end
diff --git a/lib/authn/token.rb b/lib/authn/token.rb
new file mode 100644
index 00000000000000..99f57fac00ba35
--- /dev/null
+++ b/lib/authn/token.rb
@@ -0,0 +1,28 @@
+# frozen_string_literal:true
+
+module Authn
+  class Token
+    Error = Class.new(StandardError)
+    TOKEN_TYPES = [
+      ::Authn::Tokens::DeployToken,
+      ::Authn::Tokens::FeedToken,
+      ::Authn::Tokens::PersonalAccessToken
+    ].freeze
+
+    attr_reader :revocable
+
+    def initialize(plaintext)
+      @revocable = TOKEN_TYPES.find { |x| x.prefix?(plaintext) }&.new(plaintext)
+    end
+
+    def target
+      revocable&.target
+    end
+
+    def revoke!(current_user)
+      raise ::Authn::Token::Error, "Not Found" if target.blank?
+
+      revocable.revoke!(current_user)
+    end
+  end
+end
diff --git a/lib/authn/tokens/deploy_token.rb b/lib/authn/tokens/deploy_token.rb
new file mode 100644
index 00000000000000..63db45390a1a6c
--- /dev/null
+++ b/lib/authn/tokens/deploy_token.rb
@@ -0,0 +1,27 @@
+# frozen_string_literal:true
+
+module Authn
+  module Tokens
+    class DeployToken
+      def self.prefix?(plaintext)
+        plaintext.start_with?(::DeployToken::DEPLOY_TOKEN_PREFIX)
+      end
+
+      attr_reader :target
+
+      def initialize(plaintext)
+        @target = ::DeployToken.find_by_token(plaintext)
+      end
+
+      def revoke!(current_user)
+        service = ::Groups::DeployTokens::RevokeService.new(
+          target.group,
+          current_user,
+          { id: target.id }
+        )
+        service.source = :group_token_revocation_service
+        service.execute
+      end
+    end
+  end
+end
diff --git a/lib/authn/tokens/feed_token.rb b/lib/authn/tokens/feed_token.rb
new file mode 100644
index 00000000000000..6fa39dcaaf9b17
--- /dev/null
+++ b/lib/authn/tokens/feed_token.rb
@@ -0,0 +1,25 @@
+# frozen_string_literal:true
+
+module Authn
+  module Tokens
+    class FeedToken
+      def self.prefix?(plaintext)
+        plaintext.start_with?(::User::FEED_TOKEN_PREFIX)
+      end
+
+      attr_reader :target
+
+      def initialize(plaintext)
+        @target = User.find_by_feed_token(plaintext)
+      end
+
+      def revoke!(current_user)
+        Users::ResetFeedTokenService.new(
+          current_user,
+          user: target,
+          source: :group_token_revocation_service
+        ).execute
+      end
+    end
+  end
+end
diff --git a/lib/authn/tokens/personal_access_token.rb b/lib/authn/tokens/personal_access_token.rb
new file mode 100644
index 00000000000000..22d786c71c09e3
--- /dev/null
+++ b/lib/authn/tokens/personal_access_token.rb
@@ -0,0 +1,28 @@
+# frozen_string_literal:true
+
+module Authn
+  module Tokens
+    class PersonalAccessToken
+      def self.prefix?(plaintext)
+        plaintext.start_with?(
+          ::PersonalAccessToken.token_prefix,
+          ApplicationSetting.defaults[:personal_access_token_prefix]
+        )
+      end
+
+      attr_reader :target
+
+      def initialize(plaintext)
+        @target = ::PersonalAccessToken.find_by_token(plaintext)
+      end
+
+      def revoke!(current_user)
+        ::PersonalAccessTokens::RevokeService.new(
+          current_user,
+          token: target,
+          source: :group_token_revocation_service
+        ).execute
+      end
+    end
+  end
+end
diff --git a/spec/lib/authn/agnostic_token_handler_spec.rb b/spec/lib/authn/agnostic_token_handler_spec.rb
deleted file mode 100644
index d310c9b94c5eae..00000000000000
--- a/spec/lib/authn/agnostic_token_handler_spec.rb
+++ /dev/null
@@ -1,34 +0,0 @@
-# frozen_string_literal: true
-
-require 'spec_helper'
-
-RSpec.describe Authn::AgnosticTokenHandler, feature_category: :system_access do
-  using RSpec::Parameterized::TableSyntax
-
-  let_it_be(:user) { create(:user) }
-  let_it_be(:deploy_token) { create(:deploy_token).token }
-  let_it_be(:feed_token) { user.feed_token }
-  let_it_be(:personal_access_token) { create(:personal_access_token, user: user).token }
-
-  context 'when module is included' do
-    subject(:instance) { Class.new.include(described_class).new }
-
-    where(:plaintext, :method) do
-      ref(:personal_access_token) | :handle_personal_access_token
-      ref(:feed_token) | :handle_feed_token
-      ref(:deploy_token) | :handle_deploy_token
-      'unsupported' | :handle_token_type_not_supported
-    end
-
-    with_them do
-      it 'calls the correct method' do
-        expect(instance).to receive(method)
-        instance.handle_plaintext(plaintext)
-      end
-
-      it 'returns NotImplementedError' do
-        expect { instance.handle_plaintext(plaintext) }.to raise_error NotImplementedError
-      end
-    end
-  end
-end
-- 
GitLab


From b604c77f9230214c81e9bc3d21c1dca14a0abf36 Mon Sep 17 00:00:00 2001
From: Nicholas Wittstruck <1494491-nwittstruck@users.noreply.gitlab.com>
Date: Tue, 22 Oct 2024 11:22:27 +0200
Subject: [PATCH 3/6] Refactor Token Identification

This commit merges the token identification of the
AgnosticTokenRevocationService and the Token Information API.

This commit adds tests to improve test coverage for the new token types.

Issue: https://gitlab.com/gitlab-org/gitlab/-/issues/499247

Changelog: other
---
 .../agnostic_token_revocation_service.rb      |  8 +--
 lib/api/admin/token.rb                        |  2 +-
 lib/authn/token.rb                            |  4 +-
 lib/authn/tokens/deploy_token.rb              |  9 ++-
 lib/authn/tokens/feed_token.rb                |  9 ++-
 lib/authn/tokens/personal_access_token.rb     |  9 ++-
 spec/lib/authn/token_spec.rb                  | 57 +++++++++++++++++++
 spec/lib/authn/tokens/deploy_token_spec.rb    | 47 +++++++++++++++
 spec/lib/authn/tokens/feed_token_spec.rb      | 46 +++++++++++++++
 .../tokens/personal_access_token_spec.rb      | 46 +++++++++++++++
 10 files changed, 221 insertions(+), 16 deletions(-)
 create mode 100644 spec/lib/authn/token_spec.rb
 create mode 100644 spec/lib/authn/tokens/deploy_token_spec.rb
 create mode 100644 spec/lib/authn/tokens/feed_token_spec.rb
 create mode 100644 spec/lib/authn/tokens/personal_access_token_spec.rb

diff --git a/app/services/groups/agnostic_token_revocation_service.rb b/app/services/groups/agnostic_token_revocation_service.rb
index d46580b9d91b9b..d2ce84843f896e 100644
--- a/app/services/groups/agnostic_token_revocation_service.rb
+++ b/app/services/groups/agnostic_token_revocation_service.rb
@@ -32,7 +32,7 @@ def execute
       return error("Group cannot be a subgroup") if group.subgroup?
       return error("Unauthorized") unless can?(current_user, :admin_group, group)
 
-      @token = ::Authn::Token.new(plaintext)
+      @token = ::Authn::Token.new(plaintext, :group_token_revocation_service)
 
       # Perform checks based on token type and group scope:
       case token.revocable
@@ -68,7 +68,7 @@ def error(message)
     end
 
     def handle_personal_access_token
-      revocable = token.revocable.target
+      revocable = token.target
       return error('PAT not found') unless revocable
 
       if user_has_group_membership?(revocable.user)
@@ -103,7 +103,7 @@ def user_has_group_membership?(user)
     end
 
     def handle_deploy_token
-      revocable = token.revocable.target
+      revocable = token.target
       return error('DeployToken not found') unless revocable && revocable.group_type?
 
       if group.self_and_descendants.include?(revocable.group)
@@ -116,7 +116,7 @@ def handle_deploy_token
     end
 
     def handle_feed_token
-      revocable = token.revocable.target
+      revocable = token.target
       return error('Feed Token not found') unless revocable
 
       if user_has_group_membership?(revocable)
diff --git a/lib/api/admin/token.rb b/lib/api/admin/token.rb
index 71cce5402f5c7b..23ac7c71871090 100644
--- a/lib/api/admin/token.rb
+++ b/lib/api/admin/token.rb
@@ -7,7 +7,7 @@ class Token < ::API::Base
 
       helpers do
         def identify_token(plaintext)
-          token = ::Authn::Token.new(plaintext)
+          token = ::Authn::Token.new(plaintext, :api_admin_token)
           raise ArgumentError, 'Token type not supported.' if token.revocable.blank?
 
           token.target
diff --git a/lib/authn/token.rb b/lib/authn/token.rb
index 99f57fac00ba35..245b927fb31bf0 100644
--- a/lib/authn/token.rb
+++ b/lib/authn/token.rb
@@ -11,8 +11,8 @@ class Token
 
     attr_reader :revocable
 
-    def initialize(plaintext)
-      @revocable = TOKEN_TYPES.find { |x| x.prefix?(plaintext) }&.new(plaintext)
+    def initialize(plaintext, source)
+      @revocable = TOKEN_TYPES.find { |x| x.prefix?(plaintext) }&.new(plaintext, source)
     end
 
     def target
diff --git a/lib/authn/tokens/deploy_token.rb b/lib/authn/tokens/deploy_token.rb
index 63db45390a1a6c..8dcb0cc61682ef 100644
--- a/lib/authn/tokens/deploy_token.rb
+++ b/lib/authn/tokens/deploy_token.rb
@@ -7,19 +7,22 @@ def self.prefix?(plaintext)
         plaintext.start_with?(::DeployToken::DEPLOY_TOKEN_PREFIX)
       end
 
-      attr_reader :target
+      attr_reader :target, :source
 
-      def initialize(plaintext)
+      def initialize(plaintext, source)
         @target = ::DeployToken.find_by_token(plaintext)
+        @source = source
       end
 
       def revoke!(current_user)
+        raise ::Authn::Token::Error, "Not Found" if target.blank?
+
         service = ::Groups::DeployTokens::RevokeService.new(
           target.group,
           current_user,
           { id: target.id }
         )
-        service.source = :group_token_revocation_service
+        service.source = source
         service.execute
       end
     end
diff --git a/lib/authn/tokens/feed_token.rb b/lib/authn/tokens/feed_token.rb
index 6fa39dcaaf9b17..077f4365426745 100644
--- a/lib/authn/tokens/feed_token.rb
+++ b/lib/authn/tokens/feed_token.rb
@@ -7,17 +7,20 @@ def self.prefix?(plaintext)
         plaintext.start_with?(::User::FEED_TOKEN_PREFIX)
       end
 
-      attr_reader :target
+      attr_reader :target, :source
 
-      def initialize(plaintext)
+      def initialize(plaintext, source)
         @target = User.find_by_feed_token(plaintext)
+        @source = source
       end
 
       def revoke!(current_user)
+        raise ::Authn::Token::Error, "Not Found" if target.blank?
+
         Users::ResetFeedTokenService.new(
           current_user,
           user: target,
-          source: :group_token_revocation_service
+          source: source
         ).execute
       end
     end
diff --git a/lib/authn/tokens/personal_access_token.rb b/lib/authn/tokens/personal_access_token.rb
index 22d786c71c09e3..805daefcd48423 100644
--- a/lib/authn/tokens/personal_access_token.rb
+++ b/lib/authn/tokens/personal_access_token.rb
@@ -10,17 +10,20 @@ def self.prefix?(plaintext)
         )
       end
 
-      attr_reader :target
+      attr_reader :target, :source
 
-      def initialize(plaintext)
+      def initialize(plaintext, source)
         @target = ::PersonalAccessToken.find_by_token(plaintext)
+        @source = source
       end
 
       def revoke!(current_user)
+        raise ::Authn::Token::Error, "Not Found" if target.blank?
+
         ::PersonalAccessTokens::RevokeService.new(
           current_user,
           token: target,
-          source: :group_token_revocation_service
+          source: source
         ).execute
       end
     end
diff --git a/spec/lib/authn/token_spec.rb b/spec/lib/authn/token_spec.rb
new file mode 100644
index 00000000000000..e64dfab711c41f
--- /dev/null
+++ b/spec/lib/authn/token_spec.rb
@@ -0,0 +1,57 @@
+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe Authn::Token, feature_category: :system_access do
+  using RSpec::Parameterized::TableSyntax
+
+  let_it_be(:user) { create(:user) }
+  let_it_be(:deploy_token) { create(:deploy_token).token }
+  let_it_be(:feed_token) { user.feed_token }
+  let_it_be(:personal_access_token) { create(:personal_access_token, user: user).token }
+  let_it_be(:unsupported_token) { 'unsupported' }
+
+  let(:token) { described_class.new(plaintext, :group_token_revocation_service) }
+
+  context 'with supported token types' do
+    where(:plaintext, :token_type) do
+      ref(:personal_access_token) | ::Authn::Tokens::PersonalAccessToken
+      ref(:feed_token) | ::Authn::Tokens::FeedToken
+      ref(:deploy_token) | ::Authn::Tokens::DeployToken
+    end
+
+    with_them do
+      describe '#initialize' do
+        it 'sets the correct target token type' do
+          expect(token.revocable).to be_instance_of(token_type)
+        end
+      end
+
+      describe '#revoke!' do
+        it 'calls revoke on the token' do
+          expect(token.revocable).to receive(:revoke!)
+          token.revoke!(user)
+        end
+      end
+    end
+  end
+
+  context 'with unsupported token types' do
+    let(:plaintext) { unsupported_token }
+
+    describe '#initialize' do
+      it 'is nil when the token type is not supported' do
+        expect(token.revocable).to be_nil
+      end
+    end
+
+    describe '#revoke!' do
+      it 'raises error when the token type is not found' do
+        expect do
+          token.revoke!(user)
+        end
+          .to raise_error("Not Found")
+      end
+    end
+  end
+end
diff --git a/spec/lib/authn/tokens/deploy_token_spec.rb b/spec/lib/authn/tokens/deploy_token_spec.rb
new file mode 100644
index 00000000000000..49f5edb2f50fd5
--- /dev/null
+++ b/spec/lib/authn/tokens/deploy_token_spec.rb
@@ -0,0 +1,47 @@
+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe Authn::Tokens::DeployToken, feature_category: :system_access do
+  let_it_be(:user) { create(:user) }
+  let_it_be(:group) { create(:group) }
+  let_it_be(:deploy_token) { create(:group_deploy_token, group: group).deploy_token }
+  let_it_be(:unsupported_token) { 'unsupported' }
+
+  let(:token) { described_class.new(plaintext, :group_token_revocation_service) }
+
+  context 'with valid deploy token' do
+    let(:plaintext) { deploy_token.token }
+
+    describe '#initialize' do
+      it 'finds the plaintext token' do
+        expect(token.target).to eq(deploy_token)
+      end
+    end
+
+    describe '#revoke!' do
+      it 'successfully revokes the token' do
+        expect(token.revoke!(user)).to be_truthy
+      end
+    end
+  end
+
+  context 'with unsupported token type' do
+    let(:plaintext) { unsupported_token }
+
+    describe '#initialize' do
+      it 'is nil when the token type is not supported' do
+        expect(token.target).to be_nil
+      end
+    end
+
+    describe '#revoke!' do
+      it 'raises error when the token type is not found' do
+        expect do
+          token.revoke!(user)
+        end
+          .to raise_error("Not Found")
+      end
+    end
+  end
+end
diff --git a/spec/lib/authn/tokens/feed_token_spec.rb b/spec/lib/authn/tokens/feed_token_spec.rb
new file mode 100644
index 00000000000000..7d0656a5a42374
--- /dev/null
+++ b/spec/lib/authn/tokens/feed_token_spec.rb
@@ -0,0 +1,46 @@
+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe Authn::Tokens::FeedToken, feature_category: :system_access do
+  let_it_be(:user) { create(:user) }
+  let_it_be(:feed_token) { user.feed_token }
+  let_it_be(:unsupported_token) { 'unsupported' }
+
+  let(:token) { described_class.new(plaintext, :group_token_revocation_service) }
+
+  context 'with valid feed token' do
+    let(:plaintext) { feed_token }
+
+    describe '#initialize' do
+      it 'finds the plaintext token' do
+        expect(token.target).to eq(user)
+      end
+    end
+
+    describe '#revoke!' do
+      it 'successfully revokes the token' do
+        expect(token.revoke!(user).status).to eq(:success)
+      end
+    end
+  end
+
+  context 'with unsupported token type' do
+    let(:plaintext) { unsupported_token }
+
+    describe '#initialize' do
+      it 'is nil when the token type is not supported' do
+        expect(token.target).to be_nil
+      end
+    end
+
+    describe '#revoke!' do
+      it 'raises error when the token type is not found' do
+        expect do
+          token.revoke!(user)
+        end
+          .to raise_error("Not Found")
+      end
+    end
+  end
+end
diff --git a/spec/lib/authn/tokens/personal_access_token_spec.rb b/spec/lib/authn/tokens/personal_access_token_spec.rb
new file mode 100644
index 00000000000000..c8b11c4c19aae5
--- /dev/null
+++ b/spec/lib/authn/tokens/personal_access_token_spec.rb
@@ -0,0 +1,46 @@
+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe Authn::Tokens::PersonalAccessToken, feature_category: :system_access do
+  let_it_be(:user) { create(:user) }
+  let_it_be(:personal_access_token) { create(:personal_access_token, user: user) }
+  let_it_be(:unsupported_token) { 'unsupported' }
+
+  let(:token) { described_class.new(plaintext, :group_token_revocation_service) }
+
+  context 'with valid personal access token' do
+    let(:plaintext) { personal_access_token.token }
+
+    describe '#initialize' do
+      it 'finds the plaintext token' do
+        expect(token.target).to eq(personal_access_token)
+      end
+    end
+
+    describe '#revoke!' do
+      it 'successfully revokes the token' do
+        expect(token.revoke!(user).status).to eq(:success)
+      end
+    end
+  end
+
+  context 'with unsupported token type' do
+    let(:plaintext) { unsupported_token }
+
+    describe '#initialize' do
+      it 'is nil when the token type is not supported' do
+        expect(token.target).to be_nil
+      end
+    end
+
+    describe '#revoke!' do
+      it 'raises error when the token type is not found' do
+        expect do
+          token.revoke!(user)
+        end
+          .to raise_error("Not Found")
+      end
+    end
+  end
+end
-- 
GitLab


From ec2ef75c9e1015c68220151670ffcc29a35229d9 Mon Sep 17 00:00:00 2001
From: Nicholas Wittstruck <1494491-nwittstruck@users.noreply.gitlab.com>
Date: Tue, 22 Oct 2024 13:35:49 +0200
Subject: [PATCH 4/6] Refactor Token Identification

This commit merges the token identification of the
AgnosticTokenRevocationService and the Token Information API.

This commit uses shared_examples to reduce duplication.

Issue: https://gitlab.com/gitlab-org/gitlab/-/issues/499247

Changelog: other
---
 spec/lib/authn/token_spec.rb                  | 20 +------------
 spec/lib/authn/tokens/deploy_token_spec.rb    | 27 ++---------------
 spec/lib/authn/tokens/feed_token_spec.rb      | 28 ++---------------
 .../tokens/personal_access_token_spec.rb      | 27 ++---------------
 .../authn/token_shared_examples.rb            | 30 +++++++++++++++++++
 5 files changed, 40 insertions(+), 92 deletions(-)
 create mode 100644 spec/support/shared_examples/authn/token_shared_examples.rb

diff --git a/spec/lib/authn/token_spec.rb b/spec/lib/authn/token_spec.rb
index e64dfab711c41f..7a36e6bee97471 100644
--- a/spec/lib/authn/token_spec.rb
+++ b/spec/lib/authn/token_spec.rb
@@ -9,7 +9,6 @@
   let_it_be(:deploy_token) { create(:deploy_token).token }
   let_it_be(:feed_token) { user.feed_token }
   let_it_be(:personal_access_token) { create(:personal_access_token, user: user).token }
-  let_it_be(:unsupported_token) { 'unsupported' }
 
   let(:token) { described_class.new(plaintext, :group_token_revocation_service) }
 
@@ -36,22 +35,5 @@
     end
   end
 
-  context 'with unsupported token types' do
-    let(:plaintext) { unsupported_token }
-
-    describe '#initialize' do
-      it 'is nil when the token type is not supported' do
-        expect(token.revocable).to be_nil
-      end
-    end
-
-    describe '#revoke!' do
-      it 'raises error when the token type is not found' do
-        expect do
-          token.revoke!(user)
-        end
-          .to raise_error("Not Found")
-      end
-    end
-  end
+  it_behaves_like 'token handling with unsupported token type'
 end
diff --git a/spec/lib/authn/tokens/deploy_token_spec.rb b/spec/lib/authn/tokens/deploy_token_spec.rb
index 49f5edb2f50fd5..888c243c2abbb6 100644
--- a/spec/lib/authn/tokens/deploy_token_spec.rb
+++ b/spec/lib/authn/tokens/deploy_token_spec.rb
@@ -6,18 +6,14 @@
   let_it_be(:user) { create(:user) }
   let_it_be(:group) { create(:group) }
   let_it_be(:deploy_token) { create(:group_deploy_token, group: group).deploy_token }
-  let_it_be(:unsupported_token) { 'unsupported' }
 
   let(:token) { described_class.new(plaintext, :group_token_revocation_service) }
 
   context 'with valid deploy token' do
     let(:plaintext) { deploy_token.token }
+    let(:valid_target) { deploy_token }
 
-    describe '#initialize' do
-      it 'finds the plaintext token' do
-        expect(token.target).to eq(deploy_token)
-      end
-    end
+    it_behaves_like 'finding the valid target'
 
     describe '#revoke!' do
       it 'successfully revokes the token' do
@@ -26,22 +22,5 @@
     end
   end
 
-  context 'with unsupported token type' do
-    let(:plaintext) { unsupported_token }
-
-    describe '#initialize' do
-      it 'is nil when the token type is not supported' do
-        expect(token.target).to be_nil
-      end
-    end
-
-    describe '#revoke!' do
-      it 'raises error when the token type is not found' do
-        expect do
-          token.revoke!(user)
-        end
-          .to raise_error("Not Found")
-      end
-    end
-  end
+  it_behaves_like 'token handling with unsupported token type'
 end
diff --git a/spec/lib/authn/tokens/feed_token_spec.rb b/spec/lib/authn/tokens/feed_token_spec.rb
index 7d0656a5a42374..54f595f089718f 100644
--- a/spec/lib/authn/tokens/feed_token_spec.rb
+++ b/spec/lib/authn/tokens/feed_token_spec.rb
@@ -5,18 +5,13 @@
 RSpec.describe Authn::Tokens::FeedToken, feature_category: :system_access do
   let_it_be(:user) { create(:user) }
   let_it_be(:feed_token) { user.feed_token }
-  let_it_be(:unsupported_token) { 'unsupported' }
-
   let(:token) { described_class.new(plaintext, :group_token_revocation_service) }
 
   context 'with valid feed token' do
     let(:plaintext) { feed_token }
+    let(:valid_target) { user }
 
-    describe '#initialize' do
-      it 'finds the plaintext token' do
-        expect(token.target).to eq(user)
-      end
-    end
+    it_behaves_like 'finding the valid target'
 
     describe '#revoke!' do
       it 'successfully revokes the token' do
@@ -25,22 +20,5 @@
     end
   end
 
-  context 'with unsupported token type' do
-    let(:plaintext) { unsupported_token }
-
-    describe '#initialize' do
-      it 'is nil when the token type is not supported' do
-        expect(token.target).to be_nil
-      end
-    end
-
-    describe '#revoke!' do
-      it 'raises error when the token type is not found' do
-        expect do
-          token.revoke!(user)
-        end
-          .to raise_error("Not Found")
-      end
-    end
-  end
+  it_behaves_like 'token handling with unsupported token type'
 end
diff --git a/spec/lib/authn/tokens/personal_access_token_spec.rb b/spec/lib/authn/tokens/personal_access_token_spec.rb
index c8b11c4c19aae5..4a4a2538e366d9 100644
--- a/spec/lib/authn/tokens/personal_access_token_spec.rb
+++ b/spec/lib/authn/tokens/personal_access_token_spec.rb
@@ -5,18 +5,14 @@
 RSpec.describe Authn::Tokens::PersonalAccessToken, feature_category: :system_access do
   let_it_be(:user) { create(:user) }
   let_it_be(:personal_access_token) { create(:personal_access_token, user: user) }
-  let_it_be(:unsupported_token) { 'unsupported' }
 
   let(:token) { described_class.new(plaintext, :group_token_revocation_service) }
 
   context 'with valid personal access token' do
     let(:plaintext) { personal_access_token.token }
+    let(:valid_target) { personal_access_token }
 
-    describe '#initialize' do
-      it 'finds the plaintext token' do
-        expect(token.target).to eq(personal_access_token)
-      end
-    end
+    it_behaves_like 'finding the valid target'
 
     describe '#revoke!' do
       it 'successfully revokes the token' do
@@ -25,22 +21,5 @@
     end
   end
 
-  context 'with unsupported token type' do
-    let(:plaintext) { unsupported_token }
-
-    describe '#initialize' do
-      it 'is nil when the token type is not supported' do
-        expect(token.target).to be_nil
-      end
-    end
-
-    describe '#revoke!' do
-      it 'raises error when the token type is not found' do
-        expect do
-          token.revoke!(user)
-        end
-          .to raise_error("Not Found")
-      end
-    end
-  end
+  it_behaves_like 'token handling with unsupported token type'
 end
diff --git a/spec/support/shared_examples/authn/token_shared_examples.rb b/spec/support/shared_examples/authn/token_shared_examples.rb
new file mode 100644
index 00000000000000..3e817be04d1774
--- /dev/null
+++ b/spec/support/shared_examples/authn/token_shared_examples.rb
@@ -0,0 +1,30 @@
+# frozen_string_literal: true
+
+RSpec.shared_examples 'token handling with unsupported token type' do
+  context 'with unsupported token type' do
+    let_it_be(:plaintext) { 'unsupported' }
+
+    describe '#initialize' do
+      it 'is nil when the token type is not supported' do
+        expect(token.target).to be_nil
+      end
+    end
+
+    describe '#revoke!' do
+      it 'raises error when the token type is not found' do
+        expect do
+          token.revoke!(user)
+        end
+          .to raise_error("Not Found")
+      end
+    end
+  end
+end
+
+RSpec.shared_examples 'finding the valid target' do
+  describe '#initialize' do
+    it 'finds the plaintext token' do
+      expect(token.target).to eq(valid_target)
+    end
+  end
+end
-- 
GitLab


From 32e906a6dcad6224eec90e1a007789cb86484780 Mon Sep 17 00:00:00 2001
From: Nicholas Wittstruck <1494491-nwittstruck@users.noreply.gitlab.com>
Date: Wed, 23 Oct 2024 10:44:00 +0200
Subject: [PATCH 5/6] Refactor Token Identification

This commit merges the token identification of the
AgnosticTokenRevocationService and the Token Information API.

This commit addresses review feedback.

Issue: https://gitlab.com/gitlab-org/gitlab/-/issues/499247

Changelog: other
---
 .../groups/agnostic_token_revocation_service.rb        | 10 ++--------
 lib/api/admin/token.rb                                 |  3 ++-
 lib/authn/token.rb                                     |  4 ++--
 lib/authn/tokens/deploy_token.rb                       |  2 +-
 lib/authn/tokens/feed_token.rb                         |  2 +-
 lib/authn/tokens/personal_access_token.rb              |  2 +-
 spec/lib/authn/token_spec.rb                           |  2 +-
 spec/lib/authn/tokens/deploy_token_spec.rb             |  2 +-
 spec/lib/authn/tokens/feed_token_spec.rb               |  3 ++-
 spec/lib/authn/tokens/personal_access_token_spec.rb    |  2 +-
 .../shared_examples/authn/token_shared_examples.rb     |  2 +-
 11 files changed, 15 insertions(+), 19 deletions(-)

diff --git a/app/services/groups/agnostic_token_revocation_service.rb b/app/services/groups/agnostic_token_revocation_service.rb
index d2ce84843f896e..e0b5684eca6e7a 100644
--- a/app/services/groups/agnostic_token_revocation_service.rb
+++ b/app/services/groups/agnostic_token_revocation_service.rb
@@ -19,8 +19,6 @@ module Groups # rubocop:disable Gitlab/BoundedContexts -- This service is strict
   class AgnosticTokenRevocationService < Groups::BaseService
     AUDIT_SOURCE = :group_token_revocation_service
 
-    attr_reader :token
-
     def initialize(group, current_user, plaintext)
       @group = group
       @current_user = current_user
@@ -32,7 +30,7 @@ def execute
       return error("Group cannot be a subgroup") if group.subgroup?
       return error("Unauthorized") unless can?(current_user, :admin_group, group)
 
-      @token = ::Authn::Token.new(plaintext, :group_token_revocation_service)
+      @token = ::Authn::Token.new(plaintext, AUDIT_SOURCE)
 
       # Perform checks based on token type and group scope:
       case token.revocable
@@ -49,7 +47,7 @@ def execute
 
     private
 
-    attr_reader :plaintext, :group, :current_user
+    attr_reader :plaintext, :group, :current_user, :token
 
     def success(revocable, type, api_entity: nil)
       api_entity ||= type
@@ -137,9 +135,5 @@ def handle_feed_token
       #  - rotation failed for some reason
       error('Feed token revocation failed')
     end
-
-    def handle_token_type_not_supported
-      error('Unsupported token type')
-    end
   end
 end
diff --git a/lib/api/admin/token.rb b/lib/api/admin/token.rb
index 23ac7c71871090..d57330017fdd2f 100644
--- a/lib/api/admin/token.rb
+++ b/lib/api/admin/token.rb
@@ -4,10 +4,11 @@ module API
   module Admin
     class Token < ::API::Base
       feature_category :system_access
+      AUDIT_SOURCE = :api_admin_token
 
       helpers do
         def identify_token(plaintext)
-          token = ::Authn::Token.new(plaintext, :api_admin_token)
+          token = ::Authn::Token.new(plaintext, AUDIT_SOURCE)
           raise ArgumentError, 'Token type not supported.' if token.revocable.blank?
 
           token.target
diff --git a/lib/authn/token.rb b/lib/authn/token.rb
index 245b927fb31bf0..7d816f35ebd638 100644
--- a/lib/authn/token.rb
+++ b/lib/authn/token.rb
@@ -2,7 +2,7 @@
 
 module Authn
   class Token
-    Error = Class.new(StandardError)
+    NotFoundError = Class.new(StandardError)
     TOKEN_TYPES = [
       ::Authn::Tokens::DeployToken,
       ::Authn::Tokens::FeedToken,
@@ -20,7 +20,7 @@ def target
     end
 
     def revoke!(current_user)
-      raise ::Authn::Token::Error, "Not Found" if target.blank?
+      raise ::Authn::Token::NotFoundError, 'Not Found' if revocable.blank?
 
       revocable.revoke!(current_user)
     end
diff --git a/lib/authn/tokens/deploy_token.rb b/lib/authn/tokens/deploy_token.rb
index 8dcb0cc61682ef..538383a5c04f4e 100644
--- a/lib/authn/tokens/deploy_token.rb
+++ b/lib/authn/tokens/deploy_token.rb
@@ -15,7 +15,7 @@ def initialize(plaintext, source)
       end
 
       def revoke!(current_user)
-        raise ::Authn::Token::Error, "Not Found" if target.blank?
+        raise ::Authn::Token::NotFoundError, 'Not Found' if target.blank?
 
         service = ::Groups::DeployTokens::RevokeService.new(
           target.group,
diff --git a/lib/authn/tokens/feed_token.rb b/lib/authn/tokens/feed_token.rb
index 077f4365426745..84ecdf3b302889 100644
--- a/lib/authn/tokens/feed_token.rb
+++ b/lib/authn/tokens/feed_token.rb
@@ -15,7 +15,7 @@ def initialize(plaintext, source)
       end
 
       def revoke!(current_user)
-        raise ::Authn::Token::Error, "Not Found" if target.blank?
+        raise ::Authn::Token::NotFoundError, 'Not Found' if target.blank?
 
         Users::ResetFeedTokenService.new(
           current_user,
diff --git a/lib/authn/tokens/personal_access_token.rb b/lib/authn/tokens/personal_access_token.rb
index 805daefcd48423..e816aa69b84140 100644
--- a/lib/authn/tokens/personal_access_token.rb
+++ b/lib/authn/tokens/personal_access_token.rb
@@ -18,7 +18,7 @@ def initialize(plaintext, source)
       end
 
       def revoke!(current_user)
-        raise ::Authn::Token::Error, "Not Found" if target.blank?
+        raise ::Authn::Token::NotFoundError, 'Not Found' if target.blank?
 
         ::PersonalAccessTokens::RevokeService.new(
           current_user,
diff --git a/spec/lib/authn/token_spec.rb b/spec/lib/authn/token_spec.rb
index 7a36e6bee97471..788961e958adf0 100644
--- a/spec/lib/authn/token_spec.rb
+++ b/spec/lib/authn/token_spec.rb
@@ -10,7 +10,7 @@
   let_it_be(:feed_token) { user.feed_token }
   let_it_be(:personal_access_token) { create(:personal_access_token, user: user).token }
 
-  let(:token) { described_class.new(plaintext, :group_token_revocation_service) }
+  subject(:token) { described_class.new(plaintext, :group_token_revocation_service) }
 
   context 'with supported token types' do
     where(:plaintext, :token_type) do
diff --git a/spec/lib/authn/tokens/deploy_token_spec.rb b/spec/lib/authn/tokens/deploy_token_spec.rb
index 888c243c2abbb6..3e5fc2c951d3bf 100644
--- a/spec/lib/authn/tokens/deploy_token_spec.rb
+++ b/spec/lib/authn/tokens/deploy_token_spec.rb
@@ -7,7 +7,7 @@
   let_it_be(:group) { create(:group) }
   let_it_be(:deploy_token) { create(:group_deploy_token, group: group).deploy_token }
 
-  let(:token) { described_class.new(plaintext, :group_token_revocation_service) }
+  subject(:token) { described_class.new(plaintext, :group_token_revocation_service) }
 
   context 'with valid deploy token' do
     let(:plaintext) { deploy_token.token }
diff --git a/spec/lib/authn/tokens/feed_token_spec.rb b/spec/lib/authn/tokens/feed_token_spec.rb
index 54f595f089718f..f7c0b486069d3b 100644
--- a/spec/lib/authn/tokens/feed_token_spec.rb
+++ b/spec/lib/authn/tokens/feed_token_spec.rb
@@ -5,7 +5,8 @@
 RSpec.describe Authn::Tokens::FeedToken, feature_category: :system_access do
   let_it_be(:user) { create(:user) }
   let_it_be(:feed_token) { user.feed_token }
-  let(:token) { described_class.new(plaintext, :group_token_revocation_service) }
+
+  subject(:token) { described_class.new(plaintext, :group_token_revocation_service) }
 
   context 'with valid feed token' do
     let(:plaintext) { feed_token }
diff --git a/spec/lib/authn/tokens/personal_access_token_spec.rb b/spec/lib/authn/tokens/personal_access_token_spec.rb
index 4a4a2538e366d9..83857453450803 100644
--- a/spec/lib/authn/tokens/personal_access_token_spec.rb
+++ b/spec/lib/authn/tokens/personal_access_token_spec.rb
@@ -6,7 +6,7 @@
   let_it_be(:user) { create(:user) }
   let_it_be(:personal_access_token) { create(:personal_access_token, user: user) }
 
-  let(:token) { described_class.new(plaintext, :group_token_revocation_service) }
+  subject(:token) { described_class.new(plaintext, :group_token_revocation_service) }
 
   context 'with valid personal access token' do
     let(:plaintext) { personal_access_token.token }
diff --git a/spec/support/shared_examples/authn/token_shared_examples.rb b/spec/support/shared_examples/authn/token_shared_examples.rb
index 3e817be04d1774..15c5d3fa311717 100644
--- a/spec/support/shared_examples/authn/token_shared_examples.rb
+++ b/spec/support/shared_examples/authn/token_shared_examples.rb
@@ -15,7 +15,7 @@
         expect do
           token.revoke!(user)
         end
-          .to raise_error("Not Found")
+          .to raise_error(::Authn::Token::NotFoundError, 'Not Found')
       end
     end
   end
-- 
GitLab


From c7b82b981ce7b1137c049a441e58794a02ba9172 Mon Sep 17 00:00:00 2001
From: Nicholas Wittstruck <1494491-nwittstruck@users.noreply.gitlab.com>
Date: Wed, 23 Oct 2024 12:32:51 +0200
Subject: [PATCH 6/6] Refactor Token Identification

This commit merges the token identification of the
AgnosticTokenRevocationService and the Token Information API.

This commit improves the naming and removes target.

Issue: https://gitlab.com/gitlab-org/gitlab/-/issues/499247

Changelog: other
---
 .../agnostic_token_revocation_service.rb      | 10 +++----
 lib/api/admin/token.rb                        |  6 ++--
 lib/authn/agnostic_token_identifier.rb        | 16 +++++++++++
 lib/authn/token.rb                            | 28 -------------------
 lib/authn/tokens/deploy_token.rb              | 10 +++----
 lib/authn/tokens/feed_token.rb                |  8 +++---
 lib/authn/tokens/personal_access_token.rb     |  8 +++---
 ...c.rb => agnostic_token_identifier_spec.rb} | 18 ++++--------
 spec/lib/authn/tokens/deploy_token_spec.rb    |  4 +--
 spec/lib/authn/tokens/feed_token_spec.rb      |  4 +--
 .../tokens/personal_access_token_spec.rb      |  4 +--
 .../authn/token_shared_examples.rb            |  8 +++---
 12 files changed, 51 insertions(+), 73 deletions(-)
 create mode 100644 lib/authn/agnostic_token_identifier.rb
 delete mode 100644 lib/authn/token.rb
 rename spec/lib/authn/{token_spec.rb => agnostic_token_identifier_spec.rb} (57%)

diff --git a/app/services/groups/agnostic_token_revocation_service.rb b/app/services/groups/agnostic_token_revocation_service.rb
index e0b5684eca6e7a..35e42774fe1011 100644
--- a/app/services/groups/agnostic_token_revocation_service.rb
+++ b/app/services/groups/agnostic_token_revocation_service.rb
@@ -30,10 +30,11 @@ def execute
       return error("Group cannot be a subgroup") if group.subgroup?
       return error("Unauthorized") unless can?(current_user, :admin_group, group)
 
-      @token = ::Authn::Token.new(plaintext, AUDIT_SOURCE)
+      @token = ::Authn::AgnosticTokenIdentifier.token_for(plaintext, AUDIT_SOURCE)
+      @revocable = token.revocable unless token.blank?
 
       # Perform checks based on token type and group scope:
-      case token.revocable
+      case token
       when ::Authn::Tokens::PersonalAccessToken
         handle_personal_access_token
       when ::Authn::Tokens::DeployToken
@@ -47,7 +48,7 @@ def execute
 
     private
 
-    attr_reader :plaintext, :group, :current_user, :token
+    attr_reader :plaintext, :group, :current_user, :token, :revocable
 
     def success(revocable, type, api_entity: nil)
       api_entity ||= type
@@ -66,7 +67,6 @@ def error(message)
     end
 
     def handle_personal_access_token
-      revocable = token.target
       return error('PAT not found') unless revocable
 
       if user_has_group_membership?(revocable.user)
@@ -101,7 +101,6 @@ def user_has_group_membership?(user)
     end
 
     def handle_deploy_token
-      revocable = token.target
       return error('DeployToken not found') unless revocable && revocable.group_type?
 
       if group.self_and_descendants.include?(revocable.group)
@@ -114,7 +113,6 @@ def handle_deploy_token
     end
 
     def handle_feed_token
-      revocable = token.target
       return error('Feed Token not found') unless revocable
 
       if user_has_group_membership?(revocable)
diff --git a/lib/api/admin/token.rb b/lib/api/admin/token.rb
index d57330017fdd2f..9a54c95526f154 100644
--- a/lib/api/admin/token.rb
+++ b/lib/api/admin/token.rb
@@ -8,10 +8,10 @@ class Token < ::API::Base
 
       helpers do
         def identify_token(plaintext)
-          token = ::Authn::Token.new(plaintext, AUDIT_SOURCE)
-          raise ArgumentError, 'Token type not supported.' if token.revocable.blank?
+          token = ::Authn::AgnosticTokenIdentifier.token_for(plaintext, AUDIT_SOURCE)
+          raise ArgumentError, 'Token type not supported.' if token.blank?
 
-          token.target
+          token.revocable
         end
       end
 
diff --git a/lib/authn/agnostic_token_identifier.rb b/lib/authn/agnostic_token_identifier.rb
new file mode 100644
index 00000000000000..672f03d6eba431
--- /dev/null
+++ b/lib/authn/agnostic_token_identifier.rb
@@ -0,0 +1,16 @@
+# frozen_string_literal:true
+
+module Authn
+  class AgnosticTokenIdentifier
+    NotFoundError = Class.new(StandardError)
+    TOKEN_TYPES = [
+      ::Authn::Tokens::DeployToken,
+      ::Authn::Tokens::FeedToken,
+      ::Authn::Tokens::PersonalAccessToken
+    ].freeze
+
+    def self.token_for(plaintext, source)
+      TOKEN_TYPES.find { |x| x.prefix?(plaintext) }&.new(plaintext, source)
+    end
+  end
+end
diff --git a/lib/authn/token.rb b/lib/authn/token.rb
deleted file mode 100644
index 7d816f35ebd638..00000000000000
--- a/lib/authn/token.rb
+++ /dev/null
@@ -1,28 +0,0 @@
-# frozen_string_literal:true
-
-module Authn
-  class Token
-    NotFoundError = Class.new(StandardError)
-    TOKEN_TYPES = [
-      ::Authn::Tokens::DeployToken,
-      ::Authn::Tokens::FeedToken,
-      ::Authn::Tokens::PersonalAccessToken
-    ].freeze
-
-    attr_reader :revocable
-
-    def initialize(plaintext, source)
-      @revocable = TOKEN_TYPES.find { |x| x.prefix?(plaintext) }&.new(plaintext, source)
-    end
-
-    def target
-      revocable&.target
-    end
-
-    def revoke!(current_user)
-      raise ::Authn::Token::NotFoundError, 'Not Found' if revocable.blank?
-
-      revocable.revoke!(current_user)
-    end
-  end
-end
diff --git a/lib/authn/tokens/deploy_token.rb b/lib/authn/tokens/deploy_token.rb
index 538383a5c04f4e..43b694adf172f2 100644
--- a/lib/authn/tokens/deploy_token.rb
+++ b/lib/authn/tokens/deploy_token.rb
@@ -7,20 +7,20 @@ def self.prefix?(plaintext)
         plaintext.start_with?(::DeployToken::DEPLOY_TOKEN_PREFIX)
       end
 
-      attr_reader :target, :source
+      attr_reader :revocable, :source
 
       def initialize(plaintext, source)
-        @target = ::DeployToken.find_by_token(plaintext)
+        @revocable = ::DeployToken.find_by_token(plaintext)
         @source = source
       end
 
       def revoke!(current_user)
-        raise ::Authn::Token::NotFoundError, 'Not Found' if target.blank?
+        raise ::Authn::AgnosticTokenIdentifier::NotFoundError, 'Not Found' if revocable.blank?
 
         service = ::Groups::DeployTokens::RevokeService.new(
-          target.group,
+          revocable.group,
           current_user,
-          { id: target.id }
+          { id: revocable.id }
         )
         service.source = source
         service.execute
diff --git a/lib/authn/tokens/feed_token.rb b/lib/authn/tokens/feed_token.rb
index 84ecdf3b302889..1b425c76403867 100644
--- a/lib/authn/tokens/feed_token.rb
+++ b/lib/authn/tokens/feed_token.rb
@@ -7,19 +7,19 @@ def self.prefix?(plaintext)
         plaintext.start_with?(::User::FEED_TOKEN_PREFIX)
       end
 
-      attr_reader :target, :source
+      attr_reader :revocable, :source
 
       def initialize(plaintext, source)
-        @target = User.find_by_feed_token(plaintext)
+        @revocable = User.find_by_feed_token(plaintext)
         @source = source
       end
 
       def revoke!(current_user)
-        raise ::Authn::Token::NotFoundError, 'Not Found' if target.blank?
+        raise ::Authn::AgnosticTokenIdentifier::NotFoundError, 'Not Found' if revocable.blank?
 
         Users::ResetFeedTokenService.new(
           current_user,
-          user: target,
+          user: revocable,
           source: source
         ).execute
       end
diff --git a/lib/authn/tokens/personal_access_token.rb b/lib/authn/tokens/personal_access_token.rb
index e816aa69b84140..669debaf2b47aa 100644
--- a/lib/authn/tokens/personal_access_token.rb
+++ b/lib/authn/tokens/personal_access_token.rb
@@ -10,19 +10,19 @@ def self.prefix?(plaintext)
         )
       end
 
-      attr_reader :target, :source
+      attr_reader :revocable, :source
 
       def initialize(plaintext, source)
-        @target = ::PersonalAccessToken.find_by_token(plaintext)
+        @revocable = ::PersonalAccessToken.find_by_token(plaintext)
         @source = source
       end
 
       def revoke!(current_user)
-        raise ::Authn::Token::NotFoundError, 'Not Found' if target.blank?
+        raise ::Authn::AgnosticTokenIdentifier::NotFoundError, 'Not Found' if revocable.blank?
 
         ::PersonalAccessTokens::RevokeService.new(
           current_user,
-          token: target,
+          token: revocable,
           source: source
         ).execute
       end
diff --git a/spec/lib/authn/token_spec.rb b/spec/lib/authn/agnostic_token_identifier_spec.rb
similarity index 57%
rename from spec/lib/authn/token_spec.rb
rename to spec/lib/authn/agnostic_token_identifier_spec.rb
index 788961e958adf0..b3d96cb6a46535 100644
--- a/spec/lib/authn/token_spec.rb
+++ b/spec/lib/authn/agnostic_token_identifier_spec.rb
@@ -2,7 +2,7 @@
 
 require 'spec_helper'
 
-RSpec.describe Authn::Token, feature_category: :system_access do
+RSpec.describe Authn::AgnosticTokenIdentifier, feature_category: :system_access do
   using RSpec::Parameterized::TableSyntax
 
   let_it_be(:user) { create(:user) }
@@ -10,30 +10,22 @@
   let_it_be(:feed_token) { user.feed_token }
   let_it_be(:personal_access_token) { create(:personal_access_token, user: user).token }
 
-  subject(:token) { described_class.new(plaintext, :group_token_revocation_service) }
+  subject(:token) { described_class.token_for(plaintext, :group_token_revocation_service) }
 
   context 'with supported token types' do
     where(:plaintext, :token_type) do
       ref(:personal_access_token) | ::Authn::Tokens::PersonalAccessToken
       ref(:feed_token) | ::Authn::Tokens::FeedToken
       ref(:deploy_token) | ::Authn::Tokens::DeployToken
+      'unsupported' | NilClass
     end
 
     with_them do
       describe '#initialize' do
-        it 'sets the correct target token type' do
-          expect(token.revocable).to be_instance_of(token_type)
-        end
-      end
-
-      describe '#revoke!' do
-        it 'calls revoke on the token' do
-          expect(token.revocable).to receive(:revoke!)
-          token.revoke!(user)
+        it 'finds the correct revocable token type' do
+          expect(token).to be_instance_of(token_type)
         end
       end
     end
   end
-
-  it_behaves_like 'token handling with unsupported token type'
 end
diff --git a/spec/lib/authn/tokens/deploy_token_spec.rb b/spec/lib/authn/tokens/deploy_token_spec.rb
index 3e5fc2c951d3bf..d79eaf3c40821f 100644
--- a/spec/lib/authn/tokens/deploy_token_spec.rb
+++ b/spec/lib/authn/tokens/deploy_token_spec.rb
@@ -11,9 +11,9 @@
 
   context 'with valid deploy token' do
     let(:plaintext) { deploy_token.token }
-    let(:valid_target) { deploy_token }
+    let(:valid_revocable) { deploy_token }
 
-    it_behaves_like 'finding the valid target'
+    it_behaves_like 'finding the valid revocable'
 
     describe '#revoke!' do
       it 'successfully revokes the token' do
diff --git a/spec/lib/authn/tokens/feed_token_spec.rb b/spec/lib/authn/tokens/feed_token_spec.rb
index f7c0b486069d3b..ba28db230161c2 100644
--- a/spec/lib/authn/tokens/feed_token_spec.rb
+++ b/spec/lib/authn/tokens/feed_token_spec.rb
@@ -10,9 +10,9 @@
 
   context 'with valid feed token' do
     let(:plaintext) { feed_token }
-    let(:valid_target) { user }
+    let(:valid_revocable) { user }
 
-    it_behaves_like 'finding the valid target'
+    it_behaves_like 'finding the valid revocable'
 
     describe '#revoke!' do
       it 'successfully revokes the token' do
diff --git a/spec/lib/authn/tokens/personal_access_token_spec.rb b/spec/lib/authn/tokens/personal_access_token_spec.rb
index 83857453450803..34440046cf483d 100644
--- a/spec/lib/authn/tokens/personal_access_token_spec.rb
+++ b/spec/lib/authn/tokens/personal_access_token_spec.rb
@@ -10,9 +10,9 @@
 
   context 'with valid personal access token' do
     let(:plaintext) { personal_access_token.token }
-    let(:valid_target) { personal_access_token }
+    let(:valid_revocable) { personal_access_token }
 
-    it_behaves_like 'finding the valid target'
+    it_behaves_like 'finding the valid revocable'
 
     describe '#revoke!' do
       it 'successfully revokes the token' do
diff --git a/spec/support/shared_examples/authn/token_shared_examples.rb b/spec/support/shared_examples/authn/token_shared_examples.rb
index 15c5d3fa311717..a0ff50401de40e 100644
--- a/spec/support/shared_examples/authn/token_shared_examples.rb
+++ b/spec/support/shared_examples/authn/token_shared_examples.rb
@@ -6,7 +6,7 @@
 
     describe '#initialize' do
       it 'is nil when the token type is not supported' do
-        expect(token.target).to be_nil
+        expect(token.revocable).to be_nil
       end
     end
 
@@ -15,16 +15,16 @@
         expect do
           token.revoke!(user)
         end
-          .to raise_error(::Authn::Token::NotFoundError, 'Not Found')
+          .to raise_error(::Authn::AgnosticTokenIdentifier::NotFoundError, 'Not Found')
       end
     end
   end
 end
 
-RSpec.shared_examples 'finding the valid target' do
+RSpec.shared_examples 'finding the valid revocable' do
   describe '#initialize' do
     it 'finds the plaintext token' do
-      expect(token.target).to eq(valid_target)
+      expect(token.revocable).to eq(valid_revocable)
     end
   end
 end
-- 
GitLab