Skip to content

refactor(discovery): single optional bearer user resolution (F-013) - #12

Merged
LuanTrindade95 merged 5 commits into
mainfrom
refactor/optional-bearer-user-resolver
Sep 23, 2026
Merged

LuanTrindade95 merged 5 commits into
mainfrom
refactor/optional-bearer-user-resolver

Conversation

@LuanTrindade95

@LuanTrindade95 LuanTrindade95 commented Sep 23, 2026 •

Copy link
Copy Markdown
Owner

Closes F-013 (remediation item C6): the optional-user resolution used by the public discovery endpoints was duplicated, character for character, in CatalogController and SearchController.

What changed

  • App\Services\Auth\OptionalBearerUserResolver, a stateless function of Request, injected by constructor into both controllers. The two private currentUser() methods are gone; the extracted body is character-identical to the original (486 characters normalized).
  • backend/tests/Feature/OptionalBearerUserResolutionTest.php: 8 header cases (no header, Authorization: Basic, empty Bearer, valid member token, garbage token, revoked token, valid token without membership, expired token) across the 8 public catalog and search endpoints, asserting status, item counts, and absence of private-community data.
  • ADR-31 in docs/DECISIONS.md, BRAIN-011 in the brain, F-013 closed, F-015 opened.

No route, middleware, policy, DiscoveryAccess, or response change.

Commit order is part of the evidence

05de965 holds only the test file, on a tree whose controllers are still main's. 8e1bdc4 holds the refactor. The test blob is byte-identical across both commits, so the characterization could not have been retrofitted to the new behavior.

Two behaviors recorded, not fixed

  1. A non-bearer Authorization header and an empty Bearer resolve to the public scope, not the empty scope. The guard clause returns null as soon as bearerToken() is null or empty, so the empty scope exists only for a non-empty bearer that fails to resolve. The F-013 text claimed otherwise; the code is the contract.
  2. F-015: an expired token is still accepted as its owner, because these endpoints resolve it through PersonalAccessToken::findToken() outside the auth:sanctum guard and sanctum.expiration is null. Pre-existing, locked by a test named as a gap, queued in NEXT_ACTIONS.md. Fixing it changes endpoint responses and belongs to its own branch.

Gates

  • ./vendor/bin/pest — 97 passed (544 assertions) after merging main and rebuilding the backend image; 95 before the merge.
  • ./vendor/bin/pint --test — PASS, 134 files. Frontend gates are PENDING on this branch (backend-only change) and VALIDATED on main for the CSP work merged in.
  • No skip, markTestSkipped, ->todo(, or xit( under backend/tests.

Independent adversarial audit: APPROVED. The auditor compared the three bodies whitespace-normalized, probed the header classes inside the running container, and confirmed DiscoveryAccess::communityIds() maps null to every community and an unsaved User to []. It could not re-execute the suite at 05de965 (the backend service has no bind mount), so equivalence there rests on the character identity plus the unchanged test blob.

🤖 Generated with Claude Code

Note: main took ADR-30 and BRAIN-009 for the CSP work while this branch was open, so the records here were renumbered to ADR-31 and BRAIN-011 before merging.

LuanTrindade95 and others added 5 commits September 21, 2026 10:16
Locks in the current currentUser() behavior in CatalogController and
SearchController before extracting a shared resolver: anonymous scope
for no/invalid Authorization schemes and empty bearer, owner scope for
a resolvable token, empty scope for an unresolvable non-empty bearer,
and the pre-existing gap where PersonalAccessToken::findToken() does
not check expires_at. All 9 cases pass against the unrefactored code.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
CatalogController and SearchController each duplicated the same
optional-user resolution logic for public discovery endpoints. Extract
it into a single App\Services\Auth\OptionalBearerUserResolver, injected
into both controllers, so the fail-closed behavior for an invalid
bearer token (never widen to the anonymous public scope, per ADR-23)
lives in one place.

Behavior is unchanged: the characterization suite added in the previous
commit passes identically before and after (9/9, 120 assertions each
run). Two pre-existing gaps found while characterizing, both left
untouched per the correction scope: an "Authorization: Basic ..." or an
empty "Bearer " header resolve to the public scope rather than the
empty/fail-closed scope, and PersonalAccessToken::findToken() does not
check expires_at, so an expired bearer token is still accepted as its
owner on these public endpoints.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Closes remediation item C6. Records the single optional-user
resolution point (ADR-30, BRAIN-009), marks F-013 closed, and
opens F-015 for the expired bearer token accepted on public
discovery endpoints, which was found while characterizing F-013
and deliberately left uncorrected.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…rer-user-resolver

# Conflicts:
#	brain/audits/2026-06-13-system-audit.md
#	brain/canonico/CURRENT_STATE.md
#	brain/canonico/DECISIONS.md
#	brain/canonico/NEXT_ACTIONS.md
#	docs/DECISIONS.md
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@LuanTrindade95
LuanTrindade95 merged commit 0ebb44c into main Sep 23, 2026
4 checks passed
@LuanTrindade95
LuanTrindade95 deleted the refactor/optional-bearer-user-resolver branch September 23, 2026 11:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant