keycloak-uncached

Details

diff --git a/federation/ldap/src/main/java/org/keycloak/federation/ldap/idm/query/internal/LDAPQuery.java b/federation/ldap/src/main/java/org/keycloak/federation/ldap/idm/query/internal/LDAPQuery.java
index dea173f..957ba33 100644
--- a/federation/ldap/src/main/java/org/keycloak/federation/ldap/idm/query/internal/LDAPQuery.java
+++ b/federation/ldap/src/main/java/org/keycloak/federation/ldap/idm/query/internal/LDAPQuery.java
@@ -153,7 +153,8 @@ public class LDAPQuery {
     public List<LDAPObject> getResultList() {
 
         // Apply mappers now
-        for (UserFederationMapperModel mapperModel : mappers) {
+        List<UserFederationMapperModel> sortedMappers = ldapFedProvider.sortMappersAsc(mappers);
+        for (UserFederationMapperModel mapperModel : sortedMappers) {
             LDAPFederationMapper fedMapper = ldapFedProvider.getMapper(mapperModel);
             fedMapper.beforeLDAPQuery(mapperModel, this);
         }
diff --git a/federation/ldap/src/main/java/org/keycloak/federation/ldap/LDAPFederationProvider.java b/federation/ldap/src/main/java/org/keycloak/federation/ldap/LDAPFederationProvider.java
index e800bc3..010870e 100755
--- a/federation/ldap/src/main/java/org/keycloak/federation/ldap/LDAPFederationProvider.java
+++ b/federation/ldap/src/main/java/org/keycloak/federation/ldap/LDAPFederationProvider.java
@@ -27,6 +27,7 @@ import org.keycloak.federation.ldap.idm.query.internal.LDAPQueryConditionsBuilde
 import org.keycloak.federation.ldap.idm.store.ldap.LDAPIdentityStore;
 import org.keycloak.federation.ldap.kerberos.LDAPProviderKerberosConfig;
 import org.keycloak.federation.ldap.mappers.LDAPFederationMapper;
+import org.keycloak.federation.ldap.mappers.LDAPMappersComparator;
 import org.keycloak.models.CredentialValidationOutput;
 import org.keycloak.models.GroupModel;
 import org.keycloak.models.KeycloakSession;
@@ -47,6 +48,7 @@ import org.keycloak.services.managers.UserManager;
 
 import java.util.ArrayList;
 import java.util.Arrays;
+import java.util.Collection;
 import java.util.Collections;
 import java.util.HashMap;
 import java.util.HashSet;
@@ -128,7 +130,8 @@ public class LDAPFederationProvider implements UserFederationProvider {
         }
 
         Set<UserFederationMapperModel> federationMappers = realm.getUserFederationMappersByFederationProvider(model.getId());
-        for (UserFederationMapperModel mapperModel : federationMappers) {
+        List<UserFederationMapperModel> sortedMappers = sortMappersAsc(federationMappers);
+        for (UserFederationMapperModel mapperModel : sortedMappers) {
             LDAPFederationMapper ldapMapper = getMapper(mapperModel);
             proxied = ldapMapper.proxy(mapperModel, this, ldapObject, proxied, realm);
         }
@@ -313,7 +316,8 @@ public class LDAPFederationProvider implements UserFederationProvider {
         imported.setEnabled(true);
 
         Set<UserFederationMapperModel> federationMappers = realm.getUserFederationMappersByFederationProvider(getModel().getId());
-        for (UserFederationMapperModel mapperModel : federationMappers) {
+        List<UserFederationMapperModel> sortedMappers = sortMappersDesc(federationMappers);
+        for (UserFederationMapperModel mapperModel : sortedMappers) {
             if (logger.isTraceEnabled()) {
                 logger.tracef("Using mapper %s during import user from LDAP", mapperModel);
             }
@@ -517,4 +521,13 @@ public class LDAPFederationProvider implements UserFederationProvider {
 
         return ldapMapper;
     }
+
+
+    public List<UserFederationMapperModel> sortMappersAsc(Collection<UserFederationMapperModel> mappers) {
+        return LDAPMappersComparator.sortAsc(getLdapIdentityStore().getConfig(), mappers);
+    }
+
+    protected List<UserFederationMapperModel> sortMappersDesc(Collection<UserFederationMapperModel> mappers) {
+        return LDAPMappersComparator.sortDesc(getLdapIdentityStore().getConfig(), mappers);
+    }
 }
diff --git a/federation/ldap/src/main/java/org/keycloak/federation/ldap/LDAPFederationProviderFactory.java b/federation/ldap/src/main/java/org/keycloak/federation/ldap/LDAPFederationProviderFactory.java
index f6e4026..bfc7fc2 100755
--- a/federation/ldap/src/main/java/org/keycloak/federation/ldap/LDAPFederationProviderFactory.java
+++ b/federation/ldap/src/main/java/org/keycloak/federation/ldap/LDAPFederationProviderFactory.java
@@ -354,7 +354,8 @@ public class LDAPFederationProviderFactory extends UserFederationEventAwareProvi
 
                                 // Update keycloak user
                                 Set<UserFederationMapperModel> federationMappers = currentRealm.getUserFederationMappersByFederationProvider(fedModel.getId());
-                                for (UserFederationMapperModel mapperModel : federationMappers) {
+                                List<UserFederationMapperModel> sortedMappers = ldapFedProvider.sortMappersDesc(federationMappers);
+                                for (UserFederationMapperModel mapperModel : sortedMappers) {
                                     LDAPFederationMapper ldapMapper = ldapFedProvider.getMapper(mapperModel);
                                     ldapMapper.onImportUserFromLDAP(mapperModel, ldapFedProvider, ldapUser, currentUser, currentRealm, false);
                                 }
diff --git a/federation/ldap/src/main/java/org/keycloak/federation/ldap/LDAPUtils.java b/federation/ldap/src/main/java/org/keycloak/federation/ldap/LDAPUtils.java
index a373625..4d64e2c 100755
--- a/federation/ldap/src/main/java/org/keycloak/federation/ldap/LDAPUtils.java
+++ b/federation/ldap/src/main/java/org/keycloak/federation/ldap/LDAPUtils.java
@@ -19,6 +19,7 @@ package org.keycloak.federation.ldap;
 
 import java.util.Collection;
 import java.util.HashSet;
+import java.util.List;
 import java.util.Map;
 import java.util.Set;
 
@@ -58,7 +59,8 @@ public class LDAPUtils {
         ldapUser.setObjectClasses(ldapConfig.getUserObjectClasses());
 
         Set<UserFederationMapperModel> federationMappers = realm.getUserFederationMappersByFederationProvider(ldapProvider.getModel().getId());
-        for (UserFederationMapperModel mapperModel : federationMappers) {
+        List<UserFederationMapperModel> sortedMappers = ldapProvider.sortMappersAsc(federationMappers);
+        for (UserFederationMapperModel mapperModel : sortedMappers) {
             LDAPFederationMapper ldapMapper = ldapProvider.getMapper(mapperModel);
             ldapMapper.onRegisterUserToLDAP(mapperModel, ldapProvider, ldapUser, user, realm);
         }
diff --git a/federation/ldap/src/main/java/org/keycloak/federation/ldap/mappers/LDAPMappersComparator.java b/federation/ldap/src/main/java/org/keycloak/federation/ldap/mappers/LDAPMappersComparator.java
new file mode 100644
index 0000000..c5507e6
--- /dev/null
+++ b/federation/ldap/src/main/java/org/keycloak/federation/ldap/mappers/LDAPMappersComparator.java
@@ -0,0 +1,114 @@
+/*
+ * 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.federation.ldap.mappers;
+
+import java.util.ArrayList;
+import java.util.Collection;
+import java.util.Collections;
+import java.util.Comparator;
+import java.util.List;
+
+import org.keycloak.federation.ldap.LDAPConfig;
+import org.keycloak.models.UserFederationMapperModel;
+import org.keycloak.models.UserModel;
+
+/**
+ * TODO: Possibly add "priority" to UserFederationMapper instead of hardcoding behaviour
+ *
+ * @author <a href="mailto:mposolda@redhat.com">Marek Posolda</a>
+ */
+public class LDAPMappersComparator {
+
+    public static List<UserFederationMapperModel> sortAsc(LDAPConfig ldapConfig, Collection<UserFederationMapperModel> mappers) {
+        Comparator<UserFederationMapperModel> comparator = new ImportantFirstComparator(ldapConfig);
+
+        List<UserFederationMapperModel> result = new ArrayList<>(mappers);
+        Collections.sort(result, comparator);
+        return result;
+    }
+
+    public static List<UserFederationMapperModel> sortDesc(LDAPConfig ldapConfig, Collection<UserFederationMapperModel> mappers) {
+        Comparator<UserFederationMapperModel> comparator = new ImportantFirstComparator(ldapConfig).reversed();
+
+        List<UserFederationMapperModel> result = new ArrayList<>(mappers);
+        Collections.sort(result, comparator);
+        return result;
+    }
+
+
+    private static class ImportantFirstComparator implements Comparator<UserFederationMapperModel> {
+
+        private final LDAPConfig ldapConfig;
+
+        public ImportantFirstComparator(LDAPConfig ldapConfig) {
+            this.ldapConfig = ldapConfig;
+        }
+
+        @Override
+        public int compare(UserFederationMapperModel o1, UserFederationMapperModel o2) {
+            // UserAttributeLDAPFederationMapper first
+            boolean isO1AttrMapper = o1.getFederationMapperType().equals(UserAttributeLDAPFederationMapperFactory.PROVIDER_ID);
+            boolean isO2AttrMapper = o2.getFederationMapperType().equals(UserAttributeLDAPFederationMapperFactory.PROVIDER_ID);
+            if (!isO1AttrMapper) {
+                if (isO2AttrMapper) {
+                    return 1;
+                } else {
+                    return 0;
+                }
+            } else if (!isO2AttrMapper) {
+                return -1;
+            }
+
+            // Mapper for "username" attribute first
+            String model1 = o1.getConfig().get(UserAttributeLDAPFederationMapper.USER_MODEL_ATTRIBUTE);
+            String model2 = o2.getConfig().get(UserAttributeLDAPFederationMapper.USER_MODEL_ATTRIBUTE);
+            boolean isO1UsernameMapper = model1 != null && model1.equalsIgnoreCase(UserModel.USERNAME);
+            boolean isO2UsernameMapper = model2 != null && model2.equalsIgnoreCase(UserModel.USERNAME);
+            if (!isO1UsernameMapper) {
+                if (isO2UsernameMapper) {
+                    return 1;
+                } else {
+                    return 0;
+                }
+            } else if (!isO2UsernameMapper) {
+                return -1;
+            }
+
+            // The username mapper corresponding to the same like configured username for federationProvider is first
+            String o1LdapAttr = o1.getConfig().get(UserAttributeLDAPFederationMapper.LDAP_ATTRIBUTE);
+            String o2LdapAttr = o2.getConfig().get(UserAttributeLDAPFederationMapper.LDAP_ATTRIBUTE);
+            boolean isO1LdapAttr = o1LdapAttr != null && ldapConfig.getUsernameLdapAttribute().equalsIgnoreCase(o1LdapAttr);
+            boolean isO2LdapAttr = o2LdapAttr != null && ldapConfig.getUsernameLdapAttribute().equalsIgnoreCase(o2LdapAttr);
+
+            if (!isO1LdapAttr) {
+                if (isO2LdapAttr) {
+                    return 1;
+                } else {
+                    return 0;
+                }
+            } else if (!isO2LdapAttr) {
+                return -1;
+            }
+
+            return 0;
+        }
+
+    }
+
+
+}
diff --git a/federation/ldap/src/test/java/org/keycloak/federation/ldap/idm/model/LDAPMappersComparatorTest.java b/federation/ldap/src/test/java/org/keycloak/federation/ldap/idm/model/LDAPMappersComparatorTest.java
new file mode 100644
index 0000000..7ca7ff2
--- /dev/null
+++ b/federation/ldap/src/test/java/org/keycloak/federation/ldap/idm/model/LDAPMappersComparatorTest.java
@@ -0,0 +1,117 @@
+/*
+ * 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.federation.ldap.idm.model;
+
+import java.util.HashMap;
+import java.util.HashSet;
+import java.util.List;
+import java.util.Map;
+import java.util.Set;
+
+import org.junit.Assert;
+import org.junit.Test;
+import org.keycloak.federation.ldap.LDAPConfig;
+import org.keycloak.federation.ldap.mappers.FullNameLDAPFederationMapper;
+import org.keycloak.federation.ldap.mappers.FullNameLDAPFederationMapperFactory;
+import org.keycloak.federation.ldap.mappers.LDAPMappersComparator;
+import org.keycloak.federation.ldap.mappers.UserAttributeLDAPFederationMapper;
+import org.keycloak.federation.ldap.mappers.UserAttributeLDAPFederationMapperFactory;
+import org.keycloak.models.LDAPConstants;
+import org.keycloak.models.UserFederationMapperModel;
+import org.keycloak.models.UserModel;
+import org.keycloak.models.utils.KeycloakModelUtils;
+
+/**
+ * @author <a href="mailto:mposolda@redhat.com">Marek Posolda</a>
+ */
+public class LDAPMappersComparatorTest {
+
+
+
+    @Test
+    public void testCompareWithCNUsername() {
+        Map<String, String> cfg = new HashMap<>();
+        cfg.put(LDAPConstants.USERNAME_LDAP_ATTRIBUTE, LDAPConstants.CN);
+        LDAPConfig config = new LDAPConfig(cfg);
+
+        List<UserFederationMapperModel> sorted = LDAPMappersComparator.sortAsc(config, getMappers());
+        assertOrder(sorted, "username-cn", "sAMAccountName", "first name", "full name");
+
+        sorted = LDAPMappersComparator.sortDesc(config, getMappers());
+        assertOrder(sorted, "full name", "first name", "sAMAccountName", "username-cn");
+    }
+
+    @Test
+    public void testCompareWithSAMAccountNameUsername() {
+        Map<String, String> cfg = new HashMap<>();
+        cfg.put(LDAPConstants.USERNAME_LDAP_ATTRIBUTE, LDAPConstants.SAM_ACCOUNT_NAME);
+        LDAPConfig config = new LDAPConfig(cfg);
+
+        List<UserFederationMapperModel> sorted = LDAPMappersComparator.sortAsc(config, getMappers());
+        assertOrder(sorted, "sAMAccountName", "username-cn", "first name", "full name");
+
+        sorted = LDAPMappersComparator.sortDesc(config, getMappers());
+        assertOrder(sorted, "full name", "first name", "username-cn", "sAMAccountName");
+    }
+
+    private void assertOrder(List<UserFederationMapperModel> result, String... names) {
+        Assert.assertEquals(result.size(), names.length);
+        for (int i=0 ; i<names.length ; i++) {
+            Assert.assertEquals(names[i], result.get(i).getName());
+        }
+    }
+
+    private Set<UserFederationMapperModel> getMappers() {
+        Set<UserFederationMapperModel> result = new HashSet<>();
+
+        UserFederationMapperModel mapperModel = KeycloakModelUtils.createUserFederationMapperModel("first name",  "fed-provider", UserAttributeLDAPFederationMapperFactory.PROVIDER_ID,
+                UserAttributeLDAPFederationMapper.USER_MODEL_ATTRIBUTE, UserModel.FIRST_NAME,
+                UserAttributeLDAPFederationMapper.LDAP_ATTRIBUTE, LDAPConstants.GIVENNAME,
+                UserAttributeLDAPFederationMapper.READ_ONLY, "true",
+                UserAttributeLDAPFederationMapper.ALWAYS_READ_VALUE_FROM_LDAP, "true",
+                UserAttributeLDAPFederationMapper.IS_MANDATORY_IN_LDAP, "true");
+        mapperModel.setId("idd1");
+        result.add(mapperModel);
+
+        mapperModel = KeycloakModelUtils.createUserFederationMapperModel("username-cn", "fed-provider", UserAttributeLDAPFederationMapperFactory.PROVIDER_ID,
+                UserAttributeLDAPFederationMapper.USER_MODEL_ATTRIBUTE, UserModel.USERNAME,
+                UserAttributeLDAPFederationMapper.LDAP_ATTRIBUTE, LDAPConstants.CN,
+                UserAttributeLDAPFederationMapper.READ_ONLY, "true",
+                UserAttributeLDAPFederationMapper.ALWAYS_READ_VALUE_FROM_LDAP, "false",
+                UserAttributeLDAPFederationMapper.IS_MANDATORY_IN_LDAP, "true");
+        mapperModel.setId("idd2");
+        result.add(mapperModel);
+
+        mapperModel = KeycloakModelUtils.createUserFederationMapperModel("full name", "fed-provider", FullNameLDAPFederationMapperFactory.PROVIDER_ID,
+                FullNameLDAPFederationMapper.LDAP_FULL_NAME_ATTRIBUTE, LDAPConstants.CN,
+                UserAttributeLDAPFederationMapper.READ_ONLY, "true");
+        mapperModel.setId("idd3");
+        result.add(mapperModel);
+
+        mapperModel = KeycloakModelUtils.createUserFederationMapperModel("sAMAccountName", "fed-provider", UserAttributeLDAPFederationMapperFactory.PROVIDER_ID,
+                UserAttributeLDAPFederationMapper.USER_MODEL_ATTRIBUTE, UserModel.USERNAME,
+                UserAttributeLDAPFederationMapper.LDAP_ATTRIBUTE, LDAPConstants.SAM_ACCOUNT_NAME,
+                UserAttributeLDAPFederationMapper.READ_ONLY, "false",
+                UserAttributeLDAPFederationMapper.ALWAYS_READ_VALUE_FROM_LDAP, "false",
+                UserAttributeLDAPFederationMapper.IS_MANDATORY_IN_LDAP, "true");
+        mapperModel.setId("idd4");
+        result.add(mapperModel);
+
+        return result;
+    }
+}
diff --git a/server-spi/src/main/java/org/keycloak/models/UserFederationSyncResult.java b/server-spi/src/main/java/org/keycloak/models/UserFederationSyncResult.java
index a9a32f9..380c654 100644
--- a/server-spi/src/main/java/org/keycloak/models/UserFederationSyncResult.java
+++ b/server-spi/src/main/java/org/keycloak/models/UserFederationSyncResult.java
@@ -96,7 +96,10 @@ public class UserFederationSyncResult {
         if (ignored) {
             return "Synchronization ignored as it's already in progress";
         } else {
-            String status = String.format("%d imported users, %d updated users, %d removed users", added, updated, removed);
+            String status = String.format("%d imported users, %d updated users", added, updated);
+            if (removed > 0) {
+                status += String.format(", %d removed users", removed);
+            }
             if (failed != 0) {
                 status += String.format(", %d users failed sync! See server log for more details", failed);
             }