Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
80 changes: 80 additions & 0 deletions apps/sim/ee/sso/components/sso-form.test.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,80 @@
/**
* @vitest-environment jsdom
*/
import type { ReactNode } from 'react'
import { renderToString } from 'react-dom/server'
import { beforeEach, describe, expect, it, vi } from 'vitest'

const { mockUseSearchParams } = vi.hoisted(() => ({
mockUseSearchParams: vi.fn(),
}))

vi.mock('next/navigation', () => ({
useRouter: () => ({ push: vi.fn(), replace: vi.fn() }),
useSearchParams: mockUseSearchParams,
}))

vi.mock('next/link', () => ({
default: ({ href, children }: { href: string; children?: ReactNode }) => (
<a href={href}>{children}</a>
),
}))

vi.mock('@sim/emcn', () => ({
Button: ({ children }: { children?: ReactNode }) => <button type='button'>{children}</button>,
Input: () => <input />,
Label: ({ children }: { children?: ReactNode }) => <span>{children}</span>,
cn: (...values: unknown[]) => values.filter(Boolean).join(' '),
}))

vi.mock('@/lib/auth/auth-client', () => ({
client: { signIn: { sso: vi.fn() } },
}))

vi.mock('@/app/(auth)/components', () => ({
AuthSubmitButton: ({ children }: { children?: ReactNode }) => (
<button type='submit'>{children}</button>
),
}))

vi.mock('@/lib/core/config/env', () => ({
getEnv: () => 'true',
isFalsy: (value: unknown) => value === undefined || value === 'false',
}))

import SSOForm from '@/ee/sso/components/sso-form'

function renderFirstFrame(search: string): string {
mockUseSearchParams.mockReturnValue(new URLSearchParams(search))
return renderToString(<SSOForm />)
}

