fix: address remaining PR #589 review findings

- Consolidate MCP session + login flow cleanup into _mcp_session_with_login_flow() helper,
  replacing 4 duplicated AsyncExitStack sites in app.py
- Fix get_shared_storage() race condition by using module-level anyio.Lock() init
  (reverts regression from ba59763)
- Collapse cosmetic if/else branching in scope_authorization.py
- Consolidate dual password storage paths into single store_app_password_with_scopes() call
- Mark unused request param as _ in list_supported_scopes
- Make ALL_SUPPORTED_SCOPES an immutable tuple; use list() instead of .copy()
- Add hasattr(ctx, "elicit") guard in elicitation.py, narrow except to NotImplementedError
- Add YAML comment explaining --oauth flag for mcp-login-flow service

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

docker-compose.yml

@@ -292,6 +292,8 @@ services:
   mcp-login-flow:
     build: .
     restart: always
+    # --oauth enables the OAuth/OIDC identity layer that Login Flow v2 builds on
+    # (user identity via OAuth session, Nextcloud access via app passwords)
     command: ["--transport", "streamable-http", "--oauth", "--port", "8004"]
     depends_on:
       app:

nextcloud_mcp_server/api/access.py

@@ -154,7 +154,7 @@ async def update_user_scopes(request: Request) -> JSONResponse:
         )
 
 
