diff --git a/backend/app/api/routes/auth.py b/backend/app/api/routes/auth.py index 8f279d948..e8302cf35 100644 --- a/backend/app/api/routes/auth.py +++ b/backend/app/api/routes/auth.py @@ -123,7 +123,11 @@ def _local_login_env_bypass() -> bool: an install whose SSO provider is unreachable. Accepted truthy values: ``true``, ``1``, ``yes`` (case-insensitive). """ - return env_bool("BAMBUDDY_LOCAL_LOGIN", False) + # strict=False: this runs on the login/forgot-password request path, not at + # startup. An unrecognized value must fall back to "off" (the safe default), + # never raise -- a 500 on the recovery endpoint is the opposite of what this + # bypass is for. + return env_bool("BAMBUDDY_LOCAL_LOGIN", False, strict=False) def _get_client_ip(request: Request) -> str: diff --git a/backend/app/core/oidc_env.py b/backend/app/core/oidc_env.py index d5900a6e7..fdf234e1d 100644 --- a/backend/app/core/oidc_env.py +++ b/backend/app/core/oidc_env.py @@ -39,7 +39,15 @@ class EnvOIDCConfigError(Exception): is safe to log in full (unlike client_secret, which never reaches here).""" -def env_bool(key: str, default: bool) -> bool: +def env_bool(key: str, default: bool, *, strict: bool = True) -> bool: + """Parse a boolean env var. Absent or blank -> default (empty == unset). + + strict (the default): an unrecognized non-empty value raises + EnvOIDCConfigError, so a typo is refused loudly rather than silently read as + the wrong thing. strict=False: an unrecognized value falls back to the + default instead -- for a caller on a request path where a raise would be a + 500, not a skipped startup config (see _local_login_env_bypass). + """ value = os.environ.get(key) if value is None or value.strip() == "": return default # absent or blank == unset -> default, per the module's promise @@ -48,7 +56,9 @@ def env_bool(key: str, default: bool) -> bool: return True if norm in _FALSY: return False - raise EnvOIDCConfigError(f"{key}={value!r} is not a recognized boolean (use true/1/yes or false/0/no)") + if strict: + raise EnvOIDCConfigError(f"{key}={value!r} is not a recognized boolean (use true/1/yes or false/0/no)") + return default def read_env_oidc_config() -> dict | None: diff --git a/backend/tests/integration/test_local_login_gate.py b/backend/tests/integration/test_local_login_gate.py index 0ea3bfb79..d5d09fe2c 100644 --- a/backend/tests/integration/test_local_login_gate.py +++ b/backend/tests/integration/test_local_login_gate.py @@ -96,6 +96,26 @@ class TestLocalLoginGate: ) assert response.status_code == 200, response.text + @pytest.mark.asyncio + @pytest.mark.integration + async def test_unrecognized_env_value_does_not_500_the_login_path( + self, async_client: AsyncClient, db_session: AsyncSession, monkeypatch: pytest.MonkeyPatch + ): + """The recovery bypass reads BAMBUDDY_LOCAL_LOGIN on the request path, so + an unrecognized value (BAMBUDDY_LOCAL_LOGIN=on) must fall back to "off", + never raise -- env_bool is strict for the startup OIDC reader but lenient + here. A raise would 500 the very endpoint the bypass exists to keep open.""" + await _enable_auth(async_client, "gateonval") + await _set_setting(db_session, "local_login_enabled", "false") + monkeypatch.setenv("BAMBUDDY_LOCAL_LOGIN", "on") + + response = await async_client.post( + "/api/v1/auth/login", + json={"username": "gateonval", "password": "GatePass1!"}, + ) + # Bypass stays off (same 401 as no env var), and crucially not a 500. + assert response.status_code == 401, response.text + @pytest.mark.asyncio @pytest.mark.integration async def test_forgot_password_rejected_when_local_disabled( diff --git a/backend/tests/unit/test_oidc_env_reader.py b/backend/tests/unit/test_oidc_env_reader.py index a9342f722..2fae0df2d 100644 --- a/backend/tests/unit/test_oidc_env_reader.py +++ b/backend/tests/unit/test_oidc_env_reader.py @@ -149,6 +149,16 @@ def test_env_bool_rejects_an_unrecognized_value(monkeypatch, raw): env_bool("SOME_FLAG", True) +@pytest.mark.parametrize("raw", ["on", "enabled", "y", "nonsense"]) +@pytest.mark.parametrize("default", [True, False]) +def test_env_bool_lenient_falls_back_to_default_on_unrecognized(monkeypatch, raw, default): + """strict=False (the request-path callers like BAMBUDDY_LOCAL_LOGIN): an + unrecognized value must return the default, never raise -- a raise there + would 500 a live endpoint rather than skip a startup config.""" + monkeypatch.setenv("SOME_FLAG", raw) + assert env_bool("SOME_FLAG", default, strict=False) is default + + def test_optional_strings_override_their_defaults(monkeypatch): _set_required(monkeypatch) monkeypatch.setenv("BAMBUDDY_OIDC_SCOPES", "openid profile groups")