feat: add nonce to ensure generation and verify OTP is via same session (#518)

* feat: use nonce to ascertain that same device is verifying email

* test: update tests for new nonce implementation

* feat: add and use vfn identifier fn so we know which email

* feat: simplify and vaguelify error message when nonce doesn't exist

* feat: return false if timingSafeEqual throws

Author: karrui

apps/web/src/app/(public)/sign-in/_components/wizard/email/email-step.tsx

@@ -36,6 +36,12 @@ export const EmailStep = ({ onNext }: EmailStepProps) => {
     trpc.auth.email.login.mutationOptions({
       onSuccess: onNext,
       onError: (error) => setError('email', { message: error.message }),
+      trpc: {
+        context: {
+          // Need to set session data for nonce
+          skipStreaming: true,
+        },
+      },
     }),
   )
 

apps/web/src/app/(public)/sign-in/_components/wizard/email/verification-step.tsx

@@ -56,6 +56,12 @@ export const VerificationStep = () => {
   const resendOtpMutation = useMutation(
     trpc.auth.email.login.mutationOptions({
       onError: (error) => setError('token', { message: error.message }),
+      trpc: {
+        context: {
+          // Need to set session data for nonce
+          skipStreaming: true,
+        },
+      },
     }),
   )
 

apps/web/src/server/api/routers/auth/auth.email.router.ts

@@ -1,3 +1,5 @@
+import { randomUUID } from 'node:crypto'
+import { TRPCError } from '@trpc/server'
 import z from 'zod'
 
 import { emailLogin, emailVerifyOtp } from '~/server/modules/auth/auth.service'
@@ -19,19 +21,36 @@ export const emailAuthRouter = createTRPCRouter({
         otpPrefix: z.string().length(OTP_PREFIX_LENGTH),
       }),
     )
-    .mutation(async ({ input }) => {
-      const { email, otpPrefix } = await emailLogin(input.email)
+    .mutation(async ({ input, ctx }) => {
+      const nonce = randomUUID()
+      const { email, otpPrefix } = await emailLogin({
+        email: input.email,
+        nonce,
+      })
+
+      ctx.session.nonce = nonce
+      await ctx.session.save()
       return {
         email,
         otpPrefix,
       }
     }),
   verifyOtp: publicProcedure
     .input(emailVerifyOtpSchema)
-    .mutation(async ({ input, ctx }) => {
-      await emailVerifyOtp(input)
-      const user = await upsertUserAndAccountByEmail(input.email)
+    .mutation(async ({ input: { email, token }, ctx }) => {
+      const nonce = ctx.session.nonce
+      // Ensure nonce exists in session
+      if (!nonce) {
+        throw new TRPCError({
+          code: 'BAD_REQUEST',
+          message:
+            'Something went wrong. Please request for a new OTP before retrying.',
+        })
+      }
 
+      await emailVerifyOtp({ email, token, nonce })
+      const user = await upsertUserAndAccountByEmail(email)
+      ctx.session.nonce = undefined
       ctx.session.userId = user.id
       await ctx.session.save()
       return user

apps/web/src/server/modules/auth/__tests__/auth.service.spec.ts

@@ -8,7 +8,7 @@ import { db } from '@acme/db'
 
 import * as mailService from '../../mail/mail.service'
 import { emailLogin, emailVerifyOtp } from '../auth.service'
-import { createAuthToken } from '../auth.utils'
+import { createAuthToken, createVfnIdentifier } from '../auth.utils'
 
 const mockedMailService = mock(mailService)
 
@@ -20,18 +20,20 @@ describe('auth.service', () => {
   describe('emailLogin', () => {
     it('should create a verification token and send OTP email', async () => {
       const email = '[email protected]'
+      const nonce = 'test-nonce-123'
 
-      const result = await emailLogin(email)
+      const result = await emailLogin({ email, nonce })
 
       expect(result).toEqual({
         email,
         token: expect.any(String),
         otpPrefix: expect.any(String),
       })
 
-      // Verify token was created in database
+      // Verify token was created in database with vfnIdentifier
+      const vfnIdentifier = createVfnIdentifier({ email, nonce })
       const token = await db.verificationToken.findUnique({
-        where: { identifier: email },
+        where: { identifier: vfnIdentifier },
       })
       expect(token).toBeDefined()
       expect(mockedMailService.sendMail).toHaveBeenCalledWith({
@@ -41,165 +43,242 @@ describe('auth.service', () => {
       })
     })
 
-    it('should reset attempts when sending new OTP for existing user', async () => {
+    it('should reset attempts when sending new OTP for existing nonce', async () => {
       const email = '[email protected]'
+      const nonce = 'test-nonce-123'
 
       // First login
-      await emailLogin(email)
+      await emailLogin({ email, nonce })
 
+      const vfnIdentifier = createVfnIdentifier({ email, nonce })
       // Simulate failed attempts
       await db.verificationToken.update({
-        where: { identifier: email },
+        where: { identifier: vfnIdentifier },
         data: { attempts: 3 },
       })
 
       // Second login should reset attempts
-      await emailLogin(email)
+      await emailLogin({ email, nonce })
 
       const token = await db.verificationToken.findUnique({
-        where: { identifier: email },
+        where: { identifier: vfnIdentifier },
       })
       expect(token?.attempts).toBe(0)
     })
 
-    it('should update existing token instead of creating duplicate', async () => {
+    it('should update existing token instead of creating duplicate for same nonce', async () => {
       const email = '[email protected]'
+      const nonce = 'test-nonce-123'
 
-      await emailLogin(email)
-      await emailLogin(email)
+      await emailLogin({ email, nonce })
+      await emailLogin({ email, nonce })
 
+      const vfnIdentifier = createVfnIdentifier({ email, nonce })
       // Should only have one record
       const tokens = await db.verificationToken.findMany({
-        where: { identifier: email },
+        where: { identifier: vfnIdentifier },
       })
       expect(tokens).toHaveLength(1)
     })
+
+    it('should allow different nonces for same email', async () => {
+      const email = '[email protected]'
+      const nonce1 = 'test-nonce-1'
+      const nonce2 = 'test-nonce-2'
+
+      await emailLogin({ email, nonce: nonce1 })
+      await emailLogin({ email, nonce: nonce2 })
+
+      // Should have two records with different nonces
+      const vfnIdentifier1 = createVfnIdentifier({ email, nonce: nonce1 })
+      const vfnIdentifier2 = createVfnIdentifier({ email, nonce: nonce2 })
+      const token1 = await db.verificationToken.findUnique({
+        where: { identifier: vfnIdentifier1 },
+      })
+      const token2 = await db.verificationToken.findUnique({
+        where: { identifier: vfnIdentifier2 },
+      })
+
+      expect(token1).toBeDefined()
+      expect(token2).toBeDefined()
+      expect(token1?.token).not.toBe(token2?.token)
+    })
   })
 
   describe('emailVerifyOtp', () => {
     it('should successfully verify a valid OTP', async () => {
       const email = '[email protected]'
+      const nonce = 'test-nonce-123'
 
       // Create a verification token
-      const { token } = await emailLogin(email)
+      const { token } = await emailLogin({ email, nonce })
 
       // Should not throw
-      await expect(emailVerifyOtp({ email, token })).resolves.not.toThrow()
+      await expect(
+        emailVerifyOtp({ email, token, nonce }),
+      ).resolves.not.toThrow()
 
       // Token should be deleted after successful verification
+      const vfnIdentifier = createVfnIdentifier({ email, nonce })
       const verificationToken = await db.verificationToken.findUnique({
-        where: { identifier: email },
+        where: { identifier: vfnIdentifier },
       })
       expect(verificationToken).toBeNull()
     })
 
     it('should reject an invalid OTP', async () => {
       const email = '[email protected]'
+      const nonce = 'test-nonce-123'
       const token = 'WRONG6'
 
-      await emailLogin(email)
-      await expect(emailVerifyOtp({ email, token })).rejects.toThrow(
+      await emailLogin({ email, nonce })
+      await expect(emailVerifyOtp({ email, token, nonce })).rejects.toThrow(
         'Token is invalid or has expired',
       )
     })
 
     it('should reject an expired OTP', async () => {
       const email = '[email protected]'
+      const nonce = 'test-nonce-123'
 
-      const { token, hashedToken } = createAuthToken(email)
+      const { token, hashedToken } = createAuthToken({ email, nonce })
 
+      const vfnIdentifier = createVfnIdentifier({ email, nonce })
       // Create a verification token with an old issuedAt date
       const oldDate = add(new Date(), { seconds: -700 }) // 700 seconds ago (beyond 600s expiry)
       await db.verificationToken.create({
         data: {
-          identifier: email,
+          identifier: vfnIdentifier,
           token: hashedToken,
           issuedAt: oldDate,
         },
       })
 
-      await expect(emailVerifyOtp({ email, token })).rejects.toThrow(
+      await expect(emailVerifyOtp({ email, token, nonce })).rejects.toThrow(
         'Token is invalid or has expired',
       )
     })
 
     it('should increment attempts on each verification try', async () => {
       const email = '[email protected]'
+      const nonce = 'test-nonce-123'
       const token = 'WRONG6'
 
-      await emailLogin(email)
+      await emailLogin({ email, nonce })
 
+      const vfnIdentifier = createVfnIdentifier({ email, nonce })
       // First attempt
-      await expect(emailVerifyOtp({ email, token })).rejects.toThrow()
+      await expect(emailVerifyOtp({ email, token, nonce })).rejects.toThrow()
       let verificationToken = await db.verificationToken.findUnique({
-        where: { identifier: email },
+        where: { identifier: vfnIdentifier },
       })
       expect(verificationToken?.attempts).toBe(1)
 
       // Second attempt
-      await expect(emailVerifyOtp({ email, token })).rejects.toThrow()
+      await expect(emailVerifyOtp({ email, token, nonce })).rejects.toThrow()
       verificationToken = await db.verificationToken.findUnique({
-        where: { identifier: email },
+        where: { identifier: vfnIdentifier },
       })
       expect(verificationToken?.attempts).toBe(2)
     })
 
     it('should reject after too many failed attempts (>5)', async () => {
       const email = '[email protected]'
+      const nonce = 'test-nonce-123'
       const token = 'WRONG6'
 
-      await emailLogin(email)
+      await emailLogin({ email, nonce })
 
       // Make 5 failed attempts
       for (let i = 0; i < 5; i++) {
-        await expect(emailVerifyOtp({ email, token })).rejects.toThrow()
+        await expect(emailVerifyOtp({ email, token, nonce })).rejects.toThrow()
       }
 
       // 6th attempt should give TOO_MANY_REQUESTS
-      await expect(emailVerifyOtp({ email, token })).rejects.toThrow(
+      await expect(emailVerifyOtp({ email, token, nonce })).rejects.toThrow(
         'Wrong OTP was entered too many times',
       )
     })
 
-    it('should throw error for non-existent email', async () => {
-      const email = '[email protected]'
+    it('should throw error for non-existent nonce', async () => {
+      const email = '[email protected]'
+      const nonce = 'nonexistent-nonce'
       const token = '123456'
 
-      await expect(emailVerifyOtp({ email, token })).rejects.toThrow(
-        'Invalid login email',
+      await expect(emailVerifyOtp({ email, token, nonce })).rejects.toThrow(
+        'Invalid login email or missing nonce',
       )
     })
 
     it('should delete verification token after successful verification', async () => {
-      // Arrange
       const email = '[email protected]'
-      const { token } = await emailLogin(email)
+      const nonce = 'test-nonce-123'
+      const { token } = await emailLogin({ email, nonce })
 
-      // Act
-      await emailVerifyOtp({ email, token })
+      await emailVerifyOtp({ email, token, nonce })
 
-      // Assert
       // Token should be deleted
+      const vfnIdentifier = createVfnIdentifier({ email, nonce })
       const verificationToken = await db.verificationToken.findUnique({
-        where: { identifier: email },
+        where: { identifier: vfnIdentifier },
       })
       expect(verificationToken).toBeNull()
     })
 
     it('should prevent token reuse after successful verification', async () => {
-      // Arrange
       const email = '[email protected]'
-      const { token } = await emailLogin(email)
+      const nonce = 'test-nonce-123'
+      const { token } = await emailLogin({ email, nonce })
 
-      // Act
       // First verification succeeds
-      await expect(emailVerifyOtp({ email, token })).resolves.toBeDefined()
+      await expect(
+        emailVerifyOtp({ email, token, nonce }),
+      ).resolves.toBeDefined()
 
-      // Assert
       // Second verification with same token should fail
-      await expect(emailVerifyOtp({ email, token })).rejects.toThrow(
-        'Invalid login email',
+      await expect(emailVerifyOtp({ email, token, nonce })).rejects.toThrow(
+        'Invalid login email or missing nonce',
       )
     })
+
+    it('should not allow using token with wrong nonce', async () => {
+      const email = '[email protected]'
+      const nonce1 = 'test-nonce-1'
+      const nonce2 = 'test-nonce-2'
+
+      const { token } = await emailLogin({ email, nonce: nonce1 })
+
+      // Try to verify with wrong nonce
+      await expect(
+        emailVerifyOtp({ email, token, nonce: nonce2 }),
+      ).rejects.toThrow('Invalid login email or missing nonce')
+
+      // Original token should still exist
+      const vfnIdentifier1 = createVfnIdentifier({ email, nonce: nonce1 })
+      const verificationToken = await db.verificationToken.findUnique({
+        where: { identifier: vfnIdentifier1 },
+      })
+      expect(verificationToken).toBeDefined()
+    })
+
+    it('should ensure OTP is tied to specific session via nonce', async () => {
+      const email = '[email protected]'
+      const nonce1 = 'session-1-nonce'
+      const nonce2 = 'session-2-nonce'
+
+      // Two different sessions for same email
+      const { token: token1 } = await emailLogin({ email, nonce: nonce1 })
+      const { token: token2 } = await emailLogin({ email, nonce: nonce2 })
+
+      // Each token should only work with its own nonce
+      await expect(
+        emailVerifyOtp({ email, token: token1, nonce: nonce1 }),
+      ).resolves.toBeDefined()
+
+      // token2 with nonce2 should still work (not affected by token1 verification)
+      await expect(
+        emailVerifyOtp({ email, token: token2, nonce: nonce2 }),
+      ).resolves.toBeDefined()
+    })
   })
 })