Issue personal access tokens from a first-party client, and backfill existing ones onto it

app/controllers/concerns/authentication.rb

@@ -49,7 +49,7 @@ def authenticated?
     end
 
     def oauth_grant?
-      Current.access_token&.oauth_client_id?
+      Current.access_token&.oauth? || false
     end
 
     def forbid_oauth_grant
@@ -105,7 +105,7 @@ def authenticate_with_bearer_token
         Current.access_token = access_token
         Current.identity = access_token.identity
       elsif access_token
-        request_bearer_authentication status: access_token.oauth_client_id? ? :forbidden : :unauthorized,
+        request_bearer_authentication status: access_token.connected_app ? :forbidden : :unauthorized,
           error: "insufficient_scope", error_description: "The access token is read-only", scope: "write"
       else
         request_bearer_authentication error: "invalid_token", error_description: "The access token is expired, revoked, or invalid"

app/controllers/my/access_tokens_controller.rb

@@ -15,7 +15,7 @@ def new
   # The raw token is shown in this response and on no page after it, so the
   # response must not be cached.
   def create
-    @access_token = my_access_tokens.create!(access_token_params)
+    @access_token = my_access_tokens.create!(access_token_params.merge(oauth_client: Oauth::Client.personal_access_issuer))
     no_store
 
     respond_to do |format|

app/controllers/oauth/authorizations_controller.rb

@@ -70,9 +70,16 @@ def save_oauth_return_url
       end
     end
 
+    # A client not registered for the code grant, like the personal access
+    # token issuer, has nothing to authorize here.
     def set_client
       @client = Oauth::Client.find_by(client_id: params[:client_id])
-      oauth_error("invalid_request", "Unknown client") unless @client
+
+      if @client.nil?
+        oauth_error "invalid_request", "Unknown client"
+      elsif !@client.allows_grant_type?("authorization_code")
+        oauth_error "unauthorized_client", "Client may not use the authorization code grant"
+      end
     end
 
     def validate_redirect_uri

app/controllers/oauth/revocations_controller.rb

@@ -8,9 +8,11 @@
 # with a missing or wrong secret, a Basic header that doesn't authenticate,
 # or a client secret that authenticates no client.
 #
-# A personal access token was issued to no client, so possession of it is the
-# credential: whoever presents one revokes it, client or not. Client
-# credentials presented beside it must still authenticate, by the same rules.
+# A personal access token is issued to no client that can authenticate here
+# (none at all, or the personal access token issuer, which has no secret and no
+# grant to revoke), so possession of it is the credential: whoever presents one
+# revokes it, client or not. Client credentials presented beside it must still
+# authenticate, by the same rules.
 class Oauth::RevocationsController < Oauth::BaseController
   include Oauth::ClientAuthentication
 

app/controllers/oauth/tokens_controller.rb

@@ -15,6 +15,7 @@ class Oauth::TokensController < Oauth::BaseController
   # the answer must not depend on whether the code or refresh token is live,
   # or it tells whoever holds one without the secret.
   before_action :authenticate_client
+  before_action :validate_client_grant_type
   before_action :require_params
 
   with_options if: :authorization_code_grant? do
@@ -76,6 +77,15 @@ def validate_grant_type
       end
     end
 
+    # A client is refused a grant type it isn't registered for (RFC 6749 §5.2),
+    # so the personal access token issuer, registered for none, can never
+    # mint, refresh or exchange a token here.
+    def validate_client_grant_type
+      if requesting_client && !requesting_client.allows_grant_type?(params[:grant_type])
+        oauth_error "unauthorized_client", "Client may not use the #{params[:grant_type]} grant"
+      end
+    end
+
     # A missing parameter is a malformed request (invalid_request), not a dead
     # grant (invalid_grant), which a client would act on by discarding it.
     # client_id is required of every client, in the body unless a Basic

app/controllers/users/push_subscriptions_controller.rb

@@ -9,8 +9,8 @@ def index
   def create
     # A new subscription records its app as it's inserted, so no crash between
     # two writes can leave an app's endpoint looking like a browser's.
-    subscription = @push_subscriptions.create_with(user_agent: request.user_agent, oauth_client: Current.access_token&.oauth_client).create_or_find_by!(push_subscription_params)
-    subscription.claim_for Current.access_token&.oauth_client
+    subscription = @push_subscriptions.create_with(user_agent: request.user_agent, oauth_client: Current.access_token&.connected_app).create_or_find_by!(push_subscription_params)
+    subscription.claim_for Current.access_token&.connected_app
 
     respond_to do |format|
       format.html { head :no_content }

app/models/identity/access_token.rb

