feat(keycardai-oauth): add high-level PKCE user-login flow (ACC-229)#101
Merged
Conversation
Larry-Osakwe
added a commit
that referenced
this pull request
Apr 27, 2026
Three small fixes from the review of #101: 1. PKCEClient now accepts an optional injected httpx.AsyncClient. AgentClient passes its existing http_client through, so a single connection pool covers both the agent service calls and the OAuth flow. close() only closes the client it owns. Restores the one-pool-per-AgentClient behavior from main. 2. Drop the no-op rstrip("/") + "/" round-trip in PKCEClient.authenticate when building the authorization server discovery URL. 3. Assert the discovery URL path in test_authenticate_completes_full_flow. The previous test stubbed http_mock.get with side_effects but never verified what URLs were passed; a typo from oauth-authorization-server to openid-configuration would have gone unnoticed.
| response = await self.http_client.get(discovery_url) | ||
| response.raise_for_status() | ||
| return response.json() | ||
| self._pkce = PKCEClient( |
Collaborator
There was a problem hiding this comment.
Why do we call this PKCEClient?
This should be standard client with PKCE option enabled. Is there a reason for making this standalone?
Contributor
Author
There was a problem hiding this comment.
Good call, addressed in cd3941c.
PKCEClient is gone. The user-flow is now keycardai.oauth.pkce.authenticate (module-level async function), and the OAuth-server-facing work goes through the existing AsyncClient:
- Server metadata discovery (RFC 8414) →
AsyncClient.__aenter__ - Code exchange →
AsyncClient.exchange_authorization_code(gained an optionalresource=kwarg so RFC 8707 still works through the canonical path)
The pkce module retains only what does not belong on AsyncClient: WWW-Authenticate parsing (RFC 9728), resource metadata fetch (paired with the protected resource, not the OAuth server), browser launch, and the loopback callback server (RFC 8252).
AgentClient drops the cached PKCE instance and just calls the function per /invoke retry. Tests rewritten for the new shape, 7/7 passing.
kamil-keycard
requested changes
Apr 27, 2026
Larry-Osakwe
added a commit
that referenced
this pull request
Apr 27, 2026
…n AsyncClient Per Kamil review feedback (#101): a separate PKCEClient sitting next to AsyncClient invited "which client do I use?" The OAuth-server-facing operations belong on the existing AsyncClient. Changes: - keycardai.oauth.pkce.PKCEClient (class) -> keycardai.oauth.pkce.authenticate (module-level async function). One-shot per user login, no state worth preserving across calls. - The function uses AsyncClient internally for server metadata discovery (RFC 8414) and code exchange. AsyncClient is now the only thing in keycardai.oauth that talks to OAuth servers as a client. - AsyncClient.exchange_authorization_code (and Client + the underlying operations._authorize helpers) gain an optional resource= parameter so RFC 8707 tokens still work through the canonical path. - The pkce module retains the user-flow concerns: RFC 9728 challenge parsing, resource metadata fetch (paired with the protected resource, not the OAuth server), browser launch, and the loopback callback server (RFC 8252). - AgentClient drops the cached _pkce instance and just calls the function per /invoke retry, passing its own httpx.AsyncClient through for the resource metadata fetch. - Tests rewritten for the function shape: 7/7 passing, same coverage (header parsing, discovery error paths, happy path with resource indicator, public vs confidential auth on the token endpoint). Verified: oauth 215/215, agents 81/81, mcp 560/560, starlette 49/49. ruff clean.
Larry-Osakwe
changed the title
kamil-keycard
approved these changes
Apr 28, 2026
First step of the keycardai-agents decomposition (ACC-229..232). Per the revised KEP, the OAuth PKCE user-login flow is generic OAuth code with no agents-specific concerns and belongs in keycardai-oauth next to the rest of the OAuth client primitives. New module keycardai.oauth.pkce: - PKCEClient orchestrates the full authorization-code-with-PKCE flow: parse the WWW-Authenticate challenge (RFC 9728), fetch protected resource and authorization server metadata (RFC 8414), open the browser at the authorize endpoint, capture the redirect via a local callback server, and exchange the code at the token endpoint. Returns the token endpoint response dict directly. - OAuthCallbackServer is the loopback redirect catcher (RFC 8252) used by PKCEClient; exported separately so callers running their own flow on top of the lower-level PKCEGenerator + build_authorize_url primitives can reuse the callback machinery. - 7 new tests cover header parsing, discovery error paths, the happy-path flow, and confidential vs public client auth on the token endpoint. keycardai-agents changes: - AgentClient now composes PKCEClient instead of carrying its own copy of the auth flow. AgentClient.authenticate(...) is preserved as a thin shim that returns the access_token string and updates the per-service token cache, so existing /invoke retry-on-401 behavior is unchanged. - AgentClient drops ~370 lines of duplicated PKCE/discovery/callback code. - keycardai.agents.client.oauth re-exports OAuthCallbackServer through a module __getattr__ that emits a DeprecationWarning pointing at the new canonical import path. - Stale tests in test_agent_client_oauth.py that exercised AgentClient private methods (_extract_resource_metadata_url, _fetch_resource_metadata, _fetch_authorization_server_metadata) removed; equivalent contracts now live in the keycardai-oauth PKCE test suite. Verified: oauth 215/215 (was 208 + 7 new), agents 81/81 (was 85 - 4 removed implementation tests), mcp 560/560, starlette 49/49, ruff clean. Stacked on #100 (ACC-236 a2a-sdk pin) so the agents test suite can run during validation.
Three small fixes from the review of #101: 1. PKCEClient now accepts an optional injected httpx.AsyncClient. AgentClient passes its existing http_client through, so a single connection pool covers both the agent service calls and the OAuth flow. close() only closes the client it owns. Restores the one-pool-per-AgentClient behavior from main. 2. Drop the no-op rstrip("/") + "/" round-trip in PKCEClient.authenticate when building the authorization server discovery URL. 3. Assert the discovery URL path in test_authenticate_completes_full_flow. The previous test stubbed http_mock.get with side_effects but never verified what URLs were passed; a typo from oauth-authorization-server to openid-configuration would have gone unnoticed.
…n AsyncClient Per Kamil review feedback (#101): a separate PKCEClient sitting next to AsyncClient invited "which client do I use?" The OAuth-server-facing operations belong on the existing AsyncClient. Changes: - keycardai.oauth.pkce.PKCEClient (class) -> keycardai.oauth.pkce.authenticate (module-level async function). One-shot per user login, no state worth preserving across calls. - The function uses AsyncClient internally for server metadata discovery (RFC 8414) and code exchange. AsyncClient is now the only thing in keycardai.oauth that talks to OAuth servers as a client. - AsyncClient.exchange_authorization_code (and Client + the underlying operations._authorize helpers) gain an optional resource= parameter so RFC 8707 tokens still work through the canonical path. - The pkce module retains the user-flow concerns: RFC 9728 challenge parsing, resource metadata fetch (paired with the protected resource, not the OAuth server), browser launch, and the loopback callback server (RFC 8252). - AgentClient drops the cached _pkce instance and just calls the function per /invoke retry, passing its own httpx.AsyncClient through for the resource metadata fetch. - Tests rewritten for the function shape: 7/7 passing, same coverage (header parsing, discovery error paths, happy path with resource indicator, public vs confidential auth on the token endpoint). Verified: oauth 215/215, agents 81/81, mcp 560/560, starlette 49/49. ruff clean.
Larry-Osakwe
force-pushed
the
larry/acc-229-move-pkce-to-oauth
branch
from
cd3941c to
d8f1b3e
Compare
📦 Release PreviewThis analysis shows the expected release impact: 📈 Expected Version Changes📋 Package Details[
{
"package_name": "keycardai-oauth",
"package_dir": "packages/oauth",
"has_changes": true,
"current_version": "0.10.0",
"next_version": "0.11.0",
"increment": "MINOR"
}
]📝 Changelog PreviewThis comment was automatically generated by the release preview workflow. |
8 tasks
Larry-Osakwe
added a commit
that referenced
this pull request
Apr 29, 2026
#105) * feat(keycardai-a2a)!: new package split from keycardai-agents (ACC-230) Per the KEP "Decompose keycardai-agents", the A2A delegation surface moves out of keycardai-agents into a new keycardai-a2a package, structurally analogous to keycardai-mcp. Symbols available at the new namespace: - AgentServer, create_agent_card_server, serve_agent - DelegationClient, DelegationClientSync - AgentExecutor, SimpleExecutor, LambdaExecutor - KeycardToA2AExecutorBridge - ServiceDiscovery - AgentServiceConfig The bearer middleware in server/app.py also migrates from the deprecated BearerAuthMiddleware to the canonical KeycardAuthBackend + AuthenticationMiddleware pattern from keycardai-starlette. The keycardai-mcp dependency drops from this code path. Hard cut, no transitional bridge: ACC-232 confirms no known production users of keycardai.agents.* paths. The PKCE user-login client (AgentClient) is dropped entirely. Its capability already lives in keycardai-oauth as keycardai.oauth.pkce.authenticate (ACC-229 / #101). The duplicate in keycardai-agents is removed. What stays in keycardai-agents: the CrewAI integration only, with its imports repointed at keycardai.a2a. ACC-231 will move it to a dedicated keycardai-crewai; ACC-232 will archive the now-stub source directory. BREAKING: - from keycardai.agents import AgentServer, DelegationClient, ... becomes from keycardai.a2a import ... . - from keycardai.agents.client import AgentClient is gone; use keycardai.oauth.pkce.authenticate. - keycardai-agents 0.3.0 ships with the dependency set reduced to keycardai-a2a + pydantic, mirroring its now-CrewAI-only scope. * fix(keycardai-a2a): apply migration edits to moved files (ACC-230) The first commit on this branch did the git mv's but staged the new files only; the Edit-tool modifications to the moved files (import rewrites, server/app.py bearer-wiring migration to KeycardAuthBackend + AuthenticationMiddleware, example pyproject swap from keycardai-agents to keycardai-a2a, conftest/tests import repoints, agents/__init__.py trim, agents/pyproject dep set, crewai integration repoint, top-level workspace sources, justfile test recipe, uv.lock refresh) all sat uncommitted. CI rejected the prior commit because the example pyproject still claimed keycardai-agents at packages/a2a/, conflicting with the real keycardai-agents at packages/agents/ in the workspace graph. This commit lands the actual migration content. Tests pass locally: keycardai-a2a 60/60, keycardai-agents 16/16, no regression in oauth / starlette / mcp / mcp-fastmcp / fastmcp; ruff workspace check clean. * refactor(keycardai-a2a)!: wrap a2a-sdk 1.x, drop parallel surface (ACC-230) Aligns keycardai-a2a with the wrap-do-not-reinvent pattern used in keycardai-mcp and keycardai-starlette: customers implement a2a-sdk native async AgentExecutor directly; this package contributes only Keycard auth wiring, OAuth metadata discovery, and convenience composition. Drops the parallel-protocol surface inherited from the keycardai-agents move: - AgentExecutor protocol (sync execute(task, inputs)) and SimpleExecutor / LambdaExecutor implementations - KeycardToA2AExecutorBridge (the sync->async adapter that existed only to bridge our protocol to a2a-sdk) - Custom POST /invoke endpoint with bespoke InvokeRequest / InvokeResponse Pydantic models alongside the standard A2A JSONRPC interface - AgentServiceConfig.invoke_url (replaced by jsonrpc_url) and AgentServiceConfig.to_agent_card() (the 0.x dict-shape constructor) Migrates from a2a-sdk 0.x to 1.x natively: - pyproject pin a2a-sdk[http-server]>=1.0 - Server composition uses route factories (create_jsonrpc_routes, create_agent_card_routes) instead of the gone A2AStarletteApplication - Request handler is DefaultRequestHandlerV2 (alias DefaultRequestHandler) - AgentCard built from 1.x protobuf schema (supported_interfaces, AgentCapabilities streaming/push_notifications/extended_agent_card) - Example main.py uses a2a-sdk 1.x Client via create_client + A2ACardResolver Adds a KeycardServerCallContextBuilder that subclasses a2a-sdk default builder and stashes the verified KeycardUser plus access_token into ServerCallContext.state so AgentExecutor implementations can read the bearer token from context.call_context.state["access_token"] for downstream delegated token exchange. Tests: - a2a 44/44 pass - agents 16/16 pass with crewai extra - ruff clean Note: the high-level @auth.grant decorator parity with keycardai-mcp is not yet shipped here. Customers use DelegationClient (already in this package) for explicit server-to-server delegation. The decorator port is a follow-up. * fix(keycardai-a2a): address review findings on PR #105 Three blockers caught in fresh-eyes review: 1. release.yml tag-trigger list was hardcoded; *-keycardai-a2a was missing, so the post-merge auto-bump would push the tag but the publish workflow would never trigger. Trusted Publisher being registered would have been moot. 2. DelegationClient.invoke_service hardcoded service_url + /invoke. The wrap-aligned server only exposes /a2a/jsonrpc; calling invoke_service against any 1.x server returned 404. The CrewAI delegation tool runs through this code path. Both async and sync variants now build a message/send JSONRPC envelope, POST it to /a2a/jsonrpc, and unwrap the result to preserve the legacy {result, delegation_chain} shape so the CrewAI integration keeps working unchanged. 3. discover_service in both DelegationClient and ServiceDiscovery validated the 0.x card shape (required_fields = [name, endpoints, auth]). The 1.x protobuf-derived JSON has none of endpoints / auth. Discovery against any 1.x server raised ValueError. Validation now requires only "name"; transport / auth specifics live under supportedInterfaces and the OAuth metadata routes. Plus four important findings: 4. Test mocks across conftest.py, test_a2a_client.py, test_discovery.py, and test_crewai_a2a.py used the old shape (endpoints/auth keys). Tests passed because the validator wrongly accepted them. Mocks now use the 1.x JSON shape (supportedInterfaces, capabilities object, skills with id/name). 5. A2AServiceClient and A2AServiceClientSync backward-compat aliases at the bottom of delegation.py contradicted the "hard cut, no transitional bridge" stance in the PR description. Removed. 6. TestJsonRpcAuthGate.test_jsonrpc_requires_authorization asserted status_code in (400, 401). 400 means the JSONRPC dispatcher saw the request and bailed on the body shape, not that the auth gate caught it. Pinned to == 401 with a WWW-Authenticate header check so the gate contract is enforced. 7. Zero coverage existed for _KeycardServerCallContextBuilder propagating the verified KeycardUser plus access_token into ServerCallContext.state. Added two unit tests that build the context directly: one with a KeycardUser asserting state["access_token"] is set, one with an UnauthenticatedUser asserting state["access_token"] is absent (so an executor reading it sees None rather than a stale token). Tests: - a2a 47/47 (was 44; +3 new wrap-coverage tests) - agents 16/16 with crewai extra - ruff clean * refactor(keycardai-a2a)!: ship primitives, not a server abstraction (ACC-230) Per Kamil's review on PR #105: AgentServer / create_agent_card_server / serve_agent presupposed customers want a fresh Starlette app dedicated to the agent service. The wrap-don't-reinvent stance, taken seriously, says: customers already have an a2a-sdk app in their head; we ship primitives that slot Keycard auth into THAT, not a parallel server. Public surface change: Dropped: AgentServer, create_agent_card_server, serve_agent Promoted to public (renamed off the underscore prefix): EagerKeycardAuthBackend KeycardServerCallContextBuilder build_agent_card_from_config AgentServiceConfig trimmed: dropped agent_executor (DefaultRequestHandler takes its own), port and host (uvicorn's job), status_url (no /status in the primitives layer). The composed-server flow moves to a runnable example at packages/a2a/examples/keycard_protected_server/. README quickstart rewritten to show primitive composition into an existing app; greenfield users follow the example. Tests: a2a 44/44 (was 47; net -3 from dropping the /status endpoint tests and the port-validation test) agents 16/16 with crewai extra ruff clean This change is breaking, but the package is 0.1.0-pre-publish so no customer is on these names yet. * fix(keycardai-a2a): ruff import-organization auto-fix * refactor(keycardai-starlette,keycardai-a2a): collapse EagerKeycardAuthBackend into KeycardAuthBackend kwarg (ACC-230) Per Kamil's second review observation on PR #105: with keycardai-a2a now depending on keycardai-starlette, the question of WHERE these primitives live matters. EagerKeycardAuthBackend was a 5-line subclass that flipped one branch of KeycardAuthBackend.authenticate to raise on missing Authorization, with no a2a-sdk specifics. The behavior is a policy choice ("this mount requires auth"), not a different kind of backend. Collapsed to a kwarg on the existing class: KeycardAuthBackend(verifier) # default, # mixed-route KeycardAuthBackend(verifier, require_authentication=True) # all-paths-protected The OAuth metadata bypass (RFC 9728 §2 / RFC 8414 §3) takes precedence over the kwarg: even with require_authentication=True, requests to /.well-known/oauth-* and /.well-known/jwks.json still pass through anonymously per spec. New parametrized test asserts this. Net effect: - One class instead of two; existing KeycardAuthBackend(verifier) callers unchanged. - keycardai-a2a no longer ships EagerKeycardAuthBackend; the kwarg is used directly in tests, the example, and the README quickstart. - Migration story is zero churn for existing users; new behavior is opt-in via the kwarg. Tests: starlette 40 passed (+2 new tests for the kwarg semantics) a2a 44 passed agents 16 passed ruff clean
4 tasks
Larry-Osakwe
added a commit
that referenced
this pull request
May 1, 2026
Removes packages/agents/ entirely. The PKCE user-login client moved to keycardai-oauth (#101); the A2A delegation surface moved to keycardai-a2a (#105). The CrewAI integration that briefly remained had no users; the extraction PR (#106) was closed without merging in favor of customers wiring CrewAI to Keycard primitives directly. Drops keycardai-agents from the root workspace sources and the release.yml tag allowlist so accidental tags cannot trigger republish. keycardai-agents on PyPI is frozen at 0.2.0 and is being yanked separately. Updates packages/a2a/README.md to drop the now-stale references to a forthcoming keycardai-crewai package and the in-progress archival. BREAKING: any "from keycardai.agents.*" import fails. No production users. Closes ACC-232.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
First step of the
keycardai-agentsdecomposition (ACC-229..232). Per the revised KEP, the OAuth PKCE user-login flow is generic OAuth code with no agents-specific concerns and belongs inkeycardai-oauthnext to the rest of the OAuth client primitives.What ships
New
keycardai.oauth.pkcemodule:authenticate(...)is a module-level async function that drives the full authorization-code-with-PKCE flow:WWW-Authenticatechallenge (RFC 9728)keycardai.oauth.AsyncClientagainst the discovered authorization server for server metadata discovery (RFC 8414) and code exchangeTokenResponseOAuthCallbackServeris the loopback redirect catcher used byauthenticate; exported separately so callers running their own flow on top of the lower-level primitives can reuse the callback machinery.AsyncClient/Clientgain an optionalresourcekwarg onexchange_authorization_code(and on the underlyingoperations._authorizehelpers). RFC 8707 resource indicators are now first-class on the canonical token-exchange path, which is what the PKCE flow uses internally.keycardai-agentschanges:AgentClient.authenticatenow callskeycardai.oauth.pkce.authenticate(...)per/invokeretry, returning the access token string and updating the per-service token cache. No cached PKCE instance, no shared state.AgentClientdrops ~370 lines of duplicated PKCE/discovery/callback code.keycardai.agents.client.oauthre-exportsOAuthCallbackServerthrough a module__getattr__that emits aDeprecationWarningpointing at the new canonical import path.Why a function and not a class
Per Kamil's review: a separate
PKCEClientsitting next toAsyncClientinvited "which client do I use?" The OAuth-server-facing operations (server metadata discovery, code exchange) belong on the existingAsyncClientinstead of on a peer client. The PKCE module retains only what does not belong onAsyncClient: the user-flow orchestration (RFC 9728 challenge parsing, resource metadata fetch, browser launch, loopback callback server).AsyncClientis now the only thing inkeycardai.oauththat talks to OAuth servers as a client.Test plan
packages/oauth/tests/keycardai/oauth/pkce/test_client.py:WWW-Authenticateheader parsing (present + missing)resourceindicator is sent on the token request and HTTP Basic auth is configured for confidential clientsNoneAuthon the token endpointRemoved tests, justified
test_agent_client_oauth.py::TestOAuthDiscoveryMethodsexercised private methods onAgentClient(_extract_resource_metadata_url,_fetch_resource_metadata,_fetch_authorization_server_metadata). Those methods are gone fromAgentClient; equivalent contracts are tested intest_client.pyin the keycardai-oauth suite. TheTestAgentClientInit,TestInvokeWithoutAuth, andTestContextManagerclasses coveringAgentClient's public surface stay.Follow-up tickets
AgentServer/DelegationClient/ServiceDiscoveryinto newkeycardai-a2apackagekeycardai-crewaipackagepackages/agents/source directory