diff --git a/nextcloud_mcp_server/api/webhooks.py b/nextcloud_mcp_server/api/webhooks.py index 34d68ad9..3f9a3a28 100644 --- a/nextcloud_mcp_server/api/webhooks.py +++ b/nextcloud_mcp_server/api/webhooks.py @@ -18,6 +18,7 @@ from nextcloud_mcp_server.api.management import ( extract_bearer_token, validate_token_and_get_user, ) +from nextcloud_mcp_server.auth.webhook_routes import webhook_auth_pair from nextcloud_mcp_server.client.webhooks import WebhooksClient from ..http import nextcloud_httpx_client @@ -36,7 +37,7 @@ async def get_installed_apps(request: Request) -> JSONResponse: # Validate OAuth token and extract user user_id, validated = await validate_token_and_get_user(request) except Exception as e: - logger.warning(f"Unauthorized access to /api/v1/apps: {e}") + logger.warning("Unauthorized access to /api/v1/apps: %s", e) return JSONResponse( { "error": "Unauthorized", @@ -85,7 +86,7 @@ async def get_installed_apps(request: Request) -> JSONResponse: return JSONResponse({"apps": apps}) except Exception as e: - logger.error(f"Error getting installed apps for user {user_id}: {e}") + logger.error("Error getting installed apps for user %s: %s", user_id, e) return JSONResponse( { "error": "Internal error", @@ -106,7 +107,7 @@ async def list_webhooks(request: Request) -> JSONResponse: # Validate OAuth token and extract user user_id, validated = await validate_token_and_get_user(request) except Exception as e: - logger.warning(f"Unauthorized access to /api/v1/webhooks: {e}") + logger.warning("Unauthorized access to /api/v1/webhooks: %s", e) return JSONResponse( { "error": "Unauthorized", @@ -141,7 +142,7 @@ async def list_webhooks(request: Request) -> JSONResponse: return JSONResponse({"webhooks": webhooks}) except Exception as e: - logger.error(f"Error listing webhooks for user {user_id}: {e}") + logger.error("Error listing webhooks for user %s: %s", user_id, e) return JSONResponse( { "error": "Internal error", @@ -169,7 +170,7 @@ async def create_webhook(request: Request) -> JSONResponse: # Validate OAuth token and extract user user_id, validated = await validate_token_and_get_user(request) except Exception as e: - logger.warning(f"Unauthorized access to /api/v1/webhooks: {e}") + logger.warning("Unauthorized access to /api/v1/webhooks: %s", e) return JSONResponse( { "error": "Unauthorized", @@ -213,16 +214,22 @@ async def create_webhook(request: Request) -> JSONResponse: headers={"Authorization": f"Bearer {token}"}, timeout=30.0, ) as client: - # Use WebhooksClient to create webhook + # Use WebhooksClient to create webhook. Inject auth headers when + # WEBHOOK_SECRET is configured so deliveries are authenticated. webhooks_client = WebhooksClient(client, user_id) + auth_method, auth_data = webhook_auth_pair() webhook_data = await webhooks_client.create_webhook( - event=event, uri=uri, event_filter=event_filter + event=event, + uri=uri, + event_filter=event_filter, + auth_method=auth_method, + auth_data=auth_data, ) return JSONResponse({"webhook": webhook_data}) except Exception as e: - logger.error(f"Error creating webhook for user {user_id}: {e}") + logger.error("Error creating webhook for user %s: %s", user_id, e) return JSONResponse( { "error": "Internal error", @@ -243,7 +250,7 @@ async def delete_webhook(request: Request) -> JSONResponse: # Validate OAuth token and extract user user_id, validated = await validate_token_and_get_user(request) except Exception as e: - logger.warning(f"Unauthorized access to /api/v1/webhooks: {e}") + logger.warning("Unauthorized access to /api/v1/webhooks: %s", e) return JSONResponse( { "error": "Unauthorized", @@ -294,7 +301,7 @@ async def delete_webhook(request: Request) -> JSONResponse: return JSONResponse({"success": True, "message": "Webhook deleted"}) except Exception as e: - logger.error(f"Error deleting webhook for user {user_id}: {e}") + logger.error("Error deleting webhook for user %s: %s", user_id, e) return JSONResponse( { "error": "Internal error", diff --git a/nextcloud_mcp_server/app.py b/nextcloud_mcp_server/app.py index e0b1c87a..f5e72660 100644 --- a/nextcloud_mcp_server/app.py +++ b/nextcloud_mcp_server/app.py @@ -130,6 +130,7 @@ from nextcloud_mcp_server.vector.oauth_sync import ( user_manager_task, ) from nextcloud_mcp_server.vector.qdrant_client import get_qdrant_client +from nextcloud_mcp_server.vector.webhook_receiver import handle_nextcloud_webhook logger = logging.getLogger(__name__) HTTPXClientInstrumentor().instrument() @@ -1950,30 +1951,6 @@ def get_app(transport: str = "streamable-http", enabled_apps: list[str] | None = status_code=status_code, ) - async def handle_nextcloud_webhook(request): - """Test webhook endpoint to capture and log Nextcloud webhook payloads. - - This is a temporary endpoint for testing webhook schemas and payloads. - It logs the full payload and returns 200 OK immediately. - """ - - try: - payload = await request.json() - logger.info("=" * 80) - logger.info("๐Ÿ”” Webhook received from Nextcloud:") - logger.info(json.dumps(payload, indent=2, sort_keys=True)) - logger.info("=" * 80) - - return JSONResponse( - {"status": "received", "timestamp": payload.get("time")}, - status_code=200, - ) - except Exception as e: - logger.error(f"โŒ Failed to parse webhook payload: {e}") - return JSONResponse( - {"error": "invalid_payload", "message": str(e)}, status_code=400 - ) - # Add Protected Resource Metadata (PRM) endpoint for OAuth mode routes = [] @@ -1982,11 +1959,13 @@ def get_app(transport: str = "streamable-http", enabled_apps: list[str] | None = routes.append(Route("/health/ready", health_ready, methods=["GET"])) logger.info("Health check endpoints enabled: /health/live, /health/ready") - # Add test webhook endpoint (for development/testing) + # Add Nextcloud webhook receiver (queues DocumentTasks for vector sync). + # Implementation lives in vector/webhook_receiver.py; the handler reads + # the send-stream from request.app.state.document_send_stream. routes.append( Route("/webhooks/nextcloud", handle_nextcloud_webhook, methods=["POST"]) ) - logger.info("Test webhook endpoint enabled: /webhooks/nextcloud") + logger.info("Webhook endpoint enabled: /webhooks/nextcloud") # Add management API endpoints for Nextcloud PHP app # Tier 1: Public endpoints (no auth required) diff --git a/nextcloud_mcp_server/auth/webhook_routes.py b/nextcloud_mcp_server/auth/webhook_routes.py index e7ee3c66..6e04031a 100644 --- a/nextcloud_mcp_server/auth/webhook_routes.py +++ b/nextcloud_mcp_server/auth/webhook_routes.py @@ -4,6 +4,7 @@ Provides browser-based endpoints for admin users to manage webhook configuration using preset templates. Only accessible to Nextcloud administrators. """ +import html import logging import os @@ -14,8 +15,10 @@ from starlette.responses import HTMLResponse from nextcloud_mcp_server.auth.permissions import is_nextcloud_admin from nextcloud_mcp_server.client.webhooks import WebhooksClient +from nextcloud_mcp_server.config import get_settings from nextcloud_mcp_server.server.webhook_presets import ( WEBHOOK_PRESETS, + WebhookPreset, filter_presets_by_installed_apps, get_preset, ) @@ -70,50 +73,106 @@ async def _get_installed_apps(http_client: httpx.AsyncClient) -> list[str]: app_keys = set(capabilities.keys()) - core_keys return sorted(app_keys) except Exception as e: - logger.warning(f"Failed to get installed apps from capabilities: {e}") + logger.warning("Failed to get installed apps from capabilities: %s", e) return [] def _get_webhook_uri() -> str: """Get the webhook endpoint URI for this MCP server. - This function determines the correct webhook URL based on the environment: - 1. Uses WEBHOOK_INTERNAL_URL if explicitly set (highest priority) - 2. Detects Docker environment and uses internal service name - 3. Falls back to NEXTCLOUD_MCP_SERVER_URL + Priority (highest first): + 1. ``WEBHOOK_INTERNAL_URL`` โ€” explicit override, e.g. for split + internal/external URLs (read via dynaconf, so env vars and + settings.toml both work). + 2. ``NEXTCLOUD_MCP_SERVER_URL`` โ€” the configured public URL set on + cloud deployments (ECS, k8s); the URL NC must POST to. + 3. ``/.dockerenv`` (or podman / ``DOCKER_CONTAINER=true``) โ†’ internal + docker-compose service name. Only relevant when no public URL is + configured โ€” i.e. local dev where MCP and NC share a Docker + network. + 4. ``http://localhost:8000`` โ€” last-resort fallback. - In Docker environments, Nextcloud needs to reach the MCP service using - the internal Docker network hostname (e.g., http://mcp:8000), not localhost. - - Returns: - Full webhook endpoint URL accessible from Nextcloud + Note: ECS Fargate containers also expose ``/.dockerenv``. Without this + priority order, cloud deployments would silently register an internal + docker-compose hostname (e.g. ``http://mcp:8000``) that NC cannot + resolve, dropping every webhook delivery. """ - # Explicit override (highest priority) - webhook_url = os.getenv("WEBHOOK_INTERNAL_URL") - if webhook_url: - return f"{webhook_url}/webhooks/nextcloud" + settings = get_settings() + if settings.webhook_internal_url: + return f"{settings.webhook_internal_url}/webhooks/nextcloud" - # Detect Docker environment - # Check for common Docker indicators + if settings.nextcloud_mcp_server_url: + return f"{settings.nextcloud_mcp_server_url}/webhooks/nextcloud" + + # Docker-environment markers stay on os.getenv: they're container-runtime + # signals (filesystem markers, optional service-name override) rather + # than user-facing config that would belong in settings.toml. is_docker = ( - os.path.exists("/.dockerenv") # Docker container marker file - or os.path.exists("/run/.containerenv") # Podman marker - or os.getenv("DOCKER_CONTAINER") == "true" # Explicit flag + os.path.exists("/.dockerenv") + or os.path.exists("/run/.containerenv") + or os.getenv("DOCKER_CONTAINER") == "true" ) - if is_docker: - # In Docker, use internal service name from NEXTCLOUD_MCP_SERVICE_NAME - # or default to 'mcp' (docker-compose service name) service_name = os.getenv("NEXTCLOUD_MCP_SERVICE_NAME", "mcp") port = os.getenv("NEXTCLOUD_MCP_PORT", "8000") logger.debug( - f"Docker environment detected, using internal URL: http://{service_name}:{port}" + "Docker environment detected, using internal URL: http://%s:%s", + service_name, + port, ) return f"http://{service_name}:{port}/webhooks/nextcloud" - # Fallback to configured server URL (for non-Docker deployments) - server_url = os.getenv("NEXTCLOUD_MCP_SERVER_URL", "http://localhost:8000") - return f"{server_url}/webhooks/nextcloud" + return "http://localhost:8000/webhooks/nextcloud" + + +def webhook_auth_pair() -> tuple[str, dict[str, str] | None]: + """Resolve ``(auth_method, auth_data)`` for new webhook registrations. + + When ``WEBHOOK_SECRET`` is set, returns + ``("header", {"Authorization": f"Bearer {secret}"})`` so NC stores the + credential encrypted at-rest and forwards it on every delivery. When + unset, returns ``("none", None)`` โ€” backward-compatible with deployments + that haven't rolled out webhook auth yet. + + Shared by both registration call sites: the ``/app/webhooks`` preset + flow and the Astrolabe-facing ``/api/v1/webhooks`` endpoint. + """ + secret = get_settings().webhook_secret + if not secret: + return ("none", None) + return ("header", {"Authorization": f"Bearer {secret}"}) + + +async def _register_preset_webhooks( + webhooks_client: WebhooksClient, + preset: WebhookPreset, + webhook_uri: str, +) -> list[int]: + """Register every event in a preset against a single MCP webhook URI. + + Threads the resolved ``(auth_method, auth_data)`` from + :func:`webhook_auth_pair` onto each registration call so deliveries + carry the configured ``Authorization`` header (when ``WEBHOOK_SECRET`` + is set) or fall through to ``authMethod="none"`` (backward-compatible + when it's not). + + Extracted from :func:`enable_webhook_preset` so the auth-threading + behaviour is testable without standing up a Starlette app. + """ + auth_method, auth_data = webhook_auth_pair() + registered_ids: list[int] = [] + for event_config in preset["events"]: + webhook_data = await webhooks_client.create_webhook( + event=event_config["event"], + uri=webhook_uri, + event_filter=event_config["filter"] if event_config["filter"] else None, + auth_method=auth_method, + auth_data=auth_data, + ) + webhook_id = webhook_data["id"] + registered_ids.append(webhook_id) + logger.info("Registered webhook %s for %s", webhook_id, event_config["event"]) + return registered_ids async def _get_authenticated_client(request: Request) -> httpx.AsyncClient: @@ -228,7 +287,7 @@ async def _get_enabled_presets( return enabled_presets except Exception as e: - logger.error(f"Failed to list webhooks: {e}") + logger.error("Failed to list webhooks: %s", e) return {} @@ -273,7 +332,7 @@ async def webhook_management_pane(request: Request) -> HTMLResponse: # Get installed apps to filter presets installed_apps = await _get_installed_apps(http_client) - logger.debug(f"Installed apps: {installed_apps}") + logger.debug("Installed apps: %s", installed_apps) # Get currently enabled presets (from database or API) enabled_presets = await _get_enabled_presets(webhooks_client, storage) @@ -335,7 +394,7 @@ async def webhook_management_pane(request: Request) -> HTMLResponse:

About Webhooks

Webhooks enable real-time synchronization by notifying this server when content changes in Nextcloud.

-

Endpoint: {webhook_uri}

+

Endpoint: {html.escape(webhook_uri)}

Available Presets

@@ -348,12 +407,12 @@ async def webhook_management_pane(request: Request) -> HTMLResponse: return HTMLResponse(content=html_content) except Exception as e: - logger.error(f"Error loading webhook management pane: {e}", exc_info=True) + logger.error("Error loading webhook management pane: %s", e, exc_info=True) return HTMLResponse( content=f"""

Error Loading Webhooks

-

{str(e)}

+

{html.escape(str(e))}

""", status_code=500, @@ -389,24 +448,16 @@ async def enable_webhook_preset(request: Request) -> HTMLResponse: preset = get_preset(preset_id) if not preset: return HTMLResponse( - content=f'
Unknown preset: {preset_id}
', + content=f'
Unknown preset: {html.escape(preset_id)}
', status_code=404, ) # Register webhooks webhooks_client = WebhooksClient(http_client, username) webhook_uri = _get_webhook_uri() - registered_ids = [] - - for event_config in preset["events"]: - webhook_data = await webhooks_client.create_webhook( - event=event_config["event"], - uri=webhook_uri, - event_filter=event_config["filter"] if event_config["filter"] else None, - ) - webhook_id = webhook_data["id"] - registered_ids.append(webhook_id) - logger.info(f"Registered webhook {webhook_id} for {event_config['event']}") + registered_ids = await _register_preset_webhooks( + webhooks_client, preset, webhook_uri + ) # Persist webhook IDs to database storage = _get_storage(request) @@ -414,7 +465,9 @@ async def enable_webhook_preset(request: Request) -> HTMLResponse: for webhook_id in registered_ids: await storage.store_webhook(webhook_id, preset_id) logger.info( - f"Persisted {len(registered_ids)} webhook(s) for preset '{preset_id}' to database" + "Persisted %d webhook(s) for preset '%s' to database", + len(registered_ids), + preset_id, ) # Return updated card @@ -446,9 +499,9 @@ async def enable_webhook_preset(request: Request) -> HTMLResponse: ) except Exception as e: - logger.error(f"Failed to enable preset {preset_id}: {e}", exc_info=True) + logger.error("Failed to enable preset %s: %s", preset_id, e, exc_info=True) return HTMLResponse( - content=f'
Failed to enable preset: {str(e)}
', + content=f'
Failed to enable preset: {html.escape(str(e))}
', status_code=500, ) @@ -482,7 +535,7 @@ async def disable_webhook_preset(request: Request) -> HTMLResponse: preset = get_preset(preset_id) if not preset: return HTMLResponse( - content=f'
Unknown preset: {preset_id}
', + content=f'
Unknown preset: {html.escape(preset_id)}
', status_code=404, ) @@ -500,13 +553,15 @@ async def disable_webhook_preset(request: Request) -> HTMLResponse: for webhook_id in webhook_ids: await webhooks_client.delete_webhook(webhook_id) - logger.info(f"Deleted webhook {webhook_id} from preset {preset_id}") + logger.info("Deleted webhook %s from preset %s", webhook_id, preset_id) # Remove from database if storage: deleted_count = await storage.clear_preset_webhooks(preset_id) logger.info( - f"Removed {deleted_count} webhook(s) for preset '{preset_id}' from database" + "Removed %d webhook(s) for preset '%s' from database", + deleted_count, + preset_id, ) # Return updated card @@ -536,8 +591,8 @@ async def disable_webhook_preset(request: Request) -> HTMLResponse: ) except Exception as e: - logger.error(f"Failed to disable preset {preset_id}: {e}", exc_info=True) + logger.error("Failed to disable preset %s: %s", preset_id, e, exc_info=True) return HTMLResponse( - content=f'
Failed to disable preset: {str(e)}
', + content=f'
Failed to disable preset: {html.escape(str(e))}
', status_code=500, ) diff --git a/nextcloud_mcp_server/client/webhooks.py b/nextcloud_mcp_server/client/webhooks.py index e1b206be..8934298c 100644 --- a/nextcloud_mcp_server/client/webhooks.py +++ b/nextcloud_mcp_server/client/webhooks.py @@ -1,6 +1,6 @@ """Client for Nextcloud Webhook Listeners API operations.""" -from typing import Any, Dict, List, Optional +from typing import Any from nextcloud_mcp_server.client.base import BaseNextcloudClient @@ -11,15 +11,15 @@ class WebhooksClient(BaseNextcloudClient): app_name = "webhooks" def _get_webhook_headers( - self, additional_headers: Optional[Dict[str, str]] = None - ) -> Dict[str, str]: + self, additional_headers: dict[str, str] | None = None + ) -> dict[str, str]: """Get standard headers required for Webhook Listeners API calls.""" headers = {"OCS-APIRequest": "true", "Accept": "application/json"} if additional_headers: headers.update(additional_headers) return headers - async def list_webhooks(self) -> List[Dict[str, Any]]: + async def list_webhooks(self) -> list[dict[str, Any]]: """List all registered webhooks for the current user. Returns: @@ -40,23 +40,30 @@ class WebhooksClient(BaseNextcloudClient): uri: str, http_method: str = "POST", auth_method: str = "none", - headers: Optional[Dict[str, str]] = None, - event_filter: Optional[Dict[str, Any]] = None, - ) -> Dict[str, Any]: + headers: dict[str, str] | None = None, + auth_data: dict[str, str] | None = None, + event_filter: dict[str, Any] | None = None, + ) -> dict[str, Any]: """Register a new webhook for the specified event. Args: event: Fully qualified event class name (e.g., "OCP\\Files\\Events\\Node\\NodeCreatedEvent") uri: Webhook endpoint URL to receive event notifications http_method: HTTP method for webhook delivery (default: "POST") - auth_method: Authentication method ("none", "bearer", etc.) - headers: Custom headers to include in webhook requests (e.g., Authorization header) + auth_method: Authentication method. Nextcloud's webhook_listeners + app accepts only ``"none"`` or ``"header"``. + headers: Optional static request headers attached to every + delivery (stored in clear text on the NC side). + auth_data: When ``auth_method="header"``, a dict of headers + holding the auth credentials. Stored encrypted at-rest in + Nextcloud's database and merged into the delivery request + at send time. Required when ``auth_method="header"``. event_filter: JSON object specifying event filters (e.g., {"user.uid": "bob"}) Returns: Webhook registration details including webhook ID """ - data: Dict[str, Any] = { + data: dict[str, Any] = { "httpMethod": http_method, "uri": uri, "event": event, @@ -66,6 +73,9 @@ class WebhooksClient(BaseNextcloudClient): if headers: data["headers"] = headers + if auth_data: + data["authData"] = auth_data + if event_filter: data["eventFilter"] = event_filter @@ -91,7 +101,7 @@ class WebhooksClient(BaseNextcloudClient): headers=headers, ) - async def get_webhook(self, webhook_id: int) -> Dict[str, Any]: + async def get_webhook(self, webhook_id: int) -> dict[str, Any]: """Get details of a specific webhook registration. Args: diff --git a/nextcloud_mcp_server/config.py b/nextcloud_mcp_server/config.py index b84c989c..13a829fe 100644 --- a/nextcloud_mcp_server/config.py +++ b/nextcloud_mcp_server/config.py @@ -53,6 +53,13 @@ _DEFAULTS: dict[str, Any] = { # None = ephemeral per-process tempfile (see get_token_db_path()). # Set TOKEN_STORAGE_DB to persist tokens across restarts. "token_storage_db": None, + # Webhook delivery authentication (ADR-010): when set, registrations + # tell NC to add `Authorization: Bearer ` to webhook deliveries + # and the receiver rejects unauthenticated requests. + "webhook_secret": None, + # Internal URL override for webhook registration; wins over + # NEXTCLOUD_MCP_SERVER_URL when set (e.g. split internal/external URLs). + "webhook_internal_url": None, # Vector sync "vector_sync_scan_interval": 300, "vector_sync_processor_workers": 3, @@ -430,6 +437,17 @@ class Settings: token_encryption_key: str | None = None token_storage_db: str | None = None + # Webhook delivery authentication (ADR-010). + # When set, the registrar passes Authorization: Bearer as the + # webhook authData and the receiver validates the same header on each + # delivery. When unset, registration uses authMethod="none" and the + # receiver accepts unauthenticated POSTs (backward-compatible). + webhook_secret: str | None = None + # Internal URL override for webhook registration. Highest-priority + # source for the URL we register with NC (above + # nextcloud_mcp_server_url and the docker-detection fallback). + webhook_internal_url: str | None = None + # Vector sync settings (ADR-007) vector_sync_enabled: bool = False vector_sync_scan_interval: int = 300 # seconds (5 minutes) @@ -767,6 +785,9 @@ def get_settings() -> Settings: # Token and webhook storage settings "token_encryption_key": "TOKEN_ENCRYPTION_KEY", "token_storage_db": "TOKEN_STORAGE_DB", + # Webhook auth (ADR-010) + "webhook_secret": "WEBHOOK_SECRET", + "webhook_internal_url": "WEBHOOK_INTERNAL_URL", # Vector sync settings (ADR-007) "vector_sync_scan_interval": "VECTOR_SYNC_SCAN_INTERVAL", "vector_sync_processor_workers": "VECTOR_SYNC_PROCESSOR_WORKERS", diff --git a/nextcloud_mcp_server/vector/webhook_parser.py b/nextcloud_mcp_server/vector/webhook_parser.py new file mode 100644 index 00000000..dfcb3710 --- /dev/null +++ b/nextcloud_mcp_server/vector/webhook_parser.py @@ -0,0 +1,87 @@ +"""Parse Nextcloud webhook payloads into DocumentTask objects. + +Maps Nextcloud webhook events to vector-sync DocumentTasks. The handler at +``/webhooks/nextcloud`` calls :func:`extract_document_task` and forwards any +non-None result to the same processor send-stream the scanner uses. + +Currently scoped to file (note) events. Calendar / Tables events fall through +to ``None`` for now; those parsers can be added in follow-up changes. + +See ADR-010 for the design and ``webhook-testing-findings.md`` for real +captured payloads. +""" + +import logging +import re + +from nextcloud_mcp_server.vector.scanner import DocumentTask + +logger = logging.getLogger(__name__) + +_FILE_EVENT_CREATED = "OCP\\Files\\Events\\Node\\NodeCreatedEvent" +_FILE_EVENT_WRITTEN = "OCP\\Files\\Events\\Node\\NodeWrittenEvent" +_FILE_EVENT_BEFORE_DELETED = "OCP\\Files\\Events\\Node\\BeforeNodeDeletedEvent" + +# Matches paths inside any user's Notes folder ending in .md, e.g. +# "/admin/files/Notes/Sub/Note.md" or "/alice/files/Notes/foo.md". +_NOTES_PATH_RE = re.compile(r"^/[^/]+/files/Notes/.+\.md$") + + +def extract_document_task(payload: dict) -> DocumentTask | None: + """Convert a Nextcloud webhook payload into a DocumentTask. + + Returns None for any event we don't (yet) handle, or any event whose + target isn't a markdown file under a user's Notes folder. Callers should + treat None as "ignored" โ€” not an error. + """ + try: + event = payload["event"] + event_class = event["class"] + user_id = payload["user"]["uid"] + time = int(payload.get("time", 0) or 0) + except (KeyError, TypeError, ValueError): + logger.debug("Webhook payload has missing or malformed envelope fields") + return None + + if event_class in ( + _FILE_EVENT_CREATED, + _FILE_EVENT_WRITTEN, + _FILE_EVENT_BEFORE_DELETED, + ): + return _parse_file_event(event_class, event, user_id, time) + + logger.debug("Ignoring webhook for unsupported event: %s", event_class) + return None + + +def _parse_file_event( + event_class: str, event: dict, user_id: str, time: int +) -> DocumentTask | None: + node = event.get("node") or {} + path = node.get("path", "") + node_id = node.get("id") + + if not _NOTES_PATH_RE.match(path): + # Not a note file โ€” could be a parent folder, an unrelated file, etc. + return None + + if node_id is None: + # BeforeNodeDeletedEvent should still carry node.id; if it doesn't + # we can't address the Qdrant points to delete. Skip rather than + # guess โ€” the polling scanner will catch up via its grace period. + logger.warning( + "Webhook %s for note %s missing node.id; falling back to scanner", + event_class, + path, + ) + return None + + operation = "delete" if event_class == _FILE_EVENT_BEFORE_DELETED else "index" + + return DocumentTask( + user_id=user_id, + doc_id=str(node_id), + doc_type="note", + operation=operation, + modified_at=time, + ) diff --git a/nextcloud_mcp_server/vector/webhook_receiver.py b/nextcloud_mcp_server/vector/webhook_receiver.py new file mode 100644 index 00000000..d55d666c --- /dev/null +++ b/nextcloud_mcp_server/vector/webhook_receiver.py @@ -0,0 +1,149 @@ +"""HTTP receiver for Nextcloud webhooks. + +Routes inbound webhooks to the same processor send-stream the scanner uses. +The receiver is registered as a Starlette route at ``/webhooks/nextcloud`` +in :mod:`nextcloud_mcp_server.app`. +""" + +import hmac +import logging + +import anyio +from starlette.requests import Request +from starlette.responses import JSONResponse + +from nextcloud_mcp_server.config import get_settings +from nextcloud_mcp_server.vector.webhook_parser import extract_document_task + +logger = logging.getLogger(__name__) + +_warned_about_missing_secret = False + + +def _warn_missing_secret_once() -> None: + """Log a one-time WARNING when WEBHOOK_SECRET is unset. + + The receiver still accepts unauthenticated POSTs in this case so existing + deployments keep working, but the operator should know they're running + without webhook auth. + """ + global _warned_about_missing_secret + if _warned_about_missing_secret: + return + _warned_about_missing_secret = True + logger.warning( + "WEBHOOK_SECRET is not set; /webhooks/nextcloud accepts " + "unauthenticated requests. Set WEBHOOK_SECRET and re-register " + "webhooks to enable Authorization: Bearer validation." + ) + + +async def handle_nextcloud_webhook(request: Request) -> JSONResponse: + """Receive a Nextcloud webhook and queue a DocumentTask for vector sync. + + Returns quickly so NC's webhook worker is not blocked. The send-stream is + read from ``request.app.state.document_send_stream``; when vector sync + isn't running we return 503 so NC retries delivery. + + When ``WEBHOOK_SECRET`` is set, the request must carry + ``Authorization: Bearer `` (registered via ``authData`` so NC + forwards it on every delivery); requests without a valid header are + rejected with 401 before any further work. + """ + secret = get_settings().webhook_secret + if secret: + provided = request.headers.get("authorization", "").encode("utf-8") + expected = f"Bearer {secret}".encode("utf-8") + # Use compare_digest to avoid the character-by-character short-circuit + # of `==`. Comparing as bytes is the conventional form and avoids any + # surprise with non-ASCII input. compare_digest still returns False + # for differing lengths but isn't fully constant-time across them; + # that's fine here โ€” a secret length leak is not a sensitive signal. + if not hmac.compare_digest(provided, expected): + # Intentionally omit WWW-Authenticate. RFC 7235 ยง4.1 says a 401 + # SHOULD carry it, but Nextcloud's webhook delivery worker has no + # auth-flow state machine to negotiate against โ€” the bearer is a + # static shared secret configured out-of-band via WEBHOOK_SECRET, + # and a challenge response wouldn't change client behaviour. + # Surfacing it would only mislead operators into expecting a + # renegotiation that doesn't exist. + logger.warning("Webhook rejected: missing or invalid Authorization header") + return JSONResponse( + {"status": "unauthorized"}, + status_code=401, + ) + else: + _warn_missing_secret_once() + + try: + payload = await request.json() + except Exception as e: + logger.warning("Webhook payload was not valid JSON: %s", e) + return JSONResponse( + {"status": "error", "message": "invalid JSON"}, + status_code=400, + ) + + task = extract_document_task(payload) + if task is None: + event_class = (payload.get("event") or {}).get("class", "") + logger.debug("Webhook ignored (unsupported event): %s", event_class) + return JSONResponse( + {"status": "ignored", "reason": "unsupported event"}, + status_code=200, + ) + + send_stream = getattr(request.app.state, "document_send_stream", None) + if send_stream is None: + logger.warning( + "Webhook received but vector sync is not running; rejecting so NC retries" + ) + return JSONResponse( + {"status": "unavailable", "reason": "vector sync not running"}, + status_code=503, + ) + + try: + with anyio.fail_after(1.0): + await send_stream.send(task) + except TimeoutError: + # Queue is saturated (default 10 000 tasks). Returning 503 lets NC + # retry rather than pinning this handler until its outbound timeout + # fires; the queue-pressure signal also surfaces in metrics. + logger.warning( + "Webhook task drop: queue full for %s_%s", + task.doc_type, + task.doc_id, + ) + return JSONResponse( + {"status": "unavailable", "reason": "queue full"}, + status_code=503, + ) + except Exception as e: + logger.error( + "Failed to queue webhook task for %s_%s: %s", + task.doc_type, + task.doc_id, + e, + ) + return JSONResponse( + {"status": "error", "message": "queue unavailable"}, + status_code=500, + ) + + logger.info( + "Webhook queued %s_%s (%s) for user %s", + task.doc_type, + task.doc_id, + task.operation, + task.user_id, + ) + return JSONResponse( + { + "status": "queued", + "doc_type": task.doc_type, + "doc_id": task.doc_id, + "operation": task.operation, + }, + status_code=200, + ) diff --git a/tests/client/test_webhooks_client.py b/tests/client/test_webhooks_client.py index 6c5022f9..b09bf594 100644 --- a/tests/client/test_webhooks_client.py +++ b/tests/client/test_webhooks_client.py @@ -135,8 +135,11 @@ async def test_create_webhook_with_filter(webhooks_client, mocker): @pytest.mark.unit -async def test_create_webhook_with_auth_headers(webhooks_client, mocker): - """Test creating a webhook with authentication headers.""" +async def test_create_webhook_with_static_headers(webhooks_client, mocker): + """Static request headers (the ``headers`` field) ride on every delivery + independently of ``authData``. NC's webhook_listeners app accepts only + ``authMethod="none"`` or ``"header"`` โ€” pre-existing tests previously + referenced ``"bearer"`` which is not a valid value.""" mock_response = mocker.Mock() mock_response.json.return_value = { "ocs": { @@ -144,7 +147,7 @@ async def test_create_webhook_with_auth_headers(webhooks_client, mocker): "id": 125, "uri": "http://example.com/webhook", "event": "OCP\\Files\\Events\\Node\\NodeCreatedEvent", - "authMethod": "bearer", + "authMethod": "header", } } } @@ -156,17 +159,51 @@ async def test_create_webhook_with_auth_headers(webhooks_client, mocker): webhook_data = await webhooks_client.create_webhook( event="OCP\\Files\\Events\\Node\\NodeCreatedEvent", uri="http://example.com/webhook", - auth_method="bearer", - headers={"Authorization": "Bearer secret-token"}, + auth_method="header", + headers={"X-Trace-Id": "trace-123"}, ) assert webhook_data["id"] == 125 - assert webhook_data["authMethod"] == "bearer" + assert webhook_data["authMethod"] == "header" mock_make_request.assert_called_once() call_args = mock_make_request.call_args - assert call_args[1]["json"]["authMethod"] == "bearer" - assert call_args[1]["json"]["headers"] == {"Authorization": "Bearer secret-token"} + assert call_args[1]["json"]["authMethod"] == "header" + assert call_args[1]["json"]["headers"] == {"X-Trace-Id": "trace-123"} + + +@pytest.mark.unit +async def test_create_webhook_with_auth_data(webhooks_client, mocker): + """``auth_data`` lands in the OCS body as ``authData`` so NC encrypts + the credentials at-rest and merges them in at delivery time.""" + mock_response = mocker.Mock() + mock_response.json.return_value = { + "ocs": { + "data": { + "id": 126, + "uri": "http://example.com/webhook", + "event": "OCP\\Files\\Events\\Node\\NodeCreatedEvent", + "authMethod": "header", + } + } + } + + mock_make_request = mocker.patch.object( + WebhooksClient, "_make_request", return_value=mock_response + ) + + await webhooks_client.create_webhook( + event="OCP\\Files\\Events\\Node\\NodeCreatedEvent", + uri="http://example.com/webhook", + auth_method="header", + auth_data={"Authorization": "Bearer supersecret"}, + ) + + call_args = mock_make_request.call_args + assert call_args[1]["json"]["authMethod"] == "header" + assert call_args[1]["json"]["authData"] == {"Authorization": "Bearer supersecret"} + # The static `headers` field must NOT be set when only auth_data is passed. + assert "headers" not in call_args[1]["json"] @pytest.mark.unit diff --git a/tests/unit/test_webhook_endpoint.py b/tests/unit/test_webhook_endpoint.py new file mode 100644 index 00000000..cd1b05fb --- /dev/null +++ b/tests/unit/test_webhook_endpoint.py @@ -0,0 +1,299 @@ +"""Unit tests for the ``/webhooks/nextcloud`` HTTP receiver. + +Builds a minimal Starlette app around ``handle_nextcloud_webhook`` so we can +drive it with ``TestClient`` without standing up the full FastMCP server. +""" + +import anyio +import pytest +from starlette.applications import Starlette +from starlette.routing import Route +from starlette.testclient import TestClient + +from nextcloud_mcp_server.config import Settings +from nextcloud_mcp_server.vector import webhook_receiver +from nextcloud_mcp_server.vector.webhook_receiver import handle_nextcloud_webhook + +pytestmark = pytest.mark.unit + + +@pytest.fixture(autouse=True) +def _reset_warned_flag(): + """The receiver warns once per process when WEBHOOK_SECRET is missing. + Reset between tests so each gets a clean slate.""" + webhook_receiver._warned_about_missing_secret = False + yield + webhook_receiver._warned_about_missing_secret = False + + +def _patch_secret(monkeypatch, secret: str | None) -> None: + """Make ``get_settings()`` (as called inside the receiver) return a + Settings instance with the given ``webhook_secret``.""" + monkeypatch.setattr( + webhook_receiver, + "get_settings", + lambda: Settings(webhook_secret=secret), + ) + + +def _make_app(send_stream=None) -> Starlette: + app = Starlette( + routes=[ + Route("/webhooks/nextcloud", handle_nextcloud_webhook, methods=["POST"]) + ] + ) + app.state.document_send_stream = send_stream + return app + + +_NOTE_CREATED = { + "user": {"uid": "admin", "displayName": "admin"}, + "time": 1762850245, + "event": { + "class": "OCP\\Files\\Events\\Node\\NodeCreatedEvent", + "node": { + "id": 437, + "path": "/admin/files/Notes/Webhooks/Webhook Test Note.md", + }, + }, +} + + +_NOTE_DELETED = { + "user": {"uid": "alice"}, + "time": 1762851093, + "event": { + "class": "OCP\\Files\\Events\\Node\\BeforeNodeDeletedEvent", + "node": {"id": 99, "path": "/alice/files/Notes/foo.md"}, + }, +} + + +def test_index_event_queues_task_and_returns_200(): + send_stream, receive_stream = anyio.create_memory_object_stream(max_buffer_size=4) + app = _make_app(send_stream=send_stream) + + with TestClient(app) as client: + response = client.post("/webhooks/nextcloud", json=_NOTE_CREATED) + + assert response.status_code == 200 + assert response.json()["status"] == "queued" + assert response.json()["operation"] == "index" + assert response.json()["doc_id"] == "437" + + task = receive_stream.receive_nowait() + assert task.user_id == "admin" + assert task.doc_id == "437" + assert task.operation == "index" + assert task.doc_type == "note" + + +def test_delete_event_queues_delete_task(): + send_stream, receive_stream = anyio.create_memory_object_stream(max_buffer_size=4) + app = _make_app(send_stream=send_stream) + + with TestClient(app) as client: + response = client.post("/webhooks/nextcloud", json=_NOTE_DELETED) + + assert response.status_code == 200 + assert response.json()["operation"] == "delete" + + task = receive_stream.receive_nowait() + assert task.operation == "delete" + assert task.doc_id == "99" + assert task.user_id == "alice" + + +def test_unsupported_event_is_ignored(): + send_stream, receive_stream = anyio.create_memory_object_stream(max_buffer_size=4) + app = _make_app(send_stream=send_stream) + + payload = { + "user": {"uid": "admin"}, + "time": 1, + "event": { + "class": "OCP\\Calendar\\Events\\CalendarObjectCreatedEvent", + "objectData": {"id": 7}, + }, + } + + with TestClient(app) as client: + response = client.post("/webhooks/nextcloud", json=payload) + + assert response.status_code == 200 + assert response.json()["status"] == "ignored" + + with pytest.raises(anyio.WouldBlock): + receive_stream.receive_nowait() + + +def test_invalid_json_returns_400(): + app = _make_app(send_stream=None) + + with TestClient(app) as client: + response = client.post( + "/webhooks/nextcloud", + content=b"not json", + headers={"content-type": "application/json"}, + ) + + assert response.status_code == 400 + assert response.json()["status"] == "error" + + +def test_returns_503_when_send_stream_not_wired(): + """Vector sync not running โ†’ tell NC to retry instead of dropping the + event.""" + app = _make_app(send_stream=None) + + with TestClient(app) as client: + response = client.post("/webhooks/nextcloud", json=_NOTE_CREATED) + + assert response.status_code == 503 + assert response.json()["status"] == "unavailable" + + +def test_returns_500_when_stream_is_closed(): + send_stream, receive_stream = anyio.create_memory_object_stream(max_buffer_size=1) + receive_stream.close() # close receiver โ†’ send raises BrokenResourceError + app = _make_app(send_stream=send_stream) + + with TestClient(app) as client: + response = client.post("/webhooks/nextcloud", json=_NOTE_CREATED) + + assert response.status_code == 500 + assert response.json()["status"] == "error" + + +def test_returns_503_when_queue_is_full(monkeypatch): + """When the processor queue is saturated, the handler must time out + quickly with 503 instead of pinning until NC's outbound timeout fires.""" + # Speed up the test โ€” a 1s deadline matches production but is overkill + # for a unit test that's specifically exercising the timeout branch. + # Capture the original BEFORE patching so the override doesn't recurse + # into itself (the receiver imports the same anyio module object). + real_fail_after = anyio.fail_after + monkeypatch.setattr( + "nextcloud_mcp_server.vector.webhook_receiver.anyio.fail_after", + lambda _seconds: real_fail_after(0.05), + ) + + # Buffer of 1, no consumer โ†’ first send fills it, second blocks. + send_stream, _receive_stream = anyio.create_memory_object_stream(max_buffer_size=1) + send_stream.send_nowait("sentinel") # type: ignore[arg-type] + app = _make_app(send_stream=send_stream) + + with TestClient(app) as client: + response = client.post("/webhooks/nextcloud", json=_NOTE_CREATED) + + assert response.status_code == 503 + body = response.json() + assert body["status"] == "unavailable" + assert body["reason"] == "queue full" + + +# --- WEBHOOK_SECRET authentication --------------------------------------- + + +def test_secret_set_valid_bearer_header_queues_task(monkeypatch): + _patch_secret(monkeypatch, "supersecret") + send_stream, receive_stream = anyio.create_memory_object_stream(max_buffer_size=4) + app = _make_app(send_stream=send_stream) + + with TestClient(app) as client: + response = client.post( + "/webhooks/nextcloud", + json=_NOTE_CREATED, + headers={"Authorization": "Bearer supersecret"}, + ) + + assert response.status_code == 200 + assert response.json()["status"] == "queued" + assert receive_stream.receive_nowait().doc_id == "437" + + +def test_secret_set_missing_authorization_returns_401(monkeypatch): + _patch_secret(monkeypatch, "supersecret") + send_stream, receive_stream = anyio.create_memory_object_stream(max_buffer_size=4) + app = _make_app(send_stream=send_stream) + + with TestClient(app) as client: + response = client.post("/webhooks/nextcloud", json=_NOTE_CREATED) + + assert response.status_code == 401 + assert response.json()["status"] == "unauthorized" + with pytest.raises(anyio.WouldBlock): + receive_stream.receive_nowait() + + +def test_secret_set_wrong_secret_returns_401(monkeypatch): + _patch_secret(monkeypatch, "supersecret") + send_stream, receive_stream = anyio.create_memory_object_stream(max_buffer_size=4) + app = _make_app(send_stream=send_stream) + + with TestClient(app) as client: + response = client.post( + "/webhooks/nextcloud", + json=_NOTE_CREATED, + headers={"Authorization": "Bearer wrong"}, + ) + + assert response.status_code == 401 + with pytest.raises(anyio.WouldBlock): + receive_stream.receive_nowait() + + +def test_secret_set_wrong_scheme_returns_401(monkeypatch): + """A token without the Bearer prefix is rejected.""" + _patch_secret(monkeypatch, "supersecret") + send_stream, receive_stream = anyio.create_memory_object_stream(max_buffer_size=4) + app = _make_app(send_stream=send_stream) + + with TestClient(app) as client: + response = client.post( + "/webhooks/nextcloud", + json=_NOTE_CREATED, + headers={"Authorization": "supersecret"}, + ) + + assert response.status_code == 401 + + +def test_secret_unset_accepts_unauthenticated(monkeypatch): + """Backward compat: deployments that haven't yet set WEBHOOK_SECRET keep + working โ€” the receiver accepts unauthenticated POSTs and logs a one-time + warning.""" + _patch_secret(monkeypatch, None) + send_stream, receive_stream = anyio.create_memory_object_stream(max_buffer_size=4) + app = _make_app(send_stream=send_stream) + + with TestClient(app) as client: + response = client.post("/webhooks/nextcloud", json=_NOTE_CREATED) + + assert response.status_code == 200 + assert receive_stream.receive_nowait().doc_id == "437" + + +def test_compare_digest_is_called_with_bytes(monkeypatch, mocker): + """Regression: secret comparison must run on bytes, not strings, so + that future non-ASCII secret support doesn't depend on Python's + implicit ASCII encoding.""" + _patch_secret(monkeypatch, "supersecret") + spy = mocker.spy(webhook_receiver.hmac, "compare_digest") + + send_stream, _receive = anyio.create_memory_object_stream(max_buffer_size=4) + app = _make_app(send_stream=send_stream) + + with TestClient(app) as client: + response = client.post( + "/webhooks/nextcloud", + json=_NOTE_CREATED, + headers={"Authorization": "Bearer supersecret"}, + ) + + assert response.status_code == 200 + assert spy.call_count == 1 + provided_arg, expected_arg = spy.call_args.args + assert isinstance(provided_arg, bytes) + assert isinstance(expected_arg, bytes) + assert expected_arg == b"Bearer supersecret" diff --git a/tests/unit/test_webhook_parser.py b/tests/unit/test_webhook_parser.py new file mode 100644 index 00000000..ddcc86a6 --- /dev/null +++ b/tests/unit/test_webhook_parser.py @@ -0,0 +1,218 @@ +"""Unit tests for the Nextcloud webhook payload parser. + +Payload examples are taken from real Nextcloud captures recorded in +``webhook-testing-findings.md``. +""" + +import pytest + +from nextcloud_mcp_server.vector.webhook_parser import extract_document_task + + +@pytest.mark.unit +def test_node_created_event_returns_index_task(): + payload = { + "user": {"uid": "admin", "displayName": "admin"}, + "time": 1762850245, + "event": { + "class": "OCP\\Files\\Events\\Node\\NodeCreatedEvent", + "node": { + "id": 437, + "path": "/admin/files/Notes/Webhooks/Webhook Test Note.md", + }, + }, + } + + task = extract_document_task(payload) + + assert task is not None + assert task.user_id == "admin" + assert task.doc_id == "437" + assert task.doc_type == "note" + assert task.operation == "index" + assert task.modified_at == 1762850245 + + +@pytest.mark.unit +def test_node_written_event_returns_index_task(): + payload = { + "user": {"uid": "admin", "displayName": "admin"}, + "time": 1762850960, + "event": { + "class": "OCP\\Files\\Events\\Node\\NodeWrittenEvent", + "node": { + "id": 437, + "path": "/admin/files/Notes/Webhooks/Webhook Test Note.md", + }, + }, + } + + task = extract_document_task(payload) + + assert task is not None + assert task.operation == "index" + assert task.doc_id == "437" + + +@pytest.mark.unit +def test_before_node_deleted_event_returns_delete_task(): + payload = { + "user": {"uid": "alice", "displayName": "Alice"}, + "time": 1762851093, + "event": { + "class": "OCP\\Files\\Events\\Node\\BeforeNodeDeletedEvent", + "node": { + "id": 437, + "path": "/alice/files/Notes/Webhooks/Webhook Test Note.md", + }, + }, + } + + task = extract_document_task(payload) + + assert task is not None + assert task.user_id == "alice" + assert task.operation == "delete" + assert task.doc_id == "437" + assert task.doc_type == "note" + + +@pytest.mark.unit +def test_node_id_is_normalized_to_string(): + """NC sends node.id as int; we always emit str (per ADR-010 ยงA.3).""" + payload = { + "user": {"uid": "admin"}, + "time": 1, + "event": { + "class": "OCP\\Files\\Events\\Node\\NodeCreatedEvent", + "node": {"id": 437, "path": "/admin/files/Notes/foo.md"}, + }, + } + + task = extract_document_task(payload) + + assert task is not None + assert isinstance(task.doc_id, str) + assert task.doc_id == "437" + + +@pytest.mark.unit +def test_path_outside_notes_returns_none(): + payload = { + "user": {"uid": "admin"}, + "time": 1, + "event": { + "class": "OCP\\Files\\Events\\Node\\NodeCreatedEvent", + "node": {"id": 1, "path": "/admin/files/Documents/foo.md"}, + }, + } + + assert extract_document_task(payload) is None + + +@pytest.mark.unit +def test_non_markdown_inside_notes_returns_none(): + payload = { + "user": {"uid": "admin"}, + "time": 1, + "event": { + "class": "OCP\\Files\\Events\\Node\\NodeCreatedEvent", + "node": {"id": 1, "path": "/admin/files/Notes/image.png"}, + }, + } + + assert extract_document_task(payload) is None + + +@pytest.mark.unit +def test_parent_folder_event_returns_none(): + """Creating a note fires events for the parent folder too โ€” ignore those.""" + payload = { + "user": {"uid": "admin"}, + "time": 1, + "event": { + "class": "OCP\\Files\\Events\\Node\\NodeCreatedEvent", + "node": {"id": 100, "path": "/admin/files/Notes/Webhooks"}, + }, + } + + assert extract_document_task(payload) is None + + +@pytest.mark.unit +def test_unknown_event_class_returns_none(): + payload = { + "user": {"uid": "admin"}, + "time": 1, + "event": { + "class": "OCP\\Calendar\\Events\\CalendarObjectCreatedEvent", + "objectData": {"id": 7, "uri": "x.ics"}, + }, + } + + assert extract_document_task(payload) is None + + +@pytest.mark.unit +def test_node_deleted_event_without_id_returns_none(): + """``NodeDeletedEvent`` (no node.id) is not the registered event, but if + one ever leaks through we ignore it rather than guess at the doc_id โ€” + the polling scanner will catch up via its grace period.""" + payload = { + "user": {"uid": "admin"}, + "time": 1, + "event": { + "class": "OCP\\Files\\Events\\Node\\BeforeNodeDeletedEvent", + "node": {"path": "/admin/files/Notes/foo.md"}, + }, + } + + assert extract_document_task(payload) is None + + +@pytest.mark.unit +def test_missing_user_field_returns_none(): + payload = { + "time": 1, + "event": { + "class": "OCP\\Files\\Events\\Node\\NodeCreatedEvent", + "node": {"id": 1, "path": "/admin/files/Notes/foo.md"}, + }, + } + + assert extract_document_task(payload) is None + + +@pytest.mark.unit +def test_empty_payload_returns_none(): + assert extract_document_task({}) is None + + +@pytest.mark.unit +def test_non_numeric_time_field_returns_none(): + """A malformed ``time`` field must not raise ValueError out of the parser.""" + payload = { + "user": {"uid": "admin"}, + "time": "not-a-number", + "event": { + "class": "OCP\\Files\\Events\\Node\\NodeCreatedEvent", + "node": {"id": 1, "path": "/admin/files/Notes/foo.md"}, + }, + } + + assert extract_document_task(payload) is None + + +@pytest.mark.unit +def test_missing_time_field_defaults_to_zero(): + payload = { + "user": {"uid": "admin"}, + "event": { + "class": "OCP\\Files\\Events\\Node\\NodeCreatedEvent", + "node": {"id": 1, "path": "/admin/files/Notes/foo.md"}, + }, + } + + task = extract_document_task(payload) + assert task is not None + assert task.modified_at == 0 diff --git a/tests/unit/test_webhook_routes_register.py b/tests/unit/test_webhook_routes_register.py new file mode 100644 index 00000000..ae7568f0 --- /dev/null +++ b/tests/unit/test_webhook_routes_register.py @@ -0,0 +1,84 @@ +"""Unit tests for ``_register_preset_webhooks``. + +The helper threads ``webhook_auth_pair()`` into each ``create_webhook`` +call, so it is the integration point between the secret-resolution logic +and the OCS client. These tests verify the wiring without standing up a +full Starlette app. +""" + +import pytest + +from nextcloud_mcp_server.auth import webhook_routes +from nextcloud_mcp_server.auth.webhook_routes import _register_preset_webhooks +from nextcloud_mcp_server.client.webhooks import WebhooksClient +from nextcloud_mcp_server.config import Settings +from nextcloud_mcp_server.server.webhook_presets import get_preset + +pytestmark = pytest.mark.unit + + +def _patch_secret(monkeypatch, secret: str | None) -> None: + monkeypatch.setattr( + webhook_routes, + "get_settings", + lambda: Settings(webhook_secret=secret), + ) + + +def _make_webhooks_client(mocker, ids: list[int]): + """Mock WebhooksClient.create_webhook to return one fake webhook per id.""" + client = mocker.AsyncMock(spec=WebhooksClient) + client.create_webhook.side_effect = [{"id": i} for i in ids] + return client + + +async def test_register_threads_bearer_auth_when_secret_set(monkeypatch, mocker): + _patch_secret(monkeypatch, "supersecret") + preset = get_preset("notes_sync") + assert preset is not None + client = _make_webhooks_client(mocker, ids=[101, 102, 103]) + + registered = await _register_preset_webhooks( + client, preset, "https://mcp.example.com/webhooks/nextcloud" + ) + + assert registered == [101, 102, 103] + assert client.create_webhook.await_count == len(preset["events"]) + + expected_auth = {"Authorization": "Bearer supersecret"} + for call, event_config in zip( + client.create_webhook.await_args_list, preset["events"] + ): + kwargs = call.kwargs + assert kwargs["event"] == event_config["event"] + assert kwargs["uri"] == "https://mcp.example.com/webhooks/nextcloud" + assert kwargs["auth_method"] == "header" + assert kwargs["auth_data"] == expected_auth + # notes_sync uses path filters; ensure they round-trip through the helper + assert kwargs["event_filter"] == event_config["filter"] + + +async def test_register_uses_none_auth_when_secret_unset(monkeypatch, mocker): + _patch_secret(monkeypatch, None) + preset = get_preset("notes_sync") + assert preset is not None + client = _make_webhooks_client(mocker, ids=[1, 2, 3]) + + await _register_preset_webhooks( + client, preset, "https://mcp.example.com/webhooks/nextcloud" + ) + + for call in client.create_webhook.await_args_list: + assert call.kwargs["auth_method"] == "none" + assert call.kwargs["auth_data"] is None + + +async def test_register_returns_ids_in_call_order(monkeypatch, mocker): + _patch_secret(monkeypatch, None) + preset = get_preset("notes_sync") + assert preset is not None + client = _make_webhooks_client(mocker, ids=[42, 43, 44]) + + ids = await _register_preset_webhooks(client, preset, "https://example.com/wh") + + assert ids == [42, 43, 44] diff --git a/tests/unit/test_webhook_routes_xss.py b/tests/unit/test_webhook_routes_xss.py new file mode 100644 index 00000000..5bb1d7a1 --- /dev/null +++ b/tests/unit/test_webhook_routes_xss.py @@ -0,0 +1,133 @@ +"""Unit tests verifying that user-influenced and exception-derived strings +are HTML-escaped before they are rendered into ``HTMLResponse`` content. + +The handlers under test are decorated with ``@requires("authenticated")``, +so we install an ``AuthenticationMiddleware`` backed by a trivial backend +that always reports the request as authenticated. We then monkeypatch the +internal helpers (``_get_authenticated_client``, ``is_nextcloud_admin``, +``get_preset``) to drive the handler down the specific code path we want +to exercise. +""" + +import pytest +from starlette.applications import Starlette +from starlette.authentication import ( + AuthCredentials, + AuthenticationBackend, + SimpleUser, +) +from starlette.middleware import Middleware +from starlette.middleware.authentication import AuthenticationMiddleware +from starlette.routing import Route +from starlette.testclient import TestClient + +from nextcloud_mcp_server.auth import webhook_routes +from nextcloud_mcp_server.auth.webhook_routes import ( + disable_webhook_preset, + enable_webhook_preset, +) + +pytestmark = pytest.mark.unit + + +class _AlwaysAuthBackend(AuthenticationBackend): + async def authenticate(self, conn): + return AuthCredentials(["authenticated"]), SimpleUser("testuser") + + +def _make_app() -> Starlette: + return Starlette( + routes=[ + Route( + "/app/webhooks/enable/{preset_id:path}", + enable_webhook_preset, + methods=["POST"], + ), + Route( + "/app/webhooks/disable/{preset_id:path}", + disable_webhook_preset, + methods=["DELETE"], + ), + ], + middleware=[Middleware(AuthenticationMiddleware, backend=_AlwaysAuthBackend())], + ) + + +def _stub_admin_path(monkeypatch): + """Make the handler progress past auth/admin checks without real I/O.""" + + async def _fake_client(_request): + return object() # never actually used because get_preset returns None + + async def _fake_is_admin(_request, _client): + return True + + monkeypatch.setattr(webhook_routes, "_get_authenticated_client", _fake_client) + monkeypatch.setattr(webhook_routes, "is_nextcloud_admin", _fake_is_admin) + + +def test_enable_unknown_preset_id_is_html_escaped(monkeypatch): + """A `" + + with TestClient(app) as client: + response = client.post(f"/app/webhooks/enable/{payload}") + + assert response.status_code == 404 + assert "<script>alert(1)</script>" in response.text + assert "" not in response.text + + +def test_disable_unknown_preset_id_is_html_escaped(monkeypatch): + _stub_admin_path(monkeypatch) + monkeypatch.setattr(webhook_routes, "get_preset", lambda _id: None) + + app = _make_app() + payload = "" + + with TestClient(app) as client: + response = client.delete(f"/app/webhooks/disable/{payload}") + + assert response.status_code == 404 + assert "<script>alert(2)</script>" in response.text + assert "" not in response.text + + +def test_enable_exception_message_is_html_escaped(monkeypatch): + """If the handler raises, the exception text must be escaped before + it lands in the 500 response body.""" + + async def _boom(_request): + raise RuntimeError("

") + + monkeypatch.setattr(webhook_routes, "_get_authenticated_client", _boom) + + app = _make_app() + + with TestClient(app) as client: + response = client.post("/app/webhooks/enable/notes_sync") + + assert response.status_code == 500 + assert "</p><script>x</script>" in response.text + assert "" not in response.text + + +def test_disable_exception_message_is_html_escaped(monkeypatch): + async def _boom(_request): + raise RuntimeError("

") + + monkeypatch.setattr(webhook_routes, "_get_authenticated_client", _boom) + + app = _make_app() + + with TestClient(app) as client: + response = client.delete("/app/webhooks/disable/notes_sync") + + assert response.status_code == 500 + assert "</p><script>y</script>" in response.text + assert "" not in response.text diff --git a/tests/unit/test_webhook_uri.py b/tests/unit/test_webhook_uri.py new file mode 100644 index 00000000..84045dcc --- /dev/null +++ b/tests/unit/test_webhook_uri.py @@ -0,0 +1,140 @@ +"""Unit tests for ``_get_webhook_uri`` priority order and the +``webhook_auth_pair`` registration helper. + +Cloud deployments register the webhook URI returned by this function with +Nextcloud. ECS Fargate also exposes ``/.dockerenv``, so an explicit public +URL must win over the docker auto-detection branch. The URL fields are +read via dynaconf (``Settings``), so tests patch ``get_settings`` directly. +The docker-detection markers (``/.dockerenv``, ``DOCKER_CONTAINER``, +``NEXTCLOUD_MCP_SERVICE_NAME``, ``NEXTCLOUD_MCP_PORT``) remain on +``os.getenv`` and are exercised via env-var monkeypatching. +""" + +import pytest + +from nextcloud_mcp_server.auth import webhook_routes +from nextcloud_mcp_server.auth.webhook_routes import ( + _get_webhook_uri, + webhook_auth_pair, +) +from nextcloud_mcp_server.config import Settings + +DOCKER_ENV_VARS = ( + "NEXTCLOUD_MCP_SERVICE_NAME", + "NEXTCLOUD_MCP_PORT", + "DOCKER_CONTAINER", +) + + +@pytest.fixture(autouse=True) +def _clean_env(monkeypatch): + for name in DOCKER_ENV_VARS: + monkeypatch.delenv(name, raising=False) + + +def _patch_settings(monkeypatch, **overrides) -> None: + """Make ``get_settings()`` (as called inside webhook_routes) return a + Settings instance with the given URL/secret fields set; everything else + falls back to the dataclass defaults.""" + monkeypatch.setattr( + webhook_routes, + "get_settings", + lambda: Settings(**overrides), + ) + + +def _no_docker_markers(monkeypatch): + monkeypatch.setattr( + "nextcloud_mcp_server.auth.webhook_routes.os.path.exists", + lambda _path: False, + ) + + +def _docker_markers(monkeypatch): + monkeypatch.setattr( + "nextcloud_mcp_server.auth.webhook_routes.os.path.exists", + lambda path: path == "/.dockerenv", + ) + + +@pytest.mark.unit +def test_webhook_internal_url_wins_over_everything(monkeypatch): + _patch_settings( + monkeypatch, + webhook_internal_url="https://internal.example.com", + nextcloud_mcp_server_url="https://public.example.com", + ) + _docker_markers(monkeypatch) + + assert _get_webhook_uri() == "https://internal.example.com/webhooks/nextcloud" + + +@pytest.mark.unit +def test_public_url_wins_over_docker_detection(monkeypatch): + """The bug-fix case: ECS containers have /.dockerenv but a public URL is + set. Docker auto-detection must NOT clobber the explicit public URL.""" + _patch_settings( + monkeypatch, + nextcloud_mcp_server_url="https://holy-bluegill.astrolabecloud.com", + ) + _docker_markers(monkeypatch) + + assert ( + _get_webhook_uri() + == "https://holy-bluegill.astrolabecloud.com/webhooks/nextcloud" + ) + + +@pytest.mark.unit +def test_docker_detection_used_when_no_public_url(monkeypatch): + """docker-compose dev: no public URL set, /.dockerenv exists โ†’ use the + docker-compose service name.""" + _patch_settings(monkeypatch) + _docker_markers(monkeypatch) + + assert _get_webhook_uri() == "http://mcp:8000/webhooks/nextcloud" + + +@pytest.mark.unit +def test_docker_detection_honors_service_name_and_port_overrides(monkeypatch): + _patch_settings(monkeypatch) + monkeypatch.setenv("NEXTCLOUD_MCP_SERVICE_NAME", "mcp-login-flow") + monkeypatch.setenv("NEXTCLOUD_MCP_PORT", "8004") + _docker_markers(monkeypatch) + + assert _get_webhook_uri() == "http://mcp-login-flow:8004/webhooks/nextcloud" + + +@pytest.mark.unit +def test_docker_container_env_var_triggers_docker_branch(monkeypatch): + _patch_settings(monkeypatch) + monkeypatch.setenv("DOCKER_CONTAINER", "true") + _no_docker_markers(monkeypatch) + + assert _get_webhook_uri() == "http://mcp:8000/webhooks/nextcloud" + + +@pytest.mark.unit +def test_localhost_fallback_when_nothing_set(monkeypatch): + _patch_settings(monkeypatch) + _no_docker_markers(monkeypatch) + + assert _get_webhook_uri() == "http://localhost:8000/webhooks/nextcloud" + + +# --- webhook_auth_pair() -------------------------------------------------- + + +@pytest.mark.unit +def test_auth_pair_returns_none_when_secret_unset(monkeypatch): + _patch_settings(monkeypatch, webhook_secret=None) + assert webhook_auth_pair() == ("none", None) + + +@pytest.mark.unit +def test_auth_pair_emits_bearer_header_when_secret_set(monkeypatch): + _patch_settings(monkeypatch, webhook_secret="supersecret") + assert webhook_auth_pair() == ( + "header", + {"Authorization": "Bearer supersecret"}, + )