From 7b515f4f828f9b97452ddd76954632d8e8495437 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marc-Andr=C3=A9=20Bombeck?= Date: Mon, 20 Jul 2026 11:25:48 +0200 Subject: [PATCH] fix(devices): reconcile APNs registrations --- src/app/api/devices/__tests__/route.test.ts | 77 +++++- src/app/api/devices/route.ts | 234 ++++++++++------- tests/integration/device-register.test.ts | 273 +++++++++++++++++--- 3 files changed, 437 insertions(+), 147 deletions(-) diff --git a/src/app/api/devices/__tests__/route.test.ts b/src/app/api/devices/__tests__/route.test.ts index 89ca90a10..c16b20a0d 100644 --- a/src/app/api/devices/__tests__/route.test.ts +++ b/src/app/api/devices/__tests__/route.test.ts @@ -8,7 +8,13 @@ vi.mock("@/lib/db", () => ({ findFirst: vi.fn(), create: vi.fn(), update: vi.fn(), + deleteMany: vi.fn(), }, + refreshToken: { + updateMany: vi.fn(), + }, + $queryRaw: vi.fn(), + $transaction: vi.fn(), notificationChannel: { upsert: vi.fn(), }, @@ -76,6 +82,12 @@ beforeEach(() => { vi.mocked(prisma.device.findFirst).mockResolvedValue(null); vi.mocked(prisma.device.create).mockResolvedValue({ id: "dev-1" } as never); vi.mocked(prisma.device.update).mockResolvedValue({ id: "dev-1" } as never); + vi.mocked(prisma.device.deleteMany).mockResolvedValue({ count: 0 }); + vi.mocked(prisma.refreshToken.updateMany).mockResolvedValue({ count: 0 }); + vi.mocked(prisma.$queryRaw).mockResolvedValue([] as never); + vi.mocked(prisma.$transaction).mockImplementation( + async (callback) => callback(prisma as never), + ); vi.mocked(prisma.notificationChannel.upsert).mockResolvedValue({ id: "ch-1", } as never); @@ -228,27 +240,76 @@ describe("POST /api/devices", () => { expect(prisma.device.update).not.toHaveBeenCalled(); }); - it("falls through idempotently when the apnsToken already belongs to the same user", async () => { - // v1.4.23 W6 reconcile (HIGH 3): the cross-user-hijack guard used - // to filter `NOT: { userId }` so it skipped same-user collisions - // entirely, letting `findMany({ apnsToken })` fan a single push - // out to two Device rows. The guard now reads `userId` and only - // rejects cross-user collisions; same-user collisions fall - // through to the legacy-token upsert path. + it("reconciles a changed legacy token onto the same user's APNs device", async () => { vi.mocked(getSession).mockResolvedValue(SESSION_OK as never); vi.mocked(prisma.device.findFirst).mockResolvedValue({ id: "self-dev", userId: "user-1", + token: "original-legacy-token", } as never); const res = await POST( req({ - token: "abcd1234efgh5678", + token: "changed-legacy-token", bundleId: "io.healthlog.app", apnsToken: "deadbeef".repeat(8), apnsEnvironment: "sandbox", }), ); expect(res.status).toBe(201); + expect(prisma.device.create).not.toHaveBeenCalled(); + expect(prisma.device.update).toHaveBeenCalledWith( + expect.objectContaining({ + where: { id: "self-dev", userId: "user-1" }, + data: expect.objectContaining({ token: "changed-legacy-token" }), + }), + ); + expect(prisma.refreshToken.updateMany).toHaveBeenCalledWith({ + where: { userId: "user-1", deviceId: "original-legacy-token" }, + data: { deviceId: "changed-legacy-token" }, + }); + }); + + it("merges two same-user identities and migrates owned refresh tokens", async () => { + vi.mocked(getSession).mockResolvedValue(SESSION_OK as never); + vi.mocked(prisma.device.findUnique).mockResolvedValue({ + id: "legacy-dev", + userId: "user-1", + token: "changed-legacy-token", + medicationDelivery: "client", + liveActivityPushToken: "cafebabe".repeat(8), + } as never); + vi.mocked(prisma.device.findFirst).mockResolvedValue({ + id: "apns-dev", + userId: "user-1", + token: "original-legacy-token", + } as never); + + const res = await POST( + req({ + token: "changed-legacy-token", + bundleId: "io.healthlog.app", + apnsToken: "deadbeef".repeat(8), + apnsEnvironment: "production", + }), + ); + + expect(res.status).toBe(201); + expect(prisma.refreshToken.updateMany).toHaveBeenCalledWith({ + where: { userId: "user-1", deviceId: "original-legacy-token" }, + data: { deviceId: "changed-legacy-token" }, + }); + expect(prisma.device.deleteMany).toHaveBeenCalledWith({ + where: { id: "legacy-dev", userId: "user-1" }, + }); + expect(prisma.device.update).toHaveBeenCalledWith( + expect.objectContaining({ + where: { id: "apns-dev", userId: "user-1" }, + data: expect.objectContaining({ + medicationDelivery: "client", + liveActivityPushToken: "cafebabe".repeat(8), + }), + }), + ); }); // v1.17.1 (#22) — Live Activity push-token registration. diff --git a/src/app/api/devices/route.ts b/src/app/api/devices/route.ts index caeca15f2..cbbabdaef 100644 --- a/src/app/api/devices/route.ts +++ b/src/app/api/devices/route.ts @@ -3,8 +3,8 @@ * * Native-client device registration. The iOS app calls this on login, * on APNs token rotation, and whenever a fresh `apnsToken` arrives from - * `application:didRegisterForRemoteNotificationsWithDeviceToken:`. We - * upsert by `token` (the legacy generic identifier). + * `application:didRegisterForRemoteNotificationsWithDeviceToken:`. Both the + * legacy `token` and `apnsToken` resolve to one canonical device. * * Cross-user-hijack guard: * * A device `token` belongs to exactly one user. Re-registering the @@ -162,109 +162,143 @@ export const POST = apiHandler(async (request: NextRequest) => { liveActivityPushToken, } = parsed.data; - // APNs-token collision lookup. Two cases: - // * A row for ANOTHER user owns this token → 409 hijack guard. - // APNs tokens aren't secrets, so accepting wire input that - // references another user's token would let anyone who learns - // one redirect that user's pushes. - // * A row for THIS user already owns it → idempotent fall-through. - // The upsert below either updates the matching legacy `token` - // row in place, or creates a fresh legacy `token` row that the - // migration-0041 partial unique index now blocks (the upsert - // branch will hit the unique constraint and the request will - // fail with a 500 — but that's a programmer-error path the - // iOS client should never trigger; same-user re-registration - // keeps the same legacy `token`). - // The `NOT: { userId }` clause was removed in v1.4.23 W6 reconcile — - // it skipped the same-user collision and let `findMany({ apnsToken })` - // fan a single push out to two Device rows, double-charging the APNs - // quota. - if (apnsToken) { - const existingApns = await prisma.device.findFirst({ - where: { apnsToken }, - select: { id: true, userId: true }, - }); - if (existingApns && existingApns.userId !== user.id) { - await auditLog("device.register.denied", { - userId: user.id, - details: { - reason: "apns_token_owned_by_other_user", - deviceId: existingApns.id, + const registration = await prisma.$transaction(async (tx) => { + // Token ownership is decided from one database snapshot. Advisory + // transaction locks serialize registrations that share either identity, + // including cross-user attempts and the first-create race. Sorting avoids + // deadlocks when concurrent requests present the same pair in reverse. + const lockKeys = [ + `device-token:${token}`, + ...(apnsToken ? [`device-apns:${apnsToken}`] : []), + ].sort(); + for (const lockKey of lockKeys) { + // `pg_advisory_xact_lock` returns void, which Prisma cannot deserialize + // directly. Selecting through FROM yields a plain integer row. + await tx.$queryRaw` + SELECT 1 AS locked + FROM pg_advisory_xact_lock(hashtextextended(${lockKey}, 0)) + `; + } + + const [legacyDevice, apnsDevice] = await Promise.all([ + tx.device.findUnique({ + where: { token }, + select: { + id: true, + userId: true, + token: true, + medicationDelivery: true, + liveActivityPushToken: true, }, - }); - return apiError("APNs token already registered to another account", 409); + }), + apnsToken + ? tx.device.findFirst({ + where: { apnsToken }, + select: { + id: true, + userId: true, + token: true, + medicationDelivery: true, + liveActivityPushToken: true, + }, + }) + : null, + ]); + + if (apnsDevice && apnsDevice.userId !== user.id) { + return { + conflict: "apns_token_owned_by_other_user" as const, + deviceId: apnsDevice.id, + }; } + if (legacyDevice && legacyDevice.userId !== user.id) { + return { + conflict: "token_owned_by_other_user" as const, + deviceId: legacyDevice.id, + }; + } + + // RefreshToken.deviceId stores the client-supplied Device.token value, + // not the database row id. Keep this user's token family aligned when + // APNs identifies an existing device under a changed legacy token. + if (apnsDevice && apnsDevice.token !== token) { + await tx.refreshToken.updateMany({ + where: { userId: user.id, deviceId: apnsDevice.token }, + data: { deviceId: token }, + }); + } + + // An APNs match is the canonical physical-device identity. If the newly + // presented legacy token already has a second same-user row, carry its + // explicit per-device state (including null clears), remove it, and update + // the APNs row with the newly presented metadata. + let canonicalDevice = apnsDevice ?? legacyDevice; + let duplicateDevice: typeof legacyDevice = null; + if ( + apnsDevice && + legacyDevice && + apnsDevice.id !== legacyDevice.id + ) { + duplicateDevice = legacyDevice; + await tx.device.deleteMany({ + where: { id: legacyDevice.id, userId: user.id }, + }); + canonicalDevice = apnsDevice; + } + + const deviceData = { + platform: "ios", + token, + bundleId, + locale: locale ?? null, + appVersion: appVersion ?? null, + model: model ?? null, + apnsToken: apnsToken ?? null, + apnsEnvironment: apnsEnvironment ?? null, + ...(medicationDelivery !== undefined + ? { medicationDelivery } + : duplicateDevice + ? { medicationDelivery: duplicateDevice.medicationDelivery } + : {}), + ...(liveActivityPushToken !== undefined + ? { liveActivityPushToken } + : duplicateDevice + ? { liveActivityPushToken: duplicateDevice.liveActivityPushToken } + : {}), + }; + + const device = canonicalDevice + ? await tx.device.update({ + where: { id: canonicalDevice.id, userId: user.id }, + data: { ...deviceData, lastSeen: new Date() }, + select: { id: true }, + }) + : await tx.device.create({ + data: { userId: user.id, ...deviceData }, + select: { id: true }, + }); + + return { id: device.id }; + }); + + if ("conflict" in registration) { + await auditLog("device.register.denied", { + userId: user.id, + details: { + reason: registration.conflict, + deviceId: registration.deviceId, + }, + }); + return registration.conflict === "apns_token_owned_by_other_user" + ? apiError("APNs token already registered to another account", 409) + : apiError("Device token already registered to another account", 409); } - const existing = await prisma.device.findUnique({ where: { token } }); - let id: string; - if (existing) { - // Cross-user-hijack guard: a device-token belongs to exactly one user. - // Re-registering the same token under a different account is rejected - // — APNs tokens aren't a secret, so trusting the wire input would let - // anyone who learns/guesses a token redirect another user's pushes. - if (existing.userId !== user.id) { - await auditLog("device.register.denied", { - userId: user.id, - details: { reason: "token_owned_by_other_user", deviceId: existing.id }, - }); - return apiError( - "Device token already registered to another account", - 409, - ); - } - const updated = await prisma.device.update({ - where: { token }, - data: { - platform: "ios", - bundleId, - locale: locale ?? null, - appVersion: appVersion ?? null, - model: model ?? null, - apnsToken: apnsToken ?? null, - apnsEnvironment: apnsEnvironment ?? null, - // v1.7.0 — only touch the override when the client sends the - // field, so a re-register that omits it keeps the prior value. - ...(medicationDelivery !== undefined && { medicationDelivery }), - // v1.17.1 (#22) — same omit-keeps-prior rule; `null` explicitly - // clears the token when the Activity ends. - ...(liveActivityPushToken !== undefined && { liveActivityPushToken }), - lastSeen: new Date(), - }, - select: { id: true }, - }); - id = updated.id; - } else { - const created = await prisma.device.create({ - data: { - userId: user.id, - platform: "ios", - token, - bundleId, - locale: locale ?? null, - appVersion: appVersion ?? null, - model: model ?? null, - apnsToken: apnsToken ?? null, - apnsEnvironment: apnsEnvironment ?? null, - // v1.7.0 — per-device delivery override (null = inherit default). - ...(medicationDelivery !== undefined && { medicationDelivery }), - // v1.17.1 (#22) — Live Activity push token (null = no Activity). - ...(liveActivityPushToken !== undefined && { liveActivityPushToken }), - }, - select: { id: true }, - }); - id = created.id; - } + const { id } = registration; - // Auto-create the APNS NotificationChannel row when the device - // registers with an apnsToken. Without this row the dispatcher's APNS - // branch (`dispatcher.ts:41-49`) never fires for the user — every - // production iOS install would otherwise need a manual settings - // toggle that doesn't exist. Mirrors the legacy Telegram on-first- - // dispatch auto-migration, but eager so the very first reminder - // after device registration already routes through APNs. Config is - // an encrypted empty record by design — the per-device token + - // environment live on the Device row. + // Reconcile APNs channel state only after the canonical device transaction + // commits. Config is an encrypted empty record by design — the per-device + // token and environment live on the Device row. if (apnsToken) { await prisma.notificationChannel.upsert({ where: { userId_type: { userId: user.id, type: "APNS" } }, diff --git a/tests/integration/device-register.test.ts b/tests/integration/device-register.test.ts index ebe1d54d8..94381cf5a 100644 --- a/tests/integration/device-register.test.ts +++ b/tests/integration/device-register.test.ts @@ -10,12 +10,12 @@ * 2. Registering without an `apnsToken` does NOT create the channel — * the iOS client may upsert device metadata before APNs grants a * token, and we must not enable APNs delivery prematurely. - * 3. Registering a SECOND device with the same `apnsToken` for the - * same user is idempotent — the channel row stays at one. The - * same-user collision path returns 200 (upsert) rather than 409 - * (the cross-user hijack guard). + * 3. Re-registering the same `apnsToken` is idempotent, including when the + * legacy token changes: the route returns 201 and keeps one canonical + * device and one channel. Foreign ownership still returns 409. */ import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; +import type { NextRequest } from "next/server"; process.env.ENCRYPTION_KEY ??= "0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef"; @@ -75,23 +75,26 @@ function buildRequest(body: Record) { method: "POST", headers: { "content-type": "application/json" }, body: JSON.stringify(body), - }) as unknown as import("next/server").NextRequest; + }) as unknown as NextRequest; +} + +async function registerDevice(body: Record) { + // Load after the test env and next/headers mock are installed. + const { POST } = await import("@/app/api/devices/route"); + return (POST as (request: NextRequest) => Promise)( + buildRequest(body), + ); } describe("POST /api/devices", () => { it("auto-upserts a NotificationChannel { type: APNS } when apnsToken is supplied", async () => { const user = await seedSession("device-apns-channel"); - const { POST } = await import("@/app/api/devices/route"); - - const req = buildRequest({ + const res = await registerDevice({ token: "legacy-token-1", bundleId: "io.healthlog.ios", apnsToken: "abcdef0123456789", apnsEnvironment: "sandbox", }); - const res = await ( - POST as (r: import("next/server").NextRequest) => Promise - )(req); expect(res.status).toBe(201); const channel = await getPrismaClient().notificationChannel.findFirst({ @@ -103,15 +106,10 @@ describe("POST /api/devices", () => { it("does NOT create an APNS channel when apnsToken is omitted", async () => { const user = await seedSession("device-no-apns"); - const { POST } = await import("@/app/api/devices/route"); - - const req = buildRequest({ + const res = await registerDevice({ token: "legacy-token-2", bundleId: "io.healthlog.ios", }); - const res = await ( - POST as (r: import("next/server").NextRequest) => Promise - )(req); expect(res.status).toBe(201); const channel = await getPrismaClient().notificationChannel.findFirst({ @@ -122,30 +120,20 @@ describe("POST /api/devices", () => { it("re-registering the same apnsToken for the same user keeps the channel row at one", async () => { const user = await seedSession("device-apns-idempotent"); - const { POST } = await import("@/app/api/devices/route"); - - const first = await ( - POST as (r: import("next/server").NextRequest) => Promise - )( - buildRequest({ - token: "legacy-token-3", - bundleId: "io.healthlog.ios", - apnsToken: "deadbeef", - apnsEnvironment: "sandbox", - }), - ); + const first = await registerDevice({ + token: "legacy-token-3", + bundleId: "io.healthlog.ios", + apnsToken: "deadbeef", + apnsEnvironment: "sandbox", + }); expect(first.status).toBe(201); - const second = await ( - POST as (r: import("next/server").NextRequest) => Promise - )( - buildRequest({ - token: "legacy-token-3", - bundleId: "io.healthlog.ios", - apnsToken: "deadbeef", - apnsEnvironment: "sandbox", - }), - ); + const second = await registerDevice({ + token: "legacy-token-3", + bundleId: "io.healthlog.ios", + apnsToken: "deadbeef", + apnsEnvironment: "sandbox", + }); expect(second.status).toBe(201); const channels = await getPrismaClient().notificationChannel.findMany({ @@ -153,4 +141,211 @@ describe("POST /api/devices", () => { }); expect(channels).toHaveLength(1); }); + + it("reconciles a changed legacy token onto the same user's APNs device", async () => { + const user = await seedSession("device-changed-token"); + const prisma = getPrismaClient(); + + const first = await registerDevice({ + token: "legacy-token-old", + bundleId: "io.healthlog.ios", + apnsToken: "aaaabbbbccccdddd", + apnsEnvironment: "sandbox", + appVersion: "1.0.0", + }); + expect(first.status).toBe(201); + const firstBody = (await first.json()) as { data: { id: string } }; + const refreshToken = await prisma.refreshToken.create({ + data: { + userId: user.id, + tokenHash: "changed-token-refresh-hash", + deviceId: "legacy-token-old", + expiresAt: new Date(Date.now() + 60_000), + }, + }); + + const second = await registerDevice({ + token: "legacy-token-new", + bundleId: "io.healthlog.ios.beta", + apnsToken: "aaaabbbbccccdddd", + apnsEnvironment: "production", + appVersion: "2.0.0", + }); + expect(second.status).toBe(201); + const secondBody = (await second.json()) as { data: { id: string } }; + + expect(secondBody.data.id).toBe(firstBody.data.id); + const devices = await prisma.device.findMany({ where: { userId: user.id } }); + expect(devices).toHaveLength(1); + expect(devices[0]).toMatchObject({ + id: firstBody.data.id, + token: "legacy-token-new", + bundleId: "io.healthlog.ios.beta", + apnsToken: "aaaabbbbccccdddd", + apnsEnvironment: "production", + appVersion: "2.0.0", + }); + await expect( + prisma.refreshToken.findUniqueOrThrow({ + where: { id: refreshToken.id }, + select: { deviceId: true }, + }), + ).resolves.toEqual({ deviceId: "legacy-token-new" }); + }); + + it("merges same-user device identities and migrates only owned refresh-token bindings", async () => { + const user = await seedSession("device-identity-merge"); + const prisma = getPrismaClient(); + const otherUser = await prisma.user.create({ + data: { + username: "device-identity-merge-other", + email: "device-identity-merge-other@example.test", + }, + }); + const apnsDevice = await prisma.device.create({ + data: { + userId: user.id, + platform: "ios", + token: "legacy-token-original", + bundleId: "io.healthlog.ios", + apnsToken: "1111222233334444", + apnsEnvironment: "sandbox", + medicationDelivery: "server", + liveActivityPushToken: "feedface".repeat(8), + }, + }); + const legacyDevice = await prisma.device.create({ + data: { + userId: user.id, + platform: "ios", + token: "legacy-token-current", + bundleId: "io.healthlog.ios", + medicationDelivery: null, + liveActivityPushToken: null, + }, + }); + const ownedRefreshToken = await prisma.refreshToken.create({ + data: { + userId: user.id, + tokenHash: "owned-refresh-token-hash", + deviceId: "legacy-token-original", + expiresAt: new Date(Date.now() + 60_000), + }, + }); + const foreignRefreshToken = await prisma.refreshToken.create({ + data: { + userId: otherUser.id, + tokenHash: "foreign-refresh-token-hash", + deviceId: "legacy-token-original", + expiresAt: new Date(Date.now() + 60_000), + }, + }); + + const response = await registerDevice({ + token: "legacy-token-current", + bundleId: "io.healthlog.ios.updated", + apnsToken: "1111222233334444", + apnsEnvironment: "production", + model: "iPhone17,1", + }); + expect(response.status).toBe(201); + const body = (await response.json()) as { data: { id: string } }; + expect(body.data.id).toBe(apnsDevice.id); + + const devices = await prisma.device.findMany({ where: { userId: user.id } }); + expect(devices).toHaveLength(1); + expect(devices[0]).toMatchObject({ + id: apnsDevice.id, + token: "legacy-token-current", + bundleId: "io.healthlog.ios.updated", + apnsToken: "1111222233334444", + apnsEnvironment: "production", + model: "iPhone17,1", + medicationDelivery: null, + liveActivityPushToken: null, + }); + await expect( + prisma.refreshToken.findUniqueOrThrow({ + where: { id: ownedRefreshToken.id }, + select: { deviceId: true }, + }), + ).resolves.toEqual({ deviceId: "legacy-token-current" }); + await expect( + prisma.refreshToken.findUniqueOrThrow({ + where: { id: foreignRefreshToken.id }, + select: { deviceId: true }, + }), + ).resolves.toEqual({ deviceId: "legacy-token-original" }); + }); + + it("rejects an APNs token owned by another user without mutating either account", async () => { + const user = await seedSession("device-apns-conflict"); + const prisma = getPrismaClient(); + const owner = await prisma.user.create({ + data: { + username: "device-apns-owner", + email: "device-apns-owner@example.test", + }, + }); + const ownedDevice = await prisma.device.create({ + data: { + userId: owner.id, + platform: "ios", + token: "foreign-legacy-token", + bundleId: "io.healthlog.ios", + apnsToken: "9999aaaabbbbcccc", + apnsEnvironment: "sandbox", + }, + }); + + const response = await registerDevice({ + token: "requesting-user-token", + bundleId: "io.healthlog.ios.changed", + apnsToken: "9999aaaabbbbcccc", + apnsEnvironment: "production", + }); + expect(response.status).toBe(409); + + await expect( + prisma.device.findUniqueOrThrow({ where: { id: ownedDevice.id } }), + ).resolves.toMatchObject({ + userId: owner.id, + token: "foreign-legacy-token", + bundleId: "io.healthlog.ios", + apnsEnvironment: "sandbox", + }); + await expect( + prisma.device.count({ where: { userId: user.id } }), + ).resolves.toBe(0); + }); + + it("serializes concurrent registration into one canonical device and channel", async () => { + const user = await seedSession("device-concurrent-register"); + const payload = { + token: "concurrent-legacy-token", + bundleId: "io.healthlog.ios", + apnsToken: "abcdabcdabcdabcd", + apnsEnvironment: "sandbox", + }; + + const responses = await Promise.all([ + registerDevice(payload), + registerDevice(payload), + ]); + expect(responses.map((response) => response.status)).toEqual([201, 201]); + const bodies = (await Promise.all( + responses.map((response) => response.json()), + )) as Array<{ data: { id: string } }>; + expect(bodies[0].data.id).toBe(bodies[1].data.id); + + const prisma = getPrismaClient(); + await expect( + prisma.device.count({ where: { userId: user.id } }), + ).resolves.toBe(1); + await expect( + prisma.notificationChannel.count({ + where: { userId: user.id, type: "APNS" }, + }), + ).resolves.toBe(1); + }); });