PPT-2536: heal scope-less refresh chains + bridge legacy Doorkeeper refresh tokens - #10
Merged
Merged
Conversation
… refresh tokens Steve's pre-UCLA testing (2026-07-25) hit 403s on refreshed tokens even with the scope-across-refresh fix in place, and dev was reverted to the Ruby auth. Forensics on dev's oauth_tokens table found refresh grants still minting empty-scope access tokens (Workplace + Backoffice clients), exposing two uncovered refresh classes: A. auth.cr refresh tokens minted BEFORE the scope-embedding fix carry `user_id` but no `scope` claim. Scope recovery found nothing -> "" -> every API call 403s on rest-api's can_read, and each refresh re-embeds the empty scope, so the client is stuck until a full re-login. Fix: when a refresh grant has a resource owner but no recoverable scope, fall back to `public` (Ruby parity: Doorkeeper default_scopes is :public and every PlaceOS client's grant is `public`). The re-minted refresh token embeds the healed scope, permanently repairing the chain. Client-only grants keep "" - no privilege is invented for machine tokens. B. Refresh tokens issued by the legacy RUBY service are opaque Doorkeeper strings (stored SHA256-hashed in oauth_access_tokens via hash_token_secrets), not JWTs - validate_code! rejected them outright, forcing every mid-session client through an interactive re-login at cutover. Fix: LegacyRefresh bridge - on JWT decode failure, look up the unrevoked Doorkeeper row by digest, require the redeeming client to own it, mint native tokens for its resource owner + scopes, and revoke the row (Doorkeeper rotation semantics). The replacement refresh token is an auth.cr JWT, so the chain migrates off Doorkeeper after one refresh. Also guards generate_id_token (refresh has no auth code) and routes legacy revocation off the JWT token manager. Specs (refresh_hardening_spec): pre-fix-token heal + permanent chain repair, no-scope-invention for ownerless grants, Doorkeeper redeem (user/scopes/revocation/chain migration), scope-less row heal, revoked row rejected, cross-client redemption rejected without side effects, unknown opaque token rejected, and public-client (SPA) refresh without a client secret. Red-check verified: neutering either fix fails exactly the corresponding specs. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
A row-returning `db.query*` with varargs-splatted arguments crashes the
Crystal compiler ("BUG: ... Tuple#each MacroFor ... should have been
expanded", tuple.cr:367). Reach is toolchain dependent: on CI's compiler
only the --release/--static image build failed, so the first CI run for
this branch went green on tests and red on Build; on Crystal 1.16.3 the
spec build ICEs as well. `db.exec` with splatted args is unaffected.
Bind the parameter via the `args:` Array overload instead, and read the
columns off the ResultSet rather than materialising a nilable tuple.
No behavioural change: the query still returns at most one row (LIMIT 1)
and nil when there is no unrevoked match.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
Weekend pre-UCLA testing (Steve) hit 403s on refreshed tokens on placeos-dev — despite PR #8's scope-across-refresh fix — and dev was reverted to Ruby auth on 2026-07-25.
Forensics
Dev's
oauth_tokenstable shows refresh grants still minting empty-scope access tokens after PR #8 was live (Jul 24: Workplace client; Jul 25: Backoffice — each at the same second as a refresh-rotation revocation row,subintact). Two refresh classes were uncovered:Class A — pre-fix auth.cr refresh tokens. Tokens minted before the scope-embedding fix carry
user_idbut noscopeclaim. Recovery yields""→ every API call 403s on rest-api'scan_read→ and each refresh re-embeds the empty scope, so the client is permanently stuck until a full re-login. This is what killed the weekend testing.Class B — legacy Ruby (Doorkeeper) refresh tokens. Opaque strings, stored SHA256-hashed (
hash_token_secrets) inoauth_access_tokens— 81 fresh rows since the revert.validate_code!requiresAuthly.jwt_decodeto succeed, so every one of them is rejected → every mid-session client is forced through an interactive re-login at cutover. At UCLA scale that's the whole install base.Fix
public— Ruby parity (default_scopes :public; every PlaceOS client's grant ispublic). The re-minted refresh token embeds the healed scope, permanently repairing the chain. Client-only grants keep""— no privilege invented for machine tokens.LegacyRefresh— on JWT decode failure, look up the unrevoked Doorkeeper row by digest, require the redeeming client to own it, mint native tokens for its resource owner + scopes, revoke the row (Doorkeeper rotation semantics). The replacement refresh token is a native JWT — the chain migrates off Doorkeeper after one refresh. Missing table / DB errors degrade to "not a legacy token".generate_id_tokenno longer assumes an auth code exists (refresh +openidscope would have 500'd); legacy revocation bypasses the JWT token manager (which decodes).Specs (
refresh_hardening_spec.cr)Pre-fix-token heal →
["public"]+ chain permanently repaired · no scope invented for ownerless grants · Doorkeeper redeem (user, scopes,aud/uclaims, row revoked, chain migrates to JWT) · scope-less row heals topublic· revoked row rejected · cross-client redemption rejected without side effects · unknown opaque token rejected · public client (SPA) refreshes without a client secret.Red-check verified: neutering the heal fails exactly the Class A specs; neutering the bridge fails exactly the Doorkeeper specs. 22 examples green across the refresh battery + claims + PKCE.
🤖 Generated with Claude Code