diff --git a/.planning/phases/07-user-plugin-and-authentication/07-REVIEW.md b/.planning/phases/07-user-plugin-and-authentication/07-REVIEW.md new file mode 100644 index 0000000..c063d51 --- /dev/null +++ b/.planning/phases/07-user-plugin-and-authentication/07-REVIEW.md @@ -0,0 +1,42 @@ +--- +phase: 07-user-plugin-and-authentication +reviewed: 2026-09-22T17:26: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 +findings: + critical: 0 + warning: 2 + info: 0 + total: 2 +status: issues_found +--- + +# Phase 7: Code Review Report + +**Reviewed:** 2026-09-22T17:26:00Z +**Depth:** standard +**Files Reviewed:** 4 +**Status:** issues_found + +## 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. + +## 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`. + +## Info + +None. diff --git a/.planning/phases/07-user-plugin-and-authentication/07-VERIFICATION.md b/.planning/phases/07-user-plugin-and-authentication/07-VERIFICATION.md new file mode 100644 index 0000000..b24def4 --- /dev/null +++ b/.planning/phases/07-user-plugin-and-authentication/07-VERIFICATION.md @@ -0,0 +1,110 @@ +--- +phase: 07-user-plugin-and-authentication +verified: 2026-09-22T17:26:00Z +status: gaps_found +score: 3/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 + +## Goal Achievement + +### Observable Truths + +| # | 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`. | + +**Score:** 3/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` | + +**Artifacts:** 3/4 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 | + +**Wiring:** 2/3 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 | + +**Coverage:** 4/5 requirements satisfied. AUTH-01 blocks phase sign-off. None of the five checkboxes were marked in `REQUIREMENTS.md`. + +## 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) + +## Human Verification Required + +None — the failing comparison is already in the recorded fixtures. + +## 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 + +### 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. + +## 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 + +## 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`. +**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*