🐛 Fix "signin with HF" within space + CSRF (#236)

.env

@@ -46,7 +46,8 @@ MODELS=`[
 ]`
 OLD_MODELS=`[]`# any removed models, `{ name: string, displayName?: string, id?: string }`
 
-PUBLIC_ORIGIN=#https://hf.co
+PUBLIC_ORIGIN=#https://huggingface.co
+PUBLIC_SHARE_PREFIX=#https://hf.co/chat
 PUBLIC_GOOGLE_ANALYTICS_ID=#G-XXXXXXXX / Leave empty to disable
 PUBLIC_DEPRECATED_GOOGLE_ANALYTICS_ID=#UA-XXXXXXXX-X / Leave empty to disable
 PUBLIC_ANNOUNCEMENT_BANNERS=`[

package-lock.json

@@ -1,12 +1,12 @@
 {
 	"name": "chat-ui",
-	"version": "0.1.0",
+	"version": "0.2.0",
 	"lockfileVersion": 3,
 	"requires": true,
 	"packages": {
 		"": {
 			"name": "chat-ui",
-			"version": "0.1.0",
+			"version": "0.2.0",
 			"dependencies": {
 				"@huggingface/hub": "^0.5.1",
 				"@huggingface/inference": "^2.2.0",

src/hooks.server.ts

@@ -3,6 +3,7 @@ import type { Handle } from "@sveltejs/kit";
 import {
 	PUBLIC_GOOGLE_ANALYTICS_ID,
 	PUBLIC_DEPRECATED_GOOGLE_ANALYTICS_ID,
+	PUBLIC_ORIGIN,
 } from "$env/static/public";
 import { collections } from "$lib/server/database";
 import { base } from "$app/paths";
@@ -20,25 +21,50 @@ export const handle: Handle = async ({ event, resolve }) => {
 		event.locals.user = user;
 	}
 
+	function errorResponse(status: number, message: string) {
+		const sendJson =
+			event.request.headers.get("accept")?.includes("application/json") ||
+			event.request.headers.get("content-type")?.includes("application/json");
+		return new Response(sendJson ? JSON.stringify({ error: message }) : message, {
+			status,
+			headers: {
+				"content-type": sendJson ? "application/json" : "text/plain",
+			},
+		});
+	}
+
+	// CSRF protection
+	const requestContentType = event.request.headers.get("content-type")?.split(";")[0] ?? "";
+	/** https://developer.mozilla.org/en-US/docs/Web/HTML/Element/form#attr-enctype */
+	const nativeFormContentTypes = [
+		"multipart/form-data",
+		"application/x-www-form-urlencoded",
+		"text/plain",
+	];
+	if (event.request.method === "POST" && nativeFormContentTypes.includes(requestContentType)) {
+		const referer = event.request.headers.get("referer");
+
+		if (!referer) {
+			return errorResponse(403, "Non-JSON form requests need to have a referer");
+		}
+
+		const validOrigins = [
+			new URL(event.request.url).origin,
+			...(PUBLIC_ORIGIN ? [new URL(PUBLIC_ORIGIN).origin] : []),
+		];
+
+		if (!validOrigins.includes(new URL(referer).origin)) {
+			return errorResponse(403, "Invalid referer for POST request");
+		}
+	}
+
 	if (
 		!event.url.pathname.startsWith(`${base}/login`) &&
 		!event.url.pathname.startsWith(`${base}/admin`) &&
 		!["GET", "OPTIONS", "HEAD"].includes(event.request.method)
 	) {
-		const sendJson =
-			event.request.headers.get("accept")?.includes("application/json") ||
-			event.request.headers.get("content-type")?.includes("application/json");
-
 		if (!user && requiresUser) {
-			return new Response(
-				sendJson ? JSON.stringify({ error: ERROR_MESSAGES.authOnly }) : ERROR_MESSAGES.authOnly,
-				{
-					status: 401,
-					headers: {
-						"content-type": sendJson ? "application/json" : "text/plain",
-					},
-				}
-			);
+			return errorResponse(401, ERROR_MESSAGES.authOnly);
 		}
 
 		// if login is not required and the call is not from /settings, we check if the user has accepted the ethics modal first.
@@ -50,17 +76,7 @@ export const handle: Handle = async ({ event, resolve }) => {
 			});
 
 			if (!hasAcceptedEthicsModal) {
-				return new Response(
-					sendJson
-						? JSON.stringify({ error: "You need to accept the welcome modal first" })
-						: "You need to accept the welcome modal first",
-					{
-						status: 405,
-						headers: {
-							"content-type": sendJson ? "application/json" : "text/plain",
-						},
-					}
-				);
+				return errorResponse(405, "You need to accept the welcome modal first");
 			}
 		}
 	}

src/lib/components/LoginModal.svelte

@@ -1,5 +1,5 @@
 <script lang="ts">
-	import { enhance } from "$app/forms";
+	import { browser } from "$app/environment";
 	import { base } from "$app/paths";
 	import { page } from "$app/stores";
 	import { PUBLIC_VERSION } from "$env/static/public";
@@ -9,6 +9,8 @@
 	import type { LayoutData } from "../../routes/$types";
 
 	export let settings: LayoutData["settings"];
+
+	const isIframe = browser && window.self !== window.parent;
 </script>
 
 <Modal>
@@ -35,7 +37,7 @@
 		</p>
 		<form
 			action="{base}/{$page.data.requiresLogin ? 'login' : 'settings'}"
-			use:enhance
+			target={isIframe ? "_blank" : ""}
 			method="POST"
 		>
 			{#if $page.data.requiresLogin}

src/lib/components/MobileNav.svelte

@@ -40,7 +40,7 @@
 		bind:this={openEl}><CarbonTextAlignJustify /></button
 	>
 	<span class="truncate px-4">{title}</span>
-	<a href={base || "/"} class="-mr-3 flex h-9 w-9 shrink-0 items-center justify-center"
+	<a href={`${base}/`} class="-mr-3 flex h-9 w-9 shrink-0 items-center justify-center"
 		><CarbonAdd /></a
 	>
 </nav>

src/lib/components/NavMenu.svelte

@@ -26,7 +26,7 @@
 		HuggingChat
 	</a>
 	<a
-		href={base || "/"}
+		href={`${base}/`}
 		class="flex rounded-lg border bg-white px-2 py-0.5 text-center shadow-sm hover:shadow-none dark:border-gray-600 dark:bg-gray-700"
 	>
 		New Chat

src/lib/server/auth.ts

@@ -1,15 +1,13 @@
 import { Issuer, BaseClient, type UserinfoResponse, TokenSet } from "openid-client";
-import { addDays, addYears } from "date-fns";
+import { addHours, addYears } from "date-fns";
 import {
 	COOKIE_NAME,
 	OPENID_CLIENT_ID,
 	OPENID_CLIENT_SECRET,
 	OPENID_PROVIDER_URL,
 } from "$env/static/private";
-import { PUBLIC_ORIGIN } from "$env/static/public";
 import { sha256 } from "$lib/utils/sha256";
 import { z } from "zod";
-import { base } from "$app/paths";
 import { dev } from "$app/environment";
 import type { Cookies } from "@sveltejs/kit";
 
@@ -35,8 +33,6 @@ export function refreshSessionCookie(cookies: Cookies, sessionId: string) {
 	});
 }
 
-export const getRedirectURI = (url: URL) => `${PUBLIC_ORIGIN || url.origin}${base}/login/callback`;
-
 export const OIDC_SCOPES = "openid profile";
 
 export const authCondition = (locals: App.Locals) => {
@@ -48,8 +44,11 @@ export const authCondition = (locals: App.Locals) => {
 /**
  * Generates a CSRF token using the user sessionId. Note that we don't need a secret because sessionId is enough.
  */
-export async function generateCsrfToken(sessionId: string): Promise<string> {
-	const data = { expiration: addDays(new Date(), 1).getTime() };
+export async function generateCsrfToken(sessionId: string, redirectUrl: string): Promise<string> {
+	const data = {
+		expiration: addHours(new Date(), 1).getTime(),
+		redirectUrl,
+	};
 
 	return Buffer.from(
 		JSON.stringify({
@@ -74,7 +73,7 @@ export async function getOIDCAuthorizationUrl(
 	params: { sessionId: string }
 ): Promise<string> {
 	const client = await getOIDCClient(settings);
-	const csrfToken = await generateCsrfToken(params.sessionId);
+	const csrfToken = await generateCsrfToken(params.sessionId, settings.redirectURI);
 	const url = client.authorizationUrl({
 		scope: OIDC_SCOPES,
 		state: csrfToken,
@@ -91,21 +90,30 @@ export async function getOIDCUserData(settings: OIDCSettings, code: string): Pro
 	return { token, userData };
 }
 
-export async function validateCsrfToken(token: string, sessionId: string) {
+export async function validateAndParseCsrfToken(
+	token: string,
+	sessionId: string
+): Promise<{
+	/** This is the redirect url that was passed to the OIDC provider */
+	redirectUrl: string;
+} | null> {
 	try {
 		const { data, signature } = z
 			.object({
 				data: z.object({
 					expiration: z.number().int(),
+					redirectUrl: z.string().url(),
 				}),
 				signature: z.string().length(64),
 			})
 			.parse(JSON.parse(token));
 		const reconstructSign = await sha256(JSON.stringify(data) + "##" + sessionId);
 
-		return data.expiration > Date.now() && signature === reconstructSign;
+		if (data.expiration > Date.now() && signature === reconstructSign) {
+			return { redirectUrl: data.redirectUrl };
+		}
 	} catch (e) {
 		console.error(e);
-		return false;
 	}
+	return null;
 }

src/routes/+layout.svelte

@@ -56,7 +56,7 @@
 			if ($page.params.id !== id) {
 				await invalidate(UrlDependency.ConversationList);
 			} else {
-				await goto(base || "/", { invalidateAll: true });
+				await goto(`${base}/`, { invalidateAll: true });
 			}
 		} catch (err) {
 			console.error(err);

src/routes/conversation/+server.ts

@@ -57,5 +57,5 @@ export const POST: RequestHandler = async ({ locals, request }) => {
 };
 
 export const GET: RequestHandler = async () => {
-	throw redirect(302, base || "/");
+	throw redirect(302, `${base}/`);
 };

src/routes/conversation/[id]/share/+server.ts

@@ -1,5 +1,5 @@
 import { base } from "$app/paths";
-import { PUBLIC_ORIGIN } from "$env/static/public";
+import { PUBLIC_ORIGIN, PUBLIC_SHARE_PREFIX } from "$env/static/public";
 import { authCondition } from "$lib/server/auth";
 import { collections } from "$lib/server/database";
 import type { SharedConversation } from "$lib/types/SharedConversation";
@@ -52,5 +52,5 @@ export async function POST({ params, url, locals }) {
 }
 
 function getShareUrl(url: URL, shareId: string): string {
-	return `${PUBLIC_ORIGIN || url.origin}${base}/r/${shareId}`;
+	return `${PUBLIC_SHARE_PREFIX || `${PUBLIC_ORIGIN || url.origin}${base}`}/r/${shareId}`;
 }

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

@@ -1,11 +1,13 @@
 import { redirect } from "@sveltejs/kit";
-import { getOIDCAuthorizationUrl, getRedirectURI } from "$lib/server/auth";
+import { getOIDCAuthorizationUrl } from "$lib/server/auth";
+import { base } from "$app/paths";
 
 export const actions = {
-	default: async function ({ url, locals }) {
+	default: async function ({ url, locals, request }) {
 		// TODO: Handle errors if provider is not responding
+		const referer = request.headers.get("referer");
 		const authorizationUrl = await getOIDCAuthorizationUrl(
-			{ redirectURI: getRedirectURI(url) },
+			{ redirectURI: `${(referer ? new URL(referer) : url).origin}${base}/login/callback` },
 			{ sessionId: locals.sessionId }
 		);
 

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

@@ -1,5 +1,5 @@
 import { redirect, error } from "@sveltejs/kit";
-import { getOIDCUserData, getRedirectURI, validateCsrfToken } from "$lib/server/auth";
+import { getOIDCUserData, validateAndParseCsrfToken } from "$lib/server/auth";
 import { z } from "zod";
 import { base } from "$app/paths";
 import { updateUser } from "./updateUser";
@@ -13,7 +13,7 @@ export async function GET({ url, locals, cookies }) {
 
 	if (errorName) {
 		// TODO: Display denied error on the UI
-		throw redirect(302, base || "/");
+		throw redirect(302, `${base}/`);
 	}
 
 	const { code, state } = z
@@ -25,15 +25,15 @@ export async function GET({ url, locals, cookies }) {
 
 	const csrfToken = Buffer.from(state, "base64").toString("utf-8");
 
-	const isValidToken = await validateCsrfToken(csrfToken, locals.sessionId);
+	const validatedToken = await validateAndParseCsrfToken(csrfToken, locals.sessionId);
 
-	if (!isValidToken) {
+	if (!validatedToken) {
 		throw error(403, "Invalid or expired CSRF token");
 	}
 
-	const { userData } = await getOIDCUserData({ redirectURI: getRedirectURI(url) }, code);
+	const { userData } = await getOIDCUserData({ redirectURI: validatedToken.redirectUrl }, code);
 
 	await updateUser({ userData, locals, cookies });
 
-	throw redirect(302, base || "/");
+	throw redirect(302, `${base}/`);
 }

src/routes/logout/+page.server.ts

@@ -12,6 +12,6 @@ export const actions = {
 			secure: !dev,
 			httpOnly: true,
 		});
-		throw redirect(303, base || "/");
+		throw redirect(303, `${base}/`);
 	},
 };

src/routes/settings/+page.server.ts

@@ -41,6 +41,6 @@ export const actions = {
 			}
 		);
 
-		throw redirect(303, request.headers.get("referer") || base || "/");
+		throw redirect(303, request.headers.get("referer") || `${base}/`);
 	},
 };

svelte.config.js

@@ -21,7 +21,7 @@ const config = {
 			base: process.env.APP_BASE || "",
 		},
 		csrf: {
-			// todo: fix
+			// handled in hooks.server.ts, because we can have multiple valid origins
 			checkOrigin: false,
 		},
 	},