Merge pull request #3783 from mposolda/msad-ldap

KEYCLOAK-4269

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

@@ -20,11 +20,15 @@ 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.KeycloakSession;
+import org.keycloak.models.RealmModel;
 import org.keycloak.storage.ldap.idm.store.ldap.LDAPIdentityStore;
 import org.keycloak.storage.ldap.mappers.LDAPConfigDecorator;
 
+import java.util.List;
 import java.util.Map;
 import java.util.concurrent.ConcurrentHashMap;
+import java.util.stream.Collectors;
 
 /**
  * @author <a href="mailto:mposolda@redhat.com">Marek Posolda</a>
@@ -35,7 +39,7 @@ public class LDAPIdentityStoreRegistry {
 
     private Map<String, LDAPIdentityStoreContext> ldapStores = new ConcurrentHashMap<>();
 
-    public LDAPIdentityStore getLdapStore(ComponentModel ldapModel, Map<ComponentModel, LDAPConfigDecorator> configDecorators) {
+    public LDAPIdentityStore getLdapStore(KeycloakSession session, 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
@@ -49,7 +53,7 @@ public class LDAPIdentityStoreRegistry {
         }
 
         if (context == null || !ldapConfig.equals(context.config)) {
-            logLDAPConfig(ldapModel.getName(), ldapConfig);
+            logLDAPConfig(session, ldapModel, ldapConfig);
 
             LDAPIdentityStore store = createLdapIdentityStore(ldapConfig);
             context = new LDAPIdentityStoreContext(ldapConfig, store);
@@ -59,8 +63,18 @@ public class LDAPIdentityStoreRegistry {
     }
 
     // Don't log LDAP password
-    private void logLDAPConfig(String fedProviderDisplayName, LDAPConfig ldapConfig) {
+    private void logLDAPConfig(KeycloakSession session, ComponentModel ldapModel, LDAPConfig ldapConfig) {
-        logger.infof("Creating new LDAP Store for the LDAP storage provider: '%s', LDAP Configuration: %s", fedProviderDisplayName, ldapConfig.toString());
+        logger.infof("Creating new LDAP Store for the LDAP storage provider: '%s', LDAP Configuration: %s", ldapModel.getName(), ldapConfig.toString());
+
+        if (logger.isDebugEnabled()) {
+            RealmModel realm = session.realms().getRealm(ldapModel.getParentId());
+            List<ComponentModel> mappers = realm.getComponents(ldapModel.getId());
+            mappers.stream().forEach((ComponentModel c) -> {
+
+                logger.debugf("Mapper for provider: %s, Mapper name: %s, Provider: %s, Mapper configuration: %s", ldapModel.getName(), c.getName(), c.getProviderId(), c.getConfig().toString());
+
+            });
+        }
     }
 
     /**

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

@@ -192,7 +192,7 @@ public class LDAPStorageProviderFactory implements UserStorageProviderFactory<LD
     public LDAPStorageProvider create(KeycloakSession session, ComponentModel model) {
         Map<ComponentModel, LDAPConfigDecorator> configDecorators = getLDAPConfigDecorators(session, model);
 
-        LDAPIdentityStore ldapIdentityStore = this.ldapStoreRegistry.getLdapStore(model, configDecorators);
+        LDAPIdentityStore ldapIdentityStore = this.ldapStoreRegistry.getLdapStore(session, model, configDecorators);
         return new LDAPStorageProvider(this, session, model, ldapIdentityStore);
     }
 

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

@@ -454,6 +454,11 @@ public class LDAPOperationManager {
                         values = "No values";
                     }
 
+                    String attrName = item.getAttribute().getID().toUpperCase();
+                    if (attrName.contains("PASSWORD") || attrName.contains("UNICODEPWD")) {
+                        values = "********************";
+                    }
+
                     logger.tracef("  Op [%s]: %s = %s", item.getModificationOp(), item.getAttribute().getID(), values);
                 }
 
@@ -600,7 +605,11 @@ public class LDAPOperationManager {
         }
 
         if (logger.isDebugEnabled()) {
-            logger.debugf("Creating LdapContext using properties: [%s]", env);
+            Map<String, Object> copyEnv = new HashMap<>(env);
+            if (copyEnv.containsKey(Context.SECURITY_CREDENTIALS)) {
+                copyEnv.put(Context.SECURITY_CREDENTIALS, "**************************************");
+            }
+            logger.debugf("Creating LdapContext using properties: [%s]", copyEnv);
         }
 
         return env;

federation/ldap/src/main/java/org/keycloak/storage/ldap/mappers/msad/MSADUserAccountControlStorageMapper.java

@@ -143,11 +143,15 @@ public class MSADUserAccountControlStorageMapper extends AbstractLDAPStorageMapp
         if (ldapProvider.getEditMode() == UserStorageProvider.EditMode.WRITABLE) {
             if (errorCode.equals("532") || errorCode.equals("773")) {
                 // User needs to change his MSAD password. Allow him to login, but add UPDATE_PASSWORD required action
-                user.addRequiredAction(UserModel.RequiredAction.UPDATE_PASSWORD);
+                if (!user.getRequiredActions().contains(UserModel.RequiredAction.UPDATE_PASSWORD.name())) {
+                    user.addRequiredAction(UserModel.RequiredAction.UPDATE_PASSWORD);
+                }
                 return true;
             } else if (errorCode.equals("533")) {
                 // User is disabled in MSAD. Set him to disabled in KC as well
-                user.setEnabled(false);
+                if (user.isEnabled()) {
+                    user.setEnabled(false);
+                }
                 return true;
             } else if (errorCode.equals("775")) {
                 logger.warnf("Locked user '%s' attempt to login", user.getUsername());

federation/ldap/src/main/java/org/keycloak/storage/ldap/mappers/msadlds/MSADLDSUserAccountControlStorageMapper.java

@@ -133,11 +133,15 @@ public class MSADLDSUserAccountControlStorageMapper extends AbstractLDAPStorageM
         if (ldapProvider.getEditMode() == UserStorageProvider.EditMode.WRITABLE) {
             if (errorCode.equals("532") || errorCode.equals("773")) {
                 // User needs to change his MSAD password. Allow him to login, but add UPDATE_PASSWORD required action
-                user.addRequiredAction(UserModel.RequiredAction.UPDATE_PASSWORD);
+                if (!user.getRequiredActions().contains(UserModel.RequiredAction.UPDATE_PASSWORD.name())) {
+                    user.addRequiredAction(UserModel.RequiredAction.UPDATE_PASSWORD);
+                }
                 return true;
             } else if (errorCode.equals("533")) {
                 // User is disabled in MSAD LDS. Set him to disabled in KC as well
-                user.setEnabled(false);
+                if (user.isEnabled()) {
+                    user.setEnabled(false);
+                }
                 return true;
             } else if (errorCode.equals("775")) {
                 logger.warnf("Locked user '%s' attempt to login", user.getUsername());

testsuite/integration/src/test/java/org/keycloak/testsuite/federation/storage/ldap/LDAPMSADFullNameTest.java

@@ -60,7 +60,7 @@ import org.openqa.selenium.WebDriver;
  * @author <a href="mailto:mposolda@redhat.com">Marek Posolda</a>
  */
 @FixMethodOrder(MethodSorters.NAME_ASCENDING)
-public class MSADFullNameTest {
+public class LDAPMSADFullNameTest {
 
     // Run this test just on MSAD and just when sAMAccountName is mapped to username
     private static LDAPRule ldapRule = new LDAPRule((Map<String, String> ldapConfig) -> {

testsuite/integration/src/test/java/org/keycloak/testsuite/federation/storage/ldap/LDAPMSADMapperTest.java

@@ -56,7 +56,7 @@ import org.openqa.selenium.WebDriver;
  * @author <a href="mailto:mposolda@redhat.com">Marek Posolda</a>
  */
 @FixMethodOrder(MethodSorters.NAME_ASCENDING)
-public class MSADMapperTest {
+public class LDAPMSADMapperTest {
 
     // Run this test just on MSAD
     private static LDAPRule ldapRule = new LDAPRule((Map<String, String> ldapConfig) -> {