fix(auth): stop leaking oauth client secrets (adopted from #327) - #333
Open
K4bain wants to merge 1 commit into
Open
fix(auth): stop leaking oauth client secrets (adopted from #327)#333K4bain wants to merge 1 commit into
K4bain wants to merge 1 commit into
Conversation
…ation The fake dynamic client registration endpoint echoed back the pre-registered client_secret to any caller, exposing it to every MCP client. Register the caller as a public client instead: omit client_secret from the response and force token_endpoint_auth_method to none. Adopted from tadata-org#327 (original patch by @failsafesecurity).
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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.
Adopted from #327 (original patch by @failsafesecurity) — rebased onto current main and verified.
Why this is needed
setup_oauth_fake_dynamic_register_endpoint exists because npx mcp-remote (the de-facto MCP client) requires a dynamic client registration endpoint. Instead of implementing RFC 7591, it just echoes back the pre-registered client secret to whichever client calls it — meaning the secret the server owner configured for their OAuth provider is handed out to every MCP client that hits /oauth/register.
This PR registers the caller as a public client instead:
The client_secret parameter of the setup function is kept (unchanged signature) since it is still validated upstream by AuthConfig; it is simply no longer disclosed.
Verification
Credit to @failsafesecurity for the original report and patch in #327.