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

Fix #27: TeamManager-rosterbeheer, role-escalatie gedicht, escalate-only resync #29

Merged
rve merged 1 commits from fix/issue-27-teammanager-admin-rules into main 2026-07-12 19:04:44 +00:00
Conversation 0 Commits 1 Files Changed 6 +79 -14
rve commented 2026-07-12 19:02:17 +00:00
Owner
Copy Link

Closes #27

Wat dit oplost

  1. TeamManager werkt nu voor app-admins: updateRule/deleteRule stonden rosterbeheer niet toe (alleen self-update / superuser-delete).
  2. Security: self-service privilege-escalatie gedicht — de oude self-update-rule beperkte geen velden, dus elke gebruiker kon zijn eigen role naar "admin" patchen. Nieuwe rule: self-update mag álles behalve role (@request.body.role:isset = false); alleen admins beheren rollen en verwijderen leden.
  3. Rol-resync is escalate-only: de ENTRA_ADMIN_EMAILS-allow-list garandeert admin bij elke login, maar demoteert niet meer — anders zou elke UI-promotie bij de volgende login worden teruggedraaid (ADR-004 geamendeerd). Demotie via TeamManager; allow-list-leden demoteer je door ze van de lijst te halen.

Uitgerold via migratie 1781000003 (Labs) + basis-migratie en setup-script gespiegeld; UI-toelichting bijgewerkt.

Verificatie (mock-OIDC met twee identiteiten, PocketBase v0.30.4) — 11/11

Check Resultaat
Allow-list login → admin; gewone login → user ✅
Self-escalatie role→admin geblokkeerd; onboarding-self-update werkt ✅
User kan andermans record niet wijzigen/verwijderen ✅
Admin: promoveren / demoteren / verwijderen ✅
Promotie overleeft de volgende login van het lid (escalate-only) ✅
Allow-list-admin blijft admin ✅

Regressie: #22-matrix 9/9, #24-matrix 8/8, #18-harness 15/15, npm test 112/112, eslint schoon.

Stapeling: bouwt op de #26-branch (die op #25). Deze PR mergen brengt álles binnen (#24/#26/#27); #25 en de #26-PR worden dan leeg en kunnen dicht.

🤖 Generated with Claude Code

Closes #27 ## Wat dit oplost 1. **TeamManager werkt nu voor app-admins**: `updateRule`/`deleteRule` stonden rosterbeheer niet toe (alleen self-update / superuser-delete). 2. **Security: self-service privilege-escalatie gedicht** — de oude self-update-rule beperkte geen velden, dus elke gebruiker kon zijn eigen `role` naar `"admin"` patchen. Nieuwe rule: self-update mag álles behalve `role` (`@request.body.role:isset = false`); alleen admins beheren rollen en verwijderen leden. 3. **Rol-resync is escalate-only**: de `ENTRA_ADMIN_EMAILS`-allow-list garandeert admin bij elke login, maar demoteert niet meer — anders zou elke UI-promotie bij de volgende login worden teruggedraaid (ADR-004 geamendeerd). Demotie via TeamManager; allow-list-leden demoteer je door ze van de lijst te halen. Uitgerold via migratie `1781000003` (Labs) + basis-migratie en setup-script gespiegeld; UI-toelichting bijgewerkt. ## Verificatie (mock-OIDC met twee identiteiten, PocketBase v0.30.4) — 11/11 | Check | Resultaat | |---|---| | Allow-list login → admin; gewone login → user | ✅ | | Self-escalatie `role→admin` geblokkeerd; onboarding-self-update werkt | ✅ | | User kan andermans record niet wijzigen/verwijderen | ✅ | | Admin: promoveren / demoteren / verwijderen | ✅ | | **Promotie overleeft de volgende login van het lid** (escalate-only) | ✅ | | Allow-list-admin blijft admin | ✅ | Regressie: #22-matrix 9/9, #24-matrix 8/8, #18-harness 15/15, `npm test` 112/112, eslint schoon. **Stapeling:** bouwt op de #26-branch (die op #25). Deze PR mergen brengt álles binnen (#24/#26/#27); #25 en de #26-PR worden dan leeg en kunnen dicht. 🤖 Generated with [Claude Code](https://claude.com/claude-code)
rve added 3 commits 2026-07-12 19:02:17 +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>
ci: validate both Caddyfiles in the test job (#26)
Some checks failed
On Pull Request to Main / test (pull_request) Failing after 26s
Details
On Pull Request to Main / publish (pull_request) Has been skipped
Details
On Pull Request to Main / deploy-dev (pull_request) Has been skipped
Details
6bf8bad5fb 
The test image swaps in Caddyfile.test, so the production Caddyfile was
never exercised by CI: the parse error from issue #20 sailed through
every check and only surfaced when the deploy gate failed — after the
broken container had already replaced the healthy one. Running
`caddy validate` (same caddy:2-alpine base as the Dockerfile) on both
files up front fails the PR check in seconds instead.

Touches the frozen .github/workflows/ per explicit product-owner
approval in #26.

Closes #26

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
fix: TeamManager admin rules, close role self-escalation, escalate-only resync (#27)
Some checks failed
On Pull Request to Main / test (pull_request) Failing after 24s
Details
On Pull Request to Main / publish (pull_request) Has been skipped
Details
On Pull Request to Main / deploy-dev (pull_request) Has been skipped
Details
cbce4555ff 
TeamManager could not manage the roster: updateRule allowed self-update
only and deleteRule was superuser-only. The same self-update rule also
left a privilege escalation open — it did not restrict fields, so any
authenticated user could PATCH their own role to "admin".

- team_members rules (migration 1781000003 + base migration + setup
  script mirror):
    update: (@request.auth.id = id && @request.body.role:isset = false)
            || @request.auth.role = "admin"
    delete: @request.auth.role = "admin"
  Self-service onboarding keeps working (it never touches role); the
  role field is admin-only; deletion is admin-only.
- pb_hooks/team_members.pb.js: the ENTRA_ADMIN_EMAILS resync is now
  escalate-only — it guarantees admin for allow-list members on every
  login but no longer demotes, otherwise every TeamManager promotion
  would revert on the member's next sign-in (ADR-004 amended).
- TeamManager.jsx: info text updated to the new behaviour.

Verified with a two-identity mock-OIDC matrix (11/11): allow-list vs
regular login, self-escalation blocked while onboarding self-update
still works, cross-user update/delete blocked, admin promote/demote/
delete work, promotion survives the member's next login, allow-list
admin stays admin. Regression: #22 matrix 9/9, #24 matrix 8/8, #18
harness 15/15, npm test 112/112, eslint clean.

Closes #27

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
rve merged commit 48715df147 into main 2026-07-12 19:04:44 +00:00
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#29
Delete Branch "fix/issue-27-teammanager-admin-rules"

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?