diff --git a/docker-compose.yml b/docker-compose.yml index eb5d282e..e6e237de 100644 --- a/docker-compose.yml +++ b/docker-compose.yml @@ -241,8 +241,8 @@ services: - ENABLE_TOKEN_EXCHANGE=true - TOKEN_EXCHANGE_CACHE_TTL=300 # Cache exchanged tokens for 5 minutes (default) - # Login Flow v2 (ADR-022) with external IdP - - ENABLE_LOGIN_FLOW=true + # Login Flow v2 (ADR-022) with external IdP — derived from the + # auto-detected LOGIN_FLOW deployment mode; no separate flag needed. - ENABLE_DCR=true # OAuth scopes (optional - uses defaults if not specified) @@ -275,10 +275,9 @@ services: #- NEXTCLOUD_MCP_SERVER_URL=https://nextcloud-mcp-dev.tail148d5.ts.net - NEXTCLOUD_PUBLIC_ISSUER_URL=http://localhost:8080 - # Login Flow v2 (ADR-022) — explicit mode + the flag that enables the - # browser-based app-password acquisition layer + # Login Flow v2 (ADR-022) — the deployment mode is the single switch; + # the browser-based app-password layer is derived automatically. - MCP_DEPLOYMENT_MODE=login_flow - - ENABLE_LOGIN_FLOW=true # Token storage (required for app password + session persistence). # Source the key from .env — see env.sample. To generate a fresh key: diff --git a/docs/ADR-025-dynaconf-configuration-management.md b/docs/ADR-025-dynaconf-configuration-management.md index 2dca9ca3..c4974c29 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 | 4 | `MCP_DEPLOYMENT_MODE`, `ENABLE_LOGIN_FLOW`, `ENABLE_TOKEN_EXCHANGE` | +| Mode Selection | 2 | `MCP_DEPLOYMENT_MODE`, `ENABLE_MULTI_USER_BASIC_AUTH` | | 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` | @@ -115,7 +115,8 @@ nextcloud_ca_bundle = "@none" # === Authentication Toggles === enable_multi_user_basic_auth = false -enable_login_flow = false +# `enable_login_flow` is derived from MCP_DEPLOYMENT_MODE=login_flow in +# detect_auth_mode (ADR-022 follow-up) — no separate toggle. enable_token_exchange = false # === Token Storage === @@ -202,7 +203,7 @@ enable_multi_user_basic_auth = true token_storage_db = "/app/data/tokens.db" [login_flow] -enable_login_flow = true +# enable_login_flow is now derived from the mode (ADR-022 follow-up). token_storage_db = "/app/data/tokens.db" [keycloak] @@ -266,7 +267,7 @@ Dynaconf merges configuration in this order (last wins): This means: - **File-based config is optional** — env vars alone still work (they override everything) -- **Mode-specific defaults reduce boilerplate** — `[login_flow]` sets `enable_login_flow=true` and `token_storage_db=/app/data/tokens.db` so deployers don't need to +- **Mode-specific defaults reduce boilerplate** — `[login_flow]` sets `token_storage_db=/app/data/tokens.db` so deployers don't need to - **Secrets are separated** — `.secrets.toml` holds `TOKEN_ENCRYPTION_KEY`, passwords, API keys - **Local dev overrides don't pollute** — `settings.local.toml` is gitignored diff --git a/docs/auth-flows.md b/docs/auth-flows.md index 4ebf4857..1b89372b 100644 --- a/docs/auth-flows.md +++ b/docs/auth-flows.md @@ -231,7 +231,7 @@ TOKEN_STORAGE_DB=/app/data/tokens.db ### Login Flow v2 ```bash NEXTCLOUD_HOST=https://nextcloud.example.com -ENABLE_LOGIN_FLOW=true +MCP_DEPLOYMENT_MODE=login_flow # Required for app-password storage TOKEN_ENCRYPTION_KEY= diff --git a/docs/authentication.md b/docs/authentication.md index bdc826b9..9eb402c8 100644 --- a/docs/authentication.md +++ b/docs/authentication.md @@ -75,7 +75,7 @@ The server detects the active mode from environment variables at startup: |------------------|---------------| | `NEXTCLOUD_USERNAME` + `NEXTCLOUD_PASSWORD` | Single-User (BasicAuth) | | `ENABLE_MULTI_USER_BASIC_AUTH=true` (no creds) | Multi-User (BasicAuth pass-through) | -| `ENABLE_LOGIN_FLOW=true` (no creds) | Multi-User (Login Flow v2) | +| `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.md b/docs/configuration.md index a1879596..064d716a 100644 --- a/docs/configuration.md +++ b/docs/configuration.md @@ -91,7 +91,7 @@ The recommended multi-user mode. MCP clients authenticate to the MCP server via ```dotenv NEXTCLOUD_HOST=https://your.nextcloud.instance.com -ENABLE_LOGIN_FLOW=true +MCP_DEPLOYMENT_MODE=login_flow # App-password storage (required) TOKEN_ENCRYPTION_KEY= @@ -105,7 +105,7 @@ NEXTCLOUD_PUBLIC_ISSUER_URL=https://your.nextcloud.instance.com | Variable | Required | Description | |----------|----------|-------------| | `NEXTCLOUD_HOST` | ✅ Yes | Internal URL of your Nextcloud instance (server-to-server) | -| `ENABLE_LOGIN_FLOW` | ✅ Yes | Set to `true` to enable Login Flow v2 | +| `MCP_DEPLOYMENT_MODE` | ✅ Yes | Set to `login_flow` to select this mode. The Login Flow v2 browser-app-password layer is derived from the mode automatically — no separate flag needed. | | `TOKEN_ENCRYPTION_KEY` | ✅ Yes | Fernet key for app-password encryption — generate with `python -c "from cryptography.fernet import Fernet; print(Fernet.generate_key().decode())"` | | `TOKEN_STORAGE_DB` | ✅ Yes | Path to SQLite DB for stored app passwords (use a persistent volume) | | `NEXTCLOUD_MCP_SERVER_URL` | ✅ Yes | Public URL of the MCP server (used as the audience claim and for browser redirects) | @@ -197,8 +197,7 @@ OLLAMA_BASE_URL=http://ollama:11434 **Multi-User Login Flow v2 Mode:** ```dotenv NEXTCLOUD_HOST=https://nextcloud.example.com -MCP_DEPLOYMENT_MODE=login_flow_v2 -ENABLE_LOGIN_FLOW=true +MCP_DEPLOYMENT_MODE=login_flow # Enable semantic search # In multi-user modes, this AUTOMATICALLY enables background operations! diff --git a/docs/login-flow-v2.md b/docs/login-flow-v2.md index 10558169..7d7e74e8 100644 --- a/docs/login-flow-v2.md +++ b/docs/login-flow-v2.md @@ -58,8 +58,8 @@ NEXTCLOUD_HOST=https://your.nextcloud.example.com NEXTCLOUD_OIDC_CLIENT_ID= NEXTCLOUD_OIDC_CLIENT_SECRET= -# Enable Login Flow v2 (per-user Nextcloud app-password provisioning for the data leg) -ENABLE_LOGIN_FLOW=true +# Select Login Flow v2 mode (per-user Nextcloud app-password provisioning for the data leg) +MCP_DEPLOYMENT_MODE=login_flow # App-password storage (required for persistence across restarts) TOKEN_STORAGE_DB=/app/data/tokens.db @@ -172,7 +172,7 @@ mcp-login-flow: - NEXTCLOUD_HOST=http://app:80 - NEXTCLOUD_MCP_SERVER_URL=http://localhost:8004 - NEXTCLOUD_PUBLIC_ISSUER_URL=http://localhost:8080 - - ENABLE_LOGIN_FLOW=true + - MCP_DEPLOYMENT_MODE=login_flow # Dev-only inline value. In production, mount via Docker secret and read # from a *_FILE env var or a secrets-management init step. - TOKEN_ENCRYPTION_KEY= @@ -351,7 +351,7 @@ The user revoked it from **Settings → Security → Devices & Sessions**. Delet ### Multiple worker processes -The provisioning session store is in-memory; `ENABLE_LOGIN_FLOW=true` assumes a single worker. Running with `uvicorn --workers N` will cause provisioning sessions to randomly fail. For higher concurrency, scale horizontally (multiple containers behind a sticky-session load balancer) rather than within a single process. +The provisioning session store is in-memory; `MCP_DEPLOYMENT_MODE=login_flow` assumes a single worker. Running with `uvicorn --workers N` will cause provisioning sessions to randomly fail. For higher concurrency, scale horizontally (multiple containers behind a sticky-session load balancer) rather than within a single process. > **Sticky-session keying:** route on the **user identity** (e.g. the `sub` claim from the OAuth Bearer token) — **not** the raw token value, and **not** source IP. Bearer tokens rotate on refresh, which would silently break token-value affinity if a refresh lands between the request that initiates provisioning and the polling request that completes it. MCP clients may also not maintain stable IPs across those requests. A stable per-user identifier extracted from the `Authorization` header (e.g. `sub`) is the right key. diff --git a/docs/troubleshooting.md b/docs/troubleshooting.md index fd8e683a..e99e34d7 100644 --- a/docs/troubleshooting.md +++ b/docs/troubleshooting.md @@ -148,7 +148,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 Login Flow v2: ENABLE_LOGIN_FLOW=true (no creds) +# To Login Flow v2: MCP_DEPLOYMENT_MODE=login_flow (no creds; also the default fallback) ``` Restart the server after changing modes. The active mode is logged at startup; you can also set `MCP_DEPLOYMENT_MODE` explicitly to fail fast if the env vars don't match. diff --git a/env.sample b/env.sample index 8e175bde..17e9ca92 100644 --- a/env.sample +++ b/env.sample @@ -7,8 +7,10 @@ # # Note: `login_flow` is the renamed successor of the former # `oauth_single_audience` mode (see ADR-022). The un-augmented OAuth path -# required unmerged Nextcloud user_oidc patches and is no longer supported; -# `login_flow` mode now requires ENABLE_LOGIN_FLOW=true. +# required unmerged Nextcloud user_oidc patches and is no longer supported. +# Setting MCP_DEPLOYMENT_MODE=login_flow is sufficient — the Login Flow v2 +# browser-based app-password layer is derived from the mode automatically +# (the previous ENABLE_LOGIN_FLOW=true env var has been removed). # # Recommendation: Set this for clarity and to catch configuration errors early #MCP_DEPLOYMENT_MODE=login_flow diff --git a/nextcloud_mcp_server/config.py b/nextcloud_mcp_server/config.py index dd806385..a1310c53 100644 --- a/nextcloud_mcp_server/config.py +++ b/nextcloud_mcp_server/config.py @@ -458,7 +458,10 @@ class Settings: # and passes them through to Nextcloud APIs (no storage, stateless) enable_multi_user_basic_auth: bool = False - # Login Flow v2 settings (ADR-022) + # Login Flow v2 derived flag (ADR-022). Internal — not user-settable. + # Auto-set by detect_auth_mode() when the resolved deployment mode is + # LOGIN_FLOW. Kept as a field for backward compat with the runtime call + # sites that read it (app.py, context.py, scope_authorization.py). enable_login_flow: bool = False # Token and webhook storage settings @@ -853,8 +856,10 @@ def get_settings() -> Settings: "userinfo_uri": "USERINFO_URI", # Multi-user BasicAuth pass-through mode "enable_multi_user_basic_auth": "ENABLE_MULTI_USER_BASIC_AUTH", - # Login Flow v2 settings (ADR-022) - "enable_login_flow": "ENABLE_LOGIN_FLOW", + # 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. # 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 1d497817..1b409bf4 100644 --- a/nextcloud_mcp_server/config_validators.py +++ b/nextcloud_mcp_server/config_validators.py @@ -114,7 +114,7 @@ MODE_REQUIREMENTS: dict[AuthMode, ModeRequirements] = { "Optional background sync using app passwords stored via Astrolabe.", ), AuthMode.LOGIN_FLOW: ModeRequirements( - required=["nextcloud_host", "enable_login_flow"], + required=["nextcloud_host"], optional=[ # OAuth credentials (uses DCR if not provided) "oidc_client_id", @@ -201,24 +201,42 @@ def detect_auth_mode(settings: Settings) -> AuthMode: explicit_mode = mode_map[mode_str] logger.info(f"Using explicit deployment mode: {explicit_mode.value}") + _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 # Check for single-user BasicAuth (explicit credentials) if settings.nextcloud_username and settings.nextcloud_password: + _sync_derived_flags(settings, AuthMode.SINGLE_USER_BASIC) return AuthMode.SINGLE_USER_BASIC # Default: Login Flow v2 multi-user mode (browser-based app-password flow). # The un-augmented OAuth bearer pass-through it replaced required unmerged - # Nextcloud user_oidc patches (see ADR-022); selecting LOGIN_FLOW without - # ENABLE_LOGIN_FLOW=true fails validation below. + # Nextcloud user_oidc patches (see ADR-022). + _sync_derived_flags(settings, AuthMode.LOGIN_FLOW) return AuthMode.LOGIN_FLOW +def _sync_derived_flags(settings: Settings, mode: AuthMode) -> None: + """Derive internal feature flags from the resolved deployment mode. + + 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 + removed in the ADR-022 follow-up (PR #787). + """ + settings.enable_login_flow = mode == AuthMode.LOGIN_FLOW + + def validate_configuration(settings: Settings) -> tuple[AuthMode, list[str]]: """Validate configuration for detected mode. @@ -308,16 +326,12 @@ def validate_configuration(settings: Settings) -> tuple[AuthMode, list[str]]: ) if mode == AuthMode.LOGIN_FLOW: - # ADR-022: LOGIN_FLOW requires the Login Flow v2 layer. The un-augmented - # OAuth bearer pass-through (formerly OAUTH_SINGLE_AUDIENCE without - # ENABLE_LOGIN_FLOW) needed unmerged Nextcloud user_oidc patches and - # is no longer supported. - if not settings.enable_login_flow: - errors.append( - f"[{mode.value}] ENABLE_LOGIN_FLOW=true is required for " - "login_flow mode. The un-augmented OAuth path is no longer " - "supported — see ADR-022." - ) + # ADR-022 follow-up: the un-augmented OAuth bearer pass-through (the + # old OAUTH_SINGLE_AUDIENCE without ENABLE_LOGIN_FLOW) needed unmerged + # Nextcloud user_oidc patches and is no longer supported. The + # `enable_login_flow` flag is now derived from the resolved mode by + # `_sync_derived_flags`, so users only configure the mode — no + # separate ENABLE_LOGIN_FLOW env var is needed. # If OAuth credentials not provided, DCR must be available # (This is a runtime check, not a config check, so we just warn) diff --git a/tests/unit/test_config_validators.py b/tests/unit/test_config_validators.py index 5f633754..3c63b691 100644 --- a/tests/unit/test_config_validators.py +++ b/tests/unit/test_config_validators.py @@ -289,22 +289,22 @@ class TestLoginFlowValidation: """Test validation for Login Flow v2 mode (formerly OAUTH_SINGLE_AUDIENCE).""" def test_valid_minimal_config(self): - """Test valid minimal Login Flow v2 config.""" + """Test valid minimal Login Flow v2 config — enable_login_flow is now derived.""" settings = Settings( nextcloud_host="http://localhost", - enable_login_flow=True, ) mode, errors = validate_configuration(settings) assert mode == AuthMode.LOGIN_FLOW assert len(errors) == 0 + # ADR-022 follow-up: enable_login_flow is derived from the resolved mode. + assert settings.enable_login_flow is True def test_valid_with_static_credentials(self): """Test valid config with static OAuth credentials.""" settings = Settings( nextcloud_host="http://localhost", - enable_login_flow=True, oidc_client_id="test-client", oidc_client_secret="test-secret", ) @@ -318,7 +318,6 @@ class TestLoginFlowValidation: """Test valid config with offline access.""" settings = Settings( nextcloud_host="http://localhost", - enable_login_flow=True, oidc_client_id="test-client", oidc_client_secret="test-secret", enable_offline_access=True, @@ -348,7 +347,6 @@ class TestLoginFlowValidation: """Test error when offline access enabled but encryption key missing.""" settings = Settings( nextcloud_host="http://localhost", - enable_login_flow=True, enable_offline_access=True, token_storage_db="/tmp/tokens.db", ) @@ -358,19 +356,28 @@ class TestLoginFlowValidation: assert mode == AuthMode.LOGIN_FLOW assert any("token_encryption_key" in err.lower() for err in errors) - def test_login_flow_requires_enable_login_flow_flag(self): - """ADR-022: LOGIN_FLOW mode must error when ENABLE_LOGIN_FLOW is not true.""" - settings = Settings( - nextcloud_host="http://localhost", - # enable_login_flow deliberately omitted (defaults to False) - ) - - mode, errors = validate_configuration(settings) + def test_login_flow_mode_auto_derives_enable_login_flow_flag(self): + """ADR-022 follow-up: setting MCP_DEPLOYMENT_MODE=login_flow auto-derives the flag. + Users no longer need to set ENABLE_LOGIN_FLOW=true (env var was removed); + detect_auth_mode populates settings.enable_login_flow from the resolved mode. + """ + # Default-fallback case: no auth env vars → LOGIN_FLOW. + settings = Settings(nextcloud_host="http://localhost") + assert settings.enable_login_flow is False # default before detection + mode = detect_auth_mode(settings) assert mode == AuthMode.LOGIN_FLOW - assert any("ENABLE_LOGIN_FLOW" in err for err in errors), ( - f"Expected ENABLE_LOGIN_FLOW gate error, got: {errors}" + assert settings.enable_login_flow is True + + # Non-LOGIN_FLOW mode should leave the flag False. + basic_settings = Settings( + nextcloud_host="http://localhost", + nextcloud_username="alice", + nextcloud_password="hunter2", ) + basic_mode = detect_auth_mode(basic_settings) + assert basic_mode == AuthMode.SINGLE_USER_BASIC + assert basic_settings.enable_login_flow is False def test_vector_sync_auto_enables_background_ops_in_login_flow_mode(self): """Test vector sync automatically enables background operations in Login Flow v2 mode (ADR-021).""" @@ -380,7 +387,6 @@ class TestLoginFlowValidation: os.environ, { "NEXTCLOUD_HOST": "http://localhost:8080", - "ENABLE_LOGIN_FLOW": "true", "VECTOR_SYNC_ENABLED": "true", "QDRANT_LOCATION": ":memory:", "OLLAMA_BASE_URL": "http://ollama:11434",