From 1a1467904ba4f662169650e08b8a8a0949e2db3e Mon Sep 17 00:00:00 2001 From: Romain Prieto Date: Tue, 3 Jun 2014 13:19:10 +1000 Subject: [PATCH] New generic mode that always returns '*' --- README.md | 18 +- src/actual.js | 32 +-- src/origin-matcher.js | 4 + src/preflight.js | 84 ++++--- test/cors.actual.spec.js | 26 ++ test/cors.preflight.spec.js | 227 +++++++++++------- ...{origin.spec.js => origin-matcher.spec.js} | 0 7 files changed, 256 insertions(+), 135 deletions(-) rename test/{origin.spec.js => origin-matcher.spec.js} (100%) diff --git a/README.md b/README.md index 6d9673b..fae0d65 100644 --- a/README.md +++ b/README.md @@ -19,7 +19,7 @@ const corsMiddleware = require('restify-cors-middleware') const cors = corsMiddleware({ - preflightMaxAge: 5, //Optional + preflightMaxAge: 5, origins: ['http://api.myapp.com', 'http://web.myapp.com'], allowHeaders: ['API-Token'], exposeHeaders: ['API-Token-Expiry'] @@ -31,23 +31,25 @@ server.use(cors.actual) ## Allowed origins -You can specify the full list of domains and subdomains allowed in your application: +You can specify the full list of domains and subdomains allowed in your application. +As a convenience method, you can use the `*` character as a wildcard. ```js origins: [ 'http://myapp.com', - 'http://*.myapp.com' + 'http://*.myotherapp.com' ] ``` -For added security, this middleware sets `Access-Control-Allow-Origin` to the origin that matched, not the configured wildcard. -This means callers won't know about other domains that are supported. +The `Access-Control-Allow-Origin` header will be set to the actual origin that matched, on a per-request basis. The person making the request will not know about the full configuration, like other allowed domains or any wildcards in use. -Setting `origins: ['*']` is also valid, although it comes with obvious security implications. Note that it will still return a customised response (matching Origin), so any caching layer (reverse proxy or CDN) will grow in size accordingly. +The main side-effect is that every response will include `Vary: Origin`, since the response headers depend on the origin. This is the safest setup, but will decrease your cache hit-rate / increase your cache size with every origin. -## Troubleshooting +## Open CORS setup -As per the spec, requests without an `Origin` will not receive any headers. Requests with a matching `Origin` will receive the appropriate response headers. Always be careful that any reverse proxies (e.g. Varnish) very their cache depending on the origin, so you don't serve CORS headers to the wrong request. +Using `origins: ['*']` is also a valid setup, which comes with obvious security implications. This means **any** domain will be able to query your API. However it does have performance benefits. When using `['*']`, the middleware always responds with `Access-Control-Allow-Origin: *` which means responses can be cached regardless of origins. + +Each API should weigh the security and performance angles before choosing this approach. ## Compliance to the spec diff --git a/src/actual.js b/src/actual.js index d6d4d30..fcb3fae 100644 --- a/src/actual.js +++ b/src/actual.js @@ -6,21 +6,25 @@ exports.handler = function (options) { return function (req, res, next) { var originHeader = req.headers['origin'] - // If either no origin was set, or the origin isn't supported, continue - // without setting any headers - if (!originHeader || !matcher(originHeader)) { + if (originMatcher.generic(options.origins)) { + res.setHeader(constants['AC_ALLOW_ORIGIN'], '*') + res.setHeader(constants['AC_ALLOW_CREDS'], false) // not compatible with * + res.setHeader(constants['AC_EXPOSE_HEADERS'], options.exposeHeaders.join(', ')) + return next() + } else { + // If either no origin was set, or the origin isn't supported, continue + // without setting any headers + if (!originHeader || !matcher(originHeader)) { + return next() + } + // if match was found, let's set some headers. + res.setHeader(constants['AC_ALLOW_ORIGIN'], originHeader) + res.setHeader(constants['STR_VARY'], constants['STR_ORIGIN']) + if (options.credentials) { + res.setHeader(constants['AC_ALLOW_CREDS'], 'true') + } + res.setHeader(constants['AC_EXPOSE_HEADERS'], options.exposeHeaders.join(', ')) return next() } - - // if match was found, let's set some headers. - res.setHeader(constants['AC_ALLOW_ORIGIN'], originHeader) - res.setHeader(constants['STR_VARY'], constants['STR_ORIGIN']) - if (options.credentials) { - res.setHeader(constants['AC_ALLOW_CREDS'], 'true') - } - res.setHeader(constants['AC_EXPOSE_HEADERS'], - options.exposeHeaders.join(', ')) - - return next() } } diff --git a/src/origin-matcher.js b/src/origin-matcher.js index ddfdf3e..c7b2c65 100644 --- a/src/origin-matcher.js +++ b/src/origin-matcher.js @@ -1,4 +1,8 @@ +exports.generic = function (list) { + return list.length === 1 && list[0] === '*' +} + exports.create = function (allowedOrigins) { // pre-compile list of matchers, so regexes are only built once var matchers = allowedOrigins.map(createMatcher) diff --git a/src/preflight.js b/src/preflight.js index 3edeb0a..765d841 100644 --- a/src/preflight.js +++ b/src/preflight.js @@ -3,42 +3,70 @@ var constants = require('./constants.js') exports.handler = function (options) { var matcher = originMatcher.create(options.origins) - return function (req, res, next) { - if (req.method !== 'OPTIONS') return next() - // 6.2.1 and 6.2.2 - var originHeader = req.headers['origin'] - if (!matcher(originHeader)) return next() + if (originMatcher.generic(options.origins)) { + // + // If origins = ['*'] then we always set generic CORS headers + // This is the simplest case, similar to what restify.fullResponse() used to do + // Must must keep the headers generic because they can be cached by reverse proxies + // - // 6.2.3 - var requestedMethod = req.headers[constants['AC_REQ_METHOD']] - if (!requestedMethod) return next() + return function (req, res, next) { + if (req.method !== 'OPTIONS') return next() + res.once('header', function () { + res.header(constants['AC_ALLOW_ORIGIN'], '*') + res.header(constants['AC_ALLOW_CREDS'], false) // not compatible with * + res.header(constants['AC_ALLOW_METHODS'], 'GET, PUT, POST, DELETE, OPTIONS') + res.header(constants['AC_ALLOW_HEADERS'], options.allowHeaders.join(', ')) + }) + res.send(constants['HTTP_NO_CONTENT']) + } + } else { + // + // Full CORS mode + // This is the "better" option where we have a list of origins + // In this case, we return customised CORS headers for each request + // And must set the "Vary: Origin" header + // - // 6.2.4 - // var requestedHeaders = req.headers[constants['AC_REQ_HEADERS']] - // requestedHeaders = requestedHeaders ? requestedHeaders.split(', ') : [] + return function (req, res, next) { + if (req.method !== 'OPTIONS') return next() - var allowedMethods = [requestedMethod, 'OPTIONS'] - var allowedHeaders = constants['ALLOW_HEADERS'] - .concat(options.allowHeaders) + // 6.2.1 and 6.2.2 + var originHeader = req.headers['origin'] + if (!matcher(originHeader)) return next() - res.once('header', function () { - // 6.2.7 - res.header(constants['AC_ALLOW_ORIGIN'], originHeader) - res.header(constants['AC_ALLOW_CREDS'], true) + // 6.2.3 + var requestedMethod = req.headers[constants['AC_REQ_METHOD']] + if (!requestedMethod) return next() - // 6.2.8 - if (options.preflightMaxAge) { - res.header(constants['AC_MAX_AGE'], options.preflightMaxAge) - } + // 6.2.4 + // var requestedHeaders = req.headers[constants['AC_REQ_HEADERS']] + // requestedHeaders = requestedHeaders ? requestedHeaders.split(', ') : [] + var allowedMethods = [requestedMethod, 'OPTIONS'] + var allowedHeaders = constants['ALLOW_HEADERS'].concat(options.allowHeaders) - // 6.2.9 - res.header(constants['AC_ALLOW_METHODS'], allowedMethods.join(', ')) + res.once('header', function () { + // 6.2.7 + res.header(constants['AC_ALLOW_ORIGIN'], originHeader) + res.header(constants['AC_ALLOW_CREDS'], true) - // 6.2.10 - res.header(constants['AC_ALLOW_HEADERS'], allowedHeaders.join(', ')) - }) + // 6.2.8 + if (options.preflightMaxAge) { + res.header(constants['AC_MAX_AGE'], options.preflightMaxAge) + } - res.send(constants['HTTP_NO_CONTENT']) + // 6.2.9 + res.header(constants['AC_ALLOW_METHODS'], allowedMethods.join(', ')) + + // 6.2.10 + res.header(constants['AC_ALLOW_HEADERS'], allowedHeaders.join(', ')) + + // 6.4 + res.header('Vary', 'Origin') + }) + + res.send(constants['HTTP_NO_CONTENT']) + } } } diff --git a/test/cors.actual.spec.js b/test/cors.actual.spec.js index 21e5f3a..e14baa8 100644 --- a/test/cors.actual.spec.js +++ b/test/cors.actual.spec.js @@ -94,4 +94,30 @@ describe('CORS: simple / actual requests', function () { .expect(200) .end(done) }) + + xit('6.4 FIXME Does not need the Vary header if it accepts all Origins', function (done) { + var server = test.corsServer({ + origins: ['*'] + }) + request(server) + .get('/test') + .set('Origin', 'http://api.myapp.com') + .expect('access-control-allow-origin', '*') + .expect(test.noHeader('vary')) + .expect(200) + .end(done) + }) + + xit('6.4 FIXME Sets the Vary header if it returns the actual origin', function (done) { + var server = test.corsServer({ + origins: ['http://api.myapp.com'] + }) + request(server) + .get('/test') + .set('Origin', 'http://api.myapp.com') + .expect('access-control-allow-origin', 'http://api.myapp.com') + .expect('vary', 'Origin') + .expect(200) + .end(done) + }) }) diff --git a/test/cors.preflight.spec.js b/test/cors.preflight.spec.js index 4bd3cdb..d388a82 100644 --- a/test/cors.preflight.spec.js +++ b/test/cors.preflight.spec.js @@ -10,119 +10,176 @@ var test = require('./test') var METHOD_NOT_ALLOWED = 405 describe('CORS: preflight requests', function () { - it('6.2.1 Does not set headers if Origin is missing', function (done) { + describe('Generic', function () { + // In this configuration, we want to always reply with "*" + // Responses can be cached by reverse proxies, so they must all look the same + var server = test.corsServer({ - origins: ['http://api.myapp.com', 'http://www.myapp.com'] + origins: ['*'], + allowHeaders: ['MyHeader'] }) - request(server) + + it('6.1 Always replies with generic CORS headers', function (done) { + request(server) .options('/test') - .expect(test.noHeader('access-control-allow-origin')) - .expect(METHOD_NOT_ALLOWED) + .set('Origin', 'http://api.myapp.com') + .expect('access-control-allow-origin', '*') + .expect('access-control-allow-methods', 'GET, PUT, POST, DELETE, OPTIONS') + .expect('Access-Control-Allow-Headers', /x-requested-with/) + .expect('Access-Control-Allow-Headers', /MyHeader/) + .expect(204) .end(done) - }) - - it('6.2.2 Does not set headers if Origin does not match', function (done) { - var server = test.corsServer({ - origins: ['http://api.myapp.com', 'http://www.myapp.com'] }) - request(server) + + it('6.1 Includes CORS headers even if the request has no Origin', function (done) { + request(server) .options('/test') - .set('Origin', 'http://random-website.com') - .expect(test.noHeader('access-control-allow-origin')) - .expect(METHOD_NOT_ALLOWED) + .expect('access-control-allow-origin', '*') + .expect('access-control-allow-methods', 'GET, PUT, POST, DELETE, OPTIONS') + .expect('Access-Control-Allow-Headers', /x-requested-with/) + .expect('Access-Control-Allow-Headers', /MyHeader/) + .expect(204) .end(done) - }) - - it('6.2.3 Does not set headers if Access-Control-Request-Method is missing', function (done) { - var server = test.corsServer({ - origins: ['http://api.myapp.com', 'http://www.myapp.com'] }) - request(server) + + it('6.4 Does not need the Vary header since all responses are the same', function (done) { + request(server) .options('/test') .set('Origin', 'http://api.myapp.com') - .expect(test.noHeader('access-control-allow-origin')) - .expect(test.noHeader('access-control-allow-methods')) - .expect(METHOD_NOT_ALLOWED) + .expect(test.noHeader('vary')) + .expect(204) .end(done) + }) }) - xit('6.2.4 Does not terminate if parsing of Access-Control-Request-Headers fails', function (done) { - done() - }) + describe('Full CORS mode', function () { + it('6.2.1 Does not set headers if Origin is missing', function (done) { + var server = test.corsServer({ + origins: ['http://api.myapp.com', 'http://www.myapp.com'] + }) + request(server) + .options('/test') + .expect(test.noHeader('access-control-allow-origin')) + .expect(METHOD_NOT_ALLOWED) + .end(done) + }) - xit('6.2.5 Always matches Access-Control-Request-Method (spec says it is acceptable)', function (done) { - done() - }) + it('6.2.2 Does not set headers if Origin does not match', function (done) { + var server = test.corsServer({ + origins: ['http://api.myapp.com', 'http://www.myapp.com'] + }) + request(server) + .options('/test') + .set('Origin', 'http://random-website.com') + .expect(test.noHeader('access-control-allow-origin')) + .expect(METHOD_NOT_ALLOWED) + .end(done) + }) - it('6.2.6 Does not set headers if Access-Control-Request-Headers does not match', function (done) { - var server = test.corsServer({ - origins: ['http://api.myapp.com', 'http://www.myapp.com'], - acceptHeaders: ['API-Token'] + it('6.2.3 Does not set headers if Access-Control-Request-Method is missing', function (done) { + var server = test.corsServer({ + origins: ['http://api.myapp.com', 'http://www.myapp.com'] + }) + request(server) + .options('/test') + .set('Origin', 'http://api.myapp.com') + .expect(test.noHeader('access-control-allow-origin')) + .expect(test.noHeader('access-control-allow-methods')) + .expect(METHOD_NOT_ALLOWED) + .end(done) }) - request(server) - .options('/test') - .set('Origin', 'http://api.myapp.com') - .set('Access-Control-Request-Headers', 'Weird-Header') - .expect(test.noHeader('access-control-allow-origin')) - .expect(test.noHeader('access-control-allow-methods')) - .expect(test.noHeader('access-control-allow-headers')) - .expect(METHOD_NOT_ALLOWED) - .end(done) - }) - it('6.2.7 Set the Allow-Origin header if it matches', function (done) { - var server = test.corsServer({ - origins: ['http://api.myapp.com', 'http://www.myapp.com'] + xit('6.2.4 Does not terminate if parsing of Access-Control-Request-Headers fails', function (done) { + done() }) - request(server) - .options('/test') - .set('Origin', 'http://api.myapp.com') - .set('Access-Control-Request-Method', 'GET') - .expect('Access-Control-Allow-Origin', 'http://api.myapp.com') - .expect(204) - .end(done) - }) - it('6.2.8 Set the Access-Control-Max-Age header if a max age is provided', function (done) { - var server = test.corsServer({ - preflightMaxAge: 5, - origins: ['http://api.myapp.com', 'http://www.myapp.com'] + xit('6.2.5 Always matches Access-Control-Request-Method (spec says it is acceptable)', function (done) { + done() }) - request(server) - .options('/test') - .set('Origin', 'http://api.myapp.com') - .set('Access-Control-Request-Method', 'GET') - .expect('Access-Control-Max-Age', '5') - .expect(204) - .end(done) - }) - it('6.2.9 Set the Allow-Method header', function (done) { - var server = test.corsServer({ - origins: ['http://api.myapp.com', 'http://www.myapp.com'] + it('6.2.6 Does not set headers if Access-Control-Request-Headers does not match', function (done) { + var server = test.corsServer({ + origins: ['http://api.myapp.com', 'http://www.myapp.com'], + acceptHeaders: ['API-Token'] + }) + request(server) + .options('/test') + .set('Origin', 'http://api.myapp.com') + .set('Access-Control-Request-Headers', 'Weird-Header') + .expect(test.noHeader('access-control-allow-origin')) + .expect(test.noHeader('access-control-allow-methods')) + .expect(test.noHeader('access-control-allow-headers')) + .expect(METHOD_NOT_ALLOWED) + .end(done) }) - request(server) - .options('/test') - .set('Origin', 'http://api.myapp.com') - .set('Access-Control-Request-Method', 'GET') - .expect('Access-Control-Allow-Methods', 'GET, OPTIONS') - .expect(204) - .end(done) - }) - it('6.2.10 Set the Allow-Headers to all configured custom headers', function (done) { - var server = test.corsServer({ - origins: ['http://api.myapp.com', 'http://www.myapp.com'], - allowHeaders: ['HeaderA'] + it('6.2.7 Set the Allow-Origin header if it matches', function (done) { + var server = test.corsServer({ + origins: ['http://api.myapp.com', 'http://www.myapp.com'] + }) + request(server) + .options('/test') + .set('Origin', 'http://api.myapp.com') + .set('Access-Control-Request-Method', 'GET') + .expect('Access-Control-Allow-Origin', 'http://api.myapp.com') + .expect(204) + .end(done) + }) + + it('6.2.8 Set the Access-Control-Max-Age header if a max age is provided', function (done) { + var server = test.corsServer({ + preflightMaxAge: 5, + origins: ['http://api.myapp.com', 'http://www.myapp.com'] + }) + request(server) + .options('/test') + .set('Origin', 'http://api.myapp.com') + .set('Access-Control-Request-Method', 'GET') + .expect('Access-Control-Max-Age', '5') + .expect(204) + .end(done) }) - request(server) + + it('6.2.9 Set the Allow-Method header', function (done) { + var server = test.corsServer({ + origins: ['http://api.myapp.com', 'http://www.myapp.com'] + }) + request(server) + .options('/test') + .set('Origin', 'http://api.myapp.com') + .set('Access-Control-Request-Method', 'GET') + .expect('Access-Control-Allow-Methods', 'GET, OPTIONS') + .expect(204) + .end(done) + }) + + it('6.2.10 Set the Allow-Headers to all configured custom headers', function (done) { + var server = test.corsServer({ + origins: ['http://api.myapp.com', 'http://www.myapp.com'], + allowHeaders: ['HeaderA'] + }) + request(server) + .options('/test') + .set('Origin', 'http://api.myapp.com') + .set('Access-Control-Request-Method', 'GET') + .expect('Access-Control-Allow-Headers', /accept-version/) // restify defaults + .expect('Access-Control-Allow-Headers', /x-api-version/) // restify defaults + .expect('Access-Control-Allow-Headers', /HeaderA/) // custom header + .expect(204) + .end(done) + }) + + it('6.4 Sets the Vary header if it returns the actual origin', function (done) { + var server = test.corsServer({ + origins: ['http://api.myapp.com'] + }) + request(server) .options('/test') .set('Origin', 'http://api.myapp.com') .set('Access-Control-Request-Method', 'GET') - .expect('Access-Control-Allow-Headers', /accept-version/) // restify defaults - .expect('Access-Control-Allow-Headers', /x-api-version/) // restify defaults - .expect('Access-Control-Allow-Headers', /HeaderA/) // custom header + .expect('vary', 'Origin') .expect(204) .end(done) + }) }) }) diff --git a/test/origin.spec.js b/test/origin-matcher.spec.js similarity index 100% rename from test/origin.spec.js rename to test/origin-matcher.spec.js