Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 0 additions & 7 deletions app/models/identity/access_token.rb
Original file line number Diff line number Diff line change
Expand Up @@ -51,13 +51,6 @@ class Identity::AccessToken < ApplicationRecord
# Tokens are stored only as digests (TokenDigester). The raw values exist
# only in memory, on the record that issued or rotated them, to be handed to
# their holder once.
#
# The plaintext `token` and `refresh_token` columns are ignored until they're
# dropped. Without partial_inserts, a model that knows a column names it on
# every INSERT, so only code that ignores it can keep creating tokens once
# it's gone.
self.ignored_columns += %w[ token refresh_token ]

attribute :raw_token, :string
attribute :raw_refresh_token, :string

Expand Down
37 changes: 37 additions & 0 deletions db/migrate/20261017130000_drop_identity_access_token_plaintext.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
# Drops the plaintext personal access tokens that BackfillTokenDigests and
# BackfillRemainingTokenDigests left beside their digests, and the plaintext
# refresh token column, empty since BackfillTokenDigests.
#
# Run it only once every process runs the code that shipped with
# BackfillRemainingTokenDigests, or later. That code finds tokens only by
# digest and ignores both columns, so it never names them. Code from before it
# still writes personal access tokens' plaintext, and falls back to reading
# it, so it fails once the column is gone. Code from before
# DropPlaintextOauthCredentials, which doesn't ignore refresh_token, names it
# on every INSERT.
#
# Undoing this restores both columns empty and nullable, as they were before
# any token was written. The tokens they held can't be recovered from their
# digests, so code that reads only plaintext can't authenticate them again.
class DropIdentityAccessTokenPlaintext < ActiveRecord::Migration[8.2]
def up
if connection.adapter_name.in?(%w[ Mysql2 Trilogy ])
# MySQL would drop the columns INSTANT, from the data dictionary alone,
# leaving each row's plaintext in the table's pages until that row is
# next rewritten, which for most tokens is never. Rebuilding the table
# writes every row without them, still without blocking reads or writes.
execute "ALTER TABLE identity_access_tokens DROP INDEX index_identity_access_tokens_on_refresh_token, " \
"DROP COLUMN refresh_token, DROP COLUMN token, ALGORITHM=INPLACE, LOCK=NONE"
else
remove_index :identity_access_tokens, :refresh_token, unique: true
remove_column :identity_access_tokens, :refresh_token
remove_column :identity_access_tokens, :token
end
end

def down
add_column :identity_access_tokens, :token, :string
add_column :identity_access_tokens, :refresh_token, :string
add_index :identity_access_tokens, :refresh_token, unique: true
end
end
5 changes: 1 addition & 4 deletions db/schema.rb

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

5 changes: 1 addition & 4 deletions db/schema_sqlite.rb
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@
#
# It's strongly recommended that you check this file into your version control system.

