fix(08): revise oauth plans after checker feedback

This commit is contained in:
Jakub Zych
2026-09-23 17:46:38 +02:00
parent 3c6a505c5f
commit e23cbac240
13 changed files with 444 additions and 197 deletions

View File

@@ -5,9 +5,7 @@ type: execute
wave: 4
depends_on: [08-03]
files_modified:
- wristband/authorize.go
- wristband/token.go
- wristband/authorize_test.go
- wristband/token_test.go
- ../fonoteka.go/plugins/golem15/fonoteka/classes/auth/oauth_store.go
- ../fonoteka.go/plugins/golem15/fonoteka/classes/auth/oauth_token_issuer.go
@@ -16,13 +14,11 @@ autonomous: true
requirements: [AUTH-05, AUTH-06]
must_haves:
truths:
- "D-02: Authorize reads query only and token rejects JSON before ParseForm body-over-query parsing."
- "D-02: Token rejects JSON before ParseForm body-over-query parsing."
- "D-04: S256/client-secret comparisons are constant-time and code replay has one winner."
- "D-05: Wristband owns authorize, PKCE, scope/resource policy, and atomic code exchange."
- "D-05: Wristband owns client authentication, PKCE verification, and atomic code exchange."
- "D-11: Issued access tokens retain the configured inv_ prefix."
artifacts:
- path: "wristband/authorize.go"
provides: "Ordered validation, redirects, PKCE, resource and scope policy"
- path: "wristband/token.go"
provides: "Client authentication and atomic authorization-code exchange"
key_links:
@@ -33,10 +29,10 @@ must_haves:
---
<objective>
Implement the protocol core from authorize validation through one atomic authorization-code exchange.
Complete one atomic authorization-code exchange for the assembled pending-request slice from 08-03.
Purpose: Prove PKCE, redirect, parser, scope, client-auth, and code-replay rules independently of the browser controller.
Output: Authorize/token handlers, issuer adapter, store transitions, and deterministic/concurrent tests.
Purpose: Prove token parsing, client authentication, constant-time PKCE, issuance, and code-replay rules before browser consent wiring.
Output: Token handler, issuer adapter, locked store transition, and deterministic/concurrent tests.
</objective>
<execution_context>
@@ -56,31 +52,62 @@ Output: Authorize/token handlers, issuer adapter, store transitions, and determi
<tasks>
<task type="auto" tdd="true">
<name>Task 1: Specify authorize and code exchange with executable RED tests</name>
<files>wristband/authorize.go, wristband/token.go, wristband/authorize_test.go, wristband/token_test.go, ../fonoteka.go/plugins/golem15/fonoteka/classes/auth/oauth_store.go, ../fonoteka.go/plugins/golem15/fonoteka/classes/auth/oauth_token_issuer.go</files>
<name>Task 1: Specify atomic code exchange with executable RED tests</name>
<files>wristband/token.go, wristband/token_test.go, ../fonoteka.go/plugins/golem15/fonoteka/classes/auth/oauth_store.go, ../fonoteka.go/plugins/golem15/fonoteka/classes/auth/oauth_token_issuer.go</files>
<read_first>
.planning/phases/08-oauth2-1-authorization-server/08-CONTEXT.md
.planning/phases/08-oauth2-1-authorization-server/08-PATTERNS.md
wristband/authorize.go
wristband/stores.go
../fonoteka.go/plugins/golem15/fonoteka/classes/auth/api_token_manager.go
../fonoteka.go/plugins/golem15/fonoteka/classes/auth/token_guard.go
/media/nvme/dev/golem15/fonoteka/plugins/golem15/fonoteka/controllers/api/OAuthTokenController.php
/media/nvme/dev/golem15/fonoteka/plugins/golem15/fonoteka/classes/auth/OAuthCodeManager.php
/media/nvme/dev/golem15/fonoteka/plugins/golem15/fonoteka/tests/functional/OAuthTokenTest.php
</read_first>
<behavior>
- Unknown client/unregistered redirect are local 400 without Location; later errors use exact ordered RFC3986 redirects.
- S256 is mandatory; parser precedence, Basic override, cache headers, and exact challenge are tested.
- Tests compile and fail only through `PHASE8_RED:authorize-token`.
- JSON rejection, ParseForm body-over-query precedence, Basic override, cache headers, exact challenge, and grant dispatch are tested.
- S256 code/client/redirect/resource binding and synchronized one-winner replay are tested.
- `TestPhase8RedCodeExchange` is the sole selected failure with `PHASE8_RED:code-exchange`.
</behavior>
<action>D-18: extend the existing interfaces with compiling authorize/token stubs and add deterministic unit plus real-store adapter tests. Use explicit `PHASE8_RED:authorize-token` assertions for absent behavior. Reject syntax/build/setup/missing-test failures through the shared RED verifier. Cover T-08-PKCE, CODE-REPLAY, OPEN-REDIRECT, SECRET-TIMING, SCOPE-CEILING, and REQUEST-LEAK, including synchronized concurrent code exchange.</action>
<action>D-18: extend the existing interfaces with compiling token stubs and add deterministic unit plus real-store adapter tests. Validate exact package/test/sentinel through `check-phase8-red.sh go` and `go test -json`; cover T-08-PKCE, CODE-REPLAY, SECRET-TIMING, SCOPE-CEILING, and REQUEST-LEAK, including synchronized concurrent exchange. Every unexpected failing JSON action/package/test, build/setup/panic/no-test event, or missing/duplicate sentinel must fail the RED command.</action>
<verify>
<automated>scripts/check-phase8-red.sh authorize-token go test ./wristband -run 'Test(Authorize|Token|PKCE|Code)' -count=1</automated>
<automated>scripts/check-phase8-red.sh go PHASE8_RED:code-exchange git.golem15.com/golem15/summercms/wristband TestPhase8RedCodeExchange -- go test -json ./wristband -run '^TestPhase8RedCodeExchange$' -count=1</automated>
</verify>
<done>Named tests compile and execute, with the verifier accepting only the intended missing-behavior marker.</done>
<acceptance_criteria>
- The selected RED test compiles/runs and is the only test fail event; its package is the only package fail event and emits the exact sentinel once.
- Tests assert exact status/body/no-newline, `Cache-Control: no-store`, `Pragma: no-cache`, and `Basic realm="OAuth"` only for invalid confidential client.
- Synchronized exchange proves exactly one success and one `invalid_grant`; syntax/setup/panic/no-test or any unrelated failure is rejected.
</acceptance_criteria>
<done>The fail-closed RED suite specifies the full code-exchange contract without re-owning authorize behavior.</done>
</task>
<task type="auto" tdd="true">
<name>Task 2: Implement ordered authorize and atomic code exchange</name>
<files>wristband/authorize.go, wristband/token.go, wristband/authorize_test.go, wristband/token_test.go, ../fonoteka.go/plugins/golem15/fonoteka/classes/auth/oauth_store.go, ../fonoteka.go/plugins/golem15/fonoteka/classes/auth/oauth_token_issuer.go, ../fonoteka.go/plugins/golem15/fonoteka/classes/auth/api_token_manager.go</files>
<name>Task 2: Implement atomic PKCE-bound code exchange</name>
<files>wristband/token.go, wristband/token_test.go, ../fonoteka.go/plugins/golem15/fonoteka/classes/auth/oauth_store.go, ../fonoteka.go/plugins/golem15/fonoteka/classes/auth/oauth_token_issuer.go, ../fonoteka.go/plugins/golem15/fonoteka/classes/auth/api_token_manager.go</files>
<read_first>
wristband/token_test.go
wristband/token.go
wristband/crypto.go
../fonoteka.go/plugins/golem15/fonoteka/classes/auth/oauth_store.go
../fonoteka.go/plugins/golem15/fonoteka/classes/auth/api_token_manager.go
../fonoteka.go/plugins/golem15/fonoteka/models/api_token.go
/media/nvme/dev/golem15/fonoteka/plugins/golem15/fonoteka/controllers/api/OAuthTokenController.php
/media/nvme/dev/golem15/fonoteka/plugins/golem15/fonoteka/classes/auth/OAuthCodeManager.php
</read_first>
<behavior>
- Validation order is usable client, exact redirect, response type, S256 method/challenge, scope ceiling, resource, pending creation.
- Code lock/consume, access-token mint/stamp, and refresh creation commit atomically once.
- Token parsing/authentication precedes locked code lookup; code/client/redirect/resource/verifier bindings all match.
- Code lock/consume, `inv_` access-token mint/stamp, and refresh creation commit atomically once.
</behavior>
<action>D-02: implement endpoint-specific parsing and reject JSON token calls before ParseForm. D-03: apply configured TTL/resource. D-04: compare fixed transforms with `subtle.ConstantTimeCompare`. D-05 and D-06: keep state/policy and exact raw responses in wristband. D-07: exchange under one app transaction/row lock. Build redirects from ordered pairs, never `url.Values.Encode`. D-11: make the app adapter prefix config-backed while retaining `inv_`. D-17: run expired-only sweep on token entry. Preserve exact `Basic realm="OAuth"`, no-store, and no-cache headers.</action>
<action>D-02: reject JSON before ParseForm, use body-over-query form values, and let Basic credentials override form credentials. D-03: apply configured code/access/refresh TTL and resource. D-04: compare fixed transforms with `subtle.ConstantTimeCompare`. D-05/D-06: keep grant policy and exact raw responses in wristband. D-07: lock/consume/mint/stamp/create refresh within one callback transaction. D-11: make the app issuer prefix config-backed while retaining exact `inv_`. D-17: run expired-only sweep on token entry. Preserve exact Basic challenge and cache headers.</action>
<verify>
<automated>go test ./wristband -run 'Test(Authorize|Token|PKCE|Code)' -count=1 &amp;&amp; cd ../fonoteka.go &amp;&amp; go test ./plugins/golem15/fonoteka/classes/auth -run 'TestOAuth(Code|Issuer)' -count=1</automated>
<automated>go test ./wristband -run '^Test(Token|PKCE|Code)' -count=1 &amp;&amp; (cd ../fonoteka.go &amp;&amp; go test ./plugins/golem15/fonoteka/classes/auth -run '^TestOAuth(Code|Issuer)' -count=1)</automated>
</verify>
<acceptance_criteria>
- Exact parser cases cover JSON rejection, body-over-query values, Basic-over-form credentials, unsupported/missing grant, public/confidential authentication, and challenge/no-challenge branches.
- Correct verifier exchanges once for an `inv_` access token and refresh token with configured lifetimes; wrong verifier/client/redirect/resource and repeated code return exact native errors without minting.
- Real-Postgres synchronized exchange has exactly one winner; persisted code is consumed and token/client/collection/scope bindings are correct, with no raw code/secret/verifier logged or stored.
</acceptance_criteria>
<done>One exact PKCE-bound code produces one inv_ access/refresh grant, and all validation/parser/replay failures are exact and tested.</done>
</task>
@@ -108,7 +135,7 @@ Output: Authorize/token handlers, issuer adapter, store transitions, and determi
<verification>
- Focused wristband and issuer/store tests pass.
- `go test -race ./wristband` passes at wave boundary.
- Complete race execution is reserved exclusively for Plan 08-10's final blocking checkpoint.
</verification>
<success_criteria>