fix(oidc): Allow auto_link_existing_accounts with custom email claims (Azure Entra ID)

[netscout2001]

Description

Relaxes the SEC-1 guard for auto_link_existing_accounts so that providers using a custom email_claim (e.g. preferred_username, upn) — the recommended Azure Entra ID configuration — can safely enable automatic account linking.

Previously the guard blocked all combinations except email_claim='email' + require_email_verified=True. This also blocked Azure Entra ID configurations that use a custom claim, even though those are not subject to the email-verification bypass attack.

The new guard only blocks the genuinely unsafe combination (Fall B): email_claim='email' + require_email_verified=False. Custom-claim paths (Fall C) never perform an email_verified check, so an attacker-controlled IdP cannot exploit email matching — auto-linking is safe there.

Docs ->

Related Issue

Fixes #1088

Documentation

• Docs PR(s) linked above: infos in CHANGELOG
• No docs update required — reason: internal security guard change with no new config keys or user-visible behavior change beyond unblocking a previously rejected configuration

Type of Change

• Bug fix (non-breaking change that fixes an issue)
• New feature (non-breaking change that adds functionality)
• Breaking change (fix or feature that would cause existing functionality to change)
• Documentation update
• Code refactoring
• Performance improvement
• Test addition or update

Changes Made

• models/oidc_provider.py: Updated CheckConstraint formula from require_email_verified = TRUE AND email_claim = 'email' to email_claim != 'email' OR require_email_verified = TRUE (only blocks Fall B)
• schemas/auth.py: Updated OIDCProviderCreate and OIDCProviderUpdate model validators and error message to match the new guard condition
• api/routes/mfa.py: Updated _enforce_auto_link_safety() combined-state guard to the same Fall-B-only condition
• core/database.py: Added _migrate_update_auto_link_constraint() to update the DB constraint on existing installations — SQLite via table recreation (shadow table pattern), PostgreSQL via DROP CONSTRAINT IF EXISTS + ADD CONSTRAINT; narrowed backfill SQL to only reset unsafe Fall B rows
• tests/integration/test_mfa_api.py: Updated 4 tests and added 2 new tests covering the newly allowed Fall C scenarios and continued blocking of Fall B
• tests/unit/test_db_dialect.py: Updated 2 unit tests and added 4 new tests for the new constraint formula and the SQLite migration function

Testing

• I have tested this on my local machine
• I have tested with my printer model:

163 relevant tests pass (tests/integration/test_mfa_api.py + tests/unit/test_db_dialect.py). Full test suite: no regressions.

Key scenarios verified:

Scenario	email_claim	require_email_verified	auto_link	Result
Case A (standard)	email	True	True	✅ Allowed
Case B (unsafe)	email	False	True	❌ Blocked (422)
Case C (Azure)	preferred_username	any	True	✅ Allowed
Case C (Azure)	upn	False	True	✅ Allowed

Additional Notes

Migration behavior:

• New installations: The updated CHECK constraint is applied at schema creation time — no action needed.
• Existing SQLite installations: _migrate_update_auto_link_constraint() recreates the oidc_providers table with the new constraint (SQLite does not support ALTER TABLE DROP/ADD CONSTRAINT). The old constraint is detected via sqlite_master; the migration is a no-op if already on the new formula.
• Existing PostgreSQL installations: The function issues DROP CONSTRAINT IF EXISTS + ADD CONSTRAINT — fully idempotent.
• Only Fall B rows (email_claim='email' + require_email_verified=False + auto_link=True) are reset by the backfill; Fall C rows are left unchanged.

[maziggy]

Security model — sound, but the precondition needs to be documented

The relaxation is correct for the stated case. SEC-1's attack vector is "IdP doesn't verify email but Bambuddy auto-links anyway" — that path only opens when
email_claim='email' because that's the only branch that consults email_verified. _resolve_provider_email Fall C at mfa.py:432 already skips the verification step entirely
for custom claims, so the constraint relaxation removes a guard that was protecting nothing in that branch.

But the safety claim ("Fall C is safe") quietly assumes the custom claim is tenant-administered — Azure Entra ID upn / preferred_username on a corporate tenant, where the admin sets the value and users can't change it. For an IdP that lets end users self-assert preferred_username (some Keycloak setups, custom OIDC servers, certain Authentik realm configurations), an attacker can register an account with preferred_username=alice@example.com and auto-link to a local Alice's account. That's the same SEC-1 takeover the original guard was designed to prevent — just via a different claim.

The PR description checks "No docs update required". That's not quite accurate — this is a security-model relaxation with a precondition admins need to know about. Could you
add a short note in the OIDC settings docs / wiki page along the lines of:

▎ Custom email claims are safe to auto-link when the claim is tenant-administered (e.g. Azure Entra ID upn / preferred_username). If your IdP lets end users self-assert this
▎ claim's value, do not enable Auto-link.

Bambuddy's convention is that security-affecting changes ship with corresponding doc updates in the same PR.

Code correctness — five enforcement layers, all consistent

Confirmed all five sites enforce the same condition (block iff auto_link AND email_claim='email' AND NOT require_email_verified):

