Provide session store in @sapper/app

[thgh]

I'm probably missing something, but this makes it much easier to handle sessions. Right?

[thgh]

So it turns out this allows access to (probably sensitive) session data from anywhere in the server. I would argue that stores.page could also contain sensitive data.

I tried leaking session and page data and indeed it's easy. Too easy. So I definitely agree that session data should not be readable from anywhere but the request itself.

What if the session and page data would be reset before and after the page renders? Render is synchronous so I think no other code can do a drive-by and steal any data.

// server.mjs:2330
stores.page.set({
    path: req.path,
    query: req.query,
    params: params
});
stores.session.set(session);
const { html, head, css } = App.render(props);
stores.session.set(false);
stores.page.set(false);

To block any subscribers from listening where they shouldn't, we must unsubscribe all listeners on every set():

import { writable } from 'svelte/store';
import { noop } from 'svelte/internal';

export const stores = {
	session: process.browser ? writable(false) : frozen(false),
	preloading: writable(false),
	page: process.browser ? writable(null) : frozen(null)
};

export const CONTEXT_KEY = {};

export const preload = () => ({});

function frozen (value) {
	function reset(newValue) {
		value = newValue;
	}

	function subscribe(run) {
		run(value);
		return noop;
	}

	return { reset, subscribe };
}

The new drawback is that you cannot change the session during initialisation.

I have tested this code and I can't break it as easily.

[thgh]

It's still possible to override stores.session and undoing the protection.

export const session might fix that, or Object.freeze

[thgh]

Updated so that preload runs again when the session is updated.

[Rich-Harris]

Thanks for this — finally getting round to merging PRs ahead of the v3-friendly Sapper release.

I definitely much prefer the ergonomics of this approach. Makes it much easier to e.g. have a login helper outside the component hierarchy. My one hesitation in all this is it basically commits us to always doing sync SSR, even if we support async or streaming SSR in future. Is there a way to reconcile the two?

In any case, should frozen return noop set and update functions, since session is intended to be writable just in case someone does have session.set(...) in their init code? (Or functions that throw 'you can't set session data during initialisation' errors?)

Also, would it be taking things too far to replace reset with a Symbol that the middleware had access to? Or does that not really change anything?

[Rich-Harris]

Now that I think about it, the same logic applies to the page store — if in some hypothetical future we have an async rendering mode, if page is shared between everybody then at some point it will be incorrect.

So as I see it our choices are:

• Commit to always doing SSR synchronously once data has preloaded, and sharing page and session stores between everybody, safe in the knowledge that nothing can leak. This provides the best ergonomics
• Use the context API to give everyone their own page and session stores. This is much more cumbersome for the user, but gives us room to adopt async rendering modes in the future without any danger of crosstalk
• Secret third option? Isolate every request somehow?

The API for option 2 would presumably be something like this:

<script>
  import { stores } from '@sapper/app';

  const { page, session } = stores();
</script>

Calling stores() outside component initialisation would throw an error.

[arxpoetica]

@Rich-Harris this second approach, while more verbose, sort of spells out better where things are coming from. (I like it, in other words.)

[Rich-Harris]

The more I think about this, the more I think that using the context API (for all the stores — page, preloading and session) is the most regret-proof approach, using the proposal above:

<script>
  import { stores } from '@sapper/app';

  const { page, preloading, session } = stores();
</script>

Does anyone have strong objections, or suggestions for a better (and safe) alternative?

[thgh]

I used sapper a lot for the past two months and now agree with the suggested approach. I built lots of stores and it turns out that they don't play well with preload. (especially async stores)

Maybe option 4 is syntactic sugar, but that would introduce another thing to learn.