rve/learning-platform
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Fix #24: Entra-claims uit id_token + UPN-fallback voor ontbrekende e-mailclaim #25

Closed
rve wants to merge 0 commits from fix/issue-24-missing-email-claim into main
pull from: fix/issue-24-missing-email-claim
merge into: rve:main
Conversation 1 Commits - Files Changed -
rve commented 2026-07-12 17:27:13 +00:00
Owner
Copy Link

Closes #24

Wat dit oplost

Eerste Microsoft-logins faalden met 400 — email: Cannot be blank: Graph's /oidc/userinfo geeft géén email-claim voor accounts zonder mail-attribuut, terwijl team_members een e-mail vereist (die ook de ENTRA_ADMIN_EMAILS-rolmapping voedt). De door Raymond aangeleverde response-body matchte exact de "missing email"-fingerprint uit de analyse (#24).

De fix (twee lagen)

  1. Claims uit het Entra id_token (userInfoURL: "" in de reconciler + migratie-fast-path). Het id_token bevat áltijd preferred_username (UPN) en email wanneer beschikbaar. De #18-reconciler flipt de al-gedeployede Labs-provider automatisch bij de eerstvolgende boot — geen migratie nodig.
  2. UPN-fallback in onRecordAuthWithOAuth2Request: geen e-mailclaim → lowercased UPN als e-mail. Guest-UPN's (ext_user#EXT#@tenant...) worden expliciet uitgesloten — # is RFC-geldig in een e-mail-localpart, dus zowel een naïeve regex als PocketBase's eigen validatie accepteren ze (gevangen door test V5). De hook logt bovendien elke gefaalde OAuth2-poging mét onderliggende fout naar de containerlog.

Verificatie (mock-Entra met schakelbare claims, PocketBase v0.30.4)

