diff --git a/.env.example b/.env.example index 7a2227b2e..40503b36b 100644 --- a/.env.example +++ b/.env.example @@ -98,9 +98,19 @@ LOG_TO_FILE=true # BAMBUDDY_OIDC_REQUIRE_EMAIL_VERIFIED=true # BAMBUDDY_OIDC_ICON_URL= # BAMBUDDY_OIDC_AUTOLOGIN=false +# BAMBUDDY_OIDC_DEFAULT_GROUP= # # Booleans accept true/1/yes; anything else keeps the default. # +# DEFAULT_GROUP is the group new users land in when AUTO_CREATE_USERS is on; +# without it they get Viewers. It matches a group NAME exactly (case-sensitive) +# -- group ids are assigned per install, so the same compose file would point at +# a different group on every deployment. A name that matches no group is +# refused: the provider is left as it was and the reason is logged, rather than +# quietly creating under-privileged users the locked UI could not correct. On a +# FIRST boot that means no provider is created at all and no SSO button appears +# -- create the group first. Removing the variable clears the group again. +# # AUTO_LINK_EXISTING binds an OIDC identity to an existing local account with # the same email address. With EMAIL_CLAIM=email it is refused unless # REQUIRE_EMAIL_VERIFIED=true, because an identity provider that does not diff --git a/backend/app/core/config.py b/backend/app/core/config.py index 72bbca2e5..d704b0a22 100644 --- a/backend/app/core/config.py +++ b/backend/app/core/config.py @@ -149,6 +149,7 @@ _INTENTIONAL_UNSETTINGS = { "BAMBUDDY_OIDC_REQUIRE_EMAIL_VERIFIED", "BAMBUDDY_OIDC_ICON_URL", "BAMBUDDY_OIDC_AUTOLOGIN", + "BAMBUDDY_OIDC_DEFAULT_GROUP", } _known_settings_fields = {f.upper() for f in settings.model_fields} diff --git a/backend/app/core/oidc_env.py b/backend/app/core/oidc_env.py index 2e90b7053..cda941442 100644 --- a/backend/app/core/oidc_env.py +++ b/backend/app/core/oidc_env.py @@ -58,6 +58,11 @@ def read_env_oidc_config() -> dict | None: "require_email_verified": _env_bool("BAMBUDDY_OIDC_REQUIRE_EMAIL_VERIFIED", True), "icon_url": os.environ.get("BAMBUDDY_OIDC_ICON_URL"), "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 + # against the database in apply_env_oidc_provider -- the reader has no + # session and stays dumb. + "default_group": (os.environ.get("BAMBUDDY_OIDC_DEFAULT_GROUP") or "").strip() or None, } @@ -75,6 +80,10 @@ _APPLIED_FIELDS = ( "require_email_verified", "icon_url", "is_autologin", + # Written on every boot, so a group that is no longer declared is cleared: + # the environment is the whole truth for this row, and the API lock means + # a lingering value could not be removed in the UI either. + "default_group_id", ) @@ -86,6 +95,7 @@ async def apply_env_oidc_provider(db: AsyncSession) -> None: """ # 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.group import Group from backend.app.models.oidc_provider import OIDCProvider from backend.app.schemas.auth import OIDCProviderCreate @@ -128,6 +138,27 @@ async def apply_env_oidc_provider(db: AsyncSession) -> None: # 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() + # Resolved before anything is written, so a name that matches no group + # leaves the running provider untouched. Refused rather than defaulted: + # falling back would put every auto-created user in Viewers (routes/mfa.py) + # for as long as the typo lives, and the API answers 422 for a + # default_group_id that does not exist -- env config gets the same answer. + group_name = config.pop("default_group", None) + if group_name is not None: + group = (await db.execute(select(Group).where(Group.name == group_name))).scalar_one_or_none() + if group is None: + # Spelled out because the two cases differ sharply: an existing + # provider keeps running on its last good config, while on a first + # boot nothing is created at all and the login page has no SSO + # button until the name matches. + logger.error( + "BAMBUDDY_OIDC_DEFAULT_GROUP=%r matches no group, provider not applied (%s).", + group_name, + "previous config left running" if existing is not None else "no provider created", + ) + return + config["default_group_id"] = group.id + 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). diff --git a/backend/tests/integration/test_oidc_env_apply.py b/backend/tests/integration/test_oidc_env_apply.py index 3c9a7eb38..19a40aac2 100644 --- a/backend/tests/integration/test_oidc_env_apply.py +++ b/backend/tests/integration/test_oidc_env_apply.py @@ -8,6 +8,7 @@ recreating the provider would silently unlink every account bound to it. from __future__ import annotations import logging +import os import pytest from sqlalchemy import select @@ -32,6 +33,7 @@ ALL_VARS = ( "BAMBUDDY_OIDC_REQUIRE_EMAIL_VERIFIED", "BAMBUDDY_OIDC_ICON_URL", "BAMBUDDY_OIDC_AUTOLOGIN", + "BAMBUDDY_OIDC_DEFAULT_GROUP", ) @@ -378,6 +380,96 @@ async def test_releasing_the_provider_clears_autologin(db_session, monkeypatch): assert released.is_autologin is False +# --- default group by name ----------------------------------------------------- +# Group ids are not stable across installs, so a declarative deployment cannot +# name one by id. Without this, every auto-created user falls back to Viewers +# (routes/mfa.py) and the env lock means the UI cannot correct the provider. + + +async def _group(db_session, name: str): + from backend.app.models.group import Group + + group = Group(name=name, description=f"Test group {name}") + db_session.add(group) + await db_session.commit() + return group + + +@pytest.mark.asyncio +async def test_the_default_group_is_resolved_by_name(db_session, monkeypatch): + group = await _group(db_session, "Operators") + _configure(monkeypatch, BAMBUDDY_OIDC_DEFAULT_GROUP="Operators") + + await apply_env_oidc_provider(db_session) + + assert (await _env_provider(db_session)).default_group_id == group.id + + +@pytest.mark.asyncio +async def test_an_unknown_group_name_is_rejected_rather_than_defaulted(db_session, monkeypatch, caplog): + """Silently falling back to Viewers is how a typo mints under-privileged + users for weeks. The API answers 400 for a default_group_id that does not + exist; env config gets the same answer, logged and survivable.""" + _configure(monkeypatch, BAMBUDDY_OIDC_DEFAULT_GROUP="Nope") + + with caplog.at_level(logging.ERROR): + await apply_env_oidc_provider(db_session) + + assert await _env_provider(db_session) is None + assert "BAMBUDDY_OIDC_DEFAULT_GROUP" in caplog.text + assert "Nope" in caplog.text + + +@pytest.mark.asyncio +async def test_an_unknown_group_name_leaves_the_previous_provider_intact(db_session, monkeypatch): + """Rejection happens before the upsert, so the running config survives a + bad edit -- the provider keeps working until the operator fixes the name.""" + group = await _group(db_session, "Operators") + _configure(monkeypatch, BAMBUDDY_OIDC_DEFAULT_GROUP="Operators") + await apply_env_oidc_provider(db_session) + + monkeypatch.setenv("BAMBUDDY_OIDC_DEFAULT_GROUP", "Typo") + await apply_env_oidc_provider(db_session) + + provider = await _env_provider(db_session) + assert provider is not None + assert provider.default_group_id == group.id + + +@pytest.mark.asyncio +async def test_no_group_variable_leaves_the_default_group_unset(db_session, monkeypatch): + _configure(monkeypatch) + await apply_env_oidc_provider(db_session) + + assert (await _env_provider(db_session)).default_group_id is None + + +@pytest.mark.asyncio +async def test_removing_the_group_variable_clears_the_default_group(db_session, monkeypatch): + """The environment is the whole truth for this row; a group that is no + longer declared must not linger, since the lock blocks removing it in the UI.""" + await _group(db_session, "Operators") + _configure(monkeypatch, BAMBUDDY_OIDC_DEFAULT_GROUP="Operators") + await apply_env_oidc_provider(db_session) + + monkeypatch.delenv("BAMBUDDY_OIDC_DEFAULT_GROUP") + await apply_env_oidc_provider(db_session) + + assert (await _env_provider(db_session)).default_group_id is None + + +@pytest.mark.asyncio +async def test_an_empty_group_variable_counts_as_unset(db_session, monkeypatch): + """Same rule the required vars follow: an empty value in a compose file is + a forgotten value, not a request to reject the config.""" + _configure(monkeypatch, BAMBUDDY_OIDC_DEFAULT_GROUP="") + await apply_env_oidc_provider(db_session) + + provider = await _env_provider(db_session) + assert provider is not None + assert provider.default_group_id is None + + # --- account links and collision behavior ------------------------------------ @@ -488,6 +580,36 @@ async def test_renaming_with_autologin_updates_the_exclusivity_sweep(db_session, assert ui_provider.is_autologin is False +@pytest.mark.asyncio +async def test_group_name_matching_is_case_sensitive(db_session, monkeypatch, caplog): + """Group name is resolved by exact match; 'operators' != 'Operators'.""" + await _group(db_session, "Operators") # capital O + _configure(monkeypatch, BAMBUDDY_OIDC_DEFAULT_GROUP="operators") # lowercase + + with caplog.at_level(logging.ERROR): + await apply_env_oidc_provider(db_session) + + # Config is rejected + assert await _env_provider(db_session) is None + assert "operators" in caplog.text + assert "BAMBUDDY_OIDC_DEFAULT_GROUP" in caplog.text + + +@pytest.mark.asyncio +async def test_group_name_rejection_does_not_log_the_secret(db_session, monkeypatch, caplog): + """Group resolution happens before schema validation, so the secret is + not yet in scope, but verify it's not leaked by the error path.""" + _configure(monkeypatch, BAMBUDDY_OIDC_DEFAULT_GROUP="NonExistent") + secret = os.environ["BAMBUDDY_OIDC_CLIENT_SECRET"] + + with caplog.at_level(logging.ERROR): + await apply_env_oidc_provider(db_session) + + # Config is rejected but secret is safe + assert await _env_provider(db_session) is None + assert secret not in caplog.text + + @pytest.mark.asyncio async def test_restoring_env_config_after_rename_then_unset_finds_the_original_row(db_session, monkeypatch): """Rename Keycloak → Authentik, unset everything, restore Keycloak. diff --git a/backend/tests/unit/test_oidc_env_reader.py b/backend/tests/unit/test_oidc_env_reader.py index 307bf3dba..7a9c4ed96 100644 --- a/backend/tests/unit/test_oidc_env_reader.py +++ b/backend/tests/unit/test_oidc_env_reader.py @@ -28,6 +28,7 @@ OPTIONAL = ( "BAMBUDDY_OIDC_REQUIRE_EMAIL_VERIFIED", "BAMBUDDY_OIDC_ICON_URL", "BAMBUDDY_OIDC_AUTOLOGIN", + "BAMBUDDY_OIDC_DEFAULT_GROUP", ) @@ -118,6 +119,23 @@ def test_optional_strings_override_their_defaults(monkeypatch): assert cfg["icon_url"] == "https://sso.example.com/logo.png" +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.""" + _set_required(monkeypatch) + monkeypatch.setenv("BAMBUDDY_OIDC_DEFAULT_GROUP", "Operators") + cfg = read_env_oidc_config() + assert cfg["default_group"] == "Operators" + assert "default_group_id" not in cfg, "resolution needs the database, not the reader" + + +@pytest.mark.parametrize("raw", ["", " "]) +def test_a_blank_default_group_is_unset(monkeypatch, raw): + _set_required(monkeypatch) + monkeypatch.setenv("BAMBUDDY_OIDC_DEFAULT_GROUP", raw) + assert read_env_oidc_config()["default_group"] is None + + def test_every_var_the_reader_knows_is_registered_in_the_typo_guard(): """An unregistered BAMBUDDY_* var logs "possible typo" at every boot, which would tell operators their correct config is wrong. Asserted against the