Skip to content

Commit dcfaefb

Browse files
committed
fix(auth): align /token and /revoke error codes with RFC 6749 and RFC 7009 (#3723)
Closes #3723.
1 parent 2af9e30 commit dcfaefb

4 files changed

Lines changed: 167 additions & 12 deletions

File tree

‎src/mcp/server/auth/handlers/revoke.py‎

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,7 @@ class RevocationRequest(BaseModel):
2424

2525

2626
class RevocationErrorResponse(BaseModel):
27-
error: Literal["invalid_request", "unauthorized_client"]
27+
error: Literal["invalid_request", "invalid_client", "unsupported_token_type", "unauthorized_client"]
2828
error_description: str | None = None
2929

3030

@@ -35,15 +35,24 @@ class RevocationHandler:
3535

3636
async def handle(self, request: Request) -> Response:
3737
"""Handler for the OAuth 2.0 Token Revocation endpoint."""
38+
auth_header = request.headers.get("Authorization", "")
39+
headers = {
40+
"Cache-Control": "no-store",
41+
"Pragma": "no-cache",
42+
}
43+
if auth_header.lower().startswith("basic "):
44+
headers["WWW-Authenticate"] = 'Basic realm="OAuth"'
45+
3846
try:
3947
client = await self.client_authenticator.authenticate_request(request)
4048
except AuthenticationError as e: # pragma: no cover
4149
return PydanticJSONResponse(
4250
status_code=401,
4351
content=RevocationErrorResponse(
44-
error="unauthorized_client",
52+
error="invalid_client",
4553
error_description=e.message,
4654
),
55+
headers=headers,
4756
)
4857

4958
try:

‎src/mcp/server/auth/handlers/token.py‎

Lines changed: 31 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -101,26 +101,45 @@ def response(self, obj: TokenSuccessResponse | TokenErrorResponse):
101101
)
102102

103103
async def handle(self, request: Request):
104+
auth_header = request.headers.get("Authorization", "")
104105
try:
105106
client_info = await self.client_authenticator.authenticate_request(request)
106107
except AuthenticationError as e:
107108
# Authentication failures should return 401
109+
headers = {
110+
"Cache-Control": "no-store",
111+
"Pragma": "no-cache",
112+
}
113+
if auth_header.lower().startswith("basic "):
114+
headers["WWW-Authenticate"] = 'Basic realm="OAuth"'
115+
108116
return PydanticJSONResponse(
109117
content=TokenErrorResponse(
110118
error="invalid_client",
111119
error_description=e.message,
112120
),
113121
status_code=401,
114-
headers={
115-
"Cache-Control": "no-store",
116-
"Pragma": "no-cache",
117-
},
122+
headers=headers,
123+
)
124+
125+
form_data = await request.form()
126+
form_dict = dict(form_data)
127+
grant_type = form_dict.get("grant_type")
128+
129+
supported_grants = {"authorization_code", "refresh_token"}
130+
if self.identity_assertion_enabled:
131+
supported_grants.add("urn:ietf:params:oauth:grant-type:jwt-bearer")
132+
133+
if grant_type and str(grant_type) not in supported_grants:
134+
return self.response(
135+
TokenErrorResponse(
136+
error="unsupported_grant_type",
137+
error_description=f"Unsupported grant type: {grant_type}",
138+
)
118139
)
119140

