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

Make server version of fetch() in load() copy over 'set-cookie' to the page response #1198

Closed
hgl opened this issue Apr 24, 2021 · 36 comments · Fixed by #4588
Closed

Make server version of fetch() in load() copy over 'set-cookie' to the page response #1198

hgl opened this issue Apr 24, 2021 · 36 comments · Fixed by #4588
Labels
bug Something isn't working p1-important SvelteKit cannot be used by a large number of people, basic functionality is missing, etc.
Milestone

Comments

@hgl
Copy link
Contributor

hgl commented Apr 24, 2021

Is your feature request related to a problem? Please describe.
Currently, the isomorphism of the load() function breaks when a fetch response, from an internal url, contains set-cookie. If it's called by the client, the cookie is set in the browser, if it's called by the server, the cookie is dropped.

One use case is to create a session when a guest visits a page that should only be visible to a registered user. This session should record what page to return the guest to, once she registers.

Some people might suggest passing the return url as a query to the register page, but that means we now have to sanitize the url, prevent users from being redirected to external sites, and even if we do this, we still can't prevent users from being redirected to other irrelevant internal pages (e.g., people register via an url shared online that contains the query to redirect them to an irrelevant page)

Describe the solution you'd like
The server's version of fetch should copy over cookie headers from the fetch response, from an internal url, to the page response, (including a redirect response like 302)

Describe alternatives you've considered
We can redirect guests to an endpoint that creates the session and return the cookie header, but that forces a page refresh, breaking the SPA experience that svelte kit tries so hard to deliver.

How important is this feature to you?
Very, and I think this recording return url use case should be really common.

Additional context
None.

@hgl
Copy link
Contributor Author

hgl commented Apr 24, 2021

Hmm, just realize this requires merging 'set-cookie' if there are multiple fetch()s, sounds messy (although in 99% cases, there should only be one).

Feel free to close this issue if this doesn't seem worth it.

@GrygrFlzr
Copy link
Member

Simple experimentation with default configurations shows that multiple same-origin cookies make it to the client even when SSR'd, which makes sense.

This situation you're describing seems to only occur with either:

  • Hydration disabled (export const hydrate = false;) in which case the client is not doing any fetches and it is expected that cookies are not passed through.
  • Cross-origin cookies - In the case of a thirdparty.com API, it is impossible for mydomain.com to set a thirdparty.com cookie. This is not unique to SvelteKit as it is a behavior enforced by all modern browsers.
    Setting a mydomain.com cookie with the same contents as the thirdparty.com cookie does not solve the issue as it will be sent to the wrong domain on the client side. You need to proxy the requests through your own API and store a same-origin cookie containing the information that needs to be sent to the third party API.

If neither of these describe your situation, please elaborate.

@hgl
Copy link
Contributor Author

hgl commented Apr 24, 2021

Hmm, here are my findings after experimenting with a small test case:

routes/session.ts

export async function get({ params }) {
	return {
		status: 200,
		headers: {
			'set-cookie': 'ID=1; path=/;'
		},
		body: "1"
	}
}

routes/about.svelte

<script context="module">
	export async function load({ fetch }) {
		await fetch("/session", {
			headers: {
				'content-type': 'text/plain'
			}
		});
	}
</script>

When you open /about directly in the browser:

  1. The cookie is set because the client actually makes the fetch() call.

If I change load() to

<script context="module">
	export async function load({ fetch }) {
		const res = await fetch("/session", {
			headers: {
				'content-type': 'text/plain'
			}
		});
		const text = await res.text()
		return {
			props: {text}
		}
	}
</script>
  1. the client no longer makes the call, and no cookie is set.

  2. In the first case, if you disable javascript, no cookie is set.

It seems this only works if you throw away fetch()'s body, and it relies on client javascript to set the cookie.

@hgl
Copy link
Contributor Author

hgl commented Apr 27, 2021

Another issue with setting the cookie with client js is that, because you probably want to redirect to the login page, it can prevent the current page from being returned to the browser, so the client js doesn't have the chance to make that API call to create the session:

<script context="module">
	export async function load({ fetch }) {
		await fetch("/session");
		return {
		    status: 302,
		    redirect: '...',
		};
	}
</script>

@hgl
Copy link
Contributor Author

