diff --git a/backend/app/core/oidc_env.py b/backend/app/core/oidc_env.py index a82b81dff..62269cb22 100644 --- a/backend/app/core/oidc_env.py +++ b/backend/app/core/oidc_env.py @@ -51,13 +51,13 @@ def read_env_oidc_config() -> dict | None: "issuer_url": os.environ["BAMBUDDY_OIDC_ISSUER_URL"], "client_id": os.environ["BAMBUDDY_OIDC_CLIENT_ID"], "client_secret": os.environ["BAMBUDDY_OIDC_CLIENT_SECRET"], - "scopes": os.environ.get("BAMBUDDY_OIDC_SCOPES", "openid email profile"), + "scopes": (os.environ.get("BAMBUDDY_OIDC_SCOPES") or "").strip() or "openid email profile", "is_enabled": _env_bool("BAMBUDDY_OIDC_ENABLED", True), "auto_create_users": _env_bool("BAMBUDDY_OIDC_AUTO_CREATE_USERS", False), "auto_link_existing_accounts": _env_bool("BAMBUDDY_OIDC_AUTO_LINK_EXISTING", False), - "email_claim": os.environ.get("BAMBUDDY_OIDC_EMAIL_CLAIM", "email"), + "email_claim": (os.environ.get("BAMBUDDY_OIDC_EMAIL_CLAIM") or "").strip() or "email", "require_email_verified": _env_bool("BAMBUDDY_OIDC_REQUIRE_EMAIL_VERIFIED", True), - "icon_url": os.environ.get("BAMBUDDY_OIDC_ICON_URL"), + "icon_url": (os.environ.get("BAMBUDDY_OIDC_ICON_URL") or "").strip() or None, "is_autologin": _env_bool("BAMBUDDY_OIDC_AUTOLOGIN", False), # A name, not an id: ids are assigned per install, so the same compose # file would point at a different group on every deployment. Resolved diff --git a/backend/tests/integration/test_oidc_env_apply.py b/backend/tests/integration/test_oidc_env_apply.py index 60f9b913c..76c224e85 100644 --- a/backend/tests/integration/test_oidc_env_apply.py +++ b/backend/tests/integration/test_oidc_env_apply.py @@ -547,6 +547,41 @@ async def test_an_empty_group_variable_counts_as_unset(db_session, monkeypatch): assert provider.default_group_id is None +# --- blank optional strings count as unset, not a refusal --------------------- +# `.env.example` ships `# BAMBUDDY_OIDC_ICON_URL=` commented out, so uncommenting +# it must not take the provider down -- same rule default_group already follows. + + +@pytest.mark.asyncio +async def test_a_blank_scopes_still_creates_the_provider(db_session, monkeypatch): + _configure(monkeypatch, BAMBUDDY_OIDC_SCOPES="") + await apply_env_oidc_provider(db_session) + + provider = await _env_provider(db_session) + assert provider is not None, "a blank optional var must not refuse the whole provider" + assert provider.scopes == "openid email profile" + + +@pytest.mark.asyncio +async def test_a_blank_email_claim_still_creates_the_provider(db_session, monkeypatch): + _configure(monkeypatch, BAMBUDDY_OIDC_EMAIL_CLAIM="") + await apply_env_oidc_provider(db_session) + + provider = await _env_provider(db_session) + assert provider is not None, "a blank optional var must not refuse the whole provider" + assert provider.email_claim == "email" + + +@pytest.mark.asyncio +async def test_a_blank_icon_url_still_creates_the_provider(db_session, monkeypatch): + _configure(monkeypatch, BAMBUDDY_OIDC_ICON_URL="") + await apply_env_oidc_provider(db_session) + + provider = await _env_provider(db_session) + assert provider is not None, "a blank optional var must not refuse the whole provider" + assert provider.icon_url is None + + # --- account links and collision behavior ------------------------------------ diff --git a/backend/tests/unit/test_oidc_env_reader.py b/backend/tests/unit/test_oidc_env_reader.py index 7a9c4ed96..80cc5cb17 100644 --- a/backend/tests/unit/test_oidc_env_reader.py +++ b/backend/tests/unit/test_oidc_env_reader.py @@ -119,6 +119,33 @@ def test_optional_strings_override_their_defaults(monkeypatch): assert cfg["icon_url"] == "https://sso.example.com/logo.png" +@pytest.mark.parametrize("raw", ["", " "]) +def test_a_blank_scopes_is_unset(monkeypatch, raw): + """`BAMBUDDY_OIDC_SCOPES=` in a compose file is a forgotten value, not a + request for a provider with no scopes -- same rule as default_group.""" + _set_required(monkeypatch) + monkeypatch.setenv("BAMBUDDY_OIDC_SCOPES", raw) + assert read_env_oidc_config()["scopes"] == "openid email profile" + + +@pytest.mark.parametrize("raw", ["", " "]) +def test_a_blank_email_claim_is_unset(monkeypatch, raw): + _set_required(monkeypatch) + monkeypatch.setenv("BAMBUDDY_OIDC_EMAIL_CLAIM", raw) + assert read_env_oidc_config()["email_claim"] == "email" + + +@pytest.mark.parametrize("raw", ["", " "]) +def test_a_blank_icon_url_is_unset(monkeypatch, raw): + """Uncommenting `# BAMBUDDY_OIDC_ICON_URL=` in .env.example must not take + the provider down -- the reader must still return a config, not refuse it.""" + _set_required(monkeypatch) + monkeypatch.setenv("BAMBUDDY_OIDC_ICON_URL", raw) + cfg = read_env_oidc_config() + assert cfg is not None, "a blank optional var must not refuse the whole provider" + assert cfg["icon_url"] is None + + def test_the_default_group_is_read_as_a_name(monkeypatch): """A name, not an id: group ids differ per install, so an id in a compose file would point at whatever group happened to be created third."""