From dcfaefb4b06f75023b21611bb5fcfe1cadcf7a08 Mon Sep 17 00:00:00 2001 From: Nik <84yk8btb9f@privaterelay.appleid.com> Date: Wed, 21 Oct 2026 07:27:18 +0300 Subject: [PATCH] fix(auth): align /token and /revoke error codes with RFC 6749 and RFC 7009 (#3723) Closes #3723. --- src/mcp/server/auth/handlers/revoke.py | 13 +- src/mcp/server/auth/handlers/token.py | 40 ++++-- tests/server/auth/test_error_handling.py | 124 +++++++++++++++++++ tests/server/auth/test_identity_assertion.py | 2 +- 4 files changed, 167 insertions(+), 12 deletions(-) diff --git a/src/mcp/server/auth/handlers/revoke.py b/src/mcp/server/auth/handlers/revoke.py index 4efd154001..62d8698dcb 100644 --- a/src/mcp/server/auth/handlers/revoke.py +++ b/src/mcp/server/auth/handlers/revoke.py @@ -24,7 +24,7 @@ class RevocationRequest(BaseModel): class RevocationErrorResponse(BaseModel): - error: Literal["invalid_request", "unauthorized_client"] + error: Literal["invalid_request", "invalid_client", "unsupported_token_type", "unauthorized_client"] error_description: str | None = None @@ -35,15 +35,24 @@ class RevocationHandler: async def handle(self, request: Request) -> Response: """Handler for the OAuth 2.0 Token Revocation endpoint.""" + auth_header = request.headers.get("Authorization", "") + headers = { + "Cache-Control": "no-store", + "Pragma": "no-cache", + } + if auth_header.lower().startswith("basic "): + headers["WWW-Authenticate"] = 'Basic realm="OAuth"' + try: client = await self.client_authenticator.authenticate_request(request) except AuthenticationError as e: # pragma: no cover return PydanticJSONResponse( status_code=401, content=RevocationErrorResponse( - error="unauthorized_client", + error="invalid_client", error_description=e.message, ), + headers=headers, ) try: diff --git a/src/mcp/server/auth/handlers/token.py b/src/mcp/server/auth/handlers/token.py index 00df115573..53287fad52 100644 --- a/src/mcp/server/auth/handlers/token.py +++ b/src/mcp/server/auth/handlers/token.py @@ -101,26 +101,45 @@ def response(self, obj: TokenSuccessResponse | TokenErrorResponse): ) async def handle(self, request: Request): + auth_header = request.headers.get("Authorization", "") try: client_info = await self.client_authenticator.authenticate_request(request) except AuthenticationError as e: # Authentication failures should return 401 + headers = { + "Cache-Control": "no-store", + "Pragma": "no-cache", + } + if auth_header.lower().startswith("basic "): + headers["WWW-Authenticate"] = 'Basic realm="OAuth"' + return PydanticJSONResponse( content=TokenErrorResponse( error="invalid_client", error_description=e.message, ), status_code=401, - headers={ - "Cache-Control": "no-store", - "Pragma": "no-cache", - }, + headers=headers, + ) + + form_data = await request.form() + form_dict = dict(form_data) + grant_type = form_dict.get("grant_type") + + supported_grants = {"authorization_code", "refresh_token"} + if self.identity_assertion_enabled: + supported_grants.add("urn:ietf:params:oauth:grant-type:jwt-bearer") + + if grant_type and str(grant_type) not in supported_grants: + return self.response( + TokenErrorResponse( + error="unsupported_grant_type", + error_description=f"Unsupported grant type: {grant_type}", + ) ) try: - form_data = await request.form() - # TODO(Marcelo): Can someone check if this `dict()` wrapper is necessary? - token_request = token_request_adapter.validate_python(dict(form_data)) + token_request = token_request_adapter.validate_python(form_dict) except ValidationError as validation_error: return self.response( TokenErrorResponse( @@ -132,8 +151,11 @@ async def handle(self, request: Request): if token_request.grant_type not in client_info.grant_types: return self.response( TokenErrorResponse( - error="unsupported_grant_type", - error_description=(f"Unsupported grant type (supported grant types are {client_info.grant_types})"), + error="unauthorized_client", + error_description=( + f"Client is not authorized to use grant type {token_request.grant_type} " + f"(authorized grant types are {client_info.grant_types})" + ), ) ) diff --git a/tests/server/auth/test_error_handling.py b/tests/server/auth/test_error_handling.py index f13f23ea33..c56ddc8bf4 100644 --- a/tests/server/auth/test_error_handling.py +++ b/tests/server/auth/test_error_handling.py @@ -347,3 +347,127 @@ async def test_oversized_cross_origin_request_gets_413_with_cors_headers(client: ) assert response.status_code == 413 assert response.headers["access-control-allow-origin"] == "*" + + +@pytest.mark.anyio +async def test_rfc6749_token_unauthorized_client(client: httpx2.AsyncClient, oauth_provider: MockOAuthProvider): + """RFC 6749 §5.2: when a grant is supported by the server but not authorized for this client, + the server must return error='unauthorized_client'. + """ + reg_response = await client.post( + "/register", + json={ + "client_name": "Test Client", + "redirect_uris": ["https://client.example.com/callback"], + "grant_types": ["authorization_code"], + "token_endpoint_auth_method": "client_secret_post", + }, + ) + client_data = reg_response.json() + + response = await client.post( + "/token", + data={ + "grant_type": "refresh_token", + "refresh_token": "some-token", + "client_id": client_data["client_id"], + "client_secret": client_data["client_secret"], + }, + ) + assert response.status_code == 400 + assert response.json()["error"] == "unauthorized_client" + + +@pytest.mark.anyio +async def test_rfc6749_token_unsupported_grant_type(client: httpx2.AsyncClient, oauth_provider: MockOAuthProvider): + """RFC 6749 §5.2: when a grant type is not supported by the server, + the server must return error='unsupported_grant_type'. + """ + reg_response = await client.post( + "/register", + json={ + "client_name": "Test Client", + "redirect_uris": ["https://client.example.com/callback"], + "grant_types": ["authorization_code"], + "token_endpoint_auth_method": "client_secret_post", + }, + ) + client_data = reg_response.json() + + response = await client.post( + "/token", + data={ + "grant_type": "client_credentials", + "client_id": client_data["client_id"], + "client_secret": client_data["client_secret"], + }, + ) + assert response.status_code == 400 + assert response.json()["error"] == "unsupported_grant_type" + + +@pytest.mark.anyio +async def test_rfc6749_token_basic_auth_failure_www_authenticate( + client: httpx2.AsyncClient, oauth_provider: MockOAuthProvider +): + """RFC 6749 §5.2: failed client authentication via Authorization header must return 401 + with invalid_client and a matching WWW-Authenticate header. + """ + reg_response = await client.post( + "/register", + json={ + "client_name": "Test Client", + "redirect_uris": ["https://client.example.com/callback"], + "grant_types": ["authorization_code"], + "token_endpoint_auth_method": "client_secret_basic", + }, + ) + client_data = reg_response.json() + + auth_header = "Basic " + base64.b64encode(f"{client_data['client_id']}:wrong_secret".encode()).decode() + response = await client.post( + "/token", + headers={"Authorization": auth_header}, + data={ + "grant_type": "authorization_code", + "code": "any_code", + "client_id": client_data["client_id"], + }, + ) + assert response.status_code == 401 + assert response.json()["error"] == "invalid_client" + assert "www-authenticate" in response.headers + assert response.headers["www-authenticate"].lower().startswith("basic") + + +@pytest.mark.anyio +async def test_rfc7009_revoke_invalid_client_and_www_authenticate( + client: httpx2.AsyncClient, oauth_provider: MockOAuthProvider +): + """RFC 7009 §2.2.1: failed client authentication on /revoke must return error='invalid_client' + (not 'unauthorized_client'), status 401, with matching WWW-Authenticate header. + """ + reg_response = await client.post( + "/register", + json={ + "client_name": "Test Client", + "redirect_uris": ["https://client.example.com/callback"], + "grant_types": ["authorization_code"], + "token_endpoint_auth_method": "client_secret_basic", + }, + ) + client_data = reg_response.json() + + auth_header = "Basic " + base64.b64encode(f"{client_data['client_id']}:wrong_secret".encode()).decode() + response = await client.post( + "/revoke", + headers={"Authorization": auth_header}, + data={ + "token": "any_token", + "client_id": client_data["client_id"], + }, + ) + assert response.status_code == 401 + assert response.json()["error"] == "invalid_client" + assert "www-authenticate" in response.headers + assert response.headers["www-authenticate"].lower().startswith("basic") diff --git a/tests/server/auth/test_identity_assertion.py b/tests/server/auth/test_identity_assertion.py index 9ce12cb8fb..00e196a9dd 100644 --- a/tests/server/auth/test_identity_assertion.py +++ b/tests/server/auth/test_identity_assertion.py @@ -309,7 +309,7 @@ async def test_client_without_the_grant_registered_is_rejected( ) assert response.status_code == 400 - assert response.json()["error"] == "unsupported_grant_type" + assert response.json()["error"] == "unauthorized_client" assert provider.last_params is None