Skip to content

fix(keycardai-starlette): stop rewriting the proxied authorization_endpoint (SDK-3) - #316

Merged
Larry-Osakwe merged 2 commits into
mainfrom
devin/1789596584-starlette-as-passthrough
Sep 16, 2026
Merged

Larry-Osakwe merged 2 commits into
mainfrom
devin/1789596584-starlette-as-passthrough

Conversation

@devin-ai-integration

Copy link
Copy Markdown
Contributor

Summary

Retires a compatibility shim aimed at MCP clients on the 2025-03-26 authorization spec. authorization_server_metadata in keycardai-starlette proxied the zone's RFC 8414 document and appended resource=<request origin> to the upstream authorization_endpoint. The proxy now returns the parsed upstream document unchanged; authorization_endpoint passes through byte for byte, including any query it already carries. This implements keycard-sdk-spec specs/server-bearer-auth/oauth-metadata-endpoints.md spec-version 2 (keycardai/keycard-sdk-spec#65, SDK-3).

Why: the MCP 2025-06-18 authorization spec made the RFC 8707 resource parameter mandatory for clients, and those clients fetch AS metadata from the issuer directly, so they never see the proxy. The rewrite reached only the 2025-03-26 generation, which the MCP spec retired in June 2025. Where it did fire, it produced tokens with aud equal to the bare request origin while the recommended verifier configuration binds the audience to the full endpoint, so a client following our own discovery got a token our own verifier rejected. Both packages are pre-1.0; this ships as a fix-level change.

Code:

-def _append_resource_param(endpoint: str, resource: str) -> str: ...
 ...
-                content = resp.json()
-                if isinstance(content, dict) and content.get("authorization_endpoint"):
-                    content["authorization_endpoint"] = _append_resource_param(
-                        content["authorization_endpoint"], get_base_url(request))
-                return JSONResponse(content=content, headers=CORS_HEADERS)
+                return JSONResponse(content=resp.json(), headers=CORS_HEADERS)

The urllib.parse imports go with the helper; get_base_url stays (still used by the protected-resource handler). The authorization_server_metadata docstring now says the document is returned unmodified.

Tests:

  • packages/starlette/tests/.../test_routers.py: test_authorization_endpoint_gains_resource_param and test_authorization_endpoint_preserves_existing_query are replaced by test_upstream_document_passed_through_unmodified, where upstream already carries ?resource=stale&keep=1 and the whole document is asserted equal to upstream (spec Unit Tests row 4). test_no_authorization_endpoint_left_unchanged stays.
  • packages/mcp has no separate implementation (keycardai.mcp.server.routers re-exports the starlette handlers), but its handler and integration tests pinned the rewrite through the re-export, so tests/keycardai/mcp/server/handlers/test_metadata.py and tests/integration/test_metadata.py now assert the verbatim upstream value; test_authorization_endpoint_preservation gains the ?resource=stale&keep=1 case. No mcp source changes.

No version or CHANGELOG edits; historical CHANGELOG entries are left as they are.

Verification

  • just check: All checks passed.
  • just typecheck: All checks passed (all seven packages).
  • packages/starlette tests: 121 passed.
  • packages/mcp tests: 564 passed, 18 skipped, 2 failed. Both failures (tests/integration/test_auth_provider.py::TestAuthProviderInitialization::test_auth_provider_init_with_zone_id, tests/integration/test_web_identity.py::TestWebIdentity::test_no_application_credential_does_enable_registration) fail identically on current main with this change stashed; they are unrelated to metadata and not touched here.

Link to Devin session: https://app.devin.ai/sessions/bbc062cf32b548ef9c4b8b444d2ec67f
Open in Devin Desktop: https://app.devin.ai/desktop/session/bbc062cf32b548ef9c4b8b444d2ec67f?variant=devin
Requested by: @Larry-Osakwe

…dpoint (SDK-3)

The authorization-server metadata proxy returned the upstream document with
resource=<origin> appended to authorization_endpoint, a shim for MCP clients
on the 2025-03-26 authorization spec. The proxy now returns the upstream
document unmodified, per keycard-sdk-spec oauth-metadata-endpoints spec-version 2.

Co-Authored-By: Larry Osakwe <larry@keycard.ai>
@devin-ai-integration

Copy link
Copy Markdown
Contributor Author

🤖 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

…14.2

keycardai-mcp re-exports the starlette metadata handlers and its tests now
pin the unmodified AS-proxy document, which keycardai-starlette 0.14.2 is
the first release to ship. The sibling-floors job skips this floor under the
floors-bootstrap label until that version is on PyPI.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>

@Larry-Osakwe Larry-Osakwe left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Starlette proxy returns the parsed upstream document as is, helper and urllib imports gone, docstring updated, and the pass-through test pins an existing query verbatim. Matches oauth-metadata-endpoints.md spec-version 2. I pushed 4e9232e raising keycardai-mcp's starlette floor to 0.14.2, since the mcp tests now pin behavior only that release ships; the floors-bootstrap label covers the sibling-floors leg until 0.14.2 is on PyPI, and a keycardai-mcp patch bump follows it.

@Larry-Osakwe
Larry-Osakwe merged commit 69eed70 into main Sep 16, 2026
14 checks passed
@Larry-Osakwe
Larry-Osakwe deleted the devin/1789596584-starlette-as-passthrough branch September 16, 2026 22:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants