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

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.
Marian 1 месяц назад
Родитель
Сommit
c4b5d42f48
2 измененных файлов с 45 добавлено и 1 удалено
  1. 13 1
      backend/app/core/oidc_env.py
  2. 32 0
      backend/tests/integration/test_oidc_env_apply.py

+ 13 - 1
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)

+ 32 - 0
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