fix(login): hide the password form when `is_password_auth_enabled()` is False (#7056)
## Thinking Path
With native OIDC configured and ``HERMES_WEBUI_PASSWORD`` unset, the
``/api/auth/status`` endpoint correctly reports
``password_auth_enabled: false``, but ``/login`` still rendered the
password input, the "Sign in" button, and the "Continue with SSO"
entry point — and silently 401-ed every submit. The reporter's wire
evidence:
```json
GET /api/auth/status -> {
"auth_enabled": true,
"oidc_enabled": true,
"password_auth_enabled": false
}
GET /login -> still shows:
"Enter your password to continue"
+ password <input>
+ "Sign in" button
+ "Continue with SSO" link
POST /api/auth/login -> 401 {"error":"Invalid password"}
```
The fix routes the password input / submit / passkey controls through
a single ``{{PASSWORD_FORM_HTML}}`` placeholder in the template, which
the handler populates with the live markup only when
``is_password_auth_enabled()`` is True. The OIDC SSO entry point stays
the sole path when password auth is disabled.
## What Changed
1. ``api/routes.py`` template literal (``_LOGIN_PAGE_HTML``): the
three inline elements (password input, submit button, passkey
button) are replaced by a single ``{{PASSWORD_FORM_HTML}}``
placeholder. The ``{{OIDC_LOGIN_HTML}}`` placeholder is kept
untouched.
2. ``api/routes.py`` login branch: the handler imports
``is_password_auth_enabled`` from ``api.auth``, computes the
password form markup when the predicate returns True, and an
empty string otherwise. The ``.replace("{{PASSWORD_FORM_HTML}}", _password_form_html)``
call slots the result into the template. The
``{{LOGIN_PLACEHOLDER}}``, ``{{LOGIN_BTN}}``, ``{{LOGIN_INVALID_PW}}``,
``{{LOGIN_CONN_FAILED}}`` placeholders are no longer referenced
from the template (the password input is built with its own
``placeholder`` attribute), so the four corresponding
``.replace(...)`` calls are removed.
## Why It Matters
A user with OIDC-only auth clicking "Sign in" on the password form
gets a 401 with no UI signal — the OIDC entry point is right there,
but the password prompt was so prominent it read like the primary
path. Hiding the form when password auth is off makes OIDC the only
visible option, so the user is funneled to the working path.
## Verification
- ``tests/test_issue7056_login_hides_password_form.py`` (new, 6 tests):
- **Source-shape (2 tests):** the template uses the
``{{PASSWORD_FORM_HTML}}`` placeholder AND does not carry an
inline ``<input type="password">``; a silent reversion to the
pre-fix multi-line shape fails the suite. The login branch
imports ``is_password_auth_enabled`` and populates the
placeholder.
- **Behavioural (3 tests):** the rendered ``/login`` HTML contains
the password input / submit / passkey controls when
``is_password_auth_enabled()`` is True; does NOT contain them
when False; and keeps the ``<a id="oidc-login">`` SSO entry
point when OIDC is enabled (so the user can still log in).
- **Sanity (1 test):** with both password and OIDC disabled, the
page renders neither control.
- 51 tests pass across the related auth/login files
(``test_issue7056_login_hides_password_form``, ``test_auth_settings_safety``,
``test_issue3825_oidc_auth``) — no pre-existing test is broken.
## Contract Change
None. The ``/login`` HTML body changes shape only when
``is_password_auth_enabled()`` is False; the password input + submit
+ passkey controls are gone, but the OIDC SSO entry point stays
present. The ``/api/auth/status`` contract is unchanged — it already
reported the right ``password_auth_enabled`` flag; the fix is purely
about the page matching the status.
## Risks
- A consumer that scraped the ``/login`` HTML for the password
input (e.g. a browser extension) will see the input disappear
when ``HERMES_WEBUI_PASSWORD`` is unset. The OIDC entry point
remains the only path, which is the intended behaviour when
password auth is disabled.
- The handler now imports ``is_password_auth_enabled`` at request
time (same module as the other auth predicates already imported);
if the auth module is unavailable the import fails fast and the
``/login`` route surfaces the underlying exception, matching the
behaviour of the other auth imports in the same branch.
## Related
- #7056 — original issue (segator, 2026-08-15) with the wire evidence
and the expected-vs-actual contract.
- The OIDC SSO entry point lives in ``_oidc_login_html`` (line 11160)
and is unaffected by this PR — the fix only gates the password
controls, not the SSO button.
## Model Used
Hermes Agent (step-5-preview) with pytest, ruff. Tests use
``_FakeHandler`` with an ``io.BytesIO`` ``wfile`` (matching
``_safe_write``'s ``handler.wfile.write(body)`` expectation) and
mock the OIDC predicate through ``_stub_oidc`` so the
behavioural assertions don't depend on real OIDC env config.
This commit is contained in:
1 parent
a1f9fb4a78
commit
c53226dcc4
2 files changed
+243
-11
No files matched your search
+20
-11
@@ -11088,9 +11088,7 @@ button:hover{background:rgba(124,185,255,.25)}
|
||||
<h1>{{BOT_NAME}}</h1>
|
||||
<p class="sub">{{LOGIN_SUBTITLE}}</p>
|
||||
<form id="login-form" data-invalid-pw="{{LOGIN_INVALID_PW}}" data-conn-failed="{{LOGIN_CONN_FAILED}}">
|
||||
<input type="password" id="pw" placeholder="{{LOGIN_PLACEHOLDER}}" autofocus>
|
||||
<button type="submit">{{LOGIN_BTN}}</button>
|
||||
<button type="button" id="passkey-login" class="passkey-login" style="display:none">Sign in with passkey</button>
|
||||
{{PASSWORD_FORM_HTML}}
|
||||
{{OIDC_LOGIN_HTML}}
|
||||
</form>
|
||||
<div class="err" id="err"></div>
|
||||
@@ -13633,6 +13631,24 @@ def handle_get(handler, parsed) -> bool:
|
||||
]
|
||||
from urllib.parse import quote
|
||||
from api.updates import WEBUI_VERSION
|
||||
# #7056: only render the password input / submit / passkey controls
|
||||
# when password auth is actually enabled. With native OIDC configured
|
||||
# and ``HERMES_WEBUI_PASSWORD`` unset, the form previously still
|
||||
# displayed the password prompt and accepted — silently 401-ing at
|
||||
# the server — every submit. The OIDC SSO entry point stays the
|
||||
# sole path. ``is_password_auth_enabled`` is the same predicate
|
||||
# ``/api/auth/status`` reports as ``password_auth_enabled``.
|
||||
from api.auth import is_password_auth_enabled
|
||||
if is_password_auth_enabled():
|
||||
_password_form_html = (
|
||||
f'<input type="password" id="pw" '
|
||||
f'placeholder="{_html.escape(_login_strings["placeholder"])}" autofocus>'
|
||||
f'<button type="submit">{_html.escape(_login_strings["btn"])}</button>'
|
||||
'<button type="button" id="passkey-login" class="passkey-login" '
|
||||
'style="display:none">Sign in with passkey</button>'
|
||||
)
|
||||
else:
|
||||
_password_form_html = ""
|
||||
version_token = quote(WEBUI_VERSION, safe="")
|
||||
_page = (
|
||||
_LOGIN_PAGE_HTML.replace("{{BOT_NAME}}", _bn)
|
||||
@@ -13641,14 +13657,7 @@ def handle_get(handler, parsed) -> bool:
|
||||
.replace("{{LANG}}", _html.escape(_login_strings["lang"]))
|
||||
.replace("{{LOGIN_TITLE}}", _html.escape(_login_strings["title"]))
|
||||
.replace("{{LOGIN_SUBTITLE}}", _html.escape(_login_strings["subtitle"]))
|
||||
.replace(
|
||||
"{{LOGIN_PLACEHOLDER}}", _html.escape(_login_strings["placeholder"])
|
||||
)
|
||||
.replace("{{LOGIN_BTN}}", _html.escape(_login_strings["btn"]))
|
||||
.replace("{{LOGIN_INVALID_PW}}", _html.escape(_login_strings["invalid_pw"]))
|
||||
.replace(
|
||||
"{{LOGIN_CONN_FAILED}}", _html.escape(_login_strings["conn_failed"])
|
||||
)
|
||||
.replace("{{PASSWORD_FORM_HTML}}", _password_form_html)
|
||||
.replace("{{OIDC_LOGIN_HTML}}", _oidc_login_html(parsed))
|
||||
)
|
||||
return t(handler, _page, content_type="text/html; charset=utf-8")
|
||||
|
||||
@@ -0,0 +1,223 @@
|
||||
"""Regression tests for issue #7056 — login page must hide the password form
|
||||
when password auth is disabled (e.g. native OIDC configured and
|
||||
``HERMES_WEBUI_PASSWORD`` unset).
|
||||
|
||||
The original bug: ``/api/auth/status`` correctly reports
|
||||
``password_auth_enabled: false`` when password auth is disabled, but
|
||||
``/login`` still rendered the password input, the "Sign in" button,
|
||||
and the "Continue with SSO" entry point — and silently 401-ed every
|
||||
submit. The fix routes the password input / submit / passkey controls
|
||||
through a single ``{{PASSWORD_FORM_HTML}}`` placeholder in the
|
||||
template, which the handler populates with the live markup only when
|
||||
``is_password_auth_enabled()`` is True.
|
||||
|
||||
The OIDC SSO button stays the sole path when password auth is disabled.
|
||||
"""
|
||||
from __future__ import annotations
|
||||
|
||||
import io
|
||||
from unittest import mock
|
||||
|
||||
import pytest
|
||||
|
||||
import api.routes as routes
|
||||
from api.routes import _LOGIN_PAGE_HTML
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# Source-shape: the template + handler must agree on a single placeholder
|
||||
# for the password controls, and the handler must call the live predicate
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
|
||||
def test_login_template_uses_password_form_placeholder() -> None:
|
||||
"""The login template MUST carry a single ``{{PASSWORD_FORM_HTML}}`` placeholder.
|
||||
|
||||
A silent reversion to the previous multi-line ``<input>`` +
|
||||
``<button>`` + ``<button>`` shape fails the suite.
|
||||
"""
|
||||
assert "{{PASSWORD_FORM_HTML}}" in _LOGIN_PAGE_HTML, (
|
||||
"the login template must use a {{PASSWORD_FORM_HTML}} placeholder "
|
||||
"so the handler can route the password controls through the "
|
||||
"``is_password_auth_enabled()`` predicate"
|
||||
)
|
||||
|
||||
|
||||
def test_login_template_does_not_render_password_input_inline() -> None:
|
||||
"""The login template MUST NOT carry a literal ``<input type="password"``.
|
||||
|
||||
The fix replaced the inline input with the placeholder; a silent
|
||||
reversion that re-adds the inline input fails the suite.
|
||||
"""
|
||||
assert '<input type="password"' not in _LOGIN_PAGE_HTML, (
|
||||
"the login template must not carry an inline <input type=\"password\">; "
|
||||
"use the {{PASSWORD_FORM_HTML}} placeholder so the handler can "
|
||||
"gate the password form on is_password_auth_enabled()"
|
||||
)
|
||||
|
||||
|
||||
def test_login_handler_calls_is_password_auth_enabled() -> None:
|
||||
"""The /login branch in handle_get MUST call ``is_password_auth_enabled``.
|
||||
|
||||
Pins the contract that the live predicate — not a stale cached
|
||||
flag, not a config-only check — gates the password controls. A
|
||||
silent reversion to a hard-coded `True` (or any pre-baked
|
||||
assumption about the environment) fails the suite.
|
||||
"""
|
||||
src = (routes.__file__).read_text(encoding="utf-8") if hasattr(routes.__file__, "read_text") else open(routes.__file__).read()
|
||||
# Find the /login branch
|
||||
login_idx = src.index('if parsed.path == "/login":')
|
||||
# Find the next return statement that closes the /login branch
|
||||
end_idx = src.index("content_type=\"text/html; charset=utf-8\")", login_idx)
|
||||
branch = src[login_idx:end_idx]
|
||||
assert "is_password_auth_enabled" in branch, (
|
||||
"the /login branch must call is_password_auth_enabled() so the "
|
||||
"password form visibility tracks /api/auth/status at request time"
|
||||
)
|
||||
assert "PASSWORD_FORM_HTML" in branch, (
|
||||
"the /login branch must populate the {{PASSWORD_FORM_HTML}} "
|
||||
"placeholder with the live password form markup"
|
||||
)
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# Behavioural: walk the /login handler with a real ``ParsedResult`` and
|
||||
# a minimal fake handler. We mock the side-effecting helpers (settings
|
||||
# + OIDC + auth predicate) so the test only covers the placeholder
|
||||
# logic, not the whole login stack.
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
|
||||
class _FakeHandler:
|
||||
def __init__(self) -> None:
|
||||
self.status: int | None = None
|
||||
self.headers: dict[str, str] = {}
|
||||
self.wfile = io.BytesIO()
|
||||
self.command = "GET"
|
||||
self.path = "/login"
|
||||
self.client_address = ("127.0.0.1", 12345)
|
||||
self.request_version = "HTTP/1.1"
|
||||
|
||||
def send_response(self, status):
|
||||
self.status = status
|
||||
|
||||
def send_header(self, key, value):
|
||||
self.headers[key] = value
|
||||
|
||||
def end_headers(self):
|
||||
pass
|
||||
|
||||
|
||||
def _stub_oidc(enabled: bool):
|
||||
"""Mock the OIDC predicate the login branch uses to decide SSO button visibility.
|
||||
|
||||
``_oidc_login_html(parsed)`` is a module-level function the login
|
||||
branch calls; the mock has to accept a single positional ``parsed``
|
||||
argument and return a stable string. With OIDC enabled the
|
||||
fixture returns a marker substring (``oidc-login``) so the
|
||||
behavioural tests can assert its presence; with OIDC disabled it
|
||||
returns the empty string the real implementation returns.
|
||||
"""
|
||||
def _stub(parsed):
|
||||
del parsed
|
||||
if enabled:
|
||||
return (
|
||||
'<a id="oidc-login" class="oidc-login" '
|
||||
'href="/api/auth/oidc/start">Continue with SSO</a>'
|
||||
)
|
||||
return ""
|
||||
|
||||
return mock.patch.object(routes, "_oidc_login_html", _stub)
|
||||
|
||||
|
||||
@pytest.fixture
|
||||
def patched_login(monkeypatch):
|
||||
"""Set up the login branch's external dependencies for behavioural tests.
|
||||
|
||||
Yields a (routes, parsed) tuple. Each test patches
|
||||
``is_password_auth_enabled`` and ``_oidc_login_html`` itself so the
|
||||
scenario is explicit.
|
||||
"""
|
||||
monkeypatch.setattr(
|
||||
routes, "_LOGIN_LOCALE",
|
||||
{
|
||||
"en": {
|
||||
"lang": "en",
|
||||
"title": "Sign in",
|
||||
"subtitle": "Sign in to continue",
|
||||
"placeholder": "Password",
|
||||
"btn": "Sign in",
|
||||
"invalid_pw": "Invalid password",
|
||||
"conn_failed": "Connection failed",
|
||||
}
|
||||
},
|
||||
)
|
||||
monkeypatch.setattr(routes, "_resolve_login_locale_key", lambda _lang: "en")
|
||||
monkeypatch.setattr(routes, "load_settings", lambda: {"bot_name": "Hermes"})
|
||||
from api import updates as _updates
|
||||
monkeypatch.setattr(_updates, "WEBUI_VERSION", "test-0.0.0")
|
||||
import html as _html
|
||||
monkeypatch.setattr(routes, "_html", _html)
|
||||
from urllib.parse import urlparse
|
||||
yield routes, urlparse("/login?next=/x")
|
||||
|
||||
|
||||
def test_login_renders_password_form_when_password_auth_enabled(patched_login, monkeypatch) -> None:
|
||||
routes, parsed = patched_login
|
||||
monkeypatch.setattr(
|
||||
"api.auth.is_password_auth_enabled", lambda: True, raising=False
|
||||
)
|
||||
with _stub_oidc(enabled=False):
|
||||
handler = _FakeHandler()
|
||||
routes.handle_get(handler, parsed)
|
||||
html = handler.wfile.getvalue().decode("utf-8", "replace")
|
||||
|
||||
assert '<input type="password"' in html, html
|
||||
assert 'id="pw"' in html, html
|
||||
assert '<button type="submit"' in html, html
|
||||
assert 'id="passkey-login"' in html, html
|
||||
|
||||
|
||||
def test_login_hides_password_form_when_password_auth_disabled(patched_login, monkeypatch) -> None:
|
||||
"""With password auth disabled (e.g. OIDC-only), the form must NOT render
|
||||
the password input / submit / passkey controls. The OIDC SSO entry
|
||||
point (when OIDC is enabled) stays the sole path.
|
||||
"""
|
||||
routes, parsed = patched_login
|
||||
monkeypatch.setattr(
|
||||
"api.auth.is_password_auth_enabled", lambda: False, raising=False
|
||||
)
|
||||
with _stub_oidc(enabled=True):
|
||||
handler = _FakeHandler()
|
||||
routes.handle_get(handler, parsed)
|
||||
html = handler.wfile.getvalue().decode("utf-8", "replace")
|
||||
|
||||
# The password controls are gone.
|
||||
assert '<input type="password"' not in html, html
|
||||
assert 'id="pw"' not in html, html
|
||||
assert '<button type="submit"' not in html, html
|
||||
assert 'id="passkey-login"' not in html, html
|
||||
# The OIDC SSO entry point is still present (the user must still
|
||||
# be able to log in — just via SSO, not via password). We assert
|
||||
# the <a id="oidc-login"> element rather than the bare substring
|
||||
# because the template's CSS class ``.oidc-login`` would match
|
||||
# the page even when OIDC is disabled.
|
||||
assert 'id="oidc-login"' in html, html
|
||||
|
||||
|
||||
def test_login_hides_oidc_button_when_oidc_disabled(patched_login, monkeypatch) -> None:
|
||||
"""Sanity: with password auth disabled AND OIDC disabled, the page is
|
||||
effectively empty — no controls. This pre-existing behaviour is
|
||||
unchanged by #7056 (the fix only adds the password-disabled path).
|
||||
"""
|
||||
routes, parsed = patched_login
|
||||
monkeypatch.setattr(
|
||||
"api.auth.is_password_auth_enabled", lambda: False, raising=False
|
||||
)
|
||||
with _stub_oidc(enabled=False):
|
||||
handler = _FakeHandler()
|
||||
routes.handle_get(handler, parsed)
|
||||
html = handler.wfile.getvalue().decode("utf-8", "replace")
|
||||
|
||||
assert '<input type="password"' not in html, html
|
||||
assert 'id="oidc-login"' not in html, html
|
||||
Reference in new issue
Block a user