122 lines
7.3 KiB
Markdown
122 lines
7.3 KiB
Markdown
---
|
|
phase: quick-260927-q23
|
|
plan: 01
|
|
subsystem: auth
|
|
tags: [bouncer, cabana, jwt, refresh, session-revocation, CR-01]
|
|
status: complete
|
|
requires: []
|
|
provides:
|
|
- "bouncer.RefreshAudienceFor: RefreshAudience plus the JWT guard's subject checks before minting"
|
|
- "bouncer.ErrSubjectRejected sentinel shared by the guard and refresh"
|
|
affects: [cabana admin refresh, backend JWT guard]
|
|
tech-stack:
|
|
added: []
|
|
patterns:
|
|
- "Optional check hook on the shared refreshAudience flow (nil for the golem15/user and RefreshAudience paths)"
|
|
- "One lazyBackendUsers provider shared by the backend guard and the refresh handler"
|
|
key-files:
|
|
created:
|
|
- cabana/refresh_revocation_test.go
|
|
modified:
|
|
- bouncer/jwt.go
|
|
- bouncer/refresh.go
|
|
- bouncer/refresh_test.go
|
|
- bouncer/jwt_guard_test.go
|
|
- cabana/http.go
|
|
- cabana/auth.go
|
|
- cabana/phase10_coverage_test.go
|
|
- .planning/phases/10-admin-vue-spa/10-SECURITY-REVIEW.md (uncommitted)
|
|
- .planning/phases/10-admin-vue-spa/10-REVIEW-DISPOSITION.md (uncommitted)
|
|
decisions:
|
|
- "Cookie expiry on refresh is limited to subject refusals (ErrSubjectRejected); a provider error answers 401 without expiring the cookie"
|
|
- "Subject lookup runs after all token-only checks, so unsigned, expired-window, wrong-audience or blacklisted tokens never reach Postgres"
|
|
- "bouncer.Refresh and bouncer.RefreshAudience keep their signatures and behaviour (nil hook); fonoteka.go unchanged"
|
|
metrics:
|
|
duration: 13min
|
|
completed: 2026-09-27
|
|
actuals:
|
|
tokens: 7000
|
|
tasks: 3
|
|
commits: 2
|
|
plan_head_before: 815cb903c684b25dca4eda81e61800c6351430f3
|
|
plan_head_after: a13a1214cbbaffb2ff655d1072e4a5558e5ff88c
|
|
---
|
|
|
|
# Quick 260927-q23: Admin refresh enforces tokens_valid_after and is_activated (CR-01) Summary
|
|
|
|
Admin `POST {prefix}/api/v1/auth/refresh` now loads the subject through the backend guard's own provider (`lazyBackendUsers`) and applies the guard's checks (`subjectPrincipal`, `issuedBeforeCutoff`) via the new `bouncer.RefreshAudienceFor` before it mints a token. A refresh refused for its subject over cookie transport expires `summer_admin`. So `summer admin:reset-password`, deactivation and soft-deletion now end a session the SPA keeps refreshing.
|
|
|
|
## Commits
|
|
|
|
| Task | Commit | Message |
|
|
|------|--------|---------|
|
|
| 1 | be4a923 | fix(cabana): enforce tokens_valid_after and is_activated on admin refresh |
|
|
| 2 | a13a121 | test(bouncer,cabana): cover admin refresh subject checks without a database |
|
|
| 3 | (none) | Doc edits left uncommitted for the orchestrator |
|
|
|
|
## Task 1: RED run (before the fix, not committed)
|
|
|
|
`go test ./cabana -run '^TestAdminRefreshRevocation$' -count=1 -v`:
|
|
|
|
```
|
|
refresh_revocation_test.go:88: pre-reset bearer refresh status=200 body={"data":{"access_token":"eyJ...","token_type":"bearer"},"meta":{}}, want 401 unauthenticated
|
|
refresh_revocation_test.go:103: refresh status=200 body={"data":{"token_type":"cookie","expires_in":3600},"meta":{}}, want 401 unauthenticated
|
|
refresh_revocation_test.go:114: refresh status=200 body={"data":{"token_type":"cookie","expires_in":3600},"meta":{}}, want 401 unauthenticated
|
|
--- FAIL: TestAdminRefreshRevocation (5.37s)
|
|
--- FAIL: TestAdminRefreshRevocation/pre-reset_cookie_is_refused_and_expired (0.20s)
|
|
--- FAIL: TestAdminRefreshRevocation/pre-reset_bearer_is_refused_without_cookies (0.21s)
|
|
--- FAIL: TestAdminRefreshRevocation/deactivated_admin_is_refused (0.15s)
|
|
--- FAIL: TestAdminRefreshRevocation/soft-deleted_admin_is_refused (0.14s)
|
|
--- PASS: TestAdminRefreshRevocation/active_admin_still_refreshes (0.21s)
|
|
```
|
|
|
|
The pre-reset cookie subtest failed the same way: the refresh returned status 200 with a cookie body. GREEN: all five subtests pass on Postgres. `go vet ./...` and `go test ./...` in summercms.go were green at both commits.
|
|
|
|
## Task 2: removal check
|
|
|
|
With `RefreshAudienceFor` temporarily passing `nil` instead of its check:
|
|
|
|
```
|
|
refresh_test.go:177: pre-cutoff refresh = "eyJ...", <nil>; want ErrSubjectRejected
|
|
refresh_test.go:198: missing principal: <nil>
|
|
refresh_test.go:208: nil users: <nil>
|
|
refresh_test.go:215: non-numeric sub: <nil>
|
|
refresh_test.go:223: provider error: <nil>, want a non-subject error
|
|
--- FAIL: TestRefreshAudienceForSubject (5 subtests)
|
|
phase10_coverage_test.go:462: pre-cutoff cookie: status=200 body={"data":{"token_type":"cookie","expires_in":900},"meta":{}}
|
|
--- FAIL: TestPhase10Coverage/refresh_enforces_the_guard's_subject_checks
|
|
```
|
|
|
|
Restored with `git checkout -- bouncer/refresh.go`. `git diff --quiet -- bouncer/refresh.go` held, and the tests were green again.
|
|
|
|
## Task 3: gates
|
|
|
|
- `git show --stat` of be4a923 and a13a121 shows no go.mod or go.sum change. `../fonoteka.go` has no working-tree change.
|
|
- `bash scripts/check-phase10.sh --go`: the third run exited 0 (`phase10 go passed`, with only the two accepted parity failures TestMigrateSeedsCanonicalGenres and TestSchemaMatchesPHPSnapshot). The first two runs failed on one different fonoteka golem15/user test each: `user/classes TestCodes` the first time, `user TestForgotPassword` the second. Each passed when re-run on its own (TestForgotPassword three times). A full standalone fonoteka.go `go test -json ./... ./plugins/golem15/fonoteka/... ./plugins/golem15/user/...` showed only the two parity failures. Neither test calls refresh or the JWT guard: forgot-password and code issuance are unauthenticated. TestCodes asserts `time.Since(issuedAt) <= 2s` (codes_test.go:37). Load average was 10 to 14 on 12 cores, and another project's testcontainers Postgres was running at the same time. So these look like load flakes and are not caused by this change. See Deferred Issues.
|
|
- `bash scripts/check-phase10.sh --evidence`: exit 0 (table parse, security, postgres, openapi. The admin OpenAPI document and fonoteka docs/openapi.json did not drift).
|
|
- `10-REVIEW-DISPOSITION.md` CR-01 row: `| CR-01 | critical | fixed | Fixed in be4a923 (tests a13a121), quick 260927-q23. ...`
|
|
- `10-SECURITY-REVIEW.md` T-10-05 row: 9 cells. It names RefreshAudienceFor, TestAdminRefreshRevocation, TestRefreshAudienceForSubject and WR-07, and no longer says "valid until logout or expiry".
|
|
- Both docs are modified and uncommitted.
|
|
|
|
## Deviations from Plan
|
|
|
|
None in the code. Test-structure choice: each TestAdminRefreshRevocation subtest builds its own `adminHandler`, so the per-app login throttle (5 per minute) cannot trip across the six logins in the test.
|
|
|
|
## Deferred Issues
|
|
|
|
- fonoteka.go `plugins/golem15/user/classes TestCodes` and `plugins/golem15/user TestForgotPassword` each failed once, only under the full parallel `check-phase10.sh --go` run on a loaded machine. They pass alone and passed on the third gate run. Likely cause: timing sensitivity under load (for example the 2 s `time.Since` bound in codes_test.go:37). Out of scope here, and there is no code path shared with this change.
|
|
|
|
## Known Stubs
|
|
|
|
None.
|
|
|
|
## Threat Flags
|
|
|
|
None. No new endpoint or trust boundary. The refresh endpoint now does one extra indexed primary-key lookup, and only for tokens that passed every signature, audience, window and blacklist check (T-q23-04).
|
|
|
|
## Self-Check: PASSED
|
|
|
|
- FOUND: cabana/refresh_revocation_test.go
|
|
- FOUND: be4a923, a13a121 in git log
|
|
- FOUND: `RefreshAudienceFor(r.Context(), s.users` in cabana/auth.go, `users: users` in cabana/http.go, `subjectPrincipal(r.Context(), g.users` in bouncer/jwt.go, `issuerURL, nil)` twice in bouncer/refresh.go
|