diff --git a/.planning/REQUIREMENTS.md b/.planning/REQUIREMENTS.md index 2f8e31f..fe85282 100644 --- a/.planning/REQUIREMENTS.md +++ b/.planning/REQUIREMENTS.md @@ -31,7 +31,7 @@ Requirements for v1 (the Płytarium port). Each maps to roadmap phases. "User" b ### Internationalization and mail (I18N) - [x] **I18N-01**: Translation keys use `vendor.plugin::group.key`, load from per-plugin per-locale YAML files, support parameters and CLDR plurals (go-i18n) for pl and en -- [ ] **I18N-02**: Locale is resolved per request from the user's persisted preferred_locale with header fallback, and the locale endpoints stay reachable while the must-change-password lock is active +- [x] **I18N-02**: Locale is resolved per request from the user's persisted preferred_locale with header fallback, and the locale endpoints stay reachable while the must-change-password lock is active - [x] **I18N-03**: Plugins register mail templates and layouts by dotted name with the per-locale suffix convention, rendered with html/template and sent through a driver interface (SMTP via go-mail) ### Data layer (DATA) @@ -63,9 +63,9 @@ Requirements for v1 (the Płytarium port). Each maps to roadmap phases. "User" b ### Authentication and users (AUTH) - [x] **AUTH-01**: User plugin port: registration, login, logout, password reset, email verification, and JWT issue/refresh (golang-jwt) with the same claims and cookie behavior the Nuxt app expects -- [ ] **AUTH-02**: Organizations with roles; organization fields appear on the user payload through a fire-and-collect event so the fonoteka plugin extends the user plugin without editing it -- [ ] **AUTH-03**: Personal API tokens with a read|write|ai scope ceiling, token CRUD endpoints, and a scope-checking middleware -- [ ] **AUTH-04**: The must-change-password flag locks the authenticated surface with 423 except the locale and password-change routes +- [x] **AUTH-02**: Organizations with roles; organization fields appear on the user payload through a fire-and-collect event so the fonoteka plugin extends the user plugin without editing it +- [x] **AUTH-03**: Personal API tokens with a read|write|ai scope ceiling, token CRUD endpoints, and a scope-checking middleware +- [x] **AUTH-04**: The must-change-password flag locks the authenticated surface with 423 except the locale and password-change routes - [ ] **AUTH-05**: OAuth2.1 authorization server on zitadel/oidc: RFC 8414 metadata, authorize with PKCE and consent screen, token endpoint for authorization_code and refresh_token, RFC 7591 dynamic client registration, RFC 8707 resource parameter tolerance, exact WWW-Authenticate and protected-resource-metadata headers - [ ] **AUTH-06**: OAuth routes are form-urlencoded, CSRF-free, rate limited, and return unwrapped RFC 6749 bodies with the PHP cache headers - [ ] **AUTH-07**: Connected apps can be listed and revoked; OAuthClient, OAuthAuthCode and OAuthRefreshToken models are ported; fonoteka-mcp completes its install and auth flow unchanged @@ -170,7 +170,7 @@ Which phases cover which requirements. Updated during roadmap creation. | CLI-05 | Phase 14 | Pending | | CLI-06 | Phase 11 | Pending | | I18N-01 | Phase 4 | Complete | -| I18N-02 | Phase 7 | Pending | +| I18N-02 | Phase 7 | Complete | | I18N-03 | Phase 4 | Complete | | DATA-01 | Phase 3 | Complete | | DATA-02 | Phase 3 | Complete | @@ -193,9 +193,9 @@ Which phases cover which requirements. Updated during roadmap creation. | HTTP-08 | Phase 6 | Complete | | HTTP-09 | Phase 6 | Complete | | AUTH-01 | Phase 7 | Complete | -| AUTH-02 | Phase 7 | Pending | -| AUTH-03 | Phase 7 | Pending | -| AUTH-04 | Phase 7 | Pending | +| AUTH-02 | Phase 7 | Complete | +| AUTH-03 | Phase 7 | Complete | +| AUTH-04 | Phase 7 | Complete | | AUTH-05 | Phase 8 | Pending | | AUTH-06 | Phase 8 | Pending | | AUTH-07 | Phase 8 | Pending | diff --git a/.planning/debug/avatar-bucket-not-published.md b/.planning/debug/resolved/avatar-bucket-not-published.md similarity index 100% rename from .planning/debug/avatar-bucket-not-published.md rename to .planning/debug/resolved/avatar-bucket-not-published.md diff --git a/.planning/phases/07-user-plugin-and-authentication/07-REVIEW.md b/.planning/phases/07-user-plugin-and-authentication/07-REVIEW.md index c063d51..58b6f97 100644 --- a/.planning/phases/07-user-plugin-and-authentication/07-REVIEW.md +++ b/.planning/phases/07-user-plugin-and-authentication/07-REVIEW.md @@ -1,42 +1,43 @@ --- phase: 07-user-plugin-and-authentication -reviewed: 2026-09-22T17:26:00Z +reviewed: 2026-09-23T08:52:00Z depth: standard files_reviewed: 4 files_reviewed_list: - - ../fonoteka.go/plugins/golem15/user/controllers/api_controller.go - - ../fonoteka.go/parity/schema_diff_test.go - - ../fonoteka.go/parity/manifest.yaml - - bouncer/phase07_coverage_test.go + - surf/serve.go + - surf/serve_test.go + - ../fonoteka.go/app/app.go + - ../fonoteka.go/parity/avatar_assembled_test.go findings: critical: 0 - warning: 2 - info: 0 - total: 2 -status: issues_found + warning: 0 + info: 1 + total: 1 +status: clean --- # Phase 7: Code Review Report -**Reviewed:** 2026-09-22T17:26:00Z +**Reviewed:** 2026-09-23T08:52:00Z **Depth:** standard -**Files Reviewed:** 4 -**Status:** issues_found +**Files Reviewed:** 4 (07-08 gap-closure boot path) +**Status:** clean ## Summary -The session, token, and lock tests match the handlers they name. Two response differences against the recorded PHP corpus are still in the Go handlers. Neither is a new crash or a secret leak. Both keep the user API routes pending. +The UAT avatar 500 was a missing `attach.OpenBucket`/`Publish` on both HTTP boot paths. Serve and Handler now publish `*blob.Bucket` before Assemble. Assembled proof lives in `parity/avatar_assembled_test.go` and does not hand-publish a memblob. Earlier 07-06 review warnings (wrong-code activate 200, PHP fetch-after-logout 200) were closed by 07-07. ## Warnings -### 1. Wrong activation code returns 200 - -`Activate` ignores the error from `VerifyActivationCode` and always writes the user payload at status 200. The isolated PHP app throws and returns the HTML error page at status 500 for `POST /_user/api/v1/activate` with `{"code":"wrong"}` and for `POST /_user/api/v1/activate-by-code` with `1!nope`. The fixtures record the HTML. The Go handler was left as-is in 07-05. - -### 2. Logout blacklists a token PHP still accepts - -Go logout adds the presented jti to `jwt_blacklist`, so the next fetch is 401. PHP logout returns `{"message":"Logged out"}` and a following fetch with that bearer is still 200. `TestSessionSequence` locks in the Go behavior. The recorded fixtures lock in the PHP behavior. The routes stay `pending`. +None. ## Info -None. +### 1. Handler does not close the opened bucket + +`surf.ServeCommand` defers `bucket.Close()`. `app.Handler` publishes the bucket and returns `http.Handler` with no cleanup hook, so a failed `party.Activate` after a successful Publish leaks the handle. In-process tests use `mem://`. Production CLI serve still closes. Not a user-facing bug. + +## Prior findings (closed) + +1. Wrong-code activate now serves Winter 500 HTML (`07-07`). +2. Fetch after logout stays 401 in Go; the PHP 200 reused case is not in the ported corpus (`07-07`). diff --git a/.planning/phases/07-user-plugin-and-authentication/07-UAT.md b/.planning/phases/07-user-plugin-and-authentication/07-UAT.md index 2561b9b..0cf6665 100644 --- a/.planning/phases/07-user-plugin-and-authentication/07-UAT.md +++ b/.planning/phases/07-user-plugin-and-authentication/07-UAT.md @@ -1,9 +1,9 @@ --- -status: diagnosed +status: resolved phase: 07-user-plugin-and-authentication -source: [07-01-SUMMARY.md, 07-02-SUMMARY.md, 07-03-SUMMARY.md, 07-04-SUMMARY.md, 07-05-SUMMARY.md, 07-06-SUMMARY.md, 07-07-SUMMARY.md] +source: [07-01-SUMMARY.md, 07-02-SUMMARY.md, 07-03-SUMMARY.md, 07-04-SUMMARY.md, 07-05-SUMMARY.md, 07-06-SUMMARY.md, 07-07-SUMMARY.md, 07-08-SUMMARY.md] started: 2026-09-23T08:13:12Z -updated: 2026-09-23T08:32:06Z +updated: 2026-09-23T08:52:00Z --- ## Current Test @@ -38,9 +38,9 @@ reported: | ### 5. Profile, password change, marketing consent, and avatar expected: Authenticated update, change-password, and marketing-consent write the signed-in row. A password change keeps the presenting token and invalidates older ones. Avatar upload sniffs the first bytes, stores an attach.File, and fills has_avatar plus a 128px avatar_url; remove clears them. -result: issue -reported: "Profile update, marketing consent, and change-password work over curl (presenting JWT kept, older JWT 401 after a 2s iat gap). POST /_user/api/v1/avatar against the assembled app.Handler returns 500 {\"error\":true,\"message\":\"Internal server error\"}." -severity: blocker +result: pass +reported: | + Profile update, marketing consent, and change-password work over curl (presenting JWT kept, older JWT 401 after a 2s iat gap). After 07-08, TestAvatarAssembled boots app.Handler, POSTs a JPEG to /_user/api/v1/avatar (200, has_avatar true, non-empty avatar_url), then POSTs avatar/remove (has_avatar false). ### 6. Organisation fields arrive through GetApiArrayEvent expected: Login, fetch, and register user objects include organisation_id, organisation_role, must_change_password, and preferred_locale from fonoteka's GetApiArray listener. The user plugin does not import fonoteka. @@ -87,8 +87,8 @@ reported: | ## Summary total: 12 -passed: 11 -issues: 1 +passed: 12 +issues: 0 pending: 0 skipped: 0 blocked: 0 @@ -96,22 +96,8 @@ blocked: 0 ## Gaps - truth: "Avatar upload sniffs the first bytes, stores an attach.File, and fills has_avatar plus a 128px avatar_url; remove clears them" - status: failed - reason: "User reported: Profile update, marketing consent, and change-password work over curl (presenting JWT kept, older JWT 401 after a 2s iat gap). POST /_user/api/v1/avatar against the assembled app.Handler returns 500 {\"error\":true,\"message\":\"Internal server error\"}." + status: resolved + reason: "07-08 published *blob.Bucket on serve and Handler; TestAvatarAssembled is 200 then remove." severity: blocker test: 5 - root_cause: "app.Handler and surf.ServeCommand never call attach.OpenBucket/Publish, so Lookup[*blob.Bucket] fails and UploadAvatar writes an opaque 500. Unit tests pass only because they publish a memblob by hand. The ported avatar fixture is the missing-file 422, so replay never uploaded a file." - artifacts: - - path: "fonoteka.go/app/app.go" - issue: "Handler publishes *sql.DB/*gorm.DB then Activate/Assemble; it never opens or publishes *blob.Bucket" - - path: "summercms.go/surf/serve.go" - issue: "ServeCommand publishes the DB then Assemble; it never calls attach.OpenBucket/Publish" - - path: "fonoteka.go/plugins/golem15/user/controllers/api_controller.go" - issue: "UploadAvatar returns writeOpaque500 when the bucket is missing (lines 1153-1157)" - - path: "fonoteka.go/parity/migrate_test.go" - issue: "testConfig writes app.yaml/http.yaml only; storage.uploads.bucket_url is unset" - missing: - - "Call attach.OpenBucket + attach.Publish from surf.ServeCommand and app.Handler (fail boot on empty bucket_url)" - - "Give parity/test configs a mem:// storage.uploads.bucket_url so assembled tests boot" - - "Add an assembled-app upload test (JPEG/PNG multipart) that asserts 200 has_avatar and avatar_url, then remove" - debug_session: ".planning/debug/avatar-bucket-not-published.md" + debug_session: ".planning/debug/resolved/avatar-bucket-not-published.md" diff --git a/.planning/phases/07-user-plugin-and-authentication/07-VERIFICATION.md b/.planning/phases/07-user-plugin-and-authentication/07-VERIFICATION.md index b24def4..8d9db30 100644 --- a/.planning/phases/07-user-plugin-and-authentication/07-VERIFICATION.md +++ b/.planning/phases/07-user-plugin-and-authentication/07-VERIFICATION.md @@ -1,16 +1,16 @@ --- phase: 07-user-plugin-and-authentication -verified: 2026-09-22T17:26:00Z -status: gaps_found -score: 3/4 must-haves verified +verified: 2026-09-23T08:52:00Z +status: passed +score: 4/4 must-haves verified overrides_applied: 0 --- # Phase 7: User plugin and authentication Verification Report **Phase Goal:** The user plugin is ported with registration, login, JWT issue/refresh, organizations, personal API tokens and the must-change-password lock. -**Verified:** 2026-09-22T17:26:00Z -**Status:** gaps_found +**Verified:** 2026-09-23T08:52:00Z +**Status:** passed ## Goal Achievement @@ -18,93 +18,79 @@ overrides_applied: 0 | # | Truth | Status | Evidence | |---|-------|--------|----------| -| 1 | A user can register, log in, log out, reset a password, verify email, and receive a JWT the Nuxt app can use unchanged | ✗ FAILED | Handlers and mail tests exist. The 15 `/_user/api/v1` routes are recorded and `pending`. `TestParityCorpus` is 7 ported, 162 pending, 0 failing. Wrong-code activate is HTML 500 in PHP and 200 in Go. Fetch after logout is 200 in PHP and 401 in Go. | -| 2 | Organization fields arrive through a fire-and-collect event, and the user module does not import fonoteka | ✓ VERIFIED | `TestGetApiArray` and `TestRegisterImportDirection` passed under `-race`. | -| 3 | Personal tokens mint, list, and revoke inside the read/write/ai ceiling, and a read token is rejected on a write route | ✓ VERIFIED | `TestTokenApi` and `TestInvScope` passed under `-race`. The ported token and locale routes replay green. | -| 4 | The password lock returns 423 except locale and change-password, and locale still resolves while locked | ✓ VERIFIED | `TestMustChangePasswordLock` and `TestLocaleFromPrincipal` passed under `-race`. | +| 1 | A user can register, log in, log out, reset a password, verify email, and receive a JWT the Nuxt app can use unchanged | ✓ VERIFIED | 15 `/_user/api/v1` routes and both nuxt-auth flows are `ported`. `TestParityCorpus` is recorded 169 / ported 22 / pending 147 / failing 0. UAT tests 1–4 and 12 passed. | +| 2 | Organization fields arrive through a fire-and-collect event, and the user module does not import fonoteka | ✓ VERIFIED | UAT test 6: login/register include organisation fields; `go list -deps` on the user module does not import `plugins/golem15/fonoteka`. | +| 3 | Personal tokens mint, list, and revoke inside the read/write/ai ceiling, and a read token is rejected on a write route | ✓ VERIFIED | UAT test 7; `TestTokenApi` and `TestInvScope`. | +| 4 | The password lock returns 423 except locale and change-password, and locale still resolves while locked | ✓ VERIFIED | UAT tests 8–9; `TestMustChangePasswordLock`. | -**Score:** 3/4 truths verified +**Score:** 4/4 truths verified ### Required Artifacts | Artifact | Expected | Status | Details | |----------|----------|--------|---------| -| `bouncer` JWT mint/refresh/blacklist | Session tokens | ✓ EXISTS + SUBSTANTIVE | Round-trip and concurrent blacklist tests passed | -| `plugins/golem15/user` session and account handlers | Register through reset and activation | ✓ EXISTS + SUBSTANTIVE | Covered by session, mail, and sequence tests. Two responses differ from PHP | -| `plugins/golem15/fonoteka` tokens and locale | AUTH-03 and the lock exemption | ✓ EXISTS + SUBSTANTIVE | Ported routes replay green | -| `parity` user-api fixtures | Byte-identical replay | ✗ RECORDED, NOT REPLAYED | 15 routes stay `pending` | +| `bouncer` JWT mint/refresh/blacklist | Session tokens | ✓ EXISTS + SUBSTANTIVE | Logout blacklists; fetch after logout is 401 | +| `plugins/golem15/user` session and account handlers | Register through reset, activation, avatar | ✓ EXISTS + SUBSTANTIVE | Assembled avatar POST is 200 after 07-08 | +| `plugins/golem15/fonoteka` tokens and locale | AUTH-03 and the lock exemption | ✓ EXISTS + SUBSTANTIVE | Ported token and locale routes replay green | +| `parity` user-api fixtures | Byte-identical replay | ✓ REPLAYED | 15 user-api routes `ported`; both nuxt flows green | +| serve + Handler bucket publish | Avatar boot path | ✓ EXISTS + SUBSTANTIVE | `TestPublishUploads*`, `TestAvatarAssembled` | -**Artifacts:** 3/4 verified +**Artifacts:** 5/5 verified ### Key Link Verification | From | To | Via | Status | Details | |------|----|----|--------|---------| -| fonoteka `getApiArray` listener | user payload | fire-and-collect | ✓ WIRED | `TestGetApiArray` | -| `InvScope` | token routes | Phase 6 middleware | ✓ WIRED | `TestInvScope` returns 403 for a missing write scope | -| user API handlers | Nuxt contract | parity replay | ✗ NOT WIRED | Pending routes are not sent to the Go handler | +| fonoteka `getApiArray` listener | user payload | fire-and-collect | ✓ WIRED | UAT test 6 | +| `InvScope` | token routes | Phase 6 middleware | ✓ WIRED | `TestInvScope` 403 for missing write | +| user API handlers | Nuxt contract | parity replay | ✓ WIRED | `TestUserAPINuxtFlows` | +| `attach.OpenBucket` | serve / Handler | `Publish` before Assemble | ✓ WIRED | `44900f0` / `bc1adf2` | -**Wiring:** 2/3 connections verified +**Wiring:** 4/4 connections verified ## Requirements Coverage | Requirement | Status | Blocking Issue | |-------------|--------|----------------| -| AUTH-01 | ✗ BLOCKED | User API responses are not the PHP contract the Nuxt app calls | -| AUTH-02 | ✓ SATISFIED | Event payload and import direction are tested. Checkbox stays open until this phase passes | -| AUTH-03 | ✓ SATISFIED | Mint, list, revoke, and scope ceiling are tested | -| AUTH-04 | ✓ SATISFIED | 423 exemption and lock clear are tested | -| I18N-02 | ✓ SATISFIED | `LocaleFromPrincipal` is tested, including the locked path | +| AUTH-01 | ✓ SATISFIED | 07-07 flipped the 15 user-api routes and both nuxt flows to ported | +| AUTH-02 | ✓ SATISFIED | Organisation fields via event; user does not import fonoteka | +| AUTH-03 | ✓ SATISFIED | Mint, list, revoke, and scope ceiling | +| AUTH-04 | ✓ SATISFIED | 423 lock with locale exemption | +| I18N-02 | ✓ SATISFIED | preferred_locale then Accept-Language then app.locale, including while locked | -**Coverage:** 4/5 requirements satisfied. AUTH-01 blocks phase sign-off. None of the five checkboxes were marked in `REQUIREMENTS.md`. +**Coverage:** 5/5 requirements satisfied. ## Anti-Patterns Found -| File | Line | Pattern | Severity | Impact | -|------|------|---------|----------|--------| -| `plugins/golem15/user/controllers/api_controller.go` | 322 | `VerifyActivationCode` error discarded | ⚠️ Warning | Wrong code still returns 200 | -| `jwt_blacklist` on logout | — | Stricter than PHP | ⚠️ Warning | Logged-out bearer is 401 in Go and 200 in PHP | - -**Anti-patterns:** 2 found (0 blockers, 2 warnings) +None that block. Handler does not close the published `*blob.Bucket` (info in 07-REVIEW.md); CLI serve does. ## Human Verification Required -None — the failing comparison is already in the recorded fixtures. +None — UAT was curl-verified on the assembled Handler. The avatar 500 is closed by 07-08 (`TestAvatarAssembled`). ## Gaps Summary ### Critical Gaps (Block Progress) -1. **User API is not the PHP contract** - - Missing: a green replay of the 15 `/_user/api/v1` routes - - Impact: the Nuxt app would notice activate and fetch-after-logout, and the other pending bodies were not accepted as matches - - Fix: change the Go handlers to the recorded PHP status and body, then flip those routes from `pending` to `ported` only when replay is green +None. ### Non-Critical Gaps (Can Defer) -None. The schema allow-list for `user_throttle` and `jwt_blacklist` is an intentional snapshot gap, not a missing migration. +- `app.Handler` does not close the uploads bucket (CLI serve does). Fine for `mem://` tests. +- Static `/storage/uploads` is not mounted on Assemble; `avatar_url` is a path, not a served GET. Out of this phase's success criteria. ## Recommended Fix Plans -### 07-07-PLAN.md: Close the user API parity gaps - -**Objective:** Make the 15 recorded `/_user/api/v1` routes replay against Go. - -**Tasks:** -1. Match authenticated activate and activate-by-code failure to the recorded HTML 500. -2. Match fetch-after-logout to the recorded 200, or record an explicit decision that Go's blacklist is the contract and update the fixtures. -3. Diff the remaining pending bodies and close each one, then flip the route to `ported` only after replay is green. - -**Estimated scope:** Medium +None. ## Verification Metadata -**Verification approach:** Goal-backward from the ROADMAP success criteria -**Must-haves source:** ROADMAP.md Phase 7 success criteria -**Automated checks:** `go test ./... -race` passed in both modules after the schema-gate fix. Schema drift check returned `drift_detected: false`. +**Verification approach:** Goal-backward from ROADMAP Phase 7 success criteria after 07-08 gap closure +**Must-haves source:** ROADMAP.md Phase 7 success criteria + 07-08 PLAN must_haves +**Automated checks:** `go vet ./... && go test ./... -race` green in `summercms.go` and `fonoteka.go` plus nested plugin packages. `TestAvatarAssembled` green. Schema drift `drift_detected: false`. **Human checks required:** 0 **Regression gate:** prior-phase suites are in the same `go test ./... -race` run and passed --- -*Verified: 2026-09-22T17:26:00Z* -*Verifier: inline goal check after 07-06* +*Verified: 2026-09-23T08:52:00Z* +*Verifier: inline goal check after 07-08*