keycloak-aplcache

KEYCLOAK-6106 Put dotless ids first in identity broker state (cherry

1/2/2018 1:02:08 PM

Details

diff --git a/server-spi-private/src/main/java/org/keycloak/broker/provider/util/IdentityBrokerState.java b/server-spi-private/src/main/java/org/keycloak/broker/provider/util/IdentityBrokerState.java
index b15a7f9..1783b1a 100644
--- a/server-spi-private/src/main/java/org/keycloak/broker/provider/util/IdentityBrokerState.java
+++ b/server-spi-private/src/main/java/org/keycloak/broker/provider/util/IdentityBrokerState.java
@@ -31,7 +31,7 @@ public class IdentityBrokerState {
 
 
     public static IdentityBrokerState decoded(String state, String clientId, String tabId) {
-        String encodedState = state + "." + clientId + "." + tabId;
+        String encodedState = state + "." + tabId + "." + clientId;
 
         return new IdentityBrokerState(state, clientId, tabId, encodedState);
     }
@@ -41,8 +41,8 @@ public class IdentityBrokerState {
         String[] decoded = DOT.split(encodedState, 3);
 
         String state =(decoded.length > 0) ? decoded[0] : null;
-        String clientId = (decoded.length > 1) ? decoded[1] : null;
-        String tabId = (decoded.length > 2) ? decoded[2] : null;
+        String tabId = (decoded.length > 1) ? decoded[1] : null;
+        String clientId = (decoded.length > 2) ? decoded[2] : null;
 
         return new IdentityBrokerState(state, clientId, tabId, encodedState);
     }
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 b6c05b7..8e59302 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
@@ -78,7 +78,7 @@ public abstract class AbstractBrokerTest extends AbstractBaseBrokerTest {
         if (clients != null) {
             RealmResource providerRealm = adminClient.realm(bc.providerRealmName());
             for (ClientRepresentation client : clients) {
-                log.debug("adding client " + client.getName() + " to realm " + bc.providerRealmName());
+                log.debug("adding client " + client.getClientId()+ " to realm " + bc.providerRealmName());
 
                 providerRealm.clients().create(client).close();
             }
@@ -88,7 +88,7 @@ public abstract class AbstractBrokerTest extends AbstractBaseBrokerTest {
         if (clients != null) {
             RealmResource consumerRealm = adminClient.realm(bc.consumerRealmName());
             for (ClientRepresentation client : clients) {
-                log.debug("adding client " + client.getName() + " to realm " + bc.consumerRealmName());
+                log.debug("adding client " + client.getClientId() + " to realm " + bc.consumerRealmName());
 
                 consumerRealm.clients().create(client).close();
             }
diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/broker/KcOIDCBrokerWithSignatureTest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/broker/KcOIDCBrokerWithSignatureTest.java
index 566d288..83496a3 100644
--- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/broker/KcOIDCBrokerWithSignatureTest.java
+++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/broker/KcOIDCBrokerWithSignatureTest.java
@@ -89,7 +89,7 @@ public class KcOIDCBrokerWithSignatureTest extends AbstractBaseBrokerTest {
         if (clients != null) {
             RealmResource providerRealm = adminClient.realm(bc.providerRealmName());
             for (ClientRepresentation client : clients) {
-                log.debug("adding client " + client.getName() + " to realm " + bc.providerRealmName());
+                log.debug("adding client " + client.getClientId() + " to realm " + bc.providerRealmName());
 
                 Response resp = providerRealm.clients().create(client);
                 resp.close();
@@ -100,7 +100,7 @@ public class KcOIDCBrokerWithSignatureTest extends AbstractBaseBrokerTest {
         if (clients != null) {
             RealmResource consumerRealm = adminClient.realm(bc.consumerRealmName());
             for (ClientRepresentation client : clients) {
-                log.debug("adding client " + client.getName() + " to realm " + bc.consumerRealmName());
+                log.debug("adding client " + client.getClientId() + " to realm " + bc.consumerRealmName());
 
                 Response resp = consumerRealm.clients().create(client);
                 resp.close();
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 e544c4b..6e6e8b5 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
@@ -56,9 +56,14 @@ public class KcSamlBrokerConfiguration implements BrokerConfiguration {
 
     @Override
     public List<ClientRepresentation> createProviderClients(SuiteContext suiteContext) {
+        String clientId = getIDPClientIdInProviderRealm(suiteContext);
+        return Arrays.asList(createProviderClient(suiteContext, clientId));
+    }
+
+    private ClientRepresentation createProviderClient(SuiteContext suiteContext, String clientId) {
         ClientRepresentation client = new ClientRepresentation();
 
-        client.setClientId(getIDPClientIdInProviderRealm(suiteContext));
+        client.setClientId(clientId);
         client.setEnabled(true);
         client.setProtocol(IDP_SAML_PROVIDER_ID);
         client.setRedirectUris(Collections.singletonList(
@@ -119,7 +124,7 @@ public class KcSamlBrokerConfiguration implements BrokerConfiguration {
 
         client.setProtocolMappers(Arrays.asList(emailMapper, userAttrMapper, userFriendlyAttrMapper));
 
-        return Collections.singletonList(client);
+        return client;
     }
 
     @Override
@@ -134,6 +139,16 @@ public class KcSamlBrokerConfiguration implements BrokerConfiguration {
             .addRedirectUri("http://localhost:8080/sales-post/*")
             .attribute(SamlConfigAttributes.SAML_AUTHNSTATEMENT, SamlProtocol.ATTRIBUTE_TRUE_VALUE)
             .attribute(SamlConfigAttributes.SAML_CLIENT_SIGNATURE_ATTRIBUTE, SamlProtocol.ATTRIBUTE_FALSE_VALUE)
+            .build(),
+          ClientBuilder.create()
+            .clientId(AbstractSamlTest.SAML_CLIENT_ID_SALES_POST + ".dot/ted")
+            .enabled(true)
+            .fullScopeEnabled(true)
+            .protocol(SamlProtocol.LOGIN_PROTOCOL)
+            .baseUrl("http://localhost:8080/sales-post")
+            .addRedirectUri("http://localhost:8080/sales-post/*")
+            .attribute(SamlConfigAttributes.SAML_AUTHNSTATEMENT, SamlProtocol.ATTRIBUTE_TRUE_VALUE)
+            .attribute(SamlConfigAttributes.SAML_CLIENT_SIGNATURE_ATTRIBUTE, SamlProtocol.ATTRIBUTE_FALSE_VALUE)
             .build()
         );
     }
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 1432924..3766f4a 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
@@ -5,24 +5,35 @@ import com.google.common.collect.ImmutableMap;
 import com.google.common.collect.Lists;
 import org.keycloak.broker.saml.mappers.AttributeToRoleMapper;
 import org.keycloak.broker.saml.mappers.UserAttributeMapper;
+import org.keycloak.dom.saml.v2.protocol.AuthnRequestType;
 import org.keycloak.protocol.saml.SamlProtocol;
 import org.keycloak.representations.idm.IdentityProviderMapperRepresentation;
 import org.keycloak.representations.idm.RoleRepresentation;
+import org.keycloak.saml.common.constants.JBossSAMLURIConstants;
+import org.keycloak.saml.processing.api.saml.v2.request.SAML2Request;
+import org.keycloak.saml.processing.core.saml.v2.common.SAMLDocumentHolder;
 import org.keycloak.services.resources.RealmsResource;
+import org.keycloak.testsuite.saml.AbstractSamlTest;
+import org.keycloak.testsuite.util.SamlClient;
+import org.keycloak.testsuite.util.SamlClient.Binding;
+import org.keycloak.testsuite.util.SamlClientBuilder;
 import java.net.URI;
 import java.util.Collections;
 import java.util.Set;
 import java.util.stream.Collectors;
 import javax.ws.rs.core.UriBuilder;
 import javax.ws.rs.core.UriBuilderException;
+import org.hamcrest.Matchers;
+import org.junit.Assert;
 import org.junit.Test;
-import static org.hamcrest.Matchers.containsInAnyOrder;
+import org.w3c.dom.Document;
 import static org.hamcrest.Matchers.hasItems;
 import static org.hamcrest.Matchers.not;
 import static org.junit.Assert.assertThat;
 import static org.keycloak.testsuite.broker.AbstractBrokerTest.ROLE_FRIENDLY_MANAGER;
 import static org.keycloak.testsuite.broker.AbstractBrokerTest.ROLE_MANAGER;
 import static org.keycloak.testsuite.broker.AbstractBrokerTest.ROLE_USER;
+import static org.keycloak.testsuite.util.Matchers.isSamlResponse;
 
 public class KcSamlBrokerTest extends AbstractBrokerTest {
 
@@ -123,4 +134,35 @@ public class KcSamlBrokerTest extends AbstractBrokerTest {
                 .protocolUrl(UriBuilder.fromUri(getAuthServerRoot()))
                 .build(realm, SamlProtocol.LOGIN_PROTOCOL);
     }
+
+    // KEYCLOAK-6106
+    @Test
+    public void loginClientWithDotsInName() throws Exception {
+        AuthnRequestType loginRep = SamlClient.createLoginRequestDocument(AbstractSamlTest.SAML_CLIENT_ID_SALES_POST + ".dot/ted", AbstractSamlTest.SAML_ASSERTION_CONSUMER_URL_SALES_POST, 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
+            .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));
+    }
+
 }