Skip to content

fix(security): isolate rejected OTP attempts #7008

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.

Sign up for GitHub

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
TheodoreSpeaks merged 3 commits into from
Aug 23, 2026
Merged

fix(security): isolate rejected OTP attempts #7008

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
0543de6
fix(security): isolate rejected OTP attempts
TheodoreSpeaks Aug 23, 2026
ed1b07d
fix(security): make OTP requests non-enumerating
TheodoreSpeaks Aug 23, 2026
abd36ff
fix(security): defer OTP delivery work
TheodoreSpeaks Aug 23, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
65 changes: 43 additions & 22 deletions apps/sim/app/api/chat/[identifier]/otp/route.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ const {
mockSetChatAuthCookie,
mockGetStorageMethod,
mockZodParse,
mockAfterResponse,
} = vi.hoisted(() => {
const mockRedisSet = vi.fn()
const mockRedisGet = vi.fn()
Expand All @@ -49,6 +50,7 @@ const {
const mockSetChatAuthCookie = vi.fn()
const mockGetStorageMethod = vi.fn()
const mockZodParse = vi.fn()
const mockAfterResponse = vi.fn()

return {
mockRedisSet,
Expand All @@ -62,6 +64,7 @@ const {
mockSetChatAuthCookie,
mockGetStorageMethod,
mockZodParse,
mockAfterResponse,
}
})

Expand All @@ -84,6 +87,10 @@ vi.mock('@/lib/core/rate-limiter', () => ({
},
}))

vi.mock('@/lib/core/utils/after-response', () => ({
afterResponse: mockAfterResponse,
}))

vi.mock('@/lib/messaging/email/mailer', () => ({
sendEmail: mockSendEmail,
}))
Expand Down Expand Up @@ -149,7 +156,14 @@ vi.mock('zod', () => {
}
})

import { POST, PUT } from './route'
import { PUT, POST as routePost } from './route'

const POST: typeof routePost = async (...args) => {
const response = await routePost(...args)
const task = mockAfterResponse.mock.calls.at(-1)?.[0] as (() => Promise<void>) | undefined
if (task) await task()
return response
}

