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 820717b1ac..86cab91762 100755
--- a/services/src/main/java/org/keycloak/broker/saml/SAMLEndpoint.java
+++ b/services/src/main/java/org/keycloak/broker/saml/SAMLEndpoint.java
@@ -444,6 +444,17 @@ public class SAMLEndpoint {
assertionElement = DocumentUtil.getElement(holder.getSamlDocument(), new QName(JBossSAMLConstants.ASSERTION.get()));
}
+ // Validate the response Issuer
+ final String responseIssuer = responseType.getIssuer() != null ? responseType.getIssuer().getValue(): null;
+ final boolean responseIssuerValidationSuccess = config.getIdpEntityId() == null ||
+ (responseIssuer != null && responseIssuer.equals(config.getIdpEntityId()));
+ if (!responseIssuerValidationSuccess) {
+ logger.errorf("Response Issuer validation failed: expected %s, actual %s", config.getIdpEntityId(), responseIssuer);
+ event.event(EventType.IDENTITY_PROVIDER_RESPONSE);
+ event.error(Errors.INVALID_SAML_RESPONSE);
+ return ErrorPage.error(session, authSession, Response.Status.BAD_REQUEST, Messages.INVALID_REQUESTER);
+ }
+
// Validate InResponseTo attribute: must match the generated request ID
String expectedRequestId = authSession.getClientNote(SamlProtocol.SAML_REQUEST_ID_BROKER);
final boolean inResponseToValidationSuccess = validateInResponseToAttribute(responseType, expectedRequestId);
@@ -470,7 +481,20 @@ public class SAMLEndpoint {
// This methods writes the parsed and decrypted id back on the responseType parameter:
AssertionUtil.decryptId(responseType, keys.getPrivateKey());
}
+
AssertionType assertion = responseType.getAssertions().get(0).getAssertion();
+
+ // Validate the assertion Issuer
+ final String assertionIssuer = assertion.getIssuer() != null ? assertion.getIssuer().getValue(): null;
+ final boolean assertionIssuerValidationSuccess = config.getIdpEntityId() == null ||
+ (assertionIssuer != null && assertionIssuer.equals(config.getIdpEntityId()));
+ if (!assertionIssuerValidationSuccess) {
+ logger.errorf("Assertion Issuer validation failed: expected %s, actual %s", config.getIdpEntityId(), assertionIssuer);
+ event.event(EventType.IDENTITY_PROVIDER_RESPONSE);
+ event.error(Errors.INVALID_SAML_RESPONSE);
+ return ErrorPage.error(session, authSession, Response.Status.BAD_REQUEST, Messages.INVALID_REQUESTER);
+ }
+
NameIDType subjectNameID = getSubjectNameID(assertion);
String principal = getPrincipal(assertion);
diff --git a/services/src/main/java/org/keycloak/broker/saml/SAMLIdentityProviderConfig.java b/services/src/main/java/org/keycloak/broker/saml/SAMLIdentityProviderConfig.java
index 19cfd07d09..65f89552a6 100755
--- a/services/src/main/java/org/keycloak/broker/saml/SAMLIdentityProviderConfig.java
+++ b/services/src/main/java/org/keycloak/broker/saml/SAMLIdentityProviderConfig.java
@@ -35,6 +35,7 @@ public class SAMLIdentityProviderConfig extends IdentityProviderModel {
public static final XmlKeyInfoKeyNameTransformer DEFAULT_XML_KEY_INFO_KEY_NAME_TRANSFORMER = XmlKeyInfoKeyNameTransformer.NONE;
public static final String ENTITY_ID = "entityId";
+ public static final String IDP_ENTITY_ID = "idpEntityId";
public static final String ADD_EXTENSIONS_ELEMENT_WITH_KEY_INFO = "addExtensionsElementWithKeyInfo";
public static final String BACKCHANNEL_SUPPORTED = "backchannelSupported";
public static final String ENCRYPTION_PUBLIC_KEY = "encryptionPublicKey";
@@ -78,6 +79,14 @@ public class SAMLIdentityProviderConfig extends IdentityProviderModel {
getConfig().put(ENTITY_ID, entityId);
}
+ public String getIdpEntityId() {
+ return getConfig().get(IDP_ENTITY_ID);
+ }
+
+ public void setIdpEntityId(String idpEntityId) {
+ getConfig().put(IDP_ENTITY_ID, idpEntityId);
+ }
+
public String getSingleSignOnServiceUrl() {
return getConfig().get(SINGLE_SIGN_ON_SERVICE_URL);
}
diff --git a/services/src/main/java/org/keycloak/broker/saml/SAMLIdentityProviderFactory.java b/services/src/main/java/org/keycloak/broker/saml/SAMLIdentityProviderFactory.java
index af4df91757..57699444cf 100755
--- a/services/src/main/java/org/keycloak/broker/saml/SAMLIdentityProviderFactory.java
+++ b/services/src/main/java/org/keycloak/broker/saml/SAMLIdentityProviderFactory.java
@@ -123,6 +123,7 @@ public class SAMLIdentityProviderFactory extends AbstractIdentityProviderFactory
}
}
+ samlIdentityProviderConfig.setIdpEntityId(entityType.getEntityID());
samlIdentityProviderConfig.setSingleLogoutServiceUrl(singleLogoutServiceUrl);
samlIdentityProviderConfig.setSingleSignOnServiceUrl(singleSignOnServiceUrl);
samlIdentityProviderConfig.setWantAuthnRequestsSigned(idpDescriptor.isWantAuthnRequestsSigned());
diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/admin/IdentityProviderTest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/admin/IdentityProviderTest.java
index 5d57e70729..b1bf2e16de 100755
--- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/admin/IdentityProviderTest.java
+++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/admin/IdentityProviderTest.java
@@ -924,7 +924,8 @@ public class IdentityProviderTest extends AbstractAdminTest {
"signingCertificate",
"addExtensionsElementWithKeyInfo",
"loginHint",
- "hideOnLoginPage"
+ "hideOnLoginPage",
+ "idpEntityId"
));
assertThat(config, hasEntry("validateSignature", "true"));
assertThat(config, hasEntry("singleLogoutServiceUrl", "http://localhost:8080/auth/realms/master/protocol/saml"));
@@ -935,6 +936,7 @@ public class IdentityProviderTest extends AbstractAdminTest {
assertThat(config, hasEntry("addExtensionsElementWithKeyInfo", "false"));
assertThat(config, hasEntry("nameIDPolicyFormat", "urn:oasis:names:tc:SAML:2.0:nameid-format:persistent"));
assertThat(config, hasEntry("hideOnLoginPage", "true"));
+ assertThat(config, hasEntry("idpEntityId", "http://localhost:8080/auth/realms/master"));
assertThat(config, hasEntry(is("signingCertificate"), notNullValue()));
}
diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/broker/KcSamlBrokerTest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/broker/KcSamlBrokerTest.java
index 8ed39e8961..29646e39ce 100644
--- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/broker/KcSamlBrokerTest.java
+++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/broker/KcSamlBrokerTest.java
@@ -4,6 +4,7 @@ import org.keycloak.admin.client.resource.IdentityProviderResource;
import org.keycloak.admin.client.resource.RealmResource;
import org.keycloak.admin.client.resource.UserResource;
import com.google.common.collect.ImmutableMap;
+import org.keycloak.broker.saml.SAMLIdentityProviderConfig;
import org.keycloak.broker.saml.mappers.AttributeToRoleMapper;
import org.keycloak.broker.saml.mappers.UserAttributeMapper;
import org.keycloak.dom.saml.v2.assertion.AssertionType;
@@ -25,9 +26,12 @@ import org.keycloak.saml.processing.api.saml.v2.request.SAML2Request;
import org.keycloak.saml.processing.core.parsers.saml.protocol.SAMLProtocolQNames;
import org.keycloak.saml.processing.core.saml.v2.common.SAMLDocumentHolder;
import org.keycloak.testsuite.saml.AbstractSamlTest;
+import org.keycloak.testsuite.updaters.IdentityProviderAttributeUpdater;
import org.keycloak.testsuite.util.SamlClient;
import org.keycloak.testsuite.util.SamlClient.Binding;
import org.keycloak.testsuite.util.SamlClientBuilder;
+
+import java.io.Closeable;
import java.util.Arrays;
import java.util.Collections;
import java.util.HashMap;
@@ -342,6 +346,70 @@ public final class KcSamlBrokerTest extends AbstractAdvancedBrokerTest {
}
+ // Issue #10982
+ @Test
+ public void loginWithIdpEntityIdCorrect() throws Exception {
+ // Set the expected IDP Entity ID to the correct value
+ try (Closeable idpUpdater = new IdentityProviderAttributeUpdater(identityProviderResource)
+ .setAttribute(SAMLIdentityProviderConfig.IDP_ENTITY_ID, "https://localhost:8543/auth/realms/provider")
+ .update())
+ {
+ AuthnRequestType loginRep = SamlClient.createLoginRequestDocument(AbstractSamlTest.SAML_CLIENT_ID_SALES_POST + ".dot/ted", getConsumerRoot() + "/sales-post/saml", null);
+
+ Document doc = SAML2Request.convert(loginRep);
+
+ SAMLDocumentHolder samlResponse = new SamlClientBuilder()
+ .authnRequest(getConsumerSamlEndpoint(bc.consumerRealmName()), doc, Binding.POST).build() // Request to consumer IdP
+ .login().idp(bc.getIDPAlias()).build()
+
+ .processSamlResponse(Binding.POST) // AuthnRequest to producer IdP
+ .targetAttributeSamlRequest()
+ .build()
+
+ .login().user(bc.getUserLogin(), bc.getUserPassword()).build()
+
+ .processSamlResponse(Binding.POST) // Response from producer IdP
+ .build()
+
+ // first-broker flow
+ .updateProfile().firstName("a").lastName("b").email(bc.getUserEmail()).username(bc.getUserLogin()).build()
+ .followOneRedirect()
+
+ .getSamlResponse(Binding.POST); // Response from consumer IdP
+
+ Assert.assertThat(samlResponse, Matchers.notNullValue());
+ Assert.assertThat(samlResponse.getSamlObject(), isSamlResponse(JBossSAMLURIConstants.STATUS_SUCCESS));
+ }
+ }
+
+ // Issue #10982
+ @Test
+ public void loginWithIdpEntityIdMismatchResponse() throws Exception {
+ // Set the expected IDP Entity ID to a wrong value
+ try (Closeable idpUpdater = new IdentityProviderAttributeUpdater(identityProviderResource)
+ .setAttribute(SAMLIdentityProviderConfig.IDP_ENTITY_ID, "http://my.custom.idp.entity.id")
+ .update())
+ {
+ AuthnRequestType loginRep = SamlClient.createLoginRequestDocument(AbstractSamlTest.SAML_CLIENT_ID_SALES_POST + ".dot/ted", getConsumerRoot() + "/sales-post/saml", null);
+
+ Document doc = SAML2Request.convert(loginRep);
+
+ new SamlClientBuilder()
+ .authnRequest(getConsumerSamlEndpoint(bc.consumerRealmName()), doc, Binding.POST).build() // Request to consumer IdP
+ .login().idp(bc.getIDPAlias()).build()
+
+ .processSamlResponse(Binding.POST) // AuthnRequest to producer IdP
+ .targetAttributeSamlRequest()
+ .build()
+
+ .login().user(bc.getUserLogin(), bc.getUserPassword()).build()
+
+ .processSamlResponse(Binding.POST) // Response from producer IdP
+ .build()
+ .execute(hr -> assertThat(hr, statusCodeIsHC(Response.Status.BAD_REQUEST))); // Response from consumer IdP
+ }
+ }
+
// KEYCLOAK-17935
@Test
public void loginInResponseToMismatch() throws Exception {
diff --git a/themes/src/main/resources/theme/base/admin/messages/admin-messages_en.properties b/themes/src/main/resources/theme/base/admin/messages/admin-messages_en.properties
index b31c4aaaee..d20231e264 100644
--- a/themes/src/main/resources/theme/base/admin/messages/admin-messages_en.properties
+++ b/themes/src/main/resources/theme/base/admin/messages/admin-messages_en.properties
@@ -772,6 +772,8 @@ import-from-file=Import from file
identity-provider.import-from-file.tooltip=Import metadata from a downloaded IDP discovery descriptor.
identity-provider.saml.entity-id=Service Provider Entity ID
identity-provider.saml.entity-id.tooltip=The Entity ID that will be used to uniquely identify this SAML Service Provider
+identity-provider.saml.idp-entity-id=Identity Provider Entity ID
+identity-provider.saml.idp-entity-id.tooltip=The Entity ID used to validate the Issuer for received SAML assertions. If empty, no Issuer validation is performed.
identity-provider.saml.protocol-endpoints.saml=SAML 2.0 Service Provider Metadata
identity-provider.saml.protocol-endpoints.saml.tooltip=Shows the configuration of the Service Provider endpoint
identity-provider.saml.attribute-consuming-service-index=Attribute Consuming Service Index
diff --git a/themes/src/main/resources/theme/base/admin/resources/partials/realm-identity-provider-saml.html b/themes/src/main/resources/theme/base/admin/resources/partials/realm-identity-provider-saml.html
index 01421798fd..5b2fdb93cf 100755
--- a/themes/src/main/resources/theme/base/admin/resources/partials/realm-identity-provider-saml.html
+++ b/themes/src/main/resources/theme/base/admin/resources/partials/realm-identity-provider-saml.html
@@ -140,6 +140,13 @@
{{:: 'identity-provider.saml.entity-id.tooltip' | translate}}
+