From b50fa9b82488a4f72070f86c67b64ffab1b1aeca Mon Sep 17 00:00:00 2001 From: Chris Coutinho Date: Tue, 14 Oct 2025 23:25:40 +0200 Subject: [PATCH] feat(server): Wait on OIDC client initialization --- .../post-installation/install-oidc-app.sh | 5 ++ .../auth/client_registration.py | 74 ++++++++++++++++- tests/conftest.py | 79 ++++++++++++++++--- 3 files changed, 144 insertions(+), 14 deletions(-) diff --git a/app-hooks/post-installation/install-oidc-app.sh b/app-hooks/post-installation/install-oidc-app.sh index 50c59ab..80e3c28 100755 --- a/app-hooks/post-installation/install-oidc-app.sh +++ b/app-hooks/post-installation/install-oidc-app.sh @@ -17,6 +17,11 @@ patch -u /var/www/html/custom_apps/oidc/lib/Util/DiscoveryGenerator.php -i /dock php /var/www/html/occ config:app:set oidc dynamic_client_registration --value='true' php /var/www/html/occ config:app:set oidc proof_key_for_code_exchange --value=true --type=boolean +# Set the OIDC issuer URL (defaults to http://localhost:8080 if not provided) +OIDC_ISSUER="${NEXTCLOUD_PUBLIC_ISSUER_URL:-http://localhost:8080}" +php /var/www/html/occ config:app:set oidc issuer --value="${OIDC_ISSUER}" +echo "OIDC issuer set to: ${OIDC_ISSUER}" + # Configure user_oidc to validate bearer tokens from the OIDC Identity Provider php /var/www/html/occ config:system:set user_oidc oidc_provider_bearer_validation --value=true --type=boolean diff --git a/nextcloud_mcp_server/auth/client_registration.py b/nextcloud_mcp_server/auth/client_registration.py index 2e2943d..afec07c 100644 --- a/nextcloud_mcp_server/auth/client_registration.py +++ b/nextcloud_mcp_server/auth/client_registration.py @@ -1,5 +1,6 @@ """Dynamic client registration for Nextcloud OIDC.""" +import asyncio import json import logging import os @@ -205,6 +206,65 @@ def save_client_to_file(client_info: ClientInfo, storage_path: Path): raise +async def wait_for_client_propagation( + nextcloud_url: str, + client_id: str, + max_retries: int = 10, + initial_delay: float = 0.5, + max_delay: float = 5.0, +) -> None: + """ + Wait for the registered OAuth client to be fully propagated in Nextcloud. + + This function attempts to verify the client is ready by checking if we can + access OIDC-related endpoints. Uses exponential backoff for retries. + + Args: + nextcloud_url: Base URL of the Nextcloud instance + client_id: The registered client ID + max_retries: Maximum number of retry attempts + initial_delay: Initial delay in seconds before first verification + max_delay: Maximum delay between retries + + Note: + This is a best-effort approach to mitigate race conditions between + client registration and first use. Nextcloud's OIDC provider may need + time to propagate newly registered clients to its cache/database. + """ + # Always wait at least the initial delay to give Nextcloud time to propagate + logger.debug( + f"Waiting {initial_delay}s for OAuth client {client_id[:16]}... to propagate" + ) + await asyncio.sleep(initial_delay) + + # Verify the client is accessible by checking OIDC discovery again + # (this gives Nextcloud additional time to complete any async operations) + discovery_url = f"{nextcloud_url}/.well-known/openid-configuration" + delay = initial_delay + + async with httpx.AsyncClient(timeout=10.0) as client: + for attempt in range(1, max_retries + 1): + try: + response = await client.get(discovery_url) + response.raise_for_status() + logger.debug( + f"OAuth client propagation verification successful (attempt {attempt})" + ) + return + except Exception as e: + if attempt < max_retries: + delay = min(delay * 1.5, max_delay) + logger.debug( + f"Verification attempt {attempt} failed: {e}. Retrying in {delay:.1f}s..." + ) + await asyncio.sleep(delay) + else: + logger.warning( + f"Could not verify client propagation after {max_retries} attempts. " + "Continuing anyway - first authorization may fail." + ) + + async def load_or_register_client( nextcloud_url: str, registration_endpoint: str, @@ -212,6 +272,7 @@ async def load_or_register_client( client_name: str = "Nextcloud MCP Server", redirect_uris: list[str] | None = None, force_register: bool = True, + wait_for_propagation: bool = True, ) -> ClientInfo: """ Load client from storage or register a new one if not found/expired. @@ -220,7 +281,8 @@ async def load_or_register_client( 1. Checks for existing client credentials in storage 2. Validates the credentials are not expired 3. Registers a new client if needed - 4. Saves the new client credentials + 4. Waits for the client to propagate (if newly registered) + 5. Saves the new client credentials Args: nextcloud_url: Base URL of the Nextcloud instance @@ -229,6 +291,7 @@ async def load_or_register_client( client_name: Name of the client application redirect_uris: List of redirect URIs force_register: Force registration even if valid credentials exist + wait_for_propagation: Wait for newly registered clients to propagate (default: True) Returns: ClientInfo with valid credentials @@ -254,6 +317,15 @@ async def load_or_register_client( redirect_uris=redirect_uris, ) + # Wait for client to propagate in Nextcloud's OIDC provider + # This mitigates race conditions where the client is used immediately after registration + if wait_for_propagation: + logger.info("Waiting for OAuth client to propagate in Nextcloud...") + await wait_for_client_propagation( + nextcloud_url=nextcloud_url, + client_id=client_info.client_id, + ) + # Save to storage save_client_to_file(client_info, storage_path) diff --git a/tests/conftest.py b/tests/conftest.py index 8a55fa8..7b125f6 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -652,7 +652,8 @@ def oauth_callback_server(): Fixture to create an HTTP server for OAuth callback handling. Yields a tuple of (auth_state, server_url) where: - - auth_state: A dict with {"code": None} that will be populated with the auth code + - auth_state: A dict with {"code": None, "expected_state": None, "received_state": None} + that will be populated with the auth code and state verification - server_url: The callback URL for the server (e.g., "http://localhost:8081") The server automatically shuts down when the fixture is torn down. @@ -664,7 +665,7 @@ def oauth_callback_server(): from urllib.parse import parse_qs, urlparse # Use a mutable container to share state across threads - auth_state = {"code": None} + auth_state = {"code": None, "expected_state": None, "received_state": None} httpd = None server_thread = None @@ -689,11 +690,42 @@ def oauth_callback_server(): parsed_path = urlparse(self.path) query = parse_qs(parsed_path.query) code = query.get("code", [None])[0] + state = query.get("state", [None])[0] + error = query.get("error", [None])[0] + + # Check for OAuth error + if error: + error_description = query.get("error_description", ["Unknown error"])[0] + logger.error(f"OAuth error received: {error} - {error_description}") + self.send_response(400) + self.send_header("Content-type", "text/html") + self.end_headers() + self.wfile.write( + f"

