chore(deps): use api7-lua-resty-session to fix empty arrays in OIDC userinfo#13680
Open
AlinsRan wants to merge 3 commits into
Open
chore(deps): use api7-lua-resty-session to fix empty arrays in OIDC userinfo#13680AlinsRan wants to merge 3 commits into
AlinsRan wants to merge 3 commits into
Conversation
…serinfo
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 apache#13440
…nd-trip Reproduces the apache#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.
…ycle
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":{}`.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Fixes #13440
The shared cjson instance in `lua-resty-session` (`lib/resty/session/utils.lua`) decodes JSON without the array metatable. An empty array therefore loses its array identity across the session save/load cycle and is re-encoded as an object.
With `openid-connect`, userinfo is stored in the session and read back on subsequent requests, so an empty claim such as `roles` comes back as a plain empty table and `core.json.encode` renders it as an object:
Note the corruption is not in
openid-connector in openidc's own decode:core/json.luaenablesdecode_array_with_array_mt(true)on the sharedcjson.safesingleton, which openidc reuses, so the array survives into the session. It is lost on the way out, inlua-resty-session's owncjson.safe.new()instance, which does not have that option set.Fix
Switch the dependency to
api7-lua-resty-session = 4.1.6-0, which enablesdecode_array_with_array_mton that instance (upstream PR: bungle/lua-resty-session#207).lua-resty-openidc(>= 4.0.3) andlua-resty-saml(= 4.1.5-1) still declare the upstreamlua-resty-session, so it is still installed transitively. That is fine: LuaRocks gives the canonical module path to the lexicographically smaller rock name (luarocks/repos.lua,name < cur_name), soapi7-lua-resty-sessionownsresty.session.*and the upstream copy is relegated to a versioned directory that nothing requires. This is exactly the arrangement already used byapi7-lua-resty-httpandapi7-lua-resty-jwt, which coexist with the upstream rocks pulled in by openidc.Verification
Resolving the dependency set (
api7-lua-resty-session+lua-resty-openidc+lua-resty-saml) into a clean tree:Replaying the userinfo round-trip through that tree:
The result is independent of install order, since the winner is decided by rock name rather than by installation sequence.