describe('Chat OTP API Route', () => {
const mockEmail = 'test@example.com'
Expand Down Expand Up @@ -209,7 +223,6 @@ describe('Chat OTP API Route', () => {
remaining: 10,
resetAt: new Date(Date.now() + 60_000),
})

mockZodParse.mockImplementation((data: unknown) => data)

setEnv({ NEXT_PUBLIC_APP_URL: 'http://localhost:3000', NODE_ENV: 'test' })
Expand Down Expand Up @@ -252,6 +265,27 @@ describe('Chat OTP API Route', () => {
})

describe('POST - Rate limiting', () => {
it('returns the generic acceptance response for a rejected email without a client IP', async () => {
requestUtilsMockFns.mockGetClientIp.mockReturnValueOnce(null)
queueDeployment(emailDeployment)

const request = new NextRequest('http://localhost:3000/api/chat/test/otp', {
method: 'POST',
body: JSON.stringify({ email: 'not-allowed@example.com' }),
})

const response = await POST(request, {
params: Promise.resolve({ identifier: mockIdentifier }),
})

expect(response.status).toBe(200)
await expect(response.json()).resolves.toEqual({ message: 'Verification code sent' })
expect(mockAfterResponse).toHaveBeenCalledTimes(1)
expect(mockCheckRateLimitDirect).not.toHaveBeenCalled()
expect(mockRedisSet).not.toHaveBeenCalled()
expect(mockSendEmail).not.toHaveBeenCalled()
})

it('returns 429 with Retry-After when IP rate limit is exceeded', async () => {
mockCheckRateLimitDirect.mockResolvedValueOnce({
allowed: false,
Expand Down Expand Up @@ -282,7 +316,7 @@ describe('Chat OTP API Route', () => {
expect(dbChainMockFns.select).not.toHaveBeenCalled()
})

it('returns 429 with Retry-After when email rate limit is exceeded', async () => {
it('returns the generic acceptance response when the email rate limit is exceeded', async () => {
mockCheckRateLimitDirect
.mockResolvedValueOnce({
allowed: true,
Expand All @@ -301,13 +335,6 @@ describe('Chat OTP API Route', () => {
retryAfterMs: 900_000,
})

const headerSet = vi.fn()
mockCreateErrorResponse.mockImplementationOnce((message: string, status: number) => ({
json: () => Promise.resolve({ error: message }),
status,
headers: { set: headerSet },
}))

queueDeployment(emailDeployment)

const request = new NextRequest('http://localhost:3000/api/chat/test/otp', {
Expand All @@ -319,12 +346,12 @@ describe('Chat OTP API Route', () => {
params: Promise.resolve({ identifier: mockIdentifier }),
})

expect(response.status).toBe(429)
expect(headerSet).toHaveBeenCalledWith('Retry-After', '900')
expect(response.status).toBe(200)
await expect(response.json()).resolves.toEqual({ message: 'Verification code sent' })
expect(mockSendEmail).not.toHaveBeenCalled()
})

it('returns 429 with Retry-After when the chat resource rate limit is exceeded', async () => {
it('returns the generic acceptance response when the chat resource limit is exceeded', async () => {
mockCheckRateLimitDirect
.mockResolvedValueOnce({
allowed: true,
Expand All @@ -338,13 +365,6 @@ describe('Chat OTP API Route', () => {
retryAfterMs: 900_000,
})

const headerSet = vi.fn()
mockCreateErrorResponse.mockImplementationOnce((message: string, status: number) => ({
json: () => Promise.resolve({ error: message }),
status,
headers: { set: headerSet },
}))

queueDeployment(emailDeployment)

const request = new NextRequest('http://localhost:3000/api/chat/test/otp', {
Expand All @@ -356,8 +376,8 @@ describe('Chat OTP API Route', () => {
params: Promise.resolve({ identifier: mockIdentifier }),
})

expect(response.status).toBe(429)
expect(headerSet).toHaveBeenCalledWith('Retry-After', '900')
expect(response.status).toBe(200)
await expect(response.json()).resolves.toEqual({ message: 'Verification code sent' })
expect(mockSendEmail).not.toHaveBeenCalled()
})

Expand Down Expand Up @@ -396,6 +416,7 @@ describe('Chat OTP API Route', () => {

await POST(request, { params: Promise.resolve({ identifier: mockIdentifier }) })

expect(mockAfterResponse).toHaveBeenCalledTimes(1)
expect(mockCheckRateLimitDirect).toHaveBeenCalledTimes(2)
expect(mockCheckRateLimitDirect).toHaveBeenNthCalledWith(
1,
Expand Down
113 changes: 49 additions & 64 deletions apps/sim/app/api/chat/[identifier]/otp/route.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import {
OTP_RESOURCE_RATE_LIMIT,
storeOTP,
} from '@/lib/core/security/otp'
import { afterResponse } from '@/lib/core/utils/after-response'
import { generateRequestId, getClientIp } from '@/lib/core/utils/request'
import { withRouteHandler } from '@/lib/core/utils/with-route-handler'
import { sendEmail } from '@/lib/messaging/email/mailer'
Expand All @@ -30,6 +31,49 @@ const logger = createLogger('ChatOtpAPI')

const rateLimiter = new RateLimiter()

function otpRequestAccepted() {
return createSuccessResponse({ message: 'Verification code sent' })
}

async function deliverOtp(requestId: string, deploymentId: string, title: string, email: string) {
const resourceRateLimit = await rateLimiter.checkRateLimitDirect(
`chat-otp:resource:${deploymentId}`,
OTP_RESOURCE_RATE_LIMIT,
{ failClosed: true }
)
if (!resourceRateLimit.allowed) {
logger.warn(`[${requestId}] OTP resource rate limit exceeded for chat ${deploymentId}`)
return
}

const emailRateLimit = await rateLimiter.checkRateLimitDirect(
`chat-otp:email:${deploymentId}:${email.toLowerCase()}`,
OTP_EMAIL_RATE_LIMIT,
{ failClosed: true }
)
if (!emailRateLimit.allowed) {
logger.warn(`[${requestId}] OTP email rate limit exceeded for ${email} on chat ${deploymentId}`)
return
}

const otp = generateOTP()
await storeOTP('chat', deploymentId, email, otp)

const emailHtml = await renderOTPEmail(otp, email, 'email-verification', title)
const emailResult = await sendEmail({
to: email,
subject: getOtpSubject(title),
html: emailHtml,
})

if (!emailResult.success) {
logger.error(`[${requestId}] Failed to send OTP email:`, emailResult.message)
return
}

logger.info(`[${requestId}] OTP sent to ${email} for chat ${deploymentId}`)
}

export const POST = withRouteHandler(
async (request: NextRequest, context: { params: Promise<{ identifier: string }> }) => {
const { identifier } = await context.params
Expand Down Expand Up @@ -88,72 +132,13 @@ export const POST = withRouteHandler(
const allowedEmails: string[] = Array.isArray(deployment.allowedEmails)
? deployment.allowedEmails
: []
const emailAllowed = isEmailAllowed(email, allowedEmails)

const resourceRateLimit = await rateLimiter.checkRateLimitDirect(
`chat-otp:resource:${deployment.id}`,
OTP_RESOURCE_RATE_LIMIT,
{ failClosed: true }
)
if (!resourceRateLimit.allowed) {
logger.warn(`[${requestId}] OTP resource rate limit exceeded for chat ${deployment.id}`)
const retryAfter = Math.ceil(
(resourceRateLimit.retryAfterMs ?? OTP_RESOURCE_RATE_LIMIT.refillIntervalMs) / 1000
)
const response = createErrorResponse(
'Too many verification code requests. Please try again later.',
429
)
response.headers.set('Retry-After', String(retryAfter))
return response
}

if (!isEmailAllowed(email, allowedEmails)) {
return createErrorResponse('Email not authorized for this chat', 403)
}

const emailRateLimit = await rateLimiter.checkRateLimitDirect(
`chat-otp:email:${deployment.id}:${email.toLowerCase()}`,
OTP_EMAIL_RATE_LIMIT,
{ failClosed: true }
)
if (!emailRateLimit.allowed) {
logger.warn(
`[${requestId}] OTP email rate limit exceeded for ${email} on chat ${deployment.id}`
)
const retryAfter = Math.ceil(
(emailRateLimit.retryAfterMs ?? OTP_EMAIL_RATE_LIMIT.refillIntervalMs) / 1000
)
const response = createErrorResponse(
'Too many verification code requests. Please try again later.',
429
)
response.headers.set('Retry-After', String(retryAfter))
return response
}

const otp = generateOTP()
await storeOTP('chat', deployment.id, email, otp)

const emailHtml = await renderOTPEmail(
otp,
email,
'email-verification',
deployment.title || 'Chat'
)

const emailResult = await sendEmail({
to: email,
subject: getOtpSubject(deployment.title || 'Chat'),
html: emailHtml,
afterResponse(async () => {
if (!emailAllowed) return
await deliverOtp(requestId, deployment.id, deployment.title || 'Chat', email)
})

if (!emailResult.success) {
logger.error(`[${requestId}] Failed to send OTP email:`, emailResult.message)
return createErrorResponse('Failed to send verification email', 500)
}

logger.info(`[${requestId}] OTP sent to ${email} for chat ${deployment.id}`)
return createSuccessResponse({ message: 'Verification code sent' })
return otpRequestAccepted()
} catch (error) {
logger.error(`[${requestId}] Error processing OTP request:`, error)
return createErrorResponse('Failed to process request', 500)
Expand Down
Loading
Loading