Compare commits

...

4 Commits

Author SHA1 Message Date
RaymondVerhoef
54137122a7 fix: read Entra claims from id_token + UPN e-mail fallback (#24)
All checks were successful
On Pull Request to Main / test (pull_request) Successful in 38s
On Pull Request to Main / publish (pull_request) Successful in 1m10s
On Pull Request to Main / deploy-dev (pull_request) Successful in 3m7s
First Microsoft logins failed with 400 "email: Cannot be blank": Graph's
/oidc/userinfo endpoint omits the email claim for accounts without a
mail attribute, and team_members requires an e-mail (it also drives the
ENTRA_ADMIN_EMAILS role mapping). Reproduced against a mock Entra with
PocketBase v0.30.4; the user-supplied response body matched the
missing-email fingerprint exactly.

- provider config (reconciler + migration fast-path): userInfoURL is now
  empty, so PocketBase reads the claims from the Entra id_token, which
  always carries preferred_username (UPN) and email when available. The
  #18 reconciler flips the already-deployed Labs provider automatically
  on the next boot — no migration needed.
- pb_hooks/entra_oidc.pb.js: onRecordAuthWithOAuth2Request hook falls
  back to the lowercased UPN when the email claim is absent. Guest UPNs
  (ext_user#EXT#@tenant...) are excluded explicitly — "#" is RFC-valid
  in an e-mail local part, so both a naive regex AND PocketBase's own
  validation would accept them (caught by test V5). The hook also logs
  every failed OAuth2 attempt with the underlying error to the container
  log, which PocketBase otherwise only writes to its internal logs db.

Verified with a switchable mock-Entra matrix (8/8): no-email→UPN e-mail
+ allow-list role, email claim wins when present, no duplicate on
re-login, guest UPN yields a clean validation error, anonymous REST
create stays rejected, both log lines present. Regression: issue-22
matrix 9/9 (baseline pinned to 1a1351d now that #23 is merged), DoD
harness 15/15, npm test 112/112.

Closes #24

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
2026-07-12 19:26:25 +02:00
RaymondVerhoef
897b46d4a1 fix: allow OAuth2 sign-up on team_members via @request.context rule (#22)
All checks were successful
On Pull Request to Main / test (pull_request) Successful in 39s
On Pull Request to Main / publish (pull_request) Successful in 1m9s
On Pull Request to Main / deploy-dev (pull_request) Successful in 3m9s
Every first Microsoft login failed with 403 "Only superusers can perform
this action": PocketBase applies the collection createRule to the
automatic record creation during OAuth2 sign-up, and team_members was
created with createRule null (superuser-only) on the wrong assumption
that the OAuth2 flow bypasses it. Since the pre-Azure records were
deliberately dropped, every user was a first login and nobody could get
in. Reproduced locally against a mock OIDC provider (PocketBase v0.30.4).

createRule becomes '@request.context = "oauth2"': record creation is
allowed exclusively from within the OAuth2 flow. Anonymous REST creates
(which could otherwise pre-seed rogue admin profiles) remain rejected —
covered by an explicit test.

- pb_migrations/1781000002_allow_oauth2_signup.js: applies the rule on
  already-migrated environments (Labs); idempotent, guarded, with down
- pb_migrations/1781000000_team_members_to_auth.js: same rule for fresh
  environments (filename unchanged — ledger-safe)
- scripts/setup-pb-collections.mjs: fallback entry mirrored
- docs/auth-spec.md: ADR 009 + files table

Verified with a mock-OIDC matrix (9/9): baseline reproduces the 403 on
the old rule; upgrade path heals (first login 200, role/enrollment/name
defaults, second login no duplicate, anonymous create still rejected);
fresh chain + admin allow-list mapping OK. DoD harness regression 15/15,
npm test 112/112.

Closes #22

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
2026-07-12 15:15:54 +02:00
RaymondVerhoef
80f738ddcb fix: valid Caddyfile handle_errors + frontend health-gate in deploys (#20)
All checks were successful
On Pull Request to Main / test (pull_request) Successful in 39s
On Pull Request to Main / publish (pull_request) Successful in 1m7s
On Pull Request to Main / deploy-dev (pull_request) Successful in 3m6s
The learning-platform (Caddy) container crash-looped since this morning's
89d3395: `Content-Type application/json` is not a valid subdirective of
`respond` — a Caddyfile parse error aborts Caddy at startup, taking the
whole app offline for authenticated users while every CI run stayed green.
Verified with caddy v2.10.0: "unrecognized subdirective 'Content-Type',
at Caddyfile:53"; the fixed file validates clean.

CI was blind to this twice over: test.yml swaps the real Caddyfile for
Caddyfile.test in the test image, and the deploy health-gate from #18
only covers PocketBase.

- Caddyfile: set Content-Type via a `header @api` directive before the
  `respond`; behaviour of the JSON error response is unchanged
- infra/*/site/deploy-playbook.yml: frontend health-gate (docker exec
  wget --spider on the container) with log dump + abort on failure, and
  an always-on post-deploy smoke report (compose ps, proxy health,
  team_members auth-methods, PocketBase log tail) for visibility behind
  the auth perimeter

Closes #20

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
2026-07-11 14:20:15 +02:00
1a1351ddcb Merge pull request 'Fix #18: migratie-ledger-sync + deploy-proof Azure SSO' (#19) from fix/issue-18-migration-ledger-sso into main
All checks were successful
On Push to Main / test (push) Successful in 32s
On Push to Main / publish (push) Successful in 1m8s
On Push to Main / deploy-dev (push) Successful in 3m0s
Reviewed-on: #19
2026-07-11 11:54:37 +00:00
9 changed files with 200 additions and 7 deletions

View File

@@ -48,10 +48,13 @@
handle_errors { handle_errors {
# Don't mask API errors with the SPA shell — return a proper # Don't mask API errors with the SPA shell — return a proper
# JSON error so the frontend can display a meaningful message. # 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/* @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 rewrite * /index.html
file_server file_server

View File

@@ -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 | | 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 |
| 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 ## 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/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/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_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

@@ -100,3 +100,53 @@
{{ pb_logs.stdout | default('') }} {{ pb_logs.stdout | default('') }}
{{ pb_logs.stderr | default('') }} {{ pb_logs.stderr | default('') }}
when: pb_health is failed 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 }}"

View File

@@ -100,3 +100,53 @@
{{ pb_logs.stdout | default('') }} {{ pb_logs.stdout | default('') }}
{{ pb_logs.stderr | default('') }} {{ pb_logs.stderr | default('') }}
when: pb_health is failed 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 }}"

View File

@@ -32,3 +32,31 @@ cronAdd("entraOidcReconcile", "* * * * *", () => {
console.log("entra_oidc reconcile (cron): OIDC provider (re)configured from ENTRA_* env"); 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");

View File

@@ -76,7 +76,12 @@ module.exports = {
clientSecret: clientSecret, clientSecret: clientSecret,
authURL: "https://login.microsoftonline.com/" + tenant + "/oauth2/v2.0/authorize", authURL: "https://login.microsoftonline.com/" + tenant + "/oauth2/v2.0/authorize",
tokenURL: "https://login.microsoftonline.com/" + tenant + "/oauth2/v2.0/token", 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", displayName: "Microsoft",
pkce: true, pkce: true,
}; };

View File

@@ -117,7 +117,8 @@ migrate((app) => {
clientSecret: clientSecret, clientSecret: clientSecret,
authURL: "https://login.microsoftonline.com/" + tenant + "/oauth2/v2.0/authorize", authURL: "https://login.microsoftonline.com/" + tenant + "/oauth2/v2.0/authorize",
tokenURL: "https://login.microsoftonline.com/" + tenant + "/oauth2/v2.0/token", 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", displayName: "Microsoft",
pkce: true, pkce: true,
}, },
@@ -142,10 +143,14 @@ migrate((app) => {
// Access rules: every legitimate user is authenticated (Azure-gated + // Access rules: every legitimate user is authenticated (Azure-gated +
// OIDC). Profiles are readable by any authenticated user (leaderboard, // OIDC). Profiles are readable by any authenticated user (leaderboard,
// dashboard); a user may only update their own record (onboarding flips // 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 != ""', listRule: '@request.auth.id != ""',
viewRule: '@request.auth.id != ""', viewRule: '@request.auth.id != ""',
createRule: null, createRule: '@request.context = "oauth2"',
updateRule: '@request.auth.id = id', updateRule: '@request.auth.id = id',
deleteRule: null, deleteRule: null,
authRule: "", authRule: "",

View File

@@ -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);
});

View File

@@ -111,6 +111,9 @@ const COLLECTIONS = [
name: 'team_members', name: 'team_members',
type: 'auth', type: 'auth',
...OPEN_RULES, ...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'] }, passwordAuth: { enabled: false, identityFields: ['email'] },
fields: [ fields: [
{ name: 'name', type: 'text', required: false }, { name: 'name', type: 'text', required: false },