Merge pull request #734 from cbcoutinho/fix/calendar-niquests-auth-731
fix(calendar): thread raw credentials to caldav AsyncDAVClient (fixes #731)
This commit is contained in:
@@ -65,6 +65,7 @@ async def _get_authenticated_client_for_userinfo(request: Request) -> NextcloudC
|
|||||||
base_url=nextcloud_host,
|
base_url=nextcloud_host,
|
||||||
username=username,
|
username=username,
|
||||||
auth=BasicAuth(username, password),
|
auth=BasicAuth(username, password),
|
||||||
|
password=password,
|
||||||
)
|
)
|
||||||
|
|
||||||
# OAuth mode - get token from session
|
# OAuth mode - get token from session
|
||||||
|
|||||||
@@ -63,7 +63,15 @@ class AsyncDisableCookieTransport(AsyncBaseTransport):
|
|||||||
class NextcloudClient:
|
class NextcloudClient:
|
||||||
"""Main Nextcloud client that orchestrates all app clients."""
|
"""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.username = username
|
||||||
self._client = AsyncClient(
|
self._client = AsyncClient(
|
||||||
base_url=base_url,
|
base_url=base_url,
|
||||||
@@ -77,9 +85,12 @@ class NextcloudClient:
|
|||||||
self.notes = NotesClient(self._client, username)
|
self.notes = NotesClient(self._client, username)
|
||||||
self.webdav = WebDAVClient(self._client, username)
|
self.webdav = WebDAVClient(self._client, username)
|
||||||
self.tables = TablesClient(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(
|
self.calendar = CalendarClient(
|
||||||
base_url, username, auth
|
base_url, username, password=password, token=token
|
||||||
) # Uses AsyncDavClient internally
|
)
|
||||||
self.contacts = ContactsClient(self._client, username)
|
self.contacts = ContactsClient(self._client, username)
|
||||||
self.cookbook = CookbookClient(self._client, username)
|
self.cookbook = CookbookClient(self._client, username)
|
||||||
self.collectives = CollectivesClient(self._client, username)
|
self.collectives = CollectivesClient(self._client, username)
|
||||||
@@ -101,7 +112,12 @@ class NextcloudClient:
|
|||||||
username = os.environ["NEXTCLOUD_USERNAME"]
|
username = os.environ["NEXTCLOUD_USERNAME"]
|
||||||
password = os.environ["NEXTCLOUD_PASSWORD"]
|
password = os.environ["NEXTCLOUD_PASSWORD"]
|
||||||
# Pass username to constructor
|
# 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
|
@classmethod
|
||||||
def from_token(cls, base_url: str, token: str, username: str):
|
def from_token(cls, base_url: str, token: str, username: str):
|
||||||
@@ -118,7 +134,12 @@ class NextcloudClient:
|
|||||||
from ..auth import BearerAuth # noqa: PLC0415
|
from ..auth import BearerAuth # noqa: PLC0415
|
||||||
|
|
||||||
logger.info(f"Creating NC Client for user '{username}' using OAuth token")
|
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):
|
async def capabilities(self):
|
||||||
response = await self._client.get(
|
response = await self._client.get(
|
||||||
|
|||||||
@@ -10,7 +10,6 @@ import anyio
|
|||||||
from caldav.aio import AsyncCalendar, AsyncDAVClient, AsyncEvent
|
from caldav.aio import AsyncCalendar, AsyncDAVClient, AsyncEvent
|
||||||
from caldav.elements import cdav, dav
|
from caldav.elements import cdav, dav
|
||||||
from caldav.lib import error as caldav_error
|
from caldav.lib import error as caldav_error
|
||||||
from httpx import Auth
|
|
||||||
from icalendar import Alarm, Calendar, vDDDTypes, vRecur
|
from icalendar import Alarm, Calendar, vDDDTypes, vRecur
|
||||||
from icalendar import Event as ICalEvent
|
from icalendar import Event as ICalEvent
|
||||||
from icalendar import Todo as ICalTodo
|
from icalendar import Todo as ICalTodo
|
||||||
@@ -36,22 +35,46 @@ async def _maybe_await(result: Any) -> Any:
|
|||||||
class CalendarClient:
|
class CalendarClient:
|
||||||
"""Client for Nextcloud CalDAV calendar and task operations."""
|
"""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.
|
"""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:
|
Args:
|
||||||
base_url: Nextcloud base URL
|
base_url: Nextcloud base URL
|
||||||
username: Nextcloud username
|
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.username = username
|
||||||
self.base_url = base_url
|
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
|
# AsyncDAVClient needs the full base URL for proper URL construction
|
||||||
self._dav_client = AsyncDAVClient(
|
self._dav_client = AsyncDAVClient(
|
||||||
url=f"{base_url}/remote.php/dav/",
|
url=f"{base_url}/remote.php/dav/",
|
||||||
username=username,
|
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
|
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}/"
|
self._calendar_home_url = f"{base_url}/remote.php/dav/calendars/{username}/"
|
||||||
|
|
||||||
|
|||||||
@@ -148,6 +148,7 @@ def _get_client_from_basic_auth(ctx: Context) -> NextcloudClient:
|
|||||||
base_url=settings.nextcloud_host,
|
base_url=settings.nextcloud_host,
|
||||||
username=username,
|
username=username,
|
||||||
auth=BasicAuth(username, password),
|
auth=BasicAuth(username, password),
|
||||||
|
password=password,
|
||||||
)
|
)
|
||||||
|
|
||||||
|
|
||||||
@@ -196,4 +197,5 @@ async def _get_client_from_login_flow(
|
|||||||
base_url=nextcloud_host,
|
base_url=nextcloud_host,
|
||||||
username=username,
|
username=username,
|
||||||
auth=BasicAuth(username, app_data["app_password"]),
|
auth=BasicAuth(username, app_data["app_password"]),
|
||||||
|
password=app_data["app_password"],
|
||||||
)
|
)
|
||||||
|
|||||||
@@ -109,6 +109,7 @@ async def get_user_client_basic_auth(
|
|||||||
base_url=nextcloud_host,
|
base_url=nextcloud_host,
|
||||||
username=user_id,
|
username=user_id,
|
||||||
auth=BasicAuth(user_id, app_password),
|
auth=BasicAuth(user_id, app_password),
|
||||||
|
password=app_password,
|
||||||
)
|
)
|
||||||
|
|
||||||
|
|
||||||
|
|||||||
@@ -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=<token>`` + ``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"
|
||||||
Reference in New Issue
Block a user