From 5624c196e6f42056b6ace7d582a7919627f52c17 Mon Sep 17 00:00:00 2001 From: Abhishek Choudhary Date: Thu, 27 Aug 2026 14:51:44 +0545 Subject: [PATCH 01/29] fix: accept OneTimeUse, and let the replay record honour it SAML Core 2.5.1.5 makes OneTimeUse always valid: a condition on use, asking the SP to keep a record of the assertions it has spent. It was refused as a condition this SP cannot satisfy, so an IdP stamping its assertions single-use could not log in at all. The reader now carries it as a flag. With replay_dict set the record exists and the assertion is single-use as asked; without it the login goes through and a warning names the option. Closes #46 --- README.md | 17 +++++++++++------ lua/resty/saml.lua | 9 +++++++++ src/lua_saml.c | 1 + src/saml.h | 1 + src/xml.c | 18 +++++++++++------- t/assertion-conditions.t | 32 ++++++++++++++++++++++++++++---- 6 files changed, 61 insertions(+), 17 deletions(-) diff --git a/README.md b/README.md index 86d8484..c3f0d9b 100644 --- a/README.md +++ b/README.md @@ -141,12 +141,17 @@ the delivery window is minutes and the assertion window at most an hour, and the alternative is a record nothing reclaims. The limit an operator can move is `replay_ttl`; the day cap is fixed. -**Two things it deliberately does not do.** An assertion carrying `` -is still refused outright, so an IdP asking for exactly this protection cannot log in -even with the option on; that is tracked separately and the two do not meet yet. And -re-submitting a response that already logged in is refused, which is what a browser -does when it loses the redirect that ends a login. Returning to the application starts -a fresh login, and the IdP will not ask for a password again. +**This is what `` asks for.** An IdP stamps that condition on an +assertion to ask the SP to keep exactly this record. SAML Core 2.5.1.5 makes the +condition always valid, a condition on use rather than on validity, so the login goes +through with or without the option. With it, the assertion is single-use as the IdP +asked. Without it, the login is accepted and a warning names `replay_dict`, so an IdP +that asks for this is the signal to set it. + +**One thing it deliberately does not do.** Re-submitting a response that already logged +in is refused, which is what a browser does when it loses the redirect that ends a +login. Returning to the application starts a fresh login, and the IdP will not ask for +a password again. #### Seeding the worker diff --git a/lua/resty/saml.lua b/lua/resty/saml.lua index 8db5b6f..0ab8867 100644 --- a/lua/resty/saml.lua +++ b/lua/resty/saml.lua @@ -419,6 +419,15 @@ local function assertions_acceptable(opts, assertions, expected, now) assertion.unknown_condition end + -- Core 2.5.1.5: OneTimeUse is always valid, and asks the SP to keep a + -- record of the assertions it has spent. replay_dict is that record; + -- without it the IdP's request goes unmet, and the operator is told + -- what to configure rather than the user refused + if assertion.one_time_use and not opts.replay_dict then + ngx.log(ngx.WARN, "assertion ", loggable(assertion.id), + " carries OneTimeUse, which this SP cannot enforce without replay_dict") + end + local ok, err = time_bounds_ok(assertion.not_before, assertion.not_on_or_after, now, skew) if not ok then return false, where .. err diff --git a/src/lua_saml.c b/src/lua_saml.c index f0a7d2f..8c8e653 100644 --- a/src/lua_saml.c +++ b/src/lua_saml.c @@ -698,6 +698,7 @@ static int doc_assertions(lua_State* L) { set_str_field(L, "id", a->id); set_str_field(L, "issuer", a->issuer); set_bool_field(L, "has_conditions", a->has_conditions); + set_bool_field(L, "one_time_use", a->one_time_use); set_str_field(L, "not_before", a->not_before); set_str_field(L, "not_on_or_after", a->not_on_or_after); set_str_field(L, "unknown_condition", a->unknown_condition); diff --git a/src/saml.h b/src/saml.h index 5d7592e..863372c 100644 --- a/src/saml.h +++ b/src/saml.h @@ -59,6 +59,7 @@ typedef struct { xmlChar* id; xmlChar* issuer; int has_conditions; + int one_time_use; xmlChar* not_before; xmlChar* not_on_or_after; xmlChar* unknown_condition; diff --git a/src/xml.c b/src/xml.c index 61ba856..756df42 100644 --- a/src/xml.c +++ b/src/xml.c @@ -396,18 +396,19 @@ static size_t count_assertion_el(xmlNode* parent, const char* name) { } -// Conditions this SP can actually satisfy. SAML Core 2.5.1 makes an assertion +// Conditions this SP understands. SAML Core 2.5.1 makes an assertion // carrying any other one Indeterminate rather than valid, so everything else is // reported for the caller to refuse. // -// ProxyRestriction is here because it binds an IdP issuing on behalf of another -// IdP and asks nothing of the SP consuming the assertion. OneTimeUse is not, -// because honouring it means remembering which assertions have been spent, and -// Core 2.5.1.5 tells a party that cannot keep that record to treat the -// assertion as invalid. +// ProxyRestriction binds an IdP issuing on behalf of another IdP and asks +// nothing of the SP consuming the assertion. OneTimeUse is always valid by +// Core 2.5.1.5, a condition on use rather than on validity: it asks the SP to +// keep a record of the assertions it has spent, which the caller has or has +// not, so it is reported as a flag. static int is_known_condition(xmlNode* node) { return is_assertion_el(node, "AudienceRestriction") || - is_assertion_el(node, "ProxyRestriction"); + is_assertion_el(node, "ProxyRestriction") || + is_assertion_el(node, "OneTimeUse"); } @@ -510,6 +511,9 @@ static int read_assertion(xmlDoc* doc, xmlNode* node, saml_assertion_t* a) { } for (xmlNode* child = conditions->children; child != NULL; child = child->next) { + if (is_assertion_el(child, "OneTimeUse")) { + a->one_time_use = 1; + } if (child->type == XML_ELEMENT_NODE && !is_known_condition(child)) { // the caller refuses the assertion on this name, so losing it would // let the condition through rather than fail the read diff --git a/t/assertion-conditions.t b/t/assertion-conditions.t index 2bb6dde..b08f72d 100644 --- a/t/assertion-conditions.t +++ b/t/assertion-conditions.t @@ -574,9 +574,10 @@ offers no subject confirmation this SP can satisfy ngx.say(login_with("plain", saml_response({ conditions = conditions({ body = "" }), }))) - -- OneTimeUse asks this SP to remember which assertions it has spent + -- OneTimeUse is always valid (Core 2.5.1.5); with no replay_dict it + -- asks for a record this SP does not keep, which is said, not refused ngx.say(login_with("plain", saml_response({ - conditions = conditions({ body = "" }), + id = "single", conditions = conditions({ body = "" }), }))) -- and a condition it has never heard of asks who knows what ngx.say(login_with("plain", saml_response({ @@ -589,10 +590,10 @@ offers no subject confirmation this SP can satisfy } --- response_body 302 / -401 nil +302 / 401 nil --- error_log eval -[qr/carries a condition this SP cannot satisfy: OneTimeUse/, +[qr/\[warn\] .* assertion single carries OneTimeUse, which this SP cannot enforce without replay_dict/, qr/carries a condition this SP cannot satisfy: Condition/] @@ -1386,3 +1387,26 @@ earlier: true --- response_body 302 / dated one decides: true + + + +=== TEST 48: OneTimeUse is met by the replay record where there is one +--- config + location /t { + content_by_lua_block { + ngx.shared.saml_replay:flush_all() + local xml = saml_response({ + id = "single", conditions = conditions({ body = "" }), + }) + ngx.say(login_with("replay", xml)) + ngx.say(login_with("replay", xml)) + } + } +--- response_body +302 / +401 nil +--- error_log +assertion single has been presented already +--- no_error_log +[crit] +cannot enforce without replay_dict From 8bf6364b6e055728331359a3874ce58071a75b30 Mon Sep 17 00:00:00 2001 From: Abhishek Choudhary Date: Thu, 27 Aug 2026 18:47:05 +0545 Subject: [PATCH 02/29] fix: warn on the handle the gate enforces on, and say when a full dict drops OneTimeUse assertions_acceptable read opts.replay_dict while the last gate reads the resolved self.replay_dict. They agree today, but only by the constructor's say-so; passing the handle makes it structural. A full dict already logs that the login went untracked. When the assertion carries OneTimeUse the line now says so. --- lua/resty/saml.lua | 13 ++++++++----- t/assertion-conditions.t | 32 ++++++++++++++++++++++++++++++++ 2 files changed, 40 insertions(+), 5 deletions(-) diff --git a/lua/resty/saml.lua b/lua/resty/saml.lua index 0ab8867..40d518c 100644 --- a/lua/resty/saml.lua +++ b/lua/resty/saml.lua @@ -405,7 +405,7 @@ end -- Every top-level assertion the verified signature left in the document is one -- the readers draw identity from, so every one of them has to hold up. -local function assertions_acceptable(opts, assertions, expected, now) +local function assertions_acceptable(opts, assertions, expected, now, replay_dict) local skew = opts.clock_skew or DEFAULT_CLOCK_SKEW local accepted = opts.sp_audiences or { opts.sp_issuer } @@ -422,8 +422,9 @@ local function assertions_acceptable(opts, assertions, expected, now) -- Core 2.5.1.5: OneTimeUse is always valid, and asks the SP to keep a -- record of the assertions it has spent. replay_dict is that record; -- without it the IdP's request goes unmet, and the operator is told - -- what to configure rather than the user refused - if assertion.one_time_use and not opts.replay_dict then + -- what to configure rather than the user refused. The handle is the + -- one the last gate enforces on, so the two cannot disagree + if assertion.one_time_use and not replay_dict then ngx.log(ngx.WARN, "assertion ", loggable(assertion.id), " carries OneTimeUse, which this SP cannot enforce without replay_dict") end @@ -622,7 +623,8 @@ local function spend_assertions(dict, opts, assertions, expected, now) else ngx.log(ngx.ERR, "could not remember assertion ", loggable(assertion.id), " in ", opts.replay_dict, ": ", add_err, - ", this login is not covered by replay tracking") + ", this login is not covered by replay tracking", + assertion.one_time_use and " though it carries OneTimeUse" or "") end end @@ -714,7 +716,8 @@ local function login_callback(self, opts) end local now = ngx.time() - local acceptable, reason = assertions_acceptable(opts, assertions, expected, now) + local acceptable, reason = assertions_acceptable(opts, assertions, expected, now, + self.replay_dict) if not acceptable then ngx.log(ngx.ERR, "response from IdP rejected: ", loggable(reason)) ngx.exit(ngx.HTTP_UNAUTHORIZED) diff --git a/t/assertion-conditions.t b/t/assertion-conditions.t index b08f72d..a19fa31 100644 --- a/t/assertion-conditions.t +++ b/t/assertion-conditions.t @@ -1410,3 +1410,35 @@ assertion single has been presented already --- no_error_log [crit] cannot enforce without replay_dict + + + +=== TEST 49: a full dict says when the untracked login asked for single use +--- config + location /t { + content_by_lua_block { + local dict = ngx.shared.saml_replay_full + dict:flush_all() + dict:flush_expired() + local filler = string.rep("x", 256) + local i, ok = 0, true + while ok do + ok = dict:safe_set("filler-" .. i, filler, 600) + if ok then i = i + 1 end + if i > 5000 then break end + end + local j = 0 + while dict:safe_add("small-" .. j, true, 600) do + j = j + 1 + if j > 5000 then break end + end + ngx.say(login_with("replay_full", saml_response({ + id = "untracked-stamped", + conditions = conditions({ body = "" }), + }))) + } + } +--- response_body +302 / +--- error_log +in saml_replay_full: no memory, this login is not covered by replay tracking though it carries OneTimeUse From bd5226eda0ed14258bae311a0c942c920fbd5704 Mon Sep 17 00:00:00 2001 From: Abhishek Choudhary Date: Thu, 27 Aug 2026 18:47:49 +0545 Subject: [PATCH 03/29] fix: say when an OneTimeUse assertion outlives its record The record falls back to replay_ttl when nothing bounds acceptance and is capped at a day when the IdP's window runs longer, so past it the assertion is accepted again. Both need an IdP outside shipped defaults; where that IdP also asked for single use, the login now logs that the record fell short. --- lua/resty/saml.lua | 9 +++++++++ t/assertion-conditions.t | 27 +++++++++++++++++++++++++++ 2 files changed, 36 insertions(+) diff --git a/lua/resty/saml.lua b/lua/resty/saml.lua index 40d518c..541240e 100644 --- a/lua/resty/saml.lua +++ b/lua/resty/saml.lua @@ -609,6 +609,15 @@ local function spend_assertions(dict, opts, assertions, expected, now) ttl = MAX_REPLAY_TTL end + -- the record is bounded where acceptance is not, so past it the + -- assertion is accepted again. An IdP that asked for single use is + -- told, since it is the IdP's window that made the record fall short + if assertion.one_time_use and (usable_until == nil or ttl == MAX_REPLAY_TTL) then + ngx.log(ngx.WARN, "assertion ", loggable(assertion.id), + " carries OneTimeUse but stays acceptable past its record, which lapses in ", + ttl, " seconds") + end + local key = replay_key(opts, assertion) local added, add_err = dict:safe_add(key, true, ttl) if added then diff --git a/t/assertion-conditions.t b/t/assertion-conditions.t index a19fa31..326a9cc 100644 --- a/t/assertion-conditions.t +++ b/t/assertion-conditions.t @@ -1442,3 +1442,30 @@ cannot enforce without replay_dict 302 / --- error_log in saml_replay_full: no memory, this login is not covered by replay tracking though it carries OneTimeUse + + + +=== TEST 50: an OneTimeUse assertion that outlives its record says so +--- config + location /t { + content_by_lua_block { + ngx.shared.saml_replay:flush_all() + -- nothing bounds it, so the record falls back to replay_ttl + ngx.say(login_with("replay", saml_response({ + id = "stamped-unbounded", + conditions = conditions({ body = "" }), + }))) + -- valid for years, so the record is capped at a day + ngx.say(login_with("replay", saml_response({ + id = "stamped-forever", + conditions = conditions({ not_on_or_after = "9999-12-31T23:59:59Z", + body = "" }), + }))) + } + } +--- response_body +302 / +302 / +--- error_log eval +[qr/\[warn\] .* assertion stamped-unbounded carries OneTimeUse but stays acceptable past its record, which lapses in 600 seconds/, +qr/\[warn\] .* assertion stamped-forever carries OneTimeUse but stays acceptable past its record, which lapses in 86400 seconds/] From b162b7b457417eca5ad046e5507937250ad149e2 Mon Sep 17 00:00:00 2001 From: Abhishek Choudhary Date: Thu, 27 Aug 2026 18:47:49 +0545 Subject: [PATCH 04/29] docs: name the level the OneTimeUse warning is logged at --- README.md | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/README.md b/README.md index c3f0d9b..a26cd74 100644 --- a/README.md +++ b/README.md @@ -145,8 +145,9 @@ alternative is a record nothing reclaims. The limit an operator can move is assertion to ask the SP to keep exactly this record. SAML Core 2.5.1.5 makes the condition always valid, a condition on use rather than on validity, so the login goes through with or without the option. With it, the assertion is single-use as the IdP -asked. Without it, the login is accepted and a warning names `replay_dict`, so an IdP -that asks for this is the signal to set it. +asked. Without it, the login is accepted and a line at `warn` level names `replay_dict`, +so an IdP that asks for this is the signal to set it; a deployment logging at `error` +or above does not see it. **One thing it deliberately does not do.** Re-submitting a response that already logged in is refused, which is what a browser does when it loses the redirect that ends a From a01ffb81c5932d4641137021676224042ff21548 Mon Sep 17 00:00:00 2001 From: Abhishek Choudhary Date: Thu, 27 Aug 2026 18:47:49 +0545 Subject: [PATCH 05/29] docs: bound the OneTimeUse claim by the record's own limits --- README.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/README.md b/README.md index a26cd74..a5d1125 100644 --- a/README.md +++ b/README.md @@ -144,8 +144,8 @@ alternative is a record nothing reclaims. The limit an operator can move is **This is what `` asks for.** An IdP stamps that condition on an assertion to ask the SP to keep exactly this record. SAML Core 2.5.1.5 makes the condition always valid, a condition on use rather than on validity, so the login goes -through with or without the option. With it, the assertion is single-use as the IdP -asked. Without it, the login is accepted and a line at `warn` level names `replay_dict`, +through with or without the option. With it, the assertion is single-use within the +bounds above. Without it, the login is accepted and a line at `warn` level names `replay_dict`, so an IdP that asks for this is the signal to set it; a deployment logging at `error` or above does not see it. From c769806be17fae924ee5c8b04aeb6afdedfee822 Mon Sep 17 00:00:00 2001 From: Abhishek Choudhary Date: Thu, 27 Aug 2026 18:47:49 +0545 Subject: [PATCH 06/29] fix: read OneTimeUse whatever the order of the conditions The scan stopped at the first unknown condition, so a OneTimeUse after one was never seen. The name of the first unknown is still what is reported. --- src/xml.c | 7 ++++--- t/assertion-conditions.t | 23 +++++++++++++++++++++++ 2 files changed, 27 insertions(+), 3 deletions(-) diff --git a/src/xml.c b/src/xml.c index 756df42..6483c98 100644 --- a/src/xml.c +++ b/src/xml.c @@ -514,14 +514,15 @@ static int read_assertion(xmlDoc* doc, xmlNode* node, saml_assertion_t* a) { if (is_assertion_el(child, "OneTimeUse")) { a->one_time_use = 1; } - if (child->type == XML_ELEMENT_NODE && !is_known_condition(child)) { + if (child->type == XML_ELEMENT_NODE && !is_known_condition(child) && + a->unknown_condition == NULL) { // the caller refuses the assertion on this name, so losing it would - // let the condition through rather than fail the read + // let the condition through rather than fail the read. The scan goes + // on so what else the assertion carries is read whatever the order a->unknown_condition = xmlStrdup(child->name); if (a->unknown_condition == NULL) { return -1; } - break; } } diff --git a/t/assertion-conditions.t b/t/assertion-conditions.t index 326a9cc..78a7f48 100644 --- a/t/assertion-conditions.t +++ b/t/assertion-conditions.t @@ -1469,3 +1469,26 @@ in saml_replay_full: no memory, this login is not covered by replay tracking tho --- error_log eval [qr/\[warn\] .* assertion stamped-unbounded carries OneTimeUse but stays acceptable past its record, which lapses in 600 seconds/, qr/\[warn\] .* assertion stamped-forever carries OneTimeUse but stays acceptable past its record, which lapses in 86400 seconds/] + + + +=== TEST 51: OneTimeUse is read wherever it sits among the conditions +--- config + location /t { + content_by_lua_block { + local unknown = 'sp' + for _, body in ipairs({ "" .. unknown, unknown .. "" }) do + local doc, err = parse(sign_doc(response(assertion({ + id = "ordered", conditions = conditions({ body = body }), + })))) + if err then ngx.say("err: ", err) return end + local a = saml.doc_assertions(doc)[1] + ngx.say("one_time_use=", tostring(a.one_time_use), + " unknown_condition=", tostring(a.unknown_condition)) + end + } + } +--- response_body +one_time_use=true unknown_condition=Condition +one_time_use=true unknown_condition=Condition From b8728adb8a31ed19ad1b573c6019e353e928f70d Mon Sep 17 00:00:00 2001 From: Abhishek Choudhary Date: Thu, 27 Aug 2026 18:47:49 +0545 Subject: [PATCH 07/29] test: pin that without a record an OneTimeUse assertion is accepted again --- t/assertion-conditions.t | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/t/assertion-conditions.t b/t/assertion-conditions.t index 78a7f48..296d74b 100644 --- a/t/assertion-conditions.t +++ b/t/assertion-conditions.t @@ -1492,3 +1492,23 @@ qr/\[warn\] .* assertion stamped-forever carries OneTimeUse but stays acceptable --- response_body one_time_use=true unknown_condition=Condition one_time_use=true unknown_condition=Condition + + + +=== TEST 52: without a record, an OneTimeUse assertion is accepted again +--- config + location /t { + content_by_lua_block { + local xml = saml_response({ + id = "stamped-untracked", + conditions = conditions({ not_on_or_after = at(600), body = "" }), + }) + ngx.say(login_with("plain", xml)) + ngx.say(login_with("plain", xml)) + } + } +--- response_body +302 / +302 / +--- error_log eval +qr/\[warn\] .* assertion stamped-untracked carries OneTimeUse, which this SP cannot enforce without replay_dict/ From a2e67bea9f18520be8cacf484659d962e48097dd Mon Sep 17 00:00:00 2001 From: Abhishek Choudhary Date: Thu, 27 Aug 2026 18:47:50 +0545 Subject: [PATCH 08/29] test: give the recorded OneTimeUse case an expiry, a record check, and its own ID The block passed by replaying inside replay_ttl; it now bounds the assertion and reads the record back. Its no_error_log carries the guards the preprocessor would have injected, and TEST 16 reports the new field. --- t/assertion-conditions.t | 24 ++++++++++++++++-------- 1 file changed, 16 insertions(+), 8 deletions(-) diff --git a/t/assertion-conditions.t b/t/assertion-conditions.t index 296d74b..da06df1 100644 --- a/t/assertion-conditions.t +++ b/t/assertion-conditions.t @@ -632,7 +632,7 @@ response from IdP is addressed to http://evil.example.com/acs content_by_lua_block { local xml = sign_doc(response( assertion({ id = "a1", conditions = conditions({ not_on_or_after = "2026-07-21T00:00:00Z", - body = audience("sp") }) }) .. + body = audience("sp") .. "" }) }) .. assertion({ id = "a2", name_id = "second@example.com", confirmations = confirmation({ recipient = ACS }) }))) local doc, err = parse(xml) @@ -642,14 +642,15 @@ response from IdP is addressed to http://evil.example.com/acs ngx.say(a.id, " conditions=", tostring(a.has_conditions), " expires=", tostring(a.not_on_or_after), " audiences=", #a.audience_restrictions, - " confirmations=", #a.subject_confirmations) + " confirmations=", #a.subject_confirmations, + " one_time_use=", tostring(a.one_time_use)) end ngx.say("destination: ", tostring(saml.doc_destination(doc))) } } --- response_body -a1 conditions=true expires=2026-07-21T00:00:00Z audiences=1 confirmations=0 -a2 conditions=false expires=nil audiences=0 confirmations=1 +a1 conditions=true expires=2026-07-21T00:00:00Z audiences=1 confirmations=0 one_time_use=true +a2 conditions=false expires=nil audiences=0 confirmations=1 one_time_use=false destination: nil @@ -1390,26 +1391,33 @@ dated one decides: true -=== TEST 48: OneTimeUse is met by the replay record where there is one +=== TEST 48: with a record, an OneTimeUse assertion is treated like any other --- config location /t { content_by_lua_block { ngx.shared.saml_replay:flush_all() local xml = saml_response({ - id = "single", conditions = conditions({ body = "" }), + id = "stamped", + conditions = conditions({ not_on_or_after = at(600), body = "" }), }) ngx.say(login_with("replay", xml)) + -- remembered until acceptance ends plus clock_skew, as any other + local ttl = ngx.shared.saml_replay:ttl(replay_key("stamped")) + ngx.say("recorded: ", ttl > 650 and ttl <= 660) ngx.say(login_with("replay", xml)) } } --- response_body 302 / +recorded: true 401 nil --- error_log -assertion single has been presented already +assertion stamped has been presented already --- no_error_log [crit] -cannot enforce without replay_dict +[alert] +[emerg] +OneTimeUse From 3884513100a80bdf70bd60e7d423cfbb1467e091 Mon Sep 17 00:00:00 2001 From: Abhishek Choudhary Date: Thu, 27 Aug 2026 18:47:50 +0545 Subject: [PATCH 09/29] build: rebuild both objects when a header changes Neither object rule listed the headers, so a struct change rebuilt only the object whose .c was touched and linked it against the other, stale one. --- Makefile | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Makefile b/Makefile index 7a7cdb6..a5b8f05 100644 --- a/Makefile +++ b/Makefile @@ -37,10 +37,10 @@ test: build deps/ clean: rm -rf *.so *.o xmlsec1-$(XMLSEC_VER)* -saml.o: src/*.c +saml.o: src/*.c src/*.h $(CC) -c $(CFLAGS_ALL) -o saml.o src/saml.c -lua_saml.o: src/lua_saml.c +lua_saml.o: src/lua_saml.c src/*.h $(CC) -c $(CFLAGS_ALL) -I$(LUA_INCDIR) -Isrc/ -o $@ $< saml.so: lua_saml.o saml.o From fe7774e198ecb44e039e3b06e86a8086922d9d75 Mon Sep 17 00:00:00 2001 From: Abhishek Choudhary Date: Mon, 31 Aug 2026 09:51:29 +0545 Subject: [PATCH 10/29] fix: weigh the OneTimeUse record against the cap before the clamp A window of exactly the cap read as capped and warned that the assertion outlives a record that covers it to the second. --- lua/resty/saml.lua | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/lua/resty/saml.lua b/lua/resty/saml.lua index 541240e..ce9354d 100644 --- a/lua/resty/saml.lua +++ b/lua/resty/saml.lua @@ -611,8 +611,10 @@ local function spend_assertions(dict, opts, assertions, expected, now) -- the record is bounded where acceptance is not, so past it the -- assertion is accepted again. An IdP that asked for single use is - -- told, since it is the IdP's window that made the record fall short - if assertion.one_time_use and (usable_until == nil or ttl == MAX_REPLAY_TTL) then + -- told, since it is the IdP's window that made the record fall short. + -- Weighed before the clamp: a window of exactly the cap is covered + local outlives = usable_until == nil or usable_until + skew - now > MAX_REPLAY_TTL + if assertion.one_time_use and outlives then ngx.log(ngx.WARN, "assertion ", loggable(assertion.id), " carries OneTimeUse but stays acceptable past its record, which lapses in ", ttl, " seconds") From 9fcf54f7a840481bff8cb9d8f12b12f94328ab9a Mon Sep 17 00:00:00 2001 From: Abhishek Choudhary Date: Mon, 31 Aug 2026 12:08:05 +0545 Subject: [PATCH 11/29] fix: say an OneTimeUse record falls short only once it stands The warn ran before safe_add, so it described a record the add could still refuse for room, one already holding against a replay, or one the multi-assertion rollback was about to take back. The messages are gathered per record written and said once every one of them stands; a refused response says nothing. --- lua/resty/saml.lua | 16 +++++++++++----- t/assertion-conditions.t | 5 +++++ 2 files changed, 16 insertions(+), 5 deletions(-) diff --git a/lua/resty/saml.lua b/lua/resty/saml.lua index ce9354d..ce0c2bc 100644 --- a/lua/resty/saml.lua +++ b/lua/resty/saml.lua @@ -592,6 +592,7 @@ end local function spend_assertions(dict, opts, assertions, expected, now) local skew = opts.clock_skew or DEFAULT_CLOCK_SKEW local spent = {} + local warned = {} for _, assertion in ipairs(assertions) do if not assertion.id then @@ -614,16 +615,16 @@ local function spend_assertions(dict, opts, assertions, expected, now) -- told, since it is the IdP's window that made the record fall short. -- Weighed before the clamp: a window of exactly the cap is covered local outlives = usable_until == nil or usable_until + skew - now > MAX_REPLAY_TTL - if assertion.one_time_use and outlives then - ngx.log(ngx.WARN, "assertion ", loggable(assertion.id), - " carries OneTimeUse but stays acceptable past its record, which lapses in ", - ttl, " seconds") - end local key = replay_key(opts, assertion) local added, add_err = dict:safe_add(key, true, ttl) if added then spent[#spent + 1] = key + if assertion.one_time_use and outlives then + warned[#warned + 1] = "assertion " .. loggable(assertion.id) .. + " carries OneTimeUse but stays acceptable past its record" .. + ", which lapses in " .. ttl .. " seconds" + end elseif add_err == "exists" then -- this response authenticates nobody, so the assertions already -- taken from it are handed back rather than left spent @@ -639,6 +640,11 @@ local function spend_assertions(dict, opts, assertions, expected, now) end end + -- said only once every record stands: sooner would describe a record the + -- add may yet refuse, or one the rollback above takes back + for _, message in ipairs(warned) do + ngx.log(ngx.WARN, message) + end return true end diff --git a/t/assertion-conditions.t b/t/assertion-conditions.t index da06df1..3dfcb71 100644 --- a/t/assertion-conditions.t +++ b/t/assertion-conditions.t @@ -1450,6 +1450,11 @@ OneTimeUse 302 / --- error_log in saml_replay_full: no memory, this login is not covered by replay tracking though it carries OneTimeUse +--- no_error_log +[crit] +[alert] +[emerg] +stays acceptable past its record From c3d5bc5fad65e7b3746f1e65eddf36a015be97c2 Mon Sep 17 00:00:00 2001 From: Abhishek Choudhary Date: Mon, 31 Aug 2026 12:08:05 +0545 Subject: [PATCH 12/29] test: pin that an unstamped assertion never draws the OneTimeUse warn The day-capped and fallback fixtures now assert the warn's absence, so widening its gate past one_time_use fails the suite. --- t/assertion-conditions.t | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/t/assertion-conditions.t b/t/assertion-conditions.t index 3dfcb71..c81512c 100644 --- a/t/assertion-conditions.t +++ b/t/assertion-conditions.t @@ -1156,6 +1156,9 @@ configured: true --- response_body 302 / capped: true +--- no_error_log +[error] +stays acceptable past its record === TEST 40: a full dict leaves the login working and says so @@ -1335,6 +1338,9 @@ tracked: true --- response_body 302 / fallback: true +--- no_error_log +[error] +stays acceptable past its record === TEST 46: acceptance ends at whichever close comes first From 672ccd721b10bc8f359bc9fe2af08b6e635f9c08 Mon Sep 17 00:00:00 2001 From: Abhishek Choudhary Date: Mon, 31 Aug 2026 12:08:05 +0545 Subject: [PATCH 13/29] test: pin that the full-dict line names OneTimeUse only when carried TEST 40's substring is a prefix of the stamped variant, so an unconditional suffix passed; the block now asserts the suffix's absence. --- t/assertion-conditions.t | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/t/assertion-conditions.t b/t/assertion-conditions.t index c81512c..5964a98 100644 --- a/t/assertion-conditions.t +++ b/t/assertion-conditions.t @@ -1192,6 +1192,11 @@ full: true 302 / --- error_log in saml_replay_full: no memory, this login is not covered by replay tracking +--- no_error_log +[crit] +[alert] +[emerg] +though it carries OneTimeUse === TEST 41: a login refused after the checks leaves the assertion unspent From 2c483e525aac2cba84782073a1f556f65a8cfe32 Mon Sep 17 00:00:00 2001 From: Abhishek Choudhary Date: Mon, 31 Aug 2026 14:59:44 +0545 Subject: [PATCH 14/29] fix: name the zone from new() in the full-dict line, and pin fail-open The ERR printed the caller's table field, so a table mutated after construction could name a zone the record was never written to; the name is now kept beside the handle it was resolved with. TEST 49 builds its SP from a table it then mutates, and presents the response twice: with no room the login fails open both times, a decision rather than an accident. TEST 53 pins the other direction, enforcement standing on the resolved handle after the table drops the option. --- lua/resty/saml.lua | 11 +++--- t/assertion-conditions.t | 73 ++++++++++++++++++++++++++++++++++++++-- 2 files changed, 78 insertions(+), 6 deletions(-) diff --git a/lua/resty/saml.lua b/lua/resty/saml.lua index ce0c2bc..7828aab 100644 --- a/lua/resty/saml.lua +++ b/lua/resty/saml.lua @@ -589,7 +589,7 @@ end -- protecting somebody else's login, which is what add would do on its own: the -- entry it takes belongs to another user, the login it stops protecting is -- theirs, and the warning is reported against whoever needed the space. -local function spend_assertions(dict, opts, assertions, expected, now) +local function spend_assertions(dict, dict_name, opts, assertions, expected, now) local skew = opts.clock_skew or DEFAULT_CLOCK_SKEW local spent = {} local warned = {} @@ -634,7 +634,7 @@ local function spend_assertions(dict, opts, assertions, expected, now) return false, "assertion " .. assertion.id .. " has been presented already" else ngx.log(ngx.ERR, "could not remember assertion ", loggable(assertion.id), " in ", - opts.replay_dict, ": ", add_err, + dict_name, ": ", add_err, ", this login is not covered by replay tracking", assertion.one_time_use and " though it carries OneTimeUse" or "") end @@ -777,8 +777,8 @@ local function login_callback(self, opts) -- the last gate: everything that can still refuse this login has run, so -- the assertion is spent only where it actually authenticates somebody if self.replay_dict then - local unused, used_reason = spend_assertions(self.replay_dict, opts, assertions, - expected, now) + local unused, used_reason = spend_assertions(self.replay_dict, self.replay_dict_name, + opts, assertions, expected, now) if not unused then ngx.log(ngx.ERR, "response from IdP rejected: ", loggable(used_reason)) ngx.exit(ngx.HTTP_UNAUTHORIZED) @@ -973,6 +973,9 @@ function _M.new(opts) if obj.replay_dict == nil then error("no lua_shared_dict named " .. opts.replay_dict, 2) end + -- kept beside the handle, so what the ERR names is the zone written + -- to, whatever happens to the caller's table afterwards + obj.replay_dict_name = opts.replay_dict -- it is half the key, and tostring would turn a missing one into the -- literal nil that two deployments would then share if type(opts.sp_issuer) ~= "string" then diff --git a/t/assertion-conditions.t b/t/assertion-conditions.t index 5964a98..c7d5076 100644 --- a/t/assertion-conditions.t +++ b/t/assertion-conditions.t @@ -116,6 +116,34 @@ GnHKA3uj9HpsS6fAxHNPPvWxRjO67Xj8Yw== } SPS = {} + FULL_REBOUND_OPTS = { + sp_issuer = "sp", + idp_uri = "http://127.0.0.1:1984/idp", + login_callback_uri = "/acs", + logout_uri = "/logout", + logout_callback_uri = "/sls", + logout_redirect_uri = "/logout_ok", + sp_cert = CERT_PEM, + sp_private_key = KEY_PEM, + idp_cert = CERT_PEM, + secret = "very-secret-key-that-is-32-byte!", + replay_dict = "saml_replay_full", + } + + REBOUND_OPTS = { + sp_issuer = "sp", + idp_uri = "http://127.0.0.1:1984/idp", + login_callback_uri = "/acs", + logout_uri = "/logout", + logout_callback_uri = "/sls", + logout_redirect_uri = "/logout_ok", + sp_cert = CERT_PEM, + sp_private_key = KEY_PEM, + idp_cert = CERT_PEM, + secret = "very-secret-key-that-is-32-byte!", + replay_dict = "saml_replay", + } + function sp(name) if SPS[name] == nil then local opts = { @@ -1451,14 +1479,24 @@ OneTimeUse j = j + 1 if j > 5000 then break end end - ngx.say(login_with("replay_full", saml_response({ + -- an SP of this block's own, so the table handed to new() can + -- be mutated under it the way a live plugin conf could be + SPS["full-rebound"] = require("resty.saml").new(FULL_REBOUND_OPTS) + FULL_REBOUND_OPTS.replay_dict = "renamed-away" + + -- twice: with no room the login fails open, both times, and the + -- ERR names the zone the record would have gone to + local xml = saml_response({ id = "untracked-stamped", conditions = conditions({ body = "" }), - }))) + }) + ngx.say(login_with("full-rebound", xml)) + ngx.say(login_with("full-rebound", xml)) } } --- response_body 302 / +302 / --- error_log in saml_replay_full: no memory, this login is not covered by replay tracking though it carries OneTimeUse --- no_error_log @@ -1536,3 +1574,34 @@ one_time_use=true unknown_condition=Condition 302 / --- error_log eval qr/\[warn\] .* assertion stamped-untracked carries OneTimeUse, which this SP cannot enforce without replay_dict/ + + + +=== TEST 53: the configuration read at new() governs, whatever the table does later +--- config + location /t { + content_by_lua_block { + ngx.shared.saml_replay:flush_all() + SPS["rebound"] = require("resty.saml").new(REBOUND_OPTS) + REBOUND_OPTS.replay_dict = nil + + -- tracking still stands on the resolved handle: no warn about a + -- missing replay_dict, and the second presentation is refused + local xml = saml_response({ + id = "rebound", + conditions = conditions({ not_on_or_after = at(600), body = "" }), + }) + ngx.say(login_with("rebound", xml)) + ngx.say(login_with("rebound", xml)) + } + } +--- response_body +302 / +401 nil +--- error_log +assertion rebound has been presented already +--- no_error_log +[crit] +[alert] +[emerg] +cannot enforce without replay_dict From 193da86199afb970a02918c0ee9aa1cfbd05b56b Mon Sep 17 00:00:00 2001 From: Abhishek Choudhary Date: Mon, 31 Aug 2026 16:05:15 +0545 Subject: [PATCH 15/29] refactor: answer OneTimeUse presence on its own line The flag came from inside the refusal scan, which cost the scan its break and grew two guards no test can reach. assertion_child answers presence in one line, and the scan reverts to finding the first unknown and stopping. --- src/xml.c | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/src/xml.c b/src/xml.c index 6483c98..2822672 100644 --- a/src/xml.c +++ b/src/xml.c @@ -510,19 +510,19 @@ static int read_assertion(xmlDoc* doc, xmlNode* node, saml_assertion_t* a) { return -1; } + // answered on its own, so the refusal scan below owes it nothing and + // reads the same whatever the order of the conditions + a->one_time_use = assertion_child(conditions, "OneTimeUse") != NULL; + for (xmlNode* child = conditions->children; child != NULL; child = child->next) { - if (is_assertion_el(child, "OneTimeUse")) { - a->one_time_use = 1; - } - if (child->type == XML_ELEMENT_NODE && !is_known_condition(child) && - a->unknown_condition == NULL) { + if (child->type == XML_ELEMENT_NODE && !is_known_condition(child)) { // the caller refuses the assertion on this name, so losing it would - // let the condition through rather than fail the read. The scan goes - // on so what else the assertion carries is read whatever the order + // let the condition through rather than fail the read a->unknown_condition = xmlStrdup(child->name); if (a->unknown_condition == NULL) { return -1; } + break; } } From 44e50348c7f61a23aa1102739343c9f6a75cecf3 Mon Sep 17 00:00:00 2001 From: Abhishek Choudhary Date: Mon, 31 Aug 2026 16:05:15 +0545 Subject: [PATCH 16/29] test: carry the log guards in the two OneTimeUse warn blocks Declaring error_log swaps the injected no_error_log for crit/alert/emerg, so the two blocks driving only successful logins stopped asserting that no error is logged. Both now carry the full list. --- t/assertion-conditions.t | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/t/assertion-conditions.t b/t/assertion-conditions.t index c7d5076..f1ddb07 100644 --- a/t/assertion-conditions.t +++ b/t/assertion-conditions.t @@ -1531,6 +1531,11 @@ stays acceptable past its record --- error_log eval [qr/\[warn\] .* assertion stamped-unbounded carries OneTimeUse but stays acceptable past its record, which lapses in 600 seconds/, qr/\[warn\] .* assertion stamped-forever carries OneTimeUse but stays acceptable past its record, which lapses in 86400 seconds/] +--- no_error_log +[error] +[crit] +[alert] +[emerg] @@ -1574,6 +1579,11 @@ one_time_use=true unknown_condition=Condition 302 / --- error_log eval qr/\[warn\] .* assertion stamped-untracked carries OneTimeUse, which this SP cannot enforce without replay_dict/ +--- no_error_log +[error] +[crit] +[alert] +[emerg] From b935eaf2ad5af6cdd63db3804f9cbaa5ff9dda86 Mon Sep 17 00:00:00 2001 From: Abhishek Choudhary Date: Mon, 31 Aug 2026 16:05:15 +0545 Subject: [PATCH 17/29] docs: carry the OneTimeUse diagnostics through the record's bounds The full-zone paragraph says the error names OneTimeUse, the bounds paragraph documents the warn a short record draws, and the single-use claim points at the zone with no room too. --- README.md | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/README.md b/README.md index a5d1125..54a116d 100644 --- a/README.md +++ b/README.md @@ -128,15 +128,17 @@ long as that assertion could still be used. A response normally carries one, so taking ten logins a second against an IdP issuing ten-minute assertions holds around six thousand entries at once: `1m` is too small for that and a busy deployment wants more. A zone with no room leaves that assertion untracked and logs an error naming -the assertion and the zone, rather than evicting an entry that is still protecting -somebody else. A response carrying several assertions can end up partly tracked, +the assertion and the zone, saying too when that assertion carried `OneTimeUse`, +rather than evicting an entry that is still protecting somebody else. A response carrying several assertions can end up partly tracked, which is the safe direction: a later replay still collides on whichever of them was recorded. **The record is bounded even where acceptance is not.** An assertion with no usable expiry is remembered for `replay_ttl` and accepted for good, so it is refusable only inside that window; one the IdP made valid beyond a day is remembered for the day -and accepted again past it. Both need an IdP far outside shipped defaults, where +and accepted again past it. Where either happens to an assertion carrying +``, the login says so at `warn` level, since the single use its +IdP asked for ends with the record. Both need an IdP far outside shipped defaults, where the delivery window is minutes and the assertion window at most an hour, and the alternative is a record nothing reclaims. The limit an operator can move is `replay_ttl`; the day cap is fixed. @@ -145,7 +147,7 @@ alternative is a record nothing reclaims. The limit an operator can move is assertion to ask the SP to keep exactly this record. SAML Core 2.5.1.5 makes the condition always valid, a condition on use rather than on validity, so the login goes through with or without the option. With it, the assertion is single-use within the -bounds above. Without it, the login is accepted and a line at `warn` level names `replay_dict`, +bounds above, the zone with no room included. Without it, the login is accepted and a line at `warn` level names `replay_dict`, so an IdP that asks for this is the signal to set it; a deployment logging at `error` or above does not see it. From 8dd5b26f7b8cebd3d8d66b9f9b74d59ad57dd1d4 Mon Sep 17 00:00:00 2001 From: Abhishek Choudhary Date: Mon, 31 Aug 2026 16:05:15 +0545 Subject: [PATCH 18/29] build: relink when the Makefile or the xmlsec archives change Switching OPENSSL_DIR compiled nothing and touching the static archives relinked nothing. The link recipe names its objects, since $^ would feed the archives to the linker twice. --- Makefile | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/Makefile b/Makefile index a5b8f05..b855716 100644 --- a/Makefile +++ b/Makefile @@ -37,14 +37,14 @@ test: build deps/ clean: rm -rf *.so *.o xmlsec1-$(XMLSEC_VER)* -saml.o: src/*.c src/*.h +saml.o: src/*.c src/*.h Makefile $(CC) -c $(CFLAGS_ALL) -o saml.o src/saml.c -lua_saml.o: src/lua_saml.c src/*.h +lua_saml.o: src/lua_saml.c src/*.h Makefile $(CC) -c $(CFLAGS_ALL) -I$(LUA_INCDIR) -Isrc/ -o $@ $< -saml.so: lua_saml.o saml.o - $(CC) -o $@ $^ $(LDFLAGS_ALL) +saml.so: lua_saml.o saml.o $(XMLSEC1_STATIC_LIBS) Makefile + $(CC) -o $@ lua_saml.o saml.o $(LDFLAGS_ALL) ### install: Install the library to runtime .PHONY: install From 21e0d9a218f7f0b88f82e585467b4686a9d50cfc Mon Sep 17 00:00:00 2001 From: Abhishek Choudhary Date: Mon, 31 Aug 2026 16:54:51 +0545 Subject: [PATCH 19/29] fix: claim only what the failed add knows, and name the issuer The no-memory line spoke for the whole login, which is false whenever a sibling assertion was recorded. It now says the one thing the branch has established, this assertion is not tracked, and names the issuer, since an ID is unique only within the IdP that minted it. --- lua/resty/saml.lua | 6 +++--- t/assertion-conditions.t | 4 ++-- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/lua/resty/saml.lua b/lua/resty/saml.lua index 7828aab..736ca8e 100644 --- a/lua/resty/saml.lua +++ b/lua/resty/saml.lua @@ -633,9 +633,9 @@ local function spend_assertions(dict, dict_name, opts, assertions, expected, now end return false, "assertion " .. assertion.id .. " has been presented already" else - ngx.log(ngx.ERR, "could not remember assertion ", loggable(assertion.id), " in ", - dict_name, ": ", add_err, - ", this login is not covered by replay tracking", + ngx.log(ngx.ERR, "could not remember assertion ", loggable(assertion.id), + " from ", loggable(assertion.issuer), " in ", dict_name, ": ", add_err, + ", this assertion is not tracked", assertion.one_time_use and " though it carries OneTimeUse" or "") end end diff --git a/t/assertion-conditions.t b/t/assertion-conditions.t index f1ddb07..f79155e 100644 --- a/t/assertion-conditions.t +++ b/t/assertion-conditions.t @@ -1219,7 +1219,7 @@ stays acceptable past its record full: true 302 / --- error_log -in saml_replay_full: no memory, this login is not covered by replay tracking +assertion untracked from https://idp.example.com in saml_replay_full: no memory, this assertion is not tracked --- no_error_log [crit] [alert] @@ -1498,7 +1498,7 @@ OneTimeUse 302 / 302 / --- error_log -in saml_replay_full: no memory, this login is not covered by replay tracking though it carries OneTimeUse +assertion untracked-stamped from https://idp.example.com in saml_replay_full: no memory, this assertion is not tracked though it carries OneTimeUse --- no_error_log [crit] [alert] From 57bb15f9d89528f03c8dcb3115d6ceeede40c48b Mon Sep 17 00:00:00 2001 From: Abhishek Choudhary Date: Mon, 31 Aug 2026 17:22:36 +0545 Subject: [PATCH 20/29] refactor: buffer the facts of the deferred warn, and name the issuer The buffered line was the file's one concat-built log message, paid for even where the level discards it; the buffer now holds id and ttl and the flush hands ngx.log the pieces. The table is made only when something is buffered, and both warns name the issuer beside the ID, since an ID is unique only within the IdP that minted it. --- lua/resty/saml.lua | 19 ++++++++++++------- t/assertion-conditions.t | 8 ++++---- 2 files changed, 16 insertions(+), 11 deletions(-) diff --git a/lua/resty/saml.lua b/lua/resty/saml.lua index 736ca8e..5c8061b 100644 --- a/lua/resty/saml.lua +++ b/lua/resty/saml.lua @@ -426,6 +426,7 @@ local function assertions_acceptable(opts, assertions, expected, now, replay_dic -- one the last gate enforces on, so the two cannot disagree if assertion.one_time_use and not replay_dict then ngx.log(ngx.WARN, "assertion ", loggable(assertion.id), + " from ", loggable(assertion.issuer), " carries OneTimeUse, which this SP cannot enforce without replay_dict") end @@ -592,7 +593,7 @@ end local function spend_assertions(dict, dict_name, opts, assertions, expected, now) local skew = opts.clock_skew or DEFAULT_CLOCK_SKEW local spent = {} - local warned = {} + local warned for _, assertion in ipairs(assertions) do if not assertion.id then @@ -621,9 +622,8 @@ local function spend_assertions(dict, dict_name, opts, assertions, expected, now if added then spent[#spent + 1] = key if assertion.one_time_use and outlives then - warned[#warned + 1] = "assertion " .. loggable(assertion.id) .. - " carries OneTimeUse but stays acceptable past its record" .. - ", which lapses in " .. ttl .. " seconds" + warned = warned or {} + warned[#warned + 1] = { id = assertion.id, issuer = assertion.issuer, ttl = ttl } end elseif add_err == "exists" then -- this response authenticates nobody, so the assertions already @@ -641,9 +641,14 @@ local function spend_assertions(dict, dict_name, opts, assertions, expected, now end -- said only once every record stands: sooner would describe a record the - -- add may yet refuse, or one the rollback above takes back - for _, message in ipairs(warned) do - ngx.log(ngx.WARN, message) + -- add may yet refuse, or one the rollback above takes back. The facts are + -- what was kept, so the line is built only where the level prints it + if warned then + for _, w in ipairs(warned) do + ngx.log(ngx.WARN, "assertion ", loggable(w.id), " from ", loggable(w.issuer), + " carries OneTimeUse but stays acceptable past its record, which lapses in ", + w.ttl, " seconds") + end end return true end diff --git a/t/assertion-conditions.t b/t/assertion-conditions.t index f79155e..b7cf1ad 100644 --- a/t/assertion-conditions.t +++ b/t/assertion-conditions.t @@ -621,7 +621,7 @@ offers no subject confirmation this SP can satisfy 302 / 401 nil --- error_log eval -[qr/\[warn\] .* assertion single carries OneTimeUse, which this SP cannot enforce without replay_dict/, +[qr/\[warn\] .* assertion single from https:\/\/idp\.example\.com carries OneTimeUse, which this SP cannot enforce without replay_dict/, qr/carries a condition this SP cannot satisfy: Condition/] @@ -1529,8 +1529,8 @@ stays acceptable past its record 302 / 302 / --- error_log eval -[qr/\[warn\] .* assertion stamped-unbounded carries OneTimeUse but stays acceptable past its record, which lapses in 600 seconds/, -qr/\[warn\] .* assertion stamped-forever carries OneTimeUse but stays acceptable past its record, which lapses in 86400 seconds/] +[qr/\[warn\] .* assertion stamped-unbounded from https:\/\/idp\.example\.com carries OneTimeUse but stays acceptable past its record, which lapses in 600 seconds/, +qr/\[warn\] .* assertion stamped-forever from https:\/\/idp\.example\.com carries OneTimeUse but stays acceptable past its record, which lapses in 86400 seconds/] --- no_error_log [error] [crit] @@ -1578,7 +1578,7 @@ one_time_use=true unknown_condition=Condition 302 / 302 / --- error_log eval -qr/\[warn\] .* assertion stamped-untracked carries OneTimeUse, which this SP cannot enforce without replay_dict/ +qr/\[warn\] .* assertion stamped-untracked from https:\/\/idp\.example\.com carries OneTimeUse, which this SP cannot enforce without replay_dict/ --- no_error_log [error] [crit] From 6de2bf8133307fdd67882e7ea938eb3382b93606 Mon Sep 17 00:00:00 2001 From: Abhishek Choudhary Date: Mon, 31 Aug 2026 17:30:00 +0545 Subject: [PATCH 21/29] test: anchor the fail-open line's severity Both blocks matched a bare substring, so dropping the line to warn passed green while the README calls the level a contract. The patterns now anchor [error] the way the warn blocks anchor [warn]. --- t/assertion-conditions.t | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/t/assertion-conditions.t b/t/assertion-conditions.t index b7cf1ad..81d2d3a 100644 --- a/t/assertion-conditions.t +++ b/t/assertion-conditions.t @@ -1218,8 +1218,8 @@ stays acceptable past its record --- response_body full: true 302 / ---- error_log -assertion untracked from https://idp.example.com in saml_replay_full: no memory, this assertion is not tracked +--- error_log eval +qr/\[error\] .* assertion untracked from https:\/\/idp\.example\.com in saml_replay_full: no memory, this assertion is not tracked/ --- no_error_log [crit] [alert] @@ -1497,8 +1497,8 @@ OneTimeUse --- response_body 302 / 302 / ---- error_log -assertion untracked-stamped from https://idp.example.com in saml_replay_full: no memory, this assertion is not tracked though it carries OneTimeUse +--- error_log eval +qr/\[error\] .* assertion untracked-stamped from https:\/\/idp\.example\.com in saml_replay_full: no memory, this assertion is not tracked though it carries OneTimeUse/ --- no_error_log [crit] [alert] From 603b046e68f1f11a9c45b831b384d3612d03d385 Mon Sep 17 00:00:00 2001 From: Abhishek Choudhary Date: Mon, 31 Aug 2026 17:35:09 +0545 Subject: [PATCH 22/29] test: allow the standard slack on the stamped record's TTL The 10-second window flaked on a slow runner and read as a computation bug; over 600 already proves clock_skew was added, like TEST 34 on the same fixture. --- t/assertion-conditions.t | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/t/assertion-conditions.t b/t/assertion-conditions.t index 81d2d3a..f2ef5df 100644 --- a/t/assertion-conditions.t +++ b/t/assertion-conditions.t @@ -1442,7 +1442,7 @@ dated one decides: true ngx.say(login_with("replay", xml)) -- remembered until acceptance ends plus clock_skew, as any other local ttl = ngx.shared.saml_replay:ttl(replay_key("stamped")) - ngx.say("recorded: ", ttl > 650 and ttl <= 660) + ngx.say("recorded: ", ttl > 600 and ttl <= 660) ngx.say(login_with("replay", xml)) } } From 30f4b178148e70e509c764bfbbcea178c7203ebb Mon Sep 17 00:00:00 2001 From: Abhishek Choudhary Date: Mon, 31 Aug 2026 17:46:29 +0545 Subject: [PATCH 23/29] test: fill the zone through one helper that vouches it is full TEST 49 copied TEST 40's fill loops minus the check, so a fill that stops working would read as a behaviour failure. Both blocks now assert fill_dict's answer before driving a login. --- t/assertion-conditions.t | 56 ++++++++++++++++++---------------------- 1 file changed, 25 insertions(+), 31 deletions(-) diff --git a/t/assertion-conditions.t b/t/assertion-conditions.t index f2ef5df..f3196b2 100644 --- a/t/assertion-conditions.t +++ b/t/assertion-conditions.t @@ -265,6 +265,27 @@ GnHKA3uj9HpsS6fAxHNPPvWxRjO67Xj8Yw== -- the module owns this layout; naming it once here keeps a change to -- the scheme from surfacing as a comparison against nil + -- fill a zone to refusal, so the next safe_add answers no memory. + -- Hands back whether it truly got there, for the block to assert + function fill_dict(name) + local dict = ngx.shared[name] + dict:flush_all() + dict:flush_expired() + local filler = string.rep("x", 256) + local i, ok, err = 0, true, nil + while ok do + ok, err = dict:safe_set("filler-" .. i, filler, 600) + if ok then i = i + 1 end + if i > 5000 then break end + end + local j = 0 + while dict:safe_add("small-" .. j, true, 600) do + j = j + 1 + if j > 5000 then break end + end + return i > 0 and j > 0 and err == "no memory" + end + function replay_key(id, issuer) return "sp|" .. (issuer or IDP) .. "|" .. id end @@ -1193,22 +1214,7 @@ stays acceptable past its record --- config location /t { content_by_lua_block { - local dict = ngx.shared.saml_replay_full - dict:flush_all() - dict:flush_expired() - local filler = string.rep("x", 256) - local i, ok, err = 0, true, nil - while ok do - ok, err = dict:safe_set("filler-" .. i, filler, 600) - if ok then i = i + 1 end - if i > 5000 then break end - end - local j = 0 - while dict:safe_add("small-" .. j, true, 600) do - j = j + 1 - if j > 5000 then break end - end - ngx.say("full: ", i > 0 and j > 0 and err == "no memory") + ngx.say("full: ", fill_dict("saml_replay_full")) -- evicting would take the record away from whoever holds it and -- report it against this request, so this login goes untracked @@ -1464,21 +1470,8 @@ OneTimeUse --- config location /t { content_by_lua_block { - local dict = ngx.shared.saml_replay_full - dict:flush_all() - dict:flush_expired() - local filler = string.rep("x", 256) - local i, ok = 0, true - while ok do - ok = dict:safe_set("filler-" .. i, filler, 600) - if ok then i = i + 1 end - if i > 5000 then break end - end - local j = 0 - while dict:safe_add("small-" .. j, true, 600) do - j = j + 1 - if j > 5000 then break end - end + ngx.say("full: ", fill_dict("saml_replay_full")) + -- an SP of this block's own, so the table handed to new() can -- be mutated under it the way a live plugin conf could be SPS["full-rebound"] = require("resty.saml").new(FULL_REBOUND_OPTS) @@ -1495,6 +1488,7 @@ OneTimeUse } } --- response_body +full: true 302 / 302 / --- error_log eval From ca97738a07178119b04c42f0a81b699cd4b80926 Mon Sep 17 00:00:00 2001 From: Abhishek Choudhary Date: Mon, 31 Aug 2026 18:03:45 +0545 Subject: [PATCH 24/29] docs: state the opts handover contract, and claim less for the kept name new() keeps the table by reference, which is now the documented contract: hand it over and do not mutate it, embedders with shared tables pass a copy. The comment beside replay_dict_name says why the name is kept at all, the handle has no accessor, rather than promising immunity the object does not have. --- README.md | 4 ++++ lua/resty/saml.lua | 4 ++-- 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/README.md b/README.md index 54a116d..3507000 100644 --- a/README.md +++ b/README.md @@ -68,6 +68,10 @@ local saml = resty_saml.new(opts) `opts` is a table of below items: +`new` keeps `opts` by reference and reads it for the SP's whole life: hand the +table over and do not mutate it afterwards. An embedder whose configuration table +is shared or reused passes a copy (`core.table.deepcopy(conf)` in APISIX). + | key | type | default value | Description | | ----------- | ----------- | ----------- | ----------- | | `sp_issuer` | string | None | SP name to access IdP. | diff --git a/lua/resty/saml.lua b/lua/resty/saml.lua index 5c8061b..6146e87 100644 --- a/lua/resty/saml.lua +++ b/lua/resty/saml.lua @@ -978,8 +978,8 @@ function _M.new(opts) if obj.replay_dict == nil then error("no lua_shared_dict named " .. opts.replay_dict, 2) end - -- kept beside the handle, so what the ERR names is the zone written - -- to, whatever happens to the caller's table afterwards + -- the handle carries no name accessor, so the name it was resolved + -- from rides beside it for the diagnostics obj.replay_dict_name = opts.replay_dict -- it is half the key, and tostring would turn a missing one into the -- literal nil that two deployments would then share From e73436481b4aa9b0b1cc1630736b9dc47326d4f7 Mon Sep 17 00:00:00 2001 From: Abhishek Choudhary Date: Mon, 31 Aug 2026 18:17:51 +0545 Subject: [PATCH 25/29] fix: clamp only what the assertion claims, never replay_ttl The day cap flattened an operator's explicit replay_ttl to 86400 silently, while the README calls that option the limit an operator can move. The cap now binds the IdP-derived window alone; the fallback is taken as given, already validated a number at new(). --- lua/resty/saml.lua | 13 +++++++------ t/assertion-conditions.t | 19 +++++++++++++++++++ 2 files changed, 26 insertions(+), 6 deletions(-) diff --git a/lua/resty/saml.lua b/lua/resty/saml.lua index 6146e87..45309fa 100644 --- a/lua/resty/saml.lua +++ b/lua/resty/saml.lua @@ -325,7 +325,8 @@ local DEFAULT_REPLAY_TTL = 600 -- and how long any assertion is remembered at most, whatever it claims. An -- assertion valid for years would pin a slot the dict never reclaims, and --- nobody is still trying to complete that login a day later. +-- nobody is still trying to complete that login a day later. It bounds the +-- IdP's window, never replay_ttl: that one is the operator's own choice local MAX_REPLAY_TTL = 86400 local function time_bounds_ok(not_before, not_on_or_after, now, skew) @@ -604,11 +605,11 @@ local function spend_assertions(dict, dict_name, opts, assertions, expected, now local usable_until = last_moment_usable(assertion, expected) if usable_until then ttl = usable_until + skew - now - end - if ttl < 1 then - ttl = 1 - elseif ttl > MAX_REPLAY_TTL then - ttl = MAX_REPLAY_TTL + if ttl < 1 then + ttl = 1 + elseif ttl > MAX_REPLAY_TTL then + ttl = MAX_REPLAY_TTL + end end -- the record is bounded where acceptance is not, so past it the diff --git a/t/assertion-conditions.t b/t/assertion-conditions.t index f3196b2..3453165 100644 --- a/t/assertion-conditions.t +++ b/t/assertion-conditions.t @@ -108,6 +108,7 @@ GnHKA3uj9HpsS6fAxHNPPvWxRjO67Xj8Yw== acs = { sp_acs_url = "http://127.0.0.1:1984/acs" }, replay = { replay_dict = "saml_replay" }, replay_short = { replay_dict = "saml_replay", replay_ttl = 90 }, + replay_long = { replay_dict = "saml_replay", replay_ttl = 172800 }, replay_full = { replay_dict = "saml_replay_full" }, replay_pinned = { replay_dict = "saml_replay", @@ -1609,3 +1610,21 @@ assertion rebound has been presented already [alert] [emerg] cannot enforce without replay_dict + + + +=== TEST 54: the operator's replay_ttl is taken as given, past the day too +--- config + location /t { + content_by_lua_block { + ngx.shared.saml_replay:flush_all() + -- the day cap bounds what the assertion claims; this value is + -- nobody's claim but the operator's + ngx.say(login_with("replay_long", saml_response({ id = "kept-long" }))) + local ttl = ngx.shared.saml_replay:ttl(replay_key("kept-long")) + ngx.say("kept: ", ttl > 172700 and ttl <= 172800) + } + } +--- response_body +302 / +kept: true From e8f1dad781e0a5f224ce238668d83b2222be6e65 Mon Sep 17 00:00:00 2001 From: Abhishek Choudhary Date: Mon, 31 Aug 2026 18:24:04 +0545 Subject: [PATCH 26/29] test: retire the mutation blocks the handover contract obsoleted TEST 53 and TEST 49's local SP certified behaviour under a mutated opts table, which the README now places outside the contract; the enforcement TEST 53 also carried is TEST 48 verbatim. TEST 49 rides the shared SP again, no block mutates an init global, and the two copied option tables are gone. --- t/assertion-conditions.t | 71 ++-------------------------------------- 1 file changed, 3 insertions(+), 68 deletions(-) diff --git a/t/assertion-conditions.t b/t/assertion-conditions.t index 3453165..4e97c9a 100644 --- a/t/assertion-conditions.t +++ b/t/assertion-conditions.t @@ -117,34 +117,6 @@ GnHKA3uj9HpsS6fAxHNPPvWxRjO67Xj8Yw== } SPS = {} - FULL_REBOUND_OPTS = { - sp_issuer = "sp", - idp_uri = "http://127.0.0.1:1984/idp", - login_callback_uri = "/acs", - logout_uri = "/logout", - logout_callback_uri = "/sls", - logout_redirect_uri = "/logout_ok", - sp_cert = CERT_PEM, - sp_private_key = KEY_PEM, - idp_cert = CERT_PEM, - secret = "very-secret-key-that-is-32-byte!", - replay_dict = "saml_replay_full", - } - - REBOUND_OPTS = { - sp_issuer = "sp", - idp_uri = "http://127.0.0.1:1984/idp", - login_callback_uri = "/acs", - logout_uri = "/logout", - logout_callback_uri = "/sls", - logout_redirect_uri = "/logout_ok", - sp_cert = CERT_PEM, - sp_private_key = KEY_PEM, - idp_cert = CERT_PEM, - secret = "very-secret-key-that-is-32-byte!", - replay_dict = "saml_replay", - } - function sp(name) if SPS[name] == nil then local opts = { @@ -1473,19 +1445,13 @@ OneTimeUse content_by_lua_block { ngx.say("full: ", fill_dict("saml_replay_full")) - -- an SP of this block's own, so the table handed to new() can - -- be mutated under it the way a live plugin conf could be - SPS["full-rebound"] = require("resty.saml").new(FULL_REBOUND_OPTS) - FULL_REBOUND_OPTS.replay_dict = "renamed-away" - - -- twice: with no room the login fails open, both times, and the - -- ERR names the zone the record would have gone to + -- twice: with no room the login fails open, both times local xml = saml_response({ id = "untracked-stamped", conditions = conditions({ body = "" }), }) - ngx.say(login_with("full-rebound", xml)) - ngx.say(login_with("full-rebound", xml)) + ngx.say(login_with("replay_full", xml)) + ngx.say(login_with("replay_full", xml)) } } --- response_body @@ -1582,37 +1548,6 @@ qr/\[warn\] .* assertion stamped-untracked from https:\/\/idp\.example\.com carr -=== TEST 53: the configuration read at new() governs, whatever the table does later ---- config - location /t { - content_by_lua_block { - ngx.shared.saml_replay:flush_all() - SPS["rebound"] = require("resty.saml").new(REBOUND_OPTS) - REBOUND_OPTS.replay_dict = nil - - -- tracking still stands on the resolved handle: no warn about a - -- missing replay_dict, and the second presentation is refused - local xml = saml_response({ - id = "rebound", - conditions = conditions({ not_on_or_after = at(600), body = "" }), - }) - ngx.say(login_with("rebound", xml)) - ngx.say(login_with("rebound", xml)) - } - } ---- response_body -302 / -401 nil ---- error_log -assertion rebound has been presented already ---- no_error_log -[crit] -[alert] -[emerg] -cannot enforce without replay_dict - - - === TEST 54: the operator's replay_ttl is taken as given, past the day too --- config location /t { From 1668ce8ee318f7ff5d3a8c349656ac0d88d1077b Mon Sep 17 00:00:00 2001 From: Abhishek Choudhary Date: Mon, 31 Aug 2026 18:33:22 +0545 Subject: [PATCH 27/29] test: count the fail-open line, once per presentation error_log is presence-only, so a throttle reporting only the first occurrence passed; grep_error_log_out pins two. --- t/assertion-conditions.t | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/t/assertion-conditions.t b/t/assertion-conditions.t index 4e97c9a..77b21b6 100644 --- a/t/assertion-conditions.t +++ b/t/assertion-conditions.t @@ -1460,6 +1460,11 @@ full: true 302 / --- error_log eval qr/\[error\] .* assertion untracked-stamped from https:\/\/idp\.example\.com in saml_replay_full: no memory, this assertion is not tracked though it carries OneTimeUse/ +--- grep_error_log eval +qr/could not remember assertion untracked-stamped [^,]*, this assertion is not tracked/ +--- grep_error_log_out +could not remember assertion untracked-stamped from https://idp.example.com in saml_replay_full: no memory, this assertion is not tracked +could not remember assertion untracked-stamped from https://idp.example.com in saml_replay_full: no memory, this assertion is not tracked --- no_error_log [crit] [alert] From 934000bb3ef3b5dab71c56ca9b194ca89a4d4d96 Mon Sep 17 00:00:00 2001 From: Abhishek Choudhary Date: Mon, 31 Aug 2026 20:45:04 +0545 Subject: [PATCH 28/29] fix: state outlives as the property, carry the outcome, blank the nil issuer outlives now reads: the stored record falls short of the lifetime, so no revision of the clamp can leave it behind, and the exact-cap boundary stops being an operator to flip. The full-dict line ends with the one outcome its branch decides, the login is not refused for it. An empty Issuer prints as nothing rather than nil, matching replay_key. The flush comment keeps its one true clause, the rollback. --- lua/resty/saml.lua | 19 ++++++++++--------- t/assertion-conditions.t | 11 ++++++++--- 2 files changed, 18 insertions(+), 12 deletions(-) diff --git a/lua/resty/saml.lua b/lua/resty/saml.lua index 45309fa..a7695fe 100644 --- a/lua/resty/saml.lua +++ b/lua/resty/saml.lua @@ -427,7 +427,7 @@ local function assertions_acceptable(opts, assertions, expected, now, replay_dic -- one the last gate enforces on, so the two cannot disagree if assertion.one_time_use and not replay_dict then ngx.log(ngx.WARN, "assertion ", loggable(assertion.id), - " from ", loggable(assertion.issuer), + " from ", loggable(assertion.issuer or ""), " carries OneTimeUse, which this SP cannot enforce without replay_dict") end @@ -615,8 +615,9 @@ local function spend_assertions(dict, dict_name, opts, assertions, expected, now -- the record is bounded where acceptance is not, so past it the -- assertion is accepted again. An IdP that asked for single use is -- told, since it is the IdP's window that made the record fall short. - -- Weighed before the clamp: a window of exactly the cap is covered - local outlives = usable_until == nil or usable_until + skew - now > MAX_REPLAY_TTL + -- Stated as the property itself, the stored record falling short of + -- the lifetime, so no revision of the clamp can leave this line behind + local outlives = usable_until == nil or ttl < usable_until + skew - now local key = replay_key(opts, assertion) local added, add_err = dict:safe_add(key, true, ttl) @@ -635,18 +636,18 @@ local function spend_assertions(dict, dict_name, opts, assertions, expected, now return false, "assertion " .. assertion.id .. " has been presented already" else ngx.log(ngx.ERR, "could not remember assertion ", loggable(assertion.id), - " from ", loggable(assertion.issuer), " in ", dict_name, ": ", add_err, + " from ", loggable(assertion.issuer or ""), " in ", dict_name, ": ", add_err, ", this assertion is not tracked", - assertion.one_time_use and " though it carries OneTimeUse" or "") + assertion.one_time_use and " though it carries OneTimeUse" or "", + ", and the login is not refused for it") end end - -- said only once every record stands: sooner would describe a record the - -- add may yet refuse, or one the rollback above takes back. The facts are - -- what was kept, so the line is built only where the level prints it + -- said only once every record stands: a warn spoken sooner would describe + -- a record the rollback above may yet take back if warned then for _, w in ipairs(warned) do - ngx.log(ngx.WARN, "assertion ", loggable(w.id), " from ", loggable(w.issuer), + ngx.log(ngx.WARN, "assertion ", loggable(w.id), " from ", loggable(w.issuer or ""), " carries OneTimeUse but stays acceptable past its record, which lapses in ", w.ttl, " seconds") end diff --git a/t/assertion-conditions.t b/t/assertion-conditions.t index 77b21b6..96730ec 100644 --- a/t/assertion-conditions.t +++ b/t/assertion-conditions.t @@ -645,6 +645,9 @@ response from IdP is addressed to http://evil.example.com/acs } --- response_body 302 / +--- no_error_log +[error] +cannot enforce without replay_dict @@ -1198,7 +1201,7 @@ stays acceptable past its record full: true 302 / --- error_log eval -qr/\[error\] .* assertion untracked from https:\/\/idp\.example\.com in saml_replay_full: no memory, this assertion is not tracked/ +qr/\[error\] .* assertion untracked from https:\/\/idp\.example\.com in saml_replay_full: no memory, this assertion is not tracked, and the login is not refused for it/ --- no_error_log [crit] [alert] @@ -1459,7 +1462,7 @@ full: true 302 / 302 / --- error_log eval -qr/\[error\] .* assertion untracked-stamped from https:\/\/idp\.example\.com in saml_replay_full: no memory, this assertion is not tracked though it carries OneTimeUse/ +qr/\[error\] .* assertion untracked-stamped from https:\/\/idp\.example\.com in saml_replay_full: no memory, this assertion is not tracked though it carries OneTimeUse, and the login is not refused for it/ --- grep_error_log eval qr/could not remember assertion untracked-stamped [^,]*, this assertion is not tracked/ --- grep_error_log_out @@ -1511,7 +1514,8 @@ qr/\[warn\] .* assertion stamped-forever from https:\/\/idp\.example\.com carrie content_by_lua_block { local unknown = 'sp' - for _, body in ipairs({ "" .. unknown, unknown .. "" }) do + for _, body in ipairs({ "" .. unknown, unknown .. "", + unknown }) do local doc, err = parse(sign_doc(response(assertion({ id = "ordered", conditions = conditions({ body = body }), })))) @@ -1525,6 +1529,7 @@ qr/\[warn\] .* assertion stamped-forever from https:\/\/idp\.example\.com carrie --- response_body one_time_use=true unknown_condition=Condition one_time_use=true unknown_condition=Condition +one_time_use=false unknown_condition=Condition From 0678dbf4fb44ff9ca7fdf23af51c80669c2d83cd Mon Sep 17 00:00:00 2001 From: Abhishek Choudhary Date: Mon, 31 Aug 2026 20:45:04 +0545 Subject: [PATCH 29/29] build: retire the archive prerequisite It made make -B saml.so run the two-target download recipe once per archive, two wgets and two configures in one tree. Relink-on-archive-change was a nicety; make build remains the bootstrap goal. --- Makefile | 2 +- README.md | 30 ++++++++++++++++++++---------- 2 files changed, 21 insertions(+), 11 deletions(-) diff --git a/Makefile b/Makefile index b855716..2d8673b 100644 --- a/Makefile +++ b/Makefile @@ -43,7 +43,7 @@ saml.o: src/*.c src/*.h Makefile lua_saml.o: src/lua_saml.c src/*.h Makefile $(CC) -c $(CFLAGS_ALL) -I$(LUA_INCDIR) -Isrc/ -o $@ $< -saml.so: lua_saml.o saml.o $(XMLSEC1_STATIC_LIBS) Makefile +saml.so: lua_saml.o saml.o Makefile $(CC) -o $@ lua_saml.o saml.o $(LDFLAGS_ALL) ### install: Install the library to runtime diff --git a/README.md b/README.md index 3507000..5da0e5c 100644 --- a/README.md +++ b/README.md @@ -132,18 +132,21 @@ long as that assertion could still be used. A response normally carries one, so taking ten logins a second against an IdP issuing ten-minute assertions holds around six thousand entries at once: `1m` is too small for that and a busy deployment wants more. A zone with no room leaves that assertion untracked and logs an error naming -the assertion and the zone, saying too when that assertion carried `OneTimeUse`, -rather than evicting an entry that is still protecting somebody else. A response carrying several assertions can end up partly tracked, +the assertion, its issuer and the zone, saying too when it carried `OneTimeUse` +and that the login is not refused for it, rather than evicting an entry that is +still protecting somebody else. A response carrying several assertions can end up +partly tracked, which is the safe direction: a later replay still collides on whichever of them was recorded. **The record is bounded even where acceptance is not.** An assertion with no usable expiry is remembered for `replay_ttl` and accepted for good, so it is refusable only -inside that window; one the IdP made valid beyond a day is remembered for the day -and accepted again past it. Where either happens to an assertion carrying -``, the login says so at `warn` level, since the single use its -IdP asked for ends with the record. Both need an IdP far outside shipped defaults, where -the delivery window is minutes and the assertion window at most an hour, and the +inside that window; one still acceptable more than a day from now is remembered +for the day and accepted again past it. Where either happens to an assertion +carrying ``, the login says so at `warn` level, since the single +use its IdP asked for ends with the record. Both need an IdP far outside shipped +defaults, where the delivery window is minutes and the assertion window at most an +hour, and the alternative is a record nothing reclaims. The limit an operator can move is `replay_ttl`; the day cap is fixed. @@ -151,9 +154,16 @@ alternative is a record nothing reclaims. The limit an operator can move is assertion to ask the SP to keep exactly this record. SAML Core 2.5.1.5 makes the condition always valid, a condition on use rather than on validity, so the login goes through with or without the option. With it, the assertion is single-use within the -bounds above, the zone with no room included. Without it, the login is accepted and a line at `warn` level names `replay_dict`, -so an IdP that asks for this is the signal to set it; a deployment logging at `error` -or above does not see it. +bounds above — a zone with no room among them. Without it, the login is accepted +and a line at `warn` level names the assertion, its issuer and `replay_dict`, so an +IdP that asks for this is the signal to set it; a deployment logging at `error` or +above does not see it. + +Consuming the assertion into a session is the immediate use Core 2.5.1.5 asks for; +what the login retains afterwards lives in that session, whose lifetime follows +`SessionNotOnOrAfter` where the IdP sends it and the session library's own timeouts +where it does not. `OneTimeUse` does not shorten a session: the profile gives +session lifetime its own instrument, and this SP honours that one where it is sent. **One thing it deliberately does not do.** Re-submitting a response that already logged in is refused, which is what a browser does when it loses the redirect that ends a