fix(oauth-proxy): handle MCP servers without Protected Resource Metadata#2124
Conversation
🧪 BenchmarkShould we run the MCP Gateway benchmark for this PR? React with 👍 to run the benchmark.
Benchmark will run on the next push after you react. |
Release OptionsShould a new version be published when this PR is merged? React with an emoji to vote on the release type:
Current version: Deployment
|
There was a problem hiding this comment.
1 issue found across 3 files
Prompt for AI agents (all issues)
Check if these issues are valid — if so, understand the root cause of each and fix them.
<file name="apps/mesh/src/api/app.ts">
<violation number="1" location="apps/mesh/src/api/app.ts:229">
P1: Fallback to origin root doesn't trigger when Protected Resource Metadata exists but has empty/missing `authorization_servers`. If `resourceRes.ok` is true but `authorization_servers` is undefined or empty, the code returns 404 instead of falling back. Consider restructuring to match `getOriginAuthServer()` in `oauth-proxy.ts` which falls through to the fallback when `authServer` is falsy.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
Servers like Apify support OAuth but don't implement RFC 9728 Protected Resource Metadata (/.well-known/oauth-protected-resource returns 404). They do expose /.well-known/oauth-authorization-server at the root and return WWW-Authenticate header on 401 responses. This commit adds fallback handling: 1. protectedResourceMetadataHandler: When origin returns 404 for the metadata endpoint, check if it supports OAuth via WWW-Authenticate header and generate synthetic metadata pointing to our proxy. 2. getOriginAuthServer: When Protected Resource Metadata is unavailable or has empty authorization_servers, fall back to the origin's root. 3. /oauth-proxy/:connectionId/* handler: Same fallback to origin root when Protected Resource Metadata doesn't exist. Added tests: - Synthetic metadata generation when origin supports OAuth via WWW-Authenticate - 404 passthrough when origin doesn't support OAuth at all - Fallback to origin root for auth server discovery
e0b734f to
90b3cb2
Compare
There was a problem hiding this comment.
1 issue found across 1 file (changes from recent commits).
Prompt for AI agents (all issues)
Check if these issues are valid — if so, understand the root cause of each and fix them.
<file name="apps/mesh/src/api/routes/oauth-proxy.ts">
<violation number="1" location="apps/mesh/src/api/routes/oauth-proxy.ts:332">
P2: Duplicate constant definition. `RETRY_STATUSES` and `NO_METADATA_STATUSES` contain identical values and serve the same purpose. Consider extracting this to a shared module-level constant to avoid maintenance issues if the list needs updating.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
| // If origin returns 404, 401, or 406, check if it still supports OAuth via WWW-Authenticate | ||
| // Many servers (like Apify, Grain) support OAuth but don't implement RFC 9728 metadata | ||
| // 406 is included because some servers return 406 Not Acceptable for .well-known paths | ||
| const NO_METADATA_STATUSES = [404, 401, 406]; |
There was a problem hiding this comment.
P2: Duplicate constant definition. RETRY_STATUSES and NO_METADATA_STATUSES contain identical values and serve the same purpose. Consider extracting this to a shared module-level constant to avoid maintenance issues if the list needs updating.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/mesh/src/api/routes/oauth-proxy.ts, line 332:
<comment>Duplicate constant definition. `RETRY_STATUSES` and `NO_METADATA_STATUSES` contain identical values and serve the same purpose. Consider extracting this to a shared module-level constant to avoid maintenance issues if the list needs updating.</comment>
<file context>
@@ -323,9 +326,11 @@ const protectedResourceMetadataHandler = async (c: {
+ // If origin returns 404, 401, or 406, check if it still supports OAuth via WWW-Authenticate
+ // Many servers (like Apify, Grain) support OAuth but don't implement RFC 9728 metadata
+ // 406 is included because some servers return 406 Not Acceptable for .well-known paths
+ const NO_METADATA_STATUSES = [404, 401, 406];
+ if (!response.ok && NO_METADATA_STATUSES.includes(response.status)) {
const wwwAuth = await checkOriginSupportsOAuth(connectionUrl);
</file context>
✅ Addressed in cc5d6b2
Servers like Apify support OAuth but don't implement RFC 9728 Protected Resource Metadata (/.well-known/oauth-protected-resource returns 404). They do expose /.well-known/oauth-authorization-server at the root and return WWW-Authenticate header on 401 responses.
This commit adds fallback handling:
protectedResourceMetadataHandler: When origin returns 404 for the metadata endpoint, check if it supports OAuth via WWW-Authenticate header and generate synthetic metadata pointing to our proxy.
getOriginAuthServer: When Protected Resource Metadata is unavailable or has empty authorization_servers, fall back to the origin's root.
/oauth-proxy/:connectionId/* handler: Same fallback to origin root when Protected Resource Metadata doesn't exist.
Added tests:
What is this contribution about?
Screenshots/Demonstration
Review Checklist
Summary by cubic
Makes the OAuth proxy work with MCP servers that support OAuth but don’t expose RFC 9728 Protected Resource Metadata (e.g., Apify). Adds fallback discovery and synthetic metadata so auth flows still work.
Written for commit ecfcce8. Summary will update on new commits.
Note
Improves OAuth compatibility with MCP servers that don’t expose RFC 9728 metadata.
NO_METADATA_STATUSESand updatesfetchProtectedResourceMetadatato try multiple well-known paths and treat 404/401/406 as "no metadata"checkOriginSupportsOAuthto detect OAuth support viaWWW-Authenticateand generate synthetic resource metadata pointing to our proxygetOriginAuthServerto fall back to origin root whenauthorization_serversis missing/empty/oauth-proxy/:connectionId/*handling inapp.tsto use the new fallback discoveryresourceandauthorization_serversto proxy endpointsWritten by Cursor Bugbot for commit ecfcce8. This will update automatically on new commits. Configure here.