Prefix newly issued access tokens, refresh tokens and personal access tokens

app/models/identity/access_token.rb

@@ -29,9 +29,26 @@ class Identity::AccessToken < ApplicationRecord
   scope :unlapsed, -> { where(updated_at: REFRESH_IDLE_LIMIT.ago..) }
   scope :lapsed, -> { oauth.where(updated_at: ...REFRESH_IDLE_LIMIT.ago) }
 
-  has_secure_token
+  # A prefix makes a token recognizable on sight, in logs, pastes and
+  # repositories, and names its kind. Tokens issued before prefixes carry none
+  # and keep working: every lookup matches the whole string. OAuth client
+  # secrets are drawn here too (Oauth::Client), so every credential Fizzy
+  # issues has one shape.
+  PREFIXES = { personal: "fizzy_pat_", access: "fizzy_at_", refresh: "fizzy_rt_", client_secret: "fizzy_cs_" }
+
+  # The base58 alphabet has_secure_token uses: no 0, O, I or l. Drawn through
+  # Ruby's own SecureRandom, so it doesn't depend on ActiveSupport's base58
+  # extension, which only has_secure_token loads.
+  BASE58 = [ *"1".."9", *"A".."H", *"J".."N", *"P".."Z", *"a".."k", *"m".."z" ]
+
+  # 44 base58 characters carry 257 bits, at least the 256 bits of randomness
+  # bc3's tokens have (32 bytes). Tokens issued earlier are shorter and keep
+  # working.
+  BODY_LENGTH = 44
+
   enum :permission, %w[ read write ].index_by(&:itself), default: :read
 
+  before_create :set_token
   before_create :set_expiry_and_refresh_token, if: :oauth_client_id?
 
   # Issuing or destroying a grant restarts its client's retention period
@@ -50,6 +67,10 @@ def find_honored(token)
       end
     end
 
+    def generate_token(kind)
+      PREFIXES.fetch(kind) + SecureRandom.alphanumeric(BODY_LENGTH, chars: BASE58)
+    end
+
     # Each grant is rechecked under its lock, so one renewed after it was
     # selected (by code from before idle expiry, mid-deploy, whose rotation
     # doesn't check for lapse) is spared.
@@ -106,8 +127,8 @@ def lapsed?
   # token is retired, not forgotten, so presenting it again is recognized as
   # a retry or a replay (see Oauth::RetiredRefreshToken).
   def refresh(permission: self.permission)
