session icon indicating copy to clipboard operation
session copied to clipboard

Request: Option for refreshing the session ID

Open gk0us opened this issue 8 years ago • 18 comments

Sometimes, there is the need to refresh the session ID without loosing the session data.

Examples:

  1. Refreshing session ID after authentication (to protect against session fixation attacks) https://www.owasp.org/index.php/Session_fixation https://github.com/jaredhanson/passport/issues/192
  2. Manually refreshing session ID before it expires (e.g. if the user wants to keep working after the maximum session lifetime, but we do not want the same session ID to be used)

gk0us avatar Feb 09 '17 14:02 gk0us

I don't see any particular reason something like this cannot be added here. Perhaps as an option to the current req.session.regenerate (like {copy: true}?).

dougwilson avatar Feb 10 '17 02:02 dougwilson

I cannot see any reason why not, either. The {copy:true} option is an efficient solution.

gk0us avatar Feb 10 '17 11:02 gk0us

I quite like how the did it in PHP (I know, the horror): http://php.net/manual/en/function.session-regenerate-id.php

default is to keep the old session data

joh-klein avatar Feb 13 '17 12:02 joh-klein

How would you go about it? Just copy the whole req.session to temp and then back again?

joh-klein avatar Feb 14 '17 09:02 joh-klein

@joh-klein, I am not sure if this will work, since the old session.id will be copied back too. Correct me if I am wrong.

I've found this stack exchange solution but I was not able to figure it out. http://stackoverflow.com/a/30468384

gk0us avatar Feb 14 '17 15:02 gk0us

If I understand this correctly, id is not actually part of the session object, which correlates with my experience when I console.log(req.session)

joh-klein avatar Feb 14 '17 15:02 joh-klein

Hi @joh-klein req.session.id is indeed a thing, it's just the id property is not enumerable, so console.log just doesn't show it by default.

dougwilson avatar Feb 14 '17 22:02 dougwilson

@dougwilson , thanks for the clarification. So far, this has worked for me:

let tempSession = req.session;

req.session.regenerate((err) => {
  Object.assign(req.session, tempSession);
  res.redirect('/');
});

I have thought about deleting tempSession.cookie since in my case I am also fine with a session restart.

joh-klein avatar Feb 15 '17 09:02 joh-klein

Great idea @joh-klein using the Object.assign() function. I wasn't familiar with it. Since it only copies the enumerable properties of the session object, it does not mess with the refreshed session.id .

I have thought about deleting tempSession.cookie since in my case I am also fine with a session restart.

Can you elaborate on that? If I understand correctly, the req.session.cookie object is used to set the attributes of the cookie for the current request (e.g. path, expires, maxAge etc.).

  1. Why will the session restart if deleting this?
  2. If you really want to restart the session, why don't you just regenerate the (authenticated) session and pass the old passport object (assuming that you are using passport).

gk0us avatar Feb 17 '17 11:02 gk0us

Thanks. Yes, Object.assign() works quite well. Especially in this case.

Can you elaborate on that? If I understand correctly, the req.session.cookie object is used to set the attributes of the cookie for the current request (e.g. path, expires, maxAge etc.).

Well, my thought was, that if I copied the old cookie properties over to the new session it would use the previous settings for expires. So with restart I meant that the expiry date would be reset. But to be honest, I haven't tested that part since I am neither setting maxAge nor expires.

joh-klein avatar Feb 17 '17 12:02 joh-klein

It seems that there is no need to remove the cookie. The cookie.expires value is refreshed because of redirect(). I guess that it's the session.touch() function's doing.

Edit: Misclicked and closed the issue. Reopened.

gk0us avatar Feb 21 '17 17:02 gk0us

Please add this feature! bump!

ghost avatar May 18 '17 21:05 ghost

@Flame2057 were is the corresponding github repo?

joh-klein avatar May 23 '17 14:05 joh-klein

@joh-klein There is none. Why?

ghost avatar May 23 '17 20:05 ghost

As i am using multiple strategies of passport @jaredhanson ,so do i need to write below method (new session id generation) for every login method of strategies used or is there any other option?

passport.authenticate('local', function (err, user, info) { if (err || !user) { res.status(400).send(info); } else { req.login(user, function (err){ if (err) res.status(400).send(err); else { req.session.regenerate(function(err) { if (err) console.log(err); else{ //logic for new session regeneration //reset the cookie header with new session id //return the user object }
}) } }) } }

prabhatmishra33 avatar Jul 11 '18 18:07 prabhatmishra33

The referenced issue is still under review, is there any plan on supporting this?

mihir83in avatar Aug 11 '19 22:08 mihir83in

@omarryhan if that was stated this would have been treated different. Please follow http://expressjs.com/en/resources/contributing.html#security-policies-and-procedures to surface these types of issues.

Please contact us with your report though the appropriate channel so we can assess it. I have deleted your post for now as part of our policy and added a temporary interaction limit in the hopes you will reach out in the appropriate way regarding this, thank you.

dougwilson avatar Jul 29 '20 22:07 dougwilson

Given that this request occurred in the context of Passport, and a desire to regenerate the underlying express-session when logging in or out, I'm commenting here to let interested developers know that Passport now includes this functionality by default with the latest 0.6.0 release. Read the announcement for more info.

jaredhanson avatar May 20 '22 14:05 jaredhanson