keycloak-memoizeit

diff --git a/federation/ldap/src/main/java/org/keycloak/storage/ldap/idm/store/ldap/LDAPIdentityStore.java b/federation/ldap/src/main/java/org/keycloak/storage/ldap/idm/store/ldap/LDAPIdentityStore.java
index 36ff34a..1a9f7c0 100644
--- a/federation/ldap/src/main/java/org/keycloak/storage/ldap/idm/store/ldap/LDAPIdentityStore.java
+++ b/federation/ldap/src/main/java/org/keycloak/storage/ldap/idm/store/ldap/LDAPIdentityStore.java
@@ -92,7 +92,7 @@ public class LDAPIdentityStore implements IdentityStore {
         }
 
         String entryDN = ldapObject.getDn().toString();
-        BasicAttributes ldapAttributes = extractAttributes(ldapObject, true);
+        BasicAttributes ldapAttributes = extractAttributesForSaving(ldapObject, true);
         this.operationManager.createSubContext(entryDN, ldapAttributes);
         ldapObject.setUuid(getEntryIdentifier(ldapObject));
 
@@ -105,7 +105,7 @@ public class LDAPIdentityStore implements IdentityStore {
     public void update(LDAPObject ldapObject) {
         checkRename(ldapObject);
 
-        BasicAttributes updatedAttributes = extractAttributes(ldapObject, false);
+        BasicAttributes updatedAttributes = extractAttributesForSaving(ldapObject, false);
         NamingEnumeration<Attribute> attributes = updatedAttributes.getAll();
 
         String entryDn = ldapObject.getDn().toString();
@@ -396,47 +396,36 @@ public class LDAPIdentityStore implements IdentityStore {
     }
 
 
-    protected BasicAttributes extractAttributes(LDAPObject ldapObject, boolean isCreate) {
+    protected BasicAttributes extractAttributesForSaving(LDAPObject ldapObject, boolean isCreate) {
         BasicAttributes entryAttributes = new BasicAttributes();
 
         for (Map.Entry<String, Set<String>> attrEntry : ldapObject.getAttributes().entrySet()) {
             String attrName = attrEntry.getKey();
             Set<String> attrValue = attrEntry.getValue();
 
-            // ldapObject.getReadOnlyAttributeNames() are lower-cased
-            if (!ldapObject.getReadOnlyAttributeNames().contains(attrName.toLowerCase()) && (isCreate || !ldapObject.getRdnAttributeName().equalsIgnoreCase(attrName))) {
-
-                if (attrValue == null) {
-                    // Shouldn't happen
-                    logger.warnf("Attribute '%s' is null on LDAP object '%s' . Using empty value to be saved to LDAP", attrName, ldapObject.getDn().toString());
-                    attrValue = Collections.emptySet();
-                }
+            if (attrValue == null) {
+                // Shouldn't happen
+                logger.warnf("Attribute '%s' is null on LDAP object '%s' . Using empty value to be saved to LDAP", attrName, ldapObject.getDn().toString());
+                attrValue = Collections.emptySet();
+            }
 
-                // Ignore empty attributes during create
-                if (isCreate && attrValue.isEmpty()) {
-                    continue;
+            if (
+                // Ignore empty attributes on create (changetype: add)
+                !(isCreate && attrValue.isEmpty()) &&
+
+                // Since we're extracting for saving, skip read-only attributes. ldapObject.getReadOnlyAttributeNames() are lower-cased
+                !ldapObject.getReadOnlyAttributeNames().contains(attrName.toLowerCase()) &&
+
+                // Only extract RDN for create since it can't be changed on update
+                (isCreate || !ldapObject.getRdnAttributeName().equalsIgnoreCase(attrName))
+            ) {
+                if (getConfig().getBinaryAttributeNames().contains(attrName)) {
+                    // Binary attribute
+                    entryAttributes.put(createBinaryBasicAttribute(attrName, attrValue));
+                } else {
+                    // Text attribute
+                    entryAttributes.put(createBasicAttribute(attrName, attrValue));
                 }
-
-                BasicAttribute attr = new BasicAttribute(attrName);
-                for (String val : attrValue) {
-                    if (val == null || val.toString().trim().length() == 0) {
-                        val = LDAPConstants.EMPTY_ATTRIBUTE_VALUE;
-                    }
-
-                    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);
             }
         }
 
@@ -446,13 +435,6 @@ public class LDAPIdentityStore implements IdentityStore {
 
             for (String objectClassValue : ldapObject.getObjectClasses()) {
                 objectClassAttribute.add(objectClassValue);
-
-                if ((objectClassValue.equalsIgnoreCase(LDAPConstants.GROUP_OF_NAMES)
-                        || objectClassValue.equalsIgnoreCase(LDAPConstants.GROUP_OF_ENTRIES)
-                        || objectClassValue.equalsIgnoreCase(LDAPConstants.GROUP_OF_UNIQUE_NAMES)) &&
-                        (entryAttributes.get(LDAPConstants.MEMBER) == null)) {
-                    entryAttributes.put(LDAPConstants.MEMBER, LDAPConstants.EMPTY_MEMBER_ATTRIBUTE_VALUE);
-                }
             }
 
             entryAttributes.put(objectClassAttribute);
@@ -461,6 +443,38 @@ public class LDAPIdentityStore implements IdentityStore {
         return entryAttributes;
     }
 
+    private BasicAttribute createBasicAttribute(String attrName, Set<String> attrValue) {
+        BasicAttribute attr = new BasicAttribute(attrName);
+
+        for (String value : attrValue) {
+            if (value == null || value.trim().length() == 0) {
+                value = LDAPConstants.EMPTY_ATTRIBUTE_VALUE;
+            }
+
+            attr.add(value);
+        }
+
+        return attr;
+    }
+
+    private BasicAttribute createBinaryBasicAttribute(String attrName, Set<String> attrValue) {
+        BasicAttribute attr = new BasicAttribute(attrName);
+
+        for (String value : attrValue) {
+            if (value == null || value.trim().length() == 0) {
+                value = LDAPConstants.EMPTY_ATTRIBUTE_VALUE;
+            }
+
+            try {
+                byte[] bytes = Base64.decode(value);
+                attr.add(bytes);
+            } catch (IOException ioe) {
+                logger.warnf("Wasn't able to Base64 decode the attribute value. Ignoring attribute update. Attribute: %s, Attribute value: %s", attrName, attrValue);
+            }
+        }
+
+        return attr;
+    }
 
     protected String getEntryIdentifier(final LDAPObject ldapObject) {
         try {

diff --git a/federation/ldap/src/main/java/org/keycloak/storage/ldap/LDAPUtils.java b/federation/ldap/src/main/java/org/keycloak/storage/ldap/LDAPUtils.java
index 21f4bae..4f05ad4 100755
--- a/federation/ldap/src/main/java/org/keycloak/storage/ldap/LDAPUtils.java
+++ b/federation/ldap/src/main/java/org/keycloak/storage/ldap/LDAPUtils.java
@@ -128,13 +128,24 @@ public class LDAPUtils {
     // roles & groups
 
     public static LDAPObject createLDAPGroup(LDAPStorageProvider ldapProvider, String groupName, String groupNameAttribute, Collection<String> objectClasses,
-                                             String parentDn, Map<String, Set<String>> additionalAttributes) {
+                                             String parentDn, Map<String, Set<String>> additionalAttributes, String membershipLdapAttribute) {
         LDAPObject ldapObject = new LDAPObject();
 
         ldapObject.setRdnAttributeName(groupNameAttribute);
         ldapObject.setObjectClasses(objectClasses);
         ldapObject.setSingleAttribute(groupNameAttribute, groupName);
 
+        for (String objectClassValue : objectClasses) {
+            // On MSAD with object class "group", empty member must not be added. Specified object classes typically
+            // require empty member attribute if no members have joined yet
+            if ((objectClassValue.equalsIgnoreCase(LDAPConstants.GROUP_OF_NAMES)
+                    || objectClassValue.equalsIgnoreCase(LDAPConstants.GROUP_OF_ENTRIES)
+                    || objectClassValue.equalsIgnoreCase(LDAPConstants.GROUP_OF_UNIQUE_NAMES)) &&
+                    additionalAttributes.get(membershipLdapAttribute) == null) {
+                ldapObject.setSingleAttribute(membershipLdapAttribute, LDAPConstants.EMPTY_MEMBER_ATTRIBUTE_VALUE);
+            }
+        }
+
         LDAPDn roleDn = LDAPDn.fromString(parentDn);
         roleDn.addFirst(groupNameAttribute, groupName);
         ldapObject.setDn(roleDn);

diff --git a/federation/ldap/src/main/java/org/keycloak/storage/ldap/mappers/membership/group/GroupLDAPStorageMapper.java b/federation/ldap/src/main/java/org/keycloak/storage/ldap/mappers/membership/group/GroupLDAPStorageMapper.java
index f6a7c53..66ec3c3 100644
--- a/federation/ldap/src/main/java/org/keycloak/storage/ldap/mappers/membership/group/GroupLDAPStorageMapper.java
+++ b/federation/ldap/src/main/java/org/keycloak/storage/ldap/mappers/membership/group/GroupLDAPStorageMapper.java
@@ -122,7 +122,7 @@ public class GroupLDAPStorageMapper extends AbstractLDAPStorageMapper implements
 
     public LDAPObject createLDAPGroup(String groupName, Map<String, Set<String>> additionalAttributes) {
         LDAPObject ldapGroup = LDAPUtils.createLDAPGroup(ldapProvider, groupName, config.getGroupNameLdapAttribute(), config.getGroupObjectClasses(ldapProvider),
-                config.getGroupsDn(), additionalAttributes);
+                config.getGroupsDn(), additionalAttributes, config.getMembershipLdapAttribute());
 
         logger.debugf("Creating group [%s] to LDAP with DN [%s]", groupName, ldapGroup.getDn().toString());
         return ldapGroup;

diff --git a/federation/ldap/src/main/java/org/keycloak/storage/ldap/mappers/membership/role/RoleLDAPStorageMapper.java b/federation/ldap/src/main/java/org/keycloak/storage/ldap/mappers/membership/role/RoleLDAPStorageMapper.java
index 36c1e0d..cee571f 100644
--- a/federation/ldap/src/main/java/org/keycloak/storage/ldap/mappers/membership/role/RoleLDAPStorageMapper.java
+++ b/federation/ldap/src/main/java/org/keycloak/storage/ldap/mappers/membership/role/RoleLDAPStorageMapper.java
@@ -245,7 +245,7 @@ public class RoleLDAPStorageMapper extends AbstractLDAPStorageMapper implements 
 
     public LDAPObject createLDAPRole(String roleName) {
         LDAPObject ldapRole = LDAPUtils.createLDAPGroup(ldapProvider, roleName, config.getRoleNameLdapAttribute(), config.getRoleObjectClasses(ldapProvider),
-                config.getRolesDn(), Collections.<String, Set<String>>emptyMap());
+                config.getRolesDn(), Collections.<String, Set<String>>emptyMap(), config.getMembershipLdapAttribute());
 
         logger.debugf("Creating role [%s] to LDAP with DN [%s]", roleName, ldapRole.getDn().toString());
         return ldapRole;