From c1df31fda53a5aa2e57108edbdb67e18e529bc2e Mon Sep 17 00:00:00 2001 From: tanmayraut45 Date: Tue, 2 Jun 2026 07:53:47 +0530 Subject: [PATCH] Honor AUTH_ENABLED=false in route-level auth gate (#785) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit #622 reported "I cant even paste that hash pw and granted So auth_en =false & localbypass= true But then the host still is showing login page?" — the operator turned auth off in .env and still gets bounced to /login on every page load. The flow: The auth middleware in app.py is correctly gated on AUTH_ENABLED, so the middleware itself does not install when AUTH_ENABLED=false. The SPA front-end at static/app.js wraps window.fetch and redirects to /login on ANY 401 response from any API call. So all it takes for the operator to see a login page is one route-level 401. src/auth_helpers.require_user — the shared FastAPI dependency mounted on ~50 routes (email, contacts, personal, …) — was the source. It is documented as defense-in-depth in case the middleware was bypassed unexpectedly (SSRF from a sibling service), but the implementation treated AUTH_ENABLED=false as one of those unexpected bypasses and 401'd anyway. The loopback fall-through that would have admitted the operator does not fire under docker compose / a reverse proxy because the container sees the request arriving from the bridge gateway (172.x.x.x), not 127.0.0.1. require_user now short-circuits to "" when AUTH_ENABLED=false so the explicit operator opt-out reaches the route layer too. While in the file, also mirror LOCALHOST_BYPASS=true the same way for loopback callers — the middleware already lets them through, and routes 401'ing the same caller would produce the same /login bounce. Non-loopback callers under LOCALHOST_BYPASS are still rejected, matching the middleware's _is_trusted_loopback check. Add three focused regression tests in tests/test_security_regressions.py: docker-bridge caller is admitted under AUTH_ENABLED=false, loopback caller is admitted under LOCALHOST_BYPASS=true, LAN caller under LOCALHOST_BYPASS=true is still rejected. The existing test_require_user_rejects_unauthenticated and test_require_user_accepts_loopback_when_unconfigured tests continue to pass because neither sets AUTH_ENABLED, so the AUTH_ENABLED=true default path is unchanged. Closes #622. --- src/auth_helpers.py | 46 +++++++++++--- tests/test_security_regressions.py | 98 ++++++++++++++++++++++++++++++ 2 files changed, 137 insertions(+), 7 deletions(-) diff --git a/src/auth_helpers.py b/src/auth_helpers.py index 56de954..0e9ca23 100644 --- a/src/auth_helpers.py +++ b/src/auth_helpers.py @@ -1,5 +1,6 @@ """Shared auth helpers used by all route files.""" +import os from typing import Optional from fastapi import Request, HTTPException @@ -9,11 +10,28 @@ def get_current_user(request: Request) -> Optional[str]: return getattr(request.state, 'current_user', None) +def _auth_disabled() -> bool: + """True when the operator has explicitly turned off auth via .env. + Mirrors the AUTH_ENABLED parse in app.py / core/middleware.py so the + three call sites agree on what "off" means.""" + return os.getenv("AUTH_ENABLED", "true").lower() == "false" + + def require_user(request: Request) -> str: - """FastAPI dependency: reject unauthenticated callers, even if upstream - middleware was bypassed (LOCALHOST_BYPASS, AUTH_ENABLED=false, SSRF from - a sibling service). Returns the resolved username, or "" in unconfigured - first-run mode when the caller is on loopback. + """FastAPI dependency: reject unauthenticated callers when the upstream + auth middleware was bypassed unexpectedly (e.g. SSRF from a sibling + service). Returns the resolved username, or "" in single-user / anonymous + modes where no username is available. + + The three "" cases are: + 1. AUTH_ENABLED=false — the operator explicitly turned auth off. + The full /login flow is skipped (issue #622), so route-level + require_user must let the request through too instead of 401-ing + and forcing the browser to /login. + 2. Unconfigured first-run + loopback caller — pre-setup access from + localhost so the operator can hit the SPA before creating the + first admin. + 3. LOCALHOST_BYPASS=true + loopback caller — documented dev bypass. Use this on routes that touch user data so middleware misconfig can't open them up. @@ -21,13 +39,27 @@ def require_user(request: Request) -> str: u = get_current_user(request) if u: return u + # Operator-disabled auth: honor it at the route layer too. Without this, + # routes that depend on require_user 401, the front-end fetch wrapper + # redirects to /login, and the user sees a login page despite + # AUTH_ENABLED=false (issue #622). Docker / reverse-proxy deployments + # hit this because requests arrive from a non-loopback client.host, so + # the loopback fall-through below never fires. + if _auth_disabled(): + return "" auth_mgr = getattr(request.app.state, "auth_manager", None) + client = getattr(request, "client", None) + host = (client.host if client else "") or "" + is_loopback = host in ("127.0.0.1", "::1", "localhost") + # LOCALHOST_BYPASS=true is the dev-only "I'm on loopback, skip auth" + # switch. Mirror the middleware so routes don't 401 the same caller + # the middleware just let through. + if is_loopback and os.getenv("LOCALHOST_BYPASS", "false").lower() == "true": + return "" if auth_mgr is not None and getattr(auth_mgr, "is_configured", False): raise HTTPException(401, "Not authenticated") # Unconfigured / first-run mode: only allow loopback callers. - client = getattr(request, "client", None) - host = (client.host if client else "") or "" - if host in ("127.0.0.1", "::1", "localhost"): + if is_loopback: return "" raise HTTPException(401, "Not authenticated") diff --git a/tests/test_security_regressions.py b/tests/test_security_regressions.py index 1e5c77c..4aa1443 100644 --- a/tests/test_security_regressions.py +++ b/tests/test_security_regressions.py @@ -537,6 +537,104 @@ def test_require_user_accepts_loopback_when_unconfigured(monkeypatch): assert auth_helpers.require_user(_LoopReq()) == "" +def test_require_user_accepts_anyone_when_auth_disabled(monkeypatch): + """AUTH_ENABLED=false must let unauthenticated callers through from + any host — including the docker bridge / reverse proxy / LAN — so + the frontend's global 401 redirect doesn't bounce the user to /login + despite the operator turning auth off (issue #622).""" + monkeypatch.setenv("AUTH_ENABLED", "false") + sys.modules.pop("src.auth_helpers", None) + from src import auth_helpers # noqa: WPS433 + + class _State: + current_user = None + + class _AppState: + class _Mgr: + # Even with a prior admin account on disk, AUTH_ENABLED=false + # must take precedence over is_configured=True. + is_configured = True + auth_manager = _Mgr() + + class _App: + state = _AppState() + + class _DockerClient: + host = "172.18.0.1" # docker bridge gateway, not loopback + + class _Req: + state = _State() + app = _App() + client = _DockerClient() + + assert auth_helpers.require_user(_Req()) == "" + + +def test_require_user_localhost_bypass_admits_loopback(monkeypatch): + """LOCALHOST_BYPASS=true is the dev-only switch that admits loopback + callers without an auth cookie. require_user must mirror the auth + middleware so routes don't 401 a caller the middleware already let + through.""" + monkeypatch.setenv("AUTH_ENABLED", "true") + monkeypatch.setenv("LOCALHOST_BYPASS", "true") + sys.modules.pop("src.auth_helpers", None) + from src import auth_helpers # noqa: WPS433 + + class _State: + current_user = None + + class _AppState: + class _Mgr: + is_configured = True + auth_manager = _Mgr() + + class _App: + state = _AppState() + + class _LoopClient: + host = "127.0.0.1" + + class _LoopReq: + state = _State() + app = _App() + client = _LoopClient() + + assert auth_helpers.require_user(_LoopReq()) == "" + + +def test_require_user_localhost_bypass_still_rejects_lan(monkeypatch): + """LOCALHOST_BYPASS=true must not extend to non-loopback callers — + a LAN visitor still needs to authenticate.""" + from fastapi import HTTPException + monkeypatch.setenv("AUTH_ENABLED", "true") + monkeypatch.setenv("LOCALHOST_BYPASS", "true") + sys.modules.pop("src.auth_helpers", None) + from src import auth_helpers # noqa: WPS433 + + class _State: + current_user = None + + class _AppState: + class _Mgr: + is_configured = True + auth_manager = _Mgr() + + class _App: + state = _AppState() + + class _LanClient: + host = "192.168.1.42" + + class _LanReq: + state = _State() + app = _App() + client = _LanClient() + + with pytest.raises(HTTPException) as exc: + auth_helpers.require_user(_LanReq()) + assert exc.value.status_code == 401 + + def test_require_admin_rejects_unconfigured_public_api(monkeypatch): """First-run API mode must not treat "no users yet" as admin access.""" from fastapi import HTTPException