Add JWT audience validation and RFC 8707 warnings to auth providers#3204
Add JWT audience validation and RFC 8707 warnings to auth providers#3204
Conversation
marvin-context-protocol
Bot
added
bug
WalkthroughThis PR updates docs and examples for AuthKit, Supabase, GitHub, and DCR samples to warn that RFC 8707 resource indicators are unsupported, adjusts OAuth example initialization and environment variable names, and fixes several documentation anchors. It changes provider implementations to pass an audience to JWTVerifier: Scalekit now uses its resource_id, Supabase defaults audience="authenticated" (and emits a runtime warning), and AuthKitProvider accepts an optional client_id (used as verifier audience) and emits a warning about audience/resource-indicator limitations. Minor example/server code and README env var usage were also updated. Possibly related PRs
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 78deb13797
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| auth = AuthKitProvider( | ||
| authkit_domain=os.getenv("FASTMCP_SERVER_AUTH_AUTHKITPROVIDER_AUTHKIT_DOMAIN") | ||
| or "", | ||
| authkit_domain=os.getenv("AUTHKIT_DOMAIN") or "", |
There was a problem hiding this comment.
The server example now reads AUTHKIT_DOMAIN, but both the example README (examples/auth/authkit_dcr/README.md) and this file’s own header still instruct users to export FASTMCP_SERVER_AUTH_AUTHKITPROVIDER_AUTHKIT_DOMAIN; following those instructions leaves authkit_domain empty at runtime and the example fails to start. Please either keep reading the documented variable (or both names for compatibility) or update the accompanying docs in the same change.
Useful? React with 👍 / 👎.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
examples/auth/authkit_dcr/server.py (1)
5-7:⚠️ Potential issue | 🟡 MinorStale env var name in module docstring.
The comment on line 6 still advertises the old name
FASTMCP_SERVER_AUTH_AUTHKITPROVIDER_AUTHKIT_DOMAIN, but the code on line 18 now readsAUTHKIT_DOMAIN. A user following the module docstring will never set the right variable, causingor ""to silently pass an empty string toAuthKitProvider, which will fail with a confusing Pydantic URL validation error.🐛 Proposed fix
Required environment variables: -- FASTMCP_SERVER_AUTH_AUTHKITPROVIDER_AUTHKIT_DOMAIN: Your AuthKit domain (e.g., "https://your-app.authkit.app") +- AUTHKIT_DOMAIN: Your AuthKit domain (e.g., "https://your-app.authkit.app")
🧹 Nitpick comments (3)
src/fastmcp/server/auth/providers/supabase.py (1)
104-118: Warning fires unconditionally even when a customtoken_verifieris supplied.The
logger.warning(...)call at line 104 precedes theif token_verifier is None:guard at line 111, so a caller who passes their own well-configuredtoken_verifierstill gets the RFC 8707 alarm. The limitation is real regardless, but emitting the warning before checking whether the default verifier is even being used is misleading for users who have taken deliberate steps to handle audience externally.Move the warning inside the
if token_verifier is None:block:♻️ Proposed refactor
- logger.warning( - "SupabaseProvider cannot validate token audience for the specific resource " - "because Supabase Auth does not support RFC 8707 resource indicators. " - "This may leave the server vulnerable to cross-server token replay." - ) - # Create default JWT verifier if none provided if token_verifier is None: + logger.warning( + "SupabaseProvider cannot validate token audience for the specific resource " + "because Supabase Auth does not support RFC 8707 resource indicators. " + "This may leave the server vulnerable to cross-server token replay." + ) token_verifier = JWTVerifier(src/fastmcp/server/auth/providers/workos.py (1)
236-247: Class docstring example omits the recommendedclient_idusageThe example in the class docstring doesn't include
client_id, even though the PR adds it specifically to enable audience validation. Users reading the docstring won't know to provide it.✏️ Suggested update
Example: ```python from fastmcp.server.auth.providers.workos import AuthKitProvider - # Create AuthKit metadata provider (JWT verifier created automatically) workos_auth = AuthKitProvider( authkit_domain="https://your-workos-domain.authkit.app", + client_id="client_01ABC...", # validates JWT audience base_url="https://your-fastmcp-server.com", )docs/integrations/supabase.mdx (1)
35-50: Use theStepscomponent instead of H3 headers for sequential setup stepsThe new
### Step 1 / Step 2 / Step 3structure uses heading-based numbering. Per coding guidelines, theStepscomponent is the prescribed pattern for sequential setup instructions in Mintlify MDX docs.✏️ Suggested refactor
-### Step 1: Enable Supabase OAuth Server - -In your Supabase Dashboard: -1. Go to **Authentication → OAuth Server** -2. Enable the **OAuth Server** -3. Set your **Site URL** to where your consent UI is hosted -4. Set the **Authorization Path** (e.g., `/oauth/callback`) -5. Enable **Allow Dynamic OAuth Apps** for MCP client registration - -### Step 2: Get Supabase Project URL - -In your Supabase Dashboard: -1. Go to **Project Settings** -2. Copy your **Project URL** (e.g., `https://abc123.supabase.co`) - -### Step 3: FastMCP Configuration +<Steps> + <Step title="Enable Supabase OAuth Server"> + In your Supabase Dashboard: + 1. Go to **Authentication → OAuth Server** + 2. Enable the **OAuth Server** + 3. Set your **Site URL** to where your consent UI is hosted + 4. Set the **Authorization Path** (e.g., `/oauth/callback`) + 5. Enable **Allow Dynamic OAuth Apps** for MCP client registration + </Step> + <Step title="Get Supabase Project URL"> + In your Supabase Dashboard: + 1. Go to **Project Settings** + 2. Copy your **Project URL** (e.g., `https://abc123.supabase.co`) + </Step> + <Step title="FastMCP Configuration">(Close the
</Steps>tag after the code block that ends around line 72.)As per coding guidelines, "Use Steps component for procedures, tutorials, setup guides, and sequential instructions in MDX documentation."
| **Methods:** | ||
|
|
||
| #### `get_routes` <sup><a href="https://github.com/jlowin/fastmcp/blob/main/src/fastmcp/server/auth/providers/workos.py#L290" target="_blank"><Icon icon="github" style="width: 14px; height: 14px;" /></a></sup> | ||
| #### `get_routes` <sup><a href="https://github.com/jlowin/fastmcp/blob/main/src/fastmcp/server/auth/providers/workos.py#L302" target="_blank"><Icon icon="github" style="width: 14px; height: 14px;" /></a></sup> |
There was a problem hiding this comment.
docs/python-sdk/** should not be manually modified
As per coding guidelines, docs/python-sdk/** is automatically updated by a bot. Manual edits to this directory (including this anchor correction) will be overwritten by the next bot commit. The anchor update should be a downstream effect of the source code change, not a manual fix here.
As per coding guidelines, "Do not manually modify docs/python-sdk/** - a bot automatically updates these files via commits added to PRs."
| **Methods:** | ||
|
|
||
| #### `get_routes` <sup><a href="https://github.com/jlowin/fastmcp/blob/main/src/fastmcp/server/auth/providers/supabase.py#L120" target="_blank"><Icon icon="github" style="width: 14px; height: 14px;" /></a></sup> | ||
| #### `get_routes` <sup><a href="https://github.com/jlowin/fastmcp/blob/main/src/fastmcp/server/auth/providers/supabase.py#L126" target="_blank"><Icon icon="github" style="width: 14px; height: 14px;" /></a></sup> |
There was a problem hiding this comment.
Manual edit to auto-generated file will be overwritten
docs/python-sdk/** is updated automatically by a bot. This anchor correction will be overwritten on the next bot commit. The fix should emerge naturally from the source code change, not be patched here manually.
As per coding guidelines, "Do not manually modify docs/python-sdk/** - a bot automatically updates these files via commits added to PRs."
1 participant
RemoteAuthProvider subclasses that delegate to external authorization servers (AuthKit, Supabase, Scalekit) weren't validating the JWT
audclaim, which means tokens issued by a shared authorization server could be replayed across different resource servers. This adds audience validation where possible and documents the limitation where it isn't.Scalekit is the cleanest fix — the
resource_idis already available and Scalekit puts it in the JWT audience, so we just wire it through to JWTVerifier. AuthKit and Supabase both use static audience values (client_idand"authenticated"respectively) because neither supports RFC 8707 resource indicators, so we validate what we can and emit a startup warning about the limitation.Also updates the AuthKit and Supabase integration docs — AuthKit gets a note about the DCR
token_endpoint_auth_methodworkaround, and Supabase gets the consent UI requirement documented properly.