@@ -23,8 +23,15 @@ class Identity::AccessToken < ApplicationRecord
   # revocation waits rather than deadlocking against the retired-token delete.
   before_destroy :lock_grant, prepend: true
 
-  scope :personal, -> { where oauth_client_id: nil }
-  scope :oauth, -> { where.not oauth_client_id: nil }
+  # A personal access token is issued by the personal access client
+  # (Oauth::Client::PERSONAL_ACCESS_CLIENT_ID). Ones made before that client
+  # existed, and ones production code that predates it still makes, have no
+  # client, and are just as personal. Every rule that sets personal tokens
+  # apart (honoring while OAuth is dark, never lapsing, no expiry or refresh
+  # token, revocation by possession, Connected Apps) reads one of these, or
+  # #personal?, so it holds for both forms alike.
+  scope :personal, -> { where(oauth_client_id: nil).or(where(oauth_client_id: Oauth::Client.personal_access.select(:id))) }
+  scope :oauth, -> { where.not(oauth_client_id: nil).where.not(oauth_client_id: Oauth::Client.personal_access.select(:id)) }
   scope :active, -> { where(expires_at: nil).or(where(expires_at: Time.current..)) }
   scope :unlapsed, -> { where(updated_at: REFRESH_IDLE_LIMIT.ago..) }
   scope :lapsed, -> { oauth.where(updated_at: ...REFRESH_IDLE_LIMIT.ago) }
@@ -64,14 +71,14 @@ class Identity::AccessToken < ApplicationRecord
   attribute :raw_refresh_token, :string
 
   before_create :set_token
-  before_create :set_expiry_and_refresh_token, if: :oauth_client_id?
+  before_create :set_expiry_and_refresh_token, if: :oauth?
 
   # Issuing or destroying a grant restarts its client's retention period
   # (Oauth::Client.cleanup). The touch waits until the grant's own transaction
   # commits. A revocation locks the grant, and touching the client in the same
   # transaction would then wait on the client's lock. A replayed code exchange
   # holds that lock while it waits on the grant, so the two would deadlock.
-  after_commit :touch_oauth_client, on: %i[ create destroy ]
+  after_commit :touch_oauth_client, on: %i[ create destroy ], if: :oauth?
 
   # Every way a grant ends destroys it: disconnecting the app, revocation, the
   # idle-lapse sweep, a replayed refresh token or authorization code, and
@@ -83,7 +90,7 @@ class Identity::AccessToken < ApplicationRecord
   #
   # Checked after commit, so when two grants to one client end at once,
   # whichever commits second finds none left.
-  after_destroy_commit :end_app_plumbing, if: :oauth_client_id?
+  after_destroy_commit :end_app_plumbing, if: :oauth?
 
   class << self
     # The token a bearer request may authenticate with: active and honored.
@@ -157,9 +164,24 @@ def find_and_digest_undigested(raw)
 
   # OAuth tokens are honored only while OAuth acceptance is on (see
   # Oauth::Availability); personal tokens always are. The client is loaded only
-  # when the server is dark and a pilot exemption might apply.
+  # when the server is dark, to tell a personal token from a grant and to see
+  # whether a pilot exemption applies.
   def honored?
-    oauth_client_id.nil? || Oauth::Availability.acceptance_enabled? || Oauth::Availability.acceptance_enabled?(oauth_client&.client_id)
+    oauth_client_id.nil? || Oauth::Availability.acceptance_enabled? || personal? || Oauth::Availability.acceptance_enabled?(oauth_client&.client_id)
+  end
+
+  def personal?
+    oauth_client_id.nil? || oauth_client&.personal_access? || false
+  end
+
+  def oauth?
+    !personal?
+  end
+
+  # The app this token acts through, if it is a grant. A personal access token
+  # acts for its holder alone, so what it sets up is attributed to no app.
+  def connected_app
+    oauth_client if oauth?
   end
 
   def allows?(method)
@@ -175,7 +197,7 @@ def expires_in
   end
 
   def lapsed?
-    oauth_client_id? && (updated_at + REFRESH_IDLE_LIMIT).past?
+    oauth? && (updated_at + REFRESH_IDLE_LIMIT).past?
   end
 
   # Rotates atomically on the presented refresh token, so a concurrent
@@ -212,7 +234,7 @@ def end_app_plumbing
     end
 
     def touch_oauth_client
-      Oauth::Client.where(id: oauth_client_id).touch_all if oauth_client_id?
+      Oauth::Client.where(id: oauth_client_id).touch_all
     end
 
     def lock_grant
@@ -223,9 +245,9 @@ def lock_grant
     # for production code that reads only plaintext. An explicitly given
     # token is digested like a generated one.
     def set_token
