Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
25 commits
Select commit Hold shift + click to select a range
6f43fc9
fix: prevent auth bypass via user-controlled context query param in f…
waleedlatif1 Mar 26, 2026
be41fbc
fix: use randomized heredoc delimiter in SSH execute-script route
waleedlatif1 Mar 26, 2026
86d7a20
fix: escape workingDirectory in SSH execute-command route
waleedlatif1 Mar 26, 2026
331e9fc
fix: harden chat/form deployment auth (OTP brute-force, CSPRNG, HMAC …
waleedlatif1 Mar 26, 2026
dac7dda
fix: harden SSRF protections and input validation across API routes
waleedlatif1 Mar 26, 2026
c5ecc19
lint
waleedlatif1 Mar 26, 2026
35bc843
fix(file-serve): remove user-controlled context param from authentica…
waleedlatif1 Mar 26, 2026
dea9fbe
fix: handle legacy OTP format in decodeOTPValue for deploy-time compat
waleedlatif1 Mar 26, 2026
7e56894
fix(mcp): distinguish DNS resolution failures from SSRF policy blocks
waleedlatif1 Mar 26, 2026
16072b5
fix: make OTP attempt counting atomic to prevent TOCTOU race
waleedlatif1 Mar 26, 2026
44b8aba
fix: check attempt count before OTP comparison to prevent bypass
waleedlatif1 Mar 26, 2026
bf81938
fix: validate OIDC discovered endpoints against SSRF
waleedlatif1 Mar 26, 2026
002748f
fix: remove duplicate OIDC endpoint SSRF validation block
waleedlatif1 Mar 26, 2026
5493234
fix: validate OIDC discovered endpoints and pin DNS for 1Password Con…
waleedlatif1 Mar 26, 2026
994e711
lint
waleedlatif1 Mar 26, 2026
971888d
fix: replace KEEPTTL with TTL+EX for Redis <6.0 compat, add DB retry …
waleedlatif1 Mar 26, 2026
2f85b31
fix: address review feedback on OTP atomicity and 1Password fetch
waleedlatif1 Mar 26, 2026
1313265
fix: treat Lua nil return as locked when OTP key is missing
waleedlatif1 Mar 26, 2026
f1fd878
fix: handle Lua nil as locked OTP and add SSRF check to MCP env resol…
waleedlatif1 Mar 26, 2026
1cc6ed4
fix: narrow resolvedIP type guard instead of non-null assertion
waleedlatif1 Mar 26, 2026
3db061b
fix: bind auth tokens to deployment password for immediate revocation
waleedlatif1 Mar 26, 2026
78c0454
fix: bind auth tokens to deployment password and remove resolvedIP no…
waleedlatif1 Mar 26, 2026
b7bc591
fix: update test assertions for new encryptedPassword parameter
waleedlatif1 Mar 26, 2026
4790853
fix: format long lines in chat/form test assertions
waleedlatif1 Mar 26, 2026
33e6576
fix: pass encryptedPassword through OTP route cookie generation
waleedlatif1 Mar 26, 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
61 changes: 54 additions & 7 deletions apps/sim/app/api/auth/sso/register/route.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,10 @@ import { z } from 'zod'
import { auth, getSession } from '@/lib/auth'
import { hasSSOAccess } from '@/lib/billing'
import { env } from '@/lib/core/config/env'
import {
secureFetchWithPinnedIP,
validateUrlWithDNS,
} from '@/lib/core/security/input-validation.server'
import { REDACTED_MARKER } from '@/lib/core/security/redaction'

const logger = createLogger('SSORegisterRoute')
Expand Down Expand Up @@ -156,24 +160,66 @@ export async function POST(request: NextRequest) {
hasJwksEndpoint: !!oidcConfig.jwksEndpoint,
})

const discoveryResponse = await fetch(discoveryUrl, {
headers: { Accept: 'application/json' },
})
const urlValidation = await validateUrlWithDNS(discoveryUrl, 'OIDC discovery URL')
if (!urlValidation.isValid || !urlValidation.resolvedIP) {
logger.warn('OIDC discovery URL failed SSRF validation', {
discoveryUrl,
error: urlValidation.error,
})
return NextResponse.json(
{ error: urlValidation.error ?? 'SSRF validation failed' },
{ status: 400 }
)
}

const discoveryResponse = await secureFetchWithPinnedIP(
discoveryUrl,
urlValidation.resolvedIP,
{
headers: { Accept: 'application/json' },
}
)

if (!discoveryResponse.ok) {
logger.error('Failed to fetch OIDC discovery document', {
status: discoveryResponse.status,
statusText: discoveryResponse.statusText,
})
return NextResponse.json(
{
error: `Failed to fetch OIDC discovery document from ${discoveryUrl}. Status: ${discoveryResponse.status}. Provide all endpoints explicitly or verify the issuer URL.`,
error:
'Failed to fetch OIDC discovery document. Provide all endpoints explicitly or verify the issuer URL.',
},
{ status: 400 }
)
}

