fix(auth): align OIDC startup checks with runtime normalization (#6603)
This commit is contained in:
1 parent
28428d3753
commit
737ba90440
2 files changed
+82
-11
No files matched your search
+19
-11
@@ -512,7 +512,18 @@ def get_oidc_startup_warning() -> str | None:
|
|||||||
issuer = bool(pick("issuer", "HERMES_WEBUI_OIDC_ISSUER"))
|
issuer = bool(pick("issuer", "HERMES_WEBUI_OIDC_ISSUER"))
|
||||||
client_id = bool(pick("client_id", "HERMES_WEBUI_OIDC_CLIENT_ID"))
|
client_id = bool(pick("client_id", "HERMES_WEBUI_OIDC_CLIENT_ID"))
|
||||||
allow_claim = bool(pick("allow_claim", "HERMES_WEBUI_OIDC_ALLOW_CLAIM"))
|
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)):
|
if not any((issuer, client_id, allow_claim, allow_values)):
|
||||||
return None
|
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.
|
# Detect whitespace-only allow_values scalar that may contain multiple intended values.
|
||||||
# Runs unconditionally so the warning reaches startup even when other auth methods
|
# Runs unconditionally so the warning reaches startup even when other auth methods
|
||||||
# short-circuit is_auth_enabled() before the OIDC branch is evaluated.
|
# short-circuit is_auth_enabled() before the OIDC branch is evaluated.
|
||||||
raw_allow_env = os.getenv("HERMES_WEBUI_OIDC_ALLOW_VALUES")
|
if (
|
||||||
raw_allow = raw_allow_env if raw_allow_env is not None else raw.get("allow_values")
|
allow_values_warning is not None
|
||||||
if raw_allow is not None and not isinstance(raw_allow, (list, tuple, set)):
|
and raw_allow is not None
|
||||||
try:
|
and not isinstance(raw_allow, (list, tuple, set))
|
||||||
from api.auth_oidc import _normalize_allow_values, _ALLOW_VALUES_WHITESPACE_WARNING
|
and any(any(ch.isspace() for ch in v) for v in normalized_allow_values)
|
||||||
normalized = _normalize_allow_values(raw_allow)
|
):
|
||||||
if any(any(ch.isspace() for ch in v) for v in normalized):
|
warnings.append(allow_values_warning)
|
||||||
warnings.append(_ALLOW_VALUES_WHITESPACE_WARNING)
|
|
||||||
except Exception:
|
|
||||||
logger.debug("Failed to check OIDC allow_values for whitespace", exc_info=True)
|
|
||||||
|
|
||||||
return "\n".join(warnings) if warnings else None
|
return "\n".join(warnings) if warnings else None
|
||||||
|
|
||||||
|
|||||||
@@ -16,6 +16,17 @@ import api.auth_oidc as auth_oidc
|
|||||||
from api.auth_oidc import _enforce_allowlist, _normalize_allow_values, OIDCAuthError
|
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)
|
@pytest.fixture(autouse=True)
|
||||||
def _reset_warned_cache():
|
def _reset_warned_cache():
|
||||||
auth_oidc._warned_allow_values.clear()
|
auth_oidc._warned_allow_values.clear()
|
||||||
@@ -23,6 +34,58 @@ def _reset_warned_cache():
|
|||||||
auth_oidc._warned_allow_values.clear()
|
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):
|
def _resolve(monkeypatch, *, env=None, cfg_list=None):
|
||||||
"""Call _resolve_oidc_config with controlled allow_values input.
|
"""Call _resolve_oidc_config with controlled allow_values input.
|
||||||
|
|
||||||
|
|||||||
Reference in new issue
Block a user