ci: validate both Caddyfiles in the test job (#26)

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>

.github/workflows/test.yml

@@ -11,6 +11,18 @@ 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 .
 

Caddyfile

@@ -48,10 +48,13 @@
     handle_errors {
         # Don't mask API errors with the SPA shell — return a proper
         # JSON error so the frontend can display a meaningful message.
+        # NOTE: `respond` only accepts `body`/`close` subdirectives; the
+        # Content-Type must be set via a separate `header` directive. An
+        # invalid subdirective is a Caddyfile parse error and crash-loops
+        # the container at startup (issue #20).
         @api path /api/*
-        respond @api `{"code":{err.status_code},"message":"Backend unavailable"}` {err.status_code} {
+        header @api Content-Type application/json
-            Content-Type application/json
+        respond @api `{"code":{err.status_code},"message":"Backend unavailable"}` {err.status_code}
-        }
 
         rewrite * /index.html
         file_server

docs/auth-spec.md

@@ -44,6 +44,8 @@ Browser (SPA)                PocketBase (auth)                 Entra ID
 | 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 |
 
 ## Bestanden
 
@@ -52,6 +54,7 @@ Browser (SPA)                PocketBase (auth)                 Entra ID
 | `pb_migrations/1000000000_baseline_ledger_sync.js` | Detecteert een out-of-band geprovisionede DB (schema bestaat, `_migrations`-ledger leeg) en markeert de historische migraties als applied, zodat de replay niet crasht (issue #18). |
 | `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_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. |

infra/development/site/deploy-playbook.yml

@@ -100,3 +100,53 @@
           {{ pb_logs.stdout | default('') }}
           {{ pb_logs.stderr | default('') }}
       when: pb_health is failed
+
+    # The frontend container carries the SPA + Caddy. An invalid Caddyfile
+    # crash-loops it at startup while the PocketBase gate stays green — that
+    # outage (issue #20) was invisible to CI because the test job swaps in
+    # Caddyfile.test. Gate on the real container actually serving.
+    - name: Wait for the frontend (Caddy) to become healthy
+      ansible.builtin.command: docker exec learning-platform wget -q --spider http://127.0.0.1:80/
+      register: fe_health
+      until: fe_health.rc == 0
+      retries: 18
+      delay: 5
+      changed_when: false
+      ignore_errors: yes
+
+    - name: Collect frontend logs for diagnosis
+      ansible.builtin.command: docker logs --tail 80 learning-platform
+      register: fe_logs
+      when: fe_health is failed
+      changed_when: false
+      failed_when: false
+
+    - name: Abort deploy — frontend is not healthy
+      ansible.builtin.fail:
+        msg: |
+          The frontend (Caddy) container did not become healthy after the deploy.
+          Recent container logs:
+          {{ fe_logs.stdout | default('') }}
+          {{ fe_logs.stderr | default('') }}
+      when: fe_health is failed
+
+    # Observability behind the auth perimeter: surface the end-to-end state
+    # in the CI log on every deploy.
+    - name: Post-deploy smoke report
+      ansible.builtin.shell: |
+        cd /opt/learning-platform
+        echo '--- docker compose ps ---'
+        docker compose ps
+        echo '--- frontend -> pocketbase proxy health ---'
+        docker exec learning-platform wget -q -O - http://127.0.0.1:80/api/health || echo 'PROXY HEALTH FAILED'
+        echo '--- team_members auth methods (OIDC provider present?) ---'
+        docker exec pocketbase-learning wget -q -O - http://127.0.0.1:8090/api/collections/team_members/auth-methods || echo 'AUTH-METHODS FAILED'
+        echo '--- pocketbase logs (tail 30) ---'
+        docker compose logs --tail=30 pocketbase-learning
+      register: smoke_report
+      changed_when: false
+      failed_when: false
+
+    - name: Show smoke report
+      ansible.builtin.debug:
+        msg: "{{ smoke_report.stdout_lines }}"

infra/production/site/deploy-playbook.yml

@@ -100,3 +100,53 @@
           {{ pb_logs.stdout | default('') }}
           {{ pb_logs.stderr | default('') }}
       when: pb_health is failed
+
+    # The frontend container carries the SPA + Caddy. An invalid Caddyfile
+    # crash-loops it at startup while the PocketBase gate stays green — that
+    # outage (issue #20) was invisible to CI because the test job swaps in
+    # Caddyfile.test. Gate on the real container actually serving.
+    - name: Wait for the frontend (Caddy) to become healthy
+      ansible.builtin.command: docker exec learning-platform wget -q --spider http://127.0.0.1:80/
+      register: fe_health
+      until: fe_health.rc == 0
+      retries: 18
+      delay: 5
+      changed_when: false
+      ignore_errors: yes
+
+    - name: Collect frontend logs for diagnosis
+      ansible.builtin.command: docker logs --tail 80 learning-platform
+      register: fe_logs
+      when: fe_health is failed
+      changed_when: false
+      failed_when: false
+
+    - name: Abort deploy — frontend is not healthy
+      ansible.builtin.fail:
+        msg: |
+          The frontend (Caddy) container did not become healthy after the deploy.
+          Recent container logs:
+          {{ fe_logs.stdout | default('') }}
+          {{ fe_logs.stderr | default('') }}
+      when: fe_health is failed
+
+    # Observability behind the auth perimeter: surface the end-to-end state
+    # in the CI log on every deploy.
+    - name: Post-deploy smoke report
+      ansible.builtin.shell: |
+        cd /opt/learning-platform
+        echo '--- docker compose ps ---'
+        docker compose ps
+        echo '--- frontend -> pocketbase proxy health ---'
+        docker exec learning-platform wget -q -O - http://127.0.0.1:80/api/health || echo 'PROXY HEALTH FAILED'
+        echo '--- team_members auth methods (OIDC provider present?) ---'
+        docker exec pocketbase-learning wget -q -O - http://127.0.0.1:8090/api/collections/team_members/auth-methods || echo 'AUTH-METHODS FAILED'
+        echo '--- pocketbase logs (tail 30) ---'
+        docker compose logs --tail=30 pocketbase-learning
+      register: smoke_report
+      changed_when: false
+      failed_when: false
+
+    - name: Show smoke report
+      ansible.builtin.debug:
+        msg: "{{ smoke_report.stdout_lines }}"

pb_hooks/entra_oidc.pb.js

@@ -32,3 +32,31 @@ 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/utils.js

@@ -76,7 +76,12 @@ module.exports = {
       clientSecret: clientSecret,
       authURL:      "https://login.microsoftonline.com/" + tenant + "/oauth2/v2.0/authorize",
       tokenURL:     "https://login.microsoftonline.com/" + tenant + "/oauth2/v2.0/token",
-      userInfoURL:  "https://graph.microsoft.com/oidc/userinfo",
+      // Deliberately empty: claims are read from the Entra id_token instead of
+      // 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,7 +117,8 @@ migrate((app) => {
       clientSecret: clientSecret,
       authURL:     "https://login.microsoftonline.com/" + tenant + "/oauth2/v2.0/authorize",
       tokenURL:    "https://login.microsoftonline.com/" + tenant + "/oauth2/v2.0/token",
-      userInfoURL: "https://graph.microsoft.com/oidc/userinfo",
+      // Empty on purpose: claims come from the id_token (see utils.js, issue #24).
+      userInfoURL: "",
       displayName: "Microsoft",
       pkce: true,
     },
@@ -142,10 +143,14 @@ migrate((app) => {
     // Access rules: every legitimate user is authenticated (Azure-gated +
     // OIDC). Profiles are readable by any authenticated user (leaderboard,
     // dashboard); a user may only update their own record (onboarding flips
-    // enrollment_status). Creation/deletion happen via OAuth2 / superuser only.
+    // enrollment_status). Record creation is ONLY allowed from within the
+    // OAuth2 sign-up flow: PocketBase applies the createRule to the automatic
+    // record creation on a first OIDC login, so `null` would make every first
+    // login fail with 403 "Only superusers can perform this action" (issue
+    // #22). Plain REST creates keep being rejected for non-superusers.
     listRule:   '@request.auth.id != ""',
     viewRule:   '@request.auth.id != ""',
-    createRule: null,
+    createRule: '@request.context = "oauth2"',
     updateRule: '@request.auth.id = id',
     deleteRule: null,
     authRule:   "",

pb_migrations/1781000002_allow_oauth2_signup.js

@@ -0,0 +1,46 @@
+/// <reference path="../pb_data/types.d.ts" />
+//
+// Issue #22 — Allow OAuth2 sign-up on team_members.
+//
+// The collection was created with `createRule: null` on the assumption that
+// the OAuth2 flow bypasses it. It does not: PocketBase applies the createRule
+// to the automatic record creation on a first OIDC login, so every first
+// login failed with 403 "Only superusers can perform this action". Since the
+// pre-Azure records were intentionally dropped, EVERY user was a first login
+// and nobody could sign in.
+//
+// `@request.context = "oauth2"` scopes record creation to the OAuth2 flow
+// only — plain REST creates (e.g. anonymous POST /records, which could
+// otherwise pre-seed rogue admin profiles) keep being rejected.
+//
+// Environments that have not applied 1781000000 yet get this rule directly
+// from that (updated) migration; this follow-up exists for databases where it
+// already ran with the old `null` rule (Labs). Applying it twice is harmless.
+migrate((app) => {
+  let col;
+  try {
+    col = app.findCollectionByNameOrId("team_members");
+  } catch (_) {
+    console.log("allow_oauth2_signup: team_members does not exist — skipping.");
+    return;
+  }
+  if (col.type !== "auth") {
+    console.log("allow_oauth2_signup: team_members is not an auth collection — skipping.");
+    return;
+  }
+  col.createRule = '@request.context = "oauth2"';
+  app.save(col);
+}, (app) => {
+  // Down: restore the (broken) superuser-only rule.
+  let col;
+  try {
+    col = app.findCollectionByNameOrId("team_members");
+  } catch (_) {
+    return;
+  }
+  if (col.type !== "auth") {
+    return;
+  }
+  col.createRule = null;
+  app.save(col);
+});

scripts/setup-pb-collections.mjs

@@ -111,6 +111,9 @@ const COLLECTIONS = [
     name: 'team_members',
     type: 'auth',
     ...OPEN_RULES,
+    // Mirror of pb_migrations/1781000002: creation only via the OAuth2
+    // sign-up flow (issue #22).
+    createRule: '@request.context = "oauth2"',
     passwordAuth: { enabled: false, identityFields: ['email'] },
     fields: [
       { name: 'name', type: 'text', required: false },