session icon indicating copy to clipboard operation
session copied to clipboard

add getDomainFromRequest option, for when domain needs to be calculated

Open paulfitz opened this issue 5 years ago • 9 comments

Thanks for express-session! Here's a small addition that is perhaps a bit niche but has been useful for us.

This adds a getDomainFromRequest option which allows cookie.domain to be computed from the request rather than set as a constant.

As an example of when this is useful, imagine serving a single app from *.foo.com and *.bar.com, with the cookie domain being .foo.com for the *.foo.com domains and .bar.com for the *.bar.com domains.

This may be related to https://github.com/expressjs/session/pull/311

paulfitz avatar Apr 22 '20 21:04 paulfitz

It seems your scenario is different from that one, and seems achievable though the use of middleware, which is what makes Express so flexible. Have you considered something like

app.use(vhost('*.foo.com', session(fooConfig)))
app.use(vhost('*.bar.com', session(barConfig)))

You can pass the same store to both if you want to store sessions in the same place.

dougwilson avatar Apr 22 '20 21:04 dougwilson

Thanks for the suggestion! I did consider something similar. In my actual case, the specific domain families are not available to the code, so the middleware needed to do a bit more gymnastics. Overall it felt clumsy, and cleaner to solve in express-session, leaving me with a single session object. However, I can see this is unlikely to be a common case, so it is fine with me if you want to close this out, I won't be offended :-)

paulfitz avatar Apr 22 '20 21:04 paulfitz

It's no problem, and I'm not rejecting it to be clear :) I'm more trying to better understand the use-case so we can get to a situation where this is generically solvable and doesn't end up where this needs to be done for every other option, and then someone needs it to be async, etc. etc. :)

There may be a clean way, but I guess I would say it's hard to work together without understanding what you're trying to accomplish. I believed I understood from your initial issue, but it sounds now that what was written there doesn't actually represent your true goals to get from this change.

Is it possible to outline them?

dougwilson avatar Apr 22 '20 22:04 dougwilson

I'd now edit:

As an example of when this is useful, imagine serving a single app from *.foo.com and *.bar.com, with the cookie domain being .foo.com for the *.foo.com domains and .bar.com for the *.bar.com domains.

to be: imagine serving a single app from several domain families (*.foo.com, *.bar.com, etc.) where the cookie domain should omit the subdomain, and the set of domain families is determined outside of the code (e.g. in dns and load balancer configuration).

Does that make any more sense? Thanks for your patience.

paulfitz avatar Apr 22 '20 22:04 paulfitz

Hi @paulfitz thank you. That does make sense. Sounds like you could use vhost to accomplish this (I just wrote it up and didn't extensively test it). The nice thing is that this works with any middleware config, and not even just this module. If this is a pattern that is useful, having a module on npm to wrap around middlewares would be super useful to everyone too.

// example lib that exports a middleware to use for your use-case:
var session = require('express-session')
var vhost = require('vhost')

module.exports = function (options) {
  var cache = Object.create(null)
  return vhost(/^(?:.+\.)?([^.]+\.[^.]+)$/, function (req, res, next) {
    var domain = req.vhost[0]
    var mw = cache[domain]

    if (!mw) {
      var opts = Object.assign({}, options)
      opts.cookie = Object.assign({}, opts.cookie || {})
      opts.cookie.domain = '.' + domain
      cache[domain] = mw = session(opts)
    }

    mw(req, res, next)
  })
}

dougwilson avatar Apr 26 '20 20:04 dougwilson

Upvote for this I have the same trouble here. I want to share sessions between subdomains, it would be really nice to have a callback domain config instead of a constant something like this

 session({
  cookie: {
    domain: (req: IncommingMessage): string | undefined => "example.com" // or whatever
   }
})

Note that this signature is fully backwards compatible as session({ cookie: { domain: "example.com" } }) still works as expected

ramiroaisen avatar Mar 20 '21 19:03 ramiroaisen

Got the same usecase now. I do not like the vhost or custom middleware solutions because they all create a new session middleware at every single request. I would prefer callback function that allows to change any cookie settings in an already created session middleware (maybe someone wants to dynamically adjust the expire time?).

uriesk avatar Sep 11 '22 11:09 uriesk