From ecf53fc1ecf069f70cae048c7d50d1b180ac1b8b Mon Sep 17 00:00:00 2001 From: Abhishek Choudhary Date: Thu, 10 Sep 2026 12:54:23 +0545 Subject: [PATCH] fix(introspection): preserve coordination ownership --- ChangeLog | 2 + README.md | 3 +- lib/resty/openidc.lua | 71 ++++++++++++++++++++----------- tests/spec/introspection_spec.lua | 64 ++++++++++++++++++++++++++++ tests/spec/test_support.lua | 7 +++ 5 files changed, 121 insertions(+), 26 deletions(-) diff --git a/ChangeLog b/ChangeLog index f4fd7a0..531f56b 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,6 +1,8 @@ 09/10/2026 - coalesce concurrent introspection cache misses for the same token, allowing waiting requests to share successful and failed results +- make introspection coordination honor explicit cache bypass and isolate + temporary results and lock release by lock generation 09/13/204 - cross-tenant requests are fixed with lua-resty session 4.0.x; closes #526 diff --git a/README.md b/README.md index c51bdb7..2efbd45 100644 --- a/README.md +++ b/README.md @@ -371,7 +371,8 @@ Currently up to four caches are used token and cache segment are coalesced within an NGINX instance. One request calls the introspection endpoint while the others wait for and share its result, including endpoint failures and responses without an - expiry claim. + expiry claim. Setting `introspection_cache_ignore` disables both caching + and request coalescing. * the cache named `jwt_verification` stores the result of JWT verification. Cache items expire when the corresponding token expires. Tokens with unknown expiry are not cached for two diff --git a/lib/resty/openidc.lua b/lib/resty/openidc.lua index de7d786..9bc8ba5 100644 --- a/lib/resty/openidc.lua +++ b/lib/resty/openidc.lua @@ -61,6 +61,11 @@ local function openidc_sha256(value) return sha256:final() end +local function openidc_random_jti() + local resty_random = require("resty.random") + return b64url(resty_random.bytes(32)) +end + local log = ngx.log local DEBUG = ngx.DEBUG local ERROR = ngx.ERR @@ -1848,15 +1853,16 @@ local function decode_cached_introspection(value) return json, err end -local function acquire_introspection_lock(dict, key, timeout, exptime) - local ok, err = dict:add(key, true, exptime) +local function acquire_introspection_lock(dict, key, owner, timeout, exptime) + local ok, err = dict:add(key, owner, exptime) if ok then - return true + return true, nil, false end if err ~= "exists" then return nil, err end + local observed_owner = dict:get(key) local elapsed = 0 local step = 0.001 while elapsed < timeout do @@ -1864,13 +1870,14 @@ local function acquire_introspection_lock(dict, key, timeout, exptime) ngx.sleep(step) elapsed = elapsed + step - ok, err = dict:add(key, true, exptime) + ok, err = dict:add(key, owner, exptime) if ok then - return true + return true, nil, observed_owner end if err ~= "exists" then return nil, err end + observed_owner = dict:get(key) or observed_owner step = math.min(step * 2, 0.5) end @@ -1878,6 +1885,19 @@ local function acquire_introspection_lock(dict, key, timeout, exptime) return nil, "timeout" end +local function release_introspection_lock(dict, key, owner) + if dict:get(key) == owner then + dict:delete(key) + end +end + +local function publish_introspection_result(dict, key, value, ttl) + local ok, err = dict:set(key, value, ttl) + if not ok then + log(WARN, "failed to publish introspection result: " .. err) + end +end + -- main routine for OAuth 2.0 token introspection function openidc.introspect(opts) @@ -1907,8 +1927,8 @@ function openidc.introspect(opts) local cache_key = get_introspection_cache_key(opts, access_token) local digest = b64url(openidc_sha256(cache_key)) local lock_key = "openidc-introspection-lock:" .. digest - local result_key = "openidc-introspection-result:" .. digest - local started_at = ngx.now() + local result_key_prefix = "openidc-introspection-result:" .. digest .. ":" + local lock_owner = openidc_random_jti() local lock_timeout = opts.introspection_lock_timeout or 5 local lock_exptime = opts.introspection_lock_exptime or 30 @@ -1920,8 +1940,9 @@ function openidc.introspect(opts) end local locked - locked, err = acquire_introspection_lock(introspection_cache, lock_key, - lock_timeout, lock_exptime) + local observed_owner + locked, err, observed_owner = acquire_introspection_lock( + introspection_cache, lock_key, lock_owner, lock_timeout, lock_exptime) if not locked then return nil, "failed to acquire introspection lock: " .. err end @@ -1930,19 +1951,22 @@ function openidc.introspect(opts) -- waited for the lock. value = get_cached_introspection(opts, access_token) if value then - introspection_cache:delete(lock_key) + release_introspection_lock(introspection_cache, lock_key, lock_owner) return decode_cached_introspection(value) end -- Responses which cannot enter the regular cache (including endpoint - -- failures) are published briefly so requests that started during the same - -- in-flight lookup can share the outcome. Requests that start after the - -- lookup completed do not reuse this record. - local completed_value = introspection_cache:get(result_key) - if completed_value then + -- failures) are published briefly under the lock generation. Only requests + -- which observed that generation while waiting can share the outcome. + local completed_value = observed_owner and + introspection_cache:get(result_key_prefix .. observed_owner) + if completed_value and observed_owner then local completed = cjson_s.decode(completed_value) - if completed and completed.completed_at >= started_at then - introspection_cache:delete(lock_key) + if completed then + publish_introspection_result( + introspection_cache, result_key_prefix .. lock_owner, + completed_value, math.max(lock_timeout, 1)) + release_introspection_lock(introspection_cache, lock_key, lock_owner) return completed.json, completed.err end end @@ -1952,24 +1976,21 @@ function openidc.introspect(opts) -- A cacheable response is already visible to waiters. Publish only outcomes -- that the regular introspection cache did not retain. - if not get_cached_introspection(opts, access_token) then + if not introspection_cache:get(cache_key) then local completed, encode_err = cjson_s.encode({ - completed_at = ngx.now(), json = json, err = err, }) if completed then - local result_ttl = math.max(lock_timeout, 1) - local ok, set_err = introspection_cache:set(result_key, completed, result_ttl) - if not ok then - log(WARN, "failed to publish introspection result: " .. set_err) - end + publish_introspection_result( + introspection_cache, result_key_prefix .. lock_owner, + completed, math.max(lock_timeout, 1)) else log(WARN, "failed to encode introspection result: " .. encode_err) end end - introspection_cache:delete(lock_key) + release_introspection_lock(introspection_cache, lock_key, lock_owner) return json, err end diff --git a/tests/spec/introspection_spec.lua b/tests/spec/introspection_spec.lua index 5e6d112..774b4c7 100644 --- a/tests/spec/introspection_spec.lua +++ b/tests/spec/introspection_spec.lua @@ -418,6 +418,20 @@ describe("when a batch sends 35 concurrent requests with the same uncached token end) end) +describe("when concurrent requests explicitly bypass the introspection cache", function() + test_support.start_server({ + delay_response = { introspection = 300 }, + introspection_opts = { introspection_cache_ignore = true }, + }) + teardown(test_support.stop_server) + local jwt = test_support.trim(http.request("http://127.0.0.1/jwt")) + request_introspection_concurrently(jwt, 10) + + it("introspects every request independently", function() + assert.are.equals(10, error_log_occurrences("Received introspection request:")) + end) +end) + describe("when concurrent requests introspect a response without an expiry", function() test_support.start_server({ delay_response = { introspection = 300 }, @@ -440,6 +454,56 @@ describe("when concurrent requests introspect a response without an expiry", fun end) end) +describe("when sequential non-cacheable requests have the same ngx.now value", function() + test_support.start_server({ + fixed_ngx_now = 1000, + remove_introspection_claims = { "exp" }, + introspection_opts = { introspection_cache_ignore = false }, + }) + teardown(test_support.stop_server) + local jwt = test_support.trim(http.request("http://127.0.0.1/jwt")) + local headers = { authorization = "Bearer " .. jwt } + local _, first_status = http.request({ + url = "http://127.0.0.1/introspect", + headers = headers, + }) + local _, second_status = http.request({ + url = "http://127.0.0.1/introspect", + headers = headers, + }) + + it("does not share the first request's temporary result", function() + assert.are.equals(200, first_status) + assert.are.equals(200, second_status) + assert.are.equals(2, error_log_occurrences("Received introspection request:")) + end) +end) + +describe("when an introspection outlasts its lock generation", function() + test_support.start_server({ + delay_response = { introspection = 700 }, + remove_introspection_claims = { "exp" }, + introspection_opts = { + introspection_cache_ignore = false, + introspection_lock_exptime = 0.5, + introspection_lock_timeout = 0.1, + }, + }) + teardown(test_support.stop_server) + local jwt = test_support.trim(http.request("http://127.0.0.1/jwt")) + local curl = "curl -sS -o /dev/null -H 'Authorization: Bearer " .. jwt .. + "' http://127.0.0.1/introspect" + local command = curl .. " & first_pid=$!; sleep 0.55; " .. + curl .. " & second_pid=$!; sleep 0.25; " .. curl .. + "; wait $first_pid $second_pid" + local ok = os.execute(command) + + it("keeps the replacement generation locked", function() + assert.truthy(ok == true or ok == 0) + assert.are.equals(2, error_log_occurrences("Received introspection request:")) + end) +end) + describe("when introspection endpoint is not resolvable", function() test_support.start_server({ introspection_opts = { diff --git a/tests/spec/test_support.lua b/tests/spec/test_support.lua index 429832e..9967c00 100644 --- a/tests/spec/test_support.lua +++ b/tests/spec/test_support.lua @@ -102,6 +102,12 @@ if os.getenv('coverage') then end test_globals.oidc = require "resty.openidc" test_globals.cjson = require "cjson" +if FIXED_NGX_NOW ~= nil then + local fixed_ngx_now = FIXED_NGX_NOW + ngx.now = function() + return fixed_ngx_now + end +end test_globals.delay = function(delay_response) if delay_response > 0 then ngx.sleep(delay_response / 1000) @@ -510,6 +516,7 @@ local function write_template(out, template, custom_config) :gsub("REFRESH_ID_TOKEN", serpent.block(refresh_id_token, {comment = false })) :gsub("ID_TOKEN", serpent.block(id_token, {comment = false })) :gsub("ACCESS_TOKEN", serpent.block(access_token, {comment = false })) + :gsub("FIXED_NGX_NOW", custom_config["fixed_ngx_now"] or "nil") :gsub("UNAUTH_ACTION", custom_config["unauth_action"] and ('"' .. custom_config["unauth_action"] .. '"') or DEFAULT_UNAUTH_ACTION) out:write(content) end