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 46984db5..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). @@ -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,8 +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) - - ENABLE_LOGIN_FLOW=true + # 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 # 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-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-021-configuration-consolidation.md b/docs/ADR-021-configuration-consolidation.md index 065ff936..6298a754 100644 --- a/docs/ADR-021-configuration-consolidation.md +++ b/docs/ADR-021-configuration-consolidation.md @@ -1,9 +1,9 @@ # ADR-021: Configuration Consolidation and Simplification -**Status:** Accepted +**Status:** Accepted — partly superseded by ADR-022 (`oauth_single_audience` renamed to `login_flow`; `oauth_token_exchange` removed) **Date:** 2025-12-21 **Deciders:** Development Team -**Related:** ADR-020 (Deployment Modes), ADR-002 (Vector Sync), ADR-004 (Progressive Consent) +**Related:** ADR-020 (Deployment Modes), ADR-002 (Vector Sync), ADR-004 (Progressive Consent), ADR-022 (Deployment Mode Consolidation) ## Context @@ -86,10 +86,11 @@ Add `MCP_DEPLOYMENT_MODE` environment variable to remove detection ambiguity: ```bash # Optional: Explicitly declare deployment mode -MCP_DEPLOYMENT_MODE=oauth_single_audience +MCP_DEPLOYMENT_MODE=login_flow # Valid values: single_user_basic, multi_user_basic, # oauth_single_audience, oauth_token_exchange +# (both OAuth values removed in ADR-022 — current value: login_flow) ``` **Detection logic**: @@ -114,7 +115,7 @@ TOKEN_STORAGE_DB=/path/to/tokens.db ```bash # Multi-user OAuth with semantic search NEXTCLOUD_HOST=https://nextcloud.example.com -MCP_DEPLOYMENT_MODE=oauth_single_audience # Explicit (optional) +MCP_DEPLOYMENT_MODE=login_flow # Explicit (optional) ENABLE_SEMANTIC_SEARCH=true # Auto-enables background ops QDRANT_URL=http://qdrant:6333 TOKEN_ENCRYPTION_KEY= @@ -271,7 +272,7 @@ def enable_semantic_search(self) -> bool: - `ENABLE_SEMANTIC_SEARCH=false` → `enable_background_operations=false` (unless explicitly set) **Mode selection tests**: -- `MCP_DEPLOYMENT_MODE=oauth_single_audience` → mode correctly detected +- `MCP_DEPLOYMENT_MODE=login_flow` → mode correctly detected - `MCP_DEPLOYMENT_MODE` conflicts with detected mode → validation error - No `MCP_DEPLOYMENT_MODE` → auto-detection works as before @@ -363,7 +364,7 @@ QDRANT_URL=http://qdrant:6333 **After** (simplified): ```bash NEXTCLOUD_HOST=https://nextcloud.example.com -MCP_DEPLOYMENT_MODE=oauth_single_audience # Explicit (optional) +MCP_DEPLOYMENT_MODE=login_flow # Explicit (optional) ENABLE_SEMANTIC_SEARCH=true # Auto-enables background operations TOKEN_ENCRYPTION_KEY= TOKEN_STORAGE_DB=/path/to/tokens.db @@ -384,7 +385,7 @@ TOKEN_STORAGE_DB=/path/to/tokens.db **After** (optional migration): ```bash NEXTCLOUD_HOST=https://nextcloud.example.com -MCP_DEPLOYMENT_MODE=oauth_single_audience +MCP_DEPLOYMENT_MODE=login_flow ENABLE_BACKGROUND_OPERATIONS=true # Renamed for clarity TOKEN_ENCRYPTION_KEY= TOKEN_STORAGE_DB=/path/to/tokens.db diff --git a/docs/ADR-022-deployment-mode-consolidation.md b/docs/ADR-022-deployment-mode-consolidation.md index 9386fad6..6b958119 100644 --- a/docs/ADR-022-deployment-mode-consolidation.md +++ b/docs/ADR-022-deployment-mode-consolidation.md @@ -1,7 +1,7 @@ # ADR-022: Deployment Mode Consolidation via Login Flow v2 -**Status:** Proposed -**Date:** 2026-02-01 +**Status:** Accepted (step 1 — `OAUTH_SINGLE_AUDIENCE` → `LOGIN_FLOW` rename + validation gate. Dead-code pruning is a follow-up.) +**Date:** 2026-02-01 (accepted 2026-05-12) **Deciders:** Development Team **Related:** ADR-020 (Deployment Modes), ADR-021 (Configuration Consolidation), ADR-004 (Progressive Consent), Issue #521 diff --git a/docs/ADR-025-dynaconf-configuration-management.md b/docs/ADR-025-dynaconf-configuration-management.md index fd95e419..33defa6c 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 | 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` | @@ -108,13 +108,16 @@ nextcloud_verify_ssl = true nextcloud_ca_bundle = "@none" # === Deployment Mode === -# Auto-detected if not set. Valid: single_user_basic, multi_user_basic, -# oauth_single_audience, login_flow, keycloak +# Auto-detected if not set. Valid: single_user_basic, multi_user_basic, login_flow +# (`oauth_single_audience` was renamed to `login_flow` in ADR-022; `keycloak` +# is a planned future mode.) # mcp_deployment_mode = "" # === Authentication Toggles === -enable_multi_user_basic_auth = false -enable_login_flow = false +# 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 === @@ -197,20 +200,17 @@ 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] -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] enable_token_exchange = true token_storage_db = "/app/data/tokens.db" token_exchange_cache_ttl = 300 - -[oauth_single_audience] -token_storage_db = "/app/data/tokens.db" ``` **`.secrets.toml.example`** — Template, checked into git (actual `.secrets.toml` is gitignored): @@ -265,7 +265,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 @@ -281,7 +281,6 @@ validators = [ # Deployment mode validation — catch typos at startup Validator("MCP_DEPLOYMENT_MODE", is_in=[ "single_user_basic", "multi_user_basic", "login_flow", - "keycloak", "oauth_single_audience", ], when=Validator("MCP_DEPLOYMENT_MODE", must_exist=True)), # Type and range validation @@ -343,10 +342,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 4ebf4857..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= @@ -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..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,8 +74,8 @@ 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) | -| `ENABLE_LOGIN_FLOW=true` (no creds) | Multi-User (Login Flow v2) | +| `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 101c3bbf..3aac3740 100644 --- a/docs/configuration-migration-v2.md +++ b/docs/configuration-migration-v2.md @@ -106,7 +106,7 @@ NEXTCLOUD_USERNAME= NEXTCLOUD_PASSWORD= # Optional: Explicit mode declaration -MCP_DEPLOYMENT_MODE=oauth_single_audience +MCP_DEPLOYMENT_MODE=login_flow # One variable does it all! ENABLE_SEMANTIC_SEARCH=true # Automatically enables background operations @@ -131,7 +131,7 @@ NEXTCLOUD_OIDC_CLIENT_SECRET=secret **Migration Steps:** 1. Replace `VECTOR_SYNC_ENABLED=true` with `ENABLE_SEMANTIC_SEARCH=true` 2. Remove `ENABLE_OFFLINE_ACCESS=true` (auto-enabled) -3. Optionally add `MCP_DEPLOYMENT_MODE=oauth_single_audience` +3. Optionally add `MCP_DEPLOYMENT_MODE=login_flow` 4. Restart server 5. Check logs for confirmation: "Automatically enabled background operations for semantic search" @@ -161,7 +161,7 @@ NEXTCLOUD_USERNAME= NEXTCLOUD_PASSWORD= # Optional: Explicit mode declaration -MCP_DEPLOYMENT_MODE=oauth_single_audience +MCP_DEPLOYMENT_MODE=login_flow # Renamed for clarity ENABLE_BACKGROUND_OPERATIONS=true # Previously ENABLE_OFFLINE_ACCESS @@ -178,7 +178,7 @@ NEXTCLOUD_OIDC_CLIENT_SECRET=secret **Migration Steps:** 1. Replace `ENABLE_OFFLINE_ACCESS=true` with `ENABLE_BACKGROUND_OPERATIONS=true` -2. Optionally add `MCP_DEPLOYMENT_MODE=oauth_single_audience` +2. Optionally add `MCP_DEPLOYMENT_MODE=login_flow` 3. Restart server --- @@ -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,9 +205,6 @@ NEXTCLOUD_OIDC_CLIENT_SECRET=secret **After (v0.58.0+ - Simplified):** ```bash NEXTCLOUD_HOST=https://nextcloud.example.com -ENABLE_MULTI_USER_BASIC_AUTH=true - -# Optional: Explicit mode declaration MCP_DEPLOYMENT_MODE=multi_user_basic # One variable handles both! @@ -321,7 +318,7 @@ Only needed when you want background operations **without** semantic search: ```bash # Example: OAuth mode with background operations but NO semantic search NEXTCLOUD_HOST=https://nextcloud.example.com -MCP_DEPLOYMENT_MODE=oauth_single_audience +MCP_DEPLOYMENT_MODE=login_flow # Explicitly enable background operations for future features ENABLE_BACKGROUND_OPERATIONS=true @@ -352,7 +349,7 @@ NEXTCLOUD_HOST=https://nextcloud.example.com # Is this OAuth or Multi-User BasicAuth? Not immediately clear. # With explicit mode: -MCP_DEPLOYMENT_MODE=oauth_single_audience +MCP_DEPLOYMENT_MODE=login_flow NEXTCLOUD_HOST=https://nextcloud.example.com # Clear: This is OAuth mode ``` @@ -363,8 +360,7 @@ NEXTCLOUD_HOST=https://nextcloud.example.com |-----------|-------------| | `single_user_basic` | Single-user with username/password | | `multi_user_basic` | Multi-user with BasicAuth pass-through | -| `oauth_single_audience` | Multi-user OAuth (recommended) | -| `oauth_token_exchange` | Multi-user OAuth with token exchange | +| `login_flow` | Multi-user OAuth via Login Flow v2 (recommended) | ### Mode Detection Priority @@ -430,7 +426,7 @@ WARNING: Both ENABLE_SEMANTIC_SEARCH and VECTOR_SYNC_ENABLED are set. Using ENAB **Symptom:** ``` -Error: [oauth_single_audience] TOKEN_ENCRYPTION_KEY is required when ENABLE_SEMANTIC_SEARCH is enabled +Error: [login_flow] TOKEN_ENCRYPTION_KEY is required when ENABLE_SEMANTIC_SEARCH is enabled ``` **Solution:** @@ -442,13 +438,12 @@ When semantic search is enabled in multi-user modes, you need: ### Issue: Unexpected Mode Detected **Symptom:** -Server activates `oauth_single_audience` mode when you expected `multi_user_basic` +Server activates `login_flow` mode when you expected `multi_user_basic` **Solution:** Add explicit mode declaration: ```bash MCP_DEPLOYMENT_MODE=multi_user_basic -ENABLE_MULTI_USER_BASIC_AUTH=true ``` --- @@ -483,7 +478,7 @@ docker-compose up mcp **Expected Log Output (Multi-User OAuth + Semantic Search):** ``` -INFO: Using explicit deployment mode: oauth_single_audience +INFO: Using explicit deployment mode: login_flow INFO: Automatically enabled background operations for semantic search in multi-user mode. INFO: Vector sync enabled. Starting background scanner... ``` diff --git a/docs/configuration.md b/docs/configuration.md index a1879596..63577c9c 100644 --- a/docs/configuration.md +++ b/docs/configuration.md @@ -38,12 +38,12 @@ The server supports three deployment modes. See [Authentication](authentication. |------|-------------| | `single_user_basic` | Personal use, dev — credentials in env vars | | `multi_user_basic` | Internal deployments — clients send credentials via `Authorization: Basic` header | -| `login_flow_v2` | Hosted / OAuth-based MCP clients (claude.ai, Astrolabe Cloud) — recommended for multi-user | +| `login_flow` | Hosted / OAuth-based MCP clients (claude.ai, Astrolabe Cloud) — recommended for multi-user | You can declare the mode explicitly: ```dotenv -MCP_DEPLOYMENT_MODE=login_flow_v2 +MCP_DEPLOYMENT_MODE=login_flow ``` If `MCP_DEPLOYMENT_MODE` is not set, the server auto-detects from the other env vars below. @@ -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= @@ -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/running.md b/docs/running.md index b71ad6a7..0509447a 100644 --- a/docs/running.md +++ b/docs/running.md @@ -336,29 +336,19 @@ messages in the container logs: **At server boot (all modes):** ``` INFO ✅ Configuration validated successfully for mode +INFO Configuring MCP server for mode INFO Health check endpoints enabled: /health/live, /health/ready ``` -`` is one of `single_user_basic`, `multi_user_basic`, or -`oauth_single`, matching the `MCP_DEPLOYMENT_MODE` setting. - -**Additional BasicAuth-mode messages (at server boot):** -``` -INFO Configuring MCP server for mode -``` - -Here `` is the enum value (`single_user_basic` or `multi_user_basic`). +`` is one of `single_user_basic`, `multi_user_basic`, or `login_flow`, +matching the `MCP_DEPLOYMENT_MODE` setting. **Additional OAuth-mode messages (at server boot):** ``` -INFO Configuring MCP server for OAuth mode INFO OAuth client ready: ... INFO OAuth configuration complete ``` -Note the OAuth boot line logs the literal string `OAuth mode`, not the enum -value `oauth_single`. - **Additional single-user BasicAuth messages (per MCP session):** These fire when the first MCP client connects, not at server boot — if you diff --git a/docs/troubleshooting.md b/docs/troubleshooting.md index fd8e683a..d7e6b3a5 100644 --- a/docs/troubleshooting.md +++ b/docs/troubleshooting.md @@ -69,7 +69,7 @@ ENABLE_BACKGROUND_OPERATIONS=true **Symptom:** ``` -ValueError: Invalid MCP_DEPLOYMENT_MODE: 'oauth'. Valid values: single_user_basic, multi_user_basic, login_flow_v2 +ValueError: Invalid MCP_DEPLOYMENT_MODE: 'oauth'. Valid values: single_user_basic, multi_user_basic, login_flow ``` **Cause:** Invalid value for `MCP_DEPLOYMENT_MODE`. @@ -79,7 +79,7 @@ Use one of the valid mode values: ```bash MCP_DEPLOYMENT_MODE=single_user_basic # Single-user with username/app password MCP_DEPLOYMENT_MODE=multi_user_basic # Multi-user BasicAuth pass-through -MCP_DEPLOYMENT_MODE=login_flow_v2 # Multi-user via Login Flow v2 (recommended) +MCP_DEPLOYMENT_MODE=login_flow # Multi-user via Login Flow v2 (recommended) ``` Or remove `MCP_DEPLOYMENT_MODE` to use automatic detection. @@ -90,7 +90,7 @@ Or remove `MCP_DEPLOYMENT_MODE` to use automatic detection. **Symptom:** ``` -Error: [login_flow_v2] TOKEN_ENCRYPTION_KEY is required when ENABLE_SEMANTIC_SEARCH is enabled +Error: [login_flow] TOKEN_ENCRYPTION_KEY is required when ENABLE_SEMANTIC_SEARCH is enabled ``` **Cause:** In multi-user modes, semantic search automatically enables background operations, which require encrypted token storage. @@ -147,8 +147,8 @@ 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 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) ``` 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. @@ -441,7 +441,7 @@ If problems persist, open an issue on the [GitHub repository](https://github.com - **Server logs** (with `--log-level debug`) - **Nextcloud version** -- **Deployment mode** (single_user_basic / multi_user_basic / login_flow_v2) +- **Deployment mode** (single_user_basic / multi_user_basic / login_flow) - **Error messages** - **Steps to reproduce** - **Environment details** (OS, Python version, Docker vs local) 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 bae9d96e..600c2ffc 100644 --- a/env.sample +++ b/env.sample @@ -3,11 +3,17 @@ # ============================================ # Optional: Explicitly declare deployment mode (ADR-021) # If not set, mode is auto-detected from other settings -# Valid values: single_user_basic, multi_user_basic, oauth_single_audience, -# oauth_token_exchange +# Valid values: single_user_basic, multi_user_basic, login_flow +# +# 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. +# 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=oauth_single_audience +#MCP_DEPLOYMENT_MODE=login_flow # ============================================ # COMMON SETTINGS (Required for all modes) @@ -43,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/api/management.py b/nextcloud_mcp_server/api/management.py index 0f10d6b9..b621cec2 100644 --- a/nextcloud_mcp_server/api/management.py +++ b/nextcloud_mcp_server/api/management.py @@ -212,7 +212,7 @@ async def get_server_status(request: Request) -> JSONResponse: # Map deployment mode to auth_mode for API response # This helps clients (like Astrolabe) determine which auth flow to use - if mode == AuthMode.OAUTH_SINGLE_AUDIENCE: + if mode == AuthMode.LOGIN_FLOW: auth_mode = "oauth" elif mode == AuthMode.MULTI_USER_BASIC: auth_mode = "multi_user_basic" diff --git a/nextcloud_mcp_server/app.py b/nextcloud_mcp_server/app.py index 2fa6eff9..5bdedf86 100644 --- a/nextcloud_mcp_server/app.py +++ b/nextcloud_mcp_server/app.py @@ -1015,8 +1015,11 @@ def get_app(transport: str = "streamable-http", enabled_apps: list[str] | None = logger.info(f"✅ Configuration validated successfully for {mode.value} mode") logger.debug(f"Mode details:\n{get_mode_summary(mode)}") - # Derive helper variables for backward compatibility with existing code - oauth_enabled = mode == AuthMode.OAUTH_SINGLE_AUDIENCE + # Derive helper variables for backward compatibility with existing code. + # `oauth_enabled` is True for the LOGIN_FLOW (formerly OAUTH_SINGLE_AUDIENCE) + # multi-user OAuth mode — in this mode the MCP server is an OIDC relying + # party and Login Flow v2 acquires per-user Nextcloud app passwords. + oauth_enabled = mode == AuthMode.LOGIN_FLOW # Log hybrid authentication status for multi-user BasicAuth with offline access if mode == AuthMode.MULTI_USER_BASIC and settings.enable_offline_access: logger.info( @@ -1166,8 +1169,8 @@ def get_app(transport: str = "streamable-http", enabled_apps: list[str] | None = raise # Create MCP server based on detected mode - if mode == AuthMode.OAUTH_SINGLE_AUDIENCE: - logger.info("Configuring MCP server for OAuth mode") + if mode == AuthMode.LOGIN_FLOW: + logger.info("Configuring MCP server for %s mode", mode.value) # Asynchronously get the OAuth configuration ( @@ -1932,7 +1935,7 @@ def get_app(transport: str = "streamable-http", enabled_apps: list[str] | None = # Check authentication configuration # Report the deployment mode, not just whether OAuth is enabled # This helps clients (like Astrolabe) determine which auth flow to use - if mode == AuthMode.OAUTH_SINGLE_AUDIENCE: + if mode == AuthMode.LOGIN_FLOW: checks["auth_mode"] = "oauth" checks["auth_configured"] = "ok" elif mode == AuthMode.MULTI_USER_BASIC: diff --git a/nextcloud_mcp_server/config.py b/nextcloud_mcp_server/config.py index 3a205110..62eca989 100644 --- a/nextcloud_mcp_server/config.py +++ b/nextcloud_mcp_server/config.py @@ -408,9 +408,10 @@ def get_document_processor_config() -> dict[str, Any]: class Settings: """Application settings from environment variables.""" - # Deployment mode (ADR-021: explicit mode selection) + # Deployment mode (ADR-021: explicit mode selection; updated by ADR-022) # Optional: If not set, mode is auto-detected from other settings - # Valid values: single_user_basic, multi_user_basic, oauth_single_audience + # Valid values: single_user_basic, multi_user_basic, login_flow + # (ADR-022: `oauth_single_audience` was renamed to `login_flow`.) deployment_mode: str | None = None # OAuth/OIDC settings @@ -452,12 +453,19 @@ 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 + # Settings.__post_init__ 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 settings (ADR-022) + # Login Flow v2 derived flag (ADR-022). Internal — not user-settable. + # Auto-set by Settings.__post_init__ 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 @@ -599,6 +607,57 @@ class Settings: f"Smaller chunks may lose context. Consider using at least 1024 characters." ) + # --- ADR-022 follow-up: deployment mode is the single source of truth --- + # The ENABLE_MULTI_USER_BASIC_AUTH and ENABLE_LOGIN_FLOW env vars were + # removed in favour of MCP_DEPLOYMENT_MODE. We do TWO things here: + # + # 1. Loud-fail if a user still has either legacy env var set to a + # truthy value (silent removal would have flipped them into the + # wrong runtime mode). Only fires for truthy strings, so an + # explicit `ENABLE_LOGIN_FLOW=false` in a leftover .env passes + # through harmlessly. + # 2. Derive `enable_login_flow` and `enable_multi_user_basic_auth` + # from the resolved deployment mode here, in __post_init__, so + # every Settings instance carries correct flags. (`get_settings()` + # builds a fresh Settings on each call — without this, the + # mutation that used to live in detect_auth_mode would only stick + # on the startup Settings instance, leaving per-request handlers + # with default False values.) + _truthy = {"1", "true", "yes", "on"} + for _legacy, _replacement in ( + ("ENABLE_MULTI_USER_BASIC_AUTH", "multi_user_basic"), + ("ENABLE_LOGIN_FLOW", "login_flow"), + ): + if os.environ.get(_legacy, "").strip().lower() in _truthy: + 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." + ) + + # NOTE: this block mirrors the resolution logic in + # `config_validators.detect_auth_mode` (which works on strings via a + # `mode_map`). Both call sites resolve the deployment mode + # independently — the canonical AuthMode enum in detect_auth_mode, + # and the boolean derived flags here. **Keep them in sync when + # adding a new mode**: a new entry must be added in both places, in + # addition to `mode_map` (`config_validators.py`) and any + # MODE_REQUIREMENTS entry. + resolved_mode = (self.deployment_mode or "").strip().lower() + if not resolved_mode: + if self.nextcloud_username and self.nextcloud_password: + resolved_mode = "single_user_basic" + else: + # Default multi-user mode is Login Flow v2 (browser-based + # app-password acquisition); the un-augmented OAuth bearer + # pass-through it replaced needed unmerged Nextcloud + # user_oidc patches and is no longer supported. + resolved_mode = "login_flow" + + self.enable_multi_user_basic_auth = resolved_mode == "multi_user_basic" + self.enable_login_flow = resolved_mode == "login_flow" + def get_embedding_model_name(self) -> str: """ Get the active embedding model name based on provider priority. @@ -719,20 +778,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): @@ -744,7 +814,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 @@ -850,10 +920,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", - # Login Flow v2 settings (ADR-022) - "enable_login_flow": "ENABLE_LOGIN_FLOW", + # 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 d2e4ff8a..7434afa9 100644 --- a/nextcloud_mcp_server/config_validators.py +++ b/nextcloud_mcp_server/config_validators.py @@ -25,7 +25,7 @@ class AuthMode(Enum): SINGLE_USER_BASIC = "single_user_basic" MULTI_USER_BASIC = "multi_user_basic" - OAUTH_SINGLE_AUDIENCE = "oauth_single" + LOGIN_FLOW = "login_flow" @dataclass @@ -64,7 +64,6 @@ MODE_REQUIREMENTS: dict[AuthMode, ModeRequirements] = { "document_chunk_overlap", ], forbidden=[ - "enable_multi_user_basic_auth", "oidc_client_id", "oidc_client_secret", ], @@ -78,7 +77,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", @@ -113,7 +112,7 @@ MODE_REQUIREMENTS: dict[AuthMode, ModeRequirements] = { "Users provide credentials in request headers. " "Optional background sync using app passwords stored via Astrolabe.", ), - AuthMode.OAUTH_SINGLE_AUDIENCE: ModeRequirements( + AuthMode.LOGIN_FLOW: ModeRequirements( required=["nextcloud_host"], optional=[ # OAuth credentials (uses DCR if not provided) @@ -138,7 +137,6 @@ MODE_REQUIREMENTS: dict[AuthMode, ModeRequirements] = { forbidden=[ "nextcloud_username", "nextcloud_password", - "enable_multi_user_basic_auth", ], conditional={ "enable_offline_access": [ @@ -149,9 +147,13 @@ MODE_REQUIREMENTS: dict[AuthMode, ModeRequirements] = { # enables background operations in multi-user modes. No explicit # enable_offline_access setting required. }, - description="OAuth multi-user deployment with single-audience tokens. " - "Tokens work for both MCP server and Nextcloud APIs (pass-through). " - "Uses Dynamic Client Registration if credentials not provided.", + description="OAuth multi-user deployment using Login Flow v2 to acquire " + "per-user Nextcloud app passwords via a browser flow. The MCP server " + "is an OIDC relying party of a configurable IdP (Nextcloud's built-in " + "OIDC by default; Keycloak, AWS Cognito, etc. via OIDC_DISCOVERY_URL). " + "Uses Dynamic Client Registration if credentials not provided. " + "Replaces the deprecated direct OAuth bearer-token pass-through which " + "required unmerged user_oidc patches (see ADR-022).", ), } @@ -159,11 +161,21 @@ MODE_REQUIREMENTS: dict[AuthMode, ModeRequirements] = { def detect_auth_mode(settings: Settings) -> AuthMode: """Detect authentication mode from configuration. - Mode detection priority (ADR-021): - 0. Explicit MCP_DEPLOYMENT_MODE (if set) - NEW in ADR-021 - 1. Multi-user BasicAuth - 2. Single-user BasicAuth - 3. OAuth single-audience (default OAuth mode) + Mode detection priority (ADR-021, updated for ADR-022): + 0. Explicit MCP_DEPLOYMENT_MODE (if set) — NEW in ADR-021 + 1. Multi-user BasicAuth (only via explicit mode after ADR-022 follow-up) + 2. Single-user BasicAuth (auto-detected from credentials) + 3. Login Flow v2 (default — was OAuth single-audience pre-ADR-022) + + Pure function — the legacy-env-var deprecation and the derivation of + `enable_login_flow` / `enable_multi_user_basic_auth` now happen in + `Settings.__post_init__` so every Settings instance carries correct + flags regardless of how it was constructed. + + Keep the resolution logic here in sync with `Settings.__post_init__`: + both compute the canonical mode from `deployment_mode` (+ credentials + as a fallback). When adding a new mode, update `mode_map` *and* the + `__post_init__` resolution block in `config.py`. Args: settings: Application settings @@ -172,45 +184,53 @@ def detect_auth_mode(settings: Settings) -> AuthMode: Detected AuthMode Raises: - ValueError: If explicit deployment_mode is invalid or conflicts with detected mode + ValueError: If explicit deployment_mode is unrecognised. """ logger = logging.getLogger(__name__) - # ADR-021: Check for explicit deployment mode first + # ADR-021: explicit deployment mode wins if settings.deployment_mode: mode_str = settings.deployment_mode.lower().strip() - # Map string to AuthMode enum mode_map = { "single_user_basic": AuthMode.SINGLE_USER_BASIC, "multi_user_basic": AuthMode.MULTI_USER_BASIC, - "oauth_single_audience": AuthMode.OAUTH_SINGLE_AUDIENCE, + "login_flow": AuthMode.LOGIN_FLOW, } if mode_str not in mode_map: valid_modes = ", ".join(mode_map.keys()) + # ADR-022 migration hint: the most common upgrade pain is users + # carrying MCP_DEPLOYMENT_MODE=oauth_single_audience over from + # ADR-021. Surface a one-liner so they don't have to grep the + # changelog. + hint = ( + " (Note: 'oauth_single_audience' was renamed to 'login_flow' in ADR-022.)" + if mode_str == "oauth_single_audience" + else "" + ) raise ValueError( f"Invalid MCP_DEPLOYMENT_MODE: '{settings.deployment_mode}'. " - f"Valid values: {valid_modes}" + f"Valid values: {valid_modes}.{hint}" ) explicit_mode = mode_map[mode_str] logger.info(f"Using explicit deployment mode: {explicit_mode.value}") return explicit_mode - # Auto-detection (existing behavior) - # Check for multi-user BasicAuth - if settings.enable_multi_user_basic_auth: - 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 in + # `Settings.__post_init__`. - # Check for single-user BasicAuth (explicit credentials) if settings.nextcloud_username and settings.nextcloud_password: return AuthMode.SINGLE_USER_BASIC - # Default: OAuth single-audience mode - # This is the safest multi-user mode (no credential storage) - return AuthMode.OAUTH_SINGLE_AUDIENCE + # Default: Login Flow v2 multi-user mode (browser-based app-password flow). + return AuthMode.LOGIN_FLOW def validate_configuration(settings: Settings) -> tuple[AuthMode, list[str]]: @@ -301,7 +321,14 @@ def validate_configuration(settings: Settings) -> tuple[AuthMode, list[str]]: f"{settings.nextcloud_host}" ) - if mode == AuthMode.OAUTH_SINGLE_AUDIENCE: + if mode == AuthMode.LOGIN_FLOW: + # 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 in + # `Settings.__post_init__`, 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) if not settings.oidc_client_id or not settings.oidc_client_secret: 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/conftest.py b/tests/conftest.py index e8aade3f..f131c7e9 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -265,7 +265,7 @@ async def nc_mcp_basic_auth_client( ) -> AsyncGenerator[ClientSession, Any]: """ Fixture to create an MCP client session with BasicAuth credentials. - Connects to the multi-user BasicAuth MCP server on port 8003 with ENABLE_MULTI_USER_BASIC_AUTH=true. + Connects to the multi-user BasicAuth MCP server on port 8003 with MCP_DEPLOYMENT_MODE=multi_user_basic. Uses BasicAuth credentials for multi-user pass-through mode (ADR-020). Credentials are passed in Authorization header and forwarded to Nextcloud APIs. diff --git a/tests/unit/test_config_validators.py b/tests/unit/test_config_validators.py index 3aad9eaa..f7e97dec 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.""" @@ -43,14 +52,14 @@ class TestModeDetection: mode = detect_auth_mode(settings) assert mode == AuthMode.SINGLE_USER_BASIC - def test_oauth_single_audience_default(self): - """Test OAuth single-audience is default mode.""" + def test_login_flow_default(self): + """Test Login Flow v2 is the default multi-user mode.""" settings = Settings( nextcloud_host="http://localhost", ) mode = detect_auth_mode(settings) - assert mode == AuthMode.OAUTH_SINGLE_AUDIENCE + assert mode == AuthMode.LOGIN_FLOW class TestSingleUserBasicValidation: @@ -108,7 +117,7 @@ class TestSingleUserBasicValidation: # Mode detection requires BOTH username AND password for single-user BasicAuth # If only one is present, it defaults to OAuth single-audience - assert mode == AuthMode.OAUTH_SINGLE_AUDIENCE + assert mode == AuthMode.LOGIN_FLOW # In OAuth mode, having a password set is forbidden assert any("nextcloud_password" in err.lower() for err in errors) @@ -123,24 +132,29 @@ class TestSingleUserBasicValidation: # Mode detection requires BOTH username AND password for single-user BasicAuth # If only one is present, it defaults to OAuth single-audience - assert mode == AuthMode.OAUTH_SINGLE_AUDIENCE + assert mode == AuthMode.LOGIN_FLOW # 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", @@ -285,19 +298,21 @@ class TestMultiUserBasicValidation: assert settings.enable_offline_access is True -class TestOAuthSingleAudienceValidation: - """Test validation for OAuth single-audience mode.""" +class TestLoginFlowValidation: + """Test validation for Login Flow v2 mode (formerly OAUTH_SINGLE_AUDIENCE).""" def test_valid_minimal_config(self): - """Test valid minimal OAuth single-audience config.""" + """Test valid minimal Login Flow v2 config — enable_login_flow is now derived.""" settings = Settings( nextcloud_host="http://localhost", ) mode, errors = validate_configuration(settings) - assert mode == AuthMode.OAUTH_SINGLE_AUDIENCE + 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.""" @@ -309,7 +324,7 @@ class TestOAuthSingleAudienceValidation: mode, errors = validate_configuration(settings) - assert mode == AuthMode.OAUTH_SINGLE_AUDIENCE + assert mode == AuthMode.LOGIN_FLOW assert len(errors) == 0 def test_valid_with_offline_access(self): @@ -325,7 +340,7 @@ class TestOAuthSingleAudienceValidation: mode, errors = validate_configuration(settings) - assert mode == AuthMode.OAUTH_SINGLE_AUDIENCE + assert mode == AuthMode.LOGIN_FLOW assert len(errors) == 0 def test_forbidden_username_password(self): @@ -351,11 +366,35 @@ class TestOAuthSingleAudienceValidation: mode, errors = validate_configuration(settings) - assert mode == AuthMode.OAUTH_SINGLE_AUDIENCE + assert mode == AuthMode.LOGIN_FLOW assert any("token_encryption_key" in err.lower() for err in errors) - def test_vector_sync_auto_enables_background_ops_in_oauth_mode(self): - """Test vector sync automatically enables background operations in OAuth mode (ADR-021).""" + def test_login_flow_mode_auto_derives_enable_login_flow_flag(self): + """ADR-022 follow-up: deployment_mode=login_flow auto-derives the flag. + + Users no longer need to set ENABLE_LOGIN_FLOW=true (env var was + removed); Settings.__post_init__ populates settings.enable_login_flow + from the resolved mode at construction time, so every Settings + instance carries correct flags regardless of how it was built. + """ + # Default-fallback case: no auth env vars → LOGIN_FLOW. + settings = Settings(nextcloud_host="http://localhost") + assert settings.enable_login_flow is True + assert settings.enable_multi_user_basic_auth is False + assert detect_auth_mode(settings) == AuthMode.LOGIN_FLOW + + # Single-user BasicAuth (credentials set) → neither derived flag. + basic_settings = Settings( + nextcloud_host="http://localhost", + nextcloud_username="alice", + nextcloud_password="password", + ) + assert basic_settings.enable_login_flow is False + assert basic_settings.enable_multi_user_basic_auth is False + assert detect_auth_mode(basic_settings) == AuthMode.SINGLE_USER_BASIC + + 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).""" # Before ADR-021: This would have failed validation (required explicit ENABLE_OFFLINE_ACCESS) # After ADR-021: vector_sync_enabled auto-enables background operations in multi-user modes with patch.dict( @@ -367,7 +406,7 @@ class TestOAuthSingleAudienceValidation: "OLLAMA_BASE_URL": "http://ollama:11434", "TOKEN_ENCRYPTION_KEY": "test-key", "TOKEN_STORAGE_DB": "/tmp/test.db", - # Note: No username/password = OAuth mode + # Note: No username/password = Login Flow v2 multi-user OAuth mode }, clear=True, ): @@ -377,7 +416,7 @@ class TestOAuthSingleAudienceValidation: settings = get_settings() mode, errors = validate_configuration(settings) - assert mode == AuthMode.OAUTH_SINGLE_AUDIENCE + assert mode == AuthMode.LOGIN_FLOW # Should have no errors - background operations auto-enabled assert len(errors) == 0 # Verify background operations were auto-enabled @@ -515,8 +554,8 @@ class TestConfigurationConsolidation: settings = get_settings() assert settings.enable_offline_access is True - def test_semantic_search_auto_enables_background_ops_in_oauth_mode(self): - """Test ENABLE_SEMANTIC_SEARCH automatically enables background operations in OAuth mode.""" + def test_semantic_search_auto_enables_background_ops_in_login_flow_mode(self): + """Test ENABLE_SEMANTIC_SEARCH automatically enables background operations in Login Flow v2 mode.""" with patch.dict( os.environ, { @@ -634,7 +673,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", @@ -707,13 +746,13 @@ class TestExplicitModeSelection: assert mode == AuthMode.MULTI_USER_BASIC - def test_explicit_oauth_single_audience_mode(self): - """Test explicit oauth_single_audience mode selection.""" + def test_explicit_login_flow_mode(self): + """Test explicit login_flow mode selection.""" with patch.dict( os.environ, { "NEXTCLOUD_HOST": "http://localhost:8080", - "MCP_DEPLOYMENT_MODE": "oauth_single_audience", + "MCP_DEPLOYMENT_MODE": "login_flow", }, clear=True, ): @@ -723,7 +762,7 @@ class TestExplicitModeSelection: settings = get_settings() mode = detect_auth_mode(settings) - assert mode == AuthMode.OAUTH_SINGLE_AUDIENCE + assert mode == AuthMode.LOGIN_FLOW def test_invalid_deployment_mode_raises_error(self): """Test invalid MCP_DEPLOYMENT_MODE raises ValueError.""" @@ -749,6 +788,34 @@ class TestExplicitModeSelection: assert "invalid_mode" in str(e) assert "Valid values:" in str(e) + def test_oauth_single_audience_migration_hint(self): + """ADR-022: rejecting `oauth_single_audience` surfaces a rename hint. + + Pins the special-case branch in detect_auth_mode that helps users + upgrading from ADR-021 configurations spot the rename without + having to grep the changelog. + """ + with patch.dict( + os.environ, + { + "NEXTCLOUD_HOST": "http://localhost:8080", + "MCP_DEPLOYMENT_MODE": "oauth_single_audience", + }, + 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) + + msg = str(exc.value) + assert "oauth_single_audience" in msg + assert "login_flow" in msg + assert "ADR-022" in msg + def test_explicit_mode_overrides_auto_detection(self): """Test explicit mode takes precedence over auto-detection.""" with patch.dict( @@ -757,7 +824,7 @@ class TestExplicitModeSelection: "NEXTCLOUD_HOST": "http://localhost:8080", "NEXTCLOUD_USERNAME": "admin", # Would auto-detect as single_user_basic "NEXTCLOUD_PASSWORD": "password", - "MCP_DEPLOYMENT_MODE": "oauth_single_audience", # Explicit override + "MCP_DEPLOYMENT_MODE": "login_flow", # Explicit override }, clear=True, ): @@ -768,7 +835,7 @@ class TestExplicitModeSelection: mode = detect_auth_mode(settings) # Should use explicit mode, not auto-detected mode - assert mode == AuthMode.OAUTH_SINGLE_AUDIENCE + assert mode == AuthMode.LOGIN_FLOW def test_case_insensitive_mode_names(self): """Test MCP_DEPLOYMENT_MODE is case-insensitive.""" @@ -776,7 +843,7 @@ class TestExplicitModeSelection: os.environ, { "NEXTCLOUD_HOST": "http://localhost:8080", - "MCP_DEPLOYMENT_MODE": "OAUTH_SINGLE_AUDIENCE", # Uppercase + "MCP_DEPLOYMENT_MODE": "LOGIN_FLOW", # Uppercase }, clear=True, ): @@ -786,7 +853,7 @@ class TestExplicitModeSelection: settings = get_settings() mode = detect_auth_mode(settings) - assert mode == AuthMode.OAUTH_SINGLE_AUDIENCE + assert mode == AuthMode.LOGIN_FLOW def test_whitespace_in_mode_name_stripped(self): """Test whitespace in MCP_DEPLOYMENT_MODE is stripped.""" @@ -794,7 +861,7 @@ class TestExplicitModeSelection: os.environ, { "NEXTCLOUD_HOST": "http://localhost:8080", - "MCP_DEPLOYMENT_MODE": " oauth_single_audience ", # Whitespace + "MCP_DEPLOYMENT_MODE": " login_flow ", # Whitespace }, clear=True, ): @@ -804,4 +871,114 @@ class TestExplicitModeSelection: settings = get_settings() mode = detect_auth_mode(settings) - assert mode == AuthMode.OAUTH_SINGLE_AUDIENCE + 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. + The check lives in Settings.__post_init__ so it fires at config + load time, not only when detect_auth_mode is reached. + """ + 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() + + with pytest.raises(ValueError) as exc: + get_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 at Settings construction. + """ + 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() + + with pytest.raises(ValueError) as exc: + get_settings() + + assert "ENABLE_LOGIN_FLOW" in str(exc.value) + assert "login_flow" in str(exc.value) + + def test_legacy_env_var_check_ignores_falsy_strings(self): + """ADR-022 follow-up: a leftover ENABLE_LOGIN_FLOW=false must NOT error. + + Reviewer round 2 found that the legacy check used `os.getenv(legacy)` + which is truthy for the literal string 'false'. A user who had + explicitly set the flag to false (meaning 'I don't want this') would + get a startup ValueError after upgrading, which is wrong. The fix + only fires for explicitly-truthy values. + """ + with patch.dict( + os.environ, + { + "NEXTCLOUD_HOST": "http://localhost:8080", + "ENABLE_LOGIN_FLOW": "false", + "ENABLE_MULTI_USER_BASIC_AUTH": "0", + }, + clear=True, + ): + from nextcloud_mcp_server.config import get_settings + + _reload_config() + settings = get_settings() # Must not raise. + + # Falls through to LOGIN_FLOW (the auto-detect default) since + # no credentials are set. + assert detect_auth_mode(settings) == AuthMode.LOGIN_FLOW + + def test_derived_flags_stable_across_get_settings_calls(self): + """Regression: derived flags must persist across get_settings() calls. + + `get_settings()` builds a fresh Settings on each invocation. Earlier + commits set the derived flags as a side effect of detect_auth_mode, + which meant the *next* get_settings() call started with default + False — breaking per-request handlers in the integration tests + for `mcp-multi-user-basic` and `mcp-login-flow`. The fix moves the + derivation into Settings.__post_init__ so every instance carries + correct flags. + """ + with patch.dict( + os.environ, + { + "NEXTCLOUD_HOST": "http://localhost:8080", + "MCP_DEPLOYMENT_MODE": "multi_user_basic", + }, + clear=True, + ): + from nextcloud_mcp_server.config import get_settings + + _reload_config() + + s1 = get_settings() + s2 = get_settings() + + assert s1 is not s2 # fresh instance each call + assert s1.enable_multi_user_basic_auth is True + assert s2.enable_multi_user_basic_auth is True + assert s1.enable_login_flow is False + assert s2.enable_login_flow is False diff --git a/tests/unit/test_management_status_endpoint.py b/tests/unit/test_management_status_endpoint.py index f6ea5f40..313a12a7 100644 --- a/tests/unit/test_management_status_endpoint.py +++ b/tests/unit/test_management_status_endpoint.py @@ -173,7 +173,7 @@ class TestStatusEndpointOidcConfig: ), patch( "nextcloud_mcp_server.api.management.detect_auth_mode", - return_value=AuthMode.OAUTH_SINGLE_AUDIENCE, + return_value=AuthMode.LOGIN_FLOW, ), ): app = create_test_app() diff --git a/tests/unit/test_stdio.py b/tests/unit/test_stdio.py index a70053be..cd2418ae 100644 --- a/tests/unit/test_stdio.py +++ b/tests/unit/test_stdio.py @@ -14,8 +14,8 @@ def single_user_env(monkeypatch): monkeypatch.setenv("NEXTCLOUD_HOST", "https://cloud.example.com") monkeypatch.setenv("NEXTCLOUD_USERNAME", "admin") monkeypatch.setenv("NEXTCLOUD_PASSWORD", "secret") - # Ensure multi-user mode is off (may leak from other tests) - monkeypatch.delenv("ENABLE_MULTI_USER_BASIC_AUTH", raising=False) + # Ensure no explicit deployment mode leaks from other tests + monkeypatch.delenv("MCP_DEPLOYMENT_MODE", raising=False) _reload_config() yield _reload_config()