diff --git a/CLAUDE.md b/CLAUDE.md index 96fb337c..332bb36f 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -338,7 +338,7 @@ The server supports three deployment modes, controlled by environment variables - Best for: personal instances, local development **2. Multi-User BasicAuth** (profile: `multi-user-basic`) -- Set `ENABLE_MULTI_USER_BASIC_AUTH=true` +- Set `MCP_DEPLOYMENT_MODE=multi_user_basic` - Each MCP client provides credentials via HTTP Authorization header - Per-request client creation from extracted credentials - Best for: internal deployments where users manage their own Nextcloud credentials diff --git a/docker-compose.yml b/docker-compose.yml index e6e237de..041da0ff 100644 --- a/docker-compose.yml +++ b/docker-compose.yml @@ -146,11 +146,11 @@ services: ports: - 127.0.0.1:8003:8000 environment: - # Multi-user BasicAuth pass-through mode (ADR-020) + # Multi-user BasicAuth pass-through mode (ADR-020, ADR-022) - NEXTCLOUD_HOST=http://app:80 - NEXTCLOUD_MCP_SERVER_URL=http://localhost:8003 - NEXTCLOUD_PUBLIC_ISSUER_URL=http://localhost:8080 - - ENABLE_MULTI_USER_BASIC_AUTH=true + - MCP_DEPLOYMENT_MODE=multi_user_basic - ENABLE_BACKGROUND_OPERATIONS=true # Token storage (required for middleware initialization). diff --git a/docs/ADR-020-deployment-modes-and-configuration-validation.md b/docs/ADR-020-deployment-modes-and-configuration-validation.md index f81215c7..861bb823 100644 --- a/docs/ADR-020-deployment-modes-and-configuration-validation.md +++ b/docs/ADR-020-deployment-modes-and-configuration-validation.md @@ -1,9 +1,9 @@ # ADR-020: Deployment Modes and Configuration Validation -**Status:** Accepted +**Status:** Accepted — partly superseded by ADR-022 (`oauth_single_audience` renamed to `login_flow`; the `ENABLE_MULTI_USER_BASIC_AUTH` and `ENABLE_LOGIN_FLOW` env-var aliases were removed in favour of `MCP_DEPLOYMENT_MODE` as the single source of truth) **Date:** 2025-12-20 **Deciders:** Development Team -**Related:** ADR-002 (Vector Sync), ADR-004 (Progressive Consent), ADR-019 (Multi-user BasicAuth) +**Related:** ADR-002 (Vector Sync), ADR-004 (Progressive Consent), ADR-019 (Multi-user BasicAuth), ADR-022 (Deployment Mode Consolidation) ## Context diff --git a/docs/ADR-025-dynaconf-configuration-management.md b/docs/ADR-025-dynaconf-configuration-management.md index c4974c29..b905335f 100644 --- a/docs/ADR-025-dynaconf-configuration-management.md +++ b/docs/ADR-025-dynaconf-configuration-management.md @@ -38,7 +38,7 @@ The nextcloud-mcp-server configuration system has grown to ~80+ environment vari |----------|-------------|---------| | Core Nextcloud | 6 | `NEXTCLOUD_HOST`, `NEXTCLOUD_USERNAME`, `NEXTCLOUD_VERIFY_SSL` | | OAuth/OIDC | 12 | `OIDC_DISCOVERY_URL`, `NEXTCLOUD_OIDC_CLIENT_ID`, `JWKS_URI` | -| Mode Selection | 2 | `MCP_DEPLOYMENT_MODE`, `ENABLE_MULTI_USER_BASIC_AUTH` | +| Mode Selection | 1 | `MCP_DEPLOYMENT_MODE` | | Token Storage | 3 | `TOKEN_ENCRYPTION_KEY`, `TOKEN_STORAGE_DB` | | Semantic Search | 6 | `ENABLE_SEMANTIC_SEARCH`, `VECTOR_SYNC_SCAN_INTERVAL` | | Qdrant | 4 | `QDRANT_URL`, `QDRANT_LOCATION`, `QDRANT_API_KEY` | @@ -114,9 +114,10 @@ nextcloud_ca_bundle = "@none" # mcp_deployment_mode = "" # === Authentication Toggles === -enable_multi_user_basic_auth = false -# `enable_login_flow` is derived from MCP_DEPLOYMENT_MODE=login_flow in -# detect_auth_mode (ADR-022 follow-up) — no separate toggle. +# Both `enable_multi_user_basic_auth` and `enable_login_flow` are derived +# from MCP_DEPLOYMENT_MODE in detect_auth_mode (ADR-022 follow-up) — no +# separate toggles. Only ENABLE_TOKEN_EXCHANGE remains as an independent +# flag (separate cleanup). enable_token_exchange = false # === Token Storage === @@ -199,7 +200,7 @@ nextcloud_mcp_port = 8000 # nextcloud_password = "" (in .secrets.toml) [multi_user_basic] -enable_multi_user_basic_auth = true +# enable_multi_user_basic_auth is now derived from the mode (ADR-022 follow-up). token_storage_db = "/app/data/tokens.db" [login_flow] @@ -344,10 +345,16 @@ In **Phase 4**, this could migrate to a post-hook: # Phase 4 target (not implemented in Phases 1-3) def resolve_dependencies(settings): """Auto-enable background operations for semantic search in multi-user modes.""" + mode = (settings.get("MCP_DEPLOYMENT_MODE", "") or "").lower().strip() is_multi_user = ( - settings.get("ENABLE_MULTI_USER_BASIC_AUTH", False) + mode in {"multi_user_basic", "login_flow"} or settings.get("ENABLE_TOKEN_EXCHANGE", False) - or (not settings.get("NEXTCLOUD_USERNAME") and not settings.get("NEXTCLOUD_PASSWORD")) + or ( + mode != "single_user_basic" + and not ( + settings.get("NEXTCLOUD_USERNAME") and settings.get("NEXTCLOUD_PASSWORD") + ) + ) ) if settings.get("ENABLE_SEMANTIC_SEARCH", False) and is_multi_user: if not settings.get("ENABLE_BACKGROUND_OPERATIONS", False): diff --git a/docs/auth-flows.md b/docs/auth-flows.md index 1b89372b..7997c6f4 100644 --- a/docs/auth-flows.md +++ b/docs/auth-flows.md @@ -221,7 +221,7 @@ NEXTCLOUD_PASSWORD= ### Multi-User BasicAuth ```bash NEXTCLOUD_HOST=https://nextcloud.example.com -ENABLE_MULTI_USER_BASIC_AUTH=true +MCP_DEPLOYMENT_MODE=multi_user_basic # Optional: app-password storage for background sync TOKEN_ENCRYPTION_KEY= diff --git a/docs/authentication.md b/docs/authentication.md index 9eb402c8..25c059a9 100644 --- a/docs/authentication.md +++ b/docs/authentication.md @@ -43,7 +43,7 @@ Each MCP client sends its own credentials in an HTTP `Authorization: Basic` head ```bash NEXTCLOUD_HOST=https://your.nextcloud.example.com -ENABLE_MULTI_USER_BASIC_AUTH=true +MCP_DEPLOYMENT_MODE=multi_user_basic ``` `NEXTCLOUD_USERNAME` and `NEXTCLOUD_PASSWORD` must NOT be set in this mode. @@ -74,7 +74,7 @@ The server detects the active mode from environment variables at startup: | Env vars present | Detected mode | |------------------|---------------| | `NEXTCLOUD_USERNAME` + `NEXTCLOUD_PASSWORD` | Single-User (BasicAuth) | -| `ENABLE_MULTI_USER_BASIC_AUTH=true` (no creds) | Multi-User (BasicAuth pass-through) | +| `MCP_DEPLOYMENT_MODE=multi_user_basic` | Multi-User (BasicAuth pass-through) | | `MCP_DEPLOYMENT_MODE=login_flow` or no auth env vars set | Multi-User (Login Flow v2) | You can also force a mode via CLI flag: diff --git a/docs/configuration-migration-v2.md b/docs/configuration-migration-v2.md index 9fa6020f..8d288b78 100644 --- a/docs/configuration-migration-v2.md +++ b/docs/configuration-migration-v2.md @@ -188,7 +188,7 @@ NEXTCLOUD_OIDC_CLIENT_SECRET=secret **Before (v0.57.x):** ```bash NEXTCLOUD_HOST=https://nextcloud.example.com -ENABLE_MULTI_USER_BASIC_AUTH=true +MCP_DEPLOYMENT_MODE=multi_user_basic # Both required - redundant ENABLE_OFFLINE_ACCESS=true @@ -205,7 +205,7 @@ NEXTCLOUD_OIDC_CLIENT_SECRET=secret **After (v0.58.0+ - Simplified):** ```bash NEXTCLOUD_HOST=https://nextcloud.example.com -ENABLE_MULTI_USER_BASIC_AUTH=true +MCP_DEPLOYMENT_MODE=multi_user_basic # Optional: Explicit mode declaration MCP_DEPLOYMENT_MODE=multi_user_basic @@ -448,7 +448,7 @@ Server activates `login_flow` mode when you expected `multi_user_basic` Add explicit mode declaration: ```bash MCP_DEPLOYMENT_MODE=multi_user_basic -ENABLE_MULTI_USER_BASIC_AUTH=true +MCP_DEPLOYMENT_MODE=multi_user_basic ``` --- diff --git a/docs/configuration.md b/docs/configuration.md index 064d716a..128a768e 100644 --- a/docs/configuration.md +++ b/docs/configuration.md @@ -74,7 +74,7 @@ Each MCP client sends its own Nextcloud credentials in an `Authorization: Basic` ```dotenv NEXTCLOUD_HOST=https://your.nextcloud.instance.com -ENABLE_MULTI_USER_BASIC_AUTH=true +MCP_DEPLOYMENT_MODE=multi_user_basic # Optional: enable per-user app-password storage for background sync TOKEN_ENCRYPTION_KEY= diff --git a/docs/troubleshooting.md b/docs/troubleshooting.md index e99e34d7..6fb05915 100644 --- a/docs/troubleshooting.md +++ b/docs/troubleshooting.md @@ -147,7 +147,7 @@ For multi-user deployment issues — provisioning loops, app-password storage, O ```bash # To Single-User BasicAuth: set NEXTCLOUD_USERNAME and NEXTCLOUD_PASSWORD -# To Multi-User BasicAuth pass-through: ENABLE_MULTI_USER_BASIC_AUTH=true (no creds) +# To Multi-User BasicAuth pass-through: MCP_DEPLOYMENT_MODE=multi_user_basic (no creds) # To Login Flow v2: MCP_DEPLOYMENT_MODE=login_flow (no creds; also the default fallback) ``` diff --git a/docs/webhook-management-guide.md b/docs/webhook-management-guide.md index 0d6dc54e..c0e55003 100644 --- a/docs/webhook-management-guide.md +++ b/docs/webhook-management-guide.md @@ -77,7 +77,7 @@ php occ webhook_listeners:remove **Configuration:** ```bash NEXTCLOUD_HOST=http://nextcloud.example.com -ENABLE_MULTI_USER_BASIC_AUTH=true +MCP_DEPLOYMENT_MODE=multi_user_basic ENABLE_BACKGROUND_OPERATIONS=true TOKEN_ENCRYPTION_KEY= TOKEN_STORAGE_DB=/app/data/tokens.db diff --git a/env.sample b/env.sample index 17e9ca92..600c2ffc 100644 --- a/env.sample +++ b/env.sample @@ -49,8 +49,9 @@ NEXTCLOUD_PASSWORD= # Users provide credentials in request headers (pass-through) # Use for: Multi-user without OAuth, simple shared deployments # -# Required: -#ENABLE_MULTI_USER_BASIC_AUTH=true +# Required (sets the deployment mode; the legacy ENABLE_MULTI_USER_BASIC_AUTH +# env var was removed in the ADR-022 follow-up): +#MCP_DEPLOYMENT_MODE=multi_user_basic # # Optional - Background Operations (for semantic search, future features): # Enable background token storage using app passwords (via Astrolabe) diff --git a/nextcloud_mcp_server/config.py b/nextcloud_mcp_server/config.py index a1310c53..c0aeab42 100644 --- a/nextcloud_mcp_server/config.py +++ b/nextcloud_mcp_server/config.py @@ -453,9 +453,13 @@ class Settings: # Progressive Consent settings (always enabled - no flag needed) enable_offline_access: bool = False - # Multi-user BasicAuth pass-through mode (ADR-019 interim solution) - # When enabled, MCP server extracts BasicAuth credentials from request headers - # and passes them through to Nextcloud APIs (no storage, stateless) + # Multi-user BasicAuth pass-through mode (ADR-019 interim solution). + # Internal — not user-settable; the ENABLE_MULTI_USER_BASIC_AUTH env-var + # alias was removed in the ADR-022 follow-up. Auto-set by + # detect_auth_mode() when MCP_DEPLOYMENT_MODE=multi_user_basic. When True, + # the MCP server extracts BasicAuth credentials from request headers and + # passes them through to Nextcloud APIs (no storage, stateless). Kept + # as a field for backward compat with the runtime call sites that read it. enable_multi_user_basic_auth: bool = False # Login Flow v2 derived flag (ADR-022). Internal — not user-settable. @@ -723,20 +727,31 @@ def _get_semantic_search_enabled() -> bool: def _is_multi_user_mode() -> bool: """Detect if this is a multi-user deployment mode. + Runs early in config setup (before Settings is fully built) for + mode-conditional defaults. Must match the canonical detection in + `config_validators.detect_auth_mode`, but works directly against the + raw dynaconf store since Settings doesn't exist yet. + Multi-user modes are: - - Multi-user BasicAuth (ENABLE_MULTI_USER_BASIC_AUTH=true) - - OAuth Single-Audience (no username/password set) + - Multi-user BasicAuth (MCP_DEPLOYMENT_MODE=multi_user_basic) + - Login Flow v2 / default OAuth (MCP_DEPLOYMENT_MODE=login_flow, or no + username/password and no explicit mode) - OAuth Token Exchange (ENABLE_TOKEN_EXCHANGE=true) - Single-user modes are: + Single-user mode is: - Single-user BasicAuth (username and password both set) Returns: True if multi-user mode detected """ - # Multi-user BasicAuth explicitly enabled - if _dynaconf.get("ENABLE_MULTI_USER_BASIC_AUTH", False): + # Explicit deployment mode wins. The ENABLE_MULTI_USER_BASIC_AUTH env-var + # alias was removed in the ADR-022 follow-up; selection is now via + # MCP_DEPLOYMENT_MODE. + explicit_mode = str(_dynaconf.get("MCP_DEPLOYMENT_MODE", "") or "").lower().strip() + if explicit_mode in {"multi_user_basic", "login_flow"}: return True + if explicit_mode == "single_user_basic": + return False # Token exchange implies OAuth multi-user if _dynaconf.get("ENABLE_TOKEN_EXCHANGE", False): @@ -748,7 +763,7 @@ def _is_multi_user_mode() -> bool: if has_username and has_password: return False - # Otherwise, assume OAuth multi-user (default when no credentials provided) + # Otherwise, assume multi-user (default when no credentials provided) return True @@ -854,12 +869,10 @@ def get_settings() -> Settings: "jwks_uri": "JWKS_URI", "introspection_uri": "INTROSPECTION_URI", "userinfo_uri": "USERINFO_URI", - # Multi-user BasicAuth pass-through mode - "enable_multi_user_basic_auth": "ENABLE_MULTI_USER_BASIC_AUTH", - # NOTE: `enable_login_flow` used to have an `ENABLE_LOGIN_FLOW` env-var - # alias here, but it was removed in the ADR-022 follow-up — the flag - # is now derived from MCP_DEPLOYMENT_MODE=login_flow and set by - # detect_auth_mode() so users only need to configure the mode. + # NOTE: `enable_multi_user_basic_auth` and `enable_login_flow` no + # longer have env-var aliases — both are derived from the resolved + # MCP_DEPLOYMENT_MODE in detect_auth_mode() so users only configure + # the mode (ADR-022 follow-up). # Token and webhook storage settings "token_encryption_key": "TOKEN_ENCRYPTION_KEY", "token_storage_db": "TOKEN_STORAGE_DB", diff --git a/nextcloud_mcp_server/config_validators.py b/nextcloud_mcp_server/config_validators.py index 1b409bf4..325deeb5 100644 --- a/nextcloud_mcp_server/config_validators.py +++ b/nextcloud_mcp_server/config_validators.py @@ -9,6 +9,7 @@ See ADR-020 for detailed architecture and deployment mode documentation. """ import logging +import os from dataclasses import dataclass from enum import Enum @@ -64,7 +65,6 @@ MODE_REQUIREMENTS: dict[AuthMode, ModeRequirements] = { "document_chunk_overlap", ], forbidden=[ - "enable_multi_user_basic_auth", "oidc_client_id", "oidc_client_secret", ], @@ -78,7 +78,7 @@ MODE_REQUIREMENTS: dict[AuthMode, ModeRequirements] = { "Suitable for personal Nextcloud instances and local development.", ), AuthMode.MULTI_USER_BASIC: ModeRequirements( - required=["nextcloud_host", "enable_multi_user_basic_auth"], + required=["nextcloud_host"], optional=[ # Background sync with app passwords (via Astrolabe) "enable_offline_access", @@ -138,7 +138,6 @@ MODE_REQUIREMENTS: dict[AuthMode, ModeRequirements] = { forbidden=[ "nextcloud_username", "nextcloud_password", - "enable_multi_user_basic_auth", ], conditional={ "enable_offline_access": [ @@ -181,6 +180,21 @@ def detect_auth_mode(settings: Settings) -> AuthMode: logger = logging.getLogger(__name__) + # ADR-022 follow-up: fail loudly if a caller is still relying on the + # removed env-var aliases. Bypass dynaconf and read os.environ directly + # so the check survives even though the aliases are gone. + for legacy, replacement in ( + ("ENABLE_MULTI_USER_BASIC_AUTH", "multi_user_basic"), + ("ENABLE_LOGIN_FLOW", "login_flow"), + ): + if os.getenv(legacy): + raise ValueError( + f"{legacy} is no longer read from the environment. " + f"Set MCP_DEPLOYMENT_MODE={replacement} instead " + "(ADR-022). The deployment mode is the single source of " + "truth for selecting an auth flow." + ) + # ADR-021: Check for explicit deployment mode first if settings.deployment_mode: mode_str = settings.deployment_mode.lower().strip() @@ -204,11 +218,11 @@ def detect_auth_mode(settings: Settings) -> AuthMode: _sync_derived_flags(settings, explicit_mode) return explicit_mode - # Auto-detection (existing behavior) - # Check for multi-user BasicAuth - if settings.enable_multi_user_basic_auth: - _sync_derived_flags(settings, AuthMode.MULTI_USER_BASIC) - return AuthMode.MULTI_USER_BASIC + # Auto-detection (no explicit deployment_mode). + # MULTI_USER_BASIC is no longer auto-detectable — the ENABLE_MULTI_USER_BASIC_AUTH + # env-var alias was dropped in the ADR-022 follow-up, so the only way to + # opt into that mode is `MCP_DEPLOYMENT_MODE=multi_user_basic` (handled + # above). The legacy env var fails loudly at the top of this function. # Check for single-user BasicAuth (explicit credentials) if settings.nextcloud_username and settings.nextcloud_password: @@ -228,13 +242,12 @@ def _sync_derived_flags(settings: Settings, mode: AuthMode) -> None: Some runtime call sites (app.py, context.py, auth/scope_authorization.py) still read individual boolean flags rather than passing the mode around. Keep those flags in sync with the mode here so the mode is the single - source of truth and users don't have to set redundant env vars. - - Specifically: `enable_login_flow` is now derived from - `mode == AuthMode.LOGIN_FLOW`. The ENABLE_LOGIN_FLOW env-var alias was + source of truth and users don't have to set redundant env vars. The + ENABLE_LOGIN_FLOW and ENABLE_MULTI_USER_BASIC_AUTH env-var aliases were removed in the ADR-022 follow-up (PR #787). """ settings.enable_login_flow = mode == AuthMode.LOGIN_FLOW + settings.enable_multi_user_basic_auth = mode == AuthMode.MULTI_USER_BASIC def validate_configuration(settings: Settings) -> tuple[AuthMode, list[str]]: diff --git a/nextcloud_mcp_server/vector/oauth_sync.py b/nextcloud_mcp_server/vector/oauth_sync.py index f29443e9..7883c4db 100644 --- a/nextcloud_mcp_server/vector/oauth_sync.py +++ b/nextcloud_mcp_server/vector/oauth_sync.py @@ -7,7 +7,7 @@ Manages background vector sync for multi-user deployments: Authentication strategies are mutually exclusive by deployment mode: -Multi-user BasicAuth mode (ENABLE_MULTI_USER_BASIC_AUTH=true): +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 diff --git a/tests/unit/test_config_validators.py b/tests/unit/test_config_validators.py index 3c63b691..f8d7f907 100644 --- a/tests/unit/test_config_validators.py +++ b/tests/unit/test_config_validators.py @@ -10,6 +10,8 @@ Tests cover: import os from unittest.mock import patch +import pytest + from nextcloud_mcp_server.config import Settings, _reload_config from nextcloud_mcp_server.config_validators import ( AuthMode, @@ -23,14 +25,21 @@ class TestModeDetection: """Test auth mode detection from configuration.""" def test_multi_user_basic_mode_detection(self): - """Test multi-user BasicAuth mode is detected.""" + """Test multi-user BasicAuth mode is selected via explicit deployment_mode. + + ADR-022 follow-up: the ENABLE_MULTI_USER_BASIC_AUTH auto-detection branch + was removed; the only way to opt in is `MCP_DEPLOYMENT_MODE=multi_user_basic`. + Coverage for the explicit-mode path also lives in + TestExplicitModeSelection::test_explicit_multi_user_basic_mode. + """ settings = Settings( nextcloud_host="http://localhost", - enable_multi_user_basic_auth=True, + deployment_mode="multi_user_basic", ) mode = detect_auth_mode(settings) assert mode == AuthMode.MULTI_USER_BASIC + assert settings.enable_multi_user_basic_auth is True def test_single_user_basic_mode_detection(self): """Test single-user BasicAuth mode is detected.""" @@ -127,20 +136,25 @@ class TestSingleUserBasicValidation: # In OAuth mode, having a username set is forbidden assert any("nextcloud_username" in err.lower() for err in errors) - def test_forbidden_multi_user_basic_auth(self): - """Test error when ENABLE_MULTI_USER_BASIC_AUTH is set.""" + def test_forbidden_multi_user_basic_when_credentials_present(self): + """Test multi-user mode rejects single-user credentials. + + When MCP_DEPLOYMENT_MODE=multi_user_basic is set explicitly but + NEXTCLOUD_USERNAME/PASSWORD are also set (a misconfiguration), + the explicit mode wins and validation reports the credentials as + forbidden. + """ settings = Settings( nextcloud_host="http://localhost", nextcloud_username="admin", nextcloud_password="password", - enable_multi_user_basic_auth=True, + deployment_mode="multi_user_basic", ) - # Note: This will detect as MULTI_USER_BASIC due to priority mode, errors = validate_configuration(settings) assert mode == AuthMode.MULTI_USER_BASIC - # It will fail multi-user validation because username/password are forbidden + # Should report errors for forbidden username/password assert len(errors) > 0 def test_vector_sync_without_embedding_provider_uses_fallback(self): @@ -167,7 +181,7 @@ class TestMultiUserBasicValidation: """Test valid minimal multi-user BasicAuth config.""" settings = Settings( nextcloud_host="http://localhost", - enable_multi_user_basic_auth=True, + deployment_mode="multi_user_basic", ) mode, errors = validate_configuration(settings) @@ -179,7 +193,7 @@ class TestMultiUserBasicValidation: """Test valid config with offline access enabled.""" settings = Settings( nextcloud_host="http://localhost", - enable_multi_user_basic_auth=True, + deployment_mode="multi_user_basic", enable_offline_access=True, oidc_client_id="test-client", oidc_client_secret="test-secret", @@ -195,7 +209,7 @@ class TestMultiUserBasicValidation: def test_missing_required_host(self): """Test error when NEXTCLOUD_HOST is missing.""" settings = Settings( - enable_multi_user_basic_auth=True, + deployment_mode="multi_user_basic", ) mode, errors = validate_configuration(settings) @@ -209,13 +223,12 @@ class TestMultiUserBasicValidation: nextcloud_host="http://localhost", nextcloud_username="admin", nextcloud_password="password", - enable_multi_user_basic_auth=True, + deployment_mode="multi_user_basic", ) mode, errors = validate_configuration(settings) - # Multi-user BasicAuth has higher priority than single-user in detection - # (explicit flags come before credentials) + # Explicit MCP_DEPLOYMENT_MODE wins over auto-detection from credentials assert mode == AuthMode.MULTI_USER_BASIC # Should report errors for forbidden username/password assert any("nextcloud_username" in err.lower() for err in errors) @@ -225,7 +238,7 @@ class TestMultiUserBasicValidation: """Test that offline access works without OAuth credentials (will use DCR).""" settings = Settings( nextcloud_host="http://localhost", - enable_multi_user_basic_auth=True, + deployment_mode="multi_user_basic", enable_offline_access=True, token_encryption_key="test-key-" + "a" * 32, token_storage_db="/tmp/tokens.db", @@ -241,7 +254,7 @@ class TestMultiUserBasicValidation: """Test error when offline access enabled but encryption key missing.""" settings = Settings( nextcloud_host="http://localhost", - enable_multi_user_basic_auth=True, + deployment_mode="multi_user_basic", enable_offline_access=True, oidc_client_id="test-client", oidc_client_secret="test-secret", @@ -261,7 +274,7 @@ class TestMultiUserBasicValidation: os.environ, { "NEXTCLOUD_HOST": "http://localhost:8080", - "ENABLE_MULTI_USER_BASIC_AUTH": "true", + "MCP_DEPLOYMENT_MODE": "multi_user_basic", "VECTOR_SYNC_ENABLED": "true", # Using old name for backward compat test "QDRANT_LOCATION": ":memory:", "OLLAMA_BASE_URL": "http://ollama:11434", @@ -659,7 +672,7 @@ class TestConfigurationConsolidation: os.environ, { "NEXTCLOUD_HOST": "http://localhost:8080", - "ENABLE_MULTI_USER_BASIC_AUTH": "true", + "MCP_DEPLOYMENT_MODE": "multi_user_basic", "ENABLE_SEMANTIC_SEARCH": "true", "QDRANT_LOCATION": ":memory:", "TOKEN_ENCRYPTION_KEY": "test-key", @@ -830,3 +843,54 @@ class TestExplicitModeSelection: mode = detect_auth_mode(settings) assert mode == AuthMode.LOGIN_FLOW + + def test_legacy_enable_multi_user_basic_auth_env_var_errors(self): + """ADR-022 follow-up: ENABLE_MULTI_USER_BASIC_AUTH=true must fail loudly. + + The env-var alias was removed; users must migrate to + `MCP_DEPLOYMENT_MODE=multi_user_basic`. Silent removal would have + switched users to LOGIN_FLOW (the default) — wrong runtime mode. + """ + with patch.dict( + os.environ, + { + "NEXTCLOUD_HOST": "http://localhost:8080", + "ENABLE_MULTI_USER_BASIC_AUTH": "true", + }, + clear=True, + ): + from nextcloud_mcp_server.config import get_settings + + _reload_config() + settings = get_settings() + + with pytest.raises(ValueError) as exc: + detect_auth_mode(settings) + + assert "ENABLE_MULTI_USER_BASIC_AUTH" in str(exc.value) + assert "multi_user_basic" in str(exc.value) + + def test_legacy_enable_login_flow_env_var_errors(self): + """ADR-022 follow-up: ENABLE_LOGIN_FLOW=true must fail loudly. + + Mirrors the ENABLE_MULTI_USER_BASIC_AUTH check — both legacy aliases + now error with a one-line migration message. + """ + with patch.dict( + os.environ, + { + "NEXTCLOUD_HOST": "http://localhost:8080", + "ENABLE_LOGIN_FLOW": "true", + }, + clear=True, + ): + from nextcloud_mcp_server.config import get_settings + + _reload_config() + settings = get_settings() + + with pytest.raises(ValueError) as exc: + detect_auth_mode(settings) + + assert "ENABLE_LOGIN_FLOW" in str(exc.value) + assert "login_flow" in str(exc.value)