mirror of
https://github.com/MBombeck/HealthLog.git
synced 2026-11-08 12:32:43 +00:00
fix(devices): wrap revoke cascade in a single transaction
Both DELETE /api/devices/[id] and DELETE /api/auth/me/devices/[id] performed the same four-write cascade (refresh-token lookup, refresh revoke, access-token revoke, device delete) as separate Prisma calls. A partial failure between the third and fourth write left the device row alive with all its tokens revoked - the user's iPad showed in the device list as "still registered" but every request returned 401. Extract the cascade into a shared revokeDeviceCascade() helper and wrap the writes in prisma.$transaction so they commit atomically. Both routes now call the helper; the audit-log via tag and the wide event action name stay route-local. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
1 parent
dfffe6bb3b
commit
d25e50eaf3
6 files changed
+281
-80
No files matched your search
@@ -14,6 +14,10 @@ vi.mock("@/lib/db", () => ({
|
||||
apiToken: {
|
||||
updateMany: vi.fn(),
|
||||
},
|
||||
// v1.4.23 W6 — the cascade now runs through `prisma.$transaction`
|
||||
// (HIGH 6 fix). The mock returns the resolved batch payload so the
|
||||
// route handler walks past the transaction step.
|
||||
$transaction: vi.fn().mockResolvedValue([] as never),
|
||||
},
|
||||
}));
|
||||
|
||||
@@ -58,6 +62,7 @@ beforeEach(() => {
|
||||
vi.mocked(prisma.refreshToken.findMany).mockResolvedValue([]);
|
||||
vi.mocked(prisma.refreshToken.updateMany).mockResolvedValue({ count: 0 } as never);
|
||||
vi.mocked(prisma.apiToken.updateMany).mockResolvedValue({ count: 0 } as never);
|
||||
vi.mocked(prisma.$transaction).mockResolvedValue([] as never);
|
||||
});
|
||||
|
||||
describe("DELETE /api/auth/me/devices/[id]", () => {
|
||||
|
||||
@@ -16,13 +16,18 @@
|
||||
* the session lives on `Session.id`, not `Device.id`. Browser logouts
|
||||
* keep using /api/auth/logout. The two surfaces address different
|
||||
* blast radii.
|
||||
*
|
||||
* v1.4.23 W6 — the four-write cascade now runs through the shared
|
||||
* `revokeDeviceCascade` helper so the writes commit atomically inside
|
||||
* a single `prisma.$transaction`. Previously a partial failure could
|
||||
* leave the device row alive with all its tokens revoked.
|
||||
*/
|
||||
import { NextRequest } from "next/server";
|
||||
import { apiHandler, requireAuth } from "@/lib/api-handler";
|
||||
import { apiError, apiSuccess } from "@/lib/api-response";
|
||||
import { annotate } from "@/lib/logging/context";
|
||||
import { auditLog } from "@/lib/auth/audit";
|
||||
import { prisma } from "@/lib/db";
|
||||
import { revokeDeviceCascade } from "@/lib/devices/revoke";
|
||||
|
||||
interface RouteContext {
|
||||
params: Promise<{ id: string }>;
|
||||
@@ -33,62 +38,35 @@ export const DELETE = apiHandler(
|
||||
const { user } = await requireAuth();
|
||||
const { id } = await context.params;
|
||||
|
||||
const device = await prisma.device.findUnique({
|
||||
where: { id },
|
||||
select: { id: true, userId: true, model: true, bundleId: true },
|
||||
});
|
||||
|
||||
if (!device || device.userId !== user.id) {
|
||||
const result = await revokeDeviceCascade(user.id, id);
|
||||
if (!result) {
|
||||
// 404 on cross-user attempts — leaking "this id exists but isn't
|
||||
// yours" would let an attacker enumerate device ids.
|
||||
return apiError("Device not found", 404);
|
||||
}
|
||||
|
||||
// Revoke every refresh token bound to this device + its paired
|
||||
// access tokens.
|
||||
const liveRefreshTokens = await prisma.refreshToken.findMany({
|
||||
where: { userId: user.id, deviceId: device.id, revokedAt: null },
|
||||
select: { accessTokenHash: true },
|
||||
});
|
||||
const revokedAt = new Date();
|
||||
await prisma.refreshToken.updateMany({
|
||||
where: { userId: user.id, deviceId: device.id, revokedAt: null },
|
||||
data: { revokedAt },
|
||||
});
|
||||
const accessHashes = liveRefreshTokens
|
||||
.map((r) => r.accessTokenHash)
|
||||
.filter((v): v is string => Boolean(v));
|
||||
if (accessHashes.length > 0) {
|
||||
await prisma.apiToken.updateMany({
|
||||
where: { tokenHash: { in: accessHashes }, revoked: false },
|
||||
data: { revoked: true },
|
||||
});
|
||||
}
|
||||
|
||||
await prisma.device.delete({ where: { id: device.id } });
|
||||
|
||||
await auditLog("devices.revoke", {
|
||||
userId: user.id,
|
||||
details: {
|
||||
deviceId: device.id,
|
||||
label: device.model ?? device.bundleId ?? null,
|
||||
refreshTokensRevoked: liveRefreshTokens.length,
|
||||
accessTokensRevoked: accessHashes.length,
|
||||
deviceId: result.id,
|
||||
label: result.label,
|
||||
refreshTokensRevoked: result.refreshTokensRevoked,
|
||||
accessTokensRevoked: result.accessTokensRevoked,
|
||||
},
|
||||
});
|
||||
annotate({
|
||||
action: { name: "auth.me.devices.revoke", entity_type: "device", entity_id: device.id },
|
||||
action: { name: "auth.me.devices.revoke", entity_type: "device", entity_id: result.id },
|
||||
meta: {
|
||||
refresh_tokens_revoked: liveRefreshTokens.length,
|
||||
access_tokens_revoked: accessHashes.length,
|
||||
refresh_tokens_revoked: result.refreshTokensRevoked,
|
||||
access_tokens_revoked: result.accessTokensRevoked,
|
||||
},
|
||||
});
|
||||
|
||||
return apiSuccess({
|
||||
id: device.id,
|
||||
id: result.id,
|
||||
revoked: true,
|
||||
refreshTokensRevoked: liveRefreshTokens.length,
|
||||
accessTokensRevoked: accessHashes.length,
|
||||
refreshTokensRevoked: result.refreshTokensRevoked,
|
||||
accessTokensRevoked: result.accessTokensRevoked,
|
||||
});
|
||||
},
|
||||
);
|
||||
@@ -14,6 +14,10 @@ vi.mock("@/lib/db", () => ({
|
||||
apiToken: {
|
||||
updateMany: vi.fn(),
|
||||
},
|
||||
// v1.4.23 W6 — the cascade now runs through `prisma.$transaction`
|
||||
// (HIGH 6 fix). The mock returns the resolved batch payload so the
|
||||
// route handler walks past the transaction step.
|
||||
$transaction: vi.fn().mockResolvedValue([] as never),
|
||||
},
|
||||
}));
|
||||
|
||||
@@ -58,6 +62,7 @@ beforeEach(() => {
|
||||
vi.mocked(prisma.refreshToken.findMany).mockResolvedValue([]);
|
||||
vi.mocked(prisma.refreshToken.updateMany).mockResolvedValue({ count: 0 } as never);
|
||||
vi.mocked(prisma.apiToken.updateMany).mockResolvedValue({ count: 0 } as never);
|
||||
vi.mocked(prisma.$transaction).mockResolvedValue([] as never);
|
||||
});
|
||||
|
||||
describe("DELETE /api/devices/[id] (iOS token-rotation endpoint)", () => {
|
||||
|
||||
@@ -9,15 +9,17 @@
|
||||
* The duplication is deliberate: the W3 open question called out
|
||||
* needing a native-friendly URL for the iOS client's APNs rotation
|
||||
* loop, and the `/api/auth/me/...` path is built for the settings-
|
||||
* surface listing. Two URLs, one implementation: the underlying
|
||||
* Prisma calls are identical to keep the audit trail consistent.
|
||||
* surface listing. Two URLs share one transactional helper
|
||||
* (`revokeDeviceCascade`) so the cascade stays atomic — the W6 audit
|
||||
* collapsed the previous duplicated four-write sequence into a single
|
||||
* `prisma.$transaction` call.
|
||||
*/
|
||||
import { NextRequest } from "next/server";
|
||||
import { apiHandler, requireAuth } from "@/lib/api-handler";
|
||||
import { apiError, apiSuccess } from "@/lib/api-response";
|
||||
import { annotate } from "@/lib/logging/context";
|
||||
import { auditLog } from "@/lib/auth/audit";
|
||||
import { prisma } from "@/lib/db";
|
||||
import { revokeDeviceCascade } from "@/lib/devices/revoke";
|
||||
|
||||
interface RouteContext {
|
||||
params: Promise<{ id: string }>;
|
||||
@@ -28,59 +30,34 @@ export const DELETE = apiHandler(
|
||||
const { user } = await requireAuth();
|
||||
const { id } = await context.params;
|
||||
|
||||
const device = await prisma.device.findUnique({
|
||||
where: { id },
|
||||
select: { id: true, userId: true, model: true, bundleId: true },
|
||||
});
|
||||
|
||||
if (!device || device.userId !== user.id) {
|
||||
const result = await revokeDeviceCascade(user.id, id);
|
||||
if (!result) {
|
||||
return apiError("Device not found", 404);
|
||||
}
|
||||
|
||||
const liveRefreshTokens = await prisma.refreshToken.findMany({
|
||||
where: { userId: user.id, deviceId: device.id, revokedAt: null },
|
||||
select: { accessTokenHash: true },
|
||||
});
|
||||
const revokedAt = new Date();
|
||||
await prisma.refreshToken.updateMany({
|
||||
where: { userId: user.id, deviceId: device.id, revokedAt: null },
|
||||
data: { revokedAt },
|
||||
});
|
||||
const accessHashes = liveRefreshTokens
|
||||
.map((r) => r.accessTokenHash)
|
||||
.filter((v): v is string => Boolean(v));
|
||||
if (accessHashes.length > 0) {
|
||||
await prisma.apiToken.updateMany({
|
||||
where: { tokenHash: { in: accessHashes }, revoked: false },
|
||||
data: { revoked: true },
|
||||
});
|
||||
}
|
||||
|
||||
await prisma.device.delete({ where: { id: device.id } });
|
||||
|
||||
await auditLog("devices.revoke", {
|
||||
userId: user.id,
|
||||
details: {
|
||||
deviceId: device.id,
|
||||
label: device.model ?? device.bundleId ?? null,
|
||||
refreshTokensRevoked: liveRefreshTokens.length,
|
||||
accessTokensRevoked: accessHashes.length,
|
||||
deviceId: result.id,
|
||||
label: result.label,
|
||||
refreshTokensRevoked: result.refreshTokensRevoked,
|
||||
accessTokensRevoked: result.accessTokensRevoked,
|
||||
via: "ios.rotation",
|
||||
},
|
||||
});
|
||||
annotate({
|
||||
action: { name: "devices.revoke", entity_type: "device", entity_id: device.id },
|
||||
action: { name: "devices.revoke", entity_type: "device", entity_id: result.id },
|
||||
meta: {
|
||||
refresh_tokens_revoked: liveRefreshTokens.length,
|
||||
access_tokens_revoked: accessHashes.length,
|
||||
refresh_tokens_revoked: result.refreshTokensRevoked,
|
||||
access_tokens_revoked: result.accessTokensRevoked,
|
||||
},
|
||||
});
|
||||
|
||||
return apiSuccess({
|
||||
id: device.id,
|
||||
id: result.id,
|
||||
revoked: true,
|
||||
refreshTokensRevoked: liveRefreshTokens.length,
|
||||
accessTokensRevoked: accessHashes.length,
|
||||
refreshTokensRevoked: result.refreshTokensRevoked,
|
||||
accessTokensRevoked: result.accessTokensRevoked,
|
||||
});
|
||||
},
|
||||
);
|
||||
@@ -0,0 +1,149 @@
|
||||
import { describe, it, expect, vi, beforeEach } from "vitest";
|
||||
|
||||
// v1.4.23 W6 (HIGH 6) — verify the device-revoke cascade wraps every
|
||||
// write inside `prisma.$transaction` so a partial failure doesn't
|
||||
// leave the device row alive with all its tokens revoked. Also pins
|
||||
// the cross-user 404 path (returns null) and the no-access-token
|
||||
// short-circuit (no apiToken.updateMany call when there are no live
|
||||
// tokens).
|
||||
|
||||
vi.mock("@/lib/db", () => ({
|
||||
prisma: {
|
||||
device: {
|
||||
findUnique: vi.fn(),
|
||||
delete: vi.fn(),
|
||||
},
|
||||
refreshToken: {
|
||||
findMany: vi.fn(),
|
||||
updateMany: vi.fn(),
|
||||
},
|
||||
apiToken: {
|
||||
updateMany: vi.fn(),
|
||||
},
|
||||
$transaction: vi.fn(),
|
||||
},
|
||||
}));
|
||||
|
||||
import { prisma } from "@/lib/db";
|
||||
import { revokeDeviceCascade } from "../revoke";
|
||||
|
||||
beforeEach(() => {
|
||||
vi.resetAllMocks();
|
||||
});
|
||||
|
||||
describe("revokeDeviceCascade", () => {
|
||||
it("returns null and runs no writes when the device id is unknown", async () => {
|
||||
vi.mocked(prisma.device.findUnique).mockResolvedValue(null);
|
||||
|
||||
const result = await revokeDeviceCascade("user-1", "missing-id");
|
||||
|
||||
expect(result).toBeNull();
|
||||
expect(vi.mocked(prisma.$transaction)).not.toHaveBeenCalled();
|
||||
expect(vi.mocked(prisma.refreshToken.updateMany)).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("returns null on cross-user attempts (caller is not the owner)", async () => {
|
||||
vi.mocked(prisma.device.findUnique).mockResolvedValue({
|
||||
id: "dev-1",
|
||||
userId: "other-user",
|
||||
model: "iPhone 15",
|
||||
bundleId: "io.healthlog.ios",
|
||||
} as never);
|
||||
|
||||
const result = await revokeDeviceCascade("user-1", "dev-1");
|
||||
|
||||
expect(result).toBeNull();
|
||||
expect(vi.mocked(prisma.$transaction)).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("wraps the cascade in a single $transaction and reports counts", async () => {
|
||||
vi.mocked(prisma.device.findUnique).mockResolvedValue({
|
||||
id: "dev-1",
|
||||
userId: "user-1",
|
||||
model: "iPhone 15",
|
||||
bundleId: "io.healthlog.ios",
|
||||
} as never);
|
||||
vi.mocked(prisma.refreshToken.findMany).mockResolvedValue([
|
||||
{ accessTokenHash: "hash-a" },
|
||||
{ accessTokenHash: "hash-b" },
|
||||
{ accessTokenHash: null },
|
||||
] as never);
|
||||
|
||||
// Track which prisma method calls the helper queues into the
|
||||
// transaction array. We don't actually execute them; the helper
|
||||
// hands the array to `$transaction` and the contract is "all of
|
||||
// these commit atomically or none do".
|
||||
vi.mocked(prisma.refreshToken.updateMany).mockReturnValue(
|
||||
"REFRESH_UPDATE" as never,
|
||||
);
|
||||
vi.mocked(prisma.apiToken.updateMany).mockReturnValue(
|
||||
"API_TOKEN_UPDATE" as never,
|
||||
);
|
||||
vi.mocked(prisma.device.delete).mockReturnValue("DEVICE_DELETE" as never);
|
||||
vi.mocked(prisma.$transaction).mockResolvedValue([] as never);
|
||||
|
||||
const result = await revokeDeviceCascade("user-1", "dev-1");
|
||||
|
||||
expect(result).toEqual({
|
||||
id: "dev-1",
|
||||
label: "iPhone 15",
|
||||
refreshTokensRevoked: 3,
|
||||
accessTokensRevoked: 2,
|
||||
});
|
||||
expect(vi.mocked(prisma.$transaction)).toHaveBeenCalledTimes(1);
|
||||
const txArg = vi.mocked(prisma.$transaction).mock.calls[0]?.[0];
|
||||
expect(Array.isArray(txArg)).toBe(true);
|
||||
expect(txArg).toEqual([
|
||||
"REFRESH_UPDATE",
|
||||
"API_TOKEN_UPDATE",
|
||||
"DEVICE_DELETE",
|
||||
]);
|
||||
});
|
||||
|
||||
it("omits the apiToken.updateMany write when no access-token hashes exist", async () => {
|
||||
vi.mocked(prisma.device.findUnique).mockResolvedValue({
|
||||
id: "dev-2",
|
||||
userId: "user-1",
|
||||
model: null,
|
||||
bundleId: "io.healthlog.ios",
|
||||
} as never);
|
||||
vi.mocked(prisma.refreshToken.findMany).mockResolvedValue([
|
||||
{ accessTokenHash: null },
|
||||
] as never);
|
||||
|
||||
vi.mocked(prisma.refreshToken.updateMany).mockReturnValue(
|
||||
"REFRESH_UPDATE" as never,
|
||||
);
|
||||
vi.mocked(prisma.device.delete).mockReturnValue("DEVICE_DELETE" as never);
|
||||
vi.mocked(prisma.$transaction).mockResolvedValue([] as never);
|
||||
|
||||
const result = await revokeDeviceCascade("user-1", "dev-2");
|
||||
|
||||
expect(result?.accessTokensRevoked).toBe(0);
|
||||
expect(vi.mocked(prisma.apiToken.updateMany)).not.toHaveBeenCalled();
|
||||
const txArg = vi.mocked(prisma.$transaction).mock.calls[0]?.[0];
|
||||
expect(txArg).toEqual(["REFRESH_UPDATE", "DEVICE_DELETE"]);
|
||||
});
|
||||
|
||||
it("propagates transaction failure (rollback path) — no swallowed error", async () => {
|
||||
vi.mocked(prisma.device.findUnique).mockResolvedValue({
|
||||
id: "dev-3",
|
||||
userId: "user-1",
|
||||
model: "iPad",
|
||||
bundleId: null,
|
||||
} as never);
|
||||
vi.mocked(prisma.refreshToken.findMany).mockResolvedValue([
|
||||
{ accessTokenHash: "hash-c" },
|
||||
] as never);
|
||||
vi.mocked(prisma.refreshToken.updateMany).mockReturnValue("R" as never);
|
||||
vi.mocked(prisma.apiToken.updateMany).mockReturnValue("A" as never);
|
||||
vi.mocked(prisma.device.delete).mockReturnValue("D" as never);
|
||||
vi.mocked(prisma.$transaction).mockRejectedValue(
|
||||
new Error("postgres-blip"),
|
||||
);
|
||||
|
||||
await expect(
|
||||
revokeDeviceCascade("user-1", "dev-3"),
|
||||
).rejects.toThrow("postgres-blip");
|
||||
});
|
||||
});
|
||||
@@ -0,0 +1,87 @@
|
||||
/**
|
||||
* v1.4.23 W6 (HIGH 6 / S-01) — shared device-revoke cascade.
|
||||
*
|
||||
* Both `DELETE /api/auth/me/devices/[id]` and `DELETE /api/devices/[id]`
|
||||
* historically performed the same four-write cascade (refresh-token
|
||||
* lookup → refresh revoke → access-token revoke → device delete) as
|
||||
* separate Prisma calls. Without a transaction, a partial failure
|
||||
* (Postgres connection blip, app crash) left the device row alive
|
||||
* with all its tokens revoked — the user's iPad showed in the device
|
||||
* list as "still registered" but every request returned 401, and the
|
||||
* only fix was admin intervention.
|
||||
*
|
||||
* `revokeDeviceCascade` wraps the four writes in `prisma.$transaction`
|
||||
* so the cascade is atomic. Both routes call this helper and only
|
||||
* differ in the audit-log `details.via` slot + the wide-event action
|
||||
* name they emit afterwards (those stay route-local because they
|
||||
* carry surface-specific context).
|
||||
*
|
||||
* Returns `null` when the device doesn't exist or belongs to another
|
||||
* user (the route turns this into a 404 — leaking "this id exists but
|
||||
* isn't yours" would let an attacker enumerate device ids).
|
||||
*/
|
||||
import { prisma } from "@/lib/db";
|
||||
|
||||
export interface RevokeDeviceCascadeResult {
|
||||
id: string;
|
||||
label: string | null;
|
||||
refreshTokensRevoked: number;
|
||||
accessTokensRevoked: number;
|
||||
}
|
||||
|
||||
export async function revokeDeviceCascade(
|
||||
userId: string,
|
||||
deviceId: string,
|
||||
): Promise<RevokeDeviceCascadeResult | null> {
|
||||
// Ownership lookup outside the transaction — the cascade only runs
|
||||
// for an owned, existing device, so resolving the row first keeps
|
||||
// the transactional window tight.
|
||||
const device = await prisma.device.findUnique({
|
||||
where: { id: deviceId },
|
||||
select: { id: true, userId: true, model: true, bundleId: true },
|
||||
});
|
||||
|
||||
if (!device || device.userId !== userId) {
|
||||
return null;
|
||||
}
|
||||
|
||||
const liveRefreshTokens = await prisma.refreshToken.findMany({
|
||||
where: { userId, deviceId: device.id, revokedAt: null },
|
||||
select: { accessTokenHash: true },
|
||||
});
|
||||
const accessHashes = liveRefreshTokens
|
||||
.map((r) => r.accessTokenHash)
|
||||
.filter((v): v is string => Boolean(v));
|
||||
const revokedAt = new Date();
|
||||
|
||||
// Build the write set as a `$transaction` array so all writes
|
||||
// commit together or none of them do. Avoids the "tokens revoked
|
||||
// but device row alive" half-state the W6 review flagged.
|
||||
// The array uses `unknown` because Prisma's overloaded
|
||||
// $transaction type narrows on the literal array shape; the
|
||||
// operations are still strongly typed at construction.
|
||||
const writes: unknown[] = [
|
||||
prisma.refreshToken.updateMany({
|
||||
where: { userId, deviceId: device.id, revokedAt: null },
|
||||
data: { revokedAt },
|
||||
}),
|
||||
];
|
||||
if (accessHashes.length > 0) {
|
||||
writes.push(
|
||||
prisma.apiToken.updateMany({
|
||||
where: { tokenHash: { in: accessHashes }, revoked: false },
|
||||
data: { revoked: true },
|
||||
}),
|
||||
);
|
||||
}
|
||||
writes.push(prisma.device.delete({ where: { id: device.id } }));
|
||||
|
||||
await prisma.$transaction(writes as never);
|
||||
|
||||
return {
|
||||
id: device.id,
|
||||
label: device.model ?? device.bundleId ?? null,
|
||||
refreshTokensRevoked: liveRefreshTokens.length,
|
||||
accessTokensRevoked: accessHashes.length,
|
||||
};
|
||||
}
|
||||
Reference in new issue
Block a user