From 81579fc66e3a48bf9da5d53eb831ac6c5a3d9b29 Mon Sep 17 00:00:00 2001 From: Hockenba Date: Tue, 1 Sep 2026 10:16:29 -0400 Subject: [PATCH 1/6] Propagate session destroy failures from logout. When session:destroy() fails (for example because the revocation store is unreachable in closed fail mode), return the error from openidc_logout and authenticate instead of continuing with the logout redirect. --- lib/resty/openidc.lua | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/lib/resty/openidc.lua b/lib/resty/openidc.lua index f50210a..2e486c8 100644 --- a/lib/resty/openidc.lua +++ b/lib/resty/openidc.lua @@ -1871,7 +1871,11 @@ local function openidc_logout(opts, session) end if is_session_present(session) then - session:destroy() + local ok, destroy_err = session:destroy() + if not ok then + log(ERROR, "failed to destroy session: " .. destroy_err) + return destroy_err + end end if opts.revoke_tokens_on_logout then @@ -2084,7 +2088,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 From c17e617256f290f34980761f72e531d1aca3db0c Mon Sep 17 00:00:00 2001 From: Hockenba Date: Wed, 2 Sep 2026 11:19:06 -0400 Subject: [PATCH 2/6] changes --- lib/resty/openidc.lua | 56 ++++++++++++++++++++------ lua-resty-openidc-1.9.0-1.rockspec | 2 +- tests/Dockerfile | 2 +- tests/spec/logout_revocation_spec.lua | 58 +++++++++++++++++++++++++++ tests/spec/test_support.lua | 32 ++++++++++++++- 5 files changed, 135 insertions(+), 15 deletions(-) create mode 100644 tests/spec/logout_revocation_spec.lua diff --git a/lib/resty/openidc.lua b/lib/resty/openidc.lua index 2e486c8..622c78c 100644 --- a/lib/resty/openidc.lua +++ b/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,12 +1901,9 @@ local function openidc_logout(opts, session) end end - if is_session_present(session) then - local ok, destroy_err = session:destroy() - if not ok then - log(ERROR, "failed to destroy session: " .. destroy_err) - return destroy_err - end + err = openidc_destroy_session(session) + if err then + return err end if opts.revoke_tokens_on_logout then @@ -2051,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 @@ -2192,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 diff --git a/lua-resty-openidc-1.9.0-1.rockspec b/lua-resty-openidc-1.9.0-1.rockspec index 3be0206..6a8a4df 100644 --- a/lua-resty-openidc-1.9.0-1.rockspec +++ b/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", "lua-resty-jwt >= 0.2.0", "lua-resty-openssl >= 1.8.0" } diff --git a/tests/Dockerfile b/tests/Dockerfile index 44355dd..adffcf3 100644 --- a/tests/Dockerfile +++ b/tests/Dockerfile @@ -1,7 +1,7 @@ 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"] diff --git a/tests/spec/logout_revocation_spec.lua b/tests/spec/logout_revocation_spec.lua new file mode 100644 index 0000000..ca94f5b --- /dev/null +++ b/tests/spec/logout_revocation_spec.lua @@ -0,0 +1,58 @@ +local http = require("socket.http") +local test_support = require("test_support") +require 'busted.runner'() + +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, + }, + unauth_action = "deny", + }) + teardown(test_support.stop_server) + + local _, _, cookie = test_support.login() + + local body, status = http.request({ + url = "http://127.0.0.1/default/logout", + headers = { cookie = cookie }, + redirect = false, + }) + + 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) diff --git a/tests/spec/test_support.lua b/tests/spec/test_support.lua index e173f91..42390fe 100644 --- a/tests/spec/test_support.lua +++ b/tests/spec/test_support.lua @@ -95,6 +95,10 @@ local DEFAULT_SHARE_OIDC_OPTS = "false" local DEFAULT_DELAY_RESPONSE = "0" +local DEFAULT_REVOCATION_TEST_ENABLED = "false" +local DEFAULT_REVOCATION_FAIL_MODE = '"closed"' +local DEFAULT_REVOCATION_SET_FAILS = "false" + local DEFAULT_INIT_TEMPLATE = [[ local test_globals = {} local sign_secret = [=[ @@ -146,6 +150,26 @@ test_globals.body_decorator = function(req) return req end test_globals.jwks = [=[JWK]=] +test_globals.session_opts = nil +if REVOCATION_TEST_ENABLED then + local revoked = ngx.shared.revocation_test + test_globals.session_opts = { + storage = "cookie", + revocation_fail_mode = REVOCATION_FAIL_MODE, + revocation = { + set = function(_, key, value, ttl) + if REVOCATION_SET_FAILS then + return nil, "connection refused" + end + revoked:set(key, value, ttl) + return true + end, + get = function(_, key) + return revoked:get(key) + end, + }, + } +end return test_globals ]] @@ -163,6 +187,7 @@ http { lua_package_path '~/lua/?.lua;/tmp/server/conf/?.lua;;'; lua_shared_dict discovery 1m; lua_shared_dict jwt_verification 1m; + lua_shared_dict revocation_test 1m; init_by_lua_block { test_globals = require("test_globals") } @@ -213,7 +238,7 @@ http { if opts.decorate then opts.http_request_decorator = opts.decorate == "body" and test_globals.body_decorator or test_globals.query_decorator end - local res, err, target, session = test_globals.oidc.authenticate(opts, nil, UNAUTH_ACTION) + local res, err, target, session = test_globals.oidc.authenticate(opts, nil, UNAUTH_ACTION, test_globals.session_opts) if err then ngx.status = 401 ngx.log(ngx.ERR, "authenticate failed: " .. err) @@ -654,6 +679,11 @@ local function write_template(out, template, custom_config) :gsub("FIXED_NGX_TIME", custom_config["fixed_ngx_time"] or "nil") :gsub("UNAUTH_ACTION", custom_config["unauth_action"] and ('"' .. custom_config["unauth_action"] .. '"') or DEFAULT_UNAUTH_ACTION) :gsub("SHARE_OIDC_OPTS", custom_config["share_oidc_opts"] and "true" or DEFAULT_SHARE_OIDC_OPTS) + :gsub("REVOCATION_TEST_ENABLED", custom_config["revocation_test"] and "true" or DEFAULT_REVOCATION_TEST_ENABLED) + :gsub("REVOCATION_FAIL_MODE", custom_config["revocation_test"] and + ('"' .. (custom_config["revocation_test"].fail_mode or "closed") .. '"') or DEFAULT_REVOCATION_FAIL_MODE) + :gsub("REVOCATION_SET_FAILS", custom_config["revocation_test"] and + (custom_config["revocation_test"].set_fails and "true" or "false") or DEFAULT_REVOCATION_SET_FAILS) out:write(content) end From d128dc9c289f169ec9b89a211c856226b537b931 Mon Sep 17 00:00:00 2001 From: Hockenba Date: Wed, 2 Sep 2026 13:25:10 -0400 Subject: [PATCH 3/6] tests fixed --- ChangeLog | 2 ++ tests/Dockerfile | 2 +- tests/spec/logout_revocation_spec.lua | 7 +++++-- 3 files changed, 8 insertions(+), 3 deletions(-) diff --git a/ChangeLog b/ChangeLog index da6e721..88bd2f6 100644 --- a/ChangeLog +++ b/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. diff --git a/tests/Dockerfile b/tests/Dockerfile index adffcf3..9d6de97 100644 --- a/tests/Dockerfile +++ b/tests/Dockerfile @@ -7,7 +7,7 @@ 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"] diff --git a/tests/spec/logout_revocation_spec.lua b/tests/spec/logout_revocation_spec.lua index ca94f5b..854b80e 100644 --- a/tests/spec/logout_revocation_spec.lua +++ b/tests/spec/logout_revocation_spec.lua @@ -1,4 +1,5 @@ local http = require("socket.http") +local ltn12 = require("ltn12") local test_support = require("test_support") require 'busted.runner'() @@ -8,17 +9,19 @@ describe("when revocation_fail_mode is closed and the revocation store is unreac fail_mode = "closed", set_fails = true, }, - unauth_action = "deny", }) teardown(test_support.stop_server) local _, _, cookie = test_support.login() - local body, status = http.request({ + 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) From 02bbadc4d3fdc65dde18c4a816bf39e5aa62a66f Mon Sep 17 00:00:00 2001 From: Hockenba Date: Thu, 3 Sep 2026 12:02:18 -0400 Subject: [PATCH 4/6] addressing comments --- lua-resty-openidc-1.9.0-1.rockspec | 2 +- tests/spec/access_token_access_spec.lua | 29 ++++++++++++++++++++++++ tests/spec/logout_revocation_spec.lua | 30 +++++++++++++++++++++++++ tests/spec/test_support.lua | 10 ++++++++- 4 files changed, 69 insertions(+), 2 deletions(-) diff --git a/lua-resty-openidc-1.9.0-1.rockspec b/lua-resty-openidc-1.9.0-1.rockspec index 6a8a4df..8dbcca0 100644 --- a/lua-resty-openidc-1.9.0-1.rockspec +++ b/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.2.0", + "lua-resty-session = 4.2.0", "lua-resty-jwt >= 0.2.0", "lua-resty-openssl >= 1.8.0" } diff --git a/tests/spec/access_token_access_spec.lua b/tests/spec/access_token_access_spec.lua index bd2347b..8490c7e 100644 --- a/tests/spec/access_token_access_spec.lua +++ b/tests/spec/access_token_access_spec.lua @@ -39,6 +39,35 @@ 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 there is an active non-expired login but access token is not stored in session", function() test_support.start_server({ access_token_opts = { diff --git a/tests/spec/logout_revocation_spec.lua b/tests/spec/logout_revocation_spec.lua index 854b80e..d0326bd 100644 --- a/tests/spec/logout_revocation_spec.lua +++ b/tests/spec/logout_revocation_spec.lua @@ -3,6 +3,36 @@ local ltn12 = require("ltn12") local test_support = require("test_support") require 'busted.runner'() +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 closed and the revocation store is unreachable during logout", function() test_support.start_server({ revocation_test = { diff --git a/tests/spec/test_support.lua b/tests/spec/test_support.lua index 42390fe..234cbe9 100644 --- a/tests/spec/test_support.lua +++ b/tests/spec/test_support.lua @@ -98,6 +98,7 @@ local DEFAULT_DELAY_RESPONSE = "0" local DEFAULT_REVOCATION_TEST_ENABLED = "false" local DEFAULT_REVOCATION_FAIL_MODE = '"closed"' local DEFAULT_REVOCATION_SET_FAILS = "false" +local DEFAULT_REVOCATION_GET_FAILS_AFTER = "nil" local DEFAULT_INIT_TEMPLATE = [[ local test_globals = {} @@ -165,6 +166,10 @@ if REVOCATION_TEST_ENABLED then return true end, get = function(_, key) + local calls = revoked:incr("get_calls", 1, 0) + if REVOCATION_GET_FAILS_AFTER and calls > REVOCATION_GET_FAILS_AFTER then + return nil, "connection refused" + end return revoked:get(key) end, }, @@ -539,10 +544,11 @@ http { location /access_token { content_by_lua_block { - local access_token, err = test_globals.oidc.access_token(ACCESS_TOKEN_OPTS) + local access_token, err = test_globals.oidc.access_token(ACCESS_TOKEN_OPTS, test_globals.session_opts) if not access_token then ngx.status = 401 ngx.log(ngx.ERR, "access_token error: " .. (err or 'no message')) + ngx.say("access_token failed: " .. (err or 'no message')) else ngx.header.content_type = 'text/plain' ngx.say(access_token) @@ -684,6 +690,8 @@ local function write_template(out, template, custom_config) ('"' .. (custom_config["revocation_test"].fail_mode or "closed") .. '"') or DEFAULT_REVOCATION_FAIL_MODE) :gsub("REVOCATION_SET_FAILS", custom_config["revocation_test"] and (custom_config["revocation_test"].set_fails and "true" or "false") or DEFAULT_REVOCATION_SET_FAILS) + :gsub("REVOCATION_GET_FAILS_AFTER", custom_config["revocation_test"] and + tostring(custom_config["revocation_test"].get_fails_after) or DEFAULT_REVOCATION_GET_FAILS_AFTER) out:write(content) end From dc93e2944acf464f3ffc414007b9fa50b741432b Mon Sep 17 00:00:00 2001 From: Hockenba Date: Thu, 3 Sep 2026 12:38:05 -0400 Subject: [PATCH 5/6] test open-mode revocation read failures --- tests/spec/access_token_access_spec.lua | 24 ++++++++++++++ tests/spec/logout_revocation_spec.lua | 44 +++++++++++++++++++++++++ tests/spec/test_support.lua | 7 ++++ 3 files changed, 75 insertions(+) diff --git a/tests/spec/access_token_access_spec.lua b/tests/spec/access_token_access_spec.lua index 8490c7e..174f9b3 100644 --- a/tests/spec/access_token_access_spec.lua +++ b/tests/spec/access_token_access_spec.lua @@ -68,6 +68,30 @@ describe("if revocation_fail_mode is closed and the revocation store read fails" 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 = { diff --git a/tests/spec/logout_revocation_spec.lua b/tests/spec/logout_revocation_spec.lua index d0326bd..a1d591d 100644 --- a/tests/spec/logout_revocation_spec.lua +++ b/tests/spec/logout_revocation_spec.lua @@ -3,6 +3,25 @@ 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 = { @@ -33,6 +52,31 @@ describe("when revocation_fail_mode is closed and the revocation store read fail 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 = { diff --git a/tests/spec/test_support.lua b/tests/spec/test_support.lua index 234cbe9..2250a15 100644 --- a/tests/spec/test_support.lua +++ b/tests/spec/test_support.lua @@ -99,6 +99,7 @@ local DEFAULT_REVOCATION_TEST_ENABLED = "false" local DEFAULT_REVOCATION_FAIL_MODE = '"closed"' local DEFAULT_REVOCATION_SET_FAILS = "false" local DEFAULT_REVOCATION_GET_FAILS_AFTER = "nil" +local DEFAULT_SESSION_START_FAILS = "false" local DEFAULT_INIT_TEMPLATE = [[ local test_globals = {} @@ -108,6 +109,11 @@ JWT_SIGN_SECRET]=] if os.getenv('coverage') then require("luacov.runner")("/spec/luacov/settings.luacov") end +if SESSION_START_FAILS then + require("resty.session").start = function() + return nil, "session start failed" + end +end test_globals.oidc = require "resty.openidc" test_globals.cjson = require "cjson" if FIXED_NGX_TIME ~= nil then @@ -692,6 +698,7 @@ local function write_template(out, template, custom_config) (custom_config["revocation_test"].set_fails and "true" or "false") or DEFAULT_REVOCATION_SET_FAILS) :gsub("REVOCATION_GET_FAILS_AFTER", custom_config["revocation_test"] and tostring(custom_config["revocation_test"].get_fails_after) or DEFAULT_REVOCATION_GET_FAILS_AFTER) + :gsub("SESSION_START_FAILS", custom_config["session_start_fails"] and "true" or DEFAULT_SESSION_START_FAILS) out:write(content) end From 0445d7096c25179da031385fd1550783dcf95061 Mon Sep 17 00:00:00 2001 From: LIAN_XIAOYI <13826660+kevinlzw@users.noreply.github.com> Date: Sat, 5 Sep 2026 11:11:33 +0900 Subject: [PATCH 6/6] Fix lua-resty-session rockspec constraint --- lua-resty-openidc-1.9.0-1.rockspec | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lua-resty-openidc-1.9.0-1.rockspec b/lua-resty-openidc-1.9.0-1.rockspec index 8dbcca0..51ae2d4 100644 --- a/lua-resty-openidc-1.9.0-1.rockspec +++ b/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.2.0", + "lua-resty-session == 4.2.0-1", "lua-resty-jwt >= 0.2.0", "lua-resty-openssl >= 1.8.0" }