-
Notifications
You must be signed in to change notification settings - Fork 2.5k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
bug: openid-connect plugin - existing session randomly unavailable for introspection / token renewal #9306
Comments
Thanks for the detailed explanation. A question: Are all the tests passing in the patched version? And which provider are you using? |
I have not run any of the included tests - i'm assuming that would be these: but can - would just take some research into the test environment requirements since I haven't run those before. I am using keycloak for the identify provider. |
Just raise a pull request to your fork of the project the github actions CI will run the tests. |
I'm seeing similar behavior |
I wonder if it is related to zmartzone/lua-resty-openidc#334 |
Actually, after reviewing the code a bit more and doing some experimenting, setting |
Related issue: zmartzone/lua-resty-openidc#278 |
@james-mchugh Are you using APISIX in standalone mode? Because in traditional / decoupled mode, secret should be randomly generated and saved to etcd if not explicitly configured. |
@brentmjohnson is this issue still current / have you opened a PR to this repo? |
Try adding following under openid-connect extension |
Current Behavior
The openid-connect plugin will randomly redirect requests with valid session cookie and non-expired tokens back through the authentication flow. No errors were generated as the redirect happens exactly the same way a request with missing / expired session cookie is handled.
Note about current configuration where this is observed: apisix / openid-connect plugin configured for server-side sessions in redis-cluster with regenerate session-strategy (but could be an issue with other configuration).
After a lot of troubleshooting potential configuration issues across apisix, nginx, and lua-resty-session config, it now appears there is a timing issue with the reference to conf.session in this invocation of openidc.authenticate:
apisix/apisix/plugins/openid-connect.lua
Line 350 in f39cadd
When the call is modified to:
response, err, _, session = openidc.authenticate(conf, nil, unauth_action, conf)
The behavior is resolved. Token renewal occurs silently (to user) and session cookies are updated appropriately with no random redirects to the authentication flow as if there is a missing / expired session cookie.
Sending the full conf / opts object rather than just the session is supported by lua-resty-openidc:
https://github.com/zmartzone/lua-resty-openidc/blob/734a3f4dba0faf037abe993c678e43b1bab3025a/lib/resty/openidc.lua#L1440-L1459
Currently running a patched version of the openid-connect plugin (with this change) without issue (for described configuration).
Expected Behavior
With a valid session cookie and non-expired tokens, a user should not be redirected to the authentication flow.
Error Logs
No response
Steps to Reproduce
Environment
apisix version
):3.2.0
uname -a
):Linux apisix-54f9cdf6cf-t6m66 5.15.0-69-generic #76-Ubuntu SMP Fri Mar 17 17:19:29 UTC 2023 x86_64 GNU/Linux
openresty -V
ornginx -V
):curl http://127.0.0.1:9090/v1/server_info
):luarocks --version
):The text was updated successfully, but these errors were encountered: