From e82fe7d9e317b689d1746f9efbcb4f9285bc38c0 Mon Sep 17 00:00:00 2001 From: Lorent Lempereur Date: Fri, 24 Jul 2020 21:41:57 +0200 Subject: [PATCH] KEYCLOAK-13950 SAML2 Identity Provider - Send Subject in SAML requests --- .../saml/SAML2AuthnRequestBuilder.java | 22 +++++- .../core/saml/v2/writers/BaseWriter.java | 13 +++- .../saml/v2/writers/SAMLRequestWriter.java | 9 ++- .../saml/v2/writers/SAMLResponseWriter.java | 3 +- .../models/IdentityProviderModel.java | 9 +++ .../oidc/OAuth2IdentityProviderConfig.java | 8 --- .../broker/saml/SAMLIdentityProvider.java | 6 +- .../saml/SAMLIdentityProviderFactory.java | 1 + .../resources/IdentityBrokerService.java | 15 ++-- .../broker/saml/SAMLDataMarshallerTest.java | 6 +- .../testsuite/admin/IdentityProviderTest.java | 3 +- .../broker/AbstractSamlLoginHintTest.java | 71 +++++++++++++++++++ .../broker/KcOidcBrokerLoginHintTest.java | 3 +- .../broker/KcOidcBrokerNoLoginHintTest.java | 3 +- .../broker/KcSamlBrokerConfiguration.java | 12 ++++ ...BrokerLoginHintWithOptionDisabledTest.java | 8 +++ ...lBrokerLoginHintWithOptionEnabledTest.java | 36 ++++++++++ .../broker/KcSamlFirstBrokerLoginTest.java | 1 - .../realm-identity-provider-saml.html | 7 ++ 19 files changed, 210 insertions(+), 26 deletions(-) create mode 100644 testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/broker/AbstractSamlLoginHintTest.java create mode 100644 testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/broker/KcSamlBrokerLoginHintWithOptionDisabledTest.java create mode 100644 testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/broker/KcSamlBrokerLoginHintWithOptionEnabledTest.java 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 a4867ddd58..11bb00b44b 100755 --- a/saml-core/src/main/java/org/keycloak/saml/SAML2AuthnRequestBuilder.java +++ b/saml-core/src/main/java/org/keycloak/saml/SAML2AuthnRequestBuilder.java @@ -17,7 +17,9 @@ package org.keycloak.saml; 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.ExtensionsType; 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; @@ -26,7 +28,6 @@ import org.w3c.dom.Document; import java.net.URI; import java.util.LinkedList; import java.util.List; -import org.keycloak.dom.saml.v2.protocol.ExtensionsType; /** * @author pedroigor @@ -88,6 +89,25 @@ public class SAML2AuthnRequestBuilder implements SamlProtocolExtensionsAwareBuil return this; } + public SAML2AuthnRequestBuilder subject(String subject) { + String sanitizedSubject = subject != null ? subject.trim() : null; + if (sanitizedSubject != null && !sanitizedSubject.isEmpty()) { + this.authnRequestType.setSubject(createSubject(sanitizedSubject)); + } + return this; + } + + private SubjectType createSubject(String value) { + NameIDType nameId = new NameIDType(); + nameId.setValue(value); + nameId.setFormat(this.authnRequestType.getNameIDPolicy() != null ? this.authnRequestType.getNameIDPolicy().getFormat() : null); + SubjectType subject = new SubjectType(); + SubjectType.STSubType subType = new SubjectType.STSubType(); + subType.addBaseID(nameId); + subject.setSubType(subType); + return subject; + } + public Document toDocument() { try { AuthnRequestType authnRequestType = createAuthnRequest(); diff --git a/saml-core/src/main/java/org/keycloak/saml/processing/core/saml/v2/writers/BaseWriter.java b/saml-core/src/main/java/org/keycloak/saml/processing/core/saml/v2/writers/BaseWriter.java index 24b2f615e0..c5ca05ecee 100755 --- a/saml-core/src/main/java/org/keycloak/saml/processing/core/saml/v2/writers/BaseWriter.java +++ b/saml-core/src/main/java/org/keycloak/saml/processing/core/saml/v2/writers/BaseWriter.java @@ -81,10 +81,12 @@ public class BaseWriter { * * @throws org.keycloak.saml.common.exceptions.ProcessingException */ - public void write(NameIDType nameIDType, QName tag) throws ProcessingException { + public void write(NameIDType nameIDType, QName tag, boolean writeNamespace) throws ProcessingException { StaxUtil.writeStartElement(writer, tag.getPrefix(), tag.getLocalPart(), tag.getNamespaceURI()); - StaxUtil.writeNameSpace(writer, ASSERTION_PREFIX, ASSERTION_NSURI.get()); + if (writeNamespace) { + StaxUtil.writeNameSpace(writer, ASSERTION_PREFIX, ASSERTION_NSURI.get()); + } URI format = nameIDType.getFormat(); if (format != null) { @@ -115,6 +117,13 @@ public class BaseWriter { StaxUtil.flush(writer); } + /** + * Write {@code NameIDType} to stream without writing a namespace + */ + public void write(NameIDType nameIDType, QName tag) throws ProcessingException { + this.write(nameIDType, tag, false); + } + /** * Write an {@code AttributeType} to stream * diff --git a/saml-core/src/main/java/org/keycloak/saml/processing/core/saml/v2/writers/SAMLRequestWriter.java b/saml-core/src/main/java/org/keycloak/saml/processing/core/saml/v2/writers/SAMLRequestWriter.java index fcc43270ad..72e5cfae7c 100755 --- a/saml-core/src/main/java/org/keycloak/saml/processing/core/saml/v2/writers/SAMLRequestWriter.java +++ b/saml-core/src/main/java/org/keycloak/saml/processing/core/saml/v2/writers/SAMLRequestWriter.java @@ -64,6 +64,7 @@ public class SAMLRequestWriter extends BaseWriter { public void write(AuthnRequestType request) throws ProcessingException { StaxUtil.writeStartElement(writer, PROTOCOL_PREFIX, JBossSAMLConstants.AUTHN_REQUEST.get(), PROTOCOL_NSURI.get()); StaxUtil.writeNameSpace(writer, PROTOCOL_PREFIX, PROTOCOL_NSURI.get()); + StaxUtil.writeNameSpace(writer, ASSERTION_PREFIX, ASSERTION_NSURI.get()); StaxUtil.writeDefaultNameSpace(writer, ASSERTION_NSURI.get()); // Attributes @@ -119,7 +120,12 @@ public class SAMLRequestWriter extends BaseWriter { NameIDType issuer = request.getIssuer(); if (issuer != null) { - write(issuer, new QName(ASSERTION_NSURI.get(), JBossSAMLConstants.ISSUER.get(), ASSERTION_PREFIX)); + write(issuer, new QName(ASSERTION_NSURI.get(), JBossSAMLConstants.ISSUER.get(), ASSERTION_PREFIX), false); + } + + SubjectType subject = request.getSubject(); + if (subject != null) { + write(subject); } Element sig = request.getSignature(); @@ -157,6 +163,7 @@ public class SAMLRequestWriter extends BaseWriter { StaxUtil.writeStartElement(writer, PROTOCOL_PREFIX, JBossSAMLConstants.LOGOUT_REQUEST.get(), PROTOCOL_NSURI.get()); StaxUtil.writeNameSpace(writer, PROTOCOL_PREFIX, PROTOCOL_NSURI.get()); + StaxUtil.writeNameSpace(writer, ASSERTION_PREFIX, ASSERTION_NSURI.get()); StaxUtil.writeDefaultNameSpace(writer, ASSERTION_NSURI.get()); // Attributes diff --git a/saml-core/src/main/java/org/keycloak/saml/processing/core/saml/v2/writers/SAMLResponseWriter.java b/saml-core/src/main/java/org/keycloak/saml/processing/core/saml/v2/writers/SAMLResponseWriter.java index 1bc57f11d2..d385eac4f9 100755 --- a/saml-core/src/main/java/org/keycloak/saml/processing/core/saml/v2/writers/SAMLResponseWriter.java +++ b/saml-core/src/main/java/org/keycloak/saml/processing/core/saml/v2/writers/SAMLResponseWriter.java @@ -162,12 +162,13 @@ public class SAMLResponseWriter extends BaseWriter { } StaxUtil.writeNameSpace(writer, PROTOCOL_PREFIX, JBossSAMLURIConstants.PROTOCOL_NSURI.get()); + StaxUtil.writeNameSpace(writer, ASSERTION_PREFIX, JBossSAMLURIConstants.ASSERTION_NSURI.get()); StaxUtil.writeDefaultNameSpace(writer, JBossSAMLURIConstants.ASSERTION_NSURI.get()); writeBaseAttributes(response); NameIDType issuer = response.getIssuer(); - write(issuer, new QName(JBossSAMLURIConstants.ASSERTION_NSURI.get(), JBossSAMLConstants.ISSUER.get(), ASSERTION_PREFIX)); + write(issuer, new QName(JBossSAMLURIConstants.ASSERTION_NSURI.get(), JBossSAMLConstants.ISSUER.get())); Element sig = response.getSignature(); if (sig != null) { diff --git a/server-spi/src/main/java/org/keycloak/models/IdentityProviderModel.java b/server-spi/src/main/java/org/keycloak/models/IdentityProviderModel.java index a1f8040505..f09e6b8401 100755 --- a/server-spi/src/main/java/org/keycloak/models/IdentityProviderModel.java +++ b/server-spi/src/main/java/org/keycloak/models/IdentityProviderModel.java @@ -29,6 +29,7 @@ import java.util.Map; public class IdentityProviderModel implements Serializable { public static final String ALLOWED_CLOCK_SKEW = "allowedClockSkew"; + public static final String LOGIN_HINT = "loginHint"; public static final String SYNC_MODE = "syncMode"; @@ -218,4 +219,12 @@ public class IdentityProviderModel implements Serializable { public void setSyncMode(IdentityProviderSyncMode syncMode) { getConfig().put(SYNC_MODE, syncMode.toString()); } + + public boolean isLoginHint() { + return Boolean.valueOf(getConfig().get(LOGIN_HINT)); + } + + public void setLoginHint(boolean loginHint) { + getConfig().put(LOGIN_HINT, String.valueOf(loginHint)); + } } diff --git a/services/src/main/java/org/keycloak/broker/oidc/OAuth2IdentityProviderConfig.java b/services/src/main/java/org/keycloak/broker/oidc/OAuth2IdentityProviderConfig.java index 5838376f50..e360eed8fe 100644 --- a/services/src/main/java/org/keycloak/broker/oidc/OAuth2IdentityProviderConfig.java +++ b/services/src/main/java/org/keycloak/broker/oidc/OAuth2IdentityProviderConfig.java @@ -92,14 +92,6 @@ public class OAuth2IdentityProviderConfig extends IdentityProviderModel { public void setDefaultScope(String defaultScope) { getConfig().put("defaultScope", defaultScope); } - - public boolean isLoginHint() { - return Boolean.valueOf(getConfig().get("loginHint")); - } - - public void setLoginHint(boolean loginHint) { - getConfig().put("loginHint", String.valueOf(loginHint)); - } public boolean isJWTAuthentication() { if (getClientAuthMethod().equals(OIDCLoginProtocol.CLIENT_SECRET_JWT) diff --git a/services/src/main/java/org/keycloak/broker/saml/SAMLIdentityProvider.java b/services/src/main/java/org/keycloak/broker/saml/SAMLIdentityProvider.java index 7f42c83240..b7e816b874 100755 --- a/services/src/main/java/org/keycloak/broker/saml/SAMLIdentityProvider.java +++ b/services/src/main/java/org/keycloak/broker/saml/SAMLIdentityProvider.java @@ -32,6 +32,7 @@ import org.keycloak.dom.saml.v2.protocol.ResponseType; import org.keycloak.events.EventBuilder; import org.keycloak.keys.RsaKeyMetadata; import org.keycloak.models.*; +import org.keycloak.protocol.oidc.OIDCLoginProtocol; import org.keycloak.protocol.saml.JaxrsSAML2BindingBuilder; import org.keycloak.protocol.saml.SamlSessionUtils; import org.keycloak.protocol.saml.preprocessor.SamlAuthenticationPreprocessor; @@ -63,6 +64,7 @@ import java.util.TreeSet; */ public class SAMLIdentityProvider extends AbstractIdentityProvider { protected static final Logger logger = Logger.getLogger(SAMLIdentityProvider.class); + private final DestinationValidator destinationValidator; public SAMLIdentityProvider(KeycloakSession session, SAMLIdentityProviderConfig config, DestinationValidator destinationValidator) { super(session, config); @@ -95,13 +97,15 @@ public class SAMLIdentityProvider extends AbstractIdentityProvider nameIdFormatList = idpDescriptor.getNameIDFormat(); if (nameIdFormatList != null && !nameIdFormatList.isEmpty()) diff --git a/services/src/main/java/org/keycloak/services/resources/IdentityBrokerService.java b/services/src/main/java/org/keycloak/services/resources/IdentityBrokerService.java index fe59cba4d1..71807e9a58 100755 --- a/services/src/main/java/org/keycloak/services/resources/IdentityBrokerService.java +++ b/services/src/main/java/org/keycloak/services/resources/IdentityBrokerService.java @@ -353,8 +353,9 @@ public class IdentityBrokerService implements IdentityProvider.AuthenticationCal public Response performPostLogin(@PathParam("provider_id") String providerId, @QueryParam(LoginActionsService.SESSION_CODE) String code, @QueryParam("client_id") String clientId, - @QueryParam(Constants.TAB_ID) String tabId) { - return performLogin(providerId, code, clientId, tabId); + @QueryParam(Constants.TAB_ID) String tabId, + @QueryParam(OIDCLoginProtocol.LOGIN_HINT_PARAM) String loginHint) { + return performLogin(providerId, code, clientId, tabId, loginHint); } @GET @@ -363,7 +364,8 @@ public class IdentityBrokerService implements IdentityProvider.AuthenticationCal public Response performLogin(@PathParam("provider_id") String providerId, @QueryParam(LoginActionsService.SESSION_CODE) String code, @QueryParam("client_id") String clientId, - @QueryParam(Constants.TAB_ID) String tabId) { + @QueryParam(Constants.TAB_ID) String tabId, + @QueryParam(OIDCLoginProtocol.LOGIN_HINT_PARAM) String loginHint) { this.event.detail(Details.IDENTITY_PROVIDER, providerId); if (isDebugEnabled()) { @@ -376,15 +378,18 @@ public class IdentityBrokerService implements IdentityProvider.AuthenticationCal return parsedCode.response; } - ClientSessionCode clientSessionCode = parsedCode.clientSessionCode; + ClientSessionCode clientSessionCode = parsedCode.clientSessionCode; IdentityProviderModel identityProviderModel = realmModel.getIdentityProviderByAlias(providerId); if (identityProviderModel == null) { throw new IdentityBrokerException("Identity Provider [" + providerId + "] not found."); } if (identityProviderModel.isLinkOnly()) { throw new IdentityBrokerException("Identity Provider [" + providerId + "] is not allowed to perform a login."); - } + if (clientSessionCode != null && clientSessionCode.getClientSession() != null && loginHint != null) { + clientSessionCode.getClientSession().setClientNote(OIDCLoginProtocol.LOGIN_HINT_PARAM, loginHint); + } + IdentityProviderFactory providerFactory = getIdentityProviderFactory(session, identityProviderModel); IdentityProvider identityProvider = providerFactory.create(session, identityProviderModel); diff --git a/services/src/test/java/org/keycloak/test/broker/saml/SAMLDataMarshallerTest.java b/services/src/test/java/org/keycloak/test/broker/saml/SAMLDataMarshallerTest.java index 914f7ee2aa..93927328fd 100755 --- a/services/src/test/java/org/keycloak/test/broker/saml/SAMLDataMarshallerTest.java +++ b/services/src/test/java/org/keycloak/test/broker/saml/SAMLDataMarshallerTest.java @@ -36,11 +36,11 @@ import static org.junit.Assert.assertThat; */ public class SAMLDataMarshallerTest { - private static final String TEST_RESPONSE = "http://localhost:8082/auth/realms/realm-with-saml-idp-basichttp://localhost:8082/auth/realms/realm-with-saml-idp-basictest-userhttp://localhost:8081/auth/realms/realm-with-brokerurn:oasis:names:tc:SAML:2.0:ac:classes:unspecified617-666-7777test-user@localhostmanager"; + private static final String TEST_RESPONSE = "http://localhost:8082/auth/realms/realm-with-saml-idp-basichttp://localhost:8082/auth/realms/realm-with-saml-idp-basictest-userhttp://localhost:8081/auth/realms/realm-with-brokerurn:oasis:names:tc:SAML:2.0:ac:classes:unspecified617-666-7777test-user@localhostmanager"; - private static final String TEST_ASSERTION = "http://localhost:8082/auth/realms/realm-with-saml-idp-basictest-userhttp://localhost:8081/auth/realms/realm-with-brokerurn:oasis:names:tc:SAML:2.0:ac:classes:unspecified617-666-7777test-user@localhostmanager"; + private static final String TEST_ASSERTION = "http://localhost:8082/auth/realms/realm-with-saml-idp-basictest-userhttp://localhost:8081/auth/realms/realm-with-brokerurn:oasis:names:tc:SAML:2.0:ac:classes:unspecified617-666-7777test-user@localhostmanager"; - private static final String TEST_ASSERTION_WITH_NAME_ID = "http://localhost:8082/auth/realms/realm-with-saml-idp-basictest-userhttp://localhost:8081/auth/realms/realm-with-brokerurn:oasis:names:tc:SAML:2.0:ac:classes:unspecified617-666-7777test-user@localhostb2c6275838784dba219c92f53ea5493c8ef4da09"; + private static final String TEST_ASSERTION_WITH_NAME_ID = "http://localhost:8082/auth/realms/realm-with-saml-idp-basictest-userhttp://localhost:8081/auth/realms/realm-with-brokerurn:oasis:names:tc:SAML:2.0:ac:classes:unspecified617-666-7777test-user@localhostb2c6275838784dba219c92f53ea5493c8ef4da09"; private static final String TEST_AUTHN_TYPE = "urn:oasis:names:tc:SAML:2.0:ac:classes:unspecified"; 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 e42870b3f9..ddd5051845 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 @@ -888,7 +888,8 @@ public class IdentityProviderTest extends AbstractAdminTest { "wantAuthnRequestsSigned", "nameIDPolicyFormat", "signingCertificate", - "addExtensionsElementWithKeyInfo" + "addExtensionsElementWithKeyInfo", + "loginHint" )); assertThat(config, hasEntry("validateSignature", "true")); assertThat(config, hasEntry("singleLogoutServiceUrl", "http://localhost:8080/auth/realms/master/protocol/saml")); diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/broker/AbstractSamlLoginHintTest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/broker/AbstractSamlLoginHintTest.java new file mode 100644 index 0000000000..3e5bf7b547 --- /dev/null +++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/broker/AbstractSamlLoginHintTest.java @@ -0,0 +1,71 @@ +package org.keycloak.testsuite.broker; + +import org.junit.Test; +import org.keycloak.protocol.oidc.OIDCLoginProtocol; +import org.keycloak.testsuite.Assert; +import org.openqa.selenium.JavascriptExecutor; +import org.openqa.selenium.WebElement; + +import static org.junit.Assert.assertEquals; +import static org.keycloak.testsuite.broker.BrokerTestTools.getConsumerRoot; +import static org.keycloak.testsuite.broker.BrokerTestTools.waitForPage; + +/** + * Test of various scenarios related to the use of login hint + */ +public abstract class AbstractSamlLoginHintTest extends AbstractInitializedBaseBrokerTest { + + // KEYCLOAK-13950 + @Test + public void testPassLoginHintShouldSendSubjectAndPrefillUsername() { + String username = "all-info-set@localhost.com"; + createUser(bc.providerRealmName(), username, "password"); + + driver.navigate().to(getAccountUrl(getConsumerRoot(), bc.consumerRealmName())); + log.debug("Clicking social " + bc.getIDPAlias()); + addLoginHintOnSocialButton(username); + loginPage.clickSocial(bc.getIDPAlias()); + waitForPage(driver, "log in to", true); + Assert.assertTrue("Driver should be on the provider realm page right now", + driver.getCurrentUrl().contains("/auth/realms/" + bc.providerRealmName() + "/")); + log.debug("Logging in"); + + if (isLoginHintOptionEnabled()) { + assertEquals("Username input should contain the SAML subject", loginPage.getUsername(), username); + } else { + assertEquals("Username input should the SAML subject", loginPage.getUsername(), ""); + } + } + + // KEYCLOAK-13950 + @Test + public void testPassEmptyLoginHintShouldNotSendSubjectAndShouldNotPrefillUsername() { + String username = "all-info-set@localhost.com"; + createUser(bc.providerRealmName(), username, "password", "FirstName"); + + driver.navigate().to(getAccountUrl(getConsumerRoot(), bc.consumerRealmName())); + log.debug("Clicking social " + bc.getIDPAlias()); + addLoginHintOnSocialButton(""); + loginPage.clickSocial(bc.getIDPAlias()); + waitForPage(driver, "log in to", true); + Assert.assertTrue("Driver should be on the provider realm page right now", + driver.getCurrentUrl().contains("/auth/realms/" + bc.providerRealmName() + "/")); + log.debug("Logging in"); + + assertEquals("Username input should not contain any username", loginPage.getUsername(), ""); + } + + abstract boolean isLoginHintOptionEnabled(); + + protected void addLoginHintOnSocialButton(String hint) { + JavascriptExecutor executor = (JavascriptExecutor) driver; + WebElement button = loginPage.findSocialButton(bc.getIDPAlias()); + String url = button.getAttribute("href") + "&"+ OIDCLoginProtocol.LOGIN_HINT_PARAM+"="+hint; + executor.executeScript("document.getElementById('"+button.getAttribute("id")+"').setAttribute('href', '"+url+"')"); + } + + @Override + protected BrokerConfiguration getBrokerConfiguration() { + return new KcSamlBrokerConfiguration(isLoginHintOptionEnabled()); + } +} diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/broker/KcOidcBrokerLoginHintTest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/broker/KcOidcBrokerLoginHintTest.java index ebec294b6b..838d5da837 100644 --- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/broker/KcOidcBrokerLoginHintTest.java +++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/broker/KcOidcBrokerLoginHintTest.java @@ -14,6 +14,7 @@ import static org.keycloak.testsuite.broker.BrokerTestTools.getConsumerRoot; import org.junit.Test; import org.keycloak.admin.client.resource.UserResource; import org.keycloak.admin.client.resource.UsersResource; +import org.keycloak.models.IdentityProviderModel; import org.keycloak.models.IdentityProviderSyncMode; import org.keycloak.representations.idm.IdentityProviderRepresentation; import org.keycloak.representations.idm.UserRepresentation; @@ -36,7 +37,7 @@ public class KcOidcBrokerLoginHintTest extends AbstractBrokerTest { Map config = idp.getConfig(); applyDefaultConfiguration(config, syncMode); - config.put("loginHint", "true"); + config.put(IdentityProviderModel.LOGIN_HINT, "true"); return idp; } } diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/broker/KcOidcBrokerNoLoginHintTest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/broker/KcOidcBrokerNoLoginHintTest.java index ac20f19441..06349d44a8 100644 --- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/broker/KcOidcBrokerNoLoginHintTest.java +++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/broker/KcOidcBrokerNoLoginHintTest.java @@ -11,6 +11,7 @@ import static org.keycloak.testsuite.broker.BrokerTestTools.waitForPage; import static org.keycloak.testsuite.broker.BrokerTestTools.getConsumerRoot; import org.apache.commons.lang3.StringUtils; import org.keycloak.admin.client.resource.UsersResource; +import org.keycloak.models.IdentityProviderModel; import org.keycloak.models.IdentityProviderSyncMode; import org.keycloak.representations.idm.IdentityProviderRepresentation; import org.keycloak.representations.idm.UserRepresentation; @@ -31,7 +32,7 @@ public class KcOidcBrokerNoLoginHintTest extends AbstractBrokerTest { Map config = idp.getConfig(); applyDefaultConfiguration(config, syncMode); - config.put("loginHint", "false"); + config.put(IdentityProviderModel.LOGIN_HINT, "false"); return idp; } } diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/broker/KcSamlBrokerConfiguration.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/broker/KcSamlBrokerConfiguration.java index fd4a841e46..cedc17d8e9 100644 --- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/broker/KcSamlBrokerConfiguration.java +++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/broker/KcSamlBrokerConfiguration.java @@ -37,6 +37,16 @@ public class KcSamlBrokerConfiguration implements BrokerConfiguration { public static final KcSamlBrokerConfiguration INSTANCE = new KcSamlBrokerConfiguration(); public static final String ATTRIBUTE_TO_MAP_FRIENDLY_NAME = "user-attribute-friendly"; + private final boolean loginHint; + + public KcSamlBrokerConfiguration() { + this(false); + } + + public KcSamlBrokerConfiguration(boolean loginHint) { + this.loginHint = loginHint; + } + @Override public RealmRepresentation createProviderRealm() { RealmRepresentation realm = new RealmRepresentation(); @@ -87,6 +97,7 @@ public class KcSamlBrokerConfiguration implements BrokerConfiguration { attributes.put(SamlConfigAttributes.SAML_SERVER_SIGNATURE, "false"); attributes.put(SamlConfigAttributes.SAML_CLIENT_SIGNATURE_ATTRIBUTE, "false"); attributes.put(SamlConfigAttributes.SAML_ENCRYPT, "false"); + attributes.put(IdentityProviderModel.LOGIN_HINT, String.valueOf(loginHint)); client.setAttributes(attributes); @@ -211,6 +222,7 @@ public class KcSamlBrokerConfiguration implements BrokerConfiguration { config.put(SINGLE_LOGOUT_SERVICE_URL, getProviderRoot() + "/auth/realms/" + REALM_PROV_NAME + "/protocol/saml"); config.put(NAME_ID_POLICY_FORMAT, "urn:oasis:names:tc:SAML:1.1:nameid-format:emailAddress"); config.put(FORCE_AUTHN, "false"); + config.put(IdentityProviderModel.LOGIN_HINT, String.valueOf(loginHint)); config.put(POST_BINDING_RESPONSE, "true"); config.put(POST_BINDING_AUTHN_REQUEST, "true"); config.put(VALIDATE_SIGNATURE, "false"); diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/broker/KcSamlBrokerLoginHintWithOptionDisabledTest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/broker/KcSamlBrokerLoginHintWithOptionDisabledTest.java new file mode 100644 index 0000000000..44dfee1c41 --- /dev/null +++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/broker/KcSamlBrokerLoginHintWithOptionDisabledTest.java @@ -0,0 +1,8 @@ +package org.keycloak.testsuite.broker; + +public class KcSamlBrokerLoginHintWithOptionDisabledTest extends AbstractSamlLoginHintTest { + @Override + boolean isLoginHintOptionEnabled() { + return false; + } +} diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/broker/KcSamlBrokerLoginHintWithOptionEnabledTest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/broker/KcSamlBrokerLoginHintWithOptionEnabledTest.java new file mode 100644 index 0000000000..068095c45e --- /dev/null +++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/broker/KcSamlBrokerLoginHintWithOptionEnabledTest.java @@ -0,0 +1,36 @@ +package org.keycloak.testsuite.broker; + +import org.junit.Test; +import org.keycloak.testsuite.Assert; + +import static org.junit.Assert.assertEquals; +import static org.keycloak.testsuite.broker.BrokerTestTools.getConsumerRoot; +import static org.keycloak.testsuite.broker.BrokerTestTools.waitForPage; + +public class KcSamlBrokerLoginHintWithOptionEnabledTest extends AbstractSamlLoginHintTest { + + + // KEYCLOAK-13950 + @Test + public void testPassLoginHintWithXmlCharShouldEncodeIt() { + String username = "all-info-set@localhost.com"; + createUser(bc.providerRealmName(), username, "password", "FirstName"); + + driver.navigate().to(getAccountUrl(getConsumerRoot(), bc.consumerRealmName())); + log.debug("Clicking social " + bc.getIDPAlias()); + String fishyLoginHint = ""; + addLoginHintOnSocialButton(fishyLoginHint); + loginPage.clickSocial(bc.getIDPAlias()); + waitForPage(driver, "log in to", true); + Assert.assertTrue("Driver should be on the provider realm page right now", + driver.getCurrentUrl().contains("/auth/realms/" + bc.providerRealmName() + "/")); + log.debug("Logging in"); + + assertEquals("Username input should contain the SAML subject", loginPage.getUsername(), fishyLoginHint); + } + + @Override + boolean isLoginHintOptionEnabled() { + return true; + } +} diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/broker/KcSamlFirstBrokerLoginTest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/broker/KcSamlFirstBrokerLoginTest.java index 6761bc51db..cfcaed8f1b 100644 --- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/broker/KcSamlFirstBrokerLoginTest.java +++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/broker/KcSamlFirstBrokerLoginTest.java @@ -12,7 +12,6 @@ public class KcSamlFirstBrokerLoginTest extends AbstractFirstBrokerLoginTest { return KcSamlBrokerConfiguration.INSTANCE; } - @Test @Override public void testUpdateProfileIfNotMissingInformation() { 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 e8bc968e38..22ac32a153 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 @@ -260,6 +260,13 @@ {{:: 'validating-x509-certificate.tooltip' | translate}} +
+ +
+ +
+ {{:: 'saml.loginHint.tooltip' | translate}} +