mirror of
https://github.com/MBombeck/HealthLog.git
synced 2026-11-09 13:01:00 +00:00
fix(devices): enforce global apnsToken uniqueness via partial index
Two collision paths let the same APNs token attach to multiple Device
rows for one user, fanning a single push out to two rows and double-
charging the APNs quota:
1. The cross-user-hijack guard filtered `NOT: { userId }` so it
skipped same-user collisions entirely.
2. The `apns_token` index was non-unique.
Drop the `NOT: { userId }` clause so the lookup catches both cases —
cross-user collisions still 409, same-user collisions fall through to
the existing legacy-token upsert path. Pair with migration 0041
introducing a partial UNIQUE index over `apns_token IS NOT NULL` and
dropping the redundant non-unique sibling. Apple's APNs contract
guarantees one token per (device, app), so global uniqueness matches
the upstream invariant.
Migration is strictly additive at the rolling-upgrade boundary; the
NULL-allowed partial-index shape preserves legacy iOS rows without an
APNs handshake. Schema-side comment documents why the partial index is
introduced via raw SQL (Prisma can't express it natively) and warns
against re-adding the non-unique sibling.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
1 parent
13977bcc70
commit
d5202e5dd8
3 files changed
+58
-8
No files matched your search
@@ -0,0 +1,33 @@
|
|||||||
|
-- v1.4.23 W6 reconcile — global uniqueness for `devices.apns_token`.
|
||||||
|
--
|
||||||
|
-- Apple's APNs contract guarantees one token per (device, app) — two
|
||||||
|
-- Device rows pointing at the same `apns_token` would mean the same
|
||||||
|
-- physical iPhone is registered twice and `sendViaApns()` would fan a
|
||||||
|
-- single notification out twice, double-charging APNs quota and
|
||||||
|
-- spamming the user.
|
||||||
|
--
|
||||||
|
-- Pair this DDL with the route-side fix in `POST /api/devices` that
|
||||||
|
-- drops `NOT: { userId }` from the cross-user-hijack guard so the
|
||||||
|
-- same-user collision path returns the existing 200 idempotent upsert
|
||||||
|
-- response, and the cross-user collision path keeps its existing 409.
|
||||||
|
--
|
||||||
|
-- Strictly additive at the rolling-upgrade boundary:
|
||||||
|
-- * The new partial unique index covers ONLY `apns_token IS NOT NULL`,
|
||||||
|
-- so the historical reality of multiple Device rows with NULL
|
||||||
|
-- `apns_token` (legacy iOS builds without the APNs handshake) stays
|
||||||
|
-- valid.
|
||||||
|
-- * The old non-unique `devices_apns_token_idx` is dropped because
|
||||||
|
-- the partial unique index supersedes it for the dispatcher's
|
||||||
|
-- per-user lookup.
|
||||||
|
-- * The CHECK from migration 0037 (token + environment paired) is
|
||||||
|
-- unaffected.
|
||||||
|
--
|
||||||
|
-- If a duplicate row exists in production at upgrade time the migration
|
||||||
|
-- will fail loudly — that's the intended signal that an iOS install
|
||||||
|
-- collision is already corrupting deliveries.
|
||||||
|
|
||||||
|
CREATE UNIQUE INDEX "devices_apns_token_unique"
|
||||||
|
ON "devices" ("apns_token")
|
||||||
|
WHERE "apns_token" IS NOT NULL;
|
||||||
|
|
||||||
|
DROP INDEX IF EXISTS "devices_apns_token_idx";
|
||||||
@@ -833,7 +833,11 @@ model Device {
|
|||||||
user User @relation(fields: [userId], references: [id], onDelete: Cascade)
|
user User @relation(fields: [userId], references: [id], onDelete: Cascade)
|
||||||
|
|
||||||
@@index([userId])
|
@@index([userId])
|
||||||
@@index([apnsToken])
|
// `devices_apns_token_unique` is a partial UNIQUE index over
|
||||||
|
// `apns_token IS NOT NULL` — see migration 0041. Prisma can't express
|
||||||
|
// a partial index in the schema language, so it's introduced via raw
|
||||||
|
// SQL and intentionally omitted here. Do not re-add `@@index([apnsToken])`
|
||||||
|
// (the non-unique sibling) — the partial unique index supersedes it.
|
||||||
@@map("devices")
|
@@map("devices")
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|||||||
@@ -84,16 +84,29 @@ export const POST = apiHandler(async (request: NextRequest) => {
|
|||||||
apnsEnvironment,
|
apnsEnvironment,
|
||||||
} = parsed.data;
|
} = parsed.data;
|
||||||
|
|
||||||
// APNs-token cross-user-hijack guard: APNs tokens aren't secrets, so
|
// APNs-token collision lookup. Two cases:
|
||||||
// accepting one already owned by another user would let any client
|
// * A row for ANOTHER user owns this token → 409 hijack guard.
|
||||||
// who learns one redirect that user's pushes. Reject before any
|
// APNs tokens aren't secrets, so accepting wire input that
|
||||||
// upsert so we never accidentally transfer ownership.
|
// 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) {
|
if (apnsToken) {
|
||||||
const existingApns = await prisma.device.findFirst({
|
const existingApns = await prisma.device.findFirst({
|
||||||
where: { apnsToken, NOT: { userId: user.id } },
|
where: { apnsToken },
|
||||||
select: { id: true },
|
select: { id: true, userId: true },
|
||||||
});
|
});
|
||||||
if (existingApns) {
|
if (existingApns && existingApns.userId !== user.id) {
|
||||||
await auditLog("device.register.denied", {
|
await auditLog("device.register.denied", {
|
||||||
userId: user.id,
|
userId: user.id,
|
||||||
details: {
|
details: {
|
||||||
|
|||||||
Reference in new issue
Block a user