diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 6b498fa..2e9d687 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -45,6 +45,8 @@ jobs: --ignore=tests/integration/test_qdrant_collection_creation.py --ignore=tests/rag_evaluation/ + # NOTE: Playwright browser tests are skipped in CI (no browser grant flow). + # These entries still run non-Playwright tests marked with the same markers. - mode: oauth profile: oauth markers: "oauth and not keycloak" @@ -66,14 +68,16 @@ jobs: with: submodules: 'true' - # Build OIDC app (third_party is always mounted into the app container) + # Build OIDC app (only needed for oauth/login-flow modes) - name: Set up PHP 8.4 + if: matrix.needs-playwright uses: shivammathur/setup-php@44454db4f0199b8b9685a5d763dc37cbf79108e1 # 2.36.0 with: php-version: 8.4 coverage: none - name: Install OIDC app composer dependencies + if: matrix.needs-playwright run: | cd third_party/oidc composer install --no-dev @@ -162,6 +166,15 @@ jobs: --timeout=300 \ ${{ matrix.extra-args }} + - name: Report skipped Playwright tests + if: matrix.needs-playwright + run: | + echo "::notice::Playwright browser tests are skipped in CI. Run locally with: uv run pytest -m '${{ matrix.markers }}' --browser firefox" + uv run pytest --collect-only -q \ + -m '${{ matrix.markers }}' \ + ${{ matrix.extra-args }} 2>/dev/null \ + | tail -1 || true + - name: Show service logs on failure if: failure() run: docker compose --profile ${{ matrix.profile }} logs --tail=100 diff --git a/docker-compose.yml b/docker-compose.yml index 9e961be..9bd0a2e 100644 --- a/docker-compose.yml +++ b/docker-compose.yml @@ -145,6 +145,8 @@ services: - ENABLE_BACKGROUND_OPERATIONS=true # Token storage (required for middleware initialization) + # DEVELOPMENT ONLY - generate a fresh key for production: + # python -c "from cryptography.fernet import Fernet; print(Fernet.generate_key().decode())" - TOKEN_ENCRYPTION_KEY=ESF1BvEQdGYsCluwMx9Cxvw3uh5pFowPH7Rg_nIliyo= - TOKEN_STORAGE_DB=/app/data/tokens.db diff --git a/nextcloud_mcp_server/auth/scope_authorization.py b/nextcloud_mcp_server/auth/scope_authorization.py index a94ac51..5cdf4f1 100644 --- a/nextcloud_mcp_server/auth/scope_authorization.py +++ b/nextcloud_mcp_server/auth/scope_authorization.py @@ -1,7 +1,6 @@ """Scope-based authorization for MCP tools.""" import logging -import os from functools import wraps from typing import Any, Callable @@ -10,6 +9,7 @@ from mcp.server.auth.provider import AccessToken from mcp.server.fastmcp import Context from mcp.server.fastmcp.utilities.context_injection import find_context_parameter +from nextcloud_mcp_server.auth.storage import RefreshTokenStorage from nextcloud_mcp_server.config import get_settings logger = logging.getLogger(__name__) @@ -123,9 +123,8 @@ 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 - if os.getenv("NEXTCLOUD_APP_PASSWORD") or os.getenv( - "NEXTCLOUD_PASSWORD" - ): + 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)" ) @@ -142,7 +141,7 @@ def require_scopes(*required_scopes: str): # In Login Flow v2 multi-user mode, OAuth tokens provide MCP session # identity only. Nextcloud API access uses stored app passwords. # Check if the user has a stored app password with appropriate scopes. - if _is_login_flow_mode(): + if get_settings().enable_login_flow: from nextcloud_mcp_server.server.oauth_tools import ( # noqa: PLC0415 extract_user_id_from_token, ) @@ -479,19 +478,16 @@ def discover_all_scopes(mcp) -> list[str]: # ── Login Flow v2 helpers ──────────────────────────────────────────────── -def _is_login_flow_mode() -> bool: - """Check if server is configured for Login Flow v2 multi-user mode. +_scope_storage_instance = None - Login Flow v2 mode is active when: - - ENABLE_LOGIN_FLOW=true is set, OR - - Multi-user BasicAuth with offline access (uses stored app passwords) - Returns: - True if Login Flow v2 enforcement should be active - """ - if os.getenv("ENABLE_LOGIN_FLOW", "false").lower() == "true": - return True - return False +async def _get_scope_storage(): + """Get initialized storage instance for scope checks (lazy singleton).""" + global _scope_storage_instance + if _scope_storage_instance is None: + _scope_storage_instance = RefreshTokenStorage.from_env() + await _scope_storage_instance.initialize() + return _scope_storage_instance async def _get_stored_scopes(user_id: str) -> list[str] | str | None: @@ -502,11 +498,8 @@ async def _get_stored_scopes(user_id: str) -> list[str] | str | None: - "all": NULL scopes in DB (legacy = all allowed) - None: No stored app password (provisioning required) """ - from nextcloud_mcp_server.auth.storage import RefreshTokenStorage # noqa: PLC0415 - try: - storage = RefreshTokenStorage.from_env() - await storage.initialize() + storage = await _get_scope_storage() data = await storage.get_app_password_with_scopes(user_id) if data is None: diff --git a/nextcloud_mcp_server/context.py b/nextcloud_mcp_server/context.py index 3b6847f..3c3147b 100644 --- a/nextcloud_mcp_server/context.py +++ b/nextcloud_mcp_server/context.py @@ -1,7 +1,6 @@ """Helper functions for accessing context in MCP tools.""" import logging -import os from httpx import BasicAuth from mcp.server.fastmcp import Context @@ -11,6 +10,7 @@ from nextcloud_mcp_server.auth.context_helper import ( get_session_client_from_context, ) from nextcloud_mcp_server.auth.scope_authorization import ProvisioningRequiredError +from nextcloud_mcp_server.auth.storage import RefreshTokenStorage from nextcloud_mcp_server.client import NextcloudClient from nextcloud_mcp_server.config import ( DeploymentMode, @@ -82,7 +82,7 @@ async def get_client(ctx: Context) -> NextcloudClient: # Login Flow v2 multi-user mode: app password is REQUIRED for NC API access # OAuth token is only used for MCP session identity, not NC API calls - if hasattr(lifespan_ctx, "nextcloud_host") and _is_login_flow_mode(): + if hasattr(lifespan_ctx, "nextcloud_host") and settings.enable_login_flow: return await _get_client_from_login_flow(ctx, lifespan_ctx.nextcloud_host) # BasicAuth mode - use shared client (no token exchange) @@ -254,9 +254,16 @@ def _get_client_from_basic_auth(ctx: Context) -> NextcloudClient: ) -def _is_login_flow_mode() -> bool: - """Check if Login Flow v2 multi-user mode is active.""" - return os.getenv("ENABLE_LOGIN_FLOW", "false").lower() == "true" +_login_flow_storage_instance = None + + +async def _get_login_flow_storage(): + """Get initialized storage instance for login flow (lazy singleton).""" + global _login_flow_storage_instance + if _login_flow_storage_instance is None: + _login_flow_storage_instance = RefreshTokenStorage.from_env() + await _login_flow_storage_instance.initialize() + return _login_flow_storage_instance async def _get_client_from_login_flow( @@ -277,7 +284,6 @@ async def _get_client_from_login_flow( Raises: ProvisioningRequiredError: If no stored app password exists """ - from nextcloud_mcp_server.auth.storage import RefreshTokenStorage # noqa: PLC0415 from nextcloud_mcp_server.server.oauth_tools import ( # noqa: PLC0415 extract_user_id_from_token, ) @@ -288,8 +294,7 @@ async def _get_client_from_login_flow( "Cannot determine user identity from MCP token." ) - storage = RefreshTokenStorage.from_env() - await storage.initialize() + storage = await _get_login_flow_storage() app_data = await storage.get_app_password_with_scopes(user_id) if not app_data: diff --git a/nextcloud_mcp_server/server/auth_tools.py b/nextcloud_mcp_server/server/auth_tools.py index e3ace33..68373a5 100644 --- a/nextcloud_mcp_server/server/auth_tools.py +++ b/nextcloud_mcp_server/server/auth_tools.py @@ -28,11 +28,16 @@ from nextcloud_mcp_server.server.oauth_tools import extract_user_id_from_token logger = logging.getLogger(__name__) +_storage_instance: RefreshTokenStorage | None = None + + async def _get_storage() -> RefreshTokenStorage: - """Get initialized storage instance.""" - storage = RefreshTokenStorage.from_env() - await storage.initialize() - return storage + """Get initialized storage instance (lazy singleton).""" + global _storage_instance + if _storage_instance is None: + _storage_instance = RefreshTokenStorage.from_env() + await _storage_instance.initialize() + return _storage_instance def register_auth_tools(mcp) -> None: @@ -241,7 +246,12 @@ def register_auth_tools(mcp) -> None: if poll_result.status == "completed": # Store the app password with scopes - assert poll_result.app_password is not None + if poll_result.app_password is None: + return ProvisionStatusResponse( + status="error", + message="Login Flow completed but no app password was returned.", + success=False, + ) await storage.store_app_password_with_scopes( user_id=user_id, app_password=poll_result.app_password, @@ -287,8 +297,8 @@ def register_auth_tools(mcp) -> None: title="Update Nextcloud Access Scopes", description=( "Update the scopes for your Nextcloud access. " - "This revokes the current app password and starts a new Login Flow " - "with the combined scope set." + "This starts a new Login Flow with the combined scope set. " + "The current app password remains valid until the new one is obtained." ), annotations=ToolAnnotations( idempotentHint=False, @@ -357,11 +367,10 @@ def register_auth_tools(mcp) -> None: success=False, ) - # Delete existing app password from storage (user must revoke in NC Security settings) - if existing: - await storage.delete_app_password(user_id) - # Initiate new Login Flow v2 + # Note: existing app password stays valid until the new flow completes. + # store_app_password_with_scopes() does an upsert, so the old password + # is replaced atomically when nc_auth_check_status stores the new one. settings = get_settings() nextcloud_host = settings.nextcloud_host if not nextcloud_host: diff --git a/tests/unit/test_scope_authorization_stored.py b/tests/unit/test_scope_authorization_stored.py index 87d383e..0370b3b 100644 --- a/tests/unit/test_scope_authorization_stored.py +++ b/tests/unit/test_scope_authorization_stored.py @@ -4,41 +4,17 @@ Tests the third enforcement mode in scope_authorization.py that checks application-level scopes stored alongside app passwords. """ -import os from unittest.mock import AsyncMock, patch import pytest from nextcloud_mcp_server.auth.scope_authorization import ( _get_stored_scopes, - _is_login_flow_mode, ) pytestmark = pytest.mark.unit -def test_is_login_flow_mode_disabled(): - """Test that login flow mode is off by default.""" - with patch.dict(os.environ, {}, clear=True): - assert _is_login_flow_mode() is False - - -def test_is_login_flow_mode_enabled(): - """Test that login flow mode is enabled when env var is set.""" - with patch.dict(os.environ, {"ENABLE_LOGIN_FLOW": "true"}): - assert _is_login_flow_mode() is True - - -def test_is_login_flow_mode_case_insensitive(): - """Test case insensitivity of the env var.""" - with patch.dict(os.environ, {"ENABLE_LOGIN_FLOW": "True"}): - assert _is_login_flow_mode() is True - with patch.dict(os.environ, {"ENABLE_LOGIN_FLOW": "TRUE"}): - assert _is_login_flow_mode() is True - with patch.dict(os.environ, {"ENABLE_LOGIN_FLOW": "false"}): - assert _is_login_flow_mode() is False - - async def test_get_stored_scopes_with_scopes(): """Test getting specific scopes from storage.""" mock_storage = AsyncMock() @@ -49,10 +25,9 @@ async def test_get_stored_scopes_with_scopes(): "created_at": 1000, "updated_at": 1000, } - mock_storage.initialize = AsyncMock() with patch( - "nextcloud_mcp_server.auth.storage.RefreshTokenStorage.from_env", + "nextcloud_mcp_server.auth.scope_authorization._get_scope_storage", return_value=mock_storage, ): result = await _get_stored_scopes("alice") @@ -70,10 +45,9 @@ async def test_get_stored_scopes_null_scopes(): "created_at": 1000, "updated_at": 1000, } - mock_storage.initialize = AsyncMock() with patch( - "nextcloud_mcp_server.auth.storage.RefreshTokenStorage.from_env", + "nextcloud_mcp_server.auth.scope_authorization._get_scope_storage", return_value=mock_storage, ): result = await _get_stored_scopes("bob") @@ -85,10 +59,9 @@ async def test_get_stored_scopes_no_password(): """Test that missing app password returns None.""" mock_storage = AsyncMock() mock_storage.get_app_password_with_scopes.return_value = None - mock_storage.initialize = AsyncMock() with patch( - "nextcloud_mcp_server.auth.storage.RefreshTokenStorage.from_env", + "nextcloud_mcp_server.auth.scope_authorization._get_scope_storage", return_value=mock_storage, ): result = await _get_stored_scopes("nobody") @@ -99,10 +72,10 @@ async def test_get_stored_scopes_no_password(): async def test_get_stored_scopes_storage_error(): """Test that storage errors return None (fail-closed).""" mock_storage = AsyncMock() - mock_storage.initialize.side_effect = RuntimeError("DB error") + mock_storage.get_app_password_with_scopes.side_effect = RuntimeError("DB error") with patch( - "nextcloud_mcp_server.auth.storage.RefreshTokenStorage.from_env", + "nextcloud_mcp_server.auth.scope_authorization._get_scope_storage", return_value=mock_storage, ): result = await _get_stored_scopes("alice")