fix(client): route /apps/* through /index.php for non-pretty-URL installs
Bare /apps/<app>/... URLs return 404 on Nextcloud installs without Pretty URLs (URL rewriting), which is opt-in and not the default — see #732. The /index.php/apps/... form is the universal entry point and works regardless of web-server config, matching how /remote.php/dav and /ocs/v2.php already have dedicated entry points. Add a small _resolve_url helper on BaseNextcloudClient that rewrites /apps/... → /index.php/apps/... at the top of _make_request, so every current call site (notes, deck, cookbook, news) and any future ones are covered transparently with no per-client churn. Other path prefixes (/remote.php, /ocs, absolute URLs, already-prefixed /index.php/apps) pass through unchanged. New unit tests in tests/unit/client/test_base.py pin all six cases. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Opus 4.7
parent
4e669781ee
commit
3fb3680a83
@@ -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/<app>/...`` returns 404 — see
|
||||
issue #732. ``/index.php/apps/<app>/...`` 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
|
||||
|
||||
@@ -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/<app>/...`` 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"
|
||||
Reference in New Issue
Block a user