fix(discovery): expired bearer tokens fail closed on public endpoints (F-015) - #14
Merged
Merged
Conversation
OptionalBearerUserResolver resolved bearer tokens via PersonalAccessToken::findToken() outside the auth:sanctum guard, which never checks expires_at. A token expired in the past kept granting its owner's community scope on the public catalog and search endpoints, defeating token expiry as a revocation mechanism there (ADR-31 gap, tracked as F-015). The resolver now treats a resolved-but-expired token the same as any other bearer that fails to resolve: an empty, unpersisted User (empty scope), never the owner's scope and never the anonymous public scope (ADR-23). A token without expires_at is unaffected and keeps resolving to its owner, since sanctum.expiration stays out of scope here. The characterization test that locked in the old, wrong behavior is inverted in this same commit to assert the new empty-scope outcome, and gains coverage for a token that expired seconds ago, a token with no expires_at, and confirmation that the other 7 header cases are unchanged. An unrelated pre-existing unused import in the same test file is also removed to keep Pint green. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5 <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
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.
Closes F-015, the gap ADR-31 recorded on purpose instead of fixing inside a refactor: the public discovery endpoints resolve the optional bearer outside the
auth:sanctumguard, andPersonalAccessToken::findToken()does not checkexpires_at, so an expired token still granted its owner's scope.What changed
App\Services\Auth\OptionalBearerUserResolvershort-circuits betweenfindToken()and thetokenablecheck: a resolved token whoseexpires_atis past returns an unpersistedUser— the same empty scope as any non-empty bearer that fails to resolve, never the public scope.expires_atis untouched and still resolves to its owner.expires_at, and a case pinning the other seven header cases as unchanged.docs/DECISIONS.md; F-015 closed in the audit; the brain updated.Only the expired-token row of the 8-case matrix moves, from owner to empty scope.
Deliberately not changed
config('sanctum.expiration')stays null, so a token issued without an explicitexpires_atstill never lapses, on every surface. Setting a lifetime changes login and session behavior through the guard — a product decision, queued inNEXT_ACTIONS.md. Routes, private middleware, policies andDiscoveryAccessuntouched.Evidence
Independent adversarial audit: APPROVED. It probed the resolver inside the container against a copy of main's class, case by case, and proved the
expires_atnull edge by execution rather than by reading?->. It also showed the test bites: with main's resolver and this branch's test file, exactly the two expired-token cases fail (Expected response status code [404] but received 200) while the other ten pass. The inverted assertion is stronger than the one it replaces, because 404 on the owner's own plugin separates empty scope from both owner (200) and public (200).Assertion inventory main → branch: nothing shrank (
assertNotFound11 → 18,assertOk17 → 20, the rest equal). Noskip,markTestSkipped,->todo(, orxit(.Gates
./vendor/bin/pest— 100 passed, on a rebuilt image whose file hashes were confirmed against the branch../vendor/bin/pint --test— PASS, 134 files.PENDINGhere: this branch is backend-only.🤖 Generated with Claude Code