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

Reviewed-on: #23

.github/workflows/test.yml

@@ -11,18 +11,6 @@ jobs:
       - name: Checkout
         uses: actions/checkout@v4
 
-      # The test image below swaps in Caddyfile.test, so the production
-      # Caddyfile is otherwise never exercised by CI — an invalid config then
-      # only surfaces when the deploy health-gate fails, after the broken
-      # container already replaced the healthy one (issue #20/#26). Validate
-      # both files up front so a parse error fails the PR check in seconds.
-      - name: Validate Caddyfiles
-        run: |
-          docker run --rm -v "$PWD":/workspace -w /workspace caddy:2-alpine \
-            caddy validate --config Caddyfile --adapter caddyfile
-          docker run --rm -v "$PWD":/workspace -w /workspace caddy:2-alpine \
-            caddy validate --config Caddyfile.test --adapter caddyfile
-
       - name: Build Docker image
         run: docker build -t learning-platform .
 

docs/auth-spec.md

@@ -39,14 +39,12 @@ 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`), **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 |
+| 004 | Admin-rol via e-mail-allowlist (`ENTRA_ADMIN_EMAILS`), re-synced elke login | Geen handmatig beheer; allowlist-wijziging werkt bij volgende login |
 | 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
 
@@ -56,7 +54,6 @@ 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. |

pb_hooks/entra_oidc.pb.js

@@ -32,31 +32,3 @@ cronAdd("entraOidcReconcile", "* * * * *", () => {
     console.log("entra_oidc reconcile (cron): OIDC provider (re)configured from ENTRA_* env");
   }
 });
-
-// Entra does not always supply an `email` claim: the id_token only carries it
-// when the account has a mail attribute (issue #24). The UPN in
-// `preferred_username` is always present and equals the primary e-mail for
-// Respellion organisation accounts, so fall back to it when it looks like a
-// real address. Guest UPNs ("user_ext#EXT#@tenant...") fail the regex on
-// purpose — those still get a clear validation error instead of a bogus email.
-// The catch also surfaces the underlying OAuth2 failure in the container log,
-// which PocketBase otherwise only writes to its internal logs database.
-onRecordAuthWithOAuth2Request((e) => {
-  const u = e.oAuth2User;
-  if (u && !u.email) {
-    const upn = String((u.rawUser && u.rawUser.preferred_username) || u.username || "").trim().toLowerCase();
-    // `#` is RFC-valid in an e-mail local part, so guest UPNs
-    // ("ext_user#EXT#@tenant.onmicrosoft.com") pass a naive e-mail check AND
-    // PocketBase's own validation — exclude them explicitly.
-    if (/^[^@\s#]+@[^@\s#]+\.[^@\s#]+$/.test(upn)) {
-      u.email = upn;
-      console.log("entra_oidc: email claim absent — falling back to UPN " + upn);
-    }
-  }
-  try {
-    e.next();
-  } catch (err) {
-    console.log("entra_oidc auth-with-oauth2 FAILED:", String(err));
-    throw err;
-  }
-}, "team_members");

pb_hooks/team_members.pb.js

@@ -41,17 +41,14 @@ onRecordCreate(function (e) {
   e.next();
 }, "team_members");
 
-// On every successful auth (incl. OIDC): guarantee the admin role for
+// On every successful auth (incl. OIDC): keep the admin role in sync with the
-// allow-list members. ESCALATE-ONLY (#27): the allow-list grants admin, it no
+// allow-list, so promoting/demoting an admin only requires an env change.
-// 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") || "";
-  if (resolveRole(email) === "admin" && e.record.get("role") !== "admin") {
+  const want = resolveRole(email);
-    e.record.set("role", "admin");
+  if (e.record.get("role") !== want) {
+    e.record.set("role", want);
     e.app.save(e.record);
   }
   e.next();

pb_hooks/utils.js

@@ -76,12 +76,7 @@ module.exports = {
       clientSecret: clientSecret,
       authURL:      "https://login.microsoftonline.com/" + tenant + "/oauth2/v2.0/authorize",
       tokenURL:     "https://login.microsoftonline.com/" + tenant + "/oauth2/v2.0/token",
-      // Deliberately empty: claims are read from the Entra id_token instead of
+      userInfoURL:  "https://graph.microsoft.com/oidc/userinfo",
-      // Graph's /oidc/userinfo. The userinfo endpoint omits `email` for
-      // accounts without a mail attribute, while the id_token always carries
-      // `preferred_username` (UPN) which entra_oidc.pb.js uses as an e-mail
-      // fallback (issue #24).
-      userInfoURL:  "",
       displayName:  "Microsoft",
       pkce:         true,
     };

pb_migrations/1781000000_team_members_to_auth.js

@@ -117,8 +117,7 @@ migrate((app) => {
       clientSecret: clientSecret,
       authURL:     "https://login.microsoftonline.com/" + tenant + "/oauth2/v2.0/authorize",
       tokenURL:    "https://login.microsoftonline.com/" + tenant + "/oauth2/v2.0/token",
-      // Empty on purpose: claims come from the id_token (see utils.js, issue #24).
+      userInfoURL: "https://graph.microsoft.com/oidc/userinfo",
-      userInfoURL: "",
       displayName: "Microsoft",
       pkce: true,
     },
@@ -151,10 +150,8 @@ migrate((app) => {
     listRule:   '@request.auth.id != ""',
     viewRule:   '@request.auth.id != ""',
     createRule: '@request.context = "oauth2"',
-    // Self-update may not touch `role` (closes self-service privilege
+    updateRule: '@request.auth.id = id',
-    // escalation); admins manage the roster incl. roles and deletion (#27).
+    deleteRule: null,
-    updateRule: '(@request.auth.id = id && @request.body.role:isset = false) || @request.auth.role = "admin"',
-    deleteRule: '@request.auth.role = "admin"',
     authRule:   "",
     manageRule: null,
 

pb_migrations/1781000003_team_members_admin_rules.js

@@ -1,52 +0,0 @@
-/// <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);
-});

scripts/setup-pb-collections.mjs

@@ -114,10 +114,6 @@ 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 },

src/components/admin/TeamManager.jsx

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