Preserve requested OAuth scopes during authorization

[mattzcarey]

Summary

• preserve valid scopes requested by MCP clients instead of replacing them with the read-only preset
• always add the required identity, account, and refresh-token scopes
• reject unknown scopes with invalid_scope
• preselect requested scopes in the consent picker
• always show scope consent because /authorize has no authenticated user identity with which to look up the correct provider grant
• remove the obsolete client-ID-only approval cookie and consent-skip path
• cover the complete scope flow through registration, consent, callback, and downstream token exchange

Fixes #178.

Note: the registered scope is access.write; the reported access:write spelling is now rejected explicitly rather than silently downgraded.

Verification

• npm run check
• 17 test files passed
• 244 tests passed

[YoAm]

Thanks for picking this up — I filed #178 and the diagnosis here matches what I saw: oauth-handler.ts:562 discarded the requested scopes unconditionally, so the behaviour was spelling-independent and my access:write vs access.write note was a red herring. The catalog is dot-delimited throughout (381/381 keys contain .), so rejecting access:write explicitly is the right call.

Three follow-ups from reading the merged diff, offered as observations rather than objections. Two are test-coverage gaps where the suite passes but does not exercise the property it appears to guard.

1. The "always shows consent" regression test cannot fail.

The test constructs new Request(url) with no init object, so it never sends a Cookie header — and the removed skip path already failed closed when the cookie was absent. Both the base commit and the head return 200 + <form> for that request, so the assertion holds identically before and after the change. A fixture that sets a valid __Host-MCP_APPROVED_CLIENTS cookie for the requesting client id would distinguish them; the current one cannot.

2. The only end-to-end write assertion was removed in the same PR.

Commit eeb38f16 ("test: assert OAuth scope propagation only", +4/−26) deletes the assertion that a granted write scope enables an actual MCP write. With the suite fully MSW-mocked, nothing now demonstrates that the original symptom in #178 — a write call failing after a write scope was requested — is resolved. Scope propagation is covered; scope efficacy is not. I could not verify the fix against the live API from my side, so I am flagging rather than asserting it is unfixed.

3. MCP_COOKIE_ENCRYPTION_KEY is now orphaned.

With the HMAC helpers deleted it has zero source references, but it is still declared required in worker-configuration.d.ts and vitest.config.ts. Harmless, but it will read as a live secret to the next person auditing the deployment.

One design note, not a defect. The cookie you removed satisfied all four bullets of MCP's confused-deputy mitigation — __Host- prefix, Secure/HttpOnly/SameSite=Lax, HMAC-signed, bound to specific client ids. Its actual defect was the one your PR body identifies: no user binding, so an approval was per-browser rather than per-account and survived an account switch. Worth noting because "we removed the approval cookie" reads like a vulnerability fix, and the precise version — an approval record must name what was approved and who approved it, or it must not persist — is the more transferable lesson. I have adopted it in my own codebase off the back of this.

Happy to open a PR for the cookie-bearing test fixture in (1) if that is useful.