session icon indicating copy to clipboard operation
session copied to clipboard

Rolling session with absolute expiry time

Open apitts opened this issue 7 years ago • 23 comments

Is it possible to have both rolling sessions as well as an absolute expiry time? So, the session will resave and roll with a maxAge of 15 minutes but expire say 24 hours after first created?

apitts avatar Feb 18 '18 23:02 apitts

That isn't a feature currently, unfortunately. But I don't see why that can't be added if someone wanted to make a pull request to add it 👍

dougwilson avatar Feb 19 '18 19:02 dougwilson

So the cookies expire after 15 minutes but the database session expires after 24 hours no matter what?

gabeio avatar Feb 19 '18 19:02 gabeio

@dougwilson Thanks for letting me know! I did take a look and couldn't find it but it's helpful to know for certain.

@gabeio Yes, exactly. The advantage being that if a cookie is compromised for some reason then access would be limited to say that 24 hour period. As it stands, an attacker could ping the server say every 15 minutes and roll the session. An absolute timeout in OWASP terms: https://www.owasp.org/index.php/Session_Management_Cheat_Sheet.

My understanding is that both an idle and absolute timeout together is best practice.

apitts avatar Feb 19 '18 22:02 apitts

I would like to add this feature. If I am correct we have to save either the expiration or the creation date within the session object to be able to achieve this. That would mean that a name for this variable needs to be reserved within the session object and can not be used for other purposes by the user.

However, it seems to me like bad practice storing this kind of metadata within the session object and forcing the user to be careful not to accidentally overwrite it with other data.

One naive idea to solve that would be not to store the session object directly, but to wrap it into a parent object(eg: {meta: {absoluteTimeout: 12345}, data: originalSessionObject} ) which would allow us to store this kind of metadata without interfering with the actual session data.

Do you have any suggestions on that?

mauricedoepke avatar Apr 09 '18 23:04 mauricedoepke

I just submitted a PR with code changes, unit tests and readme updates for this issue. If it passes your tests, can you please merge it in?

justinwatkinsact avatar Jun 18 '18 19:06 justinwatkinsact

All tests now pass and the code coverage is right.

justinwatkinsact avatar Jun 20 '18 14:06 justinwatkinsact

Any update on this PR ? It's exactly the feature we want to integrate in our session. Which is also an OWASP recommandation.

jfbelisle avatar Oct 11 '18 15:10 jfbelisle

Any news on accepting this?

jdheywood avatar Oct 26 '18 13:10 jdheywood

I can take a look. What PR are you folks looking to get accepted (this is just an issue)?

dougwilson avatar Oct 26 '18 14:10 dougwilson

This is the issue, https://github.com/expressjs/session/pull/595

Thanks for taking the time

jfbelisle avatar Oct 26 '18 14:10 jfbelisle

I apologize I did not have an opportunity to review it over the weekend, but will early this week. I set aside some time.

dougwilson avatar Oct 29 '18 12:10 dougwilson

@dougwilson - I too have the same issue. This would be great to get in! Any update? @justinjdickow - Minor comment on PR - specifying the time in seconds rather than milliseconds is inconsistent with MaxAge (happy to have the change without this ATM thought!)

jfstephe avatar May 09 '19 09:05 jfstephe

Hi! Are there any updates on this front? it's been almost a year, was wondering if the PR above will be merged soon. Are we just waiting to change from seconds to milliseconds? Thanks!

thesofakillers avatar Apr 10 '20 10:04 thesofakillers

Thank you @thesofakillers for following up on the issue. The express ecosystem has formed a team of triagers who are fronting many of the issues to give the maintainers space to do their work. Our roll is to react to messages like yours. We are in the process of allocating more specific resources to individual parts of the eco system like the express-session module.

I have been taking a particular interest in this module and there is a plan to go back over all the outstanding PRs raised. However we have a plan of execution and will be getting to the already raised PRs over the next coming months.

On April 8th 2020 the express-session plan was discussed as part of the ExpressJS TC meeting. The video for this will shortly appear on the Express.js channel on you tube.

Thank you for your continued patience. Your issues are important and are still open.

I am noting the PR raised by @ justinwatkinsact

ghinks avatar Apr 10 '20 12:04 ghinks

@ghinks Awesome, thank you so much for your work. I wasn't aware of this triager system and am always more and more impressed by the open source community. I was surprised why this was still open so decided to comment asking for updates. I'm happy to see that it is in the plans for the next coming months. Thanks again, I'll stay subscribed to this issue to keep up to date.

PS: the PR of note is #595

thesofakillers avatar Apr 10 '20 12:04 thesofakillers

this feature would be great to see in an official release.

colmaengus avatar May 19 '20 09:05 colmaengus

Any update on this guys?

ajayaldo avatar Jul 09 '20 11:07 ajayaldo

@ghinks any update? We are close to a year since your update and this issue with a working PR is still outstanding, it would be awesome if we could get it merged.

Varedis avatar Jan 07 '21 16:01 Varedis

Apologies I'm not active on this repo any longer

ghinks avatar Jan 07 '21 16:01 ghinks

In the meantime, I have published https://www.npmjs.com/package/@varedis/express-session with the maxDurtion change if anyone needs it

Varedis avatar Jan 11 '21 09:01 Varedis

In the meantime, I have published https://www.npmjs.com/package/@varedis/express-session with the maxDurtion change if anyone needs it

Hi Varedis, I tried using this but could not use. When I checked, it's not present in latest version of express-session module.

Can you suggest if this has only been added to the documentation and not to the actual code? Or am I missing something here.

tushar-compro avatar Sep 15 '21 11:09 tushar-compro

Any update on the merge to master for this fix?

ghprud avatar Dec 18 '21 05:12 ghprud

I also have this requirement for my current project. For now, I build up my own middleware to have an absolute session expiry time. It would be nice to have this feature withing express-session.

@dougwilson @justinwatkinsact @apitts how can I support? Whats up with the open pull request?

amjmhs avatar Sep 19 '22 10:09 amjmhs