Refactor Token Identification for Agnostic Token Revocation Service and Token Information API

Summary

As a follow-up to #443597 (closed), we'd like to refactor the idenfication of the token type. Currently, the pattern matching to identify the token type is duplicated in two places - Admin Token Information API and AgnosticTokenRevocationService.

Since we are about to extend the pattern matching to support additional token types, we should first consolidate the matching in some kind of helper module.

Implementation Plan

@nmalcolm suggested some approaches in the previous issue.

My suggested approach would be a slight variation of an concern-like pattern, though I'm open to the original approach as well:

The difference is that I'd like to pass the plaintext token and the resolved_token as parameters, instead of relying on an existing method/attribute/reader. The reason is that unlike in the AgnosticTokenService, in API::Admin::Token the token comes from a param, and I would have to store the parameter in Token first. I think this is fine for a service, but I'm unsure if we should to that in a REST API object.

# lib/agnostic_token_handler.rb
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])
      resolved_token = PersonalAccessToken.find_by_token(plaintext)
      handle_personal_access_token(resolved_token)
    elsif plaintext.start_with?(DeployToken::DEPLOY_TOKEN_PREFIX)
      resolved_token = DeployToken.find_by_token(plaintext)
      handle_deploy_token(resolved_token)
    else
      handle_not_found
    end
  end

  def handle_personal_access_token(resolved_token)
    raise 'Not implemented'
  end

  def handle_deploy_token(resolved_token)
    raise 'Not implemented'
  end

  def handle_not_found
    raise 'Not implemented'
  end
end

# app/services/groups/agnostic_token_revocation_service.rb
module Groups # rubocop:disable Gitlab/BoundedContexts -- This service is strictly related to groups
  class AgnosticTokenRevocationService < Groups::BaseService
    include AgnosticTokenHandler
    ...
    def execute
      return error("Feature not enabled") unless Feature.enabled?(:group_agnostic_token_revocation, group)
      return error("Group cannot be a subgroup") if group.subgroup?
      return error("Unauthorized") unless can?(current_user, :admin_group, group)
    
      handle_plaintext(plaintext)
    end
    
    # Implement our own handlers here
    def handle_personal_access_token(resolved_token)
      if user_has_group_membership?(resolved_token.user)
       ...
    end
    
    def handle_deploy_token(resolved_token)
      handle_not_found unless resolved_token.group_type?
    ...

I prefer this approach over the other suggestions:

One way would be a simple resolver with an interface like TokenResolver.new(plaintext).token that just gives you a token / nil. That'd be useful to all the endpoints, revocation or not.

I really like the simplicity of this approach. However, we already have code paths that depend on the type of the token - so we would have to check again after resolving the token. Even though it would be simpler now (e.g. we could check based on the class instead of doing pattern matching on a string). Therefore, I prefer the suggested handler.

A third way would be some kind of revocation module where the logic to check whether to revoke is implemented by the specific classes.

That was actually my first thought, however I prefer the explicitness of the concern-like pattern above. I'm not sure if adding another level of indirection is helpful to (potentially) save a few lines of code.

Improvements

• Reduce code duplication
• When changes to token patterns occur, we only need to be update one place

Risks

Both APIs could break, however both have good test coverage and are behind a feature flag.

Involved components

app/services/groups/agnostic_token_revocation_service.rb

lib/api/admin/token.rb