From f43519a16edf2f6c8065bd97c4669d180253fead Mon Sep 17 00:00:00 2001 From: Hynek Mlnarik Date: Thu, 26 Jul 2018 19:00:38 +0200 Subject: [PATCH] KEYCLOAK-6708 Fix NPE when email not set for email NameIDFormat --- .../api/saml/v2/request/SAML2Request.java | 2 +- .../api/saml/v2/response/SAML2Response.java | 2 +- .../keycloak/protocol/saml/SamlProtocol.java | 76 +++++---- .../profile/ecp/SamlEcpProfileService.java | 2 +- .../updaters/IdentityProviderCreator.java | 5 + .../org/keycloak/testsuite/util/Matchers.java | 5 +- .../SamlStatusResponseTypeMatcher.java | 37 ++++- .../testsuite/saml/AbstractSamlTest.java | 5 + .../keycloak/testsuite/saml/BrokerTest.java | 156 ++++++++++++++++++ .../keycloak/testsuite/saml/LogoutTest.java | 7 - .../testsuite/saml/SamlConsentTest.java | 13 +- 11 files changed, 257 insertions(+), 53 deletions(-) create mode 100644 testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/saml/BrokerTest.java diff --git a/saml-core/src/main/java/org/keycloak/saml/processing/api/saml/v2/request/SAML2Request.java b/saml-core/src/main/java/org/keycloak/saml/processing/api/saml/v2/request/SAML2Request.java index 5a131d28d3..8c558d19b0 100755 --- a/saml-core/src/main/java/org/keycloak/saml/processing/api/saml/v2/request/SAML2Request.java +++ b/saml-core/src/main/java/org/keycloak/saml/processing/api/saml/v2/request/SAML2Request.java @@ -107,7 +107,7 @@ public class SAML2Request { // Create a default NameIDPolicy NameIDPolicyType nameIDPolicy = new NameIDPolicyType(); nameIDPolicy.setAllowCreate(Boolean.TRUE); - nameIDPolicy.setFormat(URI.create(this.nameIDFormat)); + nameIDPolicy.setFormat(this.nameIDFormat == null ? null : URI.create(this.nameIDFormat)); authnRequest.setNameIDPolicy(nameIDPolicy); 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 e9a86b15cf..ab78fffdf6 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 @@ -244,7 +244,7 @@ public class SAML2Response { // subject -> nameid NameIDType nameIDType = new NameIDType(); - nameIDType.setFormat(URI.create(idp.getNameIDFormat())); + nameIDType.setFormat(idp.getNameIDFormat() == null ? null : URI.create(idp.getNameIDFormat())); nameIDType.setValue(idp.getNameIDFormatValue()); SubjectType.STSubType subType = new SubjectType.STSubType(); diff --git a/services/src/main/java/org/keycloak/protocol/saml/SamlProtocol.java b/services/src/main/java/org/keycloak/protocol/saml/SamlProtocol.java index 49d2e3c13f..52d271aa11 100755 --- a/services/src/main/java/org/keycloak/protocol/saml/SamlProtocol.java +++ b/services/src/main/java/org/keycloak/protocol/saml/SamlProtocol.java @@ -165,7 +165,7 @@ public class SamlProtocol implements LoginProtocol { try { ClientModel client = authSession.getClient(); - if ("true".equals(client.getAttribute(SAML_IDP_INITIATED_LOGIN))) { + if ("true".equals(authSession.getClientNote(SAML_IDP_INITIATED_LOGIN))) { if (error == Error.CANCELLED_BY_USER) { UriBuilder builder = RealmsResource.protocolUrl(uriInfo).path(SamlService.class, "idpInitiatedSSO"); Map params = new HashMap<>(); @@ -178,39 +178,48 @@ public class SamlProtocol implements LoginProtocol { return ErrorPage.error(session, authSession, Response.Status.BAD_REQUEST, translateErrorToIdpInitiatedErrorMessage(error)); } } else { - SAML2ErrorResponseBuilder builder = new SAML2ErrorResponseBuilder().destination(authSession.getRedirectUri()).issuer(getResponseIssuer(realm)).status(translateErrorToSAMLStatus(error).get()); - try { - JaxrsSAML2BindingBuilder binding = new JaxrsSAML2BindingBuilder().relayState(authSession.getClientNote(GeneralConstants.RELAY_STATE)); - SamlClient samlClient = new SamlClient(client); - KeyManager keyManager = session.keys(); - if (samlClient.requiresRealmSignature()) { - KeyManager.ActiveRsaKey keys = keyManager.getActiveRsaKey(realm); - String keyName = samlClient.getXmlSigKeyInfoKeyNameTransformer().getKeyName(keys.getKid(), keys.getCertificate()); - String canonicalization = samlClient.getCanonicalizationMethod(); - if (canonicalization != null) { - binding.canonicalizationMethod(canonicalization); - } - binding.signatureAlgorithm(samlClient.getSignatureAlgorithm()).signWith(keyName, keys.getPrivateKey(), keys.getPublicKey(), keys.getCertificate()).signDocument(); - } - // There is no support for encrypting status messages in SAML. - // Only assertions, attributes, base ID and name ID can be encrypted - // See Chapter 6 of saml-core-2.0-os.pdf - Document document = builder.buildDocument(); - return buildErrorResponse(authSession, binding, document); - } catch (Exception e) { - return ErrorPage.error(session, authSession, Response.Status.BAD_REQUEST, Messages.FAILED_TO_PROCESS_RESPONSE); - } + return samlErrorMessage( + authSession, new SamlClient(client), isPostBinding(authSession), + authSession.getRedirectUri(), translateErrorToSAMLStatus(error), authSession.getClientNote(GeneralConstants.RELAY_STATE) + ); } } finally { new AuthenticationSessionManager(session).removeAuthenticationSession(realm, authSession, true); } } - protected Response buildErrorResponse(AuthenticationSessionModel authSession, JaxrsSAML2BindingBuilder binding, Document document) throws ConfigurationException, ProcessingException, IOException { - if (isPostBinding(authSession)) { - return binding.postBinding(document).response(authSession.getRedirectUri()); + private Response samlErrorMessage( + AuthenticationSessionModel authSession, SamlClient samlClient, boolean isPostBinding, + String destination, JBossSAMLURIConstants statusDetail, String relayState) { + JaxrsSAML2BindingBuilder binding = new JaxrsSAML2BindingBuilder().relayState(relayState); + SAML2ErrorResponseBuilder builder = new SAML2ErrorResponseBuilder().destination(destination).issuer(getResponseIssuer(realm)).status(statusDetail.get()); + KeyManager keyManager = session.keys(); + if (samlClient.requiresRealmSignature()) { + KeyManager.ActiveRsaKey keys = keyManager.getActiveRsaKey(realm); + String keyName = samlClient.getXmlSigKeyInfoKeyNameTransformer().getKeyName(keys.getKid(), keys.getCertificate()); + String canonicalization = samlClient.getCanonicalizationMethod(); + if (canonicalization != null) { + binding.canonicalizationMethod(canonicalization); + } + binding.signatureAlgorithm(samlClient.getSignatureAlgorithm()).signWith(keyName, keys.getPrivateKey(), keys.getPublicKey(), keys.getCertificate()).signDocument(); + } + + try { + // There is no support for encrypting status messages in SAML. + // Only assertions, attributes, base ID and name ID can be encrypted + // See Chapter 6 of saml-core-2.0-os.pdf + Document document = builder.buildDocument(); + return buildErrorResponse(isPostBinding, destination, binding, document); + } catch (Exception e) { + return ErrorPage.error(session, authSession, Response.Status.BAD_REQUEST, Messages.FAILED_TO_PROCESS_RESPONSE); + } + } + + protected Response buildErrorResponse(boolean isPostBinding, String destination, JaxrsSAML2BindingBuilder binding, Document document) throws ConfigurationException, ProcessingException, IOException { + if (isPostBinding) { + return binding.postBinding(document).response(destination); } else { - return binding.redirectBinding(document).response(authSession.getRedirectUri()); + return binding.redirectBinding(document).response(destination); } } @@ -302,7 +311,11 @@ public class SamlProtocol implements LoginProtocol { protected String getNameId(String nameIdFormat, CommonClientSessionModel clientSession, UserSessionModel userSession) { if (nameIdFormat.equals(JBossSAMLURIConstants.NAMEID_FORMAT_EMAIL.get())) { - return userSession.getUser().getEmail(); + final String email = userSession.getUser().getEmail(); + if (email == null) { + logger.debugf("E-mail of the user %s has to be set for %s NameIDFormat", userSession.getUser().getUsername(), JBossSAMLURIConstants.NAMEID_FORMAT_EMAIL.get()); + } + return email; } else if (nameIdFormat.equals(JBossSAMLURIConstants.NAMEID_FORMAT_TRANSIENT.get())) { // "G-" stands for "generated" Add this for the slight possibility of collisions. return "G-" + UUID.randomUUID().toString(); @@ -365,6 +378,13 @@ public class SamlProtocol implements LoginProtocol { String nameIdFormat = getNameIdFormat(samlClient, clientSession); String nameId = getNameId(nameIdFormat, clientSession, userSession); + if (nameId == null) { + return samlErrorMessage( + null, samlClient, isPostBinding(clientSession), + redirectUri, JBossSAMLURIConstants.STATUS_INVALID_NAMEIDPOLICY, relayState + ); + } + // save NAME_ID and format in clientSession as they may be persistent or transient or email and not username // we'll need to send this back on a logout clientSession.setNote(SAML_NAME_ID, nameId); diff --git a/services/src/main/java/org/keycloak/protocol/saml/profile/ecp/SamlEcpProfileService.java b/services/src/main/java/org/keycloak/protocol/saml/profile/ecp/SamlEcpProfileService.java index c15444bbc5..56b804b2d6 100755 --- a/services/src/main/java/org/keycloak/protocol/saml/profile/ecp/SamlEcpProfileService.java +++ b/services/src/main/java/org/keycloak/protocol/saml/profile/ecp/SamlEcpProfileService.java @@ -135,7 +135,7 @@ public class SamlEcpProfileService extends SamlService { } @Override - protected Response buildErrorResponse(AuthenticationSessionModel authSession, JaxrsSAML2BindingBuilder binding, Document document) throws ConfigurationException, ProcessingException, IOException { + protected Response buildErrorResponse(boolean isPostBinding, String uri, JaxrsSAML2BindingBuilder binding, Document document) throws ConfigurationException, ProcessingException, IOException { return Soap.createMessage().addToBody(document).build(); } diff --git a/testsuite/integration-arquillian/tests/base/src/main/java/org/keycloak/testsuite/updaters/IdentityProviderCreator.java b/testsuite/integration-arquillian/tests/base/src/main/java/org/keycloak/testsuite/updaters/IdentityProviderCreator.java index 15c50cf9c1..de3da0668a 100644 --- a/testsuite/integration-arquillian/tests/base/src/main/java/org/keycloak/testsuite/updaters/IdentityProviderCreator.java +++ b/testsuite/integration-arquillian/tests/base/src/main/java/org/keycloak/testsuite/updaters/IdentityProviderCreator.java @@ -16,6 +16,7 @@ */ package org.keycloak.testsuite.updaters; +import org.keycloak.admin.client.resource.IdentityProviderResource; import org.keycloak.admin.client.resource.IdentityProvidersResource; import java.io.Closeable; import javax.ws.rs.NotFoundException; @@ -48,6 +49,10 @@ public class IdentityProviderCreator implements Closeable { return this.resource; } + public IdentityProviderResource identityProvider() { + return this.resource().get(alias); + } + @Override public void close() throws IOException { try { diff --git a/testsuite/integration-arquillian/tests/base/src/main/java/org/keycloak/testsuite/util/Matchers.java b/testsuite/integration-arquillian/tests/base/src/main/java/org/keycloak/testsuite/util/Matchers.java index 5985431adc..ffc2de4cbe 100644 --- a/testsuite/integration-arquillian/tests/base/src/main/java/org/keycloak/testsuite/util/Matchers.java +++ b/testsuite/integration-arquillian/tests/base/src/main/java/org/keycloak/testsuite/util/Matchers.java @@ -25,6 +25,7 @@ import org.keycloak.saml.common.constants.JBossSAMLURIConstants; import org.keycloak.testsuite.util.matchers.*; import java.net.URI; +import java.util.Arrays; import java.util.Map; import javax.ws.rs.core.Response; import org.apache.http.HttpResponse; @@ -147,10 +148,10 @@ public class Matchers { * @param expectedStatusCode * @return */ - public static Matcher isSamlStatusResponse(JBossSAMLURIConstants expectedStatus) { + public static Matcher isSamlStatusResponse(JBossSAMLURIConstants... expectedStatus) { return allOf( instanceOf(StatusResponseType.class), - new SamlStatusResponseTypeMatcher(is(expectedStatus.getUri())) + new SamlStatusResponseTypeMatcher(Arrays.stream(expectedStatus).map(JBossSAMLURIConstants::getUri).toArray(i -> new URI[i])) ); } } diff --git a/testsuite/integration-arquillian/tests/base/src/main/java/org/keycloak/testsuite/util/matchers/SamlStatusResponseTypeMatcher.java b/testsuite/integration-arquillian/tests/base/src/main/java/org/keycloak/testsuite/util/matchers/SamlStatusResponseTypeMatcher.java index d76a6dd56c..e12f95b3e4 100644 --- a/testsuite/integration-arquillian/tests/base/src/main/java/org/keycloak/testsuite/util/matchers/SamlStatusResponseTypeMatcher.java +++ b/testsuite/integration-arquillian/tests/base/src/main/java/org/keycloak/testsuite/util/matchers/SamlStatusResponseTypeMatcher.java @@ -6,8 +6,11 @@ package org.keycloak.testsuite.util.matchers; import org.keycloak.dom.saml.v2.SAML2Object; +import org.keycloak.dom.saml.v2.protocol.StatusCodeType; import org.keycloak.dom.saml.v2.protocol.StatusResponseType; import java.net.URI; +import java.util.ArrayList; +import java.util.List; import org.hamcrest.*; import static org.hamcrest.Matchers.*; @@ -17,28 +20,46 @@ import static org.hamcrest.Matchers.*; */ public class SamlStatusResponseTypeMatcher extends BaseMatcher { - private final Matcher statusMatcher; + private final List> statusMatchers; - public SamlStatusResponseTypeMatcher(URI statusMatcher) { - this.statusMatcher = is(statusMatcher); + public SamlStatusResponseTypeMatcher(URI... statusMatchers) { + this.statusMatchers = new ArrayList(statusMatchers.length); + for (URI uri : statusMatchers) { + this.statusMatchers.add(is(uri)); + } } - public SamlStatusResponseTypeMatcher(Matcher statusMatcher) { - this.statusMatcher = statusMatcher; + public SamlStatusResponseTypeMatcher(List> statusMatchers) { + this.statusMatchers = statusMatchers; } @Override public boolean matches(Object item) { - return statusMatcher.matches(((StatusResponseType) item).getStatus().getStatusCode().getValue()); + StatusCodeType statusCode = ((StatusResponseType) item).getStatus().getStatusCode(); + for (Matcher statusMatcher : statusMatchers) { + if (! statusMatcher.matches(statusCode.getValue())) { + return false; + } + statusCode = statusCode.getStatusCode(); + } + return true; } @Override public void describeMismatch(Object item, Description description) { - description.appendText("was ").appendValue(((StatusResponseType) item).getStatus().getStatusCode().getValue()); + StatusCodeType statusCode = ((StatusResponseType) item).getStatus().getStatusCode(); + description.appendText("was "); + while (statusCode != null) { + description.appendText("/").appendValue(statusCode.getValue()); + statusCode = statusCode.getStatusCode(); + } } @Override public void describeTo(Description description) { - description.appendText("SAML status response status matches ").appendDescriptionOf(this.statusMatcher); + description.appendText("SAML status response status matches "); + for (Matcher statusMatcher : statusMatchers) { + description.appendText("/").appendDescriptionOf(statusMatcher); + } } } diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/saml/AbstractSamlTest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/saml/AbstractSamlTest.java index 46c29d80e0..f652c3cae2 100644 --- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/saml/AbstractSamlTest.java +++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/saml/AbstractSamlTest.java @@ -40,6 +40,7 @@ public abstract class AbstractSamlTest extends AbstractAuthTest { public static final String SAML_CLIENT_SALES_POST_ENC_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"; public static final String SAML_CLIENT_SALES_POST_ENC_PUBLIC_KEY = "MIGfMA0GCSqGSIb3DQEBAQUAA4GNADCBiQKBgQDb7kwJPkGdU34hicplwfp6/WmNcaLh94TSc7Jyr9Undp5pkyLgb0DE7EIE+6kSs4LsqCb8HDkB0nLD5DXbBJFd8n0WGoKstelvtg6FtVJMnwN7k7yZbfkPECWH9zF70VeOo9vbzrApNRnct8ZhH5fbflRB4JMA9L9R+LbURdoSKQIDAQAB"; + public static final String SAML_BROKER_ALIAS = "saml-broker"; @Override public void addTestRealms(List testRealms) { @@ -61,4 +62,8 @@ public abstract class AbstractSamlTest extends AbstractAuthTest { .realmBaseUrl(UriBuilder.fromUri(getAuthServerRoot())) .build(realm); } + + protected URI getSamlBrokerUrl(String realmName) { + return URI.create(getAuthServerRealmBase(realmName).toString() + "/broker/" + SAML_BROKER_ALIAS + "/endpoint"); + } } 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 new file mode 100644 index 0000000000..85809fe041 --- /dev/null +++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/saml/BrokerTest.java @@ -0,0 +1,156 @@ +/* + * 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.testsuite.saml; + +import org.keycloak.admin.client.resource.ClientsResource; +import org.keycloak.admin.client.resource.RealmResource; +import org.keycloak.authentication.authenticators.broker.IdpReviewProfileAuthenticatorFactory; +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.AttributeStatementType; +import org.keycloak.dom.saml.v2.assertion.AttributeStatementType.ASTChoiceType; +import org.keycloak.dom.saml.v2.assertion.AttributeType; +import org.keycloak.dom.saml.v2.protocol.AuthnRequestType; +import org.keycloak.dom.saml.v2.protocol.NameIDPolicyType; +import org.keycloak.dom.saml.v2.protocol.ResponseType; +import org.keycloak.models.AuthenticationExecutionModel.Requirement; +import org.keycloak.representations.idm.AuthenticationExecutionInfoRepresentation; +import org.keycloak.representations.idm.IdentityProviderRepresentation; +import org.keycloak.saml.SAML2LoginResponseBuilder; +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.testsuite.updaters.IdentityProviderCreator; +import org.keycloak.testsuite.util.IdentityProviderBuilder; +import org.keycloak.testsuite.util.SamlClientBuilder; +import java.io.IOException; +import java.util.List; +import java.util.Objects; +import java.util.UUID; +import org.junit.Assert; +import org.junit.Test; +import static org.junit.Assert.assertThat; +import static org.keycloak.testsuite.saml.AbstractSamlTest.REALM_NAME; +import static org.keycloak.testsuite.saml.AbstractSamlTest.SAML_ASSERTION_CONSUMER_URL_SALES_POST; +import static org.keycloak.testsuite.saml.AbstractSamlTest.SAML_CLIENT_ID_SALES_POST; +import static org.keycloak.testsuite.util.Matchers.isSamlStatusResponse; +import static org.keycloak.testsuite.util.SamlClient.Binding.POST; +import static org.keycloak.testsuite.util.SamlClient.Binding.REDIRECT; + +/** + * + * @author hmlnarik + */ +public class BrokerTest extends AbstractSamlTest { + + private IdentityProviderRepresentation addIdentityProvider() { + IdentityProviderRepresentation identityProvider = IdentityProviderBuilder.create() + .providerId(SAMLIdentityProviderFactory.PROVIDER_ID) + .alias(SAML_BROKER_ALIAS) + .displayName("SAML") + .setAttribute(SAMLIdentityProviderConfig.SINGLE_SIGN_ON_SERVICE_URL, "http://saml.idp/saml") + .setAttribute(SAMLIdentityProviderConfig.SINGLE_LOGOUT_SERVICE_URL, "http://saml.idp/saml") + .setAttribute(SAMLIdentityProviderConfig.NAME_ID_POLICY_FORMAT, JBossSAMLURIConstants.NAMEID_FORMAT_EMAIL.get()) + .setAttribute(SAMLIdentityProviderConfig.POST_BINDING_RESPONSE, "false") + .setAttribute(SAMLIdentityProviderConfig.POST_BINDING_AUTHN_REQUEST, "false") + .setAttribute(SAMLIdentityProviderConfig.BACKCHANNEL_SUPPORTED, "false") + .build(); + return identityProvider; + } + + private SAML2Object createAuthnResponse(SAML2Object so) { + AuthnRequestType req = (AuthnRequestType) so; + try { + final ResponseType res = new SAML2LoginResponseBuilder() + .requestID(req.getID()) + .destination(req.getAssertionConsumerServiceURL().toString()) + .issuer("http://saml.idp/saml") + .assertionExpiration(1000000) + .subjectExpiration(1000000) + .requestIssuer(getAuthServerRealmBase(REALM_NAME).toString()) + .sessionIndex("idp:" + UUID.randomUUID()) + .buildModel(); + + AttributeStatementType attrStatement = new AttributeStatementType(); + AttributeType attribute = new AttributeType("mail"); + attribute.addAttributeValue("v@w.x"); + attrStatement.addAttribute(new ASTChoiceType(attribute)); + + res.getAssertions().get(0).getAssertion().addStatement(attrStatement); + + return res; + } catch (ConfigurationException | ProcessingException ex) { + throw new RuntimeException(ex); + } + } + + @Test + public void testLogoutPropagatesToSamlIdentityProvider() throws IOException { + final RealmResource realm = adminClient.realm(REALM_NAME); + final ClientsResource clients = realm.clients(); + + AuthenticationExecutionInfoRepresentation reviewProfileAuthenticator = null; + String firstBrokerLoginFlowAlias = null; + try (IdentityProviderCreator idp = new IdentityProviderCreator(realm, addIdentityProvider())) { + IdentityProviderRepresentation idpRepresentation = idp.identityProvider().toRepresentation(); + firstBrokerLoginFlowAlias = idpRepresentation.getFirstBrokerLoginFlowAlias(); + List executions = realm.flows().getExecutions(firstBrokerLoginFlowAlias); + reviewProfileAuthenticator = executions.stream() + .filter(ex -> Objects.equals(ex.getProviderId(), IdpReviewProfileAuthenticatorFactory.PROVIDER_ID)) + .findFirst() + .orElseGet(() -> { Assert.fail("Could not find update profile in first broker login flow"); return null; }); + + reviewProfileAuthenticator.setRequirement(Requirement.DISABLED.name()); + realm.flows().updateExecutions(firstBrokerLoginFlowAlias, reviewProfileAuthenticator); + + SAMLDocumentHolder samlResponse = new SamlClientBuilder() + .authnRequest(getAuthServerSamlEndpoint(REALM_NAME), SAML_CLIENT_ID_SALES_POST, SAML_ASSERTION_CONSUMER_URL_SALES_POST, POST) + .transformObject(ar -> { + NameIDPolicyType nameIDPolicy = new NameIDPolicyType(); + nameIDPolicy.setAllowCreate(Boolean.TRUE); + nameIDPolicy.setFormat(JBossSAMLURIConstants.NAMEID_FORMAT_EMAIL.getUri()); + + ar.setNameIDPolicy(nameIDPolicy); + return ar; + }) + .build() + + .login().idp(SAML_BROKER_ALIAS).build() + + // Virtually perform login at IdP (return artificial SAML response) + .processSamlResponse(REDIRECT) + .transformObject(this::createAuthnResponse) + .targetAttributeSamlResponse() + .targetUri(getSamlBrokerUrl(REALM_NAME)) + .build() + .followOneRedirect() // first-broker-login + .followOneRedirect() // after-first-broker-login + .getSamlResponse(POST); + + assertThat(samlResponse.getSamlObject(), isSamlStatusResponse( + JBossSAMLURIConstants.STATUS_RESPONDER, + JBossSAMLURIConstants.STATUS_INVALID_NAMEIDPOLICY + )); + } finally { + reviewProfileAuthenticator.setRequirement(Requirement.REQUIRED.name()); + realm.flows().updateExecutions(firstBrokerLoginFlowAlias, reviewProfileAuthenticator); + } + } + +} diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/saml/LogoutTest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/saml/LogoutTest.java index 19881c58b9..09fb266d6c 100644 --- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/saml/LogoutTest.java +++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/saml/LogoutTest.java @@ -49,7 +49,6 @@ import org.keycloak.testsuite.util.SamlClientBuilder; import java.io.Closeable; import java.io.IOException; -import java.net.URI; import java.util.Arrays; import java.util.List; import java.util.UUID; @@ -79,8 +78,6 @@ public class LogoutTest extends AbstractSamlTest { private final AtomicReference nameIdRef = new AtomicReference<>(); private final AtomicReference sessionIndexRef = new AtomicReference<>(); - private static final String SAML_BROKER_ALIAS = "saml-broker"; - @Before public void setup() { salesRep = adminClient.realm(REALM_NAME).clients().findByClientId(SAML_CLIENT_ID_SALES_POST).get(0); @@ -414,8 +411,4 @@ public class LogoutTest extends AbstractSamlTest { } } - private URI getSamlBrokerUrl(String realmName) { - return URI.create(getAuthServerRealmBase(realmName).toString() + "/broker/" + SAML_BROKER_ALIAS + "/endpoint"); - } - } diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/saml/SamlConsentTest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/saml/SamlConsentTest.java index c91d19dcb9..ec2aedccd4 100644 --- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/saml/SamlConsentTest.java +++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/saml/SamlConsentTest.java @@ -1,5 +1,8 @@ package org.keycloak.testsuite.saml; +import org.keycloak.protocol.saml.SamlClient; +import org.keycloak.protocol.saml.SamlConfigAttributes; +import org.keycloak.protocol.saml.SamlProtocol; import org.junit.Test; import org.keycloak.representations.idm.ClientRepresentation; import org.keycloak.representations.idm.RealmRepresentation; @@ -32,7 +35,7 @@ public class SamlConsentTest extends AbstractSamlTest { public void rejectedConsentResponseTest() throws ParsingException, ConfigurationException, ProcessingException { ClientRepresentation client = adminClient.realm(REALM_NAME) .clients() - .findByClientId(SAML_CLIENT_ID_SALES_POST_ENC) + .findByClientId(SAML_CLIENT_ID_SALES_POST) .get(0); adminClient.realm(REALM_NAME) @@ -40,14 +43,14 @@ public class SamlConsentTest extends AbstractSamlTest { .get(client.getId()) .update(ClientBuilder.edit(client) .consentRequired(true) - .attribute("saml.encrypt", "false") //remove after RHSSO-797 - .attribute("saml_idp_initiated_sso_url_name", "sales-post-enc") - .attribute("saml_assertion_consumer_url_post", SAML_ASSERTION_CONSUMER_URL_SALES_POST_ENC + "saml") + .attribute(SamlProtocol.SAML_IDP_INITIATED_SSO_URL_NAME, "sales-post") + .attribute(SamlProtocol.SAML_ASSERTION_CONSUMER_URL_POST_ATTRIBUTE, SAML_ASSERTION_CONSUMER_URL_SALES_POST + "saml") + .attribute(SamlConfigAttributes.SAML_SERVER_SIGNATURE, "true") .build()); log.debug("Log in using idp initiated login"); SAMLDocumentHolder documentHolder = new SamlClientBuilder() - .idpInitiatedLogin(getAuthServerSamlEndpoint(REALM_NAME), "sales-post-enc").build() + .authnRequest(getAuthServerSamlEndpoint(REALM_NAME), SAML_CLIENT_ID_SALES_POST, SAML_ASSERTION_CONSUMER_URL_SALES_POST, Binding.POST).build() .login().user(bburkeUser).build() .consentRequired().approveConsent(false).build() .getSamlResponse(Binding.POST);