Skip to content

Commit

Permalink
fix(cors): don't send ACAO header where a single origin(not regex) is
Browse files Browse the repository at this point in the history
configured in case of non-matched.

The CORS plugin currently always sets the ACAO header based on the
plugin configuration if the configuration only has a single entry and
contains only non-PCRE metacharacters. This behavior is first introduced
in #2482 and doesn't seem to have any real impact on the functionality I
think.

But this seems is not following the [mozilla
guidelines](https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Access-Control-Allow-Origin)

> Specifies an origin. Only a single origin can be specified. If the
server supports clients from multiple origins, it must return the origin
for the specific client making the request.

This fixes behavior by no longer sending an ACAO header in this case.
  • Loading branch information
ms2008 committed May 25, 2023
1 parent 3ee58cb commit 3b4f9a4
Show file tree
Hide file tree
Showing 2 changed files with 27 additions and 42 deletions.
15 changes: 0 additions & 15 deletions kong/plugins/cors/handler.lua
Original file line number Diff line number Diff line change
Expand Up @@ -74,21 +74,6 @@ local function configure_origin(conf, header_filter)
-- https://github.com/rs/cors/issues/10
add_vary_header(header_filter)

if n_origins == 1 then
-- if this doesnt look like a regex, set the ACAO header directly
-- otherwise, we'll fall through to an iterative search and
-- set the ACAO header based on the client Origin
local from, _, err = re_find(conf.origins[1], "^[A-Za-z0-9.:/-]+$", "jo")
if err then
kong.log.err("could not inspect origin for type: ", err)
end

if from then
set_header("Access-Control-Allow-Origin", conf.origins[1])
return false
end
end

local req_origin = kong.request.get_header("origin")
if req_origin then
local cached_domains = config_cache[conf]
Expand Down
54 changes: 27 additions & 27 deletions spec/03-plugins/13-cors/01-access_spec.lua
Original file line number Diff line number Diff line change
Expand Up @@ -24,36 +24,36 @@ for _, strategy in helpers.each_strategy() do

local regex_testcases = {
{
-- single entry, host only: ignore value, always return configured data
-- single entry, host only: match on full normalized domain (i.e. all fail)
origins = { "foo.test" },
tests = {
["http://evil.test"] = "foo.test",
["http://foo.test"] = "foo.test",
["http://foo.test.evil.test"] = "foo.test",
["http://something.foo.test"] = "foo.test",
["http://evilfoo.test"] = "foo.test",
["http://foo.test:80"] = "foo.test",
["http://foo.test:8000"] = "foo.test",
["https://foo.test:8000"] = "foo.test",
["http://foo.test:90"] = "foo.test",
["http://foobtest"] = "foo.test",
["https://bar.test:1234"] = "foo.test",
["http://evil.test"] = false,
["http://foo.test"] = false,
["http://foo.test.evil.test"] = false,
["http://something.foo.test"] = false,
["http://evilfoo.test"] = false,
["http://foo.test:80"] = false,
["http://foo.test:8000"] = false,
["https://foo.test:8000"] = false,
["http://foo.test:90"] = false,
["http://foobtest"] = false,
["https://bar.test:1234"] = false,
},
},
{
-- single entry, full domain (not regex): ignore value, always return configured data
-- single entry, full domain (not regex): match on full normalized domain
origins = { "https://bar.test:1234" },
tests = {
["http://evil.test"] = "https://bar.test:1234",
["http://foo.test"] = "https://bar.test:1234",
["http://foo.test.evil.test"] = "https://bar.test:1234",
["http://something.foo.test"] = "https://bar.test:1234",
["http://evilfoo.test"] = "https://bar.test:1234",
["http://foo.test:80"] = "https://bar.test:1234",
["http://foo.test:8000"] = "https://bar.test:1234",
["https://foo.test:8000"] = "https://bar.test:1234",
["http://foo.test:90"] = "https://bar.test:1234",
["http://foobtest"] = "https://bar.test:1234",
["http://evil.test"] = false,
["http://foo.test"] = false,
["http://foo.test.evil.test"] = false,
["http://something.foo.test"] = false,
["http://evilfoo.test"] = false,
["http://foo.test:80"] = false,
["http://foo.test:8000"] = false,
["https://foo.test:8000"] = false,
["http://foo.test:90"] = false,
["http://foobtest"] = false,
["https://bar.test:1234"] = "https://bar.test:1234",
},
},
Expand Down Expand Up @@ -631,7 +631,7 @@ for _, strategy in helpers.each_strategy() do
assert.res_status(200, res)
assert.equal("0", res.headers["Content-Length"])
assert.equal("GET", res.headers["Access-Control-Allow-Methods"])
assert.equal("example.com", res.headers["Access-Control-Allow-Origin"])
assert.is_nil(res.headers["Access-Control-Allow-Origin"])
assert.equal("23", res.headers["Access-Control-Max-Age"])
assert.equal("true", res.headers["Access-Control-Allow-Credentials"])
assert.equal("origin,type,accepts", res.headers["Access-Control-Allow-Headers"])
Expand Down Expand Up @@ -753,7 +753,7 @@ for _, strategy in helpers.each_strategy() do
}
})
assert.res_status(200, res)
assert.equal("example.com", res.headers["Access-Control-Allow-Origin"])
assert.is_nil(res.headers["Access-Control-Allow-Origin"])
assert.equal("x-auth-token", res.headers["Access-Control-Expose-Headers"])
assert.equal("true", res.headers["Access-Control-Allow-Credentials"])
assert.equal("Origin", res.headers["Vary"])
Expand All @@ -770,7 +770,7 @@ for _, strategy in helpers.each_strategy() do
}
})
assert.res_status(502, res)
assert.equal("example.com", res.headers["Access-Control-Allow-Origin"])
assert.is_nil(res.headers["Access-Control-Allow-Origin"])
assert.equal("x-auth-token", res.headers["Access-Control-Expose-Headers"])
assert.equal("Origin", res.headers["Vary"])
assert.is_nil(res.headers["Access-Control-Allow-Credentials"])
Expand All @@ -787,7 +787,7 @@ for _, strategy in helpers.each_strategy() do
}
})
assert.res_status(500, res)
assert.equal("example.com", res.headers["Access-Control-Allow-Origin"])
assert.is_nil(res.headers["Access-Control-Allow-Origin"])
assert.equal("x-auth-token", res.headers["Access-Control-Expose-Headers"])
assert.equal("Origin", res.headers["Vary"])
assert.is_nil(res.headers["Access-Control-Allow-Credentials"])
Expand Down

0 comments on commit 3b4f9a4

Please sign in to comment.