KEYCLOAK-3921 LDAP binary attributes

federation/ldap/src/main/java/org/keycloak/storage/ldap/LDAPConfig.java

@@ -24,6 +24,7 @@ import org.keycloak.storage.UserStorageProvider;
 import javax.naming.directory.SearchControls;
 import java.util.Collection;
 import java.util.HashSet;
+import java.util.List;
 import java.util.Properties;
 import java.util.Set;
 
@@ -34,6 +35,7 @@ import java.util.Set;
 public class LDAPConfig {
 
     private final MultivaluedHashMap<String, String> config;
+    private final Set<String> binaryAttributeNames = new HashSet<>();
 
     public LDAPConfig(MultivaluedHashMap<String, String> config) {
         this.config = config;
@@ -184,4 +186,39 @@ public class LDAPConfig {
             return UserStorageProvider.EditMode.valueOf(editModeString);
         }
     }
+
+    public void addBinaryAttribute(String attrName) {
+        binaryAttributeNames.add(attrName);
+    }
+
+    public Set<String> getBinaryAttributeNames() {
+        return binaryAttributeNames;
+    }
+
+
+    @Override
+    public boolean equals(Object obj) {
+        if (obj == this) return true;
+        if (!(obj instanceof LDAPConfig)) return false;
+
+        LDAPConfig that = (LDAPConfig) obj;
+
+        if (!config.equals(that.config)) return false;
+        if (!binaryAttributeNames.equals(that.binaryAttributeNames)) return false;
+        return true;
+    }
+
+    @Override
+    public int hashCode() {
+        return config.hashCode() * 13 + binaryAttributeNames.hashCode();
+    }
+
+    @Override
+    public String toString() {
+        MultivaluedHashMap<String, String> copy = new MultivaluedHashMap<String, String>(config);
+        copy.remove(LDAPConstants.BIND_CREDENTIAL);
+        return new StringBuilder(copy.toString())
+                .append(", binaryAttributes: ").append(binaryAttributeNames)
+                .toString();
+    }
 }

federation/ldap/src/main/java/org/keycloak/storage/ldap/LDAPIdentityStoreRegistry.java

@@ -20,8 +20,8 @@ package org.keycloak.storage.ldap;
 import org.jboss.logging.Logger;
 import org.keycloak.common.util.MultivaluedHashMap;
 import org.keycloak.component.ComponentModel;
-import org.keycloak.models.LDAPConstants;
 import org.keycloak.storage.ldap.idm.store.ldap.LDAPIdentityStore;
+import org.keycloak.storage.ldap.mappers.LDAPConfigDecorator;
 
 import java.util.Map;
 import java.util.concurrent.ConcurrentHashMap;
