diff --git a/app-hooks/patches/0001-Fix-Bearer-token-authentication-causing-session-logo.patch b/app-hooks/patches/0001-Fix-Bearer-token-authentication-causing-session-logo.patch deleted file mode 100644 index c578441c..00000000 --- a/app-hooks/patches/0001-Fix-Bearer-token-authentication-causing-session-logo.patch +++ /dev/null @@ -1,69 +0,0 @@ -From deab2dac3d73d25f20a95c18103f327ab48f837a Mon Sep 17 00:00:00 2001 -From: Chris Coutinho -Date: Sun, 12 Oct 2025 21:09:29 +0200 -Subject: [PATCH 1/1] Fix Bearer token authentication causing session logout - -When using Bearer token authentication with OIDC, API requests to -endpoints with @CORS annotations (like Notes API) were failing with -401 Unauthorized errors. This occurred because: - -1. Bearer token validation successfully authenticated the user -2. A session was created for the authenticated user -3. Nextcloud's CORSMiddleware detected the logged-in session but no - CSRF token, causing it to call session->logout() -4. The logout invalidated the session, breaking the API request - -This fix sets the 'app_api' session flag during Bearer token -authentication, which instructs CORSMiddleware to skip the CSRF check -and logout logic. This is the same mechanism used by Nextcloud's -AppAPI framework for external application authentication. - -The flag is set at all successful Bearer token authentication points: -- Line 243: After OIDC Identity Provider validation -- Line 310: After auto-provisioning with bearer provisioning -- Line 315: After existing user authentication -- Line 337: After LDAP user sync - -Fixes: Bearer token authentication for all Nextcloud APIs -Tested-with: nextcloud-mcp-server integration tests -Signed-off-by: Chris Coutinho ---- - lib/User/Backend.php | 4 ++++ - 1 file changed, 4 insertions(+) - -diff --git a/lib/User/Backend.php b/lib/User/Backend.php -index 23cfb18..65665cc 100644 ---- a/lib/User/Backend.php -+++ b/lib/User/Backend.php -@@ -240,6 +240,7 @@ class Backend extends ABackend implements IPasswordConfirmationBackend, IGetDisp - $this->eventDispatcher->dispatchTyped($validationEvent); - $oidcProviderUserId = $validationEvent->getUserId(); - if ($oidcProviderUserId !== null) { -+ $this->session->set('app_api', true); - return $oidcProviderUserId; - } else { - $this->logger->debug('[NextcloudOidcProviderValidator] The bearer token validation has failed'); -@@ -306,10 +307,12 @@ class Backend extends ABackend implements IPasswordConfirmationBackend, IGetDisp - } - - $this->session->set('last-password-confirm', strtotime('+4 year', time())); -+ $this->session->set('app_api', true); - return $userId; - } elseif ($this->userExists($tokenUserId)) { - $this->checkFirstLogin($tokenUserId); - $this->session->set('last-password-confirm', strtotime('+4 year', time())); -+ $this->session->set('app_api', true); - return $tokenUserId; - } else { - // check if the user exists locally -@@ -331,6 +334,7 @@ class Backend extends ABackend implements IPasswordConfirmationBackend, IGetDisp - } - $this->checkFirstLogin($tokenUserId); - $this->session->set('last-password-confirm', strtotime('+4 year', time())); -+ $this->session->set('app_api', true); - return $tokenUserId; - } - } --- -2.51.0 - diff --git a/app-hooks/patches/cors-bearer-token.patch b/app-hooks/patches/cors-bearer-token.patch deleted file mode 100644 index 2186db9f..00000000 --- a/app-hooks/patches/cors-bearer-token.patch +++ /dev/null @@ -1,18 +0,0 @@ -diff --git a/lib/private/AppFramework/Middleware/Security/CORSMiddleware.php b/lib/private/AppFramework/Middleware/Security/CORSMiddleware.php -index 4453f5a7d4b..f1ca9b48d21 100644 ---- a/lib/private/AppFramework/Middleware/Security/CORSMiddleware.php -+++ b/lib/private/AppFramework/Middleware/Security/CORSMiddleware.php -@@ -73,6 +73,13 @@ class CORSMiddleware extends Middleware { - $user = array_key_exists('PHP_AUTH_USER', $this->request->server) ? $this->request->server['PHP_AUTH_USER'] : null; - $pass = array_key_exists('PHP_AUTH_PW', $this->request->server) ? $this->request->server['PHP_AUTH_PW'] : null; - -+ // Allow Bearer token authentication for CORS requests -+ // Bearer tokens are stateless and don't require CSRF protection -+ $authorizationHeader = $this->request->getHeader('Authorization'); -+ if (!empty($authorizationHeader) && str_starts_with($authorizationHeader, 'Bearer ')) { -+ return; -+ } -+ - // Allow to use the current session if a CSRF token is provided - if ($this->request->passesCSRFCheck()) { - return; diff --git a/app-hooks/post-installation/20-apply-cors-bearer-token-patch.sh b/app-hooks/post-installation/20-apply-cors-bearer-token-patch.sh deleted file mode 100755 index f05fd9ee..00000000 --- a/app-hooks/post-installation/20-apply-cors-bearer-token-patch.sh +++ /dev/null @@ -1,64 +0,0 @@ -#!/bin/bash -# -# Apply upstream CORSMiddleware Bearer token authentication patch -# -# This patch allows Bearer tokens to bypass CORS/CSRF checks, fixing -# authentication issues with app-specific APIs (Notes, Calendar, etc.) -# when using OAuth/OIDC Bearer tokens. -# -# Upstream PR: https://github.com/nextcloud/server/pull/55878 -# Commit: 8fb5e77db82 (fix(cors): Allow Bearer token authentication) -# - -set -e - -PATCH_FILE="/docker-entrypoint-hooks.d/patches/cors-bearer-token.patch" -TARGET_FILE="/var/www/html/lib/private/AppFramework/Middleware/Security/CORSMiddleware.php" - -echo "====================================================================" -echo "Applying CORSMiddleware Bearer token authentication patch..." -echo "====================================================================" - -# Check if patch file exists -if [ ! -f "$PATCH_FILE" ]; then - echo "⚠ Warning: Patch file not found: $PATCH_FILE" - echo " Skipping CORS Bearer token patch" - exit 0 -fi - -# Check if target file exists -if [ ! -f "$TARGET_FILE" ]; then - echo "⚠ Warning: Target file not found: $TARGET_FILE" - echo " Skipping CORS Bearer token patch" - exit 0 -fi - -# Check if already patched -if grep -q "Allow Bearer token authentication for CORS requests" "$TARGET_FILE"; then - echo "✓ CORSMiddleware already patched for Bearer token support" - exit 0 -fi - -echo "Applying patch to CORSMiddleware.php..." - -# Apply the patch -cd /var/www/html -if patch -p1 --dry-run < "$PATCH_FILE" > /dev/null 2>&1; then - patch -p1 < "$PATCH_FILE" - echo "✓ Patch applied successfully" -else - echo "⚠ Warning: Patch failed to apply (may already be applied or file changed)" - echo " This is expected if using a Nextcloud version that already includes the fix" - exit 0 -fi - -echo "" -echo "====================================================================" -echo "✓ CORSMiddleware Bearer token patch applied" -echo "====================================================================" -echo "" -echo "Benefits:" -echo " • Bearer tokens now work with app-specific APIs (Notes, Calendar, etc.)" -echo " • OAuth/OIDC authentication works without CORS errors" -echo " • Stateless API authentication is properly supported" -echo "" diff --git a/docker-compose.yml b/docker-compose.yml index 3108404b..18154298 100644 --- a/docker-compose.yml +++ b/docker-compose.yml @@ -230,6 +230,10 @@ services: - ENABLE_TOKEN_EXCHANGE=true - TOKEN_EXCHANGE_CACHE_TTL=300 # Cache exchanged tokens for 5 minutes (default) + # Login Flow v2 (ADR-022) with external IdP + - ENABLE_LOGIN_FLOW=true + - ENABLE_DCR=true + # OAuth scopes (optional - uses defaults if not specified) - NEXTCLOUD_OIDC_SCOPES=openid profile email offline_access notes:read notes:write calendar:read calendar:write contacts:read contacts:write cookbook:read cookbook:write deck:read deck:write tables:read tables:write files:read files:write sharing:read sharing:write todo:read todo:write diff --git a/nextcloud_mcp_server/auth/client_registry.py b/nextcloud_mcp_server/auth/client_registry.py index 5a34bc8a..b0d2183f 100644 --- a/nextcloud_mcp_server/auth/client_registry.py +++ b/nextcloud_mcp_server/auth/client_registry.py @@ -50,35 +50,57 @@ class ClientRegistry: self._load_static_clients() def _load_static_clients(self): - """Load statically configured clients from environment.""" - # Load from ALLOWED_MCP_CLIENTS environment variable + """Load statically configured clients from environment. + + Format: comma-separated entries, each either: + - Simple client ID: gets localhost redirect URIs + - client_id|redirect_uri: gets the specified redirect URI + + Redirect URI rules: + - http://localhost:* and http://127.0.0.1:* are allowed (native clients) + - https:// redirect URIs are allowed (cloud clients) + - http:// non-localhost redirect URIs are rejected with a warning + """ + # Deprecation warning for old env var + if os.getenv("ALLOWED_MCP_CLOUD_CLIENTS"): + logger.warning( + "ALLOWED_MCP_CLOUD_CLIENTS is deprecated. " + "Merge entries into ALLOWED_MCP_CLIENTS using the format: " + "client_id|https://redirect-uri" + ) + allowed_clients = os.getenv("ALLOWED_MCP_CLIENTS", "").strip() if allowed_clients: - # Parse comma-separated list - for client_id in allowed_clients.split(","): - client_id = client_id.strip() - if client_id: - # Create basic client info - # In production, would load full metadata from database - self._clients[client_id] = MCPClientInfo( - client_id=client_id, - name=self._get_client_name(client_id), - redirect_uris=["http://localhost:*", "http://127.0.0.1:*"], - allowed_scopes=["openid", "profile", "email", "mcp-server:api"], - is_public=True, - ) - logger.info(f"Registered static client: {client_id}") - - # Load cloud clients (web-based, HTTPS redirect URIs) - # Format: "client_id|redirect_uri,client_id2|redirect_uri2" - cloud_clients = os.getenv("ALLOWED_MCP_CLOUD_CLIENTS", "").strip() - if cloud_clients: - for entry in cloud_clients.split(","): + for entry in allowed_clients.split(","): entry = entry.strip() + if not entry: + continue + if "|" in entry: cid, redirect = entry.split("|", 1) cid, redirect = cid.strip(), redirect.strip() + + if not cid or not redirect: + logger.warning( + f"Skipping malformed ALLOWED_MCP_CLIENTS entry: {entry!r}" + ) + continue + + parsed = urlparse(redirect) + is_loopback = parsed.hostname in ("localhost", "127.0.0.1") + + if parsed.scheme == "https": + pass # HTTPS always allowed + elif parsed.scheme == "http" and is_loopback: + pass # HTTP localhost allowed + else: + logger.warning( + f"Rejecting client {cid!r}: HTTP redirect URIs are only " + f"allowed for localhost, got {redirect!r}" + ) + continue + self._clients[cid] = MCPClientInfo( client_id=cid, name=self._get_client_name(cid), @@ -86,7 +108,16 @@ class ClientRegistry: allowed_scopes=["*"], is_public=True, ) - logger.info(f"Registered cloud client: {cid}") + logger.info(f"Registered static client: {cid}") + else: + self._clients[entry] = MCPClientInfo( + client_id=entry, + name=self._get_client_name(entry), + redirect_uris=["http://localhost:*", "http://127.0.0.1:*"], + allowed_scopes=["*"], + is_public=True, + ) + logger.info(f"Registered static client: {entry}") # Add well-known clients if not explicitly configured if not self._clients: @@ -111,7 +142,7 @@ class ClientRegistry: client_id="claude-desktop", name="Claude Desktop", redirect_uris=["http://localhost:*", "http://127.0.0.1:*"], - allowed_scopes=["openid", "profile", "email", "mcp-server:api"], + allowed_scopes=["*"], is_public=True, metadata={"vendor": "Anthropic"}, ), @@ -119,7 +150,7 @@ class ClientRegistry: client_id="test-mcp-client", name="Test MCP Client", redirect_uris=["http://localhost:*", "http://127.0.0.1:*"], - allowed_scopes=["openid", "profile", "email", "mcp-server:api"], + allowed_scopes=["*"], is_public=True, metadata={"purpose": "testing"}, ), diff --git a/nextcloud_mcp_server/auth/oauth_routes.py b/nextcloud_mcp_server/auth/oauth_routes.py index f52fb789..f8839789 100644 --- a/nextcloud_mcp_server/auth/oauth_routes.py +++ b/nextcloud_mcp_server/auth/oauth_routes.py @@ -1267,7 +1267,7 @@ async def oauth_register_proxy(request: Request) -> JSONResponse: "error": "registration_not_supported", "error_description": "The upstream identity provider does not support " "dynamic client registration. Configure the client statically using " - "ALLOWED_MCP_CLIENTS or ALLOWED_MCP_CLOUD_CLIENTS.", + "ALLOWED_MCP_CLIENTS.", }, status_code=400, ) @@ -1283,7 +1283,7 @@ async def oauth_register_proxy(request: Request) -> JSONResponse: if response.status_code not in (200, 201): logger.error( - f"DCR proxy: Nextcloud registration failed: {response.status_code} {response.text}" + f"DCR proxy: Upstream registration failed: {response.status_code} {response.text}" ) return JSONResponse( response.json() diff --git a/tests/server/keycloak/__init__.py b/tests/server/keycloak/__init__.py new file mode 100644 index 00000000..e69de29b diff --git a/tests/server/keycloak/test_keycloak_clients.py b/tests/server/keycloak/test_keycloak_clients.py new file mode 100644 index 00000000..c6e98287 --- /dev/null +++ b/tests/server/keycloak/test_keycloak_clients.py @@ -0,0 +1,200 @@ +""" +Integration tests for DCR and static OIDC clients against the mcp-keycloak service. + +The mcp-keycloak service uses Keycloak as an external IdP. Keycloak supports +Dynamic Client Registration (RFC 7591/7592), so the DCR proxy forwards +registrations to Keycloak and registers the resulting client locally. + +Static clients configured via ALLOWED_MCP_CLIENTS should work for the +authorization flow without DCR. + +Requires: docker compose --profile keycloak up --build -d +""" + +import base64 +import hashlib +import logging +import secrets + +import httpx +import pytest + +logger = logging.getLogger(__name__) + +pytestmark = [pytest.mark.integration, pytest.mark.keycloak] + +MCP_KEYCLOAK_BASE_URL = "http://localhost:8002" +KEYCLOAK_BASE_URL = "http://localhost:8888" +KEYCLOAK_REALM = "nextcloud-mcp" + + +@pytest.fixture(scope="module") +async def keycloak_mcp_available(): + """Check that the mcp-keycloak service is reachable and return AS metadata.""" + async with httpx.AsyncClient(timeout=10.0) as client: + try: + resp = await client.get( + f"{MCP_KEYCLOAK_BASE_URL}/.well-known/oauth-authorization-server" + ) + resp.raise_for_status() + return resp.json() + except (httpx.ConnectError, httpx.HTTPStatusError) as e: + pytest.skip(f"mcp-keycloak service not available: {e}") + + +@pytest.fixture() +async def dcr_client(keycloak_mcp_available): + """Register a client via DCR proxy and clean up after test.""" + async with httpx.AsyncClient(timeout=30.0) as http: + # Register via MCP proxy + resp = await http.post( + f"{MCP_KEYCLOAK_BASE_URL}/oauth/register", + json={ + "client_name": "test-dcr-keycloak", + "redirect_uris": ["http://localhost:9999/callback"], + "grant_types": ["authorization_code"], + "response_types": ["code"], + "token_endpoint_auth_method": "client_secret_basic", + "scope": "openid profile email", + }, + ) + resp.raise_for_status() + data = resp.json() + + yield data + + # Cleanup: delete via Keycloak directly (proxy URI uses internal hostname) + client_id = data.get("client_id") + rat = data.get("registration_access_token") + if client_id and rat: + async with httpx.AsyncClient(timeout=10.0) as http: + await http.delete( + f"{KEYCLOAK_BASE_URL}/realms/{KEYCLOAK_REALM}" + f"/clients-registrations/openid-connect/{client_id}", + headers={"Authorization": f"Bearer {rat}"}, + ) + + +# --- DCR tests --- + + +async def test_dcr_proxy_registers_client(dcr_client): + """DCR proxy should forward registration to Keycloak and return + RFC 7591 response with client credentials.""" + assert "client_id" in dcr_client + assert "client_secret" in dcr_client + assert dcr_client["client_name"] == "test-dcr-keycloak" + + +async def test_dcr_proxy_returns_rfc7592_fields(dcr_client): + """DCR response should include RFC 7592 management fields for + client lifecycle management.""" + assert "registration_access_token" in dcr_client + assert dcr_client["registration_access_token"] + assert "registration_client_uri" in dcr_client + assert dcr_client["registration_client_uri"] + + +async def test_dcr_client_accepted_by_authorize(keycloak_mcp_available, dcr_client): + """A DCR-registered client should be accepted by the authorization endpoint + (redirects to IdP login rather than returning an error).""" + # Generate PKCE challenge (required by the server) + verifier = secrets.token_urlsafe(64) + challenge = hashlib.sha256(verifier.encode()).digest() + code_challenge = base64.urlsafe_b64encode(challenge).rstrip(b"=").decode() + + async with httpx.AsyncClient(timeout=30.0, follow_redirects=False) as http: + resp = await http.get( + f"{MCP_KEYCLOAK_BASE_URL}/oauth/authorize", + params={ + "response_type": "code", + "client_id": dcr_client["client_id"], + "redirect_uri": "http://localhost:9999/callback", + "state": "test-state", + "scope": "openid", + "code_challenge": code_challenge, + "code_challenge_method": "S256", + }, + ) + + # Should redirect to IdP (302) not error (400) + assert resp.status_code == 302, ( + f"Expected redirect to IdP, got {resp.status_code}: {resp.text}" + ) + + +async def test_dcr_client_deletion_via_keycloak(keycloak_mcp_available): + """Full DCR lifecycle: register via proxy, verify, delete via RFC 7592.""" + async with httpx.AsyncClient(timeout=30.0) as http: + # Register + resp = await http.post( + f"{MCP_KEYCLOAK_BASE_URL}/oauth/register", + json={ + "client_name": "test-dcr-lifecycle", + "redirect_uris": ["http://localhost:9999/callback"], + "grant_types": ["authorization_code"], + "response_types": ["code"], + }, + ) + assert resp.status_code in (200, 201) + data = resp.json() + client_id = data["client_id"] + rat = data["registration_access_token"] + + # Delete via Keycloak (external URL) + del_resp = await http.delete( + f"{KEYCLOAK_BASE_URL}/realms/{KEYCLOAK_REALM}" + f"/clients-registrations/openid-connect/{client_id}", + headers={"Authorization": f"Bearer {rat}"}, + ) + assert del_resp.status_code == 204 + + +# --- AS metadata tests --- + + +async def test_as_metadata_advertises_registration_endpoint(keycloak_mcp_available): + """AS metadata should advertise /oauth/register for DCR discovery.""" + metadata = keycloak_mcp_available + assert "registration_endpoint" in metadata + assert metadata["registration_endpoint"].endswith("/oauth/register") + + +async def test_as_metadata_has_required_fields(keycloak_mcp_available): + """Verify the AS metadata contains all RFC 8414 required fields.""" + metadata = keycloak_mcp_available + required_fields = [ + "issuer", + "authorization_endpoint", + "token_endpoint", + "response_types_supported", + "grant_types_supported", + "code_challenge_methods_supported", + ] + for field in required_fields: + assert field in metadata, f"Missing required field: {field}" + + +# --- Static client / unknown client tests --- + + +async def test_authorize_rejects_unknown_client(keycloak_mcp_available): + """Authorization endpoint should reject client_ids that are neither + statically configured nor dynamically registered.""" + async with httpx.AsyncClient(timeout=30.0, follow_redirects=False) as http: + resp = await http.get( + f"{MCP_KEYCLOAK_BASE_URL}/oauth/authorize", + params={ + "response_type": "code", + "client_id": "nonexistent-client-id", + "redirect_uri": "http://localhost:9999/callback", + "state": "test-state", + "scope": "openid", + }, + ) + + # Should return an error (400 or redirect with error) + assert resp.status_code in (400, 302) + if resp.status_code == 400: + data = resp.json() + assert "error" in data diff --git a/tests/unit/test_client_registry.py b/tests/unit/test_client_registry.py new file mode 100644 index 00000000..d67ce81e --- /dev/null +++ b/tests/unit/test_client_registry.py @@ -0,0 +1,177 @@ +"""Unit tests for ClientRegistry ALLOWED_MCP_CLIENTS parsing and validation.""" + +import logging + +import pytest + +import nextcloud_mcp_server.auth.client_registry as registry_mod + +pytestmark = pytest.mark.unit + + +@pytest.fixture(autouse=True) +def _reset_registry(): + """Reset the singleton registry before each test.""" + registry_mod._registry = None + yield + registry_mod._registry = None + + +def _get_registry(monkeypatch, value: str | None = None): + """Helper to create a registry with the given ALLOWED_MCP_CLIENTS value.""" + if value is not None: + monkeypatch.setenv("ALLOWED_MCP_CLIENTS", value) + else: + monkeypatch.delenv("ALLOWED_MCP_CLIENTS", raising=False) + monkeypatch.delenv("ALLOWED_MCP_CLOUD_CLIENTS", raising=False) + return registry_mod.get_client_registry() + + +def test_simple_client_ids(monkeypatch): + registry = _get_registry(monkeypatch, "claude-desktop, zed-editor") + clients = registry.list_clients() + assert len(clients) == 2 + + claude = registry.get_client("claude-desktop") + assert claude is not None + assert claude.redirect_uris == ["http://localhost:*", "http://127.0.0.1:*"] + assert claude.allowed_scopes == ["*"] + + zed = registry.get_client("zed-editor") + assert zed is not None + assert zed.redirect_uris == ["http://localhost:*", "http://127.0.0.1:*"] + + +def test_pipe_separated_https(monkeypatch): + registry = _get_registry(monkeypatch, "myapp|https://app.example.com/callback") + client = registry.get_client("myapp") + assert client is not None + assert client.redirect_uris == ["https://app.example.com/callback"] + assert client.allowed_scopes == ["*"] + + +def test_pipe_separated_localhost(monkeypatch): + registry = _get_registry(monkeypatch, "dev-tool|http://localhost:3000/cb") + client = registry.get_client("dev-tool") + assert client is not None + assert client.redirect_uris == ["http://localhost:3000/cb"] + + +def test_pipe_separated_loopback_ip(monkeypatch): + registry = _get_registry(monkeypatch, "dev|http://127.0.0.1:9090/cb") + client = registry.get_client("dev") + assert client is not None + assert client.redirect_uris == ["http://127.0.0.1:9090/cb"] + + +def test_mixed_entries(monkeypatch): + registry = _get_registry( + monkeypatch, "claude-desktop, cloud-app|https://cloud.example.com/cb" + ) + clients = registry.list_clients() + assert len(clients) == 2 + + claude = registry.get_client("claude-desktop") + assert claude is not None + assert claude.redirect_uris == ["http://localhost:*", "http://127.0.0.1:*"] + + cloud = registry.get_client("cloud-app") + assert cloud is not None + assert cloud.redirect_uris == ["https://cloud.example.com/cb"] + + +def test_http_non_localhost_rejected(monkeypatch, caplog): + with caplog.at_level(logging.WARNING): + registry = _get_registry(monkeypatch, "bad-client|http://evil.com/cb") + + assert registry.get_client("bad-client") is None + assert "Rejecting client" in caplog.text + assert "evil.com" in caplog.text + + +def test_empty_string_uses_well_known(monkeypatch): + registry = _get_registry(monkeypatch, "") + clients = registry.list_clients() + client_ids = {c.client_id for c in clients} + assert "claude-desktop" in client_ids + assert "test-mcp-client" in client_ids + + +def test_unset_env_uses_well_known(monkeypatch): + registry = _get_registry(monkeypatch, None) + clients = registry.list_clients() + client_ids = {c.client_id for c in clients} + assert "claude-desktop" in client_ids + assert "test-mcp-client" in client_ids + + +def test_malformed_entries_skipped_with_warning(monkeypatch, caplog): + with caplog.at_level(logging.WARNING): + registry = _get_registry(monkeypatch, "good, |, , bad|") + + # Only "good" should be registered + assert registry.get_client("good") is not None + assert len(registry.list_clients()) == 1 + assert "malformed" in caplog.text.lower() + + +def test_all_scopes_wildcard(monkeypatch): + registry = _get_registry(monkeypatch, "test-client") + client = registry.get_client("test-client") + assert client is not None + assert client.allowed_scopes == ["*"] + + +def test_validate_client_wildcard_scopes(monkeypatch): + registry = _get_registry(monkeypatch, "test-client") + valid, err = registry.validate_client( + "test-client", scopes=["anything", "goes", "here"] + ) + assert valid is True + assert err is None + + +def test_validate_redirect_uri_https_match(monkeypatch): + registry = _get_registry(monkeypatch, "cloud|https://x.com/cb") + valid, err = registry.validate_client("cloud", redirect_uri="https://x.com/cb") + assert valid is True + assert err is None + + +def test_validate_redirect_uri_https_mismatch(monkeypatch): + registry = _get_registry(monkeypatch, "cloud|https://x.com/cb") + valid, err = registry.validate_client("cloud", redirect_uri="https://other.com/cb") + assert valid is False + assert "redirect_uri" in err.lower() + + +def test_validate_redirect_uri_localhost_wildcard(monkeypatch): + registry = _get_registry(monkeypatch, "native-client") + valid, err = registry.validate_client( + "native-client", redirect_uri="http://localhost:12345/callback" + ) + assert valid is True + assert err is None + + +def test_well_known_clients_wildcard_scopes(monkeypatch): + registry = _get_registry(monkeypatch, None) + for client in registry.list_clients(): + assert client.allowed_scopes == ["*"], ( + f"Well-known client {client.client_id} should have wildcard scopes" + ) + + +def test_deprecated_cloud_clients_warning(monkeypatch, caplog): + monkeypatch.setenv("ALLOWED_MCP_CLOUD_CLIENTS", "old|https://old.com/cb") + monkeypatch.setenv("ALLOWED_MCP_CLIENTS", "new-client") + with caplog.at_level(logging.WARNING): + registry_mod.get_client_registry() + + assert "ALLOWED_MCP_CLOUD_CLIENTS is deprecated" in caplog.text + + +def test_client_name_resolution(monkeypatch): + registry = _get_registry(monkeypatch, "claude-desktop, custom-tool") + assert registry.get_client("claude-desktop").name == "Claude Desktop" + assert registry.get_client("custom-tool").name == "Custom Tool"