This is an automated email from the ASF dual-hosted git repository. coheigea pushed a commit to branch coheigea/saml-conditions in repository https://gitbox.apache.org/repos/asf/ws-wss4j.git
commit e59fdd8dccd182abd942746bade9b598927627c8 Author: Colm O hEigeartaigh <[email protected]> AuthorDate: Mon Sep 21 11:21:33 2026 +0100 Fix SAML Conditions parsing --- .../wss4j/common/saml/SamlAssertionWrapper.java | 23 +++-- .../apache/wss4j/dom/saml/SamlConditionsTest.java | 104 +++++++++++++++++++++ 2 files changed, 121 insertions(+), 6 deletions(-) diff --git a/ws-security-common/src/main/java/org/apache/wss4j/common/saml/SamlAssertionWrapper.java b/ws-security-common/src/main/java/org/apache/wss4j/common/saml/SamlAssertionWrapper.java index 29e9b3277..39f47d4f4 100644 --- a/ws-security-common/src/main/java/org/apache/wss4j/common/saml/SamlAssertionWrapper.java +++ b/ws-security-common/src/main/java/org/apache/wss4j/common/saml/SamlAssertionWrapper.java @@ -817,14 +817,19 @@ public class SamlAssertionWrapper { Instant issueInstant = null; Instant validTill = null; - if (getSamlVersion().equals(SAMLVersion.VERSION_20) - && getSaml2().getConditions() != null) { - validTill = getSaml2().getConditions().getNotOnOrAfter(); + // The IssueInstant is read whether or not the assertion carries any Conditions at all. + // It is the fallback bound on the assertion's lifetime, so reading it only when there + // are Conditions would skip that bound in precisely the case it exists for. + if (getSamlVersion().equals(SAMLVersion.VERSION_20)) { issueInstant = getSaml2().getIssueInstant(); - } else if (getSamlVersion().equals(SAMLVersion.VERSION_11) - && getSaml1().getConditions() != null) { - validTill = getSaml1().getConditions().getNotOnOrAfter(); + if (getSaml2().getConditions() != null) { + validTill = getSaml2().getConditions().getNotOnOrAfter(); + } + } else if (getSamlVersion().equals(SAMLVersion.VERSION_11)) { issueInstant = getSaml1().getIssueInstant(); + if (getSaml1().getConditions() != null) { + validTill = getSaml1().getConditions().getNotOnOrAfter(); + } } // Check the IssueInstant is not in the future, subject to the future TTL @@ -844,6 +849,12 @@ public class SamlAssertionWrapper { throw new WSSecurityException(WSSecurityException.ErrorCode.FAILURE, "invalidSAMLsecurity"); } } + } else if (validTill == null) { + // No NotOnOrAfter condition and no IssueInstant to impose a TTL on: the assertion + // states no bound on its own lifetime whatsoever, and one is not going to be + // invented for it. + LOG.warn("SAML Token has neither a NotOnOrAfter condition nor an IssueInstant"); + throw new WSSecurityException(WSSecurityException.ErrorCode.FAILURE, "invalidSAMLsecurity"); } } diff --git a/ws-security-dom/src/test/java/org/apache/wss4j/dom/saml/SamlConditionsTest.java b/ws-security-dom/src/test/java/org/apache/wss4j/dom/saml/SamlConditionsTest.java index 3fd642a6d..7bfd95d1a 100644 --- a/ws-security-dom/src/test/java/org/apache/wss4j/dom/saml/SamlConditionsTest.java +++ b/ws-security-dom/src/test/java/org/apache/wss4j/dom/saml/SamlConditionsTest.java @@ -284,6 +284,76 @@ public class SamlConditionsTest { verify(unsignedDoc); } + /** + * An assertion with no Conditions element at all has no NotOnOrAfter, so the TTL on the + * IssueInstant is the only bound on its lifetime that exists. It has to be applied, or such + * an assertion is good forever. + */ + @Test + public void testSAML2StaleIssueInstantWithNoConditions() throws Exception { + SAML2CallbackHandler callbackHandler = new SAML2CallbackHandler(); + callbackHandler.setStatement(SAML2CallbackHandler.Statement.AUTHN); + callbackHandler.setIssuer("www.example.com"); + + SAMLCallback samlCallback = new SAMLCallback(); + SAMLUtil.doSAMLCallback(callbackHandler, samlCallback); + SamlAssertionWrapper samlAssertion = new SamlAssertionWrapper(samlCallback); + + Instant issueInstant = Instant.now().minus(Duration.ofMinutes(31)); + samlAssertion.getSaml2().setIssueInstant(issueInstant); + samlAssertion.getSaml2().setConditions(null); + + Document doc = SOAPUtil.toSOAPPart(SOAPUtil.SAMPLE_SOAP_MSG); + WSSecHeader secHeader = new WSSecHeader(doc); + secHeader.insertSecurityHeader(); + + WSSecSAMLToken wsSign = new WSSecSAMLToken(secHeader); + + Document unsignedDoc = wsSign.build(samlAssertion); + + if (LOG.isDebugEnabled()) { + LOG.debug("SAML 2 Authn Assertion (sender vouches):"); + String outputString = + XMLUtils.prettyDocumentToString(unsignedDoc); + LOG.debug(outputString); + } + + try { + verify(unsignedDoc); + fail("Failure expected in processing a stale SAML Assertion"); + } catch (WSSecurityException ex) { + assertTrue(ex.getMessage().contains("SAML token security failure")); + } + } + + /** + * The same assertion within the TTL is accepted: an assertion is not required to carry + * Conditions, only to be recent when it does not. + */ + @Test + public void testSAML2FreshIssueInstantWithNoConditions() throws Exception { + SAML2CallbackHandler callbackHandler = new SAML2CallbackHandler(); + callbackHandler.setStatement(SAML2CallbackHandler.Statement.AUTHN); + callbackHandler.setIssuer("www.example.com"); + + SAMLCallback samlCallback = new SAMLCallback(); + SAMLUtil.doSAMLCallback(callbackHandler, samlCallback); + SamlAssertionWrapper samlAssertion = new SamlAssertionWrapper(samlCallback); + + samlAssertion.getSaml2().setIssueInstant(Instant.now().minusSeconds(5)); + samlAssertion.getSaml2().setConditions(null); + + Document doc = SOAPUtil.toSOAPPart(SOAPUtil.SAMPLE_SOAP_MSG); + WSSecHeader secHeader = new WSSecHeader(doc); + secHeader.insertSecurityHeader(); + + WSSecSAMLToken wsSign = new WSSecSAMLToken(secHeader); + + Document unsignedDoc = wsSign.build(samlAssertion); + + verify(unsignedDoc); + } + @Test public void testSAML1StaleIssueInstant() throws Exception { SAML1CallbackHandler callbackHandler = new SAML1CallbackHandler(); @@ -322,6 +392,40 @@ public class SamlConditionsTest { } } + /** + * The SAML 1.1 counterpart: no Conditions element, so the IssueInstant TTL is the only + * bound there is. + */ + @Test + public void testSAML1StaleIssueInstantWithNoConditions() throws Exception { + SAML1CallbackHandler callbackHandler = new SAML1CallbackHandler(); + callbackHandler.setStatement(SAML1CallbackHandler.Statement.AUTHN); + callbackHandler.setIssuer("www.example.com"); + + SAMLCallback samlCallback = new SAMLCallback(); + SAMLUtil.doSAMLCallback(callbackHandler, samlCallback); + SamlAssertionWrapper samlAssertion = new SamlAssertionWrapper(samlCallback); + + Instant issueInstant = Instant.now().minus(Duration.ofMinutes(31)); + samlAssertion.getSaml1().setIssueInstant(issueInstant); + samlAssertion.getSaml1().setConditions(null); + + Document doc = SOAPUtil.toSOAPPart(SOAPUtil.SAMPLE_SOAP_MSG); + WSSecHeader secHeader = new WSSecHeader(doc); + secHeader.insertSecurityHeader(); + + WSSecSAMLToken wsSign = new WSSecSAMLToken(secHeader); + + Document unsignedDoc = wsSign.build(samlAssertion); + + try { + verify(unsignedDoc); + fail("Failure expected in processing a stale SAML Assertion"); + } catch (WSSecurityException ex) { + assertTrue(ex.getMessage().contains("SAML token security failure")); + } + } + /** * Test that creates, sends and processes an unsigned SAML 2 authentication assertion * with an (invalid) custom Conditions statement.