hgl commented Apr 28, 2021

Related issue: #842

@benwoodward
Copy link

benwoodward commented Jul 27, 2021

I'm experiencing this with @sveltejs/kit@1.0.0-next.137 implementing the logic found in this demo repo: https://github.com/srmullen/sveltekit-magic, specifically with the fetching of /api/auth/user that normally returns a set-cookie header. The header doesn't arrive in the page response unless I remove the await res.json() logic.

The workaround I'm using is to just call /api/auth/user via onMount: srmullen/sveltekit-magic@9ab83c8

EDIT: Create a reproduction repo: https://github.com/benwoodward/sveltekit-headers-issue-demo

@benmccann
Copy link
Member

Some places to look for anyone who'd want to implement this:

headers: /** @type {Record<string, string>} */ (rendered.headers)

https://github.com/node-fetch/node-fetch#extract-set-cookie-header

@kevinrenskers
Copy link

Hey all, turns out that the problem I was running into (see my comment in #1777) was exactly this problem: I am doing the server request to login during SSR, and so the set-cookie header in the response basically had no effect. Adding export const ssr = false to my page helped to solve the problem.

I noticed in the changelog that this problem should be fixed in version 166, but that's not actually released yet. Any word on a timeline?

@benmccann
Copy link
Member

we ran into some hiccups, but 166 should be released now

@cryptodeal
Copy link

cryptodeal commented Sep 15, 2021

@benmccann I believe this is still broken when trying to set multiple cookies in load. In my endpoint, I'm setting 2 cookies as detailed in the docs:

return {
	headers: {
		'set-cookie': [cookie1, cookie2]
	}
};

However, only cookie1 is set in the browser.

If I change the code to:

return {
	headers: {
		'set-cookie': [cookie2, cookie1]
	}
};

Only cookie2 is set in the browser.

I'm more than happy to throw together/link to a simple reproduction repo if that would help!

@benmccann
Copy link
Member

Yes, a reproduction repo would help get this looked at much faster. Thanks!!

@cryptodeal
Copy link

cryptodeal commented Sep 16, 2021

Reproduction repo: https://github.com/cryptodeal/load-multiple-cookies

Further testing while throwing together a super basic repo to reproduce the bug provided a bit more insight:

  1. Both cookies are in the response header
  2. When navigating to the page via the navbar/header in the reproduction repo, both cookies are set (even w/o sveltekit:load)
  3. HOWEVER, navigating directly to the URL @ localhost:3000/bug only stores the first cookie in the 'set-cookie' array in the browser.

For context, I first encountered this issue when building out a DIY magic link auth system. I use nodemailer to send the authToken verification link to the user's email; the verification/[authToken].json.js endpoint attempts to set both an accessToken and a refreshToken, but the browser only stores the first cookie for whatever reason. (tested in Brave Browser, Chrome, and Safari).

@dishuostec
Copy link

I'v met same problem. It seems we should split multiple cookies from one string #2362 (comment).

@benmccann
Copy link
Member

The big question I'd have for the other maintainers is do we want to add another dependency to deal with this (e.g. set-cookie-parser linked to in the other thread) or do we want to try to copy/paste in the code?

@mabentley85
Copy link

Just curious as to why the SvelteKit behavior is different from Sapper's in regards requiring the same or more specific subdomain in server side fetch requests, versus just having the same domain.

Currently I have two Sapper apps, one for the frontend public site at (www.example.com) and one for our backend admin panel (admin.example.com). They both communicate with our API at api.example.com. I started migrating our admin panel to SvelteKit, but had to create a proxy domain (api.admin.example.com) for server side request to pass authentication cookies. Requests made from the browser worked just fine with api.example.com.

Is this the intended way that SvelteKit will be going forward? Or is this just a temporary issue?

@kevinrenskers
Copy link

kevinrenskers commented Oct 1, 2021

When I was working on my SvelteKit app and Django-based API locally, everything worked fine. When the web app logs in on the API, the API sends back a set-cookie header with the localhost domain, and from then on the web app knows that the user is logged in because we could read the cookie and copy it to the session:

// hooks.js
import cookie from "cookie";

export async function handle({ request, resolve }) {
  const cookies = cookie.parse(request.headers.cookie || "");
  request.locals.token = cookies.token;
  return await resolve(request);
}

export function getSession({ locals }) {
  return {
    token: locals.token,
  };
}

And on every request that's made to the API server from then on, the cookie containing the token would be sent back, and all was well.

Now I have deployed my project, and I am running into a problem. When www.example.com logs in to the API at api.example.com, the cookie is set with the api.example.com domain. That means that the web app can't read it, and it doesn't know if you're logged in. I can use example.com as the cookie domain with SameSite set to Lax and then the web app can read it and know's you're logged in, but then requests made the API won't have that cookie attached and so they fail. Question one: is this a SvelteKit bug?

I guess I could set the cookie domain to example.com and instead of relying on the credentials: "include" parameter of fetch, always send the token along an an Authorization header. But that is a pretty big refactor, I'd have to change all the places where I am making API requests to send along session.token, so question two: I was really hoping there are better alternatives?

@kevinrenskers
Copy link

So now the question is: do I wait for this bug to be fixed, or do I go with my workaround that does need a pretty big refactor. Is this something that's currently on the radar of the SvelteKit team, or would you advice me to not hold my breath? Which is a perfectly valid answer of course, not everything has the highest priority :)

@kevinrenskers
Copy link

Guys, I am completely stuck due to this issue, and it's causing serious problems in my website. I'm starting to panic a little bit :(

So I have a SvelteKit website www.example.com, and an external API at api.example.com.

What I want to do:

  • User logs in via api.example.com/auth/login, the server returns a set-cookie header with an HttpOnly cookie containing the token, with the cookie domain set to example.com.
  • SvelteKit hooks.js picks up that cookie on every request, and copies it to $session.
  • On every request I make to my API, I send along the token from the $session.

On localhost this all works fine (where both the website and the API are running on localhost, cookie domain is localhost as well). However, on the production website and API the cookie is never stored, and thus you're never logged in!

My temporary fix right now is to just set a cookie client-side: the server returns the token in the body, and I simply store it in a client side cookie. From there hooks.js puts it in $session, it gets sent back to the API on every request, that part is the same.

This workaround does work, but with one huge problem: the maximum age of the cookie is horrible, at least in Safari. Users are now logged out after 24, they have to re-login every single day!

I really need the HttpOnly cookies to work, and I can't figure out why they are not set in my production environment. I've tried setting the domain to example.com and to www.example.com, I've tried samesite strict and lax, secure true and false, I've tried turning off SSR in SvelteKit, but no matter what, I just don't get the HttpOnly only cookies to stick around. Can anyone please help?

@vpanyushenko
Copy link

I think what might be happening is that the host is not being forwarded on your live server. If you're hosting on Netlify, Vercel, GCP, etc, the actual host domain is not example.com, so the cookie is not being sent with the request. You can log the host in production to make sure.

Svelte kit has an property called hostHeader to make this work: https://kit.svelte.dev/docs#configuration-hostheader

@kevinrenskers
Copy link

I'm hosting this on my own VPS, and yeah there are definitely no other domains involved.

@benwoodward
Copy link

@kevinrenskers I'm wondering whether this issue has something to do with the fact that the cookie is coming from a different domain (because the API is on a subdomain). Have you tried serving the cookie from an endpoint that is reachable via a subdirectory URL, e.g. example.com/api/session ..? Or is that not a feasible solution?

@kevinrenskers
Copy link

api.example.com should be able to set a cookie for either example.com or even www.example.com just fine, normally speaking. Right? Calling my external API via an endpoint could be a solution 🤔 I don't think that's how SvelteKit should deal with HttpOnly cookies though?

@benwoodward
Copy link

api.example.com should be able to set a cookie for either example.com or even www.example.com just fine, normally speaking. Right? Calling my external API via an endpoint could be a solution 🤔 I don't think that's how SvelteKit should deal with HttpOnly cookies though?

Agree. But I think it's worth testing because it's possible you've discovered an edge-case there's a separate bug involved.

@kevinrenskers
Copy link

Ok, that works. I am now calling my external login endpoint via a SvelteKit endpoint, where I am setting a HttpOnly cookie. I can now login correctly, and stay logged in for more than a day 😅

@benwoodward
Copy link

Ok, that works. I am now calling my external login endpoint via a SvelteKit endpoint, where I am setting a HttpOnly cookie. I can now login correctly, and stay logged in for more than a day 😅

I recommend submitting a separate issue for this, it's maybe a bug. Even if it's not the issue itself is useful for other people stuck on the same problem.

@hdra
Copy link

hdra commented Dec 25, 2021

Ok, that works. I am now calling my external login endpoint via a SvelteKit endpoint, where I am setting a HttpOnly cookie. I can now login correctly, and stay logged in for more than a day 😅

hi @kevinrenskers to clarify, your workaround here is by making the API calls from svelte frontend -> svelte backend -> django backend, with your svelte backend checking the result of the django call result and setting an httponly cookie? or did you managed to find some way to pass httponly cookie from your django backend to the svelte frontend?

@kevinrenskers
Copy link

kevinrenskers commented Dec 25, 2021

with your svelte backend checking the result of the django call result and setting an httponly cookie

Correct. The Django backend doesn't deal with cookies anymore, I removed all of that. So the Django login endpoint simply returns a token in the body, and all other endpoints check for a token in the headers. No more setting and reading cookies in the Django API.

To clarify though, ONLY the login endpoint goes via a SvelteKit endpoint, all other requests are made directly to the Django endpoint. You could route all requests through a SvelteKit endpoint but I didn't want to be making so many extra requests.

@hdra
Copy link

hdra commented Dec 25, 2021

ah... i was afraid i had to resort to that.. thanks for clarifying.

@hdra
Copy link

hdra commented Dec 25, 2021

To clarify though, ONLY the login endpoint goes via a SvelteKit endpoint, all other requests are made directly to the Django endpoint.

You could route all requests through a SvelteKit endpoint but I didn't want to be making so many extra requests.

Feels like this is getting out of topic already so I apologize in advance to the maintainers for adding noise to the issue tracker...

but how are you doing this again? if you're checking for a token in the header anyway, how are you getting the token to set the value? and why do you even need the httponly cookie?

@kevinrenskers
Copy link

https://www.loopwerk.io/articles/2021/sveltekit-cookies-tokens/

If you have more question, probably better to just contact me directly :)

@ghost
Copy link

ghost commented Jan 20, 2022

https://www.loopwerk.io/articles/2021/sveltekit-cookies-tokens/
If you have more question, probably better to just contact me directly :)

Worked fine until export async function handle({ request, resolve }) was replace with export async function handle({ event, resolve }).

const cookies = parse(event.request.headers.cookie || ""); now always returns an empty object. Any idea why or how to work around it?

@benmccann
Copy link
Member

Try changing it to event.request.headers.get('cookie')

@pilcrowonpaper
Copy link

@benmccann I believe this is still broken when trying to set multiple cookies in load. In my endpoint, I'm setting 2 cookies as detailed in the docs:

return {
	headers: {
		'set-cookie': [cookie1, cookie2]
	}
};

However, only cookie1 is set in the browser.

If I change the code to:

return {
	headers: {
		'set-cookie': [cookie2, cookie1]
	}
};

Only cookie2 is set in the browser.

I'm more than happy to throw together/link to a simple reproduction repo if that would help!

Hi, I'm having the exact same issue. Is this currently being addressed or is it intended behavior?

@benmccann benmccann added the bug Something isn't working label Mar 17, 2022
srmullen added a commit to srmullen/sveltekit-magic that referenced this issue Mar 27, 2022
Rich-Harris added a commit that referenced this issue Apr 14, 2022
Rich-Harris added a commit that referenced this issue Apr 14, 2022
Rich-Harris added a commit that referenced this issue Apr 15, 2022
* apply set-cookie headers from page dependencies - fixes #1198

* hmmm

* deps

* accommodate commonjs laggards

* Update packages/adapter-static/package.json

Co-authored-by: Ben McCann <322311+benmccann@users.noreply.github.com>

* fix lockfile

Co-authored-by: Ben McCann <322311+benmccann@users.noreply.github.com>
@oetiker oetiker mentioned this issue Sep 7, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working p1-important SvelteKit cannot be used by a large number of people, basic functionality is missing, etc.
Projects
None yet
Development

Successfully merging a pull request may close this issue.