Fix OAuth token endpoint authentication method regression (issue #726) #736
+129
−4
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…lcontextprotocol#726) This commit resolves a regression introduced in PR modelcontextprotocol#715 where the OAuth flow always chose client_secret_post regardless of the server's token_endpoint_auth_methods_supported setting. ## Problem - PR modelcontextprotocol#715 removed client_secret from sessionStorage for security reasons - This broke the MCP SDK's ability to determine the correct authentication method - OAuth flows with servers requiring client_secret_basic would fail ## Solution - Added temporary in-memory storage for client_secret during OAuth flow - client_secret is stored temporarily when saveClientInformation() is called - client_secret is provided to the SDK when clientInformation() is called - client_secret is cleared from memory after successful token exchange or clear() - client_secret is never persisted to sessionStorage (maintains security) ## Testing - Added comprehensive tests for temporary client_secret handling - Verified client_secret is available during OAuth flow - Verified client_secret is not stored in sessionStorage - Verified client_secret is cleared after token exchange - All existing tests continue to pass 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <[email protected]>
cliffhall
reviewed
Aug 20, 2025
There was a problem hiding this comment.
Tested locally, using the Typescript SDK example.
tsx --watch src/examples/server/simpleStreamableHttp.ts --oauthIn the AS metadata, token_endpoint_auth_methods_supported is client_secret_post
However, I did not see the client_secret in the subsequent POST to the /token endpoint.
On the upside, it does not appear that client_secret was stored in localstorage.
Ultimately, connection was successful. However I'm not certain that the token endpoint was enforcing the requirement for a client_secret to appear in that post.
I went to the original issue #726 and used the Flask server that was failing to test with. See my comment there for details.
|
Closing this for now since the original issue is still reproducible with this branch. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Resolves a regression introduced in PR #715 and reported in #726 where the OAuth flow always chose client_secret_post regardless of the server's token_endpoint_auth_methods_supported setting.
Motivation and Context
How Has This Been Tested?
Added tests but still needs testing with real flow through UI.
Breaking Changes
No
Types of changes
Checklist
Additional context