docs(19.4-03): complete cutover, operations, parity harness and security review plan
This commit is contained in:
@@ -0,0 +1,351 @@
|
||||
---
|
||||
phase: 19.4-oidc-provider-on-wristband
|
||||
plan: 03
|
||||
subsystem: auth
|
||||
tags: [oidc, oauth, wristband, cabana, import, winter, key-rotation, prune, schedule, tide, parity, security-review, asvs]
|
||||
|
||||
requires:
|
||||
- phase: 19.4-01
|
||||
provides: wristband OIDC flavour (IssueOIDCClientCredentials, SubjectSector, ParsePrivateKeyPEM, ParsePublicKeyPEM, RejectRedirectURI)
|
||||
- phase: 19.4-02
|
||||
provides: sm-oauthserver-plugin core (models, stores, KeyRing with Rotate and Prune, branding sanitisers, harness bootOAuth)
|
||||
- phase: 18.1
|
||||
provides: tide record/replay and summer parity:record / parity:replay
|
||||
provides:
|
||||
- oauthserver:import-winter (six PHP tables, --key-dir with original kids, --settings, --uploads-dir, --force, --dry-run) and scripts/winter-export.sql
|
||||
- cabana admin for OIDC clients (one-time secret reveal, regenerate_secret danger action, authorized users with their sub) and the branding singleton linked from Settings
|
||||
- oauthserver:rotate-keys and oauthserver:prune, daily prune on the scheduler, opt-in rotation_interval
|
||||
- tide parity harness (ten spec flows, rules, deviations, runbook, TestPHPParityReplay, TestIDTokenShapeParity) that fails closed without recordings
|
||||
- tide captures a named input of an HTML response (from: response.form)
|
||||
- 19.4-SECURITY-REVIEW.md (ASVS L1, every T-19.4 threat verified or fixed, T-19.4-36..39 accepted)
|
||||
affects: [19.4-04, wavepath-cutover, 19.7, fonoteka]
|
||||
|
||||
# Actuals (#2632): chars/4 over the added lines of the plan's diffs
|
||||
# (sm-oauthserver-plugin f1cc3b9..f8eaf70: 229660 chars; summercms.go bb4b8d4, eaa545c, 7f56382: 36494 chars).
|
||||
actuals:
|
||||
tokens: 66500
|
||||
tasks: 7
|
||||
commits: 11 # measured: sm-oauthserver-plugin rev-list --count f1cc3b9..HEAD = 6; summercms.go rev-list --count 1e37926..HEAD = 5 before this SUMMARY, of which 8a1d413 and 3386cbe are orchestrator planning commits (todo capture, D-14 amendment), so 9 commits are this plan's own
|
||||
plan_head_before: "sm-oauthserver-plugin f1cc3b973842e3e75e0efab1de2aad46da9cbd6c; summercms.go 1e37926"
|
||||
plan_head_after: "sm-oauthserver-plugin f8eaf70; summercms.go 7f56382 (before this SUMMARY commit)"
|
||||
|
||||
tech-stack:
|
||||
added: []
|
||||
patterns:
|
||||
- "Admin credential reveal: plaintext parked in cooler under {admin}:{record} for 5 minutes and forgotten on first read"
|
||||
- "Commands never print driver messages near secrets: fixed texts, SQLSTATE only, silent GORM logger for key writes"
|
||||
- "Parity harness fails closed: no recordings skips, PHASE194_REQUIRE_PARITY=1 fails, before -short and before Docker"
|
||||
- "Security review pins every mitigation with a test; gaps fixed as test-only commits in the repository they belong to"
|
||||
|
||||
key-files:
|
||||
created:
|
||||
- ../sm-oauthserver-plugin/console/import_winter.go
|
||||
- ../sm-oauthserver-plugin/console/import_keys.go
|
||||
- ../sm-oauthserver-plugin/console/import_settings.go
|
||||
- ../sm-oauthserver-plugin/console/helpers.go
|
||||
- ../sm-oauthserver-plugin/console/rotate_keys.go
|
||||
- ../sm-oauthserver-plugin/console/prune.go
|
||||
- ../sm-oauthserver-plugin/scripts/winter-export.sql
|
||||
- ../sm-oauthserver-plugin/schedule.go
|
||||
- ../sm-oauthserver-plugin/admin.go
|
||||
- ../sm-oauthserver-plugin/admin_permissions.go
|
||||
- ../sm-oauthserver-plugin/admin_navigation.go
|
||||
- ../sm-oauthserver-plugin/controllers/admin_registry.go
|
||||
- ../sm-oauthserver-plugin/controllers/clients.go
|
||||
- ../sm-oauthserver-plugin/controllers/branding.go
|
||||
- ../sm-oauthserver-plugin/classes/reveal.go
|
||||
- ../sm-oauthserver-plugin/parity_test.go
|
||||
- ../sm-oauthserver-plugin/testdata/parity/README.md
|
||||
- ../sm-oauthserver-plugin/testdata/parity/rules.yaml
|
||||
- ../sm-oauthserver-plugin/testdata/parity/deviations.yaml
|
||||
- ../sm-oauthserver-plugin/testdata/parity/flows/*.yaml (10 spec flows)
|
||||
- ../sm-oauthserver-plugin/import_cutover_smoke_test.go
|
||||
- ../sm-oauthserver-plugin/admin_smoke_test.go
|
||||
- ../sm-oauthserver-plugin/commands_smoke_test.go
|
||||
- ../sm-oauthserver-plugin/admin_surface_test.go
|
||||
- ../sm-oauthserver-plugin/limits_smoke_test.go
|
||||
- ../sm-oauthserver-plugin/import_keys_errors_test.go
|
||||
- modules/wristband/oidc_security_test.go
|
||||
- modules/tide/capture_test.go
|
||||
- .planning/phases/19.4-oidc-provider-on-wristband/19.4-SECURITY-REVIEW.md
|
||||
modified:
|
||||
- ../sm-oauthserver-plugin/plugin.go
|
||||
- ../sm-oauthserver-plugin/servers.go
|
||||
- ../sm-oauthserver-plugin/models/client.go
|
||||
- ../sm-oauthserver-plugin/models/settings.go
|
||||
- ../sm-oauthserver-plugin/config/config.yaml
|
||||
- ../sm-oauthserver-plugin/lang/en/lang.yaml
|
||||
- ../sm-oauthserver-plugin/lang/pl/lang.yaml
|
||||
- ../sm-oauthserver-plugin/README.md
|
||||
- ../sm-oauthserver-plugin/go.mod
|
||||
- ../sm-oauthserver-plugin/authorizations_smoke_test.go
|
||||
- modules/tide/variables.go
|
||||
- modules/tide/README.md
|
||||
- docs/services/parity-testing.md
|
||||
|
||||
key-decisions:
|
||||
- "Task 5 checkpoint answered skip-parity; per the user's D-14 amendment (3386cbe) the PHP recording moves to the WavePath cutover phase as a preliminary task (todo 2026-10-19-record-php-oidc-parity-against-wavepath), and the phase verdict is not held at human_needed"
|
||||
- "Redirect URIs are checked on admin save with wristband.RejectRedirectURI; http loopback URIs only on development clients (PHP checked nothing at save)"
|
||||
- "Branding and client Rules() use only nullable|max:N; hex colours, the https font stylesheet and the font family are checked in the branding controller, because lagoon has no string/url/regex rules"
|
||||
- "Scopes are a fixed-option taglist (array mode, custom tags off), the OQ9 choice"
|
||||
- "regenerate_secret lives on the preview screen: cabana requires a preview block before it accepts record actions"
|
||||
- "Parity captures sit on the spec flow steps, not in rules.yaml (route rules match method and path only, a failed capture aborts recording); rules.yaml is kept for parity:proxy and the runbook says never pass --rules to parity:record"
|
||||
- "Security review: every T-19.4 mitigation was present in code; eight lacked a test pin and got one (summercms.go eaa545c, plugin b4223be, f8eaf70); T-19.4-36..39 accepted"
|
||||
|
||||
patterns-established:
|
||||
- "Import commands: refuse non-empty targets without --force, --dry-run reports only, one transaction, errors name columns and kids, never values"
|
||||
- "Admin partials take curated view models (SecretRevealView, AuthorizationsView), never GORM models"
|
||||
|
||||
requirements-completed: [SC-3, SC-4, D-03, D-08, D-10, D-11, D-13, D-14, D-15, D-17, D-19]
|
||||
|
||||
coverage:
|
||||
- id: D1
|
||||
description: "A WinterCMS site's OIDC tables and key_dir import into the Go plugin: an old access token reads userinfo, an old id_token verifies against the new JWKS, the old client secret redeems an imported code, the next id_token carries the imported active kid; refused without --force, dry run writes nothing, missing users abort"
|
||||
requirement: SC-4
|
||||
verification:
|
||||
- kind: integration
|
||||
ref: "../sm-oauthserver-plugin/import_cutover_smoke_test.go#TestImportWinterCutoverSmoke (Docker Postgres)"
|
||||
status: pass
|
||||
- kind: unit
|
||||
ref: "../sm-oauthserver-plugin/import_keys_errors_test.go#TestImportKeyDirErrorsNameNoKey"
|
||||
status: pass
|
||||
human_judgment: false
|
||||
- id: D2
|
||||
description: "OIDC clients and branding administered through cabana's admin API: oas_ client id, oas_cs_ secret revealed once, regenerate invalidates the old secret, no secret_hash in list or form JSON, redirect URIs one per line, pairwise sector check, 403 without manage_clients, authorizations partial lists the sub; branding singleton refuses non-hex colours and its colour and logo reach the consent page"
|
||||
requirement: D-03
|
||||
verification:
|
||||
- kind: integration
|
||||
ref: "../sm-oauthserver-plugin/admin_smoke_test.go#TestClientsAdminSmoke"
|
||||
status: pass
|
||||
- kind: integration
|
||||
ref: "../sm-oauthserver-plugin/admin_smoke_test.go#TestBrandingAdminSmoke"
|
||||
status: pass
|
||||
- kind: unit
|
||||
ref: "../sm-oauthserver-plugin/admin_surface_test.go#TestAdminSurfaceHidesCredentials"
|
||||
status: pass
|
||||
human_judgment: false
|
||||
- id: D3
|
||||
description: "The Clients and Branding screens render usably in the embedded admin SPA (secret reveal callout, taglist of scopes, authorized-users table, regenerate button on the preview screen, Settings link)"
|
||||
requirement: D-08
|
||||
verification: []
|
||||
human_judgment: true
|
||||
rationale: "The smokes drive cabana's JSON admin API; no browser test looks at how the SPA renders these partials and widgets"
|
||||
- id: D4
|
||||
description: "rotate-keys promotes a new key and keeps the old one verifiable for the grace window; prune deletes expired codes, tokens, requests, refresh tokens, MCP codes and keys past the grace with PHP's output lines; daily prune scheduled, rotation only with rotation_interval"
|
||||
requirement: SC-3
|
||||
verification:
|
||||
- kind: integration
|
||||
ref: "../sm-oauthserver-plugin/commands_smoke_test.go#TestRotateAndPruneSmoke"
|
||||
status: pass
|
||||
- kind: integration
|
||||
ref: "../sm-oauthserver-plugin/commands_smoke_test.go#TestScheduleSmoke"
|
||||
status: pass
|
||||
human_judgment: false
|
||||
- id: D5
|
||||
description: "Parity harness ships and fails closed (D-14 as amended): ten spec flows, rules, deviations citing D-numbers, runbook; without recordings TestPHPParityReplay and TestIDTokenShapeParity skip with \"no PHP recordings\" and fail with PHASE194_REQUIRE_PARITY=1; no secret under testdata"
|
||||
requirement: D-14
|
||||
verification:
|
||||
- kind: other
|
||||
ref: "Task 6 verify with PARITY_REPLY=skip-parity (no response: in any flow, skip message, secret grep clean)"
|
||||
status: pass
|
||||
- kind: other
|
||||
ref: "PHASE194_REQUIRE_PARITY=1 go test -short -run '^(TestPHPParityReplay|TestIDTokenShapeParity)$' fails both (fail-closed check)"
|
||||
status: pass
|
||||
- kind: unit
|
||||
ref: "../sm-oauthserver-plugin/parity_test.go#TestParityFixturesHoldNoSecrets"
|
||||
status: pass
|
||||
human_judgment: false
|
||||
- id: D6
|
||||
description: "Security review (ASVS L1) records every T-19.4 threat with code location, test and status; gaps fixed with test pins; nothing left unresolved"
|
||||
requirement: SC-4
|
||||
verification:
|
||||
- kind: other
|
||||
ref: "Task 7 verify (all 35 threat ids present, go vet, wristband and plugin suites green; 39 status rows, 0 unresolved)"
|
||||
status: pass
|
||||
- kind: unit
|
||||
ref: "modules/wristband/oidc_security_test.go#TestOIDCCodeBindingSmoke, #TestOIDCLogsCarryNoCredentials, #TestOIDCSubjectIsRandomSmoke"
|
||||
status: pass
|
||||
- kind: integration
|
||||
ref: "../sm-oauthserver-plugin/limits_smoke_test.go#TestTokenLimitsSmoke; authorizations_smoke_test.go#TestUserCascadeSmoke"
|
||||
status: pass
|
||||
human_judgment: false
|
||||
- id: D7
|
||||
description: "tide captures a named input of an HTML response (from: response.form), documented in the tide README and docs/services/parity-testing.md"
|
||||
verification:
|
||||
- kind: unit
|
||||
ref: "modules/tide/capture_test.go#TestCaptureResponseFormField"
|
||||
status: pass
|
||||
human_judgment: false
|
||||
|
||||
duration: 71min
|
||||
completed: 2026-10-19
|
||||
status: complete
|
||||
---
|
||||
|
||||
# Phase 19.4 Plan 03: Cutover, operations, parity harness and security review Summary
|
||||
|
||||
**A WinterCMS site's OIDC provider moves to sm-oauthserver-plugin with its original kids, client secrets and live tokens (oauthserver:import-winter with key_dir, branding and logos), administrators manage clients and branding in cabana with a one-time secret reveal, keys rotate with a grace window and expired grants are pruned daily, the tide parity harness is ready and fails closed, and an ASVS L1 review pins every T-19.4 mitigation with a test.**
|
||||
|
||||
D-14 parity: NOT RUN (skip-parity)
|
||||
|
||||
## Performance
|
||||
|
||||
- **Duration:** about 71 min wall clock over two executor contexts, including the Task 5 checkpoint pause (Tasks 1-4, then Tasks 6-7 in this continuation)
|
||||
- **Started:** 2026-10-19T17:28Z (after the 19.4-02 state commit 1e37926)
|
||||
- **Completed:** 2026-10-19T18:39Z
|
||||
- **Tasks:** 7 (5 executed with commits, 1 human checkpoint, 1 skip branch without a commit)
|
||||
- **Files:** 54 in sm-oauthserver-plugin, 6 code and docs files in summercms.go plus the review document
|
||||
|
||||
## Task 5 checkpoint reply
|
||||
|
||||
Human reply (verbatim): "skip-parity, it will be preliminary task on future wavepath cutoff."
|
||||
|
||||
The user then decided that the phase passes without the recording. The orchestrator amended D-14 in this plan, 19.4-04-PLAN.md, 19.4-CONTEXT.md and 19.4-VALIDATION.md (commit 3386cbe) and captured the todo `.planning/todos/pending/2026-10-19-record-php-oidc-parity-against-wavepath.md` (g15office govfaqir, commit 8a1d413).
|
||||
|
||||
## Accomplishments
|
||||
|
||||
- `oauthserver:import-winter` copies clients, auth codes, access tokens, auth requests, consents and subjects from the `scripts/winter-export.sql` JSON-lines export, keeping ids, hashes and timestamps and moving the sequences; `--key-dir` imports every key with its original kid, its PEMs (private encrypted with the app key) and its retired state; `--settings` and `--uploads-dir` bring branding and logos. Relying parties keep working through the cutover (`TestImportWinterCutoverSmoke`).
|
||||
- cabana Clients screen (kind oidc only): create issues `oas_` id and `oas_cs_` secret shown once, `regenerate_secret` danger action, redirect URIs one per line checked with `wristband.RejectRedirectURI`, pairwise sector check, authorized users with their sub. Branding singleton linked from Settings with logo upload.
|
||||
- `oauthserver:rotate-keys` (with `--prune`), `oauthserver:prune` with PHP's four lines plus refresh tokens and MCP codes, daily prune on the scheduler, opt-in `rotation_interval`.
|
||||
- Parity harness: ten spec flows, capture rules, deviations citing D-numbers, runbook, `TestPHPParityReplay` and `TestIDTokenShapeParity`, fail-closed under `PHASE194_REQUIRE_PARITY=1`.
|
||||
- tide gained `from: response.form` to capture a named input of an HTML response (the consent token).
|
||||
- Security review `19.4-SECURITY-REVIEW.md`: all 35 threats verified or fixed, eight test pins added, four new findings accepted.
|
||||
|
||||
## Task Commits
|
||||
|
||||
sm-oauthserver-plugin (`../sm-oauthserver-plugin`):
|
||||
1. **Task 1 (tracer):** `3d0d721` feat: oauthserver:import-winter copies the WinterCMS provider tables and key_dir
|
||||
2. **Task 2:** `7cb57cf` feat: admin screens for OIDC clients and branding, branding and logo import
|
||||
3. **Task 3:** `ffb7d85` feat: oauthserver:rotate-keys and oauthserver:prune on the schedule
|
||||
4. **Task 4:** `c7bf1a9` test: PHP parity suite ready to record (tide spec flows, deviations, replay)
|
||||
5. **Task 5:** checkpoint, no commit (reply "skip-parity")
|
||||
6. **Task 6:** skip branch, no commit (nothing recorded)
|
||||
7. **Task 7 fixes:** `b4223be` test: pin the admin credential surface and deactivated-user sign-in; `f8eaf70` test: pin the token route limits and key import error texts
|
||||
|
||||
summercms.go:
|
||||
- **Task 4 (fix):** `bb4b8d4` feat(19.4-03): tide captures a named input of an HTML response
|
||||
- **Task 7 fix:** `eaa545c` test(19.4-03): pin OIDC code binding, credential-free logs and random subjects
|
||||
- **Task 7:** `7f56382` docs(19.4): security review
|
||||
- Orchestrator planning commits during the checkpoint (not plan commits): `8a1d413` todo capture, `3386cbe` D-14 amendment
|
||||
|
||||
**Plan metadata:** this SUMMARY commit, then the state and roadmap commit.
|
||||
|
||||
## Deferred to the WavePath cutover
|
||||
|
||||
The PHP recording of the OIDC contract is a preliminary task of the future WavePath cutover phase, tracked in `.planning/todos/pending/2026-10-19-record-php-oidc-parity-against-wavepath.md` (g15office govfaqir). WavePath's OIDC provider does not move to the Go plugin until it is done.
|
||||
|
||||
- **What remains:** start WavePath locally, record all ten spec flows per `../sm-oauthserver-plugin/testdata/parity/README.md`, write `seed.json`, `winter-export.jsonl` (hashes only) and `id_token_shape.json`, then make `TestPHPParityReplay` and `TestIDTokenShapeParity` pass with `PHASE194_REQUIRE_PARITY=1`, settling every difference with a fix or a `deviations.yaml` entry that cites a D-number.
|
||||
- **Known open items for that recording:**
|
||||
- Cache-Control on OIDC redirects: Go sends `no-store, private`, PHP sends `no-cache, private`. Settle with a fix or a decision-backed deviation.
|
||||
- `summer parity:record` keeps PHP's `Set-Cookie` headers; delete them from the fixtures before committing (`TestParityFixturesHoldNoSecrets` fails while any remain).
|
||||
- id_token header member order is not compared (golang-jwt map order versus php-jwt).
|
||||
- Never pass `--rules` to `parity:record`; captures live on the spec flow steps.
|
||||
- **In this phase:** the D-14 truth (amended) is the fail-closed harness, checked by the Task 6 skip branch and the `PHASE194_REQUIRE_PARITY=1` failure. The skip is not parity evidence; it is the accepted outcome for this phase.
|
||||
|
||||
## Security review: fixed and accepted items
|
||||
|
||||
Fixed (test pins; the production code already implemented every mitigation):
|
||||
- T-19.4-02, T-19.4-09, T-19.4-13: summercms.go `eaa545c` (`TestOIDCCodeBindingSmoke`, `TestOIDCLogsCarryNoCredentials`, `TestOIDCSubjectIsRandomSmoke`).
|
||||
- T-19.4-23, T-19.4-32 (and T-19.4-15, T-19.4-27 reinforced): plugin `b4223be` (`TestAdminSurfaceHidesCredentials`, `TestUserCascadeSmoke` extended).
|
||||
- T-19.4-20, T-19.4-28: plugin `f8eaf70` (`TestTokenLimitsSmoke`, `TestImportKeyDirErrorsNameNoKey`).
|
||||
|
||||
Accepted:
|
||||
- T-19.4-33 (leader-elected River scheduler, idempotent jobs) and T-19.4-34 (app.key rotation documented), as planned.
|
||||
- New T-19.4-36: the `oauth-token` bucket keys by the form `client_id`, so Basic-auth callers can rotate it (D-18 PHP keys; 256-bit secrets).
|
||||
- New T-19.4-37: a trusted branding administrator's CSS or font stylesheet could read page content through CSS selectors.
|
||||
- New T-19.4-38: no `iss` in authorization responses (RFC 9207), PHP parity.
|
||||
- New T-19.4-39: `/oauth/mcp/authorize` has no throttle bucket when `mcp.enabled` (off by default).
|
||||
|
||||
The OpenID Foundation conformance suite was not run (optional, RESEARCH A11).
|
||||
|
||||
## Files Created/Modified
|
||||
|
||||
- `../sm-oauthserver-plugin/console/*.go`, `scripts/winter-export.sql`: import (tables, key_dir, settings, logos), rotate-keys, prune
|
||||
- `../sm-oauthserver-plugin/{admin,admin_permissions,admin_navigation,schedule}.go`, `controllers/{admin_registry,clients,branding}.go` and their YAML and partials, `classes/reveal.go`, `models/{client,settings}.go` and YAML: admin screens, reveal store, schedule
|
||||
- `../sm-oauthserver-plugin/testdata/parity/**`, `parity_test.go`: parity harness and runbook
|
||||
- `../sm-oauthserver-plugin/*_smoke_test.go`, `admin_surface_test.go`, `import_keys_errors_test.go`: smokes and review pins
|
||||
- `../sm-oauthserver-plugin/{plugin,servers}.go`, `config/config.yaml`, `lang/*`, `README.md`, `go.mod`: wiring, config keys, copy, documentation
|
||||
- `modules/tide/variables.go`, `capture_test.go`, `README.md`, `docs/services/parity-testing.md`: tide form-field capture
|
||||
- `modules/wristband/oidc_security_test.go`: review pins
|
||||
- `.planning/phases/19.4-oidc-provider-on-wristband/19.4-SECURITY-REVIEW.md`, `deferred-items.md`
|
||||
|
||||
## Decisions Made
|
||||
|
||||
See `key-decisions` in the frontmatter.
|
||||
|
||||
## Deviations from Plan
|
||||
|
||||
### Auto-fixed Issues
|
||||
|
||||
**1. [Rule 3 - Blocking] tide could not read an HTML form field**
|
||||
- **Found during:** Task 4
|
||||
- **Issue:** The consent token sits in a hidden input of the consent page; tide could capture only from headers, JSON and query strings.
|
||||
- **Fix:** Added `from: response.form` with a test, README and docs.
|
||||
- **Files modified:** modules/tide/variables.go, modules/tide/capture_test.go, modules/tide/README.md, docs/services/parity-testing.md
|
||||
- **Verification:** `TestCaptureResponseFormField`, `TestDocsTree`, `docs:build --check` green
|
||||
- **Committed in:** summercms.go bb4b8d4
|
||||
|
||||
**2. [Rule 1 - Bug] Every branding save answered 500**
|
||||
- **Found during:** Task 2
|
||||
- **Issue:** Plan 02's `Settings.Rules()` used string, url and regex rules that lagoon does not support.
|
||||
- **Fix:** Rules use only `nullable|max:N`; hex colours, the https font stylesheet and the font family are checked in the branding controller. `Client.Rules()` follows the same constraint.
|
||||
- **Verification:** `TestBrandingAdminSmoke`
|
||||
- **Committed in:** 7cb57cf
|
||||
|
||||
**3. [Rule 2 - Security] Redirect URIs unchecked at save**
|
||||
- **Found during:** Task 2
|
||||
- **Issue:** PHP stored any redirect URI an administrator typed.
|
||||
- **Fix:** Each line is checked with `wristband.RejectRedirectURI` on save; http loopback URIs only on development clients.
|
||||
- **Verification:** `TestClientsAdminSmoke`
|
||||
- **Committed in:** 7cb57cf
|
||||
|
||||
**4. [Rule 2 - Missing test pins] Eight T-19.4 mitigations had no test**
|
||||
- **Found during:** Task 7
|
||||
- **Issue:** Code binding and PKCE refusals, credential-free logs, random subjects, deactivated-user sign-in, the admin credential surface, token route limits and key import error texts were implemented but not pinned.
|
||||
- **Fix:** Test-only commits in the repository each belongs to; no production code changed.
|
||||
- **Verification:** wristband suite and the full plugin suite on Docker green
|
||||
- **Committed in:** summercms.go eaa545c; plugin b4223be, f8eaf70
|
||||
|
||||
### Plan details and names
|
||||
|
||||
5. **Captures on spec flow steps, not in rules.yaml:** tide route rules match only method and path, and a failed capture aborts recording. rules.yaml is kept for parity:proxy; the runbook says never pass `--rules` to parity:record.
|
||||
6. **`--settings` and `--uploads-dir` landed with Task 2**, not Task 1. `--uploads-dir` requires `--settings`. `ImportSettings(ctx, app, db, file, uploadsDir, out)`: the first string argument is the export file path (the plan wrote `raw`).
|
||||
7. **regenerate_secret on the preview screen:** cabana accepts record actions only with a `preview: {}` block.
|
||||
8. **Scopes:** a fixed-option taglist (array mode, custom tags off), the OQ9 choice.
|
||||
9. **Logos:** the site logo shows on the error page (as PHP); the consent page shows the client logo. `TestBrandingAdminSmoke` checks both and uploads the client logo through the admin.
|
||||
10. **"Scope outside the client"** is tested with `openid offline_access`; both PHP and Go answer "not supported".
|
||||
11. **Additive names:** root `Config.RotationInterval`, the plugin publishes its `*classes.KeyRing` for the commands; console `PruneMargin`, `WinterOptions.Settings`, `WinterOptions.UploadsDir`; classes `RevealTTL`; controllers `ActionRegenerateSecret`, `PermissionManageClients`, `PermissionManageSettings`, `SecretRevealView`, `AuthorizationsView`, `AuthorizationRow`; models.Client `RedirectURIsText`, `Fillable`, `Rules`, `SetPendingSecret`, `TakePendingSecret`.
|
||||
12. **Modules:** plugin go.mod moved gocloud.dev v0.46.0 and github.com/goccy/go-yaml v1.19.2 from indirect to direct at the same versions; no new module (T-19.4-SC holds).
|
||||
13. **Not created (skip-parity):** `testdata/parity/seed.json`, `winter-export.jsonl` and `id_token_shape.json` are written from a PHP recording, so they come with the deferred WavePath recording.
|
||||
14. **Task 6 action text:** it still describes a "Human verification pending" section after skip-parity; the amended must-have truth, acceptance criteria, done text and the user's decision supersede it, so this SUMMARY carries the `Deferred to the WavePath cutover` section instead.
|
||||
|
||||
---
|
||||
|
||||
**Total deviations:** 4 auto-fixed (1 blocking, 1 bug, 2 missing critical: one security check, one set of test pins) plus 10 recorded plan details. **Impact:** no scope creep; every planned command, controller, permission, partial and test exists.
|
||||
|
||||
## Issues Encountered
|
||||
|
||||
- **Docker-backed tests run:** the full plugin suite on real Docker (testcontainers Postgres): TestImportWinterCutoverSmoke, TestClientsAdminSmoke, TestBrandingAdminSmoke, TestRotateAndPruneSmoke, TestScheduleSmoke, TestTokenLimitsSmoke, TestUserCascadeSmoke and plan 02's smokes all pass (19 PASS); TestPHPParityReplay and TestIDTokenShapeParity skip with "no PHP recordings" and fail under `PHASE194_REQUIRE_PARITY=1` (checked).
|
||||
- **Framework:** `go vet ./...`, `go test -short ./modules/wristband/...`, the tide tests, `TestDocsTree` and `docs:build --check` green. The full framework `go test ./...` (Docker) was NOT RUN.
|
||||
- **sm-user-plugin:** `go vet` and `go test -short` green.
|
||||
- A temporary, uncommitted dry run during Task 4 recorded all ten flows against the Go plugin standing in for PHP and replayed them with zero unexplained diffs; nothing from it was committed.
|
||||
- Pre-existing gofmt issues in `modules/tide/flow_test.go` and `headers_test.go` (not from this plan) are logged in `deferred-items.md`.
|
||||
- Nothing is pushed, tagged or deployed. The plugin still builds through its `go.work` against the fonoteka checkout of sm-user-plugin; a new sm-user-plugin tag must come before the plugin is consumed by tag.
|
||||
|
||||
## User Setup Required
|
||||
|
||||
None - no external service configuration required.
|
||||
|
||||
## Next Phase Readiness
|
||||
|
||||
- Plan 04 (unit tests, last plan) can port the PHP test map and build `scripts/check-phase19.4.sh`; its gate must keep the parity tests fail-closed with `PHASE194_REQUIRE_PARITY` (NOT RUN only without the variable) and should review T-19.4-35.
|
||||
- Human check for the end-of-phase UAT (not parity): open the Clients and Branding screens in the admin SPA and confirm the secret reveal, scopes taglist, authorized-users table and regenerate button render usably (coverage D3).
|
||||
- The WavePath cutover phase starts with the parity recording (see "Deferred to the WavePath cutover").
|
||||
|
||||
---
|
||||
*Phase: 19.4-oidc-provider-on-wristband*
|
||||
*Completed: 2026-10-19*
|
||||
|
||||
## Self-Check: PASSED
|
||||
|
||||
All listed key files exist; commits 3d0d721, 7cb57cf, ffb7d85, c7bf1a9, b4223be, f8eaf70 (sm-oauthserver-plugin) and bb4b8d4, eaa545c, 7f56382 (summercms.go) exist. Task acceptance greps re-run: winter-export.sql has 8 `'table'` lines, README names oauthserver:import-winter 5 times, no secret_hash in the client YAML, no SigningKey in controllers or admin.go, one Danger action, one SingletonRecord, two fileupload fields, one pact.Daily and one pact.Every. Task 6 skip branch and Task 7 verify passed.
|
||||
@@ -4,3 +4,8 @@
|
||||
status: open
|
||||
**What:** `go test ./plugins/golem15/fonoteka/...` in `../fonoteka.go` (Docker-backed, not `-short`) reports "unexpected admin routes" for cabana routes other phases added: `GET`/`PUT /plytadmin/api/v1/account/preferences`, `GET`/`POST .../{controller}/reorder` and `GET .../{controller}/singleton`. The test's admin route allowlist predates them. Nothing in wristband or the OAuth wiring is involved; every OAuth test in that package passes.
|
||||
**Where:** `../fonoteka.go/plugins/golem15/fonoteka/admin_phase09_security_test.go:52`.
|
||||
|
||||
- tide test files are not gofmt-clean (found by 19.4-03, outside its scope)
|
||||
status: open
|
||||
**What:** `gofmt -l modules/tide/` lists `flow_test.go` and `headers_test.go`. Neither file was touched in phase 19.4 (19.4-03 changed only `variables.go`, `capture_test.go` and the docs in bb4b8d4); `go vet` and the tide tests pass.
|
||||
**Where:** `modules/tide/flow_test.go`, `modules/tide/headers_test.go`.
|
||||
|
||||
Reference in New Issue
Block a user