Merge pull request #1149 from fzs/fixLDAPbinding

Fix LDAP binding strategies

3 files changed, 623 insertions, 144 deletions

diff --git a/src/main/distrib/data/defaults.properties b/src/main/distrib/data/defaults.properties
index 208fd992..04166342 100644
--- a/src/main/distrib/data/defaults.properties
+++ b/src/main/distrib/data/defaults.properties
@@ -1812,6 +1812,10 @@ realm.salesforce.orgId = 0
 realm.ldap.server = ldap://localhost
 
 # Login username for LDAP searches.
+# This is usually a user with permissions to search LDAP users and groups.
+# It must have at least have the permission to search users. If it does not
+# have permission to search groups, the normal user logging in must have
+# the permission in LDAP to search groups.
 # If this value is unspecified, anonymous LDAP login will be used.
 # 
 # e.g. mydomain\\username
@@ -1824,8 +1828,14 @@ realm.ldap.username = cn=Directory Manager
 # SINCE 1.0.0
 realm.ldap.password = password
 
-# Bind pattern for Authentication.
-# Allow to directly authenticate an user without LDAP Searches.
+# Bind pattern for user authentication.
+# Allow to directly authenticate an user without searching for it in LDAP.
+# Use this if the LDAP server does not allow anonymous access and you don't
+# want to use a specific account to run searches. When set, it will override
+# the settings realm.ldap.username and realm.ldap.password.
+# This requires that all relevant user entries are children to the same DN,
+# and that logging users have permission to search for their groups in LDAP.
+# This will disable synchronization as a specific LDAP account is needed for that.
 # 
 # e.g. CN=${username},OU=Users,OU=UserControl,OU=MyOrganization,DC=MyDomain
 #
@@ -1941,6 +1951,9 @@ realm.ldap.email = email
 realm.ldap.uid = uid
 
 # Defines whether to synchronize all LDAP users and teams into the user service
+# This requires either anonymous LDAP access or that a specific account is set
+# in realm.ldap.username and realm.ldap.password, that has permission to read
+# users and groups in LDAP.
 #
 # Valid values: true, false
 # If left blank, false is assumed
diff --git a/src/main/java/com/gitblit/auth/LdapAuthProvider.java b/src/main/java/com/gitblit/auth/LdapAuthProvider.java
index cc772e7b..e1dec48f 100644
--- a/src/main/java/com/gitblit/auth/LdapAuthProvider.java
+++ b/src/main/java/com/gitblit/auth/LdapAuthProvider.java
@@ -39,6 +39,8 @@ import com.gitblit.service.LdapSyncService;
 import com.gitblit.utils.ArrayUtils;
 import com.gitblit.utils.StringUtils;
 import com.unboundid.ldap.sdk.Attribute;
+import com.unboundid.ldap.sdk.BindRequest;
+import com.unboundid.ldap.sdk.BindResult;
 import com.unboundid.ldap.sdk.DereferencePolicy;
 import com.unboundid.ldap.sdk.ExtendedResult;
 import com.unboundid.ldap.sdk.LDAPConnection;
@@ -107,8 +109,14 @@ public class LdapAuthProvider extends UsernamePasswordAuthenticationProvider {
 		if (enabled) {
 			logger.info("Synchronizing with LDAP @ " + settings.getRequiredString(Keys.realm.ldap.server));
 			final boolean deleteRemovedLdapUsers = settings.getBoolean(Keys.realm.ldap.removeDeletedUsers, true);
-			LDAPConnection ldapConnection = getLdapConnection();
-			if (ldapConnection != null) {
+			LdapConnection ldapConnection = new LdapConnection();
+			if (ldapConnection.connect()) {
+				if (ldapConnection.bind() == null) {
+					ldapConnection.close();
+					logger.error("Cannot synchronize with LDAP.");
+					return;
+				}
+
 				try {
 					String accountBase = settings.getString(Keys.realm.ldap.accountBase, "");
 					String uidAttribute = settings.getString(Keys.realm.ldap.uid, "uid");
@@ -179,66 +187,6 @@ public class LdapAuthProvider extends UsernamePasswordAuthenticationProvider {
 		}
 	}
 
-	private LDAPConnection getLdapConnection() {
-		try {
-
-			URI ldapUrl = new URI(settings.getRequiredString(Keys.realm.ldap.server));
-			String ldapHost = ldapUrl.getHost();
-			int ldapPort = ldapUrl.getPort();
-			String bindUserName = settings.getString(Keys.realm.ldap.username, "");
-			String bindPassword = settings.getString(Keys.realm.ldap.password, "");
-
-			LDAPConnection conn;
-			if (ldapUrl.getScheme().equalsIgnoreCase("ldaps")) {
-				// SSL
-				SSLUtil sslUtil = new SSLUtil(new TrustAllTrustManager());
-				conn = new LDAPConnection(sslUtil.createSSLSocketFactory());
-				if (ldapPort == -1) {
-					ldapPort = 636;
-				}
-			} else if (ldapUrl.getScheme().equalsIgnoreCase("ldap") || ldapUrl.getScheme().equalsIgnoreCase("ldap+tls")) {
-				// no encryption or StartTLS
-				conn = new LDAPConnection();
-				 if (ldapPort == -1) {
-					 ldapPort = 389;
-				 }
-			} else {
-				logger.error("Unsupported LDAP URL scheme: " + ldapUrl.getScheme());
-				return null;
-			}
-
-			conn.connect(ldapHost, ldapPort);
-
-			if (ldapUrl.getScheme().equalsIgnoreCase("ldap+tls")) {
-				SSLUtil sslUtil = new SSLUtil(new TrustAllTrustManager());
-				ExtendedResult extendedResult = conn.processExtendedOperation(
-						new StartTLSExtendedRequest(sslUtil.createSSLContext()));
-				if (extendedResult.getResultCode() != ResultCode.SUCCESS) {
-					throw new LDAPException(extendedResult.getResultCode());
-				}
-			}
-
-			if (StringUtils.isEmpty(bindUserName) && StringUtils.isEmpty(bindPassword)) {
-				// anonymous bind
-				conn.bind(new SimpleBindRequest());
-			} else {
-				// authenticated bind
-				conn.bind(new SimpleBindRequest(bindUserName, bindPassword));
-			}
-
-			return conn;
-
-		} catch (URISyntaxException e) {
-			logger.error("Bad LDAP URL, should be in the form: ldap(s|+tls)://<server>:<port>", e);
-		} catch (GeneralSecurityException e) {
-			logger.error("Unable to create SSL Connection", e);
-		} catch (LDAPException e) {
-			logger.error("Error Connecting to LDAP", e);
-		}
-
-		return null;
-	}
-
 	/**
 	 * Credentials are defined in the LDAP server and can not be manipulated
 	 * from Gitblit.
@@ -321,23 +269,26 @@ public class LdapAuthProvider extends UsernamePasswordAuthenticationProvider {
 	public UserModel authenticate(String username, char[] password) {
 		String simpleUsername = getSimpleUsername(username);
 
-		LDAPConnection ldapConnection = getLdapConnection();
-		if (ldapConnection != null) {
-			try {
-				boolean alreadyAuthenticated = false;
+		LdapConnection ldapConnection = new LdapConnection();
+		if (ldapConnection.connect()) {
 
-				String bindPattern = settings.getString(Keys.realm.ldap.bindpattern, "");
-				if (!StringUtils.isEmpty(bindPattern)) {
-					try {
-						String bindUser = StringUtils.replace(bindPattern, "${username}", escapeLDAPSearchFilter(simpleUsername));
-						ldapConnection.bind(bindUser, new String(password));
+			// Try to bind either to the "manager" account,
+			// or directly to the DN of the user logging in, if realm.ldap.bindpattern is configured.
+			String passwd = new String(password);
+			BindResult bindResult = null;
+			String bindPattern = settings.getString(Keys.realm.ldap.bindpattern, "");
+			if (! StringUtils.isEmpty(bindPattern)) {
+				bindResult = ldapConnection.bind(bindPattern, simpleUsername, passwd);
+			} else {
+				bindResult = ldapConnection.bind();
+			}
+			if (bindResult == null) {
+				ldapConnection.close();
+				return null;
+			}
 
-						alreadyAuthenticated = true;
-					} catch (LDAPException e) {
-						return null;
-					}
-				}
 
+			try {
 				// Find the logging in user's DN
 				String accountBase = settings.getString(Keys.realm.ldap.accountBase, "");
 				String accountPattern = settings.getString(Keys.realm.ldap.accountPattern, "(&(objectClass=person)(sAMAccountName=${username}))");
@@ -348,7 +299,7 @@ public class LdapAuthProvider extends UsernamePasswordAuthenticationProvider {
 					SearchResultEntry loggingInUser = result.getSearchEntries().get(0);
 					String loggingInUserDN = loggingInUser.getDN();
 
-					if (alreadyAuthenticated || isAuthenticated(ldapConnection, loggingInUserDN, new String(password))) {
+					if (ldapConnection.isAuthenticated(loggingInUserDN, passwd)) {
 						logger.debug("LDAP authenticated: " + username);
 
 						UserModel user = null;
@@ -462,7 +413,7 @@ public class LdapAuthProvider extends UsernamePasswordAuthenticationProvider {
 		}
 	}
 
-	private void getTeamsFromLdap(LDAPConnection ldapConnection, String simpleUsername, SearchResultEntry loggingInUser, UserModel user) {
+	private void getTeamsFromLdap(LdapConnection ldapConnection, String simpleUsername, SearchResultEntry loggingInUser, UserModel user) {
 		String loggingInUserDN = loggingInUser.getDN();
 
 		// Clear the users team memberships - we're going to get them from LDAP
@@ -479,7 +430,7 @@ public class LdapAuthProvider extends UsernamePasswordAuthenticationProvider {
 			groupMemberPattern = StringUtils.replace(groupMemberPattern, "${" + userAttribute.getName() + "}", escapeLDAPSearchFilter(userAttribute.getValue()));
 		}
 
-		SearchResult teamMembershipResult = doSearch(ldapConnection, groupBase, true, groupMemberPattern, Arrays.asList("cn"));
+		SearchResult teamMembershipResult = searchTeamsInLdap(ldapConnection, groupBase, true, groupMemberPattern, Arrays.asList("cn"));
 		if (teamMembershipResult != null && teamMembershipResult.getEntryCount() > 0) {
 			for (int i = 0; i < teamMembershipResult.getEntryCount(); i++) {
 				SearchResultEntry teamEntry = teamMembershipResult.getSearchEntries().get(i);
@@ -496,12 +447,12 @@ public class LdapAuthProvider extends UsernamePasswordAuthenticationProvider {
 		}
 	}
 
-	private void getEmptyTeamsFromLdap(LDAPConnection ldapConnection) {
+	private void getEmptyTeamsFromLdap(LdapConnection ldapConnection) {
 		logger.info("Start fetching empty teams from ldap.");
 		String groupBase = settings.getString(Keys.realm.ldap.groupBase, "");
 		String groupMemberPattern = settings.getString(Keys.realm.ldap.groupEmptyMemberPattern, "(&(objectClass=group)(!(member=*)))");
 
-		SearchResult teamMembershipResult = doSearch(ldapConnection, groupBase, true, groupMemberPattern, null);
+		SearchResult teamMembershipResult = searchTeamsInLdap(ldapConnection, groupBase, true, groupMemberPattern, null);
 		if (teamMembershipResult != null && teamMembershipResult.getEntryCount() > 0) {
 			for (int i = 0; i < teamMembershipResult.getEntryCount(); i++) {
 				SearchResultEntry teamEntry = teamMembershipResult.getSearchEntries().get(i);
@@ -519,6 +470,30 @@ public class LdapAuthProvider extends UsernamePasswordAuthenticationProvider {
 		logger.info("Finished fetching empty teams from ldap.");
 	}
 
+
+	private SearchResult searchTeamsInLdap(LdapConnection ldapConnection, String base, boolean dereferenceAliases, String filter, List<String> attributes) {
+		SearchResult result = ldapConnection.search(base, dereferenceAliases, filter, attributes);
+		if (result == null) {
+			return null;
+		}
+
+		if (result.getResultCode() != ResultCode.SUCCESS) {
+			// Retry the search with user authorization in case we searched as a manager account that could not search for teams.
+			logger.debug("Rebinding as user to search for teams in LDAP");
+			result = null;
+			if (ldapConnection.rebindAsUser()) {
+				result = ldapConnection.search(base, dereferenceAliases, filter, attributes);
+				if (result.getResultCode() != ResultCode.SUCCESS) {
+					return null;
+				}
+				logger.info("Successful search after rebinding as user.");
+			}
+		}
+
+		return result;
+	}
+
+
 	private TeamModel createTeamFromLdap(SearchResultEntry teamEntry) {
 		TeamModel answer = new TeamModel(teamEntry.getAttributeValue("cn"));
 		answer.accountType = getAccountType();
@@ -527,47 +502,21 @@ public class LdapAuthProvider extends UsernamePasswordAuthenticationProvider {
 		return answer;
 	}
 
-	private SearchResult doSearch(LDAPConnection ldapConnection, String base, String filter) {
-		try {
-			return ldapConnection.search(base, SearchScope.SUB, filter);
-		} catch (LDAPSearchException e) {
-			logger.error("Problem Searching LDAP", e);
-
-			return null;
-		}
-	}
-
-	private SearchResult doSearch(LDAPConnection ldapConnection, String base, boolean dereferenceAliases, String filter, List<String> attributes) {
+	private SearchResult doSearch(LdapConnection ldapConnection, String base, String filter) {
 		try {
 			SearchRequest searchRequest = new SearchRequest(base, SearchScope.SUB, filter);
-			if (dereferenceAliases) {
-				searchRequest.setDerefPolicy(DereferencePolicy.SEARCHING);
-			}
-			if (attributes != null) {
-				searchRequest.setAttributes(attributes);
+			SearchResult result = ldapConnection.search(searchRequest);
+			if (result.getResultCode() != ResultCode.SUCCESS) {
+				return null;
 			}
-			return ldapConnection.search(searchRequest);
-
-		} catch (LDAPSearchException e) {
-			logger.error("Problem Searching LDAP", e);
-
-			return null;
+			return result;
 		} catch (LDAPException e) {
 			logger.error("Problem creating LDAP search", e);
 			return null;
 		}
 	}
 
-	private boolean isAuthenticated(LDAPConnection ldapConnection, String userDn, String password) {
-		try {
-			// Binding will stop any LDAP-Injection Attacks since the searched-for user needs to bind to that DN
-			ldapConnection.bind(userDn, password);
-			return true;
-		} catch (LDAPException e) {
-			logger.error("Error authenticating user", e);
-			return false;
-		}
-	}
+
 
 	/**
 	 * Returns a simple username without any domain prefixes.
@@ -585,7 +534,7 @@ public class LdapAuthProvider extends UsernamePasswordAuthenticationProvider {
 	}
 
 	// From: https://www.owasp.org/index.php/Preventing_LDAP_Injection_in_Java
-	public static final String escapeLDAPSearchFilter(String filter) {
+	private static final String escapeLDAPSearchFilter(String filter) {
 		StringBuilder sb = new StringBuilder();
 		for (int i = 0; i < filter.length(); i++) {
 			char curChar = filter.charAt(i);
@@ -625,4 +574,225 @@ public class LdapAuthProvider extends UsernamePasswordAuthenticationProvider {
 		}
 	}
 
+
+
+	private class LdapConnection {
+		private LDAPConnection conn;
+		private SimpleBindRequest currentBindRequest;
+		private SimpleBindRequest managerBindRequest;
+		private SimpleBindRequest userBindRequest;
+
+
+		public LdapConnection() {
+			String bindUserName = settings.getString(Keys.realm.ldap.username, "");
+			String bindPassword = settings.getString(Keys.realm.ldap.password, "");
+			if (StringUtils.isEmpty(bindUserName) && StringUtils.isEmpty(bindPassword)) {
+				this.managerBindRequest = new SimpleBindRequest();
+			}
+			this.managerBindRequest = new SimpleBindRequest(bindUserName, bindPassword);
+		}
+
+
+		boolean connect() {
+			try {
+				URI ldapUrl = new URI(settings.getRequiredString(Keys.realm.ldap.server));
+				String ldapHost = ldapUrl.getHost();
+				int ldapPort = ldapUrl.getPort();
+
+				if (ldapUrl.getScheme().equalsIgnoreCase("ldaps")) {
+					// SSL
+					SSLUtil sslUtil = new SSLUtil(new TrustAllTrustManager());
+					conn = new LDAPConnection(sslUtil.createSSLSocketFactory());
+					if (ldapPort == -1) {
+						ldapPort = 636;
+					}
+				} else if (ldapUrl.getScheme().equalsIgnoreCase("ldap") || ldapUrl.getScheme().equalsIgnoreCase("ldap+tls")) {
+					// no encryption or StartTLS
+					conn = new LDAPConnection();
+					 if (ldapPort == -1) {
+						 ldapPort = 389;
+					 }
+				} else {
+					logger.error("Unsupported LDAP URL scheme: " + ldapUrl.getScheme());
+					return false;
+				}
+
+				conn.connect(ldapHost, ldapPort);
+
+				if (ldapUrl.getScheme().equalsIgnoreCase("ldap+tls")) {
+					SSLUtil sslUtil = new SSLUtil(new TrustAllTrustManager());
+					ExtendedResult extendedResult = conn.processExtendedOperation(
+							new StartTLSExtendedRequest(sslUtil.createSSLContext()));
+					if (extendedResult.getResultCode() != ResultCode.SUCCESS) {
+						throw new LDAPException(extendedResult.getResultCode());
+					}
+				}
+
+				return true;
+
+			} catch (URISyntaxException e) {
+				logger.error("Bad LDAP URL, should be in the form: ldap(s|+tls)://<server>:<port>", e);
+			} catch (GeneralSecurityException e) {
+				logger.error("Unable to create SSL Connection", e);
+			} catch (LDAPException e) {
+				logger.error("Error Connecting to LDAP", e);
+			}
+
+			return false;
+		}
+
+
+		void close() {
+			if (conn != null) {
+				conn.close();
+			}
+		}
+
+
+		SearchResult search(SearchRequest request) {
+			try {
+				return conn.search(request);
+			} catch (LDAPSearchException e) {
+				logger.error("Problem Searching LDAP [{}]",  e.getResultCode());
+				return e.getSearchResult();
+			}
+		}
+
+
+		SearchResult search(String base, boolean dereferenceAliases, String filter, List<String> attributes) {
+			try {
+				SearchRequest searchRequest = new SearchRequest(base, SearchScope.SUB, filter);
+				if (dereferenceAliases) {
+					searchRequest.setDerefPolicy(DereferencePolicy.SEARCHING);
+				}
+				if (attributes != null) {
+					searchRequest.setAttributes(attributes);
+				}
+				SearchResult result = search(searchRequest);
+				return result;
+
+			} catch (LDAPException e) {
+				logger.error("Problem creating LDAP search", e);
+				return null;
+			}
+		}
+
+
+
+		/**
+		 * Bind using the manager credentials set in realm.ldap.username and ..password
+		 * @return A bind result, or null if binding failed.
+		 */
+		BindResult bind() {
+			BindResult result = null;
+			try {
+				result = conn.bind(managerBindRequest);
+				currentBindRequest = managerBindRequest;
+			} catch (LDAPException e) {
+				logger.error("Error authenticating to LDAP with manager account to search the directory.");
+				logger.error("  Please check your settings for realm.ldap.username and realm.ldap.password.");
+				logger.debug("  Received exception when binding to LDAP", e);
+				return null;
+			}
+			return result;
+		}
+
+
+		/**
+		 * Bind using the given credentials, by filling in the username in the given {@code bindPattern} to
+		 * create the DN.
+		 * @return A bind result, or null if binding failed.
+		 */
+		BindResult bind(String bindPattern, String simpleUsername, String password) {
+			BindResult result = null;
+			try {
+				String bindUser = StringUtils.replace(bindPattern, "${username}", escapeLDAPSearchFilter(simpleUsername));
+				SimpleBindRequest request = new SimpleBindRequest(bindUser, password);
+				result = conn.bind(request);
+				userBindRequest = request;
+				currentBindRequest = userBindRequest;
+			} catch (LDAPException e) {
+				logger.error("Error authenticating to LDAP with user account to search the directory.");
+				logger.error("  Please check your settings for realm.ldap.bindpattern.");
+				logger.debug("  Received exception when binding to LDAP", e);
+				return null;
+			}
+			return result;
+		}
+
+
+		boolean rebindAsUser() {
+			if (userBindRequest == null || currentBindRequest == userBindRequest) {
+				return false;
+			}
+			try {
+				conn.bind(userBindRequest);
+				currentBindRequest = userBindRequest;
+			} catch (LDAPException e) {
+				conn.close();
+				logger.error("Error rebinding to LDAP with user account.", e);
+				return false;
+			}
+			return true;
+		}
+
+
+		boolean isAuthenticated(String userDn, String password) {
+			verifyCurrentBinding();
+
+			// If the currently bound DN is already the DN of the logging in user, authentication has already happened
+			// during the previous bind operation. We accept this and return with the current bind left in place.
+			// This could also be changed to always retry binding as the logging in user, to make sure that the
+			// connection binding has not been tampered with in between. So far I see no way how this could happen
+			// and thus skip the repeated binding.
+			// This check also makes sure that the DN in realm.ldap.bindpattern actually matches the DN that was found
+			// when searching the user entry.
+			String boundDN = currentBindRequest.getBindDN();
+			if (boundDN != null && boundDN.equals(userDn)) {
+				return true;
+			}
+
+			// Bind a the logging in user to check for authentication.
+			// Afterwards, bind as the original bound DN again, to restore the previous authorization.
+			boolean isAuthenticated = false;
+			try {
+				// Binding will stop any LDAP-Injection Attacks since the searched-for user needs to bind to that DN
+				SimpleBindRequest ubr = new SimpleBindRequest(userDn, password);
+				conn.bind(ubr);
+				isAuthenticated = true;
+				userBindRequest = ubr;
+			} catch (LDAPException e) {
+				logger.error("Error authenticating user ({})", userDn, e);
+			}
+
+			try {
+				conn.bind(currentBindRequest);
+			} catch (LDAPException e) {
+				logger.error("Error reinstating original LDAP authorization (code {}). Team information may be inaccurate for this log in.",
+							e.getResultCode(), e);
+			}
+			return isAuthenticated;
+		}
+
+
+
+		private boolean verifyCurrentBinding() {
+			BindRequest lastBind = conn.getLastBindRequest();
+			if (lastBind == currentBindRequest) {
+				return true;
+			}
+			logger.debug("Unexpected binding in LdapConnection. {} != {}", lastBind, currentBindRequest);
+
+			String lastBoundDN = ((SimpleBindRequest)lastBind).getBindDN();
+			String boundDN = currentBindRequest.getBindDN();
+			logger.debug("Currently bound as '{}', check authentication for '{}'", lastBoundDN, boundDN);
+			if (boundDN != null && ! boundDN.equals(lastBoundDN)) {
+				logger.warn("Unexpected binding DN in LdapConnection. '{}' != '{}'.", lastBoundDN, boundDN);
+				logger.warn("Updated binding information in LDAP connection.");
+				currentBindRequest = (SimpleBindRequest)lastBind;
+				return false;
+			}
+			return true;
+		}
+	}
 }
diff --git a/src/test/java/com/gitblit/tests/LdapAuthenticationTest.java b/src/test/java/com/gitblit/tests/LdapAuthenticationTest.java
index 84dd138d..2ade6819 100644
--- a/src/test/java/com/gitblit/tests/LdapAuthenticationTest.java
+++ b/src/test/java/com/gitblit/tests/LdapAuthenticationTest.java
@@ -16,17 +16,26 @@
  */
 package com.gitblit.tests;
 
+import static org.junit.Assume.*;
+
 import java.io.File;
-import java.io.FileInputStream;
+import java.util.Arrays;
+import java.util.Collection;
+import java.util.EnumSet;
 import java.util.HashMap;
 import java.util.Map;
 
 import org.apache.commons.io.FileUtils;
+import org.junit.AfterClass;
 import org.junit.Before;
 import org.junit.BeforeClass;
 import org.junit.Rule;
 import org.junit.Test;
 import org.junit.rules.TemporaryFolder;
+import org.junit.runner.RunWith;
+import org.junit.runners.Parameterized;
+import org.junit.runners.Parameterized.Parameter;
+import org.junit.runners.Parameterized.Parameters;
 
 import com.gitblit.Constants.AccountType;
 import com.gitblit.IStoredSettings;
@@ -43,9 +52,24 @@ import com.gitblit.utils.XssFilter;
 import com.gitblit.utils.XssFilter.AllowXssFilter;
 import com.unboundid.ldap.listener.InMemoryDirectoryServer;
 import com.unboundid.ldap.listener.InMemoryDirectoryServerConfig;
+import com.unboundid.ldap.listener.InMemoryDirectoryServerSnapshot;
 import com.unboundid.ldap.listener.InMemoryListenerConfig;
+import com.unboundid.ldap.listener.interceptor.InMemoryInterceptedRequest;
+import com.unboundid.ldap.listener.interceptor.InMemoryInterceptedResult;
+import com.unboundid.ldap.listener.interceptor.InMemoryInterceptedSearchEntry;
+import com.unboundid.ldap.listener.interceptor.InMemoryInterceptedSearchRequest;
+import com.unboundid.ldap.listener.interceptor.InMemoryInterceptedSearchResult;
+import com.unboundid.ldap.listener.interceptor.InMemoryInterceptedSimpleBindResult;
+import com.unboundid.ldap.listener.interceptor.InMemoryOperationInterceptor;
+import com.unboundid.ldap.sdk.BindRequest;
+import com.unboundid.ldap.sdk.BindResult;
+import com.unboundid.ldap.sdk.LDAPException;
+import com.unboundid.ldap.sdk.LDAPResult;
+import com.unboundid.ldap.sdk.OperationType;
+import com.unboundid.ldap.sdk.ResultCode;
 import com.unboundid.ldap.sdk.SearchResult;
 import com.unboundid.ldap.sdk.SearchScope;
+import com.unboundid.ldap.sdk.SimpleBindRequest;
 import com.unboundid.ldif.LDIFReader;
 
 /**
@@ -55,19 +79,71 @@ import com.unboundid.ldif.LDIFReader;
  * @author jcrygier
  *
  */
+@RunWith(Parameterized.class)
 public class LdapAuthenticationTest extends GitblitUnitTest {
-    @Rule
-    public TemporaryFolder folder = new TemporaryFolder();
 
 	private static final String RESOURCE_DIR = "src/test/resources/ldap/";
+	private static final String DIRECTORY_MANAGER = "cn=Directory Manager";
+	private static final String USER_MANAGER = "cn=UserManager";
+	private static final String ACCOUNT_BASE = "OU=Users,OU=UserControl,OU=MyOrganization,DC=MyDomain";
+	private static final String GROUP_BASE  = "OU=Groups,OU=UserControl,OU=MyOrganization,DC=MyDomain";
+
+
+	/**
+	 * Enumeration of different test modes, representing different use scenarios.
+	 * With ANONYMOUS anonymous binds are used to search LDAP.
+	 * DS_MANAGER will use a DIRECTORY_MANAGER to search LDAP. Normal users are prohibited to search the DS.
+	 * With USR_MANAGER, a USER_MANAGER account is used to search in LDAP. This account can only search users
+	 * but not groups. Normal users can search groups, though.
+	 *
+	 */
+	enum AuthMode {
+		ANONYMOUS(1389),
+		DS_MANAGER(2389),
+		USR_MANAGER(3389);
+
+
+		private int ldapPort;
+		private InMemoryDirectoryServer ds;
+		private InMemoryDirectoryServerSnapshot dsSnapshot;
+
+		AuthMode(int port) {
+			this.ldapPort = port;
+		}
+
+		int ldapPort() {
+			return this.ldapPort;
+		}
+
+		void setDS(InMemoryDirectoryServer ds) {
+			if (this.ds == null) {
+				this.ds = ds;
+				this.dsSnapshot = ds.createSnapshot();
+			};
+		}
+
+		InMemoryDirectoryServer getDS() {
+			return ds;
+		}
+
+		void restoreSnapshot() {
+			ds.restoreSnapshot(dsSnapshot);
+		}
+	};
+
 
-    private File usersConf;
 
-    private LdapAuthProvider ldap;
+	@Parameter
+	public AuthMode authMode;
 
-	static int ldapPort = 1389;
+	@Rule
+	public TemporaryFolder folder = new TemporaryFolder();
 
-	private static InMemoryDirectoryServer ds;
+	private File usersConf;
+
+
+
+	private LdapAuthProvider ldap;
 
 	private IUserManager userManager;
 
@@ -75,21 +151,82 @@ public class LdapAuthenticationTest extends GitblitUnitTest {
 
 	private MemorySettings settings;
 
+
+	/**
+	 * Run the tests with each authentication scenario once.
+	 */
+	@Parameters(name = "{0}")
+	public static Collection<Object[]> data() {
+		return Arrays.asList(new Object[][] { {AuthMode.ANONYMOUS}, {AuthMode.DS_MANAGER}, {AuthMode.USR_MANAGER} });
+	}
+
+
+
+	/**
+	 * Create three different in memory DS.
+	 *
+	 * Each DS has a different configuration:
+	 * The first allows anonymous binds.
+	 * The second requires authentication for all operations. It will only allow the DIRECTORY_MANAGER account
+	 * to search for users and groups.
+	 * The third one is like the second, but it allows users to search for users and groups, and restricts the
+	 * USER_MANAGER from searching for groups.
+	 */
 	@BeforeClass
-	public static void createInMemoryLdapServer() throws Exception {
+	public static void init() throws Exception {
+		InMemoryDirectoryServer ds;
+		InMemoryDirectoryServerConfig config = createInMemoryLdapServerConfig(AuthMode.ANONYMOUS);
+		config.setListenerConfigs(InMemoryListenerConfig.createLDAPConfig("default", AuthMode.ANONYMOUS.ldapPort()));
+		ds = createInMemoryLdapServer(config);
+		AuthMode.ANONYMOUS.setDS(ds);
+
+
+		config = createInMemoryLdapServerConfig(AuthMode.DS_MANAGER);
+		config.setListenerConfigs(InMemoryListenerConfig.createLDAPConfig("default", AuthMode.DS_MANAGER.ldapPort()));
+		config.setAuthenticationRequiredOperationTypes(EnumSet.allOf(OperationType.class));
+		ds = createInMemoryLdapServer(config);
+		AuthMode.DS_MANAGER.setDS(ds);
+
+
+		config = createInMemoryLdapServerConfig(AuthMode.USR_MANAGER);
+		config.setListenerConfigs(InMemoryListenerConfig.createLDAPConfig("default", AuthMode.USR_MANAGER.ldapPort()));
+		config.setAuthenticationRequiredOperationTypes(EnumSet.allOf(OperationType.class));
+		ds = createInMemoryLdapServer(config);
+		AuthMode.USR_MANAGER.setDS(ds);
+
+	}
+
+	@AfterClass
+	public static void destroy() throws Exception {
+		for (AuthMode am : AuthMode.values()) {
+			am.getDS().shutDown(true);
+		}
+	}
+
+	public static InMemoryDirectoryServer createInMemoryLdapServer(InMemoryDirectoryServerConfig config) throws Exception {
+		InMemoryDirectoryServer imds = new InMemoryDirectoryServer(config);
+		imds.importFromLDIF(true, RESOURCE_DIR + "sampledata.ldif");
+		imds.startListening();
+		return imds;
+	}
+
+	public static InMemoryDirectoryServerConfig createInMemoryLdapServerConfig(AuthMode authMode) throws Exception {
 		InMemoryDirectoryServerConfig config = new InMemoryDirectoryServerConfig("dc=MyDomain");
-		config.addAdditionalBindCredentials("cn=Directory Manager", "password");
-		config.setListenerConfigs(InMemoryListenerConfig.createLDAPConfig("default", ldapPort));
+		config.addAdditionalBindCredentials(DIRECTORY_MANAGER, "password");
+		config.addAdditionalBindCredentials(USER_MANAGER, "passwd");
 		config.setSchema(null);
 
-		ds = new InMemoryDirectoryServer(config);
-		ds.startListening();
+		config.addInMemoryOperationInterceptor(new AccessInterceptor(authMode));
+
+		return config;
 	}
 
+
+
 	@Before
-	public void init() throws Exception {
-		ds.clear();
-		ds.importFromLDIF(true, new LDIFReader(new FileInputStream(RESOURCE_DIR + "sampledata.ldif")));
+	public void setup() throws Exception {
+		authMode.restoreSnapshot();
+
 		usersConf = folder.newFile("users.conf");
 		FileUtils.copyFile(new File(RESOURCE_DIR + "users.conf"), usersConf);
 		settings = getSettings();
@@ -117,15 +254,30 @@ public class LdapAuthenticationTest extends GitblitUnitTest {
 	private MemorySettings getSettings() {
 		Map<String, Object> backingMap = new HashMap<String, Object>();
 		backingMap.put(Keys.realm.userService, usersConf.getAbsolutePath());
-		backingMap.put(Keys.realm.ldap.server, "ldap://localhost:" + ldapPort);
-//		backingMap.put(Keys.realm.ldap.domain, "");
-		backingMap.put(Keys.realm.ldap.username, "cn=Directory Manager");
-		backingMap.put(Keys.realm.ldap.password, "password");
-//		backingMap.put(Keys.realm.ldap.backingUserService, "users.conf");
+		switch(authMode) {
+		case ANONYMOUS:
+			backingMap.put(Keys.realm.ldap.server, "ldap://localhost:" + authMode.ldapPort());
+			backingMap.put(Keys.realm.ldap.username, "");
+			backingMap.put(Keys.realm.ldap.password, "");
+			break;
+		case DS_MANAGER:
+			backingMap.put(Keys.realm.ldap.server, "ldap://localhost:" + authMode.ldapPort());
+			backingMap.put(Keys.realm.ldap.username, DIRECTORY_MANAGER);
+			backingMap.put(Keys.realm.ldap.password, "password");
+			break;
+		case USR_MANAGER:
+			backingMap.put(Keys.realm.ldap.server, "ldap://localhost:" + authMode.ldapPort());
+			backingMap.put(Keys.realm.ldap.username, USER_MANAGER);
+			backingMap.put(Keys.realm.ldap.password, "passwd");
+			break;
+		default:
+			throw new RuntimeException("Unimplemented AuthMode case!");
+
+		}
 		backingMap.put(Keys.realm.ldap.maintainTeams, "true");
-		backingMap.put(Keys.realm.ldap.accountBase, "OU=Users,OU=UserControl,OU=MyOrganization,DC=MyDomain");
+		backingMap.put(Keys.realm.ldap.accountBase, ACCOUNT_BASE);
 		backingMap.put(Keys.realm.ldap.accountPattern, "(&(objectClass=person)(sAMAccountName=${username}))");
-		backingMap.put(Keys.realm.ldap.groupBase, "OU=Groups,OU=UserControl,OU=MyOrganization,DC=MyDomain");
+		backingMap.put(Keys.realm.ldap.groupBase, GROUP_BASE);
 		backingMap.put(Keys.realm.ldap.groupMemberPattern, "(&(objectClass=group)(member=${dn}))");
 		backingMap.put(Keys.realm.ldap.admins, "UserThree @Git_Admins \"@Git Admins\"");
 		backingMap.put(Keys.realm.ldap.displayName, "displayName");
@@ -136,6 +288,8 @@ public class LdapAuthenticationTest extends GitblitUnitTest {
 		return ms;
 	}
 
+
+
 	@Test
 	public void testAuthenticate() {
 		UserModel userOneModel = ldap.authenticate("UserOne", "userOnePassword".toCharArray());
@@ -159,6 +313,13 @@ public class LdapAuthenticationTest extends GitblitUnitTest {
 		assertNotNull(userThreeModel.getTeam("git_users"));
 		assertNull(userThreeModel.getTeam("git_admins"));
 		assertTrue(userThreeModel.canAdmin);
+
+		UserModel userFourModel = ldap.authenticate("UserFour", "userFourPassword".toCharArray());
+		assertNotNull(userFourModel);
+		assertNotNull(userFourModel.getTeam("git_users"));
+		assertNull(userFourModel.getTeam("git_admins"));
+		assertNull(userFourModel.getTeam("git admins"));
+		assertFalse(userFourModel.canAdmin);
 	}
 
 	@Test
@@ -204,13 +365,13 @@ public class LdapAuthenticationTest extends GitblitUnitTest {
 
 	@Test
 	public void checkIfUsersConfContainsAllUsersFromSampleDataLdif() throws Exception {
-		SearchResult searchResult = ds.search("OU=Users,OU=UserControl,OU=MyOrganization,DC=MyDomain", SearchScope.SUB, "objectClass=person");
+		SearchResult searchResult = getDS().search(ACCOUNT_BASE, SearchScope.SUB, "objectClass=person");
 		assertEquals("Number of ldap users in gitblit user model", searchResult.getEntryCount(), countLdapUsersInUserManager());
 	}
 
 	@Test
 	public void addingUserInLdapShouldNotUpdateGitBlitUsersAndGroups() throws Exception {
-		ds.addEntries(LDIFReader.readEntries(RESOURCE_DIR + "adduser.ldif"));
+		getDS().addEntries(LDIFReader.readEntries(RESOURCE_DIR + "adduser.ldif"));
 		ldap.sync();
 		assertEquals("Number of ldap users in gitblit user model", 5, countLdapUsersInUserManager());
 	}
@@ -218,22 +379,25 @@ public class LdapAuthenticationTest extends GitblitUnitTest {
 	@Test
 	public void addingUserInLdapShouldUpdateGitBlitUsersAndGroups() throws Exception {
 		settings.put(Keys.realm.ldap.synchronize, "true");
-		ds.addEntries(LDIFReader.readEntries(RESOURCE_DIR + "adduser.ldif"));
+		getDS().addEntries(LDIFReader.readEntries(RESOURCE_DIR + "adduser.ldif"));
 		ldap.sync();
 		assertEquals("Number of ldap users in gitblit user model", 6, countLdapUsersInUserManager());
 	}
 
 	@Test
 	public void addingGroupsInLdapShouldNotUpdateGitBlitUsersAndGroups() throws Exception {
-		ds.addEntries(LDIFReader.readEntries(RESOURCE_DIR + "addgroup.ldif"));
+		getDS().addEntries(LDIFReader.readEntries(RESOURCE_DIR + "addgroup.ldif"));
 		ldap.sync();
 		assertEquals("Number of ldap groups in gitblit team model", 0, countLdapTeamsInUserManager());
 	}
 
 	@Test
 	public void addingGroupsInLdapShouldUpdateGitBlitUsersAndGroups() throws Exception {
+		// This test only makes sense if the authentication mode allows for synchronization.
+		assumeTrue(authMode == AuthMode.ANONYMOUS || authMode == AuthMode.DS_MANAGER);
+
 		settings.put(Keys.realm.ldap.synchronize, "true");
-		ds.addEntries(LDIFReader.readEntries(RESOURCE_DIR + "addgroup.ldif"));
+		getDS().addEntries(LDIFReader.readEntries(RESOURCE_DIR + "addgroup.ldif"));
 		ldap.sync();
 		assertEquals("Number of ldap groups in gitblit team model", 1, countLdapTeamsInUserManager());
 	}
@@ -261,11 +425,21 @@ public class LdapAuthenticationTest extends GitblitUnitTest {
 		assertNotNull(userThreeModel.getTeam("git_users"));
 		assertNull(userThreeModel.getTeam("git_admins"));
 		assertTrue(userThreeModel.canAdmin);
+
+		UserModel userFourModel = auth.authenticate("UserFour", "userFourPassword".toCharArray(), null);
+		assertNotNull(userFourModel);
+		assertNotNull(userFourModel.getTeam("git_users"));
+		assertNull(userFourModel.getTeam("git_admins"));
+		assertNull(userFourModel.getTeam("git admins"));
+		assertFalse(userFourModel.canAdmin);
 	}
 
 	@Test
 	public void testBindWithUser() {
-		settings.put(Keys.realm.ldap.bindpattern, "CN=${username},OU=US,OU=Users,OU=UserControl,OU=MyOrganization,DC=MyDomain");
+		// This test only makes sense if the user is not prevented from reading users and teams.
+		assumeTrue(authMode != AuthMode.DS_MANAGER);
+
+		settings.put(Keys.realm.ldap.bindpattern, "CN=${username},OU=US," + ACCOUNT_BASE);
 		settings.put(Keys.realm.ldap.username, "");
 		settings.put(Keys.realm.ldap.password, "");
 
@@ -276,6 +450,12 @@ public class LdapAuthenticationTest extends GitblitUnitTest {
 		assertNull(userOneModelFailedAuth);
 	}
 
+
+	private InMemoryDirectoryServer getDS()
+	{
+		return authMode.getDS();
+	}
+
 	private int countLdapUsersInUserManager() {
 		int ldapAccountCount = 0;
 		for (UserModel userModel : userManager.getAllUsers()) {
@@ -296,4 +476,120 @@ public class LdapAuthenticationTest extends GitblitUnitTest {
 		return ldapAccountCount;
 	}
 
+
+
+
+	/**
+	 * Operation interceptor for the in memory DS. This interceptor
+	 * implements access restrictions for certain user/DN combinations.
+	 *
+	 * The USER_MANAGER is only allowed to search for users, but not for groups.
+	 * This is to test the original behaviour where the teams were searched under
+	 * the user binding.
+	 * When running in a DIRECTORY_MANAGER scenario, only the manager account
+	 * is allowed to search for users and groups, while a normal user may not do so.
+	 * This tests the scenario where a normal user cannot read teams and thus the
+	 * manager account needs to be used for all searches.
+	 *
+	 */
+	private static class AccessInterceptor extends InMemoryOperationInterceptor {
+		AuthMode authMode;
+		Map<Long,String> lastSuccessfulBindDN = new HashMap<>();
+		Map<Long,Boolean> resultProhibited = new HashMap<>();
+
+		public AccessInterceptor(AuthMode authMode) {
+			this.authMode = authMode;
+		}
+
+
+		@Override
+		public void processSimpleBindResult(InMemoryInterceptedSimpleBindResult bind) {
+			BindResult result = bind.getResult();
+			if (result.getResultCode() == ResultCode.SUCCESS) {
+				 BindRequest bindRequest = bind.getRequest();
+				 lastSuccessfulBindDN.put(bind.getConnectionID(), ((SimpleBindRequest)bindRequest).getBindDN());
+				 resultProhibited.remove(bind.getConnectionID());
+			}
+		}
+
+
+
+		@Override
+		public void processSearchRequest(InMemoryInterceptedSearchRequest request) throws LDAPException {
+			String bindDN = getLastBindDN(request);
+
+			if (USER_MANAGER.equals(bindDN)) {
+				if (request.getRequest().getBaseDN().endsWith(GROUP_BASE)) {
+					throw new LDAPException(ResultCode.NO_SUCH_OBJECT);
+				}
+			}
+			else if(authMode == AuthMode.DS_MANAGER && !DIRECTORY_MANAGER.equals(bindDN)) {
+				throw new LDAPException(ResultCode.NO_SUCH_OBJECT);
+			}
+		}
+
+
+		@Override
+		public void processSearchEntry(InMemoryInterceptedSearchEntry entry) {
+			String bindDN = getLastBindDN(entry);
+
+			boolean prohibited = false;
+
+			if (USER_MANAGER.equals(bindDN)) {
+				if (entry.getSearchEntry().getDN().endsWith(GROUP_BASE)) {
+					prohibited = true;
+				}
+			}
+			else if(authMode == AuthMode.DS_MANAGER && !DIRECTORY_MANAGER.equals(bindDN)) {
+				prohibited = true;
+			}
+
+			if (prohibited) {
+				// Found entry prohibited for bound user. Setting entry to null.
+				entry.setSearchEntry(null);
+				resultProhibited.put(entry.getConnectionID(), Boolean.TRUE);
+			}
+		}
+
+		@Override
+		public void processSearchResult(InMemoryInterceptedSearchResult result) {
+			String bindDN = getLastBindDN(result);
+
+			boolean prohibited = false;
+
+			Boolean rspb = resultProhibited.get(result.getConnectionID());
+			if (USER_MANAGER.equals(bindDN)) {
+				if (rspb != null && rspb) {
+					prohibited = true;
+				}
+			}
+			else if(authMode == AuthMode.DS_MANAGER && !DIRECTORY_MANAGER.equals(bindDN)) {
+				if (rspb != null && rspb) {
+					prohibited = true;
+				}
+			}
+
+			if (prohibited) {
+				// Result prohibited for bound user. Returning error
+				result.setResult(new LDAPResult(result.getMessageID(), ResultCode.INSUFFICIENT_ACCESS_RIGHTS));
+				resultProhibited.remove(result.getConnectionID());
+			}
+		}
+
+		private String getLastBindDN(InMemoryInterceptedResult result) {
+			String bindDN = lastSuccessfulBindDN.get(result.getConnectionID());
+			if (bindDN == null) {
+				return "UNKNOWN";
+			}
+			return bindDN;
+		}
+		private String getLastBindDN(InMemoryInterceptedRequest request) {
+			String bindDN = lastSuccessfulBindDN.get(request.getConnectionID());
+			if (bindDN == null) {
+				return "UNKNOWN";
+			}
+			return bindDN;
+		}
+	}
+
 }