Skip to content

Commit 9b677bf

Browse files
committed
auth: improve ClientAuthenticator error messaging for confidential clients
1 parent 2472563 commit 9b677bf

2 files changed

Lines changed: 59 additions & 0 deletions

File tree

src/mcp/server/auth/middleware/client_auth.py

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -90,6 +90,8 @@ async def authenticate_request(self, request: Request) -> OAuthClientInformation
9090
request_client_secret = str(raw_form_data)
9191

9292
elif client.token_endpoint_auth_method == "none":
93+
if client.client_secret:
94+
raise AuthenticationError("Require valid auth method, with client secret")
9395
request_client_secret = None
9496
else:
9597
raise AuthenticationError( # pragma: no cover

tests/server/mcpserver/auth/test_auth_integration.py

Lines changed: 57 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1376,6 +1376,63 @@ async def test_none_auth_method_public_client(
13761376
token_response = response.json()
13771377
assert "access_token" in token_response
13781378

1379+
@pytest.mark.anyio
1380+
async def test_none_auth_method_fails_for_confidential_client(
1381+
self,
1382+
test_client: httpx.AsyncClient,
1383+
mock_oauth_provider: MockOAuthProvider,
1384+
pkce_challenge: dict[str, str],
1385+
):
1386+
"""Test that 'none' authentication method fails for confidential clients."""
1387+
# Create a confidential client (has a secret) but try to register with 'none'
1388+
# Actually, if we register with 'none', it won't have a secret.
1389+
# So we register with 'client_secret_post' first.
1390+
client_metadata = {
1391+
"redirect_uris": ["https://client.example.com/callback"],
1392+
"client_name": "Confidential Client",
1393+
"token_endpoint_auth_method": "client_secret_post",
1394+
"grant_types": ["authorization_code", "refresh_token"],
1395+
}
1396+
1397+
response = await test_client.post("/register", json=client_metadata)
1398+
assert response.status_code == 201
1399+
client_info = response.json()
1400+
assert client_info["client_secret"] is not None
1401+
1402+
# Manually change the client's auth method to 'none' in the provider
1403+
# to simulate a configuration conflict or a client trying to downgrade.
1404+
client_obj = await mock_oauth_provider.get_client(client_info["client_id"])
1405+
assert client_obj is not None
1406+
client_obj.token_endpoint_auth_method = "none"
1407+
1408+
auth_code = f"code_{int(time.time())}"
1409+
mock_oauth_provider.auth_codes[auth_code] = AuthorizationCode(
1410+
code=auth_code,
1411+
client_id=client_info["client_id"],
1412+
code_challenge=pkce_challenge["code_challenge"],
1413+
redirect_uri=AnyUrl("https://client.example.com/callback"),
1414+
redirect_uri_provided_explicitly=True,
1415+
scopes=["read", "write"],
1416+
expires_at=time.time() + 600,
1417+
)
1418+
1419+
# Token request using 'none' method (no secret provided)
1420+
response = await test_client.post(
1421+
"/token",
1422+
data={
1423+
"grant_type": "authorization_code",
1424+
"client_id": client_info["client_id"],
1425+
"code": auth_code,
1426+
"code_verifier": pkce_challenge["code_verifier"],
1427+
"redirect_uri": "https://client.example.com/callback",
1428+
},
1429+
)
1430+
1431+
assert response.status_code == 401
1432+
error_response = response.json()
1433+
assert error_response["error"] == "invalid_client"
1434+
assert "Require valid auth method, with client secret" in error_response["error_description"]
1435+
13791436

13801437
class TestAuthorizeEndpointErrors:
13811438
"""Test error handling in the OAuth authorization endpoint."""

0 commit comments

Comments
 (0)