Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
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
49 changes: 37 additions & 12 deletions src/routes/(auth)/auth/callback/+server.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,30 +6,44 @@ export const GET: RequestHandler = async ({
url,
locals: { supabase, log },
}) => {
const error = url.searchParams.get("error");
const errorCode = url.searchParams.get("error_code");
const errorDescription = url.searchParams.get("error_description");
if (error || errorDescription) {
const errorMessage = errorDescription || error || "An error occurred!";
logError(
400,
errorMessage,
log,
{
error,
errorCode,
errorDescription,
},
false,
);
redirect(303, `/?toast_error=${encodeURIComponent(errorMessage)}`);
Comment thread
coderabbitai[bot] marked this conversation as resolved.
}

const code = url.searchParams.get("code");
if (!code) {
logError(
500,
"Authorization code is missing.",
"Missing code parameter",
log,
new Error("Authorization code not provided"),
new Error("Missing code parameter"),
false,
);
redirect(303, "/?toast_error=Missing+code+parameter");
}

const { data, error: err } = await supabase.auth.exchangeCodeForSession(code);
if (err) {
logError(500, "Authentication failed", log, err);
logError(500, "Failed to exchange code for session", log, err, false);
redirect(303, "/?toast_error=Failed+to+exchange+code+for+session");
}

const { user } = data;
if (!user) {
logError(
500,
"User data is missing.",
log,
new Error("User not returned after authentication"),
);
}

const { data: existingProfile, error: profileFetchError } = await supabase
.from("profiles")
Expand All @@ -38,12 +52,23 @@ export const GET: RequestHandler = async ({
.maybeSingle();

if (profileFetchError) {
logError(500, "Failed to fetch profile", log, profileFetchError);
logError(500, "Failed to retrieve profile", log, profileFetchError, false);
redirect(303, `/?toast_error=Failed+to+retrieve+profile`);
}

if (existingProfile?.onboarded) {
redirect(303, "/dashboard");
}

const { error: profileUpdateError } = await supabase
.from("profiles")
.update({ verified: true })
.eq("user_id", user.id);

if (profileUpdateError) {
logError(500, "Failed to update profile", log, profileUpdateError, false);
redirect(303, `/?toast_error=Failed+to+update+profile`);
}

redirect(303, "/auth/complete-profile");
};
38 changes: 33 additions & 5 deletions src/routes/(auth)/auth/confirm/+server.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,23 +2,47 @@ import type { RequestHandler } from "./$types";
import { redirect } from "@sveltejs/kit";
import { logError } from "$lib/server/logError";

export const GET: RequestHandler = async ({ url, locals }) => {
const { supabase, log } = locals;
export const GET: RequestHandler = async ({
url,
locals: { supabase, log },
}) => {
const error = url.searchParams.get("error");
const errorCode = url.searchParams.get("error_code");
const errorDescription = url.searchParams.get("error_description");
if (error || errorDescription) {
const errorMessage = errorDescription || error || "An error occurred!";
logError(
400,
errorMessage,
log,
{
error,
errorCode,
errorDescription,
},
false,
);
redirect(303, `/?toast_error=${encodeURIComponent(errorMessage)}`);
Comment thread
Saterz marked this conversation as resolved.
}

const code = url.searchParams.get("code");
if (!code) {
logError(
400,
"Missing code parameter",
log,
new Error("Missing code parameter"),
false,
);
redirect(303, "/?toast_error=Missing+code+parameter");
}

const { data, error: authErr } =
await supabase.auth.exchangeCodeForSession(code);

if (authErr) {
logError(500, "Failed to exchange code for session", log, authErr);
logError(500, "Failed to exchange code for session", log, authErr, false);
redirect(303, "/?toast_error=Failed+to+exchange+code+for+session");
}

const user = data.user;
Expand All @@ -30,7 +54,8 @@ export const GET: RequestHandler = async ({ url, locals }) => {
.maybeSingle();

if (profileErr) {
logError(500, "Failed to check profile", log, profileErr);
logError(500, "Failed to retrieve profile", log, profileErr, false);
redirect(303, "/?toast_error=Failed+to+retrieve+profile");
}

if (!existingProfile) {
Expand All @@ -39,7 +64,9 @@ export const GET: RequestHandler = async ({ url, locals }) => {
"No existing profile was found",
log,
new Error("No existing profile was found"),
false,
);
redirect(303, "/?toast_error=No+existing+profile+was+found");
}

const { error: updateErr } = await supabase
Expand All @@ -48,7 +75,8 @@ export const GET: RequestHandler = async ({ url, locals }) => {
.eq("user_id", user.id);

if (updateErr) {
logError(500, "Failed to update profile", log, updateErr);
logError(500, "Failed to update profile", log, updateErr, false);
redirect(303, "/?toast_error=Failed+to+update+profile");
}

if (!existingProfile.onboarded) {
Expand Down
32 changes: 27 additions & 5 deletions src/routes/+layout.svelte
Original file line number Diff line number Diff line change
@@ -1,20 +1,19 @@
<script lang="ts">
// Components and style
import "../app.css";
import Navbar from "$lib/components/layout/navbar.svelte";
import { Toaster } from "svelte-sonner";
import { toast, Toaster } from "svelte-sonner";
import { SvelteKitTopLoader } from "sveltekit-top-loader";
import ClientErrorReporter from "$lib/components/misc/clientErrorReporter.svelte";
import ScrollToTop from "$lib/components/misc/scrollToTop.svelte";
import BottomNav from "$lib/components/layout/bottomNav.svelte";
import type { ResolvedMeta } from "$lib/types/meta.types";
import type { ResolvedPathname } from "$app/types";
import { page } from "$app/state";
import { NuqsAdapter } from "nuqs-svelte/adapters/svelte-kit";

let { data, children } = $props();

// Keeping user fresh in the browser
import { invalidate } from "$app/navigation";
import { goto, invalidate } from "$app/navigation";
import { onMount } from "svelte";

let { session, supabase, profile, isDevelopmentEnvironment } = $derived(data);
Expand All @@ -24,6 +23,29 @@
invalidate("supabase:auth");
}
});

const toastError = page.url.searchParams.get("toast_error");
const toastSuccess = page.url.searchParams.get("toast_success");

const removeToastParam = (newUrl: URL) =>
goto((newUrl.pathname + newUrl.search) as ResolvedPathname, {
replaceState: true,
keepFocus: true,
noScroll: true,
});

if (toastError) {
toast.error(toastError);
const newUrl = new URL(page.url);
newUrl.searchParams.delete("toast_error");
removeToastParam(newUrl);
} else if (toastSuccess) {
toast.success(toastSuccess);
const newUrl = new URL(page.url);
newUrl.searchParams.delete("toast_success");
removeToastParam(newUrl);
Comment on lines +27 to +46

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🔒 Security & Privacy | 🟠 Major | 🏗️ Heavy lift

Don’t render raw toast text from query params.

This now lets any crafted link inject arbitrary success/error copy into a trusted site toast. Please switch the URL contract to an allowlisted toast code (for example toast=auth_exchange_failed) and map that code to local strings instead of displaying free-form query-param text. As per coding guidelines, "Validate all user inputs on both the client and server."

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/routes/`+layout.svelte around lines 27 - 46, The toast handling in
+layout.svelte is rendering free-form query-param text from page.url directly
through toast.error and toast.success, which allows crafted links to inject
arbitrary copy. Update the URL contract to use an allowlisted toast code
parameter instead of toast_error/toast_success, then map that code to local
message strings inside the layout before calling the toast helpers. Keep
removeToastParam and goto for clearing the query param, but only after
validating the code against the allowed set.

Source: Coding guidelines

}

return () => data.subscription.unsubscribe();
});

Expand Down Expand Up @@ -142,7 +164,7 @@

<Banner />

<Toaster />
<Toaster richColors />
<ClientErrorReporter />

<div class="pb-16 md:pb-0">
Expand Down
Loading