Skip to content

Commit 5a003bb

Browse files
committed
test(otp): prove rejected verify requests never spend the code against real storage
Replace the mock-asserting authorization unit tests with integration cases: with email auth disabled or the address removed from the allow list, five requests carrying the correct code are refused without a cookie, and the code still works once the chat is restored.
1 parent 5c7a2ba commit 5a003bb

2 files changed

Lines changed: 31 additions & 54 deletions

File tree

‎apps/sim/app/api/chat/[identifier]/otp/route.integration.ts‎

Lines changed: 30 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@ import { createMockRequest } from '@sim/testing/mocks/request.mock'
77
import { generateId } from '@sim/utils/id'
88
import { eq, like } from 'drizzle-orm'
99
import Redis from 'ioredis'
10-
import { afterAll, beforeAll, beforeEach, describe, expect, it, vi } from 'vitest'
10+
import { afterAll, afterEach, beforeAll, beforeEach, describe, expect, it, vi } from 'vitest'
1111

1212
const { storage, redisClient } = vi.hoisted(() => ({
1313
storage: { backend: 'database' as 'database' | 'redis' },
@@ -108,6 +108,35 @@ describe.each(['database', 'redis'] as const)('%s chat OTP verification', (backe
108108
storage.backend = backend
109109
await storeOTP('chat', chatId, email, CODE)
110110
})
111+
afterEach(async () => {
112+
await db
113+
.update(chat)
114+
.set({ authType: 'email', allowedEmails: [email] })
115+
.where(eq(chat.id, chatId))
116+
})
117+
118+
it.each([
119+
{
120+
rejection: 'the chat no longer uses email auth',
121+
change: { authType: 'public' },
122+
response: { status: 400, error: 'This chat does not use email authentication' },
123+
},
124+
{
125+
rejection: 'the email is no longer allowed',
126+
change: { allowedEmails: ['someone-else@otp.invalid'] },
127+
response: { status: 403, error: 'Email not authorized' },
128+
},
129+
])('rejects without spending the code when $rejection', async ({ change, response }) => {
130+
await db.update(chat).set(change).where(eq(chat.id, chatId))
131+
for (let attempt = 0; attempt < MAX_OTP_ATTEMPTS; attempt++) {
132+
expect(await verify(CODE)).toMatchObject({ ...response, cookie: null })
133+
}
134+
await db
135+
.update(chat)
136+
.set({ authType: 'email', allowedEmails: [email] })
137+
.where(eq(chat.id, chatId))
138+
expect(await verify(CODE)).toMatchObject({ status: 200 })
139+
})
111140

112141
it('compares at most the attempt limit of guesses however many arrive concurrently', async () => {
113142
const results = await Promise.all(wrongGuesses(30).map(verify))

‎apps/sim/app/api/chat/[identifier]/otp/route.test.ts‎

Lines changed: 1 addition & 53 deletions
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,6 @@ import { afterAll, beforeEach, describe, expect, it, vi } from 'vitest'
2424

2525
const {
2626
mockRedisSet,
27-
mockRedisEval,
2827
mockRedisClient,
2928
mockSetChatAuthCookie,
3029
mockIsEmailAllowed,
@@ -33,10 +32,8 @@ const {
3332
mockAfterResponse,
3433
} = vi.hoisted(() => {
3534
const mockRedisSet = vi.fn()
36-
const mockRedisEval = vi.fn()
3735
const mockRedisClient = {
3836
set: mockRedisSet,
39-
eval: mockRedisEval,
4037
}
4138
const mockSetChatAuthCookie = vi.fn()
4239
const mockIsEmailAllowed = vi.fn((email: string, allowedEmails: string[]) => {
@@ -50,7 +47,6 @@ const {
5047

5148
return {
5249
mockRedisSet,
53-
mockRedisEval,
5450
mockRedisClient,
5551
mockSetChatAuthCookie,
5652
mockIsEmailAllowed,
@@ -127,7 +123,7 @@ vi.mock('zod', () => {
127123
}
128124
})
129125

130-
import { PUT, POST as routePost } from './route'
126+
import { POST as routePost } from './route'
131127

132128
const mockCheckRateLimitDirect = rateLimiterMockFns.mockCheckRateLimitDirect
133129
const { mockSendEmail } = emailMailerMockFns
@@ -144,7 +140,6 @@ describe('Chat OTP API Route', () => {
144140
const mockEmail = 'test@example.com'
145141
const mockChatId = 'chat-123'
146142
const mockIdentifier = 'test-chat'
147-
const mockOTP = '123456'
148143

149144
/** Queues the chat-deployment row the route reads before touching OTP storage. */
150145
const queueDeployment = (row: Record<string, unknown>) => {
@@ -297,51 +292,4 @@ describe('Chat OTP API Route', () => {
297292
)
298293
})
299294
})
300-
301-
describe('PUT - Verify OTP (authType re-check)', () => {
302-
beforeEach(() => {
303-
mockGetStorageMethod.mockReturnValue('redis')
304-
mockRedisEval.mockResolvedValue(`${mockOTP}:1`)
305-
})
306-
307-
it('rejects verification when the chat has switched away from email auth', async () => {
308-
queueDeployment({
309-
id: mockChatId,
310-
authType: 'password',
311-
password: 'encrypted-password',
312-
})
313-
314-
const request = createMockRequest({
315-
method: 'PUT',
316-
url: 'http://localhost:3000/api/chat/test/otp',
317-
rawBody: JSON.stringify({ email: mockEmail, otp: mockOTP }),
318-
})
319-
320-
await PUT(request, createRouteContext({ identifier: mockIdentifier }))
321-
322-
expect(mockCreateCodedErrorResponse).toHaveBeenCalledWith(
323-
'This chat does not use email authentication',
324-
400
325-
)
326-
expect(mockRedisEval).not.toHaveBeenCalled()
327-
expect(mockSetChatAuthCookie).not.toHaveBeenCalled()
328-
})
329-
330-
it('rejects verification when the email is no longer allowed', async () => {
331-
mockIsEmailAllowed.mockReturnValueOnce(false)
332-
queueDeployment({ id: mockChatId, authType: 'email' })
333-
334-
const request = createMockRequest({
335-
method: 'PUT',
336-
url: 'http://localhost:3000/api/chat/test/otp',
337-
rawBody: JSON.stringify({ email: mockEmail, otp: mockOTP }),
338-
})
339-
340-
await PUT(request, createRouteContext({ identifier: mockIdentifier }))
341-
342-
expect(mockCreateCodedErrorResponse).toHaveBeenCalledWith('Email not authorized', 403)
343-
expect(mockRedisEval).not.toHaveBeenCalled()
344-
expect(mockSetChatAuthCookie).not.toHaveBeenCalled()
345-
})
346-
})
347295
})

0 commit comments

Comments
 (0)