Skip to content

add getDomainFromRequest option, for when domain needs to be calculated #744

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 12 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -182,6 +182,18 @@ app.use(session({
}))
```

##### getDomainFromRequest
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe a shorter name for the function, getDomain?


Optional function to override the `Domain` `Set-Cookie` attribute.
Provide a function that returns a string that will be used as the
cookie domain. The function is given `req` as the first argument if
you want to use some value attached to `req` when generating the
domain (as for `genid`). This can be useful if the same app is
accessed from different families of domains, some of which should
share cookies.

If this function is not provided, `cookie.domain` is used instead.

##### name

The name of the session ID cookie to set in the response (and read from in the
Expand Down
13 changes: 13 additions & 0 deletions index.js
Original file line number Diff line number Diff line change
Expand Up @@ -114,10 +114,17 @@ function session(options) {
// get the cookie signing secret
var secret = opts.secret

// get a function for computing the domain from the request
var getDomainFromRequest = opts.getDomainFromRequest

if (typeof generateId !== 'function') {
throw new TypeError('genid option must be a function');
}

if (getDomainFromRequest !== undefined && typeof getDomainFromRequest !== 'function') {
throw new TypeError('getDomainFromRequest option must be a function');
}

if (resaveSession === undefined) {
deprecate('undefined resave option; provide resave option');
resaveSession = true;
Expand Down Expand Up @@ -160,6 +167,12 @@ function session(options) {
req.session = new Session(req);
req.session.cookie = new Cookie(cookieOptions);

if (getDomainFromRequest) {
// If a function was specified for computing the domain
// from the request, then use it now.
req.session.cookie.domain = getDomainFromRequest(req);
}

if (cookieOptions.secure === 'auto') {
req.session.cookie.secure = issecure(req, trustProxy);
}
Expand Down
31 changes: 31 additions & 0 deletions test/session.js
Original file line number Diff line number Diff line change
Expand Up @@ -891,6 +891,37 @@ describe('session()', function(){
});
});

describe('getDomainFromRequest option', function(){
it('should reject non-function values', function(){
assert.throws(session.bind(null, { getDomainFromRequest: 'bogus!' }), /getDomainFromRequest.*must/)
});

it('should work without getDomainFromRequest', function(done){
request(createServer())
.get('/')
.expect(shouldSetCookie('connect.sid'))
.expect(200, done)
});

it('should allow custom domain', function(done){
function getDomainFromRequest(req) { return '.do.main' }

request(createServer({ getDomainFromRequest: getDomainFromRequest }))
.get('/')
.expect(shouldSetCookieWithAttributeAndValue('connect.sid', 'Domain', '.do.main'))
.expect(200, done)
});

it('should provide req argument', function(done){
function getDomainFromRequest(req) { return req.url }

request(createServer({ getDomainFromRequest: getDomainFromRequest }))
.get('/foo')
.expect(shouldSetCookieWithAttributeAndValue('connect.sid', 'Domain', '/foo'))
.expect(200, done)
});
});

describe('key option', function(){
it('should default to "connect.sid"', function(done){
request(createServer())
Expand Down