fix(oidc): identify the env provider by name, and release it when unconfigured

Two problems, both from using is_env_managed as the provider's identity.

An operator who names the env provider after one that already exists hit the
unique constraint on `name` during the insert. That happens inside the
lifespan, so the app did not boot -- from a function whose docstring promises
it never raises. The lookup now matches on the name, which is unique, so an
existing provider is adopted and updated instead of duplicated.

And removing the config left the row disabled but still flagged, so the API
went on refusing every edit and delete while nothing managed it any more: a
dead end reachable only through the database. The flag is now cleared as well,
handing the provider back to the UI. Re-adding the config finds the same row
by name, so the account links it carries survive the round trip.

Falls out of the same change: the issuer URL and client id can be rotated
under an unchanged name without orphaning those links.

Found by Marian asking what happens when you want to change the provider --
the answer was "you cannot, ever again".

Refs #2593

backend/app/core/oidc_env.py

@@ -78,7 +78,7 @@ _APPLIED_FIELDS = (
 
 
 async def apply_env_oidc_provider(db: AsyncSession) -> None:
-    """Upsert the env-managed provider, or disable it when the config is gone.
+    """Upsert the env-managed provider, or release 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.
@@ -89,20 +89,35 @@ async def apply_env_oidc_provider(db: AsyncSession) -> None:
     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
+        # Nothing to look up by name any more, so the previously managed row is
+        # found by the flag -- and then released.
+        released = (
+            await db.execute(select(OIDCProvider).where(OIDCProvider.is_env_managed.is_(True)))
+        ).scalar_one_or_none()
+        if released is not 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.
+            # The flag is cleared as well: with no config behind it, a provider
+            # the API still refuses to edit or delete would be a dead end
+            # reachable only through the database.
+            released.is_enabled = False
+            released.is_env_managed = False
             await db.commit()
-            logger.info("BAMBUDDY_OIDC_* is unset -- env-managed provider disabled.")
+            logger.info(
+                "BAMBUDDY_OIDC_* is unset -- provider %r disabled and released to the UI.",
+                released.name,
+            )
         return
 
+    # Identity is the name, which is unique on the table. Matching on the flag
+    # instead meant an operator who named the env provider after one that
+    # already existed hit that unique constraint during startup -- and this
+    # function runs in the lifespan, so the app would not boot.
+    existing = (await db.execute(select(OIDCProvider).where(OIDCProvider.name == config["name"]))).scalar_one_or_none()
+
     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).

backend/tests/integration/test_oidc_env_apply.py

@@ -90,7 +90,10 @@ async def test_removing_the_env_config_disables_but_keeps_the_row(db_session, mo
         monkeypatch.delenv(key, raising=False)
     await apply_env_oidc_provider(db_session)
 
-    provider = await _env_provider(db_session)
+    # Looked up by name, not by the flag: releasing the provider clears the flag,
+    # and the point of this test is that the ROW survives either way.
+    result = await db_session.execute(select(OIDCProvider).where(OIDCProvider.name == "Keycloak"))
+    provider = result.scalar_one_or_none()
     assert provider is not None, "deleting would cascade away every account link"
     assert provider.id == original_id
     assert provider.is_enabled is False
@@ -158,3 +161,85 @@ async def test_applying_twice_without_changes_is_a_no_op(db_session, monkeypatch
 
     result = await db_session.execute(select(OIDCProvider).where(OIDCProvider.is_env_managed.is_(True)))
     assert len(result.scalars().all()) == 1
+
+
+# --- identity is the name, not the flag ---------------------------------------
+# The provider is looked up by BAMBUDDY_OIDC_NAME, which is unique on the table.
+# Matching on is_env_managed instead made three things impossible: adopting a
+# provider that already carries the name (the insert hit the unique constraint
+# and took startup down with it), releasing the provider when the config goes
+# away, and finding it again afterwards.
+
+
+@pytest.mark.asyncio
+async def test_a_name_collision_adopts_the_existing_provider(db_session, monkeypatch):
+    """An operator who names the env provider after one they created in the UI
+    must not end up with an app that refuses to boot."""
+    ui_provider = OIDCProvider(name="Keycloak", issuer_url="https://old.example.com", client_id="ui-client")
+    ui_provider.client_secret = "ui-secret"
+    db_session.add(ui_provider)
+    await db_session.commit()
+    original_id = ui_provider.id
+
+    _configure(monkeypatch)
+    await apply_env_oidc_provider(db_session)
+
+    provider = await _env_provider(db_session)
+    assert provider is not None
+    assert provider.id == original_id, "adopted, not duplicated"
+    assert provider.client_id == "bambuddy"
+
+    result = await db_session.execute(select(OIDCProvider).where(OIDCProvider.name == "Keycloak"))
+    assert len(result.scalars().all()) == 1
+
+
+@pytest.mark.asyncio
+async def test_removing_the_config_releases_the_provider_to_the_ui(db_session, monkeypatch):
+    """Nothing manages it any more, so the API must stop refusing edits and
+    deletes -- otherwise the row is a dead end only reachable via the database."""
+    _configure(monkeypatch)
+    await apply_env_oidc_provider(db_session)
+
+    for key in ALL_VARS:
+        monkeypatch.delenv(key, raising=False)
+    await apply_env_oidc_provider(db_session)
+
+    result = await db_session.execute(select(OIDCProvider).where(OIDCProvider.name == "Keycloak"))
+    provider = result.scalar_one()
+    assert provider.is_enabled is False
+    assert provider.is_env_managed is False
+
+
+@pytest.mark.asyncio
+async def test_restoring_the_config_finds_the_same_row_again(db_session, monkeypatch):
+    """The account links hang off this row; a second provider would orphan them."""
+    _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)
+
+    _configure(monkeypatch)
+    await apply_env_oidc_provider(db_session)
+
+    provider = await _env_provider(db_session)
+    assert provider.id == original_id
+    assert provider.is_enabled is True
+
+
+@pytest.mark.asyncio
+async def test_the_issuer_and_client_can_change_under_the_same_name(db_session, monkeypatch):
+    _configure(monkeypatch)
+    await apply_env_oidc_provider(db_session)
+    original_id = (await _env_provider(db_session)).id
+
+    monkeypatch.setenv("BAMBUDDY_OIDC_ISSUER_URL", "https://sso.example.com/realms/other")
+    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.issuer_url == "https://sso.example.com/realms/other"
+    assert provider.client_id == "rotated"