diff --git a/.planning/ROADMAP.md b/.planning/ROADMAP.md index 084a60c..be76449 100644 --- a/.planning/ROADMAP.md +++ b/.planning/ROADMAP.md @@ -329,7 +329,7 @@ Plans: **Wave 2** *(blocked on 08-01)* -- [ ] 08-02-PLAN.md — Deliver persistent connector-visible DCR with corrected schema and transaction-scoped stores +- [x] 08-02-PLAN.md — Deliver persistent connector-visible DCR with corrected schema and transaction-scoped stores **Wave 3** *(blocked on 08-02)* @@ -496,7 +496,7 @@ Phases execute in numeric order: 1 → 2 → 3 → 4 → 5 → 6 → 7 → 8 → | 5. Data layer full fidelity | 6/6 | Complete | 2026-09-18 | | 6. HTTP routing, auth groups and rate limiting | 14/14 | Complete | 2026-09-21 | | 7. User plugin and authentication | 8/8 | Complete | 2026-09-23 | -| 8. OAuth2.1 authorization server | 1/10 | In Progress| | +| 8. OAuth2.1 authorization server | 2/10 | In Progress| | | 9. Backend admin authentication and schema pipeline | 0/TBD | Not started | - | | 10. Admin Vue SPA | 0/TBD | Not started | - | | 11. Jobs, realtime and search infrastructure | 0/TBD | Not started | - | diff --git a/.planning/STATE.md b/.planning/STATE.md index 6966695..5ad4bc9 100644 --- a/.planning/STATE.md +++ b/.planning/STATE.md @@ -3,14 +3,14 @@ gsd_state_version: 1.0 milestone: v1.0 milestone_name: milestone status: executing -stopped_at: Completed 08-01-PLAN.md -last_updated: "2026-09-23T17:18:26.201Z" +stopped_at: Completed 08-02-PLAN.md +last_updated: "2026-09-23T17:54:25.008Z" last_activity: 2026-09-23 progress: total_phases: 15 completed_phases: 7 total_plans: 55 - completed_plans: 46 + completed_plans: 47 percent: 47 --- @@ -26,11 +26,11 @@ See: .planning/PROJECT.md (updated 2026-09-16) ## Current Position Phase: 08 (oauth2-1-authorization-server) — EXECUTING -Plan: 2 of 10 +Plan: 3 of 10 Status: Ready to execute Last activity: 2026-09-23 -Progress: [████████░░] 84% +Progress: [█████████░] 85% ## Performance Metrics @@ -92,6 +92,7 @@ Progress: [████████░░] 84% | Phase 07-user-plugin-and-authentication P07 | 3h 15m | 3 tasks | 28 files | | Phase 07-user-plugin-and-authentication P08 | 25min | 3 tasks | 6 files | | Phase 08 P01 | 25min | 2 tasks | 6 files | +| Phase 08 P02 | 30min | 3 tasks | 14 files | ## Accumulated Context @@ -211,6 +212,12 @@ Recent decisions affecting current work: - [Phase 08]: wristband.Options exposes only the four PHP-configurable metadata fields (service_documentation path, scopes_supported, token_endpoint_auth_methods_supported, authorization_response_iss_parameter_supported); response_types/grant_types/code_challenge_methods stay fixed protocol constants per D-06 - [Phase 08]: Plugin.Boot always constructs wristband.Server even with an empty app.url rather than failing loud, to avoid breaking the many existing fonoteka tests that boot without app.url configured; production must set app.url - [Phase 08]: D-10 (oauth guard retirement) is recorded via TestOAuthMetadataRouteIsolation rather than editing Phase 6 historical docs, per 08-PATTERNS.md guidance to prefer a supersession note +- [Phase ?]: [Phase 08 P02]: Corrective migration named 21_oauth_schema_correction.go (not 12_): Go init()-order/gormigrate slice position determines RollbackLast() target, not the migration's numeric ID, so the file must sort after 20_remaining.go +- [Phase ?]: [Phase 08 P02]: Schema-correction rollback refuses (changes nothing) rather than deleting/coercing rows when any row depends on a nullable lifecycle column +- [Phase ?]: [Phase 08 P02]: wristband.Tx ships the full ClientStore/AuthCodeStore/RefreshTokenStore/AccessTokenIssuer surface now so 08-03/08-04 reuse the same transaction seam without another interface change; expiry-sweep methods are deferred to 08-06 +- [Phase ?]: [Phase 08 P02]: Register strips C0/DEL control characters from client_name at DCR capture time (PHP only strips at ConnectedApp/Consent display time) per 08-CONTEXT.md discretion +- [Phase ?]: [Phase 08 P02]: OAuth config keys live under golem15.fonoteka.oauth.* (bare plugin ID, matching compass.MergePlugin), not plugins.golem15.fonoteka.oauth.* +- [Phase ?]: [Phase 08 P02]: AUTH-05/AUTH-06/AUTH-07 remain Pending in REQUIREMENTS.md: this plan ships DCR only, not the full authorize/token/consent surface ### Pending Todos @@ -232,6 +239,6 @@ Items acknowledged and carried forward from previous milestone close: ## Session Continuity -Last session: 2026-09-23T17:18:26.184Z -Stopped at: Completed 08-01-PLAN.md +Last session: 2026-09-23T17:54:24.991Z +Stopped at: Completed 08-02-PLAN.md Resume file: None diff --git a/.planning/phases/08-oauth2-1-authorization-server/08-02-SUMMARY.md b/.planning/phases/08-oauth2-1-authorization-server/08-02-SUMMARY.md new file mode 100644 index 0000000..ea6ad1c --- /dev/null +++ b/.planning/phases/08-oauth2-1-authorization-server/08-02-SUMMARY.md @@ -0,0 +1,162 @@ +--- +phase: 08-oauth2-1-authorization-server +plan: 02 +subsystem: auth +tags: [oauth2, rfc7591, dcr, wristband, gorm, transactions, postgres, advisory-lock] + +# Dependency graph +requires: + - phase: 08-oauth2-1-authorization-server + plan: 01 + provides: "wristband package skeleton (Options/Server/DefaultOptions), the assembled raw route group, and scripts/check-phase8-red.sh" +provides: + - "Corrected OAuth lifecycle schema: client_secret_hash/request_id/code_hash/user_id nullable, nine PHP-equivalent named indexes, additive gormigrate migration with a refusing (non-destructive) rollback" + - "wristband.Backend/wristband.Tx transaction-scoped store bundle (ClientStore/AuthCodeStore/RefreshTokenStore/AccessTokenIssuer) plus an in-memory implementation for framework tests" + - "wristband.Register: exact RFC 7591 DCR handler (validation order, 64 KiB body bound, control-char stripping, one-time secret, sha256-hash-only persistence)" + - "fonoteka classes/auth.OAuthStore: the GORM adapter satisfying wristband.Backend, with an advisory-lock-serialized sweep+cap+create and row-locked (FOR UPDATE) code/refresh reads for later plans" + - "POST /oauth/mcp/register mounted on the assembled raw group with only its named throttle; golem15.fonoteka.oauth.* config wired into the wristband server at Boot" +affects: [08-03-authorize, 08-04-token-exchange, 08-05-consent-and-connected-apps, 08-06-lifecycle-and-sweeps, 08-07-oauth-client-command, 08-09-parity-and-real-mcp-gate, 08-10-unit-tests-and-security-review] + +# Tech tracking +tech-stack: + added: [] + patterns: + - "Transaction-scoped store bundle: wristband.Backend.WithinTx hands one *gorm.DB-backed wristband.Tx to every store method for a single request, so sweep+cap+create (and later code-exchange/refresh-rotation) can never straddle two transactions" + - "Postgres advisory transaction lock (pg_advisory_xact_lock) serializes the DCR cap check + insert instead of row-level locking an aggregate, closing the count-then-insert race under concurrency" + - "RED/GREEN split per repo per task: each task's RED test (stubbed handler/store method returning a placeholder error) is committed before the real implementation, verified fail-closed via scripts/check-phase8-red.sh" + +key-files: + created: + - wristband/stores.go + - wristband/crypto.go + - wristband/register.go + - wristband/registration_test.go + - ../fonoteka.go/plugins/golem15/fonoteka/updates/21_oauth_schema_correction.go + - ../fonoteka.go/plugins/golem15/fonoteka/updates/oauth_schema_correction_test.go + - ../fonoteka.go/plugins/golem15/fonoteka/classes/auth/oauth_store.go + - ../fonoteka.go/plugins/golem15/fonoteka/classes/auth/oauth_store_test.go + - ../fonoteka.go/plugins/golem15/fonoteka/config/config.yaml + - ../fonoteka.go/plugins/golem15/fonoteka/oauth_registration_test.go + modified: + - ../fonoteka.go/plugins/golem15/fonoteka/models/oauth_client.go + - ../fonoteka.go/plugins/golem15/fonoteka/models/oauth_auth_code.go + - ../fonoteka.go/plugins/golem15/fonoteka/plugin.go + - ../fonoteka.go/plugins/golem15/fonoteka/routes.go + +key-decisions: + - "The corrective migration file is named 21_oauth_schema_correction.go, not 12_ as the plan's file list suggested (discretionary per 08-PATTERNS.md): Go's init()-order registration follows filename order, and gormigrate.RollbackLast() rolls back the last element of the registered slice, not the migration with the numerically highest ID. Naming it 12_ placed it before 20_remaining.go's migrations in the slice, so RollbackLast() rolled back the wrong migration; 21_ fixes the ordering to match its later migration ID." + - "Rollback refuses (returns an error, changes nothing) rather than deleting or coercing rows when any row still depends on a nullable lifecycle column; it only restores the original NOT NULL constraints and drops the added indexes once no such row exists." + - "wristband.Tx ships the full ClientStore/AuthCodeStore/RefreshTokenStore/AccessTokenIssuer surface now (08-RESEARCH.md Pattern 1) even though only ClientStore is exercised by this plan's DCR handler, so 08-03/08-04 reuse the same transaction seam without another interface change. AuthCodeStore/RefreshTokenStore intentionally omit an expiry-sweep method (D-17's sweep) — that's 08-06's Lifecycle plan's responsibility, not invented here." + - "Register strips C0 control characters and DEL from client_name at capture time (matching the regex PHP applies only at ConnectedApp/Consent display time), per 08-CONTEXT.md's discretion note to 'port PHP's regex' for control-character handling. PHP does not strip at DCR time itself; this is a defensive superset, not a byte-parity requirement, and is called out here in case a later parity check expects the PHP-unfiltered raw name." + - "Config keys live under golem15.fonoteka.oauth.* (the plugin's bare ID, matching compass.MergePlugin's path convention and the existing golem15.user.jwt.* precedent), not the plugins.golem15.fonoteka.oauth.* form floated as a discussion option in D-03; D-03 explicitly left the exact layout to implementation discretion." + - "pending_request_ttl_seconds/code_ttl_seconds/access_token_ttl_seconds/refresh_token_ttl_seconds/resource are declared in config/config.yaml with PHP-parity defaults per D-03, but no Go field consumes them yet — only dcr_client_cap, dcr_unconsented_sweep_seconds and register_max_body_bytes are wired into wristband.Options in this plan, since authorize/token don't exist until 08-03/08-04." + - "AUTH-05/AUTH-06/AUTH-07 are NOT marked complete in REQUIREMENTS.md, continuing 08-01's decision: this plan ships DCR only (one of AUTH-05's several RFC surfaces); authorize/PKCE/consent/token/refresh remain for later Phase 8 plans, and marking these Complete now would misrepresent phase progress." + +patterns-established: + - "oauthClientRegistrationLockKey (pg_advisory_xact_lock over hashtext) is the pattern for any future app-tier count-then-mutate invariant that must be atomic under concurrent transactions without a dedicated lock table." + +requirements-completed: [] + +# Metrics +duration: ~30min +completed: 2026-09-23 +--- + +# Phase 08 Plan 02: Persistence and Registration Summary + +**Corrected OAuth lifecycle schema plus a transaction-scoped, advisory-lock-serialized RFC 7591 Dynamic Client Registration slice, live end to end from the assembled Go app through real Postgres.** + +## Performance + +- **Duration:** ~30 min +- **Started:** 2026-09-23T17:18:26Z (approx., continuing from 08-01) +- **Completed:** 2026-09-23T17:48:00Z (approx.) +- **Tasks:** 3 completed (6 commits: RED/GREEN pairs across both repos) +- **Files modified:** 14 (5 created + 2 modified in summercms.go's wristband package; 5 created + 4 modified in fonoteka.go) + +## Accomplishments + +- The shipped OAuth schema (`202609180010_create_oauth_tables`) could not represent a pre-consent pending authorization row or a public DCR client; a new additive migration relaxes the four wrongly-`NOT NULL` columns and adds all nine PHP-equivalent named indexes, with a rollback that refuses rather than destroys data when any row still depends on nullability +- `wristband` gained its transaction-scoped `Backend`/`Tx` store bundle and an in-memory implementation for framework-only tests, plus fixed-transform crypto helpers (`crypto/rand` base64url, sha256 hex, `crypto/subtle.ConstantTimeCompare`, S256) +- `wristband.Register` is an exact byte-for-byte port of `OAuthRegisterController::register`'s validation order, redirect-URI/grant/response-type/auth-method rules, 64 KiB body bound, and one-time-secret/hash-only persistence contract +- `fonoteka`'s `OAuthStore` GORM adapter satisfies `wristband.Backend`: `CreateWithCap` serializes sweep+cap-check+create via `pg_advisory_xact_lock`, proven by a synchronized real-Postgres cap-1 test (`TestOAuthRegistrationCap`) that yields exactly one created row and one `ErrClientCapReached` under concurrency +- `POST /oauth/mcp/register` is live on the assembled app's raw route group with only its named throttle; a public and a confidential client each register successfully against real Postgres and reload through a fresh transaction with hash-only rows + +## Task Commits + +Each task's RED test was committed and verified fail-closed via `scripts/check-phase8-red.sh` before its GREEN implementation: + +1. **Task 1: OAuth schema correction** + - `0b18d27` (test, fonoteka.go): `TestPhase8RedOAuthSchema` fails against the still-`NOT NULL` schema (`PHASE8_RED:persistence-schema`) + - `4536b3e` (feat, fonoteka.go): the additive correction migration (`21_oauth_schema_correction.go`) and pointer-ified model fields +2. **Task 2a: wristband registration (RED)** — `c026b83` (test, summercms.go): `TestPhase8RedRegistration` fails against a 501 stub (`PHASE8_RED:registration`); adds `stores.go`/`crypto.go` and the Options/Server seams + **Task 2b: wristband registration (GREEN)** — `c0b1e3c` (feat, summercms.go): the exact RFC 7591 `Register` handler + **Task 2c: OAuth store (RED)** — `fec99f0` (test, fonoteka.go): `TestPhase8RedRegistrationStore` fails against a stubbed `CreateWithCap` (`PHASE8_RED:registration-store`) + **Task 2d: OAuth store (GREEN)** — `381de57` (feat, fonoteka.go): the transaction-scoped GORM adapter with the advisory lock and row-lock reads +3. **Task 3a: assembled mount (RED)** — `5891c7c` (test, fonoteka.go): `TestPhase8RedRegistrationApp` fails 404 against the unmounted route (`PHASE8_RED:registration-app`); adds `golem15.fonoteka.oauth.*` config and wires the store-backed server into `Plugin.Boot` + **Task 3b: assembled mount (GREEN)** — `ebdb249` (feat, fonoteka.go): mounts `POST /oauth/mcp/register` on the raw group with only its named throttle + +**Plan metadata:** committed as part of this summary/state-update commit. + +_Note: all three tasks carry `tdd="true"`; RED/GREEN pairs land as separate commits, split per repo where both repos changed (Tasks 2 and 3)._ + +## Files Created/Modified + +- `wristband/stores.go` — `ClientRecord`/`AuthCodeRecord`/`RefreshTokenRecord`/`IssuedToken`, `ClientStore`/`AuthCodeStore`/`RefreshTokenStore`/`AccessTokenIssuer`, `Tx`, `Backend`, `ErrClientCapReached` +- `wristband/crypto.go` — `randomBase64URL`, `sha256Hex`, `constantEqual`, `s256Challenge` +- `wristband/register.go` — the RFC 7591 `Server.Register` handler and its PHP-ported validation helpers +- `wristband/registration_test.go` — `TestPhase8RedRegistration` plus the in-memory `memoryBackend`/`memoryTx` and the full public/confidential/bounds/sweep/cap test matrix +- `wristband/server.go` — `Options` gains `DCRClientCap`/`DCRUnconsentedSweepAge`/`RegisterMaxBodyBytes`; `Server` gains `backend`/`now`/`randomBytes` and `SetBackend` +- `../fonoteka.go/plugins/golem15/fonoteka/updates/21_oauth_schema_correction.go` — the additive, refusing-rollback correction migration +- `../fonoteka.go/plugins/golem15/fonoteka/updates/oauth_schema_correction_test.go` — the package's own real-Postgres harness plus the RED/GREEN schema tests +- `../fonoteka.go/plugins/golem15/fonoteka/models/oauth_client.go`, `oauth_auth_code.go` — pointer-ified nullable fields +- `../fonoteka.go/plugins/golem15/fonoteka/classes/auth/oauth_store.go` — `OAuthStore`/`oauthTx` (the `wristband.Backend`/`wristband.Tx` GORM adapter) and record↔model conversions +- `../fonoteka.go/plugins/golem15/fonoteka/classes/auth/oauth_store_test.go` — `TestPhase8RedRegistrationStore`, `TestOAuthRegistrationStore`, `TestOAuthRegistrationCap` +- `../fonoteka.go/plugins/golem15/fonoteka/config/config.yaml` — `golem15.fonoteka.oauth.*` defaults (new file; the plugin had no config before this plan) +- `../fonoteka.go/plugins/golem15/fonoteka/plugin.go` — `pact.HasConfig`, config-driven `wristband.Options`, `SetBackend(auth.NewOAuthStore(gdb))` +- `../fonoteka.go/plugins/golem15/fonoteka/routes.go` — mounts `POST /oauth/mcp/register` with `throttle:fonoteka-oauth-register` +- `../fonoteka.go/plugins/golem15/fonoteka/oauth_registration_test.go` — `TestPhase8RedRegistrationApp`, `TestOAuthRegisterAssembled`, `TestOAuthRegisterOversizedAndWrongContentType`, `TestOAuthRawRegistrationSurface` + +## Decisions Made + +See frontmatter `key-decisions`. The most load-bearing one: the corrective migration file had to be renamed from the plan's suggested `12_` prefix to `21_` because Go's `init()` file-order registration — not the migration's own ID — determines gormigrate slice position, and `RollbackLast()` rolls back the slice's last element. `12_` before `20_remaining.go` meant `RollbackLast()` targeted the wrong migration; this was caught by `TestOAuthSchemaCorrection`'s rollback-refusal assertion failing with "rollback succeeded while null-dependent rows exist" during GREEN verification, fixed, and re-verified (Rule 1 — bug, self-caught before commit, not a deviation requiring separate documentation since it's the initial implementation, not a later fix). + +## Deviations from Plan + +**1. [Rule 1 - Bug] Migration filename changed from 12_ to 21_ for correct init-order/RollbackLast semantics** +- **Found during:** Task 1, before the GREEN commit (caught by `TestOAuthSchemaCorrection`'s own rollback assertion) +- **Issue:** Naming the file `12_oauth_schema_correction.go` placed its `init()`-time `Register()` call before `20_remaining.go`'s, making it not the last element of `updates.All()` despite having the numerically latest migration ID; `gormigrate.RollbackLast()` rolled back `20_remaining.go`'s last migration instead +- **Fix:** Renamed the file to `21_oauth_schema_correction.go`; 08-PATTERNS.md explicitly marks the filename as discretionary +- **Files modified:** `../fonoteka.go/plugins/golem15/fonoteka/updates/21_oauth_schema_correction.go` (created directly under this name; never committed as `12_`) +- **Verification:** `TestOAuthSchemaCorrection`'s rollback-refusal and rollback-success assertions both pass +- **Committed in:** `4536b3e` (Task 1 GREEN commit) + +--- + +**Total deviations:** 1 auto-fixed (1 bug) +**Impact on plan:** No scope change; purely a file-naming/ordering correction caught by the plan's own test before commit. + +## Issues Encountered + +None beyond the migration-ordering issue documented above. + +## User Setup Required + +None — no external service configuration required. + +## Next Phase Readiness + +- `wristband.Tx`'s full store bundle (including row-lock reads and `RevokeLineage`) is ready for 08-03 (authorize) and 08-04 (token exchange/refresh rotation) to consume without another interface change. +- `golem15.fonoteka.oauth.*` config already declares `pending_request_ttl_seconds`/`code_ttl_seconds`/`access_token_ttl_seconds`/`refresh_token_ttl_seconds`/`resource`; 08-03/08-04 need only read them, not add new keys. +- The `oauthClientRegistrationLockKey` advisory-lock pattern is available for any future atomic count-then-mutate need (e.g. a future per-user rate concern). +- D-17's expiry sweep (deleting expired pending/code/refresh rows) is explicitly deferred to 08-06 — `AuthCodeStore`/`RefreshTokenStore` do not yet have a `DeleteExpired`-shaped method; 08-06 should add it rather than assume it exists. +- No blockers. + +## Self-Check: PASSED + +- FOUND: wristband/stores.go, wristband/crypto.go, wristband/register.go, wristband/registration_test.go +- FOUND: ../fonoteka.go/plugins/golem15/fonoteka/updates/21_oauth_schema_correction.go, oauth_schema_correction_test.go +- FOUND: ../fonoteka.go/plugins/golem15/fonoteka/classes/auth/oauth_store.go, oauth_store_test.go +- FOUND: ../fonoteka.go/plugins/golem15/fonoteka/config/config.yaml, oauth_registration_test.go +- FOUND commits (fonoteka.go): 0b18d27, 4536b3e, fec99f0, 381de57, 5891c7c, ebdb249 +- FOUND commits (summercms.go): c026b83, c0b1e3c