fix: respect token_endpoint_auth_method=none when client_secret exists

Closed
JA
7d ago
+61-2

Summary

Fix ClientAuthenticator to honour token_endpoint_auth_method="none" even when a client_secret is stored against the client.

Fixes #1842

Problem

A client that registers with token_endpoint_auth_method="none" is a public client, so authenticate_request correctly leaves request_client_secret as None for it. The stored-secret check that follows never looked at the auth method:

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

So a secret left on the client record by an earlier confidential registration makes the token request fail with 401 invalid_client / "Client secret is required", even though the client declared itself public.

RFC 6749 section 2.1 says the authorization server SHOULD NOT make assumptions about the client type. Preferring a leftover stored secret over the client's own declaration does exactly that.

Fix

if client.token_endpoint_auth_method != "none" and client.client_secret:

Note: the and client.client_secret half is logically redundant against the misconfiguration guard added just above it, which already rejects a non-"none" client with no stored secret. It is kept because the hmac.compare_digest(client.client_secret.encode(), ...) call below relies on that narrowing.

Tests

Added test_none_auth_method_ignores_stored_client_secret in tests/server/mcpserver/auth/test_auth_integration.py, next to the existing test_none_auth_method_public_client. It registers a "none" client, puts a secret on the stored record, and asserts the token request still succeeds. Verified it fails (401 != 200) without the fix.

  • 43 passed in tests/server/mcpserver/auth/test_auth_integration.py
  • 365 passed across tests/server/auth, tests/server/mcpserver/auth, tests/interaction/auth, tests/client/test_auth.py
  • 5970 passed, 8 skipped, 1 xfailed for the full suite
  • ruff check, ruff format --check, and pyright all clean

AI assistance

AI-assisted, per CONTRIBUTING.md. I hit this bug, reviewed the change and the test, and confirmed the test fails without the fix. Questions get answered by me.