Skip to content

Commit aa56d58

Browse files
authored
fix: auth redirect query encoding (#332)
## Summary Fix auth redirect message handling so query parameters are encoded once via `URLSearchParams` instead of manually encoding/decoding values across server and client code. This updates the email callback redirect to use the shared `encodedRedirect()` helper for `message` redirects, preventing crafted `message` values from injecting sibling query parameters. ## Changes - Extend `encodedRedirect()` to support `message` redirects. - Let `URLSearchParams` handle redirect query encoding directly. - Replace raw email-callback `redirect(...?message=${message}...)` calls with `encodedRedirect('message', ...)`. - Remove unnecessary `decodeURIComponent(...)` calls from auth/account message rendering. - Add regression coverage for query parameter injection in `/api/auth/email-callback`. Closes #255
1 parent 940f6d7 commit aa56d58

12 files changed

Lines changed: 237 additions & 64 deletions

File tree

src/app/(auth)/auth/cli/page.tsx

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -177,7 +177,7 @@ export default async function CLIAuthPage({
177177
<div className="text-fg-tertiary mt-12 leading-8">
178178
<Suspense fallback={<div>Loading...</div>}>
179179
{error ? (
180-
<ErrorAlert message={decodeURIComponent(error)} />
180+
<ErrorAlert message={error} />
181181
) : (
182182
<div>Authorizing CLI...</div>
183183
)}

src/app/(auth)/sign-in/page.tsx

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -31,8 +31,8 @@ export default function Login() {
3131
const [message, setMessage] = useState<AuthMessage | undefined>(() => {
3232
const error = searchParams.get('error')
3333
const success = searchParams.get('success')
34-
if (error) return { error: decodeURIComponent(error) }
35-
if (success) return { success: decodeURIComponent(success) }
34+
if (error) return { error }
35+
if (success) return { success }
3636
return undefined
3737
})
3838

src/app/(auth)/sign-up/page.tsx

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -36,8 +36,8 @@ export default function SignUp() {
3636
const [message, setMessage] = useState<AuthMessage | undefined>(() => {
3737
const error = searchParams.get('error')
3838
const success = searchParams.get('success')
39-
if (error) return { error: decodeURIComponent(error) }
40-
if (success) return { success: decodeURIComponent(success) }
39+
if (error) return { error }
40+
if (success) return { success }
4141

4242
return undefined
4343
})

src/app/api/auth/email-callback/route.tsx

Lines changed: 8 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,3 @@
1-
import { redirect } from 'next/navigation'
21
import { PROTECTED_URLS } from '@/configs/urls'
32
import { createClient } from '@/core/shared/clients/supabase/server'
43
import { encodedRedirect } from '@/lib/utils/auth'
@@ -11,32 +10,30 @@ export async function GET(request: Request) {
1110

1211
const next = PROTECTED_URLS.ACCOUNT_SETTINGS
1312

14-
if (!code && message) {
13+
if (message) {
1514
// E-Mail updates can be validated on both e-mails. This case is for the first validation link press.
1615
// `message` should inform the user that he has to validate on the other e-mail address as well for successful update.
17-
redirect(`${next}?message=${message}&type=update_email`)
18-
}
19-
20-
if (!code && !message) {
21-
encodedRedirect('error', next, 'Invalid email verification link', {
16+
return encodedRedirect('message', next, message, {
2217
type: 'update_email',
2318
})
2419
}
2520

26-
if (message) {
27-
redirect(`${next}?message=${message}&type=update_email`)
21+
if (!code) {
22+
return encodedRedirect('error', next, 'Invalid email verification link', {
23+
type: 'update_email',
24+
})
2825
}
2926

3027
const supabase = await createClient()
3128
const { error } = await supabase.auth.exchangeCodeForSession(code!)
3229

3330
if (error) {
34-
encodedRedirect('error', next, 'Failed to update E-Mail', {
31+
return encodedRedirect('error', next, 'Failed to update E-Mail', {
3532
type: 'update_email',
3633
})
3734
}
3835

39-
encodedRedirect('success', next, 'E-Mail changed successfully', {
36+
return encodedRedirect('success', next, 'E-Mail changed successfully', {
4037
new_email: newEmail || '',
4138
type: 'update_email',
4239
})

src/app/api/auth/verify-otp/route.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -44,7 +44,7 @@ function buildRedirectUrl(
4444
*/
4545
function buildErrorRedirectUrl(origin: string, message: string): string {
4646
const url = new URL(origin + AUTH_URLS.SIGN_IN)
47-
url.searchParams.set('error', encodeURIComponent(message))
47+
url.searchParams.set('error', message)
4848
return url.toString()
4949
}
5050

src/core/server/actions/user-actions.ts

Lines changed: 13 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -49,6 +49,18 @@ export const updateUserAction = authActionClient
4949

5050
const origin = (await headers()).get('origin')
5151

52+
let emailRedirectTo: string | undefined
53+
54+
if (parsedInput.email) {
55+
if (!origin) {
56+
throw new Error('Missing origin header for email update redirect')
57+
}
58+
59+
const redirectUrl = new URL('/api/auth/email-callback', origin)
60+
redirectUrl.searchParams.set('new_email', parsedInput.email)
61+
emailRedirectTo = redirectUrl.toString()
62+
}
63+
5264
const { data: updateData, error } = await supabase.auth.updateUser(
5365
{
5466
email: parsedInput.email,
@@ -57,9 +69,7 @@ export const updateUserAction = authActionClient
5769
name: parsedInput.name,
5870
},
5971
},
60-
{
61-
emailRedirectTo: `${origin}/api/auth/email-callback?new_email=${parsedInput.email}`,
62-
}
72+
emailRedirectTo ? { emailRedirectTo } : undefined
6373
)
6474

6575
if (!error) {

src/features/auth/form-message.tsx

Lines changed: 3 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -30,25 +30,19 @@ export function AuthFormMessage({
3030
{'success' in message && (
3131
<Alert variant="success">
3232
<SuccessIcon className="h-4 w-4" />
33-
<AlertDescription>
34-
{decodeURIComponent(message.success!)}
35-
</AlertDescription>
33+
<AlertDescription>{message.success}</AlertDescription>
3634
</Alert>
3735
)}
3836
{'error' in message && (
3937
<Alert variant="error">
4038
<AlertIcon className="h-4 w-4" />
41-
<AlertDescription>
42-
{decodeURIComponent(message.error!)}
43-
</AlertDescription>
39+
<AlertDescription>{message.error}</AlertDescription>
4440
</Alert>
4541
)}
4642
{'message' in message && (
4743
<Alert variant="info">
4844
<InfoIcon className="h-4 w-4" />
49-
<AlertDescription>
50-
{decodeURIComponent(message.message!)}
51-
</AlertDescription>
45+
<AlertDescription>{message.message}</AlertDescription>
5246
</Alert>
5347
)}
5448
</motion.div>

src/features/dashboard/account/email-settings.tsx

Lines changed: 7 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -95,25 +95,21 @@ export function EmailSettings({ className }: EmailSettingsProps) {
9595
return
9696

9797
if (searchParams.get('type') === 'update_email') {
98-
if (searchParams.has('success')) {
99-
toast(
100-
defaultSuccessToast(decodeURIComponent(searchParams.get('success')!))
101-
)
98+
const success = searchParams.get('success')
99+
if (success !== null) {
100+
toast(defaultSuccessToast(success))
102101
return
103102
}
104103

105-
if (searchParams.has('message')) {
106-
toast(
107-
defaultSuccessToast(decodeURIComponent(searchParams.get('message')!))
108-
)
104+
const message = searchParams.get('message')
105+
if (message !== null) {
106+
toast(defaultSuccessToast(message))
109107
return
110108
}
111109

112110
toast(
113111
defaultErrorToast(
114-
decodeURIComponent(
115-
searchParams.get('error') ?? 'Failed to update e-mail.'
116-
)
112+
searchParams.get('error') ?? 'Failed to update e-mail.'
117113
)
118114
)
119115
}

src/lib/utils/auth.ts

Lines changed: 8 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -3,25 +3,23 @@ import { redirect } from 'next/navigation'
33

44
/**
55
* Redirects to a specified path with an encoded message as a query parameter.
6-
* @param {('error' | 'success')} type - The type of message, either 'error' or 'success'.
6+
* @param {('error' | 'success' | 'message')} type - The type of message.
77
* @param {string} path - The path to redirect to.
8-
* @param {string} message - The message to be encoded and added as a query parameter.
8+
* @param {string} message - The message to be added as a query parameter.
99
* @param {Record<string, string>} queryParams - Additional query parameters to be added to the redirect URL.
1010
* @returns {never} This function doesn't return as it triggers a redirect.
1111
*/
1212
export function encodedRedirect(
13-
type: 'error' | 'success',
13+
type: 'error' | 'success' | 'message',
1414
path: string,
1515
message: string,
1616
queryParams?: Record<string, string>
1717
) {
18-
const queryString = new URLSearchParams()
19-
queryString.set(type, encodeURIComponent(message))
20-
if (queryParams) {
21-
Object.entries(queryParams).forEach(([key, value]) => {
22-
queryString.set(key, value)
23-
})
24-
}
18+
const queryString = new URLSearchParams({
19+
[type]: message,
20+
...(queryParams ?? {}),
21+
})
22+
2523
return redirect(`${path}?${queryString.toString()}`)
2624
}
2725

tests/integration/auth-callback-route.test.ts

Lines changed: 12 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -19,11 +19,15 @@ vi.mock('@/core/shared/clients/supabase/server', () => ({
1919
}))
2020

2121
vi.mock('@/lib/utils/auth', () => ({
22-
encodedRedirect: vi.fn((type, url, message) => ({
23-
type,
24-
destination: `${url}?${type}=${encodeURIComponent(message)}`,
25-
message,
26-
})),
22+
encodedRedirect: vi.fn((type, url, message) => {
23+
const params = new URLSearchParams({ [type]: message })
24+
25+
return {
26+
type,
27+
destination: `${url}?${params.toString()}`,
28+
message,
29+
}
30+
}),
2731
}))
2832

2933
import { GET } from '@/app/api/auth/callback/route'
@@ -63,7 +67,9 @@ describe('Auth Callback Route', () => {
6367
GET(new Request('https://dashboard.e2b.dev/api/auth/callback?code=test'))
6468
).rejects.toEqual({
6569
type: 'error',
66-
destination: `${AUTH_URLS.SIGN_IN}?error=${encodeURIComponent('Missing session after auth callback')}`,
70+
destination: `${AUTH_URLS.SIGN_IN}?${new URLSearchParams({
71+
error: 'Missing session after auth callback',
72+
}).toString()}`,
6773
message: 'Missing session after auth callback',
6874
})
6975
})

0 commit comments

Comments
 (0)