@@ -33,37 +33,40 @@ public class LDAPIdentityStoreRegistry {
 
     private static final Logger logger = Logger.getLogger(LDAPIdentityStoreRegistry.class);
 
-    private Map<String, LDAPIdentityStoreContext> ldapStores = new ConcurrentHashMap<String, LDAPIdentityStoreContext>();
+    private Map<String, LDAPIdentityStoreContext> ldapStores = new ConcurrentHashMap<>();
 
-    public LDAPIdentityStore getLdapStore(ComponentModel model) {
-        LDAPIdentityStoreContext context = ldapStores.get(model.getId());
+    public LDAPIdentityStore getLdapStore(ComponentModel ldapModel, Map<ComponentModel, LDAPConfigDecorator> configDecorators) {
+        LDAPIdentityStoreContext context = ldapStores.get(ldapModel.getId());
 
         // Ldap config might have changed for the realm. In this case, we must re-initialize
-        MultivaluedHashMap<String, String> config = model.getConfig();
-        if (context == null || !config.equals(context.config)) {
-            logLDAPConfig(model.getName(), config);
+        MultivaluedHashMap<String, String> configModel = ldapModel.getConfig();
+        LDAPConfig ldapConfig = new LDAPConfig(configModel);
+        for (Map.Entry<ComponentModel, LDAPConfigDecorator> entry : configDecorators.entrySet()) {
+            ComponentModel mapperModel = entry.getKey();
+            LDAPConfigDecorator decorator = entry.getValue();
 
-            LDAPIdentityStore store = createLdapIdentityStore(config);
-            context = new LDAPIdentityStoreContext(config, store);
-            ldapStores.put(model.getId(), context);
+            decorator.updateLDAPConfig(ldapConfig, mapperModel);
+        }
+
+        if (context == null || !ldapConfig.equals(context.config)) {
+            logLDAPConfig(ldapModel.getName(), ldapConfig);
+
+            LDAPIdentityStore store = createLdapIdentityStore(ldapConfig);
+            context = new LDAPIdentityStoreContext(ldapConfig, store);
+            ldapStores.put(ldapModel.getId(), context);
         }
         return context.store;
     }
 
     // Don't log LDAP password
-    private void logLDAPConfig(String fedProviderDisplayName, MultivaluedHashMap<String, String> ldapConfig) {
-        MultivaluedHashMap<String, String> copy = new MultivaluedHashMap<String, String>(ldapConfig);
-        copy.remove(LDAPConstants.BIND_CREDENTIAL);
-        logger.infof("Creating new LDAP based partition manager for the Federation provider: " + fedProviderDisplayName + ", LDAP Configuration: " + copy);
+    private void logLDAPConfig(String fedProviderDisplayName, LDAPConfig ldapConfig) {
+        logger.infof("Creating new LDAP Store for the LDAP storage provider: '%s', LDAP Configuration: %s", fedProviderDisplayName, ldapConfig.toString());
     }
 
     /**
-     * @param ldapConfig from realm
-     * @return PartitionManager instance based on LDAP store
+     * Create LDAPIdentityStore to be cached in the local registry
      */
-    public static LDAPIdentityStore createLdapIdentityStore(MultivaluedHashMap<String, String> ldapConfig) {
-        LDAPConfig cfg = new LDAPConfig(ldapConfig);
-
+    public static LDAPIdentityStore createLdapIdentityStore(LDAPConfig cfg) {
         checkSystemProperty("com.sun.jndi.ldap.connect.pool.authentication", "none simple");
         checkSystemProperty("com.sun.jndi.ldap.connect.pool.initsize", "1");
         checkSystemProperty("com.sun.jndi.ldap.connect.pool.maxsize", "1000");
@@ -84,12 +87,12 @@ public class LDAPIdentityStoreRegistry {
 
     private class LDAPIdentityStoreContext {
 
-        private LDAPIdentityStoreContext(MultivaluedHashMap<String, String> config, LDAPIdentityStore store) {
+        private LDAPIdentityStoreContext(LDAPConfig config, LDAPIdentityStore store) {
             this.config = config;
             this.store = store;
         }
 
-        private MultivaluedHashMap<String, String> config;
+        private LDAPConfig config;
         private LDAPIdentityStore store;
     }
 }

federation/ldap/src/main/java/org/keycloak/storage/ldap/LDAPStorageProviderFactory.java

@@ -48,7 +48,9 @@ import org.keycloak.storage.ldap.idm.query.internal.LDAPQueryConditionsBuilder;
 import org.keycloak.storage.ldap.idm.store.ldap.LDAPIdentityStore;
 import org.keycloak.storage.ldap.mappers.FullNameLDAPStorageMapper;
 import org.keycloak.storage.ldap.mappers.FullNameLDAPStorageMapperFactory;
+import org.keycloak.storage.ldap.mappers.LDAPConfigDecorator;
 import org.keycloak.storage.ldap.mappers.LDAPStorageMapper;
+import org.keycloak.storage.ldap.mappers.LDAPStorageMapperFactory;
 import org.keycloak.storage.ldap.mappers.UserAttributeLDAPStorageMapper;
 import org.keycloak.storage.ldap.mappers.UserAttributeLDAPStorageMapperFactory;
 import org.keycloak.storage.ldap.mappers.msad.MSADUserAccountControlStorageMapperFactory;
@@ -57,7 +59,9 @@ import org.keycloak.storage.user.SynchronizationResult;
 import org.keycloak.utils.CredentialHelper;
 
 import java.util.Date;
+import java.util.HashMap;
 import java.util.List;
+import java.util.Map;
 
 /**
  * @author <a href="mailto:mposolda@redhat.com">Marek Posolda</a>
@@ -179,10 +183,30 @@ public class LDAPStorageProviderFactory implements UserStorageProviderFactory<LD
 
     @Override
     public LDAPStorageProvider create(KeycloakSession session, ComponentModel model) {
-        LDAPIdentityStore ldapIdentityStore = this.ldapStoreRegistry.getLdapStore(model);
+        Map<ComponentModel, LDAPConfigDecorator> configDecorators = getLDAPConfigDecorators(session, model);
+
+        LDAPIdentityStore ldapIdentityStore = this.ldapStoreRegistry.getLdapStore(model, configDecorators);
         return new LDAPStorageProvider(this, session, model, ldapIdentityStore);
     }
 
+
+    // Check if it's some performance overhead to create this map in every request. But probably not...
+    protected Map<ComponentModel, LDAPConfigDecorator> getLDAPConfigDecorators(KeycloakSession session, ComponentModel ldapModel) {
+        RealmModel realm = session.realms().getRealm(ldapModel.getParentId());
+        List<ComponentModel> mapperComponents = realm.getComponents(ldapModel.getId(), LDAPStorageMapper.class.getName());
+
+        Map<ComponentModel, LDAPConfigDecorator> result = new HashMap<>();
+        for (ComponentModel mapperModel : mapperComponents) {
+            LDAPStorageMapperFactory mapperFactory = (LDAPStorageMapperFactory) session.getKeycloakSessionFactory().getProviderFactory(LDAPStorageMapper.class, mapperModel.getProviderId());
+            if (mapperFactory instanceof LDAPConfigDecorator) {
+                result.put(mapperModel, (LDAPConfigDecorator) mapperFactory);
+            }
+        }
+
+        return result;
+    }
+
+
     @Override
     public void validateConfiguration(KeycloakSession session, RealmModel realm, ComponentModel config) throws ComponentValidationException {
         LDAPConfig cfg = new LDAPConfig(config.getConfig());

federation/ldap/src/main/java/org/keycloak/storage/ldap/idm/store/ldap/LDAPIdentityStore.java

@@ -18,6 +18,7 @@
 package org.keycloak.storage.ldap.idm.store.ldap;
 
 import org.jboss.logging.Logger;
+import org.keycloak.common.util.Base64;
 import org.keycloak.models.LDAPConstants;
 import org.keycloak.models.ModelException;
 import org.keycloak.storage.ldap.LDAPConfig;
@@ -40,6 +41,8 @@ import javax.naming.directory.DirContext;
 import javax.naming.directory.ModificationItem;
 import javax.naming.directory.SearchControls;
 import javax.naming.directory.SearchResult;
+
+import java.io.IOException;
 import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.Collection;
@@ -323,8 +326,15 @@ public class LDAPIdentityStore implements IdentityStore {
                     Set<String> attrValues = new LinkedHashSet<>();
                     NamingEnumeration<?> enumm = ldapAttribute.getAll();
                     while (enumm.hasMoreElements()) {
-                        String attrVal = enumm.next().toString().trim();
-                        attrValues.add(attrVal);
+                        Object val = enumm.next();
+
+                        if (val instanceof byte[]) { // byte[]
+                            String attrVal = Base64.encodeBytes((byte[]) val);
+                            attrValues.add(attrVal);
+                        } else { // String
+                            String attrVal = val.toString().trim();
+                            attrValues.add(attrVal);
+                        }
                     }
 
                     if (ldapAttributeName.equalsIgnoreCase(LDAPConstants.OBJECT_CLASS)) {
@@ -377,7 +387,18 @@ public class LDAPIdentityStore implements IdentityStore {
                     if (val == null || val.toString().trim().length() == 0) {
                         val = LDAPConstants.EMPTY_ATTRIBUTE_VALUE;
                     }
-                    attr.add(val);
+
+                    if (getConfig().getBinaryAttributeNames().contains(attrName)) {
+                        // Binary attribute
+                        try {
+                            byte[] bytes = Base64.decode(val);
+                            attr.add(bytes);
+                        } catch (IOException ioe) {
+                            logger.warnf("Wasn't able to Base64 decode the attribute value. Ignoring attribute update. LDAP DN: %s, Attribute: %s, Attribute value: %s" + ldapObject.getDn(), attrName, attrValue);
+                        }
+                    } else {
+                        attr.add(val);
+                    }
                 }
 
                 entryAttributes.put(attr);

federation/ldap/src/main/java/org/keycloak/storage/ldap/idm/store/ldap/LDAPOperationManager.java

@@ -515,8 +515,17 @@ public class LDAPOperationManager {
             }
         }
 
+        StringBuilder binaryAttrsBuilder = new StringBuilder();
         if (this.config.isObjectGUID()) {
-            env.put("java.naming.ldap.attributes.binary", LDAPConstants.OBJECT_GUID);
+            binaryAttrsBuilder.append(LDAPConstants.OBJECT_GUID).append(" ");
+        }
+        for (String attrName : config.getBinaryAttributeNames()) {
+            binaryAttrsBuilder.append(attrName).append(" ");
+        }
+
+        String binaryAttrs = binaryAttrsBuilder.toString().trim();
+        if (!binaryAttrs.isEmpty()) {
+            env.put("java.naming.ldap.attributes.binary", binaryAttrs);
         }
 
         if (logger.isDebugEnabled()) {

federation/ldap/src/main/java/org/keycloak/storage/ldap/mappers/LDAPConfigDecorator.java

@@ -0,0 +1,31 @@
+/*
+ * Copyright 2016 Red Hat, Inc. and/or its affiliates
+ * and other contributors as indicated by the @author tags.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.keycloak.storage.ldap.mappers;
+
+import org.keycloak.component.ComponentModel;
+import org.keycloak.storage.ldap.LDAPConfig;
+
+/**
+ *
+ * @author <a href="mailto:mposolda@redhat.com">Marek Posolda</a>
+ */
+public interface LDAPConfigDecorator {
+
+    void updateLDAPConfig(LDAPConfig ldapConfig, ComponentModel mapperModel);
+
+}

federation/ldap/src/main/java/org/keycloak/storage/ldap/mappers/UserAttributeLDAPStorageMapper.java

@@ -80,6 +80,7 @@ public class UserAttributeLDAPStorageMapper extends AbstractLDAPStorageMapper {
     public static final String READ_ONLY = "read.only";
     public static final String ALWAYS_READ_VALUE_FROM_LDAP = "always.read.value.from.ldap";
     public static final String IS_MANDATORY_IN_LDAP = "is.mandatory.in.ldap";
+    public static final String IS_BINARY_ATTRIBUTE = "is.binary.attribute";
 
     public UserAttributeLDAPStorageMapper(ComponentModel mapperModel, LDAPStorageProvider ldapProvider) {
         super(mapperModel, ldapProvider);
@@ -90,6 +91,12 @@ public class UserAttributeLDAPStorageMapper extends AbstractLDAPStorageMapper {
         String userModelAttrName = mapperModel.getConfig().getFirst(USER_MODEL_ATTRIBUTE);
         String ldapAttrName = mapperModel.getConfig().getFirst(LDAP_ATTRIBUTE);
 
+        // We won't update binary attributes to Keycloak DB. They might be too big
+        boolean isBinaryAttribute = mapperModel.get(IS_BINARY_ATTRIBUTE, false);
+        if (isBinaryAttribute) {
+            return;
+        }
+
         Property<Object> userModelProperty = userModelProperties.get(userModelAttrName.toLowerCase());
 
         if (userModelProperty != null) {
@@ -177,6 +184,7 @@ public class UserAttributeLDAPStorageMapper extends AbstractLDAPStorageMapper {
         final String ldapAttrName = mapperModel.getConfig().getFirst(LDAP_ATTRIBUTE);
         boolean isAlwaysReadValueFromLDAP = parseBooleanParameter(mapperModel, ALWAYS_READ_VALUE_FROM_LDAP);
         final boolean isMandatoryInLdap = parseBooleanParameter(mapperModel, IS_MANDATORY_IN_LDAP);
+        final boolean isBinaryAttribute = parseBooleanParameter(mapperModel, IS_BINARY_ATTRIBUTE);
 
         // For writable mode, we want to propagate writing of attribute to LDAP as well
         if (ldapProvider.getEditMode() == UserStorageProvider.EditMode.WRITABLE && !isReadOnly()) {
@@ -185,20 +193,23 @@ public class UserAttributeLDAPStorageMapper extends AbstractLDAPStorageMapper {
 
                 @Override
                 public void setSingleAttribute(String name, String value) {
-                    setLDAPAttribute(name, value);
-                    super.setSingleAttribute(name, value);
+                    if (setLDAPAttribute(name, value)) {
+                        super.setSingleAttribute(name, value);
+                    }
                 }
 
                 @Override
                 public void setAttribute(String name, List<String> values) {
-                    setLDAPAttribute(name, values);
-                    super.setAttribute(name, values);
+                    if (setLDAPAttribute(name, values)) {
+                        super.setAttribute(name, values);
+                    }
                 }
 
                 @Override
                 public void removeAttribute(String name) {
-                    setLDAPAttribute(name, null);
-                    super.removeAttribute(name);
+                    if ( setLDAPAttribute(name, null)) {
+                        super.removeAttribute(name);
+                    }
                 }
 
                 @Override
@@ -221,10 +232,10 @@ public class UserAttributeLDAPStorageMapper extends AbstractLDAPStorageMapper {
                     super.setFirstName(firstName);
                 }
 
-                protected void setLDAPAttribute(String modelAttrName, Object value) {
+                protected boolean setLDAPAttribute(String modelAttrName, Object value) {
                     if (modelAttrName.equalsIgnoreCase(userModelAttrName)) {
-                        if (logger.isTraceEnabled()) {
-                            logger.tracef("Pushing user attribute to LDAP. username: %s, Model attribute name: %s, LDAP attribute name: %s, Attribute value: %s", getUsername(), modelAttrName, ldapAttrName, value);
+                        if (UserAttributeLDAPStorageMapper.logger.isTraceEnabled()) {
+                            UserAttributeLDAPStorageMapper.logger.tracef("Pushing user attribute to LDAP. username: %s, Model attribute name: %s, LDAP attribute name: %s, Attribute value: %s", getUsername(), modelAttrName, ldapAttrName, value);
                         }
 
                         ensureTransactionStarted();
@@ -245,7 +256,53 @@ public class UserAttributeLDAPStorageMapper extends AbstractLDAPStorageMapper {
                                 ldapUser.setAttribute(ldapAttrName, new LinkedHashSet<>(asList));
                             }
                         }
+
+                        if (isBinaryAttribute) {
+                            UserAttributeLDAPStorageMapper.logger.debugf("Skip writing model attribute '%s' to DB for user '%s' as it is mapped to binary LDAP attribute.", userModelAttrName, getUsername());
+                            return false;
+                        } else {
+                            return true;
+                        }
                     }
+
+                    return true;
+                }
+
+            };
+
+        } else if (isBinaryAttribute) {
+
+            delegate = new UserModelDelegate(delegate) {
+
+                @Override
+                public void setSingleAttribute(String name, String value) {
+                    if (name.equalsIgnoreCase(userModelAttrName)) {
+                        logSkipDBWrite();
+                    } else {
+                        super.setSingleAttribute(name, value);
+                    }
+                }
+
+                @Override
+                public void setAttribute(String name, List<String> values) {
+                    if (name.equalsIgnoreCase(userModelAttrName)) {
+                        logSkipDBWrite();
+                    } else {
+                        super.setAttribute(name, values);
+                    }
+                }
+
+                @Override
+                public void removeAttribute(String name) {
+                    if (name.equalsIgnoreCase(userModelAttrName)) {
+                        logSkipDBWrite();
+                    } else {
+                        super.removeAttribute(name);
+                    }
+                }
+
+                private void logSkipDBWrite() {
+                    logger.debugf("Skip writing model attribute '%s' to DB for user '%s' as it is mapped to binary LDAP attribute", userModelAttrName, getUsername());
                 }
 
             };

federation/ldap/src/main/java/org/keycloak/storage/ldap/mappers/UserAttributeLDAPStorageMapperFactory.java

@@ -32,7 +32,7 @@ import java.util.List;
 /**
  * @author <a href="mailto:mposolda@redhat.com">Marek Posolda</a>
  */
-public class UserAttributeLDAPStorageMapperFactory extends AbstractLDAPStorageMapperFactory {
+public class UserAttributeLDAPStorageMapperFactory extends AbstractLDAPStorageMapperFactory implements LDAPConfigDecorator {
 
     public static final String PROVIDER_ID = "user-attribute-ldap-mapper";
     protected static final List<ProviderConfigProperty> configProperties;
@@ -69,6 +69,10 @@ public class UserAttributeLDAPStorageMapperFactory extends AbstractLDAPStorageMa
                                .helpText("If true, attribute is mandatory in LDAP. Hence if there is no value in Keycloak DB, the empty value will be set to be propagated to LDAP")
                                .type(ProviderConfigProperty.BOOLEAN_TYPE)
                                .defaultValue("false").add()
+                    .property().name(UserAttributeLDAPStorageMapper.IS_BINARY_ATTRIBUTE).label("Is Binary Attribute")
+                                .helpText("Should be true for binary LDAP attributes")
+                                .type(ProviderConfigProperty.BOOLEAN_TYPE)
+                                .defaultValue("false").add()
                     .build();
     }
 
@@ -92,6 +96,12 @@ public class UserAttributeLDAPStorageMapperFactory extends AbstractLDAPStorageMa
         checkMandatoryConfigAttribute(UserAttributeLDAPStorageMapper.USER_MODEL_ATTRIBUTE, "User Model Attribute", config);
         checkMandatoryConfigAttribute(UserAttributeLDAPStorageMapper.LDAP_ATTRIBUTE, "LDAP Attribute", config);
 
+        boolean isBinaryAttribute = config.get(UserAttributeLDAPStorageMapper.IS_BINARY_ATTRIBUTE, false);
+        boolean alwaysReadValueFromLDAP = config.get(UserAttributeLDAPStorageMapper.ALWAYS_READ_VALUE_FROM_LDAP, false);
+        if (isBinaryAttribute && !alwaysReadValueFromLDAP) {
+            throw new ComponentValidationException("With Binary attribute enabled, the ''Always read value from LDAP'' must be enabled too");
+        }
+
     }
 
     @Override
@@ -103,4 +113,14 @@ public class UserAttributeLDAPStorageMapperFactory extends AbstractLDAPStorageMa
     public List<ProviderConfigProperty> getConfigProperties(RealmModel realm, ComponentModel parent) {
         return getConfigProps(parent);
     }
+
+
+    @Override
+    public void updateLDAPConfig(LDAPConfig ldapConfig, ComponentModel mapperModel) {
+        boolean isBinaryAttribute = mapperModel.get(UserAttributeLDAPStorageMapper.IS_BINARY_ATTRIBUTE, false);
+        if (isBinaryAttribute) {
+            String ldapAttrName = mapperModel.getConfig().getFirst(UserAttributeLDAPStorageMapper.LDAP_ATTRIBUTE);
+            ldapConfig.addBinaryAttribute(ldapAttrName);
+        }
+    }
 }