Files
summercms/.planning/phases/08-oauth2-1-authorization-server/08-DISCUSSION-LOG.md
2026-09-23 11:37:13 +02:00

186 lines
8.0 KiB
Markdown

# Phase 8: OAuth2.1 authorization server - Discussion Log
> **Audit trail only.** Do not use as input to planning, research, or execution agents.
> Decisions are captured in CONTEXT.md — this log preserves the alternatives considered.
**Date:** 2026-09-23
**Phase:** 08-oauth2-1-authorization-server
**Areas discussed:** Server engine, Package home and the oauth guard, End-to-end proof and the header contract, Parity corpus and lifecycle coverage
---
## Server engine
| Option | Description | Selected |
|--------|-------------|----------|
| Direct port on stdlib | Port OAuthCodeManager and the four controllers on crypto/rand, sha256, subtle, net/url; decision note drops zitadel/oidc | ✓ |
| zitadel/oidc with overrides | Implement AuthStorage/OPStorage and override metadata, error and token handlers to reach parity | |
| zitadel/oidc only where it fits | Use it for PKCE and request validation only | |
**User's choice:** Direct port on stdlib.
| Option | Description | Selected |
|--------|-------------|----------|
| Form body plus query, like PHP | r.FormValue merges urlencoded body and query; JSON on /token is invalid_request; register stays JSON-only | ✓ |
| Strict form body only | Ignore the query string | |
| Match Laravel fully including JSON | Also accept application/json on /token | |
**User's choice:** Form body plus query.
| Option | Description | Selected |
|--------|-------------|----------|
| Config keys with PHP defaults | TTLs and caps under plugin config with PHP values; issuer from app.url, resource from fonoteka.mcp.resource | ✓ |
| Constants exactly as PHP | Go constants in the code manager | |
| You decide | | |
**User's choice:** Config keys with PHP defaults.
| Option | Description | Selected |
|--------|-------------|----------|
| Yes, threat model plus review | T-08-xx threats per plan and a closing 08-SECURITY-REVIEW.md mapping threats to tests | ✓ |
| Review only | Security-review agent at the end, no threat IDs | |
| You decide | | |
**User's choice:** Threat model plus review.
---
## Package home and the oauth guard
| Option | Description | Selected |
|--------|-------------|----------|
| Plugin owns it, framework gets primitives | Server in fonoteka.go, framework gains PKCE/base64url/compare/form helpers | |
| Framework oauth package with storage interfaces | Generic RFC 6749/7591/8414 server in summercms.go; fonoteka plugs its models in | ✓ |
| Everything in the plugin | No framework additions | |
**User's choice:** Framework oauth package with storage interfaces (against the recommendation).
| Option | Description | Selected |
|--------|-------------|----------|
| New package, PHP shapes are the defaults | New package beside bouncer emitting PHP's RFC-minimal bodies; metadata, scopes, paths, TTLs are config | ✓ |
| Inside bouncer | Extend bouncer | |
| New package with response hooks | Generic shapes plus app override hooks | |
**User's choice:** New package with PHP shapes as defaults; user asked for name suggestions before creation.
| Option | Description | Selected |
|--------|-------------|----------|
| wristband | Festival wristband, checked by the bouncer, issued at the gate after approval | ✓ |
| visa | Permission an outside party applies for and a consenting authority stamps | |
| lanyard | Delegated credential naming issuer and grant | |
**User's choice:** wristband.
| Option | Description | Selected |
|--------|-------------|----------|
| Wristband owns RFC surface, app owns consent and tokens | Stores and AccessTokenIssuer implemented by the app; consent, connected apps, /connect URL, collection ids stay in the app | ✓ |
| Wristband also owns consent and connected apps | More interfaces, fuller reuse | |
| You decide | | |
**User's choice:** Wristband owns the RFC surface, app owns consent and tokens.
| Option | Description | Selected |
|--------|-------------|----------|
| Retire it, document why | No oauth guard; inv_token guard authenticates OAuth-issued tokens | ✓ |
| Register oauth as a restricted alias | Guard accepting only inv_ tokens with oauth_client_id, unmounted | |
| You decide | | |
**User's choice:** Retire it. **Notes:** the user added that `inv_` is a legacy prefix from the Inventory app Płytarium was forked from and could become summer-themed.
| Option | Description | Selected |
|--------|-------------|----------|
| Keep inv_ for Płytarium, prefix becomes app config | Phase 7 token manager reads the prefix from config; summer-themed default deferred | ✓ |
| Keep inv_ hardcoded, note the rename | No code change | |
| Pick the new prefix now as the framework default | Choose it in this discussion | |
**User's choice:** Keep inv_ for Płytarium, prefix becomes app config.
---
## End-to-end proof and the header contract
| Option | Description | Selected |
|--------|-------------|----------|
| Correct the roadmap, prove the real chain | SC3 reworded; backend emits only Basic realm="OAuth" on invalid_client; no new headers on backend 401s | ✓ |
| Add RFC 6750 headers to backend 401s | Contract deviation with a harness allow-list | |
| Keep the wording, satisfy it via the MCP | | |
**User's choice:** Correct the roadmap, prove the real chain.
| Option | Description | Selected |
|--------|-------------|----------|
| Both: tide replay in go test, live MCP in a gate script | Recorded flows in go test plus check-phase8.sh running the real Node fonoteka-mcp against the Go backend | ✓ |
| Tide replay only | | |
| Live MCP only, in go test | | |
**User's choice:** Both.
| Option | Description | Selected |
|--------|-------------|----------|
| Manual UAT step with Claude, ChatGPT if available | | |
| Automated gates only | First vendor connect at cutover | ✓ |
| Required before phase close | | |
**User's choice:** Automated gates only.
---
## Parity corpus and lifecycle coverage
| Option | Description | Selected |
|--------|-------------|----------|
| Record a second PHP flow covering the full lifecycle | mcp-lifecycle: DCR, authorize, consent, token, refresh, replay kill, connected-apps, revoke, refresh-after-revoke, deny, ceiling client with client_secret_basic | ✓ |
| Existing fixtures plus Go-only tests | | |
| You decide | | |
**User's choice:** Record a second PHP flow.
| Option | Description | Selected |
|--------|-------------|----------|
| Match PHP now, Phase 11 job later | No cleanup this phase | |
| Add an expiry sweep now | Additive cleanup of expired rows | ✓ |
| You decide | | |
**User's choice:** Add an expiry sweep now.
| Option | Description | Selected |
|--------|-------------|----------|
| On /register and /token, expired rows only | Delete only rows past expires_at; revoked or rotated unexpired rows stay; no timer | ✓ |
| Background ticker in plugin Boot | | |
| You decide | | |
**User's choice:** On /register and /token, expired rows only.
| Option | Description | Selected |
|--------|-------------|----------|
| Port all as Go tests, split framework vs app | Wristband tests on in-memory store; app tests on Postgres; each PHP method maps to a named Go test | ✓ |
| Port security classes only | | |
| You decide | | |
**User's choice:** Port all as Go tests.
| Option | Description | Selected |
|--------|-------------|----------|
| Port it in fonoteka.go with the same flags | bonfire command, thin over wristband | ✓ |
| Generic wristband command in the framework | | |
| Defer the command | | |
**User's choice:** Port it in fonoteka.go with the same flags.
---
## Claude's Discretion
- Wristband interface names and signatures, transaction seam, in-memory store, helper placement.
- Config key layout on the fonoteka side.
- The gate's scripted client and Postgres provisioning.
- Logging policy, error-text constants, control-character stripping, Content-Type judgement for /register.
## Deferred Ideas
- Summer-themed default token prefix (inv_ stays for Płytarium).
- River sweep job — Phase 11.
- Framework-side client-issuing command.
- Live vendor connects — cutover UAT.
- Social login and oauth-identities routes — still deferred from Phase 7.