Add client_secret_basic authentication support (#1334)

Co-authored-by: Paul Carleton <paulc@anthropic.com>
This commit is contained in:
Jon Shea 2025-11-20 15:53:37 -05:00 • committed by GitHub
commit c51936f61f
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
2 changed files with 695 additions and 59 deletions

View file

@ -2,15 +2,18 @@
Tests for refactored OAuth client authentication implementation.
"""
import base64
import json
import time
from unittest import mock
from urllib.parse import unquote
import httpx
import pytest
from inline_snapshot import Is, snapshot
from pydantic import AnyHttpUrl, AnyUrl
from mcp.client.auth import OAuthClientProvider, PKCEParameters
from mcp.client.auth import OAuthClientProvider, OAuthRegistrationError, PKCEParameters
from mcp.client.auth.utils import (
build_oauth_authorization_server_metadata_discovery_urls,
build_protected_resource_metadata_discovery_urls,
@ -21,7 +24,13 @@ from mcp.client.auth.utils import (
get_client_metadata_scopes,
handle_registration_response,
)
from mcp.shared.auth import OAuthClientInformationFull, OAuthClientMetadata, OAuthToken, ProtectedResourceMetadata
from mcp.shared.auth import (
OAuthClientInformationFull,
OAuthClientMetadata,
OAuthMetadata,
OAuthToken,
ProtectedResourceMetadata,
)
class MockTokenStorage:
@ -592,6 +601,74 @@ class TestOAuthFallback:
request = await oauth_provider._register_client()
assert request is None
@pytest.mark.anyio
async def test_register_client_explicit_auth_method(self, mock_storage: MockTokenStorage):
"""Test that explicitly set token_endpoint_auth_method is used without auto-selection."""
async def redirect_handler(url: str) -> None:
pass # pragma: no cover
async def callback_handler() -> tuple[str, str | None]:
return "test_auth_code", "test_state" # pragma: no cover
# Create client metadata with explicit auth method
explicit_metadata = OAuthClientMetadata(
client_name="Test Client",
client_uri=AnyHttpUrl("https://example.com"),
redirect_uris=[AnyUrl("http://localhost:3030/callback")],
scope="read write",
token_endpoint_auth_method="client_secret_basic",
)
provider = OAuthClientProvider(
server_url="https://api.example.com/v1/mcp",
client_metadata=explicit_metadata,
storage=mock_storage,
redirect_handler=redirect_handler,
callback_handler=callback_handler,
)
request = await provider._register_client()
assert request is not None
body = json.loads(request.content)
# Should use the explicitly set method, not auto-select
assert body["token_endpoint_auth_method"] == "client_secret_basic"
@pytest.mark.anyio
async def test_register_client_none_auth_method_with_server_metadata(self, oauth_provider: OAuthClientProvider):
"""Test that token_endpoint_auth_method=None selects from server's supported methods."""
# Set server metadata with specific supported methods
oauth_provider.context.oauth_metadata = OAuthMetadata(
issuer=AnyHttpUrl("https://auth.example.com"),
authorization_endpoint=AnyHttpUrl("https://auth.example.com/authorize"),
token_endpoint=AnyHttpUrl("https://auth.example.com/token"),
token_endpoint_auth_methods_supported=["client_secret_post"],
)
# Ensure client_metadata has None for token_endpoint_auth_method
request = await oauth_provider._register_client()
assert request is not None
body = json.loads(request.content)
assert body["token_endpoint_auth_method"] == "client_secret_post"
@pytest.mark.anyio
async def test_register_client_none_auth_method_no_compatible(self, oauth_provider: OAuthClientProvider):
"""Test that registration raises error when no compatible auth methods."""
# Set server metadata with unsupported methods only
oauth_provider.context.oauth_metadata = OAuthMetadata(
issuer=AnyHttpUrl("https://auth.example.com"),
authorization_endpoint=AnyHttpUrl("https://auth.example.com/authorize"),
token_endpoint=AnyHttpUrl("https://auth.example.com/token"),
token_endpoint_auth_methods_supported=["private_key_jwt", "client_secret_jwt"],
)
with pytest.raises(OAuthRegistrationError) as exc_info:
await oauth_provider._register_client()
assert "No compatible authentication methods" in str(exc_info.value)
assert "private_key_jwt" in str(exc_info.value)
@pytest.mark.anyio
async def test_token_exchange_request_authorization_code(self, oauth_provider: OAuthClientProvider):
"""Test token exchange request building."""
@ -600,6 +677,7 @@ class TestOAuthFallback:
client_id="test_client",
client_secret="test_secret",
redirect_uris=[AnyUrl("http://localhost:3030/callback")],
token_endpoint_auth_method="client_secret_post",
)
request = await oauth_provider._exchange_token_authorization_code("test_auth_code", "test_verifier")
@ -625,6 +703,7 @@ class TestOAuthFallback:
client_id="test_client",
client_secret="test_secret",
redirect_uris=[AnyUrl("http://localhost:3030/callback")],
token_endpoint_auth_method="client_secret_post",
)
request = await oauth_provider._refresh_token()
@ -640,6 +719,114 @@ class TestOAuthFallback:
assert "client_id=test_client" in content
assert "client_secret=test_secret" in content
@pytest.mark.anyio
async def test_basic_auth_token_exchange(self, oauth_provider: OAuthClientProvider):
"""Test token exchange with client_secret_basic authentication."""
# Set up OAuth metadata to support basic auth
oauth_provider.context.oauth_metadata = OAuthMetadata(
issuer=AnyHttpUrl("https://auth.example.com"),
authorization_endpoint=AnyHttpUrl("https://auth.example.com/authorize"),
token_endpoint=AnyHttpUrl("https://auth.example.com/token"),
token_endpoint_auth_methods_supported=["client_secret_basic", "client_secret_post"],
)
client_id_raw = "test@client" # Include special character to test URL encoding
client_secret_raw = "test:secret" # Include colon to test URL encoding
oauth_provider.context.client_info = OAuthClientInformationFull(
client_id=client_id_raw,
client_secret=client_secret_raw,
redirect_uris=[AnyUrl("http://localhost:3030/callback")],
token_endpoint_auth_method="client_secret_basic",
)
request = await oauth_provider._exchange_token_authorization_code("test_auth_code", "test_verifier")
# Should use basic auth (registered method)
assert "Authorization" in request.headers
assert request.headers["Authorization"].startswith("Basic ")
# Decode and verify credentials are properly URL-encoded
encoded_creds = request.headers["Authorization"][6:] # Remove "Basic " prefix
decoded = base64.b64decode(encoded_creds).decode()
client_id, client_secret = decoded.split(":", 1)
# Check URL encoding was applied
assert client_id == "test%40client" # @ should be encoded as %40
assert client_secret == "test%3Asecret" # : should be encoded as %3A
# Verify decoded values match original
assert unquote(client_id) == client_id_raw
assert unquote(client_secret) == client_secret_raw
# client_secret should NOT be in body for basic auth
content = request.content.decode()
assert "client_secret=" not in content
assert "client_id=test%40client" in content # client_id still in body
@pytest.mark.anyio
async def test_basic_auth_refresh_token(self, oauth_provider: OAuthClientProvider, valid_tokens: OAuthToken):
"""Test token refresh with client_secret_basic authentication."""
oauth_provider.context.current_tokens = valid_tokens
# Set up OAuth metadata to only support basic auth
oauth_provider.context.oauth_metadata = OAuthMetadata(
issuer=AnyHttpUrl("https://auth.example.com"),
authorization_endpoint=AnyHttpUrl("https://auth.example.com/authorize"),
token_endpoint=AnyHttpUrl("https://auth.example.com/token"),
token_endpoint_auth_methods_supported=["client_secret_basic"],
)
client_id = "test_client"
client_secret = "test_secret"
oauth_provider.context.client_info = OAuthClientInformationFull(
client_id=client_id,
client_secret=client_secret,
redirect_uris=[AnyUrl("http://localhost:3030/callback")],
token_endpoint_auth_method="client_secret_basic",
)
request = await oauth_provider._refresh_token()
assert "Authorization" in request.headers
assert request.headers["Authorization"].startswith("Basic ")
encoded_creds = request.headers["Authorization"][6:]
decoded = base64.b64decode(encoded_creds).decode()
assert decoded == f"{client_id}:{client_secret}"
# client_secret should NOT be in body
content = request.content.decode()
assert "client_secret=" not in content
@pytest.mark.anyio
async def test_none_auth_method(self, oauth_provider: OAuthClientProvider):
"""Test 'none' authentication method (public client)."""
oauth_provider.context.oauth_metadata = OAuthMetadata(
issuer=AnyHttpUrl("https://auth.example.com"),
authorization_endpoint=AnyHttpUrl("https://auth.example.com/authorize"),
token_endpoint=AnyHttpUrl("https://auth.example.com/token"),
token_endpoint_auth_methods_supported=["none"],
)
client_id = "public_client"
oauth_provider.context.client_info = OAuthClientInformationFull(
client_id=client_id,
client_secret=None, # No secret for public client
redirect_uris=[AnyUrl("http://localhost:3030/callback")],
token_endpoint_auth_method="none",
)
request = await oauth_provider._exchange_token_authorization_code("test_auth_code", "test_verifier")
# Should NOT have Authorization header
assert "Authorization" not in request.headers
# Should NOT have client_secret in body
content = request.content.decode()
assert "client_secret=" not in content
assert "client_id=public_client" in content
class TestProtectedResourceMetadata:
"""Test protected resource handling."""
@ -1103,10 +1290,10 @@ def test_build_metadata(
"registration_endpoint": Is(registration_endpoint),
"scopes_supported": ["read", "write", "admin"],
"grant_types_supported": ["authorization_code", "refresh_token"],
"token_endpoint_auth_methods_supported": ["client_secret_post"],
"token_endpoint_auth_methods_supported": ["client_secret_post", "client_secret_basic"],
"service_documentation": Is(service_documentation_url),
"revocation_endpoint": Is(revocation_endpoint),
"revocation_endpoint_auth_methods_supported": ["client_secret_post"],
"revocation_endpoint_auth_methods_supported": ["client_secret_post", "client_secret_basic"],
"code_challenge_methods_supported": ["S256"],
}
)

