Copilot commented on code in PR #13081:
URL: https://github.com/apache/apisix/pull/13081#discussion_r2928815723
##########
t/plugin/openid-connect2.t:
##########
@@ -711,3 +711,277 @@ property "user1" is required
--- error_code: 400
--- response_body_like
{"error_msg":"failed to check the configuration of plugin openid-connect err:
check claim_schema failed: .*: invalid JSON type: invalid_type"}
+
+
+
+=== TEST 16: untrusted source with forged X-Forwarded-Host - should use
http_host instead
+--- yaml_config
+apisix:
+ node_listen: 1984
+ enable_admin: false
+deployment:
+ role: data_plane
+ role_data_plane:
+ config_provider: yaml
+--- apisix_yaml
+routes:
+ -
+ id: 1
+ uri: /hello
+ upstream:
+ nodes:
+ "127.0.0.1:1980": 1
+ type: roundrobin
+ plugins:
+ openid-connect:
+ client_id: kbyuFDidLLm280LIwVFiazOqjO3ty8KH
+ client_secret:
60Op4HFM0I8ajz0WdiStAbziZ-VFQttXuxixHHs2R7r7-CW8GR79l-mmLqMhc-Sa
+ discovery: http://127.0.0.1:1980/.well-known/openid-configuration
+ ssl_verify: false
+ timeout: 10
+ scope: apisix
+ unauth_action: auth
+ use_pkce: false
+ session:
+ secret: jwcE5v3pM9VhqLxmxFOH9uZaLo8u7KQK
+#END
+--- config
+ location /t {
+ content_by_lua_block {
+ local http = require "resty.http"
+ local httpc = http.new()
+ local uri = "http://127.0.0.1:"; .. ngx.var.server_port .. "/hello"
+ local res, err = httpc:request_uri(uri, {
+ method = "GET",
+ headers = {
+ ["Host"] = "localhost:8888",
+ ["X-Forwarded-Host"] = "evil.com"
+ }
+ })
+ ngx.status = res.status
+ local location = res.headers['Location']
+ if not location then
+ ngx.say("no Location header")
+ return
+ end
+ local encoded_redirect = string.match(location,
"redirect_uri=([^&]+)")
+ local redirect_uri = ngx.unescape_uri(encoded_redirect)
+ -- should use http_host (localhost:8888), NOT the forged
X-Forwarded-Host
+ if redirect_uri == "http://localhost:8888/hello/.apisix/redirect";
then
+ ngx.say(true)
+ else
+ ngx.say("redirect_uri mismatch: " .. redirect_uri)
+ end
+ }
+ }
+--- timeout: 10s
+--- response_body
+true
+--- error_code: 302
+
+
+
+=== TEST 17: trusted source with X-Forwarded-Host - should use X-Forwarded-Host
+--- yaml_config
+apisix:
+ node_listen: 1984
+ enable_admin: false
+ trusted_addresses:
+ - "127.0.0.1"
+deployment:
+ role: data_plane
+ role_data_plane:
+ config_provider: yaml
+--- apisix_yaml
+routes:
+ -
+ id: 1
+ uri: /hello
+ upstream:
+ nodes:
+ "127.0.0.1:1980": 1
+ type: roundrobin
+ plugins:
+ openid-connect:
+ client_id: kbyuFDidLLm280LIwVFiazOqjO3ty8KH
+ client_secret:
60Op4HFM0I8ajz0WdiStAbziZ-VFQttXuxixHHs2R7r7-CW8GR79l-mmLqMhc-Sa
+ discovery: http://127.0.0.1:1980/.well-known/openid-configuration
+ ssl_verify: false
+ timeout: 10
+ scope: apisix
+ unauth_action: auth
+ use_pkce: false
+ session:
+ secret: jwcE5v3pM9VhqLxmxFOH9uZaLo8u7KQK
+#END
+--- config
+ location /t {
+ content_by_lua_block {
+ local http = require "resty.http"
+ local httpc = http.new()
+ local uri = "http://127.0.0.1:"; .. ngx.var.server_port .. "/hello"
+ local res, err = httpc:request_uri(uri, {
+ method = "GET",
+ headers = {
+ ["Host"] = "internal-host:9080",
+ ["X-Forwarded-Host"] = "public.example.com:8443",
+ ["X-Forwarded-Proto"] = "https"
+ }
+ })
+ ngx.status = res.status
+ local location = res.headers['Location']
+ if not location then
+ ngx.say("no Location header")
+ return
+ end
+ local encoded_redirect = string.match(location,
"redirect_uri=([^&]+)")
+ local redirect_uri = ngx.unescape_uri(encoded_redirect)
+ if redirect_uri ==
"https://public.example.com:8443/hello/.apisix/redirect"; then
+ ngx.say(true)
+ else
+ ngx.say("redirect_uri mismatch: " .. redirect_uri)
+ end
+ }
+ }
+--- timeout: 10s
+--- response_body
+true
+--- error_code: 302
+
+
+
+=== TEST 18: trusted source with Forwarded header takes priority over
X-Forwarded-Host
+--- yaml_config
+apisix:
+ node_listen: 1984
+ enable_admin: false
+ trusted_addresses:
+ - "127.0.0.1"
+deployment:
+ role: data_plane
+ role_data_plane:
+ config_provider: yaml
+--- apisix_yaml
+routes:
+ -
+ id: 1
+ uri: /hello
+ upstream:
+ nodes:
+ "127.0.0.1:1980": 1
+ type: roundrobin
+ plugins:
+ openid-connect:
+ client_id: kbyuFDidLLm280LIwVFiazOqjO3ty8KH
+ client_secret:
60Op4HFM0I8ajz0WdiStAbziZ-VFQttXuxixHHs2R7r7-CW8GR79l-mmLqMhc-Sa
+ discovery: http://127.0.0.1:1980/.well-known/openid-configuration
+ ssl_verify: false
+ timeout: 10
+ scope: apisix
+ unauth_action: auth
+ use_pkce: false
+ session:
+ secret: jwcE5v3pM9VhqLxmxFOH9uZaLo8u7KQK
+#END
+--- config
+ location /t {
+ content_by_lua_block {
+ local http = require "resty.http"
+ local httpc = http.new()
+ local uri = "http://127.0.0.1:"; .. ngx.var.server_port .. "/hello"
+ local res, err = httpc:request_uri(uri, {
+ method = "GET",
+ headers = {
+ ["Host"] = "internal-host:9080",
+ ["X-Forwarded-Host"] = "should-be-ignored.com",
+ ["X-Forwarded-Proto"] = "http",
+ ["Forwarded"] = 'host="cdn.example.com:443";proto="https"'
+ }
+ })
+ ngx.status = res.status
+ local location = res.headers['Location']
+ if not location then
+ ngx.say("no Location header")
+ return
+ end
+ local encoded_redirect = string.match(location,
"redirect_uri=([^&]+)")
Review Comment:
`encoded_redirect` can be nil here; `ngx.unescape_uri(nil)` will throw and
turn the test failure into a Lua error. Please guard the match result and fail
with a helpful message when the Location header doesn't contain `redirect_uri`.
##########
apisix/plugins/openid-connect.lua:
##########
@@ -622,6 +623,73 @@ local function validate_claims_in_oidcauth_response(resp,
conf)
return core.schema.check(conf.claim_schema, data)
end
+
+local function get_forwarded_param(ctx, param_name)
+ local forwarded = ctx.var.http_forwarded
+ if not forwarded then
+ return nil
+ end
+ -- take only the first proxy entry (before any comma)
+ local first = forwarded:match("^([^,]+)")
+ if not first then
+ return nil
+ end
+ for part in first:gmatch("[^;]+") do
+ local name, value = part:match("^%s*([^=]+)%s*=%s*(.-)%s*$")
+ if name and name:lower() == param_name then
+ -- strip surrounding quotes
+ if value:sub(1, 1) == '"' then
+ value = value:sub(2, -2)
+ end
+ return value
+ end
+ end
+ return nil
+end
+
+
+-- Build an absolute redirect_uri from the incoming request.
+local function build_redirect_uri(ctx)
+ local suffix = "/.apisix/redirect"
+ local uri = ctx.var.uri
+ local redirect_path
+ if core.string.has_suffix(uri, suffix) then
+ -- This is the redirection response from the OIDC provider.
+ redirect_path = uri
+ else
+ if string.sub(uri, -1, -1) == "/" then
+ redirect_path = string.sub(uri, 1, -2) .. suffix
+ else
+ redirect_path = uri .. suffix
+ end
+ end
+
+ local scheme
+ local host
+
+ if trusted_addr.is_trusted(ctx.var.realip_remote_addr) then
+ local xfh = ctx.var.http_x_forwarded_host
+ if xfh then
+ -- trim to first value
+ xfh = xfh:match("^%s*([^,%s]+)")
+ end
+ host = get_forwarded_param(ctx, "host")
+ or xfh
+ or ctx.var.http_host or ctx.var.host
+ scheme = get_forwarded_param(ctx, "proto")
+ or ctx.var.http_x_forwarded_proto
+ or ctx.var.scheme
Review Comment:
When the source is trusted, `host` is derived from `Forwarded.host` or
`X-Forwarded-Host`, but `X-Forwarded-Port` is ignored. Many proxies send host
without port and put the port in `X-Forwarded-Port` (or `Forwarded` without a
port), so this can still generate a redirect_uri missing a non-default port.
Consider incorporating `ctx.var.http_x_forwarded_port` (and/or `Forwarded`
port) when the selected host has no explicit port.
##########
t/plugin/openid-connect3.t:
##########
@@ -112,3 +112,154 @@ true
--- error_code: 302
--- error_log
use http proxy
+
+
+
+=== TEST 3: Set up route with plugin matching URI `/hello` with redirect_uri
use default value.
+--- config
+ location /t {
+ content_by_lua_block {
+ local t = require("lib.test_admin").test
+ local code, body = t('/apisix/admin/routes/1',
+ ngx.HTTP_PUT,
+ [[{
+ "plugins": {
+ "openid-connect": {
+ "client_id":
"kbyuFDidLLm280LIwVFiazOqjO3ty8KH",
+ "client_secret":
"60Op4HFM0I8ajz0WdiStAbziZ-VFQttXuxixHHs2R7r7-CW8GR79l-mmLqMhc-Sa",
+ "discovery":
"http://127.0.0.1:1980/.well-known/openid-configuration";,
+ "ssl_verify": false,
+ "timeout": 10,
+ "scope": "apisix",
+ "unauth_action": "auth",
+ "use_pkce": false,
+ "session": {
+ "secret":
"jwcE5v3pM9VhqLxmxFOH9uZaLo8u7KQK"
+ }
+ }
+ },
+ "upstream": {
+ "nodes": {
+ "127.0.0.1:1980": 1
+ },
+ "type": "roundrobin"
+ },
+ "uri": "/hello"
+ }]]
+ )
+ if code >= 300 then
+ ngx.status = code
+ end
+ ngx.say(body)
+ }
+ }
+--- response_body
+passed
+
+
+
+=== TEST 4: The value of redirect_uri should be appended to `.apisix/redirect`
in the original request.
+--- config
+ location /t {
+ content_by_lua_block {
+ local http = require "resty.http"
+ local httpc = http.new()
+ local uri = "http://127.0.0.1:"; .. ngx.var.server_port .. "/hello"
+ local res, err = httpc:request_uri(uri, {method = "GET"})
+ ngx.status = res.status
+ local location = res.headers['Location']
+ if not location then
+ ngx.say("no Location header")
+ return
+ end
+ -- extract and decode the redirect_uri from the Location header
+ local encoded_redirect = string.match(location,
"redirect_uri=([^&]+)")
+ local redirect_uri = ngx.unescape_uri(encoded_redirect)
+ local expected = uri .. "/.apisix/redirect"
+ if string.find(location, 'https://samples.auth0.com/authorize', 1,
true) and
+ string.find(location, 'scope=apisix', 1, true) and
+ string.find(location,
'client_id=kbyuFDidLLm280LIwVFiazOqjO3ty8KH', 1, true) and
+ string.find(location, 'response_type=code', 1, true) and
+ redirect_uri == expected then
+ ngx.say(true)
+ else
+ ngx.say("redirect_uri mismatch, expected: " .. expected .. ",
got: " .. redirect_uri)
+ end
+ }
+ }
+--- timeout: 10s
+--- response_body
+true
+--- error_code: 302
+
+
+
+=== TEST 5: auto-generated redirect_uri preserves non-default port from Host
header
+--- config
+ location /t {
+ content_by_lua_block {
+ local http = require "resty.http"
+ local httpc = http.new()
+ local uri = "http://127.0.0.1:"; .. ngx.var.server_port .. "/hello"
+ local res, err = httpc:request_uri(uri, {
+ method = "GET",
+ headers = {
+ ["Host"] = "localhost:8888"
+ }
+ })
+ ngx.status = res.status
+ local location = res.headers['Location']
+ if not location then
+ ngx.say("no Location header")
+ return
+ end
+ local encoded_redirect = string.match(location,
"redirect_uri=([^&]+)")
+ local redirect_uri = ngx.unescape_uri(encoded_redirect)
+ local expected = "http://localhost:8888/hello/.apisix/redirect";
+ if redirect_uri == expected then
+ ngx.say(true)
+ else
+ ngx.say("redirect_uri mismatch, expected: " .. expected .. ",
got: " .. redirect_uri)
+ end
+ }
+ }
+--- timeout: 10s
+--- response_body
+true
+--- error_code: 302
+
+
+
+=== TEST 6: auto-generated redirect_uri omits port for default HTTP port
+--- config
+ location /t {
+ content_by_lua_block {
+ local http = require "resty.http"
+ local httpc = http.new()
+ local uri = "http://127.0.0.1:"; .. ngx.var.server_port .. "/hello"
+ local res, err = httpc:request_uri(uri, {
+ method = "GET",
+ headers = {
+ ["Host"] = "localhost"
+ }
+ })
+ ngx.status = res.status
+ local location = res.headers['Location']
+ if not location then
+ ngx.say("no Location header")
+ return
+ end
+ local encoded_redirect = string.match(location,
"redirect_uri=([^&]+)")
Review Comment:
`encoded_redirect` may be nil here; calling
`ngx.unescape_uri(encoded_redirect)` would throw and obscure the actual
failure. Add a nil-check for the match result and fail the test with a clear
message if redirect_uri isn't present.
##########
apisix/plugins/openid-connect.lua:
##########
@@ -622,6 +623,73 @@ local function validate_claims_in_oidcauth_response(resp,
conf)
return core.schema.check(conf.claim_schema, data)
end
+
+local function get_forwarded_param(ctx, param_name)
+ local forwarded = ctx.var.http_forwarded
+ if not forwarded then
+ return nil
+ end
+ -- take only the first proxy entry (before any comma)
+ local first = forwarded:match("^([^,]+)")
+ if not first then
+ return nil
+ end
+ for part in first:gmatch("[^;]+") do
Review Comment:
`Forwarded` can contain multiple comma-separated forwarded-elements (one per
proxy). Taking the *first* element (`forwarded:match("^([^,]+)")`) can end up
using client-supplied values when a trusted proxy appends its own element to
the end of the header, which is a potential host/proto spoofing vector. Prefer
selecting the last forwarded-element added by the trusted proxy (right-most),
or otherwise ensure you only use the element that the trusted hop set/overwrote.
##########
t/plugin/openid-connect2.t:
##########
@@ -711,3 +711,277 @@ property "user1" is required
--- error_code: 400
--- response_body_like
{"error_msg":"failed to check the configuration of plugin openid-connect err:
check claim_schema failed: .*: invalid JSON type: invalid_type"}
+
+
+
+=== TEST 16: untrusted source with forged X-Forwarded-Host - should use
http_host instead
+--- yaml_config
+apisix:
+ node_listen: 1984
+ enable_admin: false
+deployment:
+ role: data_plane
+ role_data_plane:
+ config_provider: yaml
+--- apisix_yaml
+routes:
+ -
+ id: 1
+ uri: /hello
+ upstream:
+ nodes:
+ "127.0.0.1:1980": 1
+ type: roundrobin
+ plugins:
+ openid-connect:
+ client_id: kbyuFDidLLm280LIwVFiazOqjO3ty8KH
+ client_secret:
60Op4HFM0I8ajz0WdiStAbziZ-VFQttXuxixHHs2R7r7-CW8GR79l-mmLqMhc-Sa
+ discovery: http://127.0.0.1:1980/.well-known/openid-configuration
+ ssl_verify: false
+ timeout: 10
+ scope: apisix
+ unauth_action: auth
+ use_pkce: false
+ session:
+ secret: jwcE5v3pM9VhqLxmxFOH9uZaLo8u7KQK
+#END
+--- config
+ location /t {
+ content_by_lua_block {
+ local http = require "resty.http"
+ local httpc = http.new()
+ local uri = "http://127.0.0.1:"; .. ngx.var.server_port .. "/hello"
+ local res, err = httpc:request_uri(uri, {
+ method = "GET",
+ headers = {
+ ["Host"] = "localhost:8888",
+ ["X-Forwarded-Host"] = "evil.com"
+ }
+ })
+ ngx.status = res.status
+ local location = res.headers['Location']
+ if not location then
+ ngx.say("no Location header")
+ return
+ end
+ local encoded_redirect = string.match(location,
"redirect_uri=([^&]+)")
Review Comment:
`string.match(location, "redirect_uri=([^&]+)")` can return nil;
`ngx.unescape_uri(nil)` will throw and makes the test fail with a Lua exception
instead of a clear mismatch message. Please guard for missing `redirect_uri`
and print a helpful failure output.
##########
apisix/plugins/openid-connect.lua:
##########
@@ -622,6 +623,73 @@ local function validate_claims_in_oidcauth_response(resp,
conf)
return core.schema.check(conf.claim_schema, data)
end
+
+local function get_forwarded_param(ctx, param_name)
+ local forwarded = ctx.var.http_forwarded
+ if not forwarded then
+ return nil
+ end
+ -- take only the first proxy entry (before any comma)
+ local first = forwarded:match("^([^,]+)")
+ if not first then
+ return nil
+ end
+ for part in first:gmatch("[^;]+") do
+ local name, value = part:match("^%s*([^=]+)%s*=%s*(.-)%s*$")
+ if name and name:lower() == param_name then
+ -- strip surrounding quotes
+ if value:sub(1, 1) == '"' then
+ value = value:sub(2, -2)
+ end
+ return value
+ end
+ end
+ return nil
+end
+
+
+-- Build an absolute redirect_uri from the incoming request.
+local function build_redirect_uri(ctx)
+ local suffix = "/.apisix/redirect"
+ local uri = ctx.var.uri
+ local redirect_path
+ if core.string.has_suffix(uri, suffix) then
+ -- This is the redirection response from the OIDC provider.
Review Comment:
The comment indentation here looks off (extra leading spaces) which makes
the block harder to read and inconsistent with surrounding style. Please align
the comment with the code indentation level.
##########
t/plugin/openid-connect3.t:
##########
@@ -112,3 +112,154 @@ true
--- error_code: 302
--- error_log
use http proxy
+
+
+
+=== TEST 3: Set up route with plugin matching URI `/hello` with redirect_uri
use default value.
+--- config
+ location /t {
+ content_by_lua_block {
+ local t = require("lib.test_admin").test
+ local code, body = t('/apisix/admin/routes/1',
+ ngx.HTTP_PUT,
+ [[{
+ "plugins": {
+ "openid-connect": {
+ "client_id":
"kbyuFDidLLm280LIwVFiazOqjO3ty8KH",
+ "client_secret":
"60Op4HFM0I8ajz0WdiStAbziZ-VFQttXuxixHHs2R7r7-CW8GR79l-mmLqMhc-Sa",
+ "discovery":
"http://127.0.0.1:1980/.well-known/openid-configuration";,
+ "ssl_verify": false,
+ "timeout": 10,
+ "scope": "apisix",
+ "unauth_action": "auth",
+ "use_pkce": false,
+ "session": {
+ "secret":
"jwcE5v3pM9VhqLxmxFOH9uZaLo8u7KQK"
+ }
+ }
+ },
+ "upstream": {
+ "nodes": {
+ "127.0.0.1:1980": 1
+ },
+ "type": "roundrobin"
+ },
+ "uri": "/hello"
+ }]]
+ )
+ if code >= 300 then
+ ngx.status = code
+ end
+ ngx.say(body)
+ }
+ }
+--- response_body
+passed
+
+
+
+=== TEST 4: The value of redirect_uri should be appended to `.apisix/redirect`
in the original request.
+--- config
+ location /t {
+ content_by_lua_block {
+ local http = require "resty.http"
+ local httpc = http.new()
+ local uri = "http://127.0.0.1:"; .. ngx.var.server_port .. "/hello"
+ local res, err = httpc:request_uri(uri, {method = "GET"})
+ ngx.status = res.status
+ local location = res.headers['Location']
+ if not location then
+ ngx.say("no Location header")
+ return
+ end
+ -- extract and decode the redirect_uri from the Location header
+ local encoded_redirect = string.match(location,
"redirect_uri=([^&]+)")
+ local redirect_uri = ngx.unescape_uri(encoded_redirect)
+ local expected = uri .. "/.apisix/redirect"
+ if string.find(location, 'https://samples.auth0.com/authorize', 1,
true) and
+ string.find(location, 'scope=apisix', 1, true) and
+ string.find(location,
'client_id=kbyuFDidLLm280LIwVFiazOqjO3ty8KH', 1, true) and
+ string.find(location, 'response_type=code', 1, true) and
+ redirect_uri == expected then
+ ngx.say(true)
+ else
+ ngx.say("redirect_uri mismatch, expected: " .. expected .. ",
got: " .. redirect_uri)
+ end
+ }
+ }
+--- timeout: 10s
+--- response_body
+true
+--- error_code: 302
+
+
+
+=== TEST 5: auto-generated redirect_uri preserves non-default port from Host
header
+--- config
+ location /t {
+ content_by_lua_block {
+ local http = require "resty.http"
+ local httpc = http.new()
+ local uri = "http://127.0.0.1:"; .. ngx.var.server_port .. "/hello"
+ local res, err = httpc:request_uri(uri, {
+ method = "GET",
+ headers = {
+ ["Host"] = "localhost:8888"
+ }
+ })
+ ngx.status = res.status
+ local location = res.headers['Location']
+ if not location then
+ ngx.say("no Location header")
+ return
+ end
+ local encoded_redirect = string.match(location,
"redirect_uri=([^&]+)")
+ local redirect_uri = ngx.unescape_uri(encoded_redirect)
Review Comment:
`encoded_redirect` may be nil here; calling
`ngx.unescape_uri(encoded_redirect)` would throw and obscure the actual
failure. Add a nil-check for the match result and fail the test with a clear
message if redirect_uri isn't present.
##########
t/plugin/openid-connect2.t:
##########
@@ -711,3 +711,277 @@ property "user1" is required
--- error_code: 400
--- response_body_like
{"error_msg":"failed to check the configuration of plugin openid-connect err:
check claim_schema failed: .*: invalid JSON type: invalid_type"}
+
+
+
+=== TEST 16: untrusted source with forged X-Forwarded-Host - should use
http_host instead
+--- yaml_config
+apisix:
+ node_listen: 1984
+ enable_admin: false
+deployment:
+ role: data_plane
+ role_data_plane:
+ config_provider: yaml
+--- apisix_yaml
+routes:
+ -
+ id: 1
+ uri: /hello
+ upstream:
+ nodes:
+ "127.0.0.1:1980": 1
+ type: roundrobin
+ plugins:
+ openid-connect:
+ client_id: kbyuFDidLLm280LIwVFiazOqjO3ty8KH
+ client_secret:
60Op4HFM0I8ajz0WdiStAbziZ-VFQttXuxixHHs2R7r7-CW8GR79l-mmLqMhc-Sa
+ discovery: http://127.0.0.1:1980/.well-known/openid-configuration
+ ssl_verify: false
+ timeout: 10
+ scope: apisix
+ unauth_action: auth
+ use_pkce: false
+ session:
+ secret: jwcE5v3pM9VhqLxmxFOH9uZaLo8u7KQK
+#END
+--- config
+ location /t {
+ content_by_lua_block {
+ local http = require "resty.http"
+ local httpc = http.new()
+ local uri = "http://127.0.0.1:"; .. ngx.var.server_port .. "/hello"
+ local res, err = httpc:request_uri(uri, {
+ method = "GET",
+ headers = {
+ ["Host"] = "localhost:8888",
+ ["X-Forwarded-Host"] = "evil.com"
+ }
+ })
+ ngx.status = res.status
+ local location = res.headers['Location']
+ if not location then
+ ngx.say("no Location header")
+ return
+ end
+ local encoded_redirect = string.match(location,
"redirect_uri=([^&]+)")
+ local redirect_uri = ngx.unescape_uri(encoded_redirect)
+ -- should use http_host (localhost:8888), NOT the forged
X-Forwarded-Host
+ if redirect_uri == "http://localhost:8888/hello/.apisix/redirect";
then
+ ngx.say(true)
+ else
+ ngx.say("redirect_uri mismatch: " .. redirect_uri)
+ end
+ }
+ }
+--- timeout: 10s
+--- response_body
+true
+--- error_code: 302
+
+
+
+=== TEST 17: trusted source with X-Forwarded-Host - should use X-Forwarded-Host
+--- yaml_config
+apisix:
+ node_listen: 1984
+ enable_admin: false
+ trusted_addresses:
+ - "127.0.0.1"
+deployment:
+ role: data_plane
+ role_data_plane:
+ config_provider: yaml
+--- apisix_yaml
+routes:
+ -
+ id: 1
+ uri: /hello
+ upstream:
+ nodes:
+ "127.0.0.1:1980": 1
+ type: roundrobin
+ plugins:
+ openid-connect:
+ client_id: kbyuFDidLLm280LIwVFiazOqjO3ty8KH
+ client_secret:
60Op4HFM0I8ajz0WdiStAbziZ-VFQttXuxixHHs2R7r7-CW8GR79l-mmLqMhc-Sa
+ discovery: http://127.0.0.1:1980/.well-known/openid-configuration
+ ssl_verify: false
+ timeout: 10
+ scope: apisix
+ unauth_action: auth
+ use_pkce: false
+ session:
+ secret: jwcE5v3pM9VhqLxmxFOH9uZaLo8u7KQK
+#END
+--- config
+ location /t {
+ content_by_lua_block {
+ local http = require "resty.http"
+ local httpc = http.new()
+ local uri = "http://127.0.0.1:"; .. ngx.var.server_port .. "/hello"
+ local res, err = httpc:request_uri(uri, {
+ method = "GET",
+ headers = {
+ ["Host"] = "internal-host:9080",
+ ["X-Forwarded-Host"] = "public.example.com:8443",
+ ["X-Forwarded-Proto"] = "https"
+ }
+ })
+ ngx.status = res.status
+ local location = res.headers['Location']
+ if not location then
+ ngx.say("no Location header")
+ return
+ end
+ local encoded_redirect = string.match(location,
"redirect_uri=([^&]+)")
Review Comment:
`encoded_redirect` can be nil here; calling
`ngx.unescape_uri(encoded_redirect)` will throw and obscure the assertion
failure. Add a nil-check and emit a clear message if the `redirect_uri`
parameter isn't found in the Location header.
##########
t/plugin/openid-connect3.t:
##########
@@ -112,3 +112,154 @@ true
--- error_code: 302
--- error_log
use http proxy
+
+
+
+=== TEST 3: Set up route with plugin matching URI `/hello` with redirect_uri
use default value.
+--- config
+ location /t {
+ content_by_lua_block {
+ local t = require("lib.test_admin").test
+ local code, body = t('/apisix/admin/routes/1',
+ ngx.HTTP_PUT,
+ [[{
+ "plugins": {
+ "openid-connect": {
+ "client_id":
"kbyuFDidLLm280LIwVFiazOqjO3ty8KH",
+ "client_secret":
"60Op4HFM0I8ajz0WdiStAbziZ-VFQttXuxixHHs2R7r7-CW8GR79l-mmLqMhc-Sa",
+ "discovery":
"http://127.0.0.1:1980/.well-known/openid-configuration";,
+ "ssl_verify": false,
+ "timeout": 10,
+ "scope": "apisix",
+ "unauth_action": "auth",
+ "use_pkce": false,
+ "session": {
+ "secret":
"jwcE5v3pM9VhqLxmxFOH9uZaLo8u7KQK"
+ }
+ }
+ },
+ "upstream": {
+ "nodes": {
+ "127.0.0.1:1980": 1
+ },
+ "type": "roundrobin"
+ },
+ "uri": "/hello"
+ }]]
+ )
+ if code >= 300 then
+ ngx.status = code
+ end
+ ngx.say(body)
+ }
+ }
+--- response_body
+passed
+
+
+
+=== TEST 4: The value of redirect_uri should be appended to `.apisix/redirect`
in the original request.
+--- config
+ location /t {
+ content_by_lua_block {
+ local http = require "resty.http"
+ local httpc = http.new()
+ local uri = "http://127.0.0.1:"; .. ngx.var.server_port .. "/hello"
+ local res, err = httpc:request_uri(uri, {method = "GET"})
+ ngx.status = res.status
+ local location = res.headers['Location']
+ if not location then
+ ngx.say("no Location header")
+ return
+ end
+ -- extract and decode the redirect_uri from the Location header
+ local encoded_redirect = string.match(location,
"redirect_uri=([^&]+)")
Review Comment:
`string.match(location, "redirect_uri=([^&]+)")` can return nil if the
Location header format changes or redirect_uri is missing, and
`ngx.unescape_uri(nil)` will raise an error (making the test fail with a Lua
exception rather than a clear assertion). Add a guard for `encoded_redirect ==
nil` and print a useful failure message.
##########
t/plugin/openid-connect3.t:
##########
@@ -112,3 +112,154 @@ true
--- error_code: 302
--- error_log
use http proxy
+
+
+
+=== TEST 3: Set up route with plugin matching URI `/hello` with redirect_uri
use default value.
+--- config
+ location /t {
+ content_by_lua_block {
+ local t = require("lib.test_admin").test
+ local code, body = t('/apisix/admin/routes/1',
+ ngx.HTTP_PUT,
+ [[{
+ "plugins": {
+ "openid-connect": {
+ "client_id":
"kbyuFDidLLm280LIwVFiazOqjO3ty8KH",
+ "client_secret":
"60Op4HFM0I8ajz0WdiStAbziZ-VFQttXuxixHHs2R7r7-CW8GR79l-mmLqMhc-Sa",
+ "discovery":
"http://127.0.0.1:1980/.well-known/openid-configuration";,
+ "ssl_verify": false,
+ "timeout": 10,
+ "scope": "apisix",
+ "unauth_action": "auth",
+ "use_pkce": false,
+ "session": {
+ "secret":
"jwcE5v3pM9VhqLxmxFOH9uZaLo8u7KQK"
+ }
+ }
+ },
+ "upstream": {
+ "nodes": {
+ "127.0.0.1:1980": 1
+ },
+ "type": "roundrobin"
+ },
+ "uri": "/hello"
+ }]]
+ )
+ if code >= 300 then
+ ngx.status = code
+ end
+ ngx.say(body)
+ }
+ }
+--- response_body
+passed
+
+
+
+=== TEST 4: The value of redirect_uri should be appended to `.apisix/redirect`
in the original request.
+--- config
+ location /t {
+ content_by_lua_block {
+ local http = require "resty.http"
+ local httpc = http.new()
+ local uri = "http://127.0.0.1:"; .. ngx.var.server_port .. "/hello"
+ local res, err = httpc:request_uri(uri, {method = "GET"})
+ ngx.status = res.status
+ local location = res.headers['Location']
+ if not location then
+ ngx.say("no Location header")
+ return
+ end
+ -- extract and decode the redirect_uri from the Location header
+ local encoded_redirect = string.match(location,
"redirect_uri=([^&]+)")
+ local redirect_uri = ngx.unescape_uri(encoded_redirect)
+ local expected = uri .. "/.apisix/redirect"
+ if string.find(location, 'https://samples.auth0.com/authorize', 1,
true) and
+ string.find(location, 'scope=apisix', 1, true) and
+ string.find(location,
'client_id=kbyuFDidLLm280LIwVFiazOqjO3ty8KH', 1, true) and
+ string.find(location, 'response_type=code', 1, true) and
+ redirect_uri == expected then
+ ngx.say(true)
+ else
+ ngx.say("redirect_uri mismatch, expected: " .. expected .. ",
got: " .. redirect_uri)
+ end
+ }
+ }
+--- timeout: 10s
+--- response_body
+true
+--- error_code: 302
+
+
+
+=== TEST 5: auto-generated redirect_uri preserves non-default port from Host
header
+--- config
+ location /t {
+ content_by_lua_block {
+ local http = require "resty.http"
+ local httpc = http.new()
+ local uri = "http://127.0.0.1:"; .. ngx.var.server_port .. "/hello"
+ local res, err = httpc:request_uri(uri, {
+ method = "GET",
+ headers = {
+ ["Host"] = "localhost:8888"
+ }
+ })
+ ngx.status = res.status
+ local location = res.headers['Location']
+ if not location then
+ ngx.say("no Location header")
+ return
+ end
+ local encoded_redirect = string.match(location,
"redirect_uri=([^&]+)")
+ local redirect_uri = ngx.unescape_uri(encoded_redirect)
+ local expected = "http://localhost:8888/hello/.apisix/redirect";
+ if redirect_uri == expected then
+ ngx.say(true)
+ else
+ ngx.say("redirect_uri mismatch, expected: " .. expected .. ",
got: " .. redirect_uri)
+ end
+ }
+ }
+--- timeout: 10s
+--- response_body
+true
+--- error_code: 302
+
+
+
+=== TEST 6: auto-generated redirect_uri omits port for default HTTP port
Review Comment:
The test title mentions "default HTTP port", but the test is actually
asserting behavior when the `Host` header omits an explicit port (even though
the request is made to a non-default listen port). Consider renaming the test
description to reflect what it actually validates to avoid confusion.
##########
t/plugin/openid-connect2.t:
##########
@@ -711,3 +711,277 @@ property "user1" is required
--- error_code: 400
--- response_body_like
{"error_msg":"failed to check the configuration of plugin openid-connect err:
check claim_schema failed: .*: invalid JSON type: invalid_type"}
+
+
+
+=== TEST 16: untrusted source with forged X-Forwarded-Host - should use
http_host instead
+--- yaml_config
+apisix:
+ node_listen: 1984
+ enable_admin: false
+deployment:
+ role: data_plane
+ role_data_plane:
+ config_provider: yaml
+--- apisix_yaml
+routes:
+ -
+ id: 1
+ uri: /hello
+ upstream:
+ nodes:
+ "127.0.0.1:1980": 1
+ type: roundrobin
+ plugins:
+ openid-connect:
+ client_id: kbyuFDidLLm280LIwVFiazOqjO3ty8KH
+ client_secret:
60Op4HFM0I8ajz0WdiStAbziZ-VFQttXuxixHHs2R7r7-CW8GR79l-mmLqMhc-Sa
+ discovery: http://127.0.0.1:1980/.well-known/openid-configuration
+ ssl_verify: false
+ timeout: 10
+ scope: apisix
+ unauth_action: auth
+ use_pkce: false
+ session:
+ secret: jwcE5v3pM9VhqLxmxFOH9uZaLo8u7KQK
+#END
+--- config
+ location /t {
+ content_by_lua_block {
+ local http = require "resty.http"
+ local httpc = http.new()
+ local uri = "http://127.0.0.1:"; .. ngx.var.server_port .. "/hello"
+ local res, err = httpc:request_uri(uri, {
+ method = "GET",
+ headers = {
+ ["Host"] = "localhost:8888",
+ ["X-Forwarded-Host"] = "evil.com"
+ }
+ })
+ ngx.status = res.status
+ local location = res.headers['Location']
+ if not location then
+ ngx.say("no Location header")
+ return
+ end
+ local encoded_redirect = string.match(location,
"redirect_uri=([^&]+)")
+ local redirect_uri = ngx.unescape_uri(encoded_redirect)
+ -- should use http_host (localhost:8888), NOT the forged
X-Forwarded-Host
+ if redirect_uri == "http://localhost:8888/hello/.apisix/redirect";
then
+ ngx.say(true)
+ else
+ ngx.say("redirect_uri mismatch: " .. redirect_uri)
+ end
+ }
+ }
+--- timeout: 10s
+--- response_body
+true
+--- error_code: 302
+
+
+
+=== TEST 17: trusted source with X-Forwarded-Host - should use X-Forwarded-Host
+--- yaml_config
+apisix:
+ node_listen: 1984
+ enable_admin: false
+ trusted_addresses:
+ - "127.0.0.1"
+deployment:
+ role: data_plane
+ role_data_plane:
+ config_provider: yaml
+--- apisix_yaml
+routes:
+ -
+ id: 1
+ uri: /hello
+ upstream:
+ nodes:
+ "127.0.0.1:1980": 1
+ type: roundrobin
+ plugins:
+ openid-connect:
+ client_id: kbyuFDidLLm280LIwVFiazOqjO3ty8KH
+ client_secret:
60Op4HFM0I8ajz0WdiStAbziZ-VFQttXuxixHHs2R7r7-CW8GR79l-mmLqMhc-Sa
+ discovery: http://127.0.0.1:1980/.well-known/openid-configuration
+ ssl_verify: false
+ timeout: 10
+ scope: apisix
+ unauth_action: auth
+ use_pkce: false
+ session:
+ secret: jwcE5v3pM9VhqLxmxFOH9uZaLo8u7KQK
+#END
+--- config
+ location /t {
+ content_by_lua_block {
+ local http = require "resty.http"
+ local httpc = http.new()
+ local uri = "http://127.0.0.1:"; .. ngx.var.server_port .. "/hello"
+ local res, err = httpc:request_uri(uri, {
+ method = "GET",
+ headers = {
+ ["Host"] = "internal-host:9080",
+ ["X-Forwarded-Host"] = "public.example.com:8443",
+ ["X-Forwarded-Proto"] = "https"
+ }
+ })
+ ngx.status = res.status
+ local location = res.headers['Location']
+ if not location then
+ ngx.say("no Location header")
+ return
+ end
+ local encoded_redirect = string.match(location,
"redirect_uri=([^&]+)")
+ local redirect_uri = ngx.unescape_uri(encoded_redirect)
+ if redirect_uri ==
"https://public.example.com:8443/hello/.apisix/redirect"; then
+ ngx.say(true)
+ else
+ ngx.say("redirect_uri mismatch: " .. redirect_uri)
+ end
+ }
+ }
+--- timeout: 10s
+--- response_body
+true
+--- error_code: 302
+
+
+
+=== TEST 18: trusted source with Forwarded header takes priority over
X-Forwarded-Host
+--- yaml_config
+apisix:
+ node_listen: 1984
+ enable_admin: false
+ trusted_addresses:
+ - "127.0.0.1"
+deployment:
+ role: data_plane
+ role_data_plane:
+ config_provider: yaml
+--- apisix_yaml
+routes:
+ -
+ id: 1
+ uri: /hello
+ upstream:
+ nodes:
+ "127.0.0.1:1980": 1
+ type: roundrobin
+ plugins:
+ openid-connect:
+ client_id: kbyuFDidLLm280LIwVFiazOqjO3ty8KH
+ client_secret:
60Op4HFM0I8ajz0WdiStAbziZ-VFQttXuxixHHs2R7r7-CW8GR79l-mmLqMhc-Sa
+ discovery: http://127.0.0.1:1980/.well-known/openid-configuration
+ ssl_verify: false
+ timeout: 10
+ scope: apisix
+ unauth_action: auth
+ use_pkce: false
+ session:
+ secret: jwcE5v3pM9VhqLxmxFOH9uZaLo8u7KQK
+#END
+--- config
+ location /t {
+ content_by_lua_block {
+ local http = require "resty.http"
+ local httpc = http.new()
+ local uri = "http://127.0.0.1:"; .. ngx.var.server_port .. "/hello"
+ local res, err = httpc:request_uri(uri, {
+ method = "GET",
+ headers = {
+ ["Host"] = "internal-host:9080",
+ ["X-Forwarded-Host"] = "should-be-ignored.com",
+ ["X-Forwarded-Proto"] = "http",
+ ["Forwarded"] = 'host="cdn.example.com:443";proto="https"'
+ }
+ })
+ ngx.status = res.status
+ local location = res.headers['Location']
+ if not location then
+ ngx.say("no Location header")
+ return
+ end
+ local encoded_redirect = string.match(location,
"redirect_uri=([^&]+)")
+ local redirect_uri = ngx.unescape_uri(encoded_redirect)
+ if redirect_uri ==
"https://cdn.example.com:443/hello/.apisix/redirect"; then
+ ngx.say(true)
+ else
+ ngx.say("redirect_uri mismatch: " .. redirect_uri)
+ end
+ }
+ }
+--- timeout: 10s
+--- response_body
+true
+--- error_code: 302
+
+
+
+=== TEST 19: untrusted source with forged Forwarded header - should be ignored
+--- yaml_config
+apisix:
+ node_listen: 1984
+ enable_admin: false
+deployment:
+ role: data_plane
+ role_data_plane:
+ config_provider: yaml
+--- apisix_yaml
+routes:
+ -
+ id: 1
+ uri: /hello
+ upstream:
+ nodes:
+ "127.0.0.1:1980": 1
+ type: roundrobin
+ plugins:
+ openid-connect:
+ client_id: kbyuFDidLLm280LIwVFiazOqjO3ty8KH
+ client_secret:
60Op4HFM0I8ajz0WdiStAbziZ-VFQttXuxixHHs2R7r7-CW8GR79l-mmLqMhc-Sa
+ discovery: http://127.0.0.1:1980/.well-known/openid-configuration
+ ssl_verify: false
+ timeout: 10
+ scope: apisix
+ unauth_action: auth
+ use_pkce: false
+ session:
+ secret: jwcE5v3pM9VhqLxmxFOH9uZaLo8u7KQK
+#END
+--- config
+ location /t {
+ content_by_lua_block {
+ local http = require "resty.http"
+ local httpc = http.new()
+ local uri = "http://127.0.0.1:"; .. ngx.var.server_port .. "/hello"
+ local res, err = httpc:request_uri(uri, {
+ method = "GET",
+ headers = {
+ ["Host"] = "localhost:8888",
+ ["Forwarded"] = 'host="evil.com";proto="https"',
+ ["X-Forwarded-Host"] = "also-evil.com"
+ }
+ })
+ ngx.status = res.status
+ local location = res.headers['Location']
+ if not location then
+ ngx.say("no Location header")
+ return
+ end
+ local encoded_redirect = string.match(location,
"redirect_uri=([^&]+)")
Review Comment:
`encoded_redirect` can be nil here; `ngx.unescape_uri(nil)` will throw and
obscure the real mismatch. Add a nil-check for the match result and output a
clear diagnostic when `redirect_uri` isn't present.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
