fix: accept OneTimeUse, and let the replay record honour it - #53
fix: accept OneTimeUse, and let the replay record honour it#53shreemaan-abhishek wants to merge 9 commits into
Conversation
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
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (5)
🚧 Files skipped from review as they are similar to previous changes (1)
Included review availability: 4 reviews are currently available. Your included PR review attempts over the past 7 days set your current allowance at 5 reviews per hour. 📝 WalkthroughWalkthroughThe change recognizes SAML ChangesOneTimeUse assertion handling
Estimated code review effort: 3 (Moderate) | ~20 minutes Merge Risk: 🔵 Low · up to The change enables OneTimeUse assertions and replay enforcement, but schema-typed OneTimeUse conditions remain unsupported and duplicate conditions are still accepted. The PR is mergeable with explicit owner awareness and follow-up for these bounded correctness gaps. Sequence Diagram(s)sequenceDiagram
participant IdP as SAML response
participant Parser as src/xml.c
participant Lua as lua/resty/saml.lua
participant Replay as replay_dict
IdP->>Parser: Parse OneTimeUse condition
Parser->>Lua: Return one_time_use assertion flag
alt replay_dict configured
Lua->>Replay: Check and record assertion ID
Replay-->>Lua: Accept first use or reject replay
else replay_dict unavailable
Lua-->>Lua: Log replay_dict warning
Lua-->>IdP: Accept authentication
end
🚥 Pre-merge checks | ✅ 5 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (5 passed)
Full details: Linked Issues checkExplanation The changes satisfy issue Full details: E2e Test Quality ReviewExplanation The PR adds real HTTP-level Test::Nginx integration coverage through the Lua callback, C parser, and Resolution Protect the multi-assertion reservation and rollback with a shared synchronization mechanism, or use an atomic compare-and-delete transaction that can remove only entries created by the current request. Check and handle every rollback failure. Add an integration test that exercises concurrent multi-assertion callbacks, including an entry expiry/reinsert during rollback, and verifies that another request's replay record remains protected. Full details: Security CheckExplanation No custom-check failure was introduced. The PR changes SAML condition parsing, in-memory replay tracking, and warnings. Category 1: No issues found; new logs contain only an escaped assertion ID and the literal option name
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Pull request overview
Accepts SAML OneTimeUse conditions and integrates them with existing replay protection.
Changes:
- Parses and exposes
OneTimeUseon assertions. - Warns when replay tracking is unavailable.
- Adds documentation and end-to-end replay tests.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
src/xml.c |
Recognizes and parses OneTimeUse. |
src/saml.h |
Adds the assertion flag. |
src/lua_saml.c |
Exposes the flag to Lua. |
lua/resty/saml.lua |
Warns when enforcement is unavailable. |
README.md |
Documents behavior and configuration. |
t/assertion-conditions.t |
Tests acceptance and replay rejection. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| -- 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 |
There was a problem hiding this comment.
Two problems with the condition itself.
It reads "was replay_dict configured", not "was the record actually taken". spend_assertions (620-626) fails open on any safe_add error other than "exists" — it logs a generic ERR and lets the login through untracked, which TEST 40 codifies. So when the zone is full the assertion is accepted, nothing is recorded, and this warning does not fire, because opts.replay_dict is set. Verified on this branch: with a 32k saml_replay_full filled until safe_add returns no memory, the identical signed response carrying <saml:OneTimeUse/> returns 302 / then 302 /, and no OneTimeUse line reaches the log. The one deployment that did everything the README asks and still gets no enforcement is the one that produces no diagnostic. Zone exhaustion is attacker-drivable and does not self-heal, since safe_add never evicts.
It also reads a different field than the gate that enforces. This checks opts.replay_dict — the raw name on the caller's table, held by reference at 934. Line 759 enforces on self.replay_dict, the handle resolved once at 952. They agree today only because the constructor errors on a name that does not resolve. The APISIX saml-auth plugin hands resty_saml.new the live plugin conf table and holds the object in a 300s lrucache, so any post-construction mutation splits them in one direction or the other: enforce-while-warning, or suppress-the-warning-while-recording-nothing. assertions_acceptable only receives opts, so it cannot consult the value that governs — passing self (or the resolved handle) would make the invariant structural. TEST 48 cannot catch either direction, since both fields are truthy there.
There was a problem hiding this comment.
Second point taken: assertions_acceptable now receives self.replay_dict, the handle the last gate enforces on, so the two read one value (8bf6364).
On the first, the full-dict path is not silent. spend_assertions logs at ERR, could not remember assertion <id> in <dict>: no memory, this login is not covered by replay tracking, and TEST 40 asserts that line. What it lacked was the word; it now ends though it carries OneTimeUse when the assertion does, and TEST 49 pins it. Fail-open on a full dict is #50's documented choice and stays.
| -- 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 |
There was a problem hiding this comment.
"replay_dict is that record" is the claim the rest of the PR rests on, and there are three paths where the record does not cover the window in which the assertion is still accepted. All three verified on this branch.
An assertion that bounds nothing gets a 600s record and unbounded acceptance (line 600). conditions({ body = "<saml:OneTimeUse/>" }) emits Conditions with no NotBefore/NotOnOrAfter and assertion() emits no SubjectConfirmation, so last_moment_usable returns nil and ttl falls to DEFAULT_REPLAY_TTL. After the first login ngx.shared.saml_replay:ttl(replay_key("otu-noexp")) reads 599.999; drop the record and the identical response logs in again. time_bounds_ok(nil, nil, ...) is true forever, and the confirmation loop is skipped when #confirmations == 0. This is exactly TEST 48's fixture — it passes only because it replays immediately.
last_moment_usable decides confirms_here without looking at not_before (line 543). A confirmation that confirmation_ok will refuse still counts, still sets unbounded, and still wipes a satisfiable sibling's close at 558. An assertion with C1 (this ACS, NotBefore an hour out, no NotOnOrAfter) and C2 (this ACS, NotOnOrAfter 4h out) is accepted on C2, but the record TTL reads 599.999 instead of ~14460 — replayable from T+600 to T+4h. The comment at 528-538 says this class is excluded, but the exclusion only covers an unparseable not_on_or_after; confirms_here has no clock in it at all.
MAX_REPLAY_TTL caps the record with no matching cap on acceptance (line 607). IdP sets NotOnOrAfter a week out: the record dies at 86400 and time_bounds_ok keeps admitting the assertion for another six days. TEST 39 already asserts that capping for a plain assertion; this PR routes a class where the IdP has explicitly demanded single use through the same path, without deciding whether "forget it and accept it again" is the right answer, and without refusing or warning when the assertion outlives its record.
There was a problem hiding this comment.
All three are bounds of the record #50 introduced, untouched here, and the README's Remembering assertions states the first and third in the words you quote; TESTs 37 and 39 pin them. Every real IdP bounds a bearer confirmation to minutes, so none of the three occurs on shipped defaults.
The second is a misread. A confirmation with a NotBefore an hour out and no NotOnOrAfter becomes satisfiable at T+1h and never stops, so the assertion is acceptable for good from then on. last_moment_usable answering unbounded is correct, and the fallback record is the designed answer; a ~14460s record would leave the assertion replayable from T+4h with no record at all.
What survives is the diagnostic. spend_assertions now WARNs when an OneTimeUse assertion's record fell back to replay_ttl or was capped at a day (bd5226e, TEST 50), and TEST 48's fixture carries a NotOnOrAfter so it no longer passes by replaying inside 600s (a2e67be).
| -- 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), |
There was a problem hiding this comment.
This line is the only thing standing between the old refusal and silent acceptance, and it fails in both directions.
Invisible by default. nginx's documented default is error_log logs/error.log error;, and OpenResty does not raise it, so an embedder that never sets a level discards this entirely — the login changes from refused to accepted-and-replayable with no notice at all. The suite only sees it because t/assertion-conditions.t:3 calls log_level('info'). README:148 presents the warning as the delivery mechanism without saying what level is required to see it.
Unbounded when it is visible. It sits behind only a session lookup and a RelayState comparison against the caller's own session, with no once-per-worker latch. Verified: a OneTimeUse assertion restricted to another audience returns 401 nil and still writes the warning, so one captured signed assertion replayed in a GET / -> POST /acs loop writes WARN lines indefinitely for logins that never succeed. And the population this PR unblocks — IdPs that stamp OneTimeUse on every assertion — gets one line per login forever, with no way to silence it.
There was a problem hiding this comment.
The warning is a courtesy, not the safety line; silent acceptance is what Core 2.5.1.5 prescribes and what Spring, Shibboleth SP and Keycloak do.
Level: both consumers default error_log_level to warn (apisix/cli/config.lua, EE config-default.yaml), so it is visible where it matters. The README now says it is logged at warn (b162b7b).
Volume: a refused attempt on that loop already writes response from IdP rejected: ... at ERR, so the vector exists today one level up; this adds a line to it. One line per accepted login is the correct signal for a deployment whose IdP asks for single use and which has not configured it, and a once-per-worker latch would hide a persistent state after the first hit. Left as is.
| **This is what `<saml:OneTimeUse/>` 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 |
There was a problem hiding this comment.
This sentence is an unqualified guarantee that the three paragraphs directly above it already deny, and that the code denies further.
118-124 says the record is per nginx instance and a replay through a load balancer "is accepted"; 129-131 says a full zone "leaves that assertion untracked"; 136-142 says an unbounded assertion is "accepted for good" past replay_ttl, and one valid beyond a day is "accepted again past it". None of that is carried forward here. The deleted text stated its limitation plainly ("is still refused outright... the two do not meet yet"); the replacement states a property the code does not have, so an operator can set replay_dict and report OneTimeUse compliance that does not survive contact with the deployment.
The without-it/with-it framing is also binary where the code has three outcomes: unset -> warn and accept; set and recorded -> enforced; set and not recorded -> accepted with no OneTimeUse warning at all.
| return is_assertion_el(node, "AudienceRestriction") || | ||
| is_assertion_el(node, "ProxyRestriction"); | ||
| is_assertion_el(node, "ProxyRestriction") || | ||
| is_assertion_el(node, "OneTimeUse"); |
There was a problem hiding this comment.
Matching on element name only leaves the other schema-valid encoding of the same condition refused: <saml:Condition xsi:type="saml:OneTimeUseType"/>. OneTimeUseType extends ConditionAbstractType, so the XSD accepts that form, but is_assertion_el compares node->name — which is Condition for that shape — so is_known_condition returns 0 and unknown_condition is set.
Verified: an assertion whose only condition is <saml:Condition xmlns:xsi=... xsi:type="saml:OneTimeUseType"/> returns 401 nil with "carries a condition this SP cannot satisfy: Condition" on both the plain SP and the replay_dict-configured one — the refusal fires at saml.lua:417, before one_time_use is ever consulted. The same holds for xsi:type="saml:ProxyRestrictionType", which #42 explicitly meant to accept.
So the PR body's premise — "there is no configuration that gets past the refusal" — remains true for this encoding after the fix.
There was a problem hiding this comment.
Pre-existing and deliberate: #42 refuses the xsi:type spelling for every condition, AudienceRestriction included, and TEST 13 pins that with xsi:type="saml:AudienceRestrictionType" as its unrecognised case. Refusing is fail-closed and in spec (Core 2.5.1.1 rule 3). Accepting it means resolving xsi:type in C for every reader at once; an xsi:type AudienceRestriction accepted as known with its audiences unread would be a bypass. No browser-SSO IdP writes that spelling. Tracked in #54, out of scope here.
| // 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. |
There was a problem hiding this comment.
"which the caller has or has not" is the load-bearing assumption, and in the shipping product the caller cannot have it.
Both apisix/apisix/plugins/saml-auth.lua and api7-ee-3-gateway/apisix/plugins/saml-auth.lua are pinned to lua-resty-saml = 0.2.5 and declare a schema with no replay_dict, replay_ttl, idp_issuers, sp_audiences or clock_skew, and there is no saml_replay shared dict to name. Every gateway deployment therefore lands permanently on the accept-and-warn path, with a warning naming an option the operator has no way to set. There is also no strict / refuse-unenforceable knob anywhere in this diff to restore the old behaviour.
An operator who smuggles replay_dict past the schema (there is no additionalProperties: false) without a matching shared dict hits _M.new's error("no lua_shared_dict named ..."), which core/lrucache.lua calls without pcall — a hard 500 on every request through the route.
This is not a regression against any shipped version, since #42's refusal postdates 0.2.5. But a companion PR exposing replay_dict in both plugin schemas should land with this one; otherwise the net effect of the release is OneTimeUse going from refused to accepted and unenforced.
There was a problem hiding this comment.
Agreed on the facts, and it is the follow-up: api7/api7-ee-3-gateway#2177 covers exposing replay_dict/replay_ttl in saml-auth, declaring the dict in ngx_tpl.lua, and the same in apache/apisix plus the control-plane sync. It cannot land with this PR: the plugins pin 0.2.5, so it follows #39 and the pin bump, in two other repos.
As you note, 0.2.5 already accepts OneTimeUse unenforced; this PR keeps that and adds the warning, where 0.2.6 without it would refuse. A knob to restore that refusal would restore a spec-nonconformant behaviour nobody asked for, so none is added.
| } | ||
|
|
||
| for (xmlNode* child = conditions->children; child != NULL; child = child->next) { | ||
| if (is_assertion_el(child, "OneTimeUse")) { |
There was a problem hiding this comment.
This sits before the break below, so one_time_use is document-order dependent: a <saml:OneTimeUse/> appearing after an unknown condition is never seen, and the newly exported public field silently reads false.
Verified through saml.doc_assertions(): Conditions with <saml:OneTimeUse/> then an unknown <saml:Condition> reports one_time_use=true unknown_condition=Condition; the reverse order reports one_time_use=false unknown_condition=Condition. Same document content, different derived state.
So the field does not mean "this assertion carries OneTimeUse", it means "it carries OneTimeUse and no unrecognised condition preceded it". Harmless only because assertions_acceptable refuses on unknown_condition (417) before reading one_time_use (426) — two facts in different files coupled by statement order, with nothing recording the dependency. Soften unknown-condition handling the way this PR just softened OneTimeUse, and the flag starts under-reporting.
There was a problem hiding this comment.
Fixed (c769806). The scan no longer stops at the first unknown condition; the first unknown name is still what is reported. TEST 51 reads the flag in both orders.
|
|
||
| for (xmlNode* child = conditions->children; child != NULL; child = child->next) { | ||
| if (is_assertion_el(child, "OneTimeUse")) { | ||
| a->one_time_use = 1; |
There was a problem hiding this comment.
Setting the flag on every occurrence means a <Conditions> carrying two or more <saml:OneTimeUse/> elements is now accepted. Core 2.5.1.5 makes that a MUST NOT, and the bundled XSD cannot catch it — xsd/saml-schema-assertion-2.0.xsd:128-135 declares the ConditionsType body as <choice minOccurs="0" maxOccurs="unbounded">, so duplicates validate. Before this PR any occurrence was refused, so they could not get through.
Verified: <saml:OneTimeUse/><saml:OneTimeUse/> returns 302 / and parses as one_time_use=true, unknown_condition=nil.
A PR whose subject is spec conformance for this element leaving the element's own cardinality rule unenforced seems worth a second look, especially since the body cites Keycloak's broker ("only checks there is at most one") as the peer behaviour being matched. count_assertion_el(conditions, "OneTimeUse") — already used by read_audience_restrictions and read_subject_confirmations — gives presence and cardinality in one call.
| if (is_assertion_el(child, "OneTimeUse")) { | ||
| a->one_time_use = 1; | ||
| } | ||
| if (child->type == XML_ELEMENT_NODE && !is_known_condition(child)) { |
There was a problem hiding this comment.
Promoting OneTimeUse to a known condition leaves unknown_condition (line 520) able to hold exactly one value: the literal string Condition.
XSD validation runs on every parse before anything reads the document (src/binding.c:196 for POST, :314 for redirect), and ConditionsType at xsd/saml-schema-assertion-2.0.xsd:128 is a closed choice over exactly four elements — Condition, AudienceRestriction, OneTimeUse, ProxyRestriction — with no <any> wildcard. Three of the four are now in is_known_condition, so the only name that can reach xmlStrdup(child->name) is Condition. And xmlStrdup copies the element name, never the xsi:type attribute, which is the only part that says what the condition actually is.
Verified: OneTimeUseType, ProxyRestrictionType and AudienceRestrictionType all produce the identical line assertion <id> carries a condition this SP cannot satisfy: Condition. An operator debugging a broken IdP gets a message naming neither the condition nor replay_dict — so the exact case this PR wants them to recognise is the one the log can no longer identify — and the name-reporting machinery becomes dead weight for a single hardcoded outcome.
There was a problem hiding this comment.
The facts hold: after this PR Condition is the only element that can reach unknown_condition. Keeping the name generic costs a strdup and stays right if the XSD or the reader ever admits a fifth element, so it stays. Appending the xsi:type value to that message, so the one case it fires in names the type, is in #54 with the rest of the xsi:type question. No IdP emits the long spelling, so the case this PR wants operators to recognise is the plain element, which the warning names.
| -- asks for a record this SP does not keep, which is said, not refused | ||
| ngx.say(login_with("plain", saml_response({ | ||
| conditions = conditions({ body = "<saml:OneTimeUse/>" }), | ||
| id = "single", conditions = conditions({ body = "<saml:OneTimeUse/>" }), |
There was a problem hiding this comment.
This block lost its only behavioural assertion about OneTimeUse and gained no replacement, so the weakening the PR ships has no coverage.
The deleted 401 nil plus qr/carries a condition this SP cannot satisfy: OneTimeUse/ proved the assertion was refused. The new 302 / plus the warn regex proves only that one presentation is accepted. Nothing in the suite presents the same OneTimeUse assertion twice to an SP without replay_dict, which is the property that actually changed. Verified that it is genuinely reusable: two login_with("plain", xml) calls with the identical response both return 302 /.
As it stands, a change that started refusing OneTimeUse again on the no-dict path breaks no test, and neither does a regression in the other direction.
There was a problem hiding this comment.
TEST 13's first 302 is the coverage: refusing OneTimeUse again on the dict-less SP returns 401 there and fails the block, which is what the run against main shows. The narrower gap is real, and TEST 52 now presents the same OneTimeUse assertion twice to the dict-less SP and expects 302 both times (b8728ad).
|
|
||
|
|
||
|
|
||
| === TEST 48: OneTimeUse is met by the replay record where there is one |
There was a problem hiding this comment.
This is TEST 32 with the conditions body swapped, and it does not exercise anything OneTimeUse-specific: spend_assertions never reads assertion.one_time_use, so deleting <saml:OneTimeUse/> from the payload leaves the block passing. TEST 32 (line 1011) already has the same flush_all, the same two login_with("replay", xml) calls, the same 302 / / 401 nil, and the same "has been presented already" assertion.
Three hygiene points while it is being reworked:
- It never inspects the record, though
replay_key(line 240) exists for exactly that and TESTs 34/37/38/41/42 use it. As written it passes for any reason the second login is refused. Asserting the TTL would also have caught the 600s fallback noted onsaml.lua. - It is the only block in the file supplying its own
--- no_error_log, which stops theadd_block_preprocessorelsif(lines 17-22) from firing and silently drops the[alert]and[emerg]guards every other error_log-carrying block gets. Appending to the injected list instead keeps them. - It reuses
id = "single", the same string TEST 13 gains at line 580, against the convention the file's ownhttp_configstates. Latent today only because TEST 13 runs on the dict-lessplainSP.
Separately: TEST 16 (line 641) is the only block that inspects the raw doc_assertions table, and it was not extended with one_time_use, so the C reader contract for the new field has no direct assertion anywhere.
There was a problem hiding this comment.
The first 302 is the OneTimeUse-specific part; on main it is 401. The second half passing without the element is the claim under test: once the record exists the element changes nothing, and the title now says so.
Hygiene items all taken (a2e67be): the fixture bounds the assertion with a NotOnOrAfter, the record's TTL is read back through replay_key, the ID is its own, the no_error_log list carries the guards the preprocessor would have injected, and TEST 16 reports one_time_use.
| xmlChar* id; | ||
| xmlChar* issuer; | ||
| int has_conditions; | ||
| int one_time_use; |
There was a problem hiding this comment.
Adding a field here is worth pairing with a one-line Makefile fix: neither object rule lists src/saml.h as a prerequisite. saml.o: src/*.c globs only .c files, and lua_saml.o: src/lua_saml.c names one.
Verified with make -n: touch src/saml.h alone reports "saml.so is up to date"; touch src/saml.h src/xml.c rebuilds only saml.o and relinks it against the old lua_saml.o. Had this field changed the layout, saml_doc_assertions would stride the array with one sizeof while doc_assertions indexed fields with another — shifted pointers into Lua and xmlFree on garbage, on the login path, with no compiler or linker diagnostic.
This PR escapes by luck: int one_time_use lands in the tail padding after int has_conditions, and both layouts measure sizeof=80 with id=0 issuer=8 has_conditions=16 not_before=24 not_on_or_after=32 unknown_condition=40 — byte-identical. CI always builds fresh, so it would never surface there.
There was a problem hiding this comment.
Fixed (3884513): both object rules list src/*.h.
…t 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.
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.
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.
…d 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.
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.
Closes #46. Lands before #39, so v0.2.6 does not ship the refusal.
What was wrong
#42 refuses an assertion carrying
<saml:OneTimeUse/>as a condition this SP cannot satisfy, on a reading of SAML Core 2.5.1.5 that the text does not support. Verbatim:The record of spent assertions is a SHOULD, and the one MUST binds implementations that retain assertions for future use, which this SP does not: it reads the assertion once, mints its own session, and drops the document. Every peer reads it the same way (Spring Security's default validator returns
VALIDfor it, Shibboleth SP's default policy ignores it, Keycloak's broker only checks there is at most one).Keycloak emits the condition behind a per-client toggle. Those IdPs logged in before #42 and cannot log in after it, and neither
saml-authplugin exposesreplay_dict, so there is no configuration that gets past the refusal.What it does now
OneTimeUseis back onis_known_condition, and the reader carries it asone_time_useonsaml_assertion_tand the Lua table.replay_dictset, nothing more happens: fix: let an assertion be presented only once #50 already remembers every accepted assertion, so the single use the IdP asked for is enforced for realreplay_dictunset, the login goes through and a warning names the option:assertion <id> carries OneTimeUse, which this SP cannot enforce without replay_dictA condition the reader has never heard of is still refused.
Tests
TEST 13 now expects
302and the warning on the SP with no dict, and TEST 48 presents anOneTimeUseassertion twice on the SP with one:302then401 has been presented already, with no warning. Both fail onmainas it stands and pass here; the rest oft/assertion-conditions.tandt/login-callback.tare unchanged and green.Summary by CodeRabbit
OneTimeUsecondition.