Add support for Secret function

[DavidTPate]

This PR adds the ability for a function to be specified for the secret option. When a function is passed it is called with req passed to it so that the secret can be varied based upon an incoming request. Additionally, this adds corresponding tests and documentation.

The reason for this is that for my implementation I need the ability to vary my secret based upon an incoming request. In my case I'm wanting to reduce my resource usage (memory & server) by keeping multiple express-session instances in memoized and in memory and then manually changing them based on the request.

Here's a trivial example of what I'm having to do right now:

var memoizedSessions = {};
app.use(function (req, res, next) {
    if (!memoizedSessions[req.client]) {
        memoizedSessions[req.client] = session({
            secret: 'keyboard cat' + req.client
        });
    }

    memoizedSessions[req.client](req, res, next);
});

For this implementation I started off with the simplest path that I could see to getting the changes in there. The only other method I saw was that I could update the functions to pass req around, but I don't think that will result in more performance but instead just passing around of the variable to multiple functions.

[dougwilson]

The way secrets work with HMACs, adding something that is user-controlled does not gain anything. Can you provide an example that is more true to a real use-case in the documentation and tests?

[DavidTPate]

Will do.

[dougwilson]

The error "secret option array must contain one or more strings" is now misleading. Would you be willing to make the following changes:

1. Do not accept req.secret to be a function.
2. Only accept a single function for the secret instead of letting any element in the array be a function? The function can return an array if it wants to provide multiple.

[DavidTPate]

Both sound good to me. I only added support for the function in an array to try and be consistent.

[DavidTPate]

@dougwilson Thanks for the feedback, I incorporated it into the changes and squashed the commits. Let me know if you have other desired changes or feedback.

[DavidTPate]

@dougwilson Any updates on this? Been looking forward to using this on my projects instead of having to memoize a bunch of express-session instances and having many connections to my redis nodes which aren't needed.

[dougwilson]

Hi @DavidTPate , sorry for the delay. I'll check it out tonight :)

[dougwilson]

and having many connections to my redis nodes which aren't needed.

You know, you can just share the same store instance between all the express-session instances :)

[DavidTPate]

@dougwilson No worries, thanks!

You know, you can just share the same store instance between all the express-session instances :)

I do that right now, but there's a larger number of connections that occur than I'd anticipate when adding many instances of express-session. For ~20 instances of express-session we're seeing multiple connections go on (as well as a large number of event listeners, for obvious reasons). I've been digging into the multiple connections piece more as it seems to be an issue with connect-redis but haven't found anything concrete yet. We see a similar issue with connect-memcached too when using each as our session stores.

[dougwilson]

I'm not able to reproduce your issue with connect-redis. If you are seeing this across all those stores, it sounds like an issue with your code. Are you sure you are actually using the same store instance and not newing it up every time you new up a session middleware?

Also, the documentation for this is still not helpful and very confusing. Can you please update it to provide a real-world use-case? People love to copy the examples, so it is important they make sense. Right now your example is building a secret from something in the request, which completely violates the entire premise of a server-only secret.

[DavidTPate]

@dougwilson Yeah, from digging into what I'm seeing with connect-redis it makes sense when I dig into everything. What's happening is that I have to instantiate express-session for each of my unique secrets, this instantiation adds 2 event listeners for each time I instantiate it to my session store. From looking through the modules, this makes sense as they aren't really designed with the use of multiple secrets (and multiple instances as I have to right now) in mind.

As for the documentation, I'll get a more concrete example of using something from the client to resolve something on the server. I didn't want to make it complex so I just kept it simple. But I'll put together a mock of how we do it. In this case, I'll use the subdomain to resolve a secret key which is stored server side.

[dougwilson]

Hi @DavidTPate , and update on this?

[DavidTPate]

@dougwilson Sorry for the delay, as I mentioned in the other issue my workload has been crazy as of late.

I'm really having some trouble in coming up with a real-world use case for this which someone can copy and paste as you mention (I know many people do this). My example was very simple on purpose because I didn't want to make assumptions on how someone would use this.

In my case for example, I'm using this feature so that I can determine the client that a user is connecting for and retrieve the appropriate key from our key store to decrypt the session. The reason for this is that sessions on my system are not global (so logging into one tenant doesn't allow your session to be used for another). So one client might login at google.example.com while another logs in at microsoft.example.com, with these the session should not be able to be retrieved for another tenant (I've typically seen this referred to as Single instance, multi-tenant model).

Simply setting the domain that the cookie exists at would allow this to be restricted during normal use if we depend solely on the browser, but not for a other use where someone is manually setting the cookie from another domain on their requests.

As I mentioned in the other issue, you could see a similar model in like Securing Session Tokens or Tomcat via Host Paths.

That said, I'm not sure what type of example I could put together that would be ready for copy and paste, especially since this is more of an edge case in terms of how multi-tenant systems are created. If you have some particular example in mind (or another way to achieve such functionality) I'd love your opinion.

[DavidTPate]

@dougwilson I went through and added a very primitive key rotation controlled fully server side via a setInterval call to both the docs and the tests. Please let me know if that is more what you are thinking.