120141
try:
121-
form_data = await request.form()
122-
# TODO(Marcelo): Can someone check if this `dict()` wrapper is necessary?
123-
token_request = token_request_adapter.validate_python(dict(form_data))
142+
token_request = token_request_adapter.validate_python(form_dict)
124143
except ValidationError as validation_error:
125144
return self.response(
126145
TokenErrorResponse(
@@ -132,8 +151,11 @@ async def handle(self, request: Request):
132151
if token_request.grant_type not in client_info.grant_types:
133152
return self.response(
134153
TokenErrorResponse(
135-
error="unsupported_grant_type",
136-
error_description=(f"Unsupported grant type (supported grant types are {client_info.grant_types})"),
154+
error="unauthorized_client",
155+
error_description=(
156+
f"Client is not authorized to use grant type {token_request.grant_type} "
157+
f"(authorized grant types are {client_info.grant_types})"
158+
),
137159
)
138160
)
139161

‎tests/server/auth/test_error_handling.py‎

Lines changed: 124 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -347,3 +347,127 @@ async def test_oversized_cross_origin_request_gets_413_with_cors_headers(client:
347347
)
348348
assert response.status_code == 413
349349
assert response.headers["access-control-allow-origin"] == "*"
350+
351+
352+
@pytest.mark.anyio
353+
async def test_rfc6749_token_unauthorized_client(client: httpx2.AsyncClient, oauth_provider: MockOAuthProvider):
354+
"""RFC 6749 §5.2: when a grant is supported by the server but not authorized for this client,
355+
the server must return error='unauthorized_client'.
356+
"""
357+
reg_response = await client.post(
358+
"/register",
359+
json={
360+
"client_name": "Test Client",
361+
"redirect_uris": ["https://client.example.com/callback"],
362+
"grant_types": ["authorization_code"],
363+
"token_endpoint_auth_method": "client_secret_post",
364+
},
365+
)
366+
client_data = reg_response.json()
367+
368+
response = await client.post(
369+
"/token",
370+
data={
371+
"grant_type": "refresh_token",
372+
"refresh_token": "some-token",
373+
"client_id": client_data["client_id"],
374+
"client_secret": client_data["client_secret"],
375+
},
376+
)
377+
assert response.status_code == 400
378+
assert response.json()["error"] == "unauthorized_client"
379+
380+
381+
@pytest.mark.anyio
382+
async def test_rfc6749_token_unsupported_grant_type(client: httpx2.AsyncClient, oauth_provider: MockOAuthProvider):
383+
"""RFC 6749 §5.2: when a grant type is not supported by the server,
384+
the server must return error='unsupported_grant_type'.
385+
"""
386+
reg_response = await client.post(
387+
"/register",
388+
json={
389+
"client_name": "Test Client",
390+
"redirect_uris": ["https://client.example.com/callback"],
391+
"grant_types": ["authorization_code"],
392+
"token_endpoint_auth_method": "client_secret_post",
393+
},
394+
)
395+
client_data = reg_response.json()
396+
397+
response = await client.post(
398+
"/token",
399+
data={
400+
"grant_type": "client_credentials",
401+
"client_id": client_data["client_id"],
402+
"client_secret": client_data["client_secret"],
403+
},
404+
)
405+
assert response.status_code == 400
406+
assert response.json()["error"] == "unsupported_grant_type"
407+
408+
409+
@pytest.mark.anyio
410+
async def test_rfc6749_token_basic_auth_failure_www_authenticate(
411+
client: httpx2.AsyncClient, oauth_provider: MockOAuthProvider
412+
):
413+
"""RFC 6749 §5.2: failed client authentication via Authorization header must return 401
414+
with invalid_client and a matching WWW-Authenticate header.
415+
"""
416+
reg_response = await client.post(
417+
"/register",
418+
json={
419+
"client_name": "Test Client",
420+
"redirect_uris": ["https://client.example.com/callback"],
421+
"grant_types": ["authorization_code"],
422+
"token_endpoint_auth_method": "client_secret_basic",
423+
},
424+
)
425+
client_data = reg_response.json()
426+
427+
auth_header = "Basic " + base64.b64encode(f"{client_data['client_id']}:wrong_secret".encode()).decode()
428+
response = await client.post(
429+
"/token",
430+
headers={"Authorization": auth_header},
431+
data={
432+
"grant_type": "authorization_code",
433+
"code": "any_code",
434+
"client_id": client_data["client_id"],
435+
},
436+
)
437+
assert response.status_code == 401
438+
assert response.json()["error"] == "invalid_client"
439+
assert "www-authenticate" in response.headers
440+
assert response.headers["www-authenticate"].lower().startswith("basic")
441+
442+
443+
@pytest.mark.anyio
444+
async def test_rfc7009_revoke_invalid_client_and_www_authenticate(
445+
client: httpx2.AsyncClient, oauth_provider: MockOAuthProvider
446+
):
447+
"""RFC 7009 §2.2.1: failed client authentication on /revoke must return error='invalid_client'
448+
(not 'unauthorized_client'), status 401, with matching WWW-Authenticate header.
449+
"""
450+
reg_response = await client.post(
451+
"/register",
452+
json={
453+
"client_name": "Test Client",
454+
"redirect_uris": ["https://client.example.com/callback"],
455+
"grant_types": ["authorization_code"],
456+
"token_endpoint_auth_method": "client_secret_basic",
457+
},
458+
)
459+
client_data = reg_response.json()
460+
461+
auth_header = "Basic " + base64.b64encode(f"{client_data['client_id']}:wrong_secret".encode()).decode()
462+
response = await client.post(
463+
"/revoke",
464+
headers={"Authorization": auth_header},
465+
data={
466+
"token": "any_token",
467+
"client_id": client_data["client_id"],
468+
},
469+
)
470+
assert response.status_code == 401
471+
assert response.json()["error"] == "invalid_client"
472+
assert "www-authenticate" in response.headers
473+
assert response.headers["www-authenticate"].lower().startswith("basic")

‎tests/server/auth/test_identity_assertion.py‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -309,7 +309,7 @@ async def test_client_without_the_grant_registered_is_rejected(
309309
)
310310

311311
assert response.status_code == 400
312-
assert response.json()["error"] == "unsupported_grant_type"
312+
assert response.json()["error"] == "unauthorized_client"
313313
assert provider.last_params is None
314314

315315

0 commit comments

Comments
 (0)