[PR #1103] [MERGED] feat(oidc): Azure Entra ID support — configurable email claim & verification + Remember Me persistent login #1164

New issue

Closed

opened 2026-05-07 00:16:24 +02:00 by BreizhHardware · 0 comments

BreizhHardware commented

2026-05-07 00:16:24 +02:00

Owner

📋 Pull Request Information

Original PR: https://github.com/maziggy/bambuddy/pull/1103
Author: @netscout2001
Created: 4/23/2026
Status: ✅ Merged
Merged: 4/24/2026
Merged by: @maziggy

Base: dev ← Head: feature/2fa-oidc-authentication

📝 Commits (9)

4e54fb1 feat(oidc): add Azure Entra ID support with configurable email claim resolution
904c83e fix(oidc): harden email claim resolution, guards, and test coverage
a9ec562 fix(oidc): apply maintainer review fixes for Azure Entra ID support
144b932 feat(auth): add Remember Me option to login page
3eb68fc fix(db): swallow 'no such column' in _safe_execute for idempotent RENAME COLUMN migrations
f22cd0f fix(oidc): harden _safe_execute, extract email/autolink helpers, add test coverage
029be23 fix(db): restore 'no such column' and 'duplicate column name' to _safe_execute swallow list
6fdee8f Merge branch 'dev' into feature/2fa-oidc-authentication
f0483d8 fix(oidc): polish Remember-Me persistence + email_claim UX + shape regex tests

📊 Changes

25 files changed (+2494 additions, -514 deletions)

View changed files

📝 backend/app/api/routes/mfa.py (+118 -18)
📝 backend/app/core/database.py (+76 -10)
📝 backend/app/models/oidc_provider.py (+24 -1)
📝 backend/app/schemas/auth.py (+56 -3)
📝 backend/tests/integration/test_mfa_api.py (+768 -0)
📝 backend/tests/unit/test_db_dialect.py (+143 -4)
📝 backend/tests/unit/test_mfa_helpers.py (+45 -1)
➕ docs/authentication/entra-id.md (+82 -0)
📝 frontend/src/__tests__/api/client.test.ts (+46 -0)
➕ frontend/src/__tests__/components/OIDCProviderSettings.test.tsx (+151 -0)
📝 frontend/src/__tests__/pages/LoginPage.test.tsx (+328 -1)
📝 frontend/src/api/client.ts (+26 -12)
📝 frontend/src/components/OIDCProviderSettings.tsx (+51 -1)
📝 frontend/src/contexts/AuthContext.tsx (+9 -9)
📝 frontend/src/i18n/locales/de.ts (+8 -0)
📝 frontend/src/i18n/locales/en.ts (+8 -0)
📝 frontend/src/i18n/locales/fr.ts (+8 -0)
📝 frontend/src/i18n/locales/it.ts (+8 -0)
📝 frontend/src/i18n/locales/ja.ts (+8 -0)
📝 frontend/src/i18n/locales/pt-BR.ts (+8 -0)

...and 5 more files

📄 Description

Description

This PR combines two improvements to the OIDC / authentication flow:

1 — Azure Entra ID support + hardening (maintainer review fixes)

Adds Azure Entra ID support to the OIDC provider configuration by introducing two new fields: email_claim and require_email_verified.

Azure Entra ID never includes the email_verified claim in its ID tokens. The previous code dropped the email whenever email_verified was absent (fail-closed), breaking Azure logins. This PR introduces three configurable email resolution paths:

Case A (default): email_claim="email" + require_email_verified=true — standard behaviour, unchanged
Case B: email_claim="email" + require_email_verified=false — permissive mode, accepts email when email_verified is absent
Case C: email_claim != "email" — custom claim (preferred_username, upn), no email_verified check; recommended for Azure Entra ID

auto_link_existing_accounts is blocked at schema level, route level, and DB level when either new field would make auto-linking unsafe (SEC-1/SEC-6).

This PR also addresses the subsequent maintainer review feedback:

Regex hardening: _EMAIL_SHAPE_RE is now precompiled at module level and uses fullmatch() with a stricter pattern that requires a dot in the domain (local@domain.tld) — rejects x@nodot, @domain, x@, etc.
PII removed from logs: The raw email value is no longer logged when the shape check fails; only provider_id, email_claim name, and sub are emitted.
PostgreSQL-compatible CheckConstraint: Literals changed from = 0 / = 1 to = FALSE / = TRUE so the constraint compiles correctly on PostgreSQL.
_safe_execute re-raises on real errors: Non-idempotency errors are now logged with logger.error and re-raised so a failed migration aborts startup loudly instead of silently continuing.
provider_email lowercasing migration: Normalises existing mixed-case UPN values (common with Azure Entra ID) to lowercase for consistent matching.
Azure Entra ID setup guide: New docs/authentication/entra-id.md covering app registration, client secret, issuer URL, BamBuddy provider config, preferred_username vs email claim, and a troubleshooting table.
New tests: Real in-memory SQLite integration tests for _safe_execute reraise/swallow behaviour, provider_email lowercasing migration, and FALSE/TRUE CheckConstraint enforcement.

"Remember Me" checkbox added to the login form (unchecked by default).
When checked, the JWT is written to both sessionStorage and localStorage, so the session survives a tab close / browser restart.
Introduces TokenPersistence = 'session' | 'persistent' type, replacing the previous persist?: boolean parameter throughout client.ts, AuthContext, and LoginPage.
OIDC round-trip support: The preference is saved to sessionStorage as auth_remember_me=1 before the provider redirect and consumed (read + removed atomically) in the OIDC callback useEffect, so the setting carries through the external redirect where React state is lost.
Persistence also threads through the OIDC + 2FA combined flow.
All storage writes are wrapped in try-catch with console.warn to handle private browsing / quota errors gracefully (in-memory token still works for the current tab).
rememberMe i18n key added to all 8 locale files (en, de, fr, it, pt-BR, zh-CN, ja, zh-TW).
Full test coverage: persistence mode tests in client.test.ts; Remember Me checkbox, session/persistent storage, OIDC carry-through, OIDC+2FA carry-through, error cleanup, and negative cases in LoginPage.test.tsx.

Fixes #902 #1088

Documentation

https://github.com/maziggy/bambuddy-wiki/pull/19

Type of Change

Bug fix (non-breaking change that fixes an issue)
New feature (non-breaking change that adds functionality)
Documentation update
Test addition or update

Changes Made

Backend

backend/app/api/routes/mfa.py — precompiled _EMAIL_SHAPE_RE, PII-free log, three-case email resolution (Case A/B/C), _is_valid_email_shaped helper, Combined-State-Guard in PUT handler
backend/app/models/oidc_provider.py — email_claim and require_email_verified ORM fields, CheckConstraint uses FALSE/TRUE
backend/app/core/database.py — column migrations for new fields, PostgreSQL ADD CONSTRAINT migration, _safe_execute logs + re-raises non-idempotency errors, provider_email lowercasing migration
backend/app/schemas/auth.py — new fields on OIDCProviderCreate / OIDCProviderUpdate / OIDCProviderResponse, model_validator for SEC-1/SEC-6 on create and update
backend/tests/integration/test_mfa_api.py — 19 new tests: 13-case parametrized matrix (Case A/B/C), SEC-1/SEC-6 guard tests, Combined-State-Guard tests, response field coverage, no-dot-in-domain matrix case
backend/tests/unit/test_db_dialect.py — real SQLite integration tests for _safe_execute and migrations
docs/authentication/entra-id.md — new Azure Entra ID setup guide

Frontend

frontend/src/api/client.ts — TokenPersistence type, updated setAuthToken signature with try-catch, new fields on OIDCProvider and OIDCProviderCreate interfaces
frontend/src/contexts/AuthContext.tsx — login/loginWithToken accept TokenPersistence
frontend/src/pages/LoginPage.tsx — Remember Me checkbox, consumeSavedRememberMe(), toPersistence() helper, OIDC sessionStorage flag
frontend/src/components/OIDCProviderSettings.tsx — toggle for require_email_verified, input for email_claim, both shown in provider info view
frontend/src/i18n/locales/*.ts — rememberMe key + OIDC settings keys in all 8 locale files
frontend/src/__tests__/api/client.test.ts — 3 new persistence mode tests
frontend/src/__tests__/pages/LoginPage.test.tsx — 9 new Remember Me / OIDC carry-through tests

Screenshots

Testing

I have tested this on my local machine

117 backend integration tests pass (pytest tests/integration/test_mfa_api.py). 1407 frontend tests pass (vitest run). No ruff or TypeScript warnings.

Checklist

My code follows the project's coding style
My changes generate no new warnings
I have tested my changes thoroughly

Additional Notes

Existing OIDC providers and login flows are fully backwards-compatible. Both new features are opt-in by default: require_email_verified=true / email_claim='email' preserves the previous behaviour; the Remember Me checkbox defaults to unchecked.

_{🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.}

## 📋 Pull Request Information **Original PR:** https://github.com/maziggy/bambuddy/pull/1103 **Author:** [@netscout2001](https://github.com/netscout2001) **Created:** 4/23/2026 **Status:** ✅ Merged **Merged:** 4/24/2026 **Merged by:** [@maziggy](https://github.com/maziggy) **Base:** `dev` ← **Head:** `feature/2fa-oidc-authentication` --- ### 📝 Commits (9) - [`4e54fb1`](https://github.com/maziggy/bambuddy/commit/4e54fb17669ddc0bf5e7a53ce62fc808560564eb) feat(oidc): add Azure Entra ID support with configurable email claim resolution - [`904c83e`](https://github.com/maziggy/bambuddy/commit/904c83ea08f2d4ae5beccd2b6e79b329591eb568) fix(oidc): harden email claim resolution, guards, and test coverage - [`a9ec562`](https://github.com/maziggy/bambuddy/commit/a9ec5622a3e5742fd986bfd6df32fff4507a40c3) fix(oidc): apply maintainer review fixes for Azure Entra ID support - [`144b932`](https://github.com/maziggy/bambuddy/commit/144b9321d70272f740ffcb0f293b8e2fa8b676e6) feat(auth): add Remember Me option to login page - [`3eb68fc`](https://github.com/maziggy/bambuddy/commit/3eb68fcf24269ff239dfc2ba0e3781e11364331e) fix(db): swallow 'no such column' in _safe_execute for idempotent RENAME COLUMN migrations - [`f22cd0f`](https://github.com/maziggy/bambuddy/commit/f22cd0f855560f816bb8af0d003e1faecffa202e) fix(oidc): harden _safe_execute, extract email/autolink helpers, add test coverage - [`029be23`](https://github.com/maziggy/bambuddy/commit/029be23e257d162fd010850c65e043c1eb95528c) fix(db): restore 'no such column' and 'duplicate column name' to _safe_execute swallow list - [`6fdee8f`](https://github.com/maziggy/bambuddy/commit/6fdee8fbc3d094aea73fbfb6978560c2e51e88b9) Merge branch 'dev' into feature/2fa-oidc-authentication - [`f0483d8`](https://github.com/maziggy/bambuddy/commit/f0483d8bddb9204ba114c060798942adb224eb97) fix(oidc): polish Remember-Me persistence + email_claim UX + shape regex tests ### 📊 Changes **25 files changed** (+2494 additions, -514 deletions) <details> <summary>View changed files</summary> 📝 `backend/app/api/routes/mfa.py` (+118 -18) 📝 `backend/app/core/database.py` (+76 -10) 📝 `backend/app/models/oidc_provider.py` (+24 -1) 📝 `backend/app/schemas/auth.py` (+56 -3) 📝 `backend/tests/integration/test_mfa_api.py` (+768 -0) 📝 `backend/tests/unit/test_db_dialect.py` (+143 -4) 📝 `backend/tests/unit/test_mfa_helpers.py` (+45 -1) ➕ `docs/authentication/entra-id.md` (+82 -0) 📝 `frontend/src/__tests__/api/client.test.ts` (+46 -0) ➕ `frontend/src/__tests__/components/OIDCProviderSettings.test.tsx` (+151 -0) 📝 `frontend/src/__tests__/pages/LoginPage.test.tsx` (+328 -1) 📝 `frontend/src/api/client.ts` (+26 -12) 📝 `frontend/src/components/OIDCProviderSettings.tsx` (+51 -1) 📝 `frontend/src/contexts/AuthContext.tsx` (+9 -9) 📝 `frontend/src/i18n/locales/de.ts` (+8 -0) 📝 `frontend/src/i18n/locales/en.ts` (+8 -0) 📝 `frontend/src/i18n/locales/fr.ts` (+8 -0) 📝 `frontend/src/i18n/locales/it.ts` (+8 -0) 📝 `frontend/src/i18n/locales/ja.ts` (+8 -0) 📝 `frontend/src/i18n/locales/pt-BR.ts` (+8 -0) _...and 5 more files_ </details> ### 📄 Description ## Description This PR combines two improvements to the OIDC / authentication flow: ### 1 — Azure Entra ID support + hardening (maintainer review fixes) Adds Azure Entra ID support to the OIDC provider configuration by introducing two new fields: `email_claim` and `require_email_verified`. Azure Entra ID never includes the `email_verified` claim in its ID tokens. The previous code dropped the email whenever `email_verified` was absent (fail-closed), breaking Azure logins. This PR introduces three configurable email resolution paths: - **Case A** *(default)*: `email_claim="email"` + `require_email_verified=true` — standard behaviour, unchanged - **Case B**: `email_claim="email"` + `require_email_verified=false` — permissive mode, accepts email when `email_verified` is absent - **Case C**: `email_claim != "email"` — custom claim (`preferred_username`, `upn`), no `email_verified` check; recommended for Azure Entra ID `auto_link_existing_accounts` is blocked at schema level, route level, and DB level when either new field would make auto-linking unsafe (SEC-1/SEC-6). This PR also addresses the subsequent maintainer review feedback: - **Regex hardening**: `_EMAIL_SHAPE_RE` is now precompiled at module level and uses `fullmatch()` with a stricter pattern that requires a dot in the domain (`local@domain.tld`) — rejects `x@nodot`, `@domain`, `x@`, etc. - **PII removed from logs**: The raw email value is no longer logged when the shape check fails; only `provider_id`, `email_claim` name, and `sub` are emitted. - **PostgreSQL-compatible CheckConstraint**: Literals changed from `= 0` / `= 1` to `= FALSE` / `= TRUE` so the constraint compiles correctly on PostgreSQL. - **`_safe_execute` re-raises on real errors**: Non-idempotency errors are now logged with `logger.error` and re-raised so a failed migration aborts startup loudly instead of silently continuing. - **`provider_email` lowercasing migration**: Normalises existing mixed-case UPN values (common with Azure Entra ID) to lowercase for consistent matching. - **Azure Entra ID setup guide**: New `docs/authentication/entra-id.md` covering app registration, client secret, issuer URL, BamBuddy provider config, `preferred_username` vs `email` claim, and a troubleshooting table. - **New tests**: Real in-memory SQLite integration tests for `_safe_execute` reraise/swallow behaviour, `provider_email` lowercasing migration, and `FALSE`/`TRUE` CheckConstraint enforcement. ### 2 — Remember Me persistent login - **"Remember Me" checkbox** added to the login form (unchecked by default). - When checked, the JWT is written to both `sessionStorage` **and** `localStorage`, so the session survives a tab close / browser restart. - Introduces `TokenPersistence = 'session' | 'persistent'` type, replacing the previous `persist?: boolean` parameter throughout `client.ts`, `AuthContext`, and `LoginPage`. - **OIDC round-trip support**: The preference is saved to `sessionStorage` as `auth_remember_me=1` before the provider redirect and consumed (read + removed atomically) in the OIDC callback `useEffect`, so the setting carries through the external redirect where React state is lost. - Persistence also threads through the **OIDC + 2FA** combined flow. - All storage writes are wrapped in `try-catch` with `console.warn` to handle private browsing / quota errors gracefully (in-memory token still works for the current tab). - `rememberMe` i18n key added to all 8 locale files (en, de, fr, it, pt-BR, zh-CN, ja, zh-TW). - Full test coverage: persistence mode tests in `client.test.ts`; Remember Me checkbox, session/persistent storage, OIDC carry-through, OIDC+2FA carry-through, error cleanup, and negative cases in `LoginPage.test.tsx`. ## Related Issues Fixes #902 #1088 ## Documentation [https://github.com/maziggy/bambuddy-wiki/pull/19](https://github.com/maziggy/bambuddy-wiki/pull/19) ## Type of Change - [x] Bug fix (non-breaking change that fixes an issue) - [x] New feature (non-breaking change that adds functionality) - [x] Documentation update - [x] Test addition or update ## Changes Made **Backend** - `backend/app/api/routes/mfa.py` — precompiled `_EMAIL_SHAPE_RE`, PII-free log, three-case email resolution (Case A/B/C), `_is_valid_email_shaped` helper, Combined-State-Guard in PUT handler - `backend/app/models/oidc_provider.py` — `email_claim` and `require_email_verified` ORM fields, `CheckConstraint` uses `FALSE`/`TRUE` - `backend/app/core/database.py` — column migrations for new fields, PostgreSQL `ADD CONSTRAINT` migration, `_safe_execute` logs + re-raises non-idempotency errors, `provider_email` lowercasing migration - `backend/app/schemas/auth.py` — new fields on `OIDCProviderCreate` / `OIDCProviderUpdate` / `OIDCProviderResponse`, `model_validator` for SEC-1/SEC-6 on create and update - `backend/tests/integration/test_mfa_api.py` — 19 new tests: 13-case parametrized matrix (Case A/B/C), SEC-1/SEC-6 guard tests, Combined-State-Guard tests, response field coverage, `no-dot-in-domain` matrix case - `backend/tests/unit/test_db_dialect.py` — real SQLite integration tests for `_safe_execute` and migrations - `docs/authentication/entra-id.md` — new Azure Entra ID setup guide **Frontend** - `frontend/src/api/client.ts` — `TokenPersistence` type, updated `setAuthToken` signature with try-catch, new fields on `OIDCProvider` and `OIDCProviderCreate` interfaces - `frontend/src/contexts/AuthContext.tsx` — `login`/`loginWithToken` accept `TokenPersistence` - `frontend/src/pages/LoginPage.tsx` — Remember Me checkbox, `consumeSavedRememberMe()`, `toPersistence()` helper, OIDC sessionStorage flag - `frontend/src/components/OIDCProviderSettings.tsx` — toggle for `require_email_verified`, input for `email_claim`, both shown in provider info view - `frontend/src/i18n/locales/*.ts` — `rememberMe` key + OIDC settings keys in all 8 locale files - `frontend/src/__tests__/api/client.test.ts` — 3 new persistence mode tests - `frontend/src/__tests__/pages/LoginPage.test.tsx` — 9 new Remember Me / OIDC carry-through tests ## Screenshots <img width="1120" height="880" alt="image" src="https://github.com/user-attachments/assets/7aa12bfb-6ed7-4d2c-b803-06da6f01b217" /> <img width="460" height="716" alt="image" src="https://github.com/user-attachments/assets/68437d8f-887f-4841-9860-a69b7c2fcec1" /> ## Testing - [x] I have tested this on my local machine 117 backend integration tests pass (`pytest tests/integration/test_mfa_api.py`). 1407 frontend tests pass (`vitest run`). No ruff or TypeScript warnings. ## Checklist - [x] My code follows the project's coding style - [x] My changes generate no new warnings - [x] I have tested my changes thoroughly ## Additional Notes Existing OIDC providers and login flows are fully backwards-compatible. Both new features are opt-in by default: `require_email_verified=true` / `email_claim='email'` preserves the previous behaviour; the Remember Me checkbox defaults to unchecked. --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>