-    rotated = { token: self.class.generate_unique_secure_token,
-      refresh_token: self.class.generate_unique_secure_token,
+    rotated = { token: self.class.generate_token(:access),
+      refresh_token: self.class.generate_token(:refresh),
       expires_at: EXPIRES_IN.from_now, permission: permission, updated_at: Time.current }
 
     transaction do
@@ -128,8 +149,12 @@ def lock_grant
       self.class.lock.where(id: id).pluck(:id)
     end
 
+    def set_token
+      self.token ||= self.class.generate_token(oauth_client_id? ? :access : :personal)
+    end
+
     def set_expiry_and_refresh_token
       self.expires_at ||= EXPIRES_IN.from_now
-      self.refresh_token ||= self.class.generate_unique_secure_token
+      self.refresh_token ||= self.class.generate_token(:refresh)
     end
 end

app/models/oauth/client.rb

@@ -103,7 +103,7 @@ def lock_client
     end
 
     def generate_client_secret
-      self.client_secret ||= self.class.generate_unique_secure_token
+      self.client_secret ||= Identity::AccessToken.generate_token(:client_secret)
     end
 
     def redirect_uris_are_valid

docs/oauth/spec_conformance.md

@@ -84,6 +84,12 @@ Status key:
 | Secrets stored in plaintext, as access tokens are | **Deliberate** (#3082 body) |
 | An OAuth grant can't reach a credential or consent surface: personal access tokens, Connected Apps, passkey registration, transfer links, email changes or OAuth consent. Anything it minted there would outlive disconnecting the app, and anything it removed would belong to another app | **Fixed** #2296 `196b9b0b3` "Keep OAuth grants out of every credential and consent surface" (`Authentication.disallow_oauth_grants`, declared on each such controller). Personal access tokens may still manage personal access tokens (fizzy-cli) |
 
+## Token format
+
+| Requirement | Status |
+|---|---|
+| Newly issued tokens carry a distinctive prefix naming their kind: `fizzy_pat_` (personal access token), `fizzy_at_` (OAuth access token), `fizzy_rt_` (OAuth refresh token). Tokens issued earlier have no prefix and still work | **Fixed** #3163. bc3 prefixes similarly (`bc_at_`, `bc_rt_`, and PATs as `bc_at_`). Client secrets carry `fizzy_cs_`, as bc3's carry `bc_cs_`. New tokens and secrets carry 44 base58 characters after the prefix, 257 bits, at least bc3's 32 bytes (rubric TK-1); earlier ones are 24 characters and keep working. Nobody has registered with GitHub's secret-scanning partner program, either here or in bc3. |
+
 ## Refresh tokens (RFC 6749 §6, OAuth 2.1 §4.3, RFC 9700 §4.14)
 
 | Requirement | Status |

test/integration/oauth_flow_test.rb

@@ -524,8 +524,8 @@ class OauthFlowTest < ActionDispatch::IntegrationTest
     assert_response :success
     body = response.parsed_body
 
-    assert_not_nil body["access_token"]
-    assert_not_nil body["refresh_token"]
+    assert body["access_token"].start_with?("fizzy_at_")
+    assert body["refresh_token"].start_with?("fizzy_rt_")
     assert_operator body["expires_in"], :>, 0
     assert_equal "Bearer", body["token_type"]
     assert_equal "read", body["scope"]
@@ -1166,6 +1166,29 @@ class OauthFlowTest < ActionDispatch::IntegrationTest
     assert_not_equal old_refresh_token, body["refresh_token"]
   end
 
+  test "a grant issued before prefixes still authenticates, and refreshes into prefixed tokens" do
+    client = oauth_clients(:mcp_client)
+    grant = identities(:david).access_tokens.create!(oauth_client: client,
+      token: "legacyAccessToken0123456789", refresh_token: "legacyRefreshToken0123456789")
+
+    get user_path(users(:david)), env: { "HTTP_AUTHORIZATION" => "Bearer #{grant.token}" }, as: :json
+    assert_response :success
+
+    untenanted do
+      post oauth_token_path, params: { grant_type: "refresh_token", refresh_token: grant.refresh_token, client_id: client.client_id }, as: :json
+    end
+
+    assert_response :success
+    assert response.parsed_body["access_token"].start_with?("fizzy_at_")
+    assert response.parsed_body["refresh_token"].start_with?("fizzy_rt_")
+  end
+
+  test "a personal access token issued before prefixes still authenticates" do
+    get user_path(users(:david)), env: { "HTTP_AUTHORIZATION" => "Bearer #{identity_access_tokens(:davids_api_token).token}" }, as: :json
+
+    assert_response :success
+  end
+
   test "refresh grant for a token whose client is gone fails as invalid_grant" do
     client = oauth_clients(:mcp_client)
     token = identities(:david).access_tokens.create!(oauth_client: client)

test/integration/oauth_revocation_test.rb

@@ -128,6 +128,18 @@ class OauthRevocationTest < ActionDispatch::IntegrationTest
     assert Identity::AccessToken.exists?(token.id)
   end
 
+  test "tokens issued before prefixes revoke as before" do
+    personal = identities(:david).access_tokens.create!(permission: :read, token: "legacyPersonalToken0123456789")
+    grant = identities(:david).access_tokens.create!(oauth_client: oauth_clients(:mcp_client),
+      token: "legacyAccessToken0123456789", refresh_token: "legacyRefreshToken0123456789")
+
+    revoke personal.token
+    revoke grant.refresh_token, params: { client_id: oauth_clients(:mcp_client).client_id }
+
+    assert_not Identity::AccessToken.exists?(personal.id)
+    assert_not Identity::AccessToken.exists?(grant.id)
+  end
+
   test "a revoked personal access token no longer authenticates" do
     token = personal_token
     get user_path(users(:david)), env: { "HTTP_AUTHORIZATION" => "Bearer #{token.token}" }, as: :json

test/models/identity/access_token_test.rb

@@ -16,6 +16,25 @@ class Identity::AccessTokenTest < ActiveSupport::TestCase
     assert_not token.expired?
   end
 
+  test "new tokens carry a prefix naming their kind" do
+    personal = identities(:david).access_tokens.create!(description: "Personal")
+    grant = identities(:david).access_tokens.create!(oauth_client: oauth_clients(:mcp_client))
+
+    assert_match(/\Afizzy_pat_[1-9A-HJ-NP-Za-km-z]{44}\z/, personal.token)
+    assert_match(/\Afizzy_at_[1-9A-HJ-NP-Za-km-z]{44}\z/, grant.token)
+    assert_match(/\Afizzy_rt_[1-9A-HJ-NP-Za-km-z]{44}\z/, grant.refresh_token)
+  end
+
+  test "a rotation issues prefixed tokens, even for a grant issued before prefixes" do
+    grant = identities(:david).access_tokens.create!(oauth_client: oauth_clients(:mcp_client),
+      token: "legacyAccessToken0123456789", refresh_token: "legacyRefreshToken0123456789")
+
+    assert grant.refresh
+
+    assert grant.token.start_with?("fizzy_at_")
+    assert grant.refresh_token.start_with?("fizzy_rt_")
+  end
+
   test "expired?" do
     token = identities(:david).access_tokens.create!(oauth_client: oauth_clients(:mcp_client))
 

test/models/oauth/client_test.rb

@@ -246,7 +246,7 @@ class Oauth::ClientTest < ActiveSupport::TestCase
       token_endpoint_auth_method: "client_secret_post"
     )
     assert client.confidential?
-    assert_not_nil client.client_secret
+    assert_match(/\Afizzy_cs_[1-9A-HJ-NP-Za-km-z]{44}\z/, client.client_secret)
   end
 
   test "public clients get no client secret" do