Fix #22: OAuth2 sign-up toegestaan via @request.context-rule (eerste Microsoft-login werkt weer) #23

Merged

rve merged 2 commits from fix/issue-22-oauth2-signup-rule into main 2026-07-12 15:11:25 +00:00

Conversation 1 Commits 2 Files Changed 7 +163 -5

rve commented 2026-07-12 13:16:29 +00:00

Owner

Copy Link

Closes #22

Wat dit oplost

Elke eerste Microsoft-login faalde met 403 "Only superusers can perform this action" op auth-with-oauth2. PocketBase past de createRule toe op het automatisch aanmaken van het record tijdens de OAuth2-sign-up; team_members stond op createRule: null (= superuser-only). Omdat de pre-Azure-records bewust gedropt zijn, was íedereen een eerste login → niemand kon erin. Volledige analyse + lokale repro in #22.

De fix

createRule: '@request.context = "oauth2"' — record-creatie mag uitsluitend vanuit de OAuth2-flow. Anonieme REST-creates (waarmee anders vooraf rogue admin-profielen klaargezet zouden kunnen worden) blijven geweigerd; expliciet getest.

• 1781000002_allow_oauth2_signup.js (nieuw): past de rule toe op reeds-gemigreerde omgevingen (Labs), guarded + down-migratie
• 1781000000_team_members_to_auth.js: zelfde rule voor verse omgevingen (bestandsnaam ongewijzigd — ledger-veilig)
• scripts/setup-pb-collections.mjs gespiegeld; docs/auth-spec.md ADR 009

Verificatie (mock-OIDC tegen PocketBase v0.30.4)

#	Scenario	Resultaat
A0	Baseline met oude regel	403 exact gereproduceerd
A1–A4	Upgrade-pad (= Labs): eerste login	200, record met role uit allow-list, enrollment_status=not_started, naam uit OIDC-claim
A5	Tweede login	Zelfde record, geen duplicaat
A6	Anonieme REST-create	Geweigerd (400)
B1–B2	Verse DB + admin-mapping	200, role=admin via ENTRA_ADMIN_EMAILS

Regressie: DoD-harness #18 15/15, npm test 112/112.

Let op bij de merge

