Honour OneTimeUse when replay tracking is configured

[shreemaan-abhishek]

What

OneTimeUse is refused as of #42, on the grounds that honouring it means remembering which assertions have been spent and this SP keeps no such record. The refusal rests on a reading of SAML Core 2.5.1.5 that the text does not support. Verbatim:

For the purposes of determining the validity of the <Conditions> element, the <OneTimeUse> is considered to always be valid. That is, this condition does not affect validity but is a condition on use.

The record is a SHOULD ("a relying party should maintain a cache of the assertions it has processed containing such a condition"), 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. So an assertion carrying the condition is understood and Valid whatever the SP has configured, and refusing it as Indeterminate under 2.5.1.1 rule 3 (an element "not understood") is the SP getting the spec wrong.

Every peer reads it that way. Spring Security's default validator returns VALID for OneTimeUse ("applications should validate their own OneTimeUse conditions"), Shibboleth SP's default policy ignores it, Keycloak's broker checks only that there is at most one, python3-saml and passport-saml do nothing with it. ADFS 2.0 is the one refuser, and Microsoft's own remedy is to switch the condition off at the IdP.

What it should do

Accept OneTimeUse in every case.

• carry the condition through the reader, as a flag on saml_assertion_t and a field on the Lua table, rather than letting it fall into unknown_condition
• put it back on is_known_condition in src/xml.c
• with replay_dict set, nothing more is needed: 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 real
• with replay_dict unset, let the login through and log a warning naming replay_dict, so an operator reads it as something to configure

Why it is worth doing

Keycloak emits the condition behind a per-client toggle, "Include OneTimeUse Condition" (saml.onetimeuse.condition). Before #42 those IdPs logged in and simply got no single-use enforcement. #42 turned that into an outright refusal, so an IdP that worked before stops working, and neither the APISIX nor the EE saml-auth plugin exposes replay_dict, so there is no configuration that gets past it. That regression ships the moment v0.2.6 goes out, so this lands before #39.

Refusing buys no security: the flag is the IdP's advisory to the SP, and a party replaying a captured assertion does not care whether it is present.

Notes

Raised by @jarvis9443 reviewing #42. Depends on #50, which has merged.

[coderabbitai]

🔗 Related PRs

#42 - fix: weigh the conditions an assertion attaches to itself [open]

---

📝 Issue Planner

_{Check the box below or use the @coderabbitai plan command to generate an implementation plan and prompts that you can use with your favorite coding assistant.}

• Create Plan

---

🧪 Issue enrichment is currently in open beta.

You can configure auto-planning by selecting labels in the issue_enrichment configuration.

To disable automatic issue enrichment, add the following to your .coderabbit.yaml:

issue_enrichment:
  auto_enrich:
    enabled: false

💬 Have feedback or questions? Drop into our discord!

[shreemaan-abhishek]

Two updates from #50, which replaced #44 after it was closed.

The record this depends on now exists there: replay_dict remembers every accepted assertion until it expires, which is the state OneTimeUse demands. The refusal is untouched, so a deployment with the dict configured still cannot log in against an IdP that stamps its assertions single-use. Measured on #50 by @jarvis9443: 401, carries a condition this SP cannot satisfy: OneTimeUse.

The second is about when this has to land. Before #42 the condition was silently ignored, so those IdPs logged in and simply got no single-use enforcement. #42 turned that into an outright refusal, which means an IdP that worked before stops working. That regression ships the moment v0.2.6 goes out, so this needs to be merged before #39 rather than whenever it is convenient.

Scope is smaller than this issue assumes, if the C reader is left alone: the refusal already carries the element name, so assertions_acceptable can accept OneTimeUse while replay_dict is set and keep refusing it while it is not. Carrying it through saml_assertion_t as its own flag is the tidier shape and can follow.