## 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
100 lines
5.0 KiB
TypeScript
100 lines
5.0 KiB
TypeScript
import { createHmac } from 'node:crypto';
|
|
import { readFileSync, writeFileSync } from 'node:fs';
|
|
import { tmpdir } from 'node:os';
|
|
import { join } from 'node:path';
|
|
import { expect, type Page } from '@playwright/test';
|
|
|
|
// Every portal login in the suite goes through this module — citizen realms (mock DigiD) and the
|
|
// medewerker realm alike — so the shared Keycloak form handling lives in exactly one place.
|
|
|
|
// The medewerker realm enforces MFA (S-15c), so a staff login is two steps: password, then a TOTP
|
|
// code. The realm export seeds every medewerker with this fixture secret — Keycloak HMACs the raw
|
|
// secret bytes — so the e2e can compute a valid code instead of enrolling an authenticator.
|
|
const OTP_SECRET = 'BIGMEDEWERKEROTPSEED';
|
|
|
|
export const OTP_PERIOD_MS = 30_000;
|
|
|
|
/**
|
|
* How long a Keycloak form gets to appear. Generous enough for a cold first browser launch and a
|
|
* loaded stack, far short of the 90-second test timeout an auto-waiting action would otherwise eat.
|
|
*/
|
|
const FORM_TIMEOUT_MS = 20_000;
|
|
const FORM_NEVER_APPEARED =
|
|
'the Keycloak login form never appeared — the portal did not reach Keycloak (check its ' +
|
|
'config.json fetch and the OIDC discovery on the authority it was built with)';
|
|
const OTP_NEVER_APPEARED =
|
|
'the Keycloak OTP form never appeared — the password step did not complete (check the ' +
|
|
'medewerker realm seeded this user with both a password and a TOTP credential)';
|
|
|
|
// RFC 6238 TOTP: HMAC-SHA1 over the 30-second counter, dynamically truncated to 6 digits.
|
|
export function totp(secret = OTP_SECRET, at = Date.now()): string {
|
|
const counter = Buffer.alloc(8);
|
|
counter.writeBigUInt64BE(BigInt(Math.floor(at / OTP_PERIOD_MS)));
|
|
const mac = createHmac('sha1', secret).update(counter).digest();
|
|
const offset = mac[mac.length - 1] & 0x0f;
|
|
return String((mac.readUInt32BE(offset) & 0x7fffffff) % 1_000_000).padStart(6, '0');
|
|
}
|
|
|
|
// Keycloak refuses a TOTP code it has already accepted (its otpPolicyCodeReusable defaults to
|
|
// false), so two logins as the same medewerker inside one 30-second window would both submit the
|
|
// same code and the second is rejected. Spend the first counter this medewerker has left.
|
|
export function nextUnusedCounter(now: number, spent: number): number {
|
|
return Math.max(Math.floor(now / OTP_PERIOD_MS), spent + 1);
|
|
}
|
|
|
|
// The spent counter lives on disk rather than in module state: Playwright starts a fresh worker
|
|
// process for a retry, which would otherwise forget it and resubmit the rejected code.
|
|
function spendCounter(username: string): number {
|
|
const file = join(tmpdir(), `otp-counter-${username}`);
|
|
let spent = -1;
|
|
try {
|
|
spent = Number(readFileSync(file, 'utf8')) || -1;
|
|
} catch {
|
|
// first login as this medewerker in this run
|
|
}
|
|
const counter = nextUnusedCounter(Date.now(), spent);
|
|
writeFileSync(file, String(counter));
|
|
return counter;
|
|
}
|
|
|
|
/**
|
|
* Fill Keycloak's login form. Every portal is guarded, so the first navigation redirects here; the
|
|
* form ids are stable across themes.
|
|
*
|
|
* The form is asserted visible *before* it is filled. A portal that never reaches Keycloak — its
|
|
* runtime `config.json` fetch or the OIDC discovery behind `authorize()` failed, so it never
|
|
* bootstrapped and shows a blank page (main.ts only logs to the console) — would otherwise leave
|
|
* `fill()` auto-waiting until the whole test times out: 90 seconds spent to report
|
|
* `locator.fill: Test timeout of 90000ms exceeded`, naming the symptom and not the cause. That is
|
|
* how #161's catalogus.spec burned 1.8 minutes. This fails in a quarter of the time and says which
|
|
* step never happened.
|
|
*/
|
|
async function submitPassword(page: Page, username: string): Promise<void> {
|
|
await expect(page.locator('#username'), FORM_NEVER_APPEARED).toBeVisible({ timeout: FORM_TIMEOUT_MS });
|
|
await page.locator('#username').fill(username);
|
|
await page.locator('#password').fill('test123');
|
|
await page.locator('#kc-login').click();
|
|
}
|
|
|
|
/** A citizen login on a mock-DigiD realm — no second factor (ADR-0031). */
|
|
export async function loginBurger(page: Page, username: string): Promise<void> {
|
|
await submitPassword(page, username);
|
|
}
|
|
|
|
/** A staff login on the medewerker realm: password, then the enforced TOTP second factor. */
|
|
export async function loginMedewerker(page: Page, username: string): Promise<void> {
|
|
await submitPassword(page, username);
|
|
|
|
// Keycloak's conditional-OTP step. Same reasoning as the password form above: assert it arrived
|
|
// rather than letting `fill()` swallow the test timeout.
|
|
await expect(page.locator('#otp'), OTP_NEVER_APPEARED).toBeVisible({ timeout: FORM_TIMEOUT_MS });
|
|
|
|
// Wait out the rest of the window if the counter we may spend is still in the future; Keycloak's
|
|
// lookAheadWindow would accept the code a moment early, but only by one counter — waiting keeps a
|
|
// third login in the same window valid too.
|
|
const counter = spendCounter(username);
|
|
await page.waitForTimeout(Math.max(0, counter * OTP_PERIOD_MS - Date.now()));
|
|
await page.locator('#otp').fill(totp(OTP_SECRET, counter * OTP_PERIOD_MS));
|
|
await page.locator('#kc-login').click();
|
|
}
|