fix(08): revise plans based on checker feedback
This commit is contained in:
@@ -5,55 +5,38 @@ type: execute
|
||||
wave: 3
|
||||
depends_on: [08-02]
|
||||
files_modified:
|
||||
- wristband/token.go
|
||||
- wristband/token_test.go
|
||||
- wristband/stores.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/controllers/api/connected_app_controller.go
|
||||
- ../fonoteka.go/plugins/golem15/fonoteka/controllers/api/connected_app_controller_test.go
|
||||
- ../fonoteka.go/plugins/golem15/fonoteka/config/config.yaml
|
||||
- ../fonoteka.go/config/app.yaml
|
||||
- ../fonoteka.go/plugins/golem15/fonoteka/plugin.go
|
||||
- ../fonoteka.go/plugins/golem15/fonoteka/routes.go
|
||||
- ../fonoteka.go/plugins/golem15/fonoteka/oauth_lifecycle_test.go
|
||||
- ../fonoteka.go/plugins/golem15/fonoteka/oauth_registration_test.go
|
||||
autonomous: true
|
||||
requirements: [AUTH-05, AUTH-06, AUTH-07]
|
||||
must_haves:
|
||||
truths:
|
||||
- "A valid refresh token rotates to a new access/refresh pair while revoking the prior access token."
|
||||
- "Replaying a spent refresh token commits revocation of the whole lineage, leaving no usable branch."
|
||||
- "A user sees only their live connected OAuth apps and can revoke one access token plus its refresh lineage atomically."
|
||||
- "D-03: The assembled app exposes configured PHP-default TTLs, caps, issuer, resource, consent URL, and registration bound."
|
||||
- "D-09: Metadata and registration are raw routes and registration alone carries its named throttle."
|
||||
- "D-10: No oauth guard is registered; OAuth access remains on inv_token."
|
||||
- "D-12: Backend challenge ownership stays unchanged and RFC 9728 behavior remains in fonoteka-mcp."
|
||||
artifacts:
|
||||
- path: "wristband/token.go"
|
||||
provides: "Refresh grant rotation, replay detection, and committed lineage kill"
|
||||
- path: "../fonoteka.go/plugins/golem15/fonoteka/classes/auth/oauth_store.go"
|
||||
provides: "Row-locked refresh traversal, revoke, and expiry sweep storage"
|
||||
- path: "../fonoteka.go/plugins/golem15/fonoteka/controllers/api/connected_app_controller.go"
|
||||
provides: "Owner-scoped connected-app list and revoke handlers"
|
||||
- path: "../fonoteka.go/plugins/golem15/fonoteka/plugin.go"
|
||||
provides: "Configured store-backed wristband server construction"
|
||||
- path: "../fonoteka.go/plugins/golem15/fonoteka/routes.go"
|
||||
provides: "Raw metadata and register route mounting"
|
||||
key_links:
|
||||
- from: "wristband/token.go"
|
||||
to: "oauth_store.go"
|
||||
via: "transaction outcome commits lineage kill before returning invalid_grant"
|
||||
pattern: "WithinTx"
|
||||
- from: "connected_app_controller.go"
|
||||
to: "wristband.Server"
|
||||
via: "cascade revoke operation rather than direct refresh-row deletion"
|
||||
pattern: "Revoke"
|
||||
- from: "connected_app_controller.go"
|
||||
to: "token_api_controller.go"
|
||||
via: "reuse of positive allow-list token serializer"
|
||||
pattern: "serializeToken"
|
||||
- from: "plugin.go"
|
||||
to: "wristband.New"
|
||||
via: "configured Options and GORM backend"
|
||||
pattern: "wristband\\.New"
|
||||
---
|
||||
|
||||
<objective>
|
||||
Deliver the durable OAuth lifecycle slice: safe refresh rotation/replay handling and user-visible connected-app listing/revocation.
|
||||
Mount the proven discovery/DCR engine on the real application with persistent state and exact raw-route isolation.
|
||||
|
||||
Purpose: Ensure stolen or replayed refresh tokens cannot create surviving branches and the unchanged Settings UI controls the same grant lineage.
|
||||
Output: Refresh-grant state machine, row-locked store operations, connected-app controllers/routes, and concurrency-backed lifecycle tests.
|
||||
Purpose: Deliver the first connector-visible vertical outcome without mixing schema work into protocol implementation.
|
||||
Output: OAuth config, boot wiring, raw routes, and assembled Postgres-backed tests.
|
||||
</objective>
|
||||
|
||||
## Phase Goal
|
||||
|
||||
**As a** connected-app user, **I want to** refresh access safely and revoke applications from Settings, **so that** replayed or revoked credentials immediately lose access.
|
||||
|
||||
<execution_context>
|
||||
@/home/jin/.codex/get-shit-done/workflows/execute-plan.md
|
||||
@/home/jin/.codex/get-shit-done/templates/summary.md
|
||||
@@ -64,115 +47,39 @@ Output: Refresh-grant state machine, row-locked store operations, connected-app
|
||||
@.planning/ROADMAP.md
|
||||
@.planning/STATE.md
|
||||
@.planning/phases/08-oauth2-1-authorization-server/08-CONTEXT.md
|
||||
@.planning/phases/08-oauth2-1-authorization-server/08-RESEARCH.md
|
||||
@.planning/phases/08-oauth2-1-authorization-server/08-PATTERNS.md
|
||||
@.planning/phases/08-oauth2-1-authorization-server/08-UI-SPEC.md
|
||||
@.planning/phases/08-oauth2-1-authorization-server/08-02-SUMMARY.md
|
||||
|
||||
<interfaces>
|
||||
From Plan 08-02:
|
||||
- Token handler already dispatches `authorization_code` and recognizes the configured refresh grant.
|
||||
- `wristband.Tx` provides row-lock-capable refresh/code stores and the transaction-bound access-token issuer.
|
||||
- OAuth access tokens are `models.ApiToken` rows distinguished by non-null `OAuthClientID`.
|
||||
|
||||
Existing serializer:
|
||||
- `serializeToken(gdb, *models.ApiToken) map[string]any` is the required positive allow-list for connected-app responses.
|
||||
</interfaces>
|
||||
</context>
|
||||
|
||||
<tasks>
|
||||
|
||||
<task type="auto" tdd="true">
|
||||
<name>Task 1: Specify rotation, replay, list, and revoke as one lifecycle</name>
|
||||
<files>wristband/token_test.go, ../fonoteka.go/plugins/golem15/fonoteka/classes/auth/oauth_store_test.go, ../fonoteka.go/plugins/golem15/fonoteka/controllers/api/connected_app_controller_test.go, ../fonoteka.go/plugins/golem15/fonoteka/oauth_lifecycle_test.go</files>
|
||||
<read_first>
|
||||
.planning/phases/08-oauth2-1-authorization-server/08-UI-SPEC.md
|
||||
.planning/phases/08-oauth2-1-authorization-server/08-VALIDATION.md
|
||||
wristband/token.go
|
||||
wristband/stores.go
|
||||
../fonoteka.go/plugins/golem15/fonoteka/classes/auth/oauth_store.go
|
||||
../fonoteka.go/plugins/golem15/fonoteka/controllers/api/token_api_controller.go
|
||||
/media/nvme/dev/golem15/fonoteka/plugins/golem15/fonoteka/classes/auth/OAuthCodeManager.php
|
||||
/media/nvme/dev/golem15/fonoteka/plugins/golem15/fonoteka/controllers/api/ConnectedAppController.php
|
||||
/media/nvme/dev/golem15/fonoteka/plugins/golem15/fonoteka/tests/security/OAuthRefreshRotationTest.php
|
||||
/media/nvme/dev/golem15/fonoteka/plugins/golem15/fonoteka/tests/security/OAuthRevocationTest.php
|
||||
</read_first>
|
||||
<name>Task 1: Specify assembled discovery and registration in RED</name>
|
||||
<files>../fonoteka.go/plugins/golem15/fonoteka/oauth_registration_test.go</files>
|
||||
<behavior>
|
||||
- Normal refresh revokes the old access token, marks predecessor `rotated_to_id`, and returns a new same-scope/same-collection pair.
|
||||
- Sequential or concurrent spent-token replay returns `invalid_grant` only after the complete lineage and current access token are durably revoked.
|
||||
- Expiry sweep removes only expired pending/code/refresh rows and retains unexpired rotated/revoked refresh rows as replay evidence.
|
||||
- Connected-app list is newest-first, owner-only, live OAuth tokens only, with manual count separate and no secret/client-id fields.
|
||||
- Revoke of an owned OAuth token kills its refresh lineage; foreign, missing, and manual token IDs share the exact 404.
|
||||
- Assembled routes return exact metadata and persistent public/confidential DCR responses.
|
||||
- Route table rejects JWT, inv_token, inv.scope, body-limit, and house middleware; register has only its named throttle.
|
||||
- Failures use `PHASE8_RED:registration-app`, not compile/setup/missing-test failure.
|
||||
</behavior>
|
||||
<action>Per D-04, D-08, D-16, D-17, and D-18, extend the RED suite with deterministic in-memory tests and synchronized real-Postgres contention tests for T-08-REFRESH-REPLAY, T-08-CROSS-USER, T-08-SCOPE-CEILING, T-08-REQUEST-LEAK, and T-08-SURFACE. Include an assembled lifecycle that starts with a grant from 08-02, refreshes, replays the spent predecessor, verifies the new branch and access token are dead, creates another grant, lists it, revokes it, and proves refresh afterward fails. Assert exact UI response allow-lists and 404 bytes.</action>
|
||||
<action>D-18: add an assembled-router real-Postgres test against existing boot seams. Assert bytes and headers before decoding, exact configured defaults, route isolation, and no oauth guard. Keep the test compiling against 08-01/08-02 contracts and mark only absent app wiring with `PHASE8_RED:registration-app`.</action>
|
||||
<verify>
|
||||
<automated>rg -n 'TestOAuth(Refresh|Replay|Connected|Revoke|Sweep)' wristband/token_test.go ../fonoteka.go/plugins/golem15/fonoteka/{oauth_lifecycle_test.go,classes/auth/oauth_store_test.go,controllers/api/connected_app_controller_test.go}</automated>
|
||||
<automated>scripts/check-phase8-red.sh registration-app bash -lc "cd ../fonoteka.go && go test ./plugins/golem15/fonoteka/... -run 'TestOAuth(Metadata|Register|RawRoute|Config)' -count=1"</automated>
|
||||
</verify>
|
||||
<acceptance_criteria>
|
||||
- Tests include sequential replay, a barrier-synchronized double refresh, committed lineage kill, expiry retention, owner isolation, manual-token exclusion, list ordering, and post-revoke refresh failure.
|
||||
- The replay test explicitly reloads database rows after the `invalid_grant` response and asserts revoked lineage/access state, preventing rollback-hidden false positives.
|
||||
- Tests fail on missing refresh/connected-app implementation while all 08-02 happy-path tests remain green.
|
||||
</acceptance_criteria>
|
||||
<done>The RED lifecycle suite detects branch survival, rollback of replay revocation, ownership leaks, serialization leaks, and route misplacement.</done>
|
||||
<done>Assembled tests execute and fail solely because config/boot/routes are not wired.</done>
|
||||
</task>
|
||||
|
||||
<task type="auto" tdd="true">
|
||||
<name>Task 2: Implement refresh rotation, committed replay kill, and exact sweeps</name>
|
||||
<files>wristband/token.go, wristband/stores.go, ../fonoteka.go/plugins/golem15/fonoteka/classes/auth/oauth_store.go, ../fonoteka.go/plugins/golem15/fonoteka/classes/auth/oauth_store_test.go</files>
|
||||
<read_first>
|
||||
wristband/token_test.go
|
||||
wristband/token.go
|
||||
wristband/stores.go
|
||||
../fonoteka.go/plugins/golem15/fonoteka/classes/auth/oauth_store_test.go
|
||||
../fonoteka.go/plugins/golem15/fonoteka/classes/auth/oauth_store.go
|
||||
../fonoteka.go/plugins/golem15/fonoteka/models/oauth_refresh_token.go
|
||||
../fonoteka.go/plugins/golem15/fonoteka/models/api_token.go
|
||||
</read_first>
|
||||
<name>Task 2: Configure, boot, and route persistent discovery and DCR</name>
|
||||
<files>../fonoteka.go/plugins/golem15/fonoteka/config/config.yaml, ../fonoteka.go/config/app.yaml, ../fonoteka.go/plugins/golem15/fonoteka/plugin.go, ../fonoteka.go/plugins/golem15/fonoteka/routes.go, ../fonoteka.go/plugins/golem15/fonoteka/oauth_registration_test.go</files>
|
||||
<behavior>
|
||||
- Presented refresh lookup uses SHA-256 hash and a row lock inside the single transaction boundary.
|
||||
- Replay revocation returns success from the transaction callback, then maps the recorded outcome to `invalid_grant` outside it.
|
||||
- Rotation keeps predecessor/successor relationships and unexpired evidence rows.
|
||||
- Defaults are pending/code 600s, access 3600s, refresh 30 days, DCR cap 200, stale age 24h, resource URL, and 65,536-byte register maximum.
|
||||
- Issuer trims the app URL once; metadata and registration use the actual GORM backend.
|
||||
</behavior>
|
||||
<action>Implement D-04, D-05, D-07, and D-17's refresh branch in wristband and the GORM adapter. Authenticate the client using the same Basic-over-form rule, hash the presented refresh secret, lock its row, reject expired/revoked/wrong-client grants, and rotate atomically by revoking the old access token, minting/persisting its successor, creating the next refresh secret/hash, and linking `rotated_to_id`. If a spent token is presented, traverse and revoke the whole lineage and associated access tokens, return nil from the transaction so the kill commits, then return `invalid_grant` from the handler. Keep the old scopes, collection IDs, offline flag, and client binding. Sweep only rows whose `expires_at` is past; do not delete unexpired replay evidence.</action>
|
||||
<action>D-03: add `plugins.golem15.fonoteka.oauth.*` defaults and use `app.url` as issuer. Construct the backend and wristband server in Plugin.Boot and retain it for later route/command factories. D-09: mount metadata and register inside the existing raw group; pass `throttle:fonoteka-oauth-register` only to register. D-10: register no oauth guard. D-12: preserve the exact backend personal-token 401 and do not add protected-resource metadata or rich Bearer challenges.</action>
|
||||
<verify>
|
||||
<automated>go test ./wristband -run 'Test(Refresh|Replay|Sweep)' -count=1 && cd ../fonoteka.go && go test ./plugins/golem15/fonoteka/classes/auth -run 'TestOAuth(Refresh|Replay|Sweep)' -count=1</automated>
|
||||
<automated>cd ../fonoteka.go && go test ./plugins/golem15/fonoteka/... -run 'TestOAuth(Metadata|Register|RawRoute|Config)' -count=1</automated>
|
||||
</verify>
|
||||
<acceptance_criteria>
|
||||
- Normal refresh and sequential/concurrent replay tests pass under real Postgres.
|
||||
- A spent-token replay leaves every lineage refresh row and its live access token revoked after the response transaction commits.
|
||||
- `go test -race ./wristband` passes and the app contention test produces a single usable branch.
|
||||
- Sweep tests prove expired rows are removed and unexpired rotated/revoked rows remain.
|
||||
</acceptance_criteria>
|
||||
<done>Refresh rotation is atomic, preserves replay evidence, and commits whole-lineage revocation before emitting the protocol error.</done>
|
||||
</task>
|
||||
|
||||
<task type="auto" tdd="true">
|
||||
<name>Task 3: Expose connected-app list and atomic revoke to the unchanged Settings UI</name>
|
||||
<files>../fonoteka.go/plugins/golem15/fonoteka/controllers/api/connected_app_controller.go, ../fonoteka.go/plugins/golem15/fonoteka/controllers/api/connected_app_controller_test.go, ../fonoteka.go/plugins/golem15/fonoteka/routes.go, ../fonoteka.go/plugins/golem15/fonoteka/oauth_lifecycle_test.go</files>
|
||||
<read_first>
|
||||
../fonoteka.go/plugins/golem15/fonoteka/controllers/api/connected_app_controller_test.go
|
||||
../fonoteka.go/plugins/golem15/fonoteka/oauth_lifecycle_test.go
|
||||
../fonoteka.go/plugins/golem15/fonoteka/controllers/api/token_api_controller.go
|
||||
../fonoteka.go/plugins/golem15/fonoteka/routes.go
|
||||
/media/nvme/dev/golem15/fonoteka/vue-fonoteka-app/app/components/settings/ConnectedAppsManager.vue
|
||||
/media/nvme/dev/golem15/fonoteka/vue-fonoteka-app/app/stores/fonoteka.ts
|
||||
</read_first>
|
||||
<behavior>
|
||||
- List returns `data` and numeric `manual_tokens_count`, filters to owner/live/OAuth tokens, and orders created_at descending.
|
||||
- Every row is `serializeToken` plus sanitized client name and contains no raw credential, hash, OAuth client id, redirect URI, or other-user data.
|
||||
- Revoke completes access-token and refresh-lineage revocation before returning `{"data":{"revoked":true}}`.
|
||||
</behavior>
|
||||
<action>Per D-08 and the UI-SPEC, add GET and DELETE connected-app controllers in the JWT group. Reuse `serializeToken`; append only the sanitized/truncated client name, initialize collection/scope arrays as arrays, count live manual tokens separately, and order OAuth tokens newest first. Scope every query by `bouncer.User`. For DELETE, require an owned OAuth token, invoke the wristband lineage-revoke operation in the same committed transaction, and collapse missing/foreign/manual IDs to exact `{"error":"Token not found"}` 404. Mount only under `/_fonoteka/api/v1/oauth`; do not expose these routes on the personal-token or raw groups.</action>
|
||||
<verify>
|
||||
<automated>cd ../fonoteka.go && go test ./plugins/golem15/fonoteka/... -run 'TestOAuth(ConnectedApps|Revoke|Lifecycle|Surface)' -count=1</automated>
|
||||
</verify>
|
||||
<acceptance_criteria>
|
||||
- Empty, populated, manual-count, newest-first, foreign/manual 404, and successful atomic revoke tests pass with exact bytes.
|
||||
- The lifecycle test proves the revoked app disappears on the next list and its refresh token returns `invalid_grant`.
|
||||
- JSON assertions reject `token`, `token_hash`, `oauth_client_id`, `client_secret`, `refresh_token`, `request_id`, and `redirect_uris` anywhere in list output.
|
||||
- The Nuxt repository remains unchanged.
|
||||
</acceptance_criteria>
|
||||
<done>The existing Settings → Integrations UI can list and revoke only the current user's connected applications, and revoke kills the entire grant lineage.</done>
|
||||
<done>An unchanged connector can discover and dynamically register against the assembled app with persistent Postgres state and exact route boundaries.</done>
|
||||
</task>
|
||||
|
||||
</tasks>
|
||||
@@ -182,32 +89,26 @@ Existing serializer:
|
||||
|
||||
| Boundary | Description |
|
||||
|----------|-------------|
|
||||
| Refresh credential → token endpoint | A bearer-like long-lived credential requests a new grant branch. |
|
||||
| JWT principal → connected-app API | User-controlled ids request listing/revocation of durable credentials. |
|
||||
| Transaction outcome → OAuth error | Security revocation must commit even though the protocol response is an error. |
|
||||
| Internet → raw routes | Unauthenticated protocol traffic enters the assembled app. |
|
||||
| Config → public metadata | Deployment values become client trust anchors. |
|
||||
|
||||
## STRIDE Threat Register
|
||||
|
||||
| Threat ID | Category | Component | Disposition | Mitigation Plan |
|
||||
|-----------|----------|-----------|-------------|-----------------|
|
||||
| T-08-REFRESH-REPLAY | Spoofing/Elevation | refresh state machine | mitigate | Row lock, rotation chain, single-winner concurrency, commit lineage kill before `invalid_grant`. |
|
||||
| T-08-CROSS-USER | Elevation | connected-app controllers | mitigate | Owner-scoped reads/deletes and indistinguishable missing/foreign/manual 404. |
|
||||
| T-08-SCOPE-CEILING | Elevation | refresh rotation | mitigate | Copy only stored granted scopes/collection ids; refresh cannot add request-provided authority. |
|
||||
| T-08-REQUEST-LEAK | Information Disclosure | list response/logs | mitigate | Positive allow-list serializer and explicit forbidden-field tests. |
|
||||
| T-08-SURFACE | Elevation | route groups | mitigate | JWT-only management route inspection; token endpoint remains raw. |
|
||||
| T-08-SC | Tampering | dependencies | mitigate | No install; standard library and existing GORM only. |
|
||||
| T-08-DCR-FLOOD | Denial of Service | register route | mitigate | Named per-IP limiter plus framework body/cap controls. |
|
||||
| T-08-SURFACE | Elevation | route groups | mitigate | Assembled route-table test for exact middleware. |
|
||||
| T-08-SC | Tampering | dependencies | mitigate | No new package. |
|
||||
</threat_model>
|
||||
|
||||
<verification>
|
||||
- `go test ./wristband -run 'Test(Refresh|Replay|Sweep)' -count=1`
|
||||
- `cd ../fonoteka.go && go test ./plugins/golem15/fonoteka/... -run 'TestOAuth(Refresh|Replay|ConnectedApps|Revoke|Lifecycle|Surface)' -count=1`
|
||||
- `cd ../fonoteka.go && go test -race ./plugins/golem15/fonoteka/classes/auth ./plugins/golem15/fonoteka`
|
||||
- Focused assembled discovery/DCR tests pass.
|
||||
- `go vet ./... && go test ./...` passes in both repositories at the wave boundary.
|
||||
</verification>
|
||||
|
||||
<success_criteria>
|
||||
- Refresh rotation has exactly one usable successor and replay durably kills the entire lineage.
|
||||
- Connected-app output matches the UI contract and cannot disclose secrets or other users.
|
||||
- Revocation removes the app from the list and invalidates both access and refresh credentials before success is returned.
|
||||
- Metadata and DCR are reachable through the real app with exact PHP-compatible responses.
|
||||
- Public/confidential clients persist and raw routes remain isolated.
|
||||
</success_criteria>
|
||||
|
||||
<output>
|
||||
|
||||
Reference in New Issue
Block a user