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

hgl commented 3 years ago

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 commented 3 years ago

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 commented 3 years ago

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 commented 3 years ago

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:

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>

the client no longer makes the call, and no cookie is set.
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 commented 3 years ago

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 commented 3 years ago

benwoodward commented 3 years ago

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: https://github.com/srmullen/sveltekit-magic/pull/2/commits/9ab83c8a4305a0d6919d66726e9902c7e1609b16

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

benmccann commented 3 years ago

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

https://github.com/sveltejs/kit/blob/621e07106045f7348587c5a1e940b3720b3d029a/packages/kit/src/runtime/server/page/load_node.js#L174 https://github.com/node-fetch/node-fetch#extract-set-cookie-header

kevinrenskers commented 2 years ago

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 commented 2 years ago

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

cryptodeal commented 2 years ago

@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 commented 2 years ago

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

cryptodeal commented 2 years ago

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:

Both cookies are in the response header
When navigating to the page via the navbar/header in the reproduction repo, both cookies are set (even w/o sveltekit:load)
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 commented 2 years ago

I'v met same problem. It seems we should split multiple cookies from one string https://github.com/sveltejs/kit/pull/2362#discussion_r710880323.

benmccann commented 2 years ago

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 commented 2 years ago

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 commented 2 years ago

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?

benmccann commented 2 years ago

That doesn't sound like a SvelteKit bug. That sounds like the way browsers and cookies work

kevinrenskers commented 2 years ago

But if the cookie domain is set to example.com, then www.example.com doing a request to api.example.com should send that cookie along, right? That's at least what I also understood from https://github.com/sveltejs/kit/issues/1777.

benmccann commented 2 years ago

Ah. Yeah. I lost that part in all the other description

kevinrenskers commented 2 years ago

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 commented 2 years ago

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 commented 2 years ago

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 commented 2 years ago

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

benwoodward commented 2 years ago

@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 commented 2 years ago

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 commented 2 years ago

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 commented 2 years ago

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 commented 2 years ago

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 commented 2 years ago

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 commented 2 years ago

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 commented 2 years ago

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

hdra commented 2 years ago

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 commented 2 years ago

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

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

ghost commented 2 years ago

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 commented 2 years ago

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

pilcrowOnPaper commented 2 years ago

@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?

sveltejs / kit

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