Copilot commented on code in PR #13496:
URL: https://github.com/apache/apisix/pull/13496#discussion_r3384896227
##########
t/plugin/openid-connect.t:
##########
@@ -1842,3 +1842,200 @@ done
--- response_body
property "client_secret" is required
done
+
+
+
+=== TEST 51: Configure plugin with a custom session.cookie_name.
+--- 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": {
+ "discovery":
"http://127.0.0.1:8080/realms/University/.well-known/openid-configuration";,
+ "realm": "University",
+ "client_id": "course_management",
+ "client_secret":
"d1ec69e9-55d2-4109-a3ea-befa071579d5",
+ "redirect_uri": "http://127.0.0.1:]] ..
ngx.var.server_port .. [[/authenticated",
+ "ssl_verify": false,
+ "timeout": 10,
+ "session": {
+ "secret":
"jwcE5v3pM9VhqLxmxFOH9uZaLo8u7KQK",
+ "cookie_name": "custom_session"
+ }
+ }
+ },
+ "upstream": {
+ "nodes": {
+ "127.0.0.1:1980": 1
+ },
+ "type": "roundrobin"
+ },
+ "uri": "/*"
+ }]]
+ )
+
+ if code >= 300 then
+ ngx.status = code
+ end
+ ngx.say(body)
+ }
+ }
+--- response_body
+passed
+
+
+
+=== TEST 52: Full OIDC login issues the session cookie under the configured
cookie_name.
+--- config
+ location /t {
+ content_by_lua_block {
+ local http = require "resty.http"
+ local login_keycloak = require("lib.keycloak").login_keycloak
+ local concatenate_cookies =
require("lib.keycloak").concatenate_cookies
+
+ local httpc = http.new()
+
+ local uri = "http://127.0.0.1:"; .. ngx.var.server_port .. "/uri"
+ local res, err = login_keycloak(uri, "[email protected]", "123456")
+ if err then
+ ngx.status = 500
+ ngx.say(err)
+ return
+ end
+
+ local cookie_str = concatenate_cookies(res.headers['Set-Cookie'])
+ -- The session cookie must use the configured name, not the
default "session".
+ if not cookie_str:find("custom_session=", 1, true) then
+ ngx.status = 500
+ ngx.say("expected custom_session cookie, got: " .. cookie_str)
+ return
+ end
+
+ -- The renamed cookie must be a working session: the protected URI
returns 200.
+ local redirect_uri = "http://127.0.0.1:"; .. ngx.var.server_port ..
res.headers['Location']
+ res, err = httpc:request_uri(redirect_uri, {
+ method = "GET",
+ headers = {
+ ["Cookie"] = cookie_str
+ }
+ })
+ if not res then
+ ngx.status = 500
+ ngx.say(err)
+ return
+ elseif res.status ~= 200 then
+ ngx.status = 500
+ ngx.say("authenticated request with renamed cookie failed: "
.. res.status)
+ return
+ end
+
+ ngx.say("passed")
+ }
+ }
+--- response_body
+passed
+
+
+
+=== TEST 53: Configure plugin with a short session.absolute_timeout.
+--- 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": {
+ "discovery":
"http://127.0.0.1:8080/realms/University/.well-known/openid-configuration";,
+ "realm": "University",
+ "client_id": "course_management",
+ "client_secret":
"d1ec69e9-55d2-4109-a3ea-befa071579d5",
+ "redirect_uri": "http://127.0.0.1:]] ..
ngx.var.server_port .. [[/authenticated",
+ "ssl_verify": false,
+ "timeout": 10,
+ "session": {
+ "secret":
"jwcE5v3pM9VhqLxmxFOH9uZaLo8u7KQK",
+ "absolute_timeout": 2
+ }
+ }
+ },
+ "upstream": {
+ "nodes": {
+ "127.0.0.1:1980": 1
+ },
+ "type": "roundrobin"
+ },
+ "uri": "/*"
+ }]]
+ )
+
+ if code >= 300 then
+ ngx.status = code
+ end
+ ngx.say(body)
+ }
+ }
+--- response_body
+passed
+
+
+
+=== TEST 54: Session is rejected once absolute_timeout elapses, re-initiating
authentication.
+--- config
+ location /t {
+ content_by_lua_block {
+ local http = require "resty.http"
+ local login_keycloak = require("lib.keycloak").login_keycloak
+ local concatenate_cookies =
require("lib.keycloak").concatenate_cookies
+
+ local httpc = http.new()
+
+ local uri = "http://127.0.0.1:"; .. ngx.var.server_port .. "/uri"
+ local res, err = login_keycloak(uri, "[email protected]", "123456")
+ if err then
+ ngx.status = 500
+ ngx.say(err)
+ return
+ end
+ local cookie_str = concatenate_cookies(res.headers['Set-Cookie'])
+
+ -- Right after login the session is valid.
+ local redirect_uri = "http://127.0.0.1:"; .. ngx.var.server_port ..
res.headers['Location']
+ local res1 = httpc:request_uri(redirect_uri, {
+ method = "GET",
+ headers = { ["Cookie"] = cookie_str }
+ })
+ if not res1 or res1.status ~= 200 then
+ ngx.status = 500
+ ngx.say("session should be valid right after login, got: "
+ .. (res1 and res1.status or "nil"))
+ return
+ end
+
+ -- Once absolute_timeout (2s) passes, the session is no longer
accepted
+ -- and the request is redirected back to the ID provider for
re-authentication.
+ ngx.sleep(3)
+ local res2 = httpc:request_uri(uri, {
Review Comment:
This wait uses hard-coded timing based on `absolute_timeout` and should be
kept comfortably above the configured timeout to avoid boundary-related
flakiness. If you increase `absolute_timeout` (recommended), this sleep/comment
should be updated accordingly.
##########
t/plugin/openid-connect.t:
##########
@@ -1842,3 +1842,200 @@ done
--- response_body
property "client_secret" is required
done
+
+
+
+=== TEST 51: Configure plugin with a custom session.cookie_name.
+--- 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": {
+ "discovery":
"http://127.0.0.1:8080/realms/University/.well-known/openid-configuration";,
+ "realm": "University",
+ "client_id": "course_management",
+ "client_secret":
"d1ec69e9-55d2-4109-a3ea-befa071579d5",
+ "redirect_uri": "http://127.0.0.1:]] ..
ngx.var.server_port .. [[/authenticated",
+ "ssl_verify": false,
+ "timeout": 10,
+ "session": {
+ "secret":
"jwcE5v3pM9VhqLxmxFOH9uZaLo8u7KQK",
+ "cookie_name": "custom_session"
+ }
+ }
+ },
+ "upstream": {
+ "nodes": {
+ "127.0.0.1:1980": 1
+ },
+ "type": "roundrobin"
+ },
+ "uri": "/*"
+ }]]
+ )
+
+ if code >= 300 then
+ ngx.status = code
+ end
+ ngx.say(body)
+ }
+ }
+--- response_body
+passed
+
+
+
+=== TEST 52: Full OIDC login issues the session cookie under the configured
cookie_name.
+--- config
+ location /t {
+ content_by_lua_block {
+ local http = require "resty.http"
+ local login_keycloak = require("lib.keycloak").login_keycloak
+ local concatenate_cookies =
require("lib.keycloak").concatenate_cookies
+
+ local httpc = http.new()
+
+ local uri = "http://127.0.0.1:"; .. ngx.var.server_port .. "/uri"
+ local res, err = login_keycloak(uri, "[email protected]", "123456")
+ if err then
+ ngx.status = 500
+ ngx.say(err)
+ return
+ end
+
+ local cookie_str = concatenate_cookies(res.headers['Set-Cookie'])
+ -- The session cookie must use the configured name, not the
default "session".
+ if not cookie_str:find("custom_session=", 1, true) then
+ ngx.status = 500
+ ngx.say("expected custom_session cookie, got: " .. cookie_str)
+ return
+ end
+
+ -- The renamed cookie must be a working session: the protected URI
returns 200.
+ local redirect_uri = "http://127.0.0.1:"; .. ngx.var.server_port ..
res.headers['Location']
+ res, err = httpc:request_uri(redirect_uri, {
+ method = "GET",
+ headers = {
+ ["Cookie"] = cookie_str
+ }
+ })
+ if not res then
+ ngx.status = 500
+ ngx.say(err)
+ return
+ elseif res.status ~= 200 then
+ ngx.status = 500
+ ngx.say("authenticated request with renamed cookie failed: "
.. res.status)
+ return
+ end
+
+ ngx.say("passed")
+ }
+ }
+--- response_body
+passed
+
+
+
+=== TEST 53: Configure plugin with a short session.absolute_timeout.
+--- 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": {
+ "discovery":
"http://127.0.0.1:8080/realms/University/.well-known/openid-configuration";,
+ "realm": "University",
+ "client_id": "course_management",
+ "client_secret":
"d1ec69e9-55d2-4109-a3ea-befa071579d5",
+ "redirect_uri": "http://127.0.0.1:]] ..
ngx.var.server_port .. [[/authenticated",
+ "ssl_verify": false,
+ "timeout": 10,
+ "session": {
+ "secret":
"jwcE5v3pM9VhqLxmxFOH9uZaLo8u7KQK",
+ "absolute_timeout": 2
+ }
Review Comment:
`absolute_timeout` is set to 2s, which is likely too short for the
multi-request Keycloak login flow in CI and can make the test flaky (the
pre-auth session/state may expire before the callback completes). Consider
using a slightly larger timeout so the login portion is reliable, while still
validating expiry behavior in the next test.
##########
t/plugin/openid-connect.t:
##########
@@ -1842,3 +1842,200 @@ done
--- response_body
property "client_secret" is required
done
+
+
+
+=== TEST 51: Configure plugin with a custom session.cookie_name.
+--- 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": {
+ "discovery":
"http://127.0.0.1:8080/realms/University/.well-known/openid-configuration";,
+ "realm": "University",
+ "client_id": "course_management",
+ "client_secret":
"d1ec69e9-55d2-4109-a3ea-befa071579d5",
+ "redirect_uri": "http://127.0.0.1:]] ..
ngx.var.server_port .. [[/authenticated",
+ "ssl_verify": false,
+ "timeout": 10,
+ "session": {
+ "secret":
"jwcE5v3pM9VhqLxmxFOH9uZaLo8u7KQK",
+ "cookie_name": "custom_session"
+ }
+ }
+ },
+ "upstream": {
+ "nodes": {
+ "127.0.0.1:1980": 1
+ },
+ "type": "roundrobin"
+ },
+ "uri": "/*"
+ }]]
+ )
+
+ if code >= 300 then
+ ngx.status = code
+ end
+ ngx.say(body)
+ }
+ }
+--- response_body
+passed
+
+
+
+=== TEST 52: Full OIDC login issues the session cookie under the configured
cookie_name.
+--- config
+ location /t {
+ content_by_lua_block {
+ local http = require "resty.http"
+ local login_keycloak = require("lib.keycloak").login_keycloak
+ local concatenate_cookies =
require("lib.keycloak").concatenate_cookies
+
+ local httpc = http.new()
+
+ local uri = "http://127.0.0.1:"; .. ngx.var.server_port .. "/uri"
+ local res, err = login_keycloak(uri, "[email protected]", "123456")
+ if err then
+ ngx.status = 500
+ ngx.say(err)
+ return
+ end
+
+ local cookie_str = concatenate_cookies(res.headers['Set-Cookie'])
+ -- The session cookie must use the configured name, not the
default "session".
+ if not cookie_str:find("custom_session=", 1, true) then
+ ngx.status = 500
+ ngx.say("expected custom_session cookie, got: " .. cookie_str)
+ return
+ end
+
+ -- The renamed cookie must be a working session: the protected URI
returns 200.
+ local redirect_uri = "http://127.0.0.1:"; .. ngx.var.server_port ..
res.headers['Location']
+ res, err = httpc:request_uri(redirect_uri, {
+ method = "GET",
+ headers = {
+ ["Cookie"] = cookie_str
+ }
+ })
+ if not res then
+ ngx.status = 500
+ ngx.say(err)
+ return
+ elseif res.status ~= 200 then
+ ngx.status = 500
+ ngx.say("authenticated request with renamed cookie failed: "
.. res.status)
+ return
+ end
+
+ ngx.say("passed")
+ }
+ }
+--- response_body
+passed
+
+
+
+=== TEST 53: Configure plugin with a short session.absolute_timeout.
+--- 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": {
+ "discovery":
"http://127.0.0.1:8080/realms/University/.well-known/openid-configuration";,
+ "realm": "University",
+ "client_id": "course_management",
+ "client_secret":
"d1ec69e9-55d2-4109-a3ea-befa071579d5",
+ "redirect_uri": "http://127.0.0.1:]] ..
ngx.var.server_port .. [[/authenticated",
+ "ssl_verify": false,
+ "timeout": 10,
+ "session": {
+ "secret":
"jwcE5v3pM9VhqLxmxFOH9uZaLo8u7KQK",
+ "absolute_timeout": 2
+ }
+ }
+ },
+ "upstream": {
+ "nodes": {
+ "127.0.0.1:1980": 1
+ },
+ "type": "roundrobin"
+ },
+ "uri": "/*"
+ }]]
+ )
+
+ if code >= 300 then
+ ngx.status = code
+ end
+ ngx.say(body)
+ }
+ }
+--- response_body
+passed
+
+
+
+=== TEST 54: Session is rejected once absolute_timeout elapses, re-initiating
authentication.
+--- config
+ location /t {
+ content_by_lua_block {
+ local http = require "resty.http"
+ local login_keycloak = require("lib.keycloak").login_keycloak
+ local concatenate_cookies =
require("lib.keycloak").concatenate_cookies
+
+ local httpc = http.new()
+
+ local uri = "http://127.0.0.1:"; .. ngx.var.server_port .. "/uri"
+ local res, err = login_keycloak(uri, "[email protected]", "123456")
+ if err then
+ ngx.status = 500
+ ngx.say(err)
+ return
+ end
+ local cookie_str = concatenate_cookies(res.headers['Set-Cookie'])
+
+ -- Right after login the session is valid.
+ local redirect_uri = "http://127.0.0.1:"; .. ngx.var.server_port ..
res.headers['Location']
+ local res1 = httpc:request_uri(redirect_uri, {
+ method = "GET",
+ headers = { ["Cookie"] = cookie_str }
+ })
+ if not res1 or res1.status ~= 200 then
+ ngx.status = 500
+ ngx.say("session should be valid right after login, got: "
+ .. (res1 and res1.status or "nil"))
+ return
+ end
+
+ -- Once absolute_timeout (2s) passes, the session is no longer
accepted
+ -- and the request is redirected back to the ID provider for
re-authentication.
+ ngx.sleep(3)
+ local res2 = httpc:request_uri(uri, {
+ method = "GET",
+ headers = { ["Cookie"] = cookie_str }
+ })
+ if not res2 then
+ ngx.status = 500
+ ngx.say("no response after timeout")
+ return
+ elseif res2.status ~= 302 then
+ ngx.status = 500
+ ngx.say("expired session should trigger re-auth (302), got: "
.. res2.status)
+ return
+ end
Review Comment:
The test asserts a 302 after expiry but doesn't verify that the redirect is
actually to the IdP authorization endpoint. Adding a `Location` assertion makes
the test more specific to the intended behavior (re-initiating OIDC
authentication) and reduces the chance of a false positive from some other
redirect.
--
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]
