From d6c7fd5a2799feeaf15264842e0201826e0fcf5c Mon Sep 17 00:00:00 2001 From: Jakub Zych Date: Fri, 23 Oct 2026 16:32:44 +0200 Subject: [PATCH] docs(19.7-07): complete lane B gap-closure plan with auth_time and the E-2 seam --- .../19.7-07-SUMMARY.md | 218 ++++++++++++++++++ 1 file changed, 218 insertions(+) create mode 100644 .planning/phases/19.7-generic-mcp-module/19.7-07-SUMMARY.md diff --git a/.planning/phases/19.7-generic-mcp-module/19.7-07-SUMMARY.md b/.planning/phases/19.7-generic-mcp-module/19.7-07-SUMMARY.md new file mode 100644 index 00000000..e8bc0df9 --- /dev/null +++ b/.planning/phases/19.7-generic-mcp-module/19.7-07-SUMMARY.md @@ -0,0 +1,218 @@ +--- +phase: 19.7-generic-mcp-module +plan: 07 +subsystem: auth +status: complete +gap_closure: true +tags: [bouncer, jwt, auth_time, pact, token-scopes, pier, mcp, fan-out, towel, cabana, admin-spa, hotkeys, openapi, gap-closure] + +requires: + - phase: 19.7-06 + provides: pier session and stateless matching after lane A (session.go, stateless.go, events.go), pact.MCPPrompt.Writes + - phase: 19.7-05 + provides: scripts/check-phase19.7.sh (--go, --admin, --docs, --marker stages) +provides: + - bouncer.ClaimAuthTime and bouncer.VerifyAuthTime; auth_time stamped at every sign-in mint and copied unchanged by every refresh path + - pact.FrontendTokenScope, pact.HasFrontendTokenScopes and pact.ValidTokenScopeName (E-2 framework seam, decision note) + - pact.MCPSubjectChanged.TokenID / ClientID and pier.Event token_id / client_id; narrowed revocation in pier + - towel.WithClientIP / towel.ClientIP; pier puts the client address on the resolve context; cabana.ResolveBackendToken logs it + - cabana My account seam: http(s)/same-origin URL rule, per-section `error` flag, localized status; localized prompt refusal; corrected record action contract + - admin SPA: shadow-DOM-aware plain keys, blur-before-click shortcuts, safeUrl, section error line and toast, My account confirm/stale texts; rebuilt dist +affects: [19.7-08 (user plugin reads VerifyAuthTime, compiles HasFrontendTokenScopes, fires client-specific revocations), 19.7-09 (named unit tests), framework v0.5.0] + +actuals: + tokens: 28086 + tasks: 5 + commits: 6 +plan_head_before: 60a86677f22aef02d11acc812ba110b94797e1ed +plan_head_after: 88559d5731c35d8071fb500ee93d7c8c287312e9 + +tech-stack: + added: [] + patterns: + - "A refresh copies a sign-in claim from the presented token instead of re-deriving it (mintAudience with an explicit authTime)" + - "Frontend twin of a pact capability (FrontendTokenScope next to TokenScope, as FrontendPermission next to Permission) with one shared validation function" + - "Revocation events narrow by optional fields; pier selects sessions with a predicate (matchingFunc / evictFunc), the old (guard, user) signatures stay as wrappers" + - "Optional host capability discovered by type assertion (clientIPHost), so pier.Host does not change" + +key-files: + created: + - modules/bouncer/auth_time_smoke_test.go + - .planning/notes/2026-10-23-frontend-token-scopes.md + modified: + - modules/bouncer/mint.go + - modules/bouncer/refresh.go + - modules/bouncer/jwt.go + - modules/pact/capabilities.go + - modules/pact/mcp.go + - modules/cabana/token_scopes.go + - modules/pier/events.go + - modules/pier/stateless.go + - modules/pier/store.go + - modules/pier/pier.go + - modules/pier/host.go + - modules/surf/pier_on.go + - modules/towel/context.go + - modules/cabana/account_tokens.go + - modules/cabana/subject_events.go + - modules/cabana/token_resolver.go + - modules/cabana/account_contributions.go + - modules/cabana/actions.go + - modules/cabana/admin_openapi.go + - admin/src/app/hotkeys.ts + - admin/src/components/account/AccountContributions.vue + - admin/openapi/admin.json + - admin/src/api/schema.d.ts + - modules/boardwalk/dist + +key-decisions: + - "P-29: auth_time (OIDC name, NumericDate seconds) is stamped equal to iat by every MintAudience call; refreshAudience copies the presented value and never stamps a missing one; plugins read it with bouncer.VerifyAuthTime (VerifyClaims checks for AudienceUser, VerifyClaimsAudience checks otherwise; ok false when absent)" + - "P-30: member token scopes use a separate capability pact.HasFrontendTokenScopes, not HasTokenScopes and not an audience field; pact.ValidTokenScopeName is the one name rule; recorded in .planning/notes/2026-10-23-frontend-token-scopes.md" + - "P-31: a revoked event with TokenID ends only sessions whose subject under Guard is UserID with that TokenID and no ClientID; with ClientID only that client's sessions of UserID; neither (and every deleted event) the whole subject" + - "P-32: pier puts the client address on the resolve context (surf's adapter applies http.trusted_proxies, else the host part of RemoteAddr)" + - "P-33: account action URLs kept only for http(s) with a host or a same-origin path (one leading /, not // or /\\); a failing Status marks its section error: true, a failing Applies drops that action" + +patterns-established: + - "Sign-in freshness is proven by auth_time, never iat" + +requirements-completed: [SC-4, D-04, D-05, D-17, D-19, D-26, D-27, D-34, E-2] + +duration: 62min +completed: 2026-10-23 +--- + +# Phase 19.7 Plan 07: Lane B gap closure, auth_time and the E-2 seam Summary + +Lane B of the phase code review is closed, and the framework gains the two pieces plan 08's plugins build on. Every sign-in token now carries `auth_time`, which a refresh copies unchanged, so the account link's fresh-sign-in proof can no longer be satisfied by `POST /refresh`. Plugins can declare member (`usr_`) token scopes through `pact.HasFrontendTokenScopes`, which the admin catalog never reads. Revoking one admin token ends only that token's MCP sessions, and failed `g15_` attempts at `/mcp` are logged with the client address. The My account seam only hands the browser http(s) or same-origin URLs and survives one plugin's failure. The SPA's plain-key and save shortcuts behave correctly around shadow DOM and pending taglist drafts. + +## Performance + +- **Duration:** about 62 min +- **Started:** 2026-10-23T14:11:39Z +- **Completed:** 2026-10-23 +- **Tasks:** 5 of 5 +- **Files modified:** 43 (2 created, generated dist assets renamed) + +## Commits and findings + +| Commit | Task | Findings | +|--------|------|----------| +| c14238b `feat(bouncer): stamp auth_time at sign-in and carry it unchanged through refresh` | 1 (tracer) | C-CR-02 (framework part) | +| 9eedfbb `feat(pact): add HasFrontendTokenScopes and the shared ValidTokenScopeName rule` | 2 | E-2 (framework seam) | +| 0ec00f7 `docs(19.7-07): record the member token scope design decision` | 2 | E-2 (P-30 note) | +| 24cf6cd `fix(pier): end only the revoked token's or client's MCP sessions and log the client address` | 3 | B-WR-04, B-IN-04, C-IN-02 (framework half) | +| 44835e4 `fix(cabana): safe account action URLs, per-section isolation and the prompted record action contract` | 4 | B-WR-03 (server), B-WR-05, B-WR-06 (server), B-IN-02, B-IN-03, B-IN-05 (contract text) | +| 88559d5 `fix(admin): shadow-DOM-aware plain keys, commit drafts before shortcut saves and safe My account links` | 5 | B-WR-01, B-WR-02, B-WR-03 (SPA), B-WR-06 (SPA), B-IN-01 | + +B-IN-06 needs no change: the visible link stays the fallback when a popup blocker stops `window.open`. The B-IN-05 dialog-stays-open UX is deferred; plan 09 records it in deferred-items. + +## Accomplishments + +- **auth_time (Task 1, tracer).** + - `bouncer.ClaimAuthTime = "auth_time"`, with `registeredClaims.AuthTime` (`omitempty`). + - `MintAudience` keeps its signature and calls the unexported `mintAudience(..., now, authTime)` with the same instant for both, so `auth_time == iat` on every sign-in mint (cabana's admin login and its re-mint after a password change included). + - `refreshAudience` reads the presented `auth_time` with `claimTime` and mints through `mintAudience`. `Refresh`, `RefreshAudience` and `RefreshAudienceFor` (cabana's backend `/refresh`) all go through it, and a token without the claim refreshes without one. + - `bouncer.VerifyAuthTime(tokenString, secret, audience)` shares `parseVerified` with `verifyClaims`. That gives the same HS256, required `exp`, audience (`AudienceUser` also accepts a token with no `aud`) and subject checks. + - `TestAuthTimeSmoke` covers mint → read, refresh keeps `auth_time` while `iat` moves, legacy → no claim, and wrong audience, bad signature and expired token → error. +- **E-2 seam (Task 2).** + - New: `pact.FrontendTokenScope{Name, Label, Description}`, `pact.HasFrontendTokenScopes` and `pact.ValidTokenScopeName` (the pattern moved from cabana). + - cabana's `validatePluginTokenScope` calls it with identical messages. cabana never references `HasFrontendTokenScopes`. + - The decision note records the context, the three options, the reasons and the consequences. +- **Narrowed revocation and client address (Task 3).** + - `MCPSubjectChanged` and `pier.Event` gain `TokenID`/`ClientID` (JSON `token_id`/`client_id`, `omitempty`), and `listenEvents` copies them. + - `revokedMatch` builds the predicate. `sessionRouter.matchingFunc` and `statelessPath.matchingFunc`/`evictFunc` apply it, and the old `matching(guard, user)`/`evict(guard, user)` remain as wrappers. + - cabana's `fireTokenRevoked` fires after commit with the token row id. The revoke handler uses it, and the misleading comment is fixed. + - `towel.WithClientIP`/`towel.ClientIP` are new. pier's `bearer` wraps the resolve context with the address. The optional `clientIPHost` is implemented by surf's `pierHost.ClientIP`, which uses `TrustedProxies(app.Config)`; the fallback is the host part of `RemoteAddr`. `ResolveBackendToken` logs `remote` on all six `admin.auth` lines. +- **cabana admin API (Task 4).** + - `safeAccountActionURL` checks the action URL. A refused URL is dropped and logged without the URL. + - `AccountContributionView.Error` (`json:"error,omitempty"`) is set when `Status` fails. A failing `Applies` drops only that action, and the list no longer answers 500 for a plugin error. + - The status goes through `localizeDetail`. + - The non-string payload refusal now uses `backend::lang.form.action_prompt_invalid` (en and pl, plus the fixture). + - The `AdminRecordAction` description, the body param and the `AdminActionRequest.Payload` comment are corrected. The toolbar description is reworded so that "payload are all refused" no longer appears anywhere. The pact prompt doc states the 422 toast behaviour. + - admin.json and schema.d.ts are regenerated. +- **Admin SPA (Task 5).** + - Plain keys: `originOf` (`composedPath()[0]`) feeds `isEditableTarget`, which also treats a custom element inside `form`/`[role=form]` as editable. + - `onModifierKey` blurs the focused element before `target.click()` and refocuses it if it is still connected. + - `safeUrl` guards both the `href` and `window.open`. + - A non-2xx GET shows the `form.error_generic` toast and keeps the previous sections. `error: true` renders `myaccount.section_error`. + - The confirm and stale texts use the new `myaccount.action_confirm` and `myaccount.action_stale` keys. + - The dist is rebuilt and committed in the same commit. + +## Verification + +- `go vet ./...` and `go vet -tags summercms_mcp ./...` are clean. +- `go test -count=1 ./...` is green with Docker available (the cabana Postgres suites ran), and `scripts/check-phase19.7.sh --go` PASSES. +- `go test -count=1 -short -tags summercms_mcp ./modules/pier/... ./modules/surf/...` is green. +- `cd examples/hello && go test ./...` is green. +- `npm --prefix admin run typecheck` is clean. `npm --prefix admin run test` passes (100 files, 1544 tests). `npm --prefix admin run build` succeeds. +- `scripts/check-admin-openapi.sh --check` exits 0 and `scripts/check-admin-dist.sh` matches. Both were also run at 44835e4: the dist matched before the SPA commit. +- `scripts/check-phase19.7.sh --docs` (includes `summer docs:build --check`: no problems), `--marker` and `--admin` PASS. +- `git diff --quiet 51d088a -- modules/cabana/account.go` holds. +- go.mod, go.sum and admin/package-lock.json are unchanged. +- An ad-hoc check of `revokedMatch` was run and then deleted, because plan 09 owns the named tests. It confirmed: + - a token event spares another token and an OAuth identity with the same TokenID; + - a client event hits only that client; + - a plain event and a deleted event hit everything. + +## Deviations from Plan + +### Auto-fixed Issues + +**1. [Rule 1 - Bug] `mintAudience` takes the issue time as well as `authTime`** +- **Found during:** Task 1 +- **Issue:** With the plan's signature (`authTime` only), `MintAudience` would read `time.Now()` once for `authTime` and `mintAudience` again for `iat`. Across a second boundary, `auth_time` would differ from `iat`. +- **Fix:** `mintAudience(secret, sub, issuerURL, ttl, audience, now, authTime)`. `MintAudience` passes one `now` for both, and refresh passes `time.Now()` and the copied value. +- **Files modified:** modules/bouncer/mint.go, modules/bouncer/refresh.go +- **Commit:** c14238b + +**2. [Rule 2 - Missing critical] `/\` treated as protocol-relative** +- **Found during:** Task 4 and Task 5 +- **Issue:** Browsers normalise `/\host` to `//host`, so a "single-slash relative path" check alone would let an off-origin URL through. +- **Fix:** Both the server's `safeAccountActionURL` and the SPA's `safeUrl` refuse a leading `/\`. +- **Commits:** 44835e4, 88559d5 + +**3. [Rule 3 - Blocking] Body param text without inner double quotes** +- **Found during:** Task 4 +- **Issue:** swag's quoted `@Param` description cannot hold `{"payload": text}`. Escaped quotes were cut to `Empty object, or {\`. +- **Fix:** The text reads `Empty object, or {payload: text} for a prompted action`. The description keeps the full `{"payload": ""}` form. +- **Commit:** 44835e4 + +**4. [Scope] Toolbar action description reworded** +- **Issue:** The acceptance grep requires zero "payload are all refused" in admin_openapi.go and admin.json, but the toolbar route's description (which correctly refuses payload) used the same words. +- **Fix:** It now reads "the route refuses record_id, values and payload", with the same meaning. +- **Commit:** 44835e4 + +**5. [Plan file list] Revocation predicate lives in events.go** +- `sessionRouter.matching` is defined in events.go, not session.go, so `matchingFunc` and `revokedMatch` went there. session.go was not touched. +- docs/backend/users-and-permissions.md also gained the My account URL and error sentences, under the docs rule. +- **Commits:** 24cf6cd, 44835e4, 88559d5 + +**6. [Rule 3] gofmt of account_tokens.go** +- The file lacked a trailing newline (pre-existing). Running gofmt on the edited file fixed it in 24cf6cd. + +### Tests changed for old behaviour + +- `modules/cabana/record_action_prompt_postgres_test.go`: the non-string payload refusal now expects "The answer must be text." instead of the hard-coded "The payload must be a string." (B-IN-02). Commit 44835e4. +- `admin/tests/account/AccountContributions.test.ts`: the stale-toast string override moves from `backend::lang.form.action_stale` to `backend::lang.myaccount.action_stale` (B-IN-01). Commit 88559d5. +- No test pinned the per-admin session kill on token revoke or the 500 for a failing `Status`/`Applies`. + +## Notes for later plans + +- Plan 08: the account-link confirm should call `bouncer.VerifyAuthTime(token, secret, bouncer.AudienceUser)`, refuse `ok == false` and compare `authTime` with the request's `created_at`. User-plugin sign-in mints go through `bouncer.Mint`/`MintAudience` and so already carry the claim. +- Plan 08: a client-specific revocation is fired as `&pact.MCPSubjectChanged{Guard: "", UserID: id, Kind: pact.MCPSubjectRevoked, ClientID: clientID}`. `UserID` must be set, because the narrowed match requires it. +- Plan 09 (named tests): `TestAuthTime`, revokedMatch and fan-out narrowing, `towel.ClientIP` on the resolver log, `safeAccountActionURL`, section isolation, `originOf`/custom-element editable, blur-before-click with a taglist draft, `safeUrl`. +- The `admin:token:revoke` CLI fires no MCP event, before or after this plan. It is out of scope; plan 09 may record it as deferred. + +## Known Stubs + +None. + +## Threat Flags + +None. The new surfaces (the `auth_time` claim, the client address on the context, the narrowed events) are the mitigations T-19.7-51 to T-19.7-56 in the plan's threat register. + +## Self-Check: PASSED + +- FOUND: modules/bouncer/auth_time_smoke_test.go, .planning/notes/2026-10-23-frontend-token-scopes.md +- FOUND: c14238b, 9eedfbb, 0ec00f7, 24cf6cd, 44835e4, 88559d5 +- The only deleted files are the replaced hashed assets under modules/boardwalk/dist (a rebuild)