-async def list_supported_scopes(request: Request) -> JSONResponse:
+async def list_supported_scopes(_: Request) -> JSONResponse:
     """GET /api/v1/scopes - List all supported application-level scopes."""
     return JSONResponse(
         {

nextcloud_mcp_server/api/passwords.py

@@ -302,14 +302,9 @@ async def provision_app_password(request: Request) -> JSONResponse:
     try:
         storage = await _get_app_password_storage(request)
 
-        if scopes is not None or nc_username is not None:
+        await storage.store_app_password_with_scopes(
-            # New path: store with scopes and username
+            username, app_password, scopes=scopes, username=nc_username
-            await storage.store_app_password_with_scopes(
+        )
-                username, app_password, scopes=scopes, username=nc_username
-            )
-        else:
-            # Legacy path: store without scopes
-            await storage.store_app_password(username, app_password)
 
         _record_rate_limit_attempt(path_user_id, success=True)
         logger.info(f"Provisioned app password for user: {username}")

nextcloud_mcp_server/app.py

@@ -1551,6 +1551,14 @@ def get_app(transport: str = "streamable-http", enabled_apps: list[str] | None =
         else:
             yield
 
+    @asynccontextmanager
+    async def _mcp_session_with_login_flow():
+        """Start MCP session manager with optional Login Flow cleanup."""
+        async with AsyncExitStack() as stack:
+            await stack.enter_async_context(mcp.session_manager.run())
+            await stack.enter_async_context(_maybe_login_flow_cleanup())
+            yield
+
     @asynccontextmanager
     async def starlette_lifespan(app: Starlette):
         # Set OAuth context for OAuth login routes (ADR-004)
@@ -1784,9 +1792,7 @@ def get_app(transport: str = "streamable-http", enabled_apps: list[str] | None =
                 )
 
                 # Run MCP session manager and yield
-                async with AsyncExitStack() as stack:
+                async with _mcp_session_with_login_flow():
-                    await stack.enter_async_context(mcp.session_manager.run())
-                    await stack.enter_async_context(_maybe_login_flow_cleanup())
                     try:
                         yield
                     finally:
@@ -1968,9 +1974,7 @@ def get_app(transport: str = "streamable-http", enabled_apps: list[str] | None =
                     )
 
                     # Run MCP session manager and yield
-                    async with AsyncExitStack() as stack:
+                    async with _mcp_session_with_login_flow():
-                        await stack.enter_async_context(mcp.session_manager.run())
-                        await stack.enter_async_context(_maybe_login_flow_cleanup())
                         try:
                             yield
                         finally:
@@ -1989,10 +1993,8 @@ def get_app(transport: str = "streamable-http", enabled_apps: list[str] | None =
                     "To enable, set NEXTCLOUD_OIDC_CLIENT_ID and NEXTCLOUD_OIDC_CLIENT_SECRET."
                 )
                 # Just run MCP session manager without vector sync
-                async with AsyncExitStack() as stack:
+                async with _mcp_session_with_login_flow():
-                    await stack.enter_async_context(mcp.session_manager.run())
+                    yield
-                    async with _maybe_login_flow_cleanup():
-                        yield
 
         else:
             # No vector sync - just run MCP session manager
@@ -2011,10 +2013,8 @@ def get_app(transport: str = "streamable-http", enabled_apps: list[str] | None =
                     logger.warning(
                         "Vector sync enabled but TOKEN_ENCRYPTION_KEY not set"
                     )
-            async with AsyncExitStack() as stack:
+            async with _mcp_session_with_login_flow():
-                await stack.enter_async_context(mcp.session_manager.run())
+                yield
-                async with _maybe_login_flow_cleanup():
-                    yield
 
     # Health check endpoints for Kubernetes probes
     def health_live(request):

nextcloud_mcp_server/auth/elicitation.py

@@ -49,6 +49,12 @@ async def present_login_url(
             f"Then check the box below and click OK."
         )
 
+    if not hasattr(ctx, "elicit"):
+        logger.debug(
+            "Elicitation not available (no elicit method), returning URL in message"
+        )
+        return "message_only"
+
     try:
         result = await ctx.elicit(
             message=message,
@@ -70,7 +76,7 @@ async def present_login_url(
             logger.info("User cancelled login flow")
             return "cancelled"
 
-    except (AttributeError, NotImplementedError) as e:
+    except 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"

nextcloud_mcp_server/auth/scope_authorization.py

@@ -128,17 +128,10 @@ def require_scopes(*required_scopes: str):
             )
 
             if access_token is None:
-                # No OAuth token — either BasicAuth with env var credentials
+                # No OAuth token — BasicAuth mode bypasses scope checks
-                # or BasicAuth without explicit credentials. Both bypass scope checks.
+                logger.debug(
-                settings = get_settings()
+                    f"No access token for {func_name} - allowing (BasicAuth mode)"
-                if settings.nextcloud_app_password or settings.nextcloud_password:
+                )
-                    logger.debug(
-                        f"No access token for {func_name} - allowing (env var app password)"
-                    )
-                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 ──

nextcloud_mcp_server/auth/storage.py

@@ -1861,7 +1861,7 @@ class RefreshTokenStorage:
 
 
 _shared_instance: RefreshTokenStorage | None = None
-_shared_lock: anyio.Lock | None = None
+_shared_lock: anyio.Lock = anyio.Lock()
 
 
 async def get_shared_storage() -> RefreshTokenStorage:
@@ -1871,9 +1871,7 @@ async def get_shared_storage() -> RefreshTokenStorage:
     creating their own lazy singletons. The lock ensures thread-safe
     initialization on concurrent first-access.
     """
-    global _shared_instance, _shared_lock
+    global _shared_instance
-    if _shared_lock is None:
-        _shared_lock = anyio.Lock()
     async with _shared_lock:
         if _shared_instance is None:
             _shared_instance = RefreshTokenStorage.from_env()

nextcloud_mcp_server/models/auth.py

@@ -52,7 +52,7 @@ class UpdateScopesResponse(BaseResponse):
 
 
 # All supported application-level scopes
-ALL_SUPPORTED_SCOPES = [
+ALL_SUPPORTED_SCOPES = (
     "notes:read",
     "notes:write",
     "calendar:read",
@@ -73,4 +73,4 @@ ALL_SUPPORTED_SCOPES = [
     "sharing:write",
     "news:read",
     "news:write",
-]
+)

nextcloud_mcp_server/server/auth_tools.py

@@ -84,7 +84,7 @@ def register_auth_tools(mcp: FastMCP) -> None:
             )
 
         # Determine scopes
-        requested_scopes = scopes if scopes else ALL_SUPPORTED_SCOPES.copy()
+        requested_scopes = scopes if scopes else list(ALL_SUPPORTED_SCOPES)
 
         # Validate requested scopes
         invalid_scopes = [s for s in requested_scopes if s not in ALL_SUPPORTED_SCOPES]