diff --git a/nextcloud_mcp_server/client/base.py b/nextcloud_mcp_server/client/base.py index 7e6a196b..8a538899 100644 --- a/nextcloud_mcp_server/client/base.py +++ b/nextcloud_mcp_server/client/base.py @@ -99,6 +99,21 @@ class BaseNextcloudClient(ABC): """Helper to get the base WebDAV path for the authenticated user.""" return f"/remote.php/dav/files/{self.username}" + @staticmethod + def _resolve_url(url: str) -> str: + """Prefix bare ``/apps/...`` paths with ``/index.php``. + + Pretty URLs (URL rewriting that strips ``index.php``) are an opt-in + Nextcloud feature; without them, ``/apps//...`` returns 404 — see + issue #732. ``/index.php/apps//...`` is the universal entry point + and works on every Nextcloud install regardless of web-server config, + so we route all app-API calls through it. ``/remote.php/dav/...`` and + ``/ocs/...`` have their own dedicated entry points and are unaffected. + """ + if url.startswith("/apps/"): + return "/index.php" + url + return url + @retry_on_429 async def _make_request(self, method: str, url: str, **kwargs): """Common request wrapper with logging, tracing, and error handling. @@ -111,6 +126,7 @@ class BaseNextcloudClient(ABC): Returns: Response object """ + url = self._resolve_url(url) logger.debug(f"Making {method} request to {url}") # Start timer for metrics diff --git a/tests/unit/client/test_base.py b/tests/unit/client/test_base.py new file mode 100644 index 00000000..1697ea55 --- /dev/null +++ b/tests/unit/client/test_base.py @@ -0,0 +1,70 @@ +"""Unit tests for BaseNextcloudClient. + +These cover the URL prefix logic that routes ``/apps/...`` calls through the +universal ``/index.php`` entry point so the server works on Nextcloud installs +without Pretty URLs (issue #732). +""" + +import pytest + +from nextcloud_mcp_server.client.base import BaseNextcloudClient + +pytestmark = pytest.mark.unit + + +def test_apps_path_is_prefixed_with_index_php(): + """Bare ``/apps//...`` 404s on Nextcloud without Pretty URLs; the + universal ``/index.php/apps/...`` form must be used instead. + """ + assert ( + BaseNextcloudClient._resolve_url("/apps/notes/api/v1/notes") + == "/index.php/apps/notes/api/v1/notes" + ) + assert ( + BaseNextcloudClient._resolve_url("/apps/deck/api/v1.0/boards/1/stacks/2/cards") + == "/index.php/apps/deck/api/v1.0/boards/1/stacks/2/cards" + ) + + +def test_remote_php_dav_unchanged(): + """WebDAV/CalDAV/CardDAV paths use a dedicated entry point and don't need + rewriting — leave them alone so we don't break the working call sites. + """ + assert ( + BaseNextcloudClient._resolve_url("/remote.php/dav/files/alice/foo.txt") + == "/remote.php/dav/files/alice/foo.txt" + ) + + +def test_ocs_path_unchanged(): + """The ``ocs/v2.php`` prefix is also a dedicated entry point — no rewrite.""" + assert ( + BaseNextcloudClient._resolve_url("/ocs/v2.php/cloud/users") + == "/ocs/v2.php/cloud/users" + ) + + +def test_already_prefixed_unchanged(): + """If a caller already passed ``/index.php/apps/...`` we must not double-prefix.""" + assert ( + BaseNextcloudClient._resolve_url("/index.php/apps/notes/api/v1/notes") + == "/index.php/apps/notes/api/v1/notes" + ) + + +def test_absolute_url_unchanged(): + """Absolute URLs (full ``https://...``) are pass-through; only path-prefix + matching is intentional, and an ``http://...`` URL doesn't start with + ``/apps/``. + """ + assert ( + BaseNextcloudClient._resolve_url("https://cloud.example.org/apps/notes") + == "https://cloud.example.org/apps/notes" + ) + + +def test_empty_or_unrelated_paths_unchanged(): + """Defensive cases: empty strings, root, and non-apps paths must pass through.""" + assert BaseNextcloudClient._resolve_url("") == "" + assert BaseNextcloudClient._resolve_url("/") == "/" + assert BaseNextcloudClient._resolve_url("/status.php") == "/status.php"