fix(oauth): include client_id in token request body for client_secret_post (#2185)
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
This commit is contained in:
parent
528abfab86
commit
7c0224828b
2 changed files with 69 additions and 2 deletions
|
|
@ -205,8 +205,9 @@ class OAuthContext:
|
||||||
headers["Authorization"] = f"Basic {encoded_credentials}"
|
headers["Authorization"] = f"Basic {encoded_credentials}"
|
||||||
# Don't include client_secret in body for basic auth
|
# Don't include client_secret in body for basic auth
|
||||||
data = {k: v for k, v in data.items() if k != "client_secret"}
|
data = {k: v for k, v in data.items() if k != "client_secret"}
|
||||||
elif auth_method == "client_secret_post" and self.client_info.client_secret:
|
elif auth_method == "client_secret_post" and self.client_info.client_id and self.client_info.client_secret:
|
||||||
# Include client_secret in request body
|
# Include client_id and client_secret in request body (RFC 6749 §2.3.1)
|
||||||
|
data["client_id"] = self.client_info.client_id
|
||||||
data["client_secret"] = self.client_info.client_secret
|
data["client_secret"] = self.client_info.client_secret
|
||||||
# For auth_method == "none", don't add any client_secret
|
# For auth_method == "none", don't add any client_secret
|
||||||
|
|
||||||
|
|
|
||||||
|
|
@ -252,6 +252,72 @@ class TestClientCredentialsOAuthProvider:
|
||||||
assert "scope=read write" in content
|
assert "scope=read write" in content
|
||||||
assert "resource=https://api.example.com/v1/mcp" in content
|
assert "resource=https://api.example.com/v1/mcp" in content
|
||||||
|
|
||||||
|
@pytest.mark.anyio
|
||||||
|
async def test_exchange_token_client_secret_post_includes_client_id(self, mock_storage: MockTokenStorage):
|
||||||
|
"""Test that client_secret_post includes both client_id and client_secret in body (RFC 6749 §2.3.1)."""
|
||||||
|
provider = ClientCredentialsOAuthProvider(
|
||||||
|
server_url="https://api.example.com/v1/mcp",
|
||||||
|
storage=mock_storage,
|
||||||
|
client_id="test-client-id",
|
||||||
|
client_secret="test-client-secret",
|
||||||
|
token_endpoint_auth_method="client_secret_post",
|
||||||
|
scopes="read write",
|
||||||
|
)
|
||||||
|
await provider._initialize()
|
||||||
|
provider.context.oauth_metadata = OAuthMetadata(
|
||||||
|
issuer=AnyHttpUrl("https://api.example.com"),
|
||||||
|
authorization_endpoint=AnyHttpUrl("https://api.example.com/authorize"),
|
||||||
|
token_endpoint=AnyHttpUrl("https://api.example.com/token"),
|
||||||
|
)
|
||||||
|
provider.context.protocol_version = "2025-06-18"
|
||||||
|
|
||||||
|
request = await provider._perform_authorization()
|
||||||
|
|
||||||
|
content = urllib.parse.unquote_plus(request.content.decode())
|
||||||
|
assert "grant_type=client_credentials" in content
|
||||||
|
assert "client_id=test-client-id" in content
|
||||||
|
assert "client_secret=test-client-secret" in content
|
||||||
|
# Should NOT have Basic auth header
|
||||||
|
assert "Authorization" not in request.headers
|
||||||
|
|
||||||
|
@pytest.mark.anyio
|
||||||
|
async def test_exchange_token_client_secret_post_without_client_id(self, mock_storage: MockTokenStorage):
|
||||||
|
"""Test client_secret_post skips body credentials when client_id is None."""
|
||||||
|
provider = ClientCredentialsOAuthProvider(
|
||||||
|
server_url="https://api.example.com/v1/mcp",
|
||||||
|
storage=mock_storage,
|
||||||
|
client_id="placeholder",
|
||||||
|
client_secret="test-client-secret",
|
||||||
|
token_endpoint_auth_method="client_secret_post",
|
||||||
|
scopes="read write",
|
||||||
|
)
|
||||||
|
await provider._initialize()
|
||||||
|
provider.context.oauth_metadata = OAuthMetadata(
|
||||||
|
issuer=AnyHttpUrl("https://api.example.com"),
|
||||||
|
authorization_endpoint=AnyHttpUrl("https://api.example.com/authorize"),
|
||||||
|
token_endpoint=AnyHttpUrl("https://api.example.com/token"),
|
||||||
|
)
|
||||||
|
provider.context.protocol_version = "2025-06-18"
|
||||||
|
# Override client_info to have client_id=None (edge case)
|
||||||
|
provider.context.client_info = OAuthClientInformationFull(
|
||||||
|
redirect_uris=None,
|
||||||
|
client_id=None,
|
||||||
|
client_secret="test-client-secret",
|
||||||
|
grant_types=["client_credentials"],
|
||||||
|
token_endpoint_auth_method="client_secret_post",
|
||||||
|
scope="read write",
|
||||||
|
)
|
||||||
|
|
||||||
|
request = await provider._perform_authorization()
|
||||||
|
|
||||||
|
content = urllib.parse.unquote_plus(request.content.decode())
|
||||||
|
assert "grant_type=client_credentials" in content
|
||||||
|
# Neither client_id nor client_secret should be in body since client_id is None
|
||||||
|
# (RFC 6749 §2.3.1 requires both for client_secret_post)
|
||||||
|
assert "client_id=" not in content
|
||||||
|
assert "client_secret=" not in content
|
||||||
|
assert "Authorization" not in request.headers
|
||||||
|
|
||||||
@pytest.mark.anyio
|
@pytest.mark.anyio
|
||||||
async def test_exchange_token_without_scopes(self, mock_storage: MockTokenStorage):
|
async def test_exchange_token_without_scopes(self, mock_storage: MockTokenStorage):
|
||||||
"""Test token exchange without scopes."""
|
"""Test token exchange without scopes."""
|
||||||
|
|
|
||||||
Loading…
Reference in a new issue