Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
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
28 changes: 27 additions & 1 deletion src/lib/server/auth.ts
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ export const sameSite = z
.default(!secure || dev || config.ALLOW_INSECURE_COOKIES === "true" ? "lax" : "none")
.parse(config.COOKIE_SAMESITE === "" ? undefined : config.COOKIE_SAMESITE);

function sanitizeReturnPath(path: string | undefined | null): string | undefined {
export function sanitizeReturnPath(path: string | undefined | null): string | undefined {
if (!path) {
return undefined;
}
Expand All @@ -80,6 +80,32 @@ function sanitizeReturnPath(path: string | undefined | null): string | undefined
return path;
}

/**
* One-shot guard used when restarting the OAuth flow after a callback that was started
* in another browser (e.g. "Open in Safari" from an in-app browser). Prevents redirect loops.
*/
const loginRetryCookieName = "hfChat-loginRetry";

export function hasLoginRetryCookie(cookies: Cookies): boolean {
return cookies.get(loginRetryCookieName) === "1";
}

export function setLoginRetryCookie(cookies: Cookies) {
cookies.set(loginRetryCookieName, "1", {
path: "/",
// `strict` would keep this cookie from being sent on the cross-site IdP -> callback
// navigation, which is exactly where the loop guard must be observable
sameSite: sameSite === "strict" ? "lax" : sameSite,
secure,
httpOnly: true,
maxAge: 5 * 60,
});
}

export function clearLoginRetryCookie(cookies: Cookies) {
cookies.delete(loginRetryCookieName, { path: "/" });
}

export function refreshSessionCookie(cookies: Cookies, sessionId: string) {
cookies.set(config.COOKIE_NAME, sessionId, {
path: "/",
Expand Down
41 changes: 34 additions & 7 deletions src/routes/login/callback/+server.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,12 @@
import { error, redirect } from "@sveltejs/kit";
import { getOIDCUserData, validateAndParseCsrfToken } from "$lib/server/auth";
import {
clearLoginRetryCookie,
getOIDCUserData,
hasLoginRetryCookie,
sanitizeReturnPath,
setLoginRetryCookie,
validateAndParseCsrfToken,
} from "$lib/server/auth";
import { z } from "zod";
import { base } from "$app/paths";
import { config } from "$lib/server/config";
Expand Down Expand Up @@ -47,14 +54,32 @@ export async function GET({ url, locals, cookies, request, getClientAddress }) {
const csrfToken = Buffer.from(state, "base64").toString("utf-8");

const validatedToken = await validateAndParseCsrfToken(csrfToken, locals.sessionId);
const codeVerifier = cookies.get("hfChat-codeVerifier");

if (!validatedToken) {
throw error(403, "Invalid or expired CSRF token");
}
if (!validatedToken || !codeVerifier) {
// The `state` token and PKCE code verifier are bound to the browser that started the
// login flow, so either check can fail on its own; when the callback lands in a
// different browser (e.g. "Open in Safari" from an in-app browser) both do. Restart
// the flow once in this browser instead of dead-ending on a 403.
if (!hasLoginRetryCookie(cookies)) {
setLoginRetryCookie(cookies);

// Best-effort recovery of the return path from the (unverified) state payload;
// it is re-sanitized to an in-app absolute path before use.
let next: string | undefined;
try {
next = sanitizeReturnPath(JSON.parse(csrfToken)?.data?.next);
} catch {
next = undefined;
}

return redirect(302, `${base}/login${next ? `?next=${encodeURIComponent(next)}` : ""}`);

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Preserve alternate callback when retrying OAuth

When the original flow was started as /login?callback=<allowed alt redirect>, this retry sends the browser back to /login with only next. triggerOauthFlow only selects an alternate redirect URI from the callback query parameter (auth.ts lines 557-560), so the restarted authorization request falls back to ${url.origin}${base}/login/callback; deployments relying on ALTERNATIVE_REDIRECT_URLS then hit the provider with an unregistered or wrong redirect URI instead of recovering. Carry a whitelisted redirectUrl from the state back as callback when restarting.

Useful? React with 👍 / 👎.

}

const codeVerifier = cookies.get("hfChat-codeVerifier");
if (!codeVerifier) {
throw error(403, "Code verifier cookie not found");
throw error(
403,
validatedToken ? "Code verifier cookie not found" : "Invalid or expired CSRF token"
);
}

const { userData, token } = await getOIDCUserData(
Expand Down Expand Up @@ -93,6 +118,8 @@ export async function GET({ url, locals, cookies, request, getClientAddress }) {
ip: getClientAddress(),
});

clearLoginRetryCookie(cookies);

// Prefer returning the user to their original in-app path when provided.
// `validatedToken.next` is sanitized server-side to avoid protocol-relative redirects.
const next = validatedToken.next;
Expand Down
Loading