From 64b6f382d35e1fea0172b222277dae0312f274e4 Mon Sep 17 00:00:00 2001 From: James Moger <james.moger@gitblit.com> Date: Mon, 25 Feb 2013 09:41:49 -0500 Subject: [PATCH] Merged LDAP user/team pre-fetching and synchronization enhancement from mschaefers --- docs/04_releases.mkd | 1 src/com/gitblit/LdapUserService.java | 147 +++++++++++++++++++++++++++++++----- src/com/gitblit/ConfigUserService.java | 6 src/com/gitblit/IUserService.java | 5 distrib/gitblit.properties | 28 +++++++ src/com/gitblit/GitblitUserService.java | 5 src/com/gitblit/FileUserService.java | 7 + 7 files changed, 168 insertions(+), 31 deletions(-) diff --git a/distrib/gitblit.properties b/distrib/gitblit.properties index f5cc19b..80790d3 100644 --- a/distrib/gitblit.properties +++ b/distrib/gitblit.properties @@ -1143,6 +1143,34 @@ # 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 +# +# RESTART REQUIRED +realm.ldap.ldapCachePeriod = 2 MINUTES + +# Defines whether to synchronize all LDAP users into the backing user service +# +# Valid values: true, false +# If left blank, false is assumed +realm.ldap.synchronizeUsers.enable = false + +# Defines whether to delete non-existent LDAP users from the backing user service +# during synchronization. depends on realm.ldap.synchronizeUsers.enable = 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 + # The RedmineUserService must be backed by another user service for standard user # and team management. # default: users.conf diff --git a/docs/04_releases.mkd b/docs/04_releases.mkd index 93263c6..efce794 100644 --- a/docs/04_releases.mkd +++ b/docs/04_releases.mkd @@ -10,6 +10,7 @@ #### additions + - Optional periodic LDAP user and team pre-fetching & synchronization (github/mschaefers) - Display name and version in Tomcat Manager (github/thefake) - FogBugz post-receive hook script (github/djschny) - Implemented multiple repository owners (github/akquinet) diff --git a/src/com/gitblit/ConfigUserService.java b/src/com/gitblit/ConfigUserService.java index 67ad053..7aa0998 100644 --- a/src/com/gitblit/ConfigUserService.java +++ b/src/com/gitblit/ConfigUserService.java @@ -20,6 +20,7 @@ import java.text.MessageFormat; import java.util.ArrayList; import java.util.Arrays; +import java.util.Collection; import java.util.Collections; import java.util.HashSet; import java.util.List; @@ -286,7 +287,7 @@ * @since 1.2.0 */ @Override - public boolean updateUserModels(List<UserModel> models) { + public boolean updateUserModels(Collection<UserModel> models) { try { read(); for (UserModel model : models) { @@ -566,7 +567,7 @@ * @since 1.2.0 */ @Override - public boolean updateTeamModels(List<TeamModel> models) { + public boolean updateTeamModels(Collection<TeamModel> models) { try { read(); for (TeamModel team : models) { @@ -808,7 +809,6 @@ /** * Writes the properties file. * - * @param properties * @throws IOException */ private synchronized void write() throws IOException { diff --git a/src/com/gitblit/FileUserService.java b/src/com/gitblit/FileUserService.java index 056df82..32c24cc 100644 --- a/src/com/gitblit/FileUserService.java +++ b/src/com/gitblit/FileUserService.java @@ -20,6 +20,7 @@ import java.io.IOException; import java.text.MessageFormat; import java.util.ArrayList; +import java.util.Collection; import java.util.Collections; import java.util.HashSet; import java.util.List; @@ -270,12 +271,12 @@ /** * Updates/writes all specified user objects. * - * @param model a list of user models + * @param models a list of user models * @return true if update is successful * @since 1.2.0 */ @Override - public boolean updateUserModels(List<UserModel> models) { + public boolean updateUserModels(Collection<UserModel> models) { try { Properties allUsers = read(); for (UserModel model : models) { @@ -997,7 +998,7 @@ * @return true if update is successful * @since 1.2.0 */ - public boolean updateTeamModels(List<TeamModel> models) { + public boolean updateTeamModels(Collection<TeamModel> models) { try { Properties allUsers = read(); for (TeamModel model : models) { diff --git a/src/com/gitblit/GitblitUserService.java b/src/com/gitblit/GitblitUserService.java index 37f22b0..fe35db9 100644 --- a/src/com/gitblit/GitblitUserService.java +++ b/src/com/gitblit/GitblitUserService.java @@ -18,6 +18,7 @@ import java.io.File; import java.io.IOException; import java.text.MessageFormat; +import java.util.Collection; import java.util.List; import org.slf4j.Logger; @@ -178,7 +179,7 @@ } @Override - public boolean updateUserModels(List<UserModel> models) { + public boolean updateUserModels(Collection<UserModel> models) { return serviceImpl.updateUserModels(models); } @@ -267,7 +268,7 @@ } @Override - public boolean updateTeamModels(List<TeamModel> models) { + public boolean updateTeamModels(Collection<TeamModel> models) { return serviceImpl.updateTeamModels(models); } diff --git a/src/com/gitblit/IUserService.java b/src/com/gitblit/IUserService.java index 059d648..a57b0da 100644 --- a/src/com/gitblit/IUserService.java +++ b/src/com/gitblit/IUserService.java @@ -15,6 +15,7 @@ */ package com.gitblit; +import java.util.Collection; import java.util.List; import com.gitblit.models.TeamModel; @@ -133,7 +134,7 @@ * @return true if update is successful * @since 1.2.0 */ - boolean updateUserModels(List<UserModel> models); + boolean updateUserModels(Collection<UserModel> models); /** * Adds/updates a user object keyed by username. This method allows for @@ -243,7 +244,7 @@ * @return true if update is successful * @since 1.2.0 */ - boolean updateTeamModels(List<TeamModel> models); + boolean updateTeamModels(Collection<TeamModel> models); /** * Updates/writes and replaces a complete team object keyed by teamname. diff --git a/src/com/gitblit/LdapUserService.java b/src/com/gitblit/LdapUserService.java index 595c658..2867b88 100644 --- a/src/com/gitblit/LdapUserService.java +++ b/src/com/gitblit/LdapUserService.java @@ -20,7 +20,11 @@ import java.net.URI; import java.net.URISyntaxException; import java.security.GeneralSecurityException; +import java.util.HashMap; import java.util.List; +import java.util.Map; +import java.util.concurrent.TimeUnit; +import java.util.concurrent.atomic.AtomicLong; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -53,20 +57,107 @@ public static final Logger logger = LoggerFactory.getLogger(LdapUserService.class); private IStoredSettings settings; + private AtomicLong lastLdapUserSync = new AtomicLong(0L); public LdapUserService() { super(); } + private long getSynchronizationPeriod() { + final String cacheDuration = settings.getString(Keys.realm.ldap.ldapCachePeriod, "2 MINUTES"); + try { + final String[] s = cacheDuration.split(" ", 2); + long duration = 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'"); + } + } + @Override public void setup(IStoredSettings settings) { this.settings = settings; String file = settings.getString(Keys.realm.ldap.backingUserService, "${baseFolder}/users.conf"); File realmFile = GitBlit.getFileOrFolder(file); - + serviceImpl = createUserService(realmFile); logger.info("LDAP User Service backed by " + serviceImpl.toString()); + + synchronizeLdapUsers(); } + + 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 = 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 : super.getAllUsers()) { + if (ExternalAccount.equals(userModel.password)) { + if (! ldapUsers.containsKey(userModel.username)) { + logger.info("deleting removed LDAP user " + userModel.username + " from backing user service"); + super.deleteUser(userModel.username); + } + } + } + } + + super.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); + } + } + updateTeamModels(userTeams.values()); + } + } + lastLdapUserSync.set(System.currentTimeMillis()); + } finally { + ldapConnection.close(); + } + } + } + } + } private LDAPConnection getLdapConnection() { try { @@ -187,28 +278,31 @@ if (isAuthenticated(ldapConnection, loggingInUserDN, new String(password))) { logger.debug("LDAP authenticated: " + username); - UserModel user = getUserModel(simpleUsername); - if (user == null) // create user object for new authenticated user - user = new UserModel(simpleUsername); + UserModel user = null; + synchronized (this) { + user = getUserModel(simpleUsername); + if (user == null) // create user object for new authenticated user + user = new UserModel(simpleUsername); - // create a user cookie - if (StringUtils.isEmpty(user.cookie) && !ArrayUtils.isEmpty(password)) { - user.cookie = StringUtils.getSHA1(user.username + new String(password)); + // create a user cookie + if (StringUtils.isEmpty(user.cookie) && !ArrayUtils.isEmpty(password)) { + user.cookie = StringUtils.getSHA1(user.username + new String(password)); + } + + if (!supportsTeamMembershipChanges()) + getTeamsFromLdap(ldapConnection, simpleUsername, loggingInUser, user); + + // Get User Attributes + setUserAttributes(user, loggingInUser); + + // Push the ldap looked up values to backing file + super.updateUserModel(user); + if (!supportsTeamMembershipChanges()) { + for (TeamModel userTeam : user.teams) + updateTeamModel(userTeam); + } } - - if (!supportsTeamMembershipChanges()) - getTeamsFromLdap(ldapConnection, simpleUsername, loggingInUser, user); - - // Get User Attributes - setUserAttributes(user, loggingInUser); - - // Push the ldap looked up values to backing file - super.updateUserModel(user); - if (!supportsTeamMembershipChanges()) { - for (TeamModel userTeam : user.teams) - updateTeamModel(userTeam); - } - + return user; } } @@ -345,6 +439,17 @@ } } + @Override + public List<String> getAllUsernames() { + synchronizeLdapUsers(); + return super.getAllUsernames(); + } + + @Override + public List<UserModel> getAllUsers() { + synchronizeLdapUsers(); + return super.getAllUsers(); + } /** * Returns a simple username without any domain prefixes. -- Gitblit v1.9.1