bug: openid-connect: Logout not working using Firefox Version >= 128.0

[markusmueller]

Current Behavior

Using Firefox version >= 128.0 the /logout path provided by the openid-connect plugin is not redirecting to the configured IDP for logout but instead returning a 1x1 pixel image.

Root Cause: Firefox Version 128.0 changed the network.http.accept default value also adding image/png.

This is causing lua-resty-openidc used by the openid-connect plugin to return the image.

There is already a discussion on lua-resty-openidc.

Once the issue is resolved upstream the dependency should be updated.

A possible workaround in the meantime is patching the openid-connect plugin and overwriting the Accept header for the /logout path in openid-connect.lua:

ngx.req.set_header("Accept", "text/html")

Expected Behavior

Using Firefox v 128.0 the path /logout will redirect to the configured IDP for logout.

Error Logs

No response

Steps to Reproduce

Configure Apisix openid-connect plugin with logout and hit /logout using Firefox version >= 128.0.

Environment

• APISIX version (run apisix version):
• Operating system (run uname -a):
• OpenResty / Nginx version (run openresty -V or nginx -V):
• etcd version, if relevant (run curl http://127.0.0.1:9090/v1/server_info):
• APISIX Dashboard version, if relevant:
• Plugin runner version, for issues related to plugin runners:
• LuaRocks version, for installation issues (run luarocks --version):

[dosubot[bot]]

Hey @markusmueller! 👋 I'm here to help you with any bugs, questions, or contributions you have regarding our repository.

To address the issue with the /logout path not redirecting correctly in Firefox version 128.0 or higher, you can patch the APISIX openid-connect plugin to overwrite the Accept header. Here is how you can do it:

1. Modify the Plugin Code: Add a function to handle the /logout path and set the Accept header to text/html.

   local function handle_logout_request()
       local uri = ngx.var.uri
       if uri == "/logout" then
           ngx.req.set_header("Accept", "text/html")
       end
   end
   

2. Integrate the Function: Call this function in the main handler function of the plugin. The main handler function in the openid-connect plugin is the rewrite function.

   function _M.rewrite(plugin_conf, ctx)
       local conf = core.table.clone(plugin_conf)
   
       -- Previously, we multiply conf.timeout before storing it in etcd.
       -- If the timeout is too large, we should not multiply it again.
       if not (conf.timeout >= 1000 and conf.timeout % 1000 == 0) then
           conf.timeout = conf.timeout * 1000
       end
   
       local path = ctx.var.request_uri
   
       -- Call the function to handle the logout request
       handle_logout_request()
   
       -- Continue with the rest of the plugin logic
   end
   

3. Test the Patch: Ensure that the logout request is properly handled in the plugin's test cases. You can add a test case to verify that the Accept header is set correctly during the logout process.

   local http = require "resty.http"
   local httpc = http.new()
   local uri = "http://127.0.0.1:" .. ngx.var.server_port .. "/logout"
   local res, err = httpc:request_uri(uri, {method = "GET"})
   ngx.status = res.status
   local location = res.headers['Location']
   if location and string.find(location, 'https://iresty.com') ~= -1 and
       string.find(location, 'post_logout_redirect_uri=https://iresty.com') ~= -1 then
       ngx.say(true)
   end
   

This patch ensures that when the /logout path is accessed, the Accept header is set to text/html, which should resolve the redirection issue in Firefox version 128.0 or higher ^[1][2][3].

To continue talking to Dosu, mention @dosu.

[francescodedomenico]

@dosu I tried modifying the original openid-connect plugin, however I get the following error when trying: 2024-08-12 20:20:40 2024/08/12 18:20:40 [error] 40#40: *48955 [lua] config_yaml.lua:269: failed to fetch data from local file /usr/local/apisix/conf/apisix.yaml: /usr/local/apisix/apisix/plugins/openid-connect.lua:310: attempt to call field 'check_https' (a nil value), apisix.yaml key: plugin_configs, context: ngx.timer

the original plugin works correctly with the same configuration.

Update

I have personally solved following a different approach, I have written a patched openidc.lua that has the lines:

-- if header and header:find("image/png") then
  --   ngx.header["Cache-Control"] = "no-cache, no-store"
  --   ngx.header["Pragma"] = "no-cache"
  --   ngx.header["P3P"] = "CAO PSA OUR"
  --   ngx.header["Expires"] = "0"
  --   ngx.header["X-Frame-Options"] = "DENY"
  --   ngx.header.content_type = "image/png"
  --   ngx.print(openidc_transparent_pixel)
  --   ngx.exit(ngx.OK)
  --   return

commented (lines from 1314 to 1323) and I have replaced the original one shipped with apisix: /usr/local/apisix/deps/share/lua/5.1/resty/openidc.lua

Works perfectly for me

[francescodedomenico]

@markusmueller the issue is due to the decision of lua-resty-openidc to support a peculiar Single Logout implemented by PingFederate, so it's nothing apisix can do unless forking the library and removing this custom PF logout support. I did perform the investigation here if you're interested: https://github.com/zmartzone/lua-resty-openidc/issues/521#issuecomment-2284890175

[markusmueller]

Interesting to see where the PNG based logout originated, thanks for the update @francescodedomenico!

[remydewa]

What I did as quick win for the moment: Using the serverless-pre-function plugin and apply a part of the code mentioned here: https://github.com/apache/apisix/issues/11492#issuecomment-2275389245

"serverless-pre-function": {
  "functions": [
    "return function(conf, ctx)\n\n local ngx = ngx \n local uri = ngx.var.uri \n if uri == '/users/logout' then \n ngx.req.set_header('Accept', 'text/html') \n end \n end"
  ],
  "phase": "rewrite"
}

[amaino]

The bug is fixed in the lua-resty-openidc now. Was this code included in the last release? https://github.com/zmartzone/lua-resty-openidc/pull/525

[Baoyuantop]

PR is welcome.