diff --git a/nextcloud_mcp_server/app.py b/nextcloud_mcp_server/app.py index 9fcb49f..3873de4 100644 --- a/nextcloud_mcp_server/app.py +++ b/nextcloud_mcp_server/app.py @@ -1826,6 +1826,19 @@ def get_app(transport: str = "streamable-http", enabled_apps: list[str] | None = f"Cannot start vector sync - Qdrant initialization failed: {e}" ) from e + # Clean up stale app passwords at startup (BasicAuth mode only) + if not oauth_enabled: + try: + removed = await token_storage.cleanup_invalid_app_passwords( + nextcloud_host=nextcloud_host_for_sync + ) + if removed: + logger.info( + f"Cleaned up {len(removed)} stale app password(s): {removed}" + ) + except Exception as e: + logger.warning(f"App password cleanup failed (non-fatal): {e}") + # Initialize shared state send_stream, receive_stream = anyio.create_memory_object_stream( max_buffer_size=settings.vector_sync_queue_max_size diff --git a/nextcloud_mcp_server/auth/storage.py b/nextcloud_mcp_server/auth/storage.py index 4424ce8..fe42cc6 100644 --- a/nextcloud_mcp_server/auth/storage.py +++ b/nextcloud_mcp_server/auth/storage.py @@ -1414,6 +1414,67 @@ class RefreshTokenStorage: logger.debug(f"Found {len(user_ids)} users with app passwords") return user_ids + async def cleanup_invalid_app_passwords(self, nextcloud_host: str) -> list[str]: + """ + Validate stored app passwords against Nextcloud and remove invalid ones. + + Makes a lightweight OCS request for each stored user to check if credentials + are still valid. Removes entries that return 401/403. + + Args: + nextcloud_host: Nextcloud base URL + + Returns: + List of user IDs whose app passwords were removed + """ + import httpx + + if not self._initialized: + await self.initialize() + + user_ids = await self.get_all_app_password_user_ids() + if not user_ids: + return [] + + removed: list[str] = [] + + for user_id in user_ids: + app_password = await self.get_app_password(user_id) + if not app_password: + continue + + try: + async with httpx.AsyncClient( + base_url=nextcloud_host, + auth=httpx.BasicAuth(user_id, app_password), + timeout=10.0, + ) as client: + response = await client.get( + "/ocs/v2.php/cloud/user", + headers={ + "OCS-APIRequest": "true", + "Accept": "application/json", + }, + ) + + if response.status_code in (401, 403): + logger.info( + f"App password for {user_id} is invalid " + f"(HTTP {response.status_code}), removing" + ) + await self.delete_app_password(user_id) + removed.append(user_id) + else: + logger.debug( + f"App password for {user_id} validated " + f"(HTTP {response.status_code})" + ) + + except Exception as e: + logger.warning(f"Could not validate app password for {user_id}: {e}") + + return removed + async def generate_encryption_key() -> str: """ diff --git a/nextcloud_mcp_server/vector/oauth_sync.py b/nextcloud_mcp_server/vector/oauth_sync.py index c85e4ee..f69b747 100644 --- a/nextcloud_mcp_server/vector/oauth_sync.py +++ b/nextcloud_mcp_server/vector/oauth_sync.py @@ -31,7 +31,7 @@ from anyio.streams.memory import ( MemoryObjectReceiveStream, MemoryObjectSendStream, ) -from httpx import BasicAuth +from httpx import BasicAuth, HTTPStatusError from nextcloud_mcp_server.client import NextcloudClient from nextcloud_mcp_server.config import get_settings @@ -210,9 +210,36 @@ async def user_scanner_task( mode_label = "BasicAuth" if use_basic_auth else "OAuth" logger.info(f"[{mode_label}] Scanner started for user: {user_id}") settings = get_settings() + max_consecutive_errors = 5 task_status.started() + # 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 + ) + try: + await nc_client.capabilities() # Lightweight OCS call to validate creds + logger.info(f"[{mode_label}] 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"(HTTP {e.response.status_code}), not starting scan loop" + ) + return + raise + finally: + await nc_client.close() + except NotProvisionedError: + logger.warning( + f"[{mode_label}] User {user_id} not provisioned, not starting scan loop" + ) + return + + consecutive_errors = 0 + while not shutdown_event.is_set(): nc_client = None try: @@ -228,21 +255,61 @@ async def user_scanner_task( nc_client=nc_client, ) + consecutive_errors = 0 # Reset on success + except NotProvisionedError: logger.warning( f"[{mode_label}] User {user_id} no longer provisioned, stopping scanner" ) break + except HTTPStatusError as e: + status_code = e.response.status_code + if status_code in (401, 403): + logger.warning( + f"[{mode_label}] Scanner auth failed for {user_id} " + f"(HTTP {status_code}), stopping scanner. " + f"User may need to re-provision credentials." + ) + break + elif status_code == 429: + logger.warning( + f"[{mode_label}] Scanner rate-limited for {user_id}, " + f"backing off 60s" + ) + try: + with anyio.move_on_after(60): + await shutdown_event.wait() + except anyio.get_cancelled_exc_class(): + break + continue + else: + consecutive_errors += 1 + logger.error( + f"[{mode_label}] Scanner HTTP error for {user_id}: {e} " + f"({consecutive_errors}/{max_consecutive_errors})", + exc_info=True, + ) + except Exception as e: + consecutive_errors += 1 logger.error( - f"[{mode_label}] Scanner error for {user_id}: {e}", exc_info=True + f"[{mode_label}] Scanner error for {user_id}: {e} " + f"({consecutive_errors}/{max_consecutive_errors})", + exc_info=True, ) finally: if nc_client: await nc_client.close() + if consecutive_errors >= max_consecutive_errors: + logger.error( + f"[{mode_label}] Scanner for {user_id} hit {max_consecutive_errors} " + f"consecutive errors, stopping scanner" + ) + break + # Sleep until next interval or wake event try: with anyio.move_on_after(settings.vector_sync_scan_interval): diff --git a/tests/conftest.py b/tests/conftest.py index 61d953d..868051e 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -2400,6 +2400,30 @@ async def test_users_setup(anyio_backend, nc_client: NextcloudClient): except Exception as e: logger.warning(f"Error deleting test user {username}: {e}") + # Clean up app passwords from MCP server to prevent stale scanners + for username in created_users: + try: + import subprocess + + subprocess.run( + [ + "docker", + "compose", + "exec", + "-T", + "mcp-multi-user-basic", + "sqlite3", + "/app/data/tokens.db", + f"DELETE FROM app_passwords WHERE user_id = '{username}';", + ], + capture_output=True, + text=True, + timeout=10, + ) + logger.info(f"Cleaned up app password for {username}") + except Exception as e: + logger.debug(f"App password cleanup for {username}: {e}") + async def _get_oauth_token_for_user( browser, diff --git a/tests/integration/test_astrolabe_multi_user_background_sync.py b/tests/integration/test_astrolabe_multi_user_background_sync.py index 0a3c56a..cc70293 100644 --- a/tests/integration/test_astrolabe_multi_user_background_sync.py +++ b/tests/integration/test_astrolabe_multi_user_background_sync.py @@ -861,6 +861,43 @@ async def test_multi_user_astrolabe_background_sync_enablement( This tests ADR-002 Tier 2 authentication: User-specific app passwords for background operations in multi-user BasicAuth deployments. """ + # Clear stale state from previous test runs + logger.info("Clearing stale app passwords and bruteforce entries...") + subprocess.run( + [ + "docker", + "compose", + "exec", + "-T", + "mcp-multi-user-basic", + "sqlite3", + "/app/data/tokens.db", + "DELETE FROM app_passwords;", + ], + capture_output=True, + text=True, + timeout=10, + ) + subprocess.run( + [ + "docker", + "compose", + "exec", + "-T", + "db", + "mariadb", + "-u", + "root", + "-ppassword", + "nextcloud", + "-e", + "DELETE FROM oc_bruteforce_attempts;", + ], + capture_output=True, + text=True, + timeout=10, + ) + # Configure Astrolabe to point to the mcp-multi-user-basic server logger.info("Configuring Astrolabe for mcp-multi-user-basic server...") await configure_astrolabe_for_mcp_server( @@ -1198,6 +1235,64 @@ async def test_revoke_background_sync_access( This tests the fix for the issue where POST requests to the revoke endpoint were returning errors due to HTTP method mismatch (was DELETE, now POST). """ + # Clear stale state from previous test runs + logger.info( + "Clearing stale app passwords, bruteforce entries, and Astrolabe preferences..." + ) + subprocess.run( + [ + "docker", + "compose", + "exec", + "-T", + "mcp-multi-user-basic", + "sqlite3", + "/app/data/tokens.db", + "DELETE FROM app_passwords;", + ], + capture_output=True, + text=True, + timeout=10, + ) + subprocess.run( + [ + "docker", + "compose", + "exec", + "-T", + "db", + "mariadb", + "-u", + "root", + "-ppassword", + "nextcloud", + "-e", + "DELETE FROM oc_bruteforce_attempts;", + ], + capture_output=True, + text=True, + timeout=10, + ) + subprocess.run( + [ + "docker", + "compose", + "exec", + "-T", + "db", + "mariadb", + "-u", + "root", + "-ppassword", + "nextcloud", + "-e", + "DELETE FROM oc_preferences WHERE appid = 'astrolabe';", + ], + capture_output=True, + text=True, + timeout=10, + ) + # Configure Astrolabe to point to the mcp-multi-user-basic server logger.info("Configuring Astrolabe for mcp-multi-user-basic server...") await configure_astrolabe_for_mcp_server( @@ -1218,9 +1313,14 @@ async def test_revoke_background_sync_access( # Step 1: Login to Nextcloud await login_to_nextcloud(page, username, password) - # Step 2: Generate app password and enable background sync - app_password = await generate_app_password(page, username) - await enable_background_sync_via_app_password(page, username, app_password) + # Step 2: Complete full authorization (OAuth Step 1 + App Password Step 2) + auth_result = await complete_astrolabe_authorization(page, username, password) + assert auth_result["step1"], ( + f"OAuth authorization (Step 1) failed for {username}" + ) + assert auth_result["step2"], ( + f"App password setup (Step 2) failed for {username}" + ) # Step 3: Verify background sync is enabled assert await verify_app_password_created(username), (