diff --git a/src/main/java/org/olat/basesecurity/BaseSecurityManager.java b/src/main/java/org/olat/basesecurity/BaseSecurityManager.java index 65635d6948a5054a304f33fd70cfc652eb5c1e8c..41566256ffd9d6e49936fa28203bf250e06128c6 100644 --- a/src/main/java/org/olat/basesecurity/BaseSecurityManager.java +++ b/src/main/java/org/olat/basesecurity/BaseSecurityManager.java @@ -767,24 +767,7 @@ public class BaseSecurityManager implements BaseSecurity, UserDataDeletable { @Override public Authentication findAuthentication(IdentityRef identity, String provider) { - if (identity==null) { - throw new IllegalArgumentException("identity must not be null"); - } - - StringBuilder sb = new StringBuilder(); - sb.append("select auth from ").append(AuthenticationImpl.class.getName()) - .append(" as auth where auth.identity.key=:identityKey and auth.provider=:provider"); - - List<Authentication> results = dbInstance.getCurrentEntityManager() - .createQuery(sb.toString(), Authentication.class) - .setParameter("identityKey", identity.getKey()) - .setParameter("provider", provider) - .getResultList(); - if (results == null || results.isEmpty()) return null; - if (results.size() > 1) { - throw new AssertException("Found more than one Authentication for a given subject and a given provider."); - } - return results.get(0); + return authenticationDao.getAuthentication(identity, provider); } @Override diff --git a/src/main/java/org/olat/basesecurity/manager/AuthenticationDAO.java b/src/main/java/org/olat/basesecurity/manager/AuthenticationDAO.java index 6c5a5e456cbaa32353ee9289b5dbaf7f44cbd8fc..0dd9f9088e62efe47b9e71e87f1708571849f984 100644 --- a/src/main/java/org/olat/basesecurity/manager/AuthenticationDAO.java +++ b/src/main/java/org/olat/basesecurity/manager/AuthenticationDAO.java @@ -32,6 +32,7 @@ import org.olat.basesecurity.IdentityRef; import org.olat.core.commons.persistence.DB; import org.olat.core.id.Identity; import org.olat.core.logging.AssertException; +import org.olat.core.util.StringHelper; import org.springframework.beans.factory.annotation.Autowired; import org.springframework.stereotype.Service; @@ -126,6 +127,48 @@ public class AuthenticationDAO { .getResultList(); } + /** + * + * @param provider The authentication provider + * @return A list of identities (the user is not fetched) + */ + public List<Authentication> getAuthentications(String provider) { + StringBuilder sb = new StringBuilder(256); + sb.append("select auth from ").append(AuthenticationImpl.class.getName()).append(" as auth") + .append(" inner join fetch auth.identity as ident") + .append(" inner join fetch ident.user as identUser") + .append(" where auth.provider=:provider"); + return dbInstance.getCurrentEntityManager() + .createQuery(sb.toString(), Authentication.class) + .setParameter("provider", provider) + .getResultList(); + } + + public Authentication getAuthentication(IdentityRef identity, String provider) { + if (identity == null || !StringHelper.containsNonWhitespace(provider)) { + throw new IllegalArgumentException("identity must not be null"); + } + + StringBuilder sb = new StringBuilder(); + sb.append("select auth from ").append(AuthenticationImpl.class.getName()).append(" as auth") + .append(" inner join auth.identity as ident") + .append(" inner join ident.user as user") + .append(" where auth.identity.key=:identityKey and auth.provider=:provider"); + + List<Authentication> results = dbInstance.getCurrentEntityManager() + .createQuery(sb.toString(), Authentication.class) + .setParameter("identityKey", identity.getKey()) + .setParameter("provider", provider) + .getResultList(); + if (results == null || results.isEmpty()) { + return null; + } + if (results.size() > 1) { + throw new AssertException("Found more than one Authentication for a given subject and a given provider."); + } + return results.get(0); + } + public Authentication getAuthentication(String authUsername, String provider) { StringBuilder sb = new StringBuilder(256); sb.append("select auth from ").append(AuthenticationImpl.class.getName()).append(" as auth") diff --git a/src/main/java/org/olat/ldap/LDAPLoginManager.java b/src/main/java/org/olat/ldap/LDAPLoginManager.java index fb2fd84a067c9fa998522e8ee37ba00709f0f61f..a937094eb06c88f854d11195074ed513deb943cb 100644 --- a/src/main/java/org/olat/ldap/LDAPLoginManager.java +++ b/src/main/java/org/olat/ldap/LDAPLoginManager.java @@ -51,7 +51,7 @@ public interface LDAPLoginManager { public Map<String,String> prepareUserPropertyForSync(Attributes attributes, Identity identity); - public List<Identity> getIdentitysDeletedInLdap(LdapContext ctx); + public List<Identity> getIdentitiesDeletedInLdap(LdapContext ctx); public Identity findIdentityByLdapAuthentication(Attributes attrs, LDAPError errors); diff --git a/src/main/java/org/olat/ldap/manager/LDAPLoginManagerImpl.java b/src/main/java/org/olat/ldap/manager/LDAPLoginManagerImpl.java index 9657a9724121109c87d5e20f37ccc8a722199c95..d2e893f920e82124ffcd5a08b7902862638935f3 100644 --- a/src/main/java/org/olat/ldap/manager/LDAPLoginManagerImpl.java +++ b/src/main/java/org/olat/ldap/manager/LDAPLoginManagerImpl.java @@ -409,7 +409,9 @@ public class LDAPLoginManagerImpl implements LDAPLoginManager, AuthenticationPro */ @Override public boolean changePassword(Identity identity, String pwd, LDAPError errors) { - String uid = identity.getName(); + String uid = identity.getName();//TODO username + + String ldapUserPasswordAttribute = syncConfiguration.getLdapUserPasswordAttribute(); try { LdapContext ctx = bindSystem(); @@ -724,10 +726,12 @@ public class LDAPLoginManagerImpl implements LDAPLoginManager, AuthenticationPro LdapContext ctx = bindSystem(); if (ctx == null) { log.error("could not bind to ldap"); + return; } String ldapUserIDAttribute = syncConfiguration.getOlatPropertyToLdapAttribute(LDAPConstants.LDAP_USER_IDENTIFYER); - String filter = ldapDao.buildSearchUserFilter(ldapUserIDAttribute, identity.getName()); + Authentication authentication = authenticationDao.getAuthentication(identity, LDAPAuthenticationController.PROVIDER_LDAP); + String filter = ldapDao.buildSearchUserFilter(ldapUserIDAttribute, authentication.getAuthusername()); boolean withCoacheOfGroups = StringHelper.containsNonWhitespace(syncConfiguration.getCoachedGroupAttribute()); List<String> ldapBases = syncConfiguration.getLdapBases(); @@ -874,10 +878,11 @@ public class LDAPLoginManagerImpl implements LDAPLoginManager, AuthenticationPro * @throws NamingException */ @Override - public List<Identity> getIdentitysDeletedInLdap(LdapContext ctx) { + public List<Identity> getIdentitiesDeletedInLdap(LdapContext ctx) { if (ctx == null) { - return null; + return Collections.emptyList(); } + // Find all LDAP Users String userID = syncConfiguration.getOlatPropertyToLdapAttribute(LDAPConstants.LDAP_USER_IDENTIFYER); String userFilter = syncConfiguration.getLdapUserFilter(); @@ -897,18 +902,18 @@ public class LDAPLoginManagerImpl implements LDAPLoginManager, AuthenticationPro }, (userFilter == null ? "" : userFilter), new String[] { userID }, ctx); if (ldapList.isEmpty()) { - log.warn("No users in LDAP found, can't create deletionList!!"); + log.warn("No users in LDAP found, can't create the deletion list."); return Collections.emptyList(); } - + List<Identity> identityListToDelete = new ArrayList<>(); - List<Identity> olatListIdentity = authenticationDao.getIdentitiesWithAuthentication(LDAPAuthenticationController.PROVIDER_LDAP); - for (Identity ida:olatListIdentity) { - // compare usernames with lowercase - if (!ldapList.contains(ida.getName().toLowerCase())) { - identityListToDelete.add(ida); + List<Authentication> ldapAuthentications = authenticationDao.getAuthentications(LDAPAuthenticationController.PROVIDER_LDAP); + for (Authentication ldapAuthentication:ldapAuthentications) { + if (!ldapList.contains(ldapAuthentication.getAuthusername().toLowerCase())) { + identityListToDelete.add(ldapAuthentication.getIdentity()); } } + dbInstance.commitAndCloseSession(); return identityListToDelete; } @@ -1167,7 +1172,7 @@ public class LDAPLoginManagerImpl implements LDAPLoginManager, AuthenticationPro break; } default: { - log.error("LDAP Role synchronization not supported for: " + role); + log.error("LDAP Role synchronization not supported for: {}", role); } } } @@ -1175,11 +1180,12 @@ public class LDAPLoginManagerImpl implements LDAPLoginManager, AuthenticationPro private void doBatchSyncDeletedUsers(LdapContext ctx, String sinceSentence) { // create User to Delete List - List<Identity> deletedUserList = getIdentitysDeletedInLdap(ctx); + List<Identity> deletedUserList = getIdentitiesDeletedInLdap(ctx); // delete old users if (deletedUserList == null || deletedUserList.isEmpty()) { - log.info("LDAP batch sync: no users to delete" + sinceSentence); + log.info("LDAP batch sync: no users to delete {}", sinceSentence); } else { + int deletedUserListSize = deletedUserList.size(); if (ldapLoginModule.isDeleteRemovedLDAPUsersOnSync()) { // check if more not more than the defined percentages of // users managed in LDAP should be deleted @@ -1189,31 +1195,24 @@ public class LDAPLoginManagerImpl implements LDAPLoginManager, AuthenticationPro if (olatListIdentity.isEmpty()) log.info("No users managed by LDAP, can't delete users"); else { - int prozente = (int) (((float)deletedUserList.size() / (float) olatListIdentity.size())*100); - if (prozente >= ldapLoginModule.getDeleteRemovedLDAPUsersPercentage()) { - log.info("LDAP batch sync: more than " - + ldapLoginModule.getDeleteRemovedLDAPUsersPercentage() - + "% of LDAP managed users should be deleted. Please use Admin Deletion Job. Or increase deleteRemovedLDAPUsersPercentage. " - + prozente - + "% tried to delete."); + int prozente = (int) (((float)deletedUserListSize / (float) olatListIdentity.size()) * 100.0); + int cutValue = ldapLoginModule.getDeleteRemovedLDAPUsersPercentage(); + if (prozente >= cutValue) { + log.info("LDAP batch sync: more than {}% of LDAP managed users should be deleted. Please use Admin Deletion Job. Or increase deleteRemovedLDAPUsersPercentage. {}% tried to delete.", cutValue, prozente); } else { // delete users deleteIdentities(deletedUserList, null); - log.info("LDAP batch sync: " + deletedUserList.size() + " users deleted" + sinceSentence); + log.info("LDAP batch sync: {} users deleted {}", deletedUserListSize, sinceSentence); } } } else { // Do nothing, only log users to logfile - StringBuilder users = new StringBuilder(); + StringBuilder users = new StringBuilder(deletedUserListSize * 42); for (Identity toBeDeleted : deletedUserList) { - users.append(toBeDeleted.getName()).append(','); + users.append(toBeDeleted.getKey()).append(','); } - log.info("LDAP batch sync: " - + deletedUserList.size() - + " users detected as to be deleted" - + sinceSentence - + ". Automatic deleting is disabled in LDAPLoginModule, delete these users manually::[" - + users.toString() + "]"); + log.info("LDAP batch sync: {} users detected as to be deleted {}. Automatic deleting is disabled in LDAPLoginModule, delete these users manually::[{}]", + deletedUserListSize, sinceSentence, users); } } dbInstance.commitAndCloseSession(); @@ -1558,7 +1557,7 @@ public class LDAPLoginManagerImpl implements LDAPLoginManager, AuthenticationPro } String ldapUserIDAttribute = syncConfiguration.getLdapUserLoginAttribute(); - Authentication authentication = securityManager.findAuthentication(ident, LDAPAuthenticationController.PROVIDER_LDAP); + Authentication authentication = authenticationDao.getAuthentication(ident, LDAPAuthenticationController.PROVIDER_LDAP); String filter = ldapDao.buildSearchUserFilter(ldapUserIDAttribute, authentication.getAuthusername()); List<Attributes> ldapUserAttrs = new ArrayList<>(); diff --git a/src/main/java/org/olat/ldap/ui/LDAPAdminController.java b/src/main/java/org/olat/ldap/ui/LDAPAdminController.java index 4c8cc68041fa4f9f9dd417fd9596ec2303799b7b..b5df6f6f3bf7cc5e87b718b3eccd4c09ad74fca5 100644 --- a/src/main/java/org/olat/ldap/ui/LDAPAdminController.java +++ b/src/main/java/org/olat/ldap/ui/LDAPAdminController.java @@ -190,7 +190,7 @@ public class LDAPAdminController extends BasicController implements GenericEvent // get deleted users List<Identity> identitiesToDelete = null; try { - identitiesToDelete = ldapLoginManager.getIdentitysDeletedInLdap(ctx); + identitiesToDelete = ldapLoginManager.getIdentitiesDeletedInLdap(ctx); ctx.close(); } catch (NamingException e) { showError("delete.error.connection.close"); diff --git a/src/test/java/org/olat/basesecurity/manager/AuthenticationDAOTest.java b/src/test/java/org/olat/basesecurity/manager/AuthenticationDAOTest.java index 722c3b224e3d43712170ea436ce81476d9105be7..eb116d908e5f8e36009ec54170264f3cd88f6f33 100644 --- a/src/test/java/org/olat/basesecurity/manager/AuthenticationDAOTest.java +++ b/src/test/java/org/olat/basesecurity/manager/AuthenticationDAOTest.java @@ -70,6 +70,17 @@ public class AuthenticationDAOTest extends OlatTestCase { Assert.assertEquals(newToken, updatedAuth.getCredential()); } + @Test + public void getAuthenticationIdentityProvider() { + IdentityWithLogin ident = JunitTestHelper.createAndPersistRndUser("authdao-1-"); + dbInstance.commitAndCloseSession(); + + //check if the new token was saved + Authentication authentication = authenticationDao.getAuthentication(ident.getIdentity(), "OLAT"); + Assert.assertEquals(ident.getIdentity(), authentication.getIdentity()); + Assert.assertEquals(ident.getLogin(), authentication.getAuthusername()); + } + @Test public void getIdentitiesWithAuthentication() { String token = UUID.randomUUID().toString(); @@ -142,6 +153,19 @@ public class AuthenticationDAOTest extends OlatTestCase { Assert.assertEquals(ident.getIdentity(), olatAuthentications.get(0).getIdentity()); } + @Test + public void getAuthenticationsByProvider() { + IdentityWithLogin ident = JunitTestHelper.createAndPersistRndUser("authdao-2-"); + dbInstance.commitAndCloseSession(); + Assert.assertNotNull(ident); + + List<Authentication> olatAuthentications = authenticationDao.getAuthentications("OLAT"); + Assert.assertNotNull(olatAuthentications); + for(Authentication authentication:olatAuthentications) { + Assert.assertEquals("OLAT", authentication.getProvider()); + } + } + @Test public void getAuthenticationsByAuthusername_providersList() { String token = UUID.randomUUID().toString(); diff --git a/src/test/java/org/olat/ldap/manager/LDAPLoginManagerTest.java b/src/test/java/org/olat/ldap/manager/LDAPLoginManagerTest.java index e107d4b668ec7b4e245d3c8e1dfd1c042fcb95f5..a8bbc666384c147c9dfd3ac7c4e616f2dc31bc7c 100644 --- a/src/test/java/org/olat/ldap/manager/LDAPLoginManagerTest.java +++ b/src/test/java/org/olat/ldap/manager/LDAPLoginManagerTest.java @@ -22,9 +22,11 @@ package org.olat.ldap.manager; import java.util.ArrayList; import java.util.Collections; import java.util.List; +import java.util.UUID; import java.util.stream.Collectors; import javax.naming.directory.Attributes; +import javax.naming.ldap.LdapContext; import org.junit.Assert; import org.junit.Assume; @@ -47,6 +49,7 @@ import org.olat.ldap.LDAPLoginManager; import org.olat.ldap.LDAPLoginModule; import org.olat.ldap.LDAPSyncConfiguration; import org.olat.ldap.ui.LDAPAuthenticationController; +import org.olat.test.JunitTestHelper; import org.olat.test.OlatTestCase; import org.olat.user.UserManager; import org.springframework.beans.factory.annotation.Autowired; @@ -106,6 +109,27 @@ public class LDAPLoginManagerTest extends OlatTestCase { Assert.assertNotNull(identity); } + @Test + public void getIdentitiesDeletedInLdap() { + Assume.assumeTrue(ldapLoginModule.isLDAPEnabled()); + Identity orphan = JunitTestHelper.createAndPersistIdentityAsRndUser("ldap-orphan"); + securityManager.createAndPersistAuthentication(orphan, LDAPAuthenticationController.PROVIDER_LDAP, + UUID.randomUUID().toString(), null, null); + dbInstance.commitAndCloseSession(); + + LdapContext ctx = ldapManager.bindSystem(); + List<Identity> identities = ldapManager.getIdentitiesDeletedInLdap(ctx); + + Assert.assertNotNull(identities); + Assert.assertTrue(identities.contains(orphan)); + + // historic + Identity identity1 = userManager.findUniqueIdentityByEmail("hhuerlimann@openolat.com"); + Assert.assertFalse(identities.contains(identity1)); + Identity identity2 = userManager.findUniqueIdentityByEmail("ahentschel@openolat.com"); + Assert.assertFalse(identities.contains(identity2)); + } + @Test public void doSyncSingleUserWithLoginAttribute() throws LDAPException { Assume.assumeTrue(ldapLoginModule.isLDAPEnabled());