psa-systems/mokosh-server

Watch

Fork 1

feat(e2e): un-fixme OIDC token-flow via replayed OP cookies (PMS-143) #132

Merged

nrupard merged 3 commits from feat/pms-143-oidc-unfixme into main

2026-06-08 00:45:55 +02:00

nrupard commented

2026-06-08 00:33:10 +02:00

Owner

Summary

Closes PMS-143. The OIDC PKCE token-flow test (e2e/tests/oidc.spec.ts) was quarantined in PMS-140 because the api project's request context only carried a Bearer header. Bunyip's /oauth2/authorize gates code minting on a server-validated OP session cookie (bunyip PR #67), so authorize 302'd to the hub login screen instead of the registered redirect_uri and the test failed at the state mismatch assertion.

This PR adds an OP-cookie replay path through the setup project and switches the OIDC test onto it.

Changes

e2e/tests/global.setup.ts: after the bearer is captured, also persist the browser context's cookies filtered to the OP host and its parent domain (e.g. api.a8n.systems plus a8n.systems) to e2e/.auth/op-state.json in Playwright storageState shape. Throws if zero cookies match, with a diagnostic that names what was actually in the context.
e2e/lib/auth-state.ts: expose OP_STORAGE_STATE_FILE and readOpStorageState(). Both file-missing and empty-cookies cases throw with messages that point at the likely cause (setup did not run, or filter excluded everything).
e2e/lib/fixtures.ts: add an oidcTest fixture that builds a request context with the replayed OP cookies via storageState and deliberately omits the Bearer header. The OP reads the session from the cookie; an inbound bearer with the wrong audience would just be noise.
e2e/tests/oidc.spec.ts: switch to the oidcTest fixture (import { oidcTest as test } from '../lib/fixtures'), drop test.fixme, replace the historical PMS-140 quarantine comment with a short PMS-143 note.
e2e/README.md: drop the OIDC test.fixme annotation in the What it covers table; update the harness-shape paragraph to describe the new dual-fixture layout (bearer for PSA-API specs, storageState for OIDC).

.auth/ is already in e2e/.gitignore so op-state.json will not be committed.

Why this works against bunyip PR #67

The OP session cookie is named bunyip_op_session (bunyip/crates/bunyip-domain/src/middleware/auth.rs:248), opaque, validated server-side. The hub login handler sets it alongside access_token / refresh_token on a successful login. After the setup project's loginViaSpa completes, all three cookies live in the browser context; capturing them into op-state.json and replaying via storageState gives the OIDC test the same session signal a real browser would carry to /oauth2/authorize.

Test plan

CI Check green.
CI e2e post-merge: setup logs persisted N OP cookie(s) line, OIDC test passes the full happy path (authorize 302 -> code -> token -> userinfo -> refresh).
Two consecutive post-merge runs show OIDC passing on the first attempt (per PMS-143 AC).

Rollback plan

If the cookie replay does not establish the OP session on staging (e.g. bunyip pins sessions to user-agent or IP), re-fixme oidc.spec.ts and fall back to the alternative the spec named: drive the OIDC flow through a real Page that already completed the SPA login. That trade-off costs a second login per CI run (rate-limit pressure) but is guaranteed to inherit the session.

## Summary Closes PMS-143. The OIDC PKCE token-flow test (`e2e/tests/oidc.spec.ts`) was quarantined in PMS-140 because the `api` project's request context only carried a Bearer header. Bunyip's `/oauth2/authorize` gates code minting on a server-validated OP session cookie (bunyip PR #67), so authorize 302'd to the hub login screen instead of the registered redirect_uri and the test failed at the `state mismatch` assertion. This PR adds an OP-cookie replay path through the setup project and switches the OIDC test onto it. ## Changes - `e2e/tests/global.setup.ts`: after the bearer is captured, also persist the browser context's cookies filtered to the OP host and its parent domain (e.g. `api.a8n.systems` plus `a8n.systems`) to `e2e/.auth/op-state.json` in Playwright `storageState` shape. Throws if zero cookies match, with a diagnostic that names what was actually in the context. - `e2e/lib/auth-state.ts`: expose `OP_STORAGE_STATE_FILE` and `readOpStorageState()`. Both file-missing and empty-cookies cases throw with messages that point at the likely cause (setup did not run, or filter excluded everything). - `e2e/lib/fixtures.ts`: add an `oidcTest` fixture that builds a request context with the replayed OP cookies via `storageState` and deliberately omits the Bearer header. The OP reads the session from the cookie; an inbound bearer with the wrong audience would just be noise. - `e2e/tests/oidc.spec.ts`: switch to the `oidcTest` fixture (`import { oidcTest as test } from '../lib/fixtures'`), drop `test.fixme`, replace the historical PMS-140 quarantine comment with a short PMS-143 note. - `e2e/README.md`: drop the OIDC `test.fixme` annotation in the `What it covers` table; update the harness-shape paragraph to describe the new dual-fixture layout (bearer for PSA-API specs, storageState for OIDC). `.auth/` is already in `e2e/.gitignore` so `op-state.json` will not be committed. ## Why this works against bunyip PR #67 The OP session cookie is named `bunyip_op_session` (`bunyip/crates/bunyip-domain/src/middleware/auth.rs:248`), opaque, validated server-side. The hub login handler sets it alongside `access_token` / `refresh_token` on a successful login. After the setup project's `loginViaSpa` completes, all three cookies live in the browser context; capturing them into `op-state.json` and replaying via `storageState` gives the OIDC test the same session signal a real browser would carry to `/oauth2/authorize`. ## Test plan - [ ] CI `Check` green. - [ ] CI `e2e` post-merge: setup logs `persisted N OP cookie(s)` line, OIDC test passes the full happy path (authorize 302 -> code -> token -> userinfo -> refresh). - [ ] Two consecutive post-merge runs show OIDC passing on the first attempt (per PMS-143 AC). ## Rollback plan If the cookie replay does not establish the OP session on staging (e.g. bunyip pins sessions to user-agent or IP), re-fixme `oidc.spec.ts` and fall back to the alternative the spec named: drive the OIDC flow through a real Page that already completed the SPA login. That trade-off costs a second login per CI run (rate-limit pressure) but is guaranteed to inherit the session.

nrupard added 1 commit

2026-06-08 00:33:10 +02:00

feat(e2e): un-fixme OIDC token-flow via replayed OP cookies (PMS-143)

E2E (staging) / Playwright against staging (pull_request) Failing after 26s

Details

Check / fmt + clippy + compile + tests (pull_request) Failing after 59s

Details

0368bff22e

The OIDC PKCE token-flow test (`e2e/tests/oidc.spec.ts`) was quarantined in PMS-140 because the api project's request context only carried a Bearer header. Bunyip's `/oauth2/authorize` gates code minting on a server-validated OP session cookie (bunyip PR #67), so authorize 302'd to the hub login screen instead of the registered redirect_uri and the test failed at the `state mismatch` assertion.

Add an OP-cookie replay path through setup:

- `e2e/tests/global.setup.ts`: after capturing the bearer, also persist the browser context's cookies filtered to the OP host and its parent domain (e.g. `api.a8n.systems` plus `a8n.systems`) to `e2e/.auth/op-state.json` in Playwright `storageState` shape.
- `e2e/lib/auth-state.ts`: expose `OP_STORAGE_STATE_FILE` + `readOpStorageState()` with empty-cookies / missing-file guards that point at the most likely cause.
- `e2e/lib/fixtures.ts`: add an `oidcTest` fixture that builds a request context with the replayed OP cookies via `storageState` and deliberately omits the Bearer header (the OP reads the session from the cookie; an inbound bearer with the wrong audience would just be noise).
- `e2e/tests/oidc.spec.ts`: switch to the `oidcTest` fixture, drop `test.fixme`, replace the historical PMS-140 quarantine comment with a short PMS-143 note.
- `e2e/README.md`: drop the OIDC `test.fixme` annotation; update the harness-shape paragraph to describe the new dual-fixture layout (bearer for PSA-API, storageState for OIDC).

`auth-state.ts` re-declares the storageState cookie type rather than importing it from `@playwright/test` so a `node`-target consumer of the helper does not transitively pull the Playwright typings.

The `.auth/` directory is already gitignored, so `op-state.json` will not be committed.

#PMS-143

nrupard added 1 commit

2026-06-08 00:37:37 +02:00

fix(e2e): skip parent-domain derivation for IP literals

E2E (staging) / Playwright against staging (pull_request) Failing after 25s

Details

Check / fmt + clippy + compile + tests (pull_request) Successful in 55s

Details

868658ce83

Code review of PMS-143 flagged `computeOpCookieDomains` would compute a bogus `0.0.1` parent from `127.0.0.1`. The bogus entry would never match a real cookie so the captured set stayed correct in practice, but it clutters the diagnostic line and reads as "we think this is a parent domain" when it is not.

Guard with `node:net.isIP()`. For IP literals (v4 or v6) and single-label hosts, return only the exact host; for proper FQDNs the existing label-strip still derives the apex-adjacent parent (e.g. `api.a8n.systems` -> add `a8n.systems`).

#PMS-143

nrupard added 1 commit

2026-06-08 00:40:24 +02:00

chore(e2e): surface bunyip body on OIDC authorize non-3xx

Check / fmt + clippy + compile + tests (pull_request) Successful in 56s

Details

E2E (staging) / Playwright against staging (pull_request) Successful in 24s

Details

Create release / Create release from merged PR (pull_request) Has been skipped

Details

0de7285f80

Setup persists OP cookies correctly (`persisted 3 OP cookie(s) (api.a8n.systems, a8n.systems)`), but the authorize call returns HTTP 400 instead of the expected 3xx. The current assertion only reports the status code, so the actual rejection (invalid_request? unknown client_id? unregistered redirect_uri?) was invisible.

Replace the `expect(...).toContain(status)` with an explicit branch that, on a non-3xx response, throws an error carrying:

- the configured `client_id` and `redirect_uri` (so the most likely mismatch is at hand without grepping CI secrets)
- the response `content-type` and the first 2000 chars of the body (bunyip's OIDC errors are JSON `{error, error_description}`, and AppError-derived 400s emit a similar text body, both small enough to fit)

Comment names the three most plausible root causes so the next iteration off the diagnostic is targeted, not speculative.

#PMS-143