Просмотр исходного кода

fix(oidc): keep the local-login bypass lenient under strict env_bool

Promoting env_bool to strict rejection made BAMBUDDY_LOCAL_LOGIN=on raise
EnvOIDCConfigError uncaught on the login/forgot-password path -- a 500 on
the exact recovery endpoint the bypass exists to keep open. env_bool gains
a strict flag (default True for the startup OIDC reader); the local-login
caller opts out so an unrecognized value falls back to "off" instead.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_016q8EAf9Rj7ZHL92sPnXYxy
Marian 1 месяц назад
Родитель
Сommit
9e783fbad6

+ 5 - 1
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:

+ 12 - 2
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:

+ 20 - 0
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(

+ 10 - 0
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")