From ee5b8844bfc3991f876ae90291d3745e5c583934 Mon Sep 17 00:00:00 2001 From: Jakub Zych Date: Thu, 22 Oct 2026 04:57:09 +0200 Subject: [PATCH] docs(todo): user logout bug confirmed, fixed in the plugin and the demo frontend --- .../debug/user-logout-session-stays-valid.md | 81 +++++++++++++++++++ ...26-10-22-user-logout-cookie-stays-valid.md | 17 ++++ 2 files changed, 98 insertions(+) create mode 100644 .planning/debug/user-logout-session-stays-valid.md diff --git a/.planning/debug/user-logout-session-stays-valid.md b/.planning/debug/user-logout-session-stays-valid.md new file mode 100644 index 00000000..791a8f8e --- /dev/null +++ b/.planning/debug/user-logout-session-stays-valid.md @@ -0,0 +1,81 @@ +--- +status: awaiting_human_verify +trigger: "User plugin logout: auth cookies not invalidated at once; requests keep authenticating ~60 min until token expiry (todo 2026-10-22-user-logout-cookie-stays-valid)" +created: 2026-10-22T00:00:00Z +updated: 2026-10-22T00:00:00Z +--- + +## Current Focus + +bug_class: Bohrbug (deterministic) +reasoning_checkpoint: + hypothesis: "Logout reads only the bearer header (cookie sessions get 401, nothing revoked, cookie kept, jwt guard accepts the cookie until exp = 60 min), never writes a forget cookie, and blacklists the jti only until the access exp (so /refresh revives a logged-out token after exp + sweep)" + confirming_evidence: + - "probe: cookie-only logout 401, then GET /tokens with the cookie 200" + - "probe: jwt_blacklist.expires_at == exp; after the sweep, /refresh of the logged-out token 200" + - "bearer logout already refuses the bearer at once (guard checks blacklist) - so server-side check exists; the gap is what logout revokes and for how long" + falsification_test: "if cookie-only logout after the fix still leaves GET /tokens 200, or /refresh after the sweep still 200, hypothesis/fix wrong" + fix_rationale: "read the token exactly as the jwt guard does, revoke it (and any other session token the request carries) until the refresh window closes, accept an expired-but-refreshable token like cabana does, and expire the session cookies with auth.cookie attributes" + blind_spots: "demo frontend never calls /logout (app-side, not fixable here); auth_token cookie is frontend-owned, its real attributes unknown (forget uses auth.cookie path/domain)" + candidate_causes: + - "code: Logout bearer-only extraction + exp-only blacklist expiry" + - "config: blacklist store not published (no DB) -> no revocation; ruled out, Boot publishes PostgresBlacklist when *sql.DB/*gorm.DB exist and bearer logout works" + - "environment/frontend: demo frontend logout only clears its own cookie and never calls /logout" + and_gate: "no for the cookie case (extraction alone suffices); the refresh revival is an independent second defect" +next_action: human verification in a real app (cookie-only logout, then a jwt.auth request with the old cookie must be 401); after confirmation archive to resolved/ and add the KB entry + +## Symptoms + +expected: after POST /_user/api/v1/logout the old token/cookie is refused (401) on the next request, and the cookies are cleared +actual: requests keep authenticating for about 60 minutes, until the token expires +errors: none +reproduction: login, logout, repeat an authenticated request with the old cookie or token +started: reported 2026-10-22 + +## Eliminated + +## Evidence + +- timestamp: 2026-10-22 + checked: bouncer jwtGuard.Authenticate (summercms.go modules/bouncer/jwt.go) + found: access JWTs are checked for HS256 signature, exp, audience, the subject user (GormUsers.FindByID), the jti blacklist (BlacklistStore) and users.tokens_valid_after; token taken from bearer first, then cookies "token", "auth_token" (plugin.go Boot) + implication: server-side revocation exists (jwt_blacklist); a blacklisted jti is refused on the next request +- timestamp: 2026-10-22 + checked: controllers.Logout (api_controller.go) + found: reads bearerToken(r) only (Authorization header); no cookie fallback; never writes a Set-Cookie; blacklists with Add(jti, exp, now) i.e. storage expiry = access exp + implication: cookie-only logout => 401, nothing revoked, cookie kept; blacklist row is dropped after exp although the refresh window (20160 min) is still open +- timestamp: 2026-10-22 + checked: sociallogin writeSession (token_cookie: true) and bouncer.CookieConfig.TokenCookie + found: sets httpOnly `token` cookie (auth.cookie.* name/path/domain/samesite, Max-Age = refresh_ttl) and leaves token out of the body, so the browser cannot send a bearer header + implication: such a frontend can only log out with the cookie => H1 path +- timestamp: 2026-10-22 + checked: bouncer.refreshAudience comment and blacklist storage expiry + found: Refresh stores the old jti until max(exp, iat+refreshTTL)+1min precisely so "a logged-out token cannot be refreshed again"; Logout does not + implication: H2 plausible +- timestamp: 2026-10-22 + checked: PHP ApiController::logout + AuthManager::logout(true) + found: token from Bearer or jwt_token param; JWTAuth::invalidate(forceForever=true) => blacklisted forever; body {"message":"Logged out"} + implication: Go logout should keep the jti blocked at least for the refresh window + +- timestamp: 2026-10-22 + checked: cabana admin logout (summercms.go modules/cabana/auth.go logout) + found: framework reference: always expires the cookie, reads bearer then cookie, VerifyRefreshableClaimsAudience, blacklists until max(exp, iat+refreshTTL)+1min + implication: the plugin's logout lags the framework's own pattern +- timestamp: 2026-10-22 + checked: Docker probe through the assembled router (newAdminEnv), pre-fix + found: (A) cookie-only POST /logout -> 401 {"error":true,"message":"Unauthorized"}, no Set-Cookie; GET /tokens (jwt.auth) with the same cookie afterwards -> 200; token exp-iat = 1h0m0s. (B) bearer logout -> 200 and the bearer is refused at once (401), but jwt_blacklist.expires_at == token exp; the sweep after exp deletes the row and POST /refresh with the logged-out token -> 200 with a fresh token + implication: H1 and H2 CONFIRMED. New test logout_test.go fails 7/8 subtests pre-fix (bearer passes) + +## Resolution + +root_cause: "controllers.Logout read the token only from the Authorization bearer header, although the jwt guard (and RequestUser) also accept the token/auth_token cookies: a cookie-carried session (e.g. the httpOnly token cookie sociallogin token_cookie sets) got 401 at /logout, nothing was blacklisted and no cookie was cleared, so the cookie kept authenticating jwt.auth routes until exp (golem15.user.jwt.ttl, 60 min). Logout also never wrote a forget cookie; and it (and EndRequestSession) kept the jwt_blacklist row only until the access exp, so after the sweep /refresh revived a logged-out token for the rest of the 20160-min refresh window." +fix: "classes.EndSession (new export): revokes every session JWT the request carries (bearer, token, auth_token) that verifies - unexpired, or user-audience within its refresh window - until max(exp, iat+refresh_ttl)+1m; always writes the auth.cookie forget cookie plus forgets for other carried session cookies with the same attributes; returns the guard-order subject. Logout uses it (responses unchanged: 200 Logged out / 401 Unauthorized / 500); EndRequestSession is now its best-effort wrapper." +verification: + target_test: { result: pass, test: "logout_test.go TestLogoutEndsTheSession (10 subtests); TestEndRequestSession updated" } + mutation_check: { result: pass, reason_if_skipped: "no Stryker for Go - manual mutants", mutant_killed: "7/7 (expiry-at-exp, bearer-only, no-forget, no-refreshable-verify, primary-only revoke, config-cookie-only forget, no legacy fallback)" } + no_op_deletion: { result: pass, deletion_justified_by_rca: true } + adjacent_tests: { result: pass, suites_run: ["sm-user-plugin go test ./... (Docker, all ok)", "sm-demo-app workspace: oauthserver + sociallogin go test (only TestClientRegistry/verify_secret and TestPHPMethodMap fail: they read ../summercms.go and ../fonoteka.go paths that exist only in the plugins' own repo layout; unrelated to the change)"] } + revert_and_reconfirm: { result: pass, bug_returned_on_revert: true, fixed_on_reapply: true } + guardrail_verdict: accepted + commit: 358ee5d (sm-user-plugin, unpushed) +files_changed: [classes/session_user.go, controllers/api_controller.go, session_user_test.go, logout_test.go, README.md] diff --git a/.planning/todos/pending/2026-10-22-user-logout-cookie-stays-valid.md b/.planning/todos/pending/2026-10-22-user-logout-cookie-stays-valid.md index ea622e57..a64a0629 100644 --- a/.planning/todos/pending/2026-10-22-user-logout-cookie-stays-valid.md +++ b/.planning/todos/pending/2026-10-22-user-logout-cookie-stays-valid.md @@ -31,3 +31,20 @@ Logout revokes the server-side session so the access token is refused on the nex (same mechanism c3712dc uses for deactivate/ban), clears the cookies with matching attributes, and a test pins "request after logout is 401". No breaking change to the plugin API (core plugin, many projects). + +## Outcome (2026-10-22) + +Confirmed and fixed locally; closes when released. Debug session: +`.planning/debug/user-logout-session-stays-valid.md`. + +- Plugin root cause: `Logout` read the token from the `Authorization` header only, so a + cookie-only logout (sociallogin `token_cookie: true`) answered 401 and revoked nothing, and the + cookie kept working for the 60-minute access TTL; logout cleared no cookie; the blacklist row + lived only until `exp`, so a logged-out token could be refreshed again once it was swept. + Fix: sm-user-plugin `358ee5d` (new `classes.EndSession`, blacklist until the end of the refresh + window, forget cookies with the configured attributes, `TestLogoutEndsTheSession`). +- Demo frontend root cause: `useDemoAuth().logout()` only cleared its own cookie and never called + `/logout`. Fix: vue-demo-app `f397df2`. +- Ships in sm-user-plugin v0.1.8 (with `aa83969`, needs SummerCMS v0.4.3) and the sm-demo-app + pointer bumps. Hand check after deploy: cookie-only logout, then a `jwt.auth` route with the old + cookie answers 401.