Get/set/destroy session based on request details

[boenrobot]

I have an application that serves multiple domains ("Host" header values). Requests for each domain go to a different database, but it's still the same logic for all. I also want to store sessions for each domain in its respective database.

The current architecture of session stores doesn't allow that though. Everything about any store is set prior to requests, so one can't get/save/destroy based on the request.

This could change by adding the request as an additional argument to all required and reccomended methods, allowing stores to potentially accept config per request, and keep their existing behaviour when such a config is not supplied.

I realize this may be a breaking change for some stores (and for the rest, there wouldn't be immediate benefit, until they take advantage of it), so if implemented, a major version bump would probably be in order.

EDIT: I have an implementation at #712 that is backwards compatible with existing stores. It relies on checking the expected number of arguments in the handling function. A tactic used by some other libraries for similar purposes (notably, passport).

[SerayaEryn]

Is my assumption that you wrote your own store correct?
If yes, as an alternative you could store the host in the request.session object and read it in your store to write to the correct database.

[boenrobot]

I haven't written a custom store yet, though I was considering it. I guess your idea would work, sure, but I was hoping I could avoid custom stuff like this, because it only increases the learning curve for people joining the project.

Ideally, after this change is made into this package, I could contribute support for it in the existing stores I use, let this feature be adopted by others for the nodejs ecosystem's greater good, and my bosses would be able to have new nodejs devs become productive sooner... win, win, win.

[gireeshpunathil]

This could change by adding the request as an additional argument to all required and reccomended methods, allowing stores to potentially accept config per request, and keep their existing behaviour when such a config is not supplied.

@boenrobot - can you please elaborate this part a little more? which methods you have in mind? who will implement the logic of checking the host field and deciding the store locations etc.?

[boenrobot]

Stores wishing to support this functionality may give users a new option for a function that accepts the request, and will return an override of the global settings.

So the store is asked to get the sesssion, it passes the request to the request processing function (which will in my case check the hostname), that function returns settings that the store then uses to fetch the session data (in my case, connecting to a different DB server and a different schema). Similarly for set, destroy and touch (the other store methods that are actually called by the middleware).

A store may use whatever means it wants to improve performance even with this feature in mind, such as requring all connections to be predefined in a separate option, and then pick one based on the key that the request processing callback returns.

Think like f.e.

const connections = {
  'default': connectionDetails,
  'example.com': otherConnectionDetails,
   ...moreConnectionDetails
};
express.use(
  session({
    secret: 'keyboard cat',
    store: new DbStore({
      connections,
      useConnection: (req, op/* 'get' | 'set' | 'touch' | 'destroy' */) =>
        connections.keys().includes(req.hostname) ? req.hostname : 'default'
    })
  })
);

[gireeshpunathil]

thanks @boenrobot . How about this:

var session_map = new Map()
var store_proxy = {
  get: function (sid, cb) {
    const db = session_map.get(sid)
    if (db == null)
      cb(null, null)
    db.find({sid: sid}, (e, d) => {
      cb(null, d)
    })
  },
  set: function (sid, d, cb) {
    const db = session_map.get(sid)
    if (db == null) {
       session_map.set(sid, computedb(d.req.hostname)) // basically d is your session, and d.req is your request
       db = computedb(d.req.hostname)
    }
    db.update(sid, d)
    cb(null)
  }
}

express.use(session{store: new store_proxy(), ...})

My point is that the session object already has a handle to the request, and the store is already an interface that the user can implement, so defining a proxy like this behind the actual store, we should be able to achieve this, without making any changes to the API. Does it sound reasonable to you?

[gireeshpunathil]

of course, in the setter, we will also store back the sid / db pair into the map.

[boenrobot]

A session ID could potentially repeat across the different criteria (in my case, it could repeat per hostname), so using it to determine the connection is error prone. Sure, it's unlikely, but still.

It also doesn't help the fact that this is extra memory on the web server per opened session per node instance... In my case, I'm hoping to run the application in cluster mode. And if I need to have a remote store (e.g. redis) for the map, that's just a waste... I already use Redis as a cache for the DB, no need to duplicate it.

[ghinks]

@boenrobot thank you for raising this issue. It is if anything interesting. I'm helping out with some of the issues in this repo. I have made a comment on your PR. Please give me until the weekend to comment further.

I would however echo @gireeshpunathil that this initially looks like it can be done in the store. This would limit issues and allow for a separation of concerns.

[ghinks]

@boenrobot After looking at the PR raised and reviewing it I want to ask you who is going to benefit from this change?

I understand that it can be implemented but there will be a complexity cost and I want to understand if the cost is justified.

[ghinks]

I have contacted both the redis and mongo store maintainers. I am not sure if this new feature would be used widely.

redis store question
mongo store question