const discovery = await discoveryResponse.json()
const discovery = (await discoveryResponse.json()) as Record<string, unknown>

const discoveredEndpoints: Record<string, unknown> = {
authorization_endpoint: discovery.authorization_endpoint,
token_endpoint: discovery.token_endpoint,
userinfo_endpoint: discovery.userinfo_endpoint,
jwks_uri: discovery.jwks_uri,
}

for (const [key, value] of Object.entries(discoveredEndpoints)) {
if (typeof value === 'string') {
const endpointValidation = await validateUrlWithDNS(value, `OIDC ${key}`)
if (!endpointValidation.isValid) {
logger.warn('OIDC discovered endpoint failed SSRF validation', {
endpoint: key,
url: value,
error: endpointValidation.error,
})
return NextResponse.json(
{
error: `Discovered OIDC ${key} failed security validation: ${endpointValidation.error}`,
},
{ status: 400 }
)
}
}
}

oidcConfig.authorizationEndpoint =
oidcConfig.authorizationEndpoint || discovery.authorization_endpoint
Expand All @@ -196,7 +242,8 @@ export async function POST(request: NextRequest) {
})
return NextResponse.json(
{
error: `Failed to fetch OIDC discovery document from ${discoveryUrl}. Please verify the issuer URL is correct or provide all endpoints explicitly.`,
error:
'Failed to fetch OIDC discovery document. Please verify the issuer URL is correct or provide all endpoints explicitly.',
},
{ status: 400 }
)
Expand Down
136 changes: 128 additions & 8 deletions apps/sim/app/api/chat/[identifier]/otp/route.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,11 +10,14 @@ const {
mockRedisSet,
mockRedisGet,
mockRedisDel,
mockRedisTtl,
mockRedisEval,
mockGetRedisClient,
mockRedisClient,
mockDbSelect,
mockDbInsert,
mockDbDelete,
mockDbUpdate,
mockSendEmail,
mockRenderOTPEmail,
mockAddCorsHeaders,
Expand All @@ -29,15 +32,20 @@ const {
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 mockGetRedisClient = vi.fn()
const mockDbSelect = vi.fn()
const mockDbInsert = vi.fn()
const mockDbDelete = vi.fn()
const mockDbUpdate = vi.fn()
const mockSendEmail = vi.fn()
const mockRenderOTPEmail = vi.fn()
const mockAddCorsHeaders = vi.fn()
Expand All @@ -53,11 +61,14 @@ const {
mockRedisSet,
mockRedisGet,
mockRedisDel,
mockRedisTtl,
mockRedisEval,
mockGetRedisClient,
mockRedisClient,
mockDbSelect,
mockDbInsert,
mockDbDelete,
mockDbUpdate,
mockSendEmail,
mockRenderOTPEmail,
mockAddCorsHeaders,
Expand All @@ -80,11 +91,13 @@ vi.mock('@sim/db', () => ({
select: mockDbSelect,
insert: mockDbInsert,
delete: mockDbDelete,
update: mockDbUpdate,
transaction: vi.fn(async (callback: (tx: Record<string, unknown>) => unknown) => {
return callback({
select: mockDbSelect,
insert: mockDbInsert,
delete: mockDbDelete,
update: mockDbUpdate,
})
}),
},
Expand Down Expand Up @@ -126,12 +139,24 @@ vi.mock('@/lib/messaging/email/mailer', () => ({
sendEmail: mockSendEmail,
}))

vi.mock('@/components/emails/render-email', () => ({
vi.mock('@/components/emails', () => ({
renderOTPEmail: mockRenderOTPEmail,
}))

vi.mock('@/app/api/chat/utils', () => ({
vi.mock('@/lib/core/security/deployment', () => ({
addCorsHeaders: mockAddCorsHeaders,
isEmailAllowed: (email: string, allowedEmails: string[]) => {
if (allowedEmails.includes(email)) return true
const atIndex = email.indexOf('@')
if (atIndex > 0) {
const domain = email.substring(atIndex + 1)
if (domain && allowedEmails.some((allowed: string) => allowed === `@${domain}`)) return true
}
return false
},
}))

vi.mock('@/app/api/chat/utils', () => ({
setChatAuthCookie: mockSetChatAuthCookie,
}))

Expand Down Expand Up @@ -209,6 +234,7 @@ describe('Chat OTP API Route', () => {
mockRedisSet.mockResolvedValue('OK')
mockRedisGet.mockResolvedValue(null)
mockRedisDel.mockResolvedValue(1)
mockRedisTtl.mockResolvedValue(600)

const createDbChain = (result: unknown) => ({
from: vi.fn().mockReturnValue({
Expand All @@ -225,6 +251,11 @@ describe('Chat OTP API Route', () => {
mockDbDelete.mockImplementation(() => ({
where: vi.fn().mockResolvedValue(undefined),
}))
mockDbUpdate.mockImplementation(() => ({
set: vi.fn().mockReturnValue({
where: vi.fn().mockResolvedValue(undefined),
}),
}))

mockGetStorageMethod.mockReturnValue('redis')

Expand Down Expand Up @@ -349,7 +380,7 @@ describe('Chat OTP API Route', () => {
describe('PUT - Verify OTP (Redis path)', () => {
beforeEach(() => {
mockGetStorageMethod.mockReturnValue('redis')
mockRedisGet.mockResolvedValue(mockOTP)
mockRedisGet.mockResolvedValue(`${mockOTP}:0`)
})

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

expect(mockRedisGet).toHaveBeenCalledWith(`otp:${mockEmail}:${mockChatId}`)

expect(mockRedisDel).toHaveBeenCalledWith(`otp:${mockEmail}:${mockChatId}`)

expect(mockDbSelect).toHaveBeenCalledTimes(1)
})
})
Expand Down Expand Up @@ -405,7 +434,7 @@ describe('Chat OTP API Route', () => {
}
return Promise.resolve([
{
value: mockOTP,
value: `${mockOTP}:0`,
expiresAt: new Date(Date.now() + 10 * 60 * 1000),
},
])
Expand Down Expand Up @@ -475,7 +504,7 @@ describe('Chat OTP API Route', () => {
})

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

mockDbSelect.mockImplementationOnce(() => ({
from: vi.fn().mockReturnValue({
Expand Down Expand Up @@ -519,7 +548,7 @@ describe('Chat OTP API Route', () => {
return Promise.resolve([{ id: mockChatId, authType: 'email' }])
}
return Promise.resolve([
{ value: mockOTP, expiresAt: new Date(Date.now() + 10 * 60 * 1000) },
{ value: `${mockOTP}:0`, expiresAt: new Date(Date.now() + 10 * 60 * 1000) },
])
}),
}),
Expand All @@ -543,6 +572,97 @@ describe('Chat OTP API Route', () => {
})
})

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')

mockDbSelect.mockImplementationOnce(() => ({
from: vi.fn().mockReturnValue({
where: vi.fn().mockReturnValue({
limit: vi.fn().mockResolvedValue([{ id: mockChatId, authType: 'email' }]),
}),
}),
}))

const request = new NextRequest('http://localhost:3000/api/chat/test/otp', {
method: 'PUT',
body: JSON.stringify({ email: mockEmail, otp: 'wrong1' }),
})

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

expect(mockRedisEval).toHaveBeenCalledWith(
expect.any(String),
1,
`otp:${mockEmail}:${mockChatId}`,
5
)
expect(mockCreateErrorResponse).toHaveBeenCalledWith('Invalid verification code', 400)
})

it('should invalidate OTP and return 429 after max failed attempts', async () => {
mockRedisGet.mockResolvedValue('654321:4')
mockRedisEval.mockResolvedValue('LOCKED')

mockDbSelect.mockImplementationOnce(() => ({
from: vi.fn().mockReturnValue({
where: vi.fn().mockReturnValue({
limit: vi.fn().mockResolvedValue([{ id: mockChatId, authType: 'email' }]),
}),
}),
}))

const request = new NextRequest('http://localhost:3000/api/chat/test/otp', {
method: 'PUT',
body: JSON.stringify({ email: mockEmail, otp: 'wrong5' }),
})

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

expect(mockRedisEval).toHaveBeenCalled()
expect(mockCreateErrorResponse).toHaveBeenCalledWith(
'Too many failed attempts. Please request a new code.',
429
)
})

it('should store OTP with zero attempts on generation', async () => {
mockDbSelect.mockImplementationOnce(() => ({
from: vi.fn().mockReturnValue({
where: vi.fn().mockReturnValue({
limit: vi.fn().mockResolvedValue([
{
id: mockChatId,
authType: 'email',
allowedEmails: [mockEmail],
title: 'Test Chat',
},
]),
}),
}),
}))

const request = new NextRequest('http://localhost:3000/api/chat/test/otp', {
method: 'POST',
body: JSON.stringify({ email: mockEmail }),
})

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

expect(mockRedisSet).toHaveBeenCalledWith(
`otp:${mockEmail}:${mockChatId}`,
expect.stringMatching(/^\d{6}:0$/),
'EX',
900
)
})
})

describe('Behavior consistency between Redis and Database', () => {
it('should have same behavior for missing OTP in both storage methods', async () => {
mockGetStorageMethod.mockReturnValue('redis')
Expand Down
Loading
Loading