admin/mcp-nextcloud
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

refactor(config)!: drop ENABLE_MULTI_USER_BASIC_AUTH env var, fail loud on legacy aliases

Browse Source 
Same pattern as the ENABLE_LOGIN_FLOW removal in the previous commit:
the deployment mode (MCP_DEPLOYMENT_MODE) is the single source of truth
for selecting an auth flow. The ENABLE_MULTI_USER_BASIC_AUTH env-var
alias is redundant with `MCP_DEPLOYMENT_MODE=multi_user_basic`.

Unlike the ENABLE_LOGIN_FLOW removal — where silent removal was safe
because Login Flow v2 is the auto-detection default — silent removal
here would be a surprise: a user with only ENABLE_MULTI_USER_BASIC_AUTH=true
in their .env would auto-detect into LOGIN_FLOW after upgrade (wrong
runtime mode). Mitigation: detect_auth_mode now reads os.environ
directly for both legacy aliases and raises ValueError with a one-line
migration message if either is set. Applied retroactively to
ENABLE_LOGIN_FLOW as well — loud is better than silent.

- nextcloud_mcp_server/config.py:
  - Drop the dynaconf env-var alias entry for ENABLE_MULTI_USER_BASIC_AUTH.
  - Update the `enable_multi_user_basic_auth` field docstring to mark it
    as derived / not user-settable.
  - `_is_multi_user_mode()` (early-config helper, runs before Settings
    is built) switched to checking MCP_DEPLOYMENT_MODE directly. Now
    consistent with the canonical detection in detect_auth_mode.
- nextcloud_mcp_server/config_validators.py:
  - Drop the auto-detection branch (`if settings.enable_multi_user_basic_auth`).
    Selection of MULTI_USER_BASIC is now exclusively via the explicit
    MCP_DEPLOYMENT_MODE branch.
  - Add `enable_multi_user_basic_auth` to `_sync_derived_flags` alongside
    `enable_login_flow` — both flags are now derived from the resolved mode.
  - Drop `enable_multi_user_basic_auth` from
    `MODE_REQUIREMENTS[MULTI_USER_BASIC].required` and from the
    `forbidden` lists of SINGLE_USER_BASIC and LOGIN_FLOW (no longer
    user input → no meaningful forbidden check).
  - Add loud-deprecation `ValueError` block at the top of detect_auth_mode
    that errors with a clear migration message when ENABLE_MULTI_USER_BASIC_AUTH
    or ENABLE_LOGIN_FLOW is found in os.environ.
- tests/unit/test_config_validators.py:
  - Switch ~10 fixtures from `enable_multi_user_basic_auth=True` to
    `deployment_mode="multi_user_basic"` (mirrors `enable_login_flow`
    treatment from the previous commit).
  - Switch two `patch.dict(os.environ, {"ENABLE_MULTI_USER_BASIC_AUTH": "true"})`
    blocks to use MCP_DEPLOYMENT_MODE.
  - Rename `test_forbidden_multi_user_basic_auth` to
    `test_forbidden_multi_user_basic_when_credentials_present` — the
    scenario is now an explicit-mode + credentials conflict, not an
    env-var-flag conflict.
  - Add `test_legacy_enable_multi_user_basic_auth_env_var_errors` and
    `test_legacy_enable_login_flow_env_var_errors` to exercise the new
    loud-deprecation ValueError path.
- docker-compose.yml: mcp-multi-user-basic profile switched to
  `MCP_DEPLOYMENT_MODE=multi_user_basic`.
- env.sample: replaced `#ENABLE_MULTI_USER_BASIC_AUTH=true` example with
  `#MCP_DEPLOYMENT_MODE=multi_user_basic`.
- docs/authentication.md, configuration.md, troubleshooting.md,
  auth-flows.md, webhook-management-guide.md,
  configuration-migration-v2.md, ADR-025: replaced env-var examples
  with the canonical MCP_DEPLOYMENT_MODE form.
- docs/ADR-020: marked partly superseded by ADR-022.
- CLAUDE.md: Multi-User BasicAuth section updated to set
  MCP_DEPLOYMENT_MODE.
- nextcloud_mcp_server/vector/oauth_sync.py: module docstring updated.

BREAKING CHANGE: ENABLE_MULTI_USER_BASIC_AUTH is no longer read from
the environment, and setting it now raises a startup ValueError with
a migration message. Replace `ENABLE_MULTI_USER_BASIC_AUTH=true` with
`MCP_DEPLOYMENT_MODE=multi_user_basic`. The same loud-deprecation
check is also applied to the recently-removed ENABLE_LOGIN_FLOW —
replace with `MCP_DEPLOYMENT_MODE=login_flow` (or drop both;
`login_flow` is the auto-detect default when no other auth env vars
are set).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
Chris Coutinho
2026-05-12 20:06:16 +02:00
co-authored by Claude Opus 4.7
parent df4994e860
commit 282c245da1
15 changed files with 166 additions and 68 deletions
Download Patch File Download Diff File Expand all files Collapse all files
tests/unit/test_config_validators.py
+81 -17
View File
@@ -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)