KEYCLOAK-1359 LDAP & Active directory fixes and improvements

federation/ldap/src/main/java/org/keycloak/federation/ldap/LDAPUtils.java

@@ -45,16 +45,6 @@ public class LDAPUtils {
         return ldapUser;
     }
 
-    public static void removeAllUsers(LDAPFederationProvider ldapProvider, RealmModel realm) {
-        LDAPIdentityStore ldapStore = ldapProvider.getLdapIdentityStore();
-        LDAPIdentityQuery ldapQuery = LDAPUtils.createQueryForUserSearch(ldapProvider, realm);
-        List<LDAPObject> allUsers = ldapQuery.getResultList();
-
-        for (LDAPObject ldapUser : allUsers) {
-            ldapStore.remove(ldapUser);
-        }
-    }
-
     public static LDAPIdentityQuery createQueryForUserSearch(LDAPFederationProvider ldapProvider, RealmModel realm) {
         LDAPIdentityQuery ldapQuery = new LDAPIdentityQuery(ldapProvider);
         LDAPConfig config = ldapProvider.getLdapIdentityStore().getConfig();

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

@@ -71,7 +71,7 @@ public class LDAPIdentityStore implements IdentityStore {
     public void add(LDAPObject ldapObject) {
         // id will be assigned by the ldap server
         if (ldapObject.getUuid() != null) {
-            throw new IllegalStateException("Can't add object with already assigned uuid");
+            throw new ModelException("Can't add object with already assigned uuid");
         }
 
         String entryDN = ldapObject.getDn().toString();
@@ -108,20 +108,20 @@ public class LDAPIdentityStore implements IdentityStore {
 
     @Override
     public List<LDAPObject> fetchQueryResults(LDAPIdentityQuery identityQuery) {
+        if (identityQuery.getSorting() != null && !identityQuery.getSorting().isEmpty()) {
+            throw new ModelException("LDAP Identity Store does not yet support sorted queries.");
+        }
+
         List<LDAPObject> results = new ArrayList<>();
 
         try {
-            if (identityQuery.getSorting() != null && !identityQuery.getSorting().isEmpty()) {
-                throw new ModelException("LDAP Identity Store does not yet support sorted queries.");
-            }
-
             String baseDN = identityQuery.getSearchDn();
 
             for (Condition condition : identityQuery.getConditions()) {
 
                 // Check if we are searching by ID
                 String uuidAttrName = getConfig().getUuidLDAPAttributeName();
-                if (condition.getParameter() != null && condition.getParameter().getName().equals(uuidAttrName)) {
+                if (condition.getParameter() != null && condition.getParameter().getName().equalsIgnoreCase(uuidAttrName)) {
                     if (EqualCondition.class.isInstance(condition)) {
                         EqualCondition equalCondition = (EqualCondition) condition;
                         SearchResult search = this.operationManager
@@ -147,7 +147,7 @@ public class LDAPIdentityStore implements IdentityStore {
             }
 
             for (SearchResult result : search) {
-                if (!result.getNameInNamespace().equals(baseDN)) {
+                if (!result.getNameInNamespace().equalsIgnoreCase(baseDN)) {
                     results.add(populateAttributedType(result, identityQuery.getReturningReadOnlyLdapAttributes()));
                 }
             }
@@ -278,7 +278,7 @@ public class LDAPIdentityStore implements IdentityStore {
 
         QueryParameter queryParameter = condition.getParameter();
 
-        if (!getConfig().getUuidLDAPAttributeName().equals(queryParameter.getName())) {
+        if (!getConfig().getUuidLDAPAttributeName().equalsIgnoreCase(queryParameter.getName())) {
             String attributeName = queryParameter.getName();
 
             if (attributeName != null) {
@@ -400,7 +400,7 @@ public class LDAPIdentityStore implements IdentityStore {
 
                 String ldapAttributeName = ldapAttribute.getID();
 
-                if (ldapAttributeName.toLowerCase().equals(getConfig().getUuidLDAPAttributeName().toLowerCase())) {
+                if (ldapAttributeName.equalsIgnoreCase(getConfig().getUuidLDAPAttributeName())) {
                     Object uuidValue = ldapAttribute.get();
                     ldapObject.setUuid(this.operationManager.decodeEntryUUID(uuidValue));
                 } else {
@@ -411,7 +411,7 @@ public class LDAPIdentityStore implements IdentityStore {
                         attrValues.add(attrVal);
                     }
 
-                    if (ldapAttributeName.toLowerCase().equals(LDAPConstants.OBJECT_CLASS)) {
+                    if (ldapAttributeName.equalsIgnoreCase(LDAPConstants.OBJECT_CLASS)) {
                         ldapObject.setObjectClasses(attrValues);
                     } else {
                         if (logger.isTraceEnabled()) {
@@ -444,7 +444,7 @@ public class LDAPIdentityStore implements IdentityStore {
         for (Map.Entry<String, Object> attrEntry : ldapObject.getAttributes().entrySet()) {
             String attrName = attrEntry.getKey();
             Object attrValue = attrEntry.getValue();
-            if (!ldapObject.getReadOnlyAttributeNames().contains(attrName) && (isCreate || !ldapObject.getRdnAttributeName().equals(attrName))) {
+            if (!ldapObject.getReadOnlyAttributeNames().contains(attrName) && (isCreate || !ldapObject.getRdnAttributeName().equalsIgnoreCase(attrName))) {
 
                 if (String.class.isInstance(attrValue)) {
                     if (attrValue.toString().trim().length() == 0) {
@@ -461,7 +461,7 @@ public class LDAPIdentityStore implements IdentityStore {
                 } else if (attrValue == null || attrValue.toString().trim().length() == 0) {
                     entryAttributes.put(attrName, LDAPConstants.EMPTY_ATTRIBUTE_VALUE);
                 } else {
-                    throw new IllegalArgumentException("Unexpected type of value of argument " + attrName + ". Value is " + attrValue);
+                    throw new ModelException("Unexpected type of value of argument " + attrName + ". Value is " + attrValue);
                 }
             }
         }
@@ -473,9 +473,9 @@ public class LDAPIdentityStore implements IdentityStore {
             for (String objectClassValue : ldapObject.getObjectClasses()) {
                 objectClassAttribute.add(objectClassValue);
 
-                if (objectClassValue.equals(LDAPConstants.GROUP_OF_NAMES)
+                if (objectClassValue.equalsIgnoreCase(LDAPConstants.GROUP_OF_NAMES)
-                        || objectClassValue.equals(LDAPConstants.GROUP_OF_ENTRIES)
+                        || objectClassValue.equalsIgnoreCase(LDAPConstants.GROUP_OF_ENTRIES)
-                        || objectClassValue.equals(LDAPConstants.GROUP_OF_UNIQUE_NAMES)) {
+                        || objectClassValue.equalsIgnoreCase(LDAPConstants.GROUP_OF_UNIQUE_NAMES)) {
                     entryAttributes.put(LDAPConstants.MEMBER, LDAPConstants.EMPTY_MEMBER_ATTRIBUTE_VALUE);
                 }
             }

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

@@ -513,7 +513,6 @@ public class LDAPOperationManager {
             context = createLdapContext();
             return operation.execute(context);
         } catch (NamingException ne) {
-            logger.error("Could not create Ldap context or operation execution error.", ne);
             throw ne;
         } finally {
             if (context != null) {

federation/ldap/src/main/java/org/keycloak/federation/ldap/mappers/FullNameLDAPFederationMapper.java

@@ -1,24 +1,19 @@
 package org.keycloak.federation.ldap.mappers;
 
 import java.util.HashSet;
-import java.util.List;
 import java.util.Set;
 
 import org.keycloak.federation.ldap.LDAPFederationProvider;
-import org.keycloak.federation.ldap.LDAPUtils;
 import org.keycloak.federation.ldap.idm.model.LDAPObject;
 import org.keycloak.federation.ldap.idm.query.Condition;
 import org.keycloak.federation.ldap.idm.query.QueryParameter;
 import org.keycloak.federation.ldap.idm.query.internal.EqualCondition;
 import org.keycloak.federation.ldap.idm.query.internal.LDAPIdentityQuery;
-import org.keycloak.mappers.UserFederationMapper;
-import org.keycloak.models.KeycloakSession;
 import org.keycloak.models.LDAPConstants;
 import org.keycloak.models.RealmModel;
 import org.keycloak.models.UserFederationMapperModel;
 import org.keycloak.models.UserFederationProvider;
 import org.keycloak.models.UserModel;
-import org.keycloak.provider.ProviderConfigProperty;
 
 /**
  * Mapper useful for the LDAP deployments when some attribute (usually CN) is mapped to full name of user
@@ -130,7 +125,7 @@ public class FullNameLDAPFederationMapper extends AbstractLDAPFederationMapper {
             fullName = firstNameCondition.getValue() + " " + lastNameCondition.getValue();
         } else if (firstNameCondition != null) {
             fullName = (String) firstNameCondition.getValue();
-        } else if (firstNameCondition != null) {
+        } else if (lastNameCondition != null) {
             fullName = (String) lastNameCondition.getValue();
         } else {
             return;

federation/ldap/src/main/java/org/keycloak/federation/ldap/mappers/RoleLDAPFederationMapper.java

@@ -1,14 +1,11 @@
 package org.keycloak.federation.ldap.mappers;
 
-import java.util.Arrays;
 import java.util.Collection;
 import java.util.HashSet;
 import java.util.List;
 import java.util.Set;
 import java.util.TreeSet;
 
-import javax.naming.directory.SearchControls;
-
 import org.jboss.logging.Logger;
 import org.keycloak.federation.ldap.LDAPFederationProvider;
 import org.keycloak.federation.ldap.idm.model.LDAPDn;
@@ -126,7 +123,7 @@ public class RoleLDAPFederationMapper extends AbstractLDAPFederationMapper {
         String rolesDn = getRolesDn(mapperModel);
         ldapQuery.setSearchDn(rolesDn);
 
-        Collection<String> roleObjectClasses = getRoleObjectClasses(mapperModel);
+        Collection<String> roleObjectClasses = getRoleObjectClasses(mapperModel, ldapProvider);
         ldapQuery.addObjectClasses(roleObjectClasses);
 
         String rolesRdnAttr = getRoleNameLdapAttribute(mapperModel);
@@ -144,11 +141,11 @@ public class RoleLDAPFederationMapper extends AbstractLDAPFederationMapper {
         } else {
             String clientId = mapperModel.getConfig().get(CLIENT_ID);
             if (clientId == null) {
-                throw new IllegalStateException("Using client roles mapping is requested, but parameter client.id not found!");
+                throw new ModelException("Using client roles mapping is requested, but parameter client.id not found!");
             }
             ClientModel client = realm.getClientByClientId(clientId);
             if (client == null) {
-                throw new IllegalStateException("Can't found requested client with clientId: " + clientId);
+                throw new ModelException("Can't found requested client with clientId: " + clientId);
             }
             return client;
         }
@@ -157,7 +154,7 @@ public class RoleLDAPFederationMapper extends AbstractLDAPFederationMapper {
     protected String getRolesDn(UserFederationMapperModel mapperModel) {
         String rolesDn = mapperModel.getConfig().get(ROLES_DN);
         if (rolesDn == null) {
-            throw new IllegalStateException("Roles DN is null! Check your configuration");
+            throw new ModelException("Roles DN is null! Check your configuration");
         }
         return rolesDn;
     }
@@ -172,10 +169,11 @@ public class RoleLDAPFederationMapper extends AbstractLDAPFederationMapper {
         return membershipAttrName!=null ? membershipAttrName : LDAPConstants.MEMBER;
     }
 
-    protected Collection<String> getRoleObjectClasses(UserFederationMapperModel mapperModel) {
+    protected Collection<String> getRoleObjectClasses(UserFederationMapperModel mapperModel, LDAPFederationProvider ldapProvider) {
         String objectClasses = mapperModel.getConfig().get(ROLE_OBJECT_CLASSES);
         if (objectClasses == null) {
-            objectClasses = "groupOfNames";
+            // For Active directory, the default is 'group' . For other servers 'groupOfNames'
+            objectClasses = ldapProvider.getLdapIdentityStore().getConfig().isActiveDirectory() ? LDAPConstants.GROUP : LDAPConstants.GROUP_OF_NAMES;
         }
         String[] objClasses = objectClasses.split(",");
 
@@ -191,18 +189,18 @@ public class RoleLDAPFederationMapper extends AbstractLDAPFederationMapper {
 
     private Mode getMode(UserFederationMapperModel mapperModel) {
         String modeString = mapperModel.getConfig().get(MODE);
-        if (modeString == null || modeString.trim().length() == 0) {
+        if (modeString == null || modeString.isEmpty()) {
-            return Mode.LDAP_ONLY;
+            throw new ModelException("Mode is missing! Check your configuration");
         }
 
         return Enum.valueOf(Mode.class, modeString.toUpperCase());
     }
 
-    protected LDAPObject createLDAPRole(UserFederationMapperModel mapperModel, String roleName, LDAPFederationProvider ldapProvider) {
+    public LDAPObject createLDAPRole(UserFederationMapperModel mapperModel, String roleName, LDAPFederationProvider ldapProvider) {
         LDAPObject ldapObject = new LDAPObject();
         String roleNameAttribute = getRoleNameLdapAttribute(mapperModel);
         ldapObject.setRdnAttributeName(roleNameAttribute);
-        ldapObject.setObjectClasses(getRoleObjectClasses(mapperModel));
+        ldapObject.setObjectClasses(getRoleObjectClasses(mapperModel, ldapProvider));
         ldapObject.setAttribute(roleNameAttribute, roleName);
 
         LDAPDn roleDn = LDAPDn.fromString(getRolesDn(mapperModel));
@@ -231,8 +229,8 @@ public class RoleLDAPFederationMapper extends AbstractLDAPFederationMapper {
         Set<String> memberships = getExistingMemberships(mapperModel, ldapRole);
         memberships.remove(ldapUser.getDn().toString());
 
-        // Some membership placeholder needs to be always here as "member" is mandatory attribute on some LDAP servers
+        // Some membership placeholder needs to be always here as "member" is mandatory attribute on some LDAP servers. But on active directory! (Empty membership is not allowed here)
-        if (memberships.size() == 0) {
+        if (memberships.size() == 0 && !ldapProvider.getLdapIdentityStore().getConfig().isActiveDirectory()) {
             memberships.add(LDAPConstants.EMPTY_MEMBER_ATTRIBUTE_VALUE);
         }
 
@@ -278,23 +276,6 @@ public class RoleLDAPFederationMapper extends AbstractLDAPFederationMapper {
         return ldapQuery.getResultList();
     }
 
-    protected Set<RoleModel> getLDAPRoleMappingsConverted(UserFederationMapperModel mapperModel, LDAPFederationProvider ldapProvider, LDAPObject ldapUser, RoleContainerModel roleContainer) {
-        List<LDAPObject> ldapRoles = getLDAPRoleMappings(mapperModel, ldapProvider, ldapUser);
-
-        Set<RoleModel> roles = new HashSet<RoleModel>();
-        String roleNameLdapAttr = getRoleNameLdapAttribute(mapperModel);
-        for (LDAPObject role : ldapRoles) {
-            String roleName = role.getAttributeAsString(roleNameLdapAttr);
-            RoleModel modelRole = roleContainer.getRole(roleName);
-            if (modelRole == null) {
-                // Add role to local DB
-                modelRole = roleContainer.addRole(roleName);
-            }
-            roles.add(modelRole);
-        }
-        return roles;
-    }
-
     @Override
     public UserModel proxy(UserFederationMapperModel mapperModel, LDAPFederationProvider ldapProvider, LDAPObject ldapUser, UserModel delegate, RealmModel realm) {
         final Mode mode = getMode(mapperModel);
@@ -321,6 +302,9 @@ public class RoleLDAPFederationMapper extends AbstractLDAPFederationMapper {
         private final RealmModel realm;
         private final Mode mode;
 
+        // Avoid loading role mappings from LDAP more times per-request
+        private Set<RoleModel> cachedLDAPRoleMappings;
+
         public LDAPRoleMappingsUserDelegate(UserModel user, UserFederationMapperModel mapperModel, LDAPFederationProvider ldapProvider, LDAPObject ldapUser,
                                             RealmModel realm, Mode mode) {
             super(user);
@@ -385,6 +369,7 @@ public class RoleLDAPFederationMapper extends AbstractLDAPFederationMapper {
                 if (role.getContainer().equals(roleContainer)) {
 
                     // We need to create new role mappings in LDAP
+                    cachedLDAPRoleMappings = null;
                     addRoleMappingInLDAP(mapperModel, role.getName(), ldapProvider, ldapUser);
                 } else {
                     super.grantRole(role);
@@ -415,6 +400,30 @@ public class RoleLDAPFederationMapper extends AbstractLDAPFederationMapper {
             return modelRoleMappings;
         }
 
+        protected Set<RoleModel> getLDAPRoleMappingsConverted(UserFederationMapperModel mapperModel, LDAPFederationProvider ldapProvider, LDAPObject ldapUser, RoleContainerModel roleContainer) {
+            if (cachedLDAPRoleMappings != null) {
+                return new HashSet<>(cachedLDAPRoleMappings);
+            }
+
+            List<LDAPObject> ldapRoles = getLDAPRoleMappings(mapperModel, ldapProvider, ldapUser);
+
+            Set<RoleModel> roles = new HashSet<RoleModel>();
+            String roleNameLdapAttr = getRoleNameLdapAttribute(mapperModel);
+            for (LDAPObject role : ldapRoles) {
+                String roleName = role.getAttributeAsString(roleNameLdapAttr);
+                RoleModel modelRole = roleContainer.getRole(roleName);
+                if (modelRole == null) {
+                    // Add role to local DB
+                    modelRole = roleContainer.addRole(roleName);
+                }
+                roles.add(modelRole);
+            }
+
+            cachedLDAPRoleMappings = new HashSet<>(roles);
+
+            return roles;
+        }
+
         @Override
         public void deleteRoleMapping(RoleModel role) {
             RoleContainerModel roleContainer = getTargetRoleContainer(mapperModel, realm);
@@ -438,6 +447,7 @@ public class RoleLDAPFederationMapper extends AbstractLDAPFederationMapper {
                         throw new ModelException("Not possible to delete LDAP role mappings as mapper mode is READ_ONLY");
                     } else {
                         // Delete ldap role mappings
+                        cachedLDAPRoleMappings = null;
                         deleteRoleMappingInLDAP(mapperModel, ldapProvider, ldapUser, ldapRole);
                     }
                 }

federation/ldap/src/main/java/org/keycloak/federation/ldap/mappers/RoleLDAPFederationMapperFactory.java

@@ -1,25 +1,33 @@
 package org.keycloak.federation.ldap.mappers;
 
 import java.util.ArrayList;
-import java.util.Arrays;
 import java.util.LinkedList;
 import java.util.List;
-import java.util.Map;
 
+import org.jboss.logging.Logger;
+import org.keycloak.federation.ldap.LDAPFederationProvider;
 import org.keycloak.mappers.MapperConfigValidationException;
 import org.keycloak.mappers.UserFederationMapper;
-import org.keycloak.models.ClientModel;
 import org.keycloak.models.KeycloakSession;
+import org.keycloak.models.KeycloakSessionFactory;
 import org.keycloak.models.LDAPConstants;
 import org.keycloak.models.RealmModel;
 import org.keycloak.models.UserFederationMapperModel;
+import org.keycloak.models.UserFederationProvider;
+import org.keycloak.models.UserFederationProviderFactory;
+import org.keycloak.models.UserFederationProviderModel;
+import org.keycloak.models.utils.KeycloakModelUtils;
 import org.keycloak.provider.ProviderConfigProperty;
+import org.keycloak.provider.ProviderEvent;
+import org.keycloak.provider.ProviderEventListener;
 
 /**
  * @author <a href="mailto:mposolda@redhat.com">Marek Posolda</a>
  */
 public class RoleLDAPFederationMapperFactory extends AbstractLDAPFederationMapperFactory {
 
+    private static final Logger logger = Logger.getLogger(RoleLDAPFederationMapperFactory.class);
+
     public static final String PROVIDER_ID = "role-ldap-mapper";
 
     protected static final List<ProviderConfigProperty> configProperties = new ArrayList<ProviderConfigProperty>();
@@ -40,8 +48,8 @@ public class RoleLDAPFederationMapperFactory extends AbstractLDAPFederationMappe
         configProperties.add(membershipLDAPAttribute);
 
         ProviderConfigProperty roleObjectClasses = createConfigProperty(RoleLDAPFederationMapper.ROLE_OBJECT_CLASSES, "Role Object Classes",
-                "Object classes of the role object divided by comma (if more values needed). In typical LDAP deployment it could be 'groupOfNames' or 'groupOfEntries' ",
+                "Object class (or classes) of the role object. It's divided by comma if more classes needed. In typical LDAP deployment it could be 'groupOfNames' . In Active Directory it's usually 'group' ",
-                ProviderConfigProperty.STRING_TYPE, LDAPConstants.GROUP_OF_NAMES);
+                ProviderConfigProperty.STRING_TYPE, null);
         configProperties.add(roleObjectClasses);
 
         List<String> modes = new LinkedList<String>();
@@ -90,9 +98,46 @@ public class RoleLDAPFederationMapperFactory extends AbstractLDAPFederationMappe
         return PROVIDER_ID;
     }
 
+    // Sync roles from LDAP to Keycloak DB during creation or update of mapperModel
+    @Override
+    public void postInit(KeycloakSessionFactory factory) {
+        factory.register(new ProviderEventListener() {
+
+            @Override
+            public void onEvent(ProviderEvent event) {
+                if (event instanceof RealmModel.UserFederationMapperEvent) {
+                    RealmModel.UserFederationMapperEvent mapperEvent = (RealmModel.UserFederationMapperEvent)event;
+                    UserFederationMapperModel mapperModel = mapperEvent.getFederationMapper();
+                    RealmModel realm = mapperEvent.getRealm();
+                    KeycloakSession session = mapperEvent.getSession();
+
+                    if (mapperModel.getFederationMapperType().equals(PROVIDER_ID)) {
+                        try {
+                            String federationProviderId = mapperModel.getFederationProviderId();
+                            UserFederationProviderModel providerModel = KeycloakModelUtils.findUserFederationProviderById(federationProviderId, realm);
+                            if (providerModel == null) {
+                                throw new IllegalStateException("Can't find federation provider with ID [" + federationProviderId + "] in realm " + realm.getName());
+                            }
+
+                            UserFederationProviderFactory ldapFactory = (UserFederationProviderFactory) session.getKeycloakSessionFactory().getProviderFactory(UserFederationProvider.class, providerModel.getProviderName());
+                            LDAPFederationProvider ldapProvider = (LDAPFederationProvider) ldapFactory.getInstance(session, providerModel);
+
+                            // Sync roles
+                            new RoleLDAPFederationMapper().syncRolesFromLDAP(mapperModel, ldapProvider, realm);
+                        } catch (Exception e) {
+                            logger.warn("Exception during initial sync of roles from LDAP.", e);
+                        }
+                    }
+                }
+            }
+
+        });
+    }
+
     @Override
     public void validateConfig(UserFederationMapperModel mapperModel) throws MapperConfigValidationException {
         checkMandatoryConfigAttribute(RoleLDAPFederationMapper.ROLES_DN, "LDAP Roles DN", mapperModel);
+        checkMandatoryConfigAttribute(RoleLDAPFederationMapper.MODE, "Mode", mapperModel);
 
         String realmMappings = mapperModel.getConfig().get(RoleLDAPFederationMapper.USE_REALM_ROLES_MAPPING);
         boolean useRealmMappings = Boolean.parseBoolean(realmMappings);

federation/ldap/src/main/java/org/keycloak/federation/ldap/mappers/UserAttributeLDAPFederationMapper.java

@@ -1,18 +1,13 @@
 package org.keycloak.federation.ldap.mappers;
 
-import java.io.Serializable;
 import java.lang.reflect.Method;
-import java.util.List;
 import java.util.Map;
 
 import org.keycloak.federation.ldap.LDAPFederationProvider;
-import org.keycloak.federation.ldap.LDAPUtils;
 import org.keycloak.federation.ldap.idm.model.LDAPObject;
 import org.keycloak.federation.ldap.idm.query.Condition;
 import org.keycloak.federation.ldap.idm.query.QueryParameter;
 import org.keycloak.federation.ldap.idm.query.internal.LDAPIdentityQuery;
-import org.keycloak.mappers.UserFederationMapper;
-import org.keycloak.models.KeycloakSession;
 import org.keycloak.models.RealmModel;
 import org.keycloak.models.UserFederationMapperModel;
 import org.keycloak.models.UserFederationProvider;
@@ -20,7 +15,6 @@ import org.keycloak.models.UserModel;
 import org.keycloak.models.utils.reflection.Property;
 import org.keycloak.models.utils.reflection.PropertyCriteria;
 import org.keycloak.models.utils.reflection.PropertyQueries;
-import org.keycloak.provider.ProviderConfigProperty;
 
 /**
  * @author <a href="mailto:mposolda@redhat.com">Marek Posolda</a>
@@ -124,7 +118,7 @@ public class UserAttributeLDAPFederationMapper extends AbstractLDAPFederationMap
                 }
 
                 protected void setLDAPAttribute(String modelAttrName, String value) {
-                    if (modelAttrName.equals(userModelAttrName)) {
+                    if (modelAttrName.equalsIgnoreCase(userModelAttrName)) {
                         if (logger.isTraceEnabled()) {
                             logger.tracef("Pushing user attribute to LDAP. Model attribute name: %s, LDAP attribute name: %s, Attribute value: %s", modelAttrName, ldapAttrName, value);
                         }
@@ -157,7 +151,7 @@ public class UserAttributeLDAPFederationMapper extends AbstractLDAPFederationMap
         // Change conditions and use ldapAttribute instead of userModel
         for (Condition condition : query.getConditions()) {
             QueryParameter param = condition.getParameter();
-            if (param != null && param.getName().equals(userModelAttrName)) {
+            if (param != null && param.getName().equalsIgnoreCase(userModelAttrName)) {
                 param.setName(ldapAttrName);
             }
         }

model/api/src/main/java/org/keycloak/models/LDAPConstants.java

@@ -63,6 +63,7 @@ public class LDAPConstants {
     public static final String OBJECT_CLASS = "objectclass";
     public static final String UID = "uid";
     public static final String USER_PASSWORD_ATTRIBUTE = "userpassword";
+    public static final String GROUP = "group";
     public static final String GROUP_OF_NAMES = "groupOfNames";
     public static final String GROUP_OF_ENTRIES = "groupOfEntries";
     public static final String GROUP_OF_UNIQUE_NAMES = "groupOfUniqueNames";

model/api/src/main/java/org/keycloak/models/RealmModel.java

@@ -29,6 +29,12 @@ public interface RealmModel extends RoleContainerModel {
         RealmModel getRealm();
     }
 
+    interface UserFederationMapperEvent extends ProviderEvent {
+        UserFederationMapperModel getFederationMapper();
+        RealmModel getRealm();
+        KeycloakSession getSession();
+    }
+
     String getId();
 
     String getName();

model/api/src/main/java/org/keycloak/models/UserFederationMapperEventImpl.java

@@ -0,0 +1,33 @@
+package org.keycloak.models;
+
+/**
+ * Called during creation or update of UserFederationMapperModel
+ *
+ * @author <a href="mailto:mposolda@redhat.com">Marek Posolda</a>
+ */
+public class UserFederationMapperEventImpl implements RealmModel.UserFederationMapperEvent {
+
+    private final UserFederationMapperModel mapperModel;
+    private final RealmModel realm;
+    private final KeycloakSession session;
+
+    public UserFederationMapperEventImpl(UserFederationMapperModel mapperModel, RealmModel realm, KeycloakSession session) {
+        this.mapperModel = mapperModel;
+        this.realm = realm;
+        this.session = session;
+    }
+
+    @Override
+    public UserFederationMapperModel getFederationMapper() {
+        return mapperModel;
+    }
+
+    @Override
+    public RealmModel getRealm() {
+        return realm;
+    }
+
+    public KeycloakSession getSession() {
+        return session;
+    }
+}

model/file/src/main/java/org/keycloak/models/file/adapter/RealmAdapter.java

@@ -30,6 +30,7 @@ import org.keycloak.models.PasswordPolicy;
 import org.keycloak.models.RealmModel;
 import org.keycloak.models.RequiredCredentialModel;
 import org.keycloak.models.RoleModel;
+import org.keycloak.models.UserFederationMapperEventImpl;
 import org.keycloak.models.UserFederationMapperModel;
 import org.keycloak.models.UserFederationProviderCreationEventImpl;
 import org.keycloak.models.UserFederationProviderModel;
@@ -837,7 +838,9 @@ public class RealmAdapter implements RealmModel {
         realm.getUserFederationProviders().add(entity);
 
         UserFederationProviderModel providerModel = new UserFederationProviderModel(entity.getId(), providerName, config, priority, displayName, fullSyncPeriod, changedSyncPeriod, lastSync);
+
         session.getKeycloakSessionFactory().publish(new UserFederationProviderCreationEventImpl(this, providerModel));
+
         return providerModel;
     }
 
@@ -935,6 +938,7 @@ public class RealmAdapter implements RealmModel {
             entity.setChangedSyncPeriod(model.getChangedSyncPeriod());
             entity.setLastSync(model.getLastSync());
             entities.add(entity);
+
             session.getKeycloakSessionFactory().publish(new UserFederationProviderCreationEventImpl(this, model));
         }
 
@@ -1459,7 +1463,11 @@ public class RealmAdapter implements RealmModel {
         entity.setConfig(model.getConfig());
 
         this.realm.getUserFederationMappers().add(entity);
-        return entityToModel(entity);
+        UserFederationMapperModel mapperModel = entityToModel(entity);
+
+        session.getKeycloakSessionFactory().publish(new UserFederationMapperEventImpl(mapperModel, this, session));
+
+        return mapperModel;
     }
 
     protected UserFederationMapperEntity getUserFederationMapperEntity(String id) {
@@ -1511,6 +1519,8 @@ public class RealmAdapter implements RealmModel {
             entity.getConfig().clear();
             entity.getConfig().putAll(mapper.getConfig());
         }
+
+        session.getKeycloakSessionFactory().publish(new UserFederationMapperEventImpl(mapper, this, session));
     }
 
     @Override

model/jpa/src/main/java/org/keycloak/models/jpa/RealmAdapter.java

@@ -13,6 +13,7 @@ import org.keycloak.models.PasswordPolicy;
 import org.keycloak.models.RealmModel;
 import org.keycloak.models.RequiredCredentialModel;
 import org.keycloak.models.RoleModel;
+import org.keycloak.models.UserFederationMapperEventImpl;
 import org.keycloak.models.UserFederationMapperModel;
 import org.keycloak.models.UserFederationProviderCreationEventImpl;
 import org.keycloak.models.UserFederationProviderModel;
@@ -789,7 +790,9 @@ public class RealmAdapter implements RealmModel {
         realm.getUserFederationProviders().add(entity);
         em.flush();
         UserFederationProviderModel providerModel = new UserFederationProviderModel(entity.getId(), providerName, config, priority, displayName, fullSyncPeriod, changedSyncPeriod, lastSync);
+
         session.getKeycloakSessionFactory().publish(new UserFederationProviderCreationEventImpl(this, providerModel));
+
         return providerModel;
     }
 
@@ -907,6 +910,7 @@ public class RealmAdapter implements RealmModel {
             entity.setLastSync(model.getLastSync());
             em.persist(entity);
             realm.getUserFederationProviders().add(entity);
+
             session.getKeycloakSessionFactory().publish(new UserFederationProviderCreationEventImpl(this, model));
         }
     }
@@ -1413,7 +1417,11 @@ public class RealmAdapter implements RealmModel {
 
         em.persist(entity);
         this.realm.getUserFederationMappers().add(entity);
-        return entityToModel(entity);
+        UserFederationMapperModel mapperModel = entityToModel(entity);
+
+        session.getKeycloakSessionFactory().publish(new UserFederationMapperEventImpl(mapperModel, this, session));
+
+        return mapperModel;
     }
 
     @Override
@@ -1467,6 +1475,8 @@ public class RealmAdapter implements RealmModel {
             entity.getConfig().putAll(mapper.getConfig());
         }
         em.flush();
+
+        session.getKeycloakSessionFactory().publish(new UserFederationMapperEventImpl(mapper, this, session));
     }
 
     @Override

model/mongo/src/main/java/org/keycloak/models/mongo/keycloak/adapters/RealmAdapter.java

@@ -18,6 +18,7 @@ import org.keycloak.models.RealmModel;
 import org.keycloak.models.RealmProvider;
 import org.keycloak.models.RequiredCredentialModel;
 import org.keycloak.models.RoleModel;
+import org.keycloak.models.UserFederationMapperEventImpl;
 import org.keycloak.models.UserFederationMapperModel;
 import org.keycloak.models.UserFederationProviderCreationEventImpl;
 import org.keycloak.models.UserFederationProviderModel;
@@ -862,7 +863,9 @@ public class RealmAdapter extends AbstractMongoAdapter<MongoRealmEntity> impleme
         updateRealm();
 
         UserFederationProviderModel providerModel = new UserFederationProviderModel(entity.getId(), providerName, config, priority, displayName, fullSyncPeriod, changedSyncPeriod, lastSync);
+
         session.getKeycloakSessionFactory().publish(new UserFederationProviderCreationEventImpl(this, providerModel));
+
         return providerModel;
     }
 
@@ -962,6 +965,7 @@ public class RealmAdapter extends AbstractMongoAdapter<MongoRealmEntity> impleme
             entity.setChangedSyncPeriod(model.getChangedSyncPeriod());
             entity.setLastSync(model.getLastSync());
             entities.add(entity);
+
             session.getKeycloakSessionFactory().publish(new UserFederationProviderCreationEventImpl(this, model));
         }
 
@@ -1501,7 +1505,11 @@ public class RealmAdapter extends AbstractMongoAdapter<MongoRealmEntity> impleme
 
         getMongoEntity().getUserFederationMappers().add(entity);
         updateMongoEntity();
-        return entityToModel(entity);
+        UserFederationMapperModel mapperModel = entityToModel(entity);
+
+        session.getKeycloakSessionFactory().publish(new UserFederationMapperEventImpl(mapperModel, this, session));
+
+        return mapperModel;
     }
 
     protected UserFederationMapperEntity getUserFederationMapperEntity(String id) {
@@ -1555,6 +1563,8 @@ public class RealmAdapter extends AbstractMongoAdapter<MongoRealmEntity> impleme
             entity.getConfig().putAll(mapper.getConfig());
         }
         updateMongoEntity();
+
+        session.getKeycloakSessionFactory().publish(new UserFederationMapperEventImpl(mapper, this, session));
     }
 
     @Override

testsuite/integration/src/test/java/org/keycloak/testsuite/federation/FederationProvidersIntegrationTest.java

@@ -11,13 +11,10 @@ import org.junit.runners.MethodSorters;
 import org.keycloak.OAuth2Constants;
 import org.keycloak.federation.ldap.LDAPFederationProvider;
 import org.keycloak.federation.ldap.LDAPFederationProviderFactory;
-import org.keycloak.federation.ldap.LDAPUtils;
 import org.keycloak.federation.ldap.idm.model.LDAPObject;
 import org.keycloak.federation.ldap.mappers.FullNameLDAPFederationMapper;
 import org.keycloak.federation.ldap.mappers.FullNameLDAPFederationMapperFactory;
 import org.keycloak.federation.ldap.mappers.UserAttributeLDAPFederationMapper;
-import org.keycloak.federation.ldap.mappers.UserAttributeLDAPFederationMapperFactory;
-import org.keycloak.models.ClientModel;
 import org.keycloak.models.KeycloakSession;
 import org.keycloak.models.LDAPConstants;
 import org.keycloak.models.ModelReadOnlyException;
@@ -42,9 +39,7 @@ import org.keycloak.testsuite.rule.WebResource;
 import org.keycloak.testsuite.rule.WebRule;
 import org.openqa.selenium.WebDriver;
 
-import java.util.List;
 import java.util.Map;
-import java.util.Set;
 
 /**
  * @author <a href="mailto:mposolda@redhat.com">Marek Posolda</a>
@@ -71,7 +66,7 @@ public class FederationProvidersIntegrationTest {
 
             // Delete all LDAP users and add some new for testing
             LDAPFederationProvider ldapFedProvider = FederationTestUtils.getLdapProvider(session, ldapModel);
-            LDAPUtils.removeAllUsers(ldapFedProvider, appRealm);
+            FederationTestUtils.removeAllLDAPUsers(ldapFedProvider, appRealm);
 
             LDAPObject john = FederationTestUtils.addLDAPUser(ldapFedProvider, appRealm, "johnkeycloak", "John", "Doe", "john@email.org", "1234");
             ldapFedProvider.getLdapIdentityStore().updatePassword(john, "Password1");

testsuite/integration/src/test/java/org/keycloak/testsuite/federation/FederationTestUtils.java

@@ -1,10 +1,14 @@
 package org.keycloak.testsuite.federation;
 
+import java.util.List;
+
 import org.junit.Assert;
 import org.keycloak.federation.ldap.LDAPFederationProvider;
 import org.keycloak.federation.ldap.LDAPFederationProviderFactory;
 import org.keycloak.federation.ldap.LDAPUtils;
 import org.keycloak.federation.ldap.idm.model.LDAPObject;
+import org.keycloak.federation.ldap.idm.query.internal.LDAPIdentityQuery;
+import org.keycloak.federation.ldap.idm.store.ldap.LDAPIdentityStore;
 import org.keycloak.federation.ldap.mappers.RoleLDAPFederationMapper;
 import org.keycloak.federation.ldap.mappers.RoleLDAPFederationMapperFactory;
 import org.keycloak.federation.ldap.mappers.UserAttributeLDAPFederationMapper;
@@ -130,4 +134,30 @@ class FederationTestUtils {
             realm.addUserFederationMapper(mapperModel);
         }
     }
+
+    public static void removeAllLDAPUsers(LDAPFederationProvider ldapProvider, RealmModel realm) {
+        LDAPIdentityStore ldapStore = ldapProvider.getLdapIdentityStore();
+        LDAPIdentityQuery ldapQuery = LDAPUtils.createQueryForUserSearch(ldapProvider, realm);
+        List<LDAPObject> allUsers = ldapQuery.getResultList();
+
+        for (LDAPObject ldapUser : allUsers) {
+            ldapStore.remove(ldapUser);
+        }
+    }
+
+    public static void removeAllLDAPRoles(KeycloakSession session, RealmModel appRealm, UserFederationProviderModel ldapModel, String mapperName) {
+        UserFederationMapperModel mapperModel = appRealm.getUserFederationMapperByName(ldapModel.getId(), mapperName);
+        LDAPFederationProvider ldapProvider = FederationTestUtils.getLdapProvider(session, ldapModel);
+        LDAPIdentityQuery roleQuery = new RoleLDAPFederationMapper().createRoleQuery(mapperModel, ldapProvider);
+        List<LDAPObject> ldapRoles = roleQuery.getResultList();
+        for (LDAPObject ldapRole : ldapRoles) {
+            ldapProvider.getLdapIdentityStore().remove(ldapRole);
+        }
+    }
+
+    public static void createLDAPRole(KeycloakSession session, RealmModel appRealm, UserFederationProviderModel ldapModel, String mapperName, String roleName) {
+        UserFederationMapperModel mapperModel = appRealm.getUserFederationMapperByName(ldapModel.getId(), mapperName);
+        LDAPFederationProvider ldapProvider = FederationTestUtils.getLdapProvider(session, ldapModel);
+        new RoleLDAPFederationMapper().createLDAPRole(mapperModel, roleName, ldapProvider);
+    }
 }

testsuite/integration/src/test/java/org/keycloak/testsuite/federation/LDAPRoleMappingsTest.java

@@ -13,7 +13,6 @@ import org.junit.rules.TestRule;
 import org.junit.runners.MethodSorters;
 import org.keycloak.federation.ldap.LDAPFederationProvider;
 import org.keycloak.federation.ldap.LDAPFederationProviderFactory;
-import org.keycloak.federation.ldap.LDAPUtils;
 import org.keycloak.federation.ldap.idm.model.LDAPObject;
 import org.keycloak.federation.ldap.idm.query.Condition;
 import org.keycloak.federation.ldap.idm.query.QueryParameter;
@@ -34,11 +33,8 @@ import org.keycloak.models.UserFederationProviderModel;
 import org.keycloak.models.UserModel;
 import org.keycloak.services.managers.RealmManager;
 import org.keycloak.testsuite.OAuthClient;
-import org.keycloak.testsuite.pages.AccountPasswordPage;
-import org.keycloak.testsuite.pages.AccountUpdateProfilePage;
 import org.keycloak.testsuite.pages.AppPage;
 import org.keycloak.testsuite.pages.LoginPage;
-import org.keycloak.testsuite.pages.RegisterPage;
 import org.keycloak.testsuite.rule.KeycloakRule;
 import org.keycloak.testsuite.rule.LDAPRule;
 import org.keycloak.testsuite.rule.WebResource;
@@ -67,13 +63,19 @@ public class LDAPRoleMappingsTest {
 
             ldapModel = appRealm.addUserFederationProvider(LDAPFederationProviderFactory.PROVIDER_NAME, ldapConfig, 0, "test-ldap", -1, -1, 0);
 
-            // Delete all LDAP users and add some new for testing
+            // Delete all LDAP users
             LDAPFederationProvider ldapFedProvider = FederationTestUtils.getLdapProvider(session, ldapModel);
-            LDAPUtils.removeAllUsers(ldapFedProvider, appRealm);
+            FederationTestUtils.removeAllLDAPUsers(ldapFedProvider, appRealm);
+
+            // Delete all LDAP roles
+            FederationTestUtils.addOrUpdateRoleLDAPMappers(appRealm, ldapModel, RoleLDAPFederationMapper.Mode.LDAP_ONLY);
+            FederationTestUtils.removeAllLDAPRoles(manager.getSession(), appRealm, ldapModel, "realmRolesMapper");
+            FederationTestUtils.removeAllLDAPRoles(manager.getSession(), appRealm, ldapModel, "financeRolesMapper");
 
             // Add sample application
             ClientModel finance = appRealm.addClient("finance");
 
+            // Add some users for testing
             LDAPObject john = FederationTestUtils.addLDAPUser(ldapFedProvider, appRealm, "johnkeycloak", "John", "Doe", "john@email.org", "1234");
             ldapFedProvider.getLdapIdentityStore().updatePassword(john, "Password1");
 
@@ -83,34 +85,12 @@ public class LDAPRoleMappingsTest {
             LDAPObject rob = FederationTestUtils.addLDAPUser(ldapFedProvider, appRealm, "robkeycloak", "Rob", "Brown", "rob@email.org", "8910");
             ldapFedProvider.getLdapIdentityStore().updatePassword(rob, "Password1");
 
+            // Add some roles for testing
+            FederationTestUtils.createLDAPRole(manager.getSession(), appRealm, ldapModel, "realmRolesMapper", "realmRole1");
+            FederationTestUtils.createLDAPRole(manager.getSession(), appRealm, ldapModel, "realmRolesMapper", "realmRole2");
+            FederationTestUtils.createLDAPRole(manager.getSession(), appRealm, ldapModel, "financeRolesMapper", "financeRole1");
         }
-    }) {
+    });
-
-        @Override
-        protected void after() {
-            // Need to cleanup some LDAP objects after the test
-            update(new KeycloakRule.KeycloakSetup() {
-
-                @Override
-                public void config(RealmManager manager, RealmModel adminstrationRealm, RealmModel appRealm) {
-                    RoleLDAPFederationMapper roleMapper = new RoleLDAPFederationMapper();
-
-                    FederationTestUtils.addOrUpdateRoleLDAPMappers(appRealm, ldapModel, RoleLDAPFederationMapper.Mode.LDAP_ONLY);
-                    UserFederationMapperModel roleMapperModel = appRealm.getUserFederationMapperByName(ldapModel.getId(), "realmRolesMapper");
-                    LDAPFederationProvider ldapProvider = FederationTestUtils.getLdapProvider(session, ldapModel);
-
-                    LDAPObject ldapRole = roleMapper.loadLDAPRoleByName(roleMapperModel, ldapProvider, "realmRole3");
-                    if (ldapRole != null) {
-                        ldapProvider.getLdapIdentityStore().remove(ldapRole);
-                    }
-                }
-
-            });
-
-            super.after();
-        }
-
-    };
 
     @ClassRule
     public static TestRule chain = RuleChain
@@ -270,12 +250,20 @@ public class LDAPRoleMappingsTest {
             // Delete role mappings directly in LDAP
             deleteRoleMappingsInLDAP(roleMapperModel, roleMapper, ldapProvider, maryLdap, "realmRole1");
             deleteRoleMappingsInLDAP(roleMapperModel, roleMapper, ldapProvider, maryLdap, "realmRole2");
+        } finally {
+            keycloakRule.stopSession(session, false);
+        }
+
+        session = keycloakRule.startSession();
+        try {
+            RealmModel appRealm = session.realms().getRealmByName("test");
+            UserModel mary = session.users().getUserByUsername("marykeycloak", appRealm);
 
             // Assert role mappings is not available
-            maryRoles = mary.getRealmRoleMappings();
+            Set<RoleModel> maryRoles = mary.getRealmRoleMappings();
-            Assert.assertFalse(maryRoles.contains(realmRole1));
+            Assert.assertFalse(maryRoles.contains(appRealm.getRole("realmRole1")));
-            Assert.assertFalse(maryRoles.contains(realmRole2));
+            Assert.assertFalse(maryRoles.contains(appRealm.getRole("realmRole2")));
-            Assert.assertFalse(maryRoles.contains(realmRole3));
+            Assert.assertFalse(maryRoles.contains(appRealm.getRole("realmRole3")));
         } finally {
             keycloakRule.stopSession(session, false);
         }

testsuite/integration/src/test/java/org/keycloak/testsuite/federation/SyncProvidersTest.java

@@ -9,9 +9,7 @@ import org.junit.rules.TestRule;
 import org.junit.runners.MethodSorters;
 import org.keycloak.federation.ldap.LDAPFederationProvider;
 import org.keycloak.federation.ldap.LDAPFederationProviderFactory;
-import org.keycloak.federation.ldap.LDAPUtils;
 import org.keycloak.federation.ldap.idm.model.LDAPObject;
-import org.keycloak.models.ClientModel;
 import org.keycloak.models.KeycloakSession;
 import org.keycloak.models.KeycloakSessionFactory;
 import org.keycloak.models.LDAPConstants;
@@ -60,7 +58,7 @@ public class SyncProvidersTest {
 
             // Delete all LDAP users and add 5 new users for testing
             LDAPFederationProvider ldapFedProvider = FederationTestUtils.getLdapProvider(session, ldapModel);
-            LDAPUtils.removeAllUsers(ldapFedProvider, appRealm);
+            FederationTestUtils.removeAllLDAPUsers(ldapFedProvider, appRealm);
 
             for (int i=1 ; i<=5 ; i++) {
                 LDAPObject ldapUser = FederationTestUtils.addLDAPUser(ldapFedProvider, appRealm, "user" + i, "User" + i + "FN", "User" + i + "LN", "user" + i + "@email.org", "12" + i);

testsuite/integration/src/test/resources/ldap/users.ldif

@@ -14,26 +14,9 @@ objectclass: top
 objectclass: organizationalUnit
 ou: RealmRoles
 
-dn: cn=realmRole1,ou=RealmRoles,dc=keycloak,dc=org
-objectclass: top
-objectclass: groupOfNames
-cn: realmRole1
-member:
-
-dn: cn=realmRole2,ou=RealmRoles,dc=keycloak,dc=org
-objectclass: top
-objectclass: groupOfNames
-cn: realmRole2
-member:
-
 dn: ou=FinanceRoles,dc=keycloak,dc=org
 objectclass: top
 objectclass: organizationalUnit
 ou: FinanceRoles
 
-dn: cn=financeRole1,ou=FinanceRoles,dc=keycloak,dc=org
-objectclass: top
-objectclass: groupOfNames
-cn: financeRole1
-member: