feat: add RFC 8707 Resource Indicators support#879
Conversation
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ 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 |
|
|
Implements RFC 8707 (Resource Indicators for OAuth 2.0) by parsing the standardized "resource" form parameter alongside the existing non-standard "audience" parameter. The "resource" values are validated per RFC 8707 §2 (absolute URI, no fragment) and merged into the RequestedAudience list, so they flow through fosite's existing audience binding pipeline (DefaultAudienceMatchingStrategy and the per-flow audience checks) and end up in the issued token's aud claim with no additional plumbing. Motivation: the 2026 MCP (Model Context Protocol) spec mandates RFC 8707 to prevent confused-deputy attacks between MCP servers. Several public AS implementations now support it; this aligns fosite with the broader OAuth 2.1 / MCP ecosystem. Changes: audience_strategy.go merges resource with audience and adds ValidateResourceIndicators helper; access_request_handler.go validates resource before populating RequestedAudience; new unit and integration tests cover merging, de-dup, and invalid input cases. Backward compatibility: requests with only the "audience" parameter behave exactly as before. The "resource" parameter is opt-in. Signed-off-by: cohendvir <dvir@honeybook.com>
6560f1a to
54bfd0b
Compare
|
Shouldn't this be in the authorize code / implicit auth handlers and refresh / client credentials / device code access handlers? |
|
Actually looking more closely it's similar to refresh flow, the values requested on the auth endpoint restrict what can be requested on the token endpoint, same for the original request in refresh flows.
|
…267) End-to-end testing v0.69.0 reveals: gateway rejects every OAuth token because their `aud` claim is empty array. Logs: level=INFO msg="mcppublic.OAuth: audience mismatch" sub=c60a7665-... want="https://mcp.agent.cs.ac.cn/mcp" got=[] Root cause: Hydra v2.x (via ory/fosite v0.x) does NOT implement RFC 8707 Resource Indicators. The `resource=https://...` query parameter that spec-compliant MCP clients send on /oauth2/auth is silently dropped. The issued token therefore has no audience. Tracking PR: ory/fosite#879 (opened 2026-05-25, still in draft, CLA blocked, scope concerns from maintainer). Unblocking this upstream is months away. Workaround: when the introspected token's `aud` is empty, accept it with a warning. When `aud` IS present, mismatch is still a hard fail (so any token that DOES bind to a resource still gets proper enforcement). Safe today: single-tenant Hydra (this gateway is the only MCP server in the cluster). Risk only emerges if we add a second MCP server on the same Hydra — at that point either delete this empty-aud branch (fosite #879 merged by then) or patch the consent handler to force GrantAccessTokenAudience. 10 existing OAuth resolver tests still pass; new test TestOAuthResolver_EmptyAudienceAccepted pins the new behavior. Co-authored-by: mryao <git@mryao.org> Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Security review of #267 (empty-aud fail-open) noted the comment should enumerate the safety preconditions and the failure mode that triggers "must remove this branch." Expand the comment to spell out: 1. Why single-tenant Hydra makes it safe today (one MCP gateway). 2. DCR being off (#258) so attackers can't mint mcp:* tokens from unauthorized clients. 3. The agentserver-mcp client's fixed audience list bounds the non-empty-aud case to our gateway URL. And the action item: Before adding a second MCP server on the same Hydra, DELETE the empty-aud branch. Either ory/fosite#879 has merged (strict aud becomes reliable) or patch the consent handler to hard-inject GrantAccessTokenAudience for mcp:* scopes. No behavioral change. Co-authored-by: mryao <git@mryao.org> Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Adds support for RFC 8707 Resource Indicators in fosite.