OAuth Error

{error}: {error_description}

".encode() + ) + return + + # Verify state parameter if expected_state is set + if auth_state["expected_state"] and state != auth_state["expected_state"]: + logger.error( + f"State mismatch! Expected: {auth_state['expected_state'][:20]}..., " + f"Received: {state[:20] if state else 'None'}..." + ) + self.send_response(400) + self.send_header("Content-type", "text/html") + self.end_headers() + self.wfile.write( + b"

State Verification Failed

CSRF protection triggered.

" + ) + return # Only process if we have a valid code if code: auth_state["code"] = code + auth_state["received_state"] = state logger.info(f"OAuth callback received. Code: {code[:20]}...") + if state: + logger.debug(f"State verified: {state[:20]}...") self.send_response(200) self.send_header("Content-type", "text/html") self.end_headers() @@ -741,8 +773,10 @@ async def interactive_oauth_token(oauth_callback_server) -> str: Automatically skips when running in GitHub Actions CI. """ + import secrets import time import webbrowser + from urllib.parse import urlencode from nextcloud_mcp_server.auth.client_registration import load_or_register_client @@ -757,25 +791,38 @@ async def interactive_oauth_token(oauth_callback_server) -> str: token_endpoint = oidc_config.get("token_endpoint") registration_endpoint = oidc_config.get("registration_endpoint") authorization_endpoint = oidc_config.get("authorization_endpoint") + + # Try to load existing client first, register only if needed client_info = await load_or_register_client( nextcloud_url=nextcloud_host, registration_endpoint=registration_endpoint, storage_path=".nextcloud_oauth_test_client.json", redirect_uris=[callback_url], - force_register=True, + force_register=False, # Only register if no valid client exists + wait_for_propagation=True, ) - # First, open Nextcloud login page to establish session - login_url = f"{nextcloud_host}/login" - logger.info(f"Please log in to Nextcloud at: {login_url}") - logger.info( - "After logging in, the OAuth authorization will proceed automatically" - ) + # Generate state parameter for CSRF protection + state = secrets.token_urlsafe(32) + auth_state["expected_state"] = state - # Construct authorization URL - auth_url = f"{authorization_endpoint}?response_type=code&client_id={client_info.client_id}&redirect_uri={callback_url}&scope=openid%20profile%20email" + # Construct authorization URL with proper parameters + auth_params = { + "response_type": "code", + "client_id": client_info.client_id, + "redirect_uri": callback_url, + "scope": "openid profile email", + "state": state, + } + auth_url = f"{authorization_endpoint}?{urlencode(auth_params)}" # Open authorization URL in browser + # Nextcloud will automatically redirect to login if needed + logger.info("Opening OAuth authorization URL in browser...") + logger.info( + "Please log in to Nextcloud if prompted, then authorize the application." + ) + logger.info(f"Authorization URL: {auth_url[:80]}...") webbrowser.open(auth_url) # Wait for auth code with timeout @@ -784,8 +831,7 @@ async def interactive_oauth_token(oauth_callback_server) -> str: while not auth_state["code"]: if time.time() - start_time > timeout: raise TimeoutError("OAuth authorization timed out after 2 minutes") - logger.info("Waiting for OAuth authorization...") - time.sleep(1) + await asyncio.sleep(1) auth_code = auth_state["code"] logger.info("Received authorization code, exchanging for token...") @@ -891,6 +937,13 @@ async def playwright_oauth_token(browser) -> str: client_id = client_info_dict["client_id"] client_secret = client_info_dict["client_secret"] + # Wait for client to propagate in Nextcloud's OIDC provider + # This mitigates race conditions where the client is used immediately after registration + logger.info( + f"Waiting for OAuth client {client_id[:16]}... to propagate in Nextcloud..." + ) + await asyncio.sleep(0.5) + # Construct authorization URL auth_url = ( f"{authorization_endpoint}?"