-      self.raw_token ||= token.presence || self.class.generate_token(oauth_client_id? ? :access : :personal)
+      self.raw_token ||= token.presence || self.class.generate_token(personal? ? :personal : :access)
       self.token_digest = TokenDigester.digest(raw_token)
-      self.token = oauth_client_id? ? nil : raw_token
+      self.token = personal? ? raw_token : nil
     end
 
     def set_expiry_and_refresh_token

app/models/oauth/client.rb

@@ -9,6 +9,22 @@ class Oauth::Client < ApplicationRecord
   # this long is abandoned (RFC 7591 §5).
   UNUSED_RETENTION = 30.days
 
+  # The grants a client may obtain at the authorization and token endpoints.
+  # An allowlist, so a client is refused any grant type it doesn't list.
+  GRANT_TYPES = %w[ authorization_code refresh_token ]
+
+  # Personal access tokens are issued by this first-party client, as bc3's are
+  # by bc-personal-access, so they're one-off access tokens of the same kind
+  # as every other. It is provisioned by a migration and on first use, never
+  # registered: the sweep, which takes only self-registered clients, leaves it
+  # alone. It is listed for no grant type, so the authorization and token
+  # endpoints never mint, refresh or exchange its tokens: only
+  # My::AccessTokensController issues them. Its client_id has a hyphen, which
+  # no generated client_id (base58) can, so no other client can ever claim it.
+  PERSONAL_ACCESS_CLIENT_ID = "fizzy-personal-access"
+  PERSONAL_ACCESS_ATTRIBUTES = { client_id: PERSONAL_ACCESS_CLIENT_ID, name: "Personal Access Tokens",
+    trusted: true, dynamically_registered: false, redirect_uris: [], scopes: %w[ read write ], token_endpoint_auth_method: "none" }
+
   has_many :access_tokens, class_name: "Identity::AccessToken", foreign_key: :oauth_client_id, inverse_of: :oauth_client, dependent: :destroy
 
   has_secure_token :client_id, length: 32
@@ -18,14 +34,18 @@ class Oauth::Client < ApplicationRecord
   # same order, so no grant can land after the cascade and be stranded.
   before_destroy :lock_client, prepend: true
 
+  # Destroying the issuer would cascade to every client-backed personal access
+  # token on the install.
+  before_destroy :refuse_to_destroy_personal_access, prepend: true
+
   validates :name, presence: true, length: { maximum: 255 }
   validates :client_id, uniqueness: true, allow_nil: true
   # Registration is open and unauthenticated, so what it stores is bounded:
   # a few callbacks, each no longer than a URL browsers reliably carry.
   MAXIMUM_REDIRECT_URIS = 10
   MAXIMUM_REDIRECT_URI_LENGTH = 2000
 
-  validates :redirect_uris, presence: true, length: { maximum: MAXIMUM_REDIRECT_URIS }
+  validates :redirect_uris, presence: { unless: :personal_access? }, length: { maximum: MAXIMUM_REDIRECT_URIS }
   validates :token_endpoint_auth_method, inclusion: { in: AUTH_METHODS }
   validate :redirect_uris_are_valid
 
@@ -47,8 +67,24 @@ class Oauth::Client < ApplicationRecord
   # swept for having registered long ago.
   scope :stale, -> { dynamically_registered.where.missing(:access_tokens).where(updated_at: ...UNUSED_RETENTION.ago) }
 
-  def self.cleanup
-    stale.find_each(&:destroy_if_still_unused)
+  scope :personal_access, -> { where client_id: PERSONAL_ACCESS_CLIENT_ID }
+
+  class << self
+    def cleanup
+      stale.find_each(&:destroy_if_still_unused)
+    end
+
+    # A fresh install loads the schema rather than running migrations, so the
+    # issuer may not exist yet when the first personal access token is made.
+    # Two first uses can race: the loser either hits the unique index, which
+    # create_or_find_by! answers by finding on client_id alone, or, when the
+    # winner committed before its uniqueness validation ran, fails that
+    # validation, and then finds the winner's row the same way.
+    def personal_access_issuer
+      personal_access.take || create_or_find_by!(client_id: PERSONAL_ACCESS_CLIENT_ID) { it.assign_attributes PERSONAL_ACCESS_ATTRIBUTES }
+    rescue ActiveRecord::RecordInvalid
+      personal_access.take or raise
+    end
   end
 
   # The sweep and #redeem take the same row lock, so a grant either lands
@@ -73,6 +109,18 @@ def redeem(authorization_code, identity:, permission:)
     end
   end
 
