Make `event` available to page load() function

johnnysprinkles commented 2 years ago

Describe the problem

One common scenario in SSR rendered apps is that the initial render makes some async API calls, then later on when the user is interacting with the page, calls are made to that same API directly from the browser. For the server initiated API calls, it can simply forward on the end user's auth cookies. This is generally a good pattern as it makes your frontend service a dumb and unprivileged middleman.

Say we have a load function like this in one of our route components:

export async function load({ fetch }) {
    let response = await fetch('https://api.example.com/mylatest', {
      credentials: 'include',
      headers: {
        cookie: 'jwt_token=???',
      },
    });
}

There doesn't seem to be a way to get at the end user's incoming request. This isn't specific to fetch or cookies, more generally we need the full detail of the request to service it properly. Maybe we look at the IP and do something different based on a coarse geolocation. Maybe we look at cookies, not just for forwarded authentication but even just for display preferences. Maybe we look at user-agent and fetch a smaller dataset if it's a phone than if it's a desktop. OK these examples are a bit contrived, it is 99.9% about forward auth.

So I think what SvelteKit wants you to do is use hooks to examine the low-level details of the request and turn it into higher level details that go in locals. So I can use handle() and put stuff in event.locals, but unless I'm missing something, there's no way to get that from load(), you can only get that from a data endpoint.

One workaround that is serviceable is to put whatever you need in session. But then it leaks out to the client (clearly visible with cmd-U show source), when it's actually only needed server-side.

So I'd propose simply plumbing locals through into the load() functions.

Describe the proposed solution

See above.

Alternatives considered

See above. Edit: See below, maybe just update externalFetch?

Importance

would make my life easier

Additional Information

No response

johnnysprinkles commented 2 years ago

Or, what might actually make more sense is to make "incoming_request" available to externalFetch.

johnnysprinkles commented 2 years ago

A one-liner here would do it (verified locally): https://github.com/sveltejs/kit/blob/ee906a3b2b103e4523eb1b6bb13f04d03eb35238/packages/kit/src/runtime/server/page/load_node.js#L204

Changed to:

response = await options.hooks.externalFetch.call(null, external_request, event);

Also I think this might end up resolving several issues related to server-side data fetching for people who can't or don't want to have their API as a subdomain of their website.

Conduitry commented 2 years ago

locals not being sent to load is by design. locals is intended for things that you don't necessarily want available to the browser, and everything passed to a load function would also be sent to the browser. If you want to expose certain data to a load function, return it from getSession.

I'm not sure what passing event to externalFetch would specifically address, but having it be called differently from load and from other fetches sounds odd to me.

johnnysprinkles commented 2 years ago

That makes sense, SvelteKit does try to present a symmetry between client-side and server-side. If we had locals or even the whole request event available in load(), on the client it would just get null or empty object which might be weird.

Still though, what about externalFetch? It's one of the few places we break the client/server symmetry and seems like the right place. I'll re-title this issue as "Make request event available to externalFetch()".

The main thing this would allow is forwarding auth cookies, such as:

export async function externalFetch(request, event) {
  let cookies = cookie.parse(event.request.headers.get('cookie') || '');
  request.headers.set('cookie', `JWT=${cookies.JWT}`);
  // Or blindly send all cookies...
  // request.headers.set('cookie', event.request.headers.get('cookie'))
  // Or pass auth a different way
  // request.headers.set('authorization', `bearer ${cookies.JWT}`);
  return fetch(request);
}

This provides an out if the automatic cookie passthrough doesn't fit your needs (which it won't if you have an API that's not a subdomain of your website, or if you occasionally want to point your dev frontend at a prod backend, etc). And I think it may end up resolving #3160 . Not sure about #672 and #1777.

johnnysprinkles commented 2 years ago

Ah, reading the docs more I think the expected thing is, if you wanted to use APIs on another domain you'd create endpoints for that. It sounds tedious but maybe with rest params you could make it a single endpoint. It would add a network hop though, if you used that endpoint on the client as well as the server which seems to be what's intended. Also, you'd need to make up a path hierarchy that doesn't collide with the paths used for pages, I guess generally by using /api/[...] and stripping the api prefix to send the rest of the path to your actual API.

I mean really it seems simpler to just allow things to go direct to the API (via node-fetch) in the server-side case, and also direct using CORS in the client-side case (which we can already do). Curious if anyone else would want this.

johnnysprinkles commented 2 years ago

But back to that automatic cookie passthrough logic... the fact that we even have that suggests it's ok to call APIs where they lie, instead of shuttled through a local endpoint.

I can see it's emulating what would happen for a client-made fetch call, the only problem is it's emulating what would happen for the "did nothing for CORS" case. So now, the client fetch calls are actually more capable than the server ones because they can call any domain with cookies and auth. This seems incongruous.

Why don't we simply support {credentials: 'include'} like the client would, support it in spirit anyway. Well, more like "abuse" it the same way Svelte abuses labeled statements. It would be a small code change like this, I'd be happy to send a PR.

johnnysprinkles commented 2 years ago

Hmm, #3631 didn't end up including credentials headers (cookie, authorization) after all, so re-opening this.

johnnysprinkles commented 2 years ago

I do see #835 but that only deals with internal fetch calls, maybe I'm missing some discussion on why they decided not to do external as well?

Conduitry commented 2 years ago

The fetch that server-side load gets tries to simulate what the browser's own fetch would do, based on what it has available to it.

If the cookies live on your app's domain, server-side load fetch won't pass them along to another domain they're not for. This would be a big security issue, and isn't how browsers behave.
If the cookies live on that external domain, the server wasn't even sent them to begin with, so it can't forward them on to that domain in SSR. This is a limitation of the definition of SSR, how browsers behave, and the cookie spec, and isn't something that SvelteKit can fix.
We don't want to make the whole request object available in load, as that (among other things) defeats the purpose of HTTP-only cookies that can't be accessed in JS.

If you want the cookies to live on your app's domain, you can use a local endpoint to forward them to the remote domain, so that this will work both on the server and on the client. If you want the cookies to live on the external domain, you won't be able to use SSR for this request, and you will need to rewrite this portion to always use a client-side fetch, without the load function.

johnnysprinkles commented 2 years ago

Cool, leaving as closed but going to re-title to previous title.

Just to respond to a couple points... If the client sends a cookie to the server the server can do whatever it wants with it, including using it to make external service calls. That's not a security issue at all. But I can see how doing that with credentials: include would make it way too automatic and easy to do inadvertently. So really exposing the event as a second parameter to externalFetch seems like the best option.

No problem for me at the moment since when I run local I have my website on localhost:3000 and my API on localhost:4000, and remote I have api.mysite.com and mysite.com. If I ever moved my API to not be a subdomain then this would be an issue and the options currently are:

Put it in session and leak it to the client
Make an endpoint intermediary
Fork SvelteKit

johnnysprinkles commented 2 years ago

OK, I've thought a little more about this... my case might be usual because I'd be setting the same auth cookie on my frontend domain and my backend domain. That way ajax calls will send it to mybackend.com, but it also reaches SvelteKit node service on myfrontend.com, and from there I'd want to use it on upstream fetch() calls in my load() function. This might be an esoteric use case. And also this might all be moot anyway if third party cookies are going away. I'm satisfied on closing this issue.

f5io commented 2 years ago

I'm having a similar issue to this, I would like to use the load fetch to pass credentials to my auth service for validation... I have 2 subdomains:

auth.domain.com
dashboard.domain.com

The auth server issues a cookie with the domain set to .domain.com, however because of the follow code:

https://github.com/sveltejs/kit/blob/22a5e40d04a255eea40a5bdba375388f02d8c319/packages/kit/src/runtime/server/page/load_node.js#L216-L233

The cookie is left out when calling from the server side. This is not exactly the same issue as @johnnysprinkles, however I feel like my use-case is pretty standard. Could we make the code above slightly more clever by:

getting the cookie outside of the if block
if the cookie exists and credentials need to be included, potentially parse out the domain
use the parsed cookie domain as a precursor to the existing check

This is kind of what I'm thinking...

const cookie = event.request.headers.get('cookie');

if (cookie && opts.credentials !== 'omit') {
  const domain = cookie.match(/domain=([^;]+)/i);
  if (
    domain &&
    `.${new URL(requested).hostname}`.endsWith(domain[1]) ||
    `.${new URL(requested).hostname}`.endsWith(`.${event.url.hostname}`)
  ) {
      uses_credentials = true;
      opts.headers.set('cookie', cookie);
   }
}

I'm happy to PR this change if necessary, but I want confirmation that I'm not expecting something which isn't idiomatic

Edit: This obviously falls down in the case of multiple cookies, which I failed to realise 🤦

Edit 2: I got round this by, as @Conduitry suggested, creating a proxy endpoint on dashboard.domain.com and forwarding the request with cookies onto auth.domain.com

johnnysprinkles commented 2 years ago

Thinking a little more about this I'm back to my original thought... if the request event is available to load() that code can use request data however it wants to form outgoing requests. I don't think we'd want to do anything automatically to ferry cookies across into fetch or even to do anything in externalFetch since that would apply to all outgoing calls. I think we'd want to make it very explicit, as in my initial description.

I still think it's a good idea to add event as a parameter to load(). Yes, the client would see null there but you can't completely hide the fact that client and server are different.

bertybot commented 2 years ago

The problem I am having with my project, is that we have a load balancer at the beginning of the request that sets the authorization header from an auth service that is valid for the life time of the request. This token is set in the authorization header and is valid for every microservice behind the load balancer including the Svelte servers.

I thought it would be nice if when we were server side rendering instead of having to go through the load balancer again and reauthenticate we could just pass the Auth header to the externalFetch. But, Svelte does not pass the Auth Header at all. The hack I have used to get around it is setting a temp header in handle and resetting it to Auth in external fetch like so.

export const handle = async ({ event, resolve }) => {

    event.request.headers.set('Temp-Authorization', event.request.headers.get('Authorization'));

    const response = await resolve(event);
    return response;
};

export async function externalFetch(request) {
    request.headers.set('Authorization', request.headers.get('Temp-Authorization'));
    return fetch(request);
}

I know there is a security risk in the fact we are sending the token with every external request but, is that the only risk? I totally get that this is probably why Sveltekit doesn't pass on those headers, and that we have a weird use case. I just wish there was a way I could configure what headers are passed to external fetch with like a white list policy or something based on what endpoints we have.

johnnysprinkles commented 2 years ago

Oh right, I think Rich did make it include all the incoming request headers on all outgoing requests via fetch (except for the auth ones). This seems weird to me honestly, seems like you'd want to explicitly set the headers on the services you call. Another thing in the name of "try our damnedest to main the illusion of symmetry between client and server."

Cool hack though, better than putting it in session and leaking it to the client. You could do it conditionally based on the destination URL even.

bertybot commented 2 years ago

@johnnysprinkles Thanks dude. Totally agree we have a weird use case. Was just trying to think of a robust solution to my problem that wouldn't make Sveltekit worse but it probably would lol. Was thinking maybe a policy config, or we could pass all headers if it's a subdomain like we pass the cookie.

Probably just going to have to stick with my hack for the time being. We are definitely going to make it conditional on the URL, we just have the concern of more junior devs or a bug accidentally exposing it to the wrong API. Guess that's what PR reviews are for tho lol.

engintekin commented 2 years ago

Having the same problem here. Passing the cookie to session feels like a decent workaround. As a side effect, we end up sending the cookie back to the browser in the session, but I don't see an issue with it.

// inside hooks.js
export const getSession: GetSession = (event) => {
  const user = event.locals.user;
  if (user) {
    return {
      user: {
        ....
      },
      cookie: event.request.headers.get("cookie"),
    };
  }

  return {};
};

// inside component
<script context="module">
  export const load = async ({ params, fetch, session }) => {
    const response =  await fetch(`${HOST}/api/resource/${params.id}`, {
      headers: {
        cookie: session.cookie
      }
    })

    if (response.ok) {
      return {
        props: {
          res: await res.json()
        },
      };
    }

    return {
      status: res.status,
      error: new Error(`Could not load ${params.id}`),
    };
  }
</script>

si4dev commented 1 year ago

The load function in page.js could get the jwt token via page.server.js. Because page.server.js does received the cookie and is able to pass data to page.js. This is document at https://kit.svelte.dev/docs/load#input-properties-data. Be careful with this as this data will also be available in the browser. For the JWT token from this question it is intended to be public as the browser will also use the token to access the API directly. The page.server.js would look like:

/** @type {import('./$types').PageServerLoad} */
export async function load({ cookies }) {
  return {
    token: cookies.get('token')
  }
}

and the page.js receives the token in the data attribute. Based on the code from the question:

export async function load({ fetch, data }) {
    const {token} = data;
    let response = await fetch('https://api.example.com/mylatest', {
      credentials: 'include',
      headers: {
        cookie: 'jwt_token='+token,
      },
    });
}

In my own case I've used it to fetch my graphql api with a Bearer token. In my case the headers will be like:

headers: { 
      Authorization: `Bearer ${token}`,
    },

Just in case somebody was searching for this authorization method.

si4dev commented 1 year ago

I've posted a solution earlier to get the JWT token via the page.server.js. However this will exploit the JWT to the browser via hydration. It might not be a issue but I preferred to search for a better solution.

And I've found a cleaner solution. It will use the handleFetch on the server in hooks.server.js to passthrough the cookie for fetch used in load() on the server. And it will use document.cookie to passthrough the cookie for fetch on the browser in case of subsequent data retrieval.

The handleFetch is documented at https://kit.svelte.dev/docs/hooks#server-hooks-handlefetch

For src/hooks.server.js:

import cookie from 'cookie';

/** @type {import('@sveltejs/kit').HandleFetch} */
export async function handleFetch({ event, request, fetch }) {
  const { token } = cookie.parse(
    event.request.headers.get('cookie') ?? ''
  );

  const validUrls = ['http://127.0.0.1:5070/api', 'https://example.com/api']
  if (token && validUrls.includes(request.url)) {
    request = new Request(request,
       headers: {
          cookie: 'jwt_token='+token,
        },
    );
  }

  return fetch(request);
}

And the load function from the original post of the author will be:

import cookie from 'cookie';

export async function load({ fetch }) {

  let token = undefined;
  const browser = typeof document !== 'undefined';

  if (browser) {
    const cookies = cookie.parse(document.cookie);
    token = cookies && cookies.token;
  }

  let response = await fetch('https://api.example.com/mylatest', {
    credentials: 'include',
    headers: {
      cookie: 'jwt_token='+token,
    },
  });
}

Somewhere on the client or server you will need to set the cookie JWT token. For svelte kit it will probably be in an action login page.server.js

/** @type {import('./$types').Actions} */
export const actions = {
  default: async ({ cookies, request }) => {

  .... login ...

    cookies.set('token', token, {
      maxAge: 12 * 60 * 60, // seconds
      path: '/',
      httpOnly: false,
    });
}

sveltejs / kit