From 081ecbe4018b5b32be74da43c74f6f6d01f9d034 Mon Sep 17 00:00:00 2001 From: Chris Coutinho Date: Sun, 29 Mar 2026 21:56:16 +0200 Subject: [PATCH 01/10] feat: add web-based Login Flow v2 provisioning endpoint Add /app/provision and /app/provision/status endpoints for browser-based Login Flow v2 app password provisioning. Used by Astrolabe's "Enable Semantic Search" to chain OAuth (bearer token) + Login Flow v2 (app password) in a single user interaction. The provision page initiates Login Flow v2, opens Nextcloud's login URL in a popup, polls for completion via background task, and redirects back to the caller's redirect_uri on success. Co-Authored-By: Claude Opus 4.6 (1M context) --- nextcloud_mcp_server/app.py | 9 + nextcloud_mcp_server/auth/provision_routes.py | 466 ++++++++++++++++++ 2 files changed, 475 insertions(+) create mode 100644 nextcloud_mcp_server/auth/provision_routes.py diff --git a/nextcloud_mcp_server/app.py b/nextcloud_mcp_server/app.py index e98151ba..551b4bcc 100644 --- a/nextcloud_mcp_server/app.py +++ b/nextcloud_mcp_server/app.py @@ -73,6 +73,10 @@ from nextcloud_mcp_server.auth.oauth_routes import ( oauth_register_proxy, oauth_token_endpoint, ) +from nextcloud_mcp_server.auth.provision_routes import ( + provision_page, + provision_status, +) from nextcloud_mcp_server.auth.session_backend import SessionAuthBackend from nextcloud_mcp_server.auth.storage import RefreshTokenStorage, get_shared_storage from nextcloud_mcp_server.auth.token_broker import TokenBrokerService @@ -2317,6 +2321,11 @@ def get_app(transport: str = "streamable-http", enabled_apps: list[str] | None = chunk_context_endpoint, methods=["GET"], ), # /app/chunk-context + # Login Flow v2 web provisioning (used by Astrolabe) + Route("/provision", provision_page, methods=["GET"]), # /app/provision + Route( + "/provision/status", provision_status, methods=["GET"] + ), # /app/provision/status # Webhook management routes (admin-only) Route("/webhooks", webhook_management_pane, methods=["GET"]), # /app/webhooks Route( diff --git a/nextcloud_mcp_server/auth/provision_routes.py b/nextcloud_mcp_server/auth/provision_routes.py new file mode 100644 index 00000000..54c4b52e --- /dev/null +++ b/nextcloud_mcp_server/auth/provision_routes.py @@ -0,0 +1,466 @@ +"""Web-based Login Flow v2 provisioning routes. + +Provides browser endpoints for provisioning Nextcloud app passwords via +Login Flow v2. Used by Astrolabe's "Enable Semantic Search" flow to +chain OAuth (bearer token) with Login Flow v2 (app password) in a single +user interaction. + +Flow: +1. GET /app/provision?redirect_uri=... → Initiates LFv2, renders polling page +2. The page opens Nextcloud's login URL in a popup window +3. User clicks "Grant access" in the popup +4. Page polls GET /app/provision/status?id=... for completion +5. On success, redirects to redirect_uri +""" + +import asyncio +import logging +import secrets +import time +from urllib.parse import urlparse + +from starlette.requests import Request +from starlette.responses import HTMLResponse, JSONResponse + +from nextcloud_mcp_server.auth.login_flow import LoginFlowV2Client +from nextcloud_mcp_server.auth.storage import get_shared_storage +from nextcloud_mcp_server.config import get_nextcloud_ssl_verify, get_settings + +logger = logging.getLogger(__name__) + +# In-memory store for web provision sessions (short-lived, no persistence needed) +# Maps provision_id → session data +_provision_sessions: dict[str, dict] = {} + +# Session TTL: 20 minutes (matches Nextcloud's Login Flow v2 timeout) +_SESSION_TTL = 1200 + + +def _cleanup_expired_sessions() -> None: + """Remove expired provision sessions.""" + now = time.time() + expired = [k for k, v in _provision_sessions.items() if v["expires_at"] < now] + for k in expired: + del _provision_sessions[k] + + +def _validate_redirect_uri(redirect_uri: str) -> bool: + """Validate that redirect_uri is a reasonable URL (not javascript: etc).""" + try: + parsed = urlparse(redirect_uri) + return parsed.scheme in ("http", "https") and bool(parsed.netloc) + except Exception: + return False + + +async def _poll_and_store(provision_id: str) -> None: + """Background task: poll Login Flow v2 and store app password on completion.""" + session = _provision_sessions.get(provision_id) + if not session: + return + + settings = get_settings() + nextcloud_host = settings.nextcloud_host + if not nextcloud_host: + session["status"] = "expired" + return + + flow_client = LoginFlowV2Client( + nextcloud_host=nextcloud_host, + verify_ssl=get_nextcloud_ssl_verify(), + ) + + poll_endpoint = session["poll_endpoint"] + poll_token = session["poll_token"] + user_id = session.get("user_id") + + # Poll every 2 seconds for up to 20 minutes + max_attempts = 600 + for _ in range(max_attempts): + if provision_id not in _provision_sessions: + return # Session was cleaned up + + try: + result = await flow_client.poll(poll_endpoint, poll_token) + except Exception as e: + logger.warning( + f"Login Flow v2 poll error for provision {provision_id}: {e}" + ) + await asyncio.sleep(2) + continue + + if result.status == "completed": + # Store the app password + storage = await get_shared_storage() + effective_user_id = user_id or result.login_name or "unknown" + if not result.app_password: + session["status"] = "expired" + logger.error( + f"Login Flow v2 completed but no app_password (provision_id={provision_id})" + ) + return + await storage.store_app_password_with_scopes( + user_id=effective_user_id, + app_password=result.app_password, + scopes=None, # All scopes + username=result.login_name, + ) + session["status"] = "completed" + session["username"] = result.login_name + logger.info( + f"Login Flow v2 web provision completed for user {effective_user_id} " + f"(provision_id={provision_id})" + ) + return + + if result.status == "expired": + session["status"] = "expired" + logger.warning( + f"Login Flow v2 web provision expired (provision_id={provision_id})" + ) + return + + await asyncio.sleep(2) + + # Timed out + session["status"] = "expired" + logger.warning( + f"Login Flow v2 web provision timed out (provision_id={provision_id})" + ) + + +async def provision_page(request: Request) -> HTMLResponse: + """Render the Login Flow v2 provisioning page. + + GET /app/provision?redirect_uri=...&user_id=... + + Initiates Login Flow v2, starts background polling, and returns an HTML + page that opens Nextcloud's login URL in a popup and polls for completion. + """ + _cleanup_expired_sessions() + + redirect_uri = request.query_params.get("redirect_uri", "") + user_id = request.query_params.get("user_id", "") + + if not redirect_uri or not _validate_redirect_uri(redirect_uri): + return HTMLResponse( + content=_render_error("Missing or invalid redirect_uri parameter."), + status_code=400, + ) + + # Check if user already has an app password + if user_id: + storage = await get_shared_storage() + existing = await storage.get_app_password_with_scopes(user_id) + if existing: + logger.info(f"User {user_id} already has app password, skipping provision") + return HTMLResponse( + content=_render_redirect(redirect_uri), + status_code=200, + ) + + # Initiate Login Flow v2 + settings = get_settings() + nextcloud_host = settings.nextcloud_host + if not nextcloud_host: + return HTMLResponse( + content=_render_error("Nextcloud host not configured on server."), + status_code=500, + ) + + try: + flow_client = LoginFlowV2Client( + nextcloud_host=nextcloud_host, + verify_ssl=get_nextcloud_ssl_verify(), + ) + init_response = await flow_client.initiate( + user_agent="Astrolabe Background Sync" + ) + except Exception as e: + logger.error(f"Failed to initiate Login Flow v2 for web provision: {e}") + return HTMLResponse( + content=_render_error(f"Failed to start login flow: {e}"), + status_code=502, + ) + + # Create provision session + provision_id = secrets.token_urlsafe(32) + _provision_sessions[provision_id] = { + "status": "pending", + "login_url": init_response.login_url, + "poll_endpoint": init_response.poll_endpoint, + "poll_token": init_response.poll_token, + "redirect_uri": redirect_uri, + "user_id": user_id, + "created_at": time.time(), + "expires_at": time.time() + _SESSION_TTL, + } + + # Start background polling task + asyncio.create_task(_poll_and_store(provision_id)) + + logger.info( + f"Login Flow v2 web provision initiated (provision_id={provision_id}, " + f"user_id={user_id or 'unknown'})" + ) + + return HTMLResponse( + content=_render_provision_page( + provision_id=provision_id, + login_url=init_response.login_url, + redirect_uri=redirect_uri, + ) + ) + + +async def provision_status(request: Request) -> JSONResponse: + """Check provision session status. + + GET /app/provision/status?id=... + + Returns JSON: {"status": "pending"|"completed"|"expired", "username": "..."} + """ + provision_id = request.query_params.get("id", "") + + session = _provision_sessions.get(provision_id) + if not session: + return JSONResponse( + { + "status": "not_found", + "message": "Provision session not found or expired", + }, + status_code=404, + ) + + response: dict = {"status": session["status"]} + if session["status"] == "completed": + response["username"] = session.get("username") + # Clean up completed session after status is read + _provision_sessions.pop(provision_id, None) + + return JSONResponse(response) + + +# ── HTML rendering helpers ──────────────────────────────────────────────── + + +def _render_provision_page(provision_id: str, login_url: str, redirect_uri: str) -> str: + """Render the provisioning page HTML.""" + return f""" + + + + + Connecting to Nextcloud - Astrolabe + + + +
+

Connect to Nextcloud

+

Grant Astrolabe access to your Nextcloud account for background sync.

+ +
+ + Waiting for authorization... +
+ + + +
+ A popup window should open. Click "Grant access" in the + Nextcloud window to continue. +
+
+ + + +""" + + +def _render_error(message: str) -> str: + """Render an error page.""" + return f""" + + + + + Error - Astrolabe + + + +
+

Provisioning Error

+

{message}

+
+ +""" + + +def _render_redirect(redirect_uri: str) -> str: + """Render a page that immediately redirects (for already-provisioned users).""" + return f""" + + + + + Redirecting... + + +

Already connected. Redirecting...

+ + +""" From 1a51f5bbf5cbc555e22b9016052273eb8911c46a Mon Sep 17 00:00:00 2001 From: Chris Coutinho Date: Sun, 29 Mar 2026 22:17:54 +0200 Subject: [PATCH 02/10] refactor: use redirect-based Login Flow v2 provision instead of popup Replace the popup-based approach with a direct redirect to Nextcloud's login page. This is more compatible with Playwright E2E tests and simpler for users. The background polling task still runs server-side to store the app password when the user grants access. Co-Authored-By: Claude Opus 4.6 (1M context) --- nextcloud_mcp_server/auth/provision_routes.py | 233 ++---------------- 1 file changed, 25 insertions(+), 208 deletions(-) diff --git a/nextcloud_mcp_server/auth/provision_routes.py b/nextcloud_mcp_server/auth/provision_routes.py index 54c4b52e..1ea34eec 100644 --- a/nextcloud_mcp_server/auth/provision_routes.py +++ b/nextcloud_mcp_server/auth/provision_routes.py @@ -6,21 +6,22 @@ chain OAuth (bearer token) with Login Flow v2 (app password) in a single user interaction. Flow: -1. GET /app/provision?redirect_uri=... → Initiates LFv2, renders polling page -2. The page opens Nextcloud's login URL in a popup window -3. User clicks "Grant access" in the popup -4. Page polls GET /app/provision/status?id=... for completion -5. On success, redirects to redirect_uri +1. GET /app/provision?redirect_uri=... → Initiates LFv2, redirects to NC login +2. User clicks "Grant access" on Nextcloud's login page +3. MCP server background task polls and stores app password +4. GET /app/provision/status?id=... → Returns completion status (JSON) +5. User returns to Astrolabe settings (via redirect_uri or navigation) """ import asyncio import logging +import os import secrets import time from urllib.parse import urlparse from starlette.requests import Request -from starlette.responses import HTMLResponse, JSONResponse +from starlette.responses import HTMLResponse, JSONResponse, RedirectResponse from nextcloud_mcp_server.auth.login_flow import LoginFlowV2Client from nextcloud_mcp_server.auth.storage import get_shared_storage @@ -129,13 +130,15 @@ async def _poll_and_store(provision_id: str) -> None: ) -async def provision_page(request: Request) -> HTMLResponse: - """Render the Login Flow v2 provisioning page. +async def provision_page(request: Request) -> RedirectResponse | HTMLResponse: + """Initiate Login Flow v2 and redirect to Nextcloud's login page. GET /app/provision?redirect_uri=...&user_id=... - Initiates Login Flow v2, starts background polling, and returns an HTML - page that opens Nextcloud's login URL in a popup and polls for completion. + Initiates Login Flow v2, starts background polling, and redirects the + browser to Nextcloud's login/grant page. After the user grants access, + the background task stores the app password. The user then navigates + back to the redirect_uri (Astrolabe settings). """ _cleanup_expired_sessions() @@ -148,16 +151,13 @@ async def provision_page(request: Request) -> HTMLResponse: status_code=400, ) - # Check if user already has an app password + # Check if user already has an app password — skip straight to redirect if user_id: storage = await get_shared_storage() existing = await storage.get_app_password_with_scopes(user_id) if existing: logger.info(f"User {user_id} already has app password, skipping provision") - return HTMLResponse( - content=_render_redirect(redirect_uri), - status_code=200, - ) + return RedirectResponse(redirect_uri) # Initiate Login Flow v2 settings = get_settings() @@ -201,16 +201,18 @@ async def provision_page(request: Request) -> HTMLResponse: logger.info( f"Login Flow v2 web provision initiated (provision_id={provision_id}, " - f"user_id={user_id or 'unknown'})" + f"user_id={user_id or 'unknown'}), redirecting to NC login" ) - return HTMLResponse( - content=_render_provision_page( - provision_id=provision_id, - login_url=init_response.login_url, - redirect_uri=redirect_uri, - ) - ) + # Redirect to Nextcloud's Login Flow v2 login page. + # The login_url may use the internal Docker URL (http://app:80/...). + # Replace with the public Nextcloud URL for the browser. + login_url = init_response.login_url + public_issuer = os.getenv("NEXTCLOUD_PUBLIC_ISSUER_URL", "") + if public_issuer and nextcloud_host and nextcloud_host in login_url: + login_url = login_url.replace(nextcloud_host, public_issuer.rstrip("/")) + + return RedirectResponse(login_url) async def provision_status(request: Request) -> JSONResponse: @@ -244,175 +246,6 @@ async def provision_status(request: Request) -> JSONResponse: # ── HTML rendering helpers ──────────────────────────────────────────────── -def _render_provision_page(provision_id: str, login_url: str, redirect_uri: str) -> str: - """Render the provisioning page HTML.""" - return f""" - - - - - Connecting to Nextcloud - Astrolabe - - - -
-

