diff --git a/CHANGELOG.md b/CHANGELOG.md index 3105abc6..472b7b13 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,29 @@ All notable changes to the Nextcloud MCP Server will be documented in this file. The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), and this project adheres to [PEP 440](https://peps.python.org/pep-0440/). +## v0.75.0 (2026-04-29) + +### Feat + +- **talk**: add MCP integration for Nextcloud Talk (spreed) + +### Fix + +- **talk**: address remaining PR #741 reviewer feedback +- **talk**: address PR #741 reviewer feedback + +## v0.74.0 (2026-04-29) + +### Feat + +- **auth**: drop test-client defaults, add ALLOWED_MGMT_CLIENT allowlist + +## v0.73.2 (2026-04-29) + +### Fix + +- **oauth**: follow redirects when fetching OIDC discovery + ## v0.73.1 (2026-04-29) ### Fix diff --git a/README.md b/README.md index 97dbe65a..35402309 100644 --- a/README.md +++ b/README.md @@ -90,6 +90,7 @@ For Kubernetes, see [cbcoutinho/helm-charts](https://github.com/cbcoutinho/helm- | **Sharing** | 10+ | Create and manage shares | | **News** | 8 | Feeds, folders, items, feed health monitoring | | **Collectives** | 16 | Full CRUD on collectives, pages, and tags | +| **Talk (spreed)** | 6 | List conversations, read/post messages, mark as read, list participants | | **Semantic Search** | 2+ | Vector search for Notes, Files, News items, and Deck cards (experimental, opt-in, requires infrastructure) | Want to see another Nextcloud app supported? [Open an issue](https://github.com/cbcoutinho/nextcloud-mcp-server/issues) or contribute a pull request! diff --git a/app-hooks/post-installation/10-install-spreed-app.sh b/app-hooks/post-installation/10-install-spreed-app.sh new file mode 100755 index 00000000..8992e379 --- /dev/null +++ b/app-hooks/post-installation/10-install-spreed-app.sh @@ -0,0 +1,22 @@ +#!/bin/bash + +set -euox pipefail + +# Talk (spreed) ships its appstore release on demand; on a fresh container the +# app is not yet installed, so app:install pulls it. On rebuilds where the +# volume already has it, app:install fails with "already installed" — that +# specific failure is benign, hence the trailing `|| true`. We then run +# app:enable separately, which is the action that actually has to succeed. +# +# This split (install || true; then enable) is preferred over the previous +# `app:install || app:enable` chain because: +# - `--keep-disabled` keeps install side-effects strictly to fetching/extracting +# the app, so the enable step is the single source of truth for whether the +# app is active. +# - `--force` skips the compatibility check, locking the script to spreed's +# current behaviour rather than the appstore's view of NC compatibility. +# - If app:install dies for an unrelated reason (network outage, appstore +# unreachable on a fresh install), app:enable now fails with the clearer +# "app not found" rather than the install-time error being masked entirely. +php /var/www/html/occ app:install spreed --keep-disabled --force || true +php /var/www/html/occ app:enable spreed diff --git a/docker-compose.yml b/docker-compose.yml index 62fa8e02..8f3bda13 100644 --- a/docker-compose.yml +++ b/docker-compose.yml @@ -168,6 +168,11 @@ services: # - NEXTCLOUD_OIDC_CLIENT_ID=your_client_id # - NEXTCLOUD_OIDC_CLIENT_SECRET=your_client_secret + # Management API allowlist (ADR-018): only Astrolabe-issued tokens may + # call /api/management/*. Default test client_id used by the + # configure_astrolabe_for_mcp_server fixture. + - ALLOWED_MGMT_CLIENT=nextcloudMcpServerUIPublicClient + # NO admin credentials - credentials come from client Authorization header volumes: - multi-user-basic-data:/app/data diff --git a/env.sample b/env.sample index 07a0d247..652d9435 100644 --- a/env.sample +++ b/env.sample @@ -119,6 +119,31 @@ NEXTCLOUD_PASSWORD= # Optional features (semantic search, document processing): # See "Optional Features" section below +# ============================================ +# OAUTH CLIENT ALLOWLISTS (OAuth modes) +# ============================================ +# Both env vars are FAIL-CLOSED: if unset/empty, the corresponding auth route +# rejects every request. There are no built-in defaults. +# +# Note: ALLOWED_MCP_CLIENTS and ALLOWED_MGMT_CLIENT are currently separate to +# keep the MCP-route and management-API auth surfaces independent. They may be +# consolidated into a single env var later. + +# ===== ALLOWED_MCP_CLIENTS ===== +# Clients allowed to use the OAuth AS proxy (/oauth/authorize, /oauth/token). +# Comma-separated. Each entry is either: +# - "client_id" → loopback redirect URIs (http://localhost:*, http://127.0.0.1:*) +# - "client_id|https://app/cb" → bind to a specific HTTPS callback +# HTTP redirect URIs are rejected unless the host is loopback (localhost/127.0.0.1/::1). +#ALLOWED_MCP_CLIENTS=claude-desktop, zed-editor, cloud-app|https://cloud.example.com/cb + +# ===== ALLOWED_MGMT_CLIENT ===== +# OIDC client_ids whose tokens are accepted by the management API +# (/api/management/*). Comma-separated. The token's `client_id` claim must +# match one of these entries. Typical value: the Astrolabe NC PHP app's +# OAuth client_id (ADR-018). +#ALLOWED_MGMT_CLIENT=astrolabe + # ============================================ # OPTIONAL FEATURES (All Deployment Modes) # ============================================ diff --git a/nextcloud_mcp_server/auth/client_registry.py b/nextcloud_mcp_server/auth/client_registry.py index af46828b..1a9e317c 100644 --- a/nextcloud_mcp_server/auth/client_registry.py +++ b/nextcloud_mcp_server/auth/client_registry.py @@ -66,7 +66,15 @@ class ClientRegistry: - http://localhost:* and http://127.0.0.1:* are allowed (native clients) - https:// redirect URIs are allowed (cloud clients) - http:// non-localhost redirect URIs are rejected with a warning + + If the env var is unset or empty, the registry remains empty and + ``validate_client`` rejects every client_id (fail-closed). There are no + built-in defaults; operators must opt-in clients explicitly. """ + # NOTE: ALLOWED_MCP_CLIENTS and ALLOWED_MGMT_CLIENT are currently separate + # env vars to keep the MCP-route and management-API auth surfaces + # independent. These may be consolidated into a single env var later + # once the deployment story stabilises. allowed_clients = os.getenv("ALLOWED_MCP_CLIENTS", "").strip() if allowed_clients: @@ -123,9 +131,11 @@ class ClientRegistry: ) logger.info(f"Registered static client: {entry}") - # Add well-known clients if not explicitly configured if not self._clients: - self._add_well_known_clients() + logger.warning( + "Client registry is empty: ALLOWED_MCP_CLIENTS is unset or empty. " + "All MCP-flow OAuth requests will be rejected until configured." + ) def _get_client_name(self, client_id: str) -> str: """Get human-readable name for client_id.""" @@ -135,35 +145,9 @@ class ClientRegistry: "continue-dev": "Continue IDE Extension", "zed-editor": "Zed Editor", "vscode-mcp": "VS Code MCP Extension", - "test-mcp-client": "Test MCP Client", } return known_names.get(client_id, client_id.replace("-", " ").title()) - def _add_well_known_clients(self): - """Add well-known MCP clients for testing and development.""" - well_known = [ - MCPClientInfo( - client_id="claude-desktop", - name="Claude Desktop", - redirect_uris=["http://localhost:*", "http://127.0.0.1:*"], - allowed_scopes=["*"], - is_public=True, - metadata={"vendor": "Anthropic"}, - ), - MCPClientInfo( - client_id="test-mcp-client", - name="Test MCP Client", - redirect_uris=["http://localhost:*", "http://127.0.0.1:*"], - allowed_scopes=["*"], - is_public=True, - metadata={"purpose": "testing"}, - ), - ] - - for client in well_known: - self._clients[client.client_id] = client - logger.info(f"Registered well-known client: {client.client_id}") - def validate_client( self, client_id: str, diff --git a/nextcloud_mcp_server/auth/oauth_routes.py b/nextcloud_mcp_server/auth/oauth_routes.py index 1a726a29..1eb39286 100644 --- a/nextcloud_mcp_server/auth/oauth_routes.py +++ b/nextcloud_mcp_server/auth/oauth_routes.py @@ -136,13 +136,18 @@ def _transform_scopes_for_idp(scopes: str, resource_server_id: str) -> str: async def _get_cached_discovery(url: str) -> dict[str, Any]: - """Fetch OIDC discovery document with caching (5-minute TTL).""" + """Fetch OIDC discovery document with caching (5-minute TTL). + + Follows redirects so the configured discovery URL works against Nextcloud + instances without pretty URLs enabled, where ``/.well-known/openid-configuration`` + issues a 301 to ``/index.php/.well-known/openid-configuration``. + """ now = time.time() if url in _discovery_cache: expires_at, data = _discovery_cache[url] if now < expires_at: return data - async with nextcloud_httpx_client() as http_client: + async with nextcloud_httpx_client(follow_redirects=True) as http_client: response = await http_client.get(url) response.raise_for_status() data = response.json() diff --git a/nextcloud_mcp_server/auth/scope_authorization.py b/nextcloud_mcp_server/auth/scope_authorization.py index 1a5e7a15..5317d271 100644 --- a/nextcloud_mcp_server/auth/scope_authorization.py +++ b/nextcloud_mcp_server/auth/scope_authorization.py @@ -209,6 +209,7 @@ def require_scopes(*required_scopes: str): "files.", "tables.", "deck.", + "talk.", ] ) ] @@ -229,6 +230,7 @@ def require_scopes(*required_scopes: str): "files.", "tables.", "deck.", + "talk.", ] ) diff --git a/nextcloud_mcp_server/auth/unified_verifier.py b/nextcloud_mcp_server/auth/unified_verifier.py index fbcc9ff2..5f64f689 100644 --- a/nextcloud_mcp_server/auth/unified_verifier.py +++ b/nextcloud_mcp_server/auth/unified_verifier.py @@ -17,6 +17,7 @@ Key Design Principles: import hashlib import logging +import os import time from typing import Any @@ -97,6 +98,25 @@ class UnifiedTokenVerifier(TokenVerifier): self._token_cache: dict[str, tuple[dict[str, Any], float]] = {} self.cache_ttl = 3600 # 1 hour default + # NOTE: ALLOWED_MCP_CLIENTS and ALLOWED_MGMT_CLIENT are currently separate + # env vars to keep the MCP-route and management-API auth surfaces + # independent. These may be consolidated into a single env var later + # once the deployment story stabilises. + self._allowed_mgmt_clients: frozenset[str] = frozenset( + entry.strip() + for entry in os.getenv("ALLOWED_MGMT_CLIENT", "").split(",") + if entry.strip() + ) + if not self._allowed_mgmt_clients: + logger.warning( + "ALLOWED_MGMT_CLIENT is unset or empty: management API will reject " + "all requests until configured." + ) + else: + logger.info( + f"Management API allowlist: {sorted(self._allowed_mgmt_clients)}" + ) + logger.info( f"UnifiedTokenVerifier initialized in {self.mode} mode. " f"MCP audience: {settings.oidc_client_id} or {settings.nextcloud_mcp_server_url}, " @@ -169,10 +189,11 @@ class UnifiedTokenVerifier(TokenVerifier): token: Bearer token to verify Returns: - AccessToken if valid (regardless of audience), None otherwise + AccessToken if valid AND issued by an allowlisted client, None otherwise """ # Check cache first (using separate cache key to avoid mixing with MCP tokens) cache_key = f"mgmt:{hashlib.sha256(token.encode()).hexdigest()}" + access_token: AccessToken | None = None if cache_key in self._token_cache: userinfo, expiry = self._token_cache[cache_key] if time.time() < expiry: @@ -181,7 +202,7 @@ class UnifiedTokenVerifier(TokenVerifier): username = userinfo.get("sub") or userinfo.get("preferred_username") scope_string = userinfo.get("scope", "") scopes = scope_string.split() if scope_string else [] - return AccessToken( + access_token = AccessToken( token=token, client_id=userinfo.get("client_id", ""), scopes=scopes, @@ -191,10 +212,23 @@ class UnifiedTokenVerifier(TokenVerifier): else: del self._token_cache[cache_key] - oauth_token_cache_hits_total.labels(hit="false").inc() + if access_token is None: + oauth_token_cache_hits_total.labels(hit="false").inc() + access_token = await self._verify_without_audience_check(token, cache_key) - # Verify token without audience check - return await self._verify_without_audience_check(token, cache_key) + if access_token is None: + return None + + # Enforce ALLOWED_MGMT_CLIENT allowlist (fail-closed when unset) + token_client_id = access_token.client_id + if not token_client_id or token_client_id not in self._allowed_mgmt_clients: + logger.warning( + "Management API token rejected: client_id %r not in ALLOWED_MGMT_CLIENT", + token_client_id, + ) + return None + + return access_token async def _verify_mcp_audience(self, token: str) -> AccessToken | None: """ diff --git a/nextcloud_mcp_server/client/__init__.py b/nextcloud_mcp_server/client/__init__.py index a6379e48..bb311af4 100644 --- a/nextcloud_mcp_server/client/__init__.py +++ b/nextcloud_mcp_server/client/__init__.py @@ -23,6 +23,7 @@ from .news import NewsClient from .notes import NotesClient from .sharing import SharingClient from .tables import TablesClient +from .talk import TalkClient from .users import UsersClient from .webdav import WebDAVClient from .webhooks import WebhooksClient @@ -96,6 +97,7 @@ class NextcloudClient: self.collectives = CollectivesClient(self._client, username) self.deck = DeckClient(self._client, username) self.news = NewsClient(self._client, username) + self.talk = TalkClient(self._client, username) self.users = UsersClient(self._client, username) self.groups = GroupsClient(self._client, username) self.sharing = SharingClient(self._client, username) diff --git a/nextcloud_mcp_server/client/talk.py b/nextcloud_mcp_server/client/talk.py new file mode 100644 index 00000000..8673eb88 --- /dev/null +++ b/nextcloud_mcp_server/client/talk.py @@ -0,0 +1,283 @@ +"""HTTP client for the Nextcloud Talk (spreed) app. + +Talk exposes its REST API under ``/ocs/v2.php/apps/spreed/api/{v}/...``. +The current versions used here are: + +- conversations & participants: ``api/v4`` (Nextcloud 22+) +- chat: ``api/v1`` (Nextcloud 13+) + +All endpoints follow the OCS envelope ``{"ocs": {"meta": ..., "data": ...}}``, +require ``OCS-APIRequest: true`` and respond as JSON when ``Accept: +application/json`` is sent. +""" + +import logging +import re +from typing import Any + +from nextcloud_mcp_server.client.base import BaseNextcloudClient +from nextcloud_mcp_server.models.talk import ( + TalkConversation, + TalkMessage, + TalkParticipant, +) + +logger = logging.getLogger(__name__) + + +# Spreed conversation tokens are short alphanumeric strings (e.g. "a1b2c3d4"). +# httpx does not normalise path traversal sequences, so a pathological token +# like ``"../foo"`` would be sent verbatim. Validate up-front for clearer +# errors and defence-in-depth. +_TALK_TOKEN_RE = re.compile(r"^[A-Za-z0-9]+$") + + +def _validate_token(token: str) -> None: + if not _TALK_TOKEN_RE.fullmatch(token): + raise ValueError(f"Invalid Talk conversation token: {token!r}") + + +class TalkClient(BaseNextcloudClient): + """Client for Nextcloud Talk (spreed) app operations.""" + + app_name = "talk" + + _ROOM_BASE = "/ocs/v2.php/apps/spreed/api/v4/room" + _CHAT_BASE = "/ocs/v2.php/apps/spreed/api/v1/chat" + + def _talk_headers(self) -> dict[str, str]: + """Standard OCS+JSON headers for spreed API calls. + + ``Content-Type`` is intentionally omitted — httpx adds it + automatically (and correctly) on requests that pass ``json=``, + so setting it here would also leak it onto bodyless GETs and + DELETEs. + """ + return { + "OCS-APIRequest": "true", + "Accept": "application/json", + } + + # Conversations (rooms) + + async def list_conversations( + self, + *, + modified_since: int | None = None, + include_status: bool = False, + no_status_update: bool = True, + ) -> list[TalkConversation]: + """Return the user's Talk conversations. + + Args: + modified_since: If provided, only return conversations modified + after this Unix timestamp (server-side filter). + include_status: Include user-status info for one-to-one rooms. + no_status_update: When True (default), the call does not bump + the user's "online" status — appropriate for an MCP server + acting in the background. + """ + params: dict[str, Any] = {} + if modified_since is not None: + params["modifiedSince"] = modified_since + if include_status: + params["includeStatus"] = 1 + if no_status_update: + params["noStatusUpdate"] = 1 + response = await self._make_request( + "GET", self._ROOM_BASE, params=params, headers=self._talk_headers() + ) + data = response.json()["ocs"]["data"] + return [TalkConversation(**room) for room in data] + + async def get_conversation(self, token: str) -> TalkConversation: + """Fetch a single Talk conversation by its room token.""" + _validate_token(token) + response = await self._make_request( + "GET", f"{self._ROOM_BASE}/{token}", headers=self._talk_headers() + ) + return TalkConversation(**response.json()["ocs"]["data"]) + + async def create_conversation( + self, + *, + room_type: int = 2, + room_name: str, + invite: str | None = None, + ) -> TalkConversation: + """Create a new conversation (used for tests/fixtures). + + Args: + room_type: 1=one-to-one, 2=group, 3=public. Defaults to 2. + room_name: Display name (required for group/public rooms). + invite: Optional user/group ID to invite at creation time. + + This client method is not exposed as an MCP tool in the initial + Talk integration; it exists so integration tests can spin up + scratch rooms. + """ + body: dict[str, Any] = {"roomType": room_type, "roomName": room_name} + if invite is not None: + body["invite"] = invite + response = await self._make_request( + "POST", self._ROOM_BASE, json=body, headers=self._talk_headers() + ) + return TalkConversation(**response.json()["ocs"]["data"]) + + async def delete_conversation(self, token: str) -> None: + """Delete a conversation. Used by integration test cleanup.""" + _validate_token(token) + await self._make_request( + "DELETE", f"{self._ROOM_BASE}/{token}", headers=self._talk_headers() + ) + + # Chat + + async def get_messages( + self, + token: str, + *, + limit: int = 50, + last_known_message_id: int | None = None, + look_into_future: bool = False, + set_read_marker: bool = False, + include_last_known: bool = False, + ) -> tuple[list[TalkMessage], int | None]: + """Fetch chat messages for a conversation. + + Args: + token: Conversation token. + limit: Max messages to return. spreed caps this server-side + at 200; values outside ``[1, 200]`` are clamped here so + callers don't get a confusing mismatch between the + requested limit and the returned ``count``. + last_known_message_id: Pagination cursor — pass the value + from the previous response's ``X-Chat-Last-Given`` header. + look_into_future: When False (default), return *older* + messages relative to ``last_known_message_id`` — i.e., + read history. When True, this becomes a long-poll for + new messages, which we don't expose via MCP. + set_read_marker: When False (default), the call does not move + the user's read marker — consumers can call + ``mark_as_read`` explicitly. + include_last_known: Include the message identified by + ``last_known_message_id`` itself in the page. + + Returns: + ``(messages, x_chat_last_given)`` where the integer is the + value of the ``X-Chat-Last-Given`` response header (or None + if the header was absent or unparseable), suitable for + pagination. + """ + _validate_token(token) + clamped_limit = min(max(1, limit), 200) + params: dict[str, Any] = { + "limit": clamped_limit, + "lookIntoFuture": 1 if look_into_future else 0, + "setReadMarker": 1 if set_read_marker else 0, + "includeLastKnown": 1 if include_last_known else 0, + } + if last_known_message_id is not None: + params["lastKnownMessageId"] = last_known_message_id + response = await self._make_request( + "GET", + f"{self._CHAT_BASE}/{token}", + params=params, + headers=self._talk_headers(), + ) + # 200 OK → JSON body with messages; 304 Not Modified → no body. + # _make_request's raise_for_status() lets 3xx through for GET, but + # spreed returns 200 with an empty data list when there's nothing + # new, so we trust the JSON body here. + last_given_header = response.headers.get("X-Chat-Last-Given") + last_given: int | None = None + if last_given_header: + try: + last_given = int(last_given_header) + except ValueError: + # Defensive: spreed always sends an int, but a misbehaving + # proxy could mangle the header. Don't crash the read flow. + logger.warning( + "Invalid X-Chat-Last-Given header from spreed: %r", + last_given_header, + ) + data = response.json()["ocs"]["data"] + return [TalkMessage(**msg) for msg in data], last_given + + async def send_message( + self, + token: str, + message: str, + *, + reply_to: int | None = None, + reference_id: str | None = None, + silent: bool = False, + ) -> TalkMessage: + """Post a chat message to a conversation. + + Args: + token: Conversation token. + message: Message text (max 32000 chars per spreed docs/chat.md). + reply_to: Optional parent message ID to thread this reply. + reference_id: Optional client-provided UUID for idempotency on + retry (spreed dedupes on this within the conversation). + silent: When True, the message is delivered without push + notifications. + """ + _validate_token(token) + body: dict[str, Any] = {"message": message} + if reply_to is not None: + body["replyTo"] = reply_to + if reference_id is not None: + body["referenceId"] = reference_id + if silent: + body["silent"] = True + response = await self._make_request( + "POST", + f"{self._CHAT_BASE}/{token}", + json=body, + headers=self._talk_headers(), + ) + return TalkMessage(**response.json()["ocs"]["data"]) + + async def mark_as_read( + self, token: str, *, last_read_message: int | None = None + ) -> None: + """Mark the conversation as read. + + If ``last_read_message`` is provided it sets the read marker to + that message; otherwise spreed marks everything currently in the + room as read. + """ + _validate_token(token) + body: dict[str, Any] = {} + if last_read_message is not None: + body["lastReadMessage"] = last_read_message + # ``json=None`` makes httpx skip both the body and the + # ``Content-Type: application/json`` header — semantically correct + # for the bodyless "mark everything as read" call. + await self._make_request( + "POST", + f"{self._CHAT_BASE}/{token}/read", + json=body or None, + headers=self._talk_headers(), + ) + + # Participants + + async def list_participants( + self, token: str, *, include_status: bool = False + ) -> list[TalkParticipant]: + """List participants of a Talk conversation.""" + _validate_token(token) + params: dict[str, Any] = {} + if include_status: + params["includeStatus"] = 1 + response = await self._make_request( + "GET", + f"{self._ROOM_BASE}/{token}/participants", + params=params, + headers=self._talk_headers(), + ) + data = response.json()["ocs"]["data"] + return [TalkParticipant(**p) for p in data] diff --git a/nextcloud_mcp_server/models/talk.py b/nextcloud_mcp_server/models/talk.py new file mode 100644 index 00000000..9f5bf2f6 --- /dev/null +++ b/nextcloud_mcp_server/models/talk.py @@ -0,0 +1,171 @@ +"""Pydantic models for the Nextcloud Talk (spreed) integration.""" + +from typing import Any + +from pydantic import BaseModel, ConfigDict, Field, field_validator + +from .base import BaseResponse, StatusResponse + +# Domain models + + +class TalkMessage(BaseModel): + """A single chat message in a Talk conversation. + + See spreed docs/chat.md for the field definitions. We map only the + fields that are useful to MCP consumers; spreed returns more. + """ + + id: int + token: str + actorType: str + actorId: str + actorDisplayName: str + timestamp: int + systemMessage: str = "" + messageType: str + message: str + messageParameters: dict[str, Any] = Field(default_factory=dict) + expirationTimestamp: int | None = None + referenceId: str | None = None + markdown: bool | None = None + + @field_validator("messageParameters", mode="before") + @classmethod + def _coerce_empty_list_params(cls, v: Any) -> Any: + # spreed serializes an empty parameter map as `[]` (PHP array) rather + # than `{}`; normalize so pydantic accepts it as a dict. + if isinstance(v, list) and not v: + return {} + return v + + +class TalkConversation(BaseModel): + """A Talk conversation (room). + + See spreed docs/conversation.md for the full field reference. Many + optional fields are omitted; we keep the ones useful for chat-centric + flows. + """ + + # ``populate_by_name=True`` lets us deserialize spreed's ``type`` key + # into the ``room_type`` field while still allowing internal callers + # to construct the model with ``room_type=...`` directly. + model_config = ConfigDict(populate_by_name=True) + + id: int + token: str + # The spreed JSON wire format uses ``type`` for the room kind, but + # ``type`` shadows Python's builtin within the class scope, which + # would silently call this int field if anyone wrote ``type(...)`` + # in a validator or method on this model. Map to ``room_type`` and + # alias the wire field instead. + room_type: int = Field( + alias="type", + description=( + "Conversation kind: 1=one-to-one, 2=group, 3=public, " + "4=changelog, 5=former one-to-one, 6=note-to-self." + ), + ) + name: str + displayName: str + description: str = "" + participantType: int | None = None + unreadMessages: int = 0 + unreadMention: bool = False + lastActivity: int | None = None + # spreed returns `0` (not `null`) when the user has not read any + # message in the room yet — compare to ``None`` explicitly rather + # than relying on truthiness, since `0` is falsy but valid. + lastReadMessage: int | None = None + lastMessage: TalkMessage | None = None + readOnly: int | None = None + isFavorite: bool | None = None + notificationLevel: int | None = None + objectType: str | None = None + objectId: str | None = None + + @field_validator("lastMessage", mode="before") + @classmethod + def _coerce_empty_last_message(cls, v: Any) -> Any: + # spreed returns `lastMessage: []` (PHP empty array) when there has + # never been a message in the room; normalize to None. + if isinstance(v, list) and not v: + return None + return v + + +class TalkParticipant(BaseModel): + """A participant (attendee) in a Talk conversation.""" + + attendeeId: int + actorType: str + actorId: str + displayName: str + participantType: int + inCall: int = 0 + lastPing: int = 0 + sessionIds: list[str] = Field(default_factory=list) + status: str | None = None + statusIcon: str | None = None + statusMessage: str | None = None + + +# Response wrappers for MCP tools + + +class ListConversationsResponse(BaseResponse): + """Response model for listing Talk conversations.""" + + results: list[TalkConversation] = Field( + description="Talk conversations the user participates in" + ) + total: int = Field(description="Number of conversations returned") + + +class GetConversationResponse(BaseResponse): + """Response model for fetching a single Talk conversation.""" + + conversation: TalkConversation = Field(description="The Talk conversation") + + +class ListMessagesResponse(BaseResponse): + """Response model for fetching chat history of a conversation.""" + + conversation_token: str = Field(description="Token of the conversation") + results: list[TalkMessage] = Field(description="Chat messages in this page") + count: int = Field(description="Number of messages returned in this page") + last_known_message_id: int | None = Field( + default=None, + description=( + "ID to pass back as `last_known_message_id` to fetch the next " + "page (older history). Sourced from the `X-Chat-Last-Given` " + "response header." + ), + ) + + +class ListParticipantsResponse(BaseResponse): + """Response model for listing participants in a Talk conversation.""" + + conversation_token: str = Field(description="Token of the conversation") + results: list[TalkParticipant] = Field( + description="Participants of the conversation" + ) + count: int = Field(description="Number of participants returned") + + +class SendMessageResponse(BaseResponse): + """Response model returned after posting a chat message.""" + + message: TalkMessage = Field(description="The posted chat message") + + +class MarkAsReadResponse(StatusResponse): + """Response model for the mark-as-read operation.""" + + conversation_token: str = Field(description="Token of the conversation") + last_read_message: int | None = Field( + default=None, + description="The message ID that was marked as the last-read marker", + ) diff --git a/nextcloud_mcp_server/server/__init__.py b/nextcloud_mcp_server/server/__init__.py index 45fab885..e2f4616b 100644 --- a/nextcloud_mcp_server/server/__init__.py +++ b/nextcloud_mcp_server/server/__init__.py @@ -12,6 +12,7 @@ from .notes import configure_notes_tools from .semantic import configure_semantic_tools from .sharing import configure_sharing_tools from .tables import configure_tables_tools +from .talk import configure_talk_tools from .webdav import configure_webdav_tools # Canonical mapping of app name → tool registration function. @@ -29,6 +30,7 @@ AVAILABLE_APPS: dict[str, Callable[[FastMCP], None]] = { "cookbook": configure_cookbook_tools, "deck": configure_deck_tools, "news": configure_news_tools, + "talk": configure_talk_tools, } __all__ = [ @@ -43,5 +45,6 @@ __all__ = [ "configure_semantic_tools", "configure_sharing_tools", "configure_tables_tools", + "configure_talk_tools", "configure_webdav_tools", ] diff --git a/nextcloud_mcp_server/server/talk.py b/nextcloud_mcp_server/server/talk.py new file mode 100644 index 00000000..52cf787d --- /dev/null +++ b/nextcloud_mcp_server/server/talk.py @@ -0,0 +1,225 @@ +"""MCP tool registration for the Nextcloud Talk (spreed) integration.""" + +import logging +import uuid + +from mcp.server.fastmcp import Context, FastMCP +from mcp.types import ToolAnnotations + +from nextcloud_mcp_server.auth import require_scopes +from nextcloud_mcp_server.context import get_client +from nextcloud_mcp_server.models.talk import ( + GetConversationResponse, + ListConversationsResponse, + ListMessagesResponse, + ListParticipantsResponse, + MarkAsReadResponse, + SendMessageResponse, +) +from nextcloud_mcp_server.observability.metrics import instrument_tool + +logger = logging.getLogger(__name__) + + +# spreed advertises a 32000-character limit on chat messages (docs/chat.md); +# we enforce it client-side for a clearer error than the server's 413. +_MESSAGE_MAX_LENGTH = 32000 + + +def _validate_message_text(message: str) -> None: + # Reject both empty strings and whitespace-only strings — spreed + # would happily post the latter as a visually-blank message. + if not message or not message.strip(): + raise ValueError("Message text must not be empty or whitespace-only") + if len(message) > _MESSAGE_MAX_LENGTH: + raise ValueError( + f"Message too long: {len(message)} characters (max {_MESSAGE_MAX_LENGTH})" + ) + + +def configure_talk_tools(mcp: FastMCP) -> None: + """Configure Nextcloud Talk (spreed) MCP tools.""" + + # Read tools + + @mcp.tool( + title="List Talk Conversations", + annotations=ToolAnnotations(readOnlyHint=True, openWorldHint=True), + ) + @require_scopes("talk.read") + @instrument_tool + async def talk_list_conversations( + ctx: Context, + modified_since: int | None = None, + include_status: bool = False, + ) -> ListConversationsResponse: + """List the user's Talk conversations (rooms). + + Args: + modified_since: Optional Unix timestamp; only conversations + modified after this time are returned. + include_status: Whether to include user-status info for + one-to-one conversations. + """ + client = await get_client(ctx) + rooms = await client.talk.list_conversations( + modified_since=modified_since, + include_status=include_status, + ) + return ListConversationsResponse(results=rooms, total=len(rooms)) + + @mcp.tool( + title="Get Talk Conversation", + annotations=ToolAnnotations(readOnlyHint=True, openWorldHint=True), + ) + @require_scopes("talk.read") + @instrument_tool + async def talk_get_conversation( + ctx: Context, token: str + ) -> GetConversationResponse: + """Get details of a Talk conversation by its token. + + Args: + token: Unique room token (returned by ``talk_list_conversations``). + """ + client = await get_client(ctx) + conversation = await client.talk.get_conversation(token) + return GetConversationResponse(conversation=conversation) + + @mcp.tool( + title="Get Talk Messages", + annotations=ToolAnnotations(readOnlyHint=True, openWorldHint=True), + ) + @require_scopes("talk.read") + @instrument_tool + async def talk_get_messages( + ctx: Context, + token: str, + limit: int = 50, + last_known_message_id: int | None = None, + include_last_known: bool = False, + ) -> ListMessagesResponse: + """Read chat history for a Talk conversation. + + Returns the most recent messages (older first when paginated). + Does not move the user's read marker; call + ``talk_mark_as_read`` separately if desired. + + Args: + token: Conversation token. + limit: Max messages per page. Valid range is 1-200 (spreed + caps server-side at 200); values outside this range are + clamped. Default 50. + last_known_message_id: Pagination cursor — pass the + ``last_known_message_id`` from the previous response to + fetch the next (older) page. + include_last_known: Include the cursor message in the page + instead of starting just before it. + """ + client = await get_client(ctx) + messages, last_given = await client.talk.get_messages( + token, + limit=limit, + last_known_message_id=last_known_message_id, + look_into_future=False, + set_read_marker=False, + include_last_known=include_last_known, + ) + return ListMessagesResponse( + conversation_token=token, + results=messages, + count=len(messages), + last_known_message_id=last_given, + ) + + @mcp.tool( + title="List Talk Conversation Participants", + annotations=ToolAnnotations(readOnlyHint=True, openWorldHint=True), + ) + @require_scopes("talk.read") + @instrument_tool + async def talk_list_participants( + ctx: Context, token: str, include_status: bool = False + ) -> ListParticipantsResponse: + """List the participants of a Talk conversation. + + Args: + token: Conversation token. + include_status: Include each participant's user-status info. + """ + client = await get_client(ctx) + participants = await client.talk.list_participants( + token, include_status=include_status + ) + return ListParticipantsResponse( + conversation_token=token, + results=participants, + count=len(participants), + ) + + # Write tools + + @mcp.tool( + title="Send Talk Message", + annotations=ToolAnnotations(idempotentHint=False, openWorldHint=True), + ) + @require_scopes("talk.write") + @instrument_tool + async def talk_send_message( + ctx: Context, + token: str, + message: str, + reply_to: int | None = None, + silent: bool = False, + ) -> SendMessageResponse: + """Post a chat message into a Talk conversation as the user. + + A random ``referenceId`` is attached so spreed dedupes the post + if the request is retried. + + Args: + token: Conversation token. + message: Message text (max 32000 characters). + reply_to: Optional parent message ID to thread the reply. + silent: When True the message is delivered without push + notifications (e.g. for status updates). + """ + _validate_message_text(message) + client = await get_client(ctx) + posted = await client.talk.send_message( + token, + message, + reply_to=reply_to, + # 32 hex chars, no dashes — spreed accepts either UUID format. + reference_id=uuid.uuid4().hex, + silent=silent, + ) + return SendMessageResponse(message=posted) + + @mcp.tool( + title="Mark Talk Conversation as Read", + annotations=ToolAnnotations(idempotentHint=True, openWorldHint=True), + ) + @require_scopes("talk.write") + @instrument_tool + async def talk_mark_as_read( + ctx: Context, + token: str, + last_read_message: int | None = None, + ) -> MarkAsReadResponse: + """Move the user's read marker forward in a Talk conversation. + + Args: + token: Conversation token. + last_read_message: Optional message ID to mark as the new + read position. When omitted, spreed marks everything + currently in the room as read. + """ + client = await get_client(ctx) + await client.talk.mark_as_read(token, last_read_message=last_read_message) + return MarkAsReadResponse( + success=True, + message="Conversation marked as read", + conversation_token=token, + last_read_message=last_read_message, + ) diff --git a/pyproject.toml b/pyproject.toml index 76bdd295..1f90d88e 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -1,6 +1,6 @@ [project] name = "nextcloud-mcp-server" -version = "0.73.1" +version = "0.75.0" description = "Model Context Protocol (MCP) server for Nextcloud integration - enables AI assistants to interact with Nextcloud data" authors = [ {name = "Chris Coutinho", email = "chris@coutinho.io"} diff --git a/tests/client/conftest.py b/tests/client/conftest.py index 4fc8cc7e..c4649b4f 100644 --- a/tests/client/conftest.py +++ b/tests/client/conftest.py @@ -358,6 +358,85 @@ def create_mock_deck_comment_response( return create_mock_response(status_code=200, json_data=ocs_response) +def create_mock_talk_room_response( + conversation_id: int = 1, + token: str = "abc123", + name: str = "Test Conversation", + room_type: int = 2, + **kwargs, +) -> httpx.Response: + """Create a mock OCS response for a Nextcloud Talk conversation. + + Args: + conversation_id: Numeric conversation ID + token: Unique room token + name: Conversation name + room_type: 1=one-to-one, 2=group, 3=public, 4=changelog + **kwargs: Additional conversation fields (override defaults) + + Returns: + Mock httpx.Response with conversation data wrapped in OCS format. + """ + room_data = { + "id": conversation_id, + "token": token, + "type": room_type, + "name": name, + "displayName": name, + "description": "", + "participantType": 1, + "unreadMessages": 0, + "unreadMention": False, + "lastActivity": 1700000000, + "lastReadMessage": 0, + "lastMessage": [], + "readOnly": 0, + "isFavorite": False, + "notificationLevel": 0, + **kwargs, + } + + ocs_response = {"ocs": {"meta": {"status": "ok"}, "data": room_data}} + return create_mock_response(status_code=200, json_data=ocs_response) + + +def create_mock_talk_message_response( + message_id: int = 1, + token: str = "abc123", + text: str = "Hello, world!", + actor_id: str = "testuser", + **kwargs, +) -> httpx.Response: + """Create a mock OCS response for a Nextcloud Talk chat message. + + Args: + message_id: Numeric message ID + token: Conversation token the message belongs to + text: Message body + actor_id: User ID of the message author + **kwargs: Additional message fields (override defaults) + + Returns: + Mock httpx.Response with message data wrapped in OCS format. + """ + message_data = { + "id": message_id, + "token": token, + "actorType": "users", + "actorId": actor_id, + "actorDisplayName": "Test User", + "timestamp": 1700000000, + "systemMessage": "", + "messageType": "comment", + "message": text, + "messageParameters": {}, + **kwargs, + } + + ocs_response = {"ocs": {"meta": {"status": "ok"}, "data": message_data}} + return create_mock_response(status_code=200, json_data=ocs_response) + + def create_mock_tables_list_response( tables: list[dict] = None, ) -> httpx.Response: diff --git a/tests/client/talk/__init__.py b/tests/client/talk/__init__.py new file mode 100644 index 00000000..e69de29b diff --git a/tests/client/talk/test_talk_api.py b/tests/client/talk/test_talk_api.py new file mode 100644 index 00000000..277bbbb3 --- /dev/null +++ b/tests/client/talk/test_talk_api.py @@ -0,0 +1,465 @@ +"""Unit tests for the Nextcloud Talk (spreed) HTTP client.""" + +import logging + +import httpx +import pytest + +from nextcloud_mcp_server.client.talk import TalkClient, _validate_token +from nextcloud_mcp_server.models.talk import ( + TalkConversation, + TalkMessage, + TalkParticipant, +) +from tests.client.conftest import ( + create_mock_error_response, + create_mock_response, + create_mock_talk_message_response, + create_mock_talk_room_response, +) + +logger = logging.getLogger(__name__) +pytestmark = pytest.mark.unit + + +# Token validation + + +@pytest.mark.parametrize( + "bad_token", + [ + "", + "../foo", + "a/b", + "a b", + "a.b", + "a-b", + "token!", + ], +) +def test_validate_token_rejects_invalid(bad_token): + """_validate_token rejects anything outside the alphanumeric whitelist.""" + with pytest.raises(ValueError, match="Invalid Talk conversation token"): + _validate_token(bad_token) + + +@pytest.mark.parametrize("good_token", ["a1b2c3d4", "ABC123", "abcdef", "1"]) +def test_validate_token_accepts_valid(good_token): + """Real spreed tokens — short alphanumeric strings — pass through.""" + _validate_token(good_token) + + +async def test_talk_get_conversation_rejects_path_traversal(mocker): + """Pathological tokens never reach the HTTP layer.""" + mock_make_request = mocker.patch.object(TalkClient, "_make_request") + + client = TalkClient(mocker.AsyncMock(spec=httpx.AsyncClient), "testuser") + with pytest.raises(ValueError, match="Invalid Talk conversation token"): + await client.get_conversation("../etc/passwd") + + mock_make_request.assert_not_called() + + +# Conversation tests + + +async def test_talk_list_conversations(mocker): + """list_conversations parses the OCS-wrapped room list.""" + mock_response = create_mock_response( + status_code=200, + json_data={ + "ocs": { + "meta": {"status": "ok"}, + "data": [ + { + "id": 1, + "token": "abc", + "type": 2, + "name": "Group 1", + "displayName": "Group 1", + "lastMessage": [], + }, + { + "id": 2, + "token": "def", + "type": 3, + "name": "Public Room", + "displayName": "Public Room", + "lastMessage": [], + }, + ], + } + }, + ) + + mock_make_request = mocker.patch.object( + TalkClient, "_make_request", return_value=mock_response + ) + + client = TalkClient(mocker.AsyncMock(spec=httpx.AsyncClient), "testuser") + rooms = await client.list_conversations() + + assert len(rooms) == 2 + assert all(isinstance(r, TalkConversation) for r in rooms) + assert rooms[0].token == "abc" + assert rooms[1].room_type == 3 + + mock_make_request.assert_called_once() + call_args = mock_make_request.call_args + assert call_args[0][0] == "GET" + assert call_args[0][1] == "/ocs/v2.php/apps/spreed/api/v4/room" + # noStatusUpdate defaults to True + assert call_args[1]["params"]["noStatusUpdate"] == 1 + + +async def test_talk_list_conversations_with_modified_since(mocker): + """modifiedSince and includeStatus are forwarded as query params.""" + mock_response = create_mock_response( + status_code=200, + json_data={"ocs": {"meta": {"status": "ok"}, "data": []}}, + ) + mock_make_request = mocker.patch.object( + TalkClient, "_make_request", return_value=mock_response + ) + + client = TalkClient(mocker.AsyncMock(spec=httpx.AsyncClient), "testuser") + await client.list_conversations(modified_since=1700000000, include_status=True) + + params = mock_make_request.call_args[1]["params"] + assert params["modifiedSince"] == 1700000000 + assert params["includeStatus"] == 1 + + +async def test_talk_get_conversation(mocker): + """get_conversation parses the OCS-wrapped room object.""" + mock_response = create_mock_talk_room_response( + conversation_id=42, token="xyz", name="My Room" + ) + mock_make_request = mocker.patch.object( + TalkClient, "_make_request", return_value=mock_response + ) + + client = TalkClient(mocker.AsyncMock(spec=httpx.AsyncClient), "testuser") + room = await client.get_conversation("xyz") + + assert isinstance(room, TalkConversation) + assert room.id == 42 + assert room.token == "xyz" + assert room.name == "My Room" + + assert mock_make_request.call_args[0][0] == "GET" + assert "/api/v4/room/xyz" in mock_make_request.call_args[0][1] + + +async def test_talk_get_conversation_not_found(mocker): + """A 404 from spreed propagates as HTTPStatusError.""" + error_response = create_mock_error_response(404, "Conversation not found") + mock_make_request = mocker.patch.object(TalkClient, "_make_request") + mock_make_request.side_effect = httpx.HTTPStatusError( + "404 Not Found", + request=httpx.Request("GET", "http://test.local"), + response=error_response, + ) + + client = TalkClient(mocker.AsyncMock(spec=httpx.AsyncClient), "testuser") + with pytest.raises(httpx.HTTPStatusError) as excinfo: + await client.get_conversation("nope") + assert excinfo.value.response.status_code == 404 + + +async def test_talk_create_conversation(mocker): + """create_conversation builds the right POST body.""" + mock_response = create_mock_talk_room_response( + conversation_id=10, token="new", name="New Room" + ) + mock_make_request = mocker.patch.object( + TalkClient, "_make_request", return_value=mock_response + ) + + client = TalkClient(mocker.AsyncMock(spec=httpx.AsyncClient), "testuser") + room = await client.create_conversation(room_name="New Room") + + assert room.token == "new" + call_args = mock_make_request.call_args + assert call_args[0][0] == "POST" + assert call_args[1]["json"] == {"roomType": 2, "roomName": "New Room"} + + +async def test_talk_delete_conversation(mocker): + """delete_conversation issues DELETE on the room URL.""" + mock_response = create_mock_response( + status_code=200, + json_data={"ocs": {"meta": {"status": "ok"}, "data": []}}, + ) + mock_make_request = mocker.patch.object( + TalkClient, "_make_request", return_value=mock_response + ) + + client = TalkClient(mocker.AsyncMock(spec=httpx.AsyncClient), "testuser") + await client.delete_conversation("doomed") + + call_args = mock_make_request.call_args + assert call_args[0][0] == "DELETE" + assert "/api/v4/room/doomed" in call_args[0][1] + + +# Chat tests + + +async def test_talk_get_messages(mocker): + """get_messages parses the OCS-wrapped message list.""" + mock_response = create_mock_response( + status_code=200, + headers={"X-Chat-Last-Given": "100"}, + json_data={ + "ocs": { + "meta": {"status": "ok"}, + "data": [ + { + "id": 101, + "token": "abc", + "actorType": "users", + "actorId": "alice", + "actorDisplayName": "Alice", + "timestamp": 1700000000, + "messageType": "comment", + "message": "Hi", + }, + { + "id": 102, + "token": "abc", + "actorType": "users", + "actorId": "bob", + "actorDisplayName": "Bob", + "timestamp": 1700000010, + "messageType": "comment", + "message": "Hello", + }, + ], + } + }, + ) + mock_make_request = mocker.patch.object( + TalkClient, "_make_request", return_value=mock_response + ) + + client = TalkClient(mocker.AsyncMock(spec=httpx.AsyncClient), "testuser") + messages, last_given = await client.get_messages("abc", limit=20) + + assert len(messages) == 2 + assert all(isinstance(m, TalkMessage) for m in messages) + assert messages[0].id == 101 + assert messages[1].actorId == "bob" + assert last_given == 100 + + call_args = mock_make_request.call_args + assert call_args[0][0] == "GET" + assert "/chat/abc" in call_args[0][1] + params = call_args[1]["params"] + assert params["limit"] == 20 + # Defaults: not look_into_future, not set_read_marker + assert params["lookIntoFuture"] == 0 + assert params["setReadMarker"] == 0 + + +async def test_talk_get_messages_pagination_cursor(mocker): + """last_known_message_id is forwarded as the spreed cursor param.""" + mock_response = create_mock_response( + status_code=200, + json_data={"ocs": {"meta": {"status": "ok"}, "data": []}}, + ) + mock_make_request = mocker.patch.object( + TalkClient, "_make_request", return_value=mock_response + ) + + client = TalkClient(mocker.AsyncMock(spec=httpx.AsyncClient), "testuser") + messages, last_given = await client.get_messages( + "abc", last_known_message_id=500, include_last_known=True + ) + + assert messages == [] + assert last_given is None # No header on this response + + params = mock_make_request.call_args[1]["params"] + assert params["lastKnownMessageId"] == 500 + assert params["includeLastKnown"] == 1 + + +async def test_talk_get_messages_invalid_last_given_header(mocker, caplog): + """A non-numeric X-Chat-Last-Given falls back to None and logs a warning.""" + mock_response = create_mock_response( + status_code=200, + headers={"X-Chat-Last-Given": "not-a-number"}, + json_data={"ocs": {"meta": {"status": "ok"}, "data": []}}, + ) + mocker.patch.object(TalkClient, "_make_request", return_value=mock_response) + + client = TalkClient(mocker.AsyncMock(spec=httpx.AsyncClient), "testuser") + with caplog.at_level(logging.WARNING, logger="nextcloud_mcp_server.client.talk"): + messages, last_given = await client.get_messages("abc") + + assert messages == [] + assert last_given is None + assert any( + "Invalid X-Chat-Last-Given" in record.message for record in caplog.records + ), "Expected a warning log for the malformed header" + + +async def test_talk_send_message(mocker): + """send_message posts the message text and parses the response.""" + mock_response = create_mock_talk_message_response( + message_id=999, token="abc", text="Posted from MCP" + ) + mock_make_request = mocker.patch.object( + TalkClient, "_make_request", return_value=mock_response + ) + + client = TalkClient(mocker.AsyncMock(spec=httpx.AsyncClient), "testuser") + posted = await client.send_message("abc", "Posted from MCP") + + assert isinstance(posted, TalkMessage) + assert posted.id == 999 + assert posted.message == "Posted from MCP" + + call_args = mock_make_request.call_args + assert call_args[0][0] == "POST" + assert "/chat/abc" in call_args[0][1] + body = call_args[1]["json"] + assert body["message"] == "Posted from MCP" + # Optional fields are not sent unless explicitly set + assert "replyTo" not in body + assert "referenceId" not in body + assert "silent" not in body + + +async def test_talk_send_message_with_reference_id_and_reply(mocker): + """send_message forwards reply_to, reference_id, silent.""" + mock_response = create_mock_talk_message_response( + message_id=1000, token="abc", text="A reply" + ) + mock_make_request = mocker.patch.object( + TalkClient, "_make_request", return_value=mock_response + ) + + client = TalkClient(mocker.AsyncMock(spec=httpx.AsyncClient), "testuser") + await client.send_message( + "abc", + "A reply", + reply_to=999, + reference_id="dedupe-token", + silent=True, + ) + + body = mock_make_request.call_args[1]["json"] + assert body == { + "message": "A reply", + "replyTo": 999, + "referenceId": "dedupe-token", + "silent": True, + } + + +async def test_talk_mark_as_read_no_message(mocker): + """mark_as_read with no message ID sends no body (json=None).""" + mock_response = create_mock_response( + status_code=200, + json_data={"ocs": {"meta": {"status": "ok"}, "data": []}}, + ) + mock_make_request = mocker.patch.object( + TalkClient, "_make_request", return_value=mock_response + ) + + client = TalkClient(mocker.AsyncMock(spec=httpx.AsyncClient), "testuser") + await client.mark_as_read("abc") + + call_args = mock_make_request.call_args + assert call_args[0][0] == "POST" + assert "/chat/abc/read" in call_args[0][1] + # Empty body is sent as ``json=None`` so httpx skips both the body and + # the ``Content-Type: application/json`` header for this bodyless POST. + assert call_args[1]["json"] is None + + +async def test_talk_mark_as_read_with_message(mocker): + """mark_as_read sends lastReadMessage when provided.""" + mock_response = create_mock_response( + status_code=200, + json_data={"ocs": {"meta": {"status": "ok"}, "data": []}}, + ) + mock_make_request = mocker.patch.object( + TalkClient, "_make_request", return_value=mock_response + ) + + client = TalkClient(mocker.AsyncMock(spec=httpx.AsyncClient), "testuser") + await client.mark_as_read("abc", last_read_message=500) + + assert mock_make_request.call_args[1]["json"] == {"lastReadMessage": 500} + + +# Participant tests + + +async def test_talk_list_participants(mocker): + """list_participants parses the OCS-wrapped participant list.""" + mock_response = create_mock_response( + status_code=200, + json_data={ + "ocs": { + "meta": {"status": "ok"}, + "data": [ + { + "attendeeId": 1, + "actorType": "users", + "actorId": "alice", + "displayName": "Alice", + "participantType": 1, + "inCall": 0, + "lastPing": 1700000000, + }, + { + "attendeeId": 2, + "actorType": "users", + "actorId": "bob", + "displayName": "Bob", + "participantType": 3, + "inCall": 0, + "lastPing": 1700000000, + }, + ], + } + }, + ) + mock_make_request = mocker.patch.object( + TalkClient, "_make_request", return_value=mock_response + ) + + client = TalkClient(mocker.AsyncMock(spec=httpx.AsyncClient), "testuser") + participants = await client.list_participants("abc") + + assert len(participants) == 2 + assert all(isinstance(p, TalkParticipant) for p in participants) + assert participants[0].actorId == "alice" + assert participants[1].participantType == 3 + + call_args = mock_make_request.call_args + assert call_args[0][0] == "GET" + assert "/api/v4/room/abc/participants" in call_args[0][1] + # Default: include_status off → no includeStatus param + assert "includeStatus" not in call_args[1].get("params", {}) + + +async def test_talk_list_participants_with_include_status(mocker): + """include_status=True forwards includeStatus=1 as a query param.""" + mock_response = create_mock_response( + status_code=200, + json_data={"ocs": {"meta": {"status": "ok"}, "data": []}}, + ) + mock_make_request = mocker.patch.object( + TalkClient, "_make_request", return_value=mock_response + ) + + client = TalkClient(mocker.AsyncMock(spec=httpx.AsyncClient), "testuser") + await client.list_participants("abc", include_status=True) + + params = mock_make_request.call_args[1]["params"] + assert params["includeStatus"] == 1 diff --git a/tests/conftest.py b/tests/conftest.py index a7a3ccfc..e8aade3f 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -38,7 +38,8 @@ DEFAULT_FULL_SCOPES = ( "deck.read deck.write " "tables.read tables.write " "files.read files.write " - "sharing.read sharing.write" + "sharing.read sharing.write " + "talk.read talk.write" ) # Read-only scopes (all read scopes across apps) - should match DEFAULT_FULL_SCOPES read portion @@ -52,7 +53,8 @@ DEFAULT_READ_SCOPES = ( "deck.read " "tables.read " "files.read " - "sharing.read" + "sharing.read " + "talk.read" ) # Write-only scopes (all write scopes across apps) - should match DEFAULT_FULL_SCOPES write portion @@ -66,7 +68,8 @@ DEFAULT_WRITE_SCOPES = ( "deck.write " "tables.write " "files.write " - "sharing.write" + "sharing.write " + "talk.write" ) @@ -916,6 +919,43 @@ async def temporary_board_with_card( logger.error(f"Unexpected error deleting temporary card {card.id}: {e}") +@pytest.fixture +async def temporary_conversation(nc_client: NextcloudClient): + """Create a temporary Talk conversation and clean it up after the test. + + Yields a dict with the room ``token``, ``id``, and ``name``. + """ + unique_suffix = uuid.uuid4().hex[:8] + room_name = f"MCP Test Room {unique_suffix}" + token = None + + logger.info(f"Creating temporary Talk conversation: {room_name}") + try: + room = await nc_client.talk.create_conversation(room_name=room_name) + token = room.token + logger.info(f"Temporary Talk conversation created (token={token})") + yield {"token": token, "id": room.id, "name": room_name} + + finally: + if token: + logger.info(f"Cleaning up temporary Talk conversation token={token}") + try: + await nc_client.talk.delete_conversation(token) + logger.info(f"Successfully deleted Talk conversation token={token}") + except HTTPStatusError as e: + if e.response.status_code not in [404, 403]: + logger.error(f"HTTP error deleting Talk conversation {token}: {e}") + else: + logger.warning( + f"Talk conversation {token} already deleted or access denied " + f"({e.response.status_code})." + ) + except Exception as e: + logger.error( + f"Unexpected error deleting Talk conversation {token}: {e}" + ) + + @pytest.fixture(scope="session") def shared_test_calendar_name(): """Unique calendar name for the entire test session.""" diff --git a/tests/server/test_talk_mcp.py b/tests/server/test_talk_mcp.py new file mode 100644 index 00000000..2eb52757 --- /dev/null +++ b/tests/server/test_talk_mcp.py @@ -0,0 +1,163 @@ +"""Integration tests for the Nextcloud Talk (spreed) MCP tools.""" + +import json +import logging + +import pytest +from mcp import ClientSession + +from nextcloud_mcp_server.client import NextcloudClient + +logger = logging.getLogger(__name__) +pytestmark = pytest.mark.integration + + +EXPECTED_TALK_TOOLS = { + "talk_list_conversations", + "talk_get_conversation", + "talk_get_messages", + "talk_list_participants", + "talk_send_message", + "talk_mark_as_read", +} + + +async def test_talk_mcp_connectivity(nc_mcp_client: ClientSession): + """All six Talk tools should be registered with the MCP server.""" + tools = await nc_mcp_client.list_tools() + tool_names = {tool.name for tool in tools.tools} + + missing = EXPECTED_TALK_TOOLS - tool_names + assert not missing, f"Missing Talk tools: {missing}" + + +async def test_talk_send_and_read_workflow( + nc_mcp_client: ClientSession, + nc_client: NextcloudClient, + temporary_conversation: dict, +): + """End-to-end: post a message via MCP, read it back, mark read.""" + token = temporary_conversation["token"] + + # 1. Send a message via MCP + send_result = await nc_mcp_client.call_tool( + "talk_send_message", + {"token": token, "message": "Hello from MCP integration test"}, + ) + assert send_result.isError is False, ( + f"talk_send_message failed: {send_result.content}" + ) + send_payload = json.loads(send_result.content[0].text) + assert send_payload["success"] is True + posted = send_payload["message"] + assert posted["message"] == "Hello from MCP integration test" + assert posted["token"] == token + posted_id = posted["id"] + logger.info(f"Posted message id={posted_id} into token={token}") + + # 2. Cross-check via direct client + direct_messages, _ = await nc_client.talk.get_messages(token, limit=10) + direct_ids = [m.id for m in direct_messages] + assert posted_id in direct_ids, "Posted message not visible via direct client" + + # 3. Read messages via MCP + get_result = await nc_mcp_client.call_tool( + "talk_get_messages", {"token": token, "limit": 10} + ) + assert get_result.isError is False, ( + f"talk_get_messages failed: {get_result.content}" + ) + get_payload = json.loads(get_result.content[0].text) + assert get_payload["conversation_token"] == token + listed_ids = [m["id"] for m in get_payload["results"]] + assert posted_id in listed_ids, "Posted message not in MCP get_messages results" + + # 4. Mark conversation as read up to that message + mark_result = await nc_mcp_client.call_tool( + "talk_mark_as_read", + {"token": token, "last_read_message": posted_id}, + ) + assert mark_result.isError is False, ( + f"talk_mark_as_read failed: {mark_result.content}" + ) + mark_payload = json.loads(mark_result.content[0].text) + assert mark_payload["success"] is True + assert mark_payload["conversation_token"] == token + assert mark_payload["last_read_message"] == posted_id + + +async def test_talk_list_conversations_includes_temp_room( + nc_mcp_client: ClientSession, temporary_conversation: dict +): + """Newly created conversation should appear in talk_list_conversations.""" + token = temporary_conversation["token"] + + list_result = await nc_mcp_client.call_tool("talk_list_conversations", {}) + assert list_result.isError is False, ( + f"talk_list_conversations failed: {list_result.content}" + ) + payload = json.loads(list_result.content[0].text) + tokens = [r["token"] for r in payload["results"]] + assert token in tokens, "Temporary conversation not found in list" + + +async def test_talk_get_conversation( + nc_mcp_client: ClientSession, temporary_conversation: dict +): + """talk_get_conversation returns the same room we created.""" + token = temporary_conversation["token"] + name = temporary_conversation["name"] + + result = await nc_mcp_client.call_tool("talk_get_conversation", {"token": token}) + assert result.isError is False, f"talk_get_conversation failed: {result.content}" + payload = json.loads(result.content[0].text) + conversation = payload["conversation"] + assert conversation["token"] == token + assert conversation["name"] == name + + +async def test_talk_list_participants( + nc_mcp_client: ClientSession, temporary_conversation: dict +): + """talk_list_participants returns the room creator as a participant.""" + token = temporary_conversation["token"] + + result = await nc_mcp_client.call_tool("talk_list_participants", {"token": token}) + assert result.isError is False, f"talk_list_participants failed: {result.content}" + payload = json.loads(result.content[0].text) + assert payload["conversation_token"] == token + actor_ids = [p["actorId"] for p in payload["results"]] + # The user that created the room is always a participant. + assert len(actor_ids) >= 1 + + +@pytest.mark.parametrize("blank_text", ["", " ", "\t\n", " \t \n "]) +async def test_talk_send_message_validation_blank_text( + nc_mcp_client: ClientSession, + temporary_conversation: dict, + blank_text: str, +): + """Empty and whitespace-only message text are rejected client-side.""" + token = temporary_conversation["token"] + + result = await nc_mcp_client.call_tool( + "talk_send_message", {"token": token, "message": blank_text} + ) + assert result.isError is True, ( + f"Expected validation error for blank message {blank_text!r}" + ) + + +async def test_talk_send_message_validation_too_long( + nc_mcp_client: ClientSession, temporary_conversation: dict +): + """A message exceeding the 32000-char ceiling is rejected client-side.""" + token = temporary_conversation["token"] + + result = await nc_mcp_client.call_tool( + "talk_send_message", + {"token": token, "message": "x" * 32001}, + ) + assert result.isError is True, ( + "Expected validation error for message longer than 32000 characters" + ) diff --git a/tests/unit/test_client_registry.py b/tests/unit/test_client_registry.py index 30c5709b..75d27308 100644 --- a/tests/unit/test_client_registry.py +++ b/tests/unit/test_client_registry.py @@ -88,20 +88,23 @@ def test_http_non_localhost_rejected(monkeypatch, caplog): assert "evil.com" in caplog.text -def test_empty_string_uses_well_known(monkeypatch): - registry = _get_registry(monkeypatch, "") - clients = registry.list_clients() - client_ids = {c.client_id for c in clients} - assert "claude-desktop" in client_ids - assert "test-mcp-client" in client_ids +def test_empty_string_yields_empty_registry(monkeypatch, caplog): + with caplog.at_level(logging.WARNING): + registry = _get_registry(monkeypatch, "") + assert registry.list_clients() == [] + valid, err = registry.validate_client("claude-desktop") + assert valid is False + assert "Unknown client_id" in err + assert "ALLOWED_MCP_CLIENTS is unset or empty" in caplog.text -def test_unset_env_uses_well_known(monkeypatch): - registry = _get_registry(monkeypatch, None) - clients = registry.list_clients() - client_ids = {c.client_id for c in clients} - assert "claude-desktop" in client_ids - assert "test-mcp-client" in client_ids +def test_unset_env_yields_empty_registry(monkeypatch, caplog): + with caplog.at_level(logging.WARNING): + registry = _get_registry(monkeypatch, None) + assert registry.list_clients() == [] + valid, err = registry.validate_client("test-mcp-client") + assert valid is False + assert "Unknown client_id" in err def test_malformed_entries_skipped_with_warning(monkeypatch, caplog): @@ -153,14 +156,6 @@ def test_validate_redirect_uri_localhost_wildcard(monkeypatch): assert err is None -def test_well_known_clients_wildcard_scopes(monkeypatch): - registry = _get_registry(monkeypatch, None) - for client in registry.list_clients(): - assert client.allowed_scopes == ["*"], ( - f"Well-known client {client.client_id} should have wildcard scopes" - ) - - def test_client_name_resolution(monkeypatch): registry = _get_registry(monkeypatch, "claude-desktop, custom-tool") assert registry.get_client("claude-desktop").name == "Claude Desktop" diff --git a/tests/unit/test_oidc_discovery.py b/tests/unit/test_oidc_discovery.py new file mode 100644 index 00000000..c2459f25 --- /dev/null +++ b/tests/unit/test_oidc_discovery.py @@ -0,0 +1,61 @@ +"""Unit tests for OIDC discovery fetch in oauth_routes.""" + +from unittest.mock import patch + +import httpx +import pytest + +from nextcloud_mcp_server.auth import oauth_routes +from nextcloud_mcp_server.auth.oauth_routes import _get_cached_discovery + +pytestmark = pytest.mark.unit + + +@pytest.fixture(autouse=True) +def _clear_discovery_cache(): + """Reset the in-memory discovery cache between tests.""" + oauth_routes._discovery_cache.clear() + yield + oauth_routes._discovery_cache.clear() + + +async def test_discovery_follows_redirect_to_index_php(): + """Discovery fetch must follow 301s. + + Hetzner StorageShare and other Nextcloud installs without pretty URLs + redirect ``/.well-known/openid-configuration`` to + ``/index.php/.well-known/openid-configuration``. Without follow_redirects + the OAuth authorize handler raises HTTPStatusError and returns 500 + (see oauth_routes._get_cached_discovery). + """ + + pretty_url = "https://nx.example.com/.well-known/openid-configuration" + rewritten_url = "https://nx.example.com/index.php/.well-known/openid-configuration" + discovery_doc = { + "issuer": "https://nx.example.com", + "authorization_endpoint": "https://nx.example.com/index.php/apps/oidc/authorize", + "token_endpoint": "https://nx.example.com/index.php/apps/oidc/token", + } + + def handler(request: httpx.Request) -> httpx.Response: + if str(request.url) == pretty_url: + return httpx.Response(301, headers={"location": rewritten_url}) + if str(request.url) == rewritten_url: + return httpx.Response(200, json=discovery_doc) + return httpx.Response(404) + + transport = httpx.MockTransport(handler) + + def fake_client(**kwargs): + kwargs["transport"] = transport + return httpx.AsyncClient(**kwargs) + + with patch( + "nextcloud_mcp_server.auth.oauth_routes.nextcloud_httpx_client", + side_effect=fake_client, + ) as factory: + result = await _get_cached_discovery(pretty_url) + + assert result == discovery_doc + factory.assert_called_once() + assert factory.call_args.kwargs.get("follow_redirects") is True diff --git a/tests/unit/test_unified_verifier.py b/tests/unit/test_unified_verifier.py index 7747bbde..67b3cd0d 100644 --- a/tests/unit/test_unified_verifier.py +++ b/tests/unit/test_unified_verifier.py @@ -512,3 +512,114 @@ class TestVerifyTokenFlow: result = await verifier.verify_token("opaque-token") assert result is not None assert result.resource == "testuser" + + +class TestManagementApiAllowlist: + """Test ALLOWED_MGMT_CLIENT enforcement in verify_token_for_management_api.""" + + @staticmethod + def _underlying_token(client_id: str = "astrolabe"): + from mcp.server.auth.provider import AccessToken + + return AccessToken( + token="t", + client_id=client_id, + scopes=["openid"], + expires_at=int(time.time() + 3600), + resource="testuser", + ) + + async def test_unset_allowlist_rejects_all(self, monkeypatch, base_settings): + monkeypatch.delenv("ALLOWED_MGMT_CLIENT", raising=False) + verifier = UnifiedTokenVerifier(base_settings) + assert verifier._allowed_mgmt_clients == frozenset() + + with patch.object( + verifier, + "_verify_without_audience_check", + return_value=self._underlying_token("astrolabe"), + ): + result = await verifier.verify_token_for_management_api("any-token") + assert result is None + + async def test_empty_allowlist_rejects_all(self, monkeypatch, base_settings): + monkeypatch.setenv("ALLOWED_MGMT_CLIENT", " , ,") + verifier = UnifiedTokenVerifier(base_settings) + assert verifier._allowed_mgmt_clients == frozenset() + + with patch.object( + verifier, + "_verify_without_audience_check", + return_value=self._underlying_token("astrolabe"), + ): + result = await verifier.verify_token_for_management_api("any-token") + assert result is None + + async def test_allowlisted_client_accepted(self, monkeypatch, base_settings): + monkeypatch.setenv("ALLOWED_MGMT_CLIENT", "astrolabe, admin-tool") + verifier = UnifiedTokenVerifier(base_settings) + assert verifier._allowed_mgmt_clients == {"astrolabe", "admin-tool"} + + underlying = self._underlying_token("astrolabe") + with patch.object( + verifier, "_verify_without_audience_check", return_value=underlying + ): + result = await verifier.verify_token_for_management_api("any-token") + assert result is underlying + + async def test_non_allowlisted_client_rejected(self, monkeypatch, base_settings): + monkeypatch.setenv("ALLOWED_MGMT_CLIENT", "astrolabe") + verifier = UnifiedTokenVerifier(base_settings) + + with patch.object( + verifier, + "_verify_without_audience_check", + return_value=self._underlying_token("some-other-client"), + ): + result = await verifier.verify_token_for_management_api("any-token") + assert result is None + + async def test_token_missing_client_id_rejected(self, monkeypatch, base_settings): + monkeypatch.setenv("ALLOWED_MGMT_CLIENT", "astrolabe") + verifier = UnifiedTokenVerifier(base_settings) + + with patch.object( + verifier, + "_verify_without_audience_check", + return_value=self._underlying_token(""), + ): + result = await verifier.verify_token_for_management_api("any-token") + assert result is None + + async def test_underlying_verification_failure_propagates( + self, monkeypatch, base_settings + ): + monkeypatch.setenv("ALLOWED_MGMT_CLIENT", "astrolabe") + verifier = UnifiedTokenVerifier(base_settings) + + with patch.object( + verifier, "_verify_without_audience_check", return_value=None + ): + result = await verifier.verify_token_for_management_api("any-token") + assert result is None + + async def test_cache_hit_also_enforces_allowlist(self, monkeypatch, base_settings): + """A previously-cached token must still be re-checked against the allowlist.""" + import hashlib + + monkeypatch.setenv("ALLOWED_MGMT_CLIENT", "astrolabe") + verifier = UnifiedTokenVerifier(base_settings) + + token = "cached-token" + cache_key = f"mgmt:{hashlib.sha256(token.encode()).hexdigest()}" + verifier._token_cache[cache_key] = ( + { + "sub": "testuser", + "scope": "openid", + "client_id": "not-allowlisted", + }, + time.time() + 3600, + ) + + result = await verifier.verify_token_for_management_api(token) + assert result is None diff --git a/uv.lock b/uv.lock index 230ba865..63563382 100644 --- a/uv.lock +++ b/uv.lock @@ -2086,7 +2086,7 @@ wheels = [ [[package]] name = "nextcloud-mcp-server" -version = "0.73.1" +version = "0.75.0" source = { editable = "." } dependencies = [ { name = "aiosqlite" },