diff --git a/apps/sim/app/api/chat/[identifier]/otp/route.integration.ts b/apps/sim/app/api/chat/[identifier]/otp/route.integration.ts new file mode 100644 index 00000000000..21534fd6cc1 --- /dev/null +++ b/apps/sim/app/api/chat/[identifier]/otp/route.integration.ts @@ -0,0 +1,193 @@ +import { db } from '@sim/db' +import { chat, user, verification, workflow, workspace } from '@sim/db/schema' +import { readTestRedisUrl } from '@sim/db/testing/test-infrastructure' +import { deleteWorkspaceFixture, insertWorkspaceFixture } from '@sim/db/testing/workspace-fixtures' +import { createRouteContext } from '@sim/testing/helpers/http' +import { createMockRequest } from '@sim/testing/mocks/request.mock' +import { generateId } from '@sim/utils/id' +import { eq, like } from 'drizzle-orm' +import Redis from 'ioredis' +import { afterAll, afterEach, beforeAll, beforeEach, describe, expect, it, vi } from 'vitest' + +const { storage, redisClient } = vi.hoisted(() => ({ + storage: { backend: 'database' as 'database' | 'redis' }, + redisClient: { current: undefined as Redis | undefined }, +})) +vi.mock('@/lib/core/storage', () => ({ getStorageMethod: () => storage.backend })) +vi.mock('@/lib/core/config/redis', async (importOriginal) => ({ + ...(await importOriginal()), + getRedisClient: () => redisClient.current, +})) + +import { MAX_OTP_ATTEMPTS, storeOTP } from '@/lib/core/security/otp' +import { PUT } from '@/app/api/chat/[identifier]/otp/route' + +const redisUrl = readTestRedisUrl() +const ownerId = generateId() +const workspaceId = generateId() +const workflowId = generateId() +const chatId = generateId() +const identifier = `otp-${chatId}` +const email = `${generateId()}@otp.invalid` +const CODE = '246810' +const NO_CODE = 'No verification code found, request a new one' +const INVALID = 'Invalid verification code' +const LOCKED = 'Too many failed attempts. Please request a new code.' + +async function verify(otp: string) { + const request = createMockRequest({ + method: 'PUT', + url: `/api/chat/${identifier}/otp`, + body: { email, otp }, + }) + const response = await PUT(request, createRouteContext({ identifier })) + const body = (await response.json()) as { error?: string; message?: string } + return { + status: response.status, + error: body.error ?? body.message, + cookie: response.headers.get('set-cookie'), + } +} + +/** Distinct wrong six-digit guesses that never equal {@link CODE}. */ +function wrongGuesses(count: number) { + return Array.from({ length: count }, (_, index) => String(100000 + index)) +} + +beforeAll(async () => { + await db.insert(user).values({ + id: ownerId, + name: 'OTP fixture', + email: `${ownerId}@otp.invalid`, + emailVerified: true, + createdAt: new Date(), + updatedAt: new Date(), + }) + await insertWorkspaceFixture(db, { + id: workspaceId, + name: 'OTP fixture', + ownerId, + billedAccountUserId: ownerId, + }) + await db.insert(workflow).values({ + id: workflowId, + userId: ownerId, + workspaceId, + name: 'OTP fixture', + lastSynced: new Date(), + createdAt: new Date(), + updatedAt: new Date(), + }) + await db.insert(chat).values({ + id: chatId, + workflowId, + userId: ownerId, + identifier, + title: 'OTP fixture', + authType: 'email', + allowedEmails: [email], + }) +}) + +afterAll(async () => { + await db.delete(verification).where(like(verification.identifier, `chat-otp:${chatId}:%`)) + await deleteWorkspaceFixture(db, eq(workspace.id, workspaceId)) + await db.delete(user).where(eq(user.id, ownerId)) + await redisClient.current?.quit() +}) + +describe.each(['database', 'redis'] as const)('%s chat OTP verification', (backend) => { + describe.runIf(backend === 'database' || Boolean(redisUrl))('real storage', () => { + beforeAll(async () => { + if (backend === 'redis' && redisUrl && !redisClient.current) { + redisClient.current = new Redis(redisUrl, { lazyConnect: true, maxRetriesPerRequest: 0 }) + await redisClient.current.connect() + } + }) + beforeEach(async () => { + storage.backend = backend + await storeOTP('chat', chatId, email, CODE) + }) + afterEach(async () => { + await db + .update(chat) + .set({ authType: 'email', allowedEmails: [email] }) + .where(eq(chat.id, chatId)) + }) + + it.each([ + { + rejection: 'the chat no longer uses email auth', + change: { authType: 'public' }, + response: { status: 400, error: 'This chat does not use email authentication' }, + }, + { + rejection: 'the email is no longer allowed', + change: { allowedEmails: ['someone-else@otp.invalid'] }, + response: { status: 403, error: 'Email not authorized' }, + }, + ])('rejects without spending the code when $rejection', async ({ change, response }) => { + await db.update(chat).set(change).where(eq(chat.id, chatId)) + for (let attempt = 0; attempt < MAX_OTP_ATTEMPTS; attempt++) { + expect(await verify(CODE)).toMatchObject({ ...response, cookie: null }) + } + await db + .update(chat) + .set({ authType: 'email', allowedEmails: [email] }) + .where(eq(chat.id, chatId)) + expect(await verify(CODE)).toMatchObject({ status: 200 }) + }) + + it('compares at most the attempt limit of guesses however many arrive concurrently', async () => { + const results = await Promise.all(wrongGuesses(30).map(verify)) + + const compared = results.filter((result) => result.error !== NO_CODE) + expect(compared).toHaveLength(MAX_OTP_ATTEMPTS) + expect(compared.filter((result) => result.error === INVALID)).toHaveLength( + MAX_OTP_ATTEMPTS - 1 + ) + expect(compared.filter((result) => result.error === LOCKED)).toHaveLength(1) + expect(await verify(CODE)).toMatchObject({ status: 400, error: NO_CODE, cookie: null }) + }) + + it('accepts the correct code on the last remaining attempt and mints the auth cookie', async () => { + for (const guess of wrongGuesses(MAX_OTP_ATTEMPTS - 1)) { + expect(await verify(guess)).toMatchObject({ status: 400, error: INVALID }) + } + const accepted = await verify(CODE) + expect(accepted.status).toBe(200) + expect(accepted.cookie).toContain(`chat_auth_${chatId}=`) + expect(await verify(CODE)).toMatchObject({ status: 400, error: NO_CODE }) + }) + + it('keeps the code expiry when a failed attempt is recorded', async () => { + if (backend === 'redis') { + const key = `otp:${email}:${chatId}` + await redisClient.current?.pexpire(key, 900) + expect(await verify('999999')).toMatchObject({ status: 400, error: INVALID }) + const ttl = await redisClient.current?.pttl(key) + expect(ttl).toBeGreaterThan(0) + expect(ttl).toBeLessThanOrEqual(900) + return + } + const readExpiry = async () => + ( + await db + .select({ expiresAt: verification.expiresAt }) + .from(verification) + .where(eq(verification.identifier, `chat-otp:${chatId}:${email}`)) + )[0]?.expiresAt.getTime() + const before = await readExpiry() + expect(await verify('999999')).toMatchObject({ status: 400, error: INVALID }) + expect(await readExpiry()).toBe(before) + }) + + it.runIf(backend === 'database')('rejects an expired code', async () => { + await db + .update(verification) + .set({ expiresAt: new Date(Date.now() - 1000) }) + .where(eq(verification.identifier, `chat-otp:${chatId}:${email}`)) + expect(await verify(CODE)).toMatchObject({ status: 400, error: NO_CODE, cookie: null }) + }) + }) +}) 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 7034b748776..f4a48f6e6cf 100644 --- a/apps/sim/app/api/chat/[identifier]/otp/route.test.ts +++ b/apps/sim/app/api/chat/[identifier]/otp/route.test.ts @@ -24,10 +24,6 @@ import { afterAll, beforeEach, describe, expect, it, vi } from 'vitest' const { mockRedisSet, - mockRedisGet, - mockRedisDel, - mockRedisTtl, - mockRedisEval, mockRedisClient, mockSetChatAuthCookie, mockIsEmailAllowed, @@ -36,16 +32,8 @@ const { mockAfterResponse, } = vi.hoisted(() => { const mockRedisSet = vi.fn() - const mockRedisGet = vi.fn() - const mockRedisDel = vi.fn() - const mockRedisTtl = vi.fn() - const mockRedisEval = vi.fn() const mockRedisClient = { set: mockRedisSet, - get: mockRedisGet, - del: mockRedisDel, - ttl: mockRedisTtl, - eval: mockRedisEval, } const mockSetChatAuthCookie = vi.fn() const mockIsEmailAllowed = vi.fn((email: string, allowedEmails: string[]) => { @@ -59,10 +47,6 @@ const { return { mockRedisSet, - mockRedisGet, - mockRedisDel, - mockRedisTtl, - mockRedisEval, mockRedisClient, mockSetChatAuthCookie, mockIsEmailAllowed, @@ -139,7 +123,7 @@ vi.mock('zod', () => { } }) -import { PUT, POST as routePost } from './route' +import { POST as routePost } from './route' const mockCheckRateLimitDirect = rateLimiterMockFns.mockCheckRateLimitDirect const { mockSendEmail } = emailMailerMockFns @@ -156,7 +140,6 @@ describe('Chat OTP API Route', () => { const mockEmail = 'test@example.com' const mockChatId = 'chat-123' const mockIdentifier = 'test-chat' - const mockOTP = '123456' /** Queues the chat-deployment row the route reads before touching OTP storage. */ const queueDeployment = (row: Record) => { @@ -183,9 +166,6 @@ describe('Chat OTP API Route', () => { mockGetRedisClient.mockReturnValue(mockRedisClient) mockRedisSet.mockResolvedValue('OK') - mockRedisGet.mockResolvedValue(null) - mockRedisDel.mockResolvedValue(1) - mockRedisTtl.mockResolvedValue(600) mockGetStorageMethod.mockReturnValue('redis') @@ -312,126 +292,4 @@ describe('Chat OTP API Route', () => { ) }) }) - - describe('PUT - Verify OTP (authType re-check)', () => { - beforeEach(() => { - mockGetStorageMethod.mockReturnValue('redis') - mockRedisGet.mockResolvedValue(`${mockOTP}:0`) - }) - - it('rejects verification when the chat has switched away from email auth', async () => { - queueDeployment({ - id: mockChatId, - authType: 'password', - password: 'encrypted-password', - }) - - const request = createMockRequest({ - method: 'PUT', - url: 'http://localhost:3000/api/chat/test/otp', - rawBody: JSON.stringify({ email: mockEmail, otp: mockOTP }), - }) - - await PUT(request, createRouteContext({ identifier: mockIdentifier })) - - expect(mockCreateCodedErrorResponse).toHaveBeenCalledWith( - 'This chat does not use email authentication', - 400 - ) - expect(mockRedisGet).not.toHaveBeenCalled() - expect(mockSetChatAuthCookie).not.toHaveBeenCalled() - }) - - it('rejects verification when the email is no longer allowed', async () => { - mockIsEmailAllowed.mockReturnValueOnce(false) - queueDeployment({ id: mockChatId, authType: 'email' }) - - const request = createMockRequest({ - method: 'PUT', - url: 'http://localhost:3000/api/chat/test/otp', - rawBody: JSON.stringify({ email: mockEmail, otp: mockOTP }), - }) - - await PUT(request, createRouteContext({ identifier: mockIdentifier })) - - expect(mockCreateCodedErrorResponse).toHaveBeenCalledWith('Email not authorized', 403) - expect(mockRedisGet).not.toHaveBeenCalled() - expect(mockSetChatAuthCookie).not.toHaveBeenCalled() - }) - }) - - describe('PUT - Verify OTP (Database path)', () => { - beforeEach(() => { - mockGetStorageMethod.mockReturnValue('database') - mockGetRedisClient.mockReturnValue(null) - }) - - it('should reject expired OTP from database', async () => { - queueDeployment({ id: mockChatId, authType: 'email' }) - queueTableRows(schemaMock.verification, []) - - const request = createMockRequest({ - method: 'PUT', - url: 'http://localhost:3000/api/chat/test/otp', - rawBody: JSON.stringify({ email: mockEmail, otp: mockOTP }), - }) - - await PUT(request, createRouteContext({ identifier: mockIdentifier })) - - expect(mockCreateCodedErrorResponse).toHaveBeenCalledWith( - 'No verification code found, request a new one', - 400 - ) - }) - }) - - describe('Brute-force protection', () => { - beforeEach(() => { - mockGetStorageMethod.mockReturnValue('redis') - }) - - it('should atomically increment attempts on wrong OTP', async () => { - mockRedisGet.mockResolvedValue('654321:0') - mockRedisEval.mockResolvedValue('654321:1') - - queueDeployment({ id: mockChatId, authType: 'email' }) - - const request = createMockRequest({ - method: 'PUT', - url: 'http://localhost:3000/api/chat/test/otp', - rawBody: JSON.stringify({ email: mockEmail, otp: 'wrong1' }), - }) - - await PUT(request, createRouteContext({ identifier: mockIdentifier })) - - expect(mockRedisEval).toHaveBeenCalledWith( - expect.any(String), - 1, - `otp:${mockEmail}:${mockChatId}`, - 5 - ) - expect(mockCreateCodedErrorResponse).toHaveBeenCalledWith('Invalid verification code', 400) - }) - - it('should invalidate OTP and return 429 after max failed attempts', async () => { - mockRedisGet.mockResolvedValue('654321:4') - mockRedisEval.mockResolvedValue('LOCKED') - - queueDeployment({ id: mockChatId, authType: 'email' }) - - const request = createMockRequest({ - method: 'PUT', - url: 'http://localhost:3000/api/chat/test/otp', - rawBody: JSON.stringify({ email: mockEmail, otp: 'wrong5' }), - }) - - await PUT(request, createRouteContext({ identifier: mockIdentifier })) - - expect(mockRedisEval).toHaveBeenCalled() - expect(mockCreateCodedErrorResponse).toHaveBeenCalledWith( - 'Too many failed attempts. Please request a new code.', - 429 - ) - }) - }) }) diff --git a/apps/sim/app/api/chat/[identifier]/otp/route.ts b/apps/sim/app/api/chat/[identifier]/otp/route.ts index 6651f285702..74d6144677f 100644 --- a/apps/sim/app/api/chat/[identifier]/otp/route.ts +++ b/apps/sim/app/api/chat/[identifier]/otp/route.ts @@ -10,16 +10,12 @@ import { getValidationErrorMessage, parseRequest } from '@/lib/api/server' import { RateLimiter } from '@/lib/core/rate-limiter' import { isEmailAllowed } from '@/lib/core/security/deployment' import { - decodeOTPValue, - deleteOTP, generateOTP, - getOTP, - incrementOTPAttempts, - MAX_OTP_ATTEMPTS, OTP_EMAIL_RATE_LIMIT, OTP_IP_RATE_LIMIT, OTP_RESOURCE_RATE_LIMIT, storeOTP, + verifyOTP, } from '@/lib/core/security/otp' import { afterResponse } from '@/lib/core/utils/after-response' import { generateRequestId, getClientIp } from '@/lib/core/utils/request' @@ -196,33 +192,18 @@ export const PUT = withRouteHandler( return createCodedErrorResponse('Email not authorized', 403) } - const storedValue = await getOTP('chat', deployment.id, email) - if (!storedValue) { + const verification = await verifyOTP('chat', deployment.id, email, otp) + if (verification === 'missing') { return createCodedErrorResponse('No verification code found, request a new one', 400) } - - const { otp: storedOTP, attempts } = decodeOTPValue(storedValue) - - if (attempts >= MAX_OTP_ATTEMPTS) { - await deleteOTP('chat', deployment.id, email) - logger.warn(`OTP already locked out for ${email}`) + if (verification === 'locked') { + logger.warn('OTP invalidated after max failed attempts', { chatId: deployment.id }) return createCodedErrorResponse('Too many failed attempts. Please request a new code.', 429) } - - if (storedOTP !== otp) { - const result = await incrementOTPAttempts('chat', deployment.id, email, storedValue) - if (result === 'locked') { - logger.warn(`OTP invalidated after max failed attempts for ${email}`) - return createCodedErrorResponse( - 'Too many failed attempts. Please request a new code.', - 429 - ) - } + if (verification === 'invalid') { return createCodedErrorResponse('Invalid verification code', 400) } - await deleteOTP('chat', deployment.id, email) - const response = createSuccessResponse({ id: deployment.id, title: deployment.title, diff --git a/apps/sim/lib/core/security/otp.ts b/apps/sim/lib/core/security/otp.ts index 47435216274..3041622c660 100644 --- a/apps/sim/lib/core/security/otp.ts +++ b/apps/sim/lib/core/security/otp.ts @@ -1,8 +1,9 @@ import { randomInt } from 'crypto' import { db } from '@sim/db' import { verification } from '@sim/db/schema' +import { safeCompare } from '@sim/security/compare' import { generateId } from '@sim/utils/id' -import { and, eq, gt } from 'drizzle-orm' +import { and, eq, gt, lt, sql } from 'drizzle-orm' import { getRedisClient } from '@/lib/core/config/redis' import type { TokenBucketConfig } from '@/lib/core/rate-limiter' import { getStorageMethod } from '@/lib/core/storage' @@ -70,7 +71,7 @@ function encodeOTPValue(otp: string, attempts: number): string { return `${otp}:${attempts}` } -export function decodeOTPValue(value: string): { otp: string; attempts: number } { +function decodeOTPValue(value: string): { otp: string; attempts: number } { const lastColon = value.lastIndexOf(':') if (lastColon === -1) return { otp: value, attempts: 0 } const attempts = Number.parseInt(value.slice(lastColon + 1), 10) @@ -115,41 +116,13 @@ export async function storeOTP( }) } -export async function getOTP( - kind: DeploymentKind, - deploymentId: string, - email: string -): Promise { - const keys = OTP_KEYS[kind] - const storageMethod = getStorageMethod() - - if (storageMethod === 'redis') { - const redis = getRedisClient() - if (!redis) throw new Error('Redis configured but client unavailable') - return redis.get(keys.redisKey(email, deploymentId)) - } - - const now = new Date() - const [record] = await db - .select({ value: verification.value }) - .from(verification) - .where( - and( - eq(verification.identifier, keys.dbIdentifier(email, deploymentId)), - gt(verification.expiresAt, now) - ) - ) - .limit(1) - - return record?.value ?? null -} - /** - * Lua script for atomic OTP attempt increment in Redis. - * Returns `'LOCKED'` if max attempts reached (key deleted), new encoded value - * otherwise, nil if key missing. + * Lua script that reserves one verification attempt in Redis. Returns the + * code with the reserved attempt number, or nil when no code exists. Rewrites + * keep the remaining lifetime to the millisecond; a code without one is + * treated as expired so it can never become permanent. */ -const ATOMIC_INCREMENT_SCRIPT = ` +const RESERVE_ATTEMPT_SCRIPT = ` local val = redis.call('GET', KEYS[1]) if not val then return nil end local colon = val:find(':([^:]*)$') @@ -162,31 +135,31 @@ else attempts = 0 end attempts = attempts + 1 +local newVal = otp .. ':' .. attempts if attempts >= tonumber(ARGV[1]) then redis.call('DEL', KEYS[1]) - return 'LOCKED' + return newVal end -local newVal = otp .. ':' .. attempts -local ttl = redis.call('TTL', KEYS[1]) -if ttl > 0 then - redis.call('SET', KEYS[1], newVal, 'EX', ttl) -else - redis.call('SET', KEYS[1], newVal) +local ttl = redis.call('PTTL', KEYS[1]) +if ttl <= 0 then + redis.call('DEL', KEYS[1]) + return nil end +redis.call('SET', KEYS[1], newVal, 'PX', ttl) return newVal ` /** - * Atomically increments an OTP's failed-attempt counter. Returns `'locked'` - * if the max-attempts threshold was reached (and the OTP was deleted), or - * `'incremented'` otherwise. The DB path uses optimistic locking with retry. + * Atomically spends one of the code's {@link MAX_OTP_ATTEMPTS} attempts and + * returns the stored code with its reserved attempt count, or `null` when no + * live code is left. The reservation that spends the last attempt also + * deletes the code. */ -export async function incrementOTPAttempts( +async function reserveOTPAttempt( kind: DeploymentKind, deploymentId: string, - email: string, - currentValue: string -): Promise<'locked' | 'incremented'> { + email: string +): Promise<{ otp: string; attempts: number } | null> { const keys = OTP_KEYS[kind] const storageMethod = getStorageMethod() @@ -194,53 +167,62 @@ export async function incrementOTPAttempts( const redis = getRedisClient() if (!redis) throw new Error('Redis configured but client unavailable') const key = keys.redisKey(email, deploymentId) - const result = await redis.eval(ATOMIC_INCREMENT_SCRIPT, 1, key, MAX_OTP_ATTEMPTS) - if (result === null || result === 'LOCKED') return 'locked' - return 'incremented' + const result = await redis.eval(RESERVE_ATTEMPT_SCRIPT, 1, key, MAX_OTP_ATTEMPTS) + return typeof result === 'string' ? decodeOTPValue(result) : null } const identifier = keys.dbIdentifier(email, deploymentId) - const MAX_RETRIES = 3 - let value = currentValue - - for (let attempt = 0; attempt < MAX_RETRIES; attempt++) { - const { otp, attempts } = decodeOTPValue(value) - const newAttempts = attempts + 1 - - if (newAttempts >= MAX_OTP_ATTEMPTS) { - await db.delete(verification).where(eq(verification.identifier, identifier)) - return 'locked' - } - - const newValue = encodeOTPValue(otp, newAttempts) - const updated = await db - .update(verification) - .set({ value: newValue, updatedAt: new Date() }) - .where(and(eq(verification.identifier, identifier), eq(verification.value, value))) - .returning({ id: verification.id }) - - if (updated.length > 0) return 'incremented' - - const fresh = await getOTP(kind, deploymentId, email) - if (!fresh) return 'locked' - value = fresh + // Pattern match, not split + cast: other `verification` rows must never fail a cast. + const attemptsSql = sql`substring(${verification.value} from ':([0-9]+)$')::int` + const [reserved] = await db + .update(verification) + .set({ + value: sql`substring(${verification.value} from '^(.*):[0-9]+$') || ':' || (${attemptsSql} + 1)`, + updatedAt: new Date(), + }) + .where( + and( + eq(verification.identifier, identifier), + gt(verification.expiresAt, new Date()), + lt(attemptsSql, MAX_OTP_ATTEMPTS) + ) + ) + .returning({ value: verification.value }) + + if (!reserved) return null + const decoded = decodeOTPValue(reserved.value) + if (decoded.attempts >= MAX_OTP_ATTEMPTS) { + await db + .delete(verification) + .where(and(eq(verification.identifier, identifier), eq(verification.value, reserved.value))) } - - /** - * Retry exhaustion under heavy DB-path contention: this request did not - * succeed in writing its own +1, so the stored count may not reflect it. - * Fail closed — invalidate the OTP rather than return `'incremented'` with - * a possibly-undercounted attempt total. - */ - await db.delete(verification).where(eq(verification.identifier, identifier)) - return 'locked' + return decoded } -export async function deleteOTP( +export type OTPVerification = 'valid' | 'invalid' | 'locked' | 'missing' + +/** + * Checks a submitted code and consumes it on a match. An attempt is reserved + * before the comparison, so at most {@link MAX_OTP_ATTEMPTS} guesses are ever + * compared against one code however many requests run concurrently. + * `'locked'` means this guess spent the last attempt and the code is gone. + */ +export async function verifyOTP( kind: DeploymentKind, deploymentId: string, - email: string -): Promise { + email: string, + code: string +): Promise { + const reserved = await reserveOTPAttempt(kind, deploymentId, email) + if (!reserved) return 'missing' + if (safeCompare(reserved.otp, code)) { + await deleteOTP(kind, deploymentId, email) + return 'valid' + } + return reserved.attempts >= MAX_OTP_ATTEMPTS ? 'locked' : 'invalid' +} + +async function deleteOTP(kind: DeploymentKind, deploymentId: string, email: string): Promise { const keys = OTP_KEYS[kind] const storageMethod = getStorageMethod() diff --git a/apps/sim/lib/projects/files/__integration__/content-sharing.integration.ts b/apps/sim/lib/projects/files/__integration__/content-sharing.integration.ts index 841573b0f67..3201a0c2af7 100644 --- a/apps/sim/lib/projects/files/__integration__/content-sharing.integration.ts +++ b/apps/sim/lib/projects/files/__integration__/content-sharing.integration.ts @@ -16,6 +16,7 @@ import { publicShare, subscription, user, + verification, workspace, workspaceFileSecretProvenance, workspaceFiles, @@ -574,7 +575,14 @@ describe('Public file shares against PostgreSQL and private storage', () => { await ( await requestPublicFileShareOtp({ token: share.token, email: 'outsider@shared.invalid' }) ).deliver() - expect(await otpStore.getOTP('file', share.id, 'outsider@shared.invalid')).toBeNull() + const storedCode = async (address: string) => { + const [row] = await db + .select({ value: verification.value }) + .from(verification) + .where(eq(verification.identifier, `file-otp:${share.id}:${address}`)) + return row?.value.split(':')[0] + } + expect(await storedCode('outsider@shared.invalid')).toBeUndefined() const failure = { authorized: false, status: 400, error: 'Invalid verification code' } expect( await verifyPublicFileShareOtp({ @@ -587,9 +595,8 @@ describe('Public file shares against PostgreSQL and private storage', () => { failure ) await (await requestPublicFileShareOtp({ token: share.token, email })).deliver() - const stored = await otpStore.getOTP('file', share.id, email) - if (!stored) throw new Error('Allowed email received no code') - const { otp } = otpStore.decodeOTPValue(stored) + const otp = await storedCode(email) + if (!otp) throw new Error('Allowed email received no code') expect( await verifyPublicFileShareOtp({ token: share.token, email, otp: '000000' }) ).toMatchObject({ authorized: false, status: 400 }) @@ -606,9 +613,9 @@ describe('Public file shares against PostgreSQL and private storage', () => { status: 400, }) await otpStore.storeOTP('file', share.id, email, '123456') - const get = otpStore.getOTP - const spy = vi.spyOn(otpStore, 'getOTP').mockImplementation(async (...args) => { - const result = await get(...args) + const verifyCode = otpStore.verifyOTP + const spy = vi.spyOn(otpStore, 'verifyOTP').mockImplementation(async (...args) => { + const result = await verifyCode(...args) await db .update(publicShare) .set({ allowedEmails: ['replacement@shared.invalid'] }) @@ -619,7 +626,7 @@ describe('Public file shares against PostgreSQL and private storage', () => { verifyPublicFileShareOtp({ token: share.token, email, otp: '123456' }) ).rejects.toMatchObject({ code: 'not_found' }) spy.mockRestore() - await otpStore.deleteOTP('file', share.id, email) + expect(await storedCode(email)).toBeUndefined() } ) diff --git a/apps/sim/lib/public-shares/application/credentials.ts b/apps/sim/lib/public-shares/application/credentials.ts index e8036c4e50e..27737f346c2 100644 --- a/apps/sim/lib/public-shares/application/credentials.ts +++ b/apps/sim/lib/public-shares/application/credentials.ts @@ -1,19 +1,14 @@ import { createLogger } from '@sim/logger' -import { safeCompare } from '@sim/security/compare' import { normalizeEmail } from '@sim/utils/string' import { getOtpSubject, renderOTPEmail } from '@/components/emails' import { RateLimiter, type TokenBucketConfig } from '@/lib/core/rate-limiter' import { createDeploymentAuthCookie, isEmailAllowed } from '@/lib/core/security/deployment' import { - decodeOTPValue, - deleteOTP, generateOTP, - getOTP, - incrementOTPAttempts, - MAX_OTP_ATTEMPTS, OTP_EMAIL_RATE_LIMIT, OTP_RESOURCE_RATE_LIMIT, storeOTP, + verifyOTP, } from '@/lib/core/security/otp' import { sendEmail } from '@/lib/messaging/email/mailer' import { @@ -114,19 +109,9 @@ export async function verifyPublicFileShareOtp({ isEmailAllowed(normalizedEmail, share.allowedEmails) ) if (!allowed) return OTP_FAILURE - const value = await getOTP('file', challenge.shareId, normalizedEmail) + const verification = await verifyOTP('file', challenge.shareId, normalizedEmail, otp) await challenge.withCurrentPolicy(() => undefined) - if (!value) return OTP_FAILURE - const stored = decodeOTPValue(value) - if (stored.attempts >= MAX_OTP_ATTEMPTS) { - await deleteOTP('file', challenge.shareId, normalizedEmail) - return OTP_FAILURE - } - if (!safeCompare(stored.otp, otp)) { - await incrementOTPAttempts('file', challenge.shareId, normalizedEmail, value) - return OTP_FAILURE - } - await deleteOTP('file', challenge.shareId, normalizedEmail) + if (verification !== 'valid') return OTP_FAILURE const resource = await challenge.withCurrentPolicy(({ share }) => share) const cookie = await createDeploymentAuthCookie({ cookiePrefix: 'file',