feat: harden Claude MCP OAuth transport
This commit is contained in:
+240
-4
@@ -10,6 +10,7 @@ from fastapi.testclient import TestClient
|
||||
|
||||
from aegis_gitea_mcp.config import reset_settings
|
||||
from aegis_gitea_mcp.oauth import GiteaOAuthValidator, get_oauth_validator, reset_oauth_validator
|
||||
from aegis_gitea_mcp.oauth_flow import OAuthClientRegistry, OAuthRegistrationRequest
|
||||
from aegis_gitea_mcp.request_context import (
|
||||
get_gitea_user_login,
|
||||
get_gitea_user_token,
|
||||
@@ -40,6 +41,7 @@ def mock_env_oauth(monkeypatch):
|
||||
monkeypatch.setenv("OAUTH_MODE", "true")
|
||||
monkeypatch.setenv("GITEA_OAUTH_CLIENT_ID", "test-client-id")
|
||||
monkeypatch.setenv("GITEA_OAUTH_CLIENT_SECRET", "test-client-secret")
|
||||
monkeypatch.setenv("OAUTH_STATE_SECRET", "test-state-secret-0123456789abcdef")
|
||||
monkeypatch.setenv("STARTUP_VALIDATE_GITEA", "false")
|
||||
|
||||
|
||||
@@ -57,6 +59,24 @@ def oauth_client(mock_env_oauth):
|
||||
return TestClient(app, raise_server_exceptions=False)
|
||||
|
||||
|
||||
def _register_public_client(oauth_client: TestClient, redirect_uri: str) -> dict[str, str]:
|
||||
"""Register a public OAuth client for test flows."""
|
||||
response = oauth_client.post(
|
||||
"/register",
|
||||
json={
|
||||
"client_name": "pytest-client",
|
||||
"redirect_uris": [redirect_uri],
|
||||
"token_endpoint_auth_method": "none",
|
||||
"grant_types": ["authorization_code", "refresh_token"],
|
||||
"response_types": ["code"],
|
||||
},
|
||||
)
|
||||
assert response.status_code == 200
|
||||
payload = response.json()
|
||||
assert "client_id" in payload
|
||||
return payload
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# GiteaOAuthValidator unit tests
|
||||
# ---------------------------------------------------------------------------
|
||||
@@ -248,19 +268,39 @@ def test_oauth_token_endpoint_available_when_oauth_mode_false(monkeypatch):
|
||||
mock_client_cls.return_value.__aexit__ = AsyncMock(return_value=False)
|
||||
|
||||
with TestClient(app, raise_server_exceptions=False) as client:
|
||||
response = client.post("/oauth/token", data={"code": "abc123"})
|
||||
registration = client.post(
|
||||
"/register",
|
||||
json={
|
||||
"client_name": "pytest-client",
|
||||
"redirect_uris": ["http://127.0.0.1:8080/callback"],
|
||||
"token_endpoint_auth_method": "none",
|
||||
"grant_types": ["authorization_code"],
|
||||
"response_types": ["code"],
|
||||
},
|
||||
)
|
||||
assert registration.status_code == 200
|
||||
client_id = registration.json()["client_id"]
|
||||
response = client.post(
|
||||
"/oauth/token",
|
||||
data={"client_id": client_id, "code": "abc123", "code_verifier": "pkce"},
|
||||
)
|
||||
|
||||
assert response.status_code == 200
|
||||
|
||||
|
||||
def test_oauth_token_endpoint_missing_code(oauth_client):
|
||||
"""POST /oauth/token without a code returns 400."""
|
||||
response = oauth_client.post("/oauth/token", data={})
|
||||
client_data = _register_public_client(oauth_client, "http://127.0.0.1:8080/callback")
|
||||
response = oauth_client.post(
|
||||
"/oauth/token",
|
||||
data={"client_id": client_data["client_id"], "code_verifier": "pkce"},
|
||||
)
|
||||
assert response.status_code == 400
|
||||
|
||||
|
||||
def test_oauth_token_endpoint_proxy_success(oauth_client):
|
||||
"""POST /oauth/token proxies successfully to Gitea and returns access_token."""
|
||||
client_data = _register_public_client(oauth_client, "http://127.0.0.1:8080/callback")
|
||||
mock_response = MagicMock()
|
||||
mock_response.status_code = 200
|
||||
mock_response.json.return_value = {
|
||||
@@ -276,7 +316,11 @@ def test_oauth_token_endpoint_proxy_success(oauth_client):
|
||||
|
||||
response = oauth_client.post(
|
||||
"/oauth/token",
|
||||
data={"code": "auth-code-123", "redirect_uri": "https://chat.openai.com/callback"},
|
||||
data={
|
||||
"client_id": client_data["client_id"],
|
||||
"code": "auth-code-123",
|
||||
"code_verifier": "pkce-verifier",
|
||||
},
|
||||
)
|
||||
|
||||
assert response.status_code == 200
|
||||
@@ -286,6 +330,7 @@ def test_oauth_token_endpoint_proxy_success(oauth_client):
|
||||
|
||||
def test_oauth_token_endpoint_gitea_error(oauth_client):
|
||||
"""POST /oauth/token propagates Gitea error status."""
|
||||
client_data = _register_public_client(oauth_client, "http://127.0.0.1:8080/callback")
|
||||
mock_response = MagicMock()
|
||||
mock_response.status_code = 400
|
||||
mock_response.json.return_value = {"error": "invalid_grant"}
|
||||
@@ -296,11 +341,202 @@ def test_oauth_token_endpoint_gitea_error(oauth_client):
|
||||
mock_client_cls.return_value.__aenter__ = AsyncMock(return_value=mock_client)
|
||||
mock_client_cls.return_value.__aexit__ = AsyncMock(return_value=False)
|
||||
|
||||
response = oauth_client.post("/oauth/token", data={"code": "bad-code"})
|
||||
response = oauth_client.post(
|
||||
"/oauth/token",
|
||||
data={
|
||||
"client_id": client_data["client_id"],
|
||||
"code": "bad-code",
|
||||
"code_verifier": "pkce-verifier",
|
||||
},
|
||||
)
|
||||
|
||||
assert response.status_code == 400
|
||||
|
||||
|
||||
def test_oauth_authorize_and_callback_round_trip(oauth_client):
|
||||
"""OAuth authorize/callback round-trip preserves the original redirect URI and state."""
|
||||
client_data = _register_public_client(oauth_client, "http://127.0.0.1:8080/callback")
|
||||
|
||||
authorize_response = oauth_client.get(
|
||||
"/oauth/authorize",
|
||||
params={
|
||||
"client_id": client_data["client_id"],
|
||||
"redirect_uri": "http://127.0.0.1:8080/callback",
|
||||
"state": "original-state",
|
||||
"code_challenge": "pkce-challenge",
|
||||
"code_challenge_method": "S256",
|
||||
},
|
||||
follow_redirects=False,
|
||||
)
|
||||
|
||||
assert authorize_response.status_code == 302
|
||||
location = authorize_response.headers["location"]
|
||||
assert "state=" in location
|
||||
assert "redirect_uri=http%3A%2F%2F127.0.0.1%3A8080%2Fcallback" not in location
|
||||
|
||||
from urllib.parse import parse_qs, urlparse
|
||||
|
||||
parsed = urlparse(location)
|
||||
query = parse_qs(parsed.query)
|
||||
proxy_state = query["state"][0]
|
||||
|
||||
callback_response = oauth_client.get(
|
||||
"/oauth/callback",
|
||||
params={"state": proxy_state, "code": "auth-code-123"},
|
||||
follow_redirects=False,
|
||||
)
|
||||
|
||||
assert callback_response.status_code == 302
|
||||
callback_location = callback_response.headers["location"]
|
||||
assert callback_location.startswith("http://127.0.0.1:8080/callback?")
|
||||
assert "code=auth-code-123" in callback_location
|
||||
assert "state=original-state" in callback_location
|
||||
|
||||
|
||||
def test_oauth_callback_rejects_tampered_state(oauth_client):
|
||||
"""OAuth callback rejects modified signed proxy state."""
|
||||
client_data = _register_public_client(oauth_client, "http://127.0.0.1:8080/callback")
|
||||
authorize_response = oauth_client.get(
|
||||
"/oauth/authorize",
|
||||
params={
|
||||
"client_id": client_data["client_id"],
|
||||
"redirect_uri": "http://127.0.0.1:8080/callback",
|
||||
"state": "original-state",
|
||||
"code_challenge": "pkce-challenge",
|
||||
"code_challenge_method": "S256",
|
||||
},
|
||||
follow_redirects=False,
|
||||
)
|
||||
from urllib.parse import parse_qs, urlparse
|
||||
|
||||
proxy_state = parse_qs(urlparse(authorize_response.headers["location"]).query)["state"][0]
|
||||
tampered_state = proxy_state[:-1] + ("A" if proxy_state[-1] != "A" else "B")
|
||||
|
||||
callback_response = oauth_client.get(
|
||||
"/oauth/callback",
|
||||
params={"state": tampered_state, "code": "auth-code-123"},
|
||||
)
|
||||
|
||||
assert callback_response.status_code == 400
|
||||
|
||||
|
||||
@pytest.mark.parametrize(
|
||||
"redirect_uri",
|
||||
[
|
||||
"https://claude.ai/api/mcp/auth_callback",
|
||||
"https://claude.com/api/mcp/auth_callback",
|
||||
],
|
||||
)
|
||||
def test_dcr_accepts_default_claude_callbacks(oauth_client, redirect_uri):
|
||||
"""Claude's hosted connector callback URLs are allowed by default."""
|
||||
response = oauth_client.post(
|
||||
"/register",
|
||||
json={
|
||||
"client_name": "claude-client",
|
||||
"redirect_uris": [redirect_uri],
|
||||
"token_endpoint_auth_method": "none",
|
||||
"grant_types": ["authorization_code", "refresh_token"],
|
||||
"response_types": ["code"],
|
||||
},
|
||||
)
|
||||
|
||||
assert response.status_code == 200
|
||||
|
||||
|
||||
def test_oauth_authorize_rejects_unknown_client(oauth_client):
|
||||
"""OAuth authorize returns invalid_client for unregistered client IDs."""
|
||||
response = oauth_client.get(
|
||||
"/oauth/authorize",
|
||||
params={
|
||||
"client_id": "unknown-client",
|
||||
"redirect_uri": "http://127.0.0.1:8080/callback",
|
||||
"state": "x",
|
||||
"code_challenge": "pkce-challenge",
|
||||
"code_challenge_method": "S256",
|
||||
},
|
||||
)
|
||||
|
||||
assert response.status_code == 401
|
||||
assert response.json()["detail"] == "invalid_client"
|
||||
|
||||
|
||||
def test_oauth_token_rejects_unknown_dcr_client(oauth_client):
|
||||
"""Unknown dynamic clients receive RFC 6749 invalid_client from token endpoint."""
|
||||
response = oauth_client.post(
|
||||
"/oauth/token",
|
||||
data={
|
||||
"client_id": "deleted-or-unknown-client",
|
||||
"code": "auth-code-123",
|
||||
"code_verifier": "pkce-verifier",
|
||||
},
|
||||
)
|
||||
|
||||
assert response.status_code == 401
|
||||
assert response.json() == {"error": "invalid_client"}
|
||||
|
||||
|
||||
def test_oauth_authorize_requires_pkce_s256(oauth_client):
|
||||
"""Authorization endpoint enforces PKCE S256 for public clients."""
|
||||
client_data = _register_public_client(oauth_client, "http://127.0.0.1:8080/callback")
|
||||
missing_challenge = oauth_client.get(
|
||||
"/oauth/authorize",
|
||||
params={
|
||||
"client_id": client_data["client_id"],
|
||||
"redirect_uri": "http://127.0.0.1:8080/callback",
|
||||
"state": "x",
|
||||
},
|
||||
)
|
||||
plain_method = oauth_client.get(
|
||||
"/oauth/authorize",
|
||||
params={
|
||||
"client_id": client_data["client_id"],
|
||||
"redirect_uri": "http://127.0.0.1:8080/callback",
|
||||
"state": "x",
|
||||
"code_challenge": "pkce-challenge",
|
||||
"code_challenge_method": "plain",
|
||||
},
|
||||
)
|
||||
|
||||
assert missing_challenge.status_code == 400
|
||||
assert plain_method.status_code == 400
|
||||
|
||||
|
||||
def test_register_rejects_foreign_redirect_uri(oauth_client):
|
||||
"""DCR rejects redirect URIs outside the allowlist and loopback/Claude patterns."""
|
||||
response = oauth_client.post(
|
||||
"/register",
|
||||
json={
|
||||
"client_name": "pytest-client",
|
||||
"redirect_uris": ["https://evil.example.com/callback"],
|
||||
"token_endpoint_auth_method": "none",
|
||||
"grant_types": ["authorization_code"],
|
||||
"response_types": ["code"],
|
||||
},
|
||||
)
|
||||
|
||||
assert response.status_code == 400
|
||||
|
||||
|
||||
def test_dcr_registry_persists_registered_clients(tmp_path):
|
||||
"""Registered OAuth clients survive registry reloads."""
|
||||
storage_path = tmp_path / "dcr_clients.json"
|
||||
registry = OAuthClientRegistry(storage_path)
|
||||
request = OAuthRegistrationRequest.model_validate(
|
||||
{
|
||||
"client_name": "persisted-client",
|
||||
"redirect_uris": ["http://127.0.0.1:8080/callback"],
|
||||
"token_endpoint_auth_method": "none",
|
||||
"grant_types": ["authorization_code"],
|
||||
"response_types": ["code"],
|
||||
}
|
||||
)
|
||||
|
||||
response = registry.register(request)
|
||||
reloaded = OAuthClientRegistry(storage_path)
|
||||
|
||||
assert reloaded.get(response["client_id"]) is not None
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# Config validation tests
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
Reference in New Issue
Block a user