From a79d6289de2446b90b1e89f0e2bbf78df5bd9299 Mon Sep 17 00:00:00 2001 From: mhajas Date: Tue, 7 Jan 2020 09:43:09 +0100 Subject: [PATCH] KEYCLOAK-11416 Fix nil AttributeValue handling --- .../AbstractSamlAuthenticationHandler.java | 13 +- .../core/saml/v2/writers/BaseWriter.java | 10 +- .../saml/mappers/AttributeToRoleMapper.java | 7 +- .../adapter/servlet/SendUsernameServlet.java | 53 ++-- .../keycloak/testsuite/util/TestCleanup.java | 6 +- .../AbstractSAMLServletAdapterTest.java | 39 +++ .../SAMLFilterLoginResponseHandlingTest.java | 34 +++ .../servlet/SAMLFilterServletAdapterTest.java | 14 - .../SAMLLoginResponseHandlingTest.java | 266 ++++++++++++++++++ .../servlet/SAMLServletAdapterTest.java | 167 ----------- .../SAMLServletSessionTimeoutTest.java | 16 +- .../testsuite/broker/AbstractBrokerTest.java | 3 + .../testsuite/broker/KcSamlBrokerTest.java | 87 +++++- .../testsuite/saml/ProtocolMapperTest.java | 80 ++++++ 14 files changed, 571 insertions(+), 224 deletions(-) create mode 100644 testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/adapter/servlet/SAMLFilterLoginResponseHandlingTest.java create mode 100644 testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/adapter/servlet/SAMLLoginResponseHandlingTest.java create mode 100644 testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/saml/ProtocolMapperTest.java 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 2034bf6f87..0ad095bf20 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 @@ -569,19 +569,20 @@ public abstract class AbstractSamlAuthenticationHandler implements SamlAuthentic } private String getAttributeValue(Object attrValue) { - String value = null; - if (attrValue instanceof String) { - value = (String) attrValue; + if (attrValue == null) { + return ""; + } else if (attrValue instanceof String) { + return (String) attrValue; } else if (attrValue instanceof Node) { Node roleNode = (Node) attrValue; - value = roleNode.getFirstChild().getNodeValue(); + return roleNode.getFirstChild().getNodeValue(); } else if (attrValue instanceof NameIDType) { NameIDType nameIdType = (NameIDType) attrValue; - value = nameIdType.getValue(); + return nameIdType.getValue(); } else { log.warn("Unable to extract unknown SAML assertion attribute value type: " + attrValue.getClass().getName()); } - return value; + return null; } protected boolean isRole(AttributeType attribute) { 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 1fb2e9e1fd..14c130ab1b 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 @@ -174,6 +174,8 @@ public class BaseWriter { writeNameIDTypeAttributeValue((NameIDType) attributeValue); } else throw logger.writerUnsupportedAttributeValueError(attributeValue.getClass().getName()); + } else { + writeStringAttributeValue(null); } } } @@ -191,7 +193,13 @@ public class BaseWriter { StaxUtil.writeNameSpace(writer, JBossSAMLURIConstants.XSI_PREFIX.get(), JBossSAMLURIConstants.XSI_NSURI.get()); StaxUtil.writeNameSpace(writer, "xs", JBossSAMLURIConstants.XMLSCHEMA_NSURI.get()); StaxUtil.writeAttribute(writer, "xsi", JBossSAMLURIConstants.XSI_NSURI.get(), "type", "xs:string"); - StaxUtil.writeCharacters(writer, attributeValue); + + if (attributeValue == null) { + StaxUtil.writeAttribute(writer, "xsi", JBossSAMLURIConstants.XSI_NSURI.get(), "nil", "true"); + } else { + StaxUtil.writeCharacters(writer, attributeValue); + } + StaxUtil.writeEndElement(writer); } diff --git a/services/src/main/java/org/keycloak/broker/saml/mappers/AttributeToRoleMapper.java b/services/src/main/java/org/keycloak/broker/saml/mappers/AttributeToRoleMapper.java index 091723d55e..56088d729c 100755 --- a/services/src/main/java/org/keycloak/broker/saml/mappers/AttributeToRoleMapper.java +++ b/services/src/main/java/org/keycloak/broker/saml/mappers/AttributeToRoleMapper.java @@ -36,6 +36,7 @@ import org.keycloak.provider.ProviderConfigProperty; import java.util.ArrayList; import java.util.List; +import java.util.Optional; /** * @author Bill Burke @@ -121,7 +122,7 @@ public class AttributeToRoleMapper extends AbstractIdentityProviderMapper { if (name != null && name.trim().equals("")) name = null; String friendly = mapperModel.getConfig().get(ATTRIBUTE_FRIENDLY_NAME); if (friendly != null && friendly.trim().equals("")) friendly = null; - String desiredValue = mapperModel.getConfig().get(ATTRIBUTE_VALUE); + String desiredValue = Optional.ofNullable(mapperModel.getConfig().get(ATTRIBUTE_VALUE)).orElse(""); AssertionType assertion = (AssertionType)context.getContextData().get(SAMLEndpoint.SAML_ASSERTION); for (AttributeStatementType statement : assertion.getAttributeStatements()) { for (AttributeStatementType.ASTChoiceType choice : statement.getAttributes()) { @@ -129,7 +130,9 @@ public class AttributeToRoleMapper extends AbstractIdentityProviderMapper { if (name != null && !name.equals(attr.getName())) continue; if (friendly != null && !friendly.equals(attr.getFriendlyName())) continue; for (Object val : attr.getAttributeValue()) { - if (val.equals(desiredValue)) return true; + val = Optional.ofNullable(val).orElse(""); + if (val.equals(desiredValue)) + return true; } } } diff --git a/testsuite/integration-arquillian/test-apps/servlets/src/main/java/org/keycloak/testsuite/adapter/servlet/SendUsernameServlet.java b/testsuite/integration-arquillian/test-apps/servlets/src/main/java/org/keycloak/testsuite/adapter/servlet/SendUsernameServlet.java index 2b59000295..db67b186f5 100755 --- a/testsuite/integration-arquillian/test-apps/servlets/src/main/java/org/keycloak/testsuite/adapter/servlet/SendUsernameServlet.java +++ b/testsuite/integration-arquillian/test-apps/servlets/src/main/java/org/keycloak/testsuite/adapter/servlet/SendUsernameServlet.java @@ -44,6 +44,7 @@ import java.security.Principal; import java.util.Arrays; import java.util.Collections; import java.util.List; +import java.util.Map.Entry; import javax.xml.parsers.DocumentBuilderFactory; import javax.xml.transform.OutputKeys; import javax.xml.transform.Transformer; @@ -249,26 +250,42 @@ public class SendUsernameServlet { return output + ""; } - private String getAttributes() { - SamlPrincipal principal = (SamlPrincipal) sentPrincipal; - String output = "attribute email: " + principal.getAttribute(X500SAMLProfileConstants.EMAIL.get()); - output += "
topAttribute: " + principal.getAttribute("topAttribute"); - output += "
boolean-attribute: " + principal.getAttribute("boolean-attribute"); - output += "
level2Attribute: " + principal.getAttribute("level2Attribute"); - output += "
group: " + principal.getAttributes("group").toString(); - output += "
friendlyAttribute email: " + principal.getFriendlyAttribute("email"); - output += "
phone: " + principal.getAttribute("phone"); - output += "
friendlyAttribute phone: " + principal.getFriendlyAttribute("phone"); - output += "
hardcoded-attribute: "; - for (String attr : principal.getAttributes("hardcoded-attribute")) { - output += attr + ","; - } - output += "
group-attribute: "; - for (String attr : principal.getAttributes("group-attribute")) { - output += attr + ","; + private static String joinList(String delimeter, List list) { + if (list == null || list.size() <= 0) return ""; + + StringBuilder sb = new StringBuilder(); + + for (int i = 0; i < list.size(); i++) { + + sb.append(list.get(i)); + + // if not the last item + if (i != list.size() - 1) { + sb.append(delimeter); + } + } - return output; + return sb.toString(); + } + + private String getAttributes() { + SamlPrincipal principal = (SamlPrincipal) sentPrincipal; + + StringBuilder b = new StringBuilder(); + for (Entry> e : principal.getAttributes().entrySet()) { + b.append(e.getKey()).append(": ").append(joinList(",", e.getValue())).append("
"); + } + + for (String friendlyAttributeName : principal.getFriendlyNames()) { + b.append("friendly ") + .append(friendlyAttributeName) + .append(": ") + .append(joinList(",", principal.getFriendlyAttributes(friendlyAttributeName))) + .append("
"); + } + + return b.toString(); } @GET diff --git a/testsuite/integration-arquillian/tests/base/src/main/java/org/keycloak/testsuite/util/TestCleanup.java b/testsuite/integration-arquillian/tests/base/src/main/java/org/keycloak/testsuite/util/TestCleanup.java index 2c0aa36697..b90bc31c7f 100644 --- a/testsuite/integration-arquillian/tests/base/src/main/java/org/keycloak/testsuite/util/TestCleanup.java +++ b/testsuite/integration-arquillian/tests/base/src/main/java/org/keycloak/testsuite/util/TestCleanup.java @@ -61,11 +61,12 @@ public class TestCleanup { } - public void addCleanup(Runnable r) { + public TestCleanup addCleanup(Runnable r) { genericCleanups.add(r); + return this; } - public void addCleanup(AutoCloseable c) { + public TestCleanup addCleanup(AutoCloseable c) { genericCleanups.add(() -> { try { c.close(); @@ -73,6 +74,7 @@ public class TestCleanup { // ignore } }); + return this; } public void addUserId(String userId) { diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/adapter/servlet/AbstractSAMLServletAdapterTest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/adapter/servlet/AbstractSAMLServletAdapterTest.java index f81a0576af..5230a7dbe7 100644 --- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/adapter/servlet/AbstractSAMLServletAdapterTest.java +++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/adapter/servlet/AbstractSAMLServletAdapterTest.java @@ -6,19 +6,30 @@ import org.apache.http.client.methods.HttpUriRequest; import org.apache.http.impl.client.CloseableHttpClient; import org.apache.http.impl.client.HttpClientBuilder; import org.apache.http.util.EntityUtils; +import org.keycloak.admin.client.resource.ProtocolMappersResource; +import org.keycloak.representations.idm.ProtocolMapperRepresentation; import org.keycloak.representations.idm.RealmRepresentation; import org.keycloak.testsuite.adapter.AbstractServletsAdapterTest; import org.keycloak.testsuite.adapter.filter.AdapterActionsFilter; +import org.keycloak.testsuite.auth.page.login.Login; +import org.keycloak.testsuite.page.AbstractPage; +import org.keycloak.testsuite.util.SamlClient; +import org.keycloak.testsuite.util.SamlClientBuilder; import org.keycloak.testsuite.utils.io.IOUtil; +import javax.ws.rs.core.Response; import javax.ws.rs.core.UriBuilder; import java.io.IOException; import java.util.Arrays; import java.util.List; +import java.util.Map; import java.util.regex.Matcher; import java.util.regex.Pattern; +import static org.keycloak.testsuite.admin.ApiUtil.getCreatedId; import static org.keycloak.testsuite.auth.page.AuthRealm.SAMLSERVLETDEMO; +import static org.keycloak.testsuite.util.URLAssert.assertCurrentUrlStartsWith; +import static org.keycloak.testsuite.util.WaitUtils.waitForPageToLoad; public abstract class AbstractSAMLServletAdapterTest extends AbstractServletsAdapterTest { @@ -59,4 +70,32 @@ public abstract class AbstractSAMLServletAdapterTest extends AbstractServletsAda } }); } + + protected SamlClientBuilder beginAuthenticationAndLogin(AbstractPage page, SamlClient.Binding binding) { + return new SamlClientBuilder() + .navigateTo(page.buildUri()) + .processSamlResponse(binding) // Process AuthnResponse + .build() + + .login().user(bburkeUser) + .build(); + } + + protected AutoCloseable createProtocolMapper(ProtocolMappersResource resource, String name, String protocol, String protocolMapper, Map config) { + ProtocolMapperRepresentation representation = new ProtocolMapperRepresentation(); + representation.setName(name); + representation.setProtocol(protocol); + representation.setProtocolMapper(protocolMapper); + representation.setConfig(config); + try (Response response = resource.createMapper(representation)) { + String createdId = getCreatedId(response); + return () -> resource.delete(createdId); + } + } + + protected void checkLoggedOut(AbstractPage page, Login loginPage) { + page.navigateTo(); + waitForPageToLoad(); + assertCurrentUrlStartsWith(loginPage); + } } diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/adapter/servlet/SAMLFilterLoginResponseHandlingTest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/adapter/servlet/SAMLFilterLoginResponseHandlingTest.java new file mode 100644 index 0000000000..326fc25c1f --- /dev/null +++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/adapter/servlet/SAMLFilterLoginResponseHandlingTest.java @@ -0,0 +1,34 @@ +package org.keycloak.testsuite.adapter.servlet; + +import org.junit.Ignore; +import org.junit.Test; +import org.keycloak.testsuite.arquillian.annotation.AppServerContainer; +import org.keycloak.testsuite.utils.annotation.UseServletFilter; +import org.keycloak.testsuite.utils.arquillian.ContainerConstants; + +/** + * @author mhajas + */ +@AppServerContainer(ContainerConstants.APP_SERVER_UNDERTOW) +@AppServerContainer(ContainerConstants.APP_SERVER_WILDFLY) +@AppServerContainer(ContainerConstants.APP_SERVER_WILDFLY_DEPRECATED) +@AppServerContainer(ContainerConstants.APP_SERVER_EAP) +@AppServerContainer(ContainerConstants.APP_SERVER_EAP6) +@AppServerContainer(ContainerConstants.APP_SERVER_EAP71) +@UseServletFilter(filterName = "saml-filter", filterClass = "org.keycloak.adapters.saml.servlet.SamlFilter", + filterDependency = "org.keycloak:keycloak-saml-servlet-filter-adapter") +public class SAMLFilterLoginResponseHandlingTest extends SAMLLoginResponseHandlingTest { + @Test + @Override + @Ignore + public void testErrorHandlingUnsigned() { + + } + + @Test + @Override + @Ignore + public void testErrorHandlingSigned() { + + } +} diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/adapter/servlet/SAMLFilterServletAdapterTest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/adapter/servlet/SAMLFilterServletAdapterTest.java index 685935522e..a6c12dd432 100644 --- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/adapter/servlet/SAMLFilterServletAdapterTest.java +++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/adapter/servlet/SAMLFilterServletAdapterTest.java @@ -92,20 +92,6 @@ public class SAMLFilterServletAdapterTest extends SAMLServletAdapterTest { } - @Test - @Override - @Ignore - public void testErrorHandlingUnsigned() { - - } - - @Test - @Override - @Ignore - public void testErrorHandlingSigned() { - - } - @Test @Override @Ignore diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/adapter/servlet/SAMLLoginResponseHandlingTest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/adapter/servlet/SAMLLoginResponseHandlingTest.java new file mode 100644 index 0000000000..6bf8da5d79 --- /dev/null +++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/adapter/servlet/SAMLLoginResponseHandlingTest.java @@ -0,0 +1,266 @@ +package org.keycloak.testsuite.adapter.servlet; + +import org.jboss.arquillian.container.test.api.Deployment; +import org.jboss.arquillian.graphene.page.Page; +import org.jboss.shrinkwrap.api.spec.WebArchive; +import org.junit.Assert; +import org.junit.Test; +import org.keycloak.adapters.rotation.PublicKeyLocator; +import org.keycloak.admin.client.resource.ClientResource; +import org.keycloak.admin.client.resource.ProtocolMappersResource; +import org.keycloak.dom.saml.v2.assertion.AttributeStatementType; +import org.keycloak.dom.saml.v2.assertion.AttributeType; +import org.keycloak.dom.saml.v2.assertion.StatementAbstractType; +import org.keycloak.dom.saml.v2.protocol.ResponseType; +import org.keycloak.protocol.saml.mappers.AttributeStatementHelper; +import org.keycloak.protocol.saml.mappers.RoleListMapper; +import org.keycloak.representations.idm.ProtocolMapperRepresentation; +import org.keycloak.saml.SAML2ErrorResponseBuilder; +import org.keycloak.saml.common.constants.JBossSAMLURIConstants; +import org.keycloak.saml.processing.core.saml.v2.constants.X500SAMLProfileConstants; +import org.keycloak.testsuite.adapter.filter.AdapterActionsFilter; +import org.keycloak.testsuite.adapter.page.Employee2Servlet; +import org.keycloak.testsuite.adapter.page.EmployeeSigServlet; +import org.keycloak.testsuite.admin.ApiUtil; +import org.keycloak.testsuite.arquillian.annotation.AppServerContainer; +import org.keycloak.testsuite.saml.AbstractSamlTest; +import org.keycloak.testsuite.util.Matchers; +import org.keycloak.testsuite.util.SamlClient; +import org.keycloak.testsuite.util.SamlClientBuilder; +import org.keycloak.testsuite.util.WaitUtils; +import org.keycloak.testsuite.utils.arquillian.ContainerConstants; +import org.openqa.selenium.By; +import org.w3c.dom.Document; + +import javax.ws.rs.core.Response; +import java.net.URI; +import java.util.HashMap; +import java.util.LinkedHashMap; +import java.util.Map; +import java.util.Set; + +import static org.hamcrest.Matchers.containsString; +import static org.junit.Assert.assertThat; +import static org.keycloak.testsuite.admin.ApiUtil.getCreatedId; +import static org.keycloak.testsuite.saml.AbstractSamlTest.REALM_PRIVATE_KEY; +import static org.keycloak.testsuite.saml.AbstractSamlTest.REALM_PUBLIC_KEY; +import static org.keycloak.testsuite.util.Matchers.bodyHC; +import static org.keycloak.testsuite.util.Matchers.statusCodeIsHC; +import static org.keycloak.testsuite.util.URLAssert.assertCurrentUrlStartsWith; +import static org.keycloak.testsuite.util.WaitUtils.waitForPageToLoad; +import static org.keycloak.testsuite.util.WaitUtils.waitUntilElement; + +/** + * @author mhajas + */ +@AppServerContainer(ContainerConstants.APP_SERVER_UNDERTOW) +@AppServerContainer(ContainerConstants.APP_SERVER_WILDFLY) +@AppServerContainer(ContainerConstants.APP_SERVER_WILDFLY_DEPRECATED) +@AppServerContainer(ContainerConstants.APP_SERVER_EAP) +@AppServerContainer(ContainerConstants.APP_SERVER_EAP6) +@AppServerContainer(ContainerConstants.APP_SERVER_EAP71) +@AppServerContainer(ContainerConstants.APP_SERVER_TOMCAT7) +@AppServerContainer(ContainerConstants.APP_SERVER_TOMCAT8) +@AppServerContainer(ContainerConstants.APP_SERVER_TOMCAT9) +@AppServerContainer(ContainerConstants.APP_SERVER_JETTY92) +@AppServerContainer(ContainerConstants.APP_SERVER_JETTY93) +@AppServerContainer(ContainerConstants.APP_SERVER_JETTY94) +public class SAMLLoginResponseHandlingTest extends AbstractSAMLServletAdapterTest { + + @Page + protected Employee2Servlet employee2ServletPage; + + @Page + protected EmployeeSigServlet employeeSigServletPage; + + @Deployment(name = Employee2Servlet.DEPLOYMENT_NAME) + protected static WebArchive employee2() { + return samlServletDeployment(Employee2Servlet.DEPLOYMENT_NAME, WEB_XML_WITH_ACTION_FILTER, SendUsernameServlet.class, AdapterActionsFilter.class, PublicKeyLocator.class); + } + + @Deployment(name = EmployeeSigServlet.DEPLOYMENT_NAME) + protected static WebArchive employeeSig() { + return samlServletDeployment(EmployeeSigServlet.DEPLOYMENT_NAME, SendUsernameServlet.class); + } + + @Test + public void testNilAttributeValueAttribute() { + beginAuthenticationAndLogin(employee2ServletPage, SamlClient.Binding.POST) + .processSamlResponse(SamlClient.Binding.POST) // Update response with Nil attribute + .transformObject(ob -> { + assertThat(ob, Matchers.isSamlResponse(JBossSAMLURIConstants.STATUS_SUCCESS)); + ResponseType resp = (ResponseType) ob; + + Set statements = resp.getAssertions().get(0).getAssertion().getStatements(); + + AttributeStatementType attributeType = (AttributeStatementType) statements.stream() + .filter(statement -> statement instanceof AttributeStatementType) + .findFirst().orElse(new AttributeStatementType()); + + AttributeType attr = new AttributeType("attribute-with-null-attribute-value"); + attr.addAttributeValue(null); + + attributeType.addAttribute(new AttributeStatementType.ASTChoiceType(attr)); + resp.getAssertions().get(0).getAssertion().addStatement(attributeType); + + return ob; + }) + .build() + .navigateTo(employee2ServletPage.getUriBuilder().clone().path("getAttributes").build()) + .execute(response -> { + Assert.assertThat(response, statusCodeIsHC(Response.Status.OK)); + Assert.assertThat(response, bodyHC(containsString("attribute-with-null-attribute-value:
"))); + } + ); + } + + @Test + public void testErrorHandlingUnsigned() throws Exception { + SAML2ErrorResponseBuilder builder = new SAML2ErrorResponseBuilder() + .destination(employeeSigServletPage.toString() + "/saml") + .issuer("http://localhost:" + System.getProperty("auth.server.http.port", "8180") + "/realms/demo") + .status(JBossSAMLURIConstants.STATUS_REQUEST_DENIED.get()); + Document document = builder.buildDocument(); + + new SamlClientBuilder() + .addStep((client, currentURI, currentResponse, context) -> + SamlClient.Binding.REDIRECT.createSamlUnsignedResponse(URI.create(employeeSigServletPage.toString() + "/saml"), null, document)) + .execute(closeableHttpResponse -> Assert.assertThat(closeableHttpResponse, bodyHC(containsString("INVALID_SIGNATURE")))); + } + + @Test + public void testErrorHandlingSigned() throws Exception { + SAML2ErrorResponseBuilder builder = new SAML2ErrorResponseBuilder() + .destination(employeeSigServletPage.toString() + "/saml") + .issuer("http://localhost:" + System.getProperty("auth.server.http.port", "8180") + "/realms/demo") + .status(JBossSAMLURIConstants.STATUS_REQUEST_DENIED.get()); + Document document = builder.buildDocument(); + + new SamlClientBuilder() + .addStep((client, currentURI, currentResponse, context) -> + SamlClient.Binding.REDIRECT.createSamlSignedResponse(URI.create(employeeSigServletPage.toString() + "/saml"), null, document, REALM_PRIVATE_KEY, REALM_PUBLIC_KEY)) + .execute(closeableHttpResponse -> Assert.assertThat(closeableHttpResponse, bodyHC(containsString("ERROR_STATUS")))); + } + + @Test + public void testAttributes() throws Exception { + ClientResource clientResource = ApiUtil.findClientResourceByClientId(testRealmResource(), AbstractSamlTest.SAML_CLIENT_ID_EMPLOYEE_2); + ProtocolMappersResource protocolMappersResource = clientResource.getProtocolMappers(); + + Map config = new LinkedHashMap<>(); + config.put("attribute.nameformat", "Basic"); + config.put("user.attribute", "topAttribute"); + config.put("attribute.name", "topAttribute"); + getCleanup().addCleanup(createProtocolMapper(protocolMappersResource, "topAttribute", "saml", "saml-user-attribute-mapper", config)); + + config = new LinkedHashMap<>(); + config.put("attribute.nameformat", "Basic"); + config.put("user.attribute", "level2Attribute"); + config.put("attribute.name", "level2Attribute"); + getCleanup().addCleanup(createProtocolMapper(protocolMappersResource, "level2Attribute", "saml", "saml-user-attribute-mapper", config)); + + config = new LinkedHashMap<>(); + config.put("attribute.nameformat", "Basic"); + config.put("single", "true"); + config.put("attribute.name", "group"); + getCleanup().addCleanup(createProtocolMapper(protocolMappersResource, "groups", "saml", "saml-group-membership-mapper", config)); + + setRolesToCheck("manager,user"); + + employee2ServletPage.navigateTo(); + assertCurrentUrlStartsWith(testRealmSAMLPostLoginPage); + testRealmSAMLPostLoginPage.form().login("level2GroupUser", "password"); + + driver.navigate().to(employee2ServletPage.getUriBuilder().clone().path("getAttributes").build().toURL()); + waitUntilElement(By.xpath("//body")).text().contains("topAttribute: true"); + waitUntilElement(By.xpath("//body")).text().contains("level2Attribute: true"); + waitUntilElement(By.xpath("//body")).text().contains(X500SAMLProfileConstants.EMAIL.get() + ": level2@redhat.com"); + waitUntilElement(By.xpath("//body")).text().not().contains("group: []"); + waitUntilElement(By.xpath("//body")).text().not().contains("group: null"); + waitUntilElement(By.xpath("//body")).text().not().contains("group:
"); + waitUntilElement(By.xpath("//body")).text().contains("group: level2"); + + employee2ServletPage.logout(); + checkLoggedOut(employee2ServletPage, testRealmSAMLPostLoginPage); + + setRolesToCheck("manager,employee,user"); + + employee2ServletPage.navigateTo(); + assertCurrentUrlStartsWith(testRealmSAMLPostLoginPage); + testRealmSAMLPostLoginPage.form().login(bburkeUser); + + driver.navigate().to(employee2ServletPage.getUriBuilder().clone().path("getAttributes").build().toURL()); + waitUntilElement(By.xpath("//body")).text().contains(X500SAMLProfileConstants.EMAIL.get() + ": bburke@redhat.com"); + waitUntilElement(By.xpath("//body")).text().contains("friendly email: bburke@redhat.com"); + waitUntilElement(By.xpath("//body")).text().contains("phone: 617"); + waitUntilElement(By.xpath("//body")).text().not().contains("friendly phone:"); + + driver.navigate().to(employee2ServletPage.getUriBuilder().clone().path("getAssertionFromDocument").build().toURL()); + waitForPageToLoad(); + Assert.assertEquals("", driver.getPageSource()); + + employee2ServletPage.logout(); + checkLoggedOut(employee2ServletPage, testRealmSAMLPostLoginPage); + + config = new LinkedHashMap<>(); + config.put("attribute.value", "hard"); + config.put("attribute.nameformat", "Basic"); + config.put("attribute.name", "hardcoded-attribute"); + getCleanup().addCleanup(createProtocolMapper(protocolMappersResource, "hardcoded-attribute", "saml", "saml-hardcode-attribute-mapper", config)); + + config = new LinkedHashMap<>(); + config.put("role", "hardcoded-role"); + getCleanup().addCleanup(createProtocolMapper(protocolMappersResource, "hardcoded-role", "saml", "saml-hardcode-role-mapper", config)); + + config = new LinkedHashMap<>(); + config.put("new.role.name", "pee-on"); + config.put("role", "http://localhost:8280/employee/.employee"); + getCleanup().addCleanup(createProtocolMapper(protocolMappersResource, "renamed-employee-role", "saml", "saml-role-name-mapper", config)); + + for (ProtocolMapperRepresentation mapper : clientResource.toRepresentation().getProtocolMappers()) { + if (mapper.getName().equals("role-list")) { + protocolMappersResource.delete(mapper.getId()); + Map origConfig = new HashMap<>(mapper.getConfig()); + + mapper.setId(null); + mapper.getConfig().put(RoleListMapper.SINGLE_ROLE_ATTRIBUTE, "true"); + mapper.getConfig().put(AttributeStatementHelper.SAML_ATTRIBUTE_NAME, "memberOf"); + + try (Response response = protocolMappersResource.createMapper(mapper)) { + String createdId = getCreatedId(response); + getCleanup().addCleanup((Runnable) () -> { + protocolMappersResource.delete(createdId); + mapper.setConfig(origConfig); + protocolMappersResource.createMapper(mapper).close(); + }); + } + } + } + + setRolesToCheck("pee-on,el-jefe,manager,hardcoded-role"); + + config = new LinkedHashMap<>(); + config.put("new.role.name", "el-jefe"); + config.put("role", "user"); + getCleanup().addCleanup(createProtocolMapper(protocolMappersResource, "renamed-role", "saml", "saml-role-name-mapper", config)); + + employee2ServletPage.navigateTo(); + assertCurrentUrlStartsWith(testRealmSAMLPostLoginPage); + testRealmSAMLPostLoginPage.form().login(bburkeUser); + + driver.navigate().to(employee2ServletPage.getUriBuilder().clone().path("getAttributes").build().toURL()); + waitUntilElement(By.xpath("//body")).text().contains("hardcoded-attribute: hard"); + employee2ServletPage.checkRolesEndPoint(false); + employee2ServletPage.logout(); + checkLoggedOut(employee2ServletPage, testRealmSAMLPostLoginPage); + } + + private void setRolesToCheck(String roles) throws Exception { + employee2ServletPage.navigateTo(); + assertCurrentUrlStartsWith(testRealmSAMLPostLoginPage); + testRealmSAMLPostLoginPage.form().login(bburkeUser); + driver.navigate().to(employee2ServletPage.getUriBuilder().clone().path("setCheckRoles").queryParam("roles", roles).build().toURL()); + WaitUtils.waitUntilElement(By.tagName("body")).text().contains("These roles will be checked:"); + employee2ServletPage.logout(); + } +} diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/adapter/servlet/SAMLServletAdapterTest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/adapter/servlet/SAMLServletAdapterTest.java index 9fca29ec0a..58dfeeab54 100644 --- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/adapter/servlet/SAMLServletAdapterTest.java +++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/adapter/servlet/SAMLServletAdapterTest.java @@ -515,12 +515,6 @@ public class SAMLServletAdapterTest extends AbstractSAMLServletAdapterTest { checkLoggedOut(page, loginPage); } - private void checkLoggedOut(AbstractPage page, Login loginPage) { - page.navigateTo(); - waitForPageToLoad(); - assertCurrentUrlStartsWith(loginPage); - } - @Test public void disabledClientTest() { ClientResource clientResource = ApiUtil.findClientResourceByClientId(testRealmResource(), AbstractSamlTest.SAML_CLIENT_ID_SALES_POST_SIG); @@ -1152,34 +1146,6 @@ public class SAMLServletAdapterTest extends AbstractSAMLServletAdapterTest { Assert.assertEquals(driver.getCurrentUrl(), missingAssertionSigPage.getUriBuilder().clone().path("saml").build().toASCIIString()); } - @Test - public void testErrorHandlingUnsigned() throws Exception { - SAML2ErrorResponseBuilder builder = new SAML2ErrorResponseBuilder() - .destination(employeeSigServletPage.toString() + "/saml") - .issuer("http://localhost:" + System.getProperty("auth.server.http.port", "8180") + "/realms/demo") - .status(JBossSAMLURIConstants.STATUS_REQUEST_DENIED.get()); - Document document = builder.buildDocument(); - - new SamlClientBuilder() - .addStep((client, currentURI, currentResponse, context) -> - Binding.REDIRECT.createSamlUnsignedResponse(URI.create(employeeSigServletPage.toString() + "/saml"), null, document)) - .execute(closeableHttpResponse -> Assert.assertThat(closeableHttpResponse, bodyHC(containsString("INVALID_SIGNATURE")))); - } - - @Test - public void testErrorHandlingSigned() throws Exception { - SAML2ErrorResponseBuilder builder = new SAML2ErrorResponseBuilder() - .destination(employeeSigServletPage.toString() + "/saml") - .issuer("http://localhost:" + System.getProperty("auth.server.http.port", "8180") + "/realms/demo") - .status(JBossSAMLURIConstants.STATUS_REQUEST_DENIED.get()); - Document document = builder.buildDocument(); - - new SamlClientBuilder() - .addStep((client, currentURI, currentResponse, context) -> - Binding.REDIRECT.createSamlSignedResponse(URI.create(employeeSigServletPage.toString() + "/saml"), null, document, REALM_PRIVATE_KEY, REALM_PUBLIC_KEY)) - .execute(closeableHttpResponse -> Assert.assertThat(closeableHttpResponse, bodyHC(containsString("ERROR_STATUS")))); - } - @Test public void testRelayStateEncoding() throws Exception { // this test has a hardcoded SAMLRequest and we hack a SP face servlet to get the SAMLResponse so we can look @@ -1381,118 +1347,6 @@ public class SAMLServletAdapterTest extends AbstractSAMLServletAdapterTest { } } - @Test - public void testAttributes() throws Exception { - ClientResource clientResource = ApiUtil.findClientResourceByClientId(testRealmResource(), AbstractSamlTest.SAML_CLIENT_ID_EMPLOYEE_2); - ProtocolMappersResource protocolMappersResource = clientResource.getProtocolMappers(); - - Map config = new LinkedHashMap<>(); - config.put("attribute.nameformat", "Basic"); - config.put("user.attribute", "topAttribute"); - config.put("attribute.name", "topAttribute"); - getCleanup().addCleanup(createProtocolMapper(protocolMappersResource, "topAttribute", "saml", "saml-user-attribute-mapper", config)); - - config = new LinkedHashMap<>(); - config.put("attribute.nameformat", "Basic"); - config.put("user.attribute", "level2Attribute"); - config.put("attribute.name", "level2Attribute"); - getCleanup().addCleanup(createProtocolMapper(protocolMappersResource, "level2Attribute", "saml", "saml-user-attribute-mapper", config)); - - config = new LinkedHashMap<>(); - config.put("attribute.nameformat", "Basic"); - config.put("single", "true"); - config.put("attribute.name", "group"); - getCleanup().addCleanup(createProtocolMapper(protocolMappersResource, "groups", "saml", "saml-group-membership-mapper", config)); - - setRolesToCheck("manager,user"); - - employee2ServletPage.navigateTo(); - assertCurrentUrlStartsWith(testRealmSAMLPostLoginPage); - testRealmSAMLPostLoginPage.form().login("level2GroupUser", "password"); - - driver.navigate().to(employee2ServletPage.getUriBuilder().clone().path("getAttributes").build().toURL()); - waitUntilElement(By.xpath("//body")).text().contains("topAttribute: true"); - waitUntilElement(By.xpath("//body")).text().contains("level2Attribute: true"); - waitUntilElement(By.xpath("//body")).text().contains("attribute email: level2@redhat.com"); - waitUntilElement(By.xpath("//body")).text().not().contains("group: []"); - waitUntilElement(By.xpath("//body")).text().not().contains("group: null"); - waitUntilElement(By.xpath("//body")).text().contains("group: [level2]"); - - employee2ServletPage.logout(); - checkLoggedOut(employee2ServletPage, testRealmSAMLPostLoginPage); - - setRolesToCheck("manager,employee,user"); - - employee2ServletPage.navigateTo(); - assertCurrentUrlStartsWith(testRealmSAMLPostLoginPage); - testRealmSAMLPostLoginPage.form().login(bburkeUser); - - driver.navigate().to(employee2ServletPage.getUriBuilder().clone().path("getAttributes").build().toURL()); - waitUntilElement(By.xpath("//body")).text().contains("attribute email: bburke@redhat.com"); - waitUntilElement(By.xpath("//body")).text().contains("friendlyAttribute email: bburke@redhat.com"); - waitUntilElement(By.xpath("//body")).text().contains("phone: 617"); - waitUntilElement(By.xpath("//body")).text().contains("friendlyAttribute phone: null"); - - driver.navigate().to(employee2ServletPage.getUriBuilder().clone().path("getAssertionFromDocument").build().toURL()); - waitForPageToLoad(); - Assert.assertEquals("", driver.getPageSource()); - - employee2ServletPage.logout(); - checkLoggedOut(employee2ServletPage, testRealmSAMLPostLoginPage); - - config = new LinkedHashMap<>(); - config.put("attribute.value", "hard"); - config.put("attribute.nameformat", "Basic"); - config.put("attribute.name", "hardcoded-attribute"); - getCleanup().addCleanup(createProtocolMapper(protocolMappersResource, "hardcoded-attribute", "saml", "saml-hardcode-attribute-mapper", config)); - - config = new LinkedHashMap<>(); - config.put("role", "hardcoded-role"); - getCleanup().addCleanup(createProtocolMapper(protocolMappersResource, "hardcoded-role", "saml", "saml-hardcode-role-mapper", config)); - - config = new LinkedHashMap<>(); - config.put("new.role.name", "pee-on"); - config.put("role", "http://localhost:8280/employee/.employee"); - getCleanup().addCleanup(createProtocolMapper(protocolMappersResource, "renamed-employee-role", "saml", "saml-role-name-mapper", config)); - - for (ProtocolMapperRepresentation mapper : clientResource.toRepresentation().getProtocolMappers()) { - if (mapper.getName().equals("role-list")) { - protocolMappersResource.delete(mapper.getId()); - Map origConfig = new HashMap<>(mapper.getConfig()); - - mapper.setId(null); - mapper.getConfig().put(RoleListMapper.SINGLE_ROLE_ATTRIBUTE, "true"); - mapper.getConfig().put(AttributeStatementHelper.SAML_ATTRIBUTE_NAME, "memberOf"); - - try (Response response = protocolMappersResource.createMapper(mapper)) { - String createdId = getCreatedId(response); - getCleanup().addCleanup((Runnable) () -> { - protocolMappersResource.delete(createdId); - mapper.setConfig(origConfig); - protocolMappersResource.createMapper(mapper).close(); - }); - } - } - } - - setRolesToCheck("pee-on,el-jefe,manager,hardcoded-role"); - - config = new LinkedHashMap<>(); - config.put("new.role.name", "el-jefe"); - config.put("role", "user"); - getCleanup().addCleanup(createProtocolMapper(protocolMappersResource, "renamed-role", "saml", "saml-role-name-mapper", config)); - - employee2ServletPage.navigateTo(); - assertCurrentUrlStartsWith(testRealmSAMLPostLoginPage); - testRealmSAMLPostLoginPage.form().login(bburkeUser); - - driver.navigate().to(employee2ServletPage.getUriBuilder().clone().path("getAttributes").build().toURL()); - waitUntilElement(By.xpath("//body")).text().contains("hardcoded-attribute: hard"); - employee2ServletPage.checkRolesEndPoint(false); - employee2ServletPage.logout(); - checkLoggedOut(employee2ServletPage, testRealmSAMLPostLoginPage); - } - @Test public void idpMetadataValidation() throws Exception { driver.navigate().to(authServerPage.toString() + "/realms/" + SAMLSERVLETDEMO + "/protocol/saml/descriptor"); @@ -1939,27 +1793,6 @@ public class SAMLServletAdapterTest extends AbstractSAMLServletAdapterTest { } } - private AutoCloseable createProtocolMapper(ProtocolMappersResource resource, String name, String protocol, String protocolMapper, Map config) { - ProtocolMapperRepresentation representation = new ProtocolMapperRepresentation(); - representation.setName(name); - representation.setProtocol(protocol); - representation.setProtocolMapper(protocolMapper); - representation.setConfig(config); - try (Response response = resource.createMapper(representation)) { - String createdId = getCreatedId(response); - return () -> resource.delete(createdId); - } - } - - private void setRolesToCheck(String roles) throws Exception { - employee2ServletPage.navigateTo(); - assertCurrentUrlStartsWith(testRealmSAMLPostLoginPage); - testRealmSAMLPostLoginPage.form().login(bburkeUser); - driver.navigate().to(employee2ServletPage.getUriBuilder().clone().path("setCheckRoles").queryParam("roles", roles).build().toURL()); - WaitUtils.waitUntilElement(By.tagName("body")).text().contains("These roles will be checked:"); - employee2ServletPage.logout(); - } - private void assertOnForbiddenPage() { waitUntilElement(By.xpath("//body")).is().present(); diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/adapter/servlet/SAMLServletSessionTimeoutTest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/adapter/servlet/SAMLServletSessionTimeoutTest.java index 62293b9d79..7e19bc0022 100644 --- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/adapter/servlet/SAMLServletSessionTimeoutTest.java +++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/adapter/servlet/SAMLServletSessionTimeoutTest.java @@ -79,21 +79,11 @@ public class SAMLServletSessionTimeoutTest extends AbstractSAMLServletAdapterTes return ob; } - private SamlClientBuilder beginAuthenticationAndLogin() { - return new SamlClientBuilder() - .navigateTo(employee2ServletPage.buildUri()) - .processSamlResponse(SamlClient.Binding.POST) // Process AuthnResponse - .build() - - .login().user(bburkeUser) - .build(); - } - @Test public void employee2TestSAMLRefreshingSession() { sessionNotOnOrAfter.set(null); - beginAuthenticationAndLogin() + beginAuthenticationAndLogin(employee2ServletPage, SamlClient.Binding.POST) .processSamlResponse(SamlClient.Binding.POST) // Update response with SessionNotOnOrAfter .transformObject(this::addSessionNotOnOrAfter) .build() @@ -125,7 +115,7 @@ public class SAMLServletSessionTimeoutTest extends AbstractSAMLServletAdapterTes public void employee2TestSAMLSessionTimeoutOnBothSides() { sessionNotOnOrAfter.set(null); - beginAuthenticationAndLogin() + beginAuthenticationAndLogin(employee2ServletPage, SamlClient.Binding.POST) .processSamlResponse(SamlClient.Binding.POST) // Update response with SessionNotOnOrAfter .transformObject(this::addSessionNotOnOrAfter) .build() @@ -158,7 +148,7 @@ public class SAMLServletSessionTimeoutTest extends AbstractSAMLServletAdapterTes try(AutoCloseable c = new RealmAttributeUpdater(adminClient.realm(REALM_NAME)) .updateWith(r -> r.setSsoSessionMaxLifespan(SESSION_LENGTH_IN_SECONDS)) .update()) { - beginAuthenticationAndLogin() + beginAuthenticationAndLogin(employee2ServletPage, SamlClient.Binding.POST) .processSamlResponse(SamlClient.Binding.POST) // Process response .transformObject(ob -> { // Check sessionNotOnOrAfter is present and it has correct value assertThat(ob, Matchers.isSamlResponse(JBossSAMLURIConstants.STATUS_SUCCESS)); diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/broker/AbstractBrokerTest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/broker/AbstractBrokerTest.java index 5cf53b793a..6be3775141 100644 --- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/broker/AbstractBrokerTest.java +++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/broker/AbstractBrokerTest.java @@ -32,6 +32,7 @@ public abstract class AbstractBrokerTest extends AbstractInitializedBaseBrokerTe public static final String ROLE_MANAGER = "manager"; public static final String ROLE_FRIENDLY_MANAGER = "friendly-manager"; public static final String ROLE_USER_DOT_GUIDE = "user.guide"; + public static final String EMPTY_ATTRIBUTE_ROLE = "empty.attribute.role"; @Page ConsentPage consentPage; @@ -112,11 +113,13 @@ public abstract class AbstractBrokerTest extends AbstractInitializedBaseBrokerTe RoleRepresentation friendlyManagerRole = new RoleRepresentation(ROLE_FRIENDLY_MANAGER,null, false); RoleRepresentation userRole = new RoleRepresentation(ROLE_USER,null, false); RoleRepresentation userGuideRole = new RoleRepresentation(ROLE_USER_DOT_GUIDE,null, false); + RoleRepresentation emptyAttributeRole = new RoleRepresentation(EMPTY_ATTRIBUTE_ROLE, null, false); adminClient.realm(realm).roles().create(managerRole); adminClient.realm(realm).roles().create(friendlyManagerRole); adminClient.realm(realm).roles().create(userRole); adminClient.realm(realm).roles().create(userGuideRole); + adminClient.realm(realm).roles().create(emptyAttributeRole); } static void enableUpdateProfileOnFirstLogin(AuthenticationExecutionInfoRepresentation execution, AuthenticationManagementResource flows) { 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 bcf2289ac9..5f10f5d87b 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,11 +4,19 @@ import org.keycloak.admin.client.resource.UserResource; import com.google.common.collect.ImmutableMap; import org.keycloak.broker.saml.mappers.AttributeToRoleMapper; import org.keycloak.broker.saml.mappers.UserAttributeMapper; +import org.keycloak.dom.saml.v2.assertion.AssertionType; +import org.keycloak.dom.saml.v2.assertion.AttributeStatementType; +import org.keycloak.dom.saml.v2.assertion.AttributeType; +import org.keycloak.dom.saml.v2.assertion.StatementAbstractType; import org.keycloak.dom.saml.v2.protocol.AuthnRequestType; +import org.keycloak.dom.saml.v2.protocol.ResponseType; import org.keycloak.representations.idm.IdentityProviderMapperRepresentation; import org.keycloak.representations.idm.RoleRepresentation; import org.keycloak.representations.idm.UserRepresentation; import org.keycloak.saml.common.constants.JBossSAMLURIConstants; +import org.keycloak.saml.common.exceptions.ConfigurationException; +import org.keycloak.saml.common.exceptions.ParsingException; +import org.keycloak.saml.common.exceptions.ProcessingException; import org.keycloak.saml.processing.api.saml.v2.request.SAML2Request; import org.keycloak.saml.processing.core.saml.v2.common.SAMLDocumentHolder; import org.keycloak.testsuite.saml.AbstractSamlTest; @@ -20,6 +28,8 @@ import java.util.Collections; import java.util.HashMap; import java.util.Set; import java.util.stream.Collectors; +import java.util.stream.Stream; + import org.hamcrest.Matchers; import org.junit.Assert; import org.junit.Test; @@ -30,7 +40,11 @@ import static org.junit.Assert.assertThat; import static org.keycloak.testsuite.arquillian.AuthServerTestEnricher.getAuthServerContextRoot; import static org.keycloak.testsuite.broker.AbstractBrokerTest.ROLE_MANAGER; import static org.keycloak.testsuite.broker.AbstractBrokerTest.ROLE_USER; +import static org.keycloak.testsuite.saml.RoleMapperTest.ROLE_ATTRIBUTE_NAME; import static org.keycloak.testsuite.util.Matchers.isSamlResponse; +import static org.keycloak.testsuite.util.SamlStreams.assertionsUnencrypted; +import static org.keycloak.testsuite.util.SamlStreams.attributeStatements; +import static org.keycloak.testsuite.util.SamlStreams.attributesUnecrypted; /** * Final class as it's not intended to be overriden. Feel free to remove "final" if you really know what you are doing. @@ -42,6 +56,8 @@ public final class KcSamlBrokerTest extends AbstractAdvancedBrokerTest { return KcSamlBrokerConfiguration.INSTANCE; } + private static final String EMPTY_ATTRIBUTE_NAME = "empty.attribute.name"; + @Override protected Iterable createIdentityProviderMappers() { IdentityProviderMapperRepresentation attrMapper1 = new IdentityProviderMapperRepresentation(); @@ -80,7 +96,16 @@ public final class KcSamlBrokerTest extends AbstractAdvancedBrokerTest { .put("role", ROLE_USER_DOT_GUIDE) .build()); - return Arrays.asList(new IdentityProviderMapperRepresentation[] { attrMapper1, attrMapper2, attrMapper3, attrMapper4 }); + IdentityProviderMapperRepresentation attrMapper5 = new IdentityProviderMapperRepresentation(); + attrMapper5.setName("empty-attribute-to-role-mapper"); + attrMapper5.setIdentityProviderMapper(AttributeToRoleMapper.PROVIDER_ID); + attrMapper5.setConfig(ImmutableMap.builder() + .put(UserAttributeMapper.ATTRIBUTE_NAME, EMPTY_ATTRIBUTE_NAME) + .put(ATTRIBUTE_VALUE, "") + .put("role", EMPTY_ATTRIBUTE_ROLE) + .build()); + + return Arrays.asList(new IdentityProviderMapperRepresentation[] { attrMapper1, attrMapper2, attrMapper3, attrMapper4, attrMapper5 }); } // KEYCLOAK-3987 @@ -231,4 +256,64 @@ public final class KcSamlBrokerTest extends AbstractAdvancedBrokerTest { Assert.assertThat(samlResponse.getSamlObject(), isSamlResponse(JBossSAMLURIConstants.STATUS_SUCCESS)); } + @Test + public void emptyAttributeToRoleMapperTest() throws ParsingException, ConfigurationException, ProcessingException { + createRolesForRealm(bc.consumerRealmName()); + createRoleMappersForConsumerRealm(); + + AuthnRequestType loginRep = SamlClient.createLoginRequestDocument(AbstractSamlTest.SAML_CLIENT_ID_SALES_POST + ".dot/ted", getAuthServerContextRoot() + "/sales-post/saml", null); + + Document doc = SAML2Request.convert(loginRep); + + SAMLDocumentHolder samlResponse = new SamlClientBuilder() + .authnRequest(getAuthServerSamlEndpoint(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 + .transformObject(ob -> { + assertThat(ob, org.keycloak.testsuite.util.Matchers.isSamlResponse(JBossSAMLURIConstants.STATUS_SUCCESS)); + ResponseType resp = (ResponseType) ob; + + Set statements = resp.getAssertions().get(0).getAssertion().getStatements(); + + AttributeStatementType attributeType = (AttributeStatementType) statements.stream() + .filter(statement -> statement instanceof AttributeStatementType) + .findFirst().orElse(new AttributeStatementType()); + + AttributeType attr = new AttributeType(EMPTY_ATTRIBUTE_NAME); + attr.addAttributeValue(null); + + attributeType.addAttribute(new AttributeStatementType.ASTChoiceType(attr)); + resp.getAssertions().get(0).getAssertion().addStatement(attributeType); + + return ob; + }) + .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)); + + Stream assertionTypeStream = assertionsUnencrypted(samlResponse.getSamlObject()); + Stream attributeStatementTypeStream = attributesUnecrypted(attributeStatements(assertionTypeStream)); + Set attributeValues = attributeStatementTypeStream + .filter(a -> a.getName().equals(ROLE_ATTRIBUTE_NAME)) + .flatMap(a -> a.getAttributeValue().stream()) + .map(Object::toString) + .collect(Collectors.toSet()); + + assertThat(attributeValues, hasItems(EMPTY_ATTRIBUTE_ROLE)); + + } } diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/saml/ProtocolMapperTest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/saml/ProtocolMapperTest.java new file mode 100644 index 0000000000..35f4d38a0c --- /dev/null +++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/saml/ProtocolMapperTest.java @@ -0,0 +1,80 @@ +package org.keycloak.testsuite.saml; + +import org.junit.Before; +import org.junit.Test; +import org.keycloak.dom.saml.v2.assertion.AssertionType; +import org.keycloak.dom.saml.v2.assertion.AttributeType; +import org.keycloak.protocol.saml.mappers.AttributeStatementHelper; +import org.keycloak.protocol.saml.mappers.HardcodedAttributeMapper; +import org.keycloak.saml.common.constants.JBossSAMLURIConstants; +import org.keycloak.saml.processing.core.saml.v2.common.SAMLDocumentHolder; +import org.keycloak.testsuite.updaters.ClientAttributeUpdater; +import org.keycloak.testsuite.updaters.ProtocolMappersUpdater; +import org.keycloak.testsuite.util.Matchers; +import org.keycloak.testsuite.util.SamlClient; +import org.keycloak.testsuite.util.SamlClientBuilder; + +import java.util.Collections; +import java.util.Set; +import java.util.stream.Collectors; +import java.util.stream.Stream; + +import static org.hamcrest.CoreMatchers.nullValue; +import static org.hamcrest.Matchers.hasSize; +import static org.junit.Assert.assertThat; +import static org.keycloak.testsuite.saml.RoleMapperTest.createSamlProtocolMapper; +import static org.keycloak.testsuite.util.SamlStreams.assertionsUnencrypted; +import static org.keycloak.testsuite.util.SamlStreams.attributeStatements; +import static org.keycloak.testsuite.util.SamlStreams.attributesUnecrypted; + +/** + * @author mhajas + */ +public class ProtocolMapperTest extends AbstractSamlTest { + + private ClientAttributeUpdater cau; + private ProtocolMappersUpdater pmu; + + @Before + public void cleanMappersAndScopes() { + this.cau = ClientAttributeUpdater.forClient(adminClient, REALM_NAME, SAML_CLIENT_ID_EMPLOYEE_2) + .setDefaultClientScopes(Collections.EMPTY_LIST) + .update(); + this.pmu = cau.protocolMappers() + .clear() + .update(); + + getCleanup(REALM_NAME) + .addCleanup(this.cau) + .addCleanup(this.pmu); + } + + @Test + public void hardcodedAttributeMapperWithNullValueTest() throws Exception { + pmu.add( + createSamlProtocolMapper(HardcodedAttributeMapper.PROVIDER_ID, + AttributeStatementHelper.SAML_ATTRIBUTE_NAME, "HARDCODED_ATTRIBUTE", + AttributeStatementHelper.SAML_ATTRIBUTE_NAMEFORMAT, AttributeStatementHelper.BASIC, + HardcodedAttributeMapper.ATTRIBUTE_VALUE, null + ) + ).update(); + + + SAMLDocumentHolder samlResponse = new SamlClientBuilder() + .authnRequest(getAuthServerSamlEndpoint(REALM_NAME), SAML_CLIENT_ID_EMPLOYEE_2, RoleMapperTest.SAML_ASSERTION_CONSUMER_URL_EMPLOYEE_2, SamlClient.Binding.POST) + .build() + .login().user(bburkeUser).build() + .getSamlResponse(SamlClient.Binding.POST); + + assertThat(samlResponse.getSamlObject(), Matchers.isSamlResponse(JBossSAMLURIConstants.STATUS_SUCCESS)); + + Stream assertions = assertionsUnencrypted(samlResponse.getSamlObject()); + Stream attributes = attributesUnecrypted(attributeStatements(assertions)); + Set attributeValues = attributes + .flatMap(a -> a.getAttributeValue().stream()) + .collect(Collectors.toSet()); + + assertThat(attributeValues, hasSize(1)); + assertThat(attributeValues.iterator().next(), nullValue()); + } +}