rve/learning-platform
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

CI: caddy validate van beide Caddyfiles in de test-job #28

Merged
rve merged 2 commits from fix/issue-26-ci-caddy-validate into main 2026-07-12 19:04:29 +00:00
Conversation 0 Commits 2 Files Changed 5 +49 -2
rve commented 2026-07-12 19:02:14 +00:00
Owner
Copy Link

Closes #26

caddy validate van zowel het productie-Caddyfile als Caddyfile.test als eerste stap in de test-job (caddy:2-alpine, zelfde base als de Dockerfile). De #20-klasse fouten (parse-fout → frontend-crash-loop) sneuvelen daarmee al in de PR-check i.p.v. pas bij de deploy-gate, als de kapotte container de gezonde al vervangen heeft.

⚠️ Raakt het bevroren .github/workflows/ — expliciet geautoriseerd door de product owner (#26). De test-job van déze PR-run is zelf het bewijs dat de stap werkt; de negatieve flow is in #20 al aangetoond (zelfde validate faalde exact op de toenmalige fout).

Stapeling: deze branch bouwt op fix/issue-24-missing-email-claim (PR #25, nog open) — aftakken van main zou bij de PR-deploy de #24-fix terugdraaien. Merge-volgorde: #25 → deze → PR voor #27 (of alleen de laatste; die bevat alles).

🤖 Generated with Claude Code

Closes #26 `caddy validate` van zowel het productie-`Caddyfile` als `Caddyfile.test` als eerste stap in de test-job (`caddy:2-alpine`, zelfde base als de Dockerfile). De #20-klasse fouten (parse-fout → frontend-crash-loop) sneuvelen daarmee al in de PR-check i.p.v. pas bij de deploy-gate, als de kapotte container de gezonde al vervangen heeft. ⚠️ Raakt het bevroren `.github/workflows/` — expliciet geautoriseerd door de product owner (#26). De test-job van déze PR-run is zelf het bewijs dat de stap werkt; de negatieve flow is in #20 al aangetoond (zelfde validate faalde exact op de toenmalige fout). **Stapeling:** deze branch bouwt op `fix/issue-24-missing-email-claim` (PR #25, nog open) — aftakken van main zou bij de PR-deploy de #24-fix terugdraaien. Merge-volgorde: #25 → deze → PR voor #27 (of alleen de laatste; die bevat alles). 🤖 Generated with [Claude Code](https://claude.com/claude-code)
rve added 2 commits 2026-07-12 19:02:15 +00:00
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
Details
On Pull Request to Main / publish (pull_request) Successful in 1m10s
Details
On Pull Request to Main / deploy-dev (pull_request) Successful in 3m7s
Details
54137122a7 
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>
ci: validate both Caddyfiles in the test job (#26)
Some checks failed
On Pull Request to Main / test (pull_request) Failing after 26s
Details
On Pull Request to Main / publish (pull_request) Has been skipped
Details
On Pull Request to Main / deploy-dev (pull_request) Has been skipped
Details
6bf8bad5fb 
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>
rve merged commit 85204938df into main 2026-07-12 19:04:29 +00:00
Sign in to join this conversation.
Reviewers
No Reviewers
Labels
No items
No Label
Milestone
No items
No Milestone
Projects
Clear projects
No project
Assignees
Clear assignees
eho lmi lpr not rdi rve
No Assignees
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: rve/learning-platform#28
Delete Branch "fix/issue-26-ci-caddy-validate"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?