diff --git a/services/src/main/java/org/keycloak/broker/saml/SAMLEndpoint.java b/services/src/main/java/org/keycloak/broker/saml/SAMLEndpoint.java
index 6aaecaf..a6980a9 100755
--- a/services/src/main/java/org/keycloak/broker/saml/SAMLEndpoint.java
+++ b/services/src/main/java/org/keycloak/broker/saml/SAMLEndpoint.java
@@ -87,12 +87,13 @@ import java.util.List;
import org.keycloak.rotation.HardcodedKeyLocator;
import org.keycloak.rotation.KeyLocator;
import org.keycloak.saml.processing.core.util.KeycloakKeySamlExtensionGenerator;
+import org.keycloak.saml.validators.ConditionsValidator;
import org.keycloak.saml.validators.DestinationValidator;
+import java.net.URI;
import java.security.cert.CertificateException;
import org.w3c.dom.Element;
import java.util.*;
-import javax.security.auth.x500.X500Principal;
import javax.xml.crypto.dsig.XMLSignature;
import org.w3c.dom.NodeList;
@@ -412,6 +413,22 @@ public class SAMLEndpoint {
identity.setToken(samlResponse);
}
+ ConditionsValidator.Builder cvb = new ConditionsValidator.Builder(assertion.getID(), assertion.getConditions(), destinationValidator);
+ try {
+ String issuerURL = getEntityId(session.getContext().getUri(), realm);
+ cvb.addAllowedAudience(URI.create(issuerURL));
+ // getDestination has been validated to match request URL already so it matches SAML endpoint
+ cvb.addAllowedAudience(URI.create(responseType.getDestination()));
+ } catch (IllegalArgumentException ex) {
+ // warning has been already emitted in DeploymentBuilder
+ }
+ if (! cvb.build().isValid()) {
+ logger.error("Assertion expired.");
+ event.event(EventType.IDENTITY_PROVIDER_RESPONSE);
+ event.error(Errors.INVALID_SAML_RESPONSE);
+ return ErrorPage.error(session, null, Response.Status.BAD_REQUEST, Messages.EXPIRED_CODE);
+ }
+
AuthnStatementType authn = null;
for (Object statement : assertion.getStatements()) {
if (statement instanceof AuthnStatementType) {
diff --git a/testsuite/integration-arquillian/tests/base/src/main/java/org/keycloak/testsuite/util/saml/SamlDocumentStepBuilder.java b/testsuite/integration-arquillian/tests/base/src/main/java/org/keycloak/testsuite/util/saml/SamlDocumentStepBuilder.java
index bd5d2d1..21b3126 100644
--- a/testsuite/integration-arquillian/tests/base/src/main/java/org/keycloak/testsuite/util/saml/SamlDocumentStepBuilder.java
+++ b/testsuite/integration-arquillian/tests/base/src/main/java/org/keycloak/testsuite/util/saml/SamlDocumentStepBuilder.java
@@ -36,6 +36,7 @@ import org.keycloak.testsuite.util.SamlClient.Step;
import java.io.ByteArrayInputStream;
import java.io.ByteArrayOutputStream;
import javax.xml.stream.XMLStreamWriter;
+import org.jboss.logging.Logger;
import org.junit.Assert;
import org.w3c.dom.Document;
@@ -45,6 +46,8 @@ import org.w3c.dom.Document;
*/
public abstract class SamlDocumentStepBuilder<T extends SAML2Object, This extends SamlDocumentStepBuilder<T, This>> implements Step {
+ private static final Logger LOG = Logger.getLogger(SamlDocumentStepBuilder.class);
+
@FunctionalInterface
public interface Saml2ObjectTransformer<T extends SAML2Object> {
public T transform(T original) throws Exception;
@@ -107,7 +110,9 @@ public abstract class SamlDocumentStepBuilder<T extends SAML2Object, This extend
Assert.assertNotNull("Unknown type: <null>", transformed);
Assert.fail("Unknown type: " + transformed.getClass().getName());
}
- return new String(bos.toByteArray(), GeneralConstants.SAML_CHARSET);
+ String res = new String(bos.toByteArray(), GeneralConstants.SAML_CHARSET);
+ LOG.debugf(" ---> %s", res);
+ return res;
};
return (This) this;
}
diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/saml/BrokerTest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/saml/BrokerTest.java
index 4c53d29..13ec90c 100644
--- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/saml/BrokerTest.java
+++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/saml/BrokerTest.java
@@ -22,9 +22,13 @@ import org.keycloak.authentication.authenticators.broker.IdpReviewProfileAuthent
import org.keycloak.broker.saml.SAMLIdentityProviderConfig;
import org.keycloak.broker.saml.SAMLIdentityProviderFactory;
import org.keycloak.dom.saml.v2.SAML2Object;
+import org.keycloak.dom.saml.v2.assertion.AssertionType;
import org.keycloak.dom.saml.v2.assertion.AttributeStatementType;
import org.keycloak.dom.saml.v2.assertion.AttributeStatementType.ASTChoiceType;
import org.keycloak.dom.saml.v2.assertion.AttributeType;
+import org.keycloak.dom.saml.v2.assertion.ConditionsType;
+import org.keycloak.dom.saml.v2.assertion.NameIDType;
+import org.keycloak.dom.saml.v2.assertion.SubjectType;
import org.keycloak.dom.saml.v2.protocol.AuthnRequestType;
import org.keycloak.dom.saml.v2.protocol.NameIDPolicyType;
import org.keycloak.dom.saml.v2.protocol.ResponseType;
@@ -36,6 +40,7 @@ import org.keycloak.saml.common.constants.JBossSAMLURIConstants;
import org.keycloak.saml.common.exceptions.ConfigurationException;
import org.keycloak.saml.common.exceptions.ProcessingException;
import org.keycloak.saml.processing.core.saml.v2.common.SAMLDocumentHolder;
+import org.keycloak.saml.processing.core.saml.v2.util.XMLTimeUtil;
import org.keycloak.testsuite.updaters.IdentityProviderCreator;
import org.keycloak.testsuite.util.IdentityProviderBuilder;
import org.keycloak.testsuite.util.SamlClientBuilder;
@@ -44,6 +49,8 @@ import java.net.URI;
import java.util.List;
import java.util.Objects;
import java.util.UUID;
+import javax.ws.rs.core.Response.Status;
+import javax.xml.datatype.XMLGregorianCalendar;
import org.apache.http.Header;
import org.apache.http.HttpHeaders;
import org.hamcrest.Matchers;
@@ -185,4 +192,72 @@ public class BrokerTest extends AbstractSamlTest {
}
}
+ @Test
+ public void testExpiredAssertion() throws Exception {
+ XMLGregorianCalendar now = XMLTimeUtil.getIssueInstant();
+ XMLGregorianCalendar notBeforeInPast = XMLTimeUtil.subtract(now, 60 * 60 * 1000);
+ XMLGregorianCalendar notOnOrAfterInPast = XMLTimeUtil.subtract(now, 59 * 60 * 1000);
+ XMLGregorianCalendar notBeforeInFuture = XMLTimeUtil.add(now, 59 * 60 * 1000);
+ XMLGregorianCalendar notOnOrAfterInFuture = XMLTimeUtil.add(now, 60 * 60 * 1000);
+ // Should not pass:
+ assertExpired(notBeforeInPast, notOnOrAfterInPast, false);
+ assertExpired(notBeforeInFuture, notOnOrAfterInPast, false);
+ assertExpired(null, notOnOrAfterInPast, false);
+ assertExpired(notBeforeInFuture, notOnOrAfterInFuture, false);
+ assertExpired(notBeforeInFuture, null, false);
+ // Should pass:
+ assertExpired(notBeforeInPast, notOnOrAfterInFuture, true);
+ assertExpired(notBeforeInPast, null, true);
+ assertExpired(null, notOnOrAfterInFuture, true);
+ assertExpired(null, null, true);
+ }
+
+ @Test(expected = AssertionError.class)
+ public void testNonexpiredAssertionShouldFail() throws Exception {
+ assertExpired(null, null, false); // Expected result (false) is it should fail but it should pass and throw
+ }
+
+ @Test(expected = AssertionError.class)
+ public void testExpiredAssertionShouldFail() throws Exception {
+ XMLGregorianCalendar now = XMLTimeUtil.getIssueInstant();
+ XMLGregorianCalendar notBeforeInPast = XMLTimeUtil.subtract(now, 60 * 60 * 1000);
+ XMLGregorianCalendar notOnOrAfterInPast = XMLTimeUtil.subtract(now, 59 * 60 * 1000);
+ assertExpired(notBeforeInPast, notOnOrAfterInPast, true); // Expected result (true) is it should succeed but it should pass and throw
+ }
+
+ private void assertExpired(XMLGregorianCalendar notBefore, XMLGregorianCalendar notOnOrAfter, boolean shouldPass) throws Exception {
+ Status expectedStatus = shouldPass ? Status.OK : Status.BAD_REQUEST;
+ final RealmResource realm = adminClient.realm(REALM_NAME);
+ try (IdentityProviderCreator idp = new IdentityProviderCreator(realm, addIdentityProvider("http://saml.idp/"))) {
+ new SamlClientBuilder()
+ .authnRequest(getAuthServerSamlEndpoint(REALM_NAME), SAML_CLIENT_ID_SALES_POST, SAML_ASSERTION_CONSUMER_URL_SALES_POST, POST).build()
+ .login().idp(SAML_BROKER_ALIAS).build()
+ // Virtually perform login at IdP (return artificial SAML response)
+ .processSamlResponse(REDIRECT)
+ .transformObject(this::createAuthnResponse)
+ .transformObject(resp -> { // always invent a new user identified by a different email address
+ ResponseType rt = (ResponseType) resp;
+ AssertionType a = rt.getAssertions().get(0).getAssertion();
+
+ NameIDType nameId = new NameIDType();
+ nameId.setFormat(URI.create(JBossSAMLURIConstants.NAMEID_FORMAT_EMAIL.get()));
+ nameId.setValue(UUID.randomUUID() + "@random.email.org");
+ SubjectType subject = new SubjectType();
+ SubjectType.STSubType subType = new SubjectType.STSubType();
+ subType.addBaseID(nameId);
+ subject.setSubType(subType);
+ a.setSubject(subject);
+
+ ConditionsType conditions = a.getConditions();
+ conditions.setNotBefore(notBefore);
+ conditions.setNotOnOrAfter(notOnOrAfter);
+ return rt;
+ })
+ .targetAttributeSamlResponse()
+ .targetUri(getSamlBrokerUrl(REALM_NAME))
+ .build()
+ .assertResponse(org.keycloak.testsuite.util.Matchers.statusCodeIsHC(expectedStatus))
+ .execute();
+ }
+ }
}
