Merged ticket #5 patchset 2

14 files changed, 492 insertions, 123 deletions

diff --git a/releases.moxie b/releases.moxie
index 66643cf9..65487139 100644
--- a/releases.moxie
+++ b/releases.moxie
@@ -33,6 +33,7 @@ r20: {
 	- Reversed line links in blob view (issue-309)
 	- Dashboard and Activity pages now obey the web.generateActivityGraph setting (issue-310)
 	- Do not log passwords on failed authentication attempts (issue-316)
+	- LDAP synchronization is now scheduled rather than on-demand (issue-336)
 	- Show displayname and username in palettes (issue-364)
 	- Updated default binary and Lucene ignore extensions
 	- Change the WAR baseFolder context parameter to a JNDI env-entry to improve enterprise deployments
@@ -82,6 +83,10 @@ r20: {
 	- { name: 'git.defaultAccessRestriction', defaultValue: 'PUSH' }
 	- { name: 'git.mirrorPeriod', defaultValue: '30 mins' }
 	- { name: 'realm.authenticationProviders', defaultValue: ' ' }
+	- { name: 'realm.ldap.groupEmptyMemberPattern', defaultValue: '(&(objectClass=group)(!(member=*)))' }
+	- { name: 'realm.ldap.synchronize', defaultValue: 'false' }
+	- { name: 'realm.ldap.syncPeriod', defaultValue: '5 MINUTES' }
+	- { name: 'realm.ldap.removeDeletedUsers', defaultValue: 'true' }
 	- { name: 'web.commitMessageRenderer', defaultValue: 'plain' }
 	- { name: 'web.documents', defaultValue: 'readme home index changelog contributing submitting_patches copying license notice authors' }
 	- { name: 'web.showBranchGraph', defaultValue: 'true' }
diff --git a/src/main/distrib/data/gitblit.properties b/src/main/distrib/data/gitblit.properties
index 482a8353..da638322 100644
--- a/src/main/distrib/data/gitblit.properties
+++ b/src/main/distrib/data/gitblit.properties
@@ -1460,6 +1460,15 @@ realm.ldap.groupBase = OU=Groups,OU=UserControl,OU=MyOrganization,DC=MyDomain
 # SINCE 1.0.0

 realm.ldap.groupMemberPattern = (&(objectClass=group)(member=${dn}))

 

+# Filter criteria for empty LDAP groups

+#

+# Query pattern to use when searching for an empty team. This may be any valid 

+# LDAP query expression, including the standard (&) and (|) operators.

+#

+# default: (&(objectClass=group)(!(member=*)))

+# SINCE 1.4.0

+realm.ldap.groupEmptyMemberPattern = (&(objectClass=group)(!(member=*)))

+

 # LDAP users or groups that should be given administrator privileges.

 #

 # Teams are specified with a leading '@' character.  Groups with spaces in the

@@ -1493,35 +1502,44 @@ realm.ldap.displayName = displayName
 # SINCE 1.0.0

 realm.ldap.email = email

 

-# Defines the cache period to be used when caching LDAP queries. This is currently

-# only used for LDAP user synchronization.

-#

-# Must be of the form '<long> <TimeUnit>' where <TimeUnit> is one of 'MILLISECONDS', 'SECONDS', 'MINUTES', 'HOURS', 'DAYS' 

-# default: 2 MINUTES

+# Attribute on the USER record that indicate their username to be used in gitblit

+# when synchronizing users from LDAP

+# if blank, Gitblit will use uid

+# For MS Active Directory this may be sAMAccountName

 #

-# RESTART REQUIRED

-realm.ldap.ldapCachePeriod = 2 MINUTES

+# SINCE 1.0.0

+realm.ldap.uid = uid

 

-# Defines whether to synchronize all LDAP users into the backing user service

+# Defines whether to synchronize all LDAP users and teams into the user service

 #

 # Valid values: true, false

 # If left blank, false is assumed

-realm.ldap.synchronizeUsers.enable = false

+#

+# SINCE 1.4.0

+realm.ldap.synchronize = false

 

-# Defines whether to delete non-existent LDAP users from the backing user service

-# during synchronization. depends on  realm.ldap.synchronizeUsers.enable = true

+# Defines the period to be used when synchronizing users and teams from ldap.

+#

+# Must be of the form '<long> <TimeUnit>' where <TimeUnit> is one of 'MILLISECONDS', 'SECONDS', 'MINUTES', 'HOURS', 'DAYS' 

+

+# default: 5 MINUTES

+#

+# RESTART REQUIRED

+# SINCE 1.4.0

+realm.ldap.syncPeriod = 5 MINUTES

+

+# Defines whether to delete non-existent LDAP users from the user service

+# during synchronization. depends on  realm.ldap.synchronize = true

 #

 # Valid values: true, false

 # If left blank, true is assumed

-realm.ldap.synchronizeUsers.removeDeleted = true

-

-# Attribute on the USER record that indicate their username to be used in gitblit

-# when synchronizing users from LDAP

-# if blank, Gitblit will use uid

-# For MS Active Directory this may be sAMAccountName

-realm.ldap.uid = uid

+#

+# SINCE 1.4.0

+realm.ldap.removeDeletedUsers = true

 

 # URL of the Redmine.

+#

+# SINCE 1.2.0

 realm.redmine.url = http://example.com/redmine

 

 #

diff --git a/src/main/java/com/gitblit/auth/AuthenticationProvider.java b/src/main/java/com/gitblit/auth/AuthenticationProvider.java
index f7b75fa3..29051df8 100644
--- a/src/main/java/com/gitblit/auth/AuthenticationProvider.java
+++ b/src/main/java/com/gitblit/auth/AuthenticationProvider.java
@@ -100,6 +100,8 @@ public abstract class AuthenticationProvider {
 
 	public abstract void setup();
 
+	public abstract void stop();
+
 	public abstract UserModel authenticate(String username, char[] password);
 
 	public abstract AccountType getAccountType();
@@ -145,6 +147,11 @@ public abstract class AuthenticationProvider {
     	protected UsernamePasswordAuthenticationProvider(String serviceName) {
     		super(serviceName);
     	}
+
+    	@Override
+		public void stop() {
+
+		}
     }
 
     public static class NullProvider extends AuthenticationProvider {
@@ -159,6 +166,11 @@ public abstract class AuthenticationProvider {
 		}
 
 		@Override
+		public void stop() {
+
+		}
+
+		@Override
 		public UserModel authenticate(String username, char[] password) {
 			return null;
 		}
diff --git a/src/main/java/com/gitblit/auth/LdapAuthProvider.java b/src/main/java/com/gitblit/auth/LdapAuthProvider.java
index 8fef620d..3a688d83 100644
--- a/src/main/java/com/gitblit/auth/LdapAuthProvider.java
+++ b/src/main/java/com/gitblit/auth/LdapAuthProvider.java
@@ -19,12 +19,14 @@ package com.gitblit.auth;
 import java.net.URI;
 import java.net.URISyntaxException;
 import java.security.GeneralSecurityException;
+import java.text.MessageFormat;
 import java.util.Arrays;
 import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
+import java.util.concurrent.Executors;
+import java.util.concurrent.ScheduledExecutorService;
 import java.util.concurrent.TimeUnit;
-import java.util.concurrent.atomic.AtomicLong;
 
 import com.gitblit.Constants;
 import com.gitblit.Constants.AccountType;
@@ -32,6 +34,7 @@ import com.gitblit.Keys;
 import com.gitblit.auth.AuthenticationProvider.UsernamePasswordAuthenticationProvider;
 import com.gitblit.models.TeamModel;
 import com.gitblit.models.UserModel;
+import com.gitblit.service.LdapSyncService;
 import com.gitblit.utils.ArrayUtils;
 import com.gitblit.utils.StringUtils;
 import com.unboundid.ldap.sdk.Attribute;
@@ -57,101 +60,119 @@ import com.unboundid.util.ssl.TrustAllTrustManager;
  */
 public class LdapAuthProvider extends UsernamePasswordAuthenticationProvider {
 
-    private AtomicLong lastLdapUserSync = new AtomicLong(0L);
+	private final ScheduledExecutorService scheduledExecutorService;
 
 	public LdapAuthProvider() {
 		super("ldap");
+
+		scheduledExecutorService = Executors.newSingleThreadScheduledExecutor();
 	}
 
- 	private long getSynchronizationPeriod() {
-        final String cacheDuration = settings.getString(Keys.realm.ldap.ldapCachePeriod, "2 MINUTES");
+ 	private long getSynchronizationPeriodInMilliseconds() {
+ 		String period = settings.getString(Keys.realm.ldap.syncPeriod, null);
+ 		if (StringUtils.isEmpty(period)) {
+ 	 		period = settings.getString("realm.ldap.ldapCachePeriod", null);
+ 	 		if (StringUtils.isEmpty(period)) {
+ 	 			period = "5 MINUTES";
+ 	 		} else {
+ 	 			logger.warn("realm.ldap.ldapCachePeriod is obsolete!");
+ 	 			logger.warn(MessageFormat.format("Please set {0}={1} in gitblit.properties!", Keys.realm.ldap.syncPeriod, period));
+ 	 			settings.overrideSetting(Keys.realm.ldap.syncPeriod, period);
+ 	 		}
+ 		}
+
         try {
-            final String[] s = cacheDuration.split(" ", 2);
-            long duration = Long.parseLong(s[0]);
+            final String[] s = period.split(" ", 2);
+            long duration = Math.abs(Long.parseLong(s[0]));
             TimeUnit timeUnit = TimeUnit.valueOf(s[1]);
             return timeUnit.toMillis(duration);
         } catch (RuntimeException ex) {
-            throw new IllegalArgumentException(Keys.realm.ldap.ldapCachePeriod + " must have format '<long> <TimeUnit>' where <TimeUnit> is one of 'MILLISECONDS', 'SECONDS', 'MINUTES', 'HOURS', 'DAYS'");
+            throw new IllegalArgumentException(Keys.realm.ldap.syncPeriod + " must have format '<long> <TimeUnit>' where <TimeUnit> is one of 'MILLISECONDS', 'SECONDS', 'MINUTES', 'HOURS', 'DAYS'");
         }
     }
 
 	@Override
 	public void setup() {
-		synchronizeLdapUsers();
+		configureSyncService();
 	}
 
-	protected synchronized void synchronizeLdapUsers() {
-        final boolean enabled = settings.getBoolean(Keys.realm.ldap.synchronizeUsers.enable, false);
-        if (enabled) {
-            if (System.currentTimeMillis() > (lastLdapUserSync.get() + getSynchronizationPeriod())) {
-            	logger.info("Synchronizing with LDAP @ " + settings.getRequiredString(Keys.realm.ldap.server));
-                final boolean deleteRemovedLdapUsers = settings.getBoolean(Keys.realm.ldap.synchronizeUsers.removeDeleted, true);
-                LDAPConnection ldapConnection = getLdapConnection();
-                if (ldapConnection != null) {
-                    try {
-                        String accountBase = settings.getString(Keys.realm.ldap.accountBase, "");
-                        String uidAttribute = settings.getString(Keys.realm.ldap.uid, "uid");
-                        String accountPattern = settings.getString(Keys.realm.ldap.accountPattern, "(&(objectClass=person)(sAMAccountName=${username}))");
-                        accountPattern = StringUtils.replace(accountPattern, "${username}", "*");
-
-                        SearchResult result = doSearch(ldapConnection, accountBase, accountPattern);
-                        if (result != null && result.getEntryCount() > 0) {
-                            final Map<String, UserModel> ldapUsers = new HashMap<String, UserModel>();
-
-                            for (SearchResultEntry loggingInUser : result.getSearchEntries()) {
-
-                                final String username = loggingInUser.getAttribute(uidAttribute).getValue();
-                                logger.debug("LDAP synchronizing: " + username);
-
-                                UserModel user = userManager.getUserModel(username);
-                                if (user == null) {
-                                    user = new UserModel(username);
-                                }
-
-                                if (!supportsTeamMembershipChanges()) {
-                                    getTeamsFromLdap(ldapConnection, username, loggingInUser, user);
-                                }
-
-                                // Get User Attributes
-                                setUserAttributes(user, loggingInUser);
-
-                                // store in map
-                                ldapUsers.put(username.toLowerCase(), user);
-                            }
-
-                            if (deleteRemovedLdapUsers) {
-                                logger.debug("detecting removed LDAP users...");
-
-                                for (UserModel userModel : userManager.getAllUsers()) {
-                                    if (Constants.EXTERNAL_ACCOUNT.equals(userModel.password)) {
-                                        if (!ldapUsers.containsKey(userModel.username)) {
-                                            logger.info("deleting removed LDAP user " + userModel.username + " from user service");
-                                            userManager.deleteUser(userModel.username);
-                                        }
-                                    }
-                                }
-                            }
-
-                            userManager.updateUserModels(ldapUsers.values());
-
-                            if (!supportsTeamMembershipChanges()) {
-                                final Map<String, TeamModel> userTeams = new HashMap<String, TeamModel>();
-                                for (UserModel user : ldapUsers.values()) {
-                                    for (TeamModel userTeam : user.teams) {
-                                        userTeams.put(userTeam.name, userTeam);
-                                    }
-                                }
-                                userManager.updateTeamModels(userTeams.values());
-                            }
-                        }
-                        lastLdapUserSync.set(System.currentTimeMillis());
-                    } finally {
-                        ldapConnection.close();
-                    }
-                }
-            }
-        }
-    }
+	@Override
+	public void stop() {
+		scheduledExecutorService.shutdownNow();
+	}
+
+	public synchronized void sync() {
+		final boolean enabled = settings.getBoolean(Keys.realm.ldap.synchronize, false);
+		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) {
+				try {
+					String accountBase = settings.getString(Keys.realm.ldap.accountBase, "");
+					String uidAttribute = settings.getString(Keys.realm.ldap.uid, "uid");
+					String accountPattern = settings.getString(Keys.realm.ldap.accountPattern, "(&(objectClass=person)(sAMAccountName=${username}))");
+					accountPattern = StringUtils.replace(accountPattern, "${username}", "*");
+
+					SearchResult result = doSearch(ldapConnection, accountBase, accountPattern);
+					if (result != null && result.getEntryCount() > 0) {
+						final Map<String, UserModel> ldapUsers = new HashMap<String, UserModel>();
+
+						for (SearchResultEntry loggingInUser : result.getSearchEntries()) {
+
+							final String username = loggingInUser.getAttribute(uidAttribute).getValue();
+							logger.debug("LDAP synchronizing: " + username);
+
+							UserModel user = userManager.getUserModel(username);
+							if (user == null) {
+								user = new UserModel(username);
+							}
+
+							if (!supportsTeamMembershipChanges()) {
+								getTeamsFromLdap(ldapConnection, username, loggingInUser, user);
+							}
+
+							// Get User Attributes
+							setUserAttributes(user, loggingInUser);
+
+							// store in map
+							ldapUsers.put(username.toLowerCase(), user);
+						}
+
+						if (deleteRemovedLdapUsers) {
+							logger.debug("detecting removed LDAP users...");
+
+							for (UserModel userModel : userManager.getAllUsers()) {
+								if (AccountType.LDAP == userModel.accountType) {
+									if (!ldapUsers.containsKey(userModel.username)) {
+										logger.info("deleting removed LDAP user " + userModel.username + " from user service");
+										userManager.deleteUser(userModel.username);
+									}
+								}
+							}
+						}
+
+						userManager.updateUserModels(ldapUsers.values());
+
+						if (!supportsTeamMembershipChanges()) {
+							final Map<String, TeamModel> userTeams = new HashMap<String, TeamModel>();
+							for (UserModel user : ldapUsers.values()) {
+								for (TeamModel userTeam : user.teams) {
+									userTeams.put(userTeam.name, userTeam);
+								}
+							}
+							userManager.updateTeamModels(userTeams.values());
+						}
+					}
+					if (!supportsTeamMembershipChanges()) {
+						getEmptyTeamsFromLdap(ldapConnection);
+					}
+				} finally {
+					ldapConnection.close();
+				}
+			}
+		}
+	}
 
 	private LDAPConnection getLdapConnection() {
 		try {
@@ -427,6 +448,29 @@ public class LdapAuthProvider extends UsernamePasswordAuthenticationProvider {
 		}
 	}
 
+	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);
+		if (teamMembershipResult != null && teamMembershipResult.getEntryCount() > 0) {
+			for (int i = 0; i < teamMembershipResult.getEntryCount(); i++) {
+				SearchResultEntry teamEntry = teamMembershipResult.getSearchEntries().get(i);
+				if (!teamEntry.hasAttribute("member")) {
+					String teamName = teamEntry.getAttribute("cn").getValue();
+
+					TeamModel teamModel = userManager.getTeamModel(teamName);
+					if (teamModel == null) {
+						teamModel = createTeamFromLdap(teamEntry);
+						userManager.updateTeamModel(teamModel);
+					}
+				}
+			}
+		}
+		logger.info("Finished fetching empty teams from ldap.");
+	}
+
 	private TeamModel createTeamFromLdap(SearchResultEntry teamEntry) {
 		TeamModel answer = new TeamModel(teamEntry.getAttributeValue("cn"));
 		answer.accountType = getAccountType();
@@ -519,4 +563,17 @@ public class LdapAuthProvider extends UsernamePasswordAuthenticationProvider {
 		}
 		return sb.toString();
 	}
+
+	private void configureSyncService() {
+		LdapSyncService ldapSyncService = new LdapSyncService(settings, this);
+		if (ldapSyncService.isReady()) {
+			long ldapSyncPeriod = getSynchronizationPeriodInMilliseconds();
+			int delay = 1;
+			logger.info("Ldap sync service will update users and groups every {} minutes.", ldapSyncPeriod);
+			scheduledExecutorService.scheduleAtFixedRate(ldapSyncService, delay, ldapSyncPeriod,  TimeUnit.MILLISECONDS);
+		} else {
+			logger.info("Ldap sync service is disabled.");
+		}
+	}
+
 }
diff --git a/src/main/java/com/gitblit/manager/AuthenticationManager.java b/src/main/java/com/gitblit/manager/AuthenticationManager.java
index cd4a258f..48975142 100644
--- a/src/main/java/com/gitblit/manager/AuthenticationManager.java
+++ b/src/main/java/com/gitblit/manager/AuthenticationManager.java
@@ -150,6 +150,13 @@ public class AuthenticationManager implements IAuthenticationManager {
 
 	@Override
 	public AuthenticationManager stop() {
+		for (AuthenticationProvider provider : authenticationProviders) {
+			try {
+				provider.stop();
+			} catch (Exception e) {
+				logger.error("Failed to stop " + provider.getClass().getSimpleName(), e);
+			}
+		}
 		return this;
 	}
 
diff --git a/src/main/java/com/gitblit/service/LdapSyncService.java b/src/main/java/com/gitblit/service/LdapSyncService.java
new file mode 100644
index 00000000..7ae19aad
--- /dev/null
+++ b/src/main/java/com/gitblit/service/LdapSyncService.java
@@ -0,0 +1,69 @@
+/*

+ * Copyright 2014 gitblit.com.

+ *

+ * Licensed under the Apache License, Version 2.0 (the "License");

+ * you may not use this file except in compliance with the License.

+ * You may obtain a copy of the License at

+ *

+ *     http://www.apache.org/licenses/LICENSE-2.0

+ *

+ * Unless required by applicable law or agreed to in writing, software

+ * distributed under the License is distributed on an "AS IS" BASIS,

+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.

+ * See the License for the specific language governing permissions and

+ * limitations under the License.

+ */

+package com.gitblit.service;

+

+import java.util.concurrent.atomic.AtomicBoolean;

+

+import org.slf4j.Logger;

+import org.slf4j.LoggerFactory;

+

+import com.gitblit.IStoredSettings;

+import com.gitblit.Keys;

+import com.gitblit.auth.LdapAuthProvider;

+

+/**

+ * @author Alfred Schmid

+ *

+ */

+public final class LdapSyncService implements Runnable {

+

+	private final Logger logger = LoggerFactory.getLogger(LdapSyncService.class);

+

+	private final IStoredSettings settings;

+

+	private final LdapAuthProvider ldapAuthProvider;

+

+	private final AtomicBoolean running = new AtomicBoolean(false);

+

+	public LdapSyncService(IStoredSettings settings, LdapAuthProvider ldapAuthProvider) {

+		this.settings = settings;

+		this.ldapAuthProvider = ldapAuthProvider;

+	}

+

+	/**

+	 *

+	 * @see java.lang.Runnable#run()

+	 */

+	@Override

+	public void run() {

+		logger.info("Starting user and group sync with ldap service");

+		if (!running.getAndSet(true)) {

+			try {

+				ldapAuthProvider.sync();

+			} catch (Exception e) {

+				logger.error("Failed to synchronize with ldap", e);

+			} finally {

+				running.getAndSet(false);

+			}

+		}

+		logger.info("Finished user and group sync with ldap service");

+	}

+

+	public boolean isReady() {

+		return settings.getBoolean(Keys.realm.ldap.synchronize, false);

+	}

+

+}

diff --git a/src/test/config/test-users.conf b/src/test/config/test-users.conf
index 59b6df49..1d01f846 100644
--- a/src/test/config/test-users.conf
+++ b/src/test/config/test-users.conf
@@ -4,6 +4,11 @@
 	accountType = LOCAL
 	role = "#admin"
 	role = "#notfederated"
+[user "sampleuser"]
+	password = sampleuser
+	cookie = 6e07ed42149fc166206319faffdfba2e2ec82e43
+	accountType = LOCAL
+	role = "#none"
 [team "admins"]
 	role = "#none"
 	accountType = LOCAL
diff --git a/src/test/java/com/gitblit/tests/GitBlitSuite.java b/src/test/java/com/gitblit/tests/GitBlitSuite.java
index 7b38c183..9fe7312c 100644
--- a/src/test/java/com/gitblit/tests/GitBlitSuite.java
+++ b/src/test/java/com/gitblit/tests/GitBlitSuite.java
@@ -63,7 +63,7 @@ import com.gitblit.utils.JGitUtils;
 		GitBlitTest.class, FederationTests.class, RpcTests.class, GitServletTest.class, GitDaemonTest.class,

 		GroovyScriptTest.class, LuceneExecutorTest.class, RepositoryModelTest.class,

 		FanoutServiceTest.class, Issue0259Test.class, Issue0271Test.class, HtpasswdAuthenticationTest.class,

-		ModelUtilsTest.class, JnaUtilsTest.class })

+		ModelUtilsTest.class, JnaUtilsTest.class, LdapSyncServiceTest.class })

 public class GitBlitSuite {

 

 	public static final File BASEFOLDER = new File("data");

diff --git a/src/test/java/com/gitblit/tests/LdapAuthenticationTest.java b/src/test/java/com/gitblit/tests/LdapAuthenticationTest.java
index 3cd2dc72..e4dc2db2 100644
--- a/src/test/java/com/gitblit/tests/LdapAuthenticationTest.java
+++ b/src/test/java/com/gitblit/tests/LdapAuthenticationTest.java
@@ -16,23 +16,33 @@
  */
 package com.gitblit.tests;
 
+import java.io.File;
 import java.io.FileInputStream;
 import java.util.HashMap;
 import java.util.Map;
 
+import org.apache.commons.io.FileUtils;
 import org.junit.Before;
 import org.junit.BeforeClass;
+import org.junit.Rule;
 import org.junit.Test;
+import org.junit.rules.TemporaryFolder;
 
+import com.gitblit.Constants.AccountType;
 import com.gitblit.IStoredSettings;
+import com.gitblit.Keys;
 import com.gitblit.auth.LdapAuthProvider;
+import com.gitblit.manager.IUserManager;
 import com.gitblit.manager.RuntimeManager;
 import com.gitblit.manager.UserManager;
+import com.gitblit.models.TeamModel;
 import com.gitblit.models.UserModel;
 import com.gitblit.tests.mock.MemorySettings;
 import com.unboundid.ldap.listener.InMemoryDirectoryServer;
 import com.unboundid.ldap.listener.InMemoryDirectoryServerConfig;
 import com.unboundid.ldap.listener.InMemoryListenerConfig;
+import com.unboundid.ldap.sdk.SearchResult;
+import com.unboundid.ldap.sdk.SearchScope;
 import com.unboundid.ldif.LDIFReader;
 
 /**
@@ -43,13 +53,23 @@ import com.unboundid.ldif.LDIFReader;
  *
  */
 public class LdapAuthenticationTest extends GitblitUnitTest {
+    @Rule
+    public TemporaryFolder folder = new TemporaryFolder();
 
 	private static final String RESOURCE_DIR = "src/test/resources/ldap/";
 
-	private LdapAuthProvider ldap;
+    private File usersConf;
+
+    private LdapAuthProvider ldap;
 
 	static int ldapPort = 1389;
 
+	private static InMemoryDirectoryServer ds;
+
+	private IUserManager userManager;
+
+	private MemorySettings settings;
+
 	@BeforeClass
 	public static void createInMemoryLdapServer() throws Exception {
 		InMemoryDirectoryServerConfig config = new InMemoryDirectoryServerConfig("dc=MyDomain");
@@ -57,40 +77,45 @@ public class LdapAuthenticationTest extends GitblitUnitTest {
 		config.setListenerConfigs(InMemoryListenerConfig.createLDAPConfig("default", ldapPort));
 		config.setSchema(null);
 
-		InMemoryDirectoryServer ds = new InMemoryDirectoryServer(config);
-		ds.importFromLDIF(true, new LDIFReader(new FileInputStream(RESOURCE_DIR + "sampledata.ldif")));
+		ds = new InMemoryDirectoryServer(config);
 		ds.startListening();
 	}
 
 	@Before
-	public void newLdapAuthentication() {
-		ldap = newLdapAuthentication(getSettings());
+	public void init() throws Exception {
+		ds.clear();
+		ds.importFromLDIF(true, new LDIFReader(new FileInputStream(RESOURCE_DIR + "sampledata.ldif")));
+		usersConf = folder.newFile("users.conf");
+		FileUtils.copyFile(new File(RESOURCE_DIR + "users.conf"), usersConf);
+		settings = getSettings();
+		ldap = newLdapAuthentication(settings);
 	}
 
-	public LdapAuthProvider newLdapAuthentication(IStoredSettings settings) {
+	private LdapAuthProvider newLdapAuthentication(IStoredSettings settings) {
 		RuntimeManager runtime = new RuntimeManager(settings, GitBlitSuite.BASEFOLDER).start();
-		UserManager users = new UserManager(runtime).start();
+		userManager = new UserManager(runtime).start();
 		LdapAuthProvider ldap = new LdapAuthProvider();
-		ldap.setup(runtime, users);
+		ldap.setup(runtime, userManager);
 		return ldap;
 	}
 
 	private MemorySettings getSettings() {
 		Map<String, Object> backingMap = new HashMap<String, Object>();
-		backingMap.put("realm.userService", RESOURCE_DIR + "users.conf");
-		backingMap.put("realm.ldap.server", "ldap://localhost:" + ldapPort);
-		backingMap.put("realm.ldap.domain", "");
-		backingMap.put("realm.ldap.username", "cn=Directory Manager");
-		backingMap.put("realm.ldap.password", "password");
-		backingMap.put("realm.ldap.backingUserService", "users.conf");
-		backingMap.put("realm.ldap.maintainTeams", "true");
-		backingMap.put("realm.ldap.accountBase", "OU=Users,OU=UserControl,OU=MyOrganization,DC=MyDomain");
-		backingMap.put("realm.ldap.accountPattern", "(&(objectClass=person)(sAMAccountName=${username}))");
-		backingMap.put("realm.ldap.groupBase", "OU=Groups,OU=UserControl,OU=MyOrganization,DC=MyDomain");
-		backingMap.put("realm.ldap.groupPattern", "(&(objectClass=group)(member=${dn}))");
-		backingMap.put("realm.ldap.admins", "UserThree @Git_Admins \"@Git Admins\"");
-		backingMap.put("realm.ldap.displayName", "displayName");
-		backingMap.put("realm.ldap.email", "email");
+		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");
+		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.accountPattern, "(&(objectClass=person)(sAMAccountName=${username}))");
+		backingMap.put(Keys.realm.ldap.groupBase, "OU=Groups,OU=UserControl,OU=MyOrganization,DC=MyDomain");
+		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");
+		backingMap.put(Keys.realm.ldap.email, "email");
+		backingMap.put(Keys.realm.ldap.uid, "sAMAccountName");
 
 		MemorySettings ms = new MemorySettings(backingMap);
 		return ms;
@@ -162,4 +187,60 @@ public class LdapAuthenticationTest extends GitblitUnitTest {
 		assertNull(userOneModel);
 	}
 
+	@Test
+	public void checkIfUsersConfContainsAllUsersFromSampleDataLdif() throws Exception {
+		SearchResult searchResult = ds.search("OU=Users,OU=UserControl,OU=MyOrganization,DC=MyDomain", 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"));
+		ldap.sync();
+		assertEquals("Number of ldap users in gitblit user model", 5, countLdapUsersInUserManager());
+	}
+
+	@Test
+	public void addingUserInLdapShouldUpdateGitBlitUsersAndGroups() throws Exception {
+		settings.put(Keys.realm.ldap.synchronize, "true");
+		ds.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"));
+		ldap.sync();
+		assertEquals("Number of ldap groups in gitblit team model", 0, countLdapTeamsInUserManager());
+	}
+
+	@Test
+	public void addingGroupsInLdapShouldUpdateGitBlitUsersAndGroups() throws Exception {
+		settings.put(Keys.realm.ldap.synchronize, "true");
+		ds.addEntries(LDIFReader.readEntries(RESOURCE_DIR + "addgroup.ldif"));
+		ldap.sync();
+		assertEquals("Number of ldap groups in gitblit team model", 1, countLdapTeamsInUserManager());
+	}
+
+	private int countLdapUsersInUserManager() {
+		int ldapAccountCount = 0;
+		for (UserModel userModel : userManager.getAllUsers()) {
+			if (AccountType.LDAP.equals(userModel.accountType)) {
+				ldapAccountCount++;
+			}
+		}
+		return ldapAccountCount;
+	}
+
+	private int countLdapTeamsInUserManager() {
+		int ldapAccountCount = 0;
+		for (TeamModel teamModel : userManager.getAllTeams()) {
+			if (AccountType.LDAP.equals(teamModel.accountType)) {
+				ldapAccountCount++;
+			}
+		}
+		return ldapAccountCount;
+	}
+
 }
diff --git a/src/test/java/com/gitblit/tests/LdapSyncServiceTest.java b/src/test/java/com/gitblit/tests/LdapSyncServiceTest.java
new file mode 100644
index 00000000..eda09034
--- /dev/null
+++ b/src/test/java/com/gitblit/tests/LdapSyncServiceTest.java
@@ -0,0 +1,72 @@
+/*

+ * Copyright 2014 gitblit.com.

+ *

+ * Licensed under the Apache License, Version 2.0 (the "License");

+ * you may not use this file except in compliance with the License.

+ * You may obtain a copy of the License at

+ *

+ *     http://www.apache.org/licenses/LICENSE-2.0

+ *

+ * Unless required by applicable law or agreed to in writing, software

+ * distributed under the License is distributed on an "AS IS" BASIS,

+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.

+ * See the License for the specific language governing permissions and

+ * limitations under the License.

+ */

+package com.gitblit.tests;

+

+import static org.junit.Assert.assertFalse;

+import static org.junit.Assert.assertTrue;

+

+import java.util.HashMap;

+import java.util.Map;

+

+import org.junit.Before;

+import org.junit.Test;

+

+import com.gitblit.Keys;

+import com.gitblit.service.LdapSyncService;

+import com.gitblit.tests.mock.MemorySettings;

+

+/**

+ * A behavior driven test for the LdapSyncService with in-memory Settings.

+ *

+ * @author Alfred Schmid

+ *

+ */

+public class LdapSyncServiceTest {

+

+	private MemorySettings settings;

+

+	@Before

+	public void init() throws Exception {

+		settings = getSettings();

+	}

+

+	@Test

+	public void defaultOfUnAvailableLdapSynchronizeKeyIsLdapServiceNotReady() {

+		LdapSyncService ldapSyncService = new LdapSyncService(settings, null);

+		assertFalse("When key " + Keys.realm.ldap.synchronize + " is not configured ldap sync is not ready." , ldapSyncService.isReady());

+	}

+

+	@Test

+	public void whenLdapSynchronizeKeyIsFalseLdapServiceNotReady() {

+		LdapSyncService ldapSyncService = new LdapSyncService(settings, null);

+		settings.put(Keys.realm.ldap.synchronize, "false");

+		assertFalse("When key " + Keys.realm.ldap.synchronize + " is configured with value false ldap sync is not ready." , ldapSyncService.isReady());

+	}

+

+	@Test

+	public void whenLdapSynchronizeKeyIsTrueLdapServiceIsReady() {

+		LdapSyncService ldapSyncService = new LdapSyncService(settings, null);

+		settings.put(Keys.realm.ldap.synchronize, "true");

+		assertTrue("When key " + Keys.realm.ldap.synchronize + " is configured with value true ldap sync is not ready." , ldapSyncService.isReady());

+	}

+

+	private MemorySettings getSettings() {

+		Map<String, Object> backingMap = new HashMap<String, Object>();

+		MemorySettings ms = new MemorySettings(backingMap);

+		return ms;

+	}

+

+}

diff --git a/src/test/resources/ldap/addgroup.ldif b/src/test/resources/ldap/addgroup.ldif
new file mode 100644
index 00000000..665df31b
--- /dev/null
+++ b/src/test/resources/ldap/addgroup.ldif
@@ -0,0 +1,5 @@
+dn: CN=Git_Group_Without_Members,OU=Groups,OU=UserControl,OU=MyOrganization,DC=MyDomain
+objectClass: top
+objectClass: group
+cn: Git_Group_Without_Members
+sAMAccountName: Git_Group_Without_Members
diff --git a/src/test/resources/ldap/adduser.ldif b/src/test/resources/ldap/adduser.ldif
new file mode 100644
index 00000000..b007a940
--- /dev/null
+++ b/src/test/resources/ldap/adduser.ldif
@@ -0,0 +1,11 @@
+dn: CN=UserSix,OU=Canada,OU=Users,OU=UserControl,OU=MyOrganization,DC=MyDomain
+objectClass: user
+objectClass: person
+sAMAccountName: UserSix
+userPassword: userSixPassword
+displayName: User Six
+givenName: User
+surname: Six
+personalTitle: Miss
+email: usersix@gitblit.com
+memberOf: CN=Git_Users,OU=Groups,OU=UserControl,OU=MyOrganization,DC=MyDomain
\ No newline at end of file
diff --git a/src/test/resources/ldap/sampledata.ldif b/src/test/resources/ldap/sampledata.ldif
index df79333e..6254f19c 100644
--- a/src/test/resources/ldap/sampledata.ldif
+++ b/src/test/resources/ldap/sampledata.ldif
@@ -105,4 +105,16 @@ givenName: User
 surname: Four
 personalTitle: Miss
 email: userfour@gitblit.com
+memberOf: CN=Git_Users,OU=Groups,OU=UserControl,OU=MyOrganization,DC=MyDomain
+
+dn: CN=UserFive,OU=Canada,OU=Users,OU=UserControl,OU=MyOrganization,DC=MyDomain
+objectClass: user
+objectClass: person
+sAMAccountName: UserFive
+userPassword: userFivePassword
+displayName: User Five
+givenName: User
+surname: Five
+personalTitle: Miss
+email: userfive@gitblit.com
 memberOf: CN=Git_Users,OU=Groups,OU=UserControl,OU=MyOrganization,DC=MyDomain
\ No newline at end of file
diff --git a/src/test/resources/ldap/users.conf b/src/test/resources/ldap/users.conf
index b4b4a6ea..7d1e3197 100644
--- a/src/test/resources/ldap/users.conf
+++ b/src/test/resources/ldap/users.conf
@@ -7,10 +7,17 @@
 [user "userthree"]
 	password = "#externalAccount"
 	cookie = d7d3894fc517612aa6c595555b6e1ab8e147e597
-	displayName = User Three
+	displayName = Mrs. User Three
 	emailAddress = userthree@gitblit.com
 	accountType = LDAP
 	role = "#admin"
+[user "userfive"]
+	password = "#externalAccount"
+	cookie = 220bafef069b8b399b2597644015b6b0f4667982
+	displayName = Miss. User Five
+	emailAddress = userfive@gitblit.com
+	accountType = LDAP
+	role = "#none"
 [user "userone"]
 	password = "#externalAccount"
 	cookie = c97cd38e50858cd0b389ec61b18fb9a89b4da54c
@@ -21,7 +28,7 @@
 [user "usertwo"]
 	password = "#externalAccount"
 	cookie = 498ca9bd2841d39050fa45d1d737b9f9f767858d
-	displayName = User Two
+	displayName = Mr. User Two
 	emailAddress = usertwo@gitblit.com
 	accountType = LDAP
 	role = "#admin"
@@ -37,6 +44,13 @@
 	cookie = dd94709528bb1c83d08f3088d4043f4742891f4f
 	accountType = LOCAL
 	role = "#create"
+[user "userfour"]
+	password = "#externalAccount"
+	cookie = d4fca47ebc70f0495947f8b6158ac6edb546e2fe
+	displayName = Miss. User Four
+	emailAddress = userfour@gitblit.com
+	accountType = LDAP
+	role = "#none"
 [team "Git_Admins"]
 	role = "#none"
 	accountType = LOCAL
@@ -47,6 +61,7 @@
 	user = userone
 	user = usertwo
 	user = userthree
+	user = userfour
 [team "Git Admins"]
 	role = "#none"
 	accountType = LOCAL