From 9c2525ec1afb6737dd012d3c744a4098b787b3f7 Mon Sep 17 00:00:00 2001 From: mhajas Date: Wed, 4 Sep 2019 13:30:44 +0200 Subject: [PATCH] KEYCLOAK-11245 Use transcription object for LDAP bindCredential --- .../ldap/LDAPIdentityStoreRegistry.java | 6 +- .../storage/ldap/LDAPStorageProvider.java | 127 +++++----- .../ldap/LDAPStorageProviderFactory.java | 13 ++ .../org/keycloak/storage/ldap/LDAPUtils.java | 7 +- .../ldap/idm/query/internal/LDAPQuery.java | 19 +- .../idm/store/ldap/LDAPContextManager.java | 218 ++++++++++++++++++ .../idm/store/ldap/LDAPIdentityStore.java | 10 +- .../idm/store/ldap/LDAPOperationManager.java | 189 ++++----------- .../membership/UserRolesRetrieveStrategy.java | 15 +- .../group/GroupLDAPStorageMapper.java | 110 ++++----- .../role/RoleLDAPStorageMapper.java | 50 ++-- .../vault/PlainTextVaultProvider.java | 1 + .../common/vault/master_ldap__bindCredential | 1 + .../common/vault/test_ldap__bindCredential | 1 + .../servers/auth-server/jboss/pom.xml | 2 + .../testsuite/util/LDAPTestUtils.java | 42 ++-- .../federation/ldap/LDAPSyncTest.java | 4 +- .../ldap/LDAPVaultCredentialsTest.java | 35 +++ .../vault/master_ldap__bindCredential | 1 + .../resources/vault/test_ldap__bindCredential | 1 + 20 files changed, 515 insertions(+), 337 deletions(-) create mode 100644 federation/ldap/src/main/java/org/keycloak/storage/ldap/idm/store/ldap/LDAPContextManager.java create mode 100644 testsuite/integration-arquillian/servers/auth-server/jboss/common/vault/master_ldap__bindCredential create mode 100644 testsuite/integration-arquillian/servers/auth-server/jboss/common/vault/test_ldap__bindCredential create mode 100644 testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/federation/ldap/LDAPVaultCredentialsTest.java create mode 100644 testsuite/integration-arquillian/tests/base/src/test/resources/vault/master_ldap__bindCredential create mode 100644 testsuite/integration-arquillian/tests/base/src/test/resources/vault/test_ldap__bindCredential 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 91077f859a..30d8f33675 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 @@ -55,7 +55,7 @@ public class LDAPIdentityStoreRegistry { if (context == null || !ldapConfig.equals(context.config)) { logLDAPConfig(session, ldapModel, ldapConfig); - LDAPIdentityStore store = createLdapIdentityStore(ldapConfig); + LDAPIdentityStore store = createLdapIdentityStore(session, ldapConfig); context = new LDAPIdentityStoreContext(ldapConfig, store); ldapStores.put(ldapModel.getId(), context); } @@ -80,7 +80,7 @@ public class LDAPIdentityStoreRegistry { /** * Create LDAPIdentityStore to be cached in the local registry */ - public static LDAPIdentityStore createLdapIdentityStore(LDAPConfig cfg) { + public static LDAPIdentityStore createLdapIdentityStore(KeycloakSession session, LDAPConfig cfg) { checkSystemProperty("com.sun.jndi.ldap.connect.pool.authentication", cfg.getConnectionPoolingAuthentication(), "none simple"); checkSystemProperty("com.sun.jndi.ldap.connect.pool.initsize", cfg.getConnectionPoolingInitSize(), "1"); checkSystemProperty("com.sun.jndi.ldap.connect.pool.maxsize", cfg.getConnectionPoolingMaxSize(), "1000"); @@ -89,7 +89,7 @@ public class LDAPIdentityStoreRegistry { checkSystemProperty("com.sun.jndi.ldap.connect.pool.protocol", cfg.getConnectionPoolingProtocol(), "plain"); checkSystemProperty("com.sun.jndi.ldap.connect.pool.debug", cfg.getConnectionPoolingDebug(), "off"); - return new LDAPIdentityStore(cfg); + return new LDAPIdentityStore(session, cfg); } private static void checkSystemProperty(String name, String cfgValue, String defaultValue) { diff --git a/federation/ldap/src/main/java/org/keycloak/storage/ldap/LDAPStorageProvider.java b/federation/ldap/src/main/java/org/keycloak/storage/ldap/LDAPStorageProvider.java index f95050aa4e..96bd444dc1 100755 --- a/federation/ldap/src/main/java/org/keycloak/storage/ldap/LDAPStorageProvider.java +++ b/federation/ldap/src/main/java/org/keycloak/storage/ldap/LDAPStorageProvider.java @@ -223,30 +223,30 @@ public class LDAPStorageProvider implements UserStorageProvider, @Override public List searchForUserByUserAttribute(String attrName, String attrValue, RealmModel realm) { - LDAPQuery ldapQuery = LDAPUtils.createQueryForUserSearch(this, realm); - LDAPQueryConditionsBuilder conditionsBuilder = new LDAPQueryConditionsBuilder(); + try (LDAPQuery ldapQuery = LDAPUtils.createQueryForUserSearch(this, realm)) { + LDAPQueryConditionsBuilder conditionsBuilder = new LDAPQueryConditionsBuilder(); - Condition attrCondition = conditionsBuilder.equal(attrName, attrValue, EscapeStrategy.DEFAULT); - ldapQuery.addWhereCondition(attrCondition); + Condition attrCondition = conditionsBuilder.equal(attrName, attrValue, EscapeStrategy.DEFAULT); + ldapQuery.addWhereCondition(attrCondition); - List ldapObjects = ldapQuery.getResultList(); - - if (ldapObjects == null || ldapObjects.isEmpty()) { - return Collections.emptyList(); - } - - List searchResults =new LinkedList(); - - for (LDAPObject ldapUser : ldapObjects) { - String ldapUsername = LDAPUtils.getUsername(ldapUser, this.ldapIdentityStore.getConfig()); - if (session.userLocalStorage().getUserByUsername(ldapUsername, realm) == null) { - UserModel imported = importUserFromLDAP(session, realm, ldapUser); - searchResults.add(imported); + List ldapObjects = ldapQuery.getResultList(); + + if (ldapObjects == null || ldapObjects.isEmpty()) { + return Collections.emptyList(); } - } - return searchResults; - + List searchResults = new LinkedList(); + + for (LDAPObject ldapUser : ldapObjects) { + String ldapUsername = LDAPUtils.getUsername(ldapUser, this.ldapIdentityStore.getConfig()); + if (session.userLocalStorage().getUserByUsername(ldapUsername, realm) == null) { + UserModel imported = importUserFromLDAP(session, realm, ldapUser); + searchResults.add(imported); + } + } + + return searchResults; + } } public boolean synchronizeRegistrations() { @@ -417,43 +417,46 @@ public class LDAPStorageProvider implements UserStorageProvider, List results = new ArrayList(); if (attributes.containsKey(UserModel.USERNAME)) { - LDAPQuery ldapQuery = LDAPUtils.createQueryForUserSearch(this, realm); - LDAPQueryConditionsBuilder conditionsBuilder = new LDAPQueryConditionsBuilder(); + try (LDAPQuery ldapQuery = LDAPUtils.createQueryForUserSearch(this, realm)) { + LDAPQueryConditionsBuilder conditionsBuilder = new LDAPQueryConditionsBuilder(); - // Mapper should replace "username" in parameter name with correct LDAP mapped attribute - Condition usernameCondition = conditionsBuilder.equal(UserModel.USERNAME, attributes.get(UserModel.USERNAME), EscapeStrategy.NON_ASCII_CHARS_ONLY); - ldapQuery.addWhereCondition(usernameCondition); + // Mapper should replace "username" in parameter name with correct LDAP mapped attribute + Condition usernameCondition = conditionsBuilder.equal(UserModel.USERNAME, attributes.get(UserModel.USERNAME), EscapeStrategy.NON_ASCII_CHARS_ONLY); + ldapQuery.addWhereCondition(usernameCondition); - List ldapObjects = ldapQuery.getResultList(); - results.addAll(ldapObjects); + List ldapObjects = ldapQuery.getResultList(); + results.addAll(ldapObjects); + } } if (attributes.containsKey(UserModel.EMAIL)) { - LDAPQuery ldapQuery = LDAPUtils.createQueryForUserSearch(this, realm); - LDAPQueryConditionsBuilder conditionsBuilder = new LDAPQueryConditionsBuilder(); + try (LDAPQuery ldapQuery = LDAPUtils.createQueryForUserSearch(this, realm)) { + LDAPQueryConditionsBuilder conditionsBuilder = new LDAPQueryConditionsBuilder(); - // Mapper should replace "email" in parameter name with correct LDAP mapped attribute - Condition emailCondition = conditionsBuilder.equal(UserModel.EMAIL, attributes.get(UserModel.EMAIL), EscapeStrategy.NON_ASCII_CHARS_ONLY); - ldapQuery.addWhereCondition(emailCondition); + // Mapper should replace "email" in parameter name with correct LDAP mapped attribute + Condition emailCondition = conditionsBuilder.equal(UserModel.EMAIL, attributes.get(UserModel.EMAIL), EscapeStrategy.NON_ASCII_CHARS_ONLY); + ldapQuery.addWhereCondition(emailCondition); - List ldapObjects = ldapQuery.getResultList(); - results.addAll(ldapObjects); + List ldapObjects = ldapQuery.getResultList(); + results.addAll(ldapObjects); + } } if (attributes.containsKey(UserModel.FIRST_NAME) || attributes.containsKey(UserModel.LAST_NAME)) { - LDAPQuery ldapQuery = LDAPUtils.createQueryForUserSearch(this, realm); - LDAPQueryConditionsBuilder conditionsBuilder = new LDAPQueryConditionsBuilder(); + try (LDAPQuery ldapQuery = LDAPUtils.createQueryForUserSearch(this, realm)) { + LDAPQueryConditionsBuilder conditionsBuilder = new LDAPQueryConditionsBuilder(); - // Mapper should replace parameter with correct LDAP mapped attributes - if (attributes.containsKey(UserModel.FIRST_NAME)) { - ldapQuery.addWhereCondition(conditionsBuilder.equal(UserModel.FIRST_NAME, attributes.get(UserModel.FIRST_NAME), EscapeStrategy.NON_ASCII_CHARS_ONLY)); - } - if (attributes.containsKey(UserModel.LAST_NAME)) { - ldapQuery.addWhereCondition(conditionsBuilder.equal(UserModel.LAST_NAME, attributes.get(UserModel.LAST_NAME), EscapeStrategy.NON_ASCII_CHARS_ONLY)); - } + // Mapper should replace parameter with correct LDAP mapped attributes + if (attributes.containsKey(UserModel.FIRST_NAME)) { + ldapQuery.addWhereCondition(conditionsBuilder.equal(UserModel.FIRST_NAME, attributes.get(UserModel.FIRST_NAME), EscapeStrategy.NON_ASCII_CHARS_ONLY)); + } + if (attributes.containsKey(UserModel.LAST_NAME)) { + ldapQuery.addWhereCondition(conditionsBuilder.equal(UserModel.LAST_NAME, attributes.get(UserModel.LAST_NAME), EscapeStrategy.NON_ASCII_CHARS_ONLY)); + } - List ldapObjects = ldapQuery.getResultList(); - results.addAll(ldapObjects); + List ldapObjects = ldapQuery.getResultList(); + results.addAll(ldapObjects); + } } return results; @@ -530,14 +533,15 @@ public class LDAPStorageProvider implements UserStorageProvider, } protected LDAPObject queryByEmail(RealmModel realm, String email) { - LDAPQuery ldapQuery = LDAPUtils.createQueryForUserSearch(this, realm); - LDAPQueryConditionsBuilder conditionsBuilder = new LDAPQueryConditionsBuilder(); + try (LDAPQuery ldapQuery = LDAPUtils.createQueryForUserSearch(this, realm)) { + LDAPQueryConditionsBuilder conditionsBuilder = new LDAPQueryConditionsBuilder(); - // Mapper should replace "email" in parameter name with correct LDAP mapped attribute - Condition emailCondition = conditionsBuilder.equal(UserModel.EMAIL, email, EscapeStrategy.DEFAULT); - ldapQuery.addWhereCondition(emailCondition); + // Mapper should replace "email" in parameter name with correct LDAP mapped attribute + Condition emailCondition = conditionsBuilder.equal(UserModel.EMAIL, email, EscapeStrategy.DEFAULT); + ldapQuery.addWhereCondition(emailCondition); - return ldapQuery.getFirstResult(); + return ldapQuery.getFirstResult(); + } } @@ -761,19 +765,20 @@ public class LDAPStorageProvider implements UserStorageProvider, } public LDAPObject loadLDAPUserByUsername(RealmModel realm, String username) { - LDAPQuery ldapQuery = LDAPUtils.createQueryForUserSearch(this, realm); - LDAPQueryConditionsBuilder conditionsBuilder = new LDAPQueryConditionsBuilder(); + try (LDAPQuery ldapQuery = LDAPUtils.createQueryForUserSearch(this, realm)) { + LDAPQueryConditionsBuilder conditionsBuilder = new LDAPQueryConditionsBuilder(); - String usernameMappedAttribute = this.ldapIdentityStore.getConfig().getUsernameLdapAttribute(); - Condition usernameCondition = conditionsBuilder.equal(usernameMappedAttribute, username, EscapeStrategy.DEFAULT); - ldapQuery.addWhereCondition(usernameCondition); + String usernameMappedAttribute = this.ldapIdentityStore.getConfig().getUsernameLdapAttribute(); + Condition usernameCondition = conditionsBuilder.equal(usernameMappedAttribute, username, EscapeStrategy.DEFAULT); + ldapQuery.addWhereCondition(usernameCondition); - LDAPObject ldapUser = ldapQuery.getFirstResult(); - if (ldapUser == null) { - return null; + LDAPObject ldapUser = ldapQuery.getFirstResult(); + if (ldapUser == null) { + return null; + } + + return ldapUser; } - - return ldapUser; } 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 b046b5973c..bbaad44712 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 @@ -467,6 +467,7 @@ public class LDAPStorageProviderFactory implements UserStorageProviderFactory mappers = realm.getComponents(model.getId(), LDAPStorageMapper.class.getName()); for (ComponentModel mapperModel : mappers) { @@ -508,6 +509,13 @@ public class LDAPStorageProviderFactory implements UserStorageProviderFactory conditions = new LinkedHashSet(); private final Set ordering = new LinkedHashSet(); @@ -204,8 +207,10 @@ public class LDAPQuery implements AutoCloseable{ return this; } - public LDAPQuery initPagination(LdapContext ldapContext) { - this.paginationContext = new PaginationContext(ldapContext); + public LDAPQuery initPagination() throws NamingException { + this.ldapContextManager = LDAPContextManager.create(ldapFedProvider.getSession(), + ldapFedProvider.getLdapIdentityStore().getConfig()); + this.paginationContext = new PaginationContext(ldapContextManager.getLdapContext()); return this; } @@ -220,12 +225,8 @@ public class LDAPQuery implements AutoCloseable{ @Override public void close() { - if (paginationContext != null) { - try { - paginationContext.ldapContext.close(); - } catch (NamingException ne) { - logger.error("Could not close Ldap context.", ne); - } + if (ldapContextManager != null) { + ldapContextManager.close(); } } diff --git a/federation/ldap/src/main/java/org/keycloak/storage/ldap/idm/store/ldap/LDAPContextManager.java b/federation/ldap/src/main/java/org/keycloak/storage/ldap/idm/store/ldap/LDAPContextManager.java new file mode 100644 index 0000000000..24915bd8b2 --- /dev/null +++ b/federation/ldap/src/main/java/org/keycloak/storage/ldap/idm/store/ldap/LDAPContextManager.java @@ -0,0 +1,218 @@ +package org.keycloak.storage.ldap.idm.store.ldap; + +import org.jboss.logging.Logger; +import org.keycloak.models.KeycloakSession; +import org.keycloak.models.LDAPConstants; +import org.keycloak.storage.ldap.LDAPConfig; +import org.keycloak.vault.VaultCharSecret; + +import javax.naming.Context; +import javax.naming.NamingException; +import javax.naming.ldap.InitialLdapContext; +import javax.naming.ldap.LdapContext; +import javax.naming.ldap.StartTlsRequest; +import javax.naming.ldap.StartTlsResponse; +import java.io.IOException; +import java.nio.CharBuffer; +import java.util.HashMap; +import java.util.Hashtable; +import java.util.Map; +import java.util.Optional; +import java.util.Properties; + +import static javax.naming.Context.SECURITY_CREDENTIALS; + +/** + * @author mhajas + */ +public final class LDAPContextManager implements AutoCloseable { + + private static final Logger logger = Logger.getLogger(LDAPContextManager.class); + + private final KeycloakSession session; + private final LDAPConfig ldapConfig; + private StartTlsResponse tlsResponse; + + private VaultCharSecret vaultCharSecret = new VaultCharSecret() { + @Override + public Optional get() { + return Optional.empty(); + } + + @Override + public Optional getAsArray() { + return Optional.empty(); + } + + @Override + public void close() { + + } + }; + + private LdapContext ldapContext; + + public LDAPContextManager(KeycloakSession session, LDAPConfig connectionProperties) { + this.session = session; + this.ldapConfig = connectionProperties; + } + + public static LDAPContextManager create(KeycloakSession session, LDAPConfig connectionProperties) { + return new LDAPContextManager(session, connectionProperties); + } + + private void createLdapContext() throws NamingException { + Hashtable connProp = getConnectionProperties(ldapConfig); + + if (!LDAPConstants.AUTH_TYPE_NONE.equals(ldapConfig.getAuthType())) { + vaultCharSecret = getVaultSecret(); + + if (vaultCharSecret != null && !ldapConfig.isStartTls()) { + connProp.put(SECURITY_CREDENTIALS, vaultCharSecret.getAsArray() + .orElse(ldapConfig.getBindCredential().toCharArray())); + } + } + + ldapContext = new InitialLdapContext(connProp, null); + if (ldapConfig.isStartTls()) { + tlsResponse = startTLS(ldapContext, ldapConfig.getAuthType(), ldapConfig.getBindDN(), + vaultCharSecret.getAsArray().orElse(ldapConfig.getBindCredential().toCharArray())); + } + + } + + public LdapContext getLdapContext() throws NamingException { + if (ldapContext == null) createLdapContext(); + + return ldapContext; + } + + private VaultCharSecret getVaultSecret() { + return LDAPConstants.AUTH_TYPE_NONE.equals(ldapConfig.getAuthType()) + ? null + : session.vault().getCharSecret(ldapConfig.getBindCredential()); + } + + public static StartTlsResponse startTLS(LdapContext ldapContext, String authType, String bindDN, char[] bindCredential) throws NamingException { + try { + StartTlsResponse tls = (StartTlsResponse) ldapContext.extendedOperation(new StartTlsRequest()); + tls.negotiate(); + + ldapContext.addToEnvironment(Context.SECURITY_AUTHENTICATION, authType); + + if (!LDAPConstants.AUTH_TYPE_NONE.equals(authType)) { + ldapContext.addToEnvironment(Context.SECURITY_PRINCIPAL, bindDN); + ldapContext.addToEnvironment(Context.SECURITY_CREDENTIALS, bindCredential); + } + + ldapContext.lookup(""); + + return tls; + } catch (Exception e) { + logger.error("Could not negotiate TLS", e); + } + + return null; + } + + public static Hashtable getConnectionProperties(LDAPConfig ldapConfig) { + HashMap env = new HashMap<>(); + + env.put(Context.INITIAL_CONTEXT_FACTORY, ldapConfig.getFactoryName()); + + if(!ldapConfig.isStartTls()) { + String authType = ldapConfig.getAuthType(); + + env.put(Context.SECURITY_AUTHENTICATION, authType); + + String bindDN = ldapConfig.getBindDN(); + + char[] bindCredential = null; + + if (ldapConfig.getBindCredential() != null) { + bindCredential = ldapConfig.getBindCredential().toCharArray(); + } + + if (!LDAPConstants.AUTH_TYPE_NONE.equals(authType)) { + env.put(Context.SECURITY_PRINCIPAL, bindDN); + env.put(Context.SECURITY_CREDENTIALS, bindCredential); + } + } + String url = ldapConfig.getConnectionUrl(); + + if (url != null) { + env.put(Context.PROVIDER_URL, url); + } else { + logger.warn("LDAP URL is null. LDAPOperationManager won't work correctly"); + } + + String useTruststoreSpi = ldapConfig.getUseTruststoreSpi(); + LDAPConstants.setTruststoreSpiIfNeeded(useTruststoreSpi, url, env); + + String connectionPooling = ldapConfig.getConnectionPooling(); + if (connectionPooling != null) { + env.put("com.sun.jndi.ldap.connect.pool", connectionPooling); + } + + String connectionTimeout = ldapConfig.getConnectionTimeout(); + if (connectionTimeout != null && !connectionTimeout.isEmpty()) { + env.put("com.sun.jndi.ldap.connect.timeout", connectionTimeout); + } + + String readTimeout = ldapConfig.getReadTimeout(); + if (readTimeout != null && !readTimeout.isEmpty()) { + env.put("com.sun.jndi.ldap.read.timeout", readTimeout); + } + + // Just dump the additional properties + Properties additionalProperties = ldapConfig.getAdditionalConnectionProperties(); + if (additionalProperties != null) { + for (Object key : additionalProperties.keySet()) { + env.put(key.toString(), additionalProperties.getProperty(key.toString())); + } + } + + StringBuilder binaryAttrsBuilder = new StringBuilder(); + if (ldapConfig.isObjectGUID()) { + binaryAttrsBuilder.append(LDAPConstants.OBJECT_GUID).append(" "); + } + for (String attrName : ldapConfig.getBinaryAttributeNames()) { + binaryAttrsBuilder.append(attrName).append(" "); + } + + String binaryAttrs = binaryAttrsBuilder.toString().trim(); + if (!binaryAttrs.isEmpty()) { + env.put("java.naming.ldap.attributes.binary", binaryAttrs); + } + + if (logger.isDebugEnabled()) { + 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 new Hashtable<>(env); + } + + @Override + public void close() { + if (vaultCharSecret != null) vaultCharSecret.close(); + if (tlsResponse != null) { + try { + tlsResponse.close(); + } catch (IOException e) { + logger.error("Could not close Ldap tlsResponse.", e); + } + } + + if (ldapContext != null) { + try { + ldapContext.close(); + } catch (NamingException e) { + logger.error("Could not close Ldap context.", e); + } + } + } +} diff --git a/federation/ldap/src/main/java/org/keycloak/storage/ldap/idm/store/ldap/LDAPIdentityStore.java b/federation/ldap/src/main/java/org/keycloak/storage/ldap/idm/store/ldap/LDAPIdentityStore.java index c43ebc3545..9ad23882b4 100644 --- a/federation/ldap/src/main/java/org/keycloak/storage/ldap/idm/store/ldap/LDAPIdentityStore.java +++ b/federation/ldap/src/main/java/org/keycloak/storage/ldap/idm/store/ldap/LDAPIdentityStore.java @@ -19,6 +19,7 @@ package org.keycloak.storage.ldap.idm.store.ldap; import org.jboss.logging.Logger; import org.keycloak.common.util.Base64; +import org.keycloak.models.KeycloakSession; import org.keycloak.models.LDAPConstants; import org.keycloak.models.ModelException; import org.keycloak.storage.ldap.LDAPConfig; @@ -75,14 +76,9 @@ public class LDAPIdentityStore implements IdentityStore { private final LDAPConfig config; private final LDAPOperationManager operationManager; - public LDAPIdentityStore(LDAPConfig config) { + public LDAPIdentityStore(KeycloakSession session, LDAPConfig config) { this.config = config; - - try { - this.operationManager = new LDAPOperationManager(config); - } catch (NamingException e) { - throw new ModelException("Couldn't init operation manager", e); - } + this.operationManager = new LDAPOperationManager(session, config); } @Override 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 47ef2d803b..52204590aa 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 @@ -19,6 +19,7 @@ package org.keycloak.storage.ldap.idm.store.ldap; import org.jboss.logging.Logger; import org.keycloak.common.util.Time; +import org.keycloak.models.KeycloakSession; import org.keycloak.models.LDAPConstants; import org.keycloak.models.ModelException; import org.keycloak.storage.ldap.LDAPConfig; @@ -29,7 +30,6 @@ import org.keycloak.storage.ldap.mappers.LDAPOperationDecorator; import javax.naming.AuthenticationException; import javax.naming.Binding; import javax.naming.Context; -import javax.naming.InitialContext; import javax.naming.NameAlreadyBoundException; import javax.naming.NamingEnumeration; import javax.naming.NamingException; @@ -71,12 +71,12 @@ public class LDAPOperationManager { private static final Logger perfLogger = Logger.getLogger(LDAPOperationManager.class, "perf"); + private final KeycloakSession session; private final LDAPConfig config; - private final Map connectionProperties; - public LDAPOperationManager(LDAPConfig config) throws NamingException { + public LDAPOperationManager(KeycloakSession session, LDAPConfig config) { + this.session = session; this.config = config; - this.connectionProperties = Collections.unmodifiableMap(createConnectionProperties()); } /** @@ -290,8 +290,7 @@ public class LDAPOperationManager { // Very 1st page. Pagination context is not yet present if (identityQuery.getPaginationContext() == null) { - LdapContext ldapContext = createLdapContext(); - identityQuery.initPagination(ldapContext); + identityQuery.initPagination(); } try { @@ -492,14 +491,17 @@ public class LDAPOperationManager { * */ public void authenticate(String dn, String password) throws AuthenticationException { - InitialLdapContext authCtx = null; + + if (password == null || password.isEmpty()) { + throw new AuthenticationException("Empty password used"); + } + + LdapContext authCtx = null; + StartTlsResponse tlsResponse = null; try { - if (password == null || password.isEmpty()) { - throw new AuthenticationException("Empty password used"); - } - Hashtable env = new Hashtable(this.connectionProperties); + Hashtable env = LDAPContextManager.getConnectionProperties(config); // Never use connection pool to prevent password caching env.put("com.sun.jndi.ldap.connect.pool", "false"); @@ -511,8 +513,9 @@ public class LDAPOperationManager { } authCtx = new InitialLdapContext(env, null); - startTLS(authCtx, this.config.getAuthType(), dn, password); - + if (config.isStartTls()) { + tlsResponse = LDAPContextManager.startTLS(authCtx, this.config.getAuthType(), dn, password.toCharArray()); + } } catch (AuthenticationException ae) { if (logger.isDebugEnabled()) { logger.debugf(ae, "Authentication failed for DN [%s]", dn); @@ -523,38 +526,22 @@ public class LDAPOperationManager { logger.errorf(e, "Unexpected exception when validating password of DN [%s]", dn); throw new AuthenticationException("Unexpected exception when validating password of user"); } finally { + if (tlsResponse != null) { + try { + tlsResponse.close(); + } catch (IOException e) { + e.printStackTrace(); + } + } + if (authCtx != null) { try { authCtx.close(); } catch (NamingException e) { - + e.printStackTrace(); } } - } - } - private void startTLS(LdapContext ldapContext, String authType, String bindDN, String bindCredentials) throws NamingException { - if(this.config.isStartTls()) { - try { - StartTlsResponse tls = (StartTlsResponse) ldapContext.extendedOperation(new StartTlsRequest()); - tls.negotiate(); - - char[] bindCredential = null; - - ldapContext.addToEnvironment(Context.SECURITY_AUTHENTICATION, authType); - - if (bindCredentials != null) { - bindCredential = bindCredentials.toCharArray(); - } - - if (!LDAPConstants.AUTH_TYPE_NONE.equals(authType)) { - ldapContext.addToEnvironment(Context.SECURITY_PRINCIPAL, bindDN); - ldapContext.addToEnvironment(Context.SECURITY_CREDENTIALS, bindCredential); - } - } catch (Exception e) { - logger.error("Could not negotiate TLS", e); - } - ldapContext.lookup(""); } } @@ -598,7 +585,7 @@ public class LDAPOperationManager { .toString(); } - }, null, decorator); + }, decorator); } public void modifyAttributes(final String dn, final ModificationItem[] mods, LDAPOperationDecorator decorator) { @@ -681,130 +668,34 @@ public class LDAPOperationManager { return id; } - private LdapContext createLdapContext() throws NamingException { - if(!config.isStartTls()) { - return new InitialLdapContext(new Hashtable(this.connectionProperties), null); - } else { - LdapContext ldapContext = new InitialLdapContext(new Hashtable(this.connectionProperties), null); - startTLS(ldapContext, this.config.getAuthType(), this.config.getBindDN(), this.config.getBindCredential()); - return ldapContext; - } - } - - private Map createConnectionProperties() { - HashMap env = new HashMap(); - - env.put(Context.INITIAL_CONTEXT_FACTORY, this.config.getFactoryName()); - - if(!this.config.isStartTls()) { - String authType = this.config.getAuthType(); - - env.put(Context.SECURITY_AUTHENTICATION, authType); - - String bindDN = this.config.getBindDN(); - - char[] bindCredential = null; - - if (this.config.getBindCredential() != null) { - bindCredential = this.config.getBindCredential().toCharArray(); - } - - if (!LDAPConstants.AUTH_TYPE_NONE.equals(authType)) { - env.put(Context.SECURITY_PRINCIPAL, bindDN); - env.put(Context.SECURITY_CREDENTIALS, bindCredential); - } - } - String url = this.config.getConnectionUrl(); - - if (url != null) { - env.put(Context.PROVIDER_URL, url); - } else { - logger.warn("LDAP URL is null. LDAPOperationManager won't work correctly"); - } - - String useTruststoreSpi = this.config.getUseTruststoreSpi(); - LDAPConstants.setTruststoreSpiIfNeeded(useTruststoreSpi, url, env); - - String connectionPooling = this.config.getConnectionPooling(); - if (connectionPooling != null) { - env.put("com.sun.jndi.ldap.connect.pool", connectionPooling); - } - - String connectionTimeout = config.getConnectionTimeout(); - if (connectionTimeout != null && !connectionTimeout.isEmpty()) { - env.put("com.sun.jndi.ldap.connect.timeout", connectionTimeout); - } - - String readTimeout = config.getReadTimeout(); - if (readTimeout != null && !readTimeout.isEmpty()) { - env.put("com.sun.jndi.ldap.read.timeout", readTimeout); - } - - // Just dump the additional properties - Properties additionalProperties = this.config.getAdditionalConnectionProperties(); - if (additionalProperties != null) { - for (Object key : additionalProperties.keySet()) { - env.put(key.toString(), additionalProperties.getProperty(key.toString())); - } - } - - StringBuilder binaryAttrsBuilder = new StringBuilder(); - if (this.config.isObjectGUID()) { - binaryAttrsBuilder.append(LDAPConstants.OBJECT_GUID).append(" "); - } - for (String attrName : config.getBinaryAttributeNames()) { - binaryAttrsBuilder.append(attrName).append(" "); - } - - String binaryAttrs = binaryAttrsBuilder.toString().trim(); - if (!binaryAttrs.isEmpty()) { - env.put("java.naming.ldap.attributes.binary", binaryAttrs); - } - - if (logger.isDebugEnabled()) { - 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; - } - private R execute(LdapOperation operation) throws NamingException { - return execute(operation, null, null); + return execute(operation, null); + } + + private R execute(LdapOperation operation, LDAPOperationDecorator decorator) throws NamingException { + try (LDAPContextManager ldapContextManager = LDAPContextManager.create(session, config)) { + return execute(operation, ldapContextManager.getLdapContext(), decorator); + } } private R execute(LdapOperation operation, LdapContext context, LDAPOperationDecorator decorator) throws NamingException { - // We won't manage LDAP context (create and close) in case that existing context was passed as an argument to this method - boolean manageContext = context == null; + if (context == null) { + throw new IllegalArgumentException("Ldap context cannot be null"); + } Long start = null; + if (perfLogger.isDebugEnabled()) { + start = Time.currentTimeMillis(); + } + try { - if (perfLogger.isDebugEnabled()) { - start = Time.currentTimeMillis(); - } - - if (manageContext) { - context = createLdapContext(); - } - if (decorator != null) { decorator.beforeLDAPOperation(context, operation); } return operation.execute(context); } finally { - if (context != null && manageContext) { - try { - context.close(); - } catch (NamingException ne) { - logger.error("Could not close Ldap context.", ne); - } - } - if (perfLogger.isDebugEnabled()) { long took = Time.currentTimeMillis() - start; diff --git a/federation/ldap/src/main/java/org/keycloak/storage/ldap/mappers/membership/UserRolesRetrieveStrategy.java b/federation/ldap/src/main/java/org/keycloak/storage/ldap/mappers/membership/UserRolesRetrieveStrategy.java index d1bfdddcda..c2dd2e7670 100644 --- a/federation/ldap/src/main/java/org/keycloak/storage/ldap/mappers/membership/UserRolesRetrieveStrategy.java +++ b/federation/ldap/src/main/java/org/keycloak/storage/ldap/mappers/membership/UserRolesRetrieveStrategy.java @@ -54,15 +54,16 @@ public interface UserRolesRetrieveStrategy { @Override public List getLDAPRoleMappings(CommonLDAPGroupMapper roleOrGroupMapper, LDAPObject ldapUser, LDAPConfig ldapConfig) { - LDAPQuery ldapQuery = roleOrGroupMapper.createLDAPGroupQuery(); - String membershipAttr = roleOrGroupMapper.getConfig().getMembershipLdapAttribute(); + try (LDAPQuery ldapQuery = roleOrGroupMapper.createLDAPGroupQuery()) { + String membershipAttr = roleOrGroupMapper.getConfig().getMembershipLdapAttribute(); - String membershipUserAttrName = roleOrGroupMapper.getConfig().getMembershipUserLdapAttribute(ldapConfig); - String userMembership = LDAPUtils.getMemberValueOfChildObject(ldapUser, roleOrGroupMapper.getConfig().getMembershipTypeLdapAttribute(), membershipUserAttrName); + String membershipUserAttrName = roleOrGroupMapper.getConfig().getMembershipUserLdapAttribute(ldapConfig); + String userMembership = LDAPUtils.getMemberValueOfChildObject(ldapUser, roleOrGroupMapper.getConfig().getMembershipTypeLdapAttribute(), membershipUserAttrName); - Condition membershipCondition = getMembershipCondition(membershipAttr, userMembership); - ldapQuery.addWhereCondition(membershipCondition); - return ldapQuery.getResultList(); + Condition membershipCondition = getMembershipCondition(membershipAttr, userMembership); + ldapQuery.addWhereCondition(membershipCondition); + return ldapQuery.getResultList(); + } } @Override diff --git a/federation/ldap/src/main/java/org/keycloak/storage/ldap/mappers/membership/group/GroupLDAPStorageMapper.java b/federation/ldap/src/main/java/org/keycloak/storage/ldap/mappers/membership/group/GroupLDAPStorageMapper.java index bd014f360c..be4d24b724 100644 --- a/federation/ldap/src/main/java/org/keycloak/storage/ldap/mappers/membership/group/GroupLDAPStorageMapper.java +++ b/federation/ldap/src/main/java/org/keycloak/storage/ldap/mappers/membership/group/GroupLDAPStorageMapper.java @@ -87,7 +87,7 @@ public class GroupLDAPStorageMapper extends AbstractLDAPStorageMapper implements // LDAP Group CRUD operations - + // !! This function must be always called from try-with-resources block, otherwise vault secret may be leaked !! public LDAPQuery createGroupQuery(boolean includeMemberAttribute) { LDAPQuery ldapQuery = new LDAPQuery(ldapProvider); @@ -129,10 +129,11 @@ public class GroupLDAPStorageMapper extends AbstractLDAPStorageMapper implements } public LDAPObject loadLDAPGroupByName(String groupName) { - LDAPQuery ldapQuery = createGroupQuery(true); - Condition roleNameCondition = new LDAPQueryConditionsBuilder().equal(config.getGroupNameLdapAttribute(), groupName); - ldapQuery.addWhereCondition(roleNameCondition); - return ldapQuery.getFirstResult(); + try (LDAPQuery ldapQuery = createGroupQuery(true)) { + Condition roleNameCondition = new LDAPQueryConditionsBuilder().equal(config.getGroupNameLdapAttribute(), groupName); + ldapQuery.addWhereCondition(roleNameCondition); + return ldapQuery.getFirstResult(); + } } protected Set getLDAPSubgroups(LDAPObject ldapGroup) { @@ -376,41 +377,43 @@ public class GroupLDAPStorageMapper extends AbstractLDAPStorageMapper implements logger.debugf("Syncing groups from Keycloak into LDAP. Mapper is [%s], LDAP provider is [%s]", mapperModel.getName(), ldapProvider.getModel().getName()); // Query existing LDAP groups - LDAPQuery ldapQuery = createGroupQuery(config.isPreserveGroupsInheritance()); - List ldapGroups = ldapQuery.getResultList(); + try (LDAPQuery ldapQuery = createGroupQuery(config.isPreserveGroupsInheritance())) { + List ldapGroups = ldapQuery.getResultList(); - // Convert them to Map - Map ldapGroupsMap = new HashMap<>(); - String groupsRdnAttr = config.getGroupNameLdapAttribute(); - for (LDAPObject ldapGroup : ldapGroups) { - String groupName = ldapGroup.getAttributeAsString(groupsRdnAttr); - ldapGroupsMap.put(groupName, ldapGroup); - } + // Convert them to Map + Map ldapGroupsMap = new HashMap<>(); + String groupsRdnAttr = config.getGroupNameLdapAttribute(); + for (LDAPObject ldapGroup : ldapGroups) { + String groupName = ldapGroup.getAttributeAsString(groupsRdnAttr); + ldapGroupsMap.put(groupName, ldapGroup); + } - // Map to track all LDAP groups also exists in Keycloak - Set ldapGroupNames = new HashSet<>(); - // Create or update KC groups to LDAP including their attributes - for (GroupModel kcGroup : realm.getTopLevelGroups()) { - processKeycloakGroupSyncToLDAP(kcGroup, ldapGroupsMap, ldapGroupNames, syncResult); - } + // Map to track all LDAP groups also exists in Keycloak + Set ldapGroupNames = new HashSet<>(); - // If dropNonExisting, then drop all groups, which doesn't exist in KC from LDAP as well - if (config.isDropNonExistingGroupsDuringSync()) { - Set copy = new HashSet<>(ldapGroupsMap.keySet()); - for (String groupName : copy) { - if (!ldapGroupNames.contains(groupName)) { - LDAPObject ldapGroup = ldapGroupsMap.remove(groupName); - ldapProvider.getLdapIdentityStore().remove(ldapGroup); - syncResult.increaseRemoved(); + // Create or update KC groups to LDAP including their attributes + for (GroupModel kcGroup : realm.getTopLevelGroups()) { + processKeycloakGroupSyncToLDAP(kcGroup, ldapGroupsMap, ldapGroupNames, syncResult); + } + + // If dropNonExisting, then drop all groups, which doesn't exist in KC from LDAP as well + if (config.isDropNonExistingGroupsDuringSync()) { + Set copy = new HashSet<>(ldapGroupsMap.keySet()); + for (String groupName : copy) { + if (!ldapGroupNames.contains(groupName)) { + LDAPObject ldapGroup = ldapGroupsMap.remove(groupName); + ldapProvider.getLdapIdentityStore().remove(ldapGroup); + syncResult.increaseRemoved(); + } } } - } - // Finally process memberships, - if (config.isPreserveGroupsInheritance()) { - for (GroupModel kcGroup : realm.getTopLevelGroups()) { - processKeycloakGroupMembershipsSyncToLDAP(kcGroup, ldapGroupsMap); + // Finally process memberships, + if (config.isPreserveGroupsInheritance()) { + for (GroupModel kcGroup : realm.getTopLevelGroups()) { + processKeycloakGroupMembershipsSyncToLDAP(kcGroup, ldapGroupsMap); + } } } @@ -662,30 +665,31 @@ public class GroupLDAPStorageMapper extends AbstractLDAPStorageMapper implements @Override public void leaveGroup(GroupModel group) { - LDAPQuery ldapQuery = createGroupQuery(true); - LDAPQueryConditionsBuilder conditionsBuilder = new LDAPQueryConditionsBuilder(); - Condition roleNameCondition = conditionsBuilder.equal(config.getGroupNameLdapAttribute(), group.getName()); + try (LDAPQuery ldapQuery = createGroupQuery(true)) { + LDAPQueryConditionsBuilder conditionsBuilder = new LDAPQueryConditionsBuilder(); + Condition roleNameCondition = conditionsBuilder.equal(config.getGroupNameLdapAttribute(), group.getName()); - String membershipUserLdapAttrName = getMembershipUserLdapAttribute(); - String membershipUserAttr = LDAPUtils.getMemberValueOfChildObject(ldapUser, config.getMembershipTypeLdapAttribute(), membershipUserLdapAttrName); - Condition membershipCondition = conditionsBuilder.equal(config.getMembershipLdapAttribute(), membershipUserAttr); + String membershipUserLdapAttrName = getMembershipUserLdapAttribute(); + String membershipUserAttr = LDAPUtils.getMemberValueOfChildObject(ldapUser, config.getMembershipTypeLdapAttribute(), membershipUserLdapAttrName); + Condition membershipCondition = conditionsBuilder.equal(config.getMembershipLdapAttribute(), membershipUserAttr); - ldapQuery.addWhereCondition(roleNameCondition).addWhereCondition(membershipCondition); - LDAPObject ldapGroup = ldapQuery.getFirstResult(); + ldapQuery.addWhereCondition(roleNameCondition).addWhereCondition(membershipCondition); + LDAPObject ldapGroup = ldapQuery.getFirstResult(); - if (ldapGroup == null) { - // Group mapping doesn't exist in LDAP. For LDAP_ONLY mode, we don't need to do anything. For READ_ONLY, delete it in local DB. - if (config.getMode() == LDAPGroupMapperMode.READ_ONLY) { - super.leaveGroup(group); - } - } else { - // Group mappings exists in LDAP. For LDAP_ONLY mode, we can just delete it in LDAP. For READ_ONLY we can't delete it -> throw error - if (config.getMode() == LDAPGroupMapperMode.READ_ONLY) { - throw new ModelException("Not possible to delete LDAP group mappings as mapper mode is READ_ONLY"); + if (ldapGroup == null) { + // Group mapping doesn't exist in LDAP. For LDAP_ONLY mode, we don't need to do anything. For READ_ONLY, delete it in local DB. + if (config.getMode() == LDAPGroupMapperMode.READ_ONLY) { + super.leaveGroup(group); + } } else { - // Delete ldap role mappings - cachedLDAPGroupMappings = null; - deleteGroupMappingInLDAP(ldapUser, ldapGroup); + // Group mappings exists in LDAP. For LDAP_ONLY mode, we can just delete it in LDAP. For READ_ONLY we can't delete it -> throw error + if (config.getMode() == LDAPGroupMapperMode.READ_ONLY) { + throw new ModelException("Not possible to delete LDAP group mappings as mapper mode is READ_ONLY"); + } else { + // Delete ldap role mappings + cachedLDAPGroupMappings = null; + deleteGroupMappingInLDAP(ldapUser, ldapGroup); + } } } } diff --git a/federation/ldap/src/main/java/org/keycloak/storage/ldap/mappers/membership/role/RoleLDAPStorageMapper.java b/federation/ldap/src/main/java/org/keycloak/storage/ldap/mappers/membership/role/RoleLDAPStorageMapper.java index 9f9607a228..e735f07dd7 100644 --- a/federation/ldap/src/main/java/org/keycloak/storage/ldap/mappers/membership/role/RoleLDAPStorageMapper.java +++ b/federation/ldap/src/main/java/org/keycloak/storage/ldap/mappers/membership/role/RoleLDAPStorageMapper.java @@ -270,10 +270,11 @@ public class RoleLDAPStorageMapper extends AbstractLDAPStorageMapper implements } public LDAPObject loadLDAPRoleByName(String roleName) { - LDAPQuery ldapQuery = createRoleQuery(true); - Condition roleNameCondition = new LDAPQueryConditionsBuilder().equal(config.getRoleNameLdapAttribute(), roleName); - ldapQuery.addWhereCondition(roleNameCondition); - return ldapQuery.getFirstResult(); + try (LDAPQuery ldapQuery = createRoleQuery(true)) { + Condition roleNameCondition = new LDAPQueryConditionsBuilder().equal(config.getRoleNameLdapAttribute(), roleName); + ldapQuery.addWhereCondition(roleNameCondition); + return ldapQuery.getFirstResult(); + } } protected List getLDAPRoleMappings(LDAPObject ldapUser) { @@ -436,31 +437,32 @@ public class RoleLDAPStorageMapper extends AbstractLDAPStorageMapper implements public void deleteRoleMapping(RoleModel role) { if (role.getContainer().equals(roleContainer)) { - LDAPQuery ldapQuery = createRoleQuery(true); - LDAPQueryConditionsBuilder conditionsBuilder = new LDAPQueryConditionsBuilder(); - Condition roleNameCondition = conditionsBuilder.equal(config.getRoleNameLdapAttribute(), role.getName()); + try (LDAPQuery ldapQuery = createRoleQuery(true)) { + LDAPQueryConditionsBuilder conditionsBuilder = new LDAPQueryConditionsBuilder(); + Condition roleNameCondition = conditionsBuilder.equal(config.getRoleNameLdapAttribute(), role.getName()); - String membershipUserAttrName = getMembershipUserLdapAttribute(); - String membershipUserAttr = LDAPUtils.getMemberValueOfChildObject(ldapUser, config.getMembershipTypeLdapAttribute(), membershipUserAttrName); + String membershipUserAttrName = getMembershipUserLdapAttribute(); + String membershipUserAttr = LDAPUtils.getMemberValueOfChildObject(ldapUser, config.getMembershipTypeLdapAttribute(), membershipUserAttrName); - Condition membershipCondition = conditionsBuilder.equal(config.getMembershipLdapAttribute(), membershipUserAttr); + Condition membershipCondition = conditionsBuilder.equal(config.getMembershipLdapAttribute(), membershipUserAttr); - ldapQuery.addWhereCondition(roleNameCondition).addWhereCondition(membershipCondition); - LDAPObject ldapRole = ldapQuery.getFirstResult(); + ldapQuery.addWhereCondition(roleNameCondition).addWhereCondition(membershipCondition); + LDAPObject ldapRole = ldapQuery.getFirstResult(); - if (ldapRole == null) { - // Role mapping doesn't exist in LDAP. For LDAP_ONLY mode, we don't need to do anything. For READ_ONLY, delete it in local DB. - if (config.getMode() == LDAPGroupMapperMode.READ_ONLY) { - super.deleteRoleMapping(role); - } - } else { - // Role mappings exists in LDAP. For LDAP_ONLY mode, we can just delete it in LDAP. For READ_ONLY we can't delete it -> throw error - if (config.getMode() == LDAPGroupMapperMode.READ_ONLY) { - throw new ModelException("Not possible to delete LDAP role mappings as mapper mode is READ_ONLY"); + if (ldapRole == null) { + // Role mapping doesn't exist in LDAP. For LDAP_ONLY mode, we don't need to do anything. For READ_ONLY, delete it in local DB. + if (config.getMode() == LDAPGroupMapperMode.READ_ONLY) { + super.deleteRoleMapping(role); + } } else { - // Delete ldap role mappings - cachedLDAPRoleMappings = null; - deleteRoleMappingInLDAP(ldapUser, ldapRole); + // Role mappings exists in LDAP. For LDAP_ONLY mode, we can just delete it in LDAP. For READ_ONLY we can't delete it -> throw error + if (config.getMode() == LDAPGroupMapperMode.READ_ONLY) { + throw new ModelException("Not possible to delete LDAP role mappings as mapper mode is READ_ONLY"); + } else { + // Delete ldap role mappings + cachedLDAPRoleMappings = null; + deleteRoleMappingInLDAP(ldapUser, ldapRole); + } } } } else { diff --git a/services/src/main/java/org/keycloak/vault/PlainTextVaultProvider.java b/services/src/main/java/org/keycloak/vault/PlainTextVaultProvider.java index 5694503793..6a61a4faa3 100644 --- a/services/src/main/java/org/keycloak/vault/PlainTextVaultProvider.java +++ b/services/src/main/java/org/keycloak/vault/PlainTextVaultProvider.java @@ -52,6 +52,7 @@ public class PlainTextVaultProvider implements VaultProvider { public VaultRawSecret obtainSecret(String vaultSecretId) { Path secretPath = resolveSecretPath(vaultSecretId); if (!Files.exists(secretPath)) { + logger.warnf("Cannot find secret %s in %s", vaultSecretId, secretPath); return DefaultVaultRawSecret.forBuffer(Optional.empty()); } diff --git a/testsuite/integration-arquillian/servers/auth-server/jboss/common/vault/master_ldap__bindCredential b/testsuite/integration-arquillian/servers/auth-server/jboss/common/vault/master_ldap__bindCredential new file mode 100644 index 0000000000..536aca34db --- /dev/null +++ b/testsuite/integration-arquillian/servers/auth-server/jboss/common/vault/master_ldap__bindCredential @@ -0,0 +1 @@ +secret \ No newline at end of file diff --git a/testsuite/integration-arquillian/servers/auth-server/jboss/common/vault/test_ldap__bindCredential b/testsuite/integration-arquillian/servers/auth-server/jboss/common/vault/test_ldap__bindCredential new file mode 100644 index 0000000000..536aca34db --- /dev/null +++ b/testsuite/integration-arquillian/servers/auth-server/jboss/common/vault/test_ldap__bindCredential @@ -0,0 +1 @@ +secret \ No newline at end of file diff --git a/testsuite/integration-arquillian/servers/auth-server/jboss/pom.xml b/testsuite/integration-arquillian/servers/auth-server/jboss/pom.xml index 64c85eb344..d4eb073ccf 100644 --- a/testsuite/integration-arquillian/servers/auth-server/jboss/pom.xml +++ b/testsuite/integration-arquillian/servers/auth-server/jboss/pom.xml @@ -222,6 +222,8 @@ test_smtp__key consumer_oidc__idp master_smtp__password + master_ldap__bindCredential + test_ldap__bindCredential diff --git a/testsuite/integration-arquillian/servers/auth-server/services/testsuite-providers/src/main/java/org/keycloak/testsuite/util/LDAPTestUtils.java b/testsuite/integration-arquillian/servers/auth-server/services/testsuite-providers/src/main/java/org/keycloak/testsuite/util/LDAPTestUtils.java index 4be3fa9123..dbdacaf627 100644 --- a/testsuite/integration-arquillian/servers/auth-server/services/testsuite-providers/src/main/java/org/keycloak/testsuite/util/LDAPTestUtils.java +++ b/testsuite/integration-arquillian/servers/auth-server/services/testsuite-providers/src/main/java/org/keycloak/testsuite/util/LDAPTestUtils.java @@ -229,23 +229,25 @@ public class LDAPTestUtils { public static void removeAllLDAPUsers(LDAPStorageProvider ldapProvider, RealmModel realm) { LDAPIdentityStore ldapStore = ldapProvider.getLdapIdentityStore(); - LDAPQuery ldapQuery = LDAPUtils.createQueryForUserSearch(ldapProvider, realm); - List allUsers = ldapQuery.getResultList(); + try (LDAPQuery ldapQuery = LDAPUtils.createQueryForUserSearch(ldapProvider, realm)) { + List allUsers = ldapQuery.getResultList(); - for (LDAPObject ldapUser : allUsers) { - ldapStore.remove(ldapUser); + for (LDAPObject ldapUser : allUsers) { + ldapStore.remove(ldapUser); + } } } public static void removeLDAPUserByUsername(LDAPStorageProvider ldapProvider, RealmModel realm, LDAPConfig config, String username) { LDAPIdentityStore ldapStore = ldapProvider.getLdapIdentityStore(); - LDAPQuery ldapQuery = LDAPUtils.createQueryForUserSearch(ldapProvider, realm); - List allUsers = ldapQuery.getResultList(); - - // This is ugly, we are iterating over the entire set of ldap users and deleting the one where the username matches. TODO: Find a better way! - for (LDAPObject ldapUser : allUsers) { - if (username.equals(LDAPUtils.getUsername(ldapUser, config))) { - ldapStore.remove(ldapUser); + try (LDAPQuery ldapQuery = LDAPUtils.createQueryForUserSearch(ldapProvider, realm)) { + List allUsers = ldapQuery.getResultList(); + + // This is ugly, we are iterating over the entire set of ldap users and deleting the one where the username matches. TODO: Find a better way! + for (LDAPObject ldapUser : allUsers) { + if (username.equals(LDAPUtils.getUsername(ldapUser, config))) { + ldapStore.remove(ldapUser); + } } } } @@ -253,20 +255,22 @@ public class LDAPTestUtils { public static void removeAllLDAPRoles(KeycloakSession session, RealmModel appRealm, ComponentModel ldapModel, String mapperName) { ComponentModel mapperModel = getSubcomponentByName(appRealm, ldapModel, mapperName); LDAPStorageProvider ldapProvider = LDAPTestUtils.getLdapProvider(session, ldapModel); - LDAPQuery roleQuery = getRoleMapper(mapperModel, ldapProvider, appRealm).createRoleQuery(false); - List ldapRoles = roleQuery.getResultList(); - for (LDAPObject ldapRole : ldapRoles) { - ldapProvider.getLdapIdentityStore().remove(ldapRole); + try (LDAPQuery roleQuery = getRoleMapper(mapperModel, ldapProvider, appRealm).createRoleQuery(false)) { + List ldapRoles = roleQuery.getResultList(); + for (LDAPObject ldapRole : ldapRoles) { + ldapProvider.getLdapIdentityStore().remove(ldapRole); + } } } public static void removeAllLDAPGroups(KeycloakSession session, RealmModel appRealm, ComponentModel ldapModel, String mapperName) { ComponentModel mapperModel = getSubcomponentByName(appRealm, ldapModel, mapperName); LDAPStorageProvider ldapProvider = LDAPTestUtils.getLdapProvider(session, ldapModel); - LDAPQuery roleQuery = getGroupMapper(mapperModel, ldapProvider, appRealm).createGroupQuery(false); - List ldapRoles = roleQuery.getResultList(); - for (LDAPObject ldapRole : ldapRoles) { - ldapProvider.getLdapIdentityStore().remove(ldapRole); + try (LDAPQuery roleQuery = getGroupMapper(mapperModel, ldapProvider, appRealm).createGroupQuery(false)) { + List ldapRoles = roleQuery.getResultList(); + for (LDAPObject ldapRole : ldapRoles) { + ldapProvider.getLdapIdentityStore().remove(ldapRole); + } } } diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/federation/ldap/LDAPSyncTest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/federation/ldap/LDAPSyncTest.java index c48507b7b6..6f56f599cd 100755 --- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/federation/ldap/LDAPSyncTest.java +++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/federation/ldap/LDAPSyncTest.java @@ -114,7 +114,7 @@ public class LDAPSyncTest extends AbstractLDAPTest { @Test public void test01LDAPSync() { // wait a bit - WaitUtils.pause(ldapRule.getSleepTime()); + WaitUtils.pause(getLDAPRule().getSleepTime()); // Sync 5 users from LDAP testingClient.server().run(session -> { @@ -151,7 +151,7 @@ public class LDAPSyncTest extends AbstractLDAPTest { }); // wait a bit - WaitUtils.pause(ldapRule.getSleepTime()); + WaitUtils.pause(getLDAPRule().getSleepTime()); testingClient.server().run(session -> { LDAPTestContext ctx = LDAPTestContext.init(session); diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/federation/ldap/LDAPVaultCredentialsTest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/federation/ldap/LDAPVaultCredentialsTest.java new file mode 100644 index 0000000000..4bde4230a4 --- /dev/null +++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/federation/ldap/LDAPVaultCredentialsTest.java @@ -0,0 +1,35 @@ +package org.keycloak.testsuite.federation.ldap; + +import org.junit.ClassRule; +import org.keycloak.models.LDAPConstants; +import org.keycloak.testsuite.util.LDAPRule; +import org.keycloak.testsuite.util.LDAPTestConfiguration; + +import java.util.Map; + +import static org.keycloak.models.LDAPConstants.BIND_CREDENTIAL; + +/** + * @author mhajas + */ +public class LDAPVaultCredentialsTest extends LDAPSyncTest { + + private static final String VAULT_EXPRESSION = "${vault.ldap_bindCredential}"; + + @ClassRule + public static LDAPRule ldapRule = new LDAPRule() { + @Override + public Map getConfig() { + + Map config = super.getConfig(); + // Replace secret with vault expression + config.put(BIND_CREDENTIAL, VAULT_EXPRESSION); + return config; + } + }.assumeTrue(LDAPTestConfiguration::isStartEmbeddedLdapServer); + + @Override + protected LDAPRule getLDAPRule() { + return ldapRule; + } +} diff --git a/testsuite/integration-arquillian/tests/base/src/test/resources/vault/master_ldap__bindCredential b/testsuite/integration-arquillian/tests/base/src/test/resources/vault/master_ldap__bindCredential new file mode 100644 index 0000000000..536aca34db --- /dev/null +++ b/testsuite/integration-arquillian/tests/base/src/test/resources/vault/master_ldap__bindCredential @@ -0,0 +1 @@ +secret \ No newline at end of file diff --git a/testsuite/integration-arquillian/tests/base/src/test/resources/vault/test_ldap__bindCredential b/testsuite/integration-arquillian/tests/base/src/test/resources/vault/test_ldap__bindCredential new file mode 100644 index 0000000000..536aca34db --- /dev/null +++ b/testsuite/integration-arquillian/tests/base/src/test/resources/vault/test_ldap__bindCredential @@ -0,0 +1 @@ +secret \ No newline at end of file