From c4b5d42f48886a866c1350b82c64e23a2bcd7749 Mon Sep 17 00:00:00 2001 From: Marian Date: Fri, 31 Jul 2026 13:11:15 +0000 Subject: [PATCH] fix(oidc): log distinctly when env config adopts a UI-created provider A name collision with a provider that was NOT already env-managed overwrites its issuer, client id and secret in place and locks it behind the env-managed 409 -- but it logged the same routine "applied" line as an ordinary re-apply, giving no signal a UI provider was just taken over. Adoption is now a WARNING with its own wording; a routine re-apply of an already env-managed provider keeps the INFO line. --- backend/app/core/oidc_env.py | 14 +++++++- .../tests/integration/test_oidc_env_apply.py | 32 +++++++++++++++++++ 2 files changed, 45 insertions(+), 1 deletion(-) diff --git a/backend/app/core/oidc_env.py b/backend/app/core/oidc_env.py index 16a3f3d71..123399f75 100644 --- a/backend/app/core/oidc_env.py +++ b/backend/app/core/oidc_env.py @@ -190,6 +190,11 @@ async def _apply_env_oidc_provider(db: AsyncSession) -> None: logger.error("BAMBUDDY_OIDC_* config could not be applied: %s", type(exc).__name__) return + # Computed before `existing` is reassigned below: a freshly-created row is + # not an adoption, and a found row that was already env-managed is a + # routine re-apply -- only a found row that the UI created is an adoption. + adopted_ui_provider = existing is not None and not existing.is_env_managed + if existing is None: existing = OIDCProvider(is_env_managed=True) db.add(existing) @@ -219,4 +224,11 @@ async def _apply_env_oidc_provider(db: AsyncSession) -> None: .values(is_autologin=False) ) await db.commit() - logger.info("Env-managed OIDC provider %r applied.", existing.name) + if adopted_ui_provider: + logger.warning( + "Env-managed OIDC provider %r adopted an existing UI-created provider of the " + "same name; its issuer, client and secret are now managed by BAMBUDDY_OIDC_*.", + existing.name, + ) + else: + 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 index 965ee1c78..5fce40c48 100644 --- a/backend/tests/integration/test_oidc_env_apply.py +++ b/backend/tests/integration/test_oidc_env_apply.py @@ -261,6 +261,38 @@ async def test_a_name_collision_adopts_the_existing_provider(db_session, monkeyp assert len(result.scalars().all()) == 1 +@pytest.mark.asyncio +async def test_adopting_a_ui_provider_logs_a_distinct_warning(db_session, monkeypatch, caplog): + """Overwriting a UI-created provider in place is a bigger deal than a + routine re-apply -- it must not be silent at the same INFO level.""" + 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() + + _configure(monkeypatch) + with caplog.at_level(logging.INFO): + await apply_env_oidc_provider(db_session) + + warnings = [r for r in caplog.records if r.levelname == "WARNING"] + assert any("adopted" in r.message for r in warnings) + + +@pytest.mark.asyncio +async def test_a_routine_reapply_does_not_log_an_adoption_warning(db_session, monkeypatch, caplog): + """The same provider re-applying on the next boot is not an adoption -- + it was already env-managed.""" + _configure(monkeypatch) + await apply_env_oidc_provider(db_session) + caplog.clear() + + with caplog.at_level(logging.INFO): + await apply_env_oidc_provider(db_session) + + warnings = [r for r in caplog.records if r.levelname == "WARNING"] + assert not any("adopted" in r.message for r in warnings) + + @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