Skip to content
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.

Sign up for GitHub

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

Jump to bottom

bug: openid-connect plugin - existing session randomly unavailable for introspection / token renewal #9306

Open
brentmjohnson opened this issue Apr 13, 2023 · 11 comments · May be fixed by #10737
Open

bug: openid-connect plugin - existing session randomly unavailable for introspection / token renewal #9306

brentmjohnson opened this issue Apr 13, 2023 · 11 comments · May be fixed by #10737
Labels
bug Something isn't working

Comments

@brentmjohnson
Copy link

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

response, err, _, session = openidc.authenticate(conf, nil, unauth_action, conf.session)

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:

-- main routine for OpenID Connect user authentication
function openidc.authenticate(opts, target_url, unauth_action, session_or_opts)

  if opts.redirect_uri_path then
    log(WARN, "using deprecated option `opts.redirect_uri_path`; switch to using an absolute URI and `opts.redirect_uri` instead")
  end

  local err

  local session
  if is_session(session_or_opts) then
    session = session_or_opts
  else
    local session_error
    session, session_error = r_session.start(session_or_opts)
    if session == nil then
      log(ERROR, "Error starting session: " .. session_error)
      return nil, session_error, target_url, session
    end
  end

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

  1. Run APISIX with server-side session_storage (may also be an issue for cookies). Sample config:
  httpSrv: |
    proxy_buffer_size 32k;
    proxy_buffers 8 32k;
    proxy_busy_buffers_size 32k;
    set $session_name "apisix_session";
    set $session_cookie_samesite Strict;
    large_client_header_buffers 4 16k;
    set $session_strategy regenerate;
    set $session_storage redis;
    set $session_secret XXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXX;
    set $session_redis_uselocking off;
    set $session_redis_cluster_name redis-cluster;
    set $session_redis_cluster_nodes '1 2 3 4 5 6';
  1. Configure a route protected by the openid-connect plugin:
- name: openid-connect
  enable: true
  config:
    client_id: clientid
    client_secret: XXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXX
    discovery: https://example.com/realms/example/.well-known/openid-configuration
    scope: openid profile email
    set_refresh_token_header: true
    bearer_only: false
    introspection_endpoint: https://example.com/realms/example/protocol/openid-connect/token/introspect
    introspection_endpoint_auth_method: client_secret_post
    logout_path: /logout
    post_logout_redirect_uri: https://example.com/
    redirect_uri: https://example.com/login
    use_pkce: true
  1. Continue reloading a page in a protected route and observe occasional and random redirects to the IDP for authentication

Environment

  • APISIX version (run apisix version): 3.2.0
  • Operating system (run 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 / Nginx version (run openresty -V or nginx -V):
nginx version: openresty/1.21.4.1
built by gcc 10.2.1 20210110 (Debian 10.2.1-6) 
built with OpenSSL 1.1.1s  1 Nov 2022
TLS SNI support enabled
configure arguments: --prefix=/usr/local/openresty/nginx --with-cc-opt='-O2 -DAPISIX_BASE_VER=1.21.4.1.7 -DNGX_GRPC_CLI_ENGINE_PATH=/usr/local/openresty/libgrpc_engine.so -DNGX_HTTP_GRPC_CLI_ENGINE_PATH=/usr/local/openresty/libgrpc_engine.so -DNGX_LUA_ABORT_AT_PANIC -I/usr/local/openresty/zlib/include -I/usr/local/openresty/pcre/include -I/usr/local/openresty/openssl111/include' --add-module=../ngx_devel_kit-0.3.1 --add-module=../echo-nginx-module-0.62 --add-module=../xss-nginx-module-0.06 --add-module=../ngx_coolkit-0.2 --add-module=../set-misc-nginx-module-0.33 --add-module=../form-input-nginx-module-0.12 --add-module=../encrypted-session-nginx-module-0.09 --add-module=../srcache-nginx-module-0.32 --add-module=../ngx_lua-0.10.21 --add-module=../ngx_lua_upstream-0.07 --add-module=../headers-more-nginx-module-0.33 --add-module=../array-var-nginx-module-0.05 --add-module=../memc-nginx-module-0.19 --add-module=../redis2-nginx-module-0.15 --add-module=../redis-nginx-module-0.3.9 --add-module=../ngx_stream_lua-0.0.11 --with-ld-opt='-Wl,-rpath,/usr/local/openresty/luajit/lib -Wl,-rpath,/usr/local/openresty/wasmtime-c-api/lib -L/usr/local/openresty/zlib/lib -L/usr/local/openresty/pcre/lib -L/usr/local/openresty/openssl111/lib -Wl,-rpath,/usr/local/openresty/zlib/lib:/usr/local/openresty/pcre/lib:/usr/local/openresty/openssl111/lib' --add-module=/tmp/tmp.XAafuZTCsa/openresty-1.21.4.1/../mod_dubbo-1.0.2 --add-module=/tmp/tmp.XAafuZTCsa/openresty-1.21.4.1/../ngx_multi_upstream_module-1.1.1 --add-module=/tmp/tmp.XAafuZTCsa/openresty-1.21.4.1/../apisix-nginx-module-1.12.0 --add-module=/tmp/tmp.XAafuZTCsa/openresty-1.21.4.1/../apisix-nginx-module-1.12.0/src/stream --add-module=/tmp/tmp.XAafuZTCsa/openresty-1.21.4.1/../apisix-nginx-module-1.12.0/src/meta --add-module=/tmp/tmp.XAafuZTCsa/openresty-1.21.4.1/../wasm-nginx-module-0.6.4 --add-module=/tmp/tmp.XAafuZTCsa/openresty-1.21.4.1/../lua-var-nginx-module-v0.5.3 --add-module=/tmp/tmp.XAafuZTCsa/openresty-1.21.4.1/../grpc-client-nginx-module-v0.4.2 --with-poll_module --with-pcre-jit --with-stream --with-stream_ssl_module --with-stream_ssl_preread_module --with-http_v2_module --without-mail_pop3_module --without-mail_imap_module --without-mail_smtp_module --with-http_stub_status_module --with-http_realip_module --with-http_addition_module --with-http_auth_request_module --with-http_secure_link_module --with-http_random_index_module --with-http_gzip_static_module --with-http_sub_module --with-http_dav_module --with-http_flv_module --with-http_mp4_module --with-http_gunzip_module --with-threads --with-compat --with-stream --with-http_ssl_module
  • etcd version, if relevant (run curl http://127.0.0.1:9090/v1/server_info):
etcd Version: 3.5.6
Git SHA: cecbe35ce
Go Version: go1.16.15
Go OS/Arch: linux/amd64
  • APISIX Dashboard version, if relevant:
  • Plugin runner version, for issues related to plugin runners:
  • LuaRocks version, for installation issues (run luarocks --version):
@shreemaan-abhishek
Copy link
Contributor

shreemaan-abhishek commented Apr 17, 2023 •
edited

Thanks for the detailed explanation. A question: Are all the tests passing in the patched version? And which provider are you using?

@brentmjohnson
Copy link
Author

I have not run any of the included tests - i'm assuming that would be these:
https://github.com/apache/apisix/blob/master/t/plugin/openid-connect.t
https://github.com/apache/apisix/blob/master/t/plugin/openid-connect2.t

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.

@shreemaan-abhishek
Copy link
Contributor

Just raise a pull request to your fork of the project the github actions CI will run the tests.

@james-mchugh
Copy link

I'm seeing similar behavior

@james-mchugh
Copy link

I wonder if it is related to zmartzone/lua-resty-openidc#334

@james-mchugh
Copy link

Actually, after reviewing the code a bit more and doing some experimenting, setting session.secret in the plugin config appeared to fix this for me.

@james-mchugh
Copy link

Related issue: zmartzone/lua-resty-openidc#278

@kayx23
Copy link
Member

kayx23 commented Dec 28, 2023

I'm seeing similar behavior
Actually, after reviewing the code a bit more and doing some experimenting, setting session.secret in the plugin config appeared to fix this for me.

@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.

@kayx23
Copy link
Member

kayx23 commented Dec 28, 2023

@brentmjohnson is this issue still current / have you opened a PR to this repo?

@shreemaan-abhishek shreemaan-abhishek added the bug Something isn't working label Dec 29, 2023
@brentmjohnson brentmjohnson linked a pull request Dec 29, 2023 that will close this issue
Open
5 tasks
@brentmjohnson
Copy link
Author

@kayx23 yes still current. I have been on a patched version of the plugin for a while with no issues.

PR: #10737

@pshettyk
Copy link

Try adding following under openid-connect extension session: secret:1234567891012121314

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
bug Something isn't working
Projects
Apache APISIX backlog
Status: 🏗 In progress
Milestone
No milestone
5 participants
@pshettyk @james-mchugh @kayx23 @shreemaan-abhishek @brentmjohnson