⚠️ Deze branch is gestapeld op fix/issue-20-caddyfile-crash (PR #21, nog niet gemerged) — bewust: aftakken van main zou de PR-pipeline het kapotte Caddyfile laten bouwen en Labs opnieuw neerhalen. Deze PR mergen brengt óók de #20-fix binnen (PR #21 wordt daarmee leeg en kan dicht). Volgorde-alternatief: eerst #21 mergen, dan krimpt deze diff automatisch.

Resterende menselijke check na deploy: inloggen met een echt Microsoft-account (E2E uit #18).

Follow-up (bewust buiten scope, zie #22)

TeamManager: updateRule/deleteRule staan rolbeheer door app-admins niet toe (alleen superusers via dashboard) — eigen afweging waard.

🤖 Generated with Claude Code

Closes #22 ## Wat dit oplost Elke **eerste** Microsoft-login faalde met 403 *"Only superusers can perform this action"* op `auth-with-oauth2`. PocketBase past de `createRule` toe op het automatisch aanmaken van het record tijdens de OAuth2-sign-up; `team_members` stond op `createRule: null` (= superuser-only). Omdat de pre-Azure-records bewust gedropt zijn, was íedereen een eerste login → niemand kon erin. Volledige analyse + lokale repro in #22. ## De fix `createRule: '@request.context = "oauth2"'` — record-creatie mag uitsluitend vanuit de OAuth2-flow. Anonieme REST-creates (waarmee anders vooraf rogue admin-profielen klaargezet zouden kunnen worden) blijven geweigerd; expliciet getest. - `1781000002_allow_oauth2_signup.js` (nieuw): past de rule toe op reeds-gemigreerde omgevingen (Labs), guarded + down-migratie - `1781000000_team_members_to_auth.js`: zelfde rule voor verse omgevingen (bestandsnaam ongewijzigd — ledger-veilig) - `scripts/setup-pb-collections.mjs` gespiegeld; `docs/auth-spec.md` ADR 009 ## Verificatie (mock-OIDC tegen PocketBase v0.30.4) | # | Scenario | Resultaat | |---|---|---| | A0 | Baseline met oude regel | **403 exact gereproduceerd** | | A1–A4 | Upgrade-pad (= Labs): eerste login | 200, record met role uit allow-list, `enrollment_status=not_started`, naam uit OIDC-claim | | A5 | Tweede login | Zelfde record, geen duplicaat | | A6 | Anonieme REST-create | Geweigerd (400) | | B1–B2 | Verse DB + admin-mapping | 200, `role=admin` via `ENTRA_ADMIN_EMAILS` | Regressie: DoD-harness #18 **15/15**, `npm test` **112/112**. ## Let op bij de merge ⚠️ **Deze branch is gestapeld op `fix/issue-20-caddyfile-crash` (PR #21, nog niet gemerged)** — bewust: aftakken van main zou de PR-pipeline het kapotte Caddyfile laten bouwen en Labs opnieuw neerhalen. **Deze PR mergen brengt óók de #20-fix binnen** (PR #21 wordt daarmee leeg en kan dicht). Volgorde-alternatief: eerst #21 mergen, dan krimpt deze diff automatisch. Resterende menselijke check na deploy: inloggen met een echt Microsoft-account (E2E uit #18). ## Follow-up (bewust buiten scope, zie #22) TeamManager: `updateRule`/`deleteRule` staan rolbeheer door app-admins niet toe (alleen superusers via dashboard) — eigen afweging waard. 🤖 Generated with [Claude Code](https://claude.com/claude-code)

rve added 2 commits 2026-07-12 13:16:30 +00:00

fix: valid Caddyfile handle_errors + frontend health-gate in deploys (#20) ... 80f738ddcb

The learning-platform (Caddy) container crash-looped since this morning's
89d3395: `Content-Type application/json` is not a valid subdirective of
`respond` — a Caddyfile parse error aborts Caddy at startup, taking the
whole app offline for authenticated users while every CI run stayed green.
Verified with caddy v2.10.0: "unrecognized subdirective 'Content-Type',
at Caddyfile:53"; the fixed file validates clean.

CI was blind to this twice over: test.yml swaps the real Caddyfile for
Caddyfile.test in the test image, and the deploy health-gate from #18
only covers PocketBase.

- Caddyfile: set Content-Type via a `header @api` directive before the
  `respond`; behaviour of the JSON error response is unchanged
- infra/*/site/deploy-playbook.yml: frontend health-gate (docker exec
  wget --spider on the container) with log dump + abort on failure, and
  an always-on post-deploy smoke report (compose ps, proxy health,
  team_members auth-methods, PocketBase log tail) for visibility behind
  the auth perimeter

Closes #20

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

fix: allow OAuth2 sign-up on team_members via @request.context rule (#22) ... 897b46d4a1

Every first Microsoft login failed with 403 "Only superusers can perform
this action": PocketBase applies the collection createRule to the
automatic record creation during OAuth2 sign-up, and team_members was
created with createRule null (superuser-only) on the wrong assumption
that the OAuth2 flow bypasses it. Since the pre-Azure records were
deliberately dropped, every user was a first login and nobody could get
in. Reproduced locally against a mock OIDC provider (PocketBase v0.30.4).

createRule becomes '@request.context = "oauth2"': record creation is
allowed exclusively from within the OAuth2 flow. Anonymous REST creates
(which could otherwise pre-seed rogue admin profiles) remain rejected —
covered by an explicit test.

- pb_migrations/1781000002_allow_oauth2_signup.js: applies the rule on
  already-migrated environments (Labs); idempotent, guarded, with down
- pb_migrations/1781000000_team_members_to_auth.js: same rule for fresh
  environments (filename unchanged — ledger-safe)
- scripts/setup-pb-collections.mjs: fallback entry mirrored
- docs/auth-spec.md: ADR 009 + files table

Verified with a mock-OIDC matrix (9/9): baseline reproduces the 403 on
the old rule; upgrade path heals (first login 200, role/enrollment/name
defaults, second login no duplicate, anonymous create still rejected);
fresh chain + admin allow-list mapping OK. DoD harness regression 15/15,
npm test 112/112.

Closes #22

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

rve commented 2026-07-12 13:22:31 +00:00

Author

Owner

Copy Link

Live-rollout geslaagd (run #93): deploy-dev groen, PLAY RECAP failed=0, beide gates gepasseerd, migratie 1781000002 toegepast bij boot (PB start anders niet), proxy-keten 200, OIDC-provider aanwezig, reconciler in-sync.

Lokale bewijsmatrix (mock-OIDC, v0.30.4): 9/9 — incl. baseline die de productie-403 exact reproduceert op de oude regel, en de check dat anonieme REST-creates geweigerd blijven. Regressie: #18-harness 15/15, npm test 112/112.

Laatste check (mens): log in met een Microsoft-account op Labs — de eerste login hoort nu een team_members-record aan te maken (rol via ENTRA_ADMIN_EMAILS, onboarding-status not_started).

Merge-volgorde: deze branch bevat de #20-commits (PR #21) — deze PR mergen sluit beide gaten in één keer; PR #21 kan daarna dicht.

**Live-rollout geslaagd (run #93):** deploy-dev groen, PLAY RECAP `failed=0`, beide gates gepasseerd, migratie `1781000002` toegepast bij boot (PB start anders niet), proxy-keten 200, OIDC-provider aanwezig, reconciler `in-sync`. Lokale bewijsmatrix (mock-OIDC, v0.30.4): **9/9** — incl. baseline die de productie-403 exact reproduceert op de oude regel, en de check dat anonieme REST-creates geweigerd blijven. Regressie: #18-harness 15/15, `npm test` 112/112. **Laatste check (mens):** log in met een Microsoft-account op Labs — de eerste login hoort nu een `team_members`-record aan te maken (rol via `ENTRA_ADMIN_EMAILS`, onboarding-status `not_started`). Merge-volgorde: deze branch bevat de #20-commits (PR #21) — deze PR mergen sluit beide gaten in één keer; PR #21 kan daarna dicht.

rve merged commit 8decdd454f into main 2026-07-12 15:11:25 +00:00

rve referenced this issue from a commit 2026-07-12 15:11:25 +00:00

Merge pull request 'Fix #22: OAuth2 sign-up toegestaan via @request.context-rule (eerste Microsoft-login werkt weer)' (#23) from fix/issue-22-oauth2-signup-rule into main

rve referenced this pull request 2026-07-12 15:35:03 +00:00

Microsoft-login faalt met 400 "email: Cannot be blank" — Graph userinfo levert geen e-mailclaim voor accounts zonder mail-attribuut #24

rve referenced this issue from a commit 2026-07-12 17:26:46 +00:00

fix: read Entra claims from id_token + UPN e-mail fallback (#24)

rve referenced this pull request 2026-07-12 17:27:13 +00:00

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

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

Subscribe

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: rve/learning-platform#23