Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

The SvelteKit SSR Documentation Examples Enhancements #742

Closed
kvetoslavnovak opened this issue Feb 11, 2024 · 11 comments
Closed

The SvelteKit SSR Documentation Examples Enhancements #742

kvetoslavnovak opened this issue Feb 11, 2024 · 11 comments
Labels
documentation Improvements or additions to documentation help wanted Extra attention is needed

Comments

@kvetoslavnovak
Copy link

kvetoslavnovak commented Feb 11, 2024

Improve documentation

I dived into SvelteKit implementation of Supabase SSR. The SvelteKit documentation may be improved as suggested hereunder..

A link to the page which needs improvement :
https://supabase.com/docs/guides/auth/server-side/creating-a-client?framework=sveltekit

Describe the problem

Here is my working tutorial how to implement Supabase Auth SSR in SvelteKit fully. Mainly it gives the missing part how to implement invalidation so auth/session state is in sync between server and browser or between more opened browser tabs.

Feel free to use this tutorial in Supabase website somewhere.

Describe the improvement

EDIT: I HAVILY EDITED MY COMMETS AND CODE EXAMPLES, PUT THE FINAL SOLUTION HERE AT THE TOP AND DELETED PREVIOUS IDEAS TO AVOID POSSIBLE CONFUSION. I HAVE ALSO DELETED A PULL REQUEST IN MATTER UNTIL THIS IS SOLID.

SvelteKit 2 needs a path "/" for a cookie, not ann empty string. Using name createServerClient for server client so not to be confused with browser client.

// src/routes/hooks.server.js
// SvelteKit v2
import { PUBLIC_SUPABASE_URL, PUBLIC_SUPABASE_ANON_KEY } from '$env/static/public';
import { createServerClient } from '@supabase/ssr';

export const handle = async ({ event, resolve }) => {
	event.locals.supabaseServerClient = createServerClient(PUBLIC_SUPABASE_URL, PUBLIC_SUPABASE_ANON_KEY, {
		cookies: {
			get: (key) => event.cookies.get(key),
			set: (key, value, options) => {
				event.cookies.set(key, value, { ...options, path: '/' });
			},
			remove: (key, options) => {
				event.cookies.delete(key, { ...options, path: '/' });
			}
		}
	});

	const getSessionAndUser = async () => {
		const { data: user, error: err }  = await event.locals.supabaseServerClient.auth.getUser()
	
	  let session
	   if (err) {
		   return { session, user: null }
	   }
	   else {
		 session = (await event.locals.supabaseServerClient.auth.getSession()).data?.session
	  }

		return {session, user}
	  }

	  const {session, user} = await getSessionAndUser()

	  event.locals.session = session
	  event.locals.user = user

	return resolve(event, {
		filterSerializedResponseHeaders(name) {
			return name === 'content-range';
		}
	});
};
// src/routes/+layout.server.js
export const load = async (event) => {
    return {
        session: event.locals.session,
        user: event.locals.user
    };
};
// src/routes/+layout.js
import { PUBLIC_SUPABASE_ANON_KEY, PUBLIC_SUPABASE_URL } from '$env/static/public'
import { combineChunks, createBrowserClient, isBrowser, parse } from '@supabase/ssr'

export const load = async ({ fetch, data, depends }) => {
    depends('supabase:auth')

    const supabase = createBrowserClient(PUBLIC_SUPABASE_URL, PUBLIC_SUPABASE_ANON_KEY, {
        global: {
            fetch,
        },
        cookies: {
            get(key) {
                if (!isBrowser()) {
                    return JSON.stringify(data.session)
                }
                const cookie = combineChunks(key, (name) => {
                    const cookies = parse(document.cookie)
                    return cookies[name]
                })
                return cookie
            },
        },
    })

    return {
        supabase,
        session: data.session,
        user: data.user
    }
}

Invalidation call was missing in the documentation but is importatnt to keep the layout and UI in sync.

// src/routes/+layout.svelte
<script>
	import { invalidate, invalidateAll, goto } from '$app/navigation';
	import { onMount } from 'svelte';

	export let data;

	$: ({ supabase } = data);

	onMount(async () => {
		const { data: { subscription } } = supabase.auth.onAuthStateChange((event, _session) => {
			invalidate('supabase:auth');
			// or just use "hard" invalidation refresh
			// invalidateAll();
		});
		return () => subscription.unsubscribe();
	});
</script>

...

Using prefered way to get session and user from locals which is rerun in hooks for every request. This is more secure way compared to gettting the session and user from layout data because layout data are not refreshed everytime. Also it is important to note that server load functions run all at once.
If you prefere to use layout data do not forget to call await parent()

//  src/routes/+page.server.js
import { redirect } from "@sveltejs/kit"

export const load = async ({ locals }) => {
    // protected route with redirect if there is no user's session
    if (!locals.session) {
        redirect(303, '/');
    }

    // using Supabase server client which is stored in locals
    // locals.supabaseServerClient. ...

    return {
        session: locals.session,
        user: locals.user
    }
}

// // or if you prefere to return session and user from  layout
// // don't forget to call await parent() to have fresh data
// export const load = async ({ parent } ) =>  {
// 	const data = await parent()
// 	const session = data.session
// 	const user = data.user
//     // if there is no user's sessiion redirect back to the home page
// 	if (!session) {
// 		redirect(303, '/');
// 	}
//   }

export const actions = {
	default: async (event) => {
    const { request, url, locals: { supabaseServerClient } } = event
		const formData = await request.formData()
		const email = formData.get('email') as string
		const password = formData.get('password') as string
    ...
	}
}

API Route

//  src/routes/+server.js
import { redirect } from '@sveltejs/kit';

export const GET = async (event) => {
	const { url, locals: { supabaseServerClient } } = event
	...
}

Here are the "final" examples in Typescript:

// src/routes/hooks.server.ts
// SvelteKit v2
import { PUBLIC_SUPABASE_URL, PUBLIC_SUPABASE_ANON_KEY } from '$env/static/public';
import { createServerClient } from '@supabase/ssr';
import type { Handle } from "@sveltejs/kit";

export const handle: Handle = async ({ event, resolve }) => {
	event.locals.supabaseServerClient = createServerClient(PUBLIC_SUPABASE_URL, PUBLIC_SUPABASE_ANON_KEY, {
		cookies: {
			get: (key) => event.cookies.get(key),
			set: (key, value, options) => {
				event.cookies.set(key, value, { ...options, path: '/' });
			},
			remove: (key, options) => {
				event.cookies.delete(key, { ...options, path: '/' });
			}
		}
	});

	const getSessionAndUser = async () => {
		const { data: user, error: err }  = await event.locals.supabaseServerClient.auth.getUser()
	
	  let session
	   if (err) {
		   return { session, user: null }
	   }
	   else {
		 session = (await event.locals.supabaseServerClient.auth.getSession()).data?.session
	  }

		return {session, user}
	  }

	  const {session, user} = await getSessionAndUser()

	  event.locals.session = session
	  event.locals.user = user

	return resolve(event, {
		filterSerializedResponseHeaders(name) {
			return name === 'content-range';
		}
	});
};
// src/routes/+layout.server.ts
import type { LayoutServerLoad } from "./$types";
export const load: LayoutServerLoad = async (event) => {
    return {
        session: event.locals.session,
        user: event.locals.user
    };
};
// src/routes/+layout.ts
import { PUBLIC_SUPABASE_ANON_KEY, PUBLIC_SUPABASE_URL } from '$env/static/public'
import { combineChunks, createBrowserClient, isBrowser, parse } from '@supabase/ssr'
import type { LayoutLoad } from "./$types";

