From a8e7840b37d02b5c0fa71f1b0b3139be83d7ab29 Mon Sep 17 00:00:00 2001 From: AlinsRan Date: Thu, 9 Jul 2026 16:42:45 +0800 Subject: [PATCH 1/4] chore(deps): use api7-lua-resty-session to fix empty arrays in OIDC userinfo The shared cjson instance in lua-resty-session decodes JSON without the array metatable, so an empty array loses its array identity across the session save/load cycle and is re-encoded as an object. With the openid-connect plugin this turns empty userinfo claims such as `roles` into `{}`, and `X-Userinfo` carries `"roles":{}` instead of `"roles":[]`. Switch to api7-lua-resty-session 4.1.6-0, which enables decode_array_with_array_mt on that instance. lua-resty-openidc and lua-resty-saml still depend on the upstream lua-resty-session, so it is still pulled in transitively, but api7-lua-resty-session sorts before it and therefore owns the resty.session.* module paths -- the same arrangement already used for api7-lua-resty-http and api7-lua-resty-jwt. Fixes #13440 --- apisix-master-0.rockspec | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/apisix-master-0.rockspec b/apisix-master-0.rockspec index 69ca2a12d3cd..71b876a73d7f 100644 --- a/apisix-master-0.rockspec +++ b/apisix-master-0.rockspec @@ -46,7 +46,7 @@ dependencies = { "api7-lua-resty-jwt = 0.2.6-0", "lua-resty-hmac-ffi = 0.06-1", "lua-resty-cookie = 0.4.1-1", - "lua-resty-session = 4.1.5-1", + "api7-lua-resty-session = 4.1.6-0", "lua-resty-openapi-validator = 1.0.5-1", "opentracing-openresty = 0.1-0", "lua-resty-radixtree = 2.9.2-0", From 038a74ff0877992fa777d1903c6cbc5d1dc8afde Mon Sep 17 00:00:00 2001 From: AlinsRan Date: Thu, 9 Jul 2026 16:46:44 +0800 Subject: [PATCH 2/4] test(openid-connect): guard empty userinfo claims against session round-trip Reproduces the #13440 chain without an IdP: core.json.decode (as openidc parses userinfo) -> session encode/decode -> core.json.encode (as the plugin builds X-Userinfo). With the old lua-resty-session the last step yields `{}`; the test therefore fails if the dependency is reverted. --- t/plugin/openid-connect-userinfo-array.t | 62 ++++++++++++++++++++++++ 1 file changed, 62 insertions(+) create mode 100644 t/plugin/openid-connect-userinfo-array.t diff --git a/t/plugin/openid-connect-userinfo-array.t b/t/plugin/openid-connect-userinfo-array.t new file mode 100644 index 000000000000..6a6862885415 --- /dev/null +++ b/t/plugin/openid-connect-userinfo-array.t @@ -0,0 +1,62 @@ +# +# Licensed to the Apache Software Foundation (ASF) under one or more +# contributor license agreements. See the NOTICE file distributed with +# this work for additional information regarding copyright ownership. +# The ASF licenses this file to You under the Apache License, Version 2.0 +# (the "License"); you may not use this file except in compliance with +# the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. +# +use t::APISIX 'no_plan'; + +repeat_each(1); +no_long_string(); +no_root_location(); + +add_block_preprocessor(sub { + my ($block) = @_; + + if ((!defined $block->error_log) && (!defined $block->no_error_log)) { + $block->set_value("no_error_log", "[error]"); + } + + if (!defined $block->request) { + $block->set_value("request", "GET /t"); + } +}); + +run_tests(); + +__DATA__ + +=== TEST 1: empty userinfo claims survive the session round-trip as arrays +--- config + location /t { + content_by_lua_block { + local core = require("apisix.core") + local session_utils = require("resty.session.utils") + + -- openid-connect stores the userinfo table in the session and reads it + -- back on later requests before encoding it into X-Userinfo. An empty + -- claim such as "roles" must survive that round-trip as an array; if the + -- session library decodes it without the array metatable, X-Userinfo + -- ends up carrying `"roles":{}` instead of `"roles":[]`. + local userinfo = core.json.decode('{"sub":"a UID","name":"Testuser One","roles":[]}') + + local stored = session_utils.encode_json(userinfo) + local restored = session_utils.decode_json(stored) + + ngx.say(stored:find('"roles":%[%]', 1) and "stored as array" or "stored as object") + ngx.say(core.json.encode(restored.roles)) + } + } +--- response_body +stored as array +[] From de6fb22aa8631727df5155c457e9d20c9fe8971a Mon Sep 17 00:00:00 2001 From: AlinsRan Date: Thu, 9 Jul 2026 17:08:18 +0800 Subject: [PATCH 3/4] test(openid-connect): drive X-Userinfo through the full request lifecycle Replace the direct session_utils check with an end-to-end test: a request is routed through the openid-connect plugin to an upstream that echoes the headers it received, and the test asserts the empty `roles` claim arrives in X-Userinfo as `[]`. The session is forged with the plugin's own secret so the plugin takes the already-authenticated path and restores the userinfo from the session -- the same path every request takes after the callback, and the one where the empty array used to be lost. A non-expired access token plus renew_access_token_on_expiry=false keep openidc from reaching for the discovery document, so no IdP is needed. Against the old lua-resty-session the upstream receives `"roles":{}`. --- t/plugin/openid-connect-userinfo-array.t | 104 ++++++++++++++++++++--- 1 file changed, 91 insertions(+), 13 deletions(-) diff --git a/t/plugin/openid-connect-userinfo-array.t b/t/plugin/openid-connect-userinfo-array.t index 6a6862885415..ce91e1d80e1a 100644 --- a/t/plugin/openid-connect-userinfo-array.t +++ b/t/plugin/openid-connect-userinfo-array.t @@ -19,6 +19,7 @@ use t::APISIX 'no_plan'; repeat_each(1); no_long_string(); no_root_location(); +no_shuffle(); add_block_preprocessor(sub { my ($block) = @_; @@ -36,27 +37,104 @@ run_tests(); __DATA__ -=== TEST 1: empty userinfo claims survive the session round-trip as arrays +=== TEST 1: set up a route guarded by openid-connect with set_userinfo_header --- 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", + "redirect_uri": "http://127.0.0.1:1984/callback", + "ssl_verify": false, + "use_pkce": false, + "set_userinfo_header": true, + "renew_access_token_on_expiry": false, + "session": { + "secret": "jwcE5v3pM9VhqLxmxFOH9uZaLo8u7KQK" + } + } + }, + "upstream": { + "nodes": { + "127.0.0.1:1980": 1 + }, + "type": "roundrobin" + }, + "uri": "/uri" + }]] + ) + + if code >= 300 then + ngx.status = code + end + ngx.say(body) + } + } +--- response_body +passed + + + +=== TEST 2: an empty userinfo claim reaches the upstream as an array +--- config + location /t { + content_by_lua_block { + local http = require("resty.http") + local session = require("resty.session") local core = require("apisix.core") - local session_utils = require("resty.session.utils") - -- openid-connect stores the userinfo table in the session and reads it - -- back on later requests before encoding it into X-Userinfo. An empty - -- claim such as "roles" must survive that round-trip as an array; if the - -- session library decodes it without the array metatable, X-Userinfo - -- ends up carrying `"roles":{}` instead of `"roles":[]`. - local userinfo = core.json.decode('{"sub":"a UID","name":"Testuser One","roles":[]}') + -- Forge the session openid-connect writes after a successful login, + -- so the plugin takes the "already authenticated" path and restores + -- the userinfo from the session, exactly as it does on every request + -- after the callback. No IdP is contacted: with a non-expired access + -- token and renew_access_token_on_expiry disabled, openidc never + -- reaches for the discovery document. + local s = session.new({ secret = "jwcE5v3pM9VhqLxmxFOH9uZaLo8u7KQK" }) + s:set("authenticated", true) + s:set("last_authenticated", ngx.time()) + s:set("id_token", { sub = "a UID" }) + s:set("access_token", "fake-access-token") + s:set("access_token_expiration", ngx.time() + 3600) + s:set("user", core.json.decode( + '{"sub":"a UID","name":"Testuser One","roles":[]}')) + + local ok, err = s:save() + if not ok then + ngx.say("failed to save session: ", err) + return + end + + local cookie = ngx.header["Set-Cookie"] + if type(cookie) == "table" then + cookie = cookie[1] + end + ngx.header["Set-Cookie"] = nil + + local httpc = http.new() + local res, err = httpc:request_uri("http://127.0.0.1:1984/uri", { + headers = { Cookie = cookie }, + }) + if not res then + ngx.say("request failed: ", err) + return + end - local stored = session_utils.encode_json(userinfo) - local restored = session_utils.decode_json(stored) + -- the upstream echoes back the request headers it received + local encoded = res.body:match("x%-userinfo: ([%w+/=]+)") + if not encoded then + ngx.say("no X-Userinfo reached the upstream") + return + end - ngx.say(stored:find('"roles":%[%]', 1) and "stored as array" or "stored as object") - ngx.say(core.json.encode(restored.roles)) + local userinfo = ngx.decode_base64(encoded) + ngx.say(core.json.encode(core.json.decode(userinfo).roles)) } } --- response_body -stored as array [] From e345c6b38fb02e0f283a46d8cb2f86c00ae70101 Mon Sep 17 00:00:00 2001 From: AlinsRan Date: Mon, 13 Jul 2026 15:33:49 +0800 Subject: [PATCH 4/4] fix(patch): make cjson instances inherit APISIX's cjson options lua-cjson options are per-instance and cjson.new() starts from the compile-time defaults, so enabling decode_array_with_array_mt on the cjson.safe singleton in core/json.lua never reaches a dependency that keeps a private instance. Those dependencies decode arrays without the array metatable, and an empty array is re-encoded as an object. Enable the options on both cjson singletons and wrap cjson.new / cjson.safe.new so instances created afterwards inherit them. This replaces the api7-lua-resty-session dependency swap: the fix now covers every dependency with a private instance (lua-resty-healthcheck, lua-resty-worker-events, lua-resty-aws, ...), not just lua-resty-session. encode_escape_forward_slash is set as well. It looks redundant today because the bundled lua-cjson mutates a shared escape table, so setting it anywhere already applies process-wide -- but upstream has made it per-instance, and once a runtime upgrade picks that up every private instance would silently go back to escaping '/'. Setting it here pins the current behaviour instead of relying on that implementation detail. Fixes #13440 --- apisix-master-0.rockspec | 2 +- apisix/core/json.lua | 2 -- apisix/patch.lua | 33 +++++++++++++++++++++++++++ t/core/json.t | 48 ++++++++++++++++++++++++++++++++++++++++ 4 files changed, 82 insertions(+), 3 deletions(-) diff --git a/apisix-master-0.rockspec b/apisix-master-0.rockspec index 71b876a73d7f..69ca2a12d3cd 100644 --- a/apisix-master-0.rockspec +++ b/apisix-master-0.rockspec @@ -46,7 +46,7 @@ dependencies = { "api7-lua-resty-jwt = 0.2.6-0", "lua-resty-hmac-ffi = 0.06-1", "lua-resty-cookie = 0.4.1-1", - "api7-lua-resty-session = 4.1.6-0", + "lua-resty-session = 4.1.5-1", "lua-resty-openapi-validator = 1.0.5-1", "opentracing-openresty = 0.1-0", "lua-resty-radixtree = 2.9.2-0", diff --git a/apisix/core/json.lua b/apisix/core/json.lua index 9418917b355e..a7ce18ae38e5 100644 --- a/apisix/core/json.lua +++ b/apisix/core/json.lua @@ -38,8 +38,6 @@ local rapidjson_null local rapidjson_encode_opts = { sort_keys = true } -cjson.encode_escape_forward_slash(false) -cjson.decode_array_with_array_mt(true) local _M = { version = 0.1, array_mt = cjson.array_mt, diff --git a/apisix/patch.lua b/apisix/patch.lua index f8d504b74a49..31383b770128 100644 --- a/apisix/patch.lua +++ b/apisix/patch.lua @@ -364,7 +364,40 @@ local function luasocket_tcp() end +-- lua-cjson options are per-instance, and cjson.new() always starts from the +-- compile-time defaults instead of inheriting the singleton's configuration. +-- core/json.lua only enables decode_array_with_array_mt on the `cjson.safe` +-- singleton, so every dependency holding a private instance (lua-resty-session, +-- lua-resty-healthcheck, lua-resty-worker-events, lua-resty-aws, ...) decodes +-- arrays without the array metatable, and an empty array is then re-encoded as +-- an object. Enable the option on both singletons and let every instance created +-- afterwards inherit it. This only changes the default: a library explicitly +-- calling decode_array_with_array_mt(false) on its own instance still wins. +-- +-- encode_escape_forward_slash is set here as well. It looks redundant today, +-- because in the bundled lua-cjson that setter mutates a shared escape table and +-- therefore already applies process-wide, including to instances created before +-- it was called. Upstream has since made it per-instance, so once a runtime +-- upgrade picks that up, every private instance would silently go back to +-- escaping `/`. Setting it here pins the current behaviour instead of relying on +-- that implementation detail. +local function patch_cjson(cjson) + cjson.decode_array_with_array_mt(true) + cjson.encode_escape_forward_slash(false) + + local original_new = cjson.new + cjson.new = function (...) + local instance = original_new(...) + patch_cjson(instance) + return instance + end +end + + function _M.patch() + patch_cjson(require("cjson")) + patch_cjson(require("cjson.safe")) + -- make linter happy -- luacheck: ignore ngx_socket.tcp = function () diff --git a/t/core/json.t b/t/core/json.t index 1409f54f1e7b..68fc3675cfec 100644 --- a/t/core/json.t +++ b/t/core/json.t @@ -193,3 +193,51 @@ b.d: 1 e: text a or default: default top-level null: nil + + + +=== TEST 9: cjson instances created by dependencies keep empty arrays as arrays +--- config + location /t { + content_by_lua_block { + -- a dependency that keeps a private instance, e.g. lua-resty-session + local safe_instance = require("cjson.safe").new() + ngx.say("cjson.safe instance: ", + safe_instance.encode(safe_instance.decode('{"arr":[]}'))) + + local instance = require("cjson").new() + ngx.say("cjson instance: ", instance.encode(instance.decode('{"arr":[]}'))) + + -- the plain singleton is a separate config from cjson.safe + local cjson = require("cjson") + ngx.say("cjson singleton: ", cjson.encode(cjson.decode('{"arr":[]}'))) + } + } +--- response_body +cjson.safe instance: {"arr":[]} +cjson instance: {"arr":[]} +cjson singleton: {"arr":[]} + + + +=== TEST 10: cjson instances created by dependencies do not escape forward slashes +--- config + location /t { + content_by_lua_block { + -- today the bundled lua-cjson keeps a shared escape table, so this + -- already holds; the patch pins it for when the option becomes + -- per-instance upstream + local safe_instance = require("cjson.safe").new() + ngx.say("cjson.safe instance: ", safe_instance.encode({uri = "/a/b"})) + + local instance = require("cjson").new() + ngx.say("cjson instance: ", instance.encode({uri = "/a/b"})) + + local cjson = require("cjson") + ngx.say("cjson singleton: ", cjson.encode({uri = "/a/b"})) + } + } +--- response_body +cjson.safe instance: {"uri":"/a/b"} +cjson instance: {"uri":"/a/b"} +cjson singleton: {"uri":"/a/b"}