-
Notifications
You must be signed in to change notification settings - Fork 10
feat: add nonce to ensure generation and verify OTP is via same session #518
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
karrui
merged 5 commits into
main
from
feat/add-nonce-to-ensure-otp-and-verify-in-same-session
Nov 20, 2025
Merged
Changes from 3 commits
Commits
Show all changes
5 commits
Select commit
Hold shift + click to select a range
edee9e1
feat: use nonce to ascertain that same device is verifying email
karrui e8765f3
test: update tests for new nonce implementation
karrui e164c59
feat: add and use vfn identifier fn so we know which email
karrui 3273297
feat: simplify and vaguelify error message when nonce doesn't exist
karrui 0bd419d
feat: return false if timingSafeEqual throws
karrui File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -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() | ||
| }) | ||
| }) | ||
| }) | ||
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.