keycloak-uncached

LDAP polishing

12/19/2016 2:29:04 PM

Details

diff --git a/examples/ldap/ldaprealm.json b/examples/ldap/ldaprealm.json
index 8a04b3e..1630924 100644
--- a/examples/ldap/ldaprealm.json
+++ b/examples/ldap/ldaprealm.json
@@ -129,7 +129,7 @@
           "config" : {
             "Claim JSON Type" : "String",
             "user.attribute" : "picture",
-            "claim.name" : "profile_picture",
+            "claim.name" : "picture",
             "multivalued": "false",
             "id.token.claim" : "true",
             "access.token.claim" : "true"
diff --git a/examples/ldap/src/main/java/org/keycloak/example/ldap/LDAPPictureServlet.java b/examples/ldap/src/main/java/org/keycloak/example/ldap/LDAPPictureServlet.java
index 3ca7856..fe5a983 100644
--- a/examples/ldap/src/main/java/org/keycloak/example/ldap/LDAPPictureServlet.java
+++ b/examples/ldap/src/main/java/org/keycloak/example/ldap/LDAPPictureServlet.java
@@ -45,23 +45,14 @@ public class LDAPPictureServlet extends HttpServlet {
         KeycloakSecurityContext securityContext = (KeycloakSecurityContext) req.getAttribute(KeycloakSecurityContext.class.getName());
         IDToken idToken = securityContext.getIdToken();
 
-        // TODO: Use idToken.getPicture() instead
-        Object profilePicture = idToken.getOtherClaims().get("profile_picture");
+        String profilePicture = idToken.getPicture();
 
         if (profilePicture != null) {
-            String base64EncodedPicture = getBase64EncodedPicture(profilePicture);
-            byte[] decodedPicture = Base64.decode(base64EncodedPicture);
+            byte[] decodedPicture = Base64.decode(profilePicture);
             outputStream.write(decodedPicture);
         }
 
         outputStream.flush();
     }
 
-    private String getBase64EncodedPicture(Object profilePicture) {
-        if (profilePicture instanceof List) {
-            return ((List) profilePicture).get(0).toString();
-        } else {
-            return profilePicture.toString();
-        }
-    }
 }
diff --git a/examples/ldap/src/main/webapp/index.jsp b/examples/ldap/src/main/webapp/index.jsp
index 1156049..683e5b1 100644
--- a/examples/ldap/src/main/webapp/index.jsp
+++ b/examples/ldap/src/main/webapp/index.jsp
@@ -37,21 +37,18 @@
         <p><b>Full Name: </b><%=idToken.getName()%></p>
         <p><b>First: </b><%=idToken.getGivenName()%></p>
         <p><b>Last: </b><%=idToken.getFamilyName()%></p>
+        <% if (idToken.getPicture() != null) { %>
+            <p><b>Profile picture: </b><img src='/ldap-portal/picture' /></p>
+        <% } %>
         <hr />
 
 
         <h2>ID Token - other claims</h2>
 <%
     for (Map.Entry<String, Object> claim : idToken.getOtherClaims().entrySet()) {
-        if (!claim.getKey().equals("profile_picture")) {
 %>
         <p><b><%= claim.getKey() %>: </b><%= claim.getValue().toString() %>
 <%
-        } else {
-%>
-        <p><b>Profile picture: </b><img src="/ldap-portal/picture" />
-<%
-        }
     }
 %>
         <hr />
diff --git a/federation/ldap/src/main/java/org/keycloak/storage/ldap/LDAPStorageProvider.java b/federation/ldap/src/main/java/org/keycloak/storage/ldap/LDAPStorageProvider.java
index 949c3a2..afdf668 100755
--- a/federation/ldap/src/main/java/org/keycloak/storage/ldap/LDAPStorageProvider.java
+++ b/federation/ldap/src/main/java/org/keycloak/storage/ldap/LDAPStorageProvider.java
@@ -201,6 +201,7 @@ public class LDAPStorageProvider implements UserStorageProvider,
         LDAPObject ldapUser = LDAPUtils.addUserToLDAP(this, realm, user);
         LDAPUtils.checkUuid(ldapUser, ldapIdentityStore.getConfig());
         user.setSingleAttribute(LDAPConstants.LDAP_ID, ldapUser.getUuid());
+        user.setSingleAttribute(LDAPConstants.LDAP_ENTRY_DN, ldapUser.getDn().toString());
 
         return proxy(realm, user, ldapUser);
     }
@@ -421,6 +422,7 @@ public class LDAPStorageProvider implements UserStorageProvider,
         String userDN = ldapUser.getDn().toString();
         imported.setFederationLink(model.getId());
         imported.setSingleAttribute(LDAPConstants.LDAP_ID, ldapUser.getUuid());
+        imported.setSingleAttribute(LDAPConstants.LDAP_ENTRY_DN, userDN);
 
         logger.debugf("Imported new user from LDAP to Keycloak DB. Username: [%s], Email: [%s], LDAP_ID: [%s], LDAP Entry DN: [%s]", imported.getUsername(), imported.getEmail(),
                 ldapUser.getUuid(), userDN);
diff --git a/testsuite/integration/src/test/java/org/keycloak/testsuite/federation/storage/ldap/LDAPMultipleAttributesTest.java b/testsuite/integration/src/test/java/org/keycloak/testsuite/federation/storage/ldap/LDAPMultipleAttributesTest.java
index 7de1ae9..dce0f60 100755
--- a/testsuite/integration/src/test/java/org/keycloak/testsuite/federation/storage/ldap/LDAPMultipleAttributesTest.java
+++ b/testsuite/integration/src/test/java/org/keycloak/testsuite/federation/storage/ldap/LDAPMultipleAttributesTest.java
@@ -56,6 +56,7 @@ import java.util.Arrays;
 import java.util.LinkedHashSet;
 import java.util.LinkedList;
 import java.util.List;
+import java.util.Map;
 
 /**
  * @author <a href="mailto:mposolda@redhat.com">Marek Posolda</a>
@@ -66,7 +67,15 @@ public class LDAPMultipleAttributesTest {
     protected String APP_SERVER_BASE_URL = "http://localhost:8081";
     protected String LOGIN_URL = OIDCLoginProtocolService.authUrl(UriBuilder.fromUri(APP_SERVER_BASE_URL + "/auth")).build("test").toString();
 
-    private static LDAPRule ldapRule = new LDAPRule();
+
+    // Skip this test on MSAD due to lack of supported user multivalued attributes
+    private static LDAPRule ldapRule = new LDAPRule((Map<String, String> ldapConfig) -> {
+
+        String vendor = ldapConfig.get(LDAPConstants.VENDOR);
+        return (vendor.equals(LDAPConstants.VENDOR_ACTIVE_DIRECTORY));
+
+    });
+
 
     private static ComponentModel ldapModel = null;
 
diff --git a/testsuite/integration/src/test/java/org/keycloak/testsuite/rule/KerberosRule.java b/testsuite/integration/src/test/java/org/keycloak/testsuite/rule/KerberosRule.java
index 8d46513..38a1df2 100644
--- a/testsuite/integration/src/test/java/org/keycloak/testsuite/rule/KerberosRule.java
+++ b/testsuite/integration/src/test/java/org/keycloak/testsuite/rule/KerberosRule.java
@@ -36,6 +36,11 @@ public class KerberosRule extends LDAPRule {
     private final String configLocation;
 
     public KerberosRule(String configLocation) {
+        this(configLocation, null);
+    }
+
+    public KerberosRule(String configLocation, LDAPRuleCondition condition) {
+        super(condition);
         this.configLocation = configLocation;
 
         // Global kerberos configuration
diff --git a/testsuite/integration/src/test/java/org/keycloak/testsuite/rule/LDAPRule.java b/testsuite/integration/src/test/java/org/keycloak/testsuite/rule/LDAPRule.java
index 4bf0e4c..6dbc938 100755
--- a/testsuite/integration/src/test/java/org/keycloak/testsuite/rule/LDAPRule.java
+++ b/testsuite/integration/src/test/java/org/keycloak/testsuite/rule/LDAPRule.java
@@ -17,7 +17,10 @@
 
 package org.keycloak.testsuite.rule;
 
-import org.junit.rules.ExternalResource;
+import org.jboss.logging.Logger;
+import org.junit.rules.TestRule;
+import org.junit.runner.Description;
+import org.junit.runners.model.Statement;
 import org.keycloak.testsuite.federation.ldap.LDAPTestConfiguration;
 import org.keycloak.util.ldap.LDAPEmbeddedServer;
 
@@ -25,28 +28,76 @@ import java.util.Map;
 import java.util.Properties;
 
 /**
+ * This rule handles:
+ * - Reading of LDAP configuration from properties file
+ * - Eventually start+stop of LDAP embedded server.
+ * - Eventually allows to ignore the test if particular condition is not met. This allows to run specific tests just for some LDAP vendors
+ *
  * @author <a href="mailto:mposolda@redhat.com">Marek Posolda</a>
  */
