-
-
Notifications
You must be signed in to change notification settings - Fork 3.7k
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
feat(sveltekit): add getSessionWithSetCookies
function for carrying over cookies and enabling token refresh in sveltekit
#9497
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
2 Ignored Deployments
|
if (status === 200) return data | ||
if (status === 200) return { | ||
session: data, | ||
cookies: parseCookies(Array.from(response.headers).filter(([headerName]) => headerName === 'set-cookie')) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can we use the getSetCookie
here? 😁
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah good call, modified that a bit. Also added cookie
and @types/cookie
as peer deps to @auth/svelte
since their dependencies in @auth/core
already anyway and that'll always be installed together. Does that make sense? Or should I just add them as straight-up dependencies for @auth/sveltekit
too?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't like the added work for the user.
A better API might be to wrap load
with getSession
instead (maybe rename it to auth()
to align with next-auth
too) so it can set the cookies after the user logic has run, merging the results.
This is similar to what we do here:
next-auth/packages/next-auth/src/lib/index.ts
Lines 206 to 210 in a05872d
// Preserve cookies from the session response | |
for (const cookie of sessionResponse.headers.getSetCookie()) | |
finalResponse.headers.append("set-cookie", cookie) | |
return finalResponse |
For the user, ideally, the +layout.server.ts
file could just be export const load = auth()
. 🤔
Hmm yeah i can test that, i have a sveltekit project with auth.js currently WIP. But the wrapping load strategy feels a bit un-sveltekit like, based on the little experience i have at least. I'd be curious what a more seasoned svelte person would say about this 🤔 |
…s' of ssh://github.com/nextauthjs/next-auth into ndom91/sveltekit-session-with-cookies-for-refresh-tokens
Let's merge with a (build needs to pass first) |
Okay so we had amended the pre-existing |
Just in case, I don't think "updated from client to server" is a real issue. A decorator can be better. |
Heads up anyone following this PR - feature's been merged via #9694 🙏 |
@ndom91 - can you please update documentation to specific how to use new method and how it is different then existing get-session method. Thanks! |
@aakash14goplani Yeah we're working on a new version of the docs and it'll definitely be in there. For now, the old method is still available, although marked |
☕️ Reasoning
PRed from work done by @benjaminknox (see: #8034 (comment))
When
getSession()
is called it returns only the body from the backend and the header to set the authentication cookie is lost. Ben was able to create a workaround by creating another function (getSessionWithSetCookies
) to return theset-cookie
headers and then set them myself in+layout.server.ts
Usage in
+layout.server.ts
Todo
cookies
types still 🤔🧢 Checklist
🎫 Affected issues
Fixes: #8034
📌 Resources