PPT-2536: token-endpoint + refresh-semantics specs (TK-03..08, RF-06..09) - #19
Merged
Conversation
…07/08, RF-06..09) 23 examples across two new files. Every refresh success asserts the resulting access token's exact `scope` array AND `sub` — the two claims whose loss caused both production reverts. Every negative is paired with a positive control so a refusal cannot pass for an unrelated reason (e.g. TK-06 mints two sibling codes that differ only in redirect_uri). Pins several deliberate divergences from Ruby/Doorkeeper rather than asserting the RFC text: client-auth failure returns unauthorized_client (Doorkeeper: invalid_client — status and headers do match), redirect_uri mismatch returns invalid_redirect_uri, refresh with an unknown client_id returns invalid_scope because Grant#token validates scope before authorization, and client_credentials issues a refresh_token that redeems to an empty scope. Two examples are marked KNOWN GAP and pin current behaviour deliberately: a revoked refresh token is still redeemable, because validate_code! never consults the token store. They must flip to 400 invalid_grant when that is fixed, so the fix cannot land silently. 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.
P1 coverage for the OAuth token endpoint and remaining refresh semantics. Test-only. 23 examples.
Every refresh success asserts the resulting access token's exact
scopearray andsub— the two claims whose loss caused both production reverts. Every negative is paired with a positive control so a refusal cannot pass for an unrelated reason (TK-06 mints two sibling codes differing only inredirect_uri).Deliberate divergences now pinned
Rather than asserting RFC text and failing, these record what auth.cr actually does so a change surfaces as a decision:
unauthorized_client(Doorkeeper:invalid_client; status and headers do match)redirect_urimismatch →invalid_redirect_uri(Doorkeeper:invalid_grant)client_id→ 400invalid_scope, becauseGrant#tokenvalidates scope before authorizationclient_credentialsissues arefresh_token(RFC 6749 §4.4.3 says SHOULD NOT; Doorkeeper issued none) which redeems to an empty scopeDefect pinned as KNOWN GAP — must flip when fixed
A revoked refresh token is still redeemable.
Authly::RefreshToken#validate_code!only checks that the JWT decodes; it never consults the token store, and the refreshjtiis never persisted. SoPOST /auth/revokewritesrevoked_atand the token keeps minting access tokens for its full 30-day TTL — revocation and logout do not end a session. Doorkeeper had no such gap (both tokens were columns of one row).Two examples pin the current behaviour deliberately and must flip to 400
invalid_grantwhen this is closed, so the fix cannot land silently.Independently found by two reviewers. Verified as upstream authly behaviour, not introduced by PR #10's override of that method. The fix is not trivial: naive enforcement breaks RF-05 (P0 — the SPA boot race double-submits its refresh token, and rotation already revokes the old jti), so it needs the refresh jti linked to the access-token row plus a grace window.
Also noted
refresh_hardening_spec.cr:110wraps its only assertion inif result.status_code == 200, so it passes vacuously on rejection. RF-07 here supersedes it using the real client-credentials refresh token the endpoint actually issues.195 examples, 0 failures, 1 pending.🤖 Generated with Claude Code