Migrate lua-resty-session to 4.0.3 [tested, works]

[oldium]

Complete rework of #478, all issues of #478 should be addressed. Uses session:set and session:get to manipulate the session variables.

Tested by unit tests and on real-world project.

[lordgreg]

Hi. Will this get merged? @bodewig

[ja-softdevel]

@bodewig Hello, is there an ETA on this merge? It seems this is causing issues downstream with kong oidc.

[oldium]

Adding a cute kitten usually helps speed things up 😅

[ja-softdevel]

@oldium Hello, I git cloned your branch and built it in a Dockerfile along with kong-oidc. I was hoping your changes would resolve the error I keep getting.

using Docker: Kong Gateway (OSS) service: https://cat-fact.herokuapp.com/facts route: /cats plugin: kong-oidc Keycloak for auth

The route is set to redirect to Keycloak for testing auth. But I get the following error in the Kong logs when Keycloak redirects back to the route.

[error] 1264#0: *2785 [lua] openidc.lua:1484: authenticate(): request to the redirect_uri path but there's no session state found, client: 172.25.17.1, server: kong, request: "GET /cats?state=41b59f8cd04f7404a603ba9908d18930&session_state=ba1bf1a8-cf0f-4563-8028-fcf3ddd9e0b1&code=16da05eb-78ea-4323-a742-bdbf2b8362c1.ba1bf1a8-cf0f-4563-8028-fcf3ddd9e0b1.9406568b-bc51-4841-a8ae-99a3fdb381c2 HTTP/1.1", host: "localhost"

What am I missing? The session_state object is clearly there.

Thanks.

[lordgreg]

@ja-softdevel - can you list which luarocks plugins you have installed?

usually, its lua-resty-session, which appears twice and causes the exact problem you are having.

[ja-softdevel]

@ja-softdevel - can you list which luarocks plugins you have installed?

usually, its lua-resty-session, which appears twice and causes the exact problem you are having.

FYI: I haven't had much experience with lua. C++, Python but not lua. So still figuring out how it does things.

Looks like lua-resty-session is appearing twice and lua-resty-openidc in the list:

$ luarocks list resty
Rocks installed for Lua 5.1
---------------------------
lua-resty-acme
   0.11.0-1 (installed) - /usr/local/lib/luarocks/rocks-5.1
lua-resty-counter
   0.2.1-1 (installed) - /usr/local/lib/luarocks/rocks-5.1
lua-resty-healthcheck
   1.6.2-1 (installed) - /usr/local/lib/luarocks/rocks-5.1
lua-resty-http
   0.17.1-0 (installed) - /usr/local/lib/luarocks/rocks-5.1
lua-resty-ipmatcher
   0.6.1-0 (installed) - /usr/local/lib/luarocks/rocks-5.1
lua-resty-jit-uuid
   0.0.7-2 (installed) - /usr/local/lib/luarocks/rocks-5.1
lua-resty-jwt
   0.2.3-0 (installed) - /usr/local/lib/luarocks/rocks-5.1
lua-resty-openidc
   1.7.6-3 (installed) - /usr/local/lib/luarocks/rocks-5.1
   1.6.0-1 (installed) - /usr/local/lib/luarocks/rocks-5.1
lua-resty-openssl
   0.8.23-1 (installed) - /usr/local/lib/luarocks/rocks-5.1
lua-resty-session
   4.0.4-1 (installed) - /usr/local/lib/luarocks/rocks-5.1
   3.10-1 (installed) - /usr/local/lib/luarocks/rocks-5.1
lua-resty-timer
   1.1.0-1 (installed) - /usr/local/lib/luarocks/rocks-5.1
lua-resty-timer-ng
   0.2.5-1 (installed) - /usr/local/lib/luarocks/rocks-5.1

In the Dockerfile, I'm installing 3 lua packages.:

git clone --branch resty-session-4.x https://github.com/oldium/lua-resty-openidc.git
git clone --branch v1.4.0-1 https://github.com/revomatico/kong-oidc.git
git clone --branch v1.1.0 https://github.com/dkoltsuk/kong-plugin-jwt-keycloak.git

Should I first uninstall lua-resty-openidc and lua-resty-session packages before doing the git clones? Thanks.

[lordgreg]

How did you install the plugins? You've added the git clone commands, but do you made any luarocks make *.spec?

Anyways,

luarocks uninstall lua-resty-session 4.0.4-1 --force will fix your issue. The 3.10.1 gets installed using kong-oidc > lua-resty-openidc > lua-resty-session dependency hell :)

Which is why we are talking merging this PR into the master. :)

[oldium]

Newest Kong GW images use lua-resty-session 4, so I think that would not work. I guess you have this for Kong GW, right?

