session icon indicating copy to clipboard operation
session copied to clipboard

Destroying all sessions for a given user?

Open kleydon opened this issue 3 years ago • 18 comments

Hi,

I'm maintaining the express session store for Prisma (prisma-session-store).

Recently, a developer asked whether there might be a way to destroy all sessions for a given user (as might be desirable when logging out of all devices, changing a password, etc).

While this can be accomplished at the back-end application layer, it requires (I think?) downloading all sessions, and then filtering them, which might not be ideal if there are hundreds or thousands of sessions.

I'm considering adding the ability to destroy all sessions for a given user as a feature specific to the data store that I'm maintaining, but wanted to check in first, to see if something along these lines might be in the cards for the express-session library (and the session store interface it exposes) more generally...

Do you imagine this library would surface this sort of functionality in the future? Or does this seem like the sort of thing that ought to live in each specific session store implementation?

Any advice / recommendations / rationales would be much appreciated.

kleydon avatar Jan 07 '22 03:01 kleydon

@kleydon I implemented this in the past by changing the genid function. The idea is to generate a random id that we can find later i.e $userid-random(). Then in your database you can query by all keys starting with user id $userid-*.

The trick here is to overwrite the user session id once they log in.

revington avatar Jan 17 '22 14:01 revington

@revington - thanks for this!

kleydon avatar Jan 17 '22 21:01 kleydon

The solution provided by revington is definitely a good one, it will do what you expect. Though, I am not a fan of solutions that involve loops. Especially when you have a high-traffic site, you will notice problems. On top of that, having the list of sessions per user is probably not the complete functionality requested. Usually sites that offer the functionality you describe also show what kind of device the session is associated with, the login time, the IP, and other metadata.

Therefore, my suggestion is to leave the requested functionality out of session management but persist this information in the database. Upon login, the user session ID should be stored into the database together with all the metadata you want. You might think "But then you are storing data redundantly as my session ID is already in the database", but I disagree with that. You are merely storing a reference just as you would with other relations between tables.

ultimate-tester avatar Feb 13 '22 15:02 ultimate-tester

@ultimate-tester Thanks; appreciate your insight / experience on this.

kleydon avatar Feb 15 '22 19:02 kleydon

@ultimate-tester I found this thread through a Google search. Can you confirm that I understand your suggested approach correctly:

  1. When the user logs in, I store the session id in an array on the user object
  2. When the user then changes their password (or does anything else that requires all previous sessions to be terminated), I get that array out of the user database entry and iterate over it, calling session.destroy(sid) for each of them?

So I assume it is safe to store express session ids in the database (sorry for the noob question)?

florianwalther-private avatar May 07 '22 10:05 florianwalther-private

@ultimate-tester I found this thread through a Google search. Can you confirm that I understand your suggested approach correctly:

  1. When the user logs in, I store the session id in an array on the user object

  2. When the user then changes their password (or does anything else that requires all previous sessions to be terminated), I get that array out of the user database entry and iterate over it, calling session.destroy(sid) for each of them?

So I assume it is safe to store express session ids in the database (sorry for the noob question)?

