From 2bf6d75e5755752f593ac98ed80b33d3958431ec Mon Sep 17 00:00:00 2001 From: Hynek Mlnarik Date: Wed, 5 Sep 2018 22:46:29 +0200 Subject: [PATCH] KEYCLOAK-8010 Improve handling of Conditions SAML tag --- .../config/parsers/DeploymentBuilder.java | 5 + .../AbstractSamlAuthenticationHandler.java | 11 +- .../saml/SAML2AuthnRequestBuilder.java | 7 +- .../api/saml/v2/response/SAML2Response.java | 82 +----- .../JBossSAMLAuthnResponseFactory.java | 6 +- .../core/saml/v2/util/AssertionUtil.java | 11 +- .../core/saml/v2/util/XMLTimeUtil.java | 26 +- .../saml/validators/ConditionsValidator.java | 234 ++++++++++++++++++ .../broker/KcSamlIdPInitiatedSsoTest.java | 73 ++++-- .../testsuite/broker/kc3731-broker-realm.json | 2 +- .../broker/kc3731-provider-realm.json | 24 +- 11 files changed, 355 insertions(+), 126 deletions(-) create mode 100644 saml-core/src/main/java/org/keycloak/saml/validators/ConditionsValidator.java diff --git a/adapters/saml/core/src/main/java/org/keycloak/adapters/saml/config/parsers/DeploymentBuilder.java b/adapters/saml/core/src/main/java/org/keycloak/adapters/saml/config/parsers/DeploymentBuilder.java index 2bae779ab8..c5cdb2c9e6 100755 --- a/adapters/saml/core/src/main/java/org/keycloak/adapters/saml/config/parsers/DeploymentBuilder.java +++ b/adapters/saml/core/src/main/java/org/keycloak/adapters/saml/config/parsers/DeploymentBuilder.java @@ -68,6 +68,11 @@ public class DeploymentBuilder { SP sp = adapter.getSps().get(0); deployment.setConfigured(true); deployment.setEntityID(sp.getEntityID()); + try { + URI.create(sp.getEntityID()); + } catch (IllegalArgumentException ex) { + log.warnf("Entity ID is not an URI, assertion that restricts audience will fail. Update Entity ID to be URI.", sp.getEntityID()); + } deployment.setForceAuthentication(sp.isForceAuthentication()); deployment.setIsPassive(sp.isIsPassive()); deployment.setNameIDPolicyFormat(sp.getNameIDPolicyFormat()); diff --git a/adapters/saml/core/src/main/java/org/keycloak/adapters/saml/profile/AbstractSamlAuthenticationHandler.java b/adapters/saml/core/src/main/java/org/keycloak/adapters/saml/profile/AbstractSamlAuthenticationHandler.java index da55815777..9a159df864 100644 --- a/adapters/saml/core/src/main/java/org/keycloak/adapters/saml/profile/AbstractSamlAuthenticationHandler.java +++ b/adapters/saml/core/src/main/java/org/keycloak/adapters/saml/profile/AbstractSamlAuthenticationHandler.java @@ -84,6 +84,7 @@ import org.keycloak.dom.saml.v2.protocol.ExtensionsType; import org.keycloak.rotation.KeyLocator; import org.keycloak.saml.processing.core.util.KeycloakKeySamlExtensionGenerator; import org.keycloak.saml.processing.core.util.XMLEncryptionUtil; +import org.keycloak.saml.validators.ConditionsValidator; import org.keycloak.saml.validators.DestinationValidator; /** @@ -342,7 +343,15 @@ public abstract class AbstractSamlAuthenticationHandler implements SamlAuthentic } try { assertion = AssertionUtil.getAssertion(responseHolder, responseType, deployment.getDecryptionKey()); - if (AssertionUtil.hasExpired(assertion)) { + ConditionsValidator.Builder cvb = new ConditionsValidator.Builder(assertion.getID(), assertion.getConditions(), destinationValidator); + try { + cvb.addAllowedAudience(URI.create(deployment.getEntityID())); + // 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()) { return initiateLogin(); } } catch (Exception e) { diff --git a/saml-core/src/main/java/org/keycloak/saml/SAML2AuthnRequestBuilder.java b/saml-core/src/main/java/org/keycloak/saml/SAML2AuthnRequestBuilder.java index 916f7f3a0e..7eebdadcf5 100755 --- a/saml-core/src/main/java/org/keycloak/saml/SAML2AuthnRequestBuilder.java +++ b/saml-core/src/main/java/org/keycloak/saml/SAML2AuthnRequestBuilder.java @@ -18,7 +18,6 @@ package org.keycloak.saml; import org.keycloak.dom.saml.v2.assertion.NameIDType; import org.keycloak.dom.saml.v2.protocol.AuthnRequestType; -import org.keycloak.saml.common.exceptions.ConfigurationException; import org.keycloak.saml.processing.api.saml.v2.request.SAML2Request; import org.keycloak.saml.processing.core.saml.v2.common.IDGenerator; import org.keycloak.saml.processing.core.saml.v2.util.XMLTimeUtil; @@ -56,11 +55,7 @@ public class SAML2AuthnRequestBuilder implements SamlProtocolExtensionsAwareBuil } public SAML2AuthnRequestBuilder() { - try { - this.authnRequestType = new AuthnRequestType(IDGenerator.create("ID_"), XMLTimeUtil.getIssueInstant()); - } catch (ConfigurationException e) { - throw new RuntimeException("Could not create SAML AuthnRequest builder.", e); - } + this.authnRequestType = new AuthnRequestType(IDGenerator.create("ID_"), XMLTimeUtil.getIssueInstant()); } public SAML2AuthnRequestBuilder assertionConsumerUrl(String assertionConsumerUrl) { diff --git a/saml-core/src/main/java/org/keycloak/saml/processing/api/saml/v2/response/SAML2Response.java b/saml-core/src/main/java/org/keycloak/saml/processing/api/saml/v2/response/SAML2Response.java index ab78fffdf6..4f356e9c78 100755 --- a/saml-core/src/main/java/org/keycloak/saml/processing/api/saml/v2/response/SAML2Response.java +++ b/saml-core/src/main/java/org/keycloak/saml/processing/api/saml/v2/response/SAML2Response.java @@ -150,71 +150,6 @@ public class SAML2Response { return authzDecST; } - /** - * Construct a {@link ResponseType} without calling PicketLink STS for the assertion. The {@link AssertionType} is - * generated - * within this method - * - * @param ID id of the {@link ResponseType} - * @param sp - * @param idp - * @param issuerInfo - * - * @return - * - * @throws org.keycloak.saml.common.exceptions.ConfigurationException - * @throws org.keycloak.saml.common.exceptions.ProcessingException - */ - public ResponseType createResponseType(String ID, SPInfoHolder sp, IDPInfoHolder idp, IssuerInfoHolder issuerInfo, - AssertionType assertion) throws ConfigurationException, ProcessingException { - String responseDestinationURI = sp.getResponseDestinationURI(); - - XMLGregorianCalendar issueInstant = XMLTimeUtil.getIssueInstant(); - - // Create assertion -> subject - SubjectType subjectType = new SubjectType(); - - // subject -> nameid - NameIDType nameIDType = new NameIDType(); - nameIDType.setFormat(URI.create(idp.getNameIDFormat())); - nameIDType.setValue(idp.getNameIDFormatValue()); - - SubjectType.STSubType subType = new SubjectType.STSubType(); - subType.addBaseID(nameIDType); - subjectType.setSubType(subType); - - SubjectConfirmationType subjectConfirmation = new SubjectConfirmationType(); - subjectConfirmation.setMethod(idp.getSubjectConfirmationMethod()); - - SubjectConfirmationDataType subjectConfirmationData = new SubjectConfirmationDataType(); - subjectConfirmationData.setInResponseTo(sp.getRequestID()); - subjectConfirmationData.setRecipient(responseDestinationURI); - //subjectConfirmationData.setNotBefore(issueInstant); - subjectConfirmationData.setNotOnOrAfter(issueInstant); - - subjectConfirmation.setSubjectConfirmationData(subjectConfirmationData); - - subjectType.addConfirmation(subjectConfirmation); - - ConditionsType conditions = assertion.getConditions(); - // Update the subjectConfirmationData expiry based on the assertion - if (conditions != null) { - subjectConfirmationData.setNotOnOrAfter(conditions.getNotOnOrAfter()); - //Add conditions -> AudienceRestriction - AudienceRestrictionType audience = new AudienceRestrictionType(); - audience.addAudience(URI.create(sp.getResponseDestinationURI())); - conditions.addCondition(audience); - } - - ResponseType responseType = createResponseType(ID, issuerInfo, assertion); - // InResponseTo ID - responseType.setInResponseTo(sp.getRequestID()); - // Destination - responseType.setDestination(responseDestinationURI); - - return responseType; - } - /** * Create a ResponseType * @@ -234,7 +169,7 @@ public class SAML2Response { * @throws ProcessingException */ public ResponseType createResponseType(String ID, SPInfoHolder sp, IDPInfoHolder idp, IssuerInfoHolder issuerInfo) - throws ConfigurationException, ProcessingException { + throws ProcessingException { String responseDestinationURI = sp.getResponseDestinationURI(); XMLGregorianCalendar issueInstant = XMLTimeUtil.getIssueInstant(); @@ -266,11 +201,7 @@ public class SAML2Response { AssertionType assertionType; NameIDType issuerID = issuerInfo.getIssuer(); - try { - issueInstant = XMLTimeUtil.getIssueInstant(); - } catch (ConfigurationException e) { - throw logger.processingError(e); - } + issueInstant = XMLTimeUtil.getIssueInstant(); ConditionsType conditions = null; List statements = new LinkedList<>(); @@ -303,11 +234,7 @@ public class SAML2Response { * @return */ public ResponseType createResponseType(String ID) { - try { - return new ResponseType(ID, XMLTimeUtil.getIssueInstant()); - } catch (ConfigurationException e) { - throw new RuntimeException(e); - } + return new ResponseType(ID, XMLTimeUtil.getIssueInstant()); } /** @@ -321,8 +248,7 @@ public class SAML2Response { * * @throws ConfigurationException */ - public ResponseType createResponseType(String ID, IssuerInfoHolder issuerInfo, AssertionType assertion) - throws ConfigurationException { + public ResponseType createResponseType(String ID, IssuerInfoHolder issuerInfo, AssertionType assertion){ return JBossSAMLAuthnResponseFactory.createResponseType(ID, issuerInfo, assertion); } diff --git a/saml-core/src/main/java/org/keycloak/saml/processing/core/saml/v2/factories/JBossSAMLAuthnResponseFactory.java b/saml-core/src/main/java/org/keycloak/saml/processing/core/saml/v2/factories/JBossSAMLAuthnResponseFactory.java index 1adc7049de..53853cbfdf 100755 --- a/saml-core/src/main/java/org/keycloak/saml/processing/core/saml/v2/factories/JBossSAMLAuthnResponseFactory.java +++ b/saml-core/src/main/java/org/keycloak/saml/processing/core/saml/v2/factories/JBossSAMLAuthnResponseFactory.java @@ -164,8 +164,7 @@ public class JBossSAMLAuthnResponseFactory { * * @throws ConfigurationException */ - public static ResponseType createResponseType(String ID, IssuerInfoHolder issuerInfo, AssertionType assertionType) - throws ConfigurationException { + public static ResponseType createResponseType(String ID, IssuerInfoHolder issuerInfo, AssertionType assertionType) { XMLGregorianCalendar issueInstant = XMLTimeUtil.getIssueInstant(); ResponseType responseType = new ResponseType(ID, issueInstant); @@ -195,8 +194,7 @@ public class JBossSAMLAuthnResponseFactory { * * @throws ConfigurationException */ - public static ResponseType createResponseType(String ID, IssuerInfoHolder issuerInfo, Element encryptedAssertion) - throws ConfigurationException { + public static ResponseType createResponseType(String ID, IssuerInfoHolder issuerInfo, Element encryptedAssertion) { ResponseType responseType = new ResponseType(ID, XMLTimeUtil.getIssueInstant()); // Issuer diff --git a/saml-core/src/main/java/org/keycloak/saml/processing/core/saml/v2/util/AssertionUtil.java b/saml-core/src/main/java/org/keycloak/saml/processing/core/saml/v2/util/AssertionUtil.java index 887df88aa5..640509f81c 100755 --- a/saml-core/src/main/java/org/keycloak/saml/processing/core/saml/v2/util/AssertionUtil.java +++ b/saml-core/src/main/java/org/keycloak/saml/processing/core/saml/v2/util/AssertionUtil.java @@ -43,7 +43,6 @@ import org.keycloak.saml.common.exceptions.ProcessingException; import org.keycloak.saml.common.exceptions.fed.IssueInstantMissingException; import org.keycloak.saml.common.util.DocumentUtil; import org.keycloak.saml.common.util.StaxUtil; -import org.keycloak.saml.processing.api.saml.v2.response.SAML2Response; import org.keycloak.saml.processing.api.saml.v2.sig.SAML2Signature; import org.keycloak.saml.processing.core.parsers.saml.SAMLParser; import org.keycloak.saml.processing.core.saml.v2.writers.SAMLAssertionWriter; @@ -140,12 +139,7 @@ public class AssertionUtil { * @return */ public static AssertionType createAssertion(String id, NameIDType issuer) { - XMLGregorianCalendar issueInstant = null; - try { - issueInstant = XMLTimeUtil.getIssueInstant(); - } catch (ConfigurationException e) { - throw new RuntimeException(e); - } + XMLGregorianCalendar issueInstant = XMLTimeUtil.getIssueInstant(); AssertionType assertion = new AssertionType(id, issueInstant); assertion.setIssuer(issuer); return assertion; @@ -320,7 +314,8 @@ public class AssertionUtil { } /** - * Check whether the assertion has expired + * Check whether the assertion has expired. + * Processing rules defined in Section 2.5.1.2 of saml-core-2.0-os.pdf. * * @param assertion * diff --git a/saml-core/src/main/java/org/keycloak/saml/processing/core/saml/v2/util/XMLTimeUtil.java b/saml-core/src/main/java/org/keycloak/saml/processing/core/saml/v2/util/XMLTimeUtil.java index 7b45cae988..c194eeb282 100755 --- a/saml-core/src/main/java/org/keycloak/saml/processing/core/saml/v2/util/XMLTimeUtil.java +++ b/saml-core/src/main/java/org/keycloak/saml/processing/core/saml/v2/util/XMLTimeUtil.java @@ -46,17 +46,25 @@ public class XMLTimeUtil { * Add additional time in miliseconds * * @param value calendar whose value needs to be updated - * @param milis + * @param millis * * @return calendar value with the addition * * @throws org.keycloak.saml.common.exceptions.ConfigurationException */ - public static XMLGregorianCalendar add(XMLGregorianCalendar value, long milis) { + public static XMLGregorianCalendar add(XMLGregorianCalendar value, long millis) { + if (value == null) { + return null; + } + XMLGregorianCalendar newVal = (XMLGregorianCalendar) value.clone(); + if (millis == 0) { + return newVal; + } + Duration duration; - duration = DATATYPE_FACTORY.get().newDuration(milis); + duration = DATATYPE_FACTORY.get().newDuration(millis); newVal.add(duration); return newVal; } @@ -65,16 +73,14 @@ public class XMLTimeUtil { * Subtract some miliseconds from the time value * * @param value - * @param milis miliseconds entered in a positive value + * @param millis miliseconds entered in a positive value * * @return * * @throws ConfigurationException */ - public static XMLGregorianCalendar subtract(XMLGregorianCalendar value, long milis) { - if (milis < 0) - throw logger.invalidArgumentError("milis should be a positive value"); - return add(value, -1 * milis); + public static XMLGregorianCalendar subtract(XMLGregorianCalendar value, long millis) { + return add(value, - millis); } /** @@ -106,7 +112,7 @@ public class XMLTimeUtil { * * @throws ConfigurationException */ - public static XMLGregorianCalendar getIssueInstant() throws ConfigurationException { + public static XMLGregorianCalendar getIssueInstant() { return getIssueInstant(getCurrentTimeZoneID()); } @@ -144,7 +150,7 @@ public class XMLTimeUtil { * @return */ public static boolean isValid(XMLGregorianCalendar now, XMLGregorianCalendar notbefore, XMLGregorianCalendar notOnOrAfter) { - int val = 0; + int val; if (notbefore != null) { val = notbefore.compare(now); diff --git a/saml-core/src/main/java/org/keycloak/saml/validators/ConditionsValidator.java b/saml-core/src/main/java/org/keycloak/saml/validators/ConditionsValidator.java new file mode 100644 index 0000000000..d77a4fdf2a --- /dev/null +++ b/saml-core/src/main/java/org/keycloak/saml/validators/ConditionsValidator.java @@ -0,0 +1,234 @@ +/* + * Copyright 2018 Red Hat, Inc. and/or its affiliates + * and other contributors as indicated by the @author tags. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.keycloak.saml.validators; + +import org.keycloak.dom.saml.common.CommonConditionsType; +import org.keycloak.dom.saml.v2.assertion.AudienceRestrictionType; +import org.keycloak.dom.saml.v2.assertion.ConditionAbstractType; +import org.keycloak.dom.saml.v2.assertion.ConditionsType; +import org.keycloak.dom.saml.v2.assertion.OneTimeUseType; +import org.keycloak.dom.saml.v2.assertion.ProxyRestrictionType; +import org.keycloak.saml.processing.core.saml.v2.util.XMLTimeUtil; +import java.net.URI; +import java.util.Arrays; +import java.util.Collections; +import java.util.HashSet; +import java.util.Iterator; +import java.util.Set; +import javax.xml.datatype.DatatypeConstants; +import javax.xml.datatype.XMLGregorianCalendar; +import org.jboss.logging.Logger; + +/** + * Conditions validation as per Section 2.5 of https://docs.oasis-open.org/security/saml/v2.0/saml-core-2.0-os.pdf + * @author hmlnarik + */ +public class ConditionsValidator { + + private static final Logger LOG = Logger.getLogger(ConditionsValidator.class); + + public static enum Result { + VALID { @Override public Result joinResult(Result otherResult) { return otherResult; } }, + INDETERMINATE { @Override public Result joinResult(Result otherResult) { return otherResult == INVALID ? INVALID : INDETERMINATE; } }, + INVALID { @Override public Result joinResult(Result otherResult) { return INVALID; } }; + + /** + * Returns result as per Section 2.5.1.1 + * @param otherResult + * @return + */ + protected abstract Result joinResult(Result otherResult); + }; + + public static class Builder { + + private final String assertionId; + + private final CommonConditionsType conditions; + + private final DestinationValidator destinationValidator; + + private int clockSkewInMillis = 0; + + private final Set allowedAudiences = new HashSet<>(); + + public Builder(String assertionId, CommonConditionsType conditions, DestinationValidator destinationValidator) { + this.assertionId = assertionId; + this.conditions = conditions; + this.destinationValidator = destinationValidator; + } + + public Builder clockSkewInMillis(int clockSkewInMillis) { + this.clockSkewInMillis = clockSkewInMillis; + return this; + } + + public Builder addAllowedAudience(URI... allowedAudiences) { + this.allowedAudiences.addAll(Arrays.asList(allowedAudiences)); + return this; + } + + public ConditionsValidator build() { + return new ConditionsValidator(assertionId, conditions, clockSkewInMillis, allowedAudiences, destinationValidator); + } + + } + + private final CommonConditionsType conditions; + + private final int clockSkewInMillis; + + private final String assertionId; + + private final XMLGregorianCalendar now = XMLTimeUtil.getIssueInstant(); + + private final Set allowedAudiences; + + private final DestinationValidator destinationValidator; + + private int oneTimeConditionsCount = 0; + + private int proxyRestrictionsCount = 0; + + private ConditionsValidator(String assertionId, CommonConditionsType conditions, int clockSkewInMillis, Set allowedAudiences, DestinationValidator destinationValidator) { + this.assertionId = assertionId; + this.conditions = conditions; + this.clockSkewInMillis = clockSkewInMillis; + this.allowedAudiences = allowedAudiences; + this.destinationValidator = destinationValidator; + } + + public boolean isValid() { + if (conditions == null) { + return true; + } + + Result res = validateExpiration(); + if (conditions instanceof ConditionsType) { + res = validateConditions((ConditionsType) conditions, res); + } else { + res = Result.INDETERMINATE; + LOG.infof("Unknown conditions in assertion %s: %s", assertionId, conditions == null ? "" : conditions.getClass().getSimpleName()); + } + + LOG.debugf("Assertion %s validity is %s", assertionId, res.name()); + + return Result.VALID == res; + } + + private Result validateConditions(ConditionsType ct, Result res) { + Iterator it = ct.getConditions() == null + ? Collections.emptySet().iterator() + : ct.getConditions().iterator(); + + while (it.hasNext() && res == Result.VALID) { + ConditionAbstractType cond = it.next(); + Result r; + if (cond instanceof OneTimeUseType) { + r = validateOneTimeUse((OneTimeUseType) cond); + } else if (cond instanceof AudienceRestrictionType) { + r = validateAudienceRestriction((AudienceRestrictionType) cond); + } else if (cond instanceof ProxyRestrictionType) { + r = validateProxyRestriction((ProxyRestrictionType) cond); + } else { + r = Result.INDETERMINATE; + LOG.infof("Unknown condition in assertion %s: %s", assertionId, cond == null ? "" : cond.getClass()); + } + + res = r.joinResult(res); + } + + return res; + } + + /** + * Validate as per Section 2.5.1.2 + * @return + */ + private Result validateExpiration() { + XMLGregorianCalendar notBefore = conditions.getNotBefore(); + XMLGregorianCalendar notOnOrAfter = conditions.getNotOnOrAfter(); + + if (notBefore == null && notOnOrAfter == null) { + return Result.VALID; + } + + if (notBefore != null && notOnOrAfter != null && notBefore.compare(notOnOrAfter) != DatatypeConstants.LESSER) { + return Result.INVALID; + } + + XMLGregorianCalendar updatedNotBefore = XMLTimeUtil.subtract(notBefore, clockSkewInMillis); + XMLGregorianCalendar updatedOnOrAfter = XMLTimeUtil.add(notOnOrAfter, clockSkewInMillis); + + LOG.debugf("Evaluating Conditions of Assertion %s. notBefore=%s, notOnOrAfter=%s", assertionId, notBefore, notOnOrAfter); + boolean valid = XMLTimeUtil.isValid(now, updatedNotBefore, updatedOnOrAfter); + if (! valid) { + LOG.infof("Assertion %s expired.", assertionId); + } + + return valid ? Result.VALID : Result.INVALID; + } + + /** + * Section 2.5.1.4 + * @return + */ + private Result validateAudienceRestriction(AudienceRestrictionType cond) { + for (URI aud : cond.getAudience()) { + for (URI allowedAudience : allowedAudiences) { + if (destinationValidator.validate(aud, allowedAudience)) { + return Result.VALID; + } + } + } + + LOG.infof("Assertion %s is not addressed to this SP.", assertionId); + LOG.debugf("Allowed audiences are: %s", allowedAudiences); + + return Result.INVALID; + } + + /** + * Section 2.5.1.5 + * @return + */ + private Result validateOneTimeUse(OneTimeUseType cond) { + oneTimeConditionsCount++; + + if (oneTimeConditionsCount > 1) { // line 960 + LOG.info("Invalid conditions: Multiple conditions found."); + return Result.INVALID; + } + + return Result.VALID; // See line 963 of spec + } + + /** + * Section 2.5.1.6 + * @return + */ + private Result validateProxyRestriction(ProxyRestrictionType cond) { + proxyRestrictionsCount++; + + if (proxyRestrictionsCount > 1) { // line 992 + LOG.info("Invalid conditions: Multiple conditions found."); + return Result.INVALID; + } + + return Result.VALID; // See line 994 of spec + } +} diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/broker/KcSamlIdPInitiatedSsoTest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/broker/KcSamlIdPInitiatedSsoTest.java index 80c8ffd7fa..a60be28f89 100644 --- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/broker/KcSamlIdPInitiatedSsoTest.java +++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/broker/KcSamlIdPInitiatedSsoTest.java @@ -4,6 +4,8 @@ import org.keycloak.admin.client.resource.ClientsResource; import org.keycloak.admin.client.resource.UsersResource; import org.keycloak.common.util.StreamUtil; import org.keycloak.common.util.StringPropertyReplacer; +import org.keycloak.dom.saml.v2.assertion.AssertionType; +import org.keycloak.dom.saml.v2.assertion.AudienceRestrictionType; import org.keycloak.dom.saml.v2.protocol.ResponseType; import org.keycloak.representations.idm.ClientRepresentation; import org.keycloak.representations.idm.RealmRepresentation; @@ -11,6 +13,7 @@ import org.keycloak.representations.idm.UserRepresentation; import org.keycloak.representations.idm.UserSessionRepresentation; import org.keycloak.saml.common.constants.JBossSAMLURIConstants; import org.keycloak.saml.processing.core.saml.v2.common.SAMLDocumentHolder; +import org.keycloak.saml.processing.core.saml.v2.util.AssertionUtil; import org.keycloak.testsuite.AbstractKeycloakTest; import org.keycloak.testsuite.Assert; import org.keycloak.testsuite.pages.LoginPage; @@ -24,6 +27,7 @@ import org.keycloak.testsuite.util.SamlClientBuilder; import java.io.ByteArrayInputStream; import java.io.IOException; import java.io.InputStream; +import java.net.URI; import java.util.List; import java.util.Map; import java.util.Properties; @@ -38,11 +42,16 @@ import org.openqa.selenium.WebDriver; import org.openqa.selenium.support.ui.ExpectedCondition; import org.openqa.selenium.support.ui.WebDriverWait; +import static org.hamcrest.Matchers.contains; import static org.hamcrest.Matchers.containsInAnyOrder; import static org.hamcrest.Matchers.containsString; +import static org.hamcrest.Matchers.greaterThan; import static org.hamcrest.Matchers.hasSize; +import static org.hamcrest.Matchers.instanceOf; import static org.hamcrest.Matchers.is; -import static org.keycloak.testsuite.broker.BrokerTestConstants.*; +import static org.hamcrest.Matchers.notNullValue; +import static org.keycloak.testsuite.broker.BrokerTestConstants.REALM_CONS_NAME; +import static org.keycloak.testsuite.broker.BrokerTestConstants.REALM_PROV_NAME; import static org.junit.Assert.assertThat; /** @@ -62,6 +71,10 @@ public class KcSamlIdPInitiatedSsoTest extends AbstractKeycloakTest { @Page protected UpdateAccountInformationPage updateAccountInformationPage; + private String urlRealmConsumer2; + private String urlRealmConsumer; + private String urlRealmProvider; + protected String getAuthRoot() { return suiteContext.getAuthServerInfo().getContextRoot().toString(); } @@ -86,14 +99,23 @@ public class KcSamlIdPInitiatedSsoTest extends AbstractKeycloakTest { .forEach(Response::close); } + @Before + public void initRealmUrls() { + urlRealmProvider = getAuthRoot() + "/auth/realms/" + REALM_PROV_NAME; + urlRealmConsumer = getAuthRoot() + "/auth/realms/" + REALM_CONS_NAME; + urlRealmConsumer2 = getAuthRoot() + "/auth/realms/" + REALM_CONS_NAME + "-2"; + } + @Override public void addTestRealms(List testRealms) { + initRealmUrls(); + Properties p = new Properties(); p.put("name.realm.provider", REALM_PROV_NAME); p.put("name.realm.consumer", REALM_CONS_NAME); - p.put("url.realm.provider", getAuthRoot() + "/auth/realms/" + REALM_PROV_NAME); - p.put("url.realm.consumer", getAuthRoot() + "/auth/realms/" + REALM_CONS_NAME); - p.put("url.realm.consumer-2", getAuthRoot() + "/auth/realms/" + REALM_CONS_NAME + "-2"); + p.put("url.realm.provider", urlRealmProvider); + p.put("url.realm.consumer", urlRealmConsumer); + p.put("url.realm.consumer-2", urlRealmConsumer2); testRealms.add(loadFromClasspath("kc3731-provider-realm.json", p)); testRealms.add(loadFromClasspath("kc3731-broker-realm.json", p)); @@ -153,8 +175,20 @@ public class KcSamlIdPInitiatedSsoTest extends AbstractKeycloakTest { wait.until(condition); } + private void assertAudience(ResponseType resp, String expectedAudience) throws Exception { + AssertionType a = AssertionUtil.getAssertion(null, resp, null); + assertThat(a, notNullValue()); + assertThat(a.getConditions(), notNullValue()); + assertThat(a.getConditions().getConditions(), notNullValue()); + assertThat(a.getConditions().getConditions(), hasSize(greaterThan(0))); + assertThat(a.getConditions().getConditions().get(0), instanceOf(AudienceRestrictionType.class)); + + AudienceRestrictionType ar = (AudienceRestrictionType) a.getConditions().getConditions().get(0); + assertThat(ar.getAudience(), contains(URI.create(expectedAudience))); + } + @Test - public void testProviderIdpInitiatedLoginToApp() { + public void testProviderIdpInitiatedLoginToApp() throws Exception { SAMLDocumentHolder samlResponse = new SamlClientBuilder() .navigateTo(getSamlIdpInitiatedUrl(REALM_PROV_NAME, "samlbroker")) // Login in provider realm @@ -166,6 +200,7 @@ public class KcSamlIdPInitiatedSsoTest extends AbstractKeycloakTest { assertThat(ob, Matchers.isSamlResponse(JBossSAMLURIConstants.STATUS_SUCCESS)); ResponseType resp = (ResponseType) ob; assertThat(resp.getDestination(), is(getSamlBrokerIdpInitiatedUrl(REALM_CONS_NAME, "sales"))); + assertAudience(resp, getSamlBrokerIdpInitiatedUrl(REALM_CONS_NAME, "sales")); return ob; }) .build() @@ -178,11 +213,12 @@ public class KcSamlIdPInitiatedSsoTest extends AbstractKeycloakTest { assertThat(samlResponse.getSamlObject(), Matchers.isSamlResponse(JBossSAMLURIConstants.STATUS_SUCCESS)); ResponseType resp = (ResponseType) samlResponse.getSamlObject(); - assertThat(resp.getDestination(), is("http://localhost:8180/auth/realms/" + REALM_CONS_NAME + "/app/auth")); + assertThat(resp.getDestination(), is(urlRealmConsumer + "/app/auth")); + assertAudience(resp, urlRealmConsumer + "/app/auth"); } @Test - public void testConsumerIdpInitiatedLoginToApp() { + public void testConsumerIdpInitiatedLoginToApp() throws Exception { SAMLDocumentHolder samlResponse = new SamlClientBuilder() .navigateTo(getSamlIdpInitiatedUrl(REALM_CONS_NAME, "sales")) // Request login via saml-leaf @@ -201,6 +237,7 @@ public class KcSamlIdPInitiatedSsoTest extends AbstractKeycloakTest { assertThat(ob, Matchers.isSamlResponse(JBossSAMLURIConstants.STATUS_SUCCESS)); ResponseType resp = (ResponseType) ob; assertThat(resp.getDestination(), is(getSamlBrokerUrl(REALM_CONS_NAME))); + assertAudience(resp, urlRealmConsumer); return ob; }) .build() @@ -213,11 +250,12 @@ public class KcSamlIdPInitiatedSsoTest extends AbstractKeycloakTest { assertThat(samlResponse.getSamlObject(), Matchers.isSamlResponse(JBossSAMLURIConstants.STATUS_SUCCESS)); ResponseType resp = (ResponseType) samlResponse.getSamlObject(); - assertThat(resp.getDestination(), is("http://localhost:8180/auth/realms/" + REALM_CONS_NAME + "/app/auth")); + assertThat(resp.getDestination(), is(urlRealmConsumer + "/app/auth")); + assertAudience(resp, urlRealmConsumer + "/app/auth"); } @Test - public void testTwoConsequentIdpInitiatedLogins() { + public void testTwoConsequentIdpInitiatedLogins() throws Exception { SAMLDocumentHolder samlResponse = new SamlClientBuilder() .navigateTo(getSamlIdpInitiatedUrl(REALM_PROV_NAME, "samlbroker")) // Login in provider realm @@ -229,6 +267,7 @@ public class KcSamlIdPInitiatedSsoTest extends AbstractKeycloakTest { assertThat(ob, Matchers.isSamlResponse(JBossSAMLURIConstants.STATUS_SUCCESS)); ResponseType resp = (ResponseType) ob; assertThat(resp.getDestination(), is(getSamlBrokerIdpInitiatedUrl(REALM_CONS_NAME, "sales"))); + assertAudience(resp, getSamlBrokerIdpInitiatedUrl(REALM_CONS_NAME, "sales")); return ob; }) .build() @@ -241,12 +280,12 @@ public class KcSamlIdPInitiatedSsoTest extends AbstractKeycloakTest { .transformObject(ob -> { assertThat(ob, Matchers.isSamlResponse(JBossSAMLURIConstants.STATUS_SUCCESS)); ResponseType resp = (ResponseType) ob; - assertThat(resp.getDestination(), is("http://localhost:8180/auth/realms/" + REALM_CONS_NAME + "/app/auth")); + assertThat(resp.getDestination(), is(urlRealmConsumer + "/app/auth")); + assertAudience(resp, urlRealmConsumer + "/app/auth"); return null; }) .build() - // Now login to the second app .navigateTo(getSamlIdpInitiatedUrl(REALM_PROV_NAME, "samlbroker-2")) @@ -259,6 +298,7 @@ public class KcSamlIdPInitiatedSsoTest extends AbstractKeycloakTest { assertThat(ob, Matchers.isSamlResponse(JBossSAMLURIConstants.STATUS_SUCCESS)); ResponseType resp = (ResponseType) ob; assertThat(resp.getDestination(), is(getSamlBrokerIdpInitiatedUrl(REALM_CONS_NAME, "sales2"))); + assertAudience(resp, getSamlBrokerIdpInitiatedUrl(REALM_CONS_NAME, "sales2")); return ob; }) .build() @@ -267,16 +307,17 @@ public class KcSamlIdPInitiatedSsoTest extends AbstractKeycloakTest { assertThat(samlResponse.getSamlObject(), Matchers.isSamlResponse(JBossSAMLURIConstants.STATUS_SUCCESS)); ResponseType resp = (ResponseType) samlResponse.getSamlObject(); - assertThat(resp.getDestination(), is("http://localhost:8180/auth/realms/" + REALM_CONS_NAME + "/app/auth/sales2/saml")); + assertThat(resp.getDestination(), is(urlRealmConsumer + "/app/auth2/saml")); + assertAudience(resp, urlRealmConsumer + "/app/auth2"); assertSingleUserSession(REALM_CONS_NAME, CONSUMER_CHOSEN_USERNAME, - "http://localhost:8180/auth/realms/" + REALM_CONS_NAME + "/app/auth", - "http://localhost:8180/auth/realms/" + REALM_CONS_NAME + "/app/auth2" + urlRealmConsumer + "/app/auth", + urlRealmConsumer + "/app/auth2" ); assertSingleUserSession(REALM_PROV_NAME, PROVIDER_REALM_USER_NAME, - getAuthRoot() + "/auth/realms/" + REALM_CONS_NAME, - getAuthRoot() + "/auth/realms/" + REALM_CONS_NAME + "-2" + urlRealmConsumer + "/broker/saml-leaf/endpoint/clients/sales", + urlRealmConsumer + "/broker/saml-leaf/endpoint/clients/sales2" ); } diff --git a/testsuite/integration-arquillian/tests/base/src/test/resources/org/keycloak/testsuite/broker/kc3731-broker-realm.json b/testsuite/integration-arquillian/tests/base/src/test/resources/org/keycloak/testsuite/broker/kc3731-broker-realm.json index 0ed5ed341b..76a68ad616 100644 --- a/testsuite/integration-arquillian/tests/base/src/test/resources/org/keycloak/testsuite/broker/kc3731-broker-realm.json +++ b/testsuite/integration-arquillian/tests/base/src/test/resources/org/keycloak/testsuite/broker/kc3731-broker-realm.json @@ -47,7 +47,7 @@ "saml.signature.algorithm": "RSA_SHA512", "saml.signing.certificate": "MIIB1DCCAT0CBgFJGVacCDANBgkqhkiG9w0BAQsFADAwMS4wLAYDVQQDEyVodHRwOi8vbG9jYWxob3N0OjgwODAvc2FsZXMtcG9zdC1lbmMvMB4XDTE0MTAxNjE0MjA0NloXDTI0MTAxNjE0MjIyNlowMDEuMCwGA1UEAxMlaHR0cDovL2xvY2FsaG9zdDo4MDgwL3NhbGVzLXBvc3QtZW5jLzCBnzANBgkqhkiG9w0BAQEFAAOBjQAwgYkCgYEA2+5MCT5BnVN+IYnKZcH6ev1pjXGi4feE0nOycq/VJ3aeaZMi4G9AxOxCBPupErOC7Kgm/Bw5AdJyw+Q12wSRXfJ9FhqCrLXpb7YOhbVSTJ8De5O8mW35DxAlh/cxe9FXjqPb286wKTUZ3LfGYR+X235UQeCTAPS/Ufi21EXaEikCAwEAATANBgkqhkiG9w0BAQsFAAOBgQBMrfGD9QFfx5v7ld/OAto5rjkTe3R1Qei8XRXfcs83vLaqEzjEtTuLGrJEi55kXuJgBpVmQpnwCCkkjSy0JxbqLDdVi9arfWUxEGmOr01ZHycELhDNaQcFqVMPr5kRHIHgktT8hK2IgCvd3Fy9/JCgUgCPxKfhwecyEOKxUc857g==", "saml.signing.private.key": "MIICXQIBAAKBgQDb7kwJPkGdU34hicplwfp6/WmNcaLh94TSc7Jyr9Undp5pkyLgb0DE7EIE+6kSs4LsqCb8HDkB0nLD5DXbBJFd8n0WGoKstelvtg6FtVJMnwN7k7yZbfkPECWH9zF70VeOo9vbzrApNRnct8ZhH5fbflRB4JMA9L9R+LbURdoSKQIDAQABAoGBANtbZG9bruoSGp2s5zhzLzd4hczT6Jfk3o9hYjzNb5Z60ymN3Z1omXtQAdEiiNHkRdNxK+EM7TcKBfmoJqcaeTkW8cksVEAW23ip8W9/XsLqmbU2mRrJiKa+KQNDSHqJi1VGyimi4DDApcaqRZcaKDFXg2KDr/Qt5JFD/o9IIIPZAkEA+ZENdBIlpbUfkJh6Ln+bUTss/FZ1FsrcPZWu13rChRMrsmXsfzu9kZUWdUeQ2Dj5AoW2Q7L/cqdGXS7Mm5XhcwJBAOGZq9axJY5YhKrsksvYRLhQbStmGu5LG75suF+rc/44sFq+aQM7+oeRr4VY88Mvz7mk4esdfnk7ae+cCazqJvMCQQCx1L1cZw3yfRSn6S6u8XjQMjWE/WpjulujeoRiwPPY9WcesOgLZZtYIH8nRL6ehEJTnMnahbLmlPFbttxPRUanAkA11MtSIVcKzkhp2KV2ipZrPJWwI18NuVJXb+3WtjypTrGWFZVNNkSjkLnHIeCYlJIGhDd8OL9zAiBXEm6kmgLNAkBWAg0tK2hCjvzsaA505gWQb4X56uKWdb0IzN+fOLB3Qt7+fLqbVQNQoNGzqey6B4MoS1fUKAStqdGTFYPG/+9t", - "saml_assertion_consumer_url_post" : "http://localhost:8180/auth/realms/${name.realm.consumer}/app/auth/sales2/saml", + "saml_assertion_consumer_url_post" : "http://localhost:8180/auth/realms/${name.realm.consumer}/app/auth2/saml", "saml_idp_initiated_sso_url_name" : "sales2" }, "baseUrl": "http://localhost:8180/auth/realms/${name.realm.consumer}/app/auth2", diff --git a/testsuite/integration-arquillian/tests/base/src/test/resources/org/keycloak/testsuite/broker/kc3731-provider-realm.json b/testsuite/integration-arquillian/tests/base/src/test/resources/org/keycloak/testsuite/broker/kc3731-provider-realm.json index d66011825a..82f4f18929 100644 --- a/testsuite/integration-arquillian/tests/base/src/test/resources/org/keycloak/testsuite/broker/kc3731-provider-realm.json +++ b/testsuite/integration-arquillian/tests/base/src/test/resources/org/keycloak/testsuite/broker/kc3731-provider-realm.json @@ -27,12 +27,32 @@ "saml.server.signature" : "false", "saml_assertion_consumer_url_post" : "${url.realm.consumer}/broker/saml-leaf/endpoint/clients/sales", "saml_force_name_id_format" : "false", - "saml_idp_initiated_sso_url_name" : "samlbroker", "saml_name_id_format": "email", "saml_single_logout_service_url_post" : "${url.realm.consumer}/broker/saml-leaf/endpoint" } }, { - "clientId": "${url.realm.consumer-2}", + "clientId": "${url.realm.consumer}/broker/saml-leaf/endpoint/clients/sales", + "enabled": true, + "protocol": "saml", + "fullScopeAllowed": true, + "redirectUris": [ + "${url.realm.consumer}/broker/saml-leaf/endpoint" + ], + "attributes" : { + "saml_name_id_format": "email", + "saml.assertion.signature" : "false", + "saml.authnstatement" : "true", + "saml.client.signature" : "false", + "saml.encrypt" : "false", + "saml.force.post.binding" : "true", + "saml.server.signature" : "false", + "saml_assertion_consumer_url_post" : "${url.realm.consumer}/broker/saml-leaf/endpoint/clients/sales", + "saml_force_name_id_format" : "false", + "saml_idp_initiated_sso_url_name" : "samlbroker", + "saml_single_logout_service_url_post" : "${url.realm.consumer}/broker/saml-leaf/endpoint" + } + }, { + "clientId": "${url.realm.consumer}/broker/saml-leaf/endpoint/clients/sales2", "enabled": true, "protocol": "saml", "fullScopeAllowed": true,