Skip to content

Commit 89eb886

Browse files
authored
Preserve and sanitize return path after login (#1959)
* Preserve and sanitize return path after login Adds logic to capture, sanitize, and persist a 'next' return path through the OAuth login flow, ensuring users are redirected to their intended in-app location after authentication. Prevents open redirects by only allowing absolute in-app paths and updates both the auth and login callback logic to handle the new parameter. * Update ChatMessage.svelte
1 parent f3087bf commit 89eb886

File tree

2 files changed

+55
-5
lines changed

2 files changed

+55
-5
lines changed

src/lib/server/auth.ts

Lines changed: 49 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -66,6 +66,19 @@ const secure = z
6666
.default(!(dev || config.ALLOW_INSECURE_COOKIES === "true"))
6767
.parse(config.COOKIE_SECURE === "" ? undefined : config.COOKIE_SECURE === "true");
6868

69+
function sanitizeReturnPath(path: string | undefined | null): string | undefined {
70+
if (!path) {
71+
return undefined;
72+
}
73+
if (path.startsWith("//")) {
74+
return undefined;
75+
}
76+
if (!path.startsWith("/")) {
77+
return undefined;
78+
}
79+
return path;
80+
}
81+
6982
export function refreshSessionCookie(cookies: Cookies, sessionId: string) {
7083
cookies.set(config.COOKIE_NAME, sessionId, {
7184
path: "/",
@@ -197,10 +210,20 @@ export function tokenSetToSessionOauth(tokenSet: TokenSet): Session["oauth"] {
197210
/**
198211
* Generates a CSRF token using the user sessionId. Note that we don't need a secret because sessionId is enough.
199212
*/
200-
export async function generateCsrfToken(sessionId: string, redirectUrl: string): Promise<string> {
213+
export async function generateCsrfToken(
214+
sessionId: string,
215+
redirectUrl: string,
216+
next?: string
217+
): Promise<string> {
218+
const sanitizedNext = sanitizeReturnPath(next);
201219
const data = {
202220
expiration: addHours(new Date(), 1).getTime(),
203221
redirectUrl,
222+
...(sanitizedNext ? { next: sanitizedNext } : {}),
223+
} as {
224+
expiration: number;
225+
redirectUrl: string;
226+
next?: string;
204227
};
205228

206229
return Buffer.from(
@@ -249,10 +272,14 @@ async function getOIDCClient(settings: OIDCSettings): Promise<BaseClient> {
249272

250273
export async function getOIDCAuthorizationUrl(
251274
settings: OIDCSettings,
252-
params: { sessionId: string }
275+
params: { sessionId: string; next?: string }
253276
): Promise<string> {
254277
const client = await getOIDCClient(settings);
255-
const csrfToken = await generateCsrfToken(params.sessionId, settings.redirectURI);
278+
const csrfToken = await generateCsrfToken(
279+
params.sessionId,
280+
settings.redirectURI,
281+
sanitizeReturnPath(params.next)
282+
);
256283

257284
return client.authorizationUrl({
258285
scope: OIDConfig.SCOPES,
@@ -291,13 +318,16 @@ export async function validateAndParseCsrfToken(
291318
): Promise<{
292319
/** This is the redirect url that was passed to the OIDC provider */
293320
redirectUrl: string;
321+
/** Relative path (within this app) to return to after login */
322+
next?: string;
294323
} | null> {
295324
try {
296325
const { data, signature } = z
297326
.object({
298327
data: z.object({
299328
expiration: z.number().int(),
300329
redirectUrl: z.string().url(),
330+
next: z.string().optional(),
301331
}),
302332
signature: z.string().length(64),
303333
})
@@ -306,7 +336,7 @@ export async function validateAndParseCsrfToken(
306336
const reconstructSign = await sha256(JSON.stringify(data) + "##" + sessionId);
307337

308338
if (data.expiration > Date.now() && signature === reconstructSign) {
309-
return { redirectUrl: data.redirectUrl };
339+
return { redirectUrl: data.redirectUrl, next: sanitizeReturnPath(data.next) };
310340
}
311341
} catch (e) {
312342
logger.error(e);
@@ -493,9 +523,23 @@ export async function triggerOauthFlow({
493523
}
494524
}
495525

526+
// Preserve a safe in-app return path after login.
527+
// Priority: explicit ?next=... (must be an absolute path), else the current path (when auto-login kicks in).
528+
let next: string | undefined = undefined;
529+
const nextParam = sanitizeReturnPath(url.searchParams.get("next"));
530+
if (nextParam) {
531+
// Only accept absolute in-app paths to prevent open redirects
532+
next = nextParam;
533+
} else if (!url.pathname.startsWith(`${base}/login`)) {
534+
// For automatic login on protected pages, return to the page the user was on
535+
next = sanitizeReturnPath(`${url.pathname}${url.search}`) ?? `${base}/`;
536+
} else {
537+
next = sanitizeReturnPath(`${base}/`) ?? "/";
538+
}
539+
496540
const authorizationUrl = await getOIDCAuthorizationUrl(
497541
{ redirectURI },
498-
{ sessionId: locals.sessionId }
542+
{ sessionId: locals.sessionId, next }
499543
);
500544

501545
throw redirect(302, authorizationUrl);

src/routes/login/callback/+server.ts

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -86,5 +86,11 @@ export async function GET({ url, locals, cookies, request, getClientAddress }) {
8686
ip: getClientAddress(),
8787
});
8888

89+
// Prefer returning the user to their original in-app path when provided.
90+
// `validatedToken.next` is sanitized server-side to avoid protocol-relative redirects.
91+
const next = validatedToken.next;
92+
if (next) {
93+
return redirect(302, next);
94+
}
8995
return redirect(302, `${base}/`);
9096
}

0 commit comments

Comments
 (0)