fix(auth): harden OIDC key selection and startup guidance (#3825)
This commit is contained in:
1 parent
7d0e86ecc5
commit
f7a9c98810
4 files changed
+129
No files matched your search
@@ -259,6 +259,8 @@ If an AI assistant is helping with install, reinstall, bootstrap, provider setup
|
|||||||
- Enable via `HERMES_WEBUI_PASSWORD` env var or Settings panel
|
- Enable via `HERMES_WEBUI_PASSWORD` env var or Settings panel
|
||||||
- Optional passkeys/WebAuthn -- register from Settings -> System after signing in with a password; the login page only shows passkey sign-in after at least one passkey exists
|
- Optional passkeys/WebAuthn -- register from Settings -> System after signing in with a password; the login page only shows passkey sign-in after at least one passkey exists
|
||||||
- After registering at least one passkey, Settings -> System can remove the password and keep passkey-only sign-in enabled. Password auth remains the bootstrap/recovery path until you choose to go passwordless; passkeys are same-origin and stored locally in the WebUI state directory
|
- After registering at least one passkey, Settings -> System can remove the password and keep passkey-only sign-in enabled. Password auth remains the bootstrap/recovery path until you choose to go passwordless; passkeys are same-origin and stored locally in the WebUI state directory
|
||||||
|
- Optional native OIDC login for WebUI sessions -- configure `webui_oidc.issuer`, `client_id`, `allow_claim`, and `allow_values` in `config.yaml`, or set the matching `HERMES_WEBUI_OIDC_*` environment variables. OIDC stays disabled until all four are present, and startup prints a warning if the config is partial.
|
||||||
|
- Native OIDC stores the PKCE/state nonce flow in process memory. That works for the shipped single-process server, and it also works behind a load balancer when callbacks stay sticky to the same WebUI instance. Multi-instance deployments need session affinity, or the callback can land on a different process and fail state validation.
|
||||||
- Signed HMAC HTTP-only cookie with 24h TTL
|
- Signed HMAC HTTP-only cookie with 24h TTL
|
||||||
- Minimal dark-themed login page at `/login`
|
- Minimal dark-themed login page at `/login`
|
||||||
- Security headers on all responses (X-Content-Type-Options, X-Frame-Options, Referrer-Policy)
|
- Security headers on all responses (X-Content-Type-Options, X-Frame-Options, Referrer-Policy)
|
||||||
|
|||||||
@@ -54,6 +54,34 @@ def is_oidc_enabled() -> bool:
|
|||||||
and cfg.get("allow_values")
|
and cfg.get("allow_values")
|
||||||
)
|
)
|
||||||
|
|
||||||
|
def get_oidc_startup_warning() -> str | None:
|
||||||
|
cfg = _resolve_oidc_config()
|
||||||
|
issuer = bool(cfg.get("issuer"))
|
||||||
|
client_id = bool(cfg.get("client_id"))
|
||||||
|
allow_claim = bool(cfg.get("allow_claim"))
|
||||||
|
allow_values = bool(cfg.get("allow_values"))
|
||||||
|
|
||||||
|
if not any((issuer, client_id, allow_claim, allow_values)):
|
||||||
|
return None
|
||||||
|
if issuer and client_id and allow_claim and allow_values:
|
||||||
|
return None
|
||||||
|
|
||||||
|
missing = []
|
||||||
|
if not issuer:
|
||||||
|
missing.append("issuer")
|
||||||
|
if not client_id:
|
||||||
|
missing.append("client_id")
|
||||||
|
if not allow_claim:
|
||||||
|
missing.append("allow_claim")
|
||||||
|
if not allow_values:
|
||||||
|
missing.append("allow_values")
|
||||||
|
|
||||||
|
joined = ", ".join(missing)
|
||||||
|
return (
|
||||||
|
"Native OIDC login is only partially configured; missing "
|
||||||
|
f"{joined}. The WebUI will not enable OIDC auth until all four fields are set."
|
||||||
|
)
|
||||||
|
|
||||||
|
|
||||||
def build_authorization_redirect(
|
def build_authorization_redirect(
|
||||||
request_base_url: str,
|
request_base_url: str,
|
||||||
@@ -404,11 +432,21 @@ def _select_public_key(jwks: dict[str, Any], header: dict[str, Any]):
|
|||||||
continue
|
continue
|
||||||
if key.get("alg") not in (None, alg):
|
if key.get("alg") not in (None, alg):
|
||||||
continue
|
continue
|
||||||
|
if not _jwk_matches_alg_family(key, alg):
|
||||||
|
continue
|
||||||
matches.append(key)
|
matches.append(key)
|
||||||
if not matches:
|
if not matches:
|
||||||
raise OIDCAuthError("OIDC JWKS did not contain the signing key for this id_token", status_code=502)
|
raise OIDCAuthError("OIDC JWKS did not contain the signing key for this id_token", status_code=502)
|
||||||
return _jwk_to_public_key(matches[0])
|
return _jwk_to_public_key(matches[0])
|
||||||
|
|
||||||
|
def _jwk_matches_alg_family(jwk: dict[str, Any], alg: str) -> bool:
|
||||||
|
kty = str(jwk.get("kty") or "").strip()
|
||||||
|
if alg.startswith("RS"):
|
||||||
|
return kty == "RSA"
|
||||||
|
if alg.startswith("ES"):
|
||||||
|
return kty == "EC"
|
||||||
|
return True
|
||||||
|
|
||||||
|
|
||||||
def _jwk_to_public_key(jwk: dict[str, Any]):
|
def _jwk_to_public_key(jwk: dict[str, Any]):
|
||||||
kty = str(jwk.get("kty") or "").strip()
|
kty = str(jwk.get("kty") or "").strip()
|
||||||
|
|||||||
@@ -585,6 +585,7 @@ def main() -> None:
|
|||||||
print('[ok] Running within container.', flush=True)
|
print('[ok] Running within container.', flush=True)
|
||||||
|
|
||||||
from api.auth import is_auth_enabled
|
from api.auth import is_auth_enabled
|
||||||
|
from api.auth_oidc import get_oidc_startup_warning
|
||||||
if HOST not in ('127.0.0.1', '::1', 'localhost') and not is_auth_enabled():
|
if HOST not in ('127.0.0.1', '::1', 'localhost') and not is_auth_enabled():
|
||||||
print(f'[!!] WARNING: Binding to {HOST} with NO PASSWORD SET.', flush=True)
|
print(f'[!!] WARNING: Binding to {HOST} with NO PASSWORD SET.', flush=True)
|
||||||
print(f' Anyone on the network can access your filesystem and agent.', flush=True)
|
print(f' Anyone on the network can access your filesystem and agent.', flush=True)
|
||||||
@@ -597,6 +598,10 @@ def main() -> None:
|
|||||||
print(f' and memory via the local API. Set HERMES_WEBUI_PASSWORD to', flush=True)
|
print(f' and memory via the local API. Set HERMES_WEBUI_PASSWORD to', flush=True)
|
||||||
print(f' enable authentication.', flush=True)
|
print(f' enable authentication.', flush=True)
|
||||||
|
|
||||||
|
oidc_startup_warning = get_oidc_startup_warning()
|
||||||
|
if oidc_startup_warning:
|
||||||
|
print(f'[!!] WARNING: {oidc_startup_warning}', flush=True)
|
||||||
|
|
||||||
ok, missing, errors = verify_hermes_imports()
|
ok, missing, errors = verify_hermes_imports()
|
||||||
if not ok and _HERMES_FOUND:
|
if not ok and _HERMES_FOUND:
|
||||||
print(f'[!!] Warning: Hermes agent found but missing modules: {missing}', flush=True)
|
print(f'[!!] Warning: Hermes agent found but missing modules: {missing}', flush=True)
|
||||||
|
|||||||
@@ -3,6 +3,8 @@ import json
|
|||||||
from types import SimpleNamespace
|
from types import SimpleNamespace
|
||||||
from urllib.parse import parse_qs, urlparse
|
from urllib.parse import parse_qs, urlparse
|
||||||
|
|
||||||
|
import pytest
|
||||||
|
|
||||||
|
|
||||||
class FakeHeaders(dict):
|
class FakeHeaders(dict):
|
||||||
def get(self, key, default=None):
|
def get(self, key, default=None):
|
||||||
@@ -242,3 +244,85 @@ def test_oidc_enablement_requires_explicit_allowlist(monkeypatch):
|
|||||||
)
|
)
|
||||||
|
|
||||||
assert auth_oidc.is_oidc_enabled() is False
|
assert auth_oidc.is_oidc_enabled() is False
|
||||||
|
|
||||||
|
def test_oidc_startup_warning_flags_partial_config(monkeypatch):
|
||||||
|
import api.auth_oidc as auth_oidc
|
||||||
|
|
||||||
|
monkeypatch.setattr(
|
||||||
|
auth_oidc,
|
||||||
|
"get_config",
|
||||||
|
lambda: {
|
||||||
|
"webui_oidc": {
|
||||||
|
"issuer": "https://issuer.example",
|
||||||
|
"client_id": "webui-client",
|
||||||
|
}
|
||||||
|
},
|
||||||
|
)
|
||||||
|
|
||||||
|
warning = auth_oidc.get_oidc_startup_warning()
|
||||||
|
assert warning is not None
|
||||||
|
assert "allow_claim" in warning
|
||||||
|
assert "allow_values" in warning
|
||||||
|
|
||||||
|
def test_oidc_startup_warning_ignores_complete_config(monkeypatch):
|
||||||
|
import api.auth_oidc as auth_oidc
|
||||||
|
|
||||||
|
monkeypatch.setattr(
|
||||||
|
auth_oidc,
|
||||||
|
"get_config",
|
||||||
|
lambda: {
|
||||||
|
"webui_oidc": {
|
||||||
|
"issuer": "https://issuer.example",
|
||||||
|
"client_id": "webui-client",
|
||||||
|
"allow_claim": "email",
|
||||||
|
"allow_values": ["user@example.com"],
|
||||||
|
}
|
||||||
|
},
|
||||||
|
)
|
||||||
|
|
||||||
|
assert auth_oidc.get_oidc_startup_warning() is None
|
||||||
|
|
||||||
|
def test_validate_id_token_rejects_mismatched_jwk_key_family(monkeypatch):
|
||||||
|
import api.auth_oidc as auth_oidc
|
||||||
|
from api.auth_oidc import OIDCAuthError
|
||||||
|
|
||||||
|
monkeypatch.setattr(
|
||||||
|
auth_oidc,
|
||||||
|
"_parse_jwt",
|
||||||
|
lambda _token: (
|
||||||
|
{"alg": "RS256", "kid": "key-1"},
|
||||||
|
{
|
||||||
|
"iss": "https://issuer.example",
|
||||||
|
"aud": "webui-client",
|
||||||
|
"exp": 32503680000,
|
||||||
|
"nonce": "nonce-token",
|
||||||
|
"sub": "user-123",
|
||||||
|
},
|
||||||
|
b"signed",
|
||||||
|
b"signature",
|
||||||
|
),
|
||||||
|
)
|
||||||
|
monkeypatch.setattr(
|
||||||
|
auth_oidc,
|
||||||
|
"_get_jwks_document",
|
||||||
|
lambda _jwks_uri: {
|
||||||
|
"keys": [
|
||||||
|
{
|
||||||
|
"kid": "key-1",
|
||||||
|
"kty": "EC",
|
||||||
|
"crv": "P-256",
|
||||||
|
"x": "AQ",
|
||||||
|
"y": "Ag",
|
||||||
|
}
|
||||||
|
]
|
||||||
|
},
|
||||||
|
)
|
||||||
|
|
||||||
|
with pytest.raises(OIDCAuthError, match="did not contain the signing key"):
|
||||||
|
auth_oidc._validate_id_token(
|
||||||
|
"header.payload.signature",
|
||||||
|
client_id="webui-client",
|
||||||
|
issuer="https://issuer.example",
|
||||||
|
nonce="nonce-token",
|
||||||
|
jwks_uri="https://issuer.example/jwks",
|
||||||
|
)
|
||||||
Reference in new issue
Block a user