From 2f1b0d2500042672569552312793d455fe580d89 Mon Sep 17 00:00:00 2001 From: Chris Coutinho Date: Sun, 26 Apr 2026 16:52:58 +0200 Subject: [PATCH] fix(calendar): thread raw credentials to caldav AsyncDAVClient MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit caldav 3.x lists niquests as a mandatory dependency and prefers it over httpx. Passing httpx.BasicAuth via the auth= argument breaks under the niquests backend with "Unexpected non-callable authentication" — see #731. Switch CalendarClient.__init__ from auth=Auth|None to keyword-only password/token, and forward them to AsyncDAVClient as password= plus an explicit auth_type ("basic" or "bearer"). caldav then builds whichever auth object its active backend needs (niquests.auth.HTTPBasicAuth or httpx.BasicAuth), so we stay backend-agnostic. Threaded raw credentials through NextcloudClient — added keyword-only password/token to its __init__, and updated from_env, from_token, and the four call sites that build NextcloudClient (context.py basic-auth and Login Flow paths, auth/userinfo_routes.py, vector/oauth_sync.py). Four new unit tests pin the construction wiring so the niquests regression can't recur silently — basic, bearer, no-creds, and password-precedence cases. Co-Authored-By: Claude Opus 4.7 (1M context) --- nextcloud_mcp_server/auth/userinfo_routes.py | 1 + nextcloud_mcp_server/client/__init__.py | 31 +++++- nextcloud_mcp_server/client/calendar.py | 31 +++++- nextcloud_mcp_server/context.py | 2 + nextcloud_mcp_server/vector/oauth_sync.py | 1 + tests/unit/client/test_calendar.py | 99 ++++++++++++++++++++ 6 files changed, 156 insertions(+), 9 deletions(-) create mode 100644 tests/unit/client/test_calendar.py diff --git a/nextcloud_mcp_server/auth/userinfo_routes.py b/nextcloud_mcp_server/auth/userinfo_routes.py index ad4de704..d3f20a57 100644 --- a/nextcloud_mcp_server/auth/userinfo_routes.py +++ b/nextcloud_mcp_server/auth/userinfo_routes.py @@ -65,6 +65,7 @@ async def _get_authenticated_client_for_userinfo(request: Request) -> NextcloudC base_url=nextcloud_host, username=username, auth=BasicAuth(username, password), + password=password, ) # OAuth mode - get token from session diff --git a/nextcloud_mcp_server/client/__init__.py b/nextcloud_mcp_server/client/__init__.py index 4b567efb..a6379e48 100644 --- a/nextcloud_mcp_server/client/__init__.py +++ b/nextcloud_mcp_server/client/__init__.py @@ -63,7 +63,15 @@ class AsyncDisableCookieTransport(AsyncBaseTransport): class NextcloudClient: """Main Nextcloud client that orchestrates all app clients.""" - def __init__(self, base_url: str, username: str, auth: Auth | None = None): + def __init__( + self, + base_url: str, + username: str, + auth: Auth | None = None, + *, + password: str | None = None, + token: str | None = None, + ): self.username = username self._client = AsyncClient( base_url=base_url, @@ -77,9 +85,12 @@ class NextcloudClient: self.notes = NotesClient(self._client, username) self.webdav = WebDAVClient(self._client, username) self.tables = TablesClient(self._client, username) + # CalendarClient takes raw credentials so caldav (which uses niquests as + # its preferred backend in v3.x) builds a backend-compatible auth object + # itself — passing httpx.BasicAuth here breaks under niquests (#731). self.calendar = CalendarClient( - base_url, username, auth - ) # Uses AsyncDavClient internally + base_url, username, password=password, token=token + ) self.contacts = ContactsClient(self._client, username) self.cookbook = CookbookClient(self._client, username) self.collectives = CollectivesClient(self._client, username) @@ -101,7 +112,12 @@ class NextcloudClient: username = os.environ["NEXTCLOUD_USERNAME"] password = os.environ["NEXTCLOUD_PASSWORD"] # Pass username to constructor - return cls(base_url=host, username=username, auth=BasicAuth(username, password)) + return cls( + base_url=host, + username=username, + auth=BasicAuth(username, password), + password=password, + ) @classmethod def from_token(cls, base_url: str, token: str, username: str): @@ -118,7 +134,12 @@ class NextcloudClient: from ..auth import BearerAuth # noqa: PLC0415 logger.info(f"Creating NC Client for user '{username}' using OAuth token") - return cls(base_url=base_url, username=username, auth=BearerAuth(token)) + return cls( + base_url=base_url, + username=username, + auth=BearerAuth(token), + token=token, + ) async def capabilities(self): response = await self._client.get( diff --git a/nextcloud_mcp_server/client/calendar.py b/nextcloud_mcp_server/client/calendar.py index c72258ec..993abdbb 100644 --- a/nextcloud_mcp_server/client/calendar.py +++ b/nextcloud_mcp_server/client/calendar.py @@ -10,7 +10,6 @@ import anyio from caldav.aio import AsyncCalendar, AsyncDAVClient, AsyncEvent from caldav.elements import cdav, dav from caldav.lib import error as caldav_error -from httpx import Auth from icalendar import Alarm, Calendar, vDDDTypes, vRecur from icalendar import Event as ICalEvent from icalendar import Todo as ICalTodo @@ -36,22 +35,46 @@ async def _maybe_await(result: Any) -> Any: class CalendarClient: """Client for Nextcloud CalDAV calendar and task operations.""" - def __init__(self, base_url: str, username: str, auth: Auth | None = None): + def __init__( + self, + base_url: str, + username: str, + *, + password: str | None = None, + token: str | None = None, + ): """Initialize CalendarClient with AsyncDAVClient. + Pass the raw credential plus an explicit ``auth_type`` so caldav can + build whichever auth object its active HTTP backend needs. caldav v3 + prefers ``niquests`` over ``httpx`` and won't accept an ``httpx.Auth`` + when ``niquests`` is the active backend (issue #731), so we no longer + accept a pre-built ``httpx.Auth`` here. + Args: base_url: Nextcloud base URL username: Nextcloud username - auth: httpx.Auth object (BasicAuth or BearerAuth) + password: App password / login password — selects ``auth_type="basic"`` + token: OAuth bearer token — selects ``auth_type="bearer"`` + + Pass exactly one of ``password`` or ``token``. Passing neither leaves + the underlying client unauthenticated. """ self.username = username self.base_url = base_url + + auth_kwargs: dict[str, Any] = {} + if password is not None: + auth_kwargs = {"password": password, "auth_type": "basic"} + elif token is not None: + auth_kwargs = {"password": token, "auth_type": "bearer"} + # AsyncDAVClient needs the full base URL for proper URL construction self._dav_client = AsyncDAVClient( url=f"{base_url}/remote.php/dav/", username=username, - auth=auth, ssl_verify_cert=get_nextcloud_ssl_verify(), # type: ignore[arg-type] # caldav types say bool|str but passes through to niquests which accepts SSLContext + **auth_kwargs, ) self._calendar_home_url = f"{base_url}/remote.php/dav/calendars/{username}/" diff --git a/nextcloud_mcp_server/context.py b/nextcloud_mcp_server/context.py index 352a64ac..e4ad6a3c 100644 --- a/nextcloud_mcp_server/context.py +++ b/nextcloud_mcp_server/context.py @@ -148,6 +148,7 @@ def _get_client_from_basic_auth(ctx: Context) -> NextcloudClient: base_url=settings.nextcloud_host, username=username, auth=BasicAuth(username, password), + password=password, ) @@ -196,4 +197,5 @@ async def _get_client_from_login_flow( base_url=nextcloud_host, username=username, auth=BasicAuth(username, app_data["app_password"]), + password=app_data["app_password"], ) diff --git a/nextcloud_mcp_server/vector/oauth_sync.py b/nextcloud_mcp_server/vector/oauth_sync.py index 8b4dcaae..f29443e9 100644 --- a/nextcloud_mcp_server/vector/oauth_sync.py +++ b/nextcloud_mcp_server/vector/oauth_sync.py @@ -109,6 +109,7 @@ async def get_user_client_basic_auth( base_url=nextcloud_host, username=user_id, auth=BasicAuth(user_id, app_password), + password=app_password, ) diff --git a/tests/unit/client/test_calendar.py b/tests/unit/client/test_calendar.py new file mode 100644 index 00000000..a2e0e92a --- /dev/null +++ b/tests/unit/client/test_calendar.py @@ -0,0 +1,99 @@ +"""Unit tests for the CalendarClient construction path. + +These pin the wiring into ``caldav.aio.AsyncDAVClient``. caldav v3.x prefers +``niquests`` over ``httpx`` and rejects ``httpx.Auth`` objects when ``niquests`` +is the active backend (issue #731), so we no longer build an httpx auth object +ourselves — we pass the raw credential plus an explicit ``auth_type`` and let +caldav build whichever auth its backend needs. +""" + +import pytest + +pytestmark = pytest.mark.unit + + +def test_basic_auth_passes_password_and_auth_type_basic(mocker): + """Password path: pass ``password=`` + ``auth_type='basic'``, no ``auth=`` arg. + + The previous wiring passed ``auth=httpx.BasicAuth(...)`` which caldav-on-niquests + rejects with "Unexpected non-callable authentication" — the regression #731 came + in via caldav 3.x's mandatory niquests dependency. + """ + mock_dav_client = mocker.patch( + "nextcloud_mcp_server.client.calendar.AsyncDAVClient" + ) + + from nextcloud_mcp_server.client.calendar import CalendarClient + + CalendarClient("https://cloud.example.org", "alice", password="app-pw-1234") + + mock_dav_client.assert_called_once() + call_kwargs = mock_dav_client.call_args.kwargs + assert call_kwargs["url"] == "https://cloud.example.org/remote.php/dav/" + assert call_kwargs["username"] == "alice" + assert call_kwargs["password"] == "app-pw-1234" + assert call_kwargs["auth_type"] == "basic" + # Critical: no httpx.Auth object — that's what broke under niquests. + assert "auth" not in call_kwargs + + +def test_token_passes_token_and_auth_type_bearer(mocker): + """Token path: pass ``password=`` + ``auth_type='bearer'``. + + caldav v3 reuses the ``password`` slot for bearer tokens — see + ``async_davclient.build_auth_object``. + """ + mock_dav_client = mocker.patch( + "nextcloud_mcp_server.client.calendar.AsyncDAVClient" + ) + + from nextcloud_mcp_server.client.calendar import CalendarClient + + CalendarClient("https://cloud.example.org", "alice", token="oauth-bearer-xyz") + + call_kwargs = mock_dav_client.call_args.kwargs + assert call_kwargs["password"] == "oauth-bearer-xyz" + assert call_kwargs["auth_type"] == "bearer" + assert "auth" not in call_kwargs + + +def test_no_credentials_leaves_dav_client_unauthenticated(mocker): + """Defensive: if neither credential is provided, don't pass any auth kwargs. + + AsyncDAVClient handles its own discovery when no auth is configured; we + don't want to silently inject an empty password. + """ + mock_dav_client = mocker.patch( + "nextcloud_mcp_server.client.calendar.AsyncDAVClient" + ) + + from nextcloud_mcp_server.client.calendar import CalendarClient + + CalendarClient("https://cloud.example.org", "alice") + + call_kwargs = mock_dav_client.call_args.kwargs + assert "password" not in call_kwargs + assert "auth_type" not in call_kwargs + assert "auth" not in call_kwargs + + +def test_password_takes_precedence_over_token(mocker): + """If a caller supplies both, password wins. Documents the precedence so a + future caller passing both isn't surprised by which one selects auth_type. + """ + mock_dav_client = mocker.patch( + "nextcloud_mcp_server.client.calendar.AsyncDAVClient" + ) + + from nextcloud_mcp_server.client.calendar import CalendarClient + + CalendarClient( + "https://cloud.example.org", + "alice", + password="app-pw", + token="bearer-tok", + ) + + call_kwargs = mock_dav_client.call_args.kwargs + assert call_kwargs["password"] == "app-pw" + assert call_kwargs["auth_type"] == "basic"