From 18db9d6cb3ee5b8318314373c40b078a296e0d3d Mon Sep 17 00:00:00 2001 From: Chris Coutinho Date: Tue, 12 May 2026 23:56:44 +0200 Subject: [PATCH 1/4] refactor: prune dead pre-LOGIN_FLOW config/runtime branches MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two small post-merge cleanups deferred from PR #787 (ADR-022 follow-up). Both were explicitly noted in the reviewer's "acknowledged deferred items" list. 1. config.py: drop `enable_multi_user_basic_auth` and `enable_login_flow` from the dynaconf `_DEFAULTS` dict. They were removed from `_field_map` in PR #787, so `get_settings()` never read them anyway, but their presence in `_DEFAULTS` was visually misleading — readers might think they could be set via TOML when in fact `Settings.__post_init__` derives them from `MCP_DEPLOYMENT_MODE`. Replaced with a NOTE comment pointing at the canonical derivation site. 2. app.py: the lifespan code had `use_basic_auth = not oauth_enabled or settings.enable_login_flow`, which became always-True once PR #787 enforced `oauth_enabled ↔ enable_login_flow` via __post_init__. Hard-coded to `True` with a comment explaining the invariant and pointing at the separate follow-up that will prune the now-unreachable `use_basic_auth=False` code paths in `vector/oauth_sync.py` (which includes deleting the `use_basic_auth` parameter from `user_manager_task` / `oauth_processor_task` and the OAuth-token-refresh branch in `get_user_client`). Kept the variable name and the call-site conditionals as-is for now so that follow-up is a clean mechanical diff. No runtime behaviour change: `use_basic_auth` already evaluated to True in every supported mode after PR #787, and the `_DEFAULTS` entries were already shadowed by `__post_init__`. Co-Authored-By: Claude Opus 4.7 (1M context) --- nextcloud_mcp_server/app.py | 17 +++++++++++++---- nextcloud_mcp_server/config.py | 5 +++-- 2 files changed, 16 insertions(+), 6 deletions(-) diff --git a/nextcloud_mcp_server/app.py b/nextcloud_mcp_server/app.py index 5bdedf86..3fa99a47 100644 --- a/nextcloud_mcp_server/app.py +++ b/nextcloud_mcp_server/app.py @@ -1789,10 +1789,19 @@ def get_app(transport: str = "streamable-http", enabled_apps: list[str] | None = ) break - # Determine authentication mode for background sync - # Login Flow v2 and multi-user BasicAuth: use app passwords - # OAuth mode (without Login Flow): use OAuth refresh tokens - use_basic_auth = not oauth_enabled or settings.enable_login_flow + # Background sync always uses app passwords post-ADR-022: + # `oauth_enabled` now implies `enable_login_flow` (single + # source of truth is `MCP_DEPLOYMENT_MODE`), so the old + # `not oauth_enabled or settings.enable_login_flow` was always + # True. The OAuth-refresh code paths in + # `vector/oauth_sync.py` (gated on `use_basic_auth=False`) + # are now unreachable; pruning them — and dropping the + # `use_basic_auth` parameter from `user_manager_task` / + # `oauth_processor_task` — is tracked as a separate + # follow-up. Keep the variable name + the conditional + # wiring at the call sites for now so the parallel-prune + # PR is a clean mechanical diff. + use_basic_auth = True # Start background tasks using anyio TaskGroup async with anyio.create_task_group() as tg: diff --git a/nextcloud_mcp_server/config.py b/nextcloud_mcp_server/config.py index 62eca989..25347c21 100644 --- a/nextcloud_mcp_server/config.py +++ b/nextcloud_mcp_server/config.py @@ -43,8 +43,9 @@ _DEFAULTS: dict[str, Any] = { "userinfo_uri": None, "oidc_resource_server_id": None, # Mode flags - "enable_multi_user_basic_auth": False, - "enable_login_flow": False, + # NOTE: `enable_multi_user_basic_auth` and `enable_login_flow` are + # intentionally absent — they are derived from MCP_DEPLOYMENT_MODE in + # Settings.__post_init__ (ADR-022) and not read from the dynaconf store. "enable_semantic_search": False, "enable_background_operations": False, "vector_sync_enabled": False, From 735a4019edf2c4aaa11382eaa176403aa3fcc083 Mon Sep 17 00:00:00 2001 From: "github-actions[bot]" Date: Tue, 12 May 2026 23:04:48 +0000 Subject: [PATCH 2/4] =?UTF-8?q?bump:=20version=200.86.0=20=E2=86=92=200.86?= =?UTF-8?q?.1?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- CHANGELOG.md | 6 ++++++ pyproject.toml | 2 +- uv.lock | 2 +- 3 files changed, 8 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index fb084a10..538781db 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,12 @@ All notable changes to the Nextcloud MCP Server will be documented in this file. The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), and this project adheres to [PEP 440](https://peps.python.org/pep-0440/). +## v0.86.1 (2026-05-12) + +### Refactor + +- prune dead pre-LOGIN_FLOW config/runtime branches + ## v0.86.0 (2026-05-12) ### BREAKING CHANGE diff --git a/pyproject.toml b/pyproject.toml index 19cc0d81..53237a3e 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -1,6 +1,6 @@ [project] name = "nextcloud-mcp-server" -version = "0.86.0" +version = "0.86.1" description = "Model Context Protocol (MCP) server for Nextcloud integration - enables AI assistants to interact with Nextcloud data" authors = [ {name = "Chris Coutinho", email = "chris@coutinho.io"} diff --git a/uv.lock b/uv.lock index b973f17f..ce2d7790 100644 --- a/uv.lock +++ b/uv.lock @@ -2123,7 +2123,7 @@ wheels = [ [[package]] name = "nextcloud-mcp-server" -version = "0.86.0" +version = "0.86.1" source = { editable = "." } dependencies = [ { name = "aiosqlite" }, From 65345fd6ebd8774b6aca5ced16d88924f807c1bb Mon Sep 17 00:00:00 2001 From: Chris Coutinho Date: Wed, 13 May 2026 01:13:53 +0200 Subject: [PATCH 3/4] refactor: drop OAuth-refresh background-sync path from oauth_sync.py MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Follow-up to #787/#789 (ADR-022 cleanup). After `oauth_enabled ↔ enable_login_flow` became an invariant, the `use_basic_auth=False` branch in `vector/oauth_sync.py` — and the parameter wiring that fed it — was no longer reachable from any supported deployment mode. This commit removes the dead code. - nextcloud_mcp_server/vector/oauth_sync.py: - Deleted `get_user_client_oauth` (the OAuth-token refresh helper) and its `VECTOR_SYNC_SCOPES` constant. - Deleted the `get_user_client` dispatcher. Internal callers now call `get_user_client_basic_auth` directly. - Dropped the `use_basic_auth: bool` parameter from `user_scanner_task`, `multi_user_processor_task`, `_run_user_scanner_with_scope`, and `user_manager_task`. - Dropped the `token_broker` parameter from the same four functions — they no longer need it now that the OAuth-refresh path is gone. The `TokenBrokerService` constructed in `app.py` is still used by the management API revoke endpoint, just not by background sync. - Simplified the user-list query in `user_manager_task` to always read from the `app_passwords` table. - Replaced all `mode_label = "BasicAuth" if use_basic_auth else "OAuth"` with a literal `[BasicAuth]` log prefix (keeps existing log filters working). - Updated the module docstring to describe the post-cleanup shape. - Dropped the now-unused `TYPE_CHECKING` import of `TokenBrokerService`. - nextcloud_mcp_server/app.py: dropped the `use_basic_auth = True` block and the now-stale `token_broker if not use_basic_auth else None` / `use_basic_auth` positional args from the two `tg.start(...)` calls in the multi-user vector-sync lifespan. Token broker construction stays — still consumed by the management API revoke endpoint via `app.state.oauth_context["token_broker"]`. - tests/integration/test_app_password_provisioning.py: deleted four tests that exercised the now-removed OAuth-refresh path (`test_oauth_mode_uses_refresh_token_only`, `test_oauth_mode_raises_error_without_token`, `test_get_user_client_oauth_function`, `test_oauth_mode_requires_token_broker`) plus the `test_get_user_client_dispatches_to_basic_auth` test for the deleted dispatcher. Updated the module docstring + imports accordingly. The BasicAuth-mode tests (`test_basic_auth_mode_uses_local_storage`, `test_multiple_users_basic_auth_mode`, etc.) all remain. No runtime-behaviour change in any supported deployment mode — the deleted branches were already unreachable post-PR #787. 3 files changed, +59 / -301; 1010 unit tests pass; integration jobs for `mcp-login-flow` and `mcp-multi-user-basic` are the critical regression gates before merge. Co-Authored-By: Claude Opus 4.7 (1M context) --- nextcloud_mcp_server/app.py | 29 +-- nextcloud_mcp_server/vector/oauth_sync.py | 210 ++++-------------- .../test_app_password_provisioning.py | 121 +--------- 3 files changed, 59 insertions(+), 301 deletions(-) diff --git a/nextcloud_mcp_server/app.py b/nextcloud_mcp_server/app.py index 3fa99a47..bbce2373 100644 --- a/nextcloud_mcp_server/app.py +++ b/nextcloud_mcp_server/app.py @@ -1789,21 +1789,14 @@ def get_app(transport: str = "streamable-http", enabled_apps: list[str] | None = ) break - # Background sync always uses app passwords post-ADR-022: - # `oauth_enabled` now implies `enable_login_flow` (single - # source of truth is `MCP_DEPLOYMENT_MODE`), so the old - # `not oauth_enabled or settings.enable_login_flow` was always - # True. The OAuth-refresh code paths in - # `vector/oauth_sync.py` (gated on `use_basic_auth=False`) - # are now unreachable; pruning them — and dropping the - # `use_basic_auth` parameter from `user_manager_task` / - # `oauth_processor_task` — is tracked as a separate - # follow-up. Keep the variable name + the conditional - # wiring at the call sites for now so the parallel-prune - # PR is a clean mechanical diff. - use_basic_auth = True - - # Start background tasks using anyio TaskGroup + # Background sync authenticates as each provisioned user via + # locally-stored Nextcloud app passwords (Login Flow v2 / + # multi-user BasicAuth). The earlier OAuth refresh-token + # path in vector/oauth_sync.py was removed in the ADR-022 + # cleanup — it relied on unmerged user_oidc patches and was + # never reachable from any supported deployment mode. The + # `token_broker` constructed above is still used by the + # management API revoke endpoint (via app.state.oauth_context). async with anyio.create_task_group() as tg: # Start user manager task (supervises per-user scanners) await tg.start( @@ -1811,12 +1804,10 @@ def get_app(transport: str = "streamable-http", enabled_apps: list[str] | None = send_stream, shutdown_event, scanner_wake_event, - token_broker if not use_basic_auth else None, - token_storage, # Use token_storage (works for both OAuth and multi-user BasicAuth) + token_storage, nextcloud_host_for_sync, user_states, tg, - use_basic_auth, # Pass as positional arg (before task_status) ) # Start processor pool (each gets a cloned receive stream) @@ -1826,9 +1817,7 @@ def get_app(transport: str = "streamable-http", enabled_apps: list[str] | None = i, receive_stream.clone(), shutdown_event, - token_broker if not use_basic_auth else None, nextcloud_host_for_sync, - use_basic_auth, # Pass as positional arg (before task_status) ) # Expose this long-lived task group to request-path code diff --git a/nextcloud_mcp_server/vector/oauth_sync.py b/nextcloud_mcp_server/vector/oauth_sync.py index 7883c4db..63aaa361 100644 --- a/nextcloud_mcp_server/vector/oauth_sync.py +++ b/nextcloud_mcp_server/vector/oauth_sync.py @@ -5,25 +5,22 @@ Manages background vector sync for multi-user deployments: - Per-User Scanners: One scanner task per provisioned user - Shared Processor Pool: Processes documents from all users -Authentication strategies are mutually exclusive by deployment mode: +Background sync authenticates as each provisioned user via locally-stored +Nextcloud app passwords (BasicAuth), retrieved through the management API +after the user completes Login Flow v2 (or, in multi-user BasicAuth mode, +the per-user Astrolabe provisioning flow). -Multi-user BasicAuth mode (MCP_DEPLOYMENT_MODE=multi_user_basic): -- Uses app passwords stored locally in MCP server's database -- Users provision via Astrolabe personal settings, which sends to MCP API -- OAuth is NOT used - -OAuth mode (with external IdP like Keycloak): -- Uses OAuth refresh tokens via TokenBrokerService -- Users provision via browser OAuth flow -- App passwords are NOT used - -These are separate concerns - no fallback between them. +The earlier OAuth refresh-token path was removed in the ADR-022 follow-up: +it depended on unmerged Nextcloud `user_oidc` patches for Bearer-token +validation on non-OCS endpoints, and was never reachable from any +supported deployment mode. The `TokenBrokerService` constructed in +`app.py` is retained for the management API revoke endpoint, not for +background sync. """ import logging import time from dataclasses import dataclass, field -from typing import TYPE_CHECKING import anyio from anyio.abc import TaskGroup, TaskStatus @@ -39,19 +36,8 @@ from nextcloud_mcp_server.config import get_settings from nextcloud_mcp_server.vector.processor import process_document from nextcloud_mcp_server.vector.scanner import DocumentTask, scan_user_documents -if TYPE_CHECKING: - from nextcloud_mcp_server.auth.token_broker import TokenBrokerService - logger = logging.getLogger(__name__) -# Scopes required for vector sync operations -VECTOR_SYNC_SCOPES = [ - "notes.read", - "files.read", - "deck.read", - # "news.read", # News app may not be installed -] - class NotProvisionedError(Exception): """User has not provisioned offline access or has revoked it.""" @@ -113,84 +99,13 @@ async def get_user_client_basic_auth( ) -async def get_user_client_oauth( - user_id: str, - token_broker: "TokenBrokerService", - nextcloud_host: str, -) -> NextcloudClient: - """Get an authenticated NextcloudClient using OAuth refresh token. - - For OAuth deployments with external IdP where users provision via - browser OAuth flow. App passwords are NOT used in this mode. - - Args: - user_id: User identifier - token_broker: Token broker for obtaining access tokens - nextcloud_host: Nextcloud base URL - - Returns: - Authenticated NextcloudClient with Bearer token - - Raises: - NotProvisionedError: If user has not provisioned offline access - """ - token = await token_broker.get_background_token(user_id, VECTOR_SYNC_SCOPES) - if not token: - raise NotProvisionedError( - f"User {user_id} has not provisioned offline access. " - f"User must complete the OAuth provisioning flow." - ) - - logger.info(f"Using OAuth refresh token for background sync: {user_id}") - return NextcloudClient.from_token( - base_url=nextcloud_host, - token=token, - username=user_id, - ) - - -async def get_user_client( - user_id: str, - token_broker: "TokenBrokerService | None", - nextcloud_host: str, - *, - use_basic_auth: bool = False, -) -> NextcloudClient: - """Get an authenticated NextcloudClient for a user. - - Dispatches to the appropriate authentication strategy based on mode. - These are mutually exclusive - no fallback between them. - - Args: - user_id: User identifier - token_broker: Token broker for OAuth mode (can be None for BasicAuth mode) - nextcloud_host: Nextcloud base URL - use_basic_auth: If True, use app passwords via Astrolabe (BasicAuth mode). - If False, use OAuth refresh tokens (OAuth mode). - - Returns: - Authenticated NextcloudClient - - Raises: - NotProvisionedError: If user has not provisioned access for the mode - """ - if use_basic_auth: - return await get_user_client_basic_auth(user_id, nextcloud_host) - else: - if token_broker is None: - raise ValueError("token_broker required for OAuth mode") - return await get_user_client_oauth(user_id, token_broker, nextcloud_host) - - async def user_scanner_task( user_id: str, send_stream: MemoryObjectSendStream[DocumentTask], shutdown_event: anyio.Event, wake_event: anyio.Event, - token_broker: "TokenBrokerService | None", nextcloud_host: str, *, - use_basic_auth: bool = False, task_status: TaskStatus = anyio.TASK_STATUS_IGNORED, ) -> None: """Scanner task for a single user. @@ -202,13 +117,10 @@ async def user_scanner_task( send_stream: Stream to send changed documents to processors shutdown_event: Event signaling shutdown wake_event: Event to trigger immediate scan - token_broker: Token broker for OAuth mode (None for BasicAuth mode) nextcloud_host: Nextcloud base URL - use_basic_auth: If True, use app passwords; if False, use OAuth tokens task_status: Status object for signaling task readiness """ - mode_label = "BasicAuth" if use_basic_auth else "OAuth" - logger.info(f"[{mode_label}] Scanner started for user: {user_id}") + logger.info(f"[BasicAuth] Scanner started for user: {user_id}") settings = get_settings() max_consecutive_errors = 5 @@ -216,16 +128,14 @@ async def user_scanner_task( # Pre-validate credentials before entering scan loop try: - nc_client = await get_user_client( - user_id, token_broker, nextcloud_host, use_basic_auth=use_basic_auth - ) + nc_client = await get_user_client_basic_auth(user_id, nextcloud_host) try: await nc_client.capabilities() # Lightweight OCS call to validate creds - logger.info(f"[{mode_label}] Credentials validated for {user_id}") + logger.info(f"[BasicAuth] Credentials validated for {user_id}") except HTTPStatusError as e: if e.response.status_code in (401, 403): logger.warning( - f"[{mode_label}] Credential validation failed for {user_id} " + f"[BasicAuth] Credential validation failed for {user_id} " f"(HTTP {e.response.status_code}), not starting scan loop" ) return @@ -234,12 +144,12 @@ async def user_scanner_task( await nc_client.close() except NotProvisionedError: logger.warning( - f"[{mode_label}] User {user_id} not provisioned, not starting scan loop" + f"[BasicAuth] User {user_id} not provisioned, not starting scan loop" ) return except Exception as e: logger.warning( - f"[{mode_label}] Pre-validation failed for {user_id}: {e}. " + f"[BasicAuth] Pre-validation failed for {user_id}: {e}. " f"Proceeding to scan loop (has its own error handling)." ) @@ -249,9 +159,7 @@ async def user_scanner_task( nc_client = None try: # Get fresh credentials for this scan cycle - nc_client = await get_user_client( - user_id, token_broker, nextcloud_host, use_basic_auth=use_basic_auth - ) + nc_client = await get_user_client_basic_auth(user_id, nextcloud_host) # Scan user's documents await scan_user_documents( @@ -264,7 +172,7 @@ async def user_scanner_task( except NotProvisionedError: logger.warning( - f"[{mode_label}] User {user_id} no longer provisioned, stopping scanner" + f"[BasicAuth] User {user_id} no longer provisioned, stopping scanner" ) break @@ -272,7 +180,7 @@ async def user_scanner_task( status_code = e.response.status_code if status_code in (401, 403): logger.warning( - f"[{mode_label}] Scanner auth failed for {user_id} " + f"[BasicAuth] Scanner auth failed for {user_id} " f"(HTTP {status_code}), stopping scanner. " f"User may need to re-provision credentials." ) @@ -280,7 +188,7 @@ async def user_scanner_task( elif status_code == 429: retry_after = min(int(e.response.headers.get("Retry-After", "60")), 300) logger.warning( - f"[{mode_label}] Scanner rate-limited for {user_id}, " + f"[BasicAuth] Scanner rate-limited for {user_id}, " f"backing off {retry_after}s" ) try: @@ -294,7 +202,7 @@ async def user_scanner_task( else: consecutive_errors += 1 logger.error( - f"[{mode_label}] Scanner HTTP error for {user_id}: {e} " + f"[BasicAuth] Scanner HTTP error for {user_id}: {e} " f"({consecutive_errors}/{max_consecutive_errors})", exc_info=True, ) @@ -302,7 +210,7 @@ async def user_scanner_task( except Exception as e: consecutive_errors += 1 logger.error( - f"[{mode_label}] Scanner error for {user_id}: {e} " + f"[BasicAuth] Scanner error for {user_id}: {e} " f"({consecutive_errors}/{max_consecutive_errors})", exc_info=True, ) @@ -313,7 +221,7 @@ async def user_scanner_task( if consecutive_errors >= max_consecutive_errors: logger.error( - f"[{mode_label}] Scanner for {user_id} hit {max_consecutive_errors} " + f"[BasicAuth] Scanner for {user_id} hit {max_consecutive_errors} " f"consecutive errors, stopping scanner" ) break @@ -325,16 +233,14 @@ async def user_scanner_task( except anyio.get_cancelled_exc_class(): break - logger.info(f"[{mode_label}] Scanner stopped for user: {user_id}") + logger.info(f"[BasicAuth] Scanner stopped for user: {user_id}") async def multi_user_processor_task( worker_id: int, receive_stream: MemoryObjectReceiveStream[DocumentTask], shutdown_event: anyio.Event, - token_broker: "TokenBrokerService | None", nextcloud_host: str, - use_basic_auth: bool = False, *, task_status: TaskStatus = anyio.TASK_STATUS_IGNORED, ) -> None: @@ -346,13 +252,10 @@ async def multi_user_processor_task( worker_id: Worker identifier for logging receive_stream: Stream to receive documents from shutdown_event: Event signaling shutdown - token_broker: Token broker for OAuth mode (None for BasicAuth mode) nextcloud_host: Nextcloud base URL - use_basic_auth: If True, use app passwords; if False, use OAuth tokens task_status: Status object for signaling task readiness """ - mode_label = "BasicAuth" if use_basic_auth else "OAuth" - logger.info(f"[{mode_label}] Processor {worker_id} started") + logger.info(f"[BasicAuth] Processor {worker_id} started") task_status.started() while not shutdown_event.is_set(): @@ -364,11 +267,8 @@ async def multi_user_processor_task( doc_task = await receive_stream.receive() # Get credentials for THIS document's user - nc_client = await get_user_client( - doc_task.user_id, - token_broker, - nextcloud_host, - use_basic_auth=use_basic_auth, + nc_client = await get_user_client_basic_auth( + doc_task.user_id, nextcloud_host ) # Process the document @@ -378,13 +278,13 @@ async def multi_user_processor_task( continue except anyio.EndOfStream: - logger.info(f"[{mode_label}] Processor {worker_id}: Stream closed, exiting") + logger.info(f"[BasicAuth] Processor {worker_id}: Stream closed, exiting") break except NotProvisionedError: if doc_task: logger.warning( - f"[{mode_label}] User {doc_task.user_id} not provisioned, " + f"[BasicAuth] User {doc_task.user_id} not provisioned, " f"skipping {doc_task.doc_type}_{doc_task.doc_id}" ) continue @@ -392,20 +292,20 @@ async def multi_user_processor_task( except Exception as e: if doc_task: logger.error( - f"[{mode_label}] Processor {worker_id} error processing " + f"[BasicAuth] Processor {worker_id} error processing " f"{doc_task.doc_type}_{doc_task.doc_id}: {e}", exc_info=True, ) else: logger.error( - f"[{mode_label}] Processor {worker_id} error: {e}", exc_info=True + f"[BasicAuth] Processor {worker_id} error: {e}", exc_info=True ) finally: if nc_client: await nc_client.close() - logger.info(f"[{mode_label}] Processor {worker_id} stopped") + logger.info(f"[BasicAuth] Processor {worker_id} stopped") # Backward compatibility alias @@ -418,10 +318,8 @@ async def _run_user_scanner_with_scope( send_stream: MemoryObjectSendStream[DocumentTask], shutdown_event: anyio.Event, wake_event: anyio.Event, - token_broker: "TokenBrokerService | None", nextcloud_host: str, user_states: dict[str, UserSyncState], - use_basic_auth: bool = False, ) -> None: """Wrapper to run scanner with cancellation scope. @@ -435,9 +333,7 @@ async def _run_user_scanner_with_scope( send_stream=cloned_stream, shutdown_event=shutdown_event, wake_event=wake_event, - token_broker=token_broker, nextcloud_host=nextcloud_host, - use_basic_auth=use_basic_auth, ) finally: # Clean up on exit @@ -450,12 +346,10 @@ async def user_manager_task( send_stream: MemoryObjectSendStream[DocumentTask], shutdown_event: anyio.Event, wake_event: anyio.Event, - token_broker: "TokenBrokerService | None", refresh_token_storage: "RefreshTokenStorage", nextcloud_host: str, user_states: dict[str, UserSyncState], tg: TaskGroup, - use_basic_auth: bool = False, *, task_status: TaskStatus = anyio.TASK_STATUS_IGNORED, ) -> None: @@ -469,41 +363,33 @@ async def user_manager_task( send_stream: Stream to send documents to processors shutdown_event: Event signaling shutdown wake_event: Event to wake scanners for immediate scan - token_broker: Token broker for OAuth mode (None for BasicAuth mode) refresh_token_storage: Storage for tracking provisioned users nextcloud_host: Nextcloud base URL user_states: Shared dict tracking active user scanners tg: Task group for spawning scanner tasks - use_basic_auth: If True, use app passwords; if False, use OAuth tokens task_status: Status object for signaling task readiness """ settings = get_settings() poll_interval = settings.vector_sync_user_poll_interval - mode_label = "BasicAuth" if use_basic_auth else "OAuth" - logger.info( - f"[{mode_label}] User manager started (poll interval: {poll_interval}s)" - ) + logger.info(f"[BasicAuth] User manager started (poll interval: {poll_interval}s)") task_status.started() while not shutdown_event.is_set(): try: - # Get current provisioned users based on mode - if use_basic_auth: - # BasicAuth / Login Flow v2 mode: query app_passwords table - provisioned_users = set( - await refresh_token_storage.get_all_app_password_user_ids() - ) - else: - # OAuth mode: query refresh_tokens table - provisioned_users = set(await refresh_token_storage.get_all_user_ids()) + # Query the app_passwords table — background sync always + # authenticates as the user via locally-stored Nextcloud app + # passwords (Login Flow v2 / multi-user BasicAuth). + provisioned_users = set( + await refresh_token_storage.get_all_app_password_user_ids() + ) active_users = set(user_states.keys()) # Start scanners for new users new_users = provisioned_users - active_users for user_id in new_users: logger.info( - f"[{mode_label}] Starting scanner for newly provisioned user: {user_id}" + f"[BasicAuth] Starting scanner for newly provisioned user: {user_id}" ) cancel_scope = anyio.CancelScope() user_states[user_id] = UserSyncState( @@ -519,30 +405,26 @@ async def user_manager_task( send_stream, shutdown_event, wake_event, - token_broker, nextcloud_host, user_states, - use_basic_auth, # Positional after user_states ) # Cancel scanners for revoked users revoked_users = active_users - provisioned_users for user_id in revoked_users: - logger.info( - f"[{mode_label}] Stopping scanner for revoked user: {user_id}" - ) + logger.info(f"[BasicAuth] Stopping scanner for revoked user: {user_id}") state = user_states.get(user_id) if state: state.cancel_scope.cancel() # Note: state will be removed by _run_user_scanner_with_scope on exit if new_users: - logger.info(f"[{mode_label}] Started {len(new_users)} new scanner(s)") + logger.info(f"[BasicAuth] Started {len(new_users)} new scanner(s)") if revoked_users: - logger.info(f"[{mode_label}] Stopped {len(revoked_users)} scanner(s)") + logger.info(f"[BasicAuth] Stopped {len(revoked_users)} scanner(s)") except Exception as e: - logger.error(f"[{mode_label}] User manager error: {e}", exc_info=True) + logger.error(f"[BasicAuth] User manager error: {e}", exc_info=True) # Sleep until next poll try: @@ -553,9 +435,9 @@ async def user_manager_task( # Cancel all remaining scanners on shutdown logger.info( - f"[{mode_label}] User manager shutting down, cancelling {len(user_states)} scanner(s)" + f"[BasicAuth] User manager shutting down, cancelling {len(user_states)} scanner(s)" ) for state in list(user_states.values()): state.cancel_scope.cancel() - logger.info(f"[{mode_label}] User manager stopped") + logger.info("[BasicAuth] User manager stopped") diff --git a/tests/integration/test_app_password_provisioning.py b/tests/integration/test_app_password_provisioning.py index 0e6e427c..5cf59d7e 100644 --- a/tests/integration/test_app_password_provisioning.py +++ b/tests/integration/test_app_password_provisioning.py @@ -1,12 +1,12 @@ """Integration tests for app password provisioning via management API. -Tests the complete flow for multi-user BasicAuth mode: -1. User stores app password via management API endpoint +Tests the complete flow for multi-user BasicAuth and Login Flow v2 modes: +1. User stores app password via management API endpoint (or Login Flow v2 browser flow) 2. MCP server stores it locally (encrypted) 3. Background sync uses locally stored password to access Nextcloud -These tests verify that BasicAuth and OAuth are completely separate concerns -with no fallback between them. +The earlier OAuth refresh-token background-sync path was removed in the +ADR-022 cleanup — these tests now cover the only supported path. """ import tempfile @@ -18,9 +18,7 @@ from cryptography.fernet import Fernet from nextcloud_mcp_server.auth.storage import RefreshTokenStorage from nextcloud_mcp_server.vector.oauth_sync import ( NotProvisionedError, - get_user_client, get_user_client_basic_auth, - get_user_client_oauth, ) @@ -85,117 +83,6 @@ async def test_basic_auth_mode_raises_error_without_app_password(temp_storage): assert "test_user" in str(exc_info.value) -@pytest.mark.integration -async def test_get_user_client_dispatches_to_basic_auth(temp_storage, mocker): - """Test that get_user_client dispatches to BasicAuth mode correctly.""" - # Store an app password - await temp_storage.store_app_password("alice", "aaaaa-bbbbb-ccccc-ddddd-eeeee") - - # Mock RefreshTokenStorage.from_env at the source module - mocker.patch( - "nextcloud_mcp_server.auth.storage.RefreshTokenStorage.from_env", - return_value=temp_storage, - ) - # Also mock initialize since from_env returns an uninitialized instance - mocker.patch.object(temp_storage, "initialize", return_value=None) - - # Call get_user_client in BasicAuth mode - client = await get_user_client( - user_id="alice", - token_broker=None, # No token broker needed for BasicAuth mode - nextcloud_host="http://localhost:8080", - use_basic_auth=True, - ) - - # Verify client was created successfully - assert client is not None - assert client.username == "alice" - - -@pytest.mark.integration -async def test_oauth_mode_uses_refresh_token_only(mocker): - """Test that OAuth mode uses ONLY refresh tokens, NOT app passwords. - - In OAuth mode, app passwords are NOT used. - This is a complete separation of concerns. - """ - from nextcloud_mcp_server.auth.token_broker import TokenBrokerService - - # Mock TokenBrokerService to return an access token - mock_token_broker = mocker.AsyncMock(spec=TokenBrokerService) - mock_token_broker.get_background_token.return_value = "test-access-token" - - # Call get_user_client in OAuth mode - _client = await get_user_client( - user_id="test_user", - token_broker=mock_token_broker, - nextcloud_host="http://localhost:8080", - use_basic_auth=False, # OAuth mode - ) - - # Verify token broker was called - mock_token_broker.get_background_token.assert_called_once() - - -@pytest.mark.integration -async def test_oauth_mode_raises_error_without_token(mocker): - """Test that OAuth mode raises NotProvisionedError if no refresh token. - - There is NO fallback to app passwords - if no token, user must provision. - """ - from nextcloud_mcp_server.auth.token_broker import TokenBrokerService - - # Mock TokenBrokerService to return None (no token) - mock_token_broker = mocker.AsyncMock(spec=TokenBrokerService) - mock_token_broker.get_background_token.return_value = None - - # Call get_user_client in OAuth mode - should raise NotProvisionedError - with pytest.raises(NotProvisionedError) as exc_info: - await get_user_client( - user_id="test_user", - token_broker=mock_token_broker, - nextcloud_host="http://localhost:8080", - use_basic_auth=False, - ) - - # Verify error message mentions OAuth provisioning - assert "oauth" in str(exc_info.value).lower() - assert "test_user" in str(exc_info.value) - - -@pytest.mark.integration -async def test_get_user_client_oauth_function(mocker): - """Test the dedicated get_user_client_oauth function.""" - from nextcloud_mcp_server.auth.token_broker import TokenBrokerService - - # Mock TokenBrokerService - mock_token_broker = mocker.AsyncMock(spec=TokenBrokerService) - mock_token_broker.get_background_token.return_value = "test-bearer-token" - - # Call dedicated function - client = await get_user_client_oauth( - user_id="alice", - token_broker=mock_token_broker, - nextcloud_host="http://localhost:8080", - ) - - assert client is not None - assert client.username == "alice" - mock_token_broker.get_background_token.assert_called_once() - - -@pytest.mark.integration -async def test_oauth_mode_requires_token_broker(): - """Test that OAuth mode requires a token broker.""" - with pytest.raises(ValueError, match="token_broker required"): - await get_user_client( - user_id="test_user", - token_broker=None, # Missing token broker - nextcloud_host="http://localhost:8080", - use_basic_auth=False, # OAuth mode - ) - - @pytest.mark.integration async def test_multiple_users_basic_auth_mode(temp_storage, mocker): """Test that multiple users can be provisioned independently.""" From 4f73ee0750867772fb83b3086c968d46fc9fd5b3 Mon Sep 17 00:00:00 2001 From: "github-actions[bot]" Date: Tue, 12 May 2026 23:30:02 +0000 Subject: [PATCH 4/4] =?UTF-8?q?bump:=20version=200.86.1=20=E2=86=92=200.86?= =?UTF-8?q?.2?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- CHANGELOG.md | 6 ++++++ pyproject.toml | 2 +- uv.lock | 2 +- 3 files changed, 8 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 538781db..57cf590b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,12 @@ All notable changes to the Nextcloud MCP Server will be documented in this file. The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), and this project adheres to [PEP 440](https://peps.python.org/pep-0440/). +## v0.86.2 (2026-05-12) + +### Refactor + +- drop OAuth-refresh background-sync path from oauth_sync.py + ## v0.86.1 (2026-05-12) ### Refactor diff --git a/pyproject.toml b/pyproject.toml index 53237a3e..b6e92617 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -1,6 +1,6 @@ [project] name = "nextcloud-mcp-server" -version = "0.86.1" +version = "0.86.2" description = "Model Context Protocol (MCP) server for Nextcloud integration - enables AI assistants to interact with Nextcloud data" authors = [ {name = "Chris Coutinho", email = "chris@coutinho.io"} diff --git a/uv.lock b/uv.lock index ce2d7790..9b89d07d 100644 --- a/uv.lock +++ b/uv.lock @@ -2123,7 +2123,7 @@ wheels = [ [[package]] name = "nextcloud-mcp-server" -version = "0.86.1" +version = "0.86.2" source = { editable = "." } dependencies = [ { name = "aiosqlite" },