# Scenario Resultaat
V1–V2 id_token zónder e-mailclaim 200 — e-mail uit UPN (lowercased), role=admin via allow-list
V3 Tweede login Zelfde record, geen duplicaat
V4 id_token mét e-mailclaim Claim wint, geen fallback
V5 Guest-UPN (#EXT#) Nette validatiefout, geen rommel-e-mail
V6 Anonieme REST-create Blijft geweigerd
V7–V8 Observability Fallback én gefaalde pogingen zichtbaar in containerlog

Regressie: #22-matrix 9/9 (baseline gepind op 1a1351d nu #23 gemerged is), #18-harness 15/15, npm test 112/112.

Na de merge/deploy: de smoke-report toont de reconciler-update (updated bij eerste boot — userInfoURL flipt), daarna E2E-login door Raymond.

🤖 Generated with Claude Code

Closes #24 ## Wat dit oplost Eerste Microsoft-logins faalden met `400 — email: Cannot be blank`: Graph's `/oidc/userinfo` geeft géén `email`-claim voor accounts zonder mail-attribuut, terwijl `team_members` een e-mail vereist (die ook de `ENTRA_ADMIN_EMAILS`-rolmapping voedt). De door Raymond aangeleverde response-body matchte exact de "missing email"-fingerprint uit de analyse (#24). ## De fix (twee lagen) 1. **Claims uit het Entra id_token** (`userInfoURL: ""` in de reconciler + migratie-fast-path). Het id_token bevat áltijd `preferred_username` (UPN) en `email` wanneer beschikbaar. De #18-reconciler flipt de al-gedeployede Labs-provider automatisch bij de eerstvolgende boot — geen migratie nodig. 2. **UPN-fallback in `onRecordAuthWithOAuth2Request`**: geen e-mailclaim → lowercased UPN als e-mail. Guest-UPN's (`ext_user#EXT#@tenant...`) worden expliciet uitgesloten — `#` is RFC-geldig in een e-mail-localpart, dus zowel een naïeve regex als PocketBase's eigen validatie accepteren ze (gevangen door test V5). De hook logt bovendien elke gefaalde OAuth2-poging mét onderliggende fout naar de containerlog. ## Verificatie (mock-Entra met schakelbare claims, PocketBase v0.30.4) | # | Scenario | Resultaat | |---|---|---| | V1–V2 | id_token zónder e-mailclaim | 200 — e-mail uit UPN (lowercased), `role=admin` via allow-list | | V3 | Tweede login | Zelfde record, geen duplicaat | | V4 | id_token mét e-mailclaim | Claim wint, geen fallback | | V5 | Guest-UPN (`#EXT#`) | Nette validatiefout, geen rommel-e-mail | | V6 | Anonieme REST-create | Blijft geweigerd | | V7–V8 | Observability | Fallback én gefaalde pogingen zichtbaar in containerlog | Regressie: #22-matrix **9/9** (baseline gepind op `1a1351d` nu #23 gemerged is), #18-harness **15/15**, `npm test` **112/112**. Na de merge/deploy: de smoke-report toont de reconciler-update (`updated` bij eerste boot — userInfoURL flipt), daarna E2E-login door Raymond. 🤖 Generated with [Claude Code](https://claude.com/claude-code)
rve added 1 commit 2026-07-12 17:27:14 +00:00
fix: read Entra claims from id_token + UPN e-mail fallback (#24)
All checks were successful
On Pull Request to Main / test (pull_request) Successful in 38s
Details
On Pull Request to Main / publish (pull_request) Successful in 1m10s
Details
On Pull Request to Main / deploy-dev (pull_request) Successful in 3m7s
Details
54137122a7 
First Microsoft logins failed with 400 "email: Cannot be blank": Graph's
/oidc/userinfo endpoint omits the email claim for accounts without a
mail attribute, and team_members requires an e-mail (it also drives the
ENTRA_ADMIN_EMAILS role mapping). Reproduced against a mock Entra with
PocketBase v0.30.4; the user-supplied response body matched the
missing-email fingerprint exactly.

- provider config (reconciler + migration fast-path): userInfoURL is now
  empty, so PocketBase reads the claims from the Entra id_token, which
  always carries preferred_username (UPN) and email when available. The
  #18 reconciler flips the already-deployed Labs provider automatically
  on the next boot — no migration needed.
- pb_hooks/entra_oidc.pb.js: onRecordAuthWithOAuth2Request hook falls
  back to the lowercased UPN when the email claim is absent. Guest UPNs
  (ext_user#EXT#@tenant...) are excluded explicitly — "#" is RFC-valid
  in an e-mail local part, so both a naive regex AND PocketBase's own
  validation would accept them (caught by test V5). The hook also logs
  every failed OAuth2 attempt with the underlying error to the container
  log, which PocketBase otherwise only writes to its internal logs db.

Verified with a switchable mock-Entra matrix (8/8): no-email→UPN e-mail
+ allow-list role, email claim wins when present, no duplicate on
re-login, guest UPN yields a clean validation error, anonymous REST
create stays rejected, both log lines present. Regression: issue-22
matrix 9/9 (baseline pinned to 1a1351d now that #23 is merged), DoD
harness 15/15, npm test 112/112.

Closes #24

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
rve commented 2026-07-12 18:46:31 +00:00
Author
Owner
Copy Link

Live-rollout geslaagd (run #95): deploy-dev groen, PLAY RECAP failed=0, beide gates gepasseerd. Smoke-report: entra_oidc reconcile (bootstrap): in-sync — en omdat de reconciler-vergelijking userInfoURL omvat, bewijst dit dat de live provider nu op het id_token-pad staat (de flip is door de hook-hot-reload + cron al vóór de recreate uitgevoerd).

Lokale bewijsmatrix: 8/8 (UPN-fallback, claim-wint, geen duplicaat, guest-UPN nette fout, REST-create dicht, beide loglijnen). Regressie: #22-matrix 9/9, #18-harness 15/15, npm test 112/112.

Klaar voor de E2E: Microsoft-login op Labs zou nu moeten slagen — ook voor accounts zónder mail-attribuut (e-mail wordt dan de UPN).

**Live-rollout geslaagd (run #95):** deploy-dev groen, PLAY RECAP `failed=0`, beide gates gepasseerd. Smoke-report: `entra_oidc reconcile (bootstrap): in-sync` — en omdat de reconciler-vergelijking `userInfoURL` omvat, bewijst dit dat de live provider nu op het id_token-pad staat (de flip is door de hook-hot-reload + cron al vóór de recreate uitgevoerd). Lokale bewijsmatrix: **8/8** (UPN-fallback, claim-wint, geen duplicaat, guest-UPN nette fout, REST-create dicht, beide loglijnen). Regressie: #22-matrix 9/9, #18-harness 15/15, `npm test` 112/112. **Klaar voor de E2E:** Microsoft-login op Labs zou nu moeten slagen — ook voor accounts zónder mail-attribuut (e-mail wordt dan de UPN).
rve closed this pull request 2026-07-12 19:05:00 +00:00
All checks were successful
On Pull Request to Main / test (pull_request) Successful in 38s
Details
On Pull Request to Main / publish (pull_request) Successful in 1m10s
Details
On Pull Request to Main / deploy-dev (pull_request) Successful in 3m7s
Details

Pull request closed

Please reopen this pull request to perform a merge.
Sign in to join this conversation.
Reviewers
No Reviewers
Labels
No items
No Label
Milestone
No items
No Milestone
Projects
Clear projects
No project
Assignees
Clear assignees
eho lmi lpr not rdi rve
No Assignees
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: rve/learning-platform#25
Delete Branch "fix/issue-24-missing-email-claim"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?