Hi there, you're completely right in understanding. Session IDs are not a secret so they don't need special treatment, they get sent to the client after all (although signed, but that's to prevent people from "guessing" valid session IDs)

ultimate-tester avatar May 07 '22 11:05 ultimate-tester

@ultimate-tester Thank you. Would you additionally check on each request if the sessionId is contained in the user array? I'm a bit afraid of having orphaned sessions because the destroy call failed but the session id array was emptied out.

florianwalther-private avatar May 08 '22 08:05 florianwalther-private

@ultimate-tester Thank you. Would you additionally check on each request if the sessionId is contained in the user array? I'm a bit afraid of having orphaned sessions because the destroy call failed but the session id array was emptied out.

Good question. I currently have a quite naive approach by using user login and logout routes to store and remove the session ID from the database.

ultimate-tester avatar May 08 '22 10:05 ultimate-tester

@florianwalther-private @ultimate-tester there is no need to loop nor keep a list of ids.

The way I do this:

create a genid function that prefix id with user id

function genid(req){
  const userId = req?.session?.userId || 0;
  const randomId = /* random id without "-" char*/
  return `${userId}-${randomId}`;
}

supply this function to session when you create the session middleware.

When a user logs in just copy all session data into some variable and call regenerate this will terminate previous session and create a new one. copy old data back again to session.

How do you implement destroy all? just go to redis or any other backend and delete ${userid}-* sessions

revington avatar May 09 '22 08:05 revington

@revington Thank you, that seems like a better idea and should avoid orphaned sessions (which I think can be very dangerous). I'll try that out 👍

florianwalther-private avatar May 09 '22 16:05 florianwalther-private

@revington req is undefined when I try this. How did you set this up so the userId is available there? I put the userId in the session object at login after the credentials were verified.

florianwalther-private avatar May 10 '22 09:05 florianwalther-private

@florianwalther-private are you passing your genid function to session function? https://github.com/expressjs/session#genid You don't call genid directly. It is being called by session when you call #req.session.regenerate(). So, after a user logs in, call #req.session.regenerate https://github.com/expressjs/session#sessionregeneratecallback

revington avatar May 10 '22 09:05 revington

@revington

I call this on login:

exports.createSessionForUser = async (req, user) => {
    req.session.userId = user._id;
    const sessionData = req.session;
    await new Promise((resolve, reject) => {
        req.session.regenerate(err => {
            if (err) {
                reject(err);
            } else {
                resolve();
            }
        });
    });
    req.session = sessionData;
};

And this is my session setup:

app.use(session({
    store: new RedisStore({ client: redisClient }),
    secret: process.env.SESSION_SECRET,
    saveUninitialized: false,
    resave: false,
    cookie: {
        maxAge: 31 * 24 * 60 * 60 * 1000,
        httpOnly: true,
    },
    rolling: true,
    genid: (req) => `${req?.session?.userId || 0}-${crypto.randomUUID()}`
}))

But I always get 0 for the userId. Are you sure we can get the session content in the genid function? Have you tried it out yourself?

florianwalther-private avatar May 10 '22 09:05 florianwalther-private

@revington Nevermind, I got it to work. I misunderstood your "copy the old session data to the new session". I think my line req.session = sessionData; just overwrote the session (and hence the userId was gone again).

This is my login code now:

const match = await bcrypt.compare(password, user.password);
if (match) {
    req.session.userId = user._id;

    req.session.regenerate(error => {
        if (error) {
            res.status(500).json({ error: error });
        } else {
            res.status(200).json(user);
        }
    });
} else {
    res.sendStatus(401);
}

If I don't have anything but the userId in the session, I don't need to copy any old session data to the new session, right?

florianwalther-private avatar May 14 '22 14:05 florianwalther-private

Right!, If you use passport double check that passport itself is not creating some data

El sáb., 14 may. 2022 16:19, Florian Walther @.***> escribió:

@revington https://github.com/revington Nevermind, I got it to work. I misunderstood your "copy the old session data to the new session". My line req.session = sessionData; just overwrote the session (and hence the userId was gone again).

This is my login code now:

const match = await bcrypt.compare(password, user.password); if (match) { req.session.userId = user._id;

        req.session.regenerate(error => {
            if (error) {
                res.status(500).json({ error: error });
            } else {
                res.status(200).json(user);
            }
        });
    } else {
        res.sendStatus(401);
    }

If I don't have anything but the userId in the session, I don't need to copy any old session data to the new session, right?

— Reply to this email directly, view it on GitHub https://github.com/expressjs/session/issues/865#issuecomment-1126724964, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAAWOW4EUKCDZAPAHIR7ICDVJ6ZAZANCNFSM5LN4RFGA . You are receiving this because you were mentioned.Message ID: @.***>

revington avatar May 14 '22 14:05 revington

I'm using Express-Session. But I just noticed that the user is actually logged out after page refresh. The Redis entry with the correct userId is in the database, as well as the connect.id cookie. But they seem to not be connected anymore after calling regenerate. If I remove regenerate the user stays logged in, but of course the session prefix is 0. Any idea why that could be?

florianwalther-private avatar May 14 '22 15:05 florianwalther-private

Ok I got it now. I have to put another req.session.userId = user._id after the regenerate call. So it looks like this:

req.session.userId = user._id;
await regenerateSession(req.session);
req.session.userId = user._id;

Is this how you have it too?

florianwalther-private avatar May 14 '22 15:05 florianwalther-private