diff --git a/docs/auth-spec.md b/docs/auth-spec.md index 02acfa5..9371bcc 100644 --- a/docs/auth-spec.md +++ b/docs/auth-spec.md @@ -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 | | 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 | -| 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 | | 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 | | 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 | | 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 @@ -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/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/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/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. | diff --git a/pb_hooks/team_members.pb.js b/pb_hooks/team_members.pb.js index 4147409..b366117 100644 --- a/pb_hooks/team_members.pb.js +++ b/pb_hooks/team_members.pb.js @@ -41,14 +41,17 @@ onRecordCreate(function (e) { e.next(); }, "team_members"); -// On every successful auth (incl. OIDC): keep the admin role in sync with the -// allow-list, so promoting/demoting an admin only requires an env change. +// On every successful auth (incl. OIDC): guarantee the admin role for +// 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) { const { resolveRole } = require(`${__hooks}/utils.js`); const email = e.record.get("email") || ""; - const want = resolveRole(email); - if (e.record.get("role") !== want) { - e.record.set("role", want); + if (resolveRole(email) === "admin" && e.record.get("role") !== "admin") { + e.record.set("role", "admin"); e.app.save(e.record); } e.next(); diff --git a/pb_migrations/1781000000_team_members_to_auth.js b/pb_migrations/1781000000_team_members_to_auth.js index f725a61..b08268d 100644 --- a/pb_migrations/1781000000_team_members_to_auth.js +++ b/pb_migrations/1781000000_team_members_to_auth.js @@ -151,8 +151,10 @@ migrate((app) => { listRule: '@request.auth.id != ""', viewRule: '@request.auth.id != ""', createRule: '@request.context = "oauth2"', - updateRule: '@request.auth.id = id', - deleteRule: null, + // Self-update may not touch `role` (closes self-service privilege + // 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: "", manageRule: null, diff --git a/pb_migrations/1781000003_team_members_admin_rules.js b/pb_migrations/1781000003_team_members_admin_rules.js new file mode 100644 index 0000000..07ab0a4 --- /dev/null +++ b/pb_migrations/1781000003_team_members_admin_rules.js @@ -0,0 +1,52 @@ +/// +// +// 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); +}); diff --git a/scripts/setup-pb-collections.mjs b/scripts/setup-pb-collections.mjs index 4fce69b..8d2c047 100644 --- a/scripts/setup-pb-collections.mjs +++ b/scripts/setup-pb-collections.mjs @@ -114,6 +114,10 @@ const COLLECTIONS = [ // Mirror of pb_migrations/1781000002: creation only via the OAuth2 // sign-up flow (issue #22). 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'] }, fields: [ { name: 'name', type: 'text', required: false }, diff --git a/src/components/admin/TeamManager.jsx b/src/components/admin/TeamManager.jsx index f41fc56..bfab544 100644 --- a/src/components/admin/TeamManager.jsx +++ b/src/components/admin/TeamManager.jsx @@ -8,10 +8,11 @@ import { useApp } from '../../store/AppContext'; // 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, -// switch a member between user/admin, and remove members. The admin role is -// also re-synced from the ENTRA_ADMIN_EMAILS allow-list on every login (see -// pb_hooks/team_members.pb.js), so a manual change here can be overridden on -// the member's next sign-in if their e-mail is on/off that list. +// switch a member between user/admin, and remove members. The +// ENTRA_ADMIN_EMAILS allow-list is escalate-only (see +// pb_hooks/team_members.pb.js): it guarantees admin for its members on every +// 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 { state } = useApp(); const [users, setUsers] = useState([]); @@ -67,8 +68,9 @@ const TeamManager = () => {

Team members are created automatically when they first sign in with their - Microsoft account. Admins are granted via the ENTRA_ADMIN_EMAILS{' '} - allow-list and re-synced on each login. + Microsoft account. Members of the ENTRA_ADMIN_EMAILS allow-list + are always admin; promotions made here stick, but demoting an allow-list + member is undone on their next sign-in.