View file

@ -12,7 +12,7 @@ from urllib.parse import parse_qs, urlparse
import httpx
import pytest
from pydantic import AnyHttpUrl
from pydantic import AnyHttpUrl, AnyUrl
from starlette.applications import Starlette
from mcp.server.auth.provider import (
@ -320,7 +320,7 @@ class TestAuthEndpoints:
assert metadata["revocation_endpoint"] == "https://auth.example.com/revoke"
assert metadata["response_types_supported"] == ["code"]
assert metadata["code_challenge_methods_supported"] == ["S256"]
assert metadata["token_endpoint_auth_methods_supported"] == ["client_secret_post"]
assert metadata["token_endpoint_auth_methods_supported"] == ["client_secret_post", "client_secret_basic"]
assert metadata["grant_types_supported"] == [
"authorization_code",
"refresh_token",
@ -339,8 +339,8 @@ class TestAuthEndpoints:
},
)
error_response = response.json()
assert error_response["error"] == "invalid_request"
assert "error_description" in error_response # Contains validation error messages
assert error_response["error"] == "unauthorized_client"
assert "error_description" in error_response # Contains error message
@pytest.mark.anyio
async def test_token_invalid_auth_code(
@ -976,6 +976,330 @@ class TestAuthEndpoints:
assert "response_types" in data
assert data["response_types"] == ["code"]
@pytest.mark.anyio
async def test_client_secret_basic_authentication(
self, test_client: httpx.AsyncClient, mock_oauth_provider: MockOAuthProvider, pkce_challenge: dict[str, str]
):
"""Test that client_secret_basic authentication works correctly."""
client_metadata = {
"redirect_uris": ["https://client.example.com/callback"],
"client_name": "Basic Auth Client",
"token_endpoint_auth_method": "client_secret_basic",
"grant_types": ["authorization_code", "refresh_token"],
}
response = await test_client.post("/register", json=client_metadata)
assert response.status_code == 201
client_info = response.json()
assert client_info["token_endpoint_auth_method"] == "client_secret_basic"
auth_code = f"code_{int(time.time())}"
mock_oauth_provider.auth_codes[auth_code] = AuthorizationCode(
code=auth_code,
client_id=client_info["client_id"],
code_challenge=pkce_challenge["code_challenge"],
redirect_uri=AnyUrl("https://client.example.com/callback"),
redirect_uri_provided_explicitly=True,
scopes=["read", "write"],
expires_at=time.time() + 600,
)
credentials = f"{client_info['client_id']}:{client_info['client_secret']}"
encoded_credentials = base64.b64encode(credentials.encode()).decode()
response = await test_client.post(
"/token",
headers={"Authorization": f"Basic {encoded_credentials}"},
data={
"grant_type": "authorization_code",
"client_id": client_info["client_id"],
"code": auth_code,
"code_verifier": pkce_challenge["code_verifier"],
"redirect_uri": "https://client.example.com/callback",
},
)
assert response.status_code == 200
token_response = response.json()
assert "access_token" in token_response
@pytest.mark.anyio
async def test_wrong_auth_method_without_valid_credentials_fails(
self, test_client: httpx.AsyncClient, mock_oauth_provider: MockOAuthProvider, pkce_challenge: dict[str, str]
):
"""Test that using the wrong authentication method fails when credentials are missing."""
client_metadata = {
"redirect_uris": ["https://client.example.com/callback"],
"client_name": "Post Auth Client",
"token_endpoint_auth_method": "client_secret_post",
"grant_types": ["authorization_code", "refresh_token"],
}
response = await test_client.post("/register", json=client_metadata)
assert response.status_code == 201
client_info = response.json()
assert client_info["token_endpoint_auth_method"] == "client_secret_post"
auth_code = f"code_{int(time.time())}"
mock_oauth_provider.auth_codes[auth_code] = AuthorizationCode(
code=auth_code,
client_id=client_info["client_id"],
code_challenge=pkce_challenge["code_challenge"],
redirect_uri=AnyUrl("https://client.example.com/callback"),
redirect_uri_provided_explicitly=True,
scopes=["read", "write"],
expires_at=time.time() + 600,
)
# Try to use Basic auth when client_secret_post is registered (without secret in body)
# This should fail because the secret is missing from the expected location
credentials = f"{client_info['client_id']}:{client_info['client_secret']}"
encoded_credentials = base64.b64encode(credentials.encode()).decode()
response = await test_client.post(
"/token",
headers={"Authorization": f"Basic {encoded_credentials}"},
data={
"grant_type": "authorization_code",
"client_id": client_info["client_id"],
# client_secret NOT in body where it should be
"code": auth_code,
"code_verifier": pkce_challenge["code_verifier"],
"redirect_uri": "https://client.example.com/callback",
},
)
assert response.status_code == 401
error_response = response.json()
assert error_response["error"] == "unauthorized_client"
assert "Client secret is required" in error_response["error_description"]
@pytest.mark.anyio
async def test_basic_auth_without_header_fails(
self, test_client: httpx.AsyncClient, mock_oauth_provider: MockOAuthProvider, pkce_challenge: dict[str, str]
):
"""Test that omitting Basic auth when client_secret_basic is registered fails."""
client_metadata = {
"redirect_uris": ["https://client.example.com/callback"],
"client_name": "Basic Auth Client",
"token_endpoint_auth_method": "client_secret_basic",
"grant_types": ["authorization_code", "refresh_token"],
}
response = await test_client.post("/register", json=client_metadata)
assert response.status_code == 201
client_info = response.json()
assert client_info["token_endpoint_auth_method"] == "client_secret_basic"
auth_code = f"code_{int(time.time())}"
mock_oauth_provider.auth_codes[auth_code] = AuthorizationCode(
code=auth_code,
client_id=client_info["client_id"],
code_challenge=pkce_challenge["code_challenge"],
redirect_uri=AnyUrl("https://client.example.com/callback"),
redirect_uri_provided_explicitly=True,
scopes=["read", "write"],
expires_at=time.time() + 600,
)
response = await test_client.post(
"/token",
data={
"grant_type": "authorization_code",
"client_id": client_info["client_id"],
"client_secret": client_info["client_secret"], # Secret in body (ignored)
"code": auth_code,
"code_verifier": pkce_challenge["code_verifier"],
"redirect_uri": "https://client.example.com/callback",
},
)
assert response.status_code == 401
error_response = response.json()
assert error_response["error"] == "unauthorized_client"
assert "Missing or invalid Basic authentication" in error_response["error_description"]
@pytest.mark.anyio
async def test_basic_auth_invalid_base64_fails(
self, test_client: httpx.AsyncClient, mock_oauth_provider: MockOAuthProvider, pkce_challenge: dict[str, str]
):
"""Test that invalid base64 in Basic auth header fails."""
client_metadata = {
"redirect_uris": ["https://client.example.com/callback"],
"client_name": "Basic Auth Client",
"token_endpoint_auth_method": "client_secret_basic",
"grant_types": ["authorization_code", "refresh_token"],
}
response = await test_client.post("/register", json=client_metadata)
assert response.status_code == 201
client_info = response.json()
auth_code = f"code_{int(time.time())}"
mock_oauth_provider.auth_codes[auth_code] = AuthorizationCode(
code=auth_code,
client_id=client_info["client_id"],
code_challenge=pkce_challenge["code_challenge"],
redirect_uri=AnyUrl("https://client.example.com/callback"),
redirect_uri_provided_explicitly=True,
scopes=["read", "write"],
expires_at=time.time() + 600,
)
# Send invalid base64
response = await test_client.post(
"/token",
headers={"Authorization": "Basic !!!invalid-base64!!!"},
data={
"grant_type": "authorization_code",
"client_id": client_info["client_id"],
"code": auth_code,
"code_verifier": pkce_challenge["code_verifier"],
"redirect_uri": "https://client.example.com/callback",
},
)
assert response.status_code == 401
error_response = response.json()
assert error_response["error"] == "unauthorized_client"
assert "Invalid Basic authentication header" in error_response["error_description"]
@pytest.mark.anyio
async def test_basic_auth_no_colon_fails(
self, test_client: httpx.AsyncClient, mock_oauth_provider: MockOAuthProvider, pkce_challenge: dict[str, str]
):
"""Test that Basic auth without colon separator fails."""
client_metadata = {
"redirect_uris": ["https://client.example.com/callback"],
"client_name": "Basic Auth Client",
"token_endpoint_auth_method": "client_secret_basic",
"grant_types": ["authorization_code", "refresh_token"],
}
response = await test_client.post("/register", json=client_metadata)
assert response.status_code == 201
client_info = response.json()
auth_code = f"code_{int(time.time())}"
mock_oauth_provider.auth_codes[auth_code] = AuthorizationCode(
code=auth_code,
client_id=client_info["client_id"],
code_challenge=pkce_challenge["code_challenge"],
redirect_uri=AnyUrl("https://client.example.com/callback"),
redirect_uri_provided_explicitly=True,
scopes=["read", "write"],
expires_at=time.time() + 600,
)
# Send base64 without colon (invalid format)
import base64
invalid_creds = base64.b64encode(b"no-colon-here").decode()
response = await test_client.post(
"/token",
headers={"Authorization": f"Basic {invalid_creds}"},
data={
"grant_type": "authorization_code",
"client_id": client_info["client_id"],
"code": auth_code,
"code_verifier": pkce_challenge["code_verifier"],
"redirect_uri": "https://client.example.com/callback",
},
)
assert response.status_code == 401
error_response = response.json()
assert error_response["error"] == "unauthorized_client"
assert "Invalid Basic authentication header" in error_response["error_description"]
@pytest.mark.anyio
async def test_basic_auth_client_id_mismatch_fails(
self, test_client: httpx.AsyncClient, mock_oauth_provider: MockOAuthProvider, pkce_challenge: dict[str, str]
):
"""Test that client_id mismatch between body and Basic auth fails."""
client_metadata = {
"redirect_uris": ["https://client.example.com/callback"],
"client_name": "Basic Auth Client",
"token_endpoint_auth_method": "client_secret_basic",
"grant_types": ["authorization_code", "refresh_token"],
}
response = await test_client.post("/register", json=client_metadata)
assert response.status_code == 201
client_info = response.json()
auth_code = f"code_{int(time.time())}"
mock_oauth_provider.auth_codes[auth_code] = AuthorizationCode(
code=auth_code,
client_id=client_info["client_id"],
code_challenge=pkce_challenge["code_challenge"],
redirect_uri=AnyUrl("https://client.example.com/callback"),
redirect_uri_provided_explicitly=True,
scopes=["read", "write"],
expires_at=time.time() + 600,
)
# Send different client_id in Basic auth header
import base64
wrong_creds = base64.b64encode(f"wrong-client-id:{client_info['client_secret']}".encode()).decode()
response = await test_client.post(
"/token",
headers={"Authorization": f"Basic {wrong_creds}"},
data={
"grant_type": "authorization_code",
"client_id": client_info["client_id"], # Correct client_id in body
"code": auth_code,
"code_verifier": pkce_challenge["code_verifier"],
"redirect_uri": "https://client.example.com/callback",
},
)
assert response.status_code == 401
error_response = response.json()
assert error_response["error"] == "unauthorized_client"
assert "Client ID mismatch" in error_response["error_description"]
@pytest.mark.anyio
async def test_none_auth_method_public_client(
self, test_client: httpx.AsyncClient, mock_oauth_provider: MockOAuthProvider, pkce_challenge: dict[str, str]
):
"""Test that 'none' authentication method works for public clients."""
client_metadata = {
"redirect_uris": ["https://client.example.com/callback"],
"client_name": "Public Client",
"token_endpoint_auth_method": "none",
"grant_types": ["authorization_code", "refresh_token"],
}
response = await test_client.post("/register", json=client_metadata)
assert response.status_code == 201
client_info = response.json()
assert client_info["token_endpoint_auth_method"] == "none"
# Public clients should not have a client_secret
assert "client_secret" not in client_info or client_info.get("client_secret") is None
auth_code = f"code_{int(time.time())}"
mock_oauth_provider.auth_codes[auth_code] = AuthorizationCode(
code=auth_code,
client_id=client_info["client_id"],
code_challenge=pkce_challenge["code_challenge"],
redirect_uri=AnyUrl("https://client.example.com/callback"),
redirect_uri_provided_explicitly=True,
scopes=["read", "write"],
expires_at=time.time() + 600,
)
# Token request without any client secret
response = await test_client.post(
"/token",
data={
"grant_type": "authorization_code",
"client_id": client_info["client_id"],
"code": auth_code,
"code_verifier": pkce_challenge["code_verifier"],
"redirect_uri": "https://client.example.com/callback",
},
)
assert response.status_code == 200
token_response = response.json()
assert "access_token" in token_response
class TestAuthorizeEndpointErrors:
"""Test error handling in the OAuth authorization endpoint."""