+  def personal_access?
+    client_id == PERSONAL_ACCESS_CLIENT_ID
+  end
+
+  def grant_types
+    personal_access? ? [] : GRANT_TYPES
+  end
+
+  def allows_grant_type?(grant_type)
+    grant_type.in?(grant_types)
+  end
+
   def confidential?
     token_endpoint_auth_method.in?(SECRET_AUTH_METHODS)
   end
@@ -106,6 +154,15 @@ def lock_client
       access_tokens.reset
     end
 
+    # Reads the stored client_id: lock_client no longer reloads the row, so an
+    # unsaved change to the issuer's client_id mustn't let it cascade.
+    def refuse_to_destroy_personal_access
+      if client_id_in_database == PERSONAL_ACCESS_CLIENT_ID
+        errors.add :base, "The personal access token issuer can't be deleted"
+        throw :abort
+      end
+    end
+
     def generate_client_secret
       self.raw_client_secret ||= Identity::AccessToken.generate_token(:client_secret)
       self.client_secret_digest = TokenDigester.digest(raw_client_secret)

app/models/webhook.rb

@@ -87,7 +87,7 @@ def for_slack?
   private
     def attribute_creation
       self.creator ||= Current.user
-      self.created_via ||= Current.access_token&.oauth_client
+      self.created_via ||= Current.access_token&.connected_app
     end
 
     def validate_url

db/migrate/20261015120100_backfill_token_digests.rb

@@ -6,8 +6,10 @@
 # restart it.
 #
 # Personal access tokens keep their plaintext: production still reads it until
-# this code reaches it. OAuth access tokens are digest-only, so any plaintext
-# one is cleared once digested. Refresh tokens and client secrets are moved
+# this code reaches it. That's any token without a client, or with the personal
+# access client (CreatePersonalAccessClient), since a rerun may come after
+# that client exists. OAuth access tokens are digest-only, so any plaintext one
+# is cleared once digested. Refresh tokens and client secrets are moved
 # wholesale: a refresh token's plaintext is cleared once digested, and the
 # client secret column is dropped by the next migration. The refresh token
 # column stays, empty, until it's dropped beside `token`.
@@ -31,10 +33,11 @@ class Client < ActiveRecord::Base
 
   def up
     refuse_duplicates AccessToken, :token
+    personal_client_ids = [ nil, *Client.where(client_id: "fizzy-personal-access").pluck(:id) ]
 
     each_undigested AccessToken, :token, :token_digest do |row, digest|
       AccessToken.where(id: row.id, token_digest: nil).update_all \
-        token_digest: digest, token: (row.oauth_client_id ? nil : row.token)
+        token_digest: digest, token: (row.oauth_client_id.in?(personal_client_ids) ? row.token : nil)
     end
 
     # Each is gone once dropped, so a rerun skips it.

db/migrate/20261016120000_create_personal_access_client.rb

@@ -0,0 +1,44 @@
+# Provisions the first-party client that issues personal access tokens
+# (Oauth::Client::PERSONAL_ACCESS_CLIENT_ID), on every install that migrates,
+# SaaS or self-hosted. Oauth::Client.personal_access_issuer makes it on first
+# use where the schema was loaded instead. It's never registered, so the sweep
+# of abandoned registrations passes it by.
+#
+# Down hands its tokens back to the client-less form, which every version of
+# the code reads as personal, and then removes it. Neither direction moves
+# updated_at. Run it only once no code with the client is serving: that code
+# issues the client's tokens, takes no lock doing so, and makes the client
+# again on first use, so nothing down could do would keep up with it. With
+# that settled, the order makes down safe to retry: the client stays until
+# every token is handed back, so a down that stops partway finds it again.
+class CreatePersonalAccessClient < ActiveRecord::Migration[8.2]
+  disable_ddl_transaction!
+
+  CLIENT_ID = "fizzy-personal-access"
+  BATCH_SIZE = 500
+
+  class Client < ActiveRecord::Base
+    self.table_name = "oauth_clients"
+  end
+
+  class AccessToken < ActiveRecord::Base
+    self.table_name = "identity_access_tokens"
+  end
+
+  def up
+    unless Client.exists?(client_id: CLIENT_ID)
+      Client.create! client_id: CLIENT_ID, name: "Personal Access Tokens", trusted: true, dynamically_registered: false,
+        redirect_uris: [], scopes: %w[ read write ], token_endpoint_auth_method: "none"
+    end
+  end
+
+  def down
+    if client = Client.find_by(client_id: CLIENT_ID)
+      AccessToken.where(oauth_client_id: client.id).in_batches(of: BATCH_SIZE) do |batch|
+        batch.update_all oauth_client_id: nil
+      end
+
+      client.delete
+    end
+  end
+end