feat: Option to allow regenerate() to preserve old session

[mccleanp]

Add preserve option to session.regenerate() function for use cases when it is desirable to regenerate the session, but not destroy the old session immediately. For example when there could be multiple ajax requests inflight with the old session id.

Existing functionality is preserved if the option is omitted. Existing test was improved to cover session destuction and new test was added for the preserve case.

This was motivated by the desire to implement middleware to provide session renewal, as recommended by OWASP

[dougwilson]

Nice, I think this is a great feature to add and make sense to me. I really only have two asks:

1. We'll need to get documentation added to the README.
2. I feel like the API is a little weird, and generally feel like APIs that take a Boolean as a random argument is hard to remember that a true means without looking it up and also it's even odder to have arguments appearing after a callback argument. I would think maybe the API be req.session.regenerate({ preserve: true }, callback)

[mccleanp]

I have pushed another commit to address the comments from @dougwilson. Let me know if you want me to squash the commits.

[dougwilson]

Hi @mccleanp thanks for updating the PR! What ever happened to the preserve: true above? I don't remember you mentioning that you would be implementing it a different way. The reason I ask is because usually in the JS sense, an option that defaults to false is easy to understand, since null and undefined is falsy, which people associate with default values. This implementation defaults the option to true.

[mccleanp]

@dougwilson: I am happy to change it back to preserve: true if you prefer. I had selected destroy: false simply because the destroy terminology was already used, while preserve is introducing new terminology.

[dougwilson]

Yea, if you have a better idea for the terminology, that's great, just the key constraint I'm trying to keep here is that the default value should be false, not true, for better mental mapping of undefined / null.

[dougwilson]

I just rebased the commits. I'm going to work on cleaning it up further to merge 👍

[max8hine]

I currently have a problem is that the previous sessionStore got delete after run regenerate() function.

Not sure is it could be solved with this PR. but I think to let the old sessionStore expire itself by cookie.maxAge make more sense.