fix(cors): set proper headers when request is not options

src/Cors/index.js

@@ -68,17 +68,8 @@ class Cors {
      * required headers to be defined on every request
      */
     response.header('Access-Control-Allow-Origin', this._getOrigin(request.header('origin')))
-    response.header('Access-Control-Allow-Methods', this.methods)
     response.header('Access-Control-Allow-Credentials', this.credentials)
 
-    /**
-     * setting up cors headers only if they are not set to false.
-     */
-    const corsHeaders = this._getHeaders(request.header('Access-Control-Request-Headers'))
-    if (corsHeaders) {
-      response.header('Access-Control-Allow-Headers', corsHeaders)
-    }
-
     /**
      * setting up exposed headers if defined
      */
@@ -87,22 +78,31 @@ class Cors {
     }
 
     /**
-     * setting up max age if defined
+     * if request is not for OPTIONS yield next. Otherwise set
+     * CORS headers.
      */
-    if (this.maxAge) {
-      response.header('Access-Control-Allow-Max-Age', this.maxAge)
+    if (request.method() !== 'OPTIONS') {
+      yield next
+      return
     }
 
+    response.header('Access-Control-Allow-Methods', this.methods)
+
     /**
-     * if request is for OPTIONS send 204 otherwise yield it to
-     * next middleware
+     * setting up cors headers only if they are not set to false.
      */
-    if (request.method() === 'OPTIONS') {
-      response.status(204).send()
-      return
+    const corsHeaders = this._getHeaders(request.header('access-control-request-headers'))
+    if (corsHeaders) {
+      response.header('Access-Control-Allow-Headers', corsHeaders)
     }
 
-    yield next
+    /**
+     * setting up max age if defined
+     */
+    if (this.maxAge) {
+      response.header('Access-Control-Allow-Max-Age', this.maxAge)
+    }
+    response.status(204).send()
   }
 
 }

test/cors.spec.js

@@ -485,4 +485,112 @@ describe('Cors', function () {
     })
     yield supertest(server).get('/').set('origin', 'localhost').expect(200)
   })
+
+  it('should set only required headers when request is not for OPTIONS', function * () {
+    const Config = {
+      get: function (key) {
+        key = key.split('.')[1]
+        const options = {
+          origin: true,
+          methods: 'GET, PUT, POST',
+          headers: true,
+          exposeHeaders: 'Content-Language, Content-Type',
+          credentials: true,
+          maxAge: 90
+        }
+        return options[key]
+      }
+    }
+
+    const cors = new Cors(Config)
+    const server = http.createServer(function (req, res) {
+      req.header = function (key) {
+        return req.headers[key]
+      }
+      req.method = function () {
+        return 'GET'
+      }
+      res.header = function (key, value) {
+        res.setHeader(key, value)
+      }
+      res.status = function (code) {
+        res.writeHead(code)
+        return res
+      }
+      res.send = function (code) {}
+      co(function * () {
+        return yield cors.handle(req, res, function * () {})
+      })
+        .then(function () {
+          res.end()
+        })
+        .catch(function (error) {
+          console.log(error)
+          res.writeHead(500)
+          res.end()
+        })
+    })
+
+    const response = yield supertest(server).get('/').set('origin', 'localhost').expect(200)
+    expect(response.headers['access-control-allow-origin']).to.equal('localhost')
+    expect(response.headers['access-control-allow-credentials']).to.equal('true')
+    expect(response.headers['access-control-expose-headers']).to.equal('Content-Language, Content-Type')
+    expect(response.headers['access-control-allow-methods']).to.equal(undefined)
+    expect(response.headers['access-control-allow-max-age']).to.equal(undefined)
+    expect(response.headers['access-control-allow-headers']).to.equal(undefined)
+  })
+
+  it('should set all headers when request is for OPTIONS', function * () {
+    const Config = {
+      get: function (key) {
+        key = key.split('.')[1]
+        const options = {
+          origin: true,
+          methods: 'GET, PUT, POST',
+          headers: true,
+          exposeHeaders: 'Content-Language, Content-Type',
+          credentials: true,
+          maxAge: 90
+        }
+        return options[key]
+      }
+    }
+
+    const cors = new Cors(Config)
+    const server = http.createServer(function (req, res) {
+      req.header = function (key) {
+        return req.headers[key]
+      }
+      req.method = function () {
+        return 'OPTIONS'
+      }
+      res.header = function (key, value) {
+        res.setHeader(key, value)
+      }
+      res.status = function (code) {
+        res.writeHead(code)
+        return res
+      }
+      res.send = function (code) {}
+      co(function * () {
+        return yield cors.handle(req, res, function * () {})
+      })
+        .then(function () {
+          res.end()
+        })
+        .catch(function (error) {
+          console.log(error)
+          res.writeHead(500)
+          res.end()
+        })
+    })
+
+    const response = yield supertest(server).options('/').set('origin', 'localhost').set('Access-Control-Request-Headers', 'X-Custom-Header').expect(204)
+    expect(response.headers['access-control-allow-origin']).to.equal('localhost')
+    expect(response.headers['access-control-allow-credentials']).to.equal('true')
+    expect(response.headers['access-control-expose-headers']).to.equal('Content-Language, Content-Type')
+    expect(response.headers['access-control-allow-methods']).to.equal('GET, PUT, POST')
+    expect(response.headers['access-control-allow-max-age']).to.equal('90')
+    expect(response.headers['access-control-allow-headers']).to.equal('X-Custom-Header')
+  })
 })