Compare commits

...

2 Commits

Author SHA1 Message Date
48715df147 Merge pull request 'Fix #27: TeamManager-rosterbeheer, role-escalatie gedicht, escalate-only resync' (#29) from fix/issue-27-teammanager-admin-rules into main
Some checks failed
On Push to Main / test (push) Failing after 25s
On Push to Main / publish (push) Has been skipped
On Push to Main / deploy-dev (push) Has been skipped
Reviewed-on: #29
2026-07-12 19:04:43 +00:00
RaymondVerhoef
cbce4555ff 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
On Pull Request to Main / publish (pull_request) Has been skipped
On Pull Request to Main / deploy-dev (pull_request) Has been skipped
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>
2026-07-12 21:01:28 +02:00
6 changed files with 79 additions and 14 deletions

View File

@@ -39,13 +39,14 @@ Browser (SPA) PocketBase (auth) Entra ID
| 001 | OIDC-provider tegen Entra v2-endpoints i.p.v. eigen MSAL-flow of header-trust | Sluit aan op "PocketBase = enige backend"; secret server-side; auto-provisioning gratis; werkt los van de perimeter-implementatie | | 001 | OIDC-provider tegen Entra v2-endpoints i.p.v. eigen MSAL-flow of header-trust | Sluit aan op "PocketBase = enige backend"; secret server-side; auto-provisioning gratis; werkt los van de perimeter-implementatie |
| 002 | `team_members` hergebruikt als auth-collection (zelfde naam) | Alle `user_id`-referenties blijven werken; minimale blast-radius | | 002 | `team_members` hergebruikt als auth-collection (zelfde naam) | Alle `user_id`-referenties blijven werken; minimale blast-radius |
| 003 | Provisioning + admin-rol via `pb_hooks/team_members.pb.js` | Rol-logica server-side, niet in de client | | 003 | Provisioning + admin-rol via `pb_hooks/team_members.pb.js` | Rol-logica server-side, niet in de client |
| 004 | Admin-rol via e-mail-allowlist (`ENTRA_ADMIN_EMAILS`), re-synced elke login | Geen handmatig beheer; allowlist-wijziging werkt bij volgende login | | 004 | Admin-rol via e-mail-allowlist (`ENTRA_ADMIN_EMAILS`), **escalate-only** bij elke login (aangepast in #27) | Allow-list garandeert admin (promotie werkt bij volgende login) maar demoteert niet meer — anders zou elke TeamManager-promotie bij de volgende login worden teruggedraaid. Demotie via TeamManager; allow-list-leden demoteer je door ze van de lijst te halen |
| 005 | API-rules → `@request.auth.id != ""` op alle app-collecties | Geen anonieme toegang meer; admins/users zijn beide geauthenticeerd | | 005 | API-rules → `@request.auth.id != ""` op alle app-collecties | Geen anonieme toegang meer; admins/users zijn beide geauthenticeerd |
| 006 | Baseline-ledger-sync migratie voor out-of-band geprovisionede DB's | Labs/prod draaide historisch zonder `--migrationsDir`; replay van de historie crashte PB (issue #18). De vroegst-sorterende migratie markeert de historie als applied wanneer schema bestaat maar de ledger leeg is | | 006 | Baseline-ledger-sync migratie voor out-of-band geprovisionede DB's | Labs/prod draaide historisch zonder `--migrationsDir`; replay van de historie crashte PB (issue #18). De vroegst-sorterende migratie markeert de historie als applied wanneer schema bestaat maar de ledger leeg is |
| 007 | Migratie = structuur, hook = configuratie | De OIDC-provider wordt bij elke start (en per cron-minuut) gereconcilieerd vanuit `ENTRA_*` env door `pb_hooks/entra_oidc.pb.js`; een one-shot migratie die van deploy-time env afhangt kan OAuth2 anders permanent uitschakelen | | 007 | Migratie = structuur, hook = configuratie | De OIDC-provider wordt bij elke start (en per cron-minuut) gereconcilieerd vanuit `ENTRA_*` env door `pb_hooks/entra_oidc.pb.js`; een one-shot migratie die van deploy-time env afhangt kan OAuth2 anders permanent uitschakelen |
| 008 | Hook-helpers via `require(`${__hooks}/utils.js`)` | De JSVM draait elke hook-callback als geïsoleerd programma; top-level functies zijn níet in scope op call-time | | 008 | Hook-helpers via `require(`${__hooks}/utils.js`)` | De JSVM draait elke hook-callback als geïsoleerd programma; top-level functies zijn níet in scope op call-time |
| 009 | `createRule: '@request.context = "oauth2"'` op `team_members` | PocketBase past de createRule toe op de OAuth2-sign-up (eerste login maakt het record aan); `null` blokkeerde élke eerste login met 403 (issue #22). De context-rule staat alléén de OAuth2-flow toe — anonieme REST-creates blijven geweigerd | | 009 | `createRule: '@request.context = "oauth2"'` op `team_members` | PocketBase past de createRule toe op de OAuth2-sign-up (eerste login maakt het record aan); `null` blokkeerde élke eerste login met 403 (issue #22). De context-rule staat alléén de OAuth2-flow toe — anonieme REST-creates blijven geweigerd |
| 010 | Claims uit het Entra **id_token** (`userInfoURL: ""`) + UPN-fallback voor e-mail | Graph `/oidc/userinfo` geeft géén `email`-claim voor accounts zonder mail-attribuut → 400 bij eerste login (issue #24). Het id_token bevat altijd `preferred_username` (UPN = primair e-mailadres bij Respellion); `entra_oidc.pb.js` gebruikt die als fallback (regex-guard tegen guest-UPN's) en logt gefaalde OAuth2-pogingen naar de containerlog | | 010 | Claims uit het Entra **id_token** (`userInfoURL: ""`) + UPN-fallback voor e-mail | Graph `/oidc/userinfo` geeft géén `email`-claim voor accounts zonder mail-attribuut → 400 bij eerste login (issue #24). Het id_token bevat altijd `preferred_username` (UPN = primair e-mailadres bij Respellion); `entra_oidc.pb.js` gebruikt die als fallback (regex-guard tegen guest-UPN's) en logt gefaalde OAuth2-pogingen naar de containerlog |
| 011 | `updateRule: '(@request.auth.id = id && @request.body.role:isset = false) \|\| @request.auth.role = "admin"'`, `deleteRule: '@request.auth.role = "admin"'` | TeamManager-rosterbeheer werkt nu voor app-admins (issue #27). De `role:isset`-guard sluit tegelijk de privilege-escalatie waarbij een gebruiker zijn eigen `role` naar admin kon patchen; onboarding raakt `role` niet en blijft self-service |
## Bestanden ## Bestanden
@@ -55,6 +56,7 @@ Browser (SPA) PocketBase (auth) Entra ID
| `pb_migrations/1781000000_team_members_to_auth.js` | Converteert relation-velden naar text (data blijft behouden), dropt de oude PIN-collection en maakt `team_members` als auth-collection. Idempotent; provider wordt alleen als fast-path meegenomen als de env al aanwezig is. | | `pb_migrations/1781000000_team_members_to_auth.js` | Converteert relation-velden naar text (data blijft behouden), dropt de oude PIN-collection en maakt `team_members` als auth-collection. Idempotent; provider wordt alleen als fast-path meegenomen als de env al aanwezig is. |
| `pb_migrations/1781000001_tighten_api_rules.js` | Zet alle niet-systeem-collecties op `@request.auth.id != ""`. | | `pb_migrations/1781000001_tighten_api_rules.js` | Zet alle niet-systeem-collecties op `@request.auth.id != ""`. |
| `pb_migrations/1781000002_allow_oauth2_signup.js` | Zet `createRule = '@request.context = "oauth2"'` op reeds-gemigreerde omgevingen (issue #22). | | `pb_migrations/1781000002_allow_oauth2_signup.js` | Zet `createRule = '@request.context = "oauth2"'` op reeds-gemigreerde omgevingen (issue #22). |
| `pb_migrations/1781000003_team_members_admin_rules.js` | Admin-rosterbeheer + role-escalatie-guard op reeds-gemigreerde omgevingen (issue #27). |
| `pb_hooks/entra_oidc.pb.js` | Reconcilieert de OIDC-provider vanuit `ENTRA_*` env bij elke start + cron-tick (compare-before-save). | | `pb_hooks/entra_oidc.pb.js` | Reconcilieert de OIDC-provider vanuit `ENTRA_*` env bij elke start + cron-tick (compare-before-save). |
| `pb_hooks/utils.js` | Gedeelde helpers (`resolveRole`, `reconcileEntraOidc`) — per callback binnenhalen via `require()`. | | `pb_hooks/utils.js` | Gedeelde helpers (`resolveRole`, `reconcileEntraOidc`) — per callback binnenhalen via `require()`. |
| `pb_hooks/team_members.pb.js` | Auto-provisioning (`role`, `enrollment_status`, naam-fallback) + admin-rol re-sync. | | `pb_hooks/team_members.pb.js` | Auto-provisioning (`role`, `enrollment_status`, naam-fallback) + admin-rol re-sync. |

View File

@@ -41,14 +41,17 @@ onRecordCreate(function (e) {
e.next(); e.next();
}, "team_members"); }, "team_members");
// On every successful auth (incl. OIDC): keep the admin role in sync with the // On every successful auth (incl. OIDC): guarantee the admin role for
// allow-list, so promoting/demoting an admin only requires an env change. // allow-list members. ESCALATE-ONLY (#27): the allow-list grants admin, it no
// longer demotes — otherwise every role promotion made through TeamManager
// would be reverted on the member's next login. Demotion runs through
// TeamManager; allow-list members cannot be demoted (the list wins on their
// next login).
onRecordAuthRequest(function (e) { onRecordAuthRequest(function (e) {
const { resolveRole } = require(`${__hooks}/utils.js`); const { resolveRole } = require(`${__hooks}/utils.js`);
const email = e.record.get("email") || ""; const email = e.record.get("email") || "";
const want = resolveRole(email); if (resolveRole(email) === "admin" && e.record.get("role") !== "admin") {
if (e.record.get("role") !== want) { e.record.set("role", "admin");
e.record.set("role", want);
e.app.save(e.record); e.app.save(e.record);
} }
e.next(); e.next();

View File

@@ -151,8 +151,10 @@ migrate((app) => {
listRule: '@request.auth.id != ""', listRule: '@request.auth.id != ""',
viewRule: '@request.auth.id != ""', viewRule: '@request.auth.id != ""',
createRule: '@request.context = "oauth2"', createRule: '@request.context = "oauth2"',
updateRule: '@request.auth.id = id', // Self-update may not touch `role` (closes self-service privilege
deleteRule: null, // escalation); admins manage the roster incl. roles and deletion (#27).
updateRule: '(@request.auth.id = id && @request.body.role:isset = false) || @request.auth.role = "admin"',
deleteRule: '@request.auth.role = "admin"',
authRule: "", authRule: "",
manageRule: null, manageRule: null,

View File

@@ -0,0 +1,52 @@
/// <reference path="../pb_data/types.d.ts" />
//
// Issue #27 — TeamManager roster management + close role self-escalation.
//
// The collection shipped with `updateRule: '@request.auth.id = id'` and
// `deleteRule: null`. That broke the TeamManager admin panel (admins could
// not change roles or remove members) AND left a privilege escalation open:
// the self-update rule did not restrict fields, so any authenticated user
// could PATCH their own `role` to "admin".
//
// New rules:
// update — a user may update their own record but NOT the `role` field
// (onboarding only touches enrollment fields); admins may update
// anyone, including `role`.
// delete — admins only.
//
// Environments that have not applied 1781000000 yet get these rules directly
// from that (updated) migration; this follow-up exists for databases where it
// already ran (Labs). Applying both is harmless (same values).
const UPDATE_RULE = '(@request.auth.id = id && @request.body.role:isset = false) || @request.auth.role = "admin"';
const DELETE_RULE = '@request.auth.role = "admin"';
migrate((app) => {
let col;
try {
col = app.findCollectionByNameOrId("team_members");
} catch (_) {
console.log("team_members_admin_rules: team_members does not exist — skipping.");
return;
}
if (col.type !== "auth") {
console.log("team_members_admin_rules: team_members is not an auth collection — skipping.");
return;
}
col.updateRule = UPDATE_RULE;
col.deleteRule = DELETE_RULE;
app.save(col);
}, (app) => {
// Down: restore the previous (self-update only, superuser delete) rules.
let col;
try {
col = app.findCollectionByNameOrId("team_members");
} catch (_) {
return;
}
if (col.type !== "auth") {
return;
}
col.updateRule = '@request.auth.id = id';
col.deleteRule = null;
app.save(col);
});

View File

@@ -114,6 +114,10 @@ const COLLECTIONS = [
// Mirror of pb_migrations/1781000002: creation only via the OAuth2 // Mirror of pb_migrations/1781000002: creation only via the OAuth2
// sign-up flow (issue #22). // sign-up flow (issue #22).
createRule: '@request.context = "oauth2"', createRule: '@request.context = "oauth2"',
// Mirror of pb_migrations/1781000003: self-update without role changes;
// roster management (incl. role/delete) is admin-only (issue #27).
updateRule: '(@request.auth.id = id && @request.body.role:isset = false) || @request.auth.role = "admin"',
deleteRule: '@request.auth.role = "admin"',
passwordAuth: { enabled: false, identityFields: ['email'] }, passwordAuth: { enabled: false, identityFields: ['email'] },
fields: [ fields: [
{ name: 'name', type: 'text', required: false }, { name: 'name', type: 'text', required: false },

View File

@@ -8,10 +8,11 @@ import { useApp } from '../../store/AppContext';
// Users are provisioned automatically on first Azure (Entra ID) login — admins // Users are provisioned automatically on first Azure (Entra ID) login — admins
// no longer create them by hand. This panel lets an admin review the roster, // no longer create them by hand. This panel lets an admin review the roster,
// switch a member between user/admin, and remove members. The admin role is // switch a member between user/admin, and remove members. The
// also re-synced from the ENTRA_ADMIN_EMAILS allow-list on every login (see // ENTRA_ADMIN_EMAILS allow-list is escalate-only (see
// pb_hooks/team_members.pb.js), so a manual change here can be overridden on // pb_hooks/team_members.pb.js): it guarantees admin for its members on every
// the member's next sign-in if their e-mail is on/off that list. // login, so promotions made here stick, but demoting an allow-list member is
// undone on their next sign-in — remove them from the list instead.
const TeamManager = () => { const TeamManager = () => {
const { state } = useApp(); const { state } = useApp();
const [users, setUsers] = useState([]); const [users, setUsers] = useState([]);
@@ -67,8 +68,9 @@ const TeamManager = () => {
<p className="text-sm text-fg-muted flex items-start gap-2"> <p className="text-sm text-fg-muted flex items-start gap-2">
<Info size={16} className="mt-0.5 shrink-0 text-teal" /> <Info size={16} className="mt-0.5 shrink-0 text-teal" />
Team members are created automatically when they first sign in with their Team members are created automatically when they first sign in with their
Microsoft account. Admins are granted via the <code>ENTRA_ADMIN_EMAILS</code>{' '} Microsoft account. Members of the <code>ENTRA_ADMIN_EMAILS</code> allow-list
allow-list and re-synced on each login. are always admin; promotions made here stick, but demoting an allow-list
member is undone on their next sign-in.
</p> </p>
</Card> </Card>