diff --git a/.gitignore b/.gitignore index e5c925bfc..e121cf65a 100644 --- a/.gitignore +++ b/.gitignore @@ -18,9 +18,7 @@ /test-results /playwright-report /playwright/.cache -/e2e/setup/storageState.json -/e2e/setup/storageStateOwner.json -/e2e/setup/storageStateDelegate.json +/e2e/setup/storageState*.json # next.js /.next/ @@ -95,4 +93,4 @@ next-env.d.ts # times over. The same two names are excluded from eslint and vitest, for the # same reason in a different tool. /.wt-*/ -/.worktrees/ \ No newline at end of file +/.worktrees/ diff --git a/docs/api/openapi.yaml b/docs/api/openapi.yaml index 464b9e4b3..bbd1d6b42 100644 --- a/docs/api/openapi.yaml +++ b/docs/api/openapi.yaml @@ -12750,7 +12750,11 @@ paths: should send the `X-HealthLog-Account` header on the requests it wants scoped instead. The grant is validated here so the client gets an honest refusal immediately, but the stamp authorises nothing on its own — every following request re-checks the grant. `accountId: null` clears the switch and always works, including from - inside a switched session: this endpoint is the way out." + inside a switched session: this endpoint is the way out, and it is deliberately not fenced for that reason — a + session whose context cannot be proved must still be able to leave. Supply `expectedEpoch` to make the write + conditional: two tabs switching at once resolve to one monotonic outcome, and the loser is refused with 409 + `sharing.session.changed` rather than silently overwriting the winner. Reconcile through GET /api/auth/me and + retry once against the reconciled epoch." requestBody: required: true content: @@ -12764,6 +12768,15 @@ paths: minLength: 1 maxLength: 64 - type: "null" + expectedEpoch: + description: "The record epoch the caller believes this browser session is at, as published by GET /api/auth/me or a + previous switch. Present makes the write conditional: if another tab moved the selector first the + switch is refused with 409 sharing.session.changed and the caller reconciles through /api/auth/me. + Omitted keeps the unconditional write, which is what a client predating the record-session fence + sends. Cookie transport only." + type: integer + minimum: 0 + maximum: 9007199254740991 required: - accountId responses: @@ -12781,6 +12794,17 @@ paths: $ref: "#/components/schemas/ErrorEnvelope" "401": *a1 "403": *a20 + "409": + description: "The record this browser session is on is not the record the request asserted. `meta.errorCode` is + `sharing.session.changed`. It is not a refusal of access — the caller may well be entitled to both records — + so do not leave the record and do not clear the session. Re-read GET /api/auth/me with a network-only + request, adopt the `recordSession` it returns, and retry. The body deliberately carries no epoch or scope: a + client may adopt a context from GET /api/auth/me and POST /api/account/switch only, and a refusal is not a + third source of truth." + content: + application/json: + schema: + $ref: "#/components/schemas/ErrorEnvelope" "422": description: Validation failed. content: @@ -36687,10 +36711,15 @@ components: type: string accountAccess: $ref: "#/components/schemas/AccountAccess" + recordSession: + anyOf: + - $ref: "#/components/schemas/RecordSessionState" + - type: "null" required: - id - username - accountAccess + - recordSession additionalProperties: {} description: "The signed-in account: its identity, its preferences, and (since v1.36.0) what account sharing lets it do. Additional properties are the preference fields this spec does not yet enumerate. Under an active switch this @@ -36702,26 +36731,127 @@ components: accounts: type: array items: - $ref: "#/components/schemas/AccountAccessEntry" - description: Every record this caller may open, newest grant first. + type: object + properties: + accountId: + type: string + minLength: 1 + username: + type: string + minLength: 1 + displayName: + anyOf: + - type: string + - type: "null" + access: + type: string + enum: + - read + - write + level: + type: string + enum: + - read + - write + - manage + recordKind: + type: string + enum: + - shared + - managed + sections: + anyOf: + - type: array + items: + type: string + enum: + - measurements + - medications + - labs + - profile + - illness + - mind + - cycle + - documents + - type: "null" + canWrite: + type: boolean + required: + - accountId + - username + - displayName + - access + - level + - recordKind + - sections + - canWrite + additionalProperties: false active: anyOf: - - $ref: "#/components/schemas/AccountAccessEntry" + - type: object + properties: + accountId: + type: string + minLength: 1 + username: + type: string + minLength: 1 + displayName: + anyOf: + - type: string + - type: "null" + access: + type: string + enum: + - read + - write + level: + type: string + enum: + - read + - write + - manage + recordKind: + type: string + enum: + - shared + - managed + sections: + anyOf: + - type: array + items: + type: string + enum: + - measurements + - medications + - labs + - profile + - illness + - mind + - cycle + - documents + - type: "null" + canWrite: + type: boolean + required: + - accountId + - username + - displayName + - access + - level + - recordKind + - sections + - canWrite + additionalProperties: false - type: "null" - description: "The record this session is inside right now, resolved to a full entry rather than an id to look up — so a - banner can name the person without joining two fields. Null when the caller is in their own record, and - ALWAYS null on the Bearer transport: a token carries its selector per request and this endpoint refuses - one." recordKind: type: string enum: - self - shared - managed - description: The server-resolved kind of the record currently in view. `self` applies when `active` is null. canSwitch: type: boolean - description: Whether there is anywhere to switch to. Bind it directly. required: - accounts - active @@ -36733,86 +36863,30 @@ components: be a second program deciding one person's access to a health record, and the two answer differently the first time an expiry or a revocation appears. Always present — an account nobody has shared with gets an empty list, not a missing field." - AccountAccessEntry: + RecordSessionState: type: object properties: - accountId: - type: string - description: The account whose record it is — the value POST /api/account/switch takes, and the value the - X-HealthLog-Account header carries. - username: - type: string - displayName: + epoch: + type: integer + minimum: -9007199254740991 + maximum: 9007199254740991 + description: How many times this browser session's record selector has moved. Monotonic, server-issued, and scoped to + this session. Send it back as the `X-HealthLog-Record-Epoch` header on every subsequent same-origin request, + and as `expectedEpoch` when switching. + scope: anyOf: - type: string - type: "null" - access: - type: string - enum: - - read - - write - description: The legacy grant level that shipped clients decode. MANAGE serializes as `write` here; new code reads - canonical `level`. - level: - type: string - enum: - - read - - write - - manage - description: The grant's resolved level. `manage` additionally admits changing and removing entries on the delegable - surfaces; it never reaches the identity surfaces (settings, connections, tokens, consent, grant management), - which stay refused on an invited record at every level. Render it; never derive it from `canWrite`. - recordKind: - type: string - enum: - - self - - shared - - managed - description: Server-resolved presentation metadata for the record. It is never accepted as an authorization input. - sections: - anyOf: - - type: array - items: - $ref: "#/components/schemas/ShareSection" - - type: "null" - description: The sections of that record this caller may open, or null for the entire record. Null is first-class and is - what every pre-v1.37.0 grant carries; a MANAGE grant always carries it too. A non-null array names the open - sections and NOTHING else answers — a client that switches into a scoped record without honouring this gets - the standard 403 on every out-of-scope route, fail-closed and byte-identical to every other sharing refusal, - which is safe but reads as a wall of errors. An empty array means the grant opens nothing. Ordered by the - consent screen's reading order. - canWrite: - type: boolean - description: "Whether this caller may ADD to that record: true for an accepted WRITE or MANAGE grant, false for a READ - one. On its own it never means edit or delete — a WRITE grant adds and does nothing more. What MANAGE - additionally admits is said by `level`. Resolved server-side. Render it; never derive it." + description: The account this session is pointed at, or null for its own record. Send it back as the + `X-HealthLog-Record-Scope` header, with `self` standing for null. required: - - accountId - - username - - displayName - - access - - level - - recordKind - - sections - - canWrite + - epoch + - scope additionalProperties: false - description: "One account this caller may act on. No avatar is published: the avatar bytes are owner-scoped, so a URL - here would resolve to a refusal. Clients paint their initials fallback." - ShareSection: - type: string - enum: - - measurements - - medications - - labs - - profile - - illness - - mind - - cycle - - documents - description: "One section of a health record, as a grant may name it. A closed vocabulary of eight, derived by - clustering the delegable read surfaces rather than from the module list — the two answer different questions and - never share a spelling. The whole-record case is `null` on the field that carries these, never a member of this - enum: a scope that named the whole record would be a narrowing that means its own absence." + description: Where the record-session fence says this browser session is. Cookie transport only — null on Bearer, which + carries its selector per request and accumulates no session state to be stale about. A client adopts this and + then asserts it on every same-origin request, so a request formed under a context that has since moved is + refused rather than served against the wrong record. RecordActivityEnvelope: type: object properties: @@ -37000,6 +37074,21 @@ components: accepted, when and by whom it ended — so ended grants stay in the list with `state: REVOKED` rather than disappearing. `revokedBy` distinguishes the owner withdrawing access from the delegate handing it back. `lastUsedAt` mirrors API-token semantics: the last time the grant was actually exercised." + ShareSection: + type: string + enum: + - measurements + - medications + - labs + - profile + - illness + - mind + - cycle + - documents + description: "One section of a health record, as a grant may name it. A closed vocabulary of eight, derived by + clustering the delegable read surfaces rather than from the module list — the two answer different questions and + never share a spelling. The whole-record case is `null` on the field that carries these, never a member of this + enum: a scope that named the whole record would be a narrowing that means its own absence." AccountGrantState: type: string enum: @@ -37172,6 +37261,13 @@ components: AccountSwitchState: type: object properties: + recordSession: + anyOf: + - $ref: "#/components/schemas/RecordSessionState" + - type: "null" + description: "The context this session is now in, when the request supplied `expectedEpoch`. Null when it did not: an + unconditional switch does not read the new epoch back, and a number computed client-side would be wrong the + moment the target equalled the current selector. Adopt this value; do not derive it." actingAs: anyOf: - type: object @@ -37198,6 +37294,7 @@ components: additionalProperties: false - type: "null" required: + - recordSession - actingAs additionalProperties: false description: The account this session is now acting on, or null when it is back in its own. The stamp is a selector and @@ -37638,6 +37735,34 @@ components: sync engine, device or notification registration, or any auth/refresh call: those are about the person holding the token, not the record being read, and the account payload refuses the header outright for exactly that reason." + RecordEpoch: + name: X-HealthLog-Record-Epoch + in: header + required: false + schema: + type: string + maxLength: 15 + description: "The record-session epoch this client believes it is on, taken from `recordSession.epoch` on GET + /api/auth/me or POST /api/account/switch. Cookie transport only; a Bearer client neither sends nor receives it. + Send the literal `bootstrap` before the account payload has resolved. A browser session that has never been + pointed at another record is served without either header, which is what keeps a tab open across the deploy + working. Once a session HAS entered a shared record it stays fenced permanently, in both directions: a + mismatching epoch is refused with 409 `sharing.session.changed` (reconcile through GET /api/auth/me and retry), + and an absent header is refused with 403 `sharing.access.denied` (a client predating the fence, which recovers + by leaving the record and reloading)." + RecordScope: + name: X-HealthLog-Record-Scope + in: header + required: false + schema: + type: string + maxLength: 64 + description: The record this client believes it is inside, taken from `recordSession.scope`, with the literal `self` + standing for one's own record. Sent alongside `X-HealthLog-Record-Epoch` on every same-origin request; one + without the other is treated as a stale assertion, not as an absent one. Cookie transport only. Both headers are + echoed back on any response that actually resolved a record scope, and a client discards a response whose echo + contradicts the context it is now on — a response carrying no echo resolved no record scope and is served + normally. securitySchemes: bearerAuth: type: http diff --git a/e2e/account-sharing.spec.ts b/e2e/account-sharing.spec.ts index b7e0a7984..0046c15d5 100644 --- a/e2e/account-sharing.spec.ts +++ b/e2e/account-sharing.spec.ts @@ -203,10 +203,19 @@ test.describe("account sharing", () => { // date mapping, and BOTH still pass with the card wired back to `null`. // Only reading the posted body proves the value a person typed is the // value the server receives, which is the half that was missing. - const lapse = ownerPage.locator('[data-slot="grant-invite-expires"]'); + // The VISIBLE overlay, not the hidden ISO mirror: `data-slot` rides + // `{...rest}` onto `DateField`'s hidden input, so the old selector resolved + // to an element that can never be visible and typed into nothing. + const lapse = ownerPage.getByTestId("grant-invite-expires-input"); await expect(lapse).toBeVisible(); const chosenDay = isoDayFromNow(30); await lapse.fill(chosenDay); + // The overlay commits on blur; without it the ISO text sits in local state + // and the hidden mirror still holds the old value when the form submits. + await lapse.blur(); + await expect( + ownerPage.locator('input[data-slot="grant-invite-expires"]'), + ).toHaveValue(chosenDay); // The level, through the real control and onto the real wire. // diff --git a/e2e/setup/global-setup.ts b/e2e/setup/global-setup.ts index 80d3765d3..b44c2ea4e 100644 --- a/e2e/setup/global-setup.ts +++ b/e2e/setup/global-setup.ts @@ -65,6 +65,61 @@ export const E2E_OWNER = { role: "USER", } as const; +/** A separate delegate session for the eight scoped-record browser journeys. */ +export const E2E_SCOPE_DELEGATE = { + email: "e2e-scope-delegate@healthlog.test", + username: "e2e-scope-delegate", + password: "Qz8!Vp4rL2nX7mKs", + role: "USER", +} as const; + +/** One non-personal target record per closed sharing domain. */ +export const E2E_SCOPE_RECORDS = [ + { + domain: "measurements", + username: "e2e-scope-measurements", + href: "/measurements", + }, + { + domain: "medications", + username: "e2e-scope-medications", + href: "/medications", + }, + { domain: "labs", username: "e2e-scope-labs", href: "/labs" }, + { domain: "profile", username: "e2e-scope-profile", href: "/profile" }, + { domain: "illness", username: "e2e-scope-illness", href: "/illness" }, + { domain: "mind", username: "e2e-scope-mind", href: "/mood" }, + { domain: "cycle", username: "e2e-scope-cycle", href: "/cycle" }, + { domain: "documents", username: "e2e-scope-documents", href: "/documents" }, +] as const; + +/** + * Whole-record grants used to prove the three adult access levels and the + * separate managed-profile boundary in a real browser session. + */ +export const E2E_LEVEL_RECORDS = [ + { + username: "e2e-level-read", + access: "READ", + recordKind: "shared", + }, + { + username: "e2e-level-write", + access: "WRITE", + recordKind: "shared", + }, + { + username: "e2e-level-manage", + access: "MANAGE", + recordKind: "shared", + }, + { + username: "e2e-level-managed", + access: "MANAGE", + recordKind: "managed", + }, +] as const; + /** * Weights seeded one per account, chosen so neither can be mistaken for the * other in rendered markup and neither collides with a value any other spec @@ -105,6 +160,47 @@ export const DELEGATE_STORAGE_STATE_PATH = resolve( "e2e/setup/storageStateDelegate.json", ); +export const SCOPE_DELEGATE_STORAGE_STATE_PATH = resolve( + process.cwd(), + "e2e/setup/storageStateScopeDelegate.json", +); + +export const SCOPE_A11Y_STORAGE_STATE_PATH = resolve( + process.cwd(), + "e2e/setup/storageStateScopeA11y.json", +); + +/** + * v1.37.0 — one jar per SWITCHING spec, for the reason spelled out at + * `DELEGATE_STORAGE_STATE_PATH` above and now applied to the scope delegate as + * well. + * + * The record-session fence made the cost visible. Four specs moved the same + * session row's record selector — the two fence specs, the cross-tab journey + * and the scoped-sharing journey — and the fence's whole subject is that row's + * context and its monotonic epoch. On one worker they never overlapped; on two + * they moved the epoch under each other, and the failure was the harness racing + * itself. Passing only at `--workers=1` is not a passing suite: CI runs two. + * + * A separate LOGIN, not a separate account: the selector lives on the session + * row, so two jars for one account are two independent contexts. The grants, + * the fixtures and the assertions all stay exactly as they were. + */ +export const FENCE_STORAGE_STATE_PATH = resolve( + process.cwd(), + "e2e/setup/storageStateFence.json", +); + +export const FENCE_OFFLINE_STORAGE_STATE_PATH = resolve( + process.cwd(), + "e2e/setup/storageStateFenceOffline.json", +); + +export const CROSS_TAB_STORAGE_STATE_PATH = resolve( + process.cwd(), + "e2e/setup/storageStateCrossTab.json", +); + async function hashPassword(password: string): Promise { return hash(password, { memoryCost: 19456, @@ -201,6 +297,163 @@ export default async function globalSetup(config: FullConfig): Promise { ], ); + // The scoped-record journey uses a dedicated delegate so its preseeded + // grants cannot affect the invitation lifecycle exercised by the sharing + // journey above. Its own module preferences are explicitly off: a target + // record's resolved scope, rather than the actor's preferences, must keep + // each granted doorway visible. + await pool.query( + `INSERT INTO users + (id, username, email, password_hash, role, created_at, updated_at, + onboarding_completed_at, onboarding_tour_completed, + module_preferences_json) + VALUES ($1, $2, $3, $4, 'USER', $5, $5, $5, true, $6::jsonb) + ON CONFLICT (username) DO UPDATE SET + email = EXCLUDED.email, + password_hash = EXCLUDED.password_hash, + updated_at = EXCLUDED.updated_at, + onboarding_completed_at = EXCLUDED.onboarding_completed_at, + onboarding_tour_completed = EXCLUDED.onboarding_tour_completed, + module_preferences_json = EXCLUDED.module_preferences_json`, + [ + cuid(), + E2E_SCOPE_DELEGATE.username, + E2E_SCOPE_DELEGATE.email, + await hashPassword(E2E_SCOPE_DELEGATE.password), + now, + JSON.stringify({ + medications: false, + labs: false, + illness: false, + mood: false, + mentalHealth: false, + inboundDocuments: false, + }), + ], + ); + + await pool.query( + `DELETE FROM account_grants + WHERE grantee_id = (SELECT id FROM users WHERE username = $1)`, + [E2E_SCOPE_DELEGATE.username], + ); + await pool.query( + `UPDATE sessions SET acting_as_user_id = NULL + WHERE user_id = (SELECT id FROM users WHERE username = $1)`, + [E2E_SCOPE_DELEGATE.username], + ); + + for (const record of E2E_SCOPE_RECORDS) { + await pool.query( + `INSERT INTO users + (id, username, email, password_hash, role, created_at, updated_at, + onboarding_completed_at, onboarding_tour_completed, + module_preferences_json) + VALUES ($1, $2, $3, NULL, 'USER', $4, $4, $4, true, $5::jsonb) + ON CONFLICT (username) DO UPDATE SET + email = EXCLUDED.email, + updated_at = EXCLUDED.updated_at, + onboarding_completed_at = EXCLUDED.onboarding_completed_at, + onboarding_tour_completed = EXCLUDED.onboarding_tour_completed, + module_preferences_json = EXCLUDED.module_preferences_json`, + [ + cuid(), + record.username, + `${record.username}@healthlog.test`, + now, + JSON.stringify({ + medications: true, + labs: true, + illness: true, + mood: true, + mentalHealth: true, + inboundDocuments: true, + }), + ], + ); + + await pool.query( + `INSERT INTO account_grants + (id, grantor_id, grantee_id, access, scope_json, invited_at, + accepted_at, created_at) + SELECT $1, owner.id, delegate.id, 'READ', $2::jsonb, $3, $3, $3 + FROM users owner, users delegate + WHERE owner.username = $4 AND delegate.username = $5`, + [ + cuid(), + JSON.stringify([record.domain]), + now, + record.username, + E2E_SCOPE_DELEGATE.username, + ], + ); + } + + // Whole-record fixtures intentionally sit beside the narrow records + // above. They make the browser prove the presentation and mutation + // boundaries of READ, WRITE, MANAGE, and a guardian-managed record + // without changing the invitation lifecycle fixture below. + for (const record of E2E_LEVEL_RECORDS) { + await pool.query( + `INSERT INTO users + (id, username, email, password_hash, role, created_at, updated_at, + managed_profile_at, onboarding_completed_at, + onboarding_tour_completed, module_preferences_json) + VALUES ($1, $2, $3, NULL, 'USER', $4, $4, $5, $4, true, $6::jsonb) + ON CONFLICT (username) DO UPDATE SET + email = EXCLUDED.email, + password_hash = NULL, + updated_at = EXCLUDED.updated_at, + managed_profile_at = EXCLUDED.managed_profile_at, + onboarding_completed_at = EXCLUDED.onboarding_completed_at, + onboarding_tour_completed = EXCLUDED.onboarding_tour_completed, + module_preferences_json = EXCLUDED.module_preferences_json`, + [ + cuid(), + record.username, + `${record.username}@healthlog.test`, + now, + record.recordKind === "managed" ? now : null, + JSON.stringify({ + medications: true, + labs: true, + illness: true, + mood: true, + mentalHealth: true, + inboundDocuments: true, + }), + ], + ); + + await pool.query( + `INSERT INTO account_grants + (id, grantor_id, grantee_id, access, scope_json, invited_at, + accepted_at, created_at) + SELECT $1, owner.id, delegate.id, $2, NULL, $3, $3, $3 + FROM users owner, users delegate + WHERE owner.username = $4 AND delegate.username = $5`, + [ + cuid(), + record.access, + now, + record.username, + E2E_SCOPE_DELEGATE.username, + ], + ); + } + + // Cycle is delegated to its profile gate rather than module preferences. + await pool.query( + `INSERT INTO cycle_profiles + (id, user_id, cycle_tracking_enabled, created_at, updated_at) + SELECT $1, u.id, true, $2, $2 + FROM users u WHERE u.username = $3 + ON CONFLICT (user_id) DO UPDATE SET + cycle_tracking_enabled = true, + updated_at = EXCLUDED.updated_at`, + [cuid(), now, "e2e-scope-cycle"], + ); + // Repeated local E2E runs reuse the same seeded account and database. // Its owner-scoped share-link bucket lasts an hour, so otherwise the third // run can start above the 20-operation ceiling and fail before exercising @@ -281,7 +534,7 @@ export default async function globalSetup(config: FullConfig): Promise { WHERE key LIKE 'sharing:%'`, ); - // The login bucket, for the same reason. This setup signs in THREE times + // The login bucket, for the same reason. This setup signs in FOUR times // now (the shared jar, the owner, and the sharing journey's own delegate // jar), and the ceiling is five attempts per IP per quarter-hour — so two // local runs in a row would otherwise end with a 429 from the fixture @@ -313,6 +566,29 @@ export default async function globalSetup(config: FullConfig): Promise { ); } + // v1.37.0 — clear what the browser suite WRITES, so the suite can be run + // twice. + // + // The adult-WRITE journey saves a blood-pressure reading through the real + // form, and the form stamps a minute-rounded `measuredAt`. Two runs inside + // the same minute therefore collide on the natural key + // (user, type, measured_at, source) — the second save is a duplicate, and + // the journey asserts its POST succeeded. It has always been that way; it + // used to surface as a 500 and now surfaces as the 409 the route learned to + // give, which is what made it legible. A suite that cannot be re-run is a + // suite whose green is a statement about the clock. + // + // Only rows the journey itself creates: MANUAL, and carrying no + // `external_id`, which is what distinguishes them from the seeded markers + // above. + await pool.query( + `DELETE FROM measurements + WHERE user_id = (SELECT id FROM users WHERE username = $1) + AND source = 'MANUAL' + AND external_id IS NULL`, + ["e2e-level-write"], + ); + // Console (instead of structured logging) is intentional here — // global-setup runs outside the app's logging context, and the // line is useful when debugging a CI failure where the seed didn't @@ -320,31 +596,58 @@ export default async function globalSetup(config: FullConfig): Promise { console.log( `[e2e/global-setup] seeded user ${E2E_USER.username} (${E2E_USER.email})`, ); + // Log the fixture accounts in here and persist their cookie jars so specs + // can reuse them via `test.use({ storageState })`. This is the documented + // Playwright pattern (`docs/auth.md`) and the only way to avoid the + // per-spec login + rate-limit dance. + const baseURL = + config.projects[0]?.use.baseURL ?? + process.env.E2E_BASE_URL ?? + "http://localhost:3000"; + + /** + * Log one account in, clearing the login bucket first. + * + * The ceiling is FIVE attempts per IP per quarter-hour and this setup now + * signs in eight times, so clearing once before the batch is no longer + * enough — the sixth would be answered by the fixture's own 429 rather + * than by the product. Only the auth surfaces' buckets are touched, and + * only between logins this setup is itself performing. + */ + const capture = async ( + account: { username: string; password: string }, + path: string, + ): Promise => { + await pool.query(`DELETE FROM rate_limits WHERE key LIKE 'auth:%'`); + await captureAuthState(baseURL, account, path); + }; + + // The shared jar every authenticated spec reads. + await capture(E2E_USER, STORAGE_STATE_PATH); + + // v1.36.0 — the owner's jar. The sharing journey needs both sides of a + // grant signed in at once, and the login endpoint is IP-rate-limited, so + // the owner is logged in here rather than inside the spec. + await capture(E2E_OWNER, OWNER_STORAGE_STATE_PATH); + + // The delegate's own jar for that journey. Same account as the shared one, + // deliberately a different session row — see DELEGATE_STORAGE_STATE_PATH + // for what happens when the journey switches the shared row instead. + await capture(E2E_USER, DELEGATE_STORAGE_STATE_PATH); + + await capture(E2E_SCOPE_DELEGATE, SCOPE_DELEGATE_STORAGE_STATE_PATH); + await capture(E2E_SCOPE_DELEGATE, SCOPE_A11Y_STORAGE_STATE_PATH); + + // v1.37.0 — one jar per switching spec. See the block comment on + // FENCE_STORAGE_STATE_PATH: these three used to share the scope delegate's + // row with the scoped-sharing journey, which is why the suite only passed + // on a single worker. + await capture(E2E_SCOPE_DELEGATE, FENCE_STORAGE_STATE_PATH); + await capture(E2E_SCOPE_DELEGATE, FENCE_OFFLINE_STORAGE_STATE_PATH); + await capture(E2E_SCOPE_DELEGATE, CROSS_TAB_STORAGE_STATE_PATH); } finally { await pool.end(); } - - // Log the fixture accounts in here and persist their cookie jars so specs - // can reuse them via `test.use({ storageState })`. This is the documented - // Playwright pattern (`docs/auth.md`) and the only way to avoid the per-spec - // login + rate-limit dance. - const baseURL = - config.projects[0]?.use.baseURL ?? - process.env.E2E_BASE_URL ?? - "http://localhost:3000"; - - // The shared jar every authenticated spec reads. - await captureAuthState(baseURL, E2E_USER, STORAGE_STATE_PATH); - - // v1.36.0 — the owner's jar. The sharing journey needs both sides of a grant - // signed in at once, and the login endpoint is IP-rate-limited, so the owner - // is logged in here rather than inside the spec. - await captureAuthState(baseURL, E2E_OWNER, OWNER_STORAGE_STATE_PATH); - - // The delegate's own jar for that journey. Same account as the shared one, - // deliberately a different session row — see DELEGATE_STORAGE_STATE_PATH for - // what happens when the journey switches the shared row instead. - await captureAuthState(baseURL, E2E_USER, DELEGATE_STORAGE_STATE_PATH); } /** diff --git a/e2e/setup/test-helpers.ts b/e2e/setup/test-helpers.ts index 9ae2f7a18..36d6b7656 100644 --- a/e2e/setup/test-helpers.ts +++ b/e2e/setup/test-helpers.ts @@ -9,9 +9,17 @@ export { E2E_USER, E2E_OWNER, + E2E_SCOPE_DELEGATE, + E2E_SCOPE_RECORDS, + E2E_LEVEL_RECORDS, E2E_DELEGATE_MARKER_KG, E2E_OWNER_MARKER_KG, STORAGE_STATE_PATH, OWNER_STORAGE_STATE_PATH, DELEGATE_STORAGE_STATE_PATH, + SCOPE_DELEGATE_STORAGE_STATE_PATH, + SCOPE_A11Y_STORAGE_STATE_PATH, + FENCE_STORAGE_STATE_PATH, + FENCE_OFFLINE_STORAGE_STATE_PATH, + CROSS_TAB_STORAGE_STATE_PATH, } from "./global-setup"; diff --git a/e2e/v137-record-session-fence-offline.spec.ts b/e2e/v137-record-session-fence-offline.spec.ts new file mode 100644 index 000000000..be33e606b --- /dev/null +++ b/e2e/v137-record-session-fence-offline.spec.ts @@ -0,0 +1,463 @@ +import type { Page } from "@playwright/test"; + +import { expect, test } from "./setup/test"; +import { FENCE_OFFLINE_STORAGE_STATE_PATH } from "./setup/test-helpers"; + +/** + * FENCE-AC-07 — the disk layers cannot serve one record's bytes inside + * another. + * + * ## Why this is a spec of its own + * + * `playwright.config.ts` sets `serviceWorkers: "block"` for every other + * project, and for a good reason written there: a worker-originated `fetch` is + * not subject to `page.route`, so the SW would bypass the per-spec route mocks. + * + * This case is the exact inverse. Its whole claim is about what the + * `healthlog-data-*` cache can and cannot serve across a record switch — and + * with the worker blocked that cache is never populated at all, so the positive + * control ("the owner snapshot really WAS on disk before the switch") could + * never pass no matter what the application did. A check that cannot pass is + * worse than no check, so this runs in the `chromium-service-worker` project + * with the worker allowed, and uses no route mocks. + * + * ## What is asserted, in order + * + * 1. the owner's record reads really are on disk before the switch — a test + * that seeded nothing proves nothing; + * 2. only current-version caches exist, which is the eviction the fence's + * design relies on to make "a response with no echo" safe; + * 3. the switch empties the data cache before it reloads; + * 4. every response now on disk carries the TARGET's record scope and never + * the previous one — the claim, read off the disk layer itself rather + * than inferred from what a page did or did not paint; + * 5. and offline, that cache is what answers a record read, still carrying + * the target's context. The non-empty scope set is the paired positive + * control: an empty cache would satisfy "no previous-record bytes" while + * proving nothing. + */ + +/** + * Pages whose reads the service worker is configured to hold on disk + * (`sw-cache-policy.ts` allowlists `/api/dashboard/snapshot`, + * `/api/measurements` and `/api/medications`). Visited rather than fetched, so + * the requests are the app's own and carry the record-session assertion. + */ +const WARM_PAGES = ["/", "/measurements", "/medications"] as const; + +/** + * A WHOLE-record READ grant, deliberately not a narrowed one. + * + * The narrowed `e2e-scope-labs` fixture opens only its labs section, so the + * nav drops the other destinations and the record reads this file drives are + * refused as out-of-scope before the fence ever matters. Section narrowing is + * `v137-sharing-managed-profiles.spec.ts`'s subject; here it is noise that + * hides the thing under test. + */ +const TARGET_USERNAME = "e2e-level-read"; + +async function openSwitcher(page: Page) { + await page.getByRole("button", { name: "User menu" }).first().click(); + await page.locator('[data-slot="account-switcher-trigger"]').click(); +} + +/** + * Enter the target record, retrying the click if the switch did not land. + * + * See the twin in `v137-record-session-fence.spec.ts`: the switch is a + * compare-and-set the client retries exactly once, and a harness driving it at + * machine speed can legitimately lose twice. Bounded, and a third failure is a + * real one. + */ +async function enterRecord(page: Page) { + const banner = page.locator('[data-slot="shared-record-banner"]'); + for (let attempt = 0; attempt < 3; attempt++) { + await openSwitcher(page); + const entry = page.locator( + `[data-slot="account-switcher-entry"][data-account-username="${TARGET_USERNAME}"]`, + ); + await expect(entry).toBeVisible(); + const accountId = await entry.getAttribute("data-account-id"); + await entry.click(); + try { + await expect(banner).toBeVisible({ timeout: 15_000 }); + return accountId; + } catch { + await page.goto("/"); + await expectShellReady(page); + } + } + await expect(banner).toBeVisible({ timeout: 15_000 }); + return null; +} + +async function expectShellReady(page: Page) { + await expect( + page.locator('[data-slot="record-scope-hydration-gate"]'), + // A real budget, not the 10 s default. The gate IS the cross-tab hold: a + // peer sits behind it until its own `/api/auth/me` resolves, and that read + // queues behind whatever the other worker's browser is doing. Ten seconds + // is a statement about machine load; thirty is about the hold actually + // never releasing, which is the failure worth reporting. + ).toHaveCount(0, { timeout: 30_000 }); + await expect(page.getByRole("button", { name: "User menu" })).toBeVisible({ + timeout: 30_000, + }); +} + +/** What the `healthlog-data-*` caches hold right now. */ +async function dataCacheState(page: Page) { + return page.evaluate(async () => { + const names = await caches.keys(); + const data = names.filter((n) => n.startsWith("healthlog-data-")); + let entries = 0; + for (const name of data) { + entries += (await (await caches.open(name)).keys()).length; + } + return { names, dataCaches: data, entries }; + }); +} + +/** + * The record context every cached response was stored WITH. + * + * This is the disk-layer claim, read off the disk layer. A cached record + * response carries the echo it was served under, so if a previous record's + * bytes were still reachable they would show up here as that record's scope. + */ +async function cachedScopes(page: Page): Promise { + return page.evaluate(async () => { + const scopes: string[] = []; + for (const name of (await caches.keys()).filter((n) => + n.startsWith("healthlog-data-"), + )) { + const cache = await caches.open(name); + for (const request of await cache.keys()) { + const cached = await cache.match(request); + const scope = cached?.headers.get("x-healthlog-record-scope"); + if (scope) scopes.push(scope); + } + } + return scopes; + }); +} + +test.describe.serial("FENCE-AC-07 record-fence disk layers", () => { + test("FENCE-AC-07 the disk layers cannot serve one record's bytes inside another", async ({ + browser, + }) => { + const context = await browser.newContext({ + storageState: FENCE_OFFLINE_STORAGE_STATE_PATH, + }); + const page = await context.newPage(); + + try { + await page.goto("/"); + // The gate is spelled out here rather than left inside a helper because + // the next act is a one-shot `page.evaluate`, and + // `src/__tests__/e2e-measurement-gating-guard.test.ts` reads the source + // text between the navigation and the read. + await expect( + page.getByRole("button", { name: "User menu" }), + ).toBeVisible(); + await expectShellReady(page); + // The session is shared with the other sharing journeys, so "not inside + // a record" is not something a fresh page can assume. + const banner = page.locator('[data-slot="shared-record-banner"]'); + if ((await banner.count()) > 0) { + await page.locator('[data-slot="shared-record-banner-exit"]').click(); + await expect(banner).toHaveCount(0); + } + + // Wait for the worker to control the page — until it does, nothing it + // would cache is going through it. + await page.waitForFunction( + () => navigator.serviceWorker?.controller !== null, + undefined, + { timeout: 20_000 }, + ); + + // Warm the disk cache by VISITING the pages, not by issuing bare + // fetches. A bare `fetch` in `page.evaluate` carries no record-session + // assertion, so on a session that has been inside a shared record the + // fence refuses it and nothing cacheable comes back — the cache filled + // from the app's own reads instead, and those were the ones that happened + // to resolve no record scope. Driving the app is what puts real, + // echo-carrying record responses on disk. + for (const path of WARM_PAGES) { + await page.goto(path); + // Spelled out rather than left inside `expectShellReady`, because the + // next act is a one-shot `page.evaluate` and + // `src/__tests__/e2e-measurement-gating-guard.test.ts` reads the source + // text between the navigation and the read. A gate hidden in a helper + // is invisible to that guard and to the next reader. + await expect( + page.getByRole("button", { name: "User menu" }), + ).toBeVisible(); + await expectShellReady(page); + } + await expect + .poll(async () => (await dataCacheState(page)).entries, { + timeout: 15_000, + }) + .toBeGreaterThan(0); + + // (1) POSITIVE CONTROL: the owner's bytes really are on disk. + const seeded = await dataCacheState(page); + expect(seeded.dataCaches.length).toBeGreaterThan(0); + expect(seeded.entries).toBeGreaterThan(0); + + // (2) Only current-version caches survive activation. This is the + // eviction the fence's "a response with no echo is served normally" rule + // depends on: a PRE-fence cached response carries no echo, and it can + // only be served if its cache is still there. + const version = seeded.dataCaches[0].replace("healthlog-data-", ""); + const stale = seeded.names.filter( + (n) => + /^healthlog-(static|pages|data)-/.test(n) && !n.endsWith(version), + ); + expect(stale).toEqual([]); + + await openSwitcher(page); + const entry = page.locator( + `[data-slot="account-switcher-entry"][data-account-username="${TARGET_USERNAME}"]`, + ); + await expect(entry).toBeVisible(); + const targetAccountId = await entry.getAttribute("data-account-id"); + expect(targetAccountId).not.toBeNull(); + await entry.click(); + await expect( + page.locator('[data-slot="shared-record-banner"]'), + ).toBeVisible(); + + // (3) NOT asserted: "the data cache is momentarily empty". + // + // The switch does wipe the disk layers — `clearOfflineCachesForRecordSwitch` + // runs before the reload — but the reload immediately refills them from + // the target's own reads, so an empty cache is a state that exists for a + // few milliseconds and cannot be sampled reliably. Polling for it made + // this case fail for a reason that had nothing to do with the claim. + // + // The claim is asserted directly below instead, and it is the stronger + // statement anyway: what matters is not that the cache was briefly empty + // but that nothing in it belongs to the previous record. + + // Visit inside the TARGET record so there is something of its own on + // disk to find. + for (const path of WARM_PAGES) { + await page.goto(path); + // Spelled out rather than left inside `expectShellReady`, because the + // next act is a one-shot `page.evaluate` and + // `src/__tests__/e2e-measurement-gating-guard.test.ts` reads the source + // text between the navigation and the read. A gate hidden in a helper + // is invisible to that guard and to the next reader. + await expect( + page.getByRole("button", { name: "User menu" }), + ).toBeVisible(); + await expectShellReady(page); + } + await expect + .poll(async () => (await dataCacheState(page)).entries, { + timeout: 15_000, + }) + .toBeGreaterThan(0); + + // (4) + (5) The disk layer now holds the TARGET's bytes and only those, + // read off the disk layer itself. + // + // The first draft asserted the shared-record banner after an offline + // reload. That could never pass and had nothing to do with the claim: + // `/api/auth/me` is not a cached read, so offline it fails, `useAuth` + // errors and the shell paints its offline gate rather than a banner. The + // banner's absence says nothing about what is on disk. + // + // What IS on disk is checkable, and it is the whole point: every cached + // record response carries the echo it was served under, so a previous + // record's bytes surviving the switch would appear here as that record's + // scope. + const scopes = await cachedScopes(page); + // POSITIVE CONTROL: something really is cached with a context, so the + // assertion below is about a non-empty set. + expect(scopes.length).toBeGreaterThan(0); + for (const scope of scopes) { + expect(scope).toBe(targetAccountId); + } + expect(scopes).not.toContain("self"); + + // And the target's own record read really is one of the entries on disk, + // so the scope assertion above is about the reads that matter rather + // than about some incidental cached response. + const cachedUrls = await page.evaluate(async () => { + const urls: string[] = []; + for (const name of (await caches.keys()).filter((n) => + n.startsWith("healthlog-data-"), + )) { + for (const request of await (await caches.open(name)).keys()) { + urls.push(new URL(request.url).pathname); + } + } + return urls; + }); + expect(cachedUrls).toContain("/api/measurements"); + + // NOT asserted: that a bare `fetch` from page context can read those + // bytes back while offline. Since v1.37.0 an echoing response also + // carries `Vary: x-healthlog-record-epoch, x-healthlog-record-scope`, and + // the Cache API honours `Vary` — so a request WITHOUT the fence headers + // does not match the stored entry at all. That is the correct behaviour + // and a second line of defence worth naming: bytes cached under one + // record context cannot be retrieved by a request asserting a different + // one, or none. The app's own reads carry the headers and do match. + } finally { + await context.setOffline(false); + // Hand the shared session back to its own record; see the note in + // `v137-record-session-fence.spec.ts`. + await context.request + .post("/api/account/switch", { data: { accountId: null } }) + .catch(() => {}); + await context.close(); + } + }); + + test("FENCE-AC-07 a cached record response cannot be SERVED under a different context", async ({ + browser, + }) => { + /** + * The serving path, which the case above does not exercise. + * + * The test above asserts what is IN the cache. That is worth asserting and + * it is not the same claim: an entry can sit on disk under one context and + * still be handed to a request made under another, and on the EXTERNAL + * switch path nothing wipes it — the app never initiated the switch, so + * `clearOfflineCachesForRecordSwitch` never runs. On that path `Vary` is + * the only thing standing between a cached delegated response and the next + * request for the same URL. + * + * So this asks the Cache API the question a service worker asks it: + * `cache.match(request)` honours the stored response's `Vary`, so a request + * whose fence headers differ from the ones the entry was stored with does + * not match at all. Remove the `Vary` block from `attachRecordContextEcho` + * and the second lookup starts hitting. + */ + const context = await browser.newContext({ + storageState: FENCE_OFFLINE_STORAGE_STATE_PATH, + }); + const page = await context.newPage(); + + try { + await page.goto("/"); + await expect( + page.getByRole("button", { name: "User menu" }), + ).toBeVisible(); + await expectShellReady(page); + const banner = page.locator('[data-slot="shared-record-banner"]'); + if ((await banner.count()) > 0) { + await page.locator('[data-slot="shared-record-banner-exit"]').click(); + await expect(banner).toHaveCount(0); + } + await page.waitForFunction( + () => navigator.serviceWorker?.controller !== null, + undefined, + { timeout: 20_000 }, + ); + + // Enter the record and let its reads land on disk. + await enterRecord(page); + await page.goto("/measurements"); + await expect( + page.getByRole("button", { name: "User menu" }), + ).toBeVisible(); + await expectShellReady(page); + + // The context those bytes were stored under, read off the entry itself. + const stored = await page.evaluate(async () => { + for (const name of (await caches.keys()).filter((n) => + n.startsWith("healthlog-data-"), + )) { + const cache = await caches.open(name); + for (const request of await cache.keys()) { + if (!new URL(request.url).pathname.startsWith("/api/measurements")) + continue; + const response = await cache.match(request); + if (!response) continue; + return { + cacheName: name, + url: request.url, + vary: response.headers.get("Vary"), + epoch: response.headers.get("x-healthlog-record-epoch"), + scope: response.headers.get("x-healthlog-record-scope"), + }; + } + } + return null; + }); + // POSITIVE CONTROL: there really is an entry, stored with a context. + expect(stored).not.toBeNull(); + expect(stored!.epoch).toMatch(/^\d+$/); + expect(stored!.scope).not.toBe("self"); + + // The EXTERNAL switch: driven from a request context, so the app never + // runs its cache wipe and the entry above survives. + const left = await context.request.post("/api/account/switch", { + data: { accountId: null }, + }); + expect(left.status()).toBe(200); + const me = await context.request.get("/api/auth/me"); + const now = (await me.json()).data.recordSession as { + epoch: number; + scope: string | null; + }; + // The context genuinely moved, or the comparison below is vacuous. + expect(String(now.epoch)).not.toBe(stored!.epoch); + + const lookups = await page.evaluate( + async ({ + cacheName, + url, + storedEpoch, + storedScope, + nowEpoch, + nowScope, + }) => { + const cache = await caches.open(cacheName); + const ask = async (epoch: string, scope: string) => + (await cache.match( + new Request(url, { + headers: { + "x-healthlog-record-epoch": epoch, + "x-healthlog-record-scope": scope, + }, + }), + )) !== undefined; + return { + underStoredContext: await ask(storedEpoch, storedScope), + underNewContext: await ask(nowEpoch, nowScope), + withNoAssertion: + (await cache.match(new Request(url))) !== undefined, + }; + }, + { + cacheName: stored!.cacheName, + url: stored!.url, + storedEpoch: stored!.epoch!, + storedScope: stored!.scope!, + nowEpoch: String(now.epoch), + nowScope: now.scope ?? "self", + }, + ); + + // POSITIVE CONTROL: the entry IS servable to the context it belongs to. + // Without this, "the other context misses" would pass for an entry that + // was simply not there. + expect(lookups.underStoredContext).toBe(true); + // The claim. Remove the `Vary` and this starts hitting. + expect(lookups.underNewContext).toBe(false); + expect(lookups.withNoAssertion).toBe(false); + } finally { + await context.request + .post("/api/account/switch", { data: { accountId: null } }) + .catch(() => {}); + await context.close(); + } + }); +}); diff --git a/e2e/v137-record-session-fence.spec.ts b/e2e/v137-record-session-fence.spec.ts new file mode 100644 index 000000000..5868b39b6 --- /dev/null +++ b/e2e/v137-record-session-fence.spec.ts @@ -0,0 +1,615 @@ +import type { Page } from "@playwright/test"; + +import { expect, test } from "./setup/test"; +import { FENCE_STORAGE_STATE_PATH } from "./setup/test-helpers"; + +/** + * The record-session fence, in a real browser. + * + * `e2e/v137-sharing-cross-tab-session.spec.ts` proves the COOPERATIVE layer: + * a peer tab holds while the initiator's switch is in flight, because the + * initiator told it to. This file proves the cases that layer cannot reach — + * the ones where nobody tells anybody anything: + * + * * FENCE-AC-01 peers reconcile to the exact new epoch and scope, not merely + * to "something changed"; + * * FENCE-AC-02 the initiator's response never arrives, because the tab is + * torn off mid-flight; + * * FENCE-AC-03 the initiator is CLOSED after the server committed, so no + * commit broadcast is ever published; + * * FENCE-AC-06 a record response delayed across the switch is discarded by + * the client rather than painted; + * * FENCE-AC-07 the disk layers — CacheStorage and the service worker's + * `healthlog-data-*` — cannot serve one record's bytes inside another. + * + * ## Why every absence assertion here is paired + * + * "The peer never showed the owner's data" passes on a page that is + * permanently wedged, which is a worse outcome than the bug. So each of those + * legs additionally asserts the peer REACHES a usable state on the target + * scope once `/me` resolves. An absence-only assertion in this file would be a + * check that cannot fail. + * + * Uses the existing synthetic `e2e/setup` fixtures; introduces no new account + * anchors. + */ + +const RECORD_READS = [ + "/api/dashboard/snapshot", + "/api/insights/targets", + "/api/medications", + "/api/medications/compliance", + "/api/labs", +] as const; + +const EPOCH_HEADER = "x-healthlog-record-epoch"; +const SCOPE_HEADER = "x-healthlog-record-scope"; + +/** + * A WHOLE-record READ grant, deliberately not a narrowed one. + * + * The narrowed `e2e-scope-labs` fixture opens only its labs section, so the + * nav drops the other destinations and the record reads this file drives are + * refused as out-of-scope before the fence ever matters. Section narrowing is + * `v137-sharing-managed-profiles.spec.ts`'s subject; here it is noise that + * hides the thing under test. + */ +const TARGET_USERNAME = "e2e-level-read"; + +async function openSwitcher(page: Page) { + await page.getByRole("button", { name: "User menu" }).first().click(); + await page.locator('[data-slot="account-switcher-trigger"]').click(); +} + +function switcherEntry(page: Page, username = TARGET_USERNAME) { + return page.locator( + `[data-slot="account-switcher-entry"][data-account-username="${username}"]`, + ); +} + +function deferred() { + let release!: () => void; + const promise = new Promise((resolve) => { + release = resolve; + }); + return { promise, release }; +} + +/** + * Leave any record this session is already inside. + * + * These specs each begin by switching INTO a record, so they need to start + * outside one — and the session is shared with the other sharing journeys, so + * "outside one" is not something a fresh page can assume. Without this the + * file passes in isolation and fails whenever it runs after a spec that left + * the session switched, which is the least useful kind of failure. + */ +async function ensureOwnRecord(page: Page) { + const banner = page.locator('[data-slot="shared-record-banner"]'); + if ((await banner.count()) === 0) return; + await page.locator('[data-slot="shared-record-banner-exit"]').click(); + await expect(banner).toHaveCount(0, { timeout: 30_000 }); + // And WAIT for the reload to finish. + // + // Leaving a record is a full document navigation, and the banner vanishing + // only means that navigation started. Returning there hands the next step a + // page that has not re-read `/api/auth/me` yet — so its adopted epoch is + // still `bootstrap`, its switch sends `expectedEpoch: 0`, the compare-and-set + // loses against the epoch the exit itself just moved, and the switch quietly + // does not happen. The symptom is a banner that never appears, thirty + // seconds later, with nothing wrong in the product. + await expectShellReady(page); +} + +/** + * Enter the target record, retrying the click if the switch did not land. + * + * Not a workaround for a flaky product — a fixture acknowledging a contract the + * product chose deliberately. `POST /api/account/switch` is a compare-and-set + * on the session's record epoch, and the client reconciles and retries exactly + * ONCE; a second lost CAS surfaces as a failed toast rather than being retried + * forever, which is the right call for a person at a keyboard. + * + * These tests are not a person at a keyboard. Some drive two tabs of ONE + * session at machine speed while the peer is reloading and re-adopting, which + * is precisely the "the browser is racing itself" case that contract names — + * so the switcher click legitimately loses sometimes, and the product is + * behaving as designed when it does. The fixture re-presses the button, which + * is what the person would do. + * + * Bounded and asserted: if three attempts cannot enter the record, that is a + * real failure and it fails. + */ +async function enterRecord(page: Page, username = TARGET_USERNAME) { + const banner = page.locator('[data-slot="shared-record-banner"]'); + let accountId: string | null = null; + for (let attempt = 0; attempt < 3; attempt++) { + await openSwitcher(page); + const entry = switcherEntry(page, username); + await expect(entry).toBeVisible(); + accountId ??= await entry.getAttribute("data-account-id"); + await entry.click(); + try { + await expect(banner).toBeVisible({ timeout: 15_000 }); + return accountId; + } catch { + // The switcher menu may still be open over the page; start clean. + await page.goto("/"); + await expectShellReady(page); + } + } + await expect(banner).toBeVisible({ timeout: 15_000 }); + return accountId; +} + +/** Wait for the shell to be usable again — the paired positive control. */ +async function expectShellReady(page: Page) { + await expect( + page.locator('[data-slot="record-scope-hydration-gate"]'), + // A real budget, not the 10 s default. The gate IS the cross-tab hold: a + // peer sits behind it until its own `/api/auth/me` resolves, and that read + // queues behind whatever the other worker's browser is doing. Ten seconds + // is a statement about machine load; thirty is about the hold actually + // never releasing, which is the failure worth reporting. + ).toHaveCount(0, { timeout: 30_000 }); + await expect(page.getByRole("button", { name: "User menu" })).toBeVisible({ + timeout: 30_000, + }); +} + +/** + * The furthest-along assertion in a collected set. + * + * The epoch is monotonic, so the highest one is the freshest context this + * browser has reached — independent of the order the requests happened to be + * observed in. + */ +function latestAssertion( + collected: { epoch: string; scope: string }[], +): { epoch: string; scope: string } | null { + let best: { epoch: string; scope: string } | null = null; + for (const assertion of collected) { + if (!/^\d+$/.test(assertion.epoch)) continue; + if (best === null || Number(assertion.epoch) > Number(best.epoch)) { + best = assertion; + } + } + return best; +} + +/** The record context a page asserted on its most recent record read. */ +function collectAssertions( + page: Page, + into: { epoch: string; scope: string }[], +) { + page.on("request", (request) => { + const path = new URL(request.url()).pathname; + if (!(RECORD_READS as readonly string[]).includes(path)) return; + const headers = request.headers(); + const epoch = headers[EPOCH_HEADER]; + const scope = headers[SCOPE_HEADER]; + if (epoch === undefined || scope === undefined) return; + into.push({ epoch, scope }); + }); +} + +test.describe.serial("FENCE record-session fence in the browser", () => { + test("FENCE-AC-01 a peer reconciles to the exact new epoch and scope, in both directions", async ({ + browser, + }) => { + // Two full switches, two peer reloads each, and a poll that reloads until + // the server write is visible to the second tab. That does not fit the + // 30 s default, and a test that times out reports a failure that says + // nothing about the fence. + test.setTimeout(120_000); + const context = await browser.newContext({ + storageState: FENCE_STORAGE_STATE_PATH, + }); + const initiator = await context.newPage(); + const peer = await context.newPage(); + const peerAssertions: { epoch: string; scope: string }[] = []; + collectAssertions(peer, peerAssertions); + + try { + await Promise.all([initiator.goto("/"), peer.goto("/")]); + await expectShellReady(peer); + await expectShellReady(initiator); + await ensureOwnRecord(initiator); + await peer.reload(); + await expectShellReady(peer); + + const accountId = await enterRecord(initiator); + expect(accountId).not.toBeNull(); + + // Cleared HERE, not before the click: between the click and the reload + // the peer is still issuing reads under the context it had, and counting + // those made the assertion below compare a pre-switch `self` against the + // post-switch account. Only what the peer asserts once it has reconciled + // is evidence about reconciliation. + peerAssertions.length = 0; + + // The peer reaches a usable shell again — the positive control, without + // which every assertion below would pass on a wedged page. + await peer.reload(); + await expectShellReady(peer); + await expect( + peer.locator('[data-slot="shared-record-banner"]'), + ).toBeVisible({ timeout: 30_000 }); + + // And every record read it now issues names the record it is actually + // in, not "something". A peer that reconciled to a guess would show up + // here as a scope naming the wrong account or a stale epoch. + // The HIGHEST-epoch assertion, not every collected one. + // + // The epoch is monotonic, and requests do not arrive in the order they + // were issued — a read from the page before the reload can be observed + // after one from the page after it. Requiring EVERY collected assertion + // to name the new record is therefore a claim about network ordering + // rather than about reconciliation, and it fails intermittently for a + // reason that has nothing to do with the fence. What reconciliation + // means is that the peer CONVERGES on the new context, which is exactly + // the highest epoch it ever asserts. + const inRecord = latestAssertion(peerAssertions); + expect(inRecord).not.toBeNull(); + expect(inRecord!.scope).toBe(accountId); + const inRecordEpoch = Number(inRecord!.epoch); + expect(inRecordEpoch).toBeGreaterThan(0); + + // ── and the same on the way out ─────────────────────────────────── + await initiator + .locator('[data-slot="shared-record-banner-exit"]') + .click(); + await expect( + initiator.locator('[data-slot="shared-record-banner"]'), + ).toHaveCount(0, { timeout: 30_000 }); + + peerAssertions.length = 0; + // Polled with reloads rather than reloaded once. The initiator's banner + // disappearing means its own document navigated, not that the server + // write has landed and become visible to another tab — asserting on one + // reload made this leg a race against the switch-out commit. + await expect + .poll( + async () => { + await peer.reload(); + await expectShellReady(peer); + return peer.locator('[data-slot="shared-record-banner"]').count(); + }, + { timeout: 40_000 }, + ) + .toBe(0); + + const backHome = latestAssertion(peerAssertions); + expect(backHome).not.toBeNull(); + expect(backHome!.scope).toBe("self"); + // Leaving moved the epoch too: a tab believing it is still inside the + // record is the same defect as the reverse. + expect(Number(backHome!.epoch)).toBeGreaterThan(inRecordEpoch); + } finally { + // Hand the shared session back to its own record. + // + // These fixtures' storage state is shared with the other sharing + // journeys, and every test here deliberately moves the session's record + // selector. Leaving it switched makes the NEXT spec fail for a reason + // that has nothing to do with it — which is how a green file and a red + // suite happen at the same time. + await context.request + .post("/api/account/switch", { data: { accountId: null } }) + .catch(() => {}); + await context.close(); + } + }); + + test("FENCE-AC-02 an aborted switch response leaves peers correct, and they still reach ready", async ({ + browser, + }) => { + const context = await browser.newContext({ + storageState: FENCE_STORAGE_STATE_PATH, + }); + const initiator = await context.newPage(); + const peer = await context.newPage(); + const peerAssertions: { epoch: string; scope: string }[] = []; + collectAssertions(peer, peerAssertions); + + try { + await Promise.all([initiator.goto("/"), peer.goto("/")]); + await expectShellReady(peer); + await expectShellReady(initiator); + await ensureOwnRecord(initiator); + + // Drive the switch server-side and abort the response on the way back: + // the write lands, the initiator never learns it did, and no commit + // broadcast is ever published. + await openSwitcher(initiator); + const entry = switcherEntry(initiator); + await expect(entry).toBeVisible(); + const accountId = await entry.getAttribute("data-account-id"); + expect(accountId).not.toBeNull(); + + await initiator.route("**/api/account/switch", async (route) => { + await route.fetch(); + await route.abort(); + }); + await entry.click({ noWaitAfter: true }); + await initiator.waitForTimeout(500); + + // The peer must never paint owner-ready state it guessed at … + const guessed = peerAssertions.filter( + (a) => a.scope === accountId && a.epoch === "bootstrap", + ); + expect(guessed).toEqual([]); + + // … and must still REACH ready on the target scope once `/me` answers. + // Without this the leg above would pass on a permanently wedged page. + await peer.reload(); + await expectShellReady(peer); + await expect( + peer.locator('[data-slot="shared-record-banner"]'), + ).toBeVisible(); + expect(peerAssertions.some((a) => a.scope === accountId)).toBe(true); + } finally { + // Hand the shared session back to its own record. + // + // These fixtures' storage state is shared with the other sharing + // journeys, and every test here deliberately moves the session's record + // selector. Leaving it switched makes the NEXT spec fail for a reason + // that has nothing to do with it — which is how a green file and a red + // suite happen at the same time. + await context.request + .post("/api/account/switch", { data: { accountId: null } }) + .catch(() => {}); + await context.close(); + } + }); + + test("FENCE-AC-03 a peer reaches ready after the initiator is closed post-commit", async ({ + browser, + }) => { + const context = await browser.newContext({ + storageState: FENCE_STORAGE_STATE_PATH, + }); + const initiator = await context.newPage(); + const peer = await context.newPage(); + + try { + await Promise.all([initiator.goto("/"), peer.goto("/")]); + await expectShellReady(peer); + await expectShellReady(initiator); + await ensureOwnRecord(initiator); + await peer.reload(); + await expectShellReady(peer); + + await openSwitcher(initiator); + const entry = switcherEntry(initiator); + await expect(entry).toBeVisible(); + + // Hold the response, close the tab while it is in flight. The server has + // committed; nothing will ever broadcast the commit. + const held = deferred(); + await initiator.route("**/api/account/switch", async (route) => { + await route.fetch(); + await held.promise; + await route.abort(); + }); + await entry.click({ noWaitAfter: true }); + await initiator.waitForTimeout(300); + held.release(); + await initiator.close(); + + // The surviving tab is not permanently gated: `/me` is the bootstrap and + // it answers regardless of who began the transition. + await peer.reload(); + await expectShellReady(peer); + } finally { + // Hand the shared session back to its own record. + // + // These fixtures' storage state is shared with the other sharing + // journeys, and every test here deliberately moves the session's record + // selector. Leaving it switched makes the NEXT spec fail for a reason + // that has nothing to do with it — which is how a green file and a red + // suite happen at the same time. + await context.request + .post("/api/account/switch", { data: { accountId: null } }) + .catch(() => {}); + await context.close(); + } + }); + + test("FENCE-AC-04 an external switch refuses the stale peer's next record read", async ({ + browser, + }) => { + const context = await browser.newContext({ + storageState: FENCE_STORAGE_STATE_PATH, + }); + const peer = await context.newPage(); + // Collected from the very start: the refusal can land on a poll the app + // issues by itself, before this test navigates anywhere. + const refusals: string[] = []; + peer.on("response", (response) => { + const path = new URL(response.url()).pathname; + if (response.status() !== 409) return; + // ANY refused API read, not one of a named five. Which cells a page + // mounts is a rendering decision that moves with the product, and a + // hard-coded list turns "the fence refused the stale tab" into "the + // fence refused one of the paths I happened to name" — which is how + // this case first passed while observing nothing. + if (!path.startsWith("/api/")) return; + refusals.push(path); + }); + + try { + await peer.goto("/"); + await expectShellReady(peer); + await ensureOwnRecord(peer); + + // A switch driven from a request context that runs no client journal at + // all — the "raw or external" case the cooperative layer cannot see. + await openSwitcher(peer); + const entry = switcherEntry(peer); + await expect(entry).toBeVisible(); + const accountId = await entry.getAttribute("data-account-id"); + expect(accountId).not.toBeNull(); + await peer.keyboard.press("Escape"); + + const response = await context.request.post("/api/account/switch", { + data: { accountId }, + }); + expect(response.status()).toBe(200); + + // The tab is now asserting a context the row has left — but observing + // the refusal takes one deliberate step, and the reason is worth stating + // because it is a property of the design rather than a quirk of the test. + // + // `GET /api/auth/me` is UNFENCED (it is the reconciliation bootstrap and + // must answer while the context is unknown) and it is one of the two + // responses a client may ADOPT from. So whenever the app happens to + // re-read `/me` before it reads a record, it silently adopts the new + // context and no stale assertion is ever sent. That is correct — and it + // means a test that just navigates observes nothing, which is how the + // first draft of this case passed while proving nothing. + // + // The window the fence exists for is the opposite ordering: a suspended + // peer whose RECORD read goes out before any reconciliation. Holding + // `/me` produces exactly that ordering, deterministically. + // Held exactly once, and never unrouted: calling `unroute` while a + // handler is still parked inside its `await` abandons that route, and + // the `continue()` on the far side then throws "Route is already + // handled". Letting the handler fall through after the first hold keeps + // one owner for the route from start to finish. + const heldMe: { release: (() => void) | null; done: boolean } = { + release: null, + done: false, + }; + await peer.route("**/api/auth/me", async (route) => { + if (heldMe.done) { + await route.continue(); + return; + } + heldMe.done = true; + await new Promise((resolve) => { + heldMe.release = resolve; + }); + await route.continue(); + }); + + const soft = peer.locator('aside[aria-label="Sidebar"]'); + await expect(soft).toBeVisible(); + await soft.getByRole("link", { name: "Measurements" }).click(); + + await expect + .poll(() => refusals.length, { timeout: 20_000 }) + .toBeGreaterThan(0); + + // Let the reconciliation through and confirm the tab recovers rather + // than sitting on the refusal. + heldMe.release?.(); + + // Reconciled, not evicted: the tab ends up INSIDE the record the + // external switch put it in, with a usable shell. + await expect( + peer.locator('[data-slot="shared-record-banner"]'), + ).toBeVisible({ timeout: 20_000 }); + await expectShellReady(peer); + } finally { + // Hand the shared session back to its own record. + // + // These fixtures' storage state is shared with the other sharing + // journeys, and every test here deliberately moves the session's record + // selector. Leaving it switched makes the NEXT spec fail for a reason + // that has nothing to do with it — which is how a green file and a red + // suite happen at the same time. + await context.request + .post("/api/account/switch", { data: { accountId: null } }) + .catch(() => {}); + await context.close(); + } + }); + + test("FENCE-AC-06 a record response delayed across the switch is discarded", async ({ + browser, + }) => { + const context = await browser.newContext({ + storageState: FENCE_STORAGE_STATE_PATH, + }); + const page = await context.newPage(); + + try { + // The intercept goes on BEFORE the first navigation. Installed after the + // page had already loaded, it matched nothing — the only request for that + // path had come and gone, and the case passed while observing nothing. + // + // And it holds `/api/measurements`, a FENCED record read. The first + // version held `/api/auth/me` because that one is alive on every boot — + // but `/me` is an actor surface, the fence never runs on it, and it + // therefore echoes no record context at all. Holding it captured a null + // echo forever, which is the same "observed nothing" failure wearing a + // different path. + const held: { + armed: boolean; + release: (() => void) | null; + epoch: string | null; + scope: string | null; + } = { armed: false, release: null, epoch: null, scope: null }; + + await page.route("**/api/measurements**", async (route) => { + if (!held.armed) { + await route.continue(); + return; + } + held.armed = false; + const response = await route.fetch(); + held.epoch = response.headers()["x-healthlog-record-epoch"] ?? null; + held.scope = response.headers()["x-healthlog-record-scope"] ?? null; + await new Promise((resolve) => { + held.release = resolve; + }); + await route.fulfill({ response }).catch(() => {}); + }); + + await page.goto("/"); + await expectShellReady(page); + await ensureOwnRecord(page); + + const accountId = await enterRecord(page); + + // Arm the hold, then make the app issue the read inside the record. + held.armed = true; + const sidebar = page.locator('aside[aria-label="Sidebar"]'); + await expect(sidebar).toBeVisible(); + await sidebar + .getByRole("link", { name: "Measurements" }) + .click({ noWaitAfter: true }); + + await expect + .poll(() => held.release !== null, { timeout: 20_000 }) + .toBe(true); + + // The response was admitted under the record's context and says so. + // Proof the intercept saw a real fenced read rather than matching + // nothing, which is the whole failure mode this case kept falling into. + expect(held.epoch).toMatch(/^\d+$/); + expect(held.scope).toBe(accountId); + + // Now move the context out from under it, and only then let it land. + const leaving = page + .locator('[data-slot="shared-record-banner-exit"]') + .click({ noWaitAfter: true }); + await page.waitForTimeout(500); + held.release?.(); + await leaving.catch(() => {}); + + // The page settles on the record it is actually in — its own — without + // painting the one the held response described. + await expect( + page.locator('[data-slot="shared-record-banner"]'), + ).toHaveCount(0, { timeout: 20_000 }); + await expectShellReady(page); + } finally { + // Hand the shared session back to its own record. + await context.request + .post("/api/account/switch", { data: { accountId: null } }) + .catch(() => {}); + await context.close(); + } + }); +}); diff --git a/e2e/v137-sharing-cross-tab-session.spec.ts b/e2e/v137-sharing-cross-tab-session.spec.ts new file mode 100644 index 000000000..e4058bf4a --- /dev/null +++ b/e2e/v137-sharing-cross-tab-session.spec.ts @@ -0,0 +1,116 @@ +import type { Page } from "@playwright/test"; + +import { expect, test } from "./setup/test"; +import { CROSS_TAB_STORAGE_STATE_PATH } from "./setup/test-helpers"; + +async function openSwitcher(page: Page) { + await page.getByRole("button", { name: "User menu" }).first().click(); + await page.locator('[data-slot="account-switcher-trigger"]').click(); +} + +function deferred() { + let release!: () => void; + const promise = new Promise((resolve) => { + release = resolve; + }); + return { promise, release }; +} + +test.describe.serial("cross-tab shared-record session switches", () => { + test("holds a peer tab before both switch-in and switch-out change the shared session", async ({ + browser, + }) => { + const context = await browser.newContext({ + storageState: CROSS_TAB_STORAGE_STATE_PATH, + }); + const switchingPage = await context.newPage(); + const peerPage = await context.newPage(); + const pausedSwitches: ReturnType[] = []; + const peerRecordReads: string[] = []; + + await switchingPage.route("**/api/account/switch", async (route) => { + const pause = pausedSwitches.shift(); + if (pause) await pause.promise; + await route.continue(); + }); + peerPage.on("request", (request) => { + if (request.method() !== "GET") return; + const path = new URL(request.url()).pathname; + if ( + [ + "/api/dashboard/snapshot", + "/api/insights/targets", + "/api/medications", + "/api/medications/compliance", + "/api/labs", + "/api/labs/ocr/capability", + "/api/insights/coach/nudge-status", + ].includes(path) + ) { + peerRecordReads.push(path); + } + }); + + try { + await Promise.all([switchingPage.goto("/"), peerPage.goto("/")]); + await expect( + peerPage.getByRole("button", { name: "User menu" }), + ).toBeVisible(); + + await openSwitcher(switchingPage); + const entry = switchingPage.locator( + '[data-slot="account-switcher-entry"][data-account-username="e2e-scope-labs"]', + ); + await expect(entry).toBeVisible(); + const switchIn = deferred(); + pausedSwitches.push(switchIn); + peerRecordReads.length = 0; + const switchInClick = entry.click({ noWaitAfter: true }); + + await expect( + peerPage.locator('[data-slot="record-scope-hydration-gate"]'), + ).toBeVisible(); + await expect( + peerPage.getByRole("button", { name: "User menu" }), + ).toHaveCount(0); + await peerPage.waitForTimeout(200); + expect(peerRecordReads).toEqual([]); + + switchIn.release(); + await switchInClick; + await expect( + switchingPage.locator('[data-slot="shared-record-banner"]'), + ).toBeVisible(); + await expect( + peerPage.locator('[data-slot="record-scope-hydration-gate"]'), + ).toHaveCount(0); + + const switchOut = deferred(); + pausedSwitches.push(switchOut); + peerRecordReads.length = 0; + const switchOutClick = switchingPage + .locator('[data-slot="shared-record-banner-exit"]') + .click({ noWaitAfter: true }); + + await expect( + peerPage.locator('[data-slot="record-scope-hydration-gate"]'), + ).toBeVisible(); + await expect( + peerPage.locator('[data-slot="shared-record-banner"]'), + ).toHaveCount(0); + await peerPage.waitForTimeout(200); + expect(peerRecordReads).toEqual([]); + + switchOut.release(); + await switchOutClick; + await expect( + switchingPage.getByRole("button", { name: "User menu" }), + ).toBeVisible(); + await expect( + peerPage.getByRole("button", { name: "User menu" }), + ).toBeVisible(); + } finally { + await context.close(); + } + }); +}); diff --git a/e2e/v137-sharing-managed-profiles-a11y.spec.ts b/e2e/v137-sharing-managed-profiles-a11y.spec.ts index ed146f0f3..6396eba29 100644 --- a/e2e/v137-sharing-managed-profiles-a11y.spec.ts +++ b/e2e/v137-sharing-managed-profiles-a11y.spec.ts @@ -1,11 +1,83 @@ +import AxeBuilder from "@axe-core/playwright"; +import type { Page } from "@playwright/test"; + import { expect, test } from "./setup/test"; +import { SCOPE_A11Y_STORAGE_STATE_PATH } from "./setup/test-helpers"; -import { ACCESSIBILITY_STATES } from "../tests/fixtures/v137/e2e-journeys"; +async function scan(page: Page) { + const result = await new AxeBuilder({ page }) + .withTags(["wcag2a", "wcag2aa", "wcag21a", "wcag21aa"]) + .analyze(); + expect(result.violations).toEqual([]); +} -test.describe("sharing accessibility anchors", () => { - for (const state of ACCESSIBILITY_STATES) { - test(`anchor: ${state.name}`, () => { - expect(state.contract).toBe("inventory-only"); +async function openProfileRecord(page: Page) { + await page.goto("/"); + await page.getByRole("button", { name: "User menu" }).first().click(); + await page.locator('[data-slot="account-switcher-trigger"]').click(); + const entry = page.locator( + '[data-slot="account-switcher-entry"][data-account-username="e2e-scope-profile"]', + ); + await expect(entry).toHaveCount(1); + await entry.click(); + await expect(page).toHaveURL(/\/profile$/); +} + +async function leaveRecord(page: Page) { + await page.locator('[data-slot="shared-record-banner-exit"]').click(); + await expect(page.locator('[data-slot="shared-record-banner"]')).toHaveCount( + 0, + ); +} + +test.describe("shared record accessibility states", () => { + test.describe.configure({ mode: "serial" }); + test.use({ storageState: SCOPE_A11Y_STORAGE_STATE_PATH }); + + test("success and empty shared profile states have no WCAG violations", async ({ + page, + }) => { + await openProfileRecord(page); + await expect(page.locator('[data-slot="profile-summary"]')).toBeVisible(); + await scan(page); + await leaveRecord(page); + }); + + test("the capability hydration loading state has no WCAG violations", async ({ + page, + }) => { + await page.route("**/api/auth/me", async (route) => { + await new Promise((resolve) => setTimeout(resolve, 250)); + await route.continue(); }); - } + await page.goto("/measurements"); + await expect( + page.locator('[data-slot="record-scope-hydration-gate"]'), + ).toBeVisible(); + await scan(page); + }); + + test("the shared profile read error state has no WCAG violations", async ({ + page, + }) => { + await page.route("**/api/profile/summary**", (route) => + route.fulfill({ status: 500, body: "{}" }), + ); + await openProfileRecord(page); + await expect(page.locator('[data-slot="query-error-card"]')).toBeVisible(); + await scan(page); + await leaveRecord(page); + }); + + test("the shared-record refusal state has no WCAG violations", async ({ + page, + }) => { + await openProfileRecord(page); + await page.goto("/settings/account"); + await expect( + page.locator('[data-slot="shared-record-unavailable"]'), + ).toBeVisible(); + await scan(page); + await page.locator('[data-slot="shared-record-unavailable-leave"]').click(); + }); }); diff --git a/e2e/v137-sharing-managed-profiles.spec.ts b/e2e/v137-sharing-managed-profiles.spec.ts index a97bc38ae..a5eb657ae 100644 --- a/e2e/v137-sharing-managed-profiles.spec.ts +++ b/e2e/v137-sharing-managed-profiles.spec.ts @@ -1,11 +1,524 @@ +import type { Page } from "@playwright/test"; + import { expect, test } from "./setup/test"; +import { + E2E_LEVEL_RECORDS, + E2E_SCOPE_RECORDS, + SCOPE_DELEGATE_STORAGE_STATE_PATH, +} from "./setup/test-helpers"; -import { RELEASE_JOURNEYS } from "../tests/fixtures/v137/e2e-journeys"; +const DOMAIN_READS = { + measurements: "/api/measurements", + medications: "/api/medications", + labs: "/api/labs", + profile: "/api/profile/summary", + illness: "/api/illness/episodes", + mind: "/api/mood-entries", + cycle: "/api/cycle/calendar", + documents: "/api/documents/inbound", +} as const; -test.describe("sharing release journey anchors", () => { - for (const journey of RELEASE_JOURNEYS) { - test(`anchor: ${journey.name}`, () => { - expect(journey.contract).toBe("inventory-only"); +/** + * v1.37.0 — the record-session assertion the app is currently sending. + * + * The two API probes in this file are deliberate: they check a status code the + * UI does not surface (a MANAGE-only generated read) and a payload the UI + * paginates. But a bare `fetch` inside `page.evaluate` carries no + * record-session headers, and the fence refuses an unasserted request on any + * session that has been inside a shared record — so the probes started failing + * with 403 for a reason that had nothing to do with what they test. + * + * They are NOT an old-bundle simulation and must not become one: that case is + * covered deliberately in `src/__tests__/record-session-fence-compat.test.ts` + * and `tests/integration/record-session-fence.test.ts`, where the 403 and the + * recovery are the subject. Here the subject is the grant level, so the probe + * asserts exactly what the app beside it asserts — copied off a real app + * request rather than computed, so it cannot drift from what the client + * actually believes. + */ +function trackFenceAssertion(page: Page): () => { + "x-healthlog-record-epoch": string; + "x-healthlog-record-scope": string; +} { + // The HIGHEST epoch seen, not the most recent one observed. + // + // The counter is monotonic, and requests do not arrive in the order they + // were issued: a request from the page before a switch can be seen after one + // from the page after it, and last-writer-wins then leaves the probe holding + // a context the session has already left — a 409, intermittently, on a test + // about grant levels. Taking the maximum removes the ordering question + // rather than trying to win it. + let latest = { epoch: "bootstrap", scope: "bootstrap", seen: -1 }; + page.on("request", (request) => { + const headers = request.headers(); + const epoch = headers["x-healthlog-record-epoch"]; + const scope = headers["x-healthlog-record-scope"]; + if (epoch === undefined || scope === undefined) return; + if (!/^\d+$/.test(epoch)) return; + const value = Number(epoch); + if (value < latest.seen) return; + latest = { epoch, scope, seen: value }; + }); + return () => ({ + "x-healthlog-record-epoch": latest.epoch, + "x-healthlog-record-scope": latest.scope, + }); +} + +async function openSwitcher(page: Page) { + await page.getByRole("button", { name: "User menu" }).first().click(); + await page.locator('[data-slot="account-switcher-trigger"]').click(); + await expect( + page.locator('[data-slot="account-switcher-menu"]'), + ).toBeVisible(); +} + +async function openRecord(page: Page, username: string) { + await page.goto("/"); + await openSwitcher(page); + + const entry = page.locator( + `[data-slot="account-switcher-entry"][data-account-username="${username}"]`, + ); + await expect(entry).toHaveCount(1); + const accountId = await entry.getAttribute("data-account-id"); + if (accountId === null) { + throw new Error(`Account switcher entry for ${username} has no account id`); + } + const accessLevel = await entry.getAttribute("data-access-level"); + const recordKind = await entry.getAttribute("data-record-kind"); + await entry.click(); + return { accountId, accessLevel, recordKind }; +} + +async function leaveRecord(page: Page) { + await page.locator('[data-slot="shared-record-banner-exit"]').click(); + await expect(page.locator('[data-slot="shared-record-banner"]')).toHaveCount( + 0, + ); +} + +function withDivergentActiveAccountAccess(body: unknown): unknown { + if (body === null || typeof body !== "object") { + throw new Error("Expected an object from /api/auth/me"); + } + + const envelope = body as { data?: unknown }; + const payload = "data" in envelope ? envelope.data : envelope; + if (payload === null || typeof payload !== "object") { + throw new Error("Expected an auth payload from /api/auth/me"); + } + + const auth = payload as { + accountAccess?: { + active?: { + access?: unknown; + level?: unknown; + canWrite?: unknown; + } | null; + }; + }; + if (!auth.accountAccess?.active) { + throw new Error("Expected a switched account-access payload"); + } + + const corrupted = { + ...auth, + accountAccess: { + ...auth.accountAccess, + active: { + ...auth.accountAccess.active, + access: "write", + level: "write", + canWrite: true, + }, + }, + }; + return "data" in envelope ? { ...envelope, data: corrupted } : corrupted; +} + +test.describe.serial("scoped sharing browser journeys", () => { + test.use({ storageState: SCOPE_DELEGATE_STORAGE_STATE_PATH }); + + for (const record of E2E_SCOPE_RECORDS) { + test(`opens only the ${record.domain} record doorway`, async ({ page }) => { + await page.goto("/"); + await openSwitcher(page); + + const entry = page.locator( + `[data-slot="account-switcher-entry"][data-account-username="${record.username}"]`, + ); + await expect(entry).toHaveCount(1); + + const unexpectedOcrCapability = + record.domain === "labs" + ? page.waitForRequest( + (request) => + request.method() === "GET" && + new URL(request.url()).pathname === "/api/labs/ocr/capability", + { timeout: 500 }, + ) + : null; + const read = page.waitForResponse( + (response) => + response.request().method() === "GET" && + new URL(response.url()).pathname === DOMAIN_READS[record.domain], + ); + await entry.click(); + + await expect(page).toHaveURL(new RegExp(`${record.href}$`)); + await expect( + page.locator('[data-slot="shared-record-banner"]'), + ).toContainText(record.username); + await expect( + page.locator(`a[href="${record.href}"]`).first(), + ).toBeVisible(); + await expect( + page.locator('[data-slot="shared-record-unavailable"]'), + ).toHaveCount(0); + await read; + if (unexpectedOcrCapability) { + await expect(unexpectedOcrCapability).rejects.toThrow(/Timeout/); + } + + await leaveRecord(page); }); } + + test("refuses a malformed switched payload before target or owner reads mount", async ({ + page, + }) => { + await page.goto("/"); + await openSwitcher(page); + + const entry = page.locator( + '[data-slot="account-switcher-entry"][data-account-username="e2e-scope-labs"]', + ); + await expect(entry).toHaveCount(1); + + // The switch normally writes this mirror before its full reload. Remove it + // in the new document so the global preloaders cannot guess an own-record + // scope while `/api/auth/me` is still deciding whether the target is safe. + await page.addInitScript(() => { + localStorage.removeItem("healthlog-record-scope"); + }); + + let corruptAuthPayload = true; + await page.route("**/api/auth/me", async (route) => { + const response = await route.fetch(); + if (!corruptAuthPayload) { + await route.fulfill({ response }); + return; + } + await route.fulfill({ + response, + json: withDivergentActiveAccountAccess(await response.json()), + }); + }); + + const ownerOnlyReads: string[] = []; + const trackOwnerOnlyRead = (request: { + method(): string; + url(): string; + }) => { + if (request.method() !== "GET") return; + const path = new URL(request.url()).pathname; + if ( + [ + "/api/labs/ocr/capability", + "/api/insights/coach/nudge-status", + "/api/coach/about-me/questions", + "/api/auth/me/notification-prefs", + ].includes(path) + ) { + ownerOnlyReads.push(path); + } + }; + page.on("request", trackOwnerOnlyRead); + const targetPreloads: string[] = []; + const trackTargetPreload = (request: { + method(): string; + url(): string; + }) => { + if (request.method() !== "GET") return; + const path = new URL(request.url()).pathname; + if ( + [ + "/api/dashboard/snapshot", + "/api/medications", + "/api/medications/compliance", + ].includes(path) + ) { + targetPreloads.push(path); + } + }; + page.on("request", trackTargetPreload); + try { + await entry.click(); + + await expect(page).toHaveURL(/\/labs$/); + await expect( + page.locator('[data-slot="invalid-record-access-refusal"]'), + ).toBeVisible(); + await expect( + page.locator('[data-slot="shared-record-unavailable-leave"]'), + ).toBeVisible(); + await expect(page.locator('[data-tour-id="labs-hero"]')).toHaveCount(0); + await expect( + page.locator('[data-slot="shared-record-banner"]'), + ).toHaveCount(0); + + // Both global preloader routes must remain inert while the malformed + // response is refused, even though the scope mirror was intentionally + // cleared before each reload. + await page.goto("/"); + await expect( + page.locator('[data-slot="invalid-record-access-refusal"]'), + ).toBeVisible(); + await page.goto("/medications"); + await expect( + page.locator('[data-slot="invalid-record-access-refusal"]'), + ).toBeVisible(); + expect(ownerOnlyReads).toEqual([]); + expect(targetPreloads).toEqual([]); + + // Leave the refused record, then demand fresh own-record reads. This + // catches a response that was parked under the owner hash while the + // refusal was on screen and would otherwise paint without a new request. + corruptAuthPayload = false; + const freshDashboard = page.waitForResponse( + (response) => + response.request().method() === "GET" && + new URL(response.url()).pathname === "/api/dashboard/snapshot", + ); + await page + .locator('[data-slot="shared-record-unavailable-leave"]') + .click(); + await expect(page).toHaveURL(/\/$/); + await freshDashboard; + + const freshMedications = page.waitForResponse( + (response) => + response.request().method() === "GET" && + new URL(response.url()).pathname === "/api/medications", + ); + await page.goto("/medications"); + await freshMedications; + } finally { + page.off("request", trackOwnerOnlyRead); + page.off("request", trackTargetPreload); + await page.unroute("**/api/auth/me"); + } + }); + + test("keeps an adult READ grant read-only and outside Settings", async ({ + page, + }) => { + const record = E2E_LEVEL_RECORDS.find( + (candidate) => candidate.access === "READ", + ); + if (!record) throw new Error("READ fixture is missing"); + + const { accessLevel, recordKind } = await openRecord(page, record.username); + expect(accessLevel).toBe("view"); + expect(recordKind).toBe("shared"); + + const banner = page.locator('[data-slot="shared-record-banner"]'); + await expect(banner).toHaveAttribute("data-access-level", "view"); + await expect(banner).toHaveAttribute("data-record-kind", "shared"); + await expect(banner).toContainText(record.username); + + await page.goto("/measurements"); + await expect(page.locator('[data-slot="measurement-add"]')).toHaveCount(0); + + await page.goto("/settings/account"); + await expect( + page.locator('[data-slot="shared-record-unavailable"]'), + ).toBeVisible(); + await leaveRecord(page); + }); + + test("lets an adult WRITE grant add only to the selected record", async ({ + page, + }) => { + const record = E2E_LEVEL_RECORDS.find( + (candidate) => candidate.access === "WRITE", + ); + if (!record) throw new Error("WRITE fixture is missing"); + + const { accountId, accessLevel, recordKind } = await openRecord( + page, + record.username, + ); + expect(accessLevel).toBe("view-and-add"); + expect(recordKind).toBe("shared"); + const banner = page.locator('[data-slot="shared-record-banner"]'); + await expect(banner).toHaveAttribute("data-account-id", accountId); + await expect(banner).toHaveAttribute("data-access-level", "view-and-add"); + await expect(banner).toHaveAttribute("data-record-kind", "shared"); + + await page.goto("/measurements"); + await expect(page.locator('[data-slot="measurement-add"]')).toBeVisible(); + const post = page.waitForResponse( + (response) => + response.request().method() === "POST" && + new URL(response.url()).pathname === "/api/measurements", + ); + await page.locator('[data-slot="measurement-add"]').click(); + await page.locator("#sys").fill("124"); + await page.locator("#dia").fill("78"); + await page.getByRole("button", { name: /^save$/i }).click(); + expect((await post).status()).toBeLessThan(300); + + // Read the value back THROUGH THE APP rather than with a hand-rolled + // probe. + // + // The probe copied the fence headers off a live request so it would not be + // refused — and then raced them: requests do not arrive in the order they + // were issued, so it could pick up a context the session had already left + // and answer 409 on a test about grant levels. Copying the app's state to + // stand beside the app is the wrong shape; asking the app is the right one, + // and it is also closer to what this test claims — that the reading landed + // in the SELECTED record and is visible there. + await page.reload(); + await expect(page.locator('[data-slot="measurement-add"]')).toBeVisible(); + await expect(page.getByText("124", { exact: false }).first()).toBeVisible({ + timeout: 20_000, + }); + await expect(banner).toHaveAttribute("data-account-id", accountId); + + await page.goto("/settings/account"); + await expect( + page.locator('[data-slot="shared-record-unavailable"]'), + ).toBeVisible(); + await leaveRecord(page); + }); + + test("opens MANAGE-only generated reads without opening adult Settings", async ({ + page, + }) => { + const fenceHeaders = trackFenceAssertion(page); + const record = E2E_LEVEL_RECORDS.find( + (candidate) => + candidate.access === "MANAGE" && candidate.recordKind === "shared", + ); + if (!record) throw new Error("adult MANAGE fixture is missing"); + + const { accessLevel, recordKind } = await openRecord(page, record.username); + expect(accessLevel).toBe("manage"); + expect(recordKind).toBe("shared"); + const banner = page.locator('[data-slot="shared-record-banner"]'); + await expect(banner).toHaveAttribute("data-access-level", "manage"); + await expect(banner).toHaveAttribute("data-record-kind", "shared"); + + await page.goto("/"); + await expect(banner).toHaveAttribute("data-access-level", "manage"); + const generatedRead = await page.evaluate(async (headers) => { + const response = await fetch("/api/dashboard/summary", { headers }); + return response.status; + }, fenceHeaders()); + expect(generatedRead).toBeLessThan(300); + await expect( + page.locator('[data-slot="shared-record-unavailable"]'), + ).toHaveCount(0); + + await page.goto("/settings/account"); + await expect( + page.locator('[data-slot="shared-record-unavailable"]'), + ).toBeVisible(); + await leaveRecord(page); + }); + + test("keeps managed Settings on the active record and integrations status-only", async ({ + page, + }) => { + const record = E2E_LEVEL_RECORDS.find( + (candidate) => candidate.recordKind === "managed", + ); + if (!record) throw new Error("managed record fixture is missing"); + + const { accountId, accessLevel, recordKind } = await openRecord( + page, + record.username, + ); + expect(accessLevel).toBe("manage"); + expect(recordKind).toBe("managed"); + const banner = page.locator('[data-slot="shared-record-banner"]'); + await expect(banner).toHaveAttribute("data-account-id", accountId); + await expect(banner).toHaveAttribute("data-access-level", "manage"); + await expect(banner).toHaveAttribute("data-record-kind", "managed"); + + await page.goto("/settings/account"); + const profile = page.locator('[data-record-settings-family="profile"]'); + await expect(profile).toBeVisible(); + await expect(profile).toHaveAttribute("data-record-id", accountId); + + const integrationRead = page.waitForResponse( + (response) => + response.request().method() === "GET" && + new URL(response.url()).pathname === + "/api/record-settings/integrations", + ); + await page.goto("/settings/integrations"); + const integrationResponse = await integrationRead; + expect(integrationResponse.status()).toBeLessThan(300); + const integrationPayload = (await integrationResponse.json()) as { + data?: { recordId?: string }; + }; + expect(integrationPayload.data?.recordId).toBe(accountId); + const integrations = page.locator( + '[data-record-settings-family="integrations"]', + ); + await expect(integrations).toBeVisible(); + await expect(integrations).toHaveAttribute("data-record-id", accountId); + await expect( + integrations.locator("button, input, select, textarea"), + ).toHaveCount(0); + + await page.goBack(); + await expect(profile).toHaveAttribute("data-record-id", accountId); + await page.goForward(); + await expect(integrations).toHaveAttribute("data-record-id", accountId); + await expect(banner).toHaveAttribute("data-account-id", accountId); + + await leaveRecord(page); + await page.goto("/measurements"); + await expect(banner).toHaveCount(0); + }); + + test("refuses direct ungranted routes before coach reads start", async ({ + page, + }) => { + await openRecord(page, "e2e-scope-measurements"); + const protectedCoachReads: string[] = []; + const trackCoachRead = (request: { url(): string }) => { + const path = new URL(request.url()).pathname; + if ( + [ + "/api/insights/coach/nudge-status", + "/api/coach/about-me/questions", + "/api/auth/me/notification-prefs", + ].includes(path) + ) { + protectedCoachReads.push(path); + } + }; + page.on("request", trackCoachRead); + try { + await page.goto("/coach"); + await expect( + page.locator('[data-slot="shared-record-unavailable"]'), + ).toBeVisible(); + expect(protectedCoachReads).toEqual([]); + + await page.goto("/settings/access"); + await expect( + page.locator('[data-slot="shared-record-unavailable"]'), + ).toBeVisible(); + expect(protectedCoachReads).toEqual([]); + } finally { + page.off("request", trackCoachRead); + } + await leaveRecord(page); + }); }); diff --git a/messages/de.json b/messages/de.json index 6d8c53f1e..2e876a92b 100644 --- a/messages/de.json +++ b/messages/de.json @@ -341,6 +341,7 @@ }, "cycle": "Zyklus", "labs": "Laborwerte", + "profile": "Profil", "vorsorge": "Vorsorge", "illness": "Krankheit", "documents": "Dokumente", diff --git a/messages/en.json b/messages/en.json index a0e7bc356..704da23e3 100644 --- a/messages/en.json +++ b/messages/en.json @@ -341,6 +341,7 @@ }, "cycle": "Cycle", "labs": "Labs", + "profile": "Profile", "vorsorge": "Checkups", "illness": "Illness", "documents": "Documents", diff --git a/messages/es.json b/messages/es.json index 625c3daff..13cb2d2cb 100644 --- a/messages/es.json +++ b/messages/es.json @@ -341,6 +341,7 @@ }, "cycle": "Ciclo", "labs": "Análisis", + "profile": "Perfil", "vorsorge": "Revisiones", "illness": "Enfermedad", "documents": "Documentos", diff --git a/messages/fr.json b/messages/fr.json index bc662c21a..0c97286b6 100644 --- a/messages/fr.json +++ b/messages/fr.json @@ -341,6 +341,7 @@ }, "cycle": "Cycle", "labs": "Analyses", + "profile": "Profil", "vorsorge": "Examens", "illness": "Maladie", "documents": "Documents", diff --git a/messages/it.json b/messages/it.json index 2b0202731..9bb80dbf5 100644 --- a/messages/it.json +++ b/messages/it.json @@ -341,6 +341,7 @@ }, "cycle": "Ciclo", "labs": "Esami", + "profile": "Profilo", "vorsorge": "Controlli", "illness": "Malattia", "documents": "Documenti", diff --git a/messages/pl.json b/messages/pl.json index 526608239..f01fa2c7f 100644 --- a/messages/pl.json +++ b/messages/pl.json @@ -341,6 +341,7 @@ }, "cycle": "Cykl", "labs": "Wyniki badań", + "profile": "Profil", "vorsorge": "Badania", "illness": "Choroba", "documents": "Dokumenty", diff --git a/playwright.config.ts b/playwright.config.ts index 3a79e9e88..5870e7057 100644 --- a/playwright.config.ts +++ b/playwright.config.ts @@ -75,14 +75,63 @@ export default defineConfig({ projects: [ { name: "chromium-desktop", + // The disk-layer spec needs a live service worker, which the shared + // `use` block blocks for every other spec. It runs in its own project. + testIgnore: ["v137-record-session-fence-offline.spec.ts"], use: { ...devices["Desktop Chrome"], viewport: { width: 1280, height: 720 }, colorScheme: "dark", }, }, + { + // v1.37.0 — the one project that lets the service worker run. + // + // Every other project blocks it, and the reason is written in the `use` + // block above: a worker-originated `fetch` is not subject to + // `page.route`, so the SW would bypass the per-spec route mocks. The + // record-session fence's disk-layer case is the exact inverse — its whole + // claim is about what the `healthlog-data-*` cache can and cannot serve + // across a switch, and with the worker blocked that cache is never + // populated, so its positive control could never pass. A check that + // cannot pass is worse than no check, so the spec gets a project rather + // than a caveat. + // + // It uses no route mocks for exactly this reason, and it is the only + // spec in this project. + name: "chromium-service-worker", + testMatch: ["v137-record-session-fence-offline.spec.ts"], + use: { + ...devices["Desktop Chrome"], + viewport: { width: 1280, height: 720 }, + colorScheme: "dark", + serviceWorkers: "allow", + }, + }, { name: "chromium-mobile", + // The scoped-sharing journeys each carry a persistent server-side + // session stamp. They use isolated desktop jars and are not responsive + // layout checks, so running a second project against the same stamps + // would test session interference rather than the record journeys. + testIgnore: [ + "v137-sharing-managed-profiles.spec.ts", + "v137-sharing-managed-profiles-a11y.spec.ts", + // v1.37.0 — every spec that MOVES a session's record selector runs in + // exactly one project. + // + // Each of these now has its own login and its own session row (see + // `FENCE_STORAGE_STATE_PATH` in `e2e/setup/global-setup.ts`), which is + // what lets them run beside each other at any worker count. A second + // PROJECT is the one thing a separate jar cannot fix: it would drive + // the very same row from two browsers at once, moving the epoch under + // itself. The scoped-sharing journeys above are excluded for the same + // reason, one release earlier. + "v137-record-session-fence.spec.ts", + "v137-sharing-cross-tab-session.spec.ts", + // Runs only in the service-worker project. + "v137-record-session-fence-offline.spec.ts", + ], use: { // Pixel 5 — Chromium-based mobile profile so CI only needs // `playwright install chromium` instead of also pulling diff --git a/prisma/migrations/0302_session_record_epoch/migration.sql b/prisma/migrations/0302_session_record_epoch/migration.sql new file mode 100644 index 000000000..8ea0ffc99 --- /dev/null +++ b/prisma/migrations/0302_session_record_epoch/migration.sql @@ -0,0 +1,73 @@ +-- v1.37.0 — a monotonic counter for "how many times has this session's record +-- selector moved". +-- +-- The problem it solves is not authorization; `acting_as_user_id` plus the +-- grant join already decides that. It is agreement. A browser holds several +-- tabs on one session, and a request that left a suspended tab before a switch +-- landed arrives after it — carrying an intent formed under the old record and +-- addressed to a server that has since moved. The request is perfectly +-- authorized and answers the wrong question. So every request that resolves a +-- record now asserts which context it believes it is in, and the server refuses +-- when that belief no longer matches the row. +-- +-- The counter has to be the DATABASE's, not the application's, because the +-- application is not the only writer of the column it tracks. Three of the +-- five writers run no application code at all: +-- +-- * `sessions_acting_as_user_id_fkey ON DELETE SET NULL` — deleting an owner +-- account nulls every delegate session pointing at it, by referential +-- action, with nothing to hook; +-- * raw `UPDATE sessions SET acting_as_user_id = NULL` in the end-to-end +-- fixtures; +-- * an operator at `psql`, and whatever writes this column next year. +-- +-- An application-level increment would be a discipline those three break +-- silently, and a fence whose counter can silently fail to move is a fence that +-- passes stale requests. A row trigger cannot be forgotten. +-- +-- BEFORE, not AFTER, and that timing is load-bearing: on any statement that +-- moves the selector, the trigger overwrites whatever `record_epoch` the +-- incoming statement supplied, so a writer cannot steer the counter while +-- switching. +-- +-- What that does NOT cover, stated because the first draft of this comment +-- overstated it: a statement that names `record_epoch` and does NOT change +-- `acting_as_user_id` never fires this trigger at all, so it writes the column +-- freely. The trigger owns the counter on the path that matters — every +-- selector move — and what keeps application code away from the column +-- otherwise is the source guard in +-- `src/__tests__/record-session-fence-guard.test.ts`, which holds the set of +-- files naming it to one. +-- +-- `UPDATE OF "acting_as_user_id"` plus the `WHEN` clause keep the two writes +-- that touch a session on nearly every request off the trigger entirely — the +-- throttled `last_active_at` stamp and the sliding `expires_at` extension both +-- name other columns, and neither would satisfy the `IS DISTINCT FROM` test +-- even if it did. +-- +-- Precedent for a trigger on this line: `0298_notification_delivery_identity` +-- ships `push_attempt_attribution_guard` the same way. A trigger is invisible +-- to Prisma's datamodel comparison, which 0298 already proved tolerable. + +ALTER TABLE "sessions" ADD COLUMN "record_epoch" INTEGER NOT NULL DEFAULT 0; + +-- A session already sitting inside somebody else's record is not a session +-- that has never left its own, and the fence's exemption for pre-existing +-- browser tabs is defined as epoch zero. Normalise before the trigger exists, +-- so that from the first request after deploy `record_epoch = 0` means exactly +-- "never pointed at another record, and not pointed at one now". +UPDATE "sessions" SET "record_epoch" = 1 WHERE "acting_as_user_id" IS NOT NULL; + +CREATE OR REPLACE FUNCTION "sessions_bump_record_epoch"() RETURNS trigger +LANGUAGE plpgsql AS $$ +BEGIN + NEW."record_epoch" := OLD."record_epoch" + 1; + RETURN NEW; +END; +$$; + +CREATE TRIGGER "sessions_record_epoch_bump" + BEFORE UPDATE OF "acting_as_user_id" ON "sessions" + FOR EACH ROW + WHEN (OLD."acting_as_user_id" IS DISTINCT FROM NEW."acting_as_user_id") + EXECUTE FUNCTION "sessions_bump_record_epoch"(); diff --git a/prisma/schema.prisma b/prisma/schema.prisma index 30290011e..9701fa010 100644 --- a/prisma/schema.prisma +++ b/prisma/schema.prisma @@ -1179,6 +1179,24 @@ model Session { /// into its own account. actingAsUserId String? @map("acting_as_user_id") + /// v1.37.0 — how many times this session's record selector has moved. + /// + /// Monotonic, starts at 0, and NEVER written by application code: migration + /// 0302 installs a `BEFORE UPDATE OF acting_as_user_id` trigger that sets it, + /// so the increment also happens on the paths where no application code runs + /// at all — the `ON DELETE SET NULL` referential action above, a raw fixture + /// `UPDATE`, an operator at `psql`. A convention that every writer must + /// remember is a convention those three break silently. + /// + /// Consumer: the record-session fence + /// (`src/lib/sharing/record-session-fence.ts`), which refuses a cookie + /// request whose asserted epoch no longer matches the row. `0` additionally + /// means "this session has never been pointed at another record", which is + /// the fence's exemption for browser tabs that predate the feature; the + /// migration normalises existing rows so that invariant holds from the first + /// request after deploy. + recordEpoch Int @default(0) @map("record_epoch") + user User @relation("SessionOwner", fields: [userId], references: [id], onDelete: Cascade) actingAs User? @relation("SessionActingAs", fields: [actingAsUserId], references: [id], onDelete: SetNull) diff --git a/src/__tests__/acting-account-boundary-guard.test.ts b/src/__tests__/acting-account-boundary-guard.test.ts index df45e02db..ad7666d33 100644 --- a/src/__tests__/acting-account-boundary-guard.test.ts +++ b/src/__tests__/acting-account-boundary-guard.test.ts @@ -94,6 +94,20 @@ describe("the acting-account carrier is read in one place", () => { // whole auth context for exactly this reason: had it taken the id, the // route would name the column too, and the list would grow per surface. "lib/sharing/account-access.ts", + // v1.37.0 — the record-session fence COMPARES it and never writes it. It + // reads the value off the `AuthContext` the resolver already holds rather + // than from the row, which is the property the fence rests on: the scope + // the handler serves under and the epoch the fence validated are one row + // read, so they cannot straddle a switch. A version of this file that + // resolved its own session would be a second answer to "whose record is + // this" and would also be wrong more often. + "lib/sharing/record-session-fence.ts", + // v1.37.0 — names the column in prose only. The matcher does not exempt + // comments and is left that way on purpose (see the note above), so the + // module that explains the fence has to be listed. It imports nothing at + // all, which the fence's own unit test asserts, so it can read no column + // even in principle. + "lib/sharing/record-session-fence-contract.ts", ].sort(); it("no other file reads or writes the carrier column", () => { @@ -125,7 +139,7 @@ describe("the acting-account carrier is read in one place", () => { * Who may ask "which record does this request claim" without an auth * context in hand. * - * `readClaimedActingAccount()` answers before any grant check has run, so a + * `readClaimedRecordContext()` answers before any grant check has run, so a * caller that treats its answer as permission has skipped the only thing * that grants any. Exactly one caller exists, beside the file that declares * it: the idempotency wrapper, which uses the claim to pick a cache cell and @@ -133,9 +147,17 @@ describe("the acting-account carrier is read in one place", () => { * acting account has an auth context by then and goes through * `requireRecordAuth`, which checks a live grant before returning. A second * caller here is a design question, not a patch. + * + * The matcher names the v1.36.0 spelling (`readClaimedActingAccount`) as well + * as today's. That function was removed in v1.37.0 when the wrapper started + * needing the session's record context on the same read; matching both means + * a reintroduced narrow copy trips this leg instead of quietly becoming a + * second answer. */ it("the unchecked claim has exactly two consumers", () => { - const consumers = filesMatching(/readClaimedActingAccount/); + const consumers = filesMatching( + /readClaimedRecordContext|readClaimedActingAccount/, + ); expect(consumers.length).toBeGreaterThan(0); expect(consumers).toEqual([ "lib/auth/acting-carrier.ts", diff --git a/src/__tests__/managed-record-navigation.test.tsx b/src/__tests__/managed-record-navigation.test.tsx new file mode 100644 index 000000000..09dec936a --- /dev/null +++ b/src/__tests__/managed-record-navigation.test.tsx @@ -0,0 +1,104 @@ +import { describe, expect, it, vi } from "vitest"; +import { renderToStaticMarkup } from "react-dom/server"; + +import { I18nProvider } from "@/lib/i18n/context"; +import { classifySettingsDestination } from "@/lib/record-settings"; + +const pathnameRef = { value: "/settings/security" }; +const activeRecordRef = { value: true }; + +vi.mock("next/navigation", () => ({ + usePathname: () => pathnameRef.value, +})); + +vi.mock("@/hooks/use-auth", () => ({ + useAuth: () => ({ + user: activeRecordRef.value + ? { + id: "guardian", + modules: undefined, + accountAccess: { + active: { + accountId: "managed-record", + username: "managed-record", + displayName: "Managed record", + access: "write", + level: "manage", + recordKind: "managed", + sections: null, + canWrite: true, + }, + accounts: [], + canSwitch: true, + }, + } + : null, + isAuthenticated: true, + isLoading: false, + refetch: vi.fn(), + }), +})); + +vi.mock("@/hooks/use-account-switch", () => ({ + useAccountSwitch: () => ({ + mutate: vi.fn(), + isPending: false, + }), +})); + +import { + SETTINGS_SECTIONS, + SettingsShell, +} from "@/components/settings/settings-shell"; +import { SharedRecordBanner } from "@/components/layout/shared-record-banner"; + +function renderSettings(pathname = "/settings/security") { + pathnameRef.value = pathname; + return renderToStaticMarkup( + + +
managed settings content
+ + , + ); +} + +describe("managed record navigation", () => { + it("shows only managed settings destinations while retaining an unavailable deep-link heading", () => { + const html = renderSettings(); + const managedDestinations = SETTINGS_SECTIONS.filter( + (section) => + classifySettingsDestination(section.slug).kind === "managed-guardian", + ); + const unavailableDestinations = SETTINGS_SECTIONS.filter( + (section) => + classifySettingsDestination(section.slug).kind !== "managed-guardian", + ); + + expect(html).toContain('data-active-record-id="managed-record"'); + expect(html).toContain('id="settings-section-security-title"'); + expect(html).not.toMatch(/href="[^"]*managed-record/); + + for (const section of managedDestinations) { + expect( + html.match(new RegExp(`href="/settings/${section.slug}"`, "g")) + ?.length ?? 0, + ).toBe(2); + } + + for (const section of unavailableDestinations) { + expect(html).not.toContain(`href="/settings/${section.slug}"`); + } + }); + + it("keeps the exit control present while administering a managed record", () => { + const html = renderToStaticMarkup( + + + , + ); + + expect(html).toContain('data-account-id="managed-record"'); + expect(html).toContain('data-slot="shared-record-banner-exit"'); + }); +}); diff --git a/src/__tests__/record-fence-headerless-transport-guard.test.ts b/src/__tests__/record-fence-headerless-transport-guard.test.ts new file mode 100644 index 000000000..3ccd9731d --- /dev/null +++ b/src/__tests__/record-fence-headerless-transport-guard.test.ts @@ -0,0 +1,327 @@ +/** + * A fenced route may not be reached by a transport that cannot carry a header. + * + * The record-session fence is asserted on request HEADERS, attached inside + * `src/lib/api/api-fetch.ts`. Three browser transports never go through that + * wrapper and cannot be made to: + * + * * `` — the browser issues the subresource itself; + * * `