ClientAuthenticator ignores token_endpoint_auth_method="none" when client_secret is stored

[aiwebb]

Initial Checks

• I confirm that I'm using the latest version of MCP Python SDK
• I confirm that I searched for my issue in https://github.com/modelcontextprotocol/python-sdk/issues before opening this issue

Description

ClientAuthenticator behavior is only partially honoring token_endpoint_auth_method="none":

python-sdk/src/mcp/server/auth/middleware/client_auth.py

Lines 102 to 104 in 6b69f63

	if client.client_secret: # pragma: no branch
	if not request_client_secret:
	raise AuthenticationError("Client secret is required") # pragma: no cover

Essentially:

• token_endpoint_auth_method="none" => skip extracting credentials from the request (correct)
• client.client_secret exists => raise error if no credentials were extracted from the request (incorrect)

If token_endpoint_auth_method="none" is set, it should never be checking for a client_secret value on the request, regardless of whether a secret has ever been generated for the client.

Suggested fix is to condition the client.client_secret check on token_endpoint_auth_method not being none:

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

Example Code

Python & MCP Python SDK

1.25.0

[haoxuw]

Investigation

From RFC 7591 (Section 2)
RFC 7591 (https://datatracker.ietf.org/doc/html/rfc7591#section-2) defines the token_endpoint_auth_method metadata:

token_endpoint_auth_method:
...
"none": The client is a public client as defined in OAuth 2.0, Section 2.1, and does not have a client secret.

This means token_endpoint_auth_method can only be set to "none", when the client "does not have a client secret".

Thusly an assert should trigger, predate the scenario pointed out by @aiwebb the Author of this issue, regarding:

If token_endpoint_auth_method="none" is set, it should never be checking for a client_secret value on the request, regardless of whether a secret has ever been generated for the client.

Meanwhile, the existing code actually (un)intentionally give an AuthenticationError, when both token_endpoint_auth_method="none" and client.client_secret exists, on line

python-sdk/src/mcp/server/auth/middleware/client_auth.py

Line 109 in 3eb5799

raise AuthenticationError("Invalid client_secret")

I propose a fix that improve the error message, where we add

raise AuthenticationError("Require valid auth method, with client secret")

after:

python-sdk/src/mcp/server/auth/middleware/client_auth.py

Lines 92 to 94 in 6b69f63

	elif client.token_endpoint_auth_method == "none":
	request_client_secret = None

Summary

Keep the existing logic

When the pre-configured client.client_secret is set, we disallow token_endpoint_auth_method == "none" , and disregard client_secret from request (if any)

Improved Message

For the exact scenario pointed out by the Author, change message from "Client secret is required" to"Require valid auth method, with client secret"

I considered a concern where, the new AuthenticationError may expose whether the server has pre-configured client_secret, thus decrease security. But even with the current implementation, a potential attacker could still find out that info, by playing around requests. So there's no new substantial risks.

Next steps

I'll create a PR

[aiwebb]

Thanks @haoxuw. The RFC is admittedly pretty rough here (the linked section doesn't actually relate to the content, and the sister links point to nonexistent sections), so I don't think it's on you per se, but I think you're reading this incorrectly:

"none": The client is a public client as defined in OAuth 2.0, Section 2.1, and does not have a client secret.

I don't think this means, as I take your reading to be, that clients are forbidden from using token_endpoint_auth_method="none" if they've ever had a secret generated / if the server is currently storing an old secret in client.client_secret.

During the context of a flow in which they're requesting token_endpoint_auth_method="none" without a secret, the client does not "have" a client secret. Whether or not they have ever had a secret in previous auth flows, which may or may not be stored by the server (as client.client_secret, in this case), is immaterial – prior use of a different auth method shouldn't preclude the use of this one.

[haoxuw]

Hi @aiwebb , thanks for the timely reply.

Needs you clarification

I see the confusion. Under this link Section 2, SPEC oftoken_endpoint_auth_method was defined under that section, where the usage of "none" is defined as:

"none": The client is a public client as defined in OAuth 2.0, Section 2.1, and does not have a client secret.

You mentioned:

the linked section doesn't actually relate to the content, and the sister links point to nonexistent sections

Could you please elaborate on what was not related, or nonexistent? This is a direct quote of the documentation that defines token_endpoint_auth_method, and I just tried -- all links are reachable. Please ignore Section 2.1, it is not relevant to our discussion, I only included it because it's a direct quote of the SPEC text.

Point of misaligned interpretation

I don't think this means, as I take your reading to be, that clients are forbidden from using token_endpoint_auth_method="none" if they've ever had a secret generated / if the server is currently storing an old secret in client.client_secret

I think this is exactly where our understanding differ. The SPEC doc essentially says:
Use "none", only when The client does not have a client secret. Otherwise the behavior is undefined, and we should give Auth Error

Note the secret is not referring to the secret from request, but rather the secret from client's configuration (via auth authority, like Okta or AWS Cognito or Google GCI etc).

I am confident when The client has a client secret, token_endpoint_auth_method == "none" should give an Auth Error

[aiwebb]

Side note re: links – it's referencing the parent RFC's 2.1 / 2.1.3 sections – that's why the in-spec link to 2.1 didn't make sense / the adjacent links to 2.1.3 are dead. IETF's page is auto-injecting links to the same RFC when it's actually referencing a different RFC.

Per RFC 7459 2.1, the distinguishing factor between a confidential client and a public client is whether the client is "capable of maintaining the confidentiality of their credentials". This is conditional, not immutable. A client who, for whatever reason, is no longer capable of maintaining the confidentiality of their credentials is certainly allowed to begin acting as a public client.

Note that this section also explicitly warns servers not to make assumptions about the client type, and thus the server should not be overriding (or otherwise refusing to serve) explicit client requests for a public-client authentication method based on the server's assumptions about client type (which, in this case, are fueled by its own internal cached state, e.g. client.client_secret):

The authorization server SHOULD NOT make assumptions about the client type.

[haoxuw]

Hi @aiwebb , regarding:

that's why the in-spec link to 2.1 didn't make sense / the adjacent links to 2.1.3 are dead

I don't see any mention of 2.1.3, in this issue tab, in the codebase, nor in the SPEC document https://datatracker.ietf.org/doc/html/rfc7591#section-2. Nor have I found any link that are dead in the doc.

that's why the in-spec link to 2.1 didn't make sense

I can see the conflicting philosophical design. However, if the SPEC requires change, I think we should file an issue and change the SPEC, instead of implementing logic that's diverged from the primary definition of the parameter.

[aiwebb]

Let me clarify the RFC links issue so we can set it aside. You linked and quoted RFC 7591 §2. The relevant part is this:

token_endpoint_auth_method

String indicator of the requested authentication method for the
token endpoint. Values defined by this specification are:

• "none": The client is a public client as defined in OAuth 2.0, Section 2.1, and does not have a client secret.
• "client_secret_post": The client uses the HTTP POST parameters as defined in OAuth 2.0, Section 2.3.1.
• "client_secret_basic": The client uses HTTP Basic as defined in OAuth 2.0, Section 2.3.1.

The text refers to sections §2.1 and §2.3.1 of the OAuth 2.0 spec (RFC 6749), but links to §2.1 and §2.3.1 of the DCR spec (RFC 7591), which are unrelated and nonexistent, respectively. As mentioned above, that's presumably an overzealous auto-linker on the IETF platform.

Side note: I appreciate the edits to your comment. FWIW, you've been talking to a human all along. I don't use AI-generated answers without explicit disclosure, and expect the same.

---

Back to the issue at hand – per RFC 6749 §2.1:

The authorization server SHOULD NOT make assumptions about the client type.

Right now, that's exactly what this code is doing: the server is assuming the current client type is confidential, and that token_endpoint_auth_method="none" is logically invalid, based on prior flows that resulted in client.client_secret being stored. It's effectively ignoring the client's own declaration that it is operating in a public-client capacity.

This explicitly violates 6749 §2.1.