ActiveRecord::Schema[8.2].define(version: 2026_10_17_120000) do
ActiveRecord::Schema[8.2].define(version: 2026_10_17_130000) do
create_table "accesses", id: :uuid, force: :cascade do |t|
t.datetime "accessed_at"
t.uuid "account_id", null: false
Expand Down Expand Up @@ -359,15 +359,12 @@
t.uuid "identity_id", null: false
t.uuid "oauth_client_id"
t.string "permission", limit: 255
t.string "refresh_token", limit: 255
t.string "refresh_token_digest", limit: 80
t.string "token", limit: 255
t.string "token_digest", limit: 80
t.datetime "updated_at", null: false
t.index ["authorization_code_jti"], name: "index_identity_access_tokens_on_authorization_code_jti", unique: true
t.index ["identity_id"], name: "index_access_token_on_identity_id"
t.index ["oauth_client_id"], name: "index_identity_access_tokens_on_oauth_client_id"
t.index ["refresh_token"], name: "index_identity_access_tokens_on_refresh_token", unique: true
t.index ["refresh_token_digest"], name: "index_identity_access_tokens_on_refresh_token_digest", unique: true
t.index ["token_digest"], name: "index_identity_access_tokens_on_token_digest", unique: true
end
Expand Down
4 changes: 3 additions & 1 deletion docs/docker-deployment.md
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,9 @@ Once you have one, set it in the `SECRET_KEY_BASE` environment variable:
docker run --env SECRET_KEY_BASE=abcdefabcdef ...
```

Personal access tokens and OAuth credentials are looked up by a keyed digest (HMAC-SHA256) rather than by the token itself.
Personal access tokens and OAuth credentials are stored only as a keyed digest (HMAC-SHA256), and looked up by it: the tokens themselves aren't kept.
Comment thread
jeremy marked this conversation as resolved.
Comment thread
jeremy marked this conversation as resolved.
The one exception is the token pair an OAuth refresh issues, which is kept encrypted, readable only for the 60 seconds in which a client may retry that refresh, and cleared within minutes after.
Tokens issued by versions that stored them in plaintext may still be readable in backups taken before the upgrade, and, on SQLite, in the database file's free pages until you run `VACUUM` (with the app stopped, and free disk space the size of the database).
By default the key is derived from `SECRET_KEY_BASE`, so **changing `SECRET_KEY_BASE` invalidates every personal access token and every connected OAuth app**: people have to generate new tokens and reconnect their apps.
To keep tokens working across a change of `SECRET_KEY_BASE`, set the key separately in `OAUTH_TOKEN_PEPPER` (at least 32 characters, for example from `bin/rails secret`).
Set it before upgrading to a version that digests tokens, or before anyone creates one, and then keep it: changing, adding or removing it later invalidates tokens in the same way.
Expand Down
3 changes: 2 additions & 1 deletion docs/oauth/spec_conformance.md
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ Snapshot: 2026-10-06, at the heads of the OAuth train, rebased onto main:
| [#3159](https://github.com/basecamp/fizzy/pull/3159) | `oauth-client-sweep` | Sweep of abandoned DCR clients (on #3161) |
| [#3192](https://github.com/basecamp/fizzy/pull/3192) | `oauth-token-digests` | Tokens and client secrets stored as digests, step 1 of 3 (on #3169) |
| [#3202](https://github.com/basecamp/fizzy/pull/3202) | `oauth-token-digests-d2` | Personal access tokens stored and found only by digest, step 2 of 3 (on #3192) |
| [#3203](https://github.com/basecamp/fizzy/pull/3203) | `oauth-token-digests-d3` | Plaintext token columns dropped, step 3 of 3 (on #3202) |

Commits are cited by subject, with the short SHA as of the snapshot. Rebases change
the SHAs of #3080 and above; the subjects stay the same.
Expand Down Expand Up @@ -90,7 +91,7 @@ Status key:

| Requirement | Status |
|---|---|
| Tokens are stored as HMAC-SHA256 digests under a server pepper, byte-compatible with bc3's `Oauth::TokenDigester`, and looked up by a unique index on the digest. Raw values are shown once: in the token response, the registration response, or the response that creates a personal access token | **Fixed** #3192 `oauth-token-digests`, step 1 of 3. OAuth access and refresh tokens and client secrets are digest-only, and a refresh retry within the grace window is answered from an encrypted copy of the successor pair that expires with the window. Step 2 (#3202 `oauth-token-digests-d2`) makes personal access tokens digest-only too: they're written and found only by digest, after a rerun of the backfill digests any that production wrote in plaintext before step 1 reached it. The plaintext they held before then stays in a column the code ignores, until step 3 drops it. |
| Tokens are stored as HMAC-SHA256 digests under a server pepper, byte-compatible with bc3's `Oauth::TokenDigester`, and looked up by a unique index on the digest. Raw values are shown once: in the token response, the registration response, or the response that creates a personal access token | **Fixed** #3192 `oauth-token-digests`, step 1 of 3. OAuth access and refresh tokens and client secrets are digest-only, and a refresh retry within the grace window is answered from an encrypted copy of the successor pair that expires with the window. Step 2 (#3202 `oauth-token-digests-d2`) makes personal access tokens digest-only too: they're written and found only by digest, after a rerun of the backfill digests any that production wrote in plaintext before step 1 reached it. Step 3 (#3203 `oauth-token-digests-d3`) drops the plaintext columns (personal access tokens', and refresh tokens', empty since step 1), and with them the plaintext of tokens issued before digests: no token is stored in plaintext. |
| 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)
Expand Down
103 changes: 0 additions & 103 deletions test/migrations/backfill_remaining_token_digests_test.rb

This file was deleted.

85 changes: 0 additions & 85 deletions test/migrations/backfill_token_digests_test.rb

This file was deleted.

50 changes: 50 additions & 0 deletions test/migrations/drop_identity_access_token_plaintext_test.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,50 @@
require "test_helper"
require Rails.root.join("db/migrate/20261017130000_drop_identity_access_token_plaintext")

# DropIdentityAccessTokenPlaintext runs while processes still serve the code
# that shipped with BackfillRemainingTokenDigests, which ignores both plaintext
# columns. With partial_inserts off, a process names every column it has cached
# on each INSERT, so one that loaded identity_access_tokens' columns before the
# migration must still create tokens after it.
class DropIdentityAccessTokenPlaintextTest < ActiveSupport::TestCase
# Each step is DDL, which MySQL commits on its own.
self.use_transactional_tests = false

# identity_access_tokens as the code before this migration models it.
class PreviousAccessToken < ApplicationRecord
self.table_name = "identity_access_tokens"
self.ignored_columns = %w[ token refresh_token ]
end

setup do
migrate :down
PreviousAccessToken.reset_column_information.then { PreviousAccessToken.columns }
end

teardown do
migrate :up if ActiveRecord::Base.connection.column_exists?(:identity_access_tokens, :token)
[ PreviousAccessToken, Identity::AccessToken ].each(&:reset_column_information)
PreviousAccessToken.where(description: "Cached before the migration").delete_all
end

test "drops both plaintext columns" do
migrate :up

assert_not ActiveRecord::Base.connection.column_exists?(:identity_access_tokens, :token)
assert_not ActiveRecord::Base.connection.column_exists?(:identity_access_tokens, :refresh_token)
end

test "a process that cached identity_access_tokens' columns before it creates tokens after it" do
migrate :up

assert_difference -> { PreviousAccessToken.count } do
PreviousAccessToken.create! identity_id: identities(:david).id, permission: "write", description: "Cached before the migration",
token_digest: TokenDigester.digest(SecureRandom.base58(24))
end
end

private
def migrate(direction)
DropIdentityAccessTokenPlaintext.new.tap { |migration| migration.verbose = false }.migrate(direction)
end
end
Loading
Loading