Skip to content

Commit

Permalink
Merge pull request #141 from CriticalMoments/fix_redirect_loop
Browse files Browse the repository at this point in the history
Fix a redirect loop.
  • Loading branch information
scosman authored Sep 2, 2024
2 parents 7cc85b1 + 83c8189 commit 57b974d
Show file tree
Hide file tree
Showing 6 changed files with 63 additions and 42 deletions.
3 changes: 2 additions & 1 deletion .vscode/settings.json
Original file line number Diff line number Diff line change
Expand Up @@ -9,5 +9,6 @@
"[javascript]": {
"editor.defaultFormatter": "esbenp.prettier-vscode"
},
"eslint.validate": ["javascript", "javascriptreact", "typescript", "svelte"]
"eslint.validate": ["javascript", "javascriptreact", "typescript", "svelte"],
"editor.tabSize": 2
}
11 changes: 2 additions & 9 deletions src/hooks.server.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@ import { createServerClient } from "@supabase/ssr"
import { createClient } from "@supabase/supabase-js"
import type { Handle } from "@sveltejs/kit"
import { sequence } from "@sveltejs/kit/hooks"
import { redirect } from "@sveltejs/kit"

export const supabase: Handle = async ({ event, resolve }) => {
event.locals.supabase = createServerClient(
Expand Down Expand Up @@ -85,19 +84,13 @@ export const supabase: Handle = async ({ event, resolve }) => {
})
}

// Not called for prerendered marketing pages so generally okay to call on ever server request
// Next-page CSR will mean relatively minimal calls to this hook
const authGuard: Handle = async ({ event, resolve }) => {
const { session, user } = await event.locals.safeGetSession()
event.locals.session = session
event.locals.user = user

if (!event.locals.session && event.url.pathname.startsWith("/account")) {
redirect(303, "/login")
}

if (event.locals.session && event.url.pathname === "/login") {
redirect(303, "/account")
}

return resolve(event)
}

Expand Down
47 changes: 47 additions & 0 deletions src/lib/load_helpers.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
import { isBrowser } from "@supabase/ssr"
import type { Session, SupabaseClient } from "@supabase/supabase-js"
import type { Database } from "../DatabaseDefinitions.js"

export const load_helper = async (
server_session: Session | null,
supabase: SupabaseClient<Database>,
) => {
// on server populated on server by LayoutData, using authGuard hook
let session = server_session
if (isBrowser()) {
// Only call getSession in browser where it's safe.
const getSessionResponse = await supabase.auth.getSession()
session = getSessionResponse.data.session
}
if (!session) {
return {
session: null,
user: null,
}
}

// https://github.com/supabase/auth-js/issues/888#issuecomment-2189298518
if ("suppressGetSessionWarning" in supabase.auth) {
// @ts-expect-error - suppressGetSessionWarning is not part of the official API
supabase.auth.suppressGetSessionWarning = true
} else {
console.warn(
"SupabaseAuthClient#suppressGetSessionWarning was removed. See https://github.com/supabase/auth-js/issues/888.",
)
}
const {
data: { user },
error: userError,
} = await supabase.auth.getUser()
if (userError || !user) {
return {
session: null,
user: null,
}
}

return {
session,
user,
}
}
29 changes: 3 additions & 26 deletions src/routes/(admin)/account/+layout.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import {
import { redirect } from "@sveltejs/kit"
import type { Database } from "../../../DatabaseDefinitions.js"
import { CreateProfileStep } from "../../../config"
import { load_helper } from "$lib/load_helpers"

export const load = async ({ fetch, data, depends, url }) => {
depends("supabase:auth")
Expand All @@ -31,32 +32,8 @@ export const load = async ({ fetch, data, depends, url }) => {
},
})

// on server populated on server by LayoutData, using authGuard hook
let session = data.session
if (isBrowser()) {
// Only call getSession in browser where it's safe.
const getSessionResponse = await supabase.auth.getSession()
session = getSessionResponse.data.session
}
if (!session) {
redirect(303, "/login")
}

// https://github.com/supabase/auth-js/issues/888#issuecomment-2189298518
if ("suppressGetSessionWarning" in supabase.auth) {
// @ts-expect-error - suppressGetSessionWarning is not part of the official API
supabase.auth.suppressGetSessionWarning = true
} else {
console.warn(
"SupabaseAuthClient#suppressGetSessionWarning was removed. See https://github.com/supabase/auth-js/issues/888.",
)
}
const {
data: { user },
error: userError,
} = await supabase.auth.getUser()
if (userError || !user) {
// JWT validation has failed
const { session, user } = await load_helper(data.session, supabase)
if (!session || !user) {
redirect(303, "/login")
}

Expand Down
7 changes: 1 addition & 6 deletions src/routes/(marketing)/login/+layout.server.ts
Original file line number Diff line number Diff line change
@@ -1,18 +1,13 @@
import { redirect } from "@sveltejs/kit"
import type { LayoutServerLoad } from "./$types"

export const load: LayoutServerLoad = async ({
locals: { session },
cookies,
url,
}) => {
// if the user is already logged in return them to the account page
if (session) {
redirect(303, "/account")
}

return {
url: url.origin,
cookies: cookies.getAll(),
session,
}
}
8 changes: 8 additions & 0 deletions src/routes/(marketing)/login/+layout.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,8 @@ import {
createServerClient,
isBrowser,
} from "@supabase/ssr"
import { redirect } from "@sveltejs/kit"
import { load_helper } from "$lib/load_helpers.js"

export const load = async ({ fetch, data, depends }) => {
depends("supabase:auth")
Expand All @@ -28,6 +30,12 @@ export const load = async ({ fetch, data, depends }) => {
},
})

// Redirect if already logged in
const { session, user } = await load_helper(data.session, supabase)
if (session && user) {
redirect(303, "/account")
}

const url = data.url

return { supabase, url }
Expand Down

0 comments on commit 57b974d

Please sign in to comment.