main
3
Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
 not
|
d6b3f9764f |
fix(e2e): bound the Playwright run and make a failed login say why (closes #161) (#165)
## What & why #161 is really two defects, and the second one is why the first was undiagnosable. **A wedged suite consumed the job, and took the post-mortem with it.** Nothing bounded the Playwright run, so CI stopped the job mid-suite — and `if: always()` does not survive that. Run 739's job metadata shows every step after the e2e as a **0-second failure** stamped at the kill: ``` 14 failure 09:48:17 -> 10:14:54 Self-service e2e (Playwright …) 15 failure 10:14:54 -> 10:14:54 verify-stack check summary ← if: always() 16 failure 10:14:54 -> 10:14:54 e2e spec summary ← if: always() 17 failure 10:14:54 -> 10:14:54 Dump container logs on failure ← if: failure() 18 failure 10:14:54 -> 10:14:54 Tear down ← if: always() ``` So the per-spec summary, the container-log dump and the teardown never ran, and the log lost whatever the killed process had buffered — leaving the single `✘` line the issue was filed from. `globalTimeout` now makes Playwright stop and *report*: the JSON report is written and those steps still get their turn. (A `timeout-minutes` on the job would have reproduced the same failure, so there isn't one.) The "~24-minute gap" is that kill, not necessarily a hang — note run 739 shows `run_attempt: 2`, and `concurrency.cancel-in-progress` kills an in-flight run on any re-run or push. **A login that never got its form ate the 90-second test timeout.** Playwright actions auto-wait until the *test* timeout, not `expect.timeout` — so a portal that serves its page but never bootstraps (its `config.json` fetch or the OIDC discovery behind `authorize()` failed; `main.ts` only `console.error`s) spent 90s to report `locator.fill: Test timeout of 90000ms exceeded`: the symptom, not the cause. That is catalogus.spec's 1.8 minutes. Both Keycloak forms are now asserted visible first, with a 20s budget and a message naming the step that never happened. Verified against a real blank-bootstrap portal — the beheer image served with a `config.json` that is not JSON — which fails in **20.2s** with *"the Keycloak login form never appeared — the portal did not reach Keycloak (check its config.json fetch and the OIDC discovery …)"*. **And the summary now says why.** The per-spec table (#136) rendered a verdict icon and nothing else, so even a surviving summary cost a log dive. Failing specs now carry their first error, flattened for a table cell (ANSI stripped, newlines collapsed, `|` escaped, clipped) — shape verified against a real @playwright/test 1.61 failing report, with a stdlib assert self-check on `make unit`. Closes #161 ## Definition of Done - [x] Linked Gitea issue (above). - [x] Failing test committed before the implementation. - [x] Implementation makes the test pass; refactor commit follows (login helper dedup). - [x] Conventional Commits referencing the issue (`refs #161`). - [ ] CI green — all Gitea Actions jobs. - [x] `docker compose up` from a fresh clone reaches green health checks within 3 minutes (untouched). - [x] Docs updated — `docs/runbooks/gitea-actions-gotchas.md` §9. - [x] ADR — not needed: no boundary, dependency or coupling rule touched (test/CI infra only). - [x] Demo note — not applicable: nothing user-visible. ## Notes for reviewers **What this does not do: identify why the beheerder login failed that once.** The evidence to do that was destroyed by defect 2, which is what this PR fixes. The suite ran green here five times today (catalogus.spec 1.1–5.3s each) — but a local box is not the loaded CI runner, so that is weak evidence and I am not claiming the flake is gone. What changes is that the next occurrence is bounded and self-describing: it fails in 20s naming the failing step, the JSON report survives, and the summary prints the error. Please keep #161 in mind rather than treating this as proof. **Two follow-ups I did not pull into this PR:** - *All four portals show a permanently blank page if their startup fetch fails* — `main.ts` does `fetch('config.json').then(bootstrap).catch(console.error)`, one shot, no UI and no recovery. That is a real product gap (the deliberately-broken portal above is exactly what a user would see) and wants its own slice, not a test-infra PR. - `retries: 1` is untouched. CLAUDE.md §15 says flaky tests are fixed rather than retried, but removing retries while a real flake is unexplained would trade a rare red for a frequent one. Worth revisiting once #161 recurs (or doesn't) with the new diagnostics. The login-helper rename (`medewerker-login.ts` → `keycloak-login.ts`, citizen logins routed through `loginBurger`) is its own no-behaviour-change commit: the three citizen specs each duplicated the same three-line login, so guarding the login path once meant routing them through it first.Reviewed-on: #165 |
||
|
not
|
d0fb2b3e8c |
S-15c · Enforce MFA on the medewerker (Keycloak) realm (#158)
Closes #132.
Staff logins (behandel + beheer portals) now need a second factor; the citizen realms are unchanged.
**How:** every seeded medewerker carries a TOTP credential, which activates Keycloak's stock *conditional OTP* step in both the browser flow and the direct grant — no custom browser-flow JSON in the export. `CONFIGURE_TOTP` is a default required action so a medewerker added later must enrol first. ADR-0031 records the choice and, explicitly, that the shared fixture secret is a demo posture only.
**Tests (red first,
|
||
|
not
|
0494730223 |
feat(portal-beheer): ACL default-fill configuration editor (closes #131) (#138)
## What & why S-15b, second of the S-15 (#16) split, on top of S-15a (#133). A beheerder edits the ACL's ZGW **default-fill** values from the beheer portal, and the next zaak is stamped with the new values — no restart. Closes #131 ### The vertical portal → BFF `GET/PUT /beheer/default-fill` (medewerker realm + `beheerder` role) → ACL `GET/PUT /default-fill` → a runtime-mutable in-memory store the ACL reads **per zaak**. - **ACL**: `IDefaultFillStore` / `InMemoryDefaultFillStore` (thread-safe, seeded from `Acl:Defaults`); `AclService` reads `fill.Current` per zaak (not cached at construction); `GET`/`PUT /default-fill` with required-field validation. - **BFF**: `IAclClient` gains `GetDefaultFillAsync`/`UpdateDefaultFillAsync`; `GET`/`PUT /beheer/default-fill` behind the `beheerder` policy. OpenAPI + generated client regenerated. - **Frontend**: a *Default-fill* editor page in the beheer app (load → edit → save, with saved/failure states) + nav between Catalogus and Default-fill. ### Scope decision → ADR-0026 Only the **three ZGW fill fields** (bronorganisatie, verantwoordelijke organisatie, vertrouwelijkheidaanduiding) are editable. The S-27 catalog-resolution keys stay **static config** — editing them would desync the zaaktype-URL cache (ADR-0021), and they're catalogus wiring, not "default fill". The store is **in-memory** (seeded from config): an edit reverts on restart. That's the reference-app-appropriate ceiling (no DB added to the stateless ACL); upgrade path documented. Recorded in **ADR-0026**. ## Verified locally lint (`dotnet format`) ✓ · .NET unit — acl 60 / bff 45 / domain 152 / event-subscriber 19 / acceptance 17 ✓ · frontend lint+test (8 projects) ✓ · beheer build ✓. Clean full-solution build (caught + fixed the acceptance `AclService` ctor drift). TDD red→green per layer (ACL store, ACL endpoints, BFF, frontend). ## Definition of Done - [x] Failing test committed before each implementation (red→green per layer). - [x] Conventional Commits referencing #131. - [ ] CI green — see note below. - [x] Docs: ADR-0026 + S-15b demo note. - [x] Demo note in `docs/demo-script.md`. ## Note on CI The bulk validates in the fast jobs (lint/build/unit/frontend/mutation). The **verify-stack e2e** (incl. the new `default-fill.spec.ts`) can't go green until the pre-existing **verify-stack bring-up failure on the 1.27/2.0.0 runner** is resolved (that fails on plain `main` too — unrelated to this PR). Additive change; no existing e2e touched. 🤖 Generated with [Claude Code](https://claude.com/claude-code)Reviewed-on: #138 |