Merge pull request #65 from CriticalMoments/safe_get_session

Fix a security issue: supabase/supabase#22381
CriticalMoments · May 9, 2024 · 7904d41 · 7904d41
2 parents cae14a3 + 3bbdd0b
commit 7904d41
Show file tree

Hide file tree

Showing 9 changed files with 41 additions and 26 deletions.
diff --git a/src/app.d.ts b/src/app.d.ts
@@ -8,7 +8,7 @@ declare global {
     interface Locals {
       supabase: SupabaseClient<Database>
       supabaseServiceRole: SupabaseClient<Database>
-      getSession(): Promise<Session | null>
+      safeGetSession(): Promise<{ session: Session | null; user: User | null }>
     }
     interface PageData {
       session: Session | null

diff --git a/src/hooks.server.ts b/src/hooks.server.ts
@@ -22,13 +22,28 @@ export const handle: Handle = async ({ event, resolve }) => {
   )
 
   /**
-   * A convenience helper so we can just call await getSession() instead const { data: { session } } = await supabase.auth.getSession()
+   * Unlike `supabase.auth.getSession()`, which returns the session _without_
+   * validating the JWT, this function also calls `getUser()` to validate the
+   * JWT before returning the session.
    */
-  event.locals.getSession = async () => {
+  event.locals.safeGetSession = async () => {
     const {
       data: { session },
     } = await event.locals.supabase.auth.getSession()
-    return session
+    if (!session) {
+      return { session: null, user: null }
+    }
+
+    const {
+      data: { user },
+      error,
+    } = await event.locals.supabase.auth.getUser()
+    if (error) {
+      // JWT validation has failed
+      return { session: null, user: null }
+    }
+
+    return { session, user }
   }
 
   return resolve(event, {

diff --git a/src/routes/(admin)/account/(menu)/+page.server.ts b/src/routes/(admin)/account/(menu)/+page.server.ts
@@ -1,8 +1,8 @@
 import { redirect } from "@sveltejs/kit"
 
 export const actions = {
-  signout: async ({ locals: { supabase, getSession } }) => {
-    const session = await getSession()
+  signout: async ({ locals: { supabase, safeGetSession } }) => {
+    const { session } = await safeGetSession()
     if (session) {
       await supabase.auth.signOut()
       throw redirect(303, "/")

diff --git a/src/routes/(admin)/account/(menu)/billing/+page.server.ts b/src/routes/(admin)/account/(menu)/billing/+page.server.ts
@@ -6,9 +6,9 @@ import {
 import type { PageServerLoad } from "./$types"
 
 export const load: PageServerLoad = async ({
-  locals: { getSession, supabaseServiceRole },
+  locals: { safeGetSession, supabaseServiceRole },
 }) => {
-  const session = await getSession()
+  const { session } = await safeGetSession()
   if (!session) {
     throw redirect(303, "/login")
   }

diff --git a/src/routes/(admin)/account/(menu)/billing/manage/+page.server.ts b/src/routes/(admin)/account/(menu)/billing/manage/+page.server.ts
@@ -7,9 +7,9 @@ const stripe = new Stripe(PRIVATE_STRIPE_API_KEY, { apiVersion: "2023-08-16" })
 
 export const load: PageServerLoad = async ({
   url,
-  locals: { getSession, supabaseServiceRole },
+  locals: { safeGetSession, supabaseServiceRole },
 }) => {
-  const session = await getSession()
+  const { session } = await safeGetSession()
   if (!session) {
     throw redirect(303, "/login")
   }

diff --git a/src/routes/(admin)/account/+layout.server.ts b/src/routes/(admin)/account/+layout.server.ts
@@ -2,9 +2,9 @@ import { redirect } from "@sveltejs/kit"
 import type { LayoutServerLoad } from "./$types"
 
 export const load: LayoutServerLoad = async ({
-  locals: { supabase, getSession },
+  locals: { supabase, safeGetSession },
 }) => {
-  const session = await getSession()
+  const { session } = await safeGetSession()
 
   if (!session) {
     throw redirect(303, "/login")

diff --git a/src/routes/(admin)/account/api/+page.server.ts b/src/routes/(admin)/account/api/+page.server.ts
@@ -1,8 +1,8 @@
 import { fail, redirect } from "@sveltejs/kit"
 
 export const actions = {
-  updateEmail: async ({ request, locals: { supabase, getSession } }) => {
-    const session = await getSession()
+  updateEmail: async ({ request, locals: { supabase, safeGetSession } }) => {
+    const { session } = await safeGetSession()
     if (!session) {
       throw redirect(303, "/login")
     }
@@ -43,8 +43,8 @@ export const actions = {
       email,
     }
   },
-  updatePassword: async ({ request, locals: { supabase, getSession } }) => {
-    const session = await getSession()
+  updatePassword: async ({ request, locals: { supabase, safeGetSession } }) => {
+    const { session } = await safeGetSession()
     if (!session) {
       throw redirect(303, "/login")
     }
@@ -148,9 +148,9 @@ export const actions = {
   },
   deleteAccount: async ({
     request,
-    locals: { supabase, supabaseServiceRole, getSession },
+    locals: { supabase, supabaseServiceRole, safeGetSession },
   }) => {
-    const session = await getSession()
+    const { session } = await safeGetSession()
     if (!session) {
       throw redirect(303, "/login")
     }
@@ -191,8 +191,8 @@ export const actions = {
     await supabase.auth.signOut()
     throw redirect(303, "/")
   },
-  updateProfile: async ({ request, locals: { supabase, getSession } }) => {
-    const session = await getSession()
+  updateProfile: async ({ request, locals: { supabase, safeGetSession } }) => {
+    const { session } = await safeGetSession()
     if (!session) {
       throw redirect(303, "/login")
     }
@@ -261,8 +261,8 @@ export const actions = {
       website,
     }
   },
-  signout: async ({ locals: { supabase, getSession } }) => {
-    const session = await getSession()
+  signout: async ({ locals: { supabase, safeGetSession } }) => {
+    const { session } = await safeGetSession()
     if (session) {
       await supabase.auth.signOut()
       throw redirect(303, "/")

diff --git a/src/routes/(admin)/account/subscribe/[slug]/+page.server.ts b/src/routes/(admin)/account/subscribe/[slug]/+page.server.ts
@@ -11,9 +11,9 @@ const stripe = new Stripe(PRIVATE_STRIPE_API_KEY, { apiVersion: "2023-08-16" })
 export const load: PageServerLoad = async ({
   params,
   url,
-  locals: { getSession, supabaseServiceRole },
+  locals: { safeGetSession, supabaseServiceRole },
 }) => {
-  const session = await getSession()
+  const { session } = await safeGetSession()
   if (!session) {
     throw redirect(303, "/login")
   }

diff --git a/src/routes/(marketing)/login/+layout.server.ts b/src/routes/(marketing)/login/+layout.server.ts
@@ -3,9 +3,9 @@ import type { LayoutServerLoad } from "./$types"
 
 export const load: LayoutServerLoad = async ({
   url,
-  locals: { getSession },
+  locals: { safeGetSession },
 }) => {
-  const session = await getSession()
+  const { session } = await safeGetSession()
 
   // if the user is already logged in return them to the account page
   if (session) {