Merge pull request #571 from Hockenba/fix/propagate-session-destroy-errors-on-logout

Propagate Lua-Resty-Session Revocation Store Failures

Author: kevinlzw

ChangeLog

@@ -1,4 +1,6 @@
 07/01/2026
+- propagate session destroy and closed-mode revocation store failures from
+  logout and session start instead of continuing as if logout succeeded
 - do not use the shared JWT verification cache for bearer JWT validation
   calls that pass additional claim validators, preventing a token accepted
   by one validator set from bypassing a stricter validator set on cache hit.

lib/resty/openidc.lua

@@ -292,6 +292,37 @@ local function is_session_present(session)
   return session ~= nil and next(session:get_data()) ~= nil
 end
 
+-- start a session; propagate closed-mode revocation store failures
+local function openidc_start_session(session_opts)
+  local session, err = r_session.start(session_opts)
+  if not session then
+    log(ERROR, "Error starting session: " .. (err or "unknown error"))
+    return nil, err
+  end
+
+  -- session uses either message when revocation_fail_mode is closed and the
+  -- revocation store is unavailable (on open or destroy)
+  if err and session.revocation_fail_mode == "closed"
+      and (err:find("unable to check session revocation", 1, true)
+        or err:find("unable to mark session revoked", 1, true)) then
+    log(ERROR, "Error starting session: " .. err)
+    return session, err
+  end
+
+  return session, nil
+end
+
+-- destroy an open session; propagate destroy() failures
+local function openidc_destroy_session(session)
+  if session and session.state == "open" and is_session_present(session) then
+    local ok, err = session:destroy()
+    if not ok then
+      log(ERROR, "failed to destroy session: " .. err)
+      return err
+    end
+  end
+end
+
 -- set value in server-wide cache if available
 local function openidc_cache_set(type, key, value, exp)
   local dict = ngx.shared[type]
@@ -1870,8 +1901,9 @@ local function openidc_logout(opts, session)
     end
   end
 
-  if is_session_present(session) then
-    session:destroy()
+  err = openidc_destroy_session(session)
+  if err then
+    return err
   end
 
   if opts.revoke_tokens_on_logout then
@@ -2047,11 +2079,9 @@ function openidc.authenticate(opts, target_url, unauth_action, session_or_opts)
   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
+    session, err = openidc_start_session(session_or_opts)
+    if err then
+      return nil, err, target_url, session
     end
   end
 
@@ -2084,7 +2114,10 @@ function openidc.authenticate(opts, target_url, unauth_action, session_or_opts)
       return nil, err, session:get("original_url"), session
     end
 
-    openidc_logout(opts, session)
+    err = openidc_logout(opts, session)
+    if err then
+      return nil, err, target_url, session
+    end
     return nil, nil, target_url, session
   end
 
@@ -2185,7 +2218,13 @@ end
 -- get a valid access_token (eventually refreshing the token), or nil if there's no valid access_token
 function openidc.access_token(opts, session_opts)
 
-  local session = r_session.start(session_opts)
+  local session, err = openidc_start_session(session_opts)
+  if err then
+    if session then
+      session:close()
+    end
+    return nil, err
+  end
   local token, err = openidc_access_token(opts, session, true)
   session:close()
   return token, err

lua-resty-openidc-1.9.0-1.rockspec