Connect to Nextcloud

-

Grant Astrolabe access to your Nextcloud account for background sync.

- -
- - Waiting for authorization... -
- - - -
- A popup window should open. Click "Grant access" in the - Nextcloud window to continue. -
-
- - - -""" - - def _render_error(message: str) -> str: """Render an error page.""" return f""" @@ -448,19 +281,3 @@ def _render_error(message: str) -> str: """ - - -def _render_redirect(redirect_uri: str) -> str: - """Render a page that immediately redirects (for already-provisioned users).""" - return f""" - - - - - Redirecting... - - -

Already connected. Redirecting...

- - -""" From c2f23a566cb701be8766c3b18ced82dcfc6f3cb4 Mon Sep 17 00:00:00 2001 From: Chris Coutinho Date: Sun, 29 Mar 2026 23:11:42 +0200 Subject: [PATCH 03/10] fix: handle internal hostname without port in Login Flow v2 URL rewriting Nextcloud may omit default ports in the login_url (e.g. http://app instead of http://app:80). Extract just scheme+hostname from NEXTCLOUD_HOST for the URL replacement check. Co-Authored-By: Claude Opus 4.6 (1M context) --- nextcloud_mcp_server/auth/provision_routes.py | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/nextcloud_mcp_server/auth/provision_routes.py b/nextcloud_mcp_server/auth/provision_routes.py index 1ea34eec..16d9563e 100644 --- a/nextcloud_mcp_server/auth/provision_routes.py +++ b/nextcloud_mcp_server/auth/provision_routes.py @@ -205,12 +205,17 @@ async def provision_page(request: Request) -> RedirectResponse | HTMLResponse: ) # Redirect to Nextcloud's Login Flow v2 login page. - # The login_url may use the internal Docker URL (http://app:80/...). + # The login_url may use the internal Docker hostname (http://app/...). # Replace with the public Nextcloud URL for the browser. login_url = init_response.login_url public_issuer = os.getenv("NEXTCLOUD_PUBLIC_ISSUER_URL", "") - if public_issuer and nextcloud_host and nextcloud_host in login_url: - login_url = login_url.replace(nextcloud_host, public_issuer.rstrip("/")) + if public_issuer and nextcloud_host: + # Extract just scheme+host from NEXTCLOUD_HOST for matching + # (NC may omit default ports, e.g. http://app:80 → http://app) + parsed = urlparse(nextcloud_host) + internal_origin = f"{parsed.scheme}://{parsed.hostname}" + if internal_origin in login_url: + login_url = login_url.replace(internal_origin, public_issuer.rstrip("/")) return RedirectResponse(login_url) From eefede8c47bbaee15bbe0551acc84a98f2e0e00d Mon Sep 17 00:00:00 2001 From: Chris Coutinho Date: Sun, 29 Mar 2026 23:21:41 +0200 Subject: [PATCH 04/10] fix: rewrite Login Flow v2 poll endpoint URL to use configured host Nextcloud returns poll/login URLs using its internal hostname (e.g. http://localhost/login/v2/poll) which is unreachable from the MCP server container in Docker networks. Rewrite the poll endpoint's origin to use the configured NEXTCLOUD_HOST so server-side polling works correctly. Co-Authored-By: Claude Opus 4.6 (1M context) --- nextcloud_mcp_server/auth/login_flow.py | 29 ++++++++++++++++++++++++- 1 file changed, 28 insertions(+), 1 deletion(-) diff --git a/nextcloud_mcp_server/auth/login_flow.py b/nextcloud_mcp_server/auth/login_flow.py index d13e8d5d..a6120611 100644 --- a/nextcloud_mcp_server/auth/login_flow.py +++ b/nextcloud_mcp_server/auth/login_flow.py @@ -10,6 +10,7 @@ The flow has two steps: import logging import ssl +from urllib.parse import urlparse, urlunparse from pydantic import BaseModel, Field @@ -91,9 +92,16 @@ class LoginFlowV2Client: poll_data = data.get("poll", {}) try: + raw_poll_endpoint = poll_data["endpoint"] + # Nextcloud returns URLs using its internal hostname (e.g. + # http://localhost/login/v2/poll) which may be unreachable from + # this process. Rewrite the poll endpoint to use nextcloud_host + # so server-side polling works across Docker networks. + poll_endpoint = self._rewrite_to_nextcloud_host(raw_poll_endpoint) + result = LoginFlowInitResponse( login_url=data["login"], - poll_endpoint=poll_data["endpoint"], + poll_endpoint=poll_endpoint, poll_token=poll_data["token"], ) except KeyError as e: @@ -104,6 +112,25 @@ class LoginFlowV2Client: logger.info(f"Login Flow v2 initiated: login_url={result.login_url[:60]}...") return result + def _rewrite_to_nextcloud_host(self, url: str) -> str: + """Rewrite a URL's origin to use self.nextcloud_host. + + Nextcloud may return URLs with its internal hostname (e.g. + http://localhost) which differs from the configured NEXTCLOUD_HOST + (e.g. http://app:80). This replaces the scheme+host+port while + preserving the path and query. + """ + parsed_url = urlparse(url) + parsed_host = urlparse(self.nextcloud_host) + rewritten = parsed_url._replace( + scheme=parsed_host.scheme, + netloc=parsed_host.netloc, + ) + result = urlunparse(rewritten) + if result != url: + logger.debug(f"Rewrote Login Flow v2 URL: {url} → {result}") + return result + async def poll(self, poll_endpoint: str, poll_token: str) -> LoginFlowPollResult: """Poll for Login Flow v2 completion by sending an HTTP POST to the Nextcloud instance. From 474cfe5e9869be682e94d857c5ee01c7f0ff4ac9 Mon Sep 17 00:00:00 2001 From: Chris Coutinho Date: Mon, 30 Mar 2026 00:48:02 +0200 Subject: [PATCH 05/10] fix: discover Login Flow v2 users in OAuth mode user manager When enable_login_flow is true, also check the app_passwords table for provisioned users. Previously, OAuth mode only queried the refresh_tokens table, missing users who were provisioned via Login Flow v2 (which stores app passwords, not refresh tokens). Co-Authored-By: Claude Opus 4.6 (1M context) --- Dockerfile | 1 + nextcloud_mcp_server/vector/oauth_sync.py | 8 ++++++++ 2 files changed, 9 insertions(+) diff --git a/Dockerfile b/Dockerfile index dacb75d6..7d47a2ba 100644 --- a/Dockerfile +++ b/Dockerfile @@ -6,6 +6,7 @@ COPY --from=ghcr.io/astral-sh/uv:0.10.12@sha256:72ab0aeb448090480ccabb99fb5f52b0 # 1. git (required for caldav dependency from git) # 2. sqlite for development with token db RUN apt update && apt install --no-install-recommends --no-install-suggests -y \ + curl \ git \ tesseract-ocr \ sqlite3 && apt clean diff --git a/nextcloud_mcp_server/vector/oauth_sync.py b/nextcloud_mcp_server/vector/oauth_sync.py index c4c56f7a..5f6a5b9e 100644 --- a/nextcloud_mcp_server/vector/oauth_sync.py +++ b/nextcloud_mcp_server/vector/oauth_sync.py @@ -496,6 +496,14 @@ async def user_manager_task( else: # OAuth mode: query refresh_tokens table provisioned_users = set(await refresh_token_storage.get_all_user_ids()) + # Login Flow mode: also check app_passwords table + # (users provisioned via Login Flow v2 have app passwords, + # not refresh tokens) + if settings.enable_login_flow: + app_pw_users = set( + await refresh_token_storage.get_all_app_password_user_ids() + ) + provisioned_users |= app_pw_users active_users = set(user_states.keys()) # Start scanners for new users From c21776948d469f36f29367f7058cdfa4d09a853e Mon Sep 17 00:00:00 2001 From: Chris Coutinho Date: Mon, 30 Mar 2026 00:55:31 +0200 Subject: [PATCH 06/10] fix: use app password auth for background sync in Login Flow mode Login Flow v2 is a deployment-wide mode where all users authenticate with app passwords (not OAuth refresh tokens). Set use_basic_auth=True when enable_login_flow is true so the background sync user manager queries the app_passwords table and scanners use app password authentication for Nextcloud API calls. Co-Authored-By: Claude Opus 4.6 (1M context) --- nextcloud_mcp_server/app.py | 6 +++--- nextcloud_mcp_server/vector/oauth_sync.py | 10 +--------- 2 files changed, 4 insertions(+), 12 deletions(-) diff --git a/nextcloud_mcp_server/app.py b/nextcloud_mcp_server/app.py index 551b4bcc..55028f23 100644 --- a/nextcloud_mcp_server/app.py +++ b/nextcloud_mcp_server/app.py @@ -1807,9 +1807,9 @@ def get_app(transport: str = "streamable-http", enabled_apps: list[str] | None = break # Determine authentication mode for background sync - # Multi-user BasicAuth: use app passwords via Astrolabe (NOT OAuth) - # OAuth mode: use OAuth refresh tokens (NOT app passwords) - use_basic_auth = not oauth_enabled + # Login Flow v2 and multi-user BasicAuth: use app passwords + # OAuth mode (without Login Flow): use OAuth refresh tokens + use_basic_auth = not oauth_enabled or settings.enable_login_flow # Start background tasks using anyio TaskGroup async with anyio.create_task_group() as tg: diff --git a/nextcloud_mcp_server/vector/oauth_sync.py b/nextcloud_mcp_server/vector/oauth_sync.py index 5f6a5b9e..615977e1 100644 --- a/nextcloud_mcp_server/vector/oauth_sync.py +++ b/nextcloud_mcp_server/vector/oauth_sync.py @@ -489,21 +489,13 @@ async def user_manager_task( try: # Get current provisioned users based on mode if use_basic_auth: - # BasicAuth mode: query app_passwords table + # BasicAuth / Login Flow v2 mode: query app_passwords table provisioned_users = set( await refresh_token_storage.get_all_app_password_user_ids() ) else: # OAuth mode: query refresh_tokens table provisioned_users = set(await refresh_token_storage.get_all_user_ids()) - # Login Flow mode: also check app_passwords table - # (users provisioned via Login Flow v2 have app passwords, - # not refresh tokens) - if settings.enable_login_flow: - app_pw_users = set( - await refresh_token_storage.get_all_app_password_user_ids() - ) - provisioned_users |= app_pw_users active_users = set(user_states.keys()) # Start scanners for new users From 777a09c806d187397cd7285b4236c5b9879b1b10 Mon Sep 17 00:00:00 2001 From: Chris Coutinho Date: Mon, 30 Mar 2026 08:51:41 +0200 Subject: [PATCH 07/10] =?UTF-8?q?fix:=20address=20PR=20review=20=E2=80=94?= =?UTF-8?q?=20XSS=20escape,=20asyncio=E2=86=92anyio,=20URL=20rewrite=20ded?= =?UTF-8?q?up?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Escape HTML in _render_error to prevent XSS from exception messages - Replace asyncio.create_task/sleep with anyio task group and sleep, tying poll task lifetime to the app lifespan for proper cleanup - Extract rewrite_url_origin() utility to fix duplicated URL rewriting logic and replace urlparse._replace with stable urlunparse API - Add warning log for insecure HTTP redirect URIs - Add unit tests for validation, XSS escaping, route handlers, and URL rewriting (16 new tests in test_provision_routes.py) Co-Authored-By: Claude Opus 4.6 (1M context) --- nextcloud_mcp_server/app.py | 30 ++-- nextcloud_mcp_server/auth/login_flow.py | 28 ++- nextcloud_mcp_server/auth/provision_routes.py | 35 ++-- tests/unit/test_login_flow.py | 34 ++++ tests/unit/test_provision_routes.py | 170 ++++++++++++++++++ 5 files changed, 264 insertions(+), 33 deletions(-) create mode 100644 tests/unit/test_provision_routes.py diff --git a/nextcloud_mcp_server/app.py b/nextcloud_mcp_server/app.py index 55028f23..f755e137 100644 --- a/nextcloud_mcp_server/app.py +++ b/nextcloud_mcp_server/app.py @@ -1412,22 +1412,26 @@ def get_app(transport: str = "streamable-http", enabled_apps: list[str] | None = await anyio.sleep(3600) # Every hour @asynccontextmanager - async def _maybe_login_flow_cleanup(): - """Start Login Flow cleanup task if enabled.""" - if settings.enable_login_flow: - async with anyio.create_task_group() as tg: + async def _maybe_login_flow_cleanup(app: Starlette): + """Start Login Flow cleanup task and provision poll task group.""" + async with anyio.create_task_group() as tg: + if settings.enable_login_flow: tg.start_soon(_login_flow_cleanup_loop) - yield - tg.cancel_scope.cancel() - else: + # Share task group with provision routes for background polling + for route in app.routes: + if isinstance(route, Mount) and route.path == "/app": + browser_app = cast(Starlette, route.app) + browser_app.state.poll_task_group = tg + break yield + tg.cancel_scope.cancel() @asynccontextmanager - async def _mcp_session_with_login_flow(): + async def _mcp_session_with_login_flow(app: Starlette): """Start MCP session manager with optional Login Flow cleanup.""" async with AsyncExitStack() as stack: await stack.enter_async_context(mcp.session_manager.run()) - await stack.enter_async_context(_maybe_login_flow_cleanup()) + await stack.enter_async_context(_maybe_login_flow_cleanup(app)) yield @asynccontextmanager @@ -1663,7 +1667,7 @@ def get_app(transport: str = "streamable-http", enabled_apps: list[str] | None = ) # Run MCP session manager and yield - async with _mcp_session_with_login_flow(): + async with _mcp_session_with_login_flow(app): try: yield finally: @@ -1845,7 +1849,7 @@ def get_app(transport: str = "streamable-http", enabled_apps: list[str] | None = ) # Run MCP session manager and yield - async with _mcp_session_with_login_flow(): + async with _mcp_session_with_login_flow(app): try: yield finally: @@ -1864,7 +1868,7 @@ def get_app(transport: str = "streamable-http", enabled_apps: list[str] | None = "To enable, set NEXTCLOUD_OIDC_CLIENT_ID and NEXTCLOUD_OIDC_CLIENT_SECRET." ) # Just run MCP session manager without vector sync - async with _mcp_session_with_login_flow(): + async with _mcp_session_with_login_flow(app): yield else: @@ -1884,7 +1888,7 @@ def get_app(transport: str = "streamable-http", enabled_apps: list[str] | None = logger.warning( "Vector sync enabled but TOKEN_ENCRYPTION_KEY not set" ) - async with _mcp_session_with_login_flow(): + async with _mcp_session_with_login_flow(app): yield # Health check endpoints for Kubernetes probes diff --git a/nextcloud_mcp_server/auth/login_flow.py b/nextcloud_mcp_server/auth/login_flow.py index a6120611..0bfdd10a 100644 --- a/nextcloud_mcp_server/auth/login_flow.py +++ b/nextcloud_mcp_server/auth/login_flow.py @@ -19,6 +19,26 @@ from nextcloud_mcp_server.http import nextcloud_httpx_client logger = logging.getLogger(__name__) +def rewrite_url_origin(url: str, target_host: str) -> str: + """Rewrite a URL's scheme+host+port to match target_host. + + Preserves the path, params, query, and fragment from the original URL. + Useful for rewriting internal Docker hostnames to public-facing URLs. + """ + parsed_url = urlparse(url) + parsed_host = urlparse(target_host) + return urlunparse( + ( + parsed_host.scheme, + parsed_host.netloc, + parsed_url.path, + parsed_url.params, + parsed_url.query, + parsed_url.fragment, + ) + ) + + class LoginFlowInitResponse(BaseModel): """Response from initiating Login Flow v2.""" @@ -120,13 +140,7 @@ class LoginFlowV2Client: (e.g. http://app:80). This replaces the scheme+host+port while preserving the path and query. """ - parsed_url = urlparse(url) - parsed_host = urlparse(self.nextcloud_host) - rewritten = parsed_url._replace( - scheme=parsed_host.scheme, - netloc=parsed_host.netloc, - ) - result = urlunparse(rewritten) + result = rewrite_url_origin(url, self.nextcloud_host) if result != url: logger.debug(f"Rewrote Login Flow v2 URL: {url} → {result}") return result diff --git a/nextcloud_mcp_server/auth/provision_routes.py b/nextcloud_mcp_server/auth/provision_routes.py index 16d9563e..179b6901 100644 --- a/nextcloud_mcp_server/auth/provision_routes.py +++ b/nextcloud_mcp_server/auth/provision_routes.py @@ -13,17 +13,18 @@ Flow: 5. User returns to Astrolabe settings (via redirect_uri or navigation) """ -import asyncio +import html import logging import os import secrets import time from urllib.parse import urlparse +import anyio from starlette.requests import Request from starlette.responses import HTMLResponse, JSONResponse, RedirectResponse -from nextcloud_mcp_server.auth.login_flow import LoginFlowV2Client +from nextcloud_mcp_server.auth.login_flow import LoginFlowV2Client, rewrite_url_origin from nextcloud_mcp_server.auth.storage import get_shared_storage from nextcloud_mcp_server.config import get_nextcloud_ssl_verify, get_settings @@ -87,7 +88,7 @@ async def _poll_and_store(provision_id: str) -> None: logger.warning( f"Login Flow v2 poll error for provision {provision_id}: {e}" ) - await asyncio.sleep(2) + await anyio.sleep(2) continue if result.status == "completed": @@ -121,7 +122,7 @@ async def _poll_and_store(provision_id: str) -> None: ) return - await asyncio.sleep(2) + await anyio.sleep(2) # Timed out session["status"] = "expired" @@ -151,6 +152,9 @@ async def provision_page(request: Request) -> RedirectResponse | HTMLResponse: status_code=400, ) + if urlparse(redirect_uri).scheme == "http": + logger.warning(f"Provision redirect_uri uses insecure HTTP: {redirect_uri}") + # Check if user already has an app password — skip straight to redirect if user_id: storage = await get_shared_storage() @@ -196,8 +200,18 @@ async def provision_page(request: Request) -> RedirectResponse | HTMLResponse: "expires_at": time.time() + _SESSION_TTL, } - # Start background polling task - asyncio.create_task(_poll_and_store(provision_id)) + # Start background polling task (uses task group from app lifespan) + poll_tg = getattr(request.app.state, "poll_task_group", None) + if poll_tg is None: + logger.error("No poll task group available; cannot start background polling") + _provision_sessions.pop(provision_id, None) + return HTMLResponse( + content=_render_error( + "Server configuration error: background polling unavailable." + ), + status_code=500, + ) + poll_tg.start_soon(_poll_and_store, provision_id) logger.info( f"Login Flow v2 web provision initiated (provision_id={provision_id}, " @@ -210,12 +224,7 @@ async def provision_page(request: Request) -> RedirectResponse | HTMLResponse: login_url = init_response.login_url public_issuer = os.getenv("NEXTCLOUD_PUBLIC_ISSUER_URL", "") if public_issuer and nextcloud_host: - # Extract just scheme+host from NEXTCLOUD_HOST for matching - # (NC may omit default ports, e.g. http://app:80 → http://app) - parsed = urlparse(nextcloud_host) - internal_origin = f"{parsed.scheme}://{parsed.hostname}" - if internal_origin in login_url: - login_url = login_url.replace(internal_origin, public_issuer.rstrip("/")) + login_url = rewrite_url_origin(login_url, public_issuer.rstrip("/")) return RedirectResponse(login_url) @@ -282,7 +291,7 @@ def _render_error(message: str) -> str:

Provisioning Error

-

{message}

+

{html.escape(message)}

""" diff --git a/tests/unit/test_login_flow.py b/tests/unit/test_login_flow.py index 6c7c51a0..a5dfc9bc 100644 --- a/tests/unit/test_login_flow.py +++ b/tests/unit/test_login_flow.py @@ -14,6 +14,7 @@ from nextcloud_mcp_server.auth.login_flow import ( LoginFlowInitResponse, LoginFlowPollResult, LoginFlowV2Client, + rewrite_url_origin, ) pytestmark = pytest.mark.unit @@ -208,3 +209,36 @@ async def test_login_flow_poll_result_model(): assert pending.status == "pending" assert pending.server is None assert pending.app_password is None + + +# ── rewrite_url_origin tests ───────────────────────────────────────────── + + +async def test_rewrite_url_origin_basic(): + """Test basic origin rewriting.""" + result = rewrite_url_origin( + "http://localhost/login/v2/poll", "https://cloud.example.com" + ) + assert result == "https://cloud.example.com/login/v2/poll" + + +async def test_rewrite_url_origin_preserves_port(): + """Test that port in target_host is preserved.""" + result = rewrite_url_origin("http://localhost/path", "http://app:8080") + assert result == "http://app:8080/path" + + +async def test_rewrite_url_origin_preserves_query(): + """Test that query string and fragment are preserved.""" + result = rewrite_url_origin( + "http://internal/path?token=abc&foo=bar#section", + "https://public.example.com", + ) + assert result == "https://public.example.com/path?token=abc&foo=bar#section" + + +async def test_rewrite_url_origin_noop_when_same(): + """Test that rewriting to the same origin is a no-op.""" + url = "https://cloud.example.com/login/v2/poll" + result = rewrite_url_origin(url, "https://cloud.example.com") + assert result == url diff --git a/tests/unit/test_provision_routes.py b/tests/unit/test_provision_routes.py new file mode 100644 index 00000000..5cd67b16 --- /dev/null +++ b/tests/unit/test_provision_routes.py @@ -0,0 +1,170 @@ +"""Unit tests for web-based Login Flow v2 provisioning routes. + +Tests validation, HTML escaping, URL rewriting, and route handlers. +""" + +import time +from unittest.mock import AsyncMock, MagicMock, patch + +import pytest + +from nextcloud_mcp_server.auth.provision_routes import ( + _provision_sessions, + _render_error, + _validate_redirect_uri, + provision_page, + provision_status, +) + +pytestmark = pytest.mark.unit + + +# ── _validate_redirect_uri tests ───────────────────────────────────────── + + +async def test_validate_redirect_uri_accepts_https(): + """Valid HTTPS URL is accepted.""" + assert _validate_redirect_uri("https://app.example.com/callback") is True + + +async def test_validate_redirect_uri_accepts_http_localhost(): + """Valid HTTP localhost URL is accepted.""" + assert _validate_redirect_uri("http://localhost:3000/callback") is True + + +async def test_validate_redirect_uri_rejects_javascript(): + """javascript: URIs are rejected.""" + assert _validate_redirect_uri("javascript:alert(1)") is False + + +async def test_validate_redirect_uri_rejects_relative_url(): + """Relative URLs are rejected (no scheme/netloc).""" + assert _validate_redirect_uri("/relative/path") is False + + +async def test_validate_redirect_uri_rejects_bare_hostname(): + """Bare hostnames without scheme are rejected.""" + assert _validate_redirect_uri("example.com") is False + + +async def test_validate_redirect_uri_rejects_data_uri(): + """data: URIs are rejected.""" + assert _validate_redirect_uri("data:text/html,

hi

") is False + + +async def test_validate_redirect_uri_rejects_empty(): + """Empty string is rejected.""" + assert _validate_redirect_uri("") is False + + +# ── _render_error tests ────────────────────────────────────────────────── + + +async def test_render_error_escapes_html(): + """XSS regression: HTML in error messages must be escaped.""" + html_output = _render_error("") + assert "