keycloak-aplcache

Merge pull request #1323 from mposolda/ldap Ldap: testsuite

6/3/2015 10:36:29 AM

Details

diff --git a/federation/ldap/src/main/java/org/keycloak/federation/ldap/LDAPConfig.java b/federation/ldap/src/main/java/org/keycloak/federation/ldap/LDAPConfig.java
index 983de98..4ebde77 100644
--- a/federation/ldap/src/main/java/org/keycloak/federation/ldap/LDAPConfig.java
+++ b/federation/ldap/src/main/java/org/keycloak/federation/ldap/LDAPConfig.java
@@ -1,8 +1,6 @@
 package org.keycloak.federation.ldap;
 
-import java.util.Arrays;
 import java.util.Collection;
-import java.util.Collections;
 import java.util.HashSet;
 import java.util.Map;
 import java.util.Properties;
diff --git a/federation/ldap/src/main/java/org/keycloak/federation/ldap/mappers/RoleLDAPFederationMapper.java b/federation/ldap/src/main/java/org/keycloak/federation/ldap/mappers/RoleLDAPFederationMapper.java
index 763ca3c..83e8d66 100644
--- a/federation/ldap/src/main/java/org/keycloak/federation/ldap/mappers/RoleLDAPFederationMapper.java
+++ b/federation/ldap/src/main/java/org/keycloak/federation/ldap/mappers/RoleLDAPFederationMapper.java
@@ -108,7 +108,6 @@ public class RoleLDAPFederationMapper extends AbstractLDAPFederationMapper {
                 String roleName = ldapRole.getAttributeAsString(rolesRdnAttr);
 
                 if (roleContainer.getRole(roleName) == null) {
-                    // TODO: rather change to debug
                     logger.infof("Syncing role [%s] from LDAP to keycloak DB", roleName);
                     roleContainer.addRole(roleName);
                 }
@@ -210,7 +209,6 @@ public class RoleLDAPFederationMapper extends AbstractLDAPFederationMapper {
         roleDn.addFirst(roleNameAttribute, roleName);
         ldapObject.setDn(roleDn);
 
-        // TODO: debug
         logger.infof("Creating role to [%s] to LDAP with DN [%s]", roleName, roleDn.toString());
         ldapProvider.getLdapIdentityStore().add(ldapObject);
         return ldapObject;
diff --git a/model/api/src/main/java/org/keycloak/models/LDAPConstants.java b/model/api/src/main/java/org/keycloak/models/LDAPConstants.java
index c0c24a8..3b9de4a 100644
--- a/model/api/src/main/java/org/keycloak/models/LDAPConstants.java
+++ b/model/api/src/main/java/org/keycloak/models/LDAPConstants.java
@@ -19,6 +19,7 @@ public class LDAPConstants {
 
     public static final String CONNECTION_URL = "connectionUrl";
     public static final String SECURITY_PROTOCOL = "securityProtocol";
+    public static final String BASE_DN = "baseDn"; // used for tests only
     public static final String USERS_DN = "usersDn";
     public static final String BIND_DN = "bindDn";
     public static final String BIND_CREDENTIAL = "bindCredential";
diff --git a/model/api/src/main/java/org/keycloak/models/UserFederationManager.java b/model/api/src/main/java/org/keycloak/models/UserFederationManager.java
index bd07b77..93045c1 100755
--- a/model/api/src/main/java/org/keycloak/models/UserFederationManager.java
+++ b/model/api/src/main/java/org/keycloak/models/UserFederationManager.java
@@ -20,6 +20,9 @@ public class UserFederationManager implements UserProvider {
 
     protected KeycloakSession session;
 
+    // Set of already validated/proxied users during this session. Key is user ID
+    private Map<String, UserModel> managedUsers = new HashMap<>();
+
     public UserFederationManager(KeycloakSession session) {
         this.session = session;
     }
@@ -47,7 +50,9 @@ public class UserFederationManager implements UserProvider {
             UserFederationProvider fed = getFederationProvider(federation);
             if (fed.synchronizeRegistrations()) {
                 user.setFederationLink(federation.getId());
-                return fed.register(realm, user);
+                UserModel registered = fed.register(realm, user);
+                managedUsers.put(registered.getId(), registered);
+                return registered;
             }
         }
         return user;
@@ -70,6 +75,7 @@ public class UserFederationManager implements UserProvider {
             boolean fedRemoved = link.removeUser(realm, user);
             if (fedRemoved) {
                 boolean localRemoved = session.userStorage().removeUser(realm, user);
+                managedUsers.remove(user.getId());
                 if (!localRemoved) {
                     logger.warn("User removed from federation provider, but failed to remove him from keycloak model");
                 }
@@ -84,6 +90,10 @@ public class UserFederationManager implements UserProvider {
     }
 
     protected void validateUser(RealmModel realm, UserModel user) {
+        if (managedUsers.containsKey(user.getId())) {
+            return;
+        }
+
         UserFederationProvider link = getFederationLink(realm, user);
         if (link != null  && !link.isValid(realm, user)) {
             deleteInvalidUser(realm, user);
@@ -100,7 +110,7 @@ public class UserFederationManager implements UserProvider {
             if (realmModel == null) return;
             UserModel deletedUser = tx.userStorage().getUserById(user.getId(), realmModel);
             tx.userStorage().removeUser(realmModel, deletedUser);
-            logger.debugf("Removed invalid user '%s'", user.getUsername());
+            logger.infof("Removed invalid user '%s'", user.getUsername());
             tx.getTransaction().commit();
         } finally {
             tx.close();
@@ -109,10 +119,16 @@ public class UserFederationManager implements UserProvider {
 
 
     protected UserModel validateAndProxyUser(RealmModel realm, UserModel user) {
+        UserModel managed = managedUsers.get(user.getId());
+        if (managed != null) {
+            return managed;
+        }
+
         UserFederationProvider link = getFederationLink(realm, user);
         if (link != null) {
             UserModel validatedProxyUser = link.validateAndProxy(realm, user);
             if (validatedProxyUser != null) {
+                managedUsers.put(user.getId(), user);
                 return validatedProxyUser;
             } else {
                 deleteInvalidUser(realm, user);
diff --git a/testsuite/integration/src/test/java/org/keycloak/testsuite/federation/FederationTestUtils.java b/testsuite/integration/src/test/java/org/keycloak/testsuite/federation/FederationTestUtils.java
index 80bf539..0207b0a 100644
--- a/testsuite/integration/src/test/java/org/keycloak/testsuite/federation/FederationTestUtils.java
+++ b/testsuite/integration/src/test/java/org/keycloak/testsuite/federation/FederationTestUtils.java
@@ -108,8 +108,9 @@ class FederationTestUtils {
             mapperModel.getConfig().put(RoleLDAPFederationMapper.MODE, mode.toString());
             realm.updateUserFederationMapper(mapperModel);
         } else {
+            String baseDn = providerModel.getConfig().get(LDAPConstants.BASE_DN);
             mapperModel = KeycloakModelUtils.createUserFederationMapperModel("realmRolesMapper", providerModel.getId(), RoleLDAPFederationMapperFactory.PROVIDER_ID,
-                    RoleLDAPFederationMapper.ROLES_DN, "ou=RealmRoles,dc=keycloak,dc=org",
+                    RoleLDAPFederationMapper.ROLES_DN, "ou=RealmRoles," + baseDn,
                     RoleLDAPFederationMapper.USE_REALM_ROLES_MAPPING, "true",
                     RoleLDAPFederationMapper.MODE, mode.toString());
             realm.addUserFederationMapper(mapperModel);
@@ -120,8 +121,9 @@ class FederationTestUtils {
             mapperModel.getConfig().put(RoleLDAPFederationMapper.MODE, mode.toString());
             realm.updateUserFederationMapper(mapperModel);
         } else {
+            String baseDn = providerModel.getConfig().get(LDAPConstants.BASE_DN);
             mapperModel = KeycloakModelUtils.createUserFederationMapperModel("financeRolesMapper", providerModel.getId(), RoleLDAPFederationMapperFactory.PROVIDER_ID,
-                    RoleLDAPFederationMapper.ROLES_DN, "ou=FinanceRoles,dc=keycloak,dc=org",
+                    RoleLDAPFederationMapper.ROLES_DN, "ou=FinanceRoles," + baseDn,
                     RoleLDAPFederationMapper.USE_REALM_ROLES_MAPPING, "false",
                     RoleLDAPFederationMapper.CLIENT_ID, "finance",
                     RoleLDAPFederationMapper.MODE, mode.toString());
diff --git a/testsuite/integration/src/test/java/org/keycloak/testsuite/ldap/LDAPTestConfiguration.java b/testsuite/integration/src/test/java/org/keycloak/testsuite/ldap/LDAPTestConfiguration.java
index 8b96bc7..078a09e 100644
--- a/testsuite/integration/src/test/java/org/keycloak/testsuite/ldap/LDAPTestConfiguration.java
+++ b/testsuite/integration/src/test/java/org/keycloak/testsuite/ldap/LDAPTestConfiguration.java
@@ -28,8 +28,7 @@ public class LDAPTestConfiguration {
 
     static {
         PROP_MAPPINGS.put(LDAPConstants.CONNECTION_URL, "idm.test.ldap.connection.url");
-        PROP_MAPPINGS.put("rolesDnSuffix", "idm.test.ldap.roles.dn.suffix");
-        PROP_MAPPINGS.put("groupDnSuffix", "idm.test.ldap.group.dn.suffix");
+        PROP_MAPPINGS.put(LDAPConstants.BASE_DN, "idm.test.ldap.base.dn");
         PROP_MAPPINGS.put(LDAPConstants.USERS_DN, "idm.test.ldap.user.dn.suffix");
         PROP_MAPPINGS.put(LDAPConstants.BIND_DN, "idm.test.ldap.bind.dn");
         PROP_MAPPINGS.put(LDAPConstants.BIND_CREDENTIAL, "idm.test.ldap.bind.credential");
@@ -52,8 +51,7 @@ public class LDAPTestConfiguration {
         PROP_MAPPINGS.put(KerberosConstants.USE_KERBEROS_FOR_PASSWORD_AUTHENTICATION, "idm.test.kerberos.use.kerberos.for.password.authentication");
 
         DEFAULT_VALUES.put(LDAPConstants.CONNECTION_URL, "ldap://localhost:10389");
-        DEFAULT_VALUES.put("rolesDnSuffix", "ou=Roles,dc=keycloak,dc=org");
-        DEFAULT_VALUES.put("groupDnSuffix", "ou=Groups,dc=keycloak,dc=org");
+        DEFAULT_VALUES.put(LDAPConstants.BASE_DN, "dc=keycloak,dc=org");
         DEFAULT_VALUES.put(LDAPConstants.USERS_DN, "ou=People,dc=keycloak,dc=org");
         DEFAULT_VALUES.put(LDAPConstants.BIND_DN, "uid=admin,ou=system");
         DEFAULT_VALUES.put(LDAPConstants.BIND_CREDENTIAL, "secret");
diff --git a/testsuite/integration/src/test/resources/ldap/ldap-connection.properties b/testsuite/integration/src/test/resources/ldap/ldap-connection.properties
index c759f4a..c279f49 100644
--- a/testsuite/integration/src/test/resources/ldap/ldap-connection.properties
+++ b/testsuite/integration/src/test/resources/ldap/ldap-connection.properties
@@ -1,7 +1,5 @@
 idm.test.ldap.connection.url=ldap\://localhost\:10389
 idm.test.ldap.base.dn=dc\=keycloak,dc\=org
-idm.test.ldap.roles.dn.suffix=ou\=Roles,dc\=keycloak,dc\=org
-idm.test.ldap.group.dn.suffix=ou\=Groups,dc\=keycloak,dc\=org
 idm.test.ldap.user.dn.suffix=ou\=People,dc\=keycloak,dc\=org
 idm.test.ldap.start.embedded.ldap.server=true
 idm.test.ldap.bind.dn=uid\=admin,ou\=system