From 8fd1d543a606853b5e52b5078acfc7dd6fd37404 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marc-Andr=C3=A9=20Bombeck?= Date: Sun, 25 Oct 2026 00:04:56 +0200 Subject: [PATCH] fix(medications): let a manager end a course and serve End only to who may press it POST /api/medications/{id}/courses/end resolved the caller with requireAuth(), so a delegate managing the record got 403 while the medication PUT that used to carry the end date admits MANAGE, and the reads still served canEndCourse true. The action now resolves the record at MANAGE over medications and files the previous end (C4) with the count of tombstoned placeholders. The medication reads narrow canEndCourse and canStartCourse to the caller: End needs MANAGE, starting a course stays owner-only. The list cell stays per record and the narrowing runs after it. Create and edit answers pass the grant they were resolved under instead of null. Registered in the sharing surface guard, the MANAGE handler matrix and the activity feed's verb lines. Refs #1280 --- docs/api/openapi.yaml | 27 ++-- messages/de.json | 1 + messages/en.json | 1 + messages/es.json | 1 + messages/fr.json | 1 + messages/it.json | 1 + messages/ko.json | 1 + messages/nl.json | 1 + messages/pl.json | 1 + messages/pt-BR.json | 1 + .../baselines/source-function-lines.json | 4 +- src/__tests__/sharing-surface-guard.test.ts | 17 ++- src/__tests__/v137-wave-zero-fixtures.test.ts | 2 +- .../api/medications/[id]/courses/end/route.ts | 25 +++- src/app/api/medications/[id]/route.ts | 3 +- src/app/api/medications/route.ts | 15 +- src/lib/medications/course-authority.ts | 48 +++++++ src/lib/medications/courses.ts | 23 +++- src/lib/medications/create-medication.ts | 21 ++- src/lib/medications/read-medication.ts | 8 +- .../update-medication/update-medication.ts | 6 +- src/lib/openapi/routes/medications/paths.ts | 3 +- src/lib/openapi/routes/medications/schemas.ts | 6 +- src/lib/record-activity/activity-verb.ts | 129 ++++++++++-------- src/lib/sharing/grant-access.ts | 24 ++++ tests/fixtures/v137/sharing-matrix.ts | 8 ++ .../integration/medication-course-end.test.ts | 125 ++++++++++++++++- .../sharing-manage-handler-matrix.test.ts | 20 ++- 28 files changed, 421 insertions(+), 102 deletions(-) create mode 100644 src/lib/medications/course-authority.ts diff --git a/docs/api/openapi.yaml b/docs/api/openapi.yaml index d9ce7210b..5f1922c9a 100644 --- a/docs/api/openapi.yaml +++ b/docs/api/openapi.yaml @@ -6872,8 +6872,9 @@ paths: placeholders for slots at or after the moment are removed (tombstoned for sync). A medication with no dates gets the course its window describes, from creation to today. An end date set through `PUT /api/medications/{id}` or the course editor keeps meaning the whole day; only this action ends a course at an instant. Refused with 409 - `medications.course.notRunning` while no course is running — the rule the reads publish as `canEndCourse`. - Owner-only." + `medications.course.notRunning` while no course is running — the rule the reads publish as `canEndCourse`. The + record's owner, or a delegate whose grant manages the record's medications (the level of the medication's PUT); + any other delegate is refused with 403." parameters: - in: path name: id @@ -6889,7 +6890,7 @@ paths: schema: $ref: "#/components/schemas/EndMedicationCourseResponse" "401": *a1 - "403": *a9 + "403": *a15 "404": description: Medication not found (`medications.course.notFound`), or another user's. content: @@ -53955,7 +53956,8 @@ components: canStartCourse: type: boolean description: "v1.40 — whether `POST /api/medications/{id}/courses` may start a new course now: the medication is active, - not one-time, has a course and every course has ended." + not one-time, has a course and every course has ended, and the caller is the record's owner (starting a + course is not delegable, so a delegate always reads false)." nextDueScheduleId: description: The schedule `nextDueAt` belongs to; null when nothing is due. type: @@ -53979,9 +53981,10 @@ components: canEndCourse: type: boolean description: '#1280 — whether `POST /api/medications/{id}/courses/end` may end the course now: true exactly while a - course is running (`courseStatus` CURRENT; a medication with no dates is running). Clients offer "End - course" from this and do not re-derive it; the action refuses with 409 `medications.course.notRunning` when - it is false. Read-only — computed, not stored.' + course is running (`courseStatus` CURRENT; a medication with no dates is running) and the caller may end it: + the owner of the record, or a delegate whose grant manages its medications. False to every other caller. + Clients offer "End course" from this and do not re-derive it; the action refuses with 409 + `medications.course.notRunning` when it is false. Read-only — computed, not stored.' lastTakenAt: anyOf: - type: string @@ -54414,7 +54417,8 @@ components: canStartCourse: type: boolean description: "v1.40 — whether `POST /api/medications/{id}/courses` may start a new course now: the medication is active, - not one-time, has a course and every course has ended." + not one-time, has a course and every course has ended, and the caller is the record's owner (starting a + course is not delegable, so a delegate always reads false)." courseStatus: type: string enum: @@ -54433,9 +54437,10 @@ components: canEndCourse: type: boolean description: '#1280 — whether `POST /api/medications/{id}/courses/end` may end the course now: true exactly while a - course is running (`courseStatus` CURRENT; a medication with no dates is running). Clients offer "End - course" from this and do not re-derive it; the action refuses with 409 `medications.course.notRunning` when - it is false. Read-only — computed, not stored.' + course is running (`courseStatus` CURRENT; a medication with no dates is running) and the caller may end it: + the owner of the record, or a delegate whose grant manages its medications. False to every other caller. + Clients offer "End course" from this and do not re-derive it; the action refuses with 409 + `medications.course.notRunning` when it is false. Read-only — computed, not stored.' stockUnitsRemaining: description: "v1.37.19 — usable inventory units left (same semantics as the list entry's field): NULL = inventory tracking off; 0 = tracking on, supply ran out." diff --git a/messages/de.json b/messages/de.json index bd07d9f7a..cfe7abcc4 100644 --- a/messages/de.json +++ b/messages/de.json @@ -12715,6 +12715,7 @@ "practitionerContactRestore": "{name} hat einen Adressbucheintrag wiederhergestellt", "illnessDayLogUpsert": "{name} hat einen Krankheitstag erfasst", "medicationUpdate": "{name} hat ein Medikament geändert", + "medicationCourseEnd": "{name} hat eine Medikamenteneinnahme beendet", "medicationDelete": "{name} hat ein Medikament gelöscht", "medicationGlp1Update": "{name} hat eine GLP-1-Therapie geändert", "medicationScheduleCreate": "{name} hat einen Einnahmeplan angelegt", diff --git a/messages/en.json b/messages/en.json index da7365a1e..9e7fa4317 100644 --- a/messages/en.json +++ b/messages/en.json @@ -12715,6 +12715,7 @@ "practitionerContactRestore": "{name} restored an address book entry", "illnessDayLogUpsert": "{name} recorded an illness day", "medicationUpdate": "{name} changed a medication", + "medicationCourseEnd": "{name} ended a medication course", "medicationDelete": "{name} removed a medication", "medicationGlp1Update": "{name} changed a GLP-1 course", "medicationScheduleCreate": "{name} added a dosing schedule", diff --git a/messages/es.json b/messages/es.json index a87864f1c..55dd09b69 100644 --- a/messages/es.json +++ b/messages/es.json @@ -12715,6 +12715,7 @@ "practitionerContactRestore": "{name} restauró una entrada de la agenda", "illnessDayLogUpsert": "{name} registró un día de enfermedad", "medicationUpdate": "{name} cambió un medicamento", + "medicationCourseEnd": "{name} terminó un tratamiento", "medicationDelete": "{name} eliminó un medicamento", "medicationGlp1Update": "{name} cambió un tratamiento con GLP-1", "medicationScheduleCreate": "{name} añadió una pauta de dosificación", diff --git a/messages/fr.json b/messages/fr.json index 61afc2d54..d74a6043b 100644 --- a/messages/fr.json +++ b/messages/fr.json @@ -12715,6 +12715,7 @@ "practitionerContactRestore": "{name} a restauré une entrée du carnet d'adresses", "illnessDayLogUpsert": "{name} a enregistré une journée de maladie", "medicationUpdate": "{name} a modifié un médicament", + "medicationCourseEnd": "{name} a terminé un traitement", "medicationDelete": "{name} a supprimé un médicament", "medicationGlp1Update": "{name} a modifié un traitement GLP-1", "medicationScheduleCreate": "{name} a ajouté un schéma de prise", diff --git a/messages/it.json b/messages/it.json index c1affc89d..b1de9b105 100644 --- a/messages/it.json +++ b/messages/it.json @@ -12715,6 +12715,7 @@ "practitionerContactRestore": "{name} ha ripristinato una voce della rubrica", "illnessDayLogUpsert": "{name} ha registrato una giornata di malattia", "medicationUpdate": "{name} ha modificato un farmaco", + "medicationCourseEnd": "{name} ha concluso un ciclo di terapia", "medicationDelete": "{name} ha eliminato un farmaco", "medicationGlp1Update": "{name} ha modificato una terapia GLP-1", "medicationScheduleCreate": "{name} ha aggiunto uno schema di assunzione", diff --git a/messages/ko.json b/messages/ko.json index 7c9a30697..3fbdc541b 100644 --- a/messages/ko.json +++ b/messages/ko.json @@ -12715,6 +12715,7 @@ "practitionerContactRestore": "{name} 님이 주소록 항목을 복원했어요", "illnessDayLogUpsert": "{name} 님이 아픈 날 하루를 기록했어요", "medicationUpdate": "{name} 님이 약을 수정했어요", + "medicationCourseEnd": "{name} 님이 복용 기간을 끝냈어요", "medicationDelete": "{name} 님이 약을 삭제했어요", "medicationGlp1Update": "{name} 님이 GLP-1 요법을 수정했어요", "medicationScheduleCreate": "{name} 님이 복용 일정을 추가했어요", diff --git a/messages/nl.json b/messages/nl.json index 4441b4dd8..af2efc0a4 100644 --- a/messages/nl.json +++ b/messages/nl.json @@ -12715,6 +12715,7 @@ "practitionerContactRestore": "{name} heeft een item in het adresboek hersteld", "illnessDayLogUpsert": "{name} heeft een ziektedag vastgelegd", "medicationUpdate": "{name} heeft een medicijn gewijzigd", + "medicationCourseEnd": "{name} heeft een medicijnkuur beëindigd", "medicationDelete": "{name} heeft een medicijn verwijderd", "medicationGlp1Update": "{name} heeft een GLP-1-kuur gewijzigd", "medicationScheduleCreate": "{name} heeft een doseerschema toegevoegd", diff --git a/messages/pl.json b/messages/pl.json index 8c84eafc1..b0dec38cc 100644 --- a/messages/pl.json +++ b/messages/pl.json @@ -12715,6 +12715,7 @@ "practitionerContactRestore": "{name} przywrócił wpis w książce adresowej", "illnessDayLogUpsert": "{name} zapisał dzień choroby", "medicationUpdate": "{name} zmienił lek", + "medicationCourseEnd": "{name} zakończył kurację", "medicationDelete": "{name} usunął lek", "medicationGlp1Update": "{name} zmienił terapię GLP-1", "medicationScheduleCreate": "{name} dodał schemat dawkowania", diff --git a/messages/pt-BR.json b/messages/pt-BR.json index d325f0552..3137e637c 100644 --- a/messages/pt-BR.json +++ b/messages/pt-BR.json @@ -12715,6 +12715,7 @@ "practitionerContactRestore": "{name} restaurou uma entrada do catálogo de endereços", "illnessDayLogUpsert": "{name} registrou um dia de doença", "medicationUpdate": "{name} alterou um medicamento", + "medicationCourseEnd": "{name} encerrou um tratamento", "medicationDelete": "{name} removeu um medicamento", "medicationGlp1Update": "{name} alterou um curso de GLP-1", "medicationScheduleCreate": "{name} adicionou um cronograma de doses", diff --git a/src/__tests__/baselines/source-function-lines.json b/src/__tests__/baselines/source-function-lines.json index 757ff4a68..dca5a1ead 100644 --- a/src/__tests__/baselines/source-function-lines.json +++ b/src/__tests__/baselines/source-function-lines.json @@ -1,7 +1,7 @@ { "$comment": "Production functions over 200 lines, keyed \"file :: name\", and their length. Maintained by src/__tests__/source-size-ratchet.test.ts.", "count": 351, - "total": 113672, + "total": 113619, "entries": { "app/achievements/page.tsx :: AchievementsPage": 206, "app/api/admin/backups/upload/route.ts :: apiHandler(arg)": 314, @@ -342,7 +342,7 @@ "lib/notifications/senders/web-push.ts :: sendViaWebPush": 268, "lib/personal-records/pr-detection-worker.ts :: detectPersonalRecordsForUser": 239, "lib/personal-records/pr-direction.ts :: getPRDirection": 202, - "lib/record-activity/activity-verb.ts :: recordActivityVerbLine": 325, + "lib/record-activity/activity-verb.ts :: recordActivityVerbLine": 272, "lib/strava/sync.ts :: runStravaSync": 219, "lib/support/report.ts :: collectSupportReportFacts": 332, "lib/targets/build-response.ts :: buildTargetsResponse": 330, diff --git a/src/__tests__/sharing-surface-guard.test.ts b/src/__tests__/sharing-surface-guard.test.ts index e8e67147d..827b63bfd 100644 --- a/src/__tests__/sharing-surface-guard.test.ts +++ b/src/__tests__/sharing-surface-guard.test.ts @@ -1202,6 +1202,10 @@ const DELEGABLE_ROUTES: Record = { domain: "medications", why: "One medication of the record, fetch-then-guard against the resolved user before anything is serialised.", }, + "app/api/medications/[id]/courses/end/route.ts": { + domain: "medications", + why: "Ending one medication's running course now (#1280), behind the course write's own ownership check against the resolved record. The POST is the route's only verb and is manage-level, the level the medication's PUT carried the end date at; starting, editing and removing courses stay owner-only.", + }, "app/api/medications/[id]/ingredients/route.ts": { domain: "medications", why: "One medication's composition per unit (v1.43), behind the ownership check against the resolved record. The PUT is the route's only verb and is manage-level; the label read beside it stays owner-only, because provider work is never admitted inside somebody else's record.", @@ -1882,6 +1886,11 @@ const DELEGABLE_MANAGE_ROUTES: Record = { conditions: ["C4", "C7"], why: "Replacing a medication and its schedule. C4, and C7 because the previous cadence and the tombstoned pending slots are the only unrecoverable part. The DELETE beside it stays refused.", }, + "app/api/medications/[id]/courses/end/route.ts": { + domain: "medications", + conditions: ["C4"], + why: "Ending the running course at this instant, which the medication's PUT already admitted as an end date. C4: the end the course had before is filed beside the new one, with the count of pending slots it took.", + }, "app/api/medications/[id]/ingredients/route.ts": { domain: "medications", conditions: ["C1", "C4"], @@ -2273,8 +2282,12 @@ const ACTOR_ROUTES: Record = { * side). The activity goals route does not: setting or changing goals stays * owner-only, and a delegate sees the goals only as the calendar reports them. * 270 -> 273. + * + * Ending a running course (#1280) joins the record list and the + * manage literal: it was refused under a switch while the medication's PUT, + * which used to carry the end, admitted a manager. 273 -> 275. */ -const FROZEN_ENTRY_COUNT = 273; +const FROZEN_ENTRY_COUNT = 275; /** * The two surfaces that authenticate a Bearer token outside `requireAuth` — @@ -3052,7 +3065,7 @@ describe("(g) the MANAGE route set is frozen", () => { it("keeps the admitted mutation inventory complete and discoverable", () => { expect(ADMITTED_MUTATING_HANDLERS.length).toBeGreaterThan(0); - expect(ADMITTED_MUTATING_HANDLERS.length).toBe(92); + expect(ADMITTED_MUTATING_HANDLERS.length).toBe(93); const expected = ADMITTED_MUTATING_HANDLERS.map( ({ handlerModule, action, level }) => diff --git a/src/__tests__/v137-wave-zero-fixtures.test.ts b/src/__tests__/v137-wave-zero-fixtures.test.ts index 50ea4b712..1de074106 100644 --- a/src/__tests__/v137-wave-zero-fixtures.test.ts +++ b/src/__tests__/v137-wave-zero-fixtures.test.ts @@ -26,7 +26,7 @@ describe("sharing and handler inventories", () => { }); it("keeps every admitted mutation uniquely addressable and fully controlled", () => { - expect(ADMITTED_MUTATING_HANDLERS).toHaveLength(92); + expect(ADMITTED_MUTATING_HANDLERS).toHaveLength(93); expect( new Set( ADMITTED_MUTATING_HANDLERS.map( diff --git a/src/app/api/medications/[id]/courses/end/route.ts b/src/app/api/medications/[id]/courses/end/route.ts index 3a3088282..b2d97a34f 100644 --- a/src/app/api/medications/[id]/courses/end/route.ts +++ b/src/app/api/medications/[id]/courses/end/route.ts @@ -12,12 +12,17 @@ * `medications.course.notRunning` while no course is running, the same rule * the reads publish as `canEndCourse`. * - * Owner-only, like the other course routes. + * MANAGE over the record's medications, the level the medication's own PUT + * (which used to carry the end date) is written at: a manager running + * somebody's regimen may end a course they could already end through the + * edit. Starting, editing and removing courses stay owner-only. The reads + * publish `canEndCourse` false to a caller below that level + * (`course-authority.ts`), so no client offers the button this refuses. */ import { NextRequest } from "next/server"; import { apiError, apiSuccess, getClientIp } from "@/lib/api-response"; -import { apiHandler, requireAuth } from "@/lib/api-handler"; +import { apiHandler, requireRecordAuth } from "@/lib/api-handler"; import { auditLog } from "@/lib/auth/audit"; import { invalidateUserMedications } from "@/lib/cache/invalidate"; import { withIdempotency } from "@/lib/idempotency"; @@ -28,6 +33,7 @@ import { endCurrentCourse, } from "@/lib/medications/courses"; import { dayKeyOfDate } from "@/lib/medications/course-window"; +import { overwriteDetails } from "@/lib/sharing/audit-details"; import { DEFAULT_TIMEZONE } from "@/lib/tz/format"; export const dynamic = "force-dynamic"; @@ -42,11 +48,11 @@ async function postEndCourse( request: NextRequest, { params }: RouteParams, ): Promise { - const { user } = await requireAuth(); + const { user } = await requireRecordAuth("manage", "medications"); const { id } = await params; try { - const window = await endCurrentCourse({ + const { window, previous, droppedPlaceholders } = await endCurrentCourse({ userId: user.id, medicationId: id, timeZone: user.timezone || DEFAULT_TIMEZONE, @@ -60,7 +66,16 @@ async function postEndCourse( await auditLog("medication.course.end", { userId: user.id, ipAddress: getClientIp(request), - details: { medicationId: id, ...body }, + // C4: the end the course had before, so a manager's End reads back + // out of the trail; and how many pending slots it took with it. + details: { + medicationId: id, + ...overwriteDetails({ + before: { endsOn: previous.endsOn, endedAt: previous.endedAt }, + after: { endsOn: window.endsOn, endedAt: window.endedAt }, + }), + droppedPlaceholders, + }, }); annotate({ action: { name: "medication.course.end" }, diff --git a/src/app/api/medications/[id]/route.ts b/src/app/api/medications/[id]/route.ts index 7295d4532..4049c2ab3 100644 --- a/src/app/api/medications/[id]/route.ts +++ b/src/app/api/medications/[id]/route.ts @@ -36,7 +36,7 @@ export const PUT = apiHandler( // excludes every actioned row — and the previous cadence is the one // genuinely unrecoverable part, which is what C7 puts in the audit row. // The DELETE beside it stays refused. - const { user, authMethod } = await requireRecordAuth( + const { user, authMethod, grantId } = await requireRecordAuth( "manage", "medications", ); @@ -60,6 +60,7 @@ export const PUT = apiHandler( const outcome = await updateMedication({ user, + grantId, authMethod, id, existing, diff --git a/src/app/api/medications/route.ts b/src/app/api/medications/route.ts index e65668ae3..c5c9cc5f9 100644 --- a/src/app/api/medications/route.ts +++ b/src/app/api/medications/route.ts @@ -13,12 +13,16 @@ import { unstableExternalIdShape, } from "@/lib/validations/external-id"; import { readMedicationsListCached } from "@/lib/medications/list-read"; +import { + courseActionAuthority, + withCourseActionAuthority, +} from "@/lib/medications/course-authority"; import { createMedication } from "@/lib/medications/create-medication"; import { withIdempotency } from "@/lib/idempotency"; import { NextRequest } from "next/server"; export const GET = apiHandler(async () => { - const { user } = await requireRecordAuth("read", "medications"); + const { user, grantId } = await requireRecordAuth("read", "medications"); // Cache the list shape on userId. The 60 s fresh TTL bounds the // cross-midnight staleness window for `todayEventCount` and the @@ -30,7 +34,11 @@ export const GET = apiHandler(async () => { // `invalidateUserMedications({ evict: true })` before the next read // lands; background sync paths mark the bucket stale instead. The RSC // wrapper (`src/app/medications/page.tsx`) shares this exact cell. - const result = await readMedicationsListCached(user); + const cached = await readMedicationsListCached(user); + // The cell is the record's; End and Start are narrowed to the caller after + // it, so one caller's authority never lands in another's answer (#1280). + const authority = await courseActionAuthority(grantId); + const result = cached.map((row) => withCourseActionAuthority(row, authority)); annotate({ action: { name: "medication.list" }, @@ -55,7 +63,7 @@ async function postMedication(request: NextRequest): Promise { // it afterwards — `PUT /api/medications/[id]` and the DELETE beside it stay // on `requireAuth()` and refuse under a switch. Adding is help; rewriting // the owner's cabinet is not. - const { user } = await requireRecordAuth("write", "medications"); + const { user, grantId } = await requireRecordAuth("write", "medications"); const { data: body, error: jsonError } = await safeJson(request, { maxBytes: 64 * 1024, @@ -80,6 +88,7 @@ async function postMedication(request: NextRequest): Promise { const outcome = await createMedication({ user, + grantId, data: parsed.data, ipAddress: getClientIp(request), }); diff --git a/src/lib/medications/course-authority.ts b/src/lib/medications/course-authority.ts new file mode 100644 index 000000000..6b8248944 --- /dev/null +++ b/src/lib/medications/course-authority.ts @@ -0,0 +1,48 @@ +/** + * #1280 — the course actions a medication read offers, narrowed to what the + * CALLER may do. + * + * `canEndCourse` and `canStartCourse` are resolved from the course alone + * (`intake-actionable.ts`, `courses.ts`), and the course is the record's. + * The routes behind them are not open to everyone who can read the record: + * End is a MANAGE write (`POST /courses/end`), and starting, editing or + * removing a course stays with the owner (`/courses`, `/courses/{courseId}`). + * A read that served the course's answer to a viewer would paint a button + * whose press is refused, so every read that publishes the two flags passes + * them through here with the caller's authority. + * + * The record-wide list is cached per record, never per caller, so the + * narrowing runs after the cache and never lands in it. + */ +import { grantIdAllows } from "@/lib/sharing/grant-access"; + +export interface CourseActionAuthority { + /** The caller may end the running course (MANAGE over medications). */ + mayEnd: boolean; + /** The caller may start a course (the record's owner). */ + mayStart: boolean; +} + +/** The authority of the request resolved under `grantId` (null: the owner). */ +export async function courseActionAuthority( + grantId: string | null, +): Promise { + if (grantId === null) return { mayEnd: true, mayStart: true }; + return { + mayEnd: await grantIdAllows(grantId, "manage", "medications"), + mayStart: false, + }; +} + +/** The read row with its course actions narrowed to the caller's authority. */ +export function withCourseActionAuthority>( + row: T, + authority: CourseActionAuthority, +): T { + if (authority.mayEnd && authority.mayStart) return row; + return { + ...row, + canEndCourse: authority.mayEnd && row.canEndCourse === true, + canStartCourse: authority.mayStart && row.canStartCourse === true, + }; +} diff --git a/src/lib/medications/courses.ts b/src/lib/medications/courses.ts index 73471fead..398ab281f 100644 --- a/src/lib/medications/courses.ts +++ b/src/lib/medications/courses.ts @@ -482,9 +482,14 @@ function keptEndedAt( * (`canEndCourse`): ending an ended course would reopen it up to today, and * ending one that has not started would invent a one-day course. */ -export async function endCurrentCourse( - args: WriteContext, -): Promise { +export async function endCurrentCourse(args: WriteContext): Promise<{ + /** The window the medication reads after the end. */ + window: ProjectedWindow; + /** The window it read before, for the audit row's overwrite (C4). */ + previous: ProjectedWindow; + /** How many pending placeholders at or after the moment were tombstoned. */ + droppedPlaceholders: number; +}> { const now = args.now ?? new Date(); const todayKey = userDayKey(now, args.timeZone); const { endsOn: today, endedAt } = courseEndNow(now, args.timeZone); @@ -528,7 +533,7 @@ export async function endCurrentCourse( }, }); } - await tx.medicationIntakeEvent.updateMany({ + const dropped = await tx.medicationIntakeEvent.updateMany({ where: { userId: args.userId, medicationId: args.medicationId, @@ -540,7 +545,15 @@ export async function endCurrentCourse( }, data: { deletedAt: now, syncVersion: { increment: 1 } }, }); - return project(tx, args.medicationId); + return { + window: await project(tx, args.medicationId), + previous: { + startsOn: medication.startsOn, + endsOn: medication.endsOn, + endedAt: medication.endedAt, + }, + droppedPlaceholders: dropped.count, + }; }); } diff --git a/src/lib/medications/create-medication.ts b/src/lib/medications/create-medication.ts index 609fcc1b3..74ca7b7fc 100644 --- a/src/lib/medications/create-medication.ts +++ b/src/lib/medications/create-medication.ts @@ -68,6 +68,7 @@ export type CreateMedicationOutcome = */ async function existingMirrorBody( user: ReadUser, + grantId: string | null, medication: Medication & { schedules: MedicationSchedule[] }, ): Promise { annotate({ @@ -81,7 +82,7 @@ async function existingMirrorBody( // The read model, exactly as `GET /api/medications/{id}` answers it. const body = await medicationDetail({ user, - grantId: null, + grantId, medication, announce: false, }); @@ -98,6 +99,7 @@ async function existingMirrorBody( */ async function existingMirrorOrCap( user: ReadUser, + grantId: string | null, externalSource: NonNullable, externalId: string, ): Promise { @@ -107,7 +109,7 @@ async function existingMirrorOrCap( include: { schedules: { orderBy: MEDICATION_SCHEDULE_ORDER } }, }); if (existing) { - return existingMirrorBody(user, existing); + return existingMirrorBody(user, grantId, existing); } // Growth guard. This point is only reached when the idempotency lookup @@ -410,10 +412,12 @@ function insertMedication( export async function createMedication(input: { user: ReadUser; + /** The grant the request was resolved under; null for the owner. */ + grantId: string | null; data: CreateMedicationInput; ipAddress: string | null; }): Promise { - const { user, data } = input; + const { user, grantId, data } = input; const { name, category, @@ -439,7 +443,12 @@ export async function createMedication(input: { } if (externalSource !== undefined && externalId !== undefined) { - const mirror = await existingMirrorOrCap(user, externalSource, externalId); + const mirror = await existingMirrorOrCap( + user, + grantId, + externalSource, + externalId, + ); if (mirror) return mirror; } @@ -483,7 +492,7 @@ export async function createMedication(input: { where: { userId: user.id, externalSource, externalId }, include: { schedules: { orderBy: MEDICATION_SCHEDULE_ORDER } }, }); - if (winner) return existingMirrorBody(user, winner); + if (winner) return existingMirrorBody(user, grantId, winner); } throw err; } @@ -517,7 +526,7 @@ export async function createMedication(input: { // The read model, exactly as `GET /api/medications/{id}` answers it. const body = await medicationDetail({ user, - grantId: null, + grantId, medication, announce: false, }); diff --git a/src/lib/medications/read-medication.ts b/src/lib/medications/read-medication.ts index c41504c7f..b78074058 100644 --- a/src/lib/medications/read-medication.ts +++ b/src/lib/medications/read-medication.ts @@ -36,6 +36,10 @@ import { floorWhole } from "@/lib/medications/units-per-dose"; import { resolveIntakeActionability } from "@/lib/medications/intake-actionable"; import { DEFAULT_TIMEZONE } from "@/lib/tz/format"; import { buildRelated } from "@/lib/links/related"; +import { + courseActionAuthority, + withCourseActionAuthority, +} from "@/lib/medications/course-authority"; export type ReadUser = Pick; type MedicationWithSchedules = Medication & { @@ -262,7 +266,7 @@ export async function medicationDetail(input: { ) ).get(id); - return { + const body = { ...medication, unitsPerDose: Number(medication.unitsPerDose), // `schedules: []` plus `recordedSchedules` when intake tracking is off @@ -296,4 +300,6 @@ export async function medicationDetail(input: { id, }), }; + // End and Start as the CALLER may press them (#1280). + return withCourseActionAuthority(body, await courseActionAuthority(grantId)); } diff --git a/src/lib/medications/update-medication/update-medication.ts b/src/lib/medications/update-medication/update-medication.ts index b698fb894..5cc341b57 100644 --- a/src/lib/medications/update-medication/update-medication.ts +++ b/src/lib/medications/update-medication/update-medication.ts @@ -300,13 +300,15 @@ async function auditMedicationEdit( /** Edit one medication the route found and authorised. */ export async function updateMedication(input: { user: EditUser; + /** The grant the request was resolved under; null for the owner. */ + grantId: string | null; authMethod: string; id: string; existing: EditableMedication; data: MedicationEdit; ipAddress: string | null; }): Promise { - const { user, id } = input; + const { user, grantId, id } = input; const plan = await planMedicationEdit(input); if (!plan.ok) return plan; const { write } = plan; @@ -361,7 +363,7 @@ export async function updateMedication(input: { // The read model, exactly as `GET /api/medications/{id}` answers it. const body = await medicationDetail({ user, - grantId: null, + grantId, medication, announce: false, }); diff --git a/src/lib/openapi/routes/medications/paths.ts b/src/lib/openapi/routes/medications/paths.ts index 0f8961e85..253dd9dc2 100644 --- a/src/lib/openapi/routes/medications/paths.ts +++ b/src/lib/openapi/routes/medications/paths.ts @@ -1184,7 +1184,7 @@ export const medicationPaths: NonNullable = { tags: ["Medications"], summary: "End the running course now", description: - "#1280 — \"End course\". No body. The running course's last day becomes today on the person's clock and the moment is stored as its `endedAt`: a dose slot later today is not due, not reminded and not expected by compliance, a dose taken earlier today still counts, and `courseStatus` reads ENDED at once. Pending placeholders for slots at or after the moment are removed (tombstoned for sync). A medication with no dates gets the course its window describes, from creation to today. An end date set through `PUT /api/medications/{id}` or the course editor keeps meaning the whole day; only this action ends a course at an instant. Refused with 409 `medications.course.notRunning` while no course is running — the rule the reads publish as `canEndCourse`. Owner-only.", + "#1280 — \"End course\". No body. The running course's last day becomes today on the person's clock and the moment is stored as its `endedAt`: a dose slot later today is not due, not reminded and not expected by compliance, a dose taken earlier today still counts, and `courseStatus` reads ENDED at once. Pending placeholders for slots at or after the moment are removed (tombstoned for sync). A medication with no dates gets the course its window describes, from creation to today. An end date set through `PUT /api/medications/{id}` or the course editor keeps meaning the whole day; only this action ends a course at an instant. Refused with 409 `medications.course.notRunning` while no course is running — the rule the reads publish as `canEndCourse`. The record's owner, or a delegate whose grant manages the record's medications (the level of the medication's PUT); any other delegate is refused with 403.", requestParams: { path: z.object({ id: z.string() }) }, responses: { ...idempotentWrite(), @@ -1213,6 +1213,7 @@ export const medicationPaths: NonNullable = { "No course is running now (`medications.course.notRunning`): it has ended already or has not started.", content: { "application/json": { schema: errorEnvelope } }, }, + ...recordRefusal(), ...stdResponses, }, }, diff --git a/src/lib/openapi/routes/medications/schemas.ts b/src/lib/openapi/routes/medications/schemas.ts index 6a5d44487..314a72e40 100644 --- a/src/lib/openapi/routes/medications/schemas.ts +++ b/src/lib/openapi/routes/medications/schemas.ts @@ -110,7 +110,7 @@ const medicationCourseFields = { canStartCourse: z .boolean() .describe( - "v1.40 — whether `POST /api/medications/{id}/courses` may start a new course now: the medication is active, not one-time, has a course and every course has ended.", + "v1.40 — whether `POST /api/medications/{id}/courses` may start a new course now: the medication is active, not one-time, has a course and every course has ended, and the caller is the record's owner (starting a course is not delegable, so a delegate always reads false).", ), }; @@ -463,7 +463,7 @@ export const medicationListEntry = medicationResource canEndCourse: z .boolean() .describe( - '#1280 — whether `POST /api/medications/{id}/courses/end` may end the course now: true exactly while a course is running (`courseStatus` CURRENT; a medication with no dates is running). Clients offer "End course" from this and do not re-derive it; the action refuses with 409 `medications.course.notRunning` when it is false. Read-only — computed, not stored.', + '#1280 — whether `POST /api/medications/{id}/courses/end` may end the course now: true exactly while a course is running (`courseStatus` CURRENT; a medication with no dates is running) and the caller may end it: the owner of the record, or a delegate whose grant manages its medications. False to every other caller. Clients offer "End course" from this and do not re-derive it; the action refuses with 409 `medications.course.notRunning` when it is false. Read-only — computed, not stored.', ), lastTakenAt: isoDateTime() .nullable() @@ -534,7 +534,7 @@ export const medicationDetailEntry = medicationResource canEndCourse: z .boolean() .describe( - '#1280 — whether `POST /api/medications/{id}/courses/end` may end the course now: true exactly while a course is running (`courseStatus` CURRENT; a medication with no dates is running). Clients offer "End course" from this and do not re-derive it; the action refuses with 409 `medications.course.notRunning` when it is false. Read-only — computed, not stored.', + '#1280 — whether `POST /api/medications/{id}/courses/end` may end the course now: true exactly while a course is running (`courseStatus` CURRENT; a medication with no dates is running) and the caller may end it: the owner of the record, or a delegate whose grant manages its medications. False to every other caller. Clients offer "End course" from this and do not re-derive it; the action refuses with 409 `medications.course.notRunning` when it is false. Read-only — computed, not stored.', ), stockUnitsRemaining: z .number() diff --git a/src/lib/record-activity/activity-verb.ts b/src/lib/record-activity/activity-verb.ts index cbaae53b6..801cafb19 100644 --- a/src/lib/record-activity/activity-verb.ts +++ b/src/lib/record-activity/activity-verb.ts @@ -63,6 +63,8 @@ export function recordActivityVerbLine( ): string { const refused = refusedAttemptVerb(t, action, name); if (refused !== null) return refused; + const medication = medicationManageVerb(t, action, name); + if (medication !== null) return medication; switch (action) { case "measurement.create": return t("recordSharing.activityVerb.measurementCreate", { name }); @@ -255,61 +257,6 @@ export function recordActivityVerbLine( name, }); - case "medication.update": - return t("recordSharing.activityVerb.medicationUpdate", { name }); - case "medication.category.custom.create": - return t("recordSharing.activityVerb.medicationCategoryCreate", { - name, - }); - case "medication.delete": - return t("recordSharing.activityVerb.medicationDelete", { name }); - case "medication.ingredients.update": - return t("recordSharing.activityVerb.medicationIngredientsUpdate", { - name, - }); - case "medication.glp1.update": - return t("recordSharing.activityVerb.medicationGlp1Update", { name }); - case "medication.schedule_revision.created": - return t("recordSharing.activityVerb.medicationScheduleCreate", { name }); - case "medication.schedule_revision.updated": - return t("recordSharing.activityVerb.medicationScheduleUpdate", { name }); - case "medication.schedule_revision.deleted": - return t("recordSharing.activityVerb.medicationScheduleDelete", { name }); - case "medication.inventory.create": - return t("recordSharing.activityVerb.medicationInventoryCreate", { - name, - }); - case "medication.inventory.update": - return t("recordSharing.activityVerb.medicationInventoryUpdate", { - name, - }); - case "medication.inventory.delete": - return t("recordSharing.activityVerb.medicationInventoryDelete", { - name, - }); - case "medication.sideEffect.delete": - return t("recordSharing.activityVerb.medicationSideEffectDelete", { - name, - }); - case "medication.intake.update": - return t("recordSharing.activityVerb.medicationIntakeUpdate", { name }); - case "medication.intake.delete": - return t("recordSharing.activityVerb.medicationIntakeDelete", { name }); - case "medication.intake.bulk_delete": - return t("recordSharing.activityVerb.medicationIntakeBulkDelete", { - name, - }); - // The one that empties a medication's whole history in one request. It - // says so, because "removed several doses" would read as a correction. - case "medication.intake.purge": - return t("recordSharing.activityVerb.medicationIntakePurge", { name }); - case "medication.intake.import.kickoff": - return t("recordSharing.activityVerb.medicationIntakeImport", { name }); - case "medication.intake.history.import.kickoff": - return t("recordSharing.activityVerb.medicationIntakeHistoryImport", { - name, - }); - case "moodEntry.create": return t("recordSharing.activityVerb.moodEntryCreate", { name }); case "moodEntry.update": @@ -477,3 +424,75 @@ function refusedAttemptVerb( return null; } } + +/** + * The sentence for a change to a medication the owner made (the MANAGE verbs + * of the medications section), or null when `action` is none of them. Asked + * by `recordActivityVerbLine` after the refusals. + */ +function medicationManageVerb( + t: Translator, + action: string, + name: string, +): string | null { + switch (action) { + case "medication.update": + return t("recordSharing.activityVerb.medicationUpdate", { name }); + case "medication.course.end": + return t("recordSharing.activityVerb.medicationCourseEnd", { name }); + case "medication.category.custom.create": + return t("recordSharing.activityVerb.medicationCategoryCreate", { + name, + }); + case "medication.delete": + return t("recordSharing.activityVerb.medicationDelete", { name }); + case "medication.ingredients.update": + return t("recordSharing.activityVerb.medicationIngredientsUpdate", { + name, + }); + case "medication.glp1.update": + return t("recordSharing.activityVerb.medicationGlp1Update", { name }); + case "medication.schedule_revision.created": + return t("recordSharing.activityVerb.medicationScheduleCreate", { name }); + case "medication.schedule_revision.updated": + return t("recordSharing.activityVerb.medicationScheduleUpdate", { name }); + case "medication.schedule_revision.deleted": + return t("recordSharing.activityVerb.medicationScheduleDelete", { name }); + case "medication.inventory.create": + return t("recordSharing.activityVerb.medicationInventoryCreate", { + name, + }); + case "medication.inventory.update": + return t("recordSharing.activityVerb.medicationInventoryUpdate", { + name, + }); + case "medication.inventory.delete": + return t("recordSharing.activityVerb.medicationInventoryDelete", { + name, + }); + case "medication.sideEffect.delete": + return t("recordSharing.activityVerb.medicationSideEffectDelete", { + name, + }); + case "medication.intake.update": + return t("recordSharing.activityVerb.medicationIntakeUpdate", { name }); + case "medication.intake.delete": + return t("recordSharing.activityVerb.medicationIntakeDelete", { name }); + case "medication.intake.bulk_delete": + return t("recordSharing.activityVerb.medicationIntakeBulkDelete", { + name, + }); + // The one that empties a medication's whole history in one request. It + // says so, because "removed several doses" would read as a correction. + case "medication.intake.purge": + return t("recordSharing.activityVerb.medicationIntakePurge", { name }); + case "medication.intake.import.kickoff": + return t("recordSharing.activityVerb.medicationIntakeImport", { name }); + case "medication.intake.history.import.kickoff": + return t("recordSharing.activityVerb.medicationIntakeHistoryImport", { + name, + }); + default: + return null; + } +} diff --git a/src/lib/sharing/grant-access.ts b/src/lib/sharing/grant-access.ts index 99cfd6cbd..7c380055e 100644 --- a/src/lib/sharing/grant-access.ts +++ b/src/lib/sharing/grant-access.ts @@ -265,3 +265,27 @@ export function touchGrantUsage( .then(() => undefined) .catch(() => undefined); } + +/** + * Whether the request resolved under `grantId` may `need` over `domain` — + * the question a READ answers when it publishes an action the caller may or + * may not be allowed to take (`canEndCourse` on the medication reads). + * + * `null` is the record's owner, who may do everything the record allows. A + * grant is re-read here rather than trusted from the resolver because the + * resolver only proved the level the READ route declared; the action the + * read advertises is declared on another route at another level, and the + * answer has to be the one that route would give. + */ +export async function grantIdAllows( + grantId: string | null, + need: GrantNeed, + domain: ShareScope, + db: GrantDb = prisma, + now: Date = new Date(), +): Promise { + if (grantId === null) return true; + const grant = await db.accountGrant.findUnique({ where: { id: grantId } }); + if (!grant) return false; + return grantAllows(grant, need, now) && grantCoversDomain(grant, domain); +} diff --git a/tests/fixtures/v137/sharing-matrix.ts b/tests/fixtures/v137/sharing-matrix.ts index f1e55a2d1..00df119aa 100644 --- a/tests/fixtures/v137/sharing-matrix.ts +++ b/tests/fixtures/v137/sharing-matrix.ts @@ -514,6 +514,14 @@ export const ADMITTED_MUTATING_HANDLERS = [ "manage", "update", ), + handler( + "app/api/medications/[id]/courses/end/route.ts", + "/api/medications/[id]/courses/end", + "POST", + "medications", + "manage", + "update", + ), handler( "app/api/medications/[id]/schedule-revisions/route.ts", "/api/medications/[id]/schedule-revisions", diff --git a/tests/integration/medication-course-end.test.ts b/tests/integration/medication-course-end.test.ts index f60717e17..28c76232f 100644 --- a/tests/integration/medication-course-end.test.ts +++ b/tests/integration/medication-course-end.test.ts @@ -24,7 +24,7 @@ import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; import { NextRequest } from "next/server"; 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"); @@ -448,6 +448,129 @@ describe("canEndCourse follows the course", () => { }); }); +describe("End follows what the caller may do in a shared record", () => { + /** A second person who signs in and opens the owner's record at `access`. */ + async function openAs( + ownerId: string, + access: "READ" | "WRITE" | "MANAGE", + ): Promise { + const prisma = getPrismaClient(); + const delegate = await prisma.user.create({ + data: { + username: `course-end-${access.toLowerCase()}`, + email: `course-end-${access.toLowerCase()}@example.test`, + timezone: TZ, + locale: "en", + }, + }); + const { inviteGrant, acceptGrant } = await import("@/lib/sharing/grants"); + const invited = await inviteGrant({ + grantorId: ownerId, + granteeId: delegate.id, + access, + scope: null, + }); + await acceptGrant({ grantId: invited.id, granteeId: delegate.id }); + const session = await prisma.session.create({ + data: { + userId: delegate.id, + expiresAt: new Date("2030-01-01T00:00:00.000Z"), + }, + }); + cookieJar.set("healthlog_session", session.id); + await switchSessionTo(session.id, ownerId); + return delegate.id; + } + + async function listed(id: string) { + const list = await call( + "@/app/api/medications/route", + "GET", + "/api/medications", + {}, + ); + expect(list.status).toBe(200); + return ( + list.body.data as Array<{ + id: string; + canEndCourse: boolean; + canStartCourse: boolean; + }> + ).find((m) => m.id === id); + } + + it("lets a manager end the course and names them in the trail", async () => { + at(ENDED_AT); + const ownerId = await makeUser(); + const id = await seedTwiceDaily(ownerId); + const managerId = await openAs(ownerId, "MANAGE"); + + expect((await detail(id)).canEndCourse).toBe(true); + expect((await listed(id))?.canEndCourse).toBe(true); + + const res = await endCourse(id); + expect(res.status, JSON.stringify(res.body)).toBe(200); + const row = await getPrismaClient().medication.findUniqueOrThrow({ + where: { id }, + select: { endedAt: true }, + }); + expect(row.endedAt?.toISOString()).toBe(ENDED_AT); + + const audit = await getPrismaClient().auditLog.findFirstOrThrow({ + where: { action: "medication.course.end" }, + }); + expect(audit.userId).toBe(ownerId); + expect(audit.actorUserId).toBe(managerId); + expect( + typeof audit.details === "string" + ? JSON.parse(audit.details) + : audit.details, + ).toMatchObject({ + medicationId: id, + fields: ["endedAt", "endsOn"], + previous: { endsOn: null, endedAt: null }, + }); + }); + + it.each(["READ", "WRITE"] as const)( + "offers a %s delegate neither End nor Start, and refuses End", + async (access) => { + at(ENDED_AT); + const ownerId = await makeUser(); + const id = await seedTwiceDaily(ownerId); + await openAs(ownerId, access); + + const read = await detail(id); + expect(read.courseStatus).toBe("CURRENT"); + expect(read.canEndCourse).toBe(false); + expect(read.canStartCourse).toBe(false); + const entry = await listed(id); + expect(entry?.canEndCourse).toBe(false); + expect(entry?.canStartCourse).toBe(false); + + const res = await endCourse(id); + expect(res.status).toBe(403); + const row = await getPrismaClient().medication.findUniqueOrThrow({ + where: { id }, + select: { endedAt: true }, + }); + expect(row.endedAt).toBeNull(); + }, + ); + + it("does not let the delegate's view leak into the owner's cached list", async () => { + at(ENDED_AT); + const ownerId = await makeUser(); + const id = await seedTwiceDaily(ownerId); + const ownerSession = cookieJar.get("healthlog_session")!; + await openAs(ownerId, "READ"); + expect((await listed(id))?.canEndCourse).toBe(false); + + cookieJar.set("healthlog_session", ownerSession); + expect((await listed(id))?.canEndCourse).toBe(true); + }); +}); + describe("an edit that moves the end clears the instant", () => { it("reopens the course to the end of its new last day", async () => { at(ENDED_AT); diff --git a/tests/integration/sharing-manage-handler-matrix.test.ts b/tests/integration/sharing-manage-handler-matrix.test.ts index 12debf6d2..abc6f0f37 100644 --- a/tests/integration/sharing-manage-handler-matrix.test.ts +++ b/tests/integration/sharing-manage-handler-matrix.test.ts @@ -282,7 +282,7 @@ const ROUTE_IMPORTS = import.meta.glob>( describe("the complete MANAGE handler matrix", () => { it("starts with a non-empty, exact handler inventory", () => { - expect(ADMITTED_MUTATING_HANDLERS.length).toBe(92); + expect(ADMITTED_MUTATING_HANDLERS.length).toBe(93); }); }); @@ -2133,6 +2133,20 @@ proveManageRoute("PUT /api/medications/[id]", { )?.dose === "20mg", }); +proveManageRoute("POST /api/medications/[id]/courses/end", { + route: "/api/medications/[id]/courses/end", + prepare: (ownerId) => makeMedication(ownerId), + params: (medication) => ({ id: medication.id }), + ok: 200, + auditAction: "medication.course.end", + applied: async (medication) => + ( + await getPrismaClient().medication.findUnique({ + where: { id: medication.id }, + }) + )?.endedAt != null, +}); + proveManageRoute("POST /api/medications/[id]/schedule-revisions", { route: "/api/medications/[id]/schedule-revisions", prepare: async (ownerId) => ({ @@ -3006,13 +3020,13 @@ describe("the conditions the admissions were granted on", () => { describe("the complete MANAGE handler matrix", () => { it("registers one strict actor-and-effect driver for every admission", () => { const expected = ADMITTED_MUTATING_HANDLERS.map(matrixKey).sort(); - expect(STRICT_DRIVER_KEYS.size).toBe(92); + expect(STRICT_DRIVER_KEYS.size).toBe(93); expect([...STRICT_DRIVER_KEYS].sort()).toEqual(expected); }); it("executes every registered driver through its owned effect and actor audit", () => { const expected = ADMITTED_MUTATING_HANDLERS.map(matrixKey).sort(); - expect(REAL_EFFECT_KEYS.size).toBe(92); + expect(REAL_EFFECT_KEYS.size).toBe(93); expect([...REAL_EFFECT_KEYS].sort()).toEqual(expected); }); });