Skip to content

Fix OAuth token_endpoint_auth_method to comply with OpenID Connect spec #660

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

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

gavi
Copy link

@gavi gavi commented Jul 30, 2025

This PR fixes the OAuth client authentication method to comply with the OpenID Connect specification by changing token_endpoint_auth_method from "none" to "client_secret_basic".

Motivation and Context

The current implementation hardcodes token_endpoint_auth_method as "none" in the client metadata, which violates the OpenID Connect specification. According to the spec, the default value should be "client_secret_basic". This incorrect value causes dynamic client registration to fail during the token generation phase because:

  1. Clients register with token_endpoint_auth_method: "none"
  2. The authorization server expects proper client authentication during token exchange
  3. The mismatch between registered auth method and actual requirements causes token requests to fail

How Has This Been Tested?

  • Tested OAuth flow with dynamic client registration
  • Verified successful token generation after the fix
  • Confirmed compatibility with OpenID Connect compliant authorization servers

Breaking Changes

This is a non-breaking change that fixes incorrect behavior. Existing implementations that were somehow working with "none" may need to ensure their authorization servers support client_secret_basic authentication, but this is the standard expected behavior.

Types of changes

  • Bug fix (non-breaking change which fixes an issue)

Checklist

  • I have read the [MCP Documentation](https://modelcontextprotocol.io)
  • My code follows the repository's style guidelines
  • New and existing tests pass locally
  • I have added appropriate error handling
  • I have added or updated documentation as needed

Additional context

This change aligns the MCP Inspector with the OpenID Connect Core 1.0 specification (Section 9), which defines client_secret_basic as the default authentication method for confidential clients. The fix ensures proper client authentication during the OAuth 2.0 authorization code flow, preventing token exchange failures that were occurring with the previous "none" setting.

@olaservo olaservo requested a review from pcarleton August 20, 2025 14:30
@pcarleton
Copy link
Member

👋 Hi @gavi -- I disagree about this behavior being incorrect. Inspector is currently set up to be a public client (i.e. no client secret). We could make this configurable if you want work out threading that change through to the UI, but I don't think we want to change the default.

A few notes:

  • MCP is following OAuth 2.1. While MCP supports fetching metadata from openid-configuration as a fallback, we generally don't try to be compliant with the OIDC specification.
  • The linked section 9 says "If no method is registered, the default method is client_secret_basic" -- this means if the client leaves the value blank while registering, it should be assumed that client_secret_basic is being used. This does not mean that a client should always specify that value.
  • it is probably useful to test public and private client setups, so having this be adjustable seems useful.

Are you hitting an issue with a particular authorization server requiring this field be set to that value?

Copy link
Member

@pcarleton pcarleton left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(see previous comment)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants