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

Built-in method of refreshing session from server periodically #1726

Closed
rmunn opened this issue Jun 21, 2021 · 11 comments
Closed

Built-in method of refreshing session from server periodically #1726

rmunn opened this issue Jun 21, 2021 · 11 comments
Labels
feature request New feature or request
Milestone

Comments

@rmunn
Copy link
Contributor

rmunn commented Jun 21, 2021

In #1165 (comment), Rich Harris wrote:

I almost wonder if SvelteKit should have some built-in stuff for refreshing the session from the server periodically (including when you refocus a page, swr style). #46 included some thinking along those lines.

This seems like a good idea, and it also seems to be a different feature than #1165 or #46. So I'm opening this issue to track discussion of a session-refresh feature, so this idea doesn't get lost. (I don't have any specific ideas for what the feature should look like, I'm just opening this issue so there's somewhere to discuss the idea, and hoping that other people will have a better idea of what they would want in a session-refresh feature).

@ignatiusmb ignatiusmb added the feature request New feature or request label Aug 23, 2021
@rmunn
Copy link
Contributor Author

rmunn commented Sep 6, 2021

I've thought about this and the design that @Rich-Harris sketched out in #46 (comment), and here's the design for session persistence and refreshing that I think would be best:

Session persistence

  • There's a $session.persist() method on the client-side session store
  • $session.persist() has to be called explicitly, because there are plenty of use cases where session data would be updated in a way that shouldn't be persisted yet, and the page author wants to control the timing of session persistence
  • If auto-persisting sessions is desired, it's as simple as subscribing to the session store with a function that calls $session.persist().
  • When $session.persist() is called, it sends a POST to /__session (that URL would be configurable in svelte.config.js)
    • I prefer __session (rather than __session__ which @Rich-Harris originally proposed) for its similarity to __layout.svelte and __error.svelte
  • The contents of the POST are a JSON representation of the session object, with anything non-serializable stripped out
    • In particular, functions would be stripped out by $session.persist(), so that Svelte-Kit users can add functions to their session stores if they need to, without having to worry about overriding $session.persist() if they do so
    • Dates would be serialized in the format returned by Date.toJson (e.g., '2021-06-15T12:34:56.789Z'), which should be obvious but is worth saying explicitly
  • There is a hook in hooks.ts (or .js) called persistSession() which receives the POST contents (deserialized into a Javascript object)

Session refreshing

  • There would also be a $session.refresh() method available on the session store
  • $session.refresh() would trigger a POST to /__session_refresh (URL configurable in svelte.config.js), passing it the same JSON that persistSession() would receive (e.g. functions stripped out before serializing)
  • That POST would call a refreshSession() hook if it exists; if there is no refreshSession() hook defined, then $session.refresh() calls getSession() without passing it the POST body
  • $session.refresh() will take the value returned by either refreshSession() or getSession(), copy any functions (but only functions) from the old session object to the new session object, and then assign the new session object to the store (e.g. any session subscribers get called after functions are copied over)
  • Any load() functions that depend on session (they access the session property on their parameter) will be re-run when $session.refresh() is called, if (and only if) the value of the session has changed
  • By default, $session.refresh() is called whenever the user refocuses the window, e.g. when visibilitychange is fired. Note that some libraries also listen for the focus event on the window to detect when the user comes back to the page, as some older versions of Safari may not fire visibilitychange correctly
    • This can be configured in svelte.config.js, as some use cases might not want session auto-refreshing and would only want it to happen when they explicitly call $session.refresh()
    • Other use cases might want more aggressive session refresh, such as automatically logging you out of all tabs when you log out of one tab. For those cases, it might be useful to establish a websocket connection to the server so that the server can push a session refresh to the client
    • Another option would be to have open tabs polling the server every so often for any session refresh. This is less good than websockets as client polling produces far more traffic than server push, but in some use cases websockets might not be practical. This, too, would be configurable in svelte.config.js

@korywka
Copy link

korywka commented Dec 23, 2021

Example for this feature: Session cookie:

  1. User goes to /login page
  2. Login pages handle some sign in, for example, oauth, fetches user
  3. We store user in session cookie
  4. We write getSession to parse session cookie and store user in memory
  5. User is redirected to home page
  6. All works ... but:
  7. User clicks Sign Out button
  8. User is redirected to /logout
  9. We remove session cookie
  10. We redirect user to Home page.

BUT! Out session store is not cleared until we refresh page! Because redirect from /logout to Home page is not server-side.

@korywka
Copy link

korywka commented Dec 23, 2021

The only way to reactively change session at server is something like this:

<script>
  import { onMount } from 'svelte';
  import { session } from '$app/stores';
  import { goto } from '$app/navigation';

  onMount(() => {
    session.update((obj) => {
      obj.user = null;

      return obj;
    });

    goto('/');
  })
</script>

Or to trigger SSR, which is worse solution. I wish getSession was as reactive, as handle hook. Not just at first SSR.

@korywka
Copy link

korywka commented Dec 23, 2021

BTW, @rmunn what is your opinion on refreshing session (calling getSession) on each request, not just SSR? Will it cover this issue?

@babichjacob
Copy link
Member

Example for this feature: Session cookie:

1. User goes to `/login` page

2. Login pages handle some sign in, for example, oauth, fetches user

3. We store user in session cookie

4. We write `getSession` to parse session cookie and store user in memory

5. User is redirected to home page

6. All works ... but:

7. User clicks Sign Out button

8. User is redirected to `/logout`

9. We remove session cookie

10. We redirect user to Home page.

BUT! Out session store is not cleared until we refresh page! Because redirect from /logout to Home page is not server-side.

Update the session store on the /logout page.

That could be as simple as $session = {} yourself or you can make the sign out endpoint return the value of the new session:

const response = await fetch('/sign-out', {
	method: 'POST'
});
$session = await response.json();

@korywka
Copy link

korywka commented Dec 24, 2021

@babichjacob it is impossible, because logout page have no content:

<script context="module">
  import { session } from '$app/stores';
  import { requestJSON } from '$lib/request.js';

  /** @type {import('@sveltejs/kit').Load} */
  export async function load({ fetch }) {
    await requestJSON(fetch, '/auth/logout.json');

    session.set({ user: null });

    return {
      status: 302,
      redirect: '/',
    };
  }
</script>
500
Can only set session store in browser
Error: Can only set session store in browser

It would be such cool thing to update session server-side. I hope that we don't need then any methods to refresh/sync session client-side.

@babichjacob
Copy link
Member

babichjacob commented Dec 24, 2021

@korywka Like the error suggests, you just have to wrap that session.set call with if (browser). I tested this locally and it works.

<script context="module">
  import { browser } from "$app/env";
  import { session } from "$app/stores";
  import { requestJSON } from "$lib/request.js";

  /** @type {import('@sveltejs/kit').Load} */
  export async function load({ fetch }) {
    await requestJSON(fetch, "/auth/logout.json");

    if (browser) session.set({ user: null });

    return {
      status: 302,
      redirect: "/",
    };
  }
</script>

When /logout is requested from client-side navigation, the store will be updated manually to reflect the "empty" session.

When /logout is requested directly from the server, assuming you're using cookie-based auth and the /auth/logout.json endpoint deletes the cookie, getSession will be called on the server after the / redirect so that the page reflects the new session.

@Zachiah
Copy link

Zachiah commented Apr 7, 2022

@korywka I get the following error when trying to update the session in load...
image

@MarcusCemes
Copy link
Contributor

@Zachiah I was getting the same error, the error message is misleading. You're trying to update the global store that has nothing in it, you need the contextual one that is associated with your mounted component tree. As Google leads here, I'll post a solution/explanation that might help someone.

$app/stores are contextual, there can be many of them at one time. I imagine that during SSR SvelteKit can process multiple client requests concurrently, hence why you can't just use the global session store. What you need is the session instance that is associated with a particular request, SvelteKit gives you three ways of obtaining it (on the client, I guess it doesn't matter whether you use a contextual or global store, but for the sake of SSR, it's easier to use the same approach for both client and SSR).

You have to either 1) use the session object that is passed into the load function (it's not a store, as this may have to run during SSR), 2) subscribe to the session store using the $ syntax during component initialization or 3) get the correct instance of the session store using getStores() synchronously during component initialization, which you can then pass to asynchronous functions such as on:click handlers.

Also, I don't think you should be updating stores from within load, this might work on the client, even though it's the wrong store, but it will almost definitely leak user information during SSR when multiple requests are being handled. You should either take a look at the getSession() hook or passing that down through stuff and then populate it in the browser.

@maximedupre
Copy link

I did this. Not sure if it's the best solution but it works 🤷🏻‍♂️

// src/routes/session.ts

import { getSession } from '../hooks';
import type { RequestHandler } from './__types/session';

export const get: RequestHandler = async (e) => {
    return {
        status: 200,
        body: getSession(e) as any,
    };
};
// src/routes/index.svelte
// right after logging in/setting the cookie on the server

const sessionRes = await fetch('session');

$session = await sessionRes.json();

@rmunn
Copy link
Contributor Author

rmunn commented Aug 17, 2022

This is probably moot now that #5946 has removed the special $session store. (It can now be implemented quite easily by anyone who needs it, using event.locals and a +layout.server.ts file). So I'm closing the issue; if the decision to remove the session store gets reversed, we can reopen the issue if/when that happens.

Meanwhile, the design I sketched out in #1726 (comment) can probably be implemented in user space now: create a handler to handle a /api/session_refresh URL, and add a method or two to the custom $session store that you create which will send a GET or a POST to that URL. Then put an event handler in your root layout that watches for visibilitychange and/or focus events and refreshes the session accordingly.

@rmunn rmunn closed this as completed Aug 17, 2022
@rmunn rmunn mentioned this issue Aug 17, 2022
5 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature request New feature or request
Projects
None yet
Development

No branches or pull requests

8 participants