/**
* `renderToString` produces the markup of the first frame with no effects run,
* which is exactly the window in which a callback URL seeded from an effect is
* still the `/workspace` default.
*/
describe('SSOForm callback URL', () => {
beforeEach(() => {
mockUseSearchParams.mockReset()
})

it('carries a valid callbackUrl on the first rendered frame', () => {
const html = renderFirstFrame('callbackUrl=/workspace/abc/w/xyz')

expect(html).toContain(encodeURIComponent('/workspace/abc/w/xyz'))
})

it('falls back to /workspace when no callbackUrl is present', () => {
const html = renderFirstFrame('')

expect(html).toContain(`/login?callbackUrl=${encodeURIComponent('/workspace')}`)
})

it('rejects an off-origin callbackUrl and falls back to /workspace', () => {
const html = renderFirstFrame('callbackUrl=https://evil.example.com/steal')

expect(html).not.toContain('evil.example.com')
expect(html).toContain(`/login?callbackUrl=${encodeURIComponent('/workspace')}`)
})
})
26 changes: 16 additions & 10 deletions apps/sim/ee/sso/components/sso-form.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -36,21 +36,27 @@ export default function SSOForm() {
const [email, setEmail] = useState('')
const [emailErrors, setEmailErrors] = useState<string[]>([])
const [showEmailValidationError, setShowEmailValidationError] = useState(false)
const [callbackUrl, setCallbackUrl] = useState('/workspace')

const emailEnabled = !isFalsy(getEnv('NEXT_PUBLIC_EMAIL_PASSWORD_SIGNUP_ENABLED'))

/**
* Derived during render rather than seeded into state from an effect: the
* first painted frame otherwise carries the `/workspace` default, so the
* "Sign in with email" and "Sign up" links briefly point at the wrong
* destination on any deep link carrying `?callbackUrl=`.
*/
const callbackParam = searchParams?.get('callbackUrl') ?? null
const isCallbackValid = callbackParam !== null && validateCallbackUrl(callbackParam)
const callbackUrl = callbackParam !== null && isCallbackValid ? callbackParam : '/workspace'
Comment thread
waleedlatif1 marked this conversation as resolved.
Comment thread
waleedlatif1 marked this conversation as resolved.

useEffect(() => {
if (searchParams) {
const callback = searchParams.get('callbackUrl')
if (callback) {
if (validateCallbackUrl(callback)) {
setCallbackUrl(callback)
} else {
logger.warn('Invalid callback URL detected and blocked:', { url: callback })
}
}
if (callbackParam !== null && !isCallbackValid) {
logger.warn('Invalid callback URL detected and blocked:', { url: callbackParam })
}
}, [callbackParam, isCallbackValid])

useEffect(() => {
if (searchParams) {
const emailParam = searchParams.get('email')
if (emailParam) {
setEmail(emailParam)
Expand Down
21 changes: 19 additions & 2 deletions apps/sim/lib/core/security/input-validation.test.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { envFlagsMock, resetEnvFlagsMock } from '@sim/testing'
import { defaultMockEnv, envFlagsMock, resetEnvFlagsMock, resetEnvMock, setEnv } from '@sim/testing'
import { afterAll, afterEach, beforeEach, describe, expect, it } from 'vitest'
import {
validateAirtableId,
Expand Down Expand Up @@ -2136,6 +2136,7 @@ describe('validateCallbackUrl', () => {
})

afterEach(() => {
resetEnvMock()
if (originalWindow === undefined) {
;(globalThis as { window?: unknown }).window = undefined
} else {
Expand Down Expand Up @@ -2187,12 +2188,28 @@ describe('validateCallbackUrl', () => {
;(globalThis as { window?: unknown }).window = undefined
})

it('falls back to placeholder origin and still rejects cross-origin URLs', () => {
it('resolves against the configured app origin and still rejects cross-origin URLs', () => {
expect(validateCallbackUrl('/workspace')).toBe(true)
expect(validateCallbackUrl('//evil.com')).toBe(false)
expect(validateCallbackUrl('https://evil.com')).toBe(false)
expect(validateCallbackUrl('javascript:alert(1)')).toBe(false)
})

/**
* The server verdict has to match what the browser will decide once it
* hydrates, or a callback URL derived during render yields one destination
* in the SSR markup and another after hydration.
*/
it('accepts an absolute same-origin URL, matching the browser verdict', () => {
expect(validateCallbackUrl(`${defaultMockEnv.NEXT_PUBLIC_APP_URL}/workspace/abc`)).toBe(true)
})

it('stays fail-closed on absolute URLs when the app URL is unset', () => {
setEnv({ NEXT_PUBLIC_APP_URL: undefined })

expect(validateCallbackUrl(`${defaultMockEnv.NEXT_PUBLIC_APP_URL}/workspace/abc`)).toBe(false)
expect(validateCallbackUrl('/workspace')).toBe(true)
})
})
})

Expand Down
28 changes: 27 additions & 1 deletion apps/sim/lib/core/security/input-validation.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ import { createLogger } from '@sim/logger'
import { isLoopbackIp, isPrivateIp, unwrapIpv6Brackets } from '@sim/security/ssrf'
import * as ipaddr from 'ipaddr.js'
import { isHosted } from '@/lib/core/config/env-flags'
import { getBaseUrl } from '@/lib/core/utils/urls'

const logger = createLogger('InputValidation')

Expand Down Expand Up @@ -1234,6 +1235,31 @@ export function validatePaginationCursor(

const CALLBACK_URL_SERVER_BASE = 'https://callback-url-validator.invalid'

/**
* Origin a callback URL is resolved and compared against.
*
* The browser uses its own origin. Server-side there is no `window`, so it uses
* the deployment's configured origin — which is what the browser will compare
* against once it hydrates. Using a sentinel here instead made the server reject
* every absolute URL, including the same-origin ones this function documents as
* valid, so a component deriving a callback URL during render produced one
* destination in the SSR markup and a different one after hydration.
*
* Falls back to the sentinel when the app URL is unset or unparseable, which
* keeps the server fail-closed: every absolute URL is rejected, as before.
*/
function getCallbackValidationOrigin(): string {
if (typeof window !== 'undefined') {
return window.location.origin
}

try {
return new URL(getBaseUrl()).origin
} catch {
return CALLBACK_URL_SERVER_BASE
}
}

/**
* Validates a callback URL to prevent open redirect attacks.
*
Expand Down Expand Up @@ -1263,7 +1289,7 @@ export function validateCallbackUrl(url: string): boolean {
try {
if (typeof url !== 'string' || url.length === 0) return false

const base = typeof window === 'undefined' ? CALLBACK_URL_SERVER_BASE : window.location.origin
const base = getCallbackValidationOrigin()
const parsed = new URL(url, base)
return parsed.origin === base
} catch (error) {
Expand Down
11 changes: 8 additions & 3 deletions apps/sim/lib/core/utils/urls.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -56,15 +56,20 @@ describe('getBaseUrl', () => {
expect(getBaseUrl()).toBe('https://app.example.com')
})

it('falls back to the page origin instead of throwing when the injected env is missing', () => {
/**
* Never guesses from `window.location.origin`: an opaque origin (a sandboxed
* iframe) serializes to the truthy string `'null'`, which would silently
* produce `null/api/...` rather than surfacing the misconfiguration.
*/
it('throws in the browser rather than guessing from the page origin', () => {
setLocation('https://www.sim.ai/workspace/ws-1/w/wf-1')
expect(getBaseUrl()).toBe('https://www.sim.ai')
expect(() => getBaseUrl()).toThrow('NEXT_PUBLIC_APP_URL must be configured')
})

it('treats a whitespace-only NEXT_PUBLIC_APP_URL as unset', () => {
mockGetEnv.mockImplementation((key) => (key === 'NEXT_PUBLIC_APP_URL' ? ' ' : undefined))
setLocation('https://www.sim.ai/')
expect(getBaseUrl()).toBe('https://www.sim.ai')
expect(() => getBaseUrl()).toThrow('NEXT_PUBLIC_APP_URL must be configured')
})
})

Expand Down
29 changes: 12 additions & 17 deletions apps/sim/lib/core/utils/urls.ts
Original file line number Diff line number Diff line change
Expand Up @@ -25,31 +25,26 @@ function normalizeBaseUrl(url: string): string {
* Returns the base URL of the application from NEXT_PUBLIC_APP_URL
* This ensures webhooks, callbacks, and other integrations always use the correct public URL
*
* In the browser, falls back to the page's own origin when the injected env is
* unavailable. Client-side callers only ever want a URL back to the app they are
* already served from, so the origin is a correct answer — and a throw here
* during render tears down the whole page through the error boundary. Server-side
* callers (webhooks, callbacks, emails) have no origin to fall back to and must
* still fail loudly on a misconfigured deployment.
* Deliberately has no browser fallback to `window.location.origin`. The value is
* injected before hydration by `<PublicEnvScript>`, so an empty read means the
* deployment is misconfigured — and a same-origin guess would hide that. It also
* would not be safe to guess: an opaque origin (a sandboxed iframe, and `/chat/*`
* is embeddable) serializes to the string `'null'`, which is truthy and would
* silently produce `null/api/...` at every call site.
*
* @returns The base URL string (e.g., 'http://localhost:3000' or 'https://example.com')
* @throws Error if NEXT_PUBLIC_APP_URL is not configured and no browser origin exists
* @throws Error if NEXT_PUBLIC_APP_URL is not configured
*/
export function getBaseUrl(): string {
const baseUrl = getEnv('NEXT_PUBLIC_APP_URL')?.trim()

if (baseUrl) {
return normalizeBaseUrl(baseUrl)
}

const browserOrigin = getBrowserOrigin()
if (browserOrigin) {
return browserOrigin
if (!baseUrl) {
throw new Error(
'NEXT_PUBLIC_APP_URL must be configured for webhooks and callbacks to work correctly'
)
}

throw new Error(
'NEXT_PUBLIC_APP_URL must be configured for webhooks and callbacks to work correctly'
)
return normalizeBaseUrl(baseUrl)
}

/**
Expand Down
18 changes: 7 additions & 11 deletions packages/testing/src/mocks/urls.mock.ts
Original file line number Diff line number Diff line change
Expand Up @@ -26,18 +26,14 @@ function hasHttpProtocol(url: string): boolean {

function getBaseUrlImpl(): string {
const baseUrl = readEnv('NEXT_PUBLIC_APP_URL')?.trim()
if (baseUrl) {
// Mirrors the real module: protocol-less values get https:// under isProd.
const protocol = envFlagsMock.isProd ? 'https://' : 'http://'
return hasHttpProtocol(baseUrl) ? baseUrl : `${protocol}${baseUrl}`
if (!baseUrl) {
throw new Error(
'NEXT_PUBLIC_APP_URL must be configured for webhooks and callbacks to work correctly'
)
}
// Mirrors the real module: the browser falls back to its own origin, only
// server-side (no `window`) callers throw.
const browserOrigin = getBrowserOriginImpl()
if (browserOrigin) return browserOrigin
throw new Error(
'NEXT_PUBLIC_APP_URL must be configured for webhooks and callbacks to work correctly'
)
// Mirrors the real module: protocol-less values get https:// under isProd.
const protocol = envFlagsMock.isProd ? 'https://' : 'http://'
return hasHttpProtocol(baseUrl) ? baseUrl : `${protocol}${baseUrl}`
}

function getInternalApiBaseUrlImpl(): string {
Expand Down
Loading