@@ -24,7 +24,7 @@ description = {
 dependencies = {
     "lua >= 5.1",
     "lua-resty-http >= 0.08",
-    "lua-resty-session >= 4.0.3",
+    "lua-resty-session == 4.2.0-1",
     "lua-resty-jwt >= 0.2.0",
     "lua-resty-openssl >= 1.8.0"
 }

tests/Dockerfile

@@ -1,13 +1,13 @@
 FROM openresty/openresty:focal
 
 # install dependencies
-RUN ["luarocks", "install", "lua-resty-session", "4.0.3"]
+RUN ["luarocks", "install", "lua-resty-session", "4.2.0-1"]
 RUN ["luarocks", "install", "lua-resty-http"]
 RUN ["luarocks", "install", "lua-resty-jwt"]
 
 # install test dependencies
 RUN apt-get update \
-    && apt-get install -y --no-install-recommends git \
+    && apt-get install -y --no-install-recommends ca-certificates git \
     && rm -rf /var/lib/apt/lists/*
 RUN ["luarocks", "install", "busted"]
 RUN ["luarocks", "install", "LuaSocket"]

tests/spec/access_token_access_spec.lua

@@ -39,6 +39,59 @@ describe("if there is an active non-expired login", function()
   end)
 end)
 
+describe("if revocation_fail_mode is closed and the revocation store read fails", function()
+  test_support.start_server({
+    revocation_test = {
+      fail_mode = "closed",
+      get_fails_after = 1,
+    },
+  })
+  teardown(test_support.stop_server)
+
+  local _, _, cookies = test_support.login()
+  local content_table = {}
+  local _, status = http.request({
+    url = "http://localhost/access_token",
+    redirect = false,
+    headers = { cookie = cookies },
+    sink = ltn12.sink.table(content_table)
+  })
+  local body = table.concat(content_table)
+
+  it("returns the session start failure", function()
+    assert.are.equals(401, status)
+    assert.truthy(string.find(body, "unable to check session revocation", 1, true))
+  end)
+
+  it("does not return the access token", function()
+    assert.is_nil(string.find(body, "a_token", 1, true))
+  end)
+end)
+
+describe("if revocation_fail_mode is open and the revocation store read fails", function()
+  test_support.start_server({
+    revocation_test = {
+      fail_mode = "open",
+      get_fails_after = 1,
+    },
+  })
+  teardown(test_support.stop_server)
+
+  local _, _, cookies = test_support.login()
+  local content_table = {}
+  local _, status = http.request({
+    url = "http://localhost/access_token",
+    redirect = false,
+    headers = { cookie = cookies },
+    sink = ltn12.sink.table(content_table)
+  })
+
+  it("returns the access token", function()
+    assert.are.equals(200, status)
+    assert.are.equals("a_token\n", table.concat(content_table))
+  end)
+end)
+
 describe("if there is an active non-expired login but access token is not stored in session", function()
   test_support.start_server({
     access_token_opts = {

tests/spec/logout_revocation_spec.lua

@@ -0,0 +1,135 @@
+local http = require("socket.http")
+local ltn12 = require("ltn12")
+local test_support = require("test_support")
+require 'busted.runner'()
+
+describe("when session startup returns no session", function()
+  test_support.start_server({
+    session_start_fails = true,
+  })
+  teardown(test_support.stop_server)
+
+  local response_body = {}
+  local _, status = http.request({
+    url = "http://127.0.0.1/default/t",
+    redirect = false,
+    sink = ltn12.sink.table(response_body),
+  })
+
+  it("returns the startup error without authenticating the request", function()
+    assert.are.equals(401, status)
+    assert.truthy(string.find(table.concat(response_body), "session start failed", 1, true))
+  end)
+end)
+
+describe("when revocation_fail_mode is closed and the revocation store read fails", function()
+  test_support.start_server({
+    revocation_test = {
+      fail_mode = "closed",
+      get_fails_after = 1,
+    },
+  })
+  teardown(test_support.stop_server)
+
+  local _, _, cookie = test_support.login()
+
+  local response_body = {}
+  local _, status = http.request({
+    url = "http://127.0.0.1/default/t",
+    headers = { cookie = cookie },
+    redirect = false,
+    sink = ltn12.sink.table(response_body),
+  })
+  local body = table.concat(response_body)
+
+  it("propagates the session start failure", function()
+    assert.are.equals(401, status)
+    assert.truthy(string.find(body, "unable to check session revocation", 1, true))
+  end)
+
+  it("does not authenticate the request", function()
+    assert.is_nil(string.find(body, "hello, world!", 1, true))
+  end)
+end)
+
+describe("when revocation_fail_mode is open and the revocation store read fails", function()
+  test_support.start_server({
+    revocation_test = {
+      fail_mode = "open",
+      get_fails_after = 1,
+    },
+  })
+  teardown(test_support.stop_server)
+
+  local _, _, cookie = test_support.login()
+
+  local response_body = {}
+  local _, status = http.request({
+    url = "http://127.0.0.1/default/t",
+    headers = { cookie = cookie },
+    redirect = false,
+    sink = ltn12.sink.table(response_body),
+  })
+
+  it("authenticates the request", function()
+    assert.are.equals(200, status)
+    assert.truthy(string.find(table.concat(response_body), "hello, world!", 1, true))
+  end)
+end)
+
+describe("when revocation_fail_mode is closed and the revocation store is unreachable during logout", function()
+  test_support.start_server({
+    revocation_test = {
+      fail_mode = "closed",
+      set_fails = true,
+    },
+  })
+  teardown(test_support.stop_server)
+
+  local _, _, cookie = test_support.login()
+
+  local response_body = {}
+  local _, status = http.request({
+    url = "http://127.0.0.1/default/logout",
+    headers = { cookie = cookie },
+    redirect = false,
+    sink = ltn12.sink.table(response_body),
+  })
+  local body = table.concat(response_body)
+
+  it("propagates destroy failure", function()
+    assert.are.equals(401, status)
+    assert.truthy(string.find(body, "unable to mark session revoked", 1, true))
+  end)
+
+  it("leaves the session valid for replay", function()
+    local _, replay_status = http.request({
+      url = "http://127.0.0.1/default/t",
+      headers = { cookie = cookie },
+      redirect = false,
+    })
+    assert.are.equals(200, replay_status)
+  end)
+end)
+
+describe("when revocation_fail_mode is open and the revocation store is unreachable during logout", function()
+  test_support.start_server({
+    revocation_test = {
+      fail_mode = "open",
+      set_fails = true,
+    },
+  })
+  teardown(test_support.stop_server)
+
+  local _, _, cookie = test_support.login()
+
+  local _, status = http.request({
+    url = "http://127.0.0.1/default/logout",
+    headers = { cookie = cookie },
+    redirect = false,
+  })
+
+  it("logout still completes", function()
+    assert.are.equals(200, status)
+  end)
+end)