diff --git a/nextcloud_mcp_server/app.py b/nextcloud_mcp_server/app.py index 073181c..15b9e8b 100644 --- a/nextcloud_mcp_server/app.py +++ b/nextcloud_mcp_server/app.py @@ -1531,7 +1531,6 @@ def get_app(transport: str = "streamable-http", enabled_apps: list[str] | None = async def _login_flow_cleanup_loop() -> None: """Periodically clean up expired Login Flow v2 sessions.""" while True: - await anyio.sleep(3600) # Every hour try: storage = await get_shared_storage() count = await storage.delete_expired_login_flow_sessions() @@ -1539,6 +1538,18 @@ def get_app(transport: str = "streamable-http", enabled_apps: list[str] | None = logger.info(f"Cleaned up {count} expired login flow sessions") except Exception as e: logger.warning(f"Login flow cleanup error: {e}") + await anyio.sleep(3600) # Every hour + + @asynccontextmanager + async def _maybe_login_flow_cleanup(): + """Start Login Flow cleanup task if enabled.""" + if settings.enable_login_flow: + async with anyio.create_task_group() as tg: + tg.start_soon(_login_flow_cleanup_loop) + yield + tg.cancel_scope.cancel() + else: + yield @asynccontextmanager async def starlette_lifespan(app: Starlette): @@ -1772,13 +1783,10 @@ def get_app(transport: str = "streamable-http", enabled_apps: list[str] | None = f"{settings.vector_sync_processor_workers} processors" ) - # Start Login Flow cleanup task if enabled - if settings.enable_login_flow: - tg.start_soon(_login_flow_cleanup_loop) - # Run MCP session manager and yield async with AsyncExitStack() as stack: await stack.enter_async_context(mcp.session_manager.run()) + await stack.enter_async_context(_maybe_login_flow_cleanup()) try: yield finally: @@ -1959,13 +1967,10 @@ def get_app(transport: str = "streamable-http", enabled_apps: list[str] | None = f"{settings.vector_sync_processor_workers} processors" ) - # Start Login Flow cleanup task if enabled - if settings.enable_login_flow: - tg.start_soon(_login_flow_cleanup_loop) - # Run MCP session manager and yield async with AsyncExitStack() as stack: await stack.enter_async_context(mcp.session_manager.run()) + await stack.enter_async_context(_maybe_login_flow_cleanup()) try: yield finally: @@ -1986,12 +1991,7 @@ def get_app(transport: str = "streamable-http", enabled_apps: list[str] | None = # Just run MCP session manager without vector sync async with AsyncExitStack() as stack: await stack.enter_async_context(mcp.session_manager.run()) - if settings.enable_login_flow: - async with anyio.create_task_group() as cleanup_tg: - cleanup_tg.start_soon(_login_flow_cleanup_loop) - yield - cleanup_tg.cancel_scope.cancel() - else: + async with _maybe_login_flow_cleanup(): yield else: @@ -2013,12 +2013,7 @@ def get_app(transport: str = "streamable-http", enabled_apps: list[str] | None = ) async with AsyncExitStack() as stack: await stack.enter_async_context(mcp.session_manager.run()) - if settings.enable_login_flow: - async with anyio.create_task_group() as cleanup_tg: - cleanup_tg.start_soon(_login_flow_cleanup_loop) - yield - cleanup_tg.cancel_scope.cancel() - else: + async with _maybe_login_flow_cleanup(): yield # Health check endpoints for Kubernetes probes diff --git a/nextcloud_mcp_server/auth/elicitation.py b/nextcloud_mcp_server/auth/elicitation.py index 3c1284f..3b513cc 100644 --- a/nextcloud_mcp_server/auth/elicitation.py +++ b/nextcloud_mcp_server/auth/elicitation.py @@ -70,7 +70,9 @@ async def present_login_url( logger.info("User cancelled login flow") return "cancelled" - except Exception as e: - # Elicitation not supported by this client - fall back to message - logger.debug(f"Elicitation not available ({e}), returning URL in message") + except (AttributeError, NotImplementedError) as e: + # Elicitation not supported by this client/SDK - fall back to message + logger.debug( + f"Elicitation not available ({type(e).__name__}: {e}), returning URL in message" + ) return "message_only" diff --git a/nextcloud_mcp_server/auth/login_flow.py b/nextcloud_mcp_server/auth/login_flow.py index 4ea5a55..d13e8d5 100644 --- a/nextcloud_mcp_server/auth/login_flow.py +++ b/nextcloud_mcp_server/auth/login_flow.py @@ -90,11 +90,16 @@ class LoginFlowV2Client: poll_data = data.get("poll", {}) - result = LoginFlowInitResponse( - login_url=data["login"], - poll_endpoint=poll_data["endpoint"], - poll_token=poll_data["token"], - ) + try: + result = LoginFlowInitResponse( + login_url=data["login"], + poll_endpoint=poll_data["endpoint"], + poll_token=poll_data["token"], + ) + except KeyError as e: + raise ValueError( + f"Malformed Login Flow v2 initiate response from Nextcloud (missing key: {e})" + ) from e logger.info(f"Login Flow v2 initiated: login_url={result.login_url[:60]}...") return result @@ -129,12 +134,17 @@ class LoginFlowV2Client: f"Login Flow v2 completed: server={data.get('server')}, " f"loginName={data.get('loginName')}" ) - return LoginFlowPollResult( - status="completed", - server=data["server"], - login_name=data["loginName"], - app_password=data["appPassword"], - ) + try: + return LoginFlowPollResult( + status="completed", + server=data["server"], + login_name=data["loginName"], + app_password=data["appPassword"], + ) + except KeyError as e: + raise ValueError( + f"Malformed Login Flow v2 poll response from Nextcloud (missing key: {e})" + ) from e if response.status_code == 404: logger.debug("Login Flow v2 still pending") diff --git a/nextcloud_mcp_server/auth/scope_authorization.py b/nextcloud_mcp_server/auth/scope_authorization.py index f7f189f..17bc716 100644 --- a/nextcloud_mcp_server/auth/scope_authorization.py +++ b/nextcloud_mcp_server/auth/scope_authorization.py @@ -128,20 +128,17 @@ def require_scopes(*required_scopes: str): ) if access_token is None: - # Check if single-user BasicAuth mode (env var app password) - # If NEXTCLOUD_APP_PASSWORD or NEXTCLOUD_PASSWORD is set, bypass scope checks + # No OAuth token — either BasicAuth with env var credentials + # or BasicAuth without explicit credentials. Both bypass scope checks. settings = get_settings() if settings.nextcloud_app_password or settings.nextcloud_password: logger.debug( f"No access token for {func_name} - allowing (env var app password)" ) - return await func(*args, **kwargs) - - # Not in OAuth mode (BasicAuth or no auth) - # In BasicAuth mode, all operations are allowed - logger.debug( - f"No access token present for {func_name} - allowing (BasicAuth mode)" - ) + else: + logger.debug( + f"No access token present for {func_name} - allowing (BasicAuth mode)" + ) return await func(*args, **kwargs) # ── Login Flow v2: Check stored app password scopes ── diff --git a/nextcloud_mcp_server/auth/storage.py b/nextcloud_mcp_server/auth/storage.py index b022c76..848a5c0 100644 --- a/nextcloud_mcp_server/auth/storage.py +++ b/nextcloud_mcp_server/auth/storage.py @@ -1779,7 +1779,7 @@ class RefreshTokenStorage: logger.error( f"Failed to retrieve login flow session for user {user_id}: {e}" ) - return None + raise async def delete_login_flow_session(self, user_id: str) -> bool: """Delete a Login Flow v2 session. @@ -1861,7 +1861,7 @@ class RefreshTokenStorage: _shared_instance: RefreshTokenStorage | None = None -_shared_lock = anyio.Lock() +_shared_lock: anyio.Lock | None = None async def get_shared_storage() -> RefreshTokenStorage: @@ -1871,7 +1871,9 @@ async def get_shared_storage() -> RefreshTokenStorage: creating their own lazy singletons. The lock ensures thread-safe initialization on concurrent first-access. """ - global _shared_instance + global _shared_instance, _shared_lock + if _shared_lock is None: + _shared_lock = anyio.Lock() async with _shared_lock: if _shared_instance is None: _shared_instance = RefreshTokenStorage.from_env() diff --git a/nextcloud_mcp_server/models/auth.py b/nextcloud_mcp_server/models/auth.py index 97b1982..e269306 100644 --- a/nextcloud_mcp_server/models/auth.py +++ b/nextcloud_mcp_server/models/auth.py @@ -9,7 +9,7 @@ class ProvisionAccessResponse(BaseResponse): """Response from nc_auth_provision_access tool.""" status: str = Field( - description="Provisioning status: 'login_required', 'already_provisioned', 'error'" + description="Provisioning status: 'login_required', 'already_provisioned', 'declined', 'cancelled', 'error'" ) login_url: str | None = Field( None, description="URL to open in browser for Nextcloud login" @@ -38,7 +38,9 @@ class ProvisionStatusResponse(BaseResponse): class UpdateScopesResponse(BaseResponse): """Response from nc_auth_update_scopes tool.""" - status: str = Field(description="Status: 'login_required', 'updated', 'error'") + status: str = Field( + description="Status: 'login_required', 'unchanged', 'declined', 'cancelled', 'error'" + ) login_url: str | None = Field( None, description="URL for re-provisioning with new scopes" ) diff --git a/nextcloud_mcp_server/server/auth_tools.py b/nextcloud_mcp_server/server/auth_tools.py index 45d83ba..dc6dffd 100644 --- a/nextcloud_mcp_server/server/auth_tools.py +++ b/nextcloud_mcp_server/server/auth_tools.py @@ -212,7 +212,16 @@ def register_auth_tools(mcp: FastMCP) -> None: ) # Check for pending login flow session - session = await storage.get_login_flow_session(user_id) + try: + session = await storage.get_login_flow_session(user_id) + except Exception as e: + logger.error(f"Failed to check login flow session for {user_id}: {e}") + return ProvisionStatusResponse( + status="error", + message=f"Failed to check login flow session: {e}", + user_id=user_id, + success=False, + ) if not session: return ProvisionStatusResponse( status="not_initiated", diff --git a/tests/server/login_flow/conftest.py b/tests/server/login_flow/conftest.py index 2d63e99..eb3d0f7 100644 --- a/tests/server/login_flow/conftest.py +++ b/tests/server/login_flow/conftest.py @@ -14,7 +14,7 @@ import os import secrets import time from typing import Any, AsyncGenerator -from urllib.parse import quote +from urllib.parse import quote, urlparse, urlunparse import anyio import httpx @@ -214,10 +214,11 @@ def _rewrite_login_flow_url(login_url: str) -> str: the host and needs localhost:8080 instead. """ nextcloud_host = os.getenv("NEXTCLOUD_HOST", "http://localhost:8080") - # Replace common internal Docker hostnames - url = login_url.replace("http://app:80", nextcloud_host) - url = url.replace("http://app", nextcloud_host) - return url + target = urlparse(nextcloud_host) + parsed = urlparse(login_url) + if parsed.hostname == "app": + parsed = parsed._replace(scheme=target.scheme, netloc=target.netloc) + return urlunparse(parsed) async def _complete_login_flow_v2(browser, login_url: str) -> None: