From 90c9a4e2d0196e0813f78a99fabfc036e238696a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marc-Andr=C3=A9=20Bombeck?= Date: Wed, 21 Oct 2026 21:47:39 +0200 Subject: [PATCH] fix(settings): list layout and preview switch serve the person, atomically Both routes resolved through bare requireAuth, which refuses a session acting for another record, so the list view controls and the preview switch failed in exactly the context they are used in. They are the person's own presentation and now resolve through requireActorAuth, admitted on the actor-surface list with their reasons. The layout PUT and the preview PATCH read and write the stored blob under a row lock in one transaction, so two saves at once no longer start from the same value and drop each other's change. --- src/__tests__/sharing-surface-guard.test.ts | 9 +- .../api/settings/module-list-layout/route.ts | 52 +++++---- .../api/settings/preview-features/route.ts | 37 +++--- .../integration/settings-prefs-routes.test.ts | 110 +++++++++++++++++- 4 files changed, 173 insertions(+), 35 deletions(-) diff --git a/src/__tests__/sharing-surface-guard.test.ts b/src/__tests__/sharing-surface-guard.test.ts index d6ec5452e..9b5d0704a 100644 --- a/src/__tests__/sharing-surface-guard.test.ts +++ b/src/__tests__/sharing-surface-guard.test.ts @@ -2072,6 +2072,10 @@ const ACTOR_ROUTES: Record = { "The app shell reads it on every boot, including while a switch is on: the switcher, the banner naming whose record is open, and the route back out all bind this payload. Every field it returns is the caller's own — their preferences, their modules, their identity — and the one field about the switch says only which records they may open and which they are inside. It reads no row of the owner's, and its `accountAccess` block grants nothing: the resolver re-checks the grant on every delegated request regardless of what this payload said a moment ago.", "app/api/auth/me/locale/route.ts": "The UI language belongs to the person reading the screen, never to the record on it. Under a record scope this would transplant a delegate's choice onto the owner's row and send the owner's cron mail in a language they may not read. It has to keep working rather than merely refuse safely: the switcher's mount-time backfill fires on every page load, including the shared dashboard.", + "app/api/settings/module-list-layout/route.ts": + "How the person's own labs, illness and checkups lists are laid out: card or list view, manual order, sort. It reads and writes the caller's own row only. Inside another record the ids of a saved order simply match nothing and the list keeps the server order, so serving the person's own choice there reveals nothing of the owner's and grants nothing; refusing it under a switch broke the list controls in exactly the context they are used in.", + "app/api/settings/preview-features/route.ts": + "The person's own opt-in to preview features. A preview entry is chrome of the person reading the screen, never a property of the record on it; it reads and writes the caller's own row and nothing of the owner's.", "app/api/feature-flags/route.ts": "Reads the `AppSettings` singleton and no user row at all, so there is no record for a switch to substitute. It answers about the deployment, which for this purpose is the caller's side of the request; the shell gates the Coach launcher and the assistant surfaces on it, so a 403 here is a piece of chrome deciding it does not exist.", }; @@ -2171,8 +2175,11 @@ const ACTOR_ROUTES: Record = { * * v1.44 -- a guardian connects and disconnects a managed profile's OAuth * providers: two guardian-only entries. 266 -> 268. + * + * v1.44 -- the module list layout and the preview-feature switch are the + * person's own presentation and join the actor list. 268 -> 270. */ -const FROZEN_ENTRY_COUNT = 268; +const FROZEN_ENTRY_COUNT = 270; /** * The two surfaces that authenticate a Bearer token outside `requireAuth` — diff --git a/src/app/api/settings/module-list-layout/route.ts b/src/app/api/settings/module-list-layout/route.ts index 7bd93d3b8..481e38867 100644 --- a/src/app/api/settings/module-list-layout/route.ts +++ b/src/app/api/settings/module-list-layout/route.ts @@ -8,11 +8,17 @@ * keeps for its fields. The blob lives on its own `User` column * (`module_list_layout_json`). * - * Both halves are the caller's own presentation. Inside somebody else's - * record the ids in a saved order simply do not match and the list keeps the - * server order, so no record gate is needed here. + * Both halves are the caller's own presentation, so this is an actor surface + * (`requireActorAuth`): a browser acting for another record still reads and + * saves the person's own layout instead of being refused. Inside somebody + * else's record the ids in a saved order simply do not match and the list + * keeps the server order, so no record gate is needed here. + * + * The PUT reads and writes the blob under a row lock in one transaction, so + * two saves for different lists (two tabs, a quick view switch and a reorder) + * cannot each start from the same stored value and drop the other's change. */ -import { apiHandler, requireAuth } from "@/lib/api-handler"; +import { apiHandler, requireActorAuth } from "@/lib/api-handler"; import { apiSuccess, returnAllZodIssues, safeJson } from "@/lib/api-response"; import { prisma, toJson } from "@/lib/db"; import { annotate } from "@/lib/logging/context"; @@ -25,8 +31,11 @@ import { moduleListLayoutPutSchema } from "@/lib/validations/settings-prefs"; export const dynamic = "force-dynamic"; -async function readStored(userId: string): Promise { - const row = await prisma.user.findUnique({ +async function readStored( + userId: string, + db: Pick = prisma, +): Promise { + const row = await db.user.findUnique({ where: { id: userId }, select: { moduleListLayoutJson: true }, }); @@ -34,7 +43,7 @@ async function readStored(userId: string): Promise { } export const GET = apiHandler(async () => { - const { user } = await requireAuth(); + const { user } = await requireActorAuth(); const layout = resolveModuleListLayout(await readStored(user.id)); annotate({ action: { name: "settings.module-list-layout.read" }, @@ -46,7 +55,7 @@ export const GET = apiHandler(async () => { }); export const PUT = apiHandler(async (request: Request) => { - const { user } = await requireAuth(); + const { user } = await requireActorAuth(); // 500 ids of up to 64 characters plus the envelope. const { data: body, error: jsonError } = await safeJson(request, { @@ -66,18 +75,21 @@ export const PUT = apiHandler(async (request: Request) => { // Preserve-when-absent: one stored read fills whichever field the body // left out, and every other list stays as stored. Only fields a person has // chosen are written, so a field never picked keeps following the default. - const stored = storedModuleListLayout(await readStored(user.id)); - const nextChoices = { - ...stored[list], - ...(view ? { view } : {}), - ...(order ? { order: [...new Set(order)] } : {}), - ...(sortDir ? { sortDir } : {}), - }; - const nextStored = { ...stored, [list]: nextChoices }; - - await prisma.user.update({ - where: { id: user.id }, - data: { moduleListLayoutJson: toJson({ version: 1, ...nextStored }) }, + const nextStored = await prisma.$transaction(async (tx) => { + await tx.$queryRaw`SELECT 1 FROM "users" WHERE "id" = ${user.id} FOR UPDATE`; + const stored = storedModuleListLayout(await readStored(user.id, tx)); + const nextChoices = { + ...stored[list], + ...(view ? { view } : {}), + ...(order ? { order: [...new Set(order)] } : {}), + ...(sortDir ? { sortDir } : {}), + }; + const next = { ...stored, [list]: nextChoices }; + await tx.user.update({ + where: { id: user.id }, + data: { moduleListLayoutJson: toJson({ version: 1, ...next }) }, + }); + return next; }); const layout = resolveModuleListLayout(nextStored); diff --git a/src/app/api/settings/preview-features/route.ts b/src/app/api/settings/preview-features/route.ts index 1b48c75fa..408b40a2f 100644 --- a/src/app/api/settings/preview-features/route.ts +++ b/src/app/api/settings/preview-features/route.ts @@ -5,8 +5,13 @@ * the flags the body names and keeps the rest; unknown flags are refused * with the multi-issue 422 so a typo cannot silently do nothing. The blob * lives on its own `User` column (`preview_features_json`). + * + * The opt-ins are the caller's own, so this is an actor surface + * (`requireActorAuth`): a browser acting for another record keeps reaching + * the person's own switch. The PATCH reads and writes under a row lock, so two + * flags switched at once both stay switched. */ -import { apiHandler, requireAuth } from "@/lib/api-handler"; +import { apiHandler, requireActorAuth } from "@/lib/api-handler"; import { apiSuccess, returnAllZodIssues, safeJson } from "@/lib/api-response"; import { prisma, toJson } from "@/lib/db"; import { annotate } from "@/lib/logging/context"; @@ -19,8 +24,11 @@ import { previewFeaturesPatchSchema } from "@/lib/validations/settings-prefs"; export const dynamic = "force-dynamic"; -async function readStored(userId: string): Promise { - const row = await prisma.user.findUnique({ +async function readStored( + userId: string, + db: Pick = prisma, +): Promise { + const row = await db.user.findUnique({ where: { id: userId }, select: { previewFeaturesJson: true }, }); @@ -32,7 +40,7 @@ function enabledFlags(features: PreviewFeatures): string[] { } export const GET = apiHandler(async () => { - const { user } = await requireAuth(); + const { user } = await requireActorAuth(); const features = resolvePreviewFeatures(await readStored(user.id)); annotate({ action: { name: "settings.preview-features.read" }, @@ -42,7 +50,7 @@ export const GET = apiHandler(async () => { }); export const PATCH = apiHandler(async (request: Request) => { - const { user } = await requireAuth(); + const { user } = await requireActorAuth(); const { data: body, error: jsonError } = await safeJson(request, { maxBytes: 1024, @@ -57,14 +65,17 @@ export const PATCH = apiHandler(async (request: Request) => { return returnAllZodIssues(parsed.error, 422); } - const current = resolvePreviewFeatures(await readStored(user.id)); - const next: PreviewFeatures = { - topics: parsed.data.topics ?? current.topics, - }; - - await prisma.user.update({ - where: { id: user.id }, - data: { previewFeaturesJson: toJson(next) }, + const next = await prisma.$transaction(async (tx) => { + await tx.$queryRaw`SELECT 1 FROM "users" WHERE "id" = ${user.id} FOR UPDATE`; + const current = resolvePreviewFeatures(await readStored(user.id, tx)); + const merged: PreviewFeatures = { + topics: parsed.data.topics ?? current.topics, + }; + await tx.user.update({ + where: { id: user.id }, + data: { previewFeaturesJson: toJson(merged) }, + }); + return merged; }); annotate({ diff --git a/tests/integration/settings-prefs-routes.test.ts b/tests/integration/settings-prefs-routes.test.ts index 5dcc4d7c5..61aaa5a00 100644 --- a/tests/integration/settings-prefs-routes.test.ts +++ b/tests/integration/settings-prefs-routes.test.ts @@ -15,7 +15,7 @@ process.env.ENCRYPTION_KEY ??= "0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef"; import { cookieJar, headerJar } from "./mock-next-headers"; -import { getPrismaClient, truncateAllTables } from "./setup"; +import { getPrismaClient, switchSessionTo, truncateAllTables } from "./setup"; vi.mock("next/headers", async () => { const { cookieJar, headerJar } = await import("./mock-next-headers"); @@ -214,3 +214,111 @@ describe("/api/settings/preview-features — integration", () => { expect(empty.status).toBe(422); }); }); + +describe("settings presentation follows the person, also while acting for a record", () => { + const LAYOUT = "http://localhost/api/settings/module-list-layout"; + const PREVIEW = "http://localhost/api/settings/preview-features"; + + async function actingForAnotherRecord() { + const prisma = getPrismaClient(); + const owner = await prisma.user.create({ + data: { + username: "prefs-record-owner", + email: "prefs-owner@example.test", + role: "USER", + }, + }); + const actor = await prisma.user.create({ + data: { + username: "prefs-delegate", + email: "prefs-delegate@example.test", + role: "USER", + }, + }); + await prisma.accountGrant.create({ + data: { + grantorId: owner.id, + granteeId: actor.id, + access: "WRITE", + acceptedAt: new Date(), + }, + }); + const session = await prisma.session.create({ + data: { userId: actor.id, expiresAt: new Date(Date.now() + 60_000) }, + }); + cookieJar.set("healthlog_session", session.id); + await switchSessionTo(session.id, owner.id); + return { ownerId: owner.id, actorId: actor.id }; + } + + it("reads and saves the actor's own layout and preview switch", async () => { + const { ownerId, actorId } = await actingForAnotherRecord(); + const layout = await import("@/app/api/settings/module-list-layout/route"); + const preview = await import("@/app/api/settings/preview-features/route"); + + expect((await (layout.GET as Handler)(new Request(LAYOUT))).status).toBe( + 200, + ); + const put = await (layout.PUT as Handler)( + json("PUT", LAYOUT, { version: 1, list: "labs", view: "cards" }), + ); + expect(put.status).toBe(200); + expect((await (preview.GET as Handler)(new Request(PREVIEW))).status).toBe( + 200, + ); + const patch = await (preview.PATCH as Handler)( + json("PATCH", PREVIEW, { topics: true }), + ); + expect(patch.status).toBe(200); + + const rows = await getPrismaClient().user.findMany({ + where: { id: { in: [ownerId, actorId] } }, + select: { + id: true, + moduleListLayoutJson: true, + previewFeaturesJson: true, + }, + }); + const actor = rows.find((r) => r.id === actorId)!; + const owner = rows.find((r) => r.id === ownerId)!; + expect(actor.moduleListLayoutJson).toMatchObject({ + labs: { view: "cards" }, + }); + expect(actor.previewFeaturesJson).toEqual({ topics: true }); + expect(owner.moduleListLayoutJson).toBeNull(); + expect(owner.previewFeaturesJson).toBeNull(); + }); +}); + +describe("layout saves for different lists do not overwrite each other", () => { + const URL = "http://localhost/api/settings/module-list-layout"; + + it("keeps every list a burst of concurrent saves touched", async () => { + const { userId } = await seedUser(); + const { PUT } = await import("@/app/api/settings/module-list-layout/route"); + const lists = ["labs", "illness", "vorsorge"] as const; + await Promise.all( + Array.from({ length: 9 }, (_, i) => + (PUT as Handler)( + json("PUT", URL, { + version: 1, + list: lists[i % 3], + ...(i < 3 ? { view: "cards" } : { order: [`item-${i}`] }), + }), + ), + ), + ); + const row = await getPrismaClient().user.findUniqueOrThrow({ + where: { id: userId }, + select: { moduleListLayoutJson: true }, + }); + const stored = row.moduleListLayoutJson as Record< + string, + { view?: string; order?: string[] } + >; + for (const list of lists) { + expect(stored[list]?.view, list).toBe("cards"); + expect(stored[list]?.order, list).toBeDefined(); + } + }); +});