Skip to content

Commit

Permalink
Don't allow origin: '*' and credentials: true at the same time.
Browse files Browse the repository at this point in the history 
See https://www.w3.org/TR/cors/#resource-sharing-check-0
  • Loading branch information
@sgress454
sgress454 committed Sep 27, 2016
1 parent 65ff834 commit ca43e05
Show file tree
Hide file tree
Showing 4 changed files with 99 additions and 15 deletions.
10 changes: 9 additions & 1 deletion lib/hooks/cors/index.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -29,14 +29,22 @@ module.exports = function(sails) {
cors: {
origin: '*',
allRoutes: false,
credentials: true,
credentials: false,
methods: 'GET, POST, PUT, DELETE, OPTIONS, HEAD',
headers: 'content-type',
exposeHeaders: '',
securityLevel: 0,
}
},

configure: function() {
// If the app attempts to set `origin: '*'` and `credentials: true`, log a warning
// and set `credentials` to `false`.
if (sails.config.cors.origin === '*' && sails.config.cors.credentials === true) {
sails.log.error('Invalid CORS settings: if `origin` is \'*\', `credentials` cannot be `true` (setting `credentials` to `false` for you).');
sails.config.cors.credentials = false;
}
},

/**
* When this hook loads...
Expand Down
28 changes: 24 additions & 4 deletions lib/hooks/cors/to-prepare-send-headers.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -50,8 +50,11 @@ module.exports = function (sails) {
// If we have an origin header...
if (req.headers && req.headers.origin) {

var originConfig = (routeCorsConfig.origin || sails.config.cors.origin);
var credentialsConfig = (routeCorsConfig.credentials || sails.config.cors.credentials);

// Get the allowed origins
var origins = (routeCorsConfig.origin || sails.config.cors.origin).split(',');
var origins = originConfig.split(',');

// Match the origin of the request against the allowed origins
var foundOrigin = false;
Expand All @@ -61,7 +64,7 @@ module.exports = function (sails) {
// If we find a whitelisted origin, send the Access-Control-Allow-Origin header
// to greenlight the request.
if (origin == req.headers.origin || origin == '*') {
res.set('Access-Control-Allow-Origin', req.headers.origin);
res.set('Access-Control-Allow-Origin', origin);
foundOrigin = true;
return false;
}
Expand All @@ -74,8 +77,25 @@ module.exports = function (sails) {
res.set('Access-Control-Allow-Origin', '');
}

// Determine whether or not to allow cookies to be passed cross-origin
res.set('Access-Control-Allow-Credentials', !_.isUndefined(routeCorsConfig.credentials) ? routeCorsConfig.credentials : sails.config.cors.credentials);
// Determine whether or not to allow cookies to be passed cross-origin.
// If the CORS config for this route is (or is defaulting to) `credentials: true`...
if (credentialsConfig === true) {
// If the origin is set to `*`, `credentials` can't be true, so log a warning
// and clear the access-control-allow-credentials header.
if (originConfig === '*') {
sails.log.error('Invalid CORS settings for route `' + req.url + '`: if `origin` is \'*\', `credentials` cannot be `true` (setting `credentials` to `false` for you).');
res.removeHeader('Access-Control-Allow-Credentials');
}
// Otherwise set the access-control-allow-credentials header to `true`.
else {
res.set('Access-Control-Allow-Credentials', 'true');
}
}
// Otherwise if the global or route-level CORS config is or is defaulting to
// `credentials: false`, then clear the access-control-allow-credentials header.
else {
res.removeHeader('Access-Control-Allow-Credentials');
}

// This header lets a server whitelist headers that browsers are allowed to access
res.set('Access-Control-Expose-Headers', !_.isUndefined(routeCorsConfig.exposeHeaders) ? routeCorsConfig.exposeHeaders : sails.config.cors.exposeHeaders);
Expand Down
74 changes: 65 additions & 9 deletions test/integration/hook.cors_csrf.test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ var httpHelper = require('./helpers/httpHelper.js');
var appHelper = require('./helpers/appHelper');
var path = require('path');
var fs = require('fs');

var _ = require('lodash');


describe('CORS and CSRF ::', function() {
Expand Down Expand Up @@ -145,6 +145,7 @@ describe('CORS and CSRF ::', function() {
assert.equal(response.headers['access-control-allow-origin'], 'http://www.example.com');
assert.equal(response.headers['access-control-allow-methods'], 'put');
assert.equal(response.headers['access-control-allow-headers'], 'content-type');
assert(_.isUndefined(response.headers['access-control-allow-credentials']));
done();
});

Expand Down Expand Up @@ -252,7 +253,7 @@ describe('CORS and CSRF ::', function() {
return done(err);
}
assert.equal(response.statusCode, 200);
assert.equal(response.headers['access-control-allow-origin'], 'http://www.example.com');
assert.equal(response.headers['access-control-allow-origin'], '*');
assert.equal(response.headers['access-control-allow-methods'].toLowerCase(), 'get, post, put, delete, options, head');
done();
});
Expand All @@ -272,7 +273,7 @@ describe('CORS and CSRF ::', function() {
return done(err);
}
assert.equal(response.statusCode, 200);
assert.equal(response.headers['access-control-allow-origin'], 'http://www.example.com');
assert.equal(response.headers['access-control-allow-origin'], '*');
assert.equal(response.headers['access-control-allow-methods'].toLowerCase(), 'get, post, put, delete, options, head');
done();
});
Expand All @@ -294,7 +295,7 @@ describe('CORS and CSRF ::', function() {
return done(err);
}
assert.equal(response.statusCode, 200);
assert.equal(response.headers['access-control-allow-origin'], 'http://www.example.com');
assert.equal(response.headers['access-control-allow-origin'], '*');
assert.equal(response.headers['access-control-expose-headers'], '');
done();
});
Expand All @@ -311,7 +312,7 @@ describe('CORS and CSRF ::', function() {
return done(err);
}
assert.equal(response.statusCode, 200);
assert.equal(response.headers['access-control-allow-origin'], 'http://www.example.com');
assert.equal(response.headers['access-control-allow-origin'], '*');
assert.equal(response.headers['access-control-expose-headers'], 'x-custom-header');
done();
});
Expand Down Expand Up @@ -534,7 +535,7 @@ describe('CORS and CSRF ::', function() {
return done(err);
}
assert.equal(response.statusCode, 200);
assert.equal(response.headers['access-control-allow-origin'], 'http://www.example.com');
assert.equal(response.headers['access-control-allow-origin'], '*');
done();
});
});
Expand Down Expand Up @@ -776,6 +777,26 @@ describe('CORS and CSRF ::', function() {
before(function() {
fs.writeFileSync(path.resolve('../', appName, 'config/cors.js'), "module.exports.cors = { 'origin': '*', 'allRoutes': true, 'credentials': true};");
var routeConfig = {
'GET /default': {
controller: 'TestController',
action: 'index'
},
'GET /notallowed': {
controller: 'TestController',
action: 'index',
cors: {
origin: '*',
credentials: true
}
},
'GET /test': {
controller: 'TestController',
action: 'index',
cors: {
origin: 'http://www.example.com',
credentials: true
}
},
'GET /test/destroy': {
controller: 'TestController',
action: 'destroy',
Expand All @@ -785,9 +806,44 @@ describe('CORS and CSRF ::', function() {
}
}
};

fs.writeFileSync(path.resolve('../', appName, 'config/routes.js'), "module.exports.routes = " + JSON.stringify(routeConfig));
});

it('should not allow origin: \'*\' in conjunction with \'credentials: true\' as the default (should force credentials: force and log a warning)', function(done) {
httpHelper.testRoute('get', {
url: 'default',
headers: {
'Origin': 'http://www.example.com'
},
}, function(err, response) {
if (err) {
return done(err);
}
assert.equal(response.statusCode, 200);
assert(_.isUndefined(response.headers['access-control-allow-credentials']));
assert.equal(sailsApp.config.cors.credentials, false);
done();
});
});

it('should not allow origin: \'*\' in conjunction with \'credentials: true\' set explicitly (should force credentials: false and log a warning)', function(done) {
httpHelper.testRoute('get', {
url: 'notallowed',
headers: {
'Origin': 'http://www.example.com'
},
}, function(err, response) {
if (err) {
return done(err);
}
assert.equal(response.statusCode, 200);
assert(_.isUndefined(response.headers['access-control-allow-credentials']));
done();
});

});

it('to a route without a CORS config should result in a 200 response with an Access-Control-Allow-Credentials header with value "true"', function(done) {
httpHelper.testRoute('get', {
url: 'test',
Expand All @@ -804,7 +860,7 @@ describe('CORS and CSRF ::', function() {
});
});

it('to a route with {cors: {credentials: false}} should result in a 200 response with an Access-Control-Allow-Credentials header with value "false"', function(done) {
it('to a route with {cors: {credentials: false}} should result in a 200 response with no Access-Control-Allow-Credentials header', function(done) {
httpHelper.testRoute('get', {
url: 'test/destroy',
headers: {
Expand All @@ -815,7 +871,7 @@ describe('CORS and CSRF ::', function() {
return done(err);
}
assert.equal(response.statusCode, 200);
assert.equal(response.headers['access-control-allow-credentials'], 'false');
assert(_.isUndefined(response.headers['access-control-allow-credentials']));
done();
});
});
Expand Down Expand Up @@ -850,7 +906,7 @@ describe('CORS and CSRF ::', function() {
return done(err);
}
assert.equal(response.statusCode, 200);
assert.equal(response.headers['access-control-allow-credentials'], 'false');
assert(_.isUndefined(response.headers['access-control-allow-credentials']));
done();
});
});
Expand Down
2 changes: 1 addition & 1 deletion test/integration/router.specifiedRoutes.test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ describe('router :: ', function() {
},
}, function(err, response, body) {
assert.equal(response.statusCode, 200);
assert.equal(response.headers['access-control-allow-origin'], 'https://foo.shyp.com');
assert.equal(response.headers['access-control-allow-origin'], '*');
done();
});
});
Expand Down

0 comments on commit ca43e05

Please sign in to comment.