Skip to content

Commit 331e9fc

Browse files
waleedlatif1claude
andcommitted
fix: harden chat/form deployment auth (OTP brute-force, CSPRNG, HMAC tokens)
- Add brute-force protection to OTP verification with attempt tracking (CWE-307) - Replace Math.random() with crypto.randomInt() for OTP generation (CWE-338) - Replace unsigned Base64 auth tokens with HMAC-SHA256 signed tokens (CWE-327) - Use shared isEmailAllowed utility in OTP route instead of inline duplicate - Simplify Redis OTP update to single KEEPTTL call Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
1 parent 86d7a20 commit 331e9fc

File tree

10 files changed

+349
-269
lines changed

10 files changed

+349
-269
lines changed

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

Lines changed: 123 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -10,11 +10,13 @@ const {
1010
mockRedisSet,
1111
mockRedisGet,
1212
mockRedisDel,
13+
mockRedisTtl,
1314
mockGetRedisClient,
1415
mockRedisClient,
1516
mockDbSelect,
1617
mockDbInsert,
1718
mockDbDelete,
19+
mockDbUpdate,
1820
mockSendEmail,
1921
mockRenderOTPEmail,
2022
mockAddCorsHeaders,
@@ -29,15 +31,18 @@ const {
2931
const mockRedisSet = vi.fn()
3032
const mockRedisGet = vi.fn()
3133
const mockRedisDel = vi.fn()
34+
const mockRedisTtl = vi.fn()
3235
const mockRedisClient = {
3336
set: mockRedisSet,
3437
get: mockRedisGet,
3538
del: mockRedisDel,
39+
ttl: mockRedisTtl,
3640
}
3741
const mockGetRedisClient = vi.fn()
3842
const mockDbSelect = vi.fn()
3943
const mockDbInsert = vi.fn()
4044
const mockDbDelete = vi.fn()
45+
const mockDbUpdate = vi.fn()
4146
const mockSendEmail = vi.fn()
4247
const mockRenderOTPEmail = vi.fn()
4348
const mockAddCorsHeaders = vi.fn()
@@ -53,11 +58,13 @@ const {
5358
mockRedisSet,
5459
mockRedisGet,
5560
mockRedisDel,
61+
mockRedisTtl,
5662
mockGetRedisClient,
5763
mockRedisClient,
5864
mockDbSelect,
5965
mockDbInsert,
6066
mockDbDelete,
67+
mockDbUpdate,
6168
mockSendEmail,
6269
mockRenderOTPEmail,
6370
mockAddCorsHeaders,
@@ -80,11 +87,13 @@ vi.mock('@sim/db', () => ({
8087
select: mockDbSelect,
8188
insert: mockDbInsert,
8289
delete: mockDbDelete,
90+
update: mockDbUpdate,
8391
transaction: vi.fn(async (callback: (tx: Record<string, unknown>) => unknown) => {
8492
return callback({
8593
select: mockDbSelect,
8694
insert: mockDbInsert,
8795
delete: mockDbDelete,
96+
update: mockDbUpdate,
8897
})
8998
}),
9099
},
@@ -126,12 +135,25 @@ vi.mock('@/lib/messaging/email/mailer', () => ({
126135
sendEmail: mockSendEmail,
127136
}))
128137

129-
vi.mock('@/components/emails/render-email', () => ({
138+
vi.mock('@/components/emails', () => ({
130139
renderOTPEmail: mockRenderOTPEmail,
131140
}))
132141

133-
vi.mock('@/app/api/chat/utils', () => ({
142+
vi.mock('@/lib/core/security/deployment', () => ({
134143
addCorsHeaders: mockAddCorsHeaders,
144+
isEmailAllowed: (email: string, allowedEmails: string[]) => {
145+
if (allowedEmails.includes(email)) return true
146+
const atIndex = email.indexOf('@')
147+
if (atIndex > 0) {
148+
const domain = email.substring(atIndex + 1)
149+
if (domain && allowedEmails.some((allowed: string) => allowed === `@${domain}`))
150+
return true
151+
}
152+
return false
153+
},
154+
}))
155+
156+
vi.mock('@/app/api/chat/utils', () => ({
135157
setChatAuthCookie: mockSetChatAuthCookie,
136158
}))
137159

@@ -209,6 +231,7 @@ describe('Chat OTP API Route', () => {
209231
mockRedisSet.mockResolvedValue('OK')
210232
mockRedisGet.mockResolvedValue(null)
211233
mockRedisDel.mockResolvedValue(1)
234+
mockRedisTtl.mockResolvedValue(600)
212235

213236
const createDbChain = (result: unknown) => ({
214237
from: vi.fn().mockReturnValue({
@@ -225,6 +248,11 @@ describe('Chat OTP API Route', () => {
225248
mockDbDelete.mockImplementation(() => ({
226249
where: vi.fn().mockResolvedValue(undefined),
227250
}))
251+
mockDbUpdate.mockImplementation(() => ({
252+
set: vi.fn().mockReturnValue({
253+
where: vi.fn().mockResolvedValue(undefined),
254+
}),
255+
}))
228256

229257
mockGetStorageMethod.mockReturnValue('redis')
230258

@@ -349,7 +377,7 @@ describe('Chat OTP API Route', () => {
349377
describe('PUT - Verify OTP (Redis path)', () => {
350378
beforeEach(() => {
351379
mockGetStorageMethod.mockReturnValue('redis')
352-
mockRedisGet.mockResolvedValue(mockOTP)
380+
mockRedisGet.mockResolvedValue(`${mockOTP}:0`)
353381
})
354382

355383
it('should retrieve OTP from Redis and verify successfully', async () => {
@@ -374,9 +402,7 @@ describe('Chat OTP API Route', () => {
374402
await PUT(request, { params: Promise.resolve({ identifier: mockIdentifier }) })
375403

376404
expect(mockRedisGet).toHaveBeenCalledWith(`otp:${mockEmail}:${mockChatId}`)
377-
378405
expect(mockRedisDel).toHaveBeenCalledWith(`otp:${mockEmail}:${mockChatId}`)
379-
380406
expect(mockDbSelect).toHaveBeenCalledTimes(1)
381407
})
382408
})
@@ -405,7 +431,7 @@ describe('Chat OTP API Route', () => {
405431
}
406432
return Promise.resolve([
407433
{
408-
value: mockOTP,
434+
value: `${mockOTP}:0`,
409435
expiresAt: new Date(Date.now() + 10 * 60 * 1000),
410436
},
411437
])
@@ -475,7 +501,7 @@ describe('Chat OTP API Route', () => {
475501
})
476502

477503
it('should delete OTP from Redis after verification', async () => {
478-
mockRedisGet.mockResolvedValue(mockOTP)
504+
mockRedisGet.mockResolvedValue(`${mockOTP}:0`)
479505

480506
mockDbSelect.mockImplementationOnce(() => ({
481507
from: vi.fn().mockReturnValue({
@@ -519,7 +545,7 @@ describe('Chat OTP API Route', () => {
519545
return Promise.resolve([{ id: mockChatId, authType: 'email' }])
520546
}
521547
return Promise.resolve([
522-
{ value: mockOTP, expiresAt: new Date(Date.now() + 10 * 60 * 1000) },
548+
{ value: `${mockOTP}:0`, expiresAt: new Date(Date.now() + 10 * 60 * 1000) },
523549
])
524550
}),
525551
}),
@@ -543,6 +569,95 @@ describe('Chat OTP API Route', () => {
543569
})
544570
})
545571

572+
describe('Brute-force protection', () => {
573+
beforeEach(() => {
574+
mockGetStorageMethod.mockReturnValue('redis')
575+
})
576+
577+
it('should update stored value with incremented attempts on wrong OTP', async () => {
578+
mockRedisGet.mockResolvedValue('654321:0')
579+
mockRedisTtl.mockResolvedValue(600)
580+
581+
mockDbSelect.mockImplementationOnce(() => ({
582+
from: vi.fn().mockReturnValue({
583+
where: vi.fn().mockReturnValue({
584+
limit: vi.fn().mockResolvedValue([{ id: mockChatId, authType: 'email' }]),
585+
}),
586+
}),
587+
}))
588+
589+
const request = new NextRequest('http://localhost:3000/api/chat/test/otp', {
590+
method: 'PUT',
591+
body: JSON.stringify({ email: mockEmail, otp: 'wrong1' }),
592+
})
593+
594+
await PUT(request, { params: Promise.resolve({ identifier: mockIdentifier }) })
595+
596+
expect(mockRedisSet).toHaveBeenCalledWith(
597+
`otp:${mockEmail}:${mockChatId}`,
598+
'654321:1',
599+
'KEEPTTL'
600+
)
601+
expect(mockCreateErrorResponse).toHaveBeenCalledWith('Invalid verification code', 400)
602+
})
603+
604+
it('should invalidate OTP and return 429 after 5 failed attempts', async () => {
605+
mockRedisGet.mockResolvedValue('654321:4')
606+
607+
mockDbSelect.mockImplementationOnce(() => ({
608+
from: vi.fn().mockReturnValue({
609+
where: vi.fn().mockReturnValue({
610+
limit: vi.fn().mockResolvedValue([{ id: mockChatId, authType: 'email' }]),
611+
}),
612+
}),
613+
}))
614+
615+
const request = new NextRequest('http://localhost:3000/api/chat/test/otp', {
616+
method: 'PUT',
617+
body: JSON.stringify({ email: mockEmail, otp: 'wrong5' }),
618+
})
619+
620+
await PUT(request, { params: Promise.resolve({ identifier: mockIdentifier }) })
621+
622+
expect(mockRedisDel).toHaveBeenCalledWith(`otp:${mockEmail}:${mockChatId}`)
623+
expect(mockCreateErrorResponse).toHaveBeenCalledWith(
624+
'Too many failed attempts. Please request a new code.',
625+
429
626+
)
627+
})
628+
629+
it('should store OTP with zero attempts on generation', async () => {
630+
mockDbSelect.mockImplementationOnce(() => ({
631+
from: vi.fn().mockReturnValue({
632+
where: vi.fn().mockReturnValue({
633+
limit: vi.fn().mockResolvedValue([
634+
{
635+
id: mockChatId,
636+
authType: 'email',
637+
allowedEmails: [mockEmail],
638+
title: 'Test Chat',
639+
},
640+
]),
641+
}),
642+
}),
643+
}))
644+
645+
const request = new NextRequest('http://localhost:3000/api/chat/test/otp', {
646+
method: 'POST',
647+
body: JSON.stringify({ email: mockEmail }),
648+
})
649+
650+
await POST(request, { params: Promise.resolve({ identifier: mockIdentifier }) })
651+
652+
expect(mockRedisSet).toHaveBeenCalledWith(
653+
`otp:${mockEmail}:${mockChatId}`,
654+
expect.stringMatching(/^\d{6}:0$/),
655+
'EX',
656+
900
657+
)
658+
})
659+
})
660+
546661
describe('Behavior consistency between Redis and Database', () => {
547662
it('should have same behavior for missing OTP in both storage methods', async () => {
548663
mockGetStorageMethod.mockReturnValue('redis')

0 commit comments

Comments
 (0)