[ja-softdevel]

Alrighty!

So I added this line to the Dockerfile just after my apt updates but before doing the git clones which removed 4.0.4-1.

RUN luarocks remove lua-resty-session --force

After I clone @oldium 's fork and set to the resty-session-4.x branch, that repo will cause lua-resty-session 4.0.5-1 to get installed.

Then starting docker-compose and letting decK complete, I added the kong-oidc plugin to the (oh so lovely) '/cats' route. Only items set on the plugin were: client id, client secret, discovery uri, and I changed the realm to match the one I set in keycloak.

I have deleted the image and rebuilt it. Then ran docker up/down to ensure this worked.

Thank you so much @lordgreg and @oldium for the assistance. Hopefully, @bodewig can get this merged and maybe add you two as maintainers to help prevent development stall out.

[Dark3clipse]

I've tested it and it seems to works for me too.

[ziadkh0]

I get the following error when trying to logout a user that happens to not be logged-in, which used to work:

lua entry thread aborted: runtime error: /.../lualib/resty/session.lua:2077: unable to destroy nonexistent or closed session
stack traceback:
coroutine 0:
	[C]: in function 'assert'
	/.../lualib/resty/session.lua:2077: in function 'destroy'
	/.../lualib/resty/openidc.lua:1298: in function 'openidc_logout'
	/.../lualib/resty/openidc.lua:1508: in function 'authenticate'

Replacing https://github.com/zmartzone/lua-resty-openidc/blob/734a3f4dba0faf037abe993c678e43b1bab3025a/lib/resty/openidc.lua#L1287 with the following fixes the issue:

  if next(session:get_data()) ~= nil then
    session:destroy()
  end

[oldium]

Thanks, I will check it

[giafar]

Hi. Will this get merged? @bodewig

[bodewig]

So people don't need to keep asking: I can not foresee when (or even if) I will find to properly review this or #478 myself.

It is my impression that lua-resty-session has also changed semantics so changing the code in way that Lua is happy will not be enough. Reports by people who have tried the patches in both PRs confirm this is not a simple "apply and all is well" fix. I do not have the cycles to understand what has changed between 3.x and 4.x over in lua-resty-session. lua-resty-openidc lacks any tests that would make use of sessions and personally I don't run any environment using lua-resty-openidc at all right now so wouldn't be able to properly test things myself.

[oldium]

I get the following error when trying to logout a user that happens to not be logged-in, which used to work:

lua entry thread aborted: runtime error: /.../lualib/resty/session.lua:2077: unable to destroy nonexistent or closed session
stack traceback:
coroutine 0:
	[C]: in function 'assert'
	/.../lualib/resty/session.lua:2077: in function 'destroy'
	/.../lualib/resty/openidc.lua:1298: in function 'openidc_logout'
	/.../lualib/resty/openidc.lua:1508: in function 'authenticate'

Replacing

https://github.com/zmartzone/lua-resty-openidc/blob/734a3f4dba0faf037abe993c678e43b1bab3025a/lib/resty/openidc.lua#L1287

with the following fixes the issue:

  if next(session:get_data()) ~= nil then
    session:destroy()
  end

I checked it and it seems this is a problem of lua-resty-session, so I reported a bug there. I also added the suggested workaround to lua-resty-openidc and a test case until this is fixed.

[oldium]

I have renamed the Pull Request to indicate this is more up-to-date than the second (unmaintained) one.

Anyway, enjoy 🚀😁

[oldium]

Just for info: We are using the code from this Pull Request in our project in Dockerfile in the following way:

ARG KONG_GW_FLAVOR="kong/kong-gateway"
ARG KONG_GW_VERSION="3.4.3.2-ubuntu"

FROM docker.io/${KONG_GW_FLAVOR}:${KONG_GW_VERSION}
...
# Download with https://codeload.github.com/zmartzone/lua-resty-openidc/legacy.tar.gz/pull/489/head
# See https://github.com/zmartzone/lua-resty-openidc/pull/489
ADD zmartzone-lua-resty-openidc-v1.7.6-21-g1553a39.tar.gz /install

RUN ls -l /install; cd /install/zmartzone-lua-resty-openidc-1553a39 \
    && find -name *.rockspec -exec luarocks make {} \;
...

[oldium]

It looks like the tests fail randomly - ~~may be because of timing of flushing the nginx logs to disk?~~ EDIT: This was caused by non-padded Base64 decoding

[oldium]

Fixed decoding of Base64Url in tests (old issue), so they should pass now.

[oldium]

I have to say that ChatGPT recommended the Base64 fix and it really worked...

[lordgreg]

I will also test this out tomorrow. Thank you @oldium ;)

[lukeyeager]

Tested working for me