|
|
|
|
@@ -0,0 +1,303 @@
|
|
|
|
|
---
|
|
|
|
|
phase: quick-260927-q23
|
|
|
|
|
plan: 01
|
|
|
|
|
type: execute
|
|
|
|
|
wave: 1
|
|
|
|
|
depends_on: []
|
|
|
|
|
files_modified:
|
|
|
|
|
- bouncer/jwt.go
|
|
|
|
|
- bouncer/refresh.go
|
|
|
|
|
- bouncer/refresh_test.go
|
|
|
|
|
- bouncer/jwt_guard_test.go
|
|
|
|
|
- cabana/http.go
|
|
|
|
|
- cabana/auth.go
|
|
|
|
|
- cabana/refresh_revocation_test.go
|
|
|
|
|
- cabana/phase10_coverage_test.go
|
|
|
|
|
- .planning/phases/10-admin-vue-spa/10-SECURITY-REVIEW.md
|
|
|
|
|
- .planning/phases/10-admin-vue-spa/10-REVIEW-DISPOSITION.md
|
|
|
|
|
autonomous: true
|
|
|
|
|
requirements: [ADMIN-06]
|
|
|
|
|
|
|
|
|
|
estimate:
|
|
|
|
|
tokens: 70000
|
|
|
|
|
raw_tokens: 70000
|
|
|
|
|
tasks: 3
|
|
|
|
|
confidence: low
|
|
|
|
|
|
|
|
|
|
must_haves:
|
|
|
|
|
truths:
|
|
|
|
|
- "After `summer admin:reset-password`, POST {prefix}/api/v1/auth/refresh with a token issued before the reset answers 401 with error.code unauthenticated and mints no token, over both cookie and Bearer transport"
|
|
|
|
|
- "A cookie-transport refresh refused because the admin is missing, soft-deleted, deactivated or cut off by tokens_valid_after sends Set-Cookie summer_admin with an empty value, Max-Age below 0 and Path equal to the admin prefix"
|
|
|
|
|
- "A Bearer-transport refresh never sets or expires the summer_admin cookie, including when it is refused"
|
|
|
|
|
- "An active admin whose token postdates tokens_valid_after still refreshes over cookie and Bearer with the unchanged Phase 10 response shapes"
|
|
|
|
|
- "bouncer.Refresh (the golem15/user frontend refresh) and the JWT guard keep their exact behaviour, order of checks and error messages"
|
|
|
|
|
artifacts:
|
|
|
|
|
- path: "bouncer/refresh.go"
|
|
|
|
|
provides: "RefreshAudienceFor: RefreshAudience plus the guard's subject checks before minting"
|
|
|
|
|
contains: "func RefreshAudienceFor"
|
|
|
|
|
- path: "bouncer/jwt.go"
|
|
|
|
|
provides: "ErrSubjectRejected, subjectPrincipal and issuedBeforeCutoff shared by the JWT guard and RefreshAudienceFor"
|
|
|
|
|
contains: "ErrSubjectRejected"
|
|
|
|
|
- path: "cabana/auth.go"
|
|
|
|
|
provides: "admin refresh handler using RefreshAudienceFor and expiring the cookie on a rejected subject"
|
|
|
|
|
contains: "RefreshAudienceFor(r.Context(), s.users"
|
|
|
|
|
- path: "cabana/refresh_revocation_test.go"
|
|
|
|
|
provides: "Postgres regression test for CR-01 using the real admin:reset-password command"
|
|
|
|
|
contains: "TestAdminRefreshRevocation"
|
|
|
|
|
key_links:
|
|
|
|
|
- from: "cabana/http.go"
|
|
|
|
|
to: "bouncer.NewBackendJWTGuard and service.users"
|
|
|
|
|
via: "one lazyBackendUsers value passed to both"
|
|
|
|
|
pattern: "users:\\s+users"
|
|
|
|
|
- from: "cabana/auth.go (*service).refresh"
|
|
|
|
|
to: "bouncer.RefreshAudienceFor"
|
|
|
|
|
via: "s.users provider"
|
|
|
|
|
pattern: "RefreshAudienceFor\\(r\\.Context\\(\\), s\\.users"
|
|
|
|
|
- from: "bouncer/jwt.go jwtGuard.Authenticate"
|
|
|
|
|
to: "subjectPrincipal and issuedBeforeCutoff"
|
|
|
|
|
via: "shared helper, same order and messages as before"
|
|
|
|
|
pattern: "subjectPrincipal\\(r\\.Context\\(\\), g\\.users"
|
|
|
|
|
- from: "bouncer/refresh.go Refresh"
|
|
|
|
|
to: "refreshAudience"
|
|
|
|
|
via: "nil subject check, so the golem15/user refresh contract is unchanged"
|
|
|
|
|
pattern: "AudienceUser, true, refreshTTL, bl, grace, issuerURL, nil\\)"
|
|
|
|
|
---
|
|
|
|
|
|
|
|
|
|
<objective>
|
|
|
|
|
Fix review finding CR-01 (Phase 10, 10-REVIEW.md): `POST {prefix}/api/v1/auth/refresh` (`cabana/auth.go` `(*service).refresh`) calls `bouncer.RefreshAudience`, which checks only signature, audience, refresh window and blacklist, then mints a token with iat=now. It never loads the admin, so the backend guard's `tokens_valid_after` cutoff (`bouncer/jwt.go` line 144) and the `is_activated` check are skipped. With the SPA refreshing on every 401 (`admin/src/api/client.ts`), a copied `summer_admin` cookie survives `summer admin:reset-password` for up to refresh_ttl (14 days). That breaks Phase 9 truth T-09-04.
|
|
|
|
|
|
|
|
|
|
After this plan, refresh loads the subject through the same provider and the same check helpers the guard uses, and refuses (same 401 `unauthenticated` body) a missing, soft-deleted, deactivated or pre-cutoff subject before minting. A refusal of that kind over cookie transport also expires the cookie.
|
|
|
|
|
|
|
|
|
|
Purpose: restore session revocation for the admin SPA without touching the golem15/user frontend refresh (PHP-parity contract) or the guard's behaviour.
|
|
|
|
|
Output: bouncer helper extraction plus `RefreshAudienceFor`, the cabana wiring, Postgres and no-Docker regression tests, and corrected (uncommitted) Phase 10 planning docs.
|
|
|
|
|
|
|
|
|
|
Scope notes (decisions made for this quick task):
|
|
|
|
|
- Callers checked: `bouncer.RefreshAudience` has one caller (cabana admin refresh). `bouncer.Refresh` is used by `../fonoteka.go/plugins/golem15/user/controllers/api_controller.go` (frontend user JWT refresh). Both share the unexported `refreshAudience`, so the new check is an optional hook that `Refresh` and `RefreshAudience` pass as nil. Both exported functions keep their signatures and behaviour (the framework is a shared dependency; no breaking change).
|
|
|
|
|
- Cookie expiry is limited to subject refusals (`ErrSubjectRejected`). Other refresh failures (no token, bad signature, wrong audience, outside the refresh window, blacklisted, provider error) keep today's no-Set-Cookie behaviour. `TestPhase10Coverage` pins "no cookies" for the out-of-window case, and the general stale-cookie cleanup is the separate open finding WR-01. A provider (DB) error answers 401 without expiring the cookie, so a transient lookup failure does not end the browser session.
|
|
|
|
|
- Out of scope: WR-01 (logout behind the guard) and WR-07 (sliding refresh window). The corrected residual-risk text names WR-07.
|
|
|
|
|
- fonoteka.go needs no change. Task 3 proves its tests stay green.
|
|
|
|
|
|
|
|
|
|
Source coverage (orchestrator requirements -> task):
|
|
|
|
|
| Item | Task |
|
|
|
|
|
|---|---|
|
|
|
|
|
| Load the old token's `sub` before minting and apply the guard's checks (missing, not activated, old iat before tokens_valid_after), 401 with the same error shape | 1 |
|
|
|
|
|
| Expire summer_admin on refusal over cookie transport | 1 (behaviour), 2 (unit pins) |
|
|
|
|
|
| Reuse the guard's lookup and check logic, do not duplicate | 1 (shared `lazyBackendUsers` value, extracted `subjectPrincipal`/`issuedBeforeCutoff`) |
|
|
|
|
|
| Do not change the user plugin's refresh route or PHP-parity contract | 1 (nil hook), 2 (unchanged `TestRefresh*`), 3 (fonoteka user plugin tests) |
|
|
|
|
|
| Tests written red first: pre-reset token gives 401 and an expired cookie; deactivated user gives 401; the normal path still works | 1 |
|
|
|
|
|
| Correct the T-10-05 residual-risk line and set the CR-01 disposition row to fixed with a commit reference, uncommitted | 3 |
|
|
|
|
|
| fonoteka.go stays green | 3 |
|
|
|
|
|
</objective>
|
|
|
|
|
|
|
|
|
|
<execution_context>
|
|
|
|
|
@~/.claude/gsd-core/workflows/execute-plan.md
|
|
|
|
|
@~/.claude/gsd-core/templates/summary.md
|
|
|
|
|
</execution_context>
|
|
|
|
|
|
|
|
|
|
<context>
|
|
|
|
|
@.planning/STATE.md
|
|
|
|
|
@CLAUDE.md
|
|
|
|
|
@.planning/phases/10-admin-vue-spa/10-REVIEW.md
|
|
|
|
|
@cabana/auth.go
|
|
|
|
|
@bouncer/refresh.go
|
|
|
|
|
@bouncer/jwt.go
|
|
|
|
|
|
|
|
|
|
<interfaces>
|
|
|
|
|
Current code the executor works against (extracted, do not re-explore):
|
|
|
|
|
|
|
|
|
|
bouncer/refresh.go
|
|
|
|
|
- Refresh(secret, tokenString string, refreshTTL time.Duration, bl BlacklistStore, grace time.Duration, issuerURL string) (string, error): calls refreshAudience(..., AudienceUser, true, ...). Used by fonoteka.go's golem15/user plugin. MUST stay behaviourally identical.
|
|
|
|
|
- RefreshAudience(secret, tokenString, audience string, refreshTTL, bl, grace, issuerURL) (string, error): calls refreshAudience(..., audience, false, ...). Keep exported and unchanged.
|
|
|
|
|
- refreshAudience order today: parse HS256 without claims validation, subject(claims), audience check, iat window (`iat + refreshTTL`), IsBlacklisted(old jti), exp/ttl claim check, MintAudience(secret, sub, issuerURL, ttl, audience), then bl.Add(old jti, later of iat+refreshTTL+1m and exp+1m, now+grace).
|
|
|
|
|
|
|
|
|
|
bouncer/jwt.go
|
|
|
|
|
- type UserProvider interface { FindByID(ctx context.Context, id uint) (*Principal, error) }
|
|
|
|
|
- jwtGuard.Authenticate order today: extractToken, VerifyClaimsAudience or VerifyClaims, strconv.ParseUint(sub) (err or 0 gives errors.New(msgUserNotFound)), users nil gives msgUserNotFound, users.FindByID (err gives errors.New("Authentication error"), nil user gives msgUserNotFound), IsBlacklisted(jti) (err gives "Authentication error", blocked gives msgBadSignature), then `!user.TokensValidAfter.IsZero() && iat.Before(user.TokensValidAfter)` gives msgUserNotFound.
|
|
|
|
|
- msgUserNotFound = "User not found". The package-level func Middleware (legacy frontend middleware) is out of scope. Leave it as is.
|
|
|
|
|
|
|
|
|
|
bouncer/context.go
|
|
|
|
|
- type Principal struct { ID uint; MustChangePassword bool; PreferredLocale string; TokensValidAfter time.Time; Backend, IsSuperuser bool; PermissionGrants map[string]bool }
|
|
|
|
|
|
|
|
|
|
cabana/auth.go
|
|
|
|
|
- BackendUsers.FindByID returns (nil, nil) for id 0, a missing or soft-deleted row, or `!user.IsActivated`, and fills Principal.TokensValidAfter from the column.
|
|
|
|
|
- (*service).refresh (lines 217-241): sessionToken(r) returns (raw, fromCookie). Bearer wins over the cookie. On any error it calls logAuth(r, "failed", 0) and WriteError(w, 401, "unauthenticated", msgUnauthenticated). On success, cookie transport calls writeSessionCookie plus cookieLoginData(s.ttl), and Bearer returns {access_token, token_type: bearer}.
|
|
|
|
|
- (*service).expireSessionCookie(w) sets summer_admin with value "", MaxAge -1, Path adminPrefix().
|
|
|
|
|
|
|
|
|
|
cabana/http.go
|
|
|
|
|
- service struct (lines 32-49) has no user provider field.
|
|
|
|
|
- line 112: guard := bouncer.NewBackendJWTGuard(secret, lazyBackendUsers{app: app, reg: reg}, bl, writeUnauthenticated, AdminCookieName)
|
|
|
|
|
- lazyBackendUsers.FindByID (lines 171-180) resolves *gorm.DB from app and delegates to BackendUsers{DB, Registry}.FindByID.
|
|
|
|
|
|
|
|
|
|
cabana/commands.go adminResetPassword: sets password and tokens_valid_after = time.Now().UTC().Add(time.Second).
|
|
|
|
|
|
|
|
|
|
Test helpers (package cabana_test): adminGorm(t) (shared testcontainers Postgres, skips only under -short), adminHandler(t, gdb, nil), insertAdmin(t, gdb, login, email, password, activated, deleted) cabana.BackendUser, adminTestPassword, postJSON, postAuth(t, h, method, path, token, body), getAuth, accessToken, adminAPI(rel), phase10Send(t, h, method, path, body, cookie, ajax), phase10Cookie(t, rec, path), phase10AssertCookieBody, phase10AssertBearerBody, phase10ErrorCode, commandApp(t, gdb), commandByName(t, cabana.RuntimeCommands(app), "admin:reset-password"), flagInput{args, flags}, bonfire.NewOutput(nil, &buf, &buf). The logout cookie assertion pattern is in cabana/phase10_auth_test.go lines 84-96.
|
|
|
|
|
Test helpers (package bouncer): memUsers{byID, err} in jwt_test.go, sign(t, method, claims, key), decodeClaims, claimUnix, const secret, NewMemoryBlacklist().
|
|
|
|
|
Test helpers (package cabana): the "cookie refresh of an expired access token inside the refresh window" subtest in cabana/phase10_coverage_test.go (lines 276-365) builds a bare &service{secret, ttl, refreshTTL, issuer, bl} with no app and signs tokens for sub "5".
|
|
|
|
|
</interfaces>
|
|
|
|
|
</context>
|
|
|
|
|
|
|
|
|
|
<tasks>
|
|
|
|
|
|
|
|
|
|
<task type="tracer" tdd="true">
|
|
|
|
|
<name>Task 1: End-to-end: a pre-reset, deactivated or deleted admin token cannot be refreshed (cabana refresh, bouncer.RefreshAudienceFor, shared guard provider)</name>
|
|
|
|
|
<files>cabana/refresh_revocation_test.go, bouncer/jwt.go, bouncer/refresh.go, cabana/http.go, cabana/auth.go, cabana/phase10_coverage_test.go</files>
|
|
|
|
|
<read_first>
|
|
|
|
|
- cabana/auth.go lines 31-76 (BackendUsers.FindByID, principalFrom), 193-241 (cookie helpers, refresh)
|
|
|
|
|
- cabana/http.go lines 32-49 (service struct), 105-180 (guard wiring, service literal, lazyBackendUsers)
|
|
|
|
|
- bouncer/refresh.go (all), bouncer/jwt.go lines 1-160 (UserProvider, jwtGuard.Authenticate)
|
|
|
|
|
- cabana/commands_test.go lines 86-160 (TestAdminResetPasswordCommand, commandByName, commandApp, flagInput)
|
|
|
|
|
- cabana/phase10_auth_test.go lines 27-100 and 211-300 (cookie flow and helpers)
|
|
|
|
|
- cabana/phase10_coverage_test.go lines 276-365
|
|
|
|
|
</read_first>
|
|
|
|
|
<behavior>
|
|
|
|
|
New file cabana/refresh_revocation_test.go, package cabana_test, one test TestAdminRefreshRevocation with one t.Run per case. Each case uses its own admin with a unique neutral login prefixed "rrev-" (adminGorm shares one database across the package, and the hygiene gate forbids app names in framework tests):
|
|
|
|
|
- "pre-reset cookie is refused and expired": log in over cookie transport (phase10Send with ajax true), run the real admin:reset-password command (commandByName over cabana.RuntimeCommands(commandApp(t, gdb)), flagInput args = the login, flags password = a new value). Then GET /auth/me with the old cookie gives 401. POST /auth/refresh with the old cookie and X-Requested-With gives 401 with phase10ErrorCode "unauthenticated", and the response carries a summer_admin cookie with Value "", MaxAge below 0 and Path cabana.DefaultAdminPrefix.
|
|
|
|
|
- "pre-reset bearer is refused without cookies": log in over Bearer before a reset, reset, then postAuth refresh with the Bearer token gives 401 unauthenticated and no Set-Cookie header at all.
|
|
|
|
|
- "deactivated admin is refused": log in over cookie, UPDATE backend_users SET is_activated = false for that id, then cookie refresh gives 401 plus the expiring cookie.
|
|
|
|
|
- "soft-deleted admin is refused": log in over cookie, gdb.Delete(&user), then cookie refresh gives 401 plus the expiring cookie.
|
|
|
|
|
- "active admin still refreshes": cookie refresh gives 200, phase10Cookie returns a rotated value, phase10AssertCookieBody passes, and GET /auth/me with the rotated cookie gives 200. Bearer refresh gives 200 and phase10AssertBearerBody passes.
|
|
|
|
|
Do NOT add a "log in again after the reset and refresh" step. adminResetPassword sets the cutoff to now+1s, so a token minted within that second is correctly refused, and the step would be flaky.
|
|
|
|
|
</behavior>
|
|
|
|
|
<action>
|
|
|
|
|
RED (per the orchestrator's "write them red first"): create only cabana/refresh_revocation_test.go as specified in behavior, then run `go test ./cabana -run '^TestAdminRefreshRevocation$' -count=1 -v`. Expect the four refusal subtests to fail with status 200 and "active admin still refreshes" to pass. Copy the failing lines into the SUMMARY. Do not commit the red state: the project rule is that `go vet` and `go test ./...` are green at every commit.
|
|
|
|
|
|
|
|
|
|
GREEN, in this order:
|
|
|
|
|
|
|
|
|
|
1. bouncer/jwt.go: extract the guard's subject logic into shared helpers without changing the guard's behaviour. Add an exported sentinel `ErrSubjectRejected` whose message is exactly msgUserNotFound ("User not found"). Doc: the token's subject is not a loadable user (bad sub, nil provider, provider returned nil for a missing, deleted or not-activated user), or the token was issued before Principal.TokensValidAfter. Add an unexported `errAuthentication` with the message "Authentication error". Add unexported `subjectPrincipal(ctx context.Context, users UserProvider, sub string) (*Principal, error)`: strconv.ParseUint failure or 0 gives ErrSubjectRejected, nil users gives ErrSubjectRejected, a FindByID error gives errAuthentication, and a nil principal gives ErrSubjectRejected. Add unexported `issuedBeforeCutoff(user *Principal, iat time.Time) bool`, true when TokensValidAfter is non-zero and iat is before it. Rewrite jwtGuard.Authenticate to call subjectPrincipal(r.Context(), g.users, sub), then the unchanged blacklist block, then return ErrSubjectRejected when issuedBeforeCutoff(user, iat). The order and every error message stay identical. Leave the package-level Middleware func untouched.
|
|
|
|
|
|
|
|
|
|
2. bouncer/refresh.go: give the unexported refreshAudience a trailing parameter `check func(sub string, iat time.Time) error`. Call it, when non-nil, after the blacklist lookup and the exp/ttl claim check and immediately before MintAudience. A refused subject then neither mints a token nor blacklists the old jti, and all token-only checks run before any DB lookup. Refresh and RefreshAudience pass nil. Their signatures, docs and behaviour stay unchanged, which keeps the golem15/user frontend refresh (PHP-parity contract) as it is. Add exported `RefreshAudienceFor(ctx context.Context, users UserProvider, secret, tokenString, audience string, refreshTTL time.Duration, bl BlacklistStore, grace time.Duration, issuerURL string) (string, error)`. It rejects an empty audience like RefreshAudience, requires aud (allowMissing false), and passes a check that returns subjectPrincipal's error as is and returns ErrSubjectRejected when issuedBeforeCutoff. The doc comment says it applies the JWT guard's subject checks before minting, and that only subject refusals match errors.Is(err, ErrSubjectRejected).
|
|
|
|
|
|
|
|
|
|
3. cabana/http.go: add field `users bouncer.UserProvider` to service, commented as the same provider the backend guard uses. In the constructor build one `users := lazyBackendUsers{app: app, reg: reg}`, pass it to bouncer.NewBackendJWTGuard, and set `users: users` in the service literal. The guard and refresh then share one lookup (is_activated, soft delete, role grants, tokens_valid_after), with no duplicated query.
|
|
|
|
|
|
|
|
|
|
4. cabana/auth.go refresh: call bouncer.RefreshAudienceFor(r.Context(), s.users, s.secret, raw, bouncer.AudienceBackend, s.refreshTTL, s.bl, s.grace, s.issuer). On error: when fromCookie is true and errors.Is(err, bouncer.ErrSubjectRejected), call s.expireSessionCookie(w) before writing the body. Then keep the existing logAuth(r, "failed", 0) and the same WriteError 401 "unauthenticated" msgUnauthenticated. Bearer transport never touches the cookie. The success path and the empty-token path stay unchanged. Do not edit any swag annotation comment, so the admin OpenAPI document cannot drift.
|
|
|
|
|
|
|
|
|
|
5. cabana/phase10_coverage_test.go: the "cookie refresh of an expired access token inside the refresh window" subtest builds a bare service, which now needs a provider or refresh fails closed. Define a small map-backed test provider type in that file (for example `refreshSubjects`, implementing FindByID with an optional error field for Task 2), and set `users:` to it with principal ID 5 (Backend true). Change no assertion.
|
|
|
|
|
|
|
|
|
|
Then run the verify command and commit code and tests together as `fix(cabana): enforce tokens_valid_after and is_activated on admin refresh`, with a body that names CR-01 and T-09-04. No co-author trailer (user and project rule).
|
|
|
|
|
</action>
|
|
|
|
|
<verify>
|
|
|
|
|
<automated>cd /media/nvme/dev/golem15/summercms.io/summercms/summercms.go && go vet ./bouncer ./cabana && go test ./bouncer ./cabana -count=1 -run 'TestAdminRefreshRevocation|TestPhase10Coverage|TestPhase10CookieAuth|TestAdminAuthLifecycle|TestAdminBlacklist|TestAdminInactive|TestAdminResetPasswordCommand|TestPhase10OpenAPIConformance|TestJWTGuard|TestRefresh|TestPhase09GuardIsolation|TestPhase10CookieGuard|TestMintRefreshBlacklistRoundTrip'</automated>
|
|
|
|
|
</verify>
|
|
|
|
|
<done>
|
|
|
|
|
- The red run was observed and recorded: the four refusal subtests failed with status 200 before the fix.
|
|
|
|
|
- TestAdminRefreshRevocation passes on Postgres (not skipped). Every pre-existing test in the verify regex passes unchanged.
|
|
|
|
|
- `grep -n "RefreshAudienceFor(r.Context(), s.users" cabana/auth.go` and `grep -n "users:\s*users" cabana/http.go` both match. `grep -n "subjectPrincipal(r.Context(), g.users" bouncer/jwt.go` matches. `grep -n "issuerURL, nil)" bouncer/refresh.go` matches twice (Refresh and RefreshAudience).
|
|
|
|
|
- One commit `fix(cabana): ...` holds code and tests, with no co-author trailer.
|
|
|
|
|
</done>
|
|
|
|
|
</task>
|
|
|
|
|
|
|
|
|
|
<task type="auto" tdd="true">
|
|
|
|
|
<name>Task 2: Unit coverage (no Docker) for the refresh subject checks and the shared guard helper</name>
|
|
|
|
|
<files>bouncer/refresh_test.go, bouncer/jwt_guard_test.go, cabana/phase10_coverage_test.go</files>
|
|
|
|
|
<read_first>
|
|
|
|
|
- bouncer/refresh_test.go (existing TestRefresh* style), bouncer/jwt_test.go lines 15-40 (memUsers, sign)
|
|
|
|
|
- bouncer/jwt_guard_test.go lines 96-130 (TestJWTGuardTokensValidAfter)
|
|
|
|
|
- cabana/phase10_coverage_test.go lines 276-365 (the refresh subtest and the provider type added in Task 1)
|
|
|
|
|
</read_first>
|
|
|
|
|
<behavior>
|
|
|
|
|
bouncer/refresh_test.go, new TestRefreshAudienceForSubject. Tokens are signed with aud backend, a jti, and iat 10 minutes ago. The providers are memUsers plus a small counting provider defined in the test:
|
|
|
|
|
- Active principal with no cutoff: returns a token with sub preserved and aud backend, and the old jti is blacklisted (grace 0).
|
|
|
|
|
- Cutoff 1 minute after the token's iat: errors.Is(err, ErrSubjectRejected), and the old jti is NOT blacklisted.
|
|
|
|
|
- Cutoff 1 second before iat: succeeds.
|
|
|
|
|
- Id absent from memUsers (models missing or not activated): ErrSubjectRejected, not blacklisted.
|
|
|
|
|
- nil users: ErrSubjectRejected.
|
|
|
|
|
- Non-numeric sub: ErrSubjectRejected.
|
|
|
|
|
- Provider returning an error: err non-nil, NOT errors.Is ErrSubjectRejected, and not blacklisted.
|
|
|
|
|
- Token outside the refresh window, frontend-audience token, wrong-secret token, or already-blacklisted jti: each errors and the counting provider records zero FindByID calls (token checks run before the lookup).
|
|
|
|
|
- Empty audience: errors.
|
|
|
|
|
bouncer/jwt_guard_test.go TestJWTGuardTokensValidAfter: add assertions that the cutoff refusal's err.Error() is still "User not found" and errors.Is(err, ErrSubjectRejected). This pins that the guard's message did not change through the helper extraction.
|
|
|
|
|
cabana/phase10_coverage_test.go: a new TestPhase10Coverage subtest "refresh enforces the guard's subject checks" with a bare service and the Task 1 provider, no Postgres, so it runs under -short:
|
|
|
|
|
- Cookie token with iat before the principal's TokensValidAfter: 401, phase10-style error code unauthenticated, and a summer_admin cookie with Value "", MaxAge below 0 and Path DefaultAdminPrefix.
|
|
|
|
|
- Cookie token for a sub with no principal: 401 plus the expiring cookie.
|
|
|
|
|
- Bearer token before the cutoff: 401 and zero cookies.
|
|
|
|
|
- Provider configured to return an error: 401 and zero cookies (a transient lookup failure does not end the browser session).
|
|
|
|
|
- Cookie token after the cutoff: 200 with a rotated cookie.
|
|
|
|
|
</behavior>
|
|
|
|
|
<action>
|
|
|
|
|
Write the tests in behavior against the Task 1 code. They are expected to pass on first run because Task 1 implemented the behaviour. Prove they would catch a regression with a removal check: temporarily edit RefreshAudienceFor in bouncer/refresh.go so it passes nil instead of its check to refreshAudience, then run `go test ./bouncer ./cabana -short -count=1 -run 'TestRefreshAudienceForSubject|TestPhase10Coverage'`. Confirm that the cutoff, missing, nil-users and non-numeric cases and the cabana cookie-expiry cases fail. Restore with `git checkout -- bouncer/refresh.go` (committed in Task 1, so this is safe) and re-run until green. Record the removal-check result in the SUMMARY. Use only the stdlib testing package, following the existing plain-`t.Fatalf` style in these files (no testify is imported there). Commit as `test(bouncer,cabana): cover admin refresh subject checks without a database`, with no co-author trailer.
|
|
|
|
|
</action>
|
|
|
|
|
<verify>
|
|
|
|
|
<automated>cd /media/nvme/dev/golem15/summercms.io/summercms/summercms.go && go vet ./bouncer ./cabana && go test ./bouncer ./cabana -short -count=1 -run 'TestRefreshAudienceForSubject|TestJWTGuardTokensValidAfter|TestPhase10Coverage|TestRefresh' -v</automated>
|
|
|
|
|
</verify>
|
|
|
|
|
<done>
|
|
|
|
|
- TestRefreshAudienceForSubject and TestJWTGuardTokensValidAfter pass. The new TestPhase10Coverage subtest passes under -short.
|
|
|
|
|
- The removal check was run and recorded: with the check dropped, the new cases fail. After restore, `git diff --quiet -- bouncer/refresh.go` holds.
|
|
|
|
|
- One commit `test(bouncer,cabana): ...`, with no co-author trailer.
|
|
|
|
|
</done>
|
|
|
|
|
</task>
|
|
|
|
|
|
|
|
|
|
<task type="auto">
|
|
|
|
|
<name>Task 3: Cross-repo gate and planning doc corrections (edited, not committed)</name>
|
|
|
|
|
<files>.planning/phases/10-admin-vue-spa/10-SECURITY-REVIEW.md, .planning/phases/10-admin-vue-spa/10-REVIEW-DISPOSITION.md</files>
|
|
|
|
|
<read_first>
|
|
|
|
|
- .planning/phases/10-admin-vue-spa/10-SECURITY-REVIEW.md lines 14-21 (table header and the T-10-05 row)
|
|
|
|
|
- .planning/phases/10-admin-vue-spa/10-REVIEW-DISPOSITION.md (the CR-01 row)
|
|
|
|
|
- scripts/check-phase10.sh lines 244-267 and 365-400 (the --go, --security, --postgres and --evidence stages)
|
|
|
|
|
</read_first>
|
|
|
|
|
<action>
|
|
|
|
|
1. Confirm that no dependency changed: `git show --stat` of both task commits lists neither go.mod nor go.sum. Confirm that this task wrote nothing in ../fonoteka.go.
|
|
|
|
|
|
|
|
|
|
2. Run `bash scripts/check-phase10.sh --go` from summercms.go. It runs go vet and the full go test (including testcontainers) for summercms.go, and for fonoteka.go including ./plugins/golem15/user/... (the golem15/user refresh that shares bouncer.Refresh) and ./plugins/golem15/fonoteka/... (the assembled admin refresh tests). It accepts only the two documented parity failures. It must exit 0. Use a 600000 ms timeout or run it in the background.
|
|
|
|
|
|
|
|
|
|
3. Edit 10-SECURITY-REVIEW.md, T-10-05 row only. Keep exactly nine cells and use no pipe characters inside a cell (the --evidence parser reads the row).
|
|
|
|
|
- Residual-risk cell: replace "A stolen cookie is valid until logout or expiry, the same window as a Bearer token" with text stating these points. A stolen cookie or Bearer token stays usable until logout, `summer admin:reset-password` (tokens_valid_after), deactivation or deletion of the admin, or the end of its refresh window. Before quick task 260927-q23 (CR-01), refresh skipped the tokens_valid_after and is_activated checks, so a reset did not end a session the SPA kept refreshing. The refresh window slides on every refresh, so a session refreshed at least once per refresh_ttl has no absolute expiry (WR-07, open).
|
|
|
|
|
- Production-mitigation cell: append that `refresh` loads the admin through the guard's provider and refuses a missing, deactivated or pre-cutoff subject via `bouncer.RefreshAudienceFor`, expiring the cookie on that refusal.
|
|
|
|
|
- Test cell: append `TestAdminRefreshRevocation` (cabana, Postgres) and `TestRefreshAudienceForSubject` (bouncer).
|
|
|
|
|
|
|
|
|
|
4. Edit 10-REVIEW-DISPOSITION.md, CR-01 row only. Set Disposition to `fixed`. Set the Note to "Fixed in <short hash of the Task 1 fix commit> (tests <short hash of the Task 2 commit>), quick 260927-q23", followed by one sentence: admin refresh now applies the backend guard's subject checks (activated, not deleted, iat not before tokens_valid_after) via bouncer.RefreshAudienceFor before minting, and a refused cookie refresh expires summer_admin. Take the hashes from `git log --format=%h -2`.
|
|
|
|
|
|
|
|
|
|
5. Do NOT commit either doc and do NOT touch STATE.md, ROADMAP.md, 10-VERIFICATION.md or any other planning file. The orchestrator commits planning docs separately (project rule: planning docs and code in separate commits).
|
|
|
|
|
|
|
|
|
|
6. Run `bash scripts/check-phase10.sh --evidence`. It parses the review table, then runs the --security, --postgres and --openapi stages. It must exit 0, which also proves that the admin OpenAPI document and types did not drift.
|
|
|
|
|
</action>
|
|
|
|
|
<verify>
|
|
|
|
|
<automated>cd /media/nvme/dev/golem15/summercms.io/summercms/summercms.go && bash scripts/check-phase10.sh --go && bash scripts/check-phase10.sh --evidence && grep -n '^| CR-01 | critical | fixed |' .planning/phases/10-admin-vue-spa/10-REVIEW-DISPOSITION.md && grep -n '^| T-10-05 ' .planning/phases/10-admin-vue-spa/10-SECURITY-REVIEW.md | grep -c 'RefreshAudienceFor' && git status --porcelain .planning/phases/10-admin-vue-spa</automated>
|
|
|
|
|
</verify>
|
|
|
|
|
<done>
|
|
|
|
|
- `check-phase10.sh --go` and `check-phase10.sh --evidence` both exit 0. fonoteka.go's user and fonoteka plugin tests are green with no fonoteka.go change.
|
|
|
|
|
- The T-10-05 row keeps nine cells, names RefreshAudienceFor, TestAdminRefreshRevocation and WR-07, and no longer claims "valid until logout or expiry".
|
|
|
|
|
- The CR-01 row reads `fixed` and cites the Task 1 and Task 2 commit hashes.
|
|
|
|
|
- `git status --porcelain` shows both docs as modified and uncommitted. go.mod and go.sum are untouched.
|
|
|
|
|
</done>
|
|
|
|
|
</task>
|
|
|
|
|
|
|
|
|
|
</tasks>
|
|
|
|
|
|
|
|
|
|
<threat_model>
|
|
|
|
|
## Trust Boundaries
|
|
|
|
|
|
|
|
|
|
| Boundary | Description |
|
|
|
|
|
|----------|-------------|
|
|
|
|
|
| browser or Bearer client to POST {prefix}/api/v1/auth/refresh | Untrusted token (cookie or Authorization header) is exchanged for a new admin session |
|
|
|
|
|
| cabana refresh to backend_users (Postgres) | Subject state (is_activated, deleted_at, tokens_valid_after) decides whether a session may continue |
|
|
|
|
|
| framework bouncer to fonoteka.go golem15/user plugin | Shared refresh code; the frontend refresh contract must not change |
|
|
|
|
|
|
|
|
|
|
## STRIDE Threat Register
|
|
|
|
|
|
|
|
|
|
| Threat ID | Category | Component | Severity | Disposition | Mitigation Plan |
|
|
|
|
|
|-----------|----------|-----------|----------|-------------|-----------------|
|
|
|
|
|
| T-q23-01 | Elevation of Privilege | cabana refresh after admin:reset-password | high | mitigate | RefreshAudienceFor refuses a token whose iat is before tokens_valid_after before minting; cookie refusal expires summer_admin (Task 1, TestAdminRefreshRevocation with the real reset command; Task 2 removal check) |
|
|
|
|
|
| T-q23-02 | Elevation of Privilege | cabana refresh for deactivated or deleted admin | high | mitigate | Refresh loads the subject through the guard's lazyBackendUsers provider, which returns nil for !is_activated and soft-deleted rows, mapped to ErrSubjectRejected (Task 1 deactivated and soft-deleted subtests) |
|
|
|
|
|
| T-q23-03 | Tampering | bouncer.Refresh used by golem15/user (PHP-parity) and the shared JWT guard | medium | mitigate | Subject check is an optional hook passed as nil by Refresh and RefreshAudience; guard helper extraction keeps order and messages; existing TestRefresh*, TestJWTGuard* and fonoteka user plugin tests run in Task 1 and Task 3 |
|
|
|
|
|
| T-q23-04 | Denial of Service | DB lookup added to refresh | low | mitigate | The lookup runs only after signature, audience, refresh-window and blacklist checks pass, so unsigned or garbage tokens never reach Postgres (Task 2 counting-provider cases) |
|
|
|
|
|
| T-q23-05 | Denial of Service | transient provider error during refresh | low | accept | Answers 401 like the guard but does not expire the cookie, so a DB blip does not end the browser session; the SPA retries after re-login at worst |
|
|
|
|
|
| T-q23-SC | Tampering | module and npm installs | low | accept | No new dependency; Task 3 confirms go.mod and go.sum are unchanged by both task commits |
|
|
|
|
|
</threat_model>
|
|
|
|
|
|
|
|
|
|
<verification>
|
|
|
|
|
- Task 1 red run observed (refusal subtests returned 200 before the fix), then green on Postgres.
|
|
|
|
|
- `go vet ./...` and `go test ./...` are green in summercms.go at both task commits (the Task 3 `--go` stage re-proves this for the final state, plus fonoteka.go).
|
|
|
|
|
- Removal check recorded: dropping the subject check fails the new unit tests.
|
|
|
|
|
- `scripts/check-phase10.sh --evidence` exits 0 after the doc edits (table integrity, security, postgres, openapi drift).
|
|
|
|
|
</verification>
|
|
|
|
|
|
|
|
|
|
<success_criteria>
|
|
|
|
|
- A token issued before `summer admin:reset-password` can no longer be refreshed over cookie or Bearer. The SPA's refresh-on-401 now ends at the login screen, and the stale cookie is expired.
|
|
|
|
|
- A deactivated or deleted admin cannot mint new tokens through refresh.
|
|
|
|
|
- The normal refresh path, the golem15/user frontend refresh and the JWT guard behave exactly as before.
|
|
|
|
|
- CR-01 is recorded as fixed with commit references, and the T-10-05 residual risk is stated accurately. Both docs are left uncommitted for the orchestrator.
|
|
|
|
|
</success_criteria>
|
|
|
|
|
|
|
|
|
|
<output>
|
|
|
|
|
Create `.planning/quick/260927-q23-fix-cr-01-auth-refresh-must-enforce-toke/260927-q23-SUMMARY.md` when done. Include the red-run failure lines from Task 1, the removal-check result from Task 2, both commit hashes, and the gate outputs from Task 3.
|
|
|
|
|
</output>
|
Jakub Zych