diff --git a/backend/app/core/oidc_env.py b/backend/app/core/oidc_env.py index 60f557a98..5d02bdd7b 100644 --- a/backend/app/core/oidc_env.py +++ b/backend/app/core/oidc_env.py @@ -9,8 +9,14 @@ check the UI enforces. from __future__ import annotations +import logging import os +from sqlalchemy import select, update +from sqlalchemy.ext.asyncio import AsyncSession + +logger = logging.getLogger(__name__) + # All four or nothing: a provider missing its secret would be written to the # database and then fail at authorize time, long after the operator could # connect the failure to a typo in their compose file. @@ -52,3 +58,73 @@ def read_env_oidc_config() -> dict | None: "icon_url": os.environ.get("BAMBUDDY_OIDC_ICON_URL"), "is_autologin": _env_bool("BAMBUDDY_OIDC_AUTOLOGIN", False), } + + +# Everything the schema validates and the model stores, except client_secret -- +# that one goes through the property so it is encrypted at rest. +_APPLIED_FIELDS = ( + "name", + "issuer_url", + "client_id", + "scopes", + "is_enabled", + "auto_create_users", + "auto_link_existing_accounts", + "email_claim", + "require_email_verified", + "icon_url", + "is_autologin", +) + + +async def apply_env_oidc_provider(db: AsyncSession) -> None: + """Upsert the env-managed provider, or disable it when the config is gone. + + Never raises: this runs during startup, and a typo in one variable must not + stop the app from booting. A rejected config is logged and skipped. + """ + # Imported here rather than at module scope: app.core is imported by the + # models themselves, so a top-level import would be a cycle. + from backend.app.models.oidc_provider import OIDCProvider + from backend.app.schemas.auth import OIDCProviderCreate + + config = read_env_oidc_config() + existing = ( + await db.execute(select(OIDCProvider).where(OIDCProvider.is_env_managed.is_(True))) + ).scalar_one_or_none() + + if config is None: + # Disabled, never deleted: user_oidc_links.provider_id is FK ON DELETE + # CASCADE, so removing the row would unlink every bound account and the + # links would not come back when the variables do. + if existing is not None and existing.is_enabled: + existing.is_enabled = False + await db.commit() + logger.info("BAMBUDDY_OIDC_* is unset -- env-managed provider disabled.") + return + + try: + # The same schema the API uses, so env config cannot reach a state the + # UI would have refused (notably the SEC-1 auto-link check). + validated = OIDCProviderCreate(**config) + except Exception as exc: # noqa: BLE001 -- any rejection must be survivable + logger.error("BAMBUDDY_OIDC_* config rejected, provider not applied: %s", exc) + return + + if existing is None: + existing = OIDCProvider(is_env_managed=True) + db.add(existing) + for field in _APPLIED_FIELDS: + setattr(existing, field, getattr(validated, field)) + existing.client_secret = validated.client_secret + existing.is_env_managed = True + await db.flush() # the id is needed by the autologin sweep below + + if existing.is_autologin: + await db.execute( + update(OIDCProvider) + .where(OIDCProvider.id != existing.id, OIDCProvider.is_autologin.is_(True)) + .values(is_autologin=False) + ) + await db.commit() + logger.info("Env-managed OIDC provider %r applied.", existing.name) diff --git a/backend/tests/integration/test_oidc_env_apply.py b/backend/tests/integration/test_oidc_env_apply.py new file mode 100644 index 000000000..e83aeeba9 --- /dev/null +++ b/backend/tests/integration/test_oidc_env_apply.py @@ -0,0 +1,160 @@ +"""Upserting the env-managed OIDC provider (#2593). + +Startup applies BAMBUDDY_OIDC_* to the database. The row is updated in place, +never delete-recreated: user_oidc_links.provider_id is FK ON DELETE CASCADE, so +recreating the provider would silently unlink every account bound to it. +""" + +from __future__ import annotations + +import pytest +from sqlalchemy import select + +from backend.app.core.oidc_env import apply_env_oidc_provider +from backend.app.models.oidc_provider import OIDCProvider + +REQUIRED = { + "BAMBUDDY_OIDC_NAME": "Keycloak", + "BAMBUDDY_OIDC_ISSUER_URL": "https://sso.example.com/realms/main", + "BAMBUDDY_OIDC_CLIENT_ID": "bambuddy", + "BAMBUDDY_OIDC_CLIENT_SECRET": "s3cr3t", +} + +ALL_VARS = ( + *REQUIRED, + "BAMBUDDY_OIDC_SCOPES", + "BAMBUDDY_OIDC_ENABLED", + "BAMBUDDY_OIDC_AUTO_CREATE_USERS", + "BAMBUDDY_OIDC_AUTO_LINK_EXISTING", + "BAMBUDDY_OIDC_EMAIL_CLAIM", + "BAMBUDDY_OIDC_REQUIRE_EMAIL_VERIFIED", + "BAMBUDDY_OIDC_ICON_URL", + "BAMBUDDY_OIDC_AUTOLOGIN", +) + + +@pytest.fixture(autouse=True) +def clean_env(monkeypatch): + for key in ALL_VARS: + monkeypatch.delenv(key, raising=False) + + +def _configure(monkeypatch, **overrides): + for key, value in REQUIRED.items(): + monkeypatch.setenv(key, value) + for key, value in overrides.items(): + monkeypatch.setenv(key, value) + + +async def _env_provider(db_session) -> OIDCProvider | None: + result = await db_session.execute(select(OIDCProvider).where(OIDCProvider.is_env_managed.is_(True))) + return result.scalar_one_or_none() + + +@pytest.mark.asyncio +async def test_creates_the_provider_from_env(db_session, monkeypatch): + _configure(monkeypatch) + await apply_env_oidc_provider(db_session) + + provider = await _env_provider(db_session) + assert provider is not None + assert provider.name == "Keycloak" + assert provider.client_id == "bambuddy" + assert provider.is_env_managed is True + assert provider.client_secret == "s3cr3t" # property decrypts + + +@pytest.mark.asyncio +async def test_a_changed_var_updates_the_same_row(db_session, monkeypatch): + """The id must survive: user_oidc_links references it with ON DELETE + CASCADE, so a delete-recreate would unlink every bound account.""" + _configure(monkeypatch) + await apply_env_oidc_provider(db_session) + original_id = (await _env_provider(db_session)).id + + monkeypatch.setenv("BAMBUDDY_OIDC_CLIENT_ID", "rotated") + await apply_env_oidc_provider(db_session) + + provider = await _env_provider(db_session) + assert provider.id == original_id + assert provider.client_id == "rotated" + + +@pytest.mark.asyncio +async def test_removing_the_env_config_disables_but_keeps_the_row(db_session, monkeypatch): + _configure(monkeypatch) + await apply_env_oidc_provider(db_session) + original_id = (await _env_provider(db_session)).id + + for key in ALL_VARS: + monkeypatch.delenv(key, raising=False) + await apply_env_oidc_provider(db_session) + + provider = await _env_provider(db_session) + assert provider is not None, "deleting would cascade away every account link" + assert provider.id == original_id + assert provider.is_enabled is False + + +@pytest.mark.asyncio +async def test_env_autologin_clears_it_on_other_providers(db_session, monkeypatch): + """Only one provider may be the autologin target; the env one wins.""" + ui_provider = OIDCProvider( + name="UI provider", + issuer_url="https://other.example.com", + client_id="ui", + is_autologin=True, + ) + ui_provider.client_secret = "ui-secret" + db_session.add(ui_provider) + await db_session.commit() + + _configure(monkeypatch, BAMBUDDY_OIDC_AUTOLOGIN="true") + await apply_env_oidc_provider(db_session) + + await db_session.refresh(ui_provider) + assert (await _env_provider(db_session)).is_autologin is True + assert ui_provider.is_autologin is False + + +@pytest.mark.asyncio +async def test_a_ui_provider_is_otherwise_left_alone(db_session, monkeypatch): + ui_provider = OIDCProvider(name="UI provider", issuer_url="https://other.example.com", client_id="ui") + ui_provider.client_secret = "ui-secret" + db_session.add(ui_provider) + await db_session.commit() + + _configure(monkeypatch) + await apply_env_oidc_provider(db_session) + + await db_session.refresh(ui_provider) + assert ui_provider.is_env_managed is False + assert ui_provider.is_enabled is True + assert ui_provider.client_id == "ui" + + +@pytest.mark.asyncio +async def test_an_unsafe_auto_link_config_is_skipped_not_raised(db_session, monkeypatch): + """auto-link + unverified email is the SEC-1 account-takeover shape. The + schema rejects it for the UI, and env config must not be a way around that + -- but a bad variable must not stop the app from booting either.""" + _configure( + monkeypatch, + BAMBUDDY_OIDC_AUTO_LINK_EXISTING="true", + BAMBUDDY_OIDC_REQUIRE_EMAIL_VERIFIED="false", + ) + + await apply_env_oidc_provider(db_session) + + assert await _env_provider(db_session) is None + + +@pytest.mark.asyncio +async def test_applying_twice_without_changes_is_a_no_op(db_session, monkeypatch): + """Every boot re-applies; the second run must not create a second row.""" + _configure(monkeypatch) + await apply_env_oidc_provider(db_session) + await apply_env_oidc_provider(db_session) + + result = await db_session.execute(select(OIDCProvider).where(OIDCProvider.is_env_managed.is_(True))) + assert len(result.scalars().all()) == 1