Skip to content

Commit fb97b27

Browse files
committed
fix(sso): derive the SSO callback URL during render
`callbackUrl` was seeded into `useState('/workspace')` and overwritten from a `useEffect` that read `searchParams`, so the first painted frame always carried the default. On any deep link with `?callbackUrl=`, the "Sign in with email" and "Sign up" links briefly pointed at `/workspace` instead of the requested destination, and a click landing in that window navigated to the wrong place. - derive `callbackUrl` from `searchParams` during render; the validation gate is unchanged, so an off-origin or malformed value still falls back to `/workspace` - keep the warning for a rejected value in an effect, now keyed on the param itself rather than the `searchParams` object, so it fires once per actual change instead of once per identity change - add first-frame tests via `renderToString`, which runs no effects and so pins exactly the window the old code got wrong
1 parent dd79515 commit fb97b27

2 files changed

Lines changed: 96 additions & 10 deletions

File tree

Lines changed: 80 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,80 @@
1+
/**
2+
* @vitest-environment jsdom
3+
*/
4+
import type { ReactNode } from 'react'
5+
import { renderToString } from 'react-dom/server'
6+
import { beforeEach, describe, expect, it, vi } from 'vitest'
7+
8+
const { mockUseSearchParams } = vi.hoisted(() => ({
9+
mockUseSearchParams: vi.fn(),
10+
}))
11+
12+
vi.mock('next/navigation', () => ({
13+
useRouter: () => ({ push: vi.fn(), replace: vi.fn() }),
14+
useSearchParams: mockUseSearchParams,
15+
}))
16+
17+
vi.mock('next/link', () => ({
18+
default: ({ href, children }: { href: string; children?: ReactNode }) => (
19+
<a href={href}>{children}</a>
20+
),
21+
}))
22+
23+
vi.mock('@sim/emcn', () => ({
24+
Button: ({ children }: { children?: ReactNode }) => <button type='button'>{children}</button>,
25+
Input: () => <input />,
26+
Label: ({ children }: { children?: ReactNode }) => <span>{children}</span>,
27+
cn: (...values: unknown[]) => values.filter(Boolean).join(' '),
28+
}))
29+
30+
vi.mock('@/lib/auth/auth-client', () => ({
31+
client: { signIn: { sso: vi.fn() } },
32+
}))
33+
34+
vi.mock('@/app/(auth)/components', () => ({
35+
AuthSubmitButton: ({ children }: { children?: ReactNode }) => (
36+
<button type='submit'>{children}</button>
37+
),
38+
}))
39+
40+
vi.mock('@/lib/core/config/env', () => ({
41+
getEnv: () => 'true',
42+
isFalsy: (value: unknown) => value === undefined || value === 'false',
43+
}))
44+
45+
import SSOForm from '@/ee/sso/components/sso-form'
46+
47+
function renderFirstFrame(search: string): string {
48+
mockUseSearchParams.mockReturnValue(new URLSearchParams(search))
49+
return renderToString(<SSOForm />)
50+
}
51+
52+
/**
53+
* `renderToString` produces the markup of the first frame with no effects run,
54+
* which is exactly the window in which a callback URL seeded from an effect is
55+
* still the `/workspace` default.
56+
*/
57+
describe('SSOForm callback URL', () => {
58+
beforeEach(() => {
59+
mockUseSearchParams.mockReset()
60+
})
61+
62+
it('carries a valid callbackUrl on the first rendered frame', () => {
63+
const html = renderFirstFrame('callbackUrl=/workspace/abc/w/xyz')
64+
65+
expect(html).toContain(encodeURIComponent('/workspace/abc/w/xyz'))
66+
})
67+
68+
it('falls back to /workspace when no callbackUrl is present', () => {
69+
const html = renderFirstFrame('')
70+
71+
expect(html).toContain(`/login?callbackUrl=${encodeURIComponent('/workspace')}`)
72+
})
73+
74+
it('rejects an off-origin callbackUrl and falls back to /workspace', () => {
75+
const html = renderFirstFrame('callbackUrl=https://evil.example.com/steal')
76+
77+
expect(html).not.toContain('evil.example.com')
78+
expect(html).toContain(`/login?callbackUrl=${encodeURIComponent('/workspace')}`)
79+
})
80+
})

apps/sim/ee/sso/components/sso-form.tsx

Lines changed: 16 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -36,21 +36,27 @@ export default function SSOForm() {
3636
const [email, setEmail] = useState('')
3737
const [emailErrors, setEmailErrors] = useState<string[]>([])
3838
const [showEmailValidationError, setShowEmailValidationError] = useState(false)
39-
const [callbackUrl, setCallbackUrl] = useState('/workspace')
4039

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

42+
/**
43+
* Derived during render rather than seeded into state from an effect: the
44+
* first painted frame otherwise carries the `/workspace` default, so the
45+
* "Sign in with email" and "Sign up" links briefly point at the wrong
46+
* destination on any deep link carrying `?callbackUrl=`.
47+
*/
48+
const callbackParam = searchParams?.get('callbackUrl') ?? null
49+
const isCallbackValid = callbackParam !== null && validateCallbackUrl(callbackParam)
50+
const callbackUrl = callbackParam !== null && isCallbackValid ? callbackParam : '/workspace'
51+
4352
useEffect(() => {
44-
if (searchParams) {
45-
const callback = searchParams.get('callbackUrl')
46-
if (callback) {
47-
if (validateCallbackUrl(callback)) {
48-
setCallbackUrl(callback)
49-
} else {
50-
logger.warn('Invalid callback URL detected and blocked:', { url: callback })
51-
}
52-
}
53+
if (callbackParam !== null && !isCallbackValid) {
54+
logger.warn('Invalid callback URL detected and blocked:', { url: callbackParam })
55+
}
56+
}, [callbackParam, isCallbackValid])
5357

58+
useEffect(() => {
59+
if (searchParams) {
5460
const emailParam = searchParams.get('email')
5561
if (emailParam) {
5662
setEmail(emailParam)

0 commit comments

Comments
 (0)