From 2a340154439269757209441d2551d12c563a526b Mon Sep 17 00:00:00 2001 From: Chris Coutinho Date: Sat, 4 Apr 2026 22:17:01 +0200 Subject: [PATCH 1/8] fix: support cloud OAuth clients and graceful DCR fallback Claude AI (web) sends a Cognito-issued client_id with an HTTPS redirect URI, but the client registry only supported localhost redirect URIs via ALLOWED_MCP_CLIENTS. Add ALLOWED_MCP_CLOUD_CLIENTS env var for web-based clients with format "client_id|redirect_uri". Also fix the DCR proxy to return a clear error when the upstream IdP (e.g. Cognito) doesn't support dynamic client registration, instead of silently falling back to a Nextcloud-specific endpoint that fails. Co-Authored-By: Claude Opus 4.6 (1M context) --- nextcloud_mcp_server/auth/client_registry.py | 19 +++++++++++++ nextcloud_mcp_server/auth/oauth_routes.py | 29 ++++++++++++-------- 2 files changed, 37 insertions(+), 11 deletions(-) diff --git a/nextcloud_mcp_server/auth/client_registry.py b/nextcloud_mcp_server/auth/client_registry.py index 85541cab..5a34bc8a 100644 --- a/nextcloud_mcp_server/auth/client_registry.py +++ b/nextcloud_mcp_server/auth/client_registry.py @@ -70,6 +70,24 @@ class ClientRegistry: ) 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(","): + entry = entry.strip() + if "|" in entry: + cid, redirect = entry.split("|", 1) + cid, redirect = cid.strip(), redirect.strip() + self._clients[cid] = MCPClientInfo( + client_id=cid, + name=self._get_client_name(cid), + redirect_uris=[redirect], + allowed_scopes=["*"], + is_public=True, + ) + logger.info(f"Registered cloud client: {cid}") + # Add well-known clients if not explicitly configured if not self._clients: self._add_well_known_clients() @@ -78,6 +96,7 @@ class ClientRegistry: """Get human-readable name for client_id.""" known_names = { "claude-desktop": "Claude Desktop", + "claude-ai": "Claude AI", "continue-dev": "Continue IDE Extension", "zed-editor": "Zed Editor", "vscode-mcp": "VS Code MCP Extension", diff --git a/nextcloud_mcp_server/auth/oauth_routes.py b/nextcloud_mcp_server/auth/oauth_routes.py index 63beff2b..f52fb789 100644 --- a/nextcloud_mcp_server/auth/oauth_routes.py +++ b/nextcloud_mcp_server/auth/oauth_routes.py @@ -1228,7 +1228,6 @@ async def oauth_register_proxy(request: Request) -> JSONResponse: ) oauth_config = oauth_ctx["config"] - nextcloud_host = oauth_config["nextcloud_host"] # Rate limit DCR requests per client IP client_ip = request.client.host if request.client else "unknown" @@ -1249,21 +1248,29 @@ async def oauth_register_proxy(request: Request) -> JSONResponse: timestamps.append(now) _dcr_rate_limit[client_ip] = timestamps - # Discover registration endpoint from OIDC discovery (prefer over hardcoded path) + # Discover registration endpoint from OIDC discovery discovery_url = oauth_config.get("discovery_url") + registration_endpoint = None if discovery_url: try: discovery = await _get_cached_discovery(discovery_url) - registration_endpoint = discovery.get( - "registration_endpoint", f"{nextcloud_host}/apps/oidc/register" - ) + registration_endpoint = discovery.get("registration_endpoint") except Exception: - logger.warning( - "Failed to fetch OIDC discovery for DCR endpoint, using fallback" - ) - registration_endpoint = f"{nextcloud_host}/apps/oidc/register" - else: - registration_endpoint = f"{nextcloud_host}/apps/oidc/register" + logger.warning("Failed to fetch OIDC discovery for DCR endpoint") + + if not registration_endpoint: + logger.warning( + "DCR proxy: Upstream IdP does not support dynamic client registration" + ) + return 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.", + }, + status_code=400, + ) logger.info(f"DCR proxy: Forwarding registration to {registration_endpoint}") From 91e7665f41c560b1affe607113f52b34a983edac Mon Sep 17 00:00:00 2001 From: Chris Coutinho Date: Sun, 5 Apr 2026 14:33:29 +0200 Subject: [PATCH 2/8] refactor: consolidate ALLOWED_MCP_CLIENTS and add redirect URI validation Merge ALLOWED_MCP_CLOUD_CLIENTS into a single ALLOWED_MCP_CLIENTS env var that supports both simple client IDs and pipe-separated client_id|redirect_uri entries. Enforce HTTPS for non-localhost redirect URIs, warn on malformed entries, and use wildcard scopes for all static clients (upstream IdP enforces actual scopes). Add deprecation warning for the old env var. Also fixes DCR proxy error messages to reference only ALLOWED_MCP_CLIENTS and use "Upstream" instead of "Nextcloud" for IdP-agnostic language. Enables Login Flow v2 + DCR on the mcp-keycloak docker-compose service. Adds 17 unit tests for ClientRegistry parsing/validation and 7 keycloak integration tests for DCR lifecycle, AS metadata, and client authorization. Co-Authored-By: Claude Opus 4.6 (1M context) --- ...-authentication-causing-session-logo.patch | 69 ------ app-hooks/patches/cors-bearer-token.patch | 18 -- .../20-apply-cors-bearer-token-patch.sh | 64 ------ docker-compose.yml | 4 + nextcloud_mcp_server/auth/client_registry.py | 81 ++++--- nextcloud_mcp_server/auth/oauth_routes.py | 4 +- tests/server/keycloak/__init__.py | 0 .../server/keycloak/test_keycloak_clients.py | 200 ++++++++++++++++++ tests/unit/test_client_registry.py | 177 ++++++++++++++++ 9 files changed, 439 insertions(+), 178 deletions(-) delete mode 100644 app-hooks/patches/0001-Fix-Bearer-token-authentication-causing-session-logo.patch delete mode 100644 app-hooks/patches/cors-bearer-token.patch delete mode 100755 app-hooks/post-installation/20-apply-cors-bearer-token-patch.sh create mode 100644 tests/server/keycloak/__init__.py create mode 100644 tests/server/keycloak/test_keycloak_clients.py create mode 100644 tests/unit/test_client_registry.py 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" From 7d775d2a5242737cf8a27246005333494c75ac1d Mon Sep 17 00:00:00 2001 From: Chris Coutinho Date: Sun, 5 Apr 2026 15:06:56 +0200 Subject: [PATCH 3/8] refactor: remove ALLOWED_MCP_CLOUD_CLIENTS and add keycloak CI profile MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Remove the unused ALLOWED_MCP_CLOUD_CLIENTS env var — all clients are defined via ALLOWED_MCP_CLIENTS or the static well-known defaults. Add keycloak as an integration test profile in CI now that login-flow replaces the old bearer token approach for external IdPs. Co-Authored-By: Claude Opus 4.6 (1M context) --- .github/workflows/test.yml | 37 +++++++++++++++++++- nextcloud_mcp_server/auth/client_registry.py | 8 ----- tests/unit/test_client_registry.py | 10 ------ 3 files changed, 36 insertions(+), 19 deletions(-) diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 98f03490..a1183929 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -43,6 +43,7 @@ jobs: - "single-user" - "multi-user-basic" - "login-flow" + - "keycloak" include: # Version-specific image pins — Renovate updates these via customManagers in renovate.json # Each entry is pinned to its major version (e.g., NC 31 only gets 31.x updates) @@ -81,6 +82,14 @@ jobs: needs-playwright: true extra-args: "" + - mode: keycloak + profile: keycloak + markers: "keycloak" + wait-port: 8002 + mcp-internal-url: "http://mcp-keycloak:8002" + needs-playwright: true + extra-args: "" + name: integration (${{ matrix.mode }} / nc${{ matrix.nextcloud_version }}) steps: @@ -174,14 +183,40 @@ jobs: done echo "MCP service is ready on port ${{ matrix.wait-port }}." + - name: Wait for Keycloak + if: matrix.mode == 'keycloak' + run: | + echo "Waiting for Keycloak realm at http://localhost:8888..." + max_attempts=30 + attempt=0 + until curl -sf http://localhost:8888/realms/nextcloud-mcp > /dev/null 2>&1; do + attempt=$((attempt + 1)) + if [ $attempt -ge $max_attempts ]; then + echo "Keycloak did not become ready in time." + docker compose --profile keycloak logs keycloak + exit 1 + fi + echo "Attempt $attempt/$max_attempts: Not ready, sleeping 5s..." + sleep 5 + done + echo "Keycloak is ready." + - name: Verify OIDC configuration - if: matrix.mode == 'login-flow' + if: matrix.mode == 'login-flow' || matrix.mode == 'keycloak' run: | echo "=== OIDC Discovery ===" curl -s http://localhost:8080/.well-known/openid-configuration | jq . echo "=== OIDC App Status ===" docker compose exec -T app php occ app:list --output=json 2>/dev/null | jq '.enabled.oidc // "NOT INSTALLED"' + - name: Verify Keycloak realm + if: matrix.mode == 'keycloak' + run: | + echo "=== Keycloak Realm Discovery ===" + curl -s http://localhost:8888/realms/nextcloud-mcp/.well-known/openid-configuration | jq . + echo "=== Keycloak Provider in Nextcloud ===" + docker compose exec -T app php occ user_oidc:provider keycloak 2>/dev/null || echo "Provider not yet configured" + - name: Run tests (${{ matrix.mode }}) env: NEXTCLOUD_HOST: "http://localhost:8080" diff --git a/nextcloud_mcp_server/auth/client_registry.py b/nextcloud_mcp_server/auth/client_registry.py index b0d2183f..91958e7b 100644 --- a/nextcloud_mcp_server/auth/client_registry.py +++ b/nextcloud_mcp_server/auth/client_registry.py @@ -61,14 +61,6 @@ class ClientRegistry: - 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: diff --git a/tests/unit/test_client_registry.py b/tests/unit/test_client_registry.py index d67ce81e..f8836805 100644 --- a/tests/unit/test_client_registry.py +++ b/tests/unit/test_client_registry.py @@ -23,7 +23,6 @@ def _get_registry(monkeypatch, value: str | None = 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() @@ -162,15 +161,6 @@ def test_well_known_clients_wildcard_scopes(monkeypatch): ) -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" From 3038e3b9366ac1aa2ea4bf38400f0eb13dfd6a46 Mon Sep 17 00:00:00 2001 From: "renovate-bot-cbcoutinho[bot]" <210269379+renovate-bot-cbcoutinho[bot]@users.noreply.github.com> Date: Sun, 5 Apr 2026 16:13:51 +0000 Subject: [PATCH 4/8] chore(deps): update anthropics/claude-code-action action to v1.0.89 --- .github/workflows/claude-code-review.yml | 2 +- .github/workflows/claude.yml | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/.github/workflows/claude-code-review.yml b/.github/workflows/claude-code-review.yml index 7bd7ffb7..2698e3aa 100644 --- a/.github/workflows/claude-code-review.yml +++ b/.github/workflows/claude-code-review.yml @@ -33,7 +33,7 @@ jobs: - name: Run Claude Code Review id: claude-review - uses: anthropics/claude-code-action@58dbe8ed6879f0d3b02ac295b20d5fdfe7733e0c # v1.0.85 + uses: anthropics/claude-code-action@6e2bd52842c65e914eba5c8badd17560bd26b5de # v1.0.89 with: claude_code_oauth_token: ${{ secrets.CLAUDE_CODE_OAUTH_TOKEN }} allowed_bots: "renovate-bot-cbcoutinho" diff --git a/.github/workflows/claude.yml b/.github/workflows/claude.yml index 3cdb444a..bbec39d9 100644 --- a/.github/workflows/claude.yml +++ b/.github/workflows/claude.yml @@ -32,7 +32,7 @@ jobs: - name: Run Claude Code id: claude - uses: anthropics/claude-code-action@58dbe8ed6879f0d3b02ac295b20d5fdfe7733e0c # v1.0.85 + uses: anthropics/claude-code-action@6e2bd52842c65e914eba5c8badd17560bd26b5de # v1.0.89 with: claude_code_oauth_token: ${{ secrets.CLAUDE_CODE_OAUTH_TOKEN }} From a9ca0ce98f70c65e886fb56d0f66c02cc3218fc9 Mon Sep 17 00:00:00 2001 From: "github-actions[bot]" Date: Sun, 5 Apr 2026 16:44:40 +0000 Subject: [PATCH 5/8] =?UTF-8?q?bump:=20version=200.58.26=20=E2=86=92=200.5?= =?UTF-8?q?8.27?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- charts/nextcloud-mcp-server/.cz.toml | 2 +- charts/nextcloud-mcp-server/CHANGELOG.md | 2 ++ charts/nextcloud-mcp-server/Chart.yaml | 2 +- 3 files changed, 4 insertions(+), 2 deletions(-) diff --git a/charts/nextcloud-mcp-server/.cz.toml b/charts/nextcloud-mcp-server/.cz.toml index 5f361ec5..6e716e65 100644 --- a/charts/nextcloud-mcp-server/.cz.toml +++ b/charts/nextcloud-mcp-server/.cz.toml @@ -1,6 +1,6 @@ [tool.commitizen] name = "cz_conventional_commits" -version = "0.58.26" +version = "0.58.27" tag_format = "nextcloud-mcp-server-$version" version_scheme = "semver" update_changelog_on_bump = true diff --git a/charts/nextcloud-mcp-server/CHANGELOG.md b/charts/nextcloud-mcp-server/CHANGELOG.md index faa02a02..24716a1f 100644 --- a/charts/nextcloud-mcp-server/CHANGELOG.md +++ b/charts/nextcloud-mcp-server/CHANGELOG.md @@ -14,6 +14,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Configurable resource limits - Grafana dashboard annotations +## nextcloud-mcp-server-0.58.27 (2026-04-05) + ## nextcloud-mcp-server-0.58.26 (2026-04-04) ### Fix diff --git a/charts/nextcloud-mcp-server/Chart.yaml b/charts/nextcloud-mcp-server/Chart.yaml index 1815e1b8..764f1ddb 100644 --- a/charts/nextcloud-mcp-server/Chart.yaml +++ b/charts/nextcloud-mcp-server/Chart.yaml @@ -2,7 +2,7 @@ apiVersion: v2 name: nextcloud-mcp-server description: A Helm chart for Nextcloud MCP Server - enables AI assistants to interact with Nextcloud type: application -version: 0.58.26 +version: 0.58.27 appVersion: "0.68.2" keywords: - nextcloud From b07b7131468b45ed93fbbc2b35e66bffa92b1851 Mon Sep 17 00:00:00 2001 From: Chris Coutinho Date: Sun, 5 Apr 2026 19:29:23 +0200 Subject: [PATCH 6/8] fix: address PR review feedback for client registry and DCR proxy - Document wildcard scope policy in ClientRegistry class docstring - Add hostname None guard and IPv6 loopback (::1) to redirect URI validation - Simplify redirect URI scheme validation into single guard clause - Add try/finally cleanup to DCR client deletion test - Validate 302 Location header in unknown client rejection test - Add unit tests for IPv6 loopback, malformed URIs, and DCR proxy paths Co-Authored-By: Claude Opus 4.6 (1M context) --- nextcloud_mcp_server/auth/client_registry.py | 28 ++++++-- .../server/keycloak/test_keycloak_clients.py | 32 +++++++-- tests/unit/test_client_registry.py | 33 ++++++++++ tests/unit/test_dcr_proxy.py | 65 +++++++++++++++++++ 4 files changed, 144 insertions(+), 14 deletions(-) create mode 100644 tests/unit/test_dcr_proxy.py diff --git a/nextcloud_mcp_server/auth/client_registry.py b/nextcloud_mcp_server/auth/client_registry.py index 91958e7b..af46828b 100644 --- a/nextcloud_mcp_server/auth/client_registry.py +++ b/nextcloud_mcp_server/auth/client_registry.py @@ -36,6 +36,12 @@ class ClientRegistry: 2. Integrate with IdP client registry 3. Store client metadata in database 4. Support client updates and revocation + + Scope Policy: + All clients are registered with allowed_scopes=["*"] (wildcard). + The MCP server acts as an OAuth AS proxy — it validates client + identity and redirect URIs locally, but delegates scope enforcement + to the upstream IdP (Nextcloud or Keycloak). """ def __init__(self, allow_dynamic_registration: bool = False): @@ -80,13 +86,19 @@ class ClientRegistry: continue parsed = urlparse(redirect) - is_loopback = parsed.hostname in ("localhost", "127.0.0.1") + hostname = parsed.hostname + if hostname is None: + logger.warning( + f"Skipping client {cid!r}: cannot parse hostname " + f"from {redirect!r}" + ) + continue + is_loopback = hostname in ("localhost", "127.0.0.1", "::1") - if parsed.scheme == "https": - pass # HTTPS always allowed - elif parsed.scheme == "http" and is_loopback: - pass # HTTP localhost allowed - else: + if not ( + parsed.scheme == "https" + or (parsed.scheme == "http" and is_loopback) + ): logger.warning( f"Rejecting client {cid!r}: HTTP redirect URIs are only " f"allowed for localhost, got {redirect!r}" @@ -205,6 +217,8 @@ class ClientRegistry: """ # Parse the redirect URI parsed = urlparse(redirect_uri) + if not parsed.hostname: + return False # Check against registered patterns for pattern in client.redirect_uris: @@ -213,7 +227,7 @@ class ClientRegistry: pattern_base = pattern.replace(":*", "") if redirect_uri.startswith(pattern_base + ":"): # Validate it's localhost with a port - if parsed.hostname in ["localhost", "127.0.0.1"]: + if parsed.hostname in ("localhost", "127.0.0.1", "::1"): return True elif redirect_uri == pattern: return True diff --git a/tests/server/keycloak/test_keycloak_clients.py b/tests/server/keycloak/test_keycloak_clients.py index c6e98287..d0467aaa 100644 --- a/tests/server/keycloak/test_keycloak_clients.py +++ b/tests/server/keycloak/test_keycloak_clients.py @@ -141,13 +141,25 @@ async def test_dcr_client_deletion_via_keycloak(keycloak_mcp_available): 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 + try: + # 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 + except Exception: + # Best-effort cleanup if assertion failed + try: + await http.delete( + f"{KEYCLOAK_BASE_URL}/realms/{KEYCLOAK_REALM}" + f"/clients-registrations/openid-connect/{client_id}", + headers={"Authorization": f"Bearer {rat}"}, + ) + except Exception: + pass + raise # --- AS metadata tests --- @@ -198,3 +210,9 @@ async def test_authorize_rejects_unknown_client(keycloak_mcp_available): if resp.status_code == 400: data = resp.json() assert "error" in data + else: + # 302 redirect must carry an error parameter + location = resp.headers.get("location", "") + assert "error=" in location, ( + f"302 redirect should contain error= in Location, got: {location}" + ) diff --git a/tests/unit/test_client_registry.py b/tests/unit/test_client_registry.py index f8836805..30c5709b 100644 --- a/tests/unit/test_client_registry.py +++ b/tests/unit/test_client_registry.py @@ -165,3 +165,36 @@ 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" + + +def test_ipv6_loopback_allowed(monkeypatch): + registry = _get_registry(monkeypatch, "ipv6-app|http://[::1]:3000/cb") + client = registry.get_client("ipv6-app") + assert client is not None + assert client.redirect_uris == ["http://[::1]:3000/cb"] + + +def test_malformed_uri_no_hostname_skipped(monkeypatch, caplog): + with caplog.at_level(logging.WARNING): + registry = _get_registry(monkeypatch, "bad|http:///no-host") + + assert registry.get_client("bad") is None + assert "cannot parse hostname" in caplog.text + + +def test_validate_redirect_uri_ipv6_loopback(monkeypatch): + """IPv6 loopback redirect URIs should match wildcard localhost patterns.""" + registry = _get_registry(monkeypatch, "ipv6-app|http://[::1]:3000/cb") + valid, err = registry.validate_client( + "ipv6-app", redirect_uri="http://[::1]:3000/cb" + ) + assert valid is True + assert err is None + + +def test_validate_redirect_uri_no_hostname(monkeypatch): + """Redirect URIs with no parseable hostname should be rejected.""" + registry = _get_registry(monkeypatch, "test-client") + valid, err = registry.validate_client("test-client", redirect_uri="not-a-uri") + assert valid is False + assert "redirect_uri" in err.lower() diff --git a/tests/unit/test_dcr_proxy.py b/tests/unit/test_dcr_proxy.py new file mode 100644 index 00000000..07eed461 --- /dev/null +++ b/tests/unit/test_dcr_proxy.py @@ -0,0 +1,65 @@ +"""Unit tests for DCR proxy registration_not_supported path.""" + +import json +from unittest.mock import AsyncMock, MagicMock, patch + +import pytest + +from nextcloud_mcp_server.auth.oauth_routes import oauth_register_proxy + +pytestmark = pytest.mark.unit + + +def _make_request(body: dict, oauth_config: dict) -> MagicMock: + """Create a mock Starlette Request.""" + request = AsyncMock() + request.json = AsyncMock(return_value=body) + request.client = MagicMock() + request.client.host = "127.0.0.1" + request.app = MagicMock() + request.app.state.oauth_context = {"config": oauth_config} + return request + + +_DCR_BODY = { + "client_name": "test", + "redirect_uris": ["http://localhost:9999/cb"], +} + + +async def test_registration_not_supported_when_no_endpoint(): + """When discovery doc lacks registration_endpoint, return 400.""" + request = _make_request( + body=_DCR_BODY, + oauth_config={ + "discovery_url": "https://idp.example.com/.well-known/openid-configuration" + }, + ) + + discovery_doc = { + "issuer": "https://idp.example.com", + "authorization_endpoint": "https://idp.example.com/auth", + } + + with patch( + "nextcloud_mcp_server.auth.oauth_routes._get_cached_discovery", + new_callable=AsyncMock, + return_value=discovery_doc, + ): + response = await oauth_register_proxy(request) + + assert response.status_code == 400 + body = json.loads(response.body) + assert body["error"] == "registration_not_supported" + assert "ALLOWED_MCP_CLIENTS" in body["error_description"] + + +async def test_registration_not_supported_when_no_discovery_url(): + """When no discovery_url is configured, return 400.""" + request = _make_request(body=_DCR_BODY, oauth_config={}) + + response = await oauth_register_proxy(request) + + assert response.status_code == 400 + body = json.loads(response.body) + assert body["error"] == "registration_not_supported" From 9f632fa9603713e63bbff66c168f6cb6a480acbc Mon Sep 17 00:00:00 2001 From: "github-actions[bot]" Date: Sun, 5 Apr 2026 17:40:51 +0000 Subject: [PATCH 7/8] =?UTF-8?q?bump:=20version=200.68.2=20=E2=86=92=200.68?= =?UTF-8?q?.3?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- CHANGELOG.md | 12 ++++++++++++ charts/nextcloud-mcp-server/Chart.yaml | 2 +- pyproject.toml | 2 +- uv.lock | 2 +- 4 files changed, 15 insertions(+), 3 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index c288fba2..d84d49b7 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,18 @@ All notable changes to the Nextcloud MCP Server will be documented in this file. The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), and this project adheres to [PEP 440](https://peps.python.org/pep-0440/). +## v0.68.3 (2026-04-05) + +### Fix + +- address PR review feedback for client registry and DCR proxy +- support cloud OAuth clients and graceful DCR fallback + +### Refactor + +- remove ALLOWED_MCP_CLOUD_CLIENTS and add keycloak CI profile +- consolidate ALLOWED_MCP_CLIENTS and add redirect URI validation + ## v0.68.2 (2026-04-04) ### Fix diff --git a/charts/nextcloud-mcp-server/Chart.yaml b/charts/nextcloud-mcp-server/Chart.yaml index 764f1ddb..a85465ce 100644 --- a/charts/nextcloud-mcp-server/Chart.yaml +++ b/charts/nextcloud-mcp-server/Chart.yaml @@ -3,7 +3,7 @@ name: nextcloud-mcp-server description: A Helm chart for Nextcloud MCP Server - enables AI assistants to interact with Nextcloud type: application version: 0.58.27 -appVersion: "0.68.2" +appVersion: "0.68.3" keywords: - nextcloud - mcp diff --git a/pyproject.toml b/pyproject.toml index 9900fe15..4fd62638 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -1,6 +1,6 @@ [project] name = "nextcloud-mcp-server" -version = "0.68.2" +version = "0.68.3" description = "Model Context Protocol (MCP) server for Nextcloud integration - enables AI assistants to interact with Nextcloud data" authors = [ {name = "Chris Coutinho", email = "chris@coutinho.io"} diff --git a/uv.lock b/uv.lock index e15e26ac..fa8835ab 100644 --- a/uv.lock +++ b/uv.lock @@ -2086,7 +2086,7 @@ wheels = [ [[package]] name = "nextcloud-mcp-server" -version = "0.68.2" +version = "0.68.3" source = { editable = "." } dependencies = [ { name = "aiosqlite" }, From 899b9c71916446e3af776c4c6110d6bc932f53f6 Mon Sep 17 00:00:00 2001 From: "github-actions[bot]" Date: Sun, 5 Apr 2026 17:40:51 +0000 Subject: [PATCH 8/8] =?UTF-8?q?bump:=20version=200.58.27=20=E2=86=92=200.5?= =?UTF-8?q?8.28?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- charts/nextcloud-mcp-server/.cz.toml | 2 +- charts/nextcloud-mcp-server/CHANGELOG.md | 12 ++++++++++++ charts/nextcloud-mcp-server/Chart.yaml | 2 +- 3 files changed, 14 insertions(+), 2 deletions(-) diff --git a/charts/nextcloud-mcp-server/.cz.toml b/charts/nextcloud-mcp-server/.cz.toml index 6e716e65..a582b622 100644 --- a/charts/nextcloud-mcp-server/.cz.toml +++ b/charts/nextcloud-mcp-server/.cz.toml @@ -1,6 +1,6 @@ [tool.commitizen] name = "cz_conventional_commits" -version = "0.58.27" +version = "0.58.28" tag_format = "nextcloud-mcp-server-$version" version_scheme = "semver" update_changelog_on_bump = true diff --git a/charts/nextcloud-mcp-server/CHANGELOG.md b/charts/nextcloud-mcp-server/CHANGELOG.md index 24716a1f..f0f149ae 100644 --- a/charts/nextcloud-mcp-server/CHANGELOG.md +++ b/charts/nextcloud-mcp-server/CHANGELOG.md @@ -14,6 +14,18 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Configurable resource limits - Grafana dashboard annotations +## nextcloud-mcp-server-0.58.28 (2026-04-05) + +### Fix + +- address PR review feedback for client registry and DCR proxy +- support cloud OAuth clients and graceful DCR fallback + +### Refactor + +- remove ALLOWED_MCP_CLOUD_CLIENTS and add keycloak CI profile +- consolidate ALLOWED_MCP_CLIENTS and add redirect URI validation + ## nextcloud-mcp-server-0.58.27 (2026-04-05) ## nextcloud-mcp-server-0.58.26 (2026-04-04) diff --git a/charts/nextcloud-mcp-server/Chart.yaml b/charts/nextcloud-mcp-server/Chart.yaml index a85465ce..1fc7425a 100644 --- a/charts/nextcloud-mcp-server/Chart.yaml +++ b/charts/nextcloud-mcp-server/Chart.yaml @@ -2,7 +2,7 @@ apiVersion: v2 name: nextcloud-mcp-server description: A Helm chart for Nextcloud MCP Server - enables AI assistants to interact with Nextcloud type: application -version: 0.58.27 +version: 0.58.28 appVersion: "0.68.3" keywords: - nextcloud