Merge pull request #525 from zmartzone/try-to-fix-ff-logout

restrict zero-pixel image logout to actual image requests

Author: bodewig

Diff for: ChangeLog

@@ -1,3 +1,7 @@
+08/25/2024
+- don't return a zero-pixel image in logout for Firefox 128 and later
+  see #521
+
 03/10/2023
 - when looking for a bearer token an exception occured if the
   Authorization header didn't contain any space character;

Diff for: lib/resty/openidc.lua

@@ -1280,6 +1280,38 @@ local openidc_transparent_pixel = "\137\080\078\071\013\010\026\010\000\000\000\
     "\002\007\001\002\154\028\049\113\000\000\000\000\073\069\078\068" ..
     "\174\066\096\130"
 
+local function request_prefers_png_over_html()
+  local headers = ngx.req.get_headers()
+  local header = get_first(headers['Accept'])
+  if not header then return false end
+
+  -- https://httpwg.org/specs/rfc9110.html#field.accept
+  local accepted = {}
+  local function append_accepted_type(media_range_and_quality)
+    local media_range, quality = media_range_and_quality:match("(.+)%s*;%s*q=%s*([^%s]+)")
+    if media_range and quality then
+      accepted[#accepted + 1] = {media_range=media_range, quality=tonumber(quality)}
+    else
+      accepted[#accepted + 1] = {media_range=media_range_and_quality, quality=1}
+    end
+  end
+  header:gsub("[^,]+", append_accepted_type)
+
+  table.sort(accepted, function(a1, a2)
+      return a1.quality > a2.quality
+  end)
+
+  for _, a in ipairs(accepted) do
+    if a.media_range:find("text/html") or a.media_range:find("application/xhtml%+xml") then
+      return false
+    end
+    if a.media_range:find("image/png") then
+      return true
+    end
+  end
+  return false
+end
+
 -- handle logout
 local function openidc_logout(opts, session)
   local session_token = session.data.enc_id_token
@@ -1308,9 +1340,8 @@ local function openidc_logout(opts, session)
     end
   end
 
-  local headers = ngx.req.get_headers()
-  local header = get_first(headers['Accept'])
-  if header and header:find("image/png") then
+  if request_prefers_png_over_html() then
+    -- support for Ping Federate's proprietary logout protocol
     ngx.header["Cache-Control"] = "no-cache, no-store"
     ngx.header["Pragma"] = "no-cache"
     ngx.header["P3P"] = "CAO PSA OUR"

Diff for: tests/spec/logout_spec.lua

@@ -22,6 +22,26 @@ describe("when the configured logout uri is invoked with a non-image request", f
   end)
 end)
 
+describe("when the configured logout uri is invoked with Firefox 128's default Accept", function()
+  test_support.start_server()
+  teardown(test_support.stop_server)
+  local _, _, cookie = test_support.login()
+  local _, status, headers = http.request({
+      url = "http://127.0.0.1/default/logout",
+      headers = { cookie = cookie, accept = "text/html,application/xhtml+xml,application/xml;q=0.9,image/avif,image/webp,image/png,image/svg+xml,*/*;q=0.8" },
+      redirect = false
+  })
+  it("the response contains a default HTML-page", function()
+    assert.are.equals(200, status)
+    assert.are.equals("text/html", headers["content-type"])
+    -- TODO should there be a Cache-Control header?
+  end)
+  it("the session cookie has been revoked", function()
+    assert.truthy(string.match(headers["set-cookie"],
+                               "session=; Expires=Thu, 01 Jan 1970 00:00:01 GMT.*"))
+  end)
+end)
+
 describe("when the configured logout uri is invoked with a png request", function()
   -- TODO should this really take precedence over a configured end_session_endpoint?
   test_support.start_server({
@@ -38,7 +58,7 @@ describe("when the configured logout uri is invoked with a png request", functio
       headers = { cookie = cookie, accept = "image/png" },
       redirect = false
   })
-  it("the response contains a default HTML-page", function()
+  it("the response contains a default PNG image", function()
     assert.are.equals(200, status)
     assert.are.equals("image/png", headers["content-type"])
     assert.are.equals("no-cache, no-store", headers["cache-control"])