-
Notifications
You must be signed in to change notification settings - Fork 1.6k
v2: fix simpleStreamableHttp auth example for inspector connect flow
#1427
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
v2: fix simpleStreamableHttp auth example for inspector connect flow
#1427
Conversation
|
| Name | Type |
|---|---|
| @modelcontextprotocol/client | Patch |
Click here to learn what changesets are, and how to add one.
Click here if you're a maintainer who wants to add a changeset to this PR
@modelcontextprotocol/client
@modelcontextprotocol/server
@modelcontextprotocol/express
@modelcontextprotocol/hono
@modelcontextprotocol/node
commit: |
cliffhall
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A couple of extra headers the MCP client needs to see. And explicit "*" origin.
Co-authored-by: Cliff Hall <[email protected]>
pcarleton
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These changes look good to me!
wondering if an optional conformance test for "Browser client compatibility" would be helpful to help highlight when servers don't have this set
cliffhall
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Woops. Left off a comma in the suggestion
examples/shared/src/authServer.ts
Outdated
| }); | ||
| authApp.use( | ||
| cors({ | ||
| exposedHeaders: ['WWW-Authenticate'] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| exposedHeaders: ['WWW-Authenticate'] | |
| exposedHeaders: ['WWW-Authenticate'], |
@pcarleton I believe so, yes! It'll be very helpful to nail these scenarios down, so they're kind of set in stone |
…KKonstantinov/typescript-sdk into feature/v2-example-auth-fixes-inspector
This PR fixes OAuth authentication flow issues in the better-auth example implementation, enabling compatibility with MCP Inspector in both Direct and Proxy modes.
Thanks @cliffhall for reporting this!
Motivation and Context
The better-auth OAuth implementation in the examples had several issues preventing it from working correctly with MCP clients:
/.well-known/oauth-protected-resourceinstead of RFC 9728-compliant/.well-known/oauth-protected-resource/mcp(per MCP Authorization Spec)erroranderror_descriptionfields that MCP Inspector's proxy expectsHow Has This Been Tested?
Breaking Changes
No breaking changes. The
createProtectedResourceMetadataRouter()function now accepts an optionalresourcePathparameter (defaults to'/mcp'), but existing calls without the parameter will continue to work.Types of changes
Checklist
Additional context
Key changes:
CORS Configuration:
corsmiddleware to MCP server with exposed headers forWWW-AuthenticateandMcp-Session-Idcorsmiddleware to auth server for all OAuth endpointsRFC 9728 Compliance:
createProtectedResourceMetadataRouter(resourcePath)now serves metadata at the path-specific URL (e.g.,/.well-known/oauth-protected-resource/mcp)WWW-Authenticate Header:
Bearer error="invalid_token", error_description="...", resource_metadata="..."Debug Logging:
--dangerous-logging-enabledCLI flag to enable verbose better-auth request/response loggingTypeScript Configuration:
declarationanddeclarationMapinexamples/shared/tsconfig.jsonto avoid TS4058 error with better-auth's internalMCPOptionstype - it's not exported out of thebetter-authpackage (TBD: Raise an issue on better-auth). Since we are not building and publishing this package, it should be fine to disable these for the example package.Dependency Upgrades:
better-authto latest versionbetter-sqlite3to latest version