diff --git a/federation/ldap/src/main/java/org/keycloak/storage/ldap/LDAPIdentityStoreRegistry.java b/federation/ldap/src/main/java/org/keycloak/storage/ldap/LDAPIdentityStoreRegistry.java index 67d03f2dc7..f6be1180ea 100644 --- a/federation/ldap/src/main/java/org/keycloak/storage/ldap/LDAPIdentityStoreRegistry.java +++ b/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 Marek Posolda @@ -35,7 +39,7 @@ public class LDAPIdentityStoreRegistry { private Map ldapStores = new ConcurrentHashMap<>(); - public LDAPIdentityStore getLdapStore(ComponentModel ldapModel, Map configDecorators) { + public LDAPIdentityStore getLdapStore(KeycloakSession session, ComponentModel ldapModel, Map 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) { - logger.infof("Creating new LDAP Store for the LDAP storage provider: '%s', LDAP Configuration: %s", fedProviderDisplayName, ldapConfig.toString()); + private void logLDAPConfig(KeycloakSession session, ComponentModel ldapModel, LDAPConfig ldapConfig) { + 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 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()); + + }); + } } /** diff --git a/federation/ldap/src/main/java/org/keycloak/storage/ldap/LDAPStorageProviderFactory.java b/federation/ldap/src/main/java/org/keycloak/storage/ldap/LDAPStorageProviderFactory.java index cd656046bd..6d60ee7f87 100755 --- a/federation/ldap/src/main/java/org/keycloak/storage/ldap/LDAPStorageProviderFactory.java +++ b/federation/ldap/src/main/java/org/keycloak/storage/ldap/LDAPStorageProviderFactory.java @@ -192,7 +192,7 @@ public class LDAPStorageProviderFactory implements UserStorageProviderFactory 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); } diff --git a/federation/ldap/src/main/java/org/keycloak/storage/ldap/idm/store/ldap/LDAPOperationManager.java b/federation/ldap/src/main/java/org/keycloak/storage/ldap/idm/store/ldap/LDAPOperationManager.java index f057fc888b..502fa75521 100644 --- a/federation/ldap/src/main/java/org/keycloak/storage/ldap/idm/store/ldap/LDAPOperationManager.java +++ b/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 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; diff --git a/federation/ldap/src/main/java/org/keycloak/storage/ldap/mappers/msad/MSADUserAccountControlStorageMapper.java b/federation/ldap/src/main/java/org/keycloak/storage/ldap/mappers/msad/MSADUserAccountControlStorageMapper.java index 2bbe839b39..efc0f0bc65 100644 --- a/federation/ldap/src/main/java/org/keycloak/storage/ldap/mappers/msad/MSADUserAccountControlStorageMapper.java +++ b/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()); diff --git a/federation/ldap/src/main/java/org/keycloak/storage/ldap/mappers/msadlds/MSADLDSUserAccountControlStorageMapper.java b/federation/ldap/src/main/java/org/keycloak/storage/ldap/mappers/msadlds/MSADLDSUserAccountControlStorageMapper.java index f10ac55831..7276b31405 100644 --- a/federation/ldap/src/main/java/org/keycloak/storage/ldap/mappers/msadlds/MSADLDSUserAccountControlStorageMapper.java +++ b/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()); diff --git a/testsuite/integration/src/test/java/org/keycloak/testsuite/federation/storage/ldap/MSADFullNameTest.java b/testsuite/integration/src/test/java/org/keycloak/testsuite/federation/storage/ldap/LDAPMSADFullNameTest.java similarity index 99% rename from testsuite/integration/src/test/java/org/keycloak/testsuite/federation/storage/ldap/MSADFullNameTest.java rename to testsuite/integration/src/test/java/org/keycloak/testsuite/federation/storage/ldap/LDAPMSADFullNameTest.java index fe6510d7e7..e893d55787 100644 --- a/testsuite/integration/src/test/java/org/keycloak/testsuite/federation/storage/ldap/MSADFullNameTest.java +++ b/testsuite/integration/src/test/java/org/keycloak/testsuite/federation/storage/ldap/LDAPMSADFullNameTest.java @@ -60,7 +60,7 @@ import org.openqa.selenium.WebDriver; * @author Marek Posolda */ @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 ldapConfig) -> { diff --git a/testsuite/integration/src/test/java/org/keycloak/testsuite/federation/storage/ldap/MSADMapperTest.java b/testsuite/integration/src/test/java/org/keycloak/testsuite/federation/storage/ldap/LDAPMSADMapperTest.java similarity index 99% rename from testsuite/integration/src/test/java/org/keycloak/testsuite/federation/storage/ldap/MSADMapperTest.java rename to testsuite/integration/src/test/java/org/keycloak/testsuite/federation/storage/ldap/LDAPMSADMapperTest.java index 646e039a07..ac12372712 100644 --- a/testsuite/integration/src/test/java/org/keycloak/testsuite/federation/storage/ldap/MSADMapperTest.java +++ b/testsuite/integration/src/test/java/org/keycloak/testsuite/federation/storage/ldap/LDAPMSADMapperTest.java @@ -56,7 +56,7 @@ import org.openqa.selenium.WebDriver; * @author Marek Posolda */ @FixMethodOrder(MethodSorters.NAME_ASCENDING) -public class MSADMapperTest { +public class LDAPMSADMapperTest { // Run this test just on MSAD private static LDAPRule ldapRule = new LDAPRule((Map ldapConfig) -> {