Compare commits

...

2 Commits

Author SHA1 Message Date
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
7 changed files with 163 additions and 5 deletions

View File

@@ -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} {
Content-Type application/json
}
header @api Content-Type application/json
respond @api `{"code":{err.status_code},"message":"Backend unavailable"}` {err.status_code}
rewrite * /index.html
file_server

View File

@@ -44,6 +44,7 @@ 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 |
## Bestanden
@@ -52,6 +53,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. |

View File

@@ -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 }}"

View File

@@ -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 }}"

View File

@@ -142,10 +142,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: "",

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',
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 },