• models/oidc_provider.py CheckConstraint
• schemas/auth.py OIDCProviderCreate.check_auto_link_requires_verified
• schemas/auth.py OIDCProviderUpdate.check_auto_link_requires_verified (pessimistic when email_claim is None — comment correctly defers the existing-Fall-C-with-partial-update case to the route guard, which sees the merged state)
• api/routes/mfa.py _enforce_auto_link_safety (runs after the setattr loop)
• DB CHECK constraint string

The unit tests pin the SQL string directly — good defence against the four staying in sync as the code evolves.

Migration — idempotent, one minor nit

• PostgreSQL: DROP CONSTRAINT IF EXISTS + ADD CONSTRAINT via _safe_execute. Fully idempotent.
• SQLite: detects old formula via sqlite_master, table-recreate inside begin_nested for atomicity, no-op when already on the new formula. The pre-SEC-1 case (no constraint
  at all) is documented as "skipped — app-level guards suffice", which is fine.
• The migration runs after the existing SEC-1 setup which now uses the new formula — on a fresh PG install you ADD the constraint, then DROP + ADD the same constraint.
  Wasteful but idempotent.

Nit: the SQLite recreate path doesn't SELECT count(*) before DROP TABLE oidc_providers to verify the copy landed. Low risk because of the savepoint, but a one-line guard wouldn't hurt. Not blocking.

Tests — cover the relaxation but missing one

The schema, guard, constraint, and SQLite migration layers all have coverage. What's missing: an end-to-end OIDC callback test confirming that a user with claims =
{"preferred_username": "alice@example.com"} actually links to a local alice@example.com account when auto_link=True + email_claim='preferred_username' is configured. Without that, a regression in _resolve_provider_email's Fall C path wouldn't be caught — the existing tests pin the configuration layer, not the actual link behaviour the linked issue (#1088) is asking for.

Minor

• OIDCProviderSettings.tsx adds w-full to two labels. Unrelated to the auto_link change. Either drop the change or note in the description what UX issue it's fixing.
• No CHANGELOG entry — Bambuddy's convention is to add one for any user-visible / config-affecting change. This one needs a brief ### Fixed entry under the unreleased block, mentioning the linked issue (#1088) and that custom email claims are now allowed for auto-link.

[netscout2001]

Thanks for the detailed review! I've addressed all four points:

---

1. Security note for custom email claims

"Custom claims are only safe when the value is tenant-administered (e.g. Azure Entra ID). Add a documentation note."

Added an inline warning in the OIDC settings form that appears whenever auto_link_existing_accounts is enabled and the email claim is not the default email:

"Custom claims are safe for auto-link only when the value is tenant-administered (e.g. Azure Entra ID upn / preferred_username). Do not enable auto-link if your IdP allows users to self-assert this claim."

The warning is rendered in amber in the UI and is localised across all 8 locale files. New i18n key: settings.oidc.form.emailClaimCustomClaimAutoLinkWarning.

---

2. SQLite count guard before DROP TABLE

"Not blocking, but: add a row-count comparison before DROP TABLE in _migrate_update_auto_link_constraint so a partial INSERT is caught before data loss."

Done. Between the INSERT INTO oidc_providers_v2 … SELECT … FROM oidc_providers and the DROP TABLE oidc_providers, a count guard now compares both tables and raises RuntimeError on mismatch:

original = (await conn.execute(text("SELECT count(*) FROM oidc_providers"))).scalar_one()
copied   = (await conn.execute(text("SELECT count(*) FROM oidc_providers_v2"))).scalar_one()
if copied != original:
    raise RuntimeError(
        f"auto_link constraint migration: row count mismatch after copy "
        f"({original} in source, {copied} in copy)"
    )

Covered by a new unit test test_constraint_migration_sqlite_count_guard_raises_on_mismatch in TestAutoLinkConstraintMigration (backend/tests/unit/test_db_dialect.py) that injects a mismatched count and asserts the RuntimeError is raised.

---

3. E2E integration test for Fall C (custom claim auto-link)

"Add an integration test confirming that a custom claim (e.g. preferred_username) actually links an existing local user through the full OIDC callback flow."

New test class TestOIDCFallCAutoLinkE2E in backend/tests/integration/test_mfa_api.py:

• Creates a local user fallc_alice with email = alice.fallc@example.com
• Configures an OIDCProvider with email_claim="preferred_username", auto_link_existing_accounts=True, require_email_verified=False
• Mocks the OIDC discovery, JWKS, and token exchange endpoints
• Issues fake_claims with preferred_username: "alice.fallc@example.com" and no email_verified key (verifying the Fall C path skips the verified-email gate)
• Asserts: callback → 302 with oidc_token, exchange → 200 with data["user"]["username"] == "fallc_alice", and a UserOIDCLink row with provider_user_id="azure-sub-alice" exists in DB

---

4. CHANGELOG entries

"Add a CHANGELOG entry for the auto-link change. Also note the w-full layout fix."

Two entries added to ### Fixed in [0.2.4b1] - Unreleased:

• OIDC auto_link_existing_accounts now works with custom email claims (Azure Entra ID)
• OIDC settings form: "Require email verified" toggle no longer jumps layout when auto-link is enabled

---

Frontend test

Also added a Vitest test for the new UI warning — shows security warning when auto_link is enabled with a custom email claim — uses fireEvent.change on the email claim input (necessary because onChange has a || 'email' fallback that userEvent.clear would trigger) and asserts /tenant-administered/i appears.