Skip to content

Conversation

@jayhemnani9910
Copy link
Contributor

Summary

Fix ClientAuthenticator to properly honor token_endpoint_auth_method="none" even when a client_secret is stored for the client.

Problem

When a client has token_endpoint_auth_method="none" but also has a client_secret stored (e.g., from a previous configuration or edge case), the authenticator incorrectly raises "Client secret is required" error.

The logic was:

  1. token_endpoint_auth_method="none" → skip extracting credentials from request ✓
  2. client.client_secret exists → raise error if no credentials provided ✗

If token_endpoint_auth_method="none" is set, it should never check for a client_secret on the request.

Solution

Add a check for token_endpoint_auth_method != "none" before validating the client_secret requirement:

# Before
if client.client_secret:
    if not request_client_secret:
        raise AuthenticationError("Client secret is required")

# After
if client.token_endpoint_auth_method != "none" and client.client_secret:
    if not request_client_secret:
        raise AuthenticationError("Client secret is required")

Testing

- Added integration test test_none_auth_method_ignores_stored_client_secret that:
  a. Registers a client with token_endpoint_auth_method="none"
  b. Manually adds a client_secret to the stored client
  c. Verifies token request succeeds without providing the secret
- All 43 auth integration tests pass

Fixes #1842

When token_endpoint_auth_method is set to "none", the ClientAuthenticator
should not require a client_secret on the request, even if a client_secret
is stored for that client. This can happen if a client was previously
registered with a secret but later changed to public authentication.

The fix adds a check for token_endpoint_auth_method != "none" before
validating the client_secret requirement.

Fixes modelcontextprotocol#1842

Github-Issue: modelcontextprotocol#1842
Reported-by: aiwebb
Copilot AI review requested due to automatic review settings January 9, 2026 19:36
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR fixes a bug in client authentication where token_endpoint_auth_method="none" was not properly honored when a client had a stored client_secret. The fix ensures that clients using the "none" authentication method can successfully authenticate without providing credentials, regardless of whether they have a secret stored.

  • Modified authentication logic to respect token_endpoint_auth_method="none" even when client_secret exists in storage
  • Added integration test to verify the fix for the edge case scenario

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File Description
src/mcp/server/auth/middleware/client_auth.py Added condition to skip client secret validation when token_endpoint_auth_method="none"
tests/server/fastmcp/auth/test_auth_integration.py Added integration test verifying that "none" auth method works even with a stored client secret

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

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.

1 participant