diff --git a/apps/sim/app/api/chat/[identifier]/otp/route.test.ts b/apps/sim/app/api/chat/[identifier]/otp/route.test.ts index b882ed310ff..c7b4f5d0490 100644 --- a/apps/sim/app/api/chat/[identifier]/otp/route.test.ts +++ b/apps/sim/app/api/chat/[identifier]/otp/route.test.ts @@ -31,6 +31,7 @@ const { mockSetChatAuthCookie, mockGetStorageMethod, mockZodParse, + mockAfterResponse, } = vi.hoisted(() => { const mockRedisSet = vi.fn() const mockRedisGet = vi.fn() @@ -49,6 +50,7 @@ const { const mockSetChatAuthCookie = vi.fn() const mockGetStorageMethod = vi.fn() const mockZodParse = vi.fn() + const mockAfterResponse = vi.fn() return { mockRedisSet, @@ -62,6 +64,7 @@ const { mockSetChatAuthCookie, mockGetStorageMethod, mockZodParse, + mockAfterResponse, } }) @@ -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, })) @@ -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) | undefined + if (task) await task() + return response +} describe('Chat OTP API Route', () => { const mockEmail = 'test@example.com' @@ -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' }) @@ -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, @@ -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, @@ -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', { @@ -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, @@ -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', { @@ -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() }) @@ -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, diff --git a/apps/sim/app/api/chat/[identifier]/otp/route.ts b/apps/sim/app/api/chat/[identifier]/otp/route.ts index 0b778675ccd..8a3747c5ae8 100644 --- a/apps/sim/app/api/chat/[identifier]/otp/route.ts +++ b/apps/sim/app/api/chat/[identifier]/otp/route.ts @@ -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' @@ -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 @@ -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) diff --git a/apps/sim/app/api/files/public/[token]/otp/route.test.ts b/apps/sim/app/api/files/public/[token]/otp/route.test.ts index a30aecc4bd9..94429804c40 100644 --- a/apps/sim/app/api/files/public/[token]/otp/route.test.ts +++ b/apps/sim/app/api/files/public/[token]/otp/route.test.ts @@ -18,6 +18,7 @@ const { mockRenderOTPEmail, mockSendEmail, mockCheckRateLimitDirect, + mockAfterResponse, } = vi.hoisted(() => ({ mockResolveActiveShareByToken: vi.fn(), mockIsEmailAllowed: vi.fn(), @@ -31,6 +32,7 @@ const { mockRenderOTPEmail: vi.fn(), mockSendEmail: vi.fn(), mockCheckRateLimitDirect: vi.fn(), + mockAfterResponse: vi.fn(), })) vi.mock('@/lib/public-shares/share-manager', () => ({ @@ -62,8 +64,18 @@ vi.mock('@/lib/core/rate-limiter', () => ({ checkRateLimitDirect = mockCheckRateLimitDirect }, })) +vi.mock('@/lib/core/utils/after-response', () => ({ + afterResponse: mockAfterResponse, +})) + +import { PUT, POST as routePost } from '@/app/api/files/public/[token]/otp/route' -import { POST, PUT } from '@/app/api/files/public/[token]/otp/route' +const POST: typeof routePost = async (...args) => { + const response = await routePost(...args) + const task = mockAfterResponse.mock.calls.at(-1)?.[0] as (() => Promise) | undefined + if (task) await task() + return response +} const params = (token = 'tok_1') => ({ params: Promise.resolve({ token }) }) const post = (email: string, token = 'tok_1') => @@ -98,15 +110,33 @@ describe('POST /api/files/public/[token]/otp', () => { it('sends a code to an allow-listed email', async () => { const res = await POST(post('user@acme.com'), params()) expect(res.status).toBe(200) + expect(mockAfterResponse).toHaveBeenCalledTimes(1) expect(mockStoreOTP).toHaveBeenCalledWith('file', 'sh_1', 'user@acme.com', '123456') expect(mockSendEmail).toHaveBeenCalled() }) - it('rejects an email not on the allow-list with 403', async () => { + it('returns the generic acceptance response for an email not on the allow-list', async () => { + mockIsEmailAllowed.mockReturnValueOnce(false) + const res = await POST(post('user@evil.com'), params()) + expect(res.status).toBe(200) + await expect(res.json()).resolves.toEqual({ message: 'Verification code sent' }) + expect(mockCheckRateLimitDirect).toHaveBeenCalledTimes(1) + expect(mockStoreOTP).not.toHaveBeenCalled() + expect(mockSendEmail).not.toHaveBeenCalled() + }) + + it('does not consume a send bucket for a rejected email without a client IP', async () => { + requestUtilsMockFns.mockGetClientIp.mockReturnValueOnce(null) mockIsEmailAllowed.mockReturnValueOnce(false) + const res = await POST(post('user@evil.com'), params()) - expect(res.status).toBe(403) + + expect(res.status).toBe(200) + await expect(res.json()).resolves.toEqual({ message: 'Verification code sent' }) + expect(mockAfterResponse).toHaveBeenCalledTimes(1) + expect(mockCheckRateLimitDirect).not.toHaveBeenCalled() expect(mockStoreOTP).not.toHaveBeenCalled() + expect(mockSendEmail).not.toHaveBeenCalled() }) it('lowercases the email for allow-list matching and OTP storage', async () => { @@ -131,19 +161,42 @@ describe('POST /api/files/public/[token]/otp', () => { expect(res.headers.get('Retry-After')).toBe('1') }) - it('returns 429 when the share resource rate limit is exceeded', async () => { + it('returns the generic acceptance response when the share resource limit is exceeded', async () => { mockCheckRateLimitDirect .mockResolvedValueOnce({ allowed: true }) .mockResolvedValueOnce({ allowed: false, retryAfterMs: 1000 }) const res = await POST(post('user@acme.com'), params()) - expect(res.status).toBe(429) - expect(res.headers.get('Retry-After')).toBe('1') + expect(res.status).toBe(200) + await expect(res.json()).resolves.toEqual({ message: 'Verification code sent' }) expect(mockStoreOTP).not.toHaveBeenCalled() expect(mockSendEmail).not.toHaveBeenCalled() }) + it('returns the generic acceptance response when the email rate limit is exceeded', async () => { + mockCheckRateLimitDirect + .mockResolvedValueOnce({ allowed: true }) + .mockResolvedValueOnce({ allowed: true }) + .mockResolvedValueOnce({ allowed: false, retryAfterMs: 1000 }) + + const res = await POST(post('user@acme.com'), params()) + + expect(res.status).toBe(200) + await expect(res.json()).resolves.toEqual({ message: 'Verification code sent' }) + expect(mockStoreOTP).not.toHaveBeenCalled() + expect(mockSendEmail).not.toHaveBeenCalled() + }) + + it('returns the generic acceptance response when email delivery fails', async () => { + mockSendEmail.mockResolvedValueOnce({ success: false, message: 'Delivery failed' }) + + const res = await POST(post('user@acme.com'), params()) + + expect(res.status).toBe(200) + await expect(res.json()).resolves.toEqual({ message: 'Verification code sent' }) + }) + it('retains resource and email backstops when the client IP cannot be resolved', async () => { requestUtilsMockFns.mockGetClientIp.mockReturnValueOnce(null) diff --git a/apps/sim/app/api/files/public/[token]/otp/route.ts b/apps/sim/app/api/files/public/[token]/otp/route.ts index ba5ae83593b..86c871375e1 100644 --- a/apps/sim/app/api/files/public/[token]/otp/route.ts +++ b/apps/sim/app/api/files/public/[token]/otp/route.ts @@ -22,6 +22,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' @@ -49,6 +50,48 @@ function rateLimited(retryAfterMs: number | undefined, fallbackMs: number): Next return response } +function otpRequestAccepted(): NextResponse { + return NextResponse.json({ message: 'Verification code sent' }) +} + +async function deliverOtp(requestId: string, shareId: string, email: string): Promise { + const resourceRateLimit = await rateLimiter.checkRateLimitDirect( + `file-otp:resource:${shareId}`, + OTP_RESOURCE_RATE_LIMIT, + { failClosed: true } + ) + if (!resourceRateLimit.allowed) { + logger.warn(`[${requestId}] OTP resource rate limit exceeded for share ${shareId}`) + return + } + + const emailRateLimit = await rateLimiter.checkRateLimitDirect( + `file-otp:email:${shareId}:${email}`, + OTP_EMAIL_RATE_LIMIT, + { failClosed: true } + ) + if (!emailRateLimit.allowed) { + logger.warn(`[${requestId}] OTP email rate limit exceeded for ${email}`) + return + } + + const otp = generateOTP() + await storeOTP('file', shareId, email, otp) + + const emailHtml = await renderOTPEmail(otp, email, 'email-verification', SHARE_EMAIL_LABEL) + const emailResult = await sendEmail({ + to: email, + subject: getOtpSubject(SHARE_EMAIL_LABEL), + html: emailHtml, + }) + if (!emailResult.success) { + logger.error(`[${requestId}] Failed to send OTP email:`, emailResult.message) + return + } + + logger.info(`[${requestId}] OTP sent for share ${shareId}`) +} + /** * POST /api/files/public/[token]/otp * Sends a 6-digit verification code to an allow-listed email for an email-gated share. @@ -88,49 +131,13 @@ export const POST = withRouteHandler( { status: 400 } ) } + const emailAllowed = isEmailAllowed(email, shareAllowedEmails(resolved.share.allowedEmails)) - const resourceRateLimit = await rateLimiter.checkRateLimitDirect( - `file-otp:resource:${resolved.share.id}`, - OTP_RESOURCE_RATE_LIMIT, - { failClosed: true } - ) - if (!resourceRateLimit.allowed) { - logger.warn( - `[${requestId}] OTP resource rate limit exceeded for share ${resolved.share.id}` - ) - return rateLimited(resourceRateLimit.retryAfterMs, OTP_RESOURCE_RATE_LIMIT.refillIntervalMs) - } - - if (!isEmailAllowed(email, shareAllowedEmails(resolved.share.allowedEmails))) { - return NextResponse.json({ error: 'Email not authorized for this file' }, { status: 403 }) - } - - const emailRateLimit = await rateLimiter.checkRateLimitDirect( - `file-otp:email:${resolved.share.id}:${email}`, - OTP_EMAIL_RATE_LIMIT, - { failClosed: true } - ) - if (!emailRateLimit.allowed) { - logger.warn(`[${requestId}] OTP email rate limit exceeded for ${email}`) - return rateLimited(emailRateLimit.retryAfterMs, OTP_EMAIL_RATE_LIMIT.refillIntervalMs) - } - - const otp = generateOTP() - await storeOTP('file', resolved.share.id, email, otp) - - const emailHtml = await renderOTPEmail(otp, email, 'email-verification', SHARE_EMAIL_LABEL) - const emailResult = await sendEmail({ - to: email, - subject: getOtpSubject(SHARE_EMAIL_LABEL), - html: emailHtml, + afterResponse(async () => { + if (!emailAllowed) return + await deliverOtp(requestId, resolved.share.id, email) }) - if (!emailResult.success) { - logger.error(`[${requestId}] Failed to send OTP email:`, emailResult.message) - return NextResponse.json({ error: 'Failed to send verification email' }, { status: 500 }) - } - - logger.info(`[${requestId}] OTP sent for share ${resolved.share.id}`) - return NextResponse.json({ message: 'Verification code sent' }) + return otpRequestAccepted() } catch (error) { logger.error(`[${requestId}] Error processing OTP request:`, error) return NextResponse.json({ error: 'Failed to process request' }, { status: 500 }) diff --git a/apps/sim/lib/core/utils/after-response.ts b/apps/sim/lib/core/utils/after-response.ts new file mode 100644 index 00000000000..97dff377762 --- /dev/null +++ b/apps/sim/lib/core/utils/after-response.ts @@ -0,0 +1,5 @@ +import { after } from 'next/server' + +export function afterResponse(task: () => Promise): void { + after(task) +}