Skip to content

Commit f3ad775

Browse files
authored
fix(auth): added same-origin validation to forget password route, added confirmation for disable auth FF (#2447)
* fix(auth): added same-origin validation to forget password route, added confirmation for disable auth FF * ack PR comments
1 parent 78b7643 commit f3ad775

File tree

5 files changed

+98
-20
lines changed

5 files changed

+98
-20
lines changed

apps/sim/app/api/auth/forget-password/route.test.ts

Lines changed: 31 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,10 @@
66
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'
77
import { createMockRequest, setupAuthApiMocks } from '@/app/api/__test-utils__/utils'
88

9+
vi.mock('@/lib/core/utils/urls', () => ({
10+
getBaseUrl: vi.fn(() => 'https://app.example.com'),
11+
}))
12+
913
describe('Forget Password API Route', () => {
1014
beforeEach(() => {
1115
vi.resetModules()
@@ -15,7 +19,7 @@ describe('Forget Password API Route', () => {
1519
vi.clearAllMocks()
1620
})
1721

18-
it('should send password reset email successfully', async () => {
22+
it('should send password reset email successfully with same-origin redirectTo', async () => {
1923
setupAuthApiMocks({
2024
operations: {
2125
forgetPassword: { success: true },
@@ -24,7 +28,7 @@ describe('Forget Password API Route', () => {
2428

2529
const req = createMockRequest('POST', {
2630
email: 'test@example.com',
27-
redirectTo: 'https://example.com/reset',
31+
redirectTo: 'https://app.example.com/reset',
2832
})
2933

3034
const { POST } = await import('@/app/api/auth/forget-password/route')
@@ -39,12 +43,36 @@ describe('Forget Password API Route', () => {
3943
expect(auth.auth.api.forgetPassword).toHaveBeenCalledWith({
4044
body: {
4145
email: 'test@example.com',
42-
redirectTo: 'https://example.com/reset',
46+
redirectTo: 'https://app.example.com/reset',
4347
},
4448
method: 'POST',
4549
})
4650
})
4751

52+
it('should reject external redirectTo URL', async () => {
53+
setupAuthApiMocks({
54+
operations: {
55+
forgetPassword: { success: true },
56+
},
57+
})
58+
59+
const req = createMockRequest('POST', {
60+
email: 'test@example.com',
61+
redirectTo: 'https://evil.com/phishing',
62+
})
63+
64+
const { POST } = await import('@/app/api/auth/forget-password/route')
65+
66+
const response = await POST(req)
67+
const data = await response.json()
68+
69+
expect(response.status).toBe(400)
70+
expect(data.message).toBe('Redirect URL must be a valid same-origin URL')
71+
72+
const auth = await import('@/lib/auth')
73+
expect(auth.auth.api.forgetPassword).not.toHaveBeenCalled()
74+
})
75+
4876
it('should send password reset email without redirectTo', async () => {
4977
setupAuthApiMocks({
5078
operations: {

apps/sim/app/api/auth/forget-password/route.ts

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
import { type NextRequest, NextResponse } from 'next/server'
22
import { z } from 'zod'
33
import { auth } from '@/lib/auth'
4+
import { isSameOrigin } from '@/lib/core/utils/validation'
45
import { createLogger } from '@/lib/logs/console/logger'
56

67
export const dynamic = 'force-dynamic'
@@ -13,10 +14,15 @@ const forgetPasswordSchema = z.object({
1314
.email('Please provide a valid email address'),
1415
redirectTo: z
1516
.string()
16-
.url('Redirect URL must be a valid URL')
1717
.optional()
1818
.or(z.literal(''))
19-
.transform((val) => (val === '' ? undefined : val)),
19+
.transform((val) => (val === '' || val === undefined ? undefined : val))
20+
.refine(
21+
(val) => val === undefined || (z.string().url().safeParse(val).success && isSameOrigin(val)),
22+
{
23+
message: 'Redirect URL must be a valid same-origin URL',
24+
}
25+
),
2026
})
2127

2228
export async function POST(request: NextRequest) {

apps/sim/lib/core/config/feature-flags.ts

Lines changed: 21 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -37,8 +37,28 @@ export const isEmailVerificationEnabled = isTruthy(env.EMAIL_VERIFICATION_ENABLE
3737

3838
/**
3939
* Is authentication disabled (for self-hosted deployments behind private networks)
40+
* This flag is blocked when isHosted is true.
4041
*/
41-
export const isAuthDisabled = isTruthy(env.DISABLE_AUTH)
42+
export const isAuthDisabled = isTruthy(env.DISABLE_AUTH) && !isHosted
43+
44+
if (isTruthy(env.DISABLE_AUTH)) {
45+
import('@/lib/logs/console/logger')
46+
.then(({ createLogger }) => {
47+
const logger = createLogger('FeatureFlags')
48+
if (isHosted) {
49+
logger.error(
50+
'DISABLE_AUTH is set but ignored on hosted environment. Authentication remains enabled for security.'
51+
)
52+
} else {
53+
logger.warn(
54+
'DISABLE_AUTH is enabled. Authentication is bypassed and all requests use an anonymous session. Only use this in trusted private networks.'
55+
)
56+
}
57+
})
58+
.catch(() => {
59+
// Fallback during config compilation when logger is unavailable
60+
})
61+
}
4262

4363
/**
4464
* Is user registration disabled

apps/sim/lib/core/utils.test.ts

Lines changed: 19 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -31,20 +31,25 @@ vi.mock('crypto', () => ({
3131
}),
3232
}))
3333

34-
vi.mock('@/lib/core/config/env', () => ({
35-
env: {
36-
ENCRYPTION_KEY: '0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef',
37-
OPENAI_API_KEY_1: 'test-openai-key-1',
38-
OPENAI_API_KEY_2: 'test-openai-key-2',
39-
OPENAI_API_KEY_3: 'test-openai-key-3',
40-
ANTHROPIC_API_KEY_1: 'test-anthropic-key-1',
41-
ANTHROPIC_API_KEY_2: 'test-anthropic-key-2',
42-
ANTHROPIC_API_KEY_3: 'test-anthropic-key-3',
43-
GEMINI_API_KEY_1: 'test-gemini-key-1',
44-
GEMINI_API_KEY_2: 'test-gemini-key-2',
45-
GEMINI_API_KEY_3: 'test-gemini-key-3',
46-
},
47-
}))
34+
vi.mock('@/lib/core/config/env', async (importOriginal) => {
35+
const actual = await importOriginal<typeof import('@/lib/core/config/env')>()
36+
return {
37+
...actual,
38+
env: {
39+
...actual.env,
40+
ENCRYPTION_KEY: '0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef', // fake key for testing
41+
OPENAI_API_KEY_1: 'test-openai-key-1', // fake key for testing
42+
OPENAI_API_KEY_2: 'test-openai-key-2', // fake key for testing
43+
OPENAI_API_KEY_3: 'test-openai-key-3', // fake key for testing
44+
ANTHROPIC_API_KEY_1: 'test-anthropic-key-1', // fake key for testing
45+
ANTHROPIC_API_KEY_2: 'test-anthropic-key-2', // fake key for testing
46+
ANTHROPIC_API_KEY_3: 'test-anthropic-key-3', // fake key for testing
47+
GEMINI_API_KEY_1: 'test-gemini-key-1', // fake key for testing
48+
GEMINI_API_KEY_2: 'test-gemini-key-2', // fake key for testing
49+
GEMINI_API_KEY_3: 'test-gemini-key-3', // fake key for testing
50+
},
51+
}
52+
})
4853

4954
afterEach(() => {
5055
vi.clearAllMocks()

apps/sim/lib/core/utils/validation.ts

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,22 @@
1+
import { getBaseUrl } from './urls'
2+
3+
/**
4+
* Checks if a URL is same-origin with the application's base URL.
5+
* Used to prevent open redirect vulnerabilities.
6+
*
7+
* @param url - The URL to validate
8+
* @returns True if the URL is same-origin, false otherwise (secure default)
9+
*/
10+
export function isSameOrigin(url: string): boolean {
11+
try {
12+
const targetUrl = new URL(url)
13+
const appUrl = new URL(getBaseUrl())
14+
return targetUrl.origin === appUrl.origin
15+
} catch {
16+
return false
17+
}
18+
}
19+
120
/**
221
* Validates a name by removing any characters that could cause issues
322
* with variable references or node naming.

0 commit comments

Comments
 (0)