diff --git a/.github/workflows/supersync-server-tests.yml b/.github/workflows/supersync-server-tests.yml index a9f4033929..c636ad45af 100644 --- a/.github/workflows/supersync-server-tests.yml +++ b/.github/workflows/supersync-server-tests.yml @@ -20,9 +20,25 @@ permissions: jobs: supersync-server-tests: - name: SuperSync Server Unit Tests + name: SuperSync Server Tests runs-on: ubuntu-latest timeout-minutes: 10 + env: + DATABASE_URL: postgresql://supersync:superpassword@localhost:5432/supersync_db + services: + postgres: + image: postgres:16-alpine + env: + POSTGRES_USER: supersync + POSTGRES_PASSWORD: superpassword + POSTGRES_DB: supersync_db + ports: + - 5432:5432 + options: >- + --health-cmd "pg_isready -U supersync -d supersync_db" + --health-interval 3s + --health-timeout 5s + --health-retries 10 steps: - name: Check out Git repository uses: actions/checkout@9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0 # v6 @@ -41,3 +57,9 @@ jobs: run: npm i - name: Run SuperSync Server Tests run: cd packages/super-sync-server && npm test + - name: Apply PostgreSQL Test Schema + working-directory: packages/super-sync-server + run: npx prisma db push --skip-generate + - name: Run SuperSync PostgreSQL Integration Tests + working-directory: packages/super-sync-server + run: npm run test:integration:postgres diff --git a/ARCHITECTURE-DECISIONS.md b/ARCHITECTURE-DECISIONS.md index 51427b573c..431d00f8e2 100644 --- a/ARCHITECTURE-DECISIONS.md +++ b/ARCHITECTURE-DECISIONS.md @@ -208,6 +208,57 @@ The atomic op's headline benefit — reversing the whole thing as one unit — w --- +### 6. Passkeys Stay Pending Until Email Verification + +**Status**: ✅ Active (since July 2026) + +**Decision**: A passkey submitted during account registration is stored as a +`PendingPasskeyRegistration` tied to its exact email-verification token. It is +promoted to the user's active `Passkey` set only when that token is consumed. + +**Rationale**: + +- A WebAuthn registration ceremony proves possession of a credential, not + ownership of the email address entered alongside it. +- Storing a submitted credential directly on an unverified user lets an attacker + pre-register a victim's address, then have the victim's later magic-link + verification activate the attacker's passkey. +- Keeping separate pending attempts prevents concurrent registrations from + replacing or activating one another. The email owner chooses the credential + by consuming the link produced by that same registration attempt. +- Failed email delivery leaves the bounded, expiring pending attempt in place. + Deleting the shared unverified user can race a concurrent registration and + invalidate a link that was successfully delivered. + +**Implementation**: + +- Passkey registration stores no active credential and creates one pending row + per verification token. +- Email verification atomically claims the unverified user, replaces active + passkeys with the credential bound to that token, and deletes the user's + remaining pending attempts. +- Passkey verification tokens live only on pending registrations; user-row + verification tokens belong to magic-link registrations. Consuming a user-row + token verifies the email but removes untrusted active and pending passkeys. +- The migration moves the latest legacy credential for each unverified user to + the pending table and removes all active credentials from unverified users. +- The resend cap bounds pending rows per unverified account; rows also expire + with their verification tokens. + +**Key Files**: + +- [`auth.ts`](packages/super-sync-server/src/auth.ts) +- [`passkey.ts`](packages/super-sync-server/src/passkey.ts) +- [`schema.prisma`](packages/super-sync-server/prisma/schema.prisma) + +**When to Update This Pattern**: + +- Changing passkey enrollment or email-verification flows +- Adding another credential type to registration +- Changing verification-token persistence or cleanup + +--- + ## How to Use This Document ### When Making Architectural Changes diff --git a/packages/super-sync-server/package.json b/packages/super-sync-server/package.json index 32f311752e..b310b7eb9c 100644 --- a/packages/super-sync-server/package.json +++ b/packages/super-sync-server/package.json @@ -9,6 +9,7 @@ "dev": "nodemon --watch src --ext ts --exec ts-node src/index.ts", "pretest": "prisma generate", "test": "vitest run", + "test:integration:postgres": "vitest run --config vitest.integration.config.ts --maxWorkers=1 tests/integration/registration-races.integration.spec.ts tests/integration/clean-slate-atomicity-sql.integration.spec.ts tests/integration/conflict-detection-sql.integration.spec.ts tests/integration/snapshot-vector-clock-sql.integration.spec.ts", "docker:build": "./scripts/build-and-push.sh", "docker:deploy": "./scripts/deploy.sh", "docker:deploy:build": "./scripts/deploy.sh --build", diff --git a/packages/super-sync-server/prisma/migrations/20260713000000_add_pending_passkey_registrations/migration.sql b/packages/super-sync-server/prisma/migrations/20260713000000_add_pending_passkey_registrations/migration.sql new file mode 100644 index 0000000000..36950888fd --- /dev/null +++ b/packages/super-sync-server/prisma/migrations/20260713000000_add_pending_passkey_registrations/migration.sql @@ -0,0 +1,61 @@ +CREATE TABLE "pending_passkey_registrations" ( + "id" TEXT NOT NULL, + "verification_token" TEXT NOT NULL, + "verification_token_expires_at" BIGINT NOT NULL, + "credential_id" BYTEA NOT NULL, + "public_key" BYTEA NOT NULL, + "counter" BIGINT NOT NULL DEFAULT 0, + "transports" TEXT, + "created_at" TIMESTAMP(3) NOT NULL DEFAULT CURRENT_TIMESTAMP, + "user_id" INTEGER NOT NULL, + + CONSTRAINT "pending_passkey_registrations_pkey" PRIMARY KEY ("id") +); + +CREATE UNIQUE INDEX "pending_passkey_registrations_verification_token_key" +ON "pending_passkey_registrations"("verification_token"); + +CREATE INDEX "pending_passkey_registrations_user_id_idx" +ON "pending_passkey_registrations"("user_id"); + +ALTER TABLE "pending_passkey_registrations" +ADD CONSTRAINT "pending_passkey_registrations_user_id_fkey" +FOREIGN KEY ("user_id") REFERENCES "users"("id") ON DELETE CASCADE ON UPDATE CASCADE; + +-- Move credentials created by the legacy registration flow out of the active +-- passkey set. The most recently created credential is the one associated with +-- the user's current verification token because re-registration replaced the +-- previous credential before rotating that token. +INSERT INTO "pending_passkey_registrations" ( + "id", + "verification_token", + "verification_token_expires_at", + "credential_id", + "public_key", + "counter", + "transports", + "created_at", + "user_id" +) +SELECT DISTINCT ON (p."user_id") + 'legacy-' || p."id", + u."verification_token", + u."verification_token_expires_at", + p."credential_id", + p."public_key", + p."counter", + p."transports", + p."created_at", + p."user_id" +FROM "passkeys" p +JOIN "users" u ON u."id" = p."user_id" +WHERE u."is_verified" = 0 + AND u."verification_token" IS NOT NULL + AND u."verification_token_expires_at" IS NOT NULL +ORDER BY p."user_id", p."created_at" DESC, p."id" DESC +ON CONFLICT ("verification_token") DO NOTHING; + +DELETE FROM "passkeys" p +USING "users" u +WHERE u."id" = p."user_id" + AND u."is_verified" = 0; diff --git a/packages/super-sync-server/prisma/schema.prisma b/packages/super-sync-server/prisma/schema.prisma index 2c26cb45a6..8b78994d2d 100644 --- a/packages/super-sync-server/prisma/schema.prisma +++ b/packages/super-sync-server/prisma/schema.prisma @@ -36,6 +36,7 @@ model User { syncState UserSyncState? devices SyncDevice[] passkeys Passkey[] + pendingPasskeyRegistrations PendingPasskeyRegistration[] @@index([verificationToken]) @@index([resetPasswordToken]) @@ -60,6 +61,23 @@ model Passkey { @@map("passkeys") } +model PendingPasskeyRegistration { + id String @id @default(cuid()) + verificationToken String @unique @map("verification_token") + verificationTokenExpiresAt BigInt @map("verification_token_expires_at") + credentialId Bytes @map("credential_id") + publicKey Bytes @map("public_key") + counter BigInt @default(0) + transports String? + createdAt DateTime @default(now()) @map("created_at") + + userId Int @map("user_id") + user User @relation(fields: [userId], references: [id], onDelete: Cascade) + + @@index([userId]) + @@map("pending_passkey_registrations") +} + model Operation { id String @id userId Int @map("user_id") diff --git a/packages/super-sync-server/src/auth.ts b/packages/super-sync-server/src/auth.ts index ece5d5f502..c226150e14 100644 --- a/packages/super-sync-server/src/auth.ts +++ b/packages/super-sync-server/src/auth.ts @@ -41,6 +41,51 @@ export const getJwtSecret = (): string => { const JWT_SECRET = getJwtSecret(); export const verifyEmail = async (token: string): Promise => { + const pendingPasskey = await prisma.pendingPasskeyRegistration.findUnique({ + where: { verificationToken: token }, + }); + + if (pendingPasskey) { + if (pendingPasskey.verificationTokenExpiresAt < BigInt(Date.now())) { + throw new Error('Verification token has expired'); + } + + const activated = await prisma.$transaction(async (tx) => { + const claim = await tx.user.updateMany({ + where: { id: pendingPasskey.userId, isVerified: 0 }, + data: { + isVerified: 1, + verificationToken: null, + verificationTokenExpiresAt: null, + verificationResendCount: 0, + }, + }); + if (claim.count !== 1) return false; + + // Only the credential carried by this exact email link is trusted. Other + // attempts for the same address may have been initiated by someone else. + await tx.passkey.deleteMany({ where: { userId: pendingPasskey.userId } }); + await tx.passkey.create({ + data: { + userId: pendingPasskey.userId, + credentialId: pendingPasskey.credentialId, + publicKey: pendingPasskey.publicKey, + counter: pendingPasskey.counter, + transports: pendingPasskey.transports, + }, + }); + await tx.pendingPasskeyRegistration.deleteMany({ + where: { userId: pendingPasskey.userId }, + }); + return true; + }); + + if (!activated) throw new Error('Invalid verification token'); + authCache.invalidate(pendingPasskey.userId); + Logger.info(`User verified with passkey (ID: ${pendingPasskey.userId})`); + return true; + } + const user = await prisma.user.findFirst({ where: { verificationToken: token }, }); @@ -56,15 +101,26 @@ export const verifyEmail = async (token: string): Promise => { throw new Error('Verification token has expired'); } - await prisma.user.update({ - where: { id: user.id }, - data: { - isVerified: 1, - verificationToken: null, - verificationTokenExpiresAt: null, - verificationResendCount: 0, - }, + const verified = await prisma.$transaction(async (tx) => { + const claim = await tx.user.updateMany({ + where: { id: user.id, isVerified: 0, verificationToken: token }, + data: { + isVerified: 1, + verificationToken: null, + verificationTokenExpiresAt: null, + verificationResendCount: 0, + }, + }); + if (claim.count !== 1) return false; + + // Reaching the user-token path means this is a magic-link registration: + // passkey registration tokens live only in pendingPasskeyRegistration. + // Email ownership does not prove ownership of a separately submitted key. + await tx.passkey.deleteMany({ where: { userId: user.id } }); + await tx.pendingPasskeyRegistration.deleteMany({ where: { userId: user.id } }); + return true; }); + if (!verified) throw new Error('Invalid verification token'); // AUTH_CACHE_INVALIDATION: drop any negative (isVerified:false) entry so the // now-verified user isn't denied for up to the cache TTL, and so the cache @@ -374,9 +430,15 @@ export const registerWithMagicLink = async ( } } - // Update existing unverified user with new verification token - await prisma.user.update({ - where: { id: existingUser.id }, + // Update the same still-unverified row that was checked above. If another + // request verified or removed it while the email was in flight, the link + // is simply left inactive and the response remains neutral. + const updated = await prisma.user.updateMany({ + where: { + id: existingUser.id, + isVerified: 0, + verificationResendCount: { lt: MAX_VERIFICATION_RESEND_COUNT }, + }, data: { verificationToken, verificationTokenExpiresAt: tokenExpiresAt, @@ -384,6 +446,7 @@ export const registerWithMagicLink = async ( ...(acceptedAt !== undefined && { termsAcceptedAt: acceptedAt }), }, }); + if (updated.count !== 1) return { message: REGISTRATION_SUCCESS_MESSAGE }; Logger.info( `Updated verification token for unverified user (ID: ${existingUser.id})`, @@ -403,18 +466,10 @@ export const registerWithMagicLink = async ( Logger.info(`Created new magic-link user`); if (!config.testMode?.autoVerifyUsers) { - // Send verification email; clean up new user on failure + // Keep the unverified row on delivery failure. Deleting it can race a + // concurrent registration that has already started using the same row. const emailSent = await sendVerificationEmail(normalizedEmail, verificationToken); if (!emailSent) { - // AUTH_CACHE_INVALIDATION: no invalidate needed here. This only - // deletes the user just created above (isVerified: 0). No JWT is - // issued for an unverified user, so verifyToken was never called for - // it and no authCache entry can exist. Documented to keep the - // bracket-every-delete convention auditable. - await prisma.user.deleteMany({ - where: { email: normalizedEmail, isVerified: 0 }, - }); - Logger.info(`Cleaned up failed magic-link registration for new user`); return { message: REGISTRATION_SUCCESS_MESSAGE }; } } diff --git a/packages/super-sync-server/src/passkey.ts b/packages/super-sync-server/src/passkey.ts index 98bc872574..12b5798b8d 100644 --- a/packages/super-sync-server/src/passkey.ts +++ b/packages/super-sync-server/src/passkey.ts @@ -13,10 +13,14 @@ import type { import { prisma } from './db'; import { Logger } from './logger'; import { randomBytes } from 'crypto'; -import { sendPasskeyRecoveryEmail } from './email'; +import { sendPasskeyRecoveryEmail, sendVerificationEmail } from './email'; import { Prisma } from '@prisma/client'; import { loadConfigFromEnv } from './config'; -import { VERIFICATION_TOKEN_EXPIRY_MS, MAX_VERIFICATION_RESEND_COUNT } from './auth'; +import { + VERIFICATION_TOKEN_EXPIRY_MS, + MAX_VERIFICATION_RESEND_COUNT, + verifyEmail, +} from './auth'; import { authCache } from './auth-cache'; // Constants @@ -176,6 +180,8 @@ export const verifyRegistration = async ( const acceptedAt = termsAcceptedAt ? BigInt(termsAcceptedAt) : BigInt(Date.now()); try { + const config = loadConfigFromEnv(); + // Check if unverified user exists (re-registration attempt) const existingUser = await prisma.user.findUnique({ where: { email: email.toLowerCase() }, @@ -190,73 +196,54 @@ export const verifyRegistration = async ( Logger.warn(`Verification resend cap reached (ID: ${existingUser.id})`); return { message: REGISTRATION_SUCCESS_MESSAGE }; } + } - // Update existing unverified user with new passkey - await prisma.$transaction(async (tx) => { - // Delete old passkeys - await tx.passkey.deleteMany({ where: { userId: existingUser.id } }); - - // Create new passkey - await tx.passkey.create({ - data: { - credentialId: credentialIdRawBytes, - publicKey: Buffer.from(credentialInfo.publicKey), - counter: BigInt(credentialInfo.counter), - transports: credential.response.transports - ? JSON.stringify(credential.response.transports) - : null, - userId: existingUser.id, + const pendingCreated = await prisma.$transaction(async (tx) => { + let userId: number; + if (existingUser) { + const claim = await tx.user.updateMany({ + where: { + id: existingUser.id, + isVerified: 0, + verificationResendCount: { lt: MAX_VERIFICATION_RESEND_COUNT }, }, - }); - - // Update user with new verification token - await tx.user.update({ - where: { id: existingUser.id }, data: { - verificationToken, - verificationTokenExpiresAt: tokenExpiresAt, verificationResendCount: { increment: 1 }, }, }); - }); + if (claim.count !== 1) return false; + userId = existingUser.id; + } else { + const createdUser = await tx.user.create({ + data: { + email: email.toLowerCase(), + passwordHash: null, + termsAcceptedAt: acceptedAt, + }, + }); + userId = createdUser.id; + } - Logger.info(`Updated passkey for unverified user (ID: ${existingUser.id})`); - } else { - // Create new user with passkey - await prisma.user.create({ + await tx.pendingPasskeyRegistration.create({ data: { - email: email.toLowerCase(), - passwordHash: null, // Passkey-only user + userId, verificationToken, verificationTokenExpiresAt: tokenExpiresAt, - termsAcceptedAt: acceptedAt, - passkeys: { - create: { - credentialId: credentialIdRawBytes, - publicKey: Buffer.from(credentialInfo.publicKey), - counter: BigInt(credentialInfo.counter), - transports: credential.response.transports - ? JSON.stringify(credential.response.transports) - : null, - }, - }, + credentialId: credentialIdRawBytes, + publicKey: Buffer.from(credentialInfo.publicKey), + counter: BigInt(credentialInfo.counter), + transports: credential.response.transports + ? JSON.stringify(credential.response.transports) + : null, }, }); - - Logger.info(`Created new passkey user`); - } + return true; + }); + if (!pendingCreated) return { message: REGISTRATION_SUCCESS_MESSAGE }; // In TEST_MODE with autoVerifyUsers, skip email and auto-verify - const config = loadConfigFromEnv(); if (config.testMode?.autoVerifyUsers) { - await prisma.user.update({ - where: { email: email.toLowerCase() }, - data: { - isVerified: 1, - verificationToken: null, - verificationTokenExpiresAt: null, - }, - }); + await verifyEmail(verificationToken); Logger.info(`[TEST_MODE] Auto-verified passkey user`); return { message: 'Registration successful. Your account has been automatically verified.', @@ -264,24 +251,8 @@ export const verifyRegistration = async ( } // Normal flow: send verification email - const { sendVerificationEmail } = await import('./email'); const emailSent = await sendVerificationEmail(email, verificationToken); - - if (!emailSent) { - // Clean up on email failure for new users - const user = await prisma.user.findUnique({ - where: { email: email.toLowerCase() }, - }); - if (user && user.isVerified === 0) { - // AUTH_CACHE_INVALIDATION: bracket the delete pre + post, matching every - // other user-delete site, so the convention stays uniformly auditable. - authCache.invalidate(user.id); - await prisma.user.delete({ where: { id: user.id } }); - authCache.invalidate(user.id); - Logger.info(`Cleaned up failed passkey registration (ID: ${user.id})`); - } - return { message: REGISTRATION_SUCCESS_MESSAGE }; - } + if (!emailSent) return { message: REGISTRATION_SUCCESS_MESSAGE }; Logger.info(`Passkey registration initiated`); return { message: REGISTRATION_SUCCESS_MESSAGE }; diff --git a/packages/super-sync-server/tests/integration/registration-races.integration.spec.ts b/packages/super-sync-server/tests/integration/registration-races.integration.spec.ts new file mode 100644 index 0000000000..785cfe9196 --- /dev/null +++ b/packages/super-sync-server/tests/integration/registration-races.integration.spec.ts @@ -0,0 +1,245 @@ +/** + * Real-PostgreSQL coverage for registration races that mocks cannot prove. + * + * Prerequisites: + * DATABASE_URL=postgresql://supersync:superpassword@localhost:55432/supersync_db + * + * Run with: + * npx vitest run --config vitest.integration.config.ts \ + * tests/integration/registration-races.integration.spec.ts + */ +import { + afterAll, + afterEach, + beforeAll, + beforeEach, + describe, + expect, + it, + Mock, + vi, +} from 'vitest'; +import { PrismaClient } from '@prisma/client'; +import type { RegistrationResponseJSON } from '@simplewebauthn/server'; + +vi.hoisted(() => { + process.env.JWT_SECRET = 'integration-test-jwt-secret-at-least-32-characters'; + delete process.env.TEST_MODE; + delete process.env.TEST_MODE_CONFIRM; +}); + +vi.mock('../../src/email', () => ({ + sendVerificationEmail: vi.fn(), + sendLoginMagicLinkEmail: vi.fn(), + sendPasskeyRecoveryEmail: vi.fn(), +})); + +vi.mock('@simplewebauthn/server', async (importOriginal) => ({ + ...(await importOriginal()), + verifyRegistrationResponse: vi.fn(), +})); + +import { disconnectDb } from '../../src/db'; +import { sendVerificationEmail } from '../../src/email'; +import * as webAuthn from '@simplewebauthn/server'; +import { registerWithMagicLink, verifyEmail } from '../../src/auth'; +import { generateRegistrationOptions, verifyRegistration } from '../../src/passkey'; + +const DATABASE_URL = process.env.DATABASE_URL; +const describeWithDb = DATABASE_URL ? describe : describe.skip; +const RUN_ID = `${Date.now()}-${process.pid}`; +const EMAIL_PREFIX = `registration-race-${RUN_ID}`; + +const mockSendVerificationEmail = sendVerificationEmail as Mock; +const mockVerifyRegistrationResponse = webAuthn.verifyRegistrationResponse as Mock; + +const registrationCredential = (id: string): RegistrationResponseJSON => ({ + id, + rawId: id, + type: 'public-key', + response: { + clientDataJSON: 'client-data', + attestationObject: 'attestation', + transports: ['internal'], + }, + clientExtensionResults: {}, +}); + +const preparePasskeyRegistration = (credentialId: Buffer): void => { + const credentialIdBase64url = credentialId.toString('base64url'); + mockVerifyRegistrationResponse.mockResolvedValueOnce({ + verified: true, + registrationInfo: { + credential: { + id: new Uint8Array(Buffer.from(credentialIdBase64url)), + publicKey: new Uint8Array([5, 6, 7, 8]), + counter: 0, + }, + credentialDeviceType: 'multiDevice', + credentialBackedUp: true, + }, + }); +}; + +const registerPasskey = async (email: string, credentialId: Buffer): Promise => { + preparePasskeyRegistration(credentialId); + await generateRegistrationOptions(email); + await verifyRegistration( + email, + registrationCredential(credentialId.toString('base64url')), + ); +}; + +const deferred = (): { + promise: Promise; + resolve: (value: T) => void; +} => { + let resolve!: (value: T) => void; + const promise = new Promise((resolvePromise) => { + resolve = resolvePromise; + }); + return { promise, resolve }; +}; + +describeWithDb('Registration races (PostgreSQL)', () => { + let observer: PrismaClient; + + beforeAll(() => { + if (!DATABASE_URL) throw new Error('DATABASE_URL is required for integration tests'); + observer = new PrismaClient({ datasources: { db: { url: DATABASE_URL } } }); + }); + + beforeEach(() => { + vi.clearAllMocks(); + mockSendVerificationEmail.mockResolvedValue(true); + }); + + afterEach(async () => { + await observer.user.deleteMany({ where: { email: { startsWith: EMAIL_PREFIX } } }); + }); + + afterAll(async () => { + await observer.user.deleteMany({ where: { email: { startsWith: EMAIL_PREFIX } } }); + await observer.$disconnect(); + await disconnectDb(); + }); + + it('activates only the passkey bound to the verification link', async () => { + const email = `${EMAIL_PREFIX}-existing@test.local`; + const attackerCredentialId = Buffer.from(`attacker-${RUN_ID}`); + const ownerCredentialId = Buffer.from(`owner-${RUN_ID}`); + await observer.user.create({ + data: { + email, + verificationToken: 'original-verification-token', + verificationTokenExpiresAt: BigInt(Date.now() + 60_000), + verificationResendCount: 1, + passkeys: { + create: { + credentialId: attackerCredentialId, + publicKey: Buffer.from([1, 2, 3, 4]), + }, + }, + }, + }); + + await registerPasskey(email, ownerCredentialId); + const verificationToken = mockSendVerificationEmail.mock.calls[0][1] as string; + await verifyEmail(verificationToken); + + const storedUser = await observer.user.findUniqueOrThrow({ + where: { email }, + include: { passkeys: true }, + }); + expect(storedUser.passkeys).toHaveLength(1); + expect(storedUser.passkeys[0].credentialId).toEqual(ownerCredentialId); + expect(storedUser.isVerified).toBe(1); + }); + + it('keeps a passkey registration valid when a concurrent email delivery fails', async () => { + const email = `${EMAIL_PREFIX}-passkey-cleanup@test.local`; + const firstCredentialId = Buffer.from(`first-${RUN_ID}`); + const secondCredentialId = Buffer.from(`second-${RUN_ID}`); + const firstEmailStarted = deferred(); + const secondEmailStarted = deferred(); + const firstEmailResult = deferred(); + const secondEmailResult = deferred(); + mockSendVerificationEmail + .mockImplementationOnce(async () => { + firstEmailStarted.resolve(); + return firstEmailResult.promise; + }) + .mockImplementationOnce(async () => { + secondEmailStarted.resolve(); + return secondEmailResult.promise; + }); + + const firstRegistration = registerPasskey(email, firstCredentialId); + await firstEmailStarted.promise; + const secondRegistration = registerPasskey(email, secondCredentialId); + await secondEmailStarted.promise; + firstEmailResult.resolve(false); + await firstRegistration; + secondEmailResult.resolve(true); + await secondRegistration; + const secondVerificationToken = mockSendVerificationEmail.mock.calls[1][1] as string; + await verifyEmail(secondVerificationToken); + + const storedUser = await observer.user.findUnique({ + where: { email }, + include: { passkeys: true }, + }); + expect(storedUser?.passkeys).toHaveLength(1); + expect(storedUser?.passkeys[0].credentialId).toEqual(secondCredentialId); + expect(storedUser?.isVerified).toBe(1); + }); + + it('keeps a magic-link registration valid when a concurrent email delivery fails', async () => { + const email = `${EMAIL_PREFIX}-magic-link-cleanup@test.local`; + const firstEmailStarted = deferred(); + const secondEmailStarted = deferred(); + const firstEmailResult = deferred(); + const secondEmailResult = deferred(); + mockSendVerificationEmail + .mockImplementationOnce(async () => { + firstEmailStarted.resolve(); + return firstEmailResult.promise; + }) + .mockImplementationOnce(async () => { + secondEmailStarted.resolve(); + return secondEmailResult.promise; + }); + + const firstRegistration = registerWithMagicLink(email, Date.now()); + await firstEmailStarted.promise; + const secondRegistration = registerWithMagicLink(email, Date.now()); + await secondEmailStarted.promise; + firstEmailResult.resolve(false); + await firstRegistration; + secondEmailResult.resolve(true); + await secondRegistration; + const secondVerificationToken = mockSendVerificationEmail.mock.calls[1][1] as string; + await verifyEmail(secondVerificationToken); + + const storedUser = await observer.user.findUnique({ where: { email } }); + expect(storedUser).not.toBeNull(); + expect(storedUser?.isVerified).toBe(1); + }); + + it('does not let a later passkey attempt invalidate a magic-link verification', async () => { + const email = `${EMAIL_PREFIX}-magic-before-passkey@test.local`; + await registerWithMagicLink(email, Date.now()); + const magicLinkToken = mockSendVerificationEmail.mock.calls[0][1] as string; + + await registerPasskey(email, Buffer.from(`untrusted-${RUN_ID}`)); + await verifyEmail(magicLinkToken); + + const storedUser = await observer.user.findUniqueOrThrow({ + where: { email }, + include: { passkeys: true, pendingPasskeyRegistrations: true }, + }); + expect(storedUser.isVerified).toBe(1); + expect(storedUser.passkeys).toHaveLength(0); + expect(storedUser.pendingPasskeyRegistrations).toHaveLength(0); + }); +}); diff --git a/packages/super-sync-server/tests/magic-link-registration.spec.ts b/packages/super-sync-server/tests/magic-link-registration.spec.ts index dc2bb43167..cd07f8e21d 100644 --- a/packages/super-sync-server/tests/magic-link-registration.spec.ts +++ b/packages/super-sync-server/tests/magic-link-registration.spec.ts @@ -25,6 +25,15 @@ vi.mock('../src/db', () => { delete: vi.fn(), deleteMany: vi.fn(), }, + passkey: { + create: vi.fn(), + deleteMany: vi.fn(), + }, + pendingPasskeyRegistration: { + findUnique: vi.fn(), + deleteMany: vi.fn(), + }, + $transaction: vi.fn(), }; return { prisma: mockPrisma }; }); @@ -75,6 +84,7 @@ import { Prisma } from '@prisma/client'; import { registerWithMagicLink, requestLoginMagicLink, + verifyEmail, verifyLoginMagicLink, } from '../src/auth'; @@ -98,6 +108,15 @@ describe('Magic Link Registration', () => { delete: Mock; deleteMany: Mock; }; + passkey: { + create: Mock; + deleteMany: Mock; + }; + pendingPasskeyRegistration: { + findUnique: Mock; + deleteMany: Mock; + }; + $transaction: Mock; }; const mockSendVerificationEmail = sendVerificationEmail as Mock; @@ -105,6 +124,12 @@ describe('Magic Link Registration', () => { beforeEach(() => { vi.clearAllMocks(); + mockPrisma.user.updateMany.mockResolvedValue({ count: 1 }); + mockPrisma.pendingPasskeyRegistration.findUnique.mockResolvedValue(null); + mockPrisma.$transaction.mockImplementation( + async (callback: (tx: typeof mockPrisma) => Promise) => + callback(mockPrisma), + ); }); describe('New user registration', () => { @@ -200,8 +225,6 @@ describe('Magic Link Registration', () => { isVerified: 0, verificationResendCount: 2, }); - mockPrisma.user.update.mockResolvedValue({}); - const result = await registerWithMagicLink(testEmail, Date.now()); expect(result).toEqual(registrationResponse); @@ -211,9 +234,9 @@ describe('Magic Link Registration', () => { testEmail, expect.any(String), ); - expect(mockPrisma.user.update).toHaveBeenCalledWith( + expect(mockPrisma.user.updateMany).toHaveBeenCalledWith( expect.objectContaining({ - where: { id: 1 }, + where: expect.objectContaining({ id: 1, isVerified: 0 }), data: expect.objectContaining({ verificationToken: expect.any(String), verificationTokenExpiresAt: expect.any(BigInt), @@ -235,29 +258,24 @@ describe('Magic Link Registration', () => { expect(result).toEqual(registrationResponse); expect(mockSendVerificationEmail).not.toHaveBeenCalled(); - expect(mockPrisma.user.update).not.toHaveBeenCalled(); + expect(mockPrisma.user.updateMany).not.toHaveBeenCalled(); }); }); - describe('Email failure cleanup', () => { - it('should delete newly created user when email sending fails', async () => { - mockPrisma.user.findUnique - .mockResolvedValueOnce(null) // Initial lookup: no existing user - .mockResolvedValueOnce({ id: 1, email: testEmail, isVerified: 0 }); // Cleanup lookup + describe('Email failure handling', () => { + it('should retain a newly created user when email sending fails', async () => { + mockPrisma.user.findUnique.mockResolvedValueOnce(null); mockPrisma.user.create.mockResolvedValue({ id: 1, email: testEmail, isVerified: 0, }); mockSendVerificationEmail.mockResolvedValueOnce(false); - mockPrisma.user.deleteMany.mockResolvedValue({ count: 1 }); const result = await registerWithMagicLink(testEmail, Date.now()); expect(result).toEqual(registrationResponse); - expect(mockPrisma.user.deleteMany).toHaveBeenCalledWith({ - where: { email: testEmail, isVerified: 0 }, - }); + expect(mockPrisma.user.deleteMany).not.toHaveBeenCalled(); }); it('should NOT delete or update pre-existing unverified user when email sending fails', async () => { @@ -280,6 +298,54 @@ describe('Magic Link Registration', () => { }); }); + describe('Email verification', () => { + it('should activate only the passkey bound to the supplied token', async () => { + mockPrisma.pendingPasskeyRegistration.findUnique.mockResolvedValue({ + userId: 1, + verificationToken: 'pending-token', + verificationTokenExpiresAt: BigInt(Date.now() + 60_000), + credentialId: Buffer.from('owner-credential'), + publicKey: Buffer.from('owner-public-key'), + counter: 0n, + transports: '["internal"]', + }); + + await verifyEmail('pending-token'); + + expect(mockPrisma.passkey.deleteMany).toHaveBeenCalledWith({ + where: { userId: 1 }, + }); + expect(mockPrisma.passkey.create).toHaveBeenCalledWith({ + data: expect.objectContaining({ + userId: 1, + credentialId: Buffer.from('owner-credential'), + }), + }); + expect(mockPrisma.pendingPasskeyRegistration.deleteMany).toHaveBeenCalledWith({ + where: { userId: 1 }, + }); + }); + + it('should not activate passkeys when a magic-link token verifies the email', async () => { + const token = 'legacy-or-current-magic-token'; + mockPrisma.user.findFirst.mockResolvedValue({ + id: 1, + verificationToken: token, + verificationTokenExpiresAt: BigInt(Date.now() + 60_000), + }); + + await verifyEmail(token); + + expect(mockPrisma.passkey.deleteMany).toHaveBeenCalledWith({ + where: { userId: 1 }, + }); + expect(mockPrisma.passkey.create).not.toHaveBeenCalled(); + expect(mockPrisma.pendingPasskeyRegistration.deleteMany).toHaveBeenCalledWith({ + where: { userId: 1 }, + }); + }); + }); + describe('P2002 unique constraint handling', () => { it('should return the neutral response after a P2002 registration race', async () => { mockPrisma.user.findUnique.mockResolvedValue(null); diff --git a/packages/super-sync-server/tests/passkey.spec.ts b/packages/super-sync-server/tests/passkey.spec.ts index fffdf17262..e14753f4a7 100644 --- a/packages/super-sync-server/tests/passkey.spec.ts +++ b/packages/super-sync-server/tests/passkey.spec.ts @@ -21,7 +21,7 @@ vi.mock('../src/db', () => { create: vi.fn(), update: vi.fn(), updateMany: vi.fn(), - delete: vi.fn(), + deleteMany: vi.fn(), }, passkey: { create: vi.fn(), @@ -29,6 +29,11 @@ vi.mock('../src/db', () => { deleteMany: vi.fn(), findUnique: vi.fn(), }, + pendingPasskeyRegistration: { + findUnique: vi.fn(), + create: vi.fn(), + deleteMany: vi.fn(), + }, $transaction: vi.fn(), }; return { prisma: mockPrisma }; @@ -99,7 +104,7 @@ describe('Passkey Authentication', () => { create: Mock; update: Mock; updateMany: Mock; - delete: Mock; + deleteMany: Mock; }; passkey: { create: Mock; @@ -107,6 +112,11 @@ describe('Passkey Authentication', () => { deleteMany: Mock; findUnique: Mock; }; + pendingPasskeyRegistration: { + findUnique: Mock; + create: Mock; + deleteMany: Mock; + }; $transaction: Mock; }; @@ -134,6 +144,13 @@ describe('Passkey Authentication', () => { allowCredentials: [], userVerification: 'preferred', } as PublicKeyCredentialRequestOptionsJSON); + mockPrisma.user.updateMany.mockResolvedValue({ count: 1 }); + mockPrisma.pendingPasskeyRegistration.findUnique.mockResolvedValue(null); + mockPrisma.pendingPasskeyRegistration.create.mockResolvedValue({}); + mockPrisma.$transaction.mockImplementation( + async (callback: (tx: typeof mockPrisma) => Promise) => + callback(mockPrisma), + ); }); afterEach(() => { @@ -238,15 +255,22 @@ describe('Passkey Authentication', () => { result.message.includes('automatically verified'), ).toBe(true); expect(mockPrisma.user.create).toHaveBeenCalled(); + expect(mockPrisma.pendingPasskeyRegistration.create).toHaveBeenCalledWith({ + data: expect.objectContaining({ + userId: 1, + verificationToken: expect.any(String), + credentialId: expect.any(Buffer), + publicKey: expect.any(Buffer), + }), + }); }); it('should return the neutral response without changing an existing verified user', async () => { - const mockCredentialId = new Uint8Array([1, 2, 3, 4]); mockVerifyRegistration.mockResolvedValue({ verified: true, registrationInfo: { credential: { - id: mockCredentialId, + id: new Uint8Array([1, 2, 3, 4]), publicKey: new Uint8Array([5, 6, 7, 8]), counter: 0, }, @@ -314,7 +338,96 @@ describe('Passkey Authentication', () => { expect(sendVerificationEmail).not.toHaveBeenCalled(); }); - it('should return the neutral response when verification email delivery fails', async () => { + it('should bind a re-registration passkey to its own pending email link', async () => { + mockVerifyRegistration.mockResolvedValue({ + verified: true, + registrationInfo: { + credential: { + id: new Uint8Array([9, 10, 11, 12]), + publicKey: new Uint8Array([13, 14, 15, 16]), + counter: 0, + }, + }, + }); + mockPrisma.user.findUnique.mockResolvedValue({ + id: 1, + email: testEmail, + isVerified: 0, + verificationResendCount: 1, + }); + await generateRegistrationOptions(testEmail); + + const result = await verifyRegistration(testEmail, { + id: 'attacker-credential-id', + rawId: 'raw-id', + type: 'public-key', + response: { + clientDataJSON: 'client-data', + attestationObject: 'attestation', + }, + clientExtensionResults: {}, + } as RegistrationResponseJSON); + + expect(result).toEqual(registrationResponse); + expect(mockPrisma.user.updateMany).toHaveBeenCalledWith({ + where: { + id: 1, + isVerified: 0, + verificationResendCount: { lt: 20 }, + }, + data: { + verificationResendCount: { increment: 1 }, + }, + }); + expect(mockPrisma.pendingPasskeyRegistration.create).toHaveBeenCalledWith({ + data: expect.objectContaining({ + userId: 1, + verificationToken: expect.any(String), + credentialId: expect.any(Buffer), + }), + }); + expect(mockPrisma.passkey.create).not.toHaveBeenCalled(); + expect(sendVerificationEmail).toHaveBeenCalledOnce(); + }); + + it('should keep a failed-delivery credential pending and inactive', async () => { + mockVerifyRegistration.mockResolvedValue({ + verified: true, + registrationInfo: { + credential: { + id: new Uint8Array([9, 10, 11, 12]), + publicKey: new Uint8Array([13, 14, 15, 16]), + counter: 0, + }, + }, + }); + mockPrisma.user.findUnique.mockResolvedValue({ + id: 1, + email: testEmail, + isVerified: 0, + verificationResendCount: 1, + }); + (sendVerificationEmail as Mock).mockResolvedValueOnce(false); + await generateRegistrationOptions(testEmail); + + const result = await verifyRegistration(testEmail, { + id: 'attacker-credential-id', + rawId: 'raw-id', + type: 'public-key', + response: { + clientDataJSON: 'client-data', + attestationObject: 'attestation', + }, + clientExtensionResults: {}, + } as RegistrationResponseJSON); + + expect(result).toEqual(registrationResponse); + expect(mockPrisma.pendingPasskeyRegistration.create).toHaveBeenCalledOnce(); + expect(mockPrisma.passkey.create).not.toHaveBeenCalled(); + expect(mockPrisma.user.deleteMany).not.toHaveBeenCalled(); + }); + + it('should retain a new pending registration when email delivery fails', async () => { mockVerifyRegistration.mockResolvedValue({ verified: true, registrationInfo: { @@ -325,9 +438,7 @@ describe('Passkey Authentication', () => { }, }, }); - mockPrisma.user.findUnique - .mockResolvedValueOnce(null) - .mockResolvedValueOnce({ id: 1, email: testEmail, isVerified: 0 }); + mockPrisma.user.findUnique.mockResolvedValueOnce(null); mockPrisma.user.create.mockResolvedValue({ id: 1 }); (sendVerificationEmail as Mock).mockResolvedValueOnce(false); await generateRegistrationOptions(testEmail); @@ -344,7 +455,8 @@ describe('Passkey Authentication', () => { } as RegistrationResponseJSON); expect(result).toEqual(registrationResponse); - expect(mockPrisma.user.delete).toHaveBeenCalledWith({ where: { id: 1 } }); + expect(mockPrisma.pendingPasskeyRegistration.create).toHaveBeenCalledOnce(); + expect(mockPrisma.user.deleteMany).not.toHaveBeenCalled(); }); it('should reject verification with expired challenge', async () => { diff --git a/packages/super-sync-server/tests/setup.ts b/packages/super-sync-server/tests/setup.ts index 48c24cc9e1..d3548f983e 100644 --- a/packages/super-sync-server/tests/setup.ts +++ b/packages/super-sync-server/tests/setup.ts @@ -370,6 +370,7 @@ vi.mock('../src/auth', () => ({ .mockResolvedValue({ valid: true, userId: 1, email: 'test@test.com' }), VERIFICATION_TOKEN_EXPIRY_MS: 24 * 60 * 60 * 1000, MAX_VERIFICATION_RESEND_COUNT: 20, + verifyEmail: vi.fn().mockResolvedValue(true), })); // Reset test data before each test diff --git a/packages/super-sync-server/vitest.config.ts b/packages/super-sync-server/vitest.config.ts index 2df8c4d6bb..0b1601a7cd 100644 --- a/packages/super-sync-server/vitest.config.ts +++ b/packages/super-sync-server/vitest.config.ts @@ -22,6 +22,8 @@ export default defineConfig({ 'tests/integration/multi-client-sync.integration.spec.ts', 'tests/integration/snapshot-skip-optimization.integration.spec.ts', // Integration test that requires real PostgreSQL (run with vitest.integration.config.ts) + 'tests/integration/registration-races.integration.spec.ts', + 'tests/integration/clean-slate-atomicity-sql.integration.spec.ts', 'tests/integration/snapshot-vector-clock-sql.integration.spec.ts', 'tests/integration/conflict-detection-sql.integration.spec.ts', 'tests/integration/repair-causality.integration.spec.ts',