-public class LDAPRule extends ExternalResource {
+public class LDAPRule implements TestRule {
+
+    private static final Logger logger = Logger.getLogger(LDAPRule.class);
 
     public static final String LDAP_CONNECTION_PROPERTIES_LOCATION = "ldap/ldap-connection.properties";
 
     protected LDAPTestConfiguration ldapTestConfiguration;
     protected LDAPEmbeddedServer ldapEmbeddedServer;
 
+    private final LDAPRuleCondition condition;
+
+
+    public LDAPRule() {
+        this(null);
+    }
+
+    public LDAPRule(LDAPRuleCondition condition) {
+        this.condition = condition;
+    }
+
+
     @Override
-    protected void before() throws Throwable {
+    public Statement apply(Statement base, Description description) {
+        return new Statement() {
+
+            @Override
+            public void evaluate() throws Throwable {
+                boolean skipTest = before();
+
+                if (skipTest) {
+                    logger.infof("Skip %s due to LDAPRuleCondition not met", description.getDisplayName());
+                    return;
+                }
+
+                try {
+                    base.evaluate();
+                } finally {
+                    after();
+                }
+            }
+        };
+    }
+
+
+    // Return true if test should be skipped
+    protected boolean before() throws Throwable {
         String connectionPropsLocation = getConnectionPropertiesLocation();
         ldapTestConfiguration = LDAPTestConfiguration.readConfiguration(connectionPropsLocation);
 
+        if (condition != null && condition.skipTest(ldapTestConfiguration.getLDAPConfig())) {
+            return true;
+        }
+
         if (ldapTestConfiguration.isStartEmbeddedLdapLerver()) {
             ldapEmbeddedServer = createServer();
             ldapEmbeddedServer.init();
             ldapEmbeddedServer.start();
         }
+
+        return false;
     }
 
-    @Override
+
     protected void after() {
         try {
             if (ldapEmbeddedServer != null) {
@@ -78,4 +129,12 @@ public class LDAPRule extends ExternalResource {
     public int getSleepTime() {
         return ldapTestConfiguration.getSleepTime();
     }
+
+
+    // Allows to skip particular LDAP test just under specific conditions (eg. some test running just on Active Directory)
+    public interface LDAPRuleCondition {
+
+        boolean skipTest(Map<String, String> ldapConfig);
+
+    }
 }
diff --git a/testsuite/integration/src/test/resources/log4j.properties b/testsuite/integration/src/test/resources/log4j.properties
index 22a890d..4cc1f91 100755
--- a/testsuite/integration/src/test/resources/log4j.properties
+++ b/testsuite/integration/src/test/resources/log4j.properties
@@ -60,6 +60,9 @@ log4j.logger.org.keycloak.connections.jpa.HibernateStatsReporter=debug
 # Enable to view ldap logging
 # log4j.logger.org.keycloak.storage.ldap=trace
 
+# Enable to view queries to LDAP
+# log4j.logger.org.keycloak.storage.ldap.idm.store.ldap.LDAPIdentityStore=trace
+
 # Enable to view kerberos/spnego logging
 # log4j.logger.org.keycloak.federation.kerberos=trace