export const load: LayoutLoad = async ({ fetch, data, depends }) => {
    depends('supabase:auth')

    const supabase = createBrowserClient(PUBLIC_SUPABASE_URL, PUBLIC_SUPABASE_ANON_KEY, {
        global: {
            fetch,
        },
        cookies: {
            get(key) {
                if (!isBrowser()) {
                    return JSON.stringify(data.session)
                }
                const cookie = combineChunks(key, (name) => {
                    const cookies = parse(document.cookie)
                    return cookies[name]
                })
                return cookie
            },
        },
    })

    return {
        supabase,
        session: data.session,
        user: data.user
    }
}
//  src/routes/+page.server.ts
import { redirect } from "@sveltejs/kit"
import type { PageServerLoad , Actions} from './$types';

export const load: PageServerLoad = async ({ locals }) => {
    // protected route with redirect if there is no user's session
    if (!locals.session) {
        redirect(303, '/');
    }

    // using Supabase server client which is stored in locals
    // locals.supabaseServerClient. ...

    return {
        session: locals.session,
        user: locals.user
    }
}

// // or if you prefere to return session and user from  layout
// // don't forget to call await parent() to have fresh data
// export const load: PageServerLoad = async ({ parent } ) =>  {
// 	const data = await parent()
// 	const session = data.session
// 	const user = data.user
//     // if there is no user's sessiion redirect back to the home page
// 	if (!session) {
// 		redirect(303, '/');
// 	}
//   }

export const actions: Actions = {
	default: async (event) => {
    const { request, url, locals: { supabaseServerClient } } = event
		const formData = await request.formData()
		const email = formData.get('email') as string
		const password = formData.get('password') as string
    ...
	}
}

API Route

//  src/routes/+server.ts
import { redirect } from '@sveltejs/kit';

export const GET = async (event) => {
	const { url, locals: { supabaseServerClient } } = event
	...
}

The rest full code for login, logout, update , reset etc. is in the tutorial.

Additional context

Important issue is for example to show how to protect sensitive pages with session check and redirect (or maybe more generally in hooks.server.js). You can see this in my tutorial

@kvetoslavnovak kvetoslavnovak added the documentation Improvements or additions to documentation label Feb 11, 2024
kvetoslavnovak added a commit to kvetoslavnovak/supabase that referenced this issue Feb 12, 2024
See my issue supabase/auth-helpers#742

The SvelteKit documentation may be improved mainly by adding  src/routes/+layout.svelte example with invalidation call.
@charislam charislam added the help wanted Extra attention is needed label Feb 14, 2024
@charislam
Copy link

charislam commented Feb 14, 2024

Wow, great job! Love how in-depth the tutorial is.

One thing I would point out that you might want to change in your tutorial:

event.locals.getSession = async () => {
    let {
      data: { session },
    } = await event.locals.supabase.auth.getSession()

    // solving the case if the user was deleted from the database but the browser still has a cookie/loggedin user
    // +lauout.server.js will delete the cookie if the session is null
    const { data: getUserData, error: err }  = await event.locals.supabase.auth.getUser()
    if (getUserData.user == null) {
      session = null
    }

    return session
  }

As you mentioned, we have a warning in our docs that using getSession in a server environment can open a security hole in your application, since it does not guarantee that the returned session is valid.

It's better practice to call getUser() first, since getUser will check that the stored session is signed by a valid key. So instead I would do this (also returning the user object from getUser as a secure source of user data). In fact, if you only ever need the user and not the access token, you might even consider solely using getUser here:

event.locals.getSessionAndUser = async () => {
    const { data: user, error: err }  = await event.locals.supabase.auth.getUser()

  let session
   if (err) {
       return { session, user: null }
   }
   else {
     session = (await event.locals.supabase.auth.getSession()).data?.session
  }

    return {session, user}
  }

kvetoslavnovak added a commit to kvetoslavnovak/supabase that referenced this issue Feb 14, 2024
Added getUser() call as advices by @charislam in this discussion concerning the enancement of  the documentation
supabase/auth-helpers#742 (comment)
@charislam
Copy link

Thanks! Upon reviewing my comment, I realized I wasn't quite rigorous enough, so I've edited it, the key points being:

  • Needing to check for errors in getUser
  • Passing along the user from getUser, which should be used instead of the user data in getSession

Really appreciate the PR! We're working through some changes to SvelteKit docs, including figuring out what we want to recommend as best practices, so I'm going to take a look at it in conjunction with that. Might take a bit longer than a quick review, but your work is definitely appreciated ❤️ and we're going to incorporate it somehow!

@andreapiso
Copy link

Wow, great job! Love how in-depth the tutorial is.

One thing I would point out that you might want to change in your tutorial:

event.locals.getSession = async () => {
    let {
      data: { session },
    } = await event.locals.supabase.auth.getSession()

    // solving the case if the user was deleted from the database but the browser still has a cookie/loggedin user
    // +lauout.server.js will delete the cookie if the session is null
    const { data: getUserData, error: err }  = await event.locals.supabase.auth.getUser()
    if (getUserData.user == null) {
      session = null
    }

    return session
  }

As you mentioned, we have a warning in our docs that using getSession in a server environment can open a security hole in your application, since it does not guarantee that the returned session is valid.

It's better practice to call getUser() first, since getUser will check that the stored session is signed by a valid key. So instead I would do this (also returning the user object from getUser as a secure source of user data). In fact, if you only ever need the user and not the access token, you might even consider solely using getUser here:

event.locals.getSessionAndUser = async () => {
    const { data: user, error: err }  = await event.locals.supabase.auth.getUser()

  let session
   if (err) {
       return { session, user: null }
   }
   else {
     session = (await event.locals.supabase.auth.getSession()).data?.session
  }

    return {session, user}
  }

what does this look like for typescript? The supabase documentation does not do too much of a good job explaining the types/shapes of the values returned by the auth functions...

@codepainting
Copy link

Somewhat related to the general discussion of Auth documentation. I do like the fact, that we can easily change the environment (Next.js, Sveltekit...) with tabs.

Not less convenient would be a way to just switch between JS and TS.
I believe this is important, especially for such fragile code parts as Auth.
When in doubt, JS should probably be the standard first choice.

kvetoslavnovak added a commit to kvetoslavnovak/supabase that referenced this issue Feb 21, 2024
Following the discussion how to enhance SvelteKit SSR Auth docs and introduce best practices supabase/auth-helpers#742
@kvetoslavnovak
Copy link
Author

As the majority of proposals are in the official documentation now I am closing this.

@santoshlite
Copy link

santoshlite commented Apr 17, 2024

Hey I am getting a TypeError: safeGetSession is not a function when I run your code from layout.server.ts, any idea why? Thanks!

@kvetoslavnovak
Copy link
Author

What is calling safeGetSession?
Does this happen when you follow the code templates of updated Supabase docs https://supabase.com/docs/guides/auth/server-side/creating-a-client?queryGroups=framework&framework=sveltekit&queryGroups=environment&environment=hooks ?

@codepainting
Copy link

@kvetoslavnovak @ssantoshp There is a longer discussion about problems surrounding this topic at:
supabase/supabase-js#1010 and supabase/auth-js#874
No solution so far, so I am in waiting mode as well, but since this is such a complex and security critical topic, I am super the great Supabase team will figure this out soon.

@kvetoslavnovak
Copy link
Author

@codepainting thank you for letting me know.

@henrisuurorg
Copy link

Did you find a solution to this? @kvetoslavnovak @codepainting @santoshlite

I am getting TypeError: safeGetSession is not a function following the most up-to-date docs at https://supabase.com/docs/guides/auth/server-side/sveltekit

@shen341
Copy link

shen341 commented Oct 2, 2024

@henrisuurorg
Also encountered TypeError: safeGetSession is not a function issue , beause made a mistake hooks.server.ts location.
Moving hooks.server.ts file from /src/routes/ to /src sloved the issue by making the safeGetSession function accessiable in the appropriate location.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation help wanted Extra attention is needed
Projects
None yet
Development

No branches or pull requests

7 participants