mirror of
https://github.com/MBombeck/HealthLog.git
synced 2026-11-08 12:32:43 +00:00
Make the reset-every-threshold delete ask first
DELETE /api/user/thresholds without ?metric= erased the account's whole
override set on an omitted query parameter alone: no confirmation, no dry
run, and no rate limit of its own while the PUT beside it had one.
The wide form now wants { "confirm": "RESET_THRESHOLDS" } in the body — the
same typed-confirmation shape /api/settings/account and /api/settings/data
already use — and refuses 422 without writing anything. The ?metric= form
is untouched: it resets one band the person can retype, and the targets
sheet fires it in a loop. Both forms now run the PUT's 30 / 5 min limit.
The settings card's "reset all" control already opened a confirm dialog;
it now sends that answer to the server too. Its token is compared against
the route's declaration in the tests, so the two ends cannot drift apart
into a permanent 422.
This commit is contained in:
1 parent
c4544556f7
commit
fffdc83ca4
5 files changed
+282
-15
No files matched your search
+31
-6
@@ -7701,18 +7701,27 @@ paths:
|
||||
summary: Reset one threshold override, or all of them
|
||||
description: >-
|
||||
With `?metric=` it drops that metric's override; WITHOUT the parameter it drops EVERY override on the account.
|
||||
There is no confirmation step and no dry run, so a client that means to reset one metric must send the
|
||||
parameter.
|
||||
The wide form is not reachable by omitting the parameter alone — it requires `{ "confirm": "RESET_THRESHOLDS" }`
|
||||
in the body, the same typed-confirmation shape account deletion and the data reset use. The `?metric=` form
|
||||
takes no body and is unchanged.
|
||||
|
||||
|
||||
Audit-logged with the before and after maps. Unlike the PUT this path runs no rate limit of its own.
|
||||
Audit-logged with the before and after maps. Rate-limited 30 / 5 min per user, matching the PUT.
|
||||
parameters:
|
||||
- in: query
|
||||
name: metric
|
||||
schema:
|
||||
description: The single metric to reset. Omit to reset every override on the account.
|
||||
description: The single metric to reset. Omit to reset every override on the account — which then requires the
|
||||
confirmation body.
|
||||
$ref: "#/components/schemas/ThresholdMetric"
|
||||
description: The single metric to reset. Omit to reset every override on the account.
|
||||
description: The single metric to reset. Omit to reset every override on the account — which then requires the
|
||||
confirmation body.
|
||||
requestBody:
|
||||
required: false
|
||||
content:
|
||||
application/json:
|
||||
schema:
|
||||
$ref: "#/components/schemas/ThresholdsResetAllRequest"
|
||||
responses:
|
||||
"200":
|
||||
description: The remaining override map.
|
||||
@@ -7727,7 +7736,12 @@ paths:
|
||||
schema:
|
||||
$ref: "#/components/schemas/ErrorEnvelope"
|
||||
"401": *a1
|
||||
"422": *a4
|
||||
"422":
|
||||
description: '`metric` was omitted and the body did not carry `{ "confirm": "RESET_THRESHOLDS" }`. Nothing was reset.'
|
||||
content:
|
||||
application/json:
|
||||
schema:
|
||||
$ref: "#/components/schemas/ErrorEnvelope"
|
||||
"429": *a5
|
||||
/api/auth/me/devices:
|
||||
get:
|
||||
@@ -23030,6 +23044,17 @@ components:
|
||||
- TOTAL_BODY_WATER
|
||||
- BONE_MASS
|
||||
- OXYGEN_SATURATION
|
||||
ThresholdsResetAllRequest:
|
||||
type: object
|
||||
properties:
|
||||
confirm:
|
||||
type: string
|
||||
const: RESET_THRESHOLDS
|
||||
description: Required when `metric` is omitted. Compared exactly; any other value is refused.
|
||||
required:
|
||||
- confirm
|
||||
description: Confirmation body for the reset-everything form. Send no body at all when resetting a single metric with
|
||||
`?metric=`.
|
||||
DevicePatchRequest:
|
||||
type: object
|
||||
properties:
|
||||
|
||||
@@ -32,6 +32,9 @@ vi.mock("@/lib/analytics/effective-range", async (importOriginal) => {
|
||||
};
|
||||
});
|
||||
vi.mock("@/lib/logging/transports", () => ({ emitIfSampled: vi.fn() }));
|
||||
vi.mock("@/lib/cache/invalidate", () => ({
|
||||
invalidateUserProfile: vi.fn(),
|
||||
}));
|
||||
vi.mock("@/lib/db-compat", () => ({
|
||||
ensureDbCompatibility: vi.fn().mockResolvedValue(undefined),
|
||||
}));
|
||||
@@ -44,7 +47,7 @@ vi.mock("next/headers", () => ({
|
||||
})),
|
||||
}));
|
||||
|
||||
import { GET, PUT } from "../route";
|
||||
import { GET, PUT, DELETE } from "../route";
|
||||
import { getSession } from "@/lib/auth/session";
|
||||
import { checkRateLimit } from "@/lib/rate-limit";
|
||||
import { prisma } from "@/lib/db";
|
||||
@@ -63,6 +66,31 @@ function putReq(body: unknown): NextRequest {
|
||||
});
|
||||
}
|
||||
|
||||
function deleteReq(
|
||||
options: { metric?: string; body?: unknown } = {},
|
||||
): NextRequest {
|
||||
const url = options.metric
|
||||
? `http://localhost/api/user/thresholds?metric=${options.metric}`
|
||||
: "http://localhost/api/user/thresholds";
|
||||
return new NextRequest(url, {
|
||||
method: "DELETE",
|
||||
...(options.body === undefined
|
||||
? {}
|
||||
: {
|
||||
headers: { "content-type": "application/json" },
|
||||
body: JSON.stringify(options.body),
|
||||
}),
|
||||
});
|
||||
}
|
||||
|
||||
/** The stored override set the DELETE branches read before they write. */
|
||||
function armOverrides(overrides: Record<string, unknown>) {
|
||||
vi.mocked(prisma.user.findUnique).mockResolvedValue({
|
||||
thresholdsJson: overrides,
|
||||
} as never);
|
||||
vi.mocked(prisma.user.update).mockResolvedValue({} as never);
|
||||
}
|
||||
|
||||
beforeEach(() => {
|
||||
vi.resetAllMocks();
|
||||
vi.mocked(getSession).mockResolvedValue(SESSION_OK as never);
|
||||
@@ -111,6 +139,121 @@ describe("PUT /api/user/thresholds — 422 multi-issue (v1.4.43 W6)", () => {
|
||||
});
|
||||
});
|
||||
|
||||
/**
|
||||
* The parameterless DELETE erases every band the account has tuned, and it
|
||||
* used to do it on an omitted query parameter alone — no confirmation, no dry
|
||||
* run, and no rate limit of its own while the PUT beside it had one.
|
||||
*
|
||||
* The confirmation is the same shape the other two wide deletes use
|
||||
* (`/api/settings/account` wants `DELETE_ACCOUNT`, `/api/settings/data` wants
|
||||
* `DELETE`): a literal in the body, compared exactly. What is pinned here is
|
||||
* the pair of properties that make it worth anything — the refusal happens,
|
||||
* AND nothing was written when it does. A status assertion on its own would
|
||||
* stay green if the wipe ran first and the 422 came after it.
|
||||
*
|
||||
* The single-metric form is pinned as UNCHANGED in the same block, because the
|
||||
* failure mode of a confirmation is that it spreads to the narrow path and
|
||||
* breaks the targets sheet, which resets metrics in a loop.
|
||||
*
|
||||
* Mutation check: drop the `confirm !== RESET_ALL_CONFIRMATION` branch and the
|
||||
* first two cases go red; drop the rate-limit call and the fourth goes red;
|
||||
* extend the confirmation to the `?metric=` arm and the fifth goes red.
|
||||
*/
|
||||
describe("DELETE /api/user/thresholds — the wide form asks first", () => {
|
||||
it("refuses the parameterless form with no body", async () => {
|
||||
armOverrides({ WEIGHT: { min: 60, max: 80 } });
|
||||
|
||||
const res = await DELETE(deleteReq());
|
||||
|
||||
expect(res.status).toBe(422);
|
||||
const body = (await res.json()) as { error: string };
|
||||
expect(body.error).toContain("RESET_THRESHOLDS");
|
||||
expect(prisma.user.update).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("refuses a body whose confirmation is the wrong token", async () => {
|
||||
armOverrides({ WEIGHT: { min: 60, max: 80 } });
|
||||
|
||||
const res = await DELETE(deleteReq({ body: { confirm: "DELETE" } }));
|
||||
|
||||
expect(res.status).toBe(422);
|
||||
expect(prisma.user.update).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("wipes every override once the confirmation is present", async () => {
|
||||
armOverrides({ WEIGHT: { min: 60, max: 80 }, PULSE: { min: 50, max: 90 } });
|
||||
|
||||
const res = await DELETE(
|
||||
deleteReq({ body: { confirm: "RESET_THRESHOLDS" } }),
|
||||
);
|
||||
|
||||
expect(res.status).toBe(200);
|
||||
const body = (await res.json()) as { data: { overrides: unknown } };
|
||||
expect(body.data.overrides).toEqual({});
|
||||
expect(prisma.user.update).toHaveBeenCalledTimes(1);
|
||||
});
|
||||
|
||||
it("runs the sibling's rate limit and honours a refusal", async () => {
|
||||
armOverrides({ WEIGHT: { min: 60, max: 80 } });
|
||||
vi.mocked(checkRateLimit).mockResolvedValue({ allowed: false } as never);
|
||||
|
||||
const res = await DELETE(
|
||||
deleteReq({ body: { confirm: "RESET_THRESHOLDS" } }),
|
||||
);
|
||||
|
||||
expect(res.status).toBe(429);
|
||||
expect(checkRateLimit).toHaveBeenCalledWith(
|
||||
"thresholds:reset:user-1",
|
||||
30,
|
||||
5 * 60 * 1000,
|
||||
);
|
||||
expect(prisma.user.update).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
/**
|
||||
* The other end. A confirmation the server demands and the only client that
|
||||
* sends the wide form does not send is not a safeguard, it is an outage —
|
||||
* and it would look like a passing suite, because the route tests above
|
||||
* construct their own requests. So the token is compared across the two
|
||||
* files rather than assumed to match.
|
||||
*/
|
||||
it("is the same token the settings card sends", async () => {
|
||||
const { readFileSync } = await import("node:fs");
|
||||
const { join } = await import("node:path");
|
||||
|
||||
const route = readFileSync(
|
||||
join(process.cwd(), "src/app/api/user/thresholds/route.ts"),
|
||||
"utf8",
|
||||
);
|
||||
const client = readFileSync(
|
||||
join(
|
||||
process.cwd(),
|
||||
"src/components/settings/thresholds-editor-section.tsx",
|
||||
),
|
||||
"utf8",
|
||||
);
|
||||
|
||||
const declared = route.match(/RESET_ALL_CONFIRMATION = "([^"]+)"/)?.[1];
|
||||
expect(declared, "the route no longer declares a confirmation").toBe(
|
||||
"RESET_THRESHOLDS",
|
||||
);
|
||||
expect(client).toContain(`confirm: "${declared}"`);
|
||||
});
|
||||
|
||||
it("leaves the single-metric form exactly as it was", async () => {
|
||||
armOverrides({ WEIGHT: { min: 60, max: 80 }, PULSE: { min: 50, max: 90 } });
|
||||
|
||||
// No body at all — the targets sheet sends none, and must keep working.
|
||||
const res = await DELETE(deleteReq({ metric: "WEIGHT" }));
|
||||
|
||||
expect(res.status).toBe(200);
|
||||
const body = (await res.json()) as {
|
||||
data: { overrides: Record<string, unknown> };
|
||||
};
|
||||
expect(Object.keys(body.data.overrides)).toEqual(["PULSE"]);
|
||||
});
|
||||
});
|
||||
|
||||
/**
|
||||
* The docblock promised a key the handler has never returned.
|
||||
*
|
||||
|
||||
@@ -9,7 +9,10 @@
|
||||
* it is worth stating the absence rather than just deleting the word.
|
||||
* PUT accepts a partial map — only the provided metrics are updated, others
|
||||
* keep their existing override or remain on the default.
|
||||
* DELETE with ?metric=... resets one metric to default.
|
||||
* DELETE with ?metric=... resets one metric to default. WITHOUT the parameter
|
||||
* it drops every override the account has, so it demands the typed
|
||||
* confirmation the other wide-blast-radius deletes demand — see
|
||||
* `RESET_ALL_CONFIRMATION` below.
|
||||
*/
|
||||
import { apiHandler, requireAuth } from "@/lib/api-handler";
|
||||
import {
|
||||
@@ -118,6 +121,21 @@ export const PUT = apiHandler(async (request: NextRequest) => {
|
||||
return apiSuccess({ overrides: merged });
|
||||
});
|
||||
|
||||
/**
|
||||
* What the parameterless DELETE has to be told before it wipes every override.
|
||||
*
|
||||
* Same shape as `/api/settings/account` (`DELETE_ACCOUNT`) and
|
||||
* `/api/settings/data` (`DELETE`): a literal in the JSON body, compared
|
||||
* exactly, with no default. The point is that the wide form cannot be reached
|
||||
* by dropping a query parameter — a request that means "reset everything" has
|
||||
* to say so in a second place.
|
||||
*
|
||||
* NOT applied to `?metric=`. That form resets one band the person can retype
|
||||
* from the editor beside it, and the targets sheet fires it in a loop over the
|
||||
* metrics it owns; a confirmation there would be ceremony without a decision.
|
||||
*/
|
||||
const RESET_ALL_CONFIRMATION = "RESET_THRESHOLDS";
|
||||
|
||||
export const DELETE = apiHandler(async (request: NextRequest) => {
|
||||
const { user } = await requireAuth();
|
||||
const url = new URL(request.url);
|
||||
@@ -128,6 +146,43 @@ export const DELETE = apiHandler(async (request: NextRequest) => {
|
||||
meta: { metric: metric ?? "all" },
|
||||
});
|
||||
|
||||
// The PUT's limit, on the path that can destroy more in one call than the
|
||||
// PUT can. It had none at all: an omitted query parameter erased the whole
|
||||
// override set, and nothing stopped that being repeated.
|
||||
const rl = await checkRateLimit(
|
||||
`thresholds:reset:${user.id}`,
|
||||
30,
|
||||
5 * 60 * 1000,
|
||||
);
|
||||
if (!rl.allowed) {
|
||||
return apiError("Too many requests, please slow down", 429);
|
||||
}
|
||||
|
||||
if (!metric) {
|
||||
const raw = await request.text();
|
||||
if (raw.length > 64 * 1024) {
|
||||
return apiError(`Request body exceeds ${64 * 1024} bytes`, 413);
|
||||
}
|
||||
let confirm = "";
|
||||
if (raw.length > 0) {
|
||||
try {
|
||||
const body = JSON.parse(raw) as { confirm?: unknown };
|
||||
confirm = typeof body?.confirm === "string" ? body.confirm : "";
|
||||
} catch {
|
||||
return apiError("Invalid request", 422);
|
||||
}
|
||||
}
|
||||
if (confirm !== RESET_ALL_CONFIRMATION) {
|
||||
// Names the token rather than saying "confirmation missing", so a client
|
||||
// that reached here by omitting `?metric=` by accident is told what the
|
||||
// request it sent actually meant.
|
||||
return apiError(
|
||||
`Confirmation missing. Resetting every threshold override requires { confirm: '${RESET_ALL_CONFIRMATION}' }`,
|
||||
422,
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
const existing = await prisma.user.findUnique({
|
||||
where: { id: user.id },
|
||||
select: { thresholdsJson: true },
|
||||
|
||||
@@ -108,10 +108,19 @@ export function ThresholdsEditorSection({ id }: { id: string }) {
|
||||
|
||||
const resetMutation = useMutation({
|
||||
mutationFn: async (metric: ThresholdMetric | null) => {
|
||||
const url = metric
|
||||
? `/api/user/thresholds?metric=${metric}`
|
||||
: "/api/user/thresholds";
|
||||
const res = await apiFetchRaw(url, { method: "DELETE" });
|
||||
// `metric === null` is the reset-everything arm behind the confirm
|
||||
// dialog at the foot of the card. The server asks for the same decision
|
||||
// in the body, so the dialog's answer is what gets sent — the wide form
|
||||
// is not reachable by dropping the query parameter.
|
||||
const res = metric
|
||||
? await apiFetchRaw(`/api/user/thresholds?metric=${metric}`, {
|
||||
method: "DELETE",
|
||||
})
|
||||
: await apiFetchRaw("/api/user/thresholds", {
|
||||
method: "DELETE",
|
||||
headers: { "Content-Type": "application/json" },
|
||||
body: JSON.stringify({ confirm: "RESET_THRESHOLDS" }),
|
||||
});
|
||||
if (!res.ok) throw new Error("reset failed");
|
||||
},
|
||||
onSuccess: () => {
|
||||
|
||||
@@ -653,6 +653,27 @@ const thresholdsOverridesResponse = z
|
||||
.object({ overrides: thresholdOverrideMap })
|
||||
.meta({ id: "ThresholdsOverridesResponse" });
|
||||
|
||||
/**
|
||||
* The typed confirmation the parameterless DELETE demands.
|
||||
*
|
||||
* Published because it is the difference between "reset one band" and "erase
|
||||
* every band the account has tuned", and a client that learned the endpoint
|
||||
* from the contract alone would otherwise discover the requirement as a 422.
|
||||
*/
|
||||
const thresholdsResetAllRequest = z
|
||||
.object({
|
||||
confirm: z
|
||||
.literal("RESET_THRESHOLDS")
|
||||
.describe(
|
||||
"Required when `metric` is omitted. Compared exactly; any other value is refused.",
|
||||
),
|
||||
})
|
||||
.meta({
|
||||
id: "ThresholdsResetAllRequest",
|
||||
description:
|
||||
"Confirmation body for the reset-everything form. Send no body at all when resetting a single metric with `?metric=`.",
|
||||
});
|
||||
|
||||
const thresholdsUpdateRequest = thresholdsUpdateSchema.meta({
|
||||
id: "ThresholdsUpdateRequest",
|
||||
description:
|
||||
@@ -1716,17 +1737,23 @@ export const profilePaths: NonNullable<ZodOpenApiObject["paths"]> = {
|
||||
tags: ["Analytics"],
|
||||
summary: "Reset one threshold override, or all of them",
|
||||
description:
|
||||
"With `?metric=` it drops that metric's override; WITHOUT the parameter it drops EVERY override on the account. There is no confirmation step and no dry run, so a client that means to reset one metric must send the parameter.\n\n" +
|
||||
"Audit-logged with the before and after maps. Unlike the PUT this path runs no rate limit of its own.",
|
||||
'With `?metric=` it drops that metric\'s override; WITHOUT the parameter it drops EVERY override on the account. The wide form is not reachable by omitting the parameter alone — it requires `{ "confirm": "RESET_THRESHOLDS" }` in the body, the same typed-confirmation shape account deletion and the data reset use. The `?metric=` form takes no body and is unchanged.\n\n' +
|
||||
"Audit-logged with the before and after maps. Rate-limited 30 / 5 min per user, matching the PUT.",
|
||||
requestParams: {
|
||||
query: z.object({
|
||||
metric: thresholdMetricEnum
|
||||
.optional()
|
||||
.describe(
|
||||
"The single metric to reset. Omit to reset every override on the account.",
|
||||
"The single metric to reset. Omit to reset every override on the account — which then requires the confirmation body.",
|
||||
),
|
||||
}),
|
||||
},
|
||||
requestBody: {
|
||||
required: false,
|
||||
content: {
|
||||
"application/json": { schema: thresholdsResetAllRequest },
|
||||
},
|
||||
},
|
||||
responses: {
|
||||
"200": {
|
||||
description: "The remaining override map.",
|
||||
@@ -1745,6 +1772,14 @@ export const profilePaths: NonNullable<ZodOpenApiObject["paths"]> = {
|
||||
content: { "application/json": { schema: errorEnvelope } },
|
||||
},
|
||||
...stdResponses,
|
||||
// After `stdResponses` on purpose: the generic 422 there would
|
||||
// otherwise overwrite this one and the confirmation contract would
|
||||
// vanish from the published operation.
|
||||
"422": {
|
||||
description:
|
||||
'`metric` was omitted and the body did not carry `{ "confirm": "RESET_THRESHOLDS" }`. Nothing was reset.',
|
||||
content: { "application/json": { schema: errorEnvelope } },
|
||||
},
|
||||
},
|
||||
},
|
||||
},
|
||||
|
||||
Reference in new issue
Block a user