sveltejs / kit

web development, streamlined
https://kit.svelte.dev
MIT License
18.59k stars 1.92k forks source link

Server-side `fetch` in `load` is not credentialed #672

Closed GrygrFlzr closed 3 years ago

GrygrFlzr commented 3 years ago

Describe the bug No credentials are passed on in a server-side fetch.

To Reproduce On a fresh kit project:

<!-- src/routes/index.svelte -->
<script context="module">
  export async function load({ fetch }) {
    await fetch(`http://localhost:3000/test`, {
      credentials: "include",
      mode: "cors",
    });
    return true;
  }
</script>

<h1>blah</h1>
// src/routes/test.js
export async function get(request) {
  console.log(request);
  return {
    body: {
      data: 1234,
    },
  };
}
// src/setup/index.js
export async function prepare() {
  return {
    headers: {
      "Set-Cookie": "test=1234",
    },
  };
}

Load http://localhost:3000 twice.

Expected behavior Cookie header should appear in server console on second load.

Information about your SvelteKit Installation:

  System:
    OS: Windows 10 10.0.19042
    CPU: (16) x64 AMD Ryzen 7 3700X 8-Core Processor
    Memory: 12.38 GB / 31.95 GB
  Binaries:
    Node: 14.16.0 - C:\Program Files\nodejs\node.EXE
    Yarn: 1.22.10 - ~\AppData\Roaming\npm\yarn.CMD
    npm: 7.6.1 - C:\Program Files\nodejs\npm.CMD
  Browsers:
    Chrome: 89.0.4389.90
    Edge: Spartan (44.19041.423.0), Chromium (89.0.774.54)
    Internet Explorer: 11.0.19041.1
  npmPackages:
    @sveltejs/kit: next => 1.0.0-next.59
    svelte: ^3.29.0 => 3.35.0

Severity Blocking for any project that requires credentialed server-side requests.

pixelmund commented 3 years ago

Is this related to this?

GrygrFlzr commented 3 years ago

It's apparently a five-month old comment, but probably!

https://github.com/sveltejs/kit/blob/4267d8e77028dbf56234b43a8b0ae13dbdb58631/packages/app-utils/src/render/page.ts#L104

eikaramba commented 3 years ago

this is the issue in node-red for that https://github.com/node-fetch/node-fetch/issues/1113

eikaramba commented 3 years ago

in order to move forward here a) (optional) node-fetch needs to accept the pull request https://github.com/node-fetch/node-fetch/pull/1116 b) sveltekit needs to actually set the cookie headers etc. if credentials is true. likewise to sapper

eikaramba commented 3 years ago

i was able to make it work by adding this code here after line 90

if (uses_credentials) {
  //Get any cookie & authorization header from request and apply them to our internal fetch call
  opts.headers = Object.assign({}, opts.headers);

  const cookies = Object.assign(
      {},
      parseCookie(request.headers.cookie || '')
  );

  const str = Object.keys(cookies)
      .map(key => `${key}=${cookies[key]}`)
      .join('; ');

  opts.headers.cookie = str;

  if (!opts.headers.authorization && request.headers.authorization) {
      opts.headers.authorization = request.headers.authorization;
  }
}

You also need to add

import { parse as parseCookie } from 'cookie';

at the beginning

then rebuild sveltekit via npm run build and copy the /dist/ssr.js over to your local npm folder

it works now and my server side rendering is actually sending the cookie credentials to the backend. However i am hesitant to open a Pull Request because i feel like i am not understanding 100% of the puzzle and don't want to make any troubles (e.g. sapper is actually merging the request & response cookies (by looking at the SET-Cookie header from the response) but i don't know how this can be done in sveltekit)

MirrorBytes commented 3 years ago

@eikaramba This is a good workaround for setting headers with ssr, but it only worked for me when placed above that conditional.

eikaramba commented 3 years ago

yeah it could be that it actually needs to be executed for internal and external calls. i am calling a backend api on localhost from svelte, not first fetching some internal svelte ressource, which then calls my backend api.

Rich-Harris commented 3 years ago

Closed via #835

GrygrFlzr commented 3 years ago

Reopening as this is not quite fixed. Internal fetches like fetch('/test') work.

"External" fetches like fetch('http://localhost:3000/test') (yes, same origin) do not have headers passed even if we explicitly include credentials:

await fetch('http://localhost:4000/test', {
  credentials: 'include',
  mode: 'cors'
});

Probably because we don't pass it to the external node fetch: https://github.com/sveltejs/kit/blob/108c26cb797a7d09dda5f287cd17ee9f02bc914a/packages/kit/src/runtime/server/page.js#L82-L85

Conduitry commented 3 years ago

We need to be really careful about what's 'same origin' because the server has no idea what host/path the various cookies are associated with. It just has a list of cookies that the browser had determined to be relevant for this SSR'd page, and not for any other subrequests.

GrygrFlzr commented 3 years ago

Right, I think forcing the user to be explicit about wanting to do a "cross-origin" fetch makes sense. Should credentials: 'same-origin' emit a warning in console maybe, and default to same behavior as omit?

Conduitry commented 3 years ago

What I meant is that all the server will have are the wrong cookies. It's not like on the browser where we can jump through CORS hoops to make a page on domain1 talk to domain2 using domain2's cookies. On the server, we'd have domain1 talking to domain2 with domain1's cookies.

sphinxc0re commented 3 years ago

Is there an actual solution to this problem? I was hoping for this to eventually work as it would significantly simplify the authentication in my application so this is kind of a blocker for me.

GrygrFlzr commented 3 years ago

Due to the origin issues Conduitry mentioned there's not really a way to do cross-origin requests. If you do need to passthrough your mydomain.com cookies to a thirdparty.com domain, you can technically already do that now by implementing it as an endpoint and calling fetch('/my-endpoint'). However, thirdparty.com cookies will always require client side involvement.

For same-origin I'm not sure how we can reasonably detect fetch('http://mydomain.com/blah') is equivalent to fetch('/blah') without trusting the host has been properly set.

sphinxc0re commented 3 years ago

I don't really want to re-implement all of my api endpoints to make this work. Is there a way to "mask" the thirdparty endpoints to pass through the cookies?

schibrikov commented 3 years ago

This is a blocker for me as well, as I don't really understand how to interact with my backend while it's not implemented within sveltekit and uses cookie-based authentication.

I don't know how much workaround is it, but for now I'm using this approach:

myproject/src/routes/api/[...route].ts:

import fetch from 'node-fetch';
import type { RequestHandler } from '@sveltejs/kit';

const apiUrl = 'http://localhost:5000';

const handler: RequestHandler = async function (request) {
    const response = await fetch(apiUrl + request.path, {
        headers: request.headers,
        method: request.method
    });

    let body: unknown;

    try {
        body = await response.clone().json();
    } catch (e) {
        body = await response.text();
    }

    const headers = {
        'set-cookie': response.headers.get('set-cookie')
    };

    return {
        status: response.status,
        headers,
        body
    };
};

export const get: RequestHandler = handler;

This way I'm able to pass cookie back and forth, so this code kind of works:


<script lang="ts" context="module">
    import type { Load } from '@sveltejs/kit';
    export const load: Load = async (input) => {
        const resp = await input.fetch(`/api/auth/me`);
        const me = await resp.json();
        return {
            props: { me }
        };
    };
</script>

<script lang="ts">
    export let me: any;
</script>

{JSON.stringify(me)}

Doesn't seem to me as an optimal approach, because in involves some manual parsing and non-obvious assumptions. Hope for some solution from @Rich-Harris as well.

GrygrFlzr commented 3 years ago

The problem is that the browser will not send first-party thirdparty.com cookies to mydomain.com, and vice versa. A credentialed client-side fetch to thirdparty.com can never be emulated on the server-side because the requisite information is never sent to the mydomain.com server, so if you have no control over thirdparty.com there's not much you can do.

If you need mydomain.com cookies to pass through to thirdparty.com with the potential security issues that comes with - it's better to parse the cookies and selectively forward specific ones - you can always use spread routes: src/routes/api/[...params].js and map mydomain.com/api/foo/bar to thirdparty.com/foo/bar as done above. This means that both client and server fetch calls will have to go through your /api proxy, since there's no thirdparty.com cookie as far as the browser is concerned.

If you control both domains, maybe you can make the cookie's Domain attribute contain both domains and set SameSite to none? I haven't tried this. That said, third party cookies are subject to privacy concerns and are blocked by both browser extensions and even by some browsers themselves, so they shouldn't be relied upon.

sphinxc0re commented 3 years ago

This feels like a common use case for SvelteKit. I want to write my frontend in SvelteKit and have a separate api. What would be the optimal way to do this? Using token-based auth? Something else?

acoyfellow commented 3 years ago

My typical sapper workflow had a mix of API endpoints (in express). This was like nirvana to me- i miss it in SK.

I also think that having the 'escape hatch' of expresses middleware ecosystem was one of the biggest selling points of sapper. I'd love to be able to do the same thing.

Conduitry commented 3 years ago

The original bug has been resolved. There's not a way to know about what cookies exist on other domains during SSR. Supporting the Express middleware API was discussed internally and decided against. If they want, someone could write a userland interop module between the Express API and the handle hook.

eikaramba commented 3 years ago

in how far was the original issue resolved? i still cannot use svelte-kit SSR currently with backend&frontend even on the same domain by just calling the API endpoint with cookie based authentication. disclaimer: I admit i haven't tried to encapsulate all calls by first using svelte endpoints which then call my API (see example from @schibrikov).

I use cookie based authentication.

Backend Cookie set by:

ctx.cookies.set("token", token, {
      httpOnly: true,
      secure: process.env.NODE_ENV === "production",
      sameSite: 'strict',
      maxAge: 1000 * 60 * 60 * 24 * 28, // 28 Day Age
      domain: process.env.NODE_ENV === "development" ? "localhost" : process.env.PRODUCTION_URL,
      overwrite: true
  });

Svelte-kit Hook

export async function handle({ request,render }) {
    const cookies = cookie.parse(request.headers.cookie || '');

    const token = cookies['token']

    const jwtPayload = token ? jwt.decode(token) : false;
    request.locals.authenticated = !!jwtPayload

    const response = await render(request);

    return {
        ...response,
        headers: {
            ...response.headers,
        }
    }
}

export function getSession(request) {
    return {
        ...request.locals
    }
}

if i use my graphql lib AFTER intital page load it works (no SSR), however if it is called form svelte server it does not work as authentication cookie is not retrieved. The workaround code change which i wrote at the very beginning of this issue fixes it (not saying that it should be incorporated due to the issues in this discussion in regards to 3rd party cookies)

stolinski commented 3 years ago

@schibrikov 's solution almost works for us, but unfortunately Apollo requires you to have a full URI for their config. The problem with that is that Svelte Kit, even with the proxied API route is seeing that URI as a different origin and still doesn't pass in our Headers. So while hitting that proxied url with a fetch using a relative path work, the same relative path doesn't play nice with Apollo client.

Wondering if you found any solutions @eikaramba

eikaramba commented 3 years ago

i ditched apollo, because it just does not work for me with svelteKit. in dev mode yes, but as soon as i started building it broke everything. now i am just using a simple plain fetch to the graphql endpoint.

that said, the issue from this ticket is still a huge problem. the workaround from @schibrikov is kind of working although i have a problem that the endpoint doesn't seem to accept post requests when using the rest parameter. it always fire the exported GET method even if the svelte component uses fetch with method:"POST". i guess i will settle with this workaround but it really feels quite weird to first go over the internal node server to finally access my own backend if i want to use authentication (which of course i want).

daamsie commented 3 years ago

I'd just like to add that I am also being frustrated by this bug.

A Graphql server on localhost:8080 and site on localhost:3000 and they don't play nicely. In production I'd like to have the api on a subdomain.

These are pretty normal setups and should not be so difficult to get working.

I can't understand why this issue is closed.

GrygrFlzr commented 3 years ago

@daamsie Part of the original issue has been solved. The other part of the issue is inherently problematic because cookies cannot cross origins. The issue is not closed as a "won't fix", it's closed because SvelteKit cannot change the behavior of the browser.

To solve the problem in a secure manner, the recommended setup is to proxy your API:

client <-> sveltekit <-> api

Using, for example, /src/routes/api/[...path].js.

stolinski commented 3 years ago

My understanding here is that we're experiencing this in situations where we aren't attempting to cross origin. Ie subdomain or localhost. Where some libraries like apollo require a full url and not a relative one. What I'm seeing is sub domain headers not being set because we aren't able to use a relative url. This means the proxy method described won't work for us either.

daamsie commented 3 years ago

My problems are for situations where the domain is the same though. A cookie can be shared across subdomains or ports. That is not a browser limitation.

Edit: and yes, I'm also trying to use Apollo so need to use the full URL

GrygrFlzr commented 3 years ago

I think this is beyond the scope of the original issue so I'll file a new one specifically for same-origin "external" fetches.

stolinski commented 3 years ago

I think this is beyond the scope of the original issue so I'll file a new one specifically for same-origin "external" fetches.

Awesome,thank you. I think determining that something is an external fetch call based on relative or absolute url might throw a lot of people off.

GrygrFlzr commented 3 years ago

The new issue will be tracked at #1777.