Skip to content

ClientAuthenticator ignores token_endpoint_auth_method="none" when client_secret is stored #1842

Description

@aiwebb

Initial Checks

Description

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

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

Activity

  1. added a commit that references this issue on Jan 9, 2026
    68fa608
  2. haoxuw commented on May 28, 2026

    @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

    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:

    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

  3. aiwebb commented on May 28, 2026

    @aiwebb
    Author

    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.

  4. haoxuw commented on May 28, 2026

    @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

  5. aiwebb commented on May 28, 2026

    @aiwebb
    Author

    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.

  6. haoxuw commented on May 28, 2026

    @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.

  7. aiwebb commented on May 28, 2026

    @aiwebb
    Author

    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.

  8. added
    bugSomething isn't working
    P2Moderate issues affecting some users, edge cases, potentially valuable feature
    needs confirmationNeeds confirmation that the PR is actually required or needed.
    authIssues and PRs related to Authentication / OAuth
    needs decisionIssue is actionable, needs maintainer decision on whether to implement
    on Aug 14, 2026
  9. added 2 commits that reference this issue on Sep 10, 2026
    fc762df
    3d7a908
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    P2Moderate issues affecting some users, edge cases, potentially valuable featureauthIssues and PRs related to Authentication / OAuthbugSomething isn't workingneeds confirmationNeeds confirmation that the PR is actually required or needed.needs decisionIssue is actionable, needs maintainer decision on whether to implement

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions