From bf3b4de680edfe03d5200bf790e0ceef77c57b7c Mon Sep 17 00:00:00 2001
From: srosse <stephane.rosse@frentix.com>
Date: Tue, 10 Nov 2020 13:25:30 +0100
Subject: [PATCH] OO-5052: allow for multiple LDAP login attributes

---
 .../org/olat/ldap/LDAPSyncConfiguration.java  | 25 +++++++----
 .../java/org/olat/ldap/manager/LDAPDAO.java   | 44 +++++++++++++++----
 .../ldap/manager/LDAPLoginManagerImpl.java    | 36 +++++++++------
 .../java/org/olat/ldap/LDAPLoginTest.java     | 26 +++++++++++
 .../org/olat/ldap/junittestdata/olattest.ldif | 15 +++++++
 .../ldap/manager/LDAPLoginManagerTest.java    | 44 +++++++++++++++++++
 6 files changed, 159 insertions(+), 31 deletions(-)

diff --git a/src/main/java/org/olat/ldap/LDAPSyncConfiguration.java b/src/main/java/org/olat/ldap/LDAPSyncConfiguration.java
index d3901fff406..97271ddfdea 100644
--- a/src/main/java/org/olat/ldap/LDAPSyncConfiguration.java
+++ b/src/main/java/org/olat/ldap/LDAPSyncConfiguration.java
@@ -415,10 +415,24 @@ public class LDAPSyncConfiguration {
 		this.ldapUserPasswordAttribute = attribute;
 	}
 
+	public List<String> getLdapUserLoginAttributes() {
+		if(StringHelper.containsNonWhitespace(ldapUserLoginAttribute)) {
+			String[] attrs = ldapUserLoginAttribute.split("[,]");
+			return List.of(attrs);
+		}
+		return List.of();
+	}
+	
+	/**
+	 * @return One or several attributes comma separated
+	 */
 	public String getLdapUserLoginAttribute() {
 		return ldapUserLoginAttribute;
 	}
 
+	/**
+	 * @param ldapUserLoginAttribute One or several attributes comma separated
+	 */
 	public void setLdapUserLoginAttribute(String ldapUserLoginAttribute) {
 		this.ldapUserLoginAttribute = ldapUserLoginAttribute;
 	}
@@ -511,11 +525,8 @@ public class LDAPSyncConfiguration {
 				}
 			}
 			if (!propertyExists) {
-				log.error("Error in checkIfOlatPropertiesExists(): configured LDAP attribute::"
-								+ ldapAttribute
-								+ " configured to map to OLAT user property::"
-								+ olatProperty
-								+ " but no such user property configured in olat_userconfig.xml");
+				log.error("Error in checkIfOlatPropertiesExists(): configured LDAP attribute::{} configured to map to OLAT user property::{} but no such user property configured in olat_userconfig.xml",
+						ldapAttribute, olatProperty);
 				return false;				
 			}
 		}
@@ -543,9 +554,7 @@ public class LDAPSyncConfiguration {
 				}
 			}
 			if ( ! propertyExists ) {
-				log.error("Error in checkIfStaticOlatPropertiesExists(): configured static OLAT user property::"
-						+ olatProperty
-						+ " is not configured in olat_userconfig.xml");
+				log.error("Error in checkIfStaticOlatPropertiesExists(): configured static OLAT user property::{} is not configured in olat_userconfig.xml", olatProperty);
 				return false;				
 			}			
 		}
diff --git a/src/main/java/org/olat/ldap/manager/LDAPDAO.java b/src/main/java/org/olat/ldap/manager/LDAPDAO.java
index 7d816283a5c..a3ef6599140 100644
--- a/src/main/java/org/olat/ldap/manager/LDAPDAO.java
+++ b/src/main/java/org/olat/ldap/manager/LDAPDAO.java
@@ -187,13 +187,11 @@ public class LDAPDAO {
 					}
 				}
 			} catch (SizeLimitExceededException e) {
-				log.error("SizeLimitExceededException after "
-								+ counter
-								+ " records when getting all users from LDAP, reconfigure your LDAP server, hints: http://www.ldapbrowser.com/forum/viewtopic.php?t=14", e);
+				log.error("SizeLimitExceededException after {} records when getting all users from LDAP, reconfigure your LDAP server, hints: http://www.ldapbrowser.com/forum/viewtopic.php?t=14", counter, e);
 			} catch (NamingException e) {
-				log.error("NamingException when trying to search users from LDAP using ldapBase::" + ldapBase + " on row::" + counter, e);
+				log.error("NamingException when trying to search users from LDAP using ldapBase::{} on row::{}", ldapBase, counter, e);
 			} catch (Exception e) {
-				log.error("Exception when trying to search users from LDAP using ldapBase::" + ldapBase + " on row::" + counter, e);
+				log.error("Exception when trying to search users from LDAP using ldapBase::{} on row::{}", ldapBase, counter, e);
 			}
 			log.debug("finished search for ldapBase:: {}", ldapBase);
 		}
@@ -202,8 +200,8 @@ public class LDAPDAO {
 	public String searchUserForLogin(String login, DirContext ctx) {
 		if(ctx == null) return null;
 		
-		String ldapUserIDAttribute = syncConfiguration.getLdapUserLoginAttribute();
-		String filter = buildSearchUserFilter(ldapUserIDAttribute, login);
+		List<String> ldapUserIDAttributes = syncConfiguration.getLdapUserLoginAttributes();
+		String filter = buildSearchUserFilter(ldapUserIDAttributes, login);
 		return searchUserDN(login, filter, ctx);
 	}
 	
@@ -236,7 +234,7 @@ public class LDAPDAO {
 					break;
 				}
 			} catch (NamingException e) {
-				log.error("NamingException when trying to bind user with username::" + username + " on ldapBase::" + ldapBase, e);
+				log.error("NamingException when trying to bind user with username::{} on ldapBase::{}", username, ldapBase, e);
 			}
 		}
 		
@@ -257,6 +255,34 @@ public class LDAPDAO {
 		return filter.toString();
 	}
 	
+	protected String buildSearchUserFilter(List<String> attributes, String uid) {
+		if(attributes == null || attributes.isEmpty()) {
+			return null;
+		} else if(attributes.size() == 1) {
+			return buildSearchUserFilter(attributes.get(0), uid);
+		}
+		
+		String ldapUserFilter = syncConfiguration.getLdapUserFilter();
+		StringBuilder filter = new StringBuilder(64);
+		if (ldapUserFilter != null) {
+			// merge preconfigured filter (e.g. object class, group filters) with username using AND rule
+			filter.append("(&").append(ldapUserFilter);	
+		}
+		
+		filter.append("(|");
+		for(int i=0; i<attributes.size(); i++) {
+			String attribute = attributes.get(i);
+			filter.append("(").append(attribute).append("=").append(uid).append(")");
+			
+		}
+		filter.append(")");
+		
+		if (ldapUserFilter != null) {
+			filter.append(")");	
+		}
+		return filter.toString();
+	}
+	
 	/**
 	 * 
 	 * Creates list of all LDAP Users or changed Users since syncTime
@@ -311,7 +337,7 @@ public class LDAPDAO {
 		searchInLdap(userVisitor, filter.toString(), userAttrs, ctx);
 		List<LDAPUser> ldapUserList = userVisitor.getLdapUserList();
 		if(debug) {
-			log.debug("attrib search returned " + ldapUserList.size() + " results");
+			log.debug("attrib search returned {} results", ldapUserList.size());
 		}
 		return ldapUserList;
 	}
diff --git a/src/main/java/org/olat/ldap/manager/LDAPLoginManagerImpl.java b/src/main/java/org/olat/ldap/manager/LDAPLoginManagerImpl.java
index e169572a0cb..1e6d39d17df 100644
--- a/src/main/java/org/olat/ldap/manager/LDAPLoginManagerImpl.java
+++ b/src/main/java/org/olat/ldap/manager/LDAPLoginManagerImpl.java
@@ -326,8 +326,8 @@ public class LDAPLoginManagerImpl implements LDAPLoginManager, AuthenticationPro
 		}
 		String userDN = ldapDao.searchUserForLogin(login, ctx);
 		if (userDN == null) {
-			log.info("Error when trying to bind user with username::" + login + " - user not found on LDAP server"
-					+ (ldapLoginModule.isCacheLDAPPwdAsOLATPwdOnLogin() ? ", trying with OLAT login provider" : ""));
+			log.info("Error when trying to bind user with username::{} - user not found on LDAP server {}"
+					, login, (ldapLoginModule.isCacheLDAPPwdAsOLATPwdOnLogin() ? ", trying with OLAT login provider" : ""));
 			errors.insert("Username or password incorrect");
 			return null;
 		}
@@ -843,15 +843,24 @@ public class LDAPLoginManagerImpl implements LDAPLoginManager, AuthenticationPro
 			return null;
 		}
 		
-		String token = getAttributeValue(attrs.get(syncConfiguration.getLdapUserLoginAttribute()));
-		Authentication ldapAuth = authenticationDao.getAuthentication(token, LDAPAuthenticationController.PROVIDER_LDAP);
-		if(ldapAuth != null) {
-			return ldapAuth;
+		String uidAttribute = syncConfiguration.getOlatPropertyToLdapAttribute(LDAPConstants.LDAP_USER_IDENTIFYER);
+		List<String> loginAttributes = syncConfiguration.getLdapUserLoginAttributes();
+		
+		String token = null;
+		for(String loginAttribute:loginAttributes) {
+			String loginToken = getAttributeValue(attrs.get(loginAttribute));
+			Authentication ldapAuth = authenticationDao.getAuthentication(loginToken, LDAPAuthenticationController.PROVIDER_LDAP);
+			if(ldapAuth != null) {
+				return ldapAuth;
+			}
+			// prefer the not uid attribute
+			if((loginAttributes.size() == 1 || !loginAttribute.equals(uidAttribute)) && token == null) {
+				token = loginToken;
+			}
 		}
 
-		String uid = getAttributeValue(attrs.get(syncConfiguration
-				.getOlatPropertyToLdapAttribute(LDAPConstants.LDAP_USER_IDENTIFYER)));
-		ldapAuth = authenticationDao.getAuthentication(uid, LDAPAuthenticationController.PROVIDER_LDAP);
+		String uid = getAttributeValue(attrs.get(uidAttribute));
+		Authentication ldapAuth = authenticationDao.getAuthentication(uid, LDAPAuthenticationController.PROVIDER_LDAP);
 		if(ldapAuth != null) {
 			if(StringHelper.containsNonWhitespace(token) && !token.equals(ldapAuth.getAuthusername())) {
 				ldapAuth.setAuthusername(token);
@@ -912,10 +921,8 @@ public class LDAPLoginManagerImpl implements LDAPLoginManager, AuthenticationPro
 		List<String> returningAttrList = new ArrayList<>(2);
 		String userID = syncConfiguration.getOlatPropertyToLdapAttribute(LDAPConstants.LDAP_USER_IDENTIFYER);
 		returningAttrList.add(userID);
-		String loginAttr = syncConfiguration.getLdapUserLoginAttribute();
-		if(loginAttr != null && !loginAttr.equals(userID)) {
-			returningAttrList.add(loginAttr);
-		}
+		List<String> loginAttrs = syncConfiguration.getLdapUserLoginAttributes();
+		returningAttrList.addAll(loginAttrs);
 		
 		String[] returningAttrs = returningAttrList.toArray(new String[returningAttrList.size()]);
 		String userFilter = syncConfiguration.getLdapUserFilter();
@@ -1580,8 +1587,9 @@ public class LDAPLoginManagerImpl implements LDAPLoginManager, AuthenticationPro
 			log.error("could not bind to ldap");
 		}
 		
-		String ldapUserIDAttribute = syncConfiguration.getLdapUserLoginAttribute();
 		Authentication authentication = authenticationDao.getAuthentication(ident, LDAPAuthenticationController.PROVIDER_LDAP);
+		
+		List<String> ldapUserIDAttribute = syncConfiguration.getLdapUserLoginAttributes();
 		String filter = ldapDao.buildSearchUserFilter(ldapUserIDAttribute, authentication.getAuthusername());
 		
 		List<Attributes> ldapUserAttrs = new ArrayList<>();
diff --git a/src/test/java/org/olat/ldap/LDAPLoginTest.java b/src/test/java/org/olat/ldap/LDAPLoginTest.java
index 8abfab6271c..91343f0697a 100644
--- a/src/test/java/org/olat/ldap/LDAPLoginTest.java
+++ b/src/test/java/org/olat/ldap/LDAPLoginTest.java
@@ -116,4 +116,30 @@ public class LDAPLoginTest extends OlatTestCase {
 		
 		syncConfiguration.setLdapUserLoginAttribute(currentLoginAttr);
 	}
+	
+	@Test
+	public void userBindLoginWithTwoLoginAttributes() throws Exception {
+		Assume.assumeTrue(ldapLoginModule.isLDAPEnabled());
+		String currentLoginAttr = syncConfiguration.getLdapUserLoginAttribute();
+
+		// use SN as login attribute
+		syncConfiguration.setLdapUserLoginAttribute("sn," + currentLoginAttr);
+
+		// try sn attribute
+		LDAPError errors = new LDAPError();
+		Attributes attrs = ldapManager.bindUser("Forster", "olat", errors);
+		Assert.assertNotNull(attrs);
+		Assert.assertEquals("dforster", attrs.get("uid").get());
+		Assert.assertTrue(errors.isEmpty());
+		
+		// try uid attribute
+		LDAPError altErrors = new LDAPError();
+		Attributes altAttrs = ldapManager.bindUser("dforster", "olat", altErrors);
+		Assert.assertNotNull(altAttrs);
+		Assert.assertEquals("Forster", altAttrs.get("sn").get());
+		Assert.assertEquals("dforster", altAttrs.get("uid").get());
+		Assert.assertTrue(altErrors.isEmpty());
+
+		syncConfiguration.setLdapUserLoginAttribute(currentLoginAttr);
+	}
 }
diff --git a/src/test/java/org/olat/ldap/junittestdata/olattest.ldif b/src/test/java/org/olat/ldap/junittestdata/olattest.ldif
index e560c0c1b43..ebb9a88d44e 100644
--- a/src/test/java/org/olat/ldap/junittestdata/olattest.ldif
+++ b/src/test/java/org/olat/ldap/junittestdata/olattest.ldif
@@ -210,6 +210,21 @@ uid: cguerrera
 userPassword: olat
 modifyTimestamp: 20100630180000Z
 
+dn: uid=uschelling,ou=person,dc=olattest,dc=org
+objectClass: organizationalPerson
+objectClass: person
+objectClass: inetOrgPerson
+objectClass: top
+cn: Ursula Schelling
+givenname: Ursula
+labeleduri: https://www.openolat.com/uschelling
+mail: uschelling@openolat.com
+o: Informatik
+sn: Schelling
+uid: uschelling
+userPassword: olat
+modifyTimestamp: 20100630180000Z
+
 dn: uid=lsalathe,ou=person,dc=olattest,dc=org
 objectClass: organizationalPerson
 objectClass: person
diff --git a/src/test/java/org/olat/ldap/manager/LDAPLoginManagerTest.java b/src/test/java/org/olat/ldap/manager/LDAPLoginManagerTest.java
index 1bbbeadf385..c7c7878bc8d 100644
--- a/src/test/java/org/olat/ldap/manager/LDAPLoginManagerTest.java
+++ b/src/test/java/org/olat/ldap/manager/LDAPLoginManagerTest.java
@@ -670,6 +670,50 @@ public class LDAPLoginManagerTest extends OlatTestCase {
 		ldapLoginModule.setConvertExistingLocalUsersToLDAPUsers(currentConvertExistingLocalUsers);
 	}
 	
+	@Test
+	public void findIdentityByLdapAuthenticationConvertFromOlat2Logins() {
+		LDAPError ldapError = new LDAPError();
+		
+		// Take an LDAP user, delete her LDAP authentication
+		List<String> firstLastName = Collections.singletonList("Ursula Schelling");
+		List<FindNamedIdentity> foundIdentities = securityManager.findIdentitiesBy(firstLastName);
+		Assert.assertEquals(1, foundIdentities.size());
+		
+		Identity identity = foundIdentities.get(0).getIdentity();
+		Authentication authentication = securityManager.findAuthentication(identity, LDAPAuthenticationController.PROVIDER_LDAP);
+		Assert.assertNotNull(authentication);
+		Assert.assertEquals("uschelling", authentication.getAuthusername());
+		securityManager.deleteAuthentication(authentication);
+		securityManager.createAndPersistAuthentication(identity, "OLAT", "uschelling", "secret", Algorithm.sha512);
+		dbInstance.commitAndCloseSession();
+		
+		// convert users to LDAP
+		boolean currentConvertExistingLocalUsers = ldapLoginModule.isConvertExistingLocalUsersToLDAPUsers();
+		ldapLoginModule.setConvertExistingLocalUsersToLDAPUsers(true);
+		String currentLoginAttribute = syncConfiguration.getLdapUserLoginAttribute();
+		syncConfiguration.setLdapUserLoginAttribute("sn," + currentLoginAttribute);
+		
+		// use uid as login attribute	
+		Attributes attrs = ldapManager.bindUser("uschelling", "olat", ldapError);
+		Assert.assertNotNull(attrs);
+
+		LDAPError errors = new LDAPError();
+		Identity convertedIdentity = ldapManager.findIdentityByLdapAuthentication(attrs, errors);
+		Assert.assertNotNull(convertedIdentity);
+		Assert.assertEquals("Ursula", convertedIdentity.getUser().getFirstName());
+		Assert.assertEquals("Schelling", convertedIdentity.getUser().getLastName());
+
+		Authentication updatedAuthentication = securityManager.findAuthentication(identity, LDAPAuthenticationController.PROVIDER_LDAP);
+		Assert.assertNotNull(updatedAuthentication);
+		Assert.assertEquals("Schelling", updatedAuthentication.getAuthusername());
+
+		// revert configuration
+		ldapLoginModule.setConvertExistingLocalUsersToLDAPUsers(currentConvertExistingLocalUsers);
+		syncConfiguration.setLdapUserLoginAttribute(currentLoginAttribute);
+	}
+	
+	
+	
 	@Test
 	public void findIdentityByLdapAuthenticationConvertFromIdentityName() {
 		// Take an LDAP user, delete her LDAP authentication
-- 
GitLab