From d284d715062bdce33d0482a9adbb04396b92cd91 Mon Sep 17 00:00:00 2001
From: Stefan Bodewig <stefan.bodewig@innoq.com>
Date: Sun, 25 Aug 2024 13:25:54 +0200
Subject: [PATCH 1/2] restrict zero-pixel image logout to actual image requests

should fix #521
---
 ChangeLog                  |  4 ++++
 lib/resty/openidc.lua      | 37 ++++++++++++++++++++++++++++++++++---
 tests/spec/logout_spec.lua | 22 +++++++++++++++++++++-
 3 files changed, 59 insertions(+), 4 deletions(-)

diff --git a/ChangeLog b/ChangeLog
index ef5f63f..1daa727 100644
--- a/ChangeLog
+++ b/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 --git a/lib/resty/openidc.lua b/lib/resty/openidc.lua
index e7b8872..011f892 100644
--- a/lib/resty/openidc.lua
+++ b/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") 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 --git a/tests/spec/logout_spec.lua b/tests/spec/logout_spec.lua
index 7a0fd7d..37cfb6c 100644
--- a/tests/spec/logout_spec.lua
+++ b/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"])

From ab9c3860c0c9bf3234405c6b79f1c9e18e5edc7d Mon Sep 17 00:00:00 2001
From: Stefan Bodewig <stefan.bodewig@innoq.com>
Date: Sun, 25 Aug 2024 15:35:35 +0200
Subject: [PATCH 2/2] XHTML should also win over PNG

---
 lib/resty/openidc.lua | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lib/resty/openidc.lua b/lib/resty/openidc.lua
index 011f892..44310ff 100644
--- a/lib/resty/openidc.lua
+++ b/lib/resty/openidc.lua
@@ -1302,7 +1302,7 @@ local function request_prefers_png_over_html()
   end)
 
   for _, a in ipairs(accepted) do
-    if a.media_range:find("text/html") then
+    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