diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index c41f013..5fb2303 100644 --- a/.github/workflows/test.yml +++ b/.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 . diff --git a/docs/auth-spec.md b/docs/auth-spec.md index d152845..02acfa5 100644 --- a/docs/auth-spec.md +++ b/docs/auth-spec.md @@ -45,6 +45,7 @@ Browser (SPA) PocketBase (auth) Entra ID | 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 diff --git a/pb_hooks/entra_oidc.pb.js b/pb_hooks/entra_oidc.pb.js index 1747a08..fe88df1 100644 --- a/pb_hooks/entra_oidc.pb.js +++ b/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"); diff --git a/pb_hooks/utils.js b/pb_hooks/utils.js index 6b0ff31..37f5f03 100644 --- a/pb_hooks/utils.js +++ b/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, }; diff --git a/pb_migrations/1781000000_team_members_to_auth.js b/pb_migrations/1781000000_team_members_to_auth.js index 4d02978..f725a61 100644 --- a/pb_migrations/1781000000_team_members_to_auth.js +++ b/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, },