feat(keycardai-oauth): stateless web-app authorization-code flow (spec #46)

[devin-ai-integration]

Summary

Implements the fourth layer of the authorization-code-pkce capability (spec-version 2) defined in keycard-sdk-spec#46: a stateless begin/complete pair for apps that own a registered redirect URI and receive the callback on their own route.

pkce.authenticate is the wrong shape for those apps — it runs a loopback callback server (RFC 8252), which only makes sense for processes with no HTTP surface of their own. Apps with a callback route were hand-assembling the flow from PKCEGenerator + build_authorize_url + exchange_authorization_code with a manual state and their own single pending-flow slot, which caps them at one concurrent sign-in.

New in keycardai.oauth.pkce (pkce/web.py):

redirect = await begin_authorization(          # -> AuthorizationRedirect(url, state, code_verifier)
    client_id=..., redirect_uri=..., issuer=..., scopes=[...],
)
# app stores redirect.state + redirect.code_verifier in its own session state
token = await complete_authorization(
    callback_params=request.query_params, state=..., code_verifier=...,
    client_id=..., redirect_uri=..., issuer=...,
)

The SDK holds nothing between the two calls and keeps no per-flow registry, which is what makes concurrent sign-ins and multi-process servers work without coordination. complete_authorization validates before it discovers or requests anything: error in the callback params → AuthorizationDeniedError (new, subclasses OAuthProtocolError, carries error / error_description); missing or non-matching state → StateMismatchError (new; compared with secrets.compare_digest over UTF-8 bytes, since the callback value is browser-controlled and compare_digest rejects non-ASCII str); missing code → OAuthProtocolError(error="invalid_request"). No token request is made in any of those cases.

Statelessness is the contract between the two calls, not a reason to rediscover metadata on every one, so both functions also accept pre-discovered metadata — same parameter as userinfo(metadata=...) in #232, with the app owning the cache exactly as it owns the session storage:

metadata=cached_metadata   # instead of issuer=... / www_authenticate_header=...

That makes three mutually exclusive ways to name the authorization server (issuer, www_authenticate_header, metadata); anything else is a ConfigError. Metadata mode is exclusive rather than additive because a caller holding cached AS metadata already knows the AS — allowing it alongside www_authenticate_header would keep the protected-resource-metadata fetch on the sign-in path, which is half of what the parameter exists to remove. In that mode begin_authorization builds no client at all (it only needs authorization_endpoint), and complete_authorization builds one that issues no discovery request (enable_metadata_discovery=False, token endpoint passed as an endpoints= override). Each function validates only the endpoint it uses.

Per the spec discussion, begin_authorization does not accept a caller-supplied state. Naming is left to the language idiom profile by the spec — begin_authorization / complete_authorization / AuthorizationRedirect here.

Issuer resolution moved out of pkce/client.py into a private pkce/_issuer.py shared by both flows (public keycardai.oauth.pkce.resolve_issuer_from_challenge is unchanged); authenticate's behavior is untouched apart from the ConfigError message no longer naming authenticate().

Tests cover spec cases 8–11 plus the missing-code, non-ASCII-state, public-vs-confidential-client, challenge-driven, cached-metadata (asserting no client construction / no discovery await), missing-endpoint and ConfigError paths, asserting the token exchange is never awaited on every rejection path. Adds a framework-agnostic example under packages/oauth/examples/web_authorization_code_flow/ and a README section, both issuer-first with the cached-metadata variant noted.

just check, just test-package oauth (356 passed) and scripts/changelog.py validate pass. docs/sdk was not regenerated — the keycardai.oauth.pkce package isn't in the generated reference today and just sdk-ref-oauth would pull in unrelated modules.

Link to Devin session: https://app.devin.ai/sessions/a1881ef9670c452e98caec6ccd031ad1
Requested by: @Larry-Osakwe

[devin-ai-integration]

🤖 Devin AI Engineer

I'll be helping with this pull request! Here's what you should know:

✅ I will automatically:

• Address comments on this PR. Add '(aside)' to your comment to have me ignore it.
• Look at CI failures and help fix them

Note: I can only respond to comments from users who have write access to this repository.

⚙️ Control Options:

• Disable automatic comment, CI, and merge conflict monitoring

[Larry-Osakwe]

The security-sensitive parts are right: validation ladder ordered error/state/code before any network, constant-time state compare over UTF-8 bytes with the non-ASCII case tested, exchange asserted never-awaited on every rejection path, and I diffed the moved resolve_issuer_from_challenge body side by side (verbatim, public import path preserved). One change needed before approval, one test ask, one nit inline.

[Larry-Osakwe]

Every begin and every complete builds a fresh AsyncClient and runs full metadata discovery, so a login route pays two discovery round trips per sign-in with no way to amortize. Statelessness between the two calls is the contract, but per-call rediscovery isn't. Add an optional metadata= param to both functions (same move as userinfo() in #232): when provided, skip discovery and read the endpoints from it. The app owns the caching, same philosophy as the session storage. The first real consumer is a login page, so this gets hit on every sign-in from day one.

[devin-ai-integration]

Added metadata: AuthorizationServerMetadata | None to both functions, same name/type/semantics as userinfo(metadata=...) in #232.

One design call worth flagging: I made metadata a third mutually exclusive entry mode, so the rule is now "exactly one of issuer, www_authenticate_header, or metadata". Reason: if the app has cached AS metadata it already knows the AS, and letting it also pass www_authenticate_header would keep the protected-resource-metadata fetch on the critical path — half the round trips the param exists to remove.

In begin, metadata mode builds no AsyncClient at all (it only needs authorization_endpoint). In complete, the client is still needed for the exchange but makes no discovery request: issuer=metadata.issuer, enable_metadata_discovery=False, token endpoint via the endpoints= override. Both are asserted in tests (assert_not_called / get_endpoints.assert_not_awaited).

[Larry-Osakwe]

Two things here: begin only uses the authorize endpoint, so requiring token_endpoint at this step is stricter than it needs to be (complete is where token_endpoint matters). And this ValueError branch is untested in both functions; one test each with metadata missing the endpoint would cover it.

[devin-ai-integration]

Both fixed: begin now checks only authorization_endpoint, complete only token_endpoint, each with a message naming just that endpoint. Added the missing-endpoint tests for both, in discovery mode and in metadata mode.

[Larry-Osakwe]

Nit from the refactor: challenge mode now logs "PKCE flow starting for resource None" before _resolve_auth_server_url raises when both entry modes are absent. Moving the logging after the resolver call restores the old order.

[devin-ai-integration]

Good catch — moved both log lines after _resolve_auth_server_url, so the resolver's ConfigError is raised before anything is logged.