diff --git a/CHANGELOG.md b/CHANGELOG.md index 228cb987f..02449b518 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,6 +10,8 @@ All notable changes to Bambuddy will be documented in this file. - **Home Assistant addon detection — Settings → Updates and the in-app update banner now defer to the HA Supervisor** ([#1167](https://github.com/maziggy/bambuddy/issues/1167), reported by @Spegeli) — Bambuddy already shipped `HA_URL`/`HA_TOKEN` env-var support specifically labelled "for HA Add-on deployments" ([#283](https://github.com/maziggy/bambuddy/issues/283)) and a community-maintained HA addon (`hobbypunk90/homeassistant-addon-bambuddy`) exists upstream, so an HA-supervised installation is a real first-class deployment shape. Until now though, the update UI didn't know about it: HA addon users got the same "Update available!" banner as everyone else and, if they clicked through to Settings, saw the docker-compose snippet ("`docker compose pull && docker compose up -d`") which they cannot run from inside an HA addon container — that's the Supervisor's job. Detection uses the canonical signal: HA Supervisor injects `SUPERVISOR_TOKEN` into every addon container, and that variable is not set in any other environment. A new `_is_ha_addon()` helper in `backend/app/api/routes/updates.py` flips a request-level boolean which `/updates/check` surfaces as `is_ha_addon: bool` + an extended `update_method: 'git' | 'docker' | 'ha_addon'` enum. The check is checked **before** Docker on `/updates/apply` because HA addons *are* Docker containers — checking docker first would mis-classify them and serve the wrong message; the response also keeps `is_docker: true` alongside `is_ha_addon: true` so older frontend bundles still hit a managed-deployment branch (degrading to the Docker UX) instead of rendering an in-app Install button that can't work. Frontend branches identically: `SettingsPage.tsx`'s update card checks `is_ha_addon` first and renders "Updates are managed by the Home Assistant Supervisor. Open Settings → Add-ons → Bambuddy in Home Assistant to install the new version." in place of the docker-compose hint; `Layout.tsx`'s update banner is suppressed entirely for HA addons since the HA Supervisor's own update notification already surfaces the new version natively in the HA UI and a duplicate Bambuddy banner would just be noise that links to a page that says "go to HA". Plain Docker deployments are unaffected — the existing docker-compose hint and the in-app banner still render the same way they did. Localised across all 8 UI languages (en/de/fr/it/ja/pt-BR/zh-CN/zh-TW) with full translations of the new `settings.updateViaHomeAssistant` string. 6 new tests pin the contract: 3 backend unit tests for `_is_ha_addon()` (env var present → true, absent → false, empty string treated as unset to guard against shells that export it empty), 1 backend integration test for the HA-precedes-Docker rejection on `/updates/apply` (asserts the message says "Home Assistant" and not "Docker Compose"), 2 backend integration tests for `/updates/check` covering the HA-addon branch (`update_method == "ha_addon"`, both flags true) and the plain-Docker branch (`is_ha_addon: false`, `update_method == "docker"`); 2 frontend SettingsPage tests pin the mutually-exclusive UI rendering (HA branch shows the HA copy and not the docker-compose snippet; Docker branch shows the snippet and not the HA copy, neither shows the Install button); 2 frontend Layout tests pin the banner suppression for HA and its retention for plain Docker. +- **OIDC auto-created users now get readable usernames and land in a configurable group** ([#1173](https://github.com/maziggy/bambuddy/issues/1173)) — Two improvements to the OIDC auto-create flow: (1) **Username derivation**: Bambuddy now derives the username from `preferred_username`, then `name`, before falling back to the opaque `provider_sub[:30]`. Each candidate is sanitized independently — alphanumeric plus `./-/_`, whitespace collapsed, deduplication suffix appended on collision — so a value that strips to empty (e.g. `"!!!"`) correctly falls through to the next option rather than silently producing `"oidcuser"`. (2) **Default group**: each OIDC provider gains a `default_group_id` field. When set, auto-created users are placed in that group; when unset, the existing "Viewers" fallback is preserved, so behaviour is unchanged for existing deployments. The column is nullable with `ON DELETE SET NULL`; SQLite does not enforce FK constraints here, so a deleted configured group falls through to Viewers at runtime. `default_group_id` is validated on create/update (422 on a non-existent group). Exposed in the OIDC settings form as a group dropdown. **Limitation:** to clear a configured default group, delete the group or select a different one — explicit reset-to-null is not currently supported. + - **Filament Track Switch (FTS) support — print modal filament dropdown is no longer empty when an X2D / H2D has the FTS accessory installed** ([#1162](https://github.com/maziggy/bambuddy/issues/1162), reported by @mkavalecz) — When the FTS accessory is installed the printer's MQTT changes one nibble of the per-AMS `info` bitmask: bits 8-11 flip from a fixed extruder ID (0x0 / 0x1) to `0xE` ("uninitialized"), because the AMS is no longer wired to a single nozzle — the FTS dynamically routes any slot to either extruder. Bambuddy's MQTT parser already skipped 0xE entries when building `ams_extruder_map` (matching BambuStudio's reading for boot-time transient state), so with the FTS installed the map ended up empty and the print modal's filament dropdown — which filters by `extruderId === nozzle_id` to prevent cross-nozzle assignment ("position of left hotend is abnormal" failures) — filtered out *every* loaded slot. Net effect: empty Filament Mapping dropdown on every dual-nozzle print with the FTS, even when the AMS was fully loaded with the right material. Detection comes from a new MQTT field — `print.device.fila_switch` — which is non-null only when the accessory is installed; it carries the routing topology as two arrays: `in[track] = currently fed slot (-1 = empty)` and `out[track] = extruder this track terminates at`. The fix surfaces this through a new `FilaSwitchState` dataclass on `PrinterState` (`installed`, `in_slots`, `out_extruders`, `stat`, `info`) and the equivalent `FilaSwitchResponse` Pydantic schema on the `GET /printers/{id}/status` route. Frontend (`useFilamentMapping.ts` + `FilamentMapping.tsx`) skips the per-extruder filter when `printerStatus.fila_switch?.installed === true` so any compatible AMS slot can satisfy any nozzle's filament requirement, since the FTS handles the routing. Slots currently fed into a track also get a routing badge in the dropdown — `[L]` or `[R]` — so the user can tell at a glance which slot the FTS is currently routing where (idle slots get no badge: they can be routed to either extruder on demand). The hard "no cross-nozzle assignment" filter on real dual-nozzle printers without the FTS stays untouched (still trips the same way it always has — `fila_switch == null` keeps the existing behaviour). 4 backend tests in `test_bambu_mqtt.py::TestFilamentTrackSwitchDetection` (default-not-installed, detect-from-MQTT-using-the-reporter's-bundle, no-fila_switch-field-stays-not-installed, missing-in-out-arrays-don't-crash) and 2 frontend tests in `useFilamentMapping.test.ts` (FTS-active drops the nozzle filter; explicit `fila_switch: null` keeps the filter applied). Upstream fila_switch payloads with anything other than the documented shape are tolerated — `installed` flips on the *presence* of the field, the routing arrays default to empty lists if missing, and the dropdown skips the badge for slots not currently in `in_slots`. ### Fixed diff --git a/backend/app/api/routes/mfa.py b/backend/app/api/routes/mfa.py index 7b540cdd1..1784d0e41 100644 --- a/backend/app/api/routes/mfa.py +++ b/backend/app/api/routes/mfa.py @@ -1168,6 +1168,13 @@ async def create_oidc_provider( db: AsyncSession = Depends(get_db), ) -> OIDCProviderResponse: """Create a new OIDC provider (admin only).""" + if body.default_group_id is not None: + grp_chk = await db.execute(select(Group).where(Group.id == body.default_group_id)) + if not grp_chk.scalar_one_or_none(): + raise HTTPException( + status_code=status.HTTP_422_UNPROCESSABLE_ENTITY, + detail="default_group_id references a non-existent group", + ) provider = OIDCProvider( name=body.name, issuer_url=body.issuer_url.rstrip("/"), @@ -1180,6 +1187,7 @@ async def create_oidc_provider( email_claim=body.email_claim, require_email_verified=body.require_email_verified, icon_url=body.icon_url, + default_group_id=body.default_group_id, ) # SEC-1 + SEC-6: runtime guard mirrors the OIDCProviderCreate model_validator in schemas/auth.py. # Catches any future path that bypasses Pydantic validation (direct ORM, scripts). @@ -1203,6 +1211,14 @@ async def update_oidc_provider( if not provider: raise HTTPException(status_code=status.HTTP_404_NOT_FOUND, detail="Provider not found") + if body.default_group_id is not None: + grp_chk = await db.execute(select(Group).where(Group.id == body.default_group_id)) + if not grp_chk.scalar_one_or_none(): + raise HTTPException( + status_code=status.HTTP_422_UNPROCESSABLE_ENTITY, + detail="default_group_id references a non-existent group", + ) + for field, value in body.model_dump(exclude_none=True).items(): if field == "issuer_url" and value: value = value.rstrip("/") @@ -1572,7 +1588,21 @@ async def oidc_callback( if provider_email: raw = provider_email.split("@")[0] else: - raw = provider_sub[:30] + # Prefer a human-readable IdP claim over the opaque sub. + # isinstance guards are required: claims may carry non-string + # values (e.g. a list) that would break .strip(). + # Sanitization is applied per-candidate so that a value that + # strips to empty (e.g. "!!!") correctly falls through to the + # next candidate rather than silently becoming "oidcuser". + _pref = claims.get("preferred_username") + _name = claims.get("name") + raw = "" + if isinstance(_pref, str): + raw = re.sub(r"[^a-zA-Z0-9._-]", "", _pref.strip())[:30] + if not raw and isinstance(_name, str): + raw = re.sub(r"[^a-zA-Z0-9._-]", "", _name.strip())[:30] + if not raw: + raw = provider_sub[:30] candidate = re.sub(r"[^a-zA-Z0-9._-]", "", raw)[:30] or "oidcuser" username = candidate @@ -1584,13 +1614,21 @@ async def oidc_callback( username = f"{candidate}{counter}" counter += 1 - # I9: Assign new OIDC users to the default "Viewers" group so they - # have read-only access rather than starting with no permissions. - # Fetch the group BEFORE creating the user so we can set the - # relationship before flush — accessing new_user.groups after a - # flush triggers a lazy-load which fails in async context. - viewers_result = await db.execute(select(Group).where(Group.name == "Viewers")) - viewers_group = viewers_result.scalar_one_or_none() + # I9: Assign new OIDC users to a group before flush — accessing + # new_user.groups after a flush triggers a lazy-load which fails + # in async context. Resolution order: + # 1. provider.default_group_id (operator-configured) + # 2. "Viewers" (system fallback for read-only access) + # 3. no group (last resort if Viewers was deleted) + # SQLite does not enforce ON DELETE SET NULL, so a dangling + # default_group_id returns None here and falls through to Viewers. + default_group: Group | None = None + if provider.default_group_id is not None: + dg_result = await db.execute(select(Group).where(Group.id == provider.default_group_id)) + default_group = dg_result.scalar_one_or_none() + if default_group is None: + viewers_result = await db.execute(select(Group).where(Group.name == "Viewers")) + default_group = viewers_result.scalar_one_or_none() new_user = User( username=username, @@ -1601,7 +1639,7 @@ async def oidc_callback( password_hash=None, # OIDC users never use password auth role="user", is_active=True, - groups=[viewers_group] if viewers_group else [], + groups=[default_group] if default_group else [], ) db.add(new_user) await db.flush() diff --git a/backend/app/core/database.py b/backend/app/core/database.py index f50d7cbc3..4ebbb8f8c 100644 --- a/backend/app/core/database.py +++ b/backend/app/core/database.py @@ -1745,6 +1745,14 @@ async def run_migrations(conn): # is still present in sqlite_master (SQLite cannot ALTER TABLE DROP/ADD CONSTRAINT). await _migrate_update_auto_link_constraint(conn) + # Migration: Add default_group_id to oidc_providers. + # Must run AFTER _migrate_update_auto_link_constraint to avoid being dropped during + # the SQLite table recreation that function performs on stale-formula databases. + await _safe_execute( + conn, + "ALTER TABLE oidc_providers ADD COLUMN default_group_id INTEGER REFERENCES groups(id) ON DELETE SET NULL", + ) + # Migration: Add password_changed_at to users (M-R7-B) # Tracks the last time a user's password was changed/reset. JWTs whose iat # predates this timestamp are rejected in all six auth validation paths. diff --git a/backend/app/models/oidc_provider.py b/backend/app/models/oidc_provider.py index 140b1c2be..5b00a1eec 100644 --- a/backend/app/models/oidc_provider.py +++ b/backend/app/models/oidc_provider.py @@ -73,6 +73,12 @@ class OIDCProvider(Base): # Has no effect when email_claim is not "email": the custom-claim path never # performs an email_verified check regardless of this setting. require_email_verified: Mapped[bool] = mapped_column(Boolean, default=True) + # Nullable FK — configurable default group for auto-created OIDC users. + # Falls back to "Viewers" when None. ON DELETE SET NULL fires on PostgreSQL; + # SQLite ignores it (no PRAGMA foreign_keys=ON), so runtime resolution handles dangling refs. + default_group_id: Mapped[int | None] = mapped_column( + Integer, ForeignKey("groups.id", ondelete="SET NULL"), nullable=True, default=None + ) # Optional icon URL (SVG/PNG) shown on the login button icon_url: Mapped[str | None] = mapped_column(Text, nullable=True, default=None) created_at: Mapped[datetime] = mapped_column(DateTime, server_default=func.now()) diff --git a/backend/app/schemas/auth.py b/backend/app/schemas/auth.py index e45067d68..4b0ee5233 100644 --- a/backend/app/schemas/auth.py +++ b/backend/app/schemas/auth.py @@ -371,6 +371,7 @@ class OIDCProviderCreate(BaseModel): email_claim: str = Field(default="email", max_length=64) require_email_verified: bool = True icon_url: str | None = None + default_group_id: int | None = None @field_validator("issuer_url") @classmethod @@ -426,6 +427,7 @@ class OIDCProviderUpdate(BaseModel): email_claim: str | None = Field(default=None, max_length=64) require_email_verified: bool | None = None icon_url: str | None = None + default_group_id: int | None = None @field_validator("scopes") @classmethod @@ -471,6 +473,7 @@ class OIDCProviderResponse(BaseModel): email_claim: str = "email" require_email_verified: bool = True icon_url: str | None = None + default_group_id: int | None = None class Config: from_attributes = True diff --git a/backend/tests/integration/test_mfa_api.py b/backend/tests/integration/test_mfa_api.py index 756c9dbb8..f8f456350 100644 --- a/backend/tests/integration/test_mfa_api.py +++ b/backend/tests/integration/test_mfa_api.py @@ -862,6 +862,151 @@ class TestOIDCProviders: ) assert response.status_code == 404 + @pytest.mark.asyncio + @pytest.mark.integration + async def test_create_provider_with_default_group_id(self, async_client: AsyncClient, db_session: AsyncSession): + """Creating a provider with a valid default_group_id stores and returns the value.""" + from sqlalchemy import select + + from backend.app.models.group import Group + + token = await _setup_and_login(async_client, "oidcdg_create", "OidcDgCreate1!") + grp_result = await db_session.execute(select(Group).where(Group.name == "Operators")) + operators = grp_result.scalar_one() + + resp = await async_client.post( + "/api/v1/auth/oidc/providers", + json={ + "name": "DgCreateProvider", + "issuer_url": "https://dgcreate.example.com", + "client_id": "dgcreate-client", + "client_secret": "secret", + "scopes": "openid", + "is_enabled": True, + "auto_create_users": False, + "default_group_id": operators.id, + }, + headers=_auth_header(token), + ) + assert resp.status_code == 201, resp.text + assert resp.json()["default_group_id"] == operators.id + + @pytest.mark.asyncio + @pytest.mark.integration + async def test_create_provider_invalid_default_group_id_returns_422(self, async_client: AsyncClient): + """A default_group_id referencing a non-existent group returns 422.""" + token = await _setup_and_login(async_client, "oidcdg_bad", "OidcDgBad1!") + resp = await async_client.post( + "/api/v1/auth/oidc/providers", + json={ + "name": "DgBadProvider", + "issuer_url": "https://dgbad.example.com", + "client_id": "dgbad-client", + "client_secret": "secret", + "scopes": "openid", + "is_enabled": True, + "auto_create_users": False, + "default_group_id": 999999, + }, + headers=_auth_header(token), + ) + assert resp.status_code == 422 + + @pytest.mark.asyncio + @pytest.mark.integration + async def test_create_provider_omit_default_group_id_stores_null(self, async_client: AsyncClient): + """Omitting default_group_id results in null in the response.""" + token = await _setup_and_login(async_client, "oidcdg_null", "OidcDgNull1!") + resp = await async_client.post( + "/api/v1/auth/oidc/providers", + json={ + "name": "DgNullProvider", + "issuer_url": "https://dgnull.example.com", + "client_id": "dgnull-client", + "client_secret": "secret", + "scopes": "openid", + "is_enabled": True, + "auto_create_users": False, + }, + headers=_auth_header(token), + ) + assert resp.status_code == 201, resp.text + assert resp.json()["default_group_id"] is None + + @pytest.mark.asyncio + @pytest.mark.integration + async def test_update_provider_default_group_id(self, async_client: AsyncClient, db_session: AsyncSession): + """Updating default_group_id via PUT stores the new value.""" + from sqlalchemy import select + + from backend.app.models.group import Group + + token = await _setup_and_login(async_client, "oidcdg_update", "OidcDgUpdate1!") + create_resp = await async_client.post( + "/api/v1/auth/oidc/providers", + json={ + "name": "DgUpdateProvider", + "issuer_url": "https://dgupdate.example.com", + "client_id": "dgupdate-client", + "client_secret": "secret", + "scopes": "openid", + "is_enabled": True, + "auto_create_users": False, + }, + headers=_auth_header(token), + ) + provider_id = create_resp.json()["id"] + assert create_resp.json()["default_group_id"] is None + + grp_result = await db_session.execute(select(Group).where(Group.name == "Operators")) + operators = grp_result.scalar_one() + + put_resp = await async_client.put( + f"/api/v1/auth/oidc/providers/{provider_id}", + json={"default_group_id": operators.id}, + headers=_auth_header(token), + ) + assert put_resp.status_code == 200, put_resp.text + assert put_resp.json()["default_group_id"] == operators.id + + @pytest.mark.asyncio + @pytest.mark.integration + async def test_default_group_id_in_public_and_admin_list(self, async_client: AsyncClient, db_session: AsyncSession): + """default_group_id appears in both the public and admin list responses.""" + from sqlalchemy import select + + from backend.app.models.group import Group + + token = await _setup_and_login(async_client, "oidcdg_list", "OidcDgList1!") + grp_result = await db_session.execute(select(Group).where(Group.name == "Operators")) + operators = grp_result.scalar_one() + + create_resp = await async_client.post( + "/api/v1/auth/oidc/providers", + json={ + "name": "DgListProvider", + "issuer_url": "https://dglist.example.com", + "client_id": "dglist-client", + "client_secret": "secret", + "scopes": "openid", + "is_enabled": True, + "auto_create_users": False, + "default_group_id": operators.id, + }, + headers=_auth_header(token), + ) + provider_id = create_resp.json()["id"] + + all_resp = await async_client.get("/api/v1/auth/oidc/providers/all", headers=_auth_header(token)) + match = next((p for p in all_resp.json() if p["id"] == provider_id), None) + assert match is not None + assert match["default_group_id"] == operators.id + + pub_resp = await async_client.get("/api/v1/auth/oidc/providers") + pub_match = next((p for p in pub_resp.json() if p["id"] == provider_id), None) + assert pub_match is not None + assert pub_match["default_group_id"] == operators.id + # =========================================================================== # Security: pre-auth token single-use @@ -4240,3 +4385,461 @@ class TestOIDCFallCAutoLinkE2E: link = result.scalar_one_or_none() assert link is not None, "UserOIDCLink must have been created by auto-link" assert link.provider_user_id == "azure-sub-alice" + + +class TestOIDCAutoCreateUsername: + """Username derivation priority for auto-created OIDC users (#1173). + + Priority order: email local-part > preferred_username > name > provider_sub. + Covers: plain claim, spaces-sanitized, name fallback, sub fallback, + non-string isinstance guard, sanitizes-to-empty fallback, collision counter. + """ + + # ── shared helpers ─────────────────────────────────────────────────────── + + @staticmethod + async def _create_provider(async_client: AsyncClient, admin_token: str, issuer: str, client_id: str) -> int: + resp = await async_client.post( + "/api/v1/auth/oidc/providers", + json={ + "name": f"AutoUser-{secrets.token_hex(4)}", + "issuer_url": issuer, + "client_id": client_id, + "client_secret": "secret", + "scopes": "openid profile", + "is_enabled": True, + "auto_create_users": True, + "email_claim": "email", + "require_email_verified": True, + }, + headers={"Authorization": f"Bearer {admin_token}"}, + ) + assert resp.status_code == 201, resp.text + return resp.json()["id"] + + @staticmethod + async def _exchange_username(async_client: AsyncClient, location: str) -> str: + assert "oidc_token=" in location, f"No oidc_token in redirect: {location}" + token = location.split("oidc_token=")[1].split("&")[0].split("#")[-1] + resp = await async_client.post("/api/v1/auth/oidc/exchange", json={"oidc_token": token}) + assert resp.status_code == 200, resp.text + return resp.json()["user"]["username"] + + # ── tests ──────────────────────────────────────────────────────────────── + + @pytest.mark.asyncio + @pytest.mark.integration + async def test_preferred_username_used_when_no_email(self, async_client: AsyncClient, db_session: AsyncSession): + """preferred_username='johndoe' → username 'johndoe' (no email claim present).""" + private_pem, jwks_data = _make_test_rsa_key() + issuer = "https://au-pref.example" + client_id = "au-pref-client" + admin_token = await _setup_and_login(async_client, "au_pref_adm", "AuPrefAdm1!") + provider_id = await self._create_provider(async_client, admin_token, issuer, client_id) + + location = await _run_oidc_callback( + async_client, + db_session, + provider_id=provider_id, + claims={"sub": "pref-sub-1", "preferred_username": "johndoe"}, + private_pem=private_pem, + jwks_data=jwks_data, + issuer=issuer, + client_id=client_id, + ) + username = await self._exchange_username(async_client, location) + assert username == "johndoe" + + @pytest.mark.asyncio + @pytest.mark.integration + async def test_preferred_username_spaces_sanitized(self, async_client: AsyncClient, db_session: AsyncSession): + """preferred_username='John Doe' → sanitized to 'JohnDoe'.""" + private_pem, jwks_data = _make_test_rsa_key() + issuer = "https://au-spaces.example" + client_id = "au-spaces-client" + admin_token = await _setup_and_login(async_client, "au_spaces_adm", "AuSpacesAdm1!") + provider_id = await self._create_provider(async_client, admin_token, issuer, client_id) + + location = await _run_oidc_callback( + async_client, + db_session, + provider_id=provider_id, + claims={"sub": "spaces-sub-1", "preferred_username": "John Doe"}, + private_pem=private_pem, + jwks_data=jwks_data, + issuer=issuer, + client_id=client_id, + ) + username = await self._exchange_username(async_client, location) + assert username == "JohnDoe" + + @pytest.mark.asyncio + @pytest.mark.integration + async def test_name_claim_used_when_no_preferred_username( + self, async_client: AsyncClient, db_session: AsyncSession + ): + """name='Jane Smith', no preferred_username → username 'JaneSmith'.""" + private_pem, jwks_data = _make_test_rsa_key() + issuer = "https://au-name.example" + client_id = "au-name-client" + admin_token = await _setup_and_login(async_client, "au_name_adm", "AuNameAdm1!") + provider_id = await self._create_provider(async_client, admin_token, issuer, client_id) + + location = await _run_oidc_callback( + async_client, + db_session, + provider_id=provider_id, + claims={"sub": "name-sub-1", "name": "Jane Smith"}, + private_pem=private_pem, + jwks_data=jwks_data, + issuer=issuer, + client_id=client_id, + ) + username = await self._exchange_username(async_client, location) + assert username == "JaneSmith" + + @pytest.mark.asyncio + @pytest.mark.integration + async def test_provider_sub_fallback_when_no_claims(self, async_client: AsyncClient, db_session: AsyncSession): + """No preferred_username, no name, no email → username derived from provider_sub.""" + private_pem, jwks_data = _make_test_rsa_key() + issuer = "https://au-sub.example" + client_id = "au-sub-client" + admin_token = await _setup_and_login(async_client, "au_sub_adm", "AuSubAdm1!") + provider_id = await self._create_provider(async_client, admin_token, issuer, client_id) + + location = await _run_oidc_callback( + async_client, + db_session, + provider_id=provider_id, + claims={"sub": "abc123xyz"}, + private_pem=private_pem, + jwks_data=jwks_data, + issuer=issuer, + client_id=client_id, + ) + username = await self._exchange_username(async_client, location) + assert username == "abc123xyz" + + @pytest.mark.asyncio + @pytest.mark.integration + async def test_non_string_preferred_username_falls_through_to_name( + self, async_client: AsyncClient, db_session: AsyncSession + ): + """preferred_username is a list (non-string) → isinstance guard skips it, uses name.""" + private_pem, jwks_data = _make_test_rsa_key() + issuer = "https://au-nonstr.example" + client_id = "au-nonstr-client" + admin_token = await _setup_and_login(async_client, "au_nonstr_adm", "AuNonstrAdm1!") + provider_id = await self._create_provider(async_client, admin_token, issuer, client_id) + + location = await _run_oidc_callback( + async_client, + db_session, + provider_id=provider_id, + claims={"sub": "nonstr-sub-2", "preferred_username": ["listval"], "name": "BobJones"}, + private_pem=private_pem, + jwks_data=jwks_data, + issuer=issuer, + client_id=client_id, + ) + username = await self._exchange_username(async_client, location) + assert username == "BobJones" + + @pytest.mark.asyncio + @pytest.mark.integration + async def test_preferred_username_sanitizes_to_empty_falls_through_to_name( + self, async_client: AsyncClient, db_session: AsyncSession + ): + """preferred_username='!!!' sanitizes to '' → falls through to name claim.""" + private_pem, jwks_data = _make_test_rsa_key() + issuer = "https://au-empty.example" + client_id = "au-empty-client" + admin_token = await _setup_and_login(async_client, "au_empty_adm", "AuEmptyAdm1!") + provider_id = await self._create_provider(async_client, admin_token, issuer, client_id) + + location = await _run_oidc_callback( + async_client, + db_session, + provider_id=provider_id, + claims={"sub": "empty-sub-1", "preferred_username": "!!!", "name": "bob"}, + private_pem=private_pem, + jwks_data=jwks_data, + issuer=issuer, + client_id=client_id, + ) + username = await self._exchange_username(async_client, location) + assert username == "bob" + + @pytest.mark.asyncio + @pytest.mark.integration + async def test_username_collision_appends_counter(self, async_client: AsyncClient, db_session: AsyncSession): + """When preferred_username 'collider' is already taken, counter suffix is appended.""" + from backend.app.core.auth import get_password_hash + + # Pre-create a user occupying the candidate username + existing = User( + username="collider", + email="collider@example.com", + password_hash=get_password_hash("irrelevant"), + role="user", + is_active=True, + ) + db_session.add(existing) + await db_session.commit() + + private_pem, jwks_data = _make_test_rsa_key() + issuer = "https://au-collision.example" + client_id = "au-collision-client" + admin_token = await _setup_and_login(async_client, "au_col_adm", "AuColAdm1!") + provider_id = await self._create_provider(async_client, admin_token, issuer, client_id) + + location = await _run_oidc_callback( + async_client, + db_session, + provider_id=provider_id, + claims={"sub": "col-sub-1", "preferred_username": "collider"}, + private_pem=private_pem, + jwks_data=jwks_data, + issuer=issuer, + client_id=client_id, + ) + username = await self._exchange_username(async_client, location) + assert username == "collider1" + + +# =========================================================================== +# OIDC auto-create: configurable default group (#1173 Thread 2) +# =========================================================================== + + +class TestOIDCAutoCreateDefaultGroup: + """Auto-created OIDC users receive the provider's configured default group. + + Resolution order: + 1. provider.default_group_id (configured) + 2. "Viewers" system group (fallback when default_group_id is None) + 3. no group (last resort when both are unavailable) + + All tests are DB-agnostic: they verify group membership via the OIDC + exchange response, which includes the user's group list. + """ + + @staticmethod + async def _create_provider( + async_client: AsyncClient, + admin_token: str, + *, + issuer: str, + client_id: str, + default_group_id: int | None = None, + ) -> int: + payload: dict = { + "name": f"DgAutoProvider-{secrets.token_hex(4)}", + "issuer_url": issuer, + "client_id": client_id, + "client_secret": "secret", + "scopes": "openid profile", + "is_enabled": True, + "auto_create_users": True, + "email_claim": "email", + "require_email_verified": True, + } + if default_group_id is not None: + payload["default_group_id"] = default_group_id + resp = await async_client.post( + "/api/v1/auth/oidc/providers", + json=payload, + headers={"Authorization": f"Bearer {admin_token}"}, + ) + assert resp.status_code == 201, resp.text + return resp.json()["id"] + + @staticmethod + async def _run_autocreate_and_get_groups( + async_client: AsyncClient, + db_session: AsyncSession, + *, + provider_id: int, + sub: str, + issuer: str, + client_id: str, + private_pem: bytes, + jwks_data: dict, + ) -> list[str]: + """Complete OIDC callback + exchange and return the new user's group names.""" + location = await _run_oidc_callback( + async_client, + db_session, + provider_id=provider_id, + claims={"sub": sub}, + private_pem=private_pem, + jwks_data=jwks_data, + issuer=issuer, + client_id=client_id, + ) + assert "oidc_token=" in location, f"No oidc_token in redirect: {location}" + token = location.split("oidc_token=")[1].split("&")[0].split("#")[-1] + resp = await async_client.post("/api/v1/auth/oidc/exchange", json={"oidc_token": token}) + assert resp.status_code == 200, resp.text + return [g["name"] for g in resp.json()["user"]["groups"]] + + # ── tests ──────────────────────────────────────────────────────────────── + + @pytest.mark.asyncio + @pytest.mark.integration + async def test_configured_group_assigned_to_auto_created_user( + self, async_client: AsyncClient, db_session: AsyncSession + ): + """Auto-created user is placed in the provider's configured default_group_id.""" + from sqlalchemy import select + + from backend.app.models.group import Group + + private_pem, jwks_data = _make_test_rsa_key() + issuer = "https://dg-configured.example" + client_id = "dg-configured-client" + admin_token = await _setup_and_login(async_client, "dg_cfg_adm", "DgCfgAdm1!") + + grp_result = await db_session.execute(select(Group).where(Group.name == "Operators")) + operators = grp_result.scalar_one() + + provider_id = await self._create_provider( + async_client, + admin_token, + issuer=issuer, + client_id=client_id, + default_group_id=operators.id, + ) + + group_names = await self._run_autocreate_and_get_groups( + async_client, + db_session, + provider_id=provider_id, + sub="dg-cfg-sub-1", + issuer=issuer, + client_id=client_id, + private_pem=private_pem, + jwks_data=jwks_data, + ) + assert "Operators" in group_names, f"Expected Operators, got {group_names}" + assert "Viewers" not in group_names + + @pytest.mark.asyncio + @pytest.mark.integration + async def test_null_default_group_id_falls_back_to_viewers( + self, async_client: AsyncClient, db_session: AsyncSession + ): + """When default_group_id is None, auto-created user falls back to Viewers.""" + private_pem, jwks_data = _make_test_rsa_key() + issuer = "https://dg-null.example" + client_id = "dg-null-client" + admin_token = await _setup_and_login(async_client, "dg_null_adm", "DgNullAdm1!") + + provider_id = await self._create_provider( + async_client, + admin_token, + issuer=issuer, + client_id=client_id, + ) + + group_names = await self._run_autocreate_and_get_groups( + async_client, + db_session, + provider_id=provider_id, + sub="dg-null-sub-1", + issuer=issuer, + client_id=client_id, + private_pem=private_pem, + jwks_data=jwks_data, + ) + assert "Viewers" in group_names, f"Expected Viewers, got {group_names}" + + @pytest.mark.asyncio + @pytest.mark.integration + async def test_dangling_default_group_id_falls_back_to_viewers( + self, async_client: AsyncClient, db_session: AsyncSession + ): + """When configured group is deleted, auto-created user falls back to Viewers. + + SQLite does not enforce FK ON DELETE SET NULL (no PRAGMA foreign_keys=ON), + so provider.default_group_id may point to a deleted group. The runtime + resolution chain must handle this and fall back to Viewers. + """ + from sqlalchemy import delete as sa_delete, select + + from backend.app.models.group import Group + + private_pem, jwks_data = _make_test_rsa_key() + issuer = "https://dg-dangling.example" + client_id = "dg-dangling-client" + admin_token = await _setup_and_login(async_client, "dg_dangle_adm", "DgDangleAdm1!") + + # Create a temporary group and use it as default_group_id + temp_group = Group(name="TempGroup-DgDangle", permissions=[]) + db_session.add(temp_group) + await db_session.commit() + await db_session.refresh(temp_group) + temp_group_id = temp_group.id + + provider_id = await self._create_provider( + async_client, + admin_token, + issuer=issuer, + client_id=client_id, + default_group_id=temp_group_id, + ) + + # Delete the group — simulates dangling FK (especially on SQLite) + await db_session.execute(sa_delete(Group).where(Group.id == temp_group_id)) + await db_session.commit() + + group_names = await self._run_autocreate_and_get_groups( + async_client, + db_session, + provider_id=provider_id, + sub="dg-dangle-sub-1", + issuer=issuer, + client_id=client_id, + private_pem=private_pem, + jwks_data=jwks_data, + ) + assert "Viewers" in group_names, f"Expected Viewers fallback, got {group_names}" + + @pytest.mark.asyncio + @pytest.mark.integration + async def test_administrators_group_can_be_set_as_default( + self, async_client: AsyncClient, db_session: AsyncSession + ): + """Operators can configure Administrators as the default group (e.g. single-tenant IdP).""" + from sqlalchemy import select + + from backend.app.models.group import Group + + private_pem, jwks_data = _make_test_rsa_key() + issuer = "https://dg-admin.example" + client_id = "dg-admin-client" + admin_token = await _setup_and_login(async_client, "dg_admgrp_adm", "DgAdmgrpAdm1!") + + grp_result = await db_session.execute(select(Group).where(Group.name == "Administrators")) + administrators = grp_result.scalar_one() + + provider_id = await self._create_provider( + async_client, + admin_token, + issuer=issuer, + client_id=client_id, + default_group_id=administrators.id, + ) + + group_names = await self._run_autocreate_and_get_groups( + async_client, + db_session, + provider_id=provider_id, + sub="dg-admin-sub-1", + issuer=issuer, + client_id=client_id, + private_pem=private_pem, + jwks_data=jwks_data, + ) + assert "Administrators" in group_names, f"Expected Administrators, got {group_names}" diff --git a/frontend/src/__tests__/components/OIDCProviderSettings.test.tsx b/frontend/src/__tests__/components/OIDCProviderSettings.test.tsx index 38c508879..dee8066dc 100644 --- a/frontend/src/__tests__/components/OIDCProviderSettings.test.tsx +++ b/frontend/src/__tests__/components/OIDCProviderSettings.test.tsx @@ -24,6 +24,7 @@ const mockProviders = [ email_claim: 'email', require_email_verified: true, icon_url: null, + default_group_id: null, created_at: '2026-01-01T00:00:00Z', updated_at: '2026-01-01T00:00:00Z', }, @@ -148,5 +149,81 @@ describe('OIDCProviderSettings', () => { expect(screen.getByText(/Email Claim/i)).toBeInTheDocument(); expect(screen.getByText(/Require Email Verified/i)).toBeInTheDocument(); }); + + it('renders Default Group label in provider details', async () => { + render(); + + await waitFor(() => { + expect(screen.getByText('TestIdP')).toBeInTheDocument(); + }); + + expect(screen.getByText(/Default Group/i)).toBeInTheDocument(); + }); + + it('shows Viewers fallback label when default_group_id is null', async () => { + render(); + + await waitFor(() => { + expect(screen.getByText('TestIdP')).toBeInTheDocument(); + }); + + // null default_group_id should display the Viewers fallback text + expect(screen.getByText(/Viewers.*default/i)).toBeInTheDocument(); + }); + + it('shows group name when default_group_id matches a known group', async () => { + server.use( + http.get('/api/v1/auth/oidc/providers/all', () => + HttpResponse.json([{ ...mockProviders[0], default_group_id: 2 }]) + ) + ); + render(); + + await waitFor(() => { + expect(screen.getByText('TestIdP')).toBeInTheDocument(); + }); + + // default_group_id=2 matches Operators in the global MSW mock + expect(screen.getByText('Operators')).toBeInTheDocument(); + }); + }); + + describe('ProviderForm — default group dropdown', () => { + it('renders a Default Group select in the create form', async () => { + server.use(http.get('/api/v1/auth/oidc/providers/all', () => HttpResponse.json([]))); + render(); + + await waitFor(() => { + expect(screen.getAllByRole('button', { name: /Add Provider/i })[0]).toBeInTheDocument(); + }); + await userEvent.click(screen.getAllByRole('button', { name: /Add Provider/i })[0]); + + await waitFor(() => { + expect(screen.getByText(/Default Group/i)).toBeInTheDocument(); + }); + + // Dropdown should render with Viewers fallback option + const select = screen.getByRole('combobox'); + expect(select).toBeInTheDocument(); + expect(screen.getByText(/Viewers.*default/i)).toBeInTheDocument(); + }); + + it('populates Default Group dropdown with groups from API', async () => { + server.use(http.get('/api/v1/auth/oidc/providers/all', () => HttpResponse.json([]))); + render(); + + await waitFor(() => { + expect(screen.getAllByRole('button', { name: /Add Provider/i })[0]).toBeInTheDocument(); + }); + await userEvent.click(screen.getAllByRole('button', { name: /Add Provider/i })[0]); + + await waitFor(() => { + // Global MSW mock returns Administrators, Operators, Viewers + const options = screen.getAllByRole('option'); + const optionTexts = options.map((o) => o.textContent); + expect(optionTexts).toContain('Operators'); + expect(optionTexts).toContain('Administrators'); + }); + }); }); }); diff --git a/frontend/src/api/client.ts b/frontend/src/api/client.ts index 895241d5c..9fda1dac3 100644 --- a/frontend/src/api/client.ts +++ b/frontend/src/api/client.ts @@ -2682,6 +2682,7 @@ export interface OIDCProvider { email_claim: string; require_email_verified: boolean; icon_url?: string | null; + default_group_id?: number | null; } export interface OIDCProviderCreate { @@ -2696,6 +2697,7 @@ export interface OIDCProviderCreate { email_claim?: string; require_email_verified?: boolean; icon_url?: string | null; + default_group_id?: number | null; } export interface OIDCLink { diff --git a/frontend/src/components/OIDCProviderSettings.tsx b/frontend/src/components/OIDCProviderSettings.tsx index d4f8e1658..0ad9cdb47 100644 --- a/frontend/src/components/OIDCProviderSettings.tsx +++ b/frontend/src/components/OIDCProviderSettings.tsx @@ -3,7 +3,7 @@ import { useQuery, useMutation, useQueryClient } from '@tanstack/react-query'; import { Plus, Edit2, Trash2, Globe, Check, X, RefreshCw, ExternalLink } from 'lucide-react'; import { useTranslation } from 'react-i18next'; import { api } from '../api/client'; -import type { OIDCProvider, OIDCProviderCreate } from '../api/client'; +import type { Group, OIDCProvider, OIDCProviderCreate } from '../api/client'; import { Card, CardContent, CardHeader } from './Card'; import { Button } from './Button'; import { Toggle } from './Toggle'; @@ -22,18 +22,21 @@ const EMPTY_FORM: OIDCProviderCreate = { email_claim: 'email', require_email_verified: true, icon_url: undefined, + default_group_id: null, }; // ─── Provider form (create / edit) ─────────────────────────────────────────── function ProviderForm({ initial, isEdit = false, + groups = [], onSave, onCancel, isPending, }: { initial: OIDCProviderCreate; isEdit?: boolean; + groups?: Group[]; onSave: (data: OIDCProviderCreate) => void; onCancel: () => void; isPending: boolean; @@ -157,6 +160,21 @@ function ProviderForm({ )} +
+ + +

{t('settings.oidc.form.defaultGroupDesc')}

+
+