Merge pull request Kong#1240 from Mashape/fix/cors

Fixing CORS plugin

kong/plugins/cors/handler.lua

@@ -5,6 +5,7 @@ local CorsHandler = BasePlugin:extend()
 
 CorsHandler.PRIORITY = 2000
 
+local OPTIONS = "OPTIONS"
 
 local function configure_origin(ngx, conf)
   if conf.origin == nil then
@@ -54,20 +55,30 @@ function CorsHandler:new()
 end
 
 function CorsHandler:access(conf)
-  CorsHandler.super.access(self)
-  configure_origin(ngx, conf)
-  configure_credentials(ngx, conf)
-
-  if ngx.req.get_method() == "OPTIONS" then -- Preflight request
-    configure_headers(ngx, conf, ngx.req.get_headers())
-    configure_methods(ngx, conf)
-    configure_max_age(ngx, conf)
-
-    if not conf.preflight_continue then -- Check if the preflight request should end here, or be proxied
+  CorsHandler.super.access(self) 
+
+  if ngx.req.get_method() == OPTIONS then
+    if not conf.preflight_continue then
+      configure_origin(ngx, conf)
+      configure_credentials(ngx, conf)
+      configure_headers(ngx, conf, ngx.req.get_headers())
+      configure_methods(ngx, conf)
+      configure_max_age(ngx, conf)
+      ngx.ctx.skip_response_headers = true -- Don't add response headers because we already added them all
       return responses.send_HTTP_NO_CONTENT()
+    else
+      -- Don't add any response header because we are delegating the preflight to the upstream API (conf.preflight_continue=true)
+      ngx.ctx.skip_response_headers = true
     end
+  end
+end
 
-  else
+function CorsHandler:header_filter(conf)
+  CorsHandler.super.header_filter(self)
+
+  if not ngx.ctx.skip_response_headers then
+    configure_origin(ngx, conf)
+    configure_credentials(ngx, conf)
     configure_exposed_headers(ngx, conf)
   end
 end

spec/plugins/cors/access_spec.lua

@@ -10,7 +10,9 @@ describe("CORS Plugin", function()
     spec_helper.insert_fixtures {
       api = {
         { name = "tests-cors-1", request_host = "cors1.com", upstream_url = "http://mockbin.com" },
-        { name = "tests-cors-2", request_host = "cors2.com", upstream_url = "http://mockbin.com" }
+        { name = "tests-cors-2", request_host = "cors2.com", upstream_url = "http://mockbin.com" },
+        { name = "tests-cors-3", request_host = "cors3.com", upstream_url = "http://httpbin.org" },
+        { name = "tests-cors-4", request_host = "cors4.com", upstream_url = "http://httpbin.org" }
       },
       plugin = {
         { name = "cors", config = {}, __api = 1 },
@@ -19,7 +21,21 @@ describe("CORS Plugin", function()
                                    headers = { "origin", "type", "accepts" },
                                    exposed_headers = { "x-auth-token" },
                                    max_age = 23,
-                                   credentials = true }, __api = 2 }
+                                   credentials = true }, __api = 2 },
+        { name = "cors", config = { origin = "example.com",
+                                   methods = { "GET" },
+                                   headers = { "origin", "type", "accepts" },
+                                   exposed_headers = { "x-auth-token" },
+                                   max_age = 23,
+                                   preflight_continue = true,
+                                   credentials = true }, __api = 3 },
+        { name = "cors", config = { origin = "example.com",
+                                   methods = { "GET" },
+                                   headers = { "origin", "type", "accepts" },
+                                   exposed_headers = { "x-auth-token" },
+                                   max_age = 23,
+                                   preflight_continue = false,
+                                   credentials = true }, __api = 4 }
       }
     }
 
@@ -58,6 +74,70 @@ describe("CORS Plugin", function()
       assert.are.equal(tostring(23), headers["access-control-max-age"])
       assert.are.equal(tostring(true), headers["access-control-allow-credentials"])
     end)
+
+    it("should work with preflight_continue=true", function()
+      -- An OPTIONS preflight request with preflight_continue=true should have the same response as directly invoking the final API
+
+      local response, status, headers = http_client.options(PROXY_URL.."/headers", {}, {host = "cors3.com"})
+      local response2, status2, headers2 = http_client.options("http://httpbin.org/response-headers", {}, {host = "cors3.com"})
+
+      headers["via"] = nil
+      headers["x-kong-proxy-latency"] = nil
+      headers["x-kong-upstream-latency"] = nil
+      headers["date"] = nil
+      headers2["date"] = nil
+
+      assert.are.equal(response, response2)
+      assert.are.equal(status, status2)
+      assert.are.same(headers, headers2)
+
+      -- Any other request that's not a preflight request, should match our plugin configuration
+      local _, status, headers = http_client.get(PROXY_URL.."/get", {}, {host = "cors3.com"})
+
+      assert.are.equal(200, status)
+      assert.are.equal("example.com", headers["access-control-allow-origin"])
+      assert.are.equal("x-auth-token", headers["access-control-expose-headers"])
+      assert.are.equal(tostring(true), headers["access-control-allow-credentials"])
+
+      local _, status, headers = http_client.get(PROXY_URL.."/response-headers", {["access-control-allow-origin"] = "*"}, {host = "cors3.com"})
+
+      assert.are.equal(200, status)
+      assert.are.equal("example.com", headers["access-control-allow-origin"])
+      assert.are.equal("x-auth-token", headers["access-control-expose-headers"])
+      assert.are.equal(tostring(true), headers["access-control-allow-credentials"])
+    end)
+
+    it("should work with preflight_continue=false", function()
+      -- An OPTIONS preflight request with preflight_continue=false should be handled by Kong instead
+
+      local response, status, headers = http_client.options(PROXY_URL.."/headers", {}, {host = "cors4.com"})
+      local response2, status2, headers2 = http_client.options("http://httpbin.org/response-headers", {}, {host = "cors4.com"})
+
+      headers["via"] = nil
+      headers["x-kong-proxy-latency"] = nil
+      headers["x-kong-upstream-latency"] = nil
+      headers["date"] = nil
+      headers2["date"] = nil
+
+      assert.are.equal(response, response2)
+      assert.are_not.equal(status, status2)
+      assert.are_not.same(headers, headers2)
+
+      assert.are.equal("example.com", headers["access-control-allow-origin"])
+      assert.are.equal("GET", headers["access-control-allow-methods"])
+      assert.are.equal("origin,type,accepts", headers["access-control-allow-headers"])
+      assert.are.equal(nil, headers["access-control-expose-headers"])
+      assert.are.equal(tostring(true), headers["access-control-allow-credentials"])
+      assert.are.equal(tostring(23), headers["access-control-max-age"])
+
+      -- Any other request that's not a preflight request, should match our plugin configuration
+      local _, status, headers = http_client.get(PROXY_URL.."/get", {}, {host = "cors3.com"})
+
+      assert.are.equal(200, status)
+      assert.are.equal("example.com", headers["access-control-allow-origin"])
+      assert.are.equal("x-auth-token", headers["access-control-expose-headers"])
+      assert.are.equal(tostring(true), headers["access-control-allow-credentials"])
+    end)
 
   end)