-
Notifications
You must be signed in to change notification settings - Fork 1.6k
fix: handle OAuth error responses returned with HTTP 200 status #1343
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
base: main
Are you sure you want to change the base?
fix: handle OAuth error responses returned with HTTP 200 status #1343
Conversation
Some OAuth servers (e.g., GitHub) return error responses with HTTP 200 status instead of 4xx. The SDK now checks for an `error` field in the JSON response before attempting to parse it as tokens. This provides users with meaningful error messages like: "The client_id and/or client_secret passed are incorrect." Instead of confusing Zod validation errors about missing access_token. Fixes modelcontextprotocol#1342 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
🦋 Changeset detectedLatest commit: af56916 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
@modelcontextprotocol/client
@modelcontextprotocol/server
@modelcontextprotocol/express
@modelcontextprotocol/hono
@modelcontextprotocol/node
commit: |
- Fix TypeScript error by properly typing json as unknown - Add changeset for the patch release Fixes modelcontextprotocol#1342 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
|
Thanks! One small suggestion - check for error only if token parsing fails, keeps the happy path clean. Pushing a commit with this approach. |
Co-authored-by: Paul Carleton <paulcarletonjr@gmail.com>
|
@pcarleton It's waiting for review for a while. Should we still merge this given it won't make any difference unless GitHub stops returning HTTP 200 for what should be an error? |
Summary
Some OAuth servers (e.g., GitHub) return error responses with HTTP 200 status instead of 4xx. This PR adds a check for the
errorfield in the JSON response before attempting to parse it as tokens.Problem
When GitHub's OAuth token endpoint returns an error like:
{ "error": "incorrect_client_credentials", "error_description": "The client_id and/or client_secret passed are incorrect." }The SDK's
executeTokenRequest()only checksresponse.ok(which istruefor HTTP 200), then tries to parse the response asOAuthTokensSchema, resulting in a confusing Zod validation error:Solution
Check for the
errorfield in the JSON response before attempting to parse as tokens. This surfaces the actual OAuth error message to users:Test plan
Closes #1342
🤖 Generated with Claude Code