diff --git a/api/auth.py b/api/auth.py index fa10e6fb..fef5dc95 100644 --- a/api/auth.py +++ b/api/auth.py @@ -512,7 +512,18 @@ def get_oidc_startup_warning() -> str | None: issuer = bool(pick("issuer", "HERMES_WEBUI_OIDC_ISSUER")) client_id = bool(pick("client_id", "HERMES_WEBUI_OIDC_CLIENT_ID")) allow_claim = bool(pick("allow_claim", "HERMES_WEBUI_OIDC_ALLOW_CLAIM")) - allow_values = bool(pick("allow_values", "HERMES_WEBUI_OIDC_ALLOW_VALUES")) + raw_allow_env = os.getenv("HERMES_WEBUI_OIDC_ALLOW_VALUES") + raw_allow = raw_allow_env if raw_allow_env is not None else raw.get("allow_values") + normalized_allow_values = [] + allow_values_warning = None + try: + from api import auth_oidc + + normalized_allow_values = auth_oidc._normalize_allow_values(raw_allow) + allow_values_warning = auth_oidc._ALLOW_VALUES_WHITESPACE_WARNING + except Exception: + logger.debug("Failed to normalize OIDC allow_values", exc_info=True) + allow_values = bool(normalized_allow_values) if not any((issuer, client_id, allow_claim, allow_values)): return None @@ -538,16 +549,13 @@ def get_oidc_startup_warning() -> str | None: # Detect whitespace-only allow_values scalar that may contain multiple intended values. # Runs unconditionally so the warning reaches startup even when other auth methods # short-circuit is_auth_enabled() before the OIDC branch is evaluated. - raw_allow_env = os.getenv("HERMES_WEBUI_OIDC_ALLOW_VALUES") - raw_allow = raw_allow_env if raw_allow_env is not None else raw.get("allow_values") - if raw_allow is not None and not isinstance(raw_allow, (list, tuple, set)): - try: - from api.auth_oidc import _normalize_allow_values, _ALLOW_VALUES_WHITESPACE_WARNING - normalized = _normalize_allow_values(raw_allow) - if any(any(ch.isspace() for ch in v) for v in normalized): - warnings.append(_ALLOW_VALUES_WHITESPACE_WARNING) - except Exception: - logger.debug("Failed to check OIDC allow_values for whitespace", exc_info=True) + if ( + allow_values_warning is not None + and raw_allow is not None + and not isinstance(raw_allow, (list, tuple, set)) + and any(any(ch.isspace() for ch in v) for v in normalized_allow_values) + ): + warnings.append(allow_values_warning) return "\n".join(warnings) if warnings else None diff --git a/tests/test_auth_oidc.py b/tests/test_auth_oidc.py index 74bd91c1..d36a0dd8 100644 --- a/tests/test_auth_oidc.py +++ b/tests/test_auth_oidc.py @@ -16,6 +16,17 @@ import api.auth_oidc as auth_oidc from api.auth_oidc import _enforce_allowlist, _normalize_allow_values, OIDCAuthError +@pytest.fixture(autouse=True) +def _clear_oidc_environment(monkeypatch): + for name in ( + "HERMES_WEBUI_OIDC_ISSUER", + "HERMES_WEBUI_OIDC_CLIENT_ID", + "HERMES_WEBUI_OIDC_ALLOW_CLAIM", + "HERMES_WEBUI_OIDC_ALLOW_VALUES", + ): + monkeypatch.delenv(name, raising=False) + + @pytest.fixture(autouse=True) def _reset_warned_cache(): auth_oidc._warned_allow_values.clear() @@ -23,6 +34,58 @@ def _reset_warned_cache(): auth_oidc._warned_allow_values.clear() +def _startup_warning(monkeypatch, allow_values): + import api.auth as auth + + monkeypatch.setattr( + auth, + "get_config", + lambda: { + "webui_oidc": { + "issuer": "https://issuer.example", + "client_id": "webui-client", + "allow_claim": "email", + "allow_values": allow_values, + } + }, + ) + return auth.get_oidc_startup_warning() + + +@pytest.mark.parametrize("allow_values", [[], [""]]) +def test_startup_warning_treats_empty_allowlist_shapes_as_missing(monkeypatch, allow_values): + warning = _startup_warning(monkeypatch, allow_values) + + assert warning is not None + assert "allow_values" in warning + assert auth_oidc._ALLOW_VALUES_WHITESPACE_WARNING not in warning + + +def test_startup_warning_accepts_multi_word_list(monkeypatch): + assert _startup_warning(monkeypatch, ["Hermes Users"]) is None + + +def test_startup_warning_accepts_comma_scalar(monkeypatch): + assert _startup_warning(monkeypatch, "alice@example.com,bob@example.com") is None + + +def test_startup_warning_reports_whitespace_scalar_migration(monkeypatch): + warning = _startup_warning(monkeypatch, "alice@example.com bob@example.com") + + assert warning is not None + assert "partially configured" not in warning + assert auth_oidc._ALLOW_VALUES_WHITESPACE_WARNING in warning + + +def test_startup_warning_uses_environment_allow_values_precedence(monkeypatch): + monkeypatch.setenv("HERMES_WEBUI_OIDC_ALLOW_VALUES", "") + + warning = _startup_warning(monkeypatch, ["alice@example.com"]) + + assert warning is not None + assert "allow_values" in warning + + def _resolve(monkeypatch, *, env=None, cfg_list=None): """Call _resolve_oidc_config with controlled allow_values input.