OO-4794: adapt deletion process and add unit test for it

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

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")

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);
 	

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<>();

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");

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();

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());