From 3836534205a9becb36ace9688a36425a745f343c Mon Sep 17 00:00:00 2001 From: Chris Coutinho Date: Fri, 8 Aug 2025 21:02:00 +0200 Subject: [PATCH] fix(client): Strip cookies from responses to avoid falsely raising CSRF errors --- .../post-installation/install-calendar-app.sh | 4 ++ nextcloud_mcp_server/client/__init__.py | 42 +++++++++++--- nextcloud_mcp_server/client/base.py | 57 ++++++++++++++++++- tests/conftest.py | 2 +- 4 files changed, 94 insertions(+), 11 deletions(-) diff --git a/app-hooks/post-installation/install-calendar-app.sh b/app-hooks/post-installation/install-calendar-app.sh index eb142a1..2fe4f1f 100755 --- a/app-hooks/post-installation/install-calendar-app.sh +++ b/app-hooks/post-installation/install-calendar-app.sh @@ -11,6 +11,10 @@ php /var/www/html/occ app:enable calendar echo "Waiting for calendar app to initialize..." sleep 5 +# Increase limits on calendar creation for integration tests (100 in 60s) +php occ config:app:set dav rateLimitCalendarCreation --type=integer --value=100 +php occ config:app:set dav rateLimitPeriodCalendarCreation --type=integer --value=60 + # Ensure maintenance mode is off before calendar operations php /var/www/html/occ maintenance:mode --off diff --git a/nextcloud_mcp_server/client/__init__.py b/nextcloud_mcp_server/client/__init__.py index b17162e..86be374 100644 --- a/nextcloud_mcp_server/client/__init__.py +++ b/nextcloud_mcp_server/client/__init__.py @@ -1,7 +1,15 @@ import logging import os -from httpx import AsyncClient, Auth, BasicAuth, Request, Response +from httpx import ( + AsyncClient, + Auth, + BasicAuth, + Request, + Response, + AsyncBaseTransport, + AsyncHTTPTransport, +) from ..controllers.notes_search import NotesSearchController from .calendar import CalendarClient @@ -13,19 +21,34 @@ from .webdav import WebDAVClient logger = logging.getLogger(__name__) -def log_request(request: Request): - logger.info( +async def log_request(request: Request): + logger.debug( "Request event hook: %s %s - Waiting for content", request.method, request.url, ) - logger.info("Request body: %s", request.content) - logger.info("Headers: %s", request.headers) + logger.debug("Request body: %s", request.content) + logger.debug("Headers: %s", request.headers) -def log_response(response: Response): - response.read() # Explicitly read the stream before accessing .text - logger.info("Response [%s] %s", response.status_code, response.text) +async def log_response(response: Response): + await response.aread() + logger.debug("Response [%s] %s", response.status_code, response.text) + + +class AsyncDisableCookieTransport(AsyncBaseTransport): + """This Transport disable cookies from accumulating in the httpx AsyncClient + + Thanks to: https://github.com/encode/httpx/issues/2992#issuecomment-2133258994 + """ + + def __init__(self, transport: AsyncBaseTransport): + self.transport = transport + + async def handle_async_request(self, request: Request) -> Response: + response = await self.transport.handle_async_request(request) + response.headers.pop("set-cookie", None) + return response class NextcloudClient: @@ -36,7 +59,8 @@ class NextcloudClient: self._client = AsyncClient( base_url=base_url, auth=auth, - # event_hooks={"request": [log_request], "response": [log_response]}, + transport=AsyncDisableCookieTransport(AsyncHTTPTransport()), + event_hooks={"request": [log_request], "response": [log_response]}, ) # Initialize app clients diff --git a/nextcloud_mcp_server/client/base.py b/nextcloud_mcp_server/client/base.py index 22eab6a..664353b 100644 --- a/nextcloud_mcp_server/client/base.py +++ b/nextcloud_mcp_server/client/base.py @@ -3,11 +3,65 @@ import logging from abc import ABC -from httpx import AsyncClient +from functools import wraps +import time +from httpx import HTTPStatusError, codes, RequestError, AsyncClient logger = logging.getLogger(__name__) +def retry_on_429(func): + """This decorator handles the 429 response from REST APIs + + The `func` is assumed to be a method that is similar to `httpx.Client.get`, + and returns an `httpx.Response` object. In the case of `Too Many Requests` HTTP + response, the function will wait for a couple of seconds and retry the request. + """ + + MAX_RETRIES = 5 + + @wraps(func) + async def wrapper(*args, **kwargs): + retries = 0 + + while retries < MAX_RETRIES: + try: + # Make GET API call + retries += 1 + response = await func(*args, **kwargs) + break + + except HTTPStatusError as e: + # If we get a '429 Client Error: Too Many Requests' + # error we wait a couple of seconds and do a retry + if e.response.status_code == codes.TOO_MANY_REQUESTS: + logger.warning( + f"429 Client Error: Too Many Requests, Number of attempts: {retries}" + ) + time.sleep(5) + else: + logger.warning( + f"HTTPStatusError {e.response.status_code}: {e}, Number of attempts: {retries}" + ) + raise + except RequestError as e: + logger.warning( + f"RequestError {e.request.url}: {e}, Number of attempts: {retries}" + ) + raise + + # If for loop ends without break statement + else: + logger.warning("All API call retries failed") + raise RuntimeError( + f"Maximum number of retries ({MAX_RETRIES}) exceeded without success" + ) + + return response + + return wrapper + + class BaseNextcloudClient(ABC): """Base class for all Nextcloud app clients.""" @@ -25,6 +79,7 @@ class BaseNextcloudClient(ABC): """Helper to get the base WebDAV path for the authenticated user.""" return f"/remote.php/dav/files/{self.username}" + @retry_on_429 async def _make_request(self, method: str, url: str, **kwargs): """Common request wrapper with logging and error handling. diff --git a/tests/conftest.py b/tests/conftest.py index 6bf3ea8..a514634 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -13,7 +13,7 @@ from nextcloud_mcp_server.client import NextcloudClient logger = logging.getLogger(__name__) -@pytest.fixture(scope="module") +@pytest.fixture(scope="session") async def nc_client() -> AsyncGenerator[NextcloudClient, Any]: """ Fixture to create a NextcloudClient instance for integration tests.