Add logic to get display name & email (Simply stored in user model for now).

Add code / test to defend against LDAP injection attacks.

diff --git a/distrib/gitblit.properties b/distrib/gitblit.properties
index 527b7268657e05ea23f4df6258225662d42fc3fc..62222d4e7ac43237c132380589172eb69a350248 100644 (file)
--- a/distrib/gitblit.properties
+++ b/distrib/gitblit.properties
@@ -221,6 +221,28 @@ realm.ldap.groupMemberPattern = (&(objectClass=group)(member=${dn}))
 # SINCE 1.0.0\r
 realm.ldap.admins= @Git_Admins\r
 \r
+# Attribute(s) on the USER record that indicate their display (or full) name. Leave blank\r
+# for no mapping available in LDAP\r
+#\r
+# This may be a single attribute, or a string of multiple attributes.  Examples:\r
+#  displayName - Uses the attribute 'displayName' on the user record\r
+#  ${personalTitle}. ${givenName} ${surname} - Will concatenate the 3 \r
+#       attributes together, with a '.' after personalTitle \r
+#\r
+# SINCE 1.0.0\r
+realm.ldap.displayName= displayName\r
+\r
+# Attribute(s) on the USER record that indicate their email address.  Leave blank\r
+# for no mapping available in LDAP\r
+#\r
+# This may be a single attribute, or a string of multiple attributes.  Examples:\r
+#  email - Uses the attribute 'email' on the user record\r
+#  ${givenName}.${surname}@gitblit.com -Will concatenate the 2 attributes\r
+#       together with a '.' and '@' creating something like first.last@gitblit.com \r
+#\r
+# SINCE 1.0.0\r
+realm.ldap.email = email\r
+\r
 #\r
 # Gitblit Web Settings\r
 #\r

diff --git a/src/com/gitblit/LdapUserService.java b/src/com/gitblit/LdapUserService.java
index 86b6136406a8fb750475104d9a08d5ca807cab3c..674e2a0d05634d07bd011237ef737de654a49046 100644 (file)
--- a/src/com/gitblit/LdapUserService.java
+++ b/src/com/gitblit/LdapUserService.java
@@ -137,7 +137,7 @@ public class LdapUserService extends GitblitUserService {
                        // Find the logging in user's DN\r
                        String accountBase = settings.getString(Keys.realm.ldap_accountBase, "");\r
                        String accountPattern = settings.getString(Keys.realm.ldap_accountPattern, "(&(objectClass=person)(sAMAccountName=${username}))");\r
-                       accountPattern = StringUtils.replace(accountPattern, "${username}", simpleUsername);\r
+                       accountPattern = StringUtils.replace(accountPattern, "${username}", escapeLDAPSearchFilter(simpleUsername));\r
 \r
                        SearchResult result = doSearch(ldapConnection, accountBase, accountPattern);\r
                        if (result != null && result.getEntryCount() == 1) {\r
@@ -149,15 +149,15 @@ public class LdapUserService extends GitblitUserService {
                                        \r
                                        UserModel user = getUserModel(simpleUsername);\r
                                        if (user == null)       // create user object for new authenticated user\r
-                                               user = createUserFromLdap(simpleUsername, loggingInUser);\r
+                                               user = new UserModel(simpleUsername);\r
+                                       \r
                                        \r
-                                       user.password = "StoredInLDAP";\r
                                        \r
                                        if (!supportsTeamMembershipChanges())\r
                                                getTeamsFromLdap(ldapConnection, simpleUsername, loggingInUser, user);\r
                                        \r
-                                       // Get Admin Attributes\r
-                                       setAdminAttribute(user);\r
+                                       // Get User Attributes\r
+                                       setUserAttributes(user, loggingInUser);\r
 \r
                                        // Push the ldap looked up values to backing file\r
                                        super.updateUserModel(user);\r
@@ -186,6 +186,37 @@ public class LdapUserService extends GitblitUserService {
                        user.canAdmin = true;\r
            }\r
        }\r
+       \r
+       private void setUserAttributes(UserModel user, SearchResultEntry userEntry) {\r
+               // Is this user an admin?\r
+               setAdminAttribute(user);\r
+               \r
+               // Don't want visibility into the real password, make up a dummy\r
+               user.password = "StoredInLDAP";\r
+               \r
+               // Get Attributes for full name / email\r
+               String displayName = settings.getString(Keys.realm.ldap_displayName, "displayName");\r
+               String email = settings.getString(Keys.realm.ldap_email, "email");\r
+\r
+               // Replace embedded ${} with attributes\r
+               if (displayName.contains("${")) {\r
+                       for (Attribute userAttribute : userEntry.getAttributes())\r
+                               displayName = StringUtils.replace(displayName, "${" + userAttribute.getName() + "}", userAttribute.getValue());\r
+                       \r
+                       user.displayName = displayName;\r
+               } else {\r
+                       user.displayName = userEntry.getAttribute(displayName).getValue();\r
+               }\r
+               \r
+               if (email.contains("${")) {\r
+                       for (Attribute userAttribute : userEntry.getAttributes())\r
+                               email = StringUtils.replace(email, "${" + userAttribute.getName() + "}", userAttribute.getValue());\r
+                       \r
+                       user.emailAddress = email;\r
+               } else {\r
+                       user.emailAddress = userEntry.getAttribute(email).getValue();\r
+               }\r
+       }\r
 \r
        private void getTeamsFromLdap(LDAPConnection ldapConnection, String simpleUsername, SearchResultEntry loggingInUser, UserModel user) {\r
                String loggingInUserDN = loggingInUser.getDN();\r
@@ -194,12 +225,12 @@ public class LdapUserService extends GitblitUserService {
                String groupBase = settings.getString(Keys.realm.ldap_groupBase, "");\r
                String groupMemberPattern = settings.getString(Keys.realm.ldap_groupMemberPattern, "(&(objectClass=group)(member=${dn}))");\r
                \r
-               groupMemberPattern = StringUtils.replace(groupMemberPattern, "${dn}", loggingInUserDN);\r
-               groupMemberPattern = StringUtils.replace(groupMemberPattern, "${username}", simpleUsername);\r
+               groupMemberPattern = StringUtils.replace(groupMemberPattern, "${dn}", escapeLDAPSearchFilter(loggingInUserDN));\r
+               groupMemberPattern = StringUtils.replace(groupMemberPattern, "${username}", escapeLDAPSearchFilter(simpleUsername));\r
                \r
                // Fill in attributes into groupMemberPattern\r
                for (Attribute userAttribute : loggingInUser.getAttributes())\r
-                       groupMemberPattern = StringUtils.replace(groupMemberPattern, "${" + userAttribute.getName() + "}", userAttribute.getValue());\r
+                       groupMemberPattern = StringUtils.replace(groupMemberPattern, "${" + userAttribute.getName() + "}", escapeLDAPSearchFilter(userAttribute.getValue()));\r
                \r
                SearchResult teamMembershipResult = doSearch(ldapConnection, groupBase, groupMemberPattern);\r
                if (teamMembershipResult != null && teamMembershipResult.getEntryCount() > 0) {\r
@@ -223,13 +254,6 @@ public class LdapUserService extends GitblitUserService {
                \r
                return answer;          \r
        }\r
-       \r
-       private UserModel createUserFromLdap(String simpleUserName, SearchResultEntry userEntry) {\r
-               UserModel answer = new UserModel(simpleUserName);\r
-               //If attributes other than user name ever from from LDAP, this is where to get them\r
-               \r
-               return answer;\r
-       }\r
 \r
        private SearchResult doSearch(LDAPConnection ldapConnection, String base, String filter) {\r
                try {\r
@@ -243,6 +267,7 @@ public class LdapUserService extends GitblitUserService {
        \r
        private boolean isAuthenticated(LDAPConnection ldapConnection, String userDn, String password) {\r
                try {\r
+                       // Binding will stop any LDAP-Injection Attacks since the searched-for user needs to bind to that DN\r
                        ldapConnection.bind(userDn, password);\r
                        return true;\r
                } catch (LDAPException e) {\r
@@ -263,6 +288,35 @@ public class LdapUserService extends GitblitUserService {
                if (lastSlash > -1) {\r
                        username = username.substring(lastSlash + 1);\r
                }\r
+               \r
                return username;\r
        }\r
+       \r
+       // From: https://www.owasp.org/index.php/Preventing_LDAP_Injection_in_Java\r
+       public static final String escapeLDAPSearchFilter(String filter) {\r
+               StringBuilder sb = new StringBuilder();\r
+               for (int i = 0; i < filter.length(); i++) {\r
+                       char curChar = filter.charAt(i);\r
+                       switch (curChar) {\r
+                       case '\\':\r
+                               sb.append("\\5c");\r
+                               break;\r
+                       case '*':\r
+                               sb.append("\\2a");\r
+                               break;\r
+                       case '(':\r
+                               sb.append("\\28");\r
+                               break;\r
+                       case ')':\r
+                               sb.append("\\29");\r
+                               break;\r
+                       case '\u0000': \r
+                               sb.append("\\00"); \r
+                               break;\r
+                       default:\r
+                               sb.append(curChar);\r
+                       }\r
+               }\r
+               return sb.toString();\r
+       }\r
 }\r

diff --git a/src/com/gitblit/models/UserModel.java b/src/com/gitblit/models/UserModel.java
index ecb97cfca706a1ed0c0ac2402e10ff93f275c266..925edf91ee83013bf24150e0126f0467b6c9c6c6 100644 (file)
--- a/src/com/gitblit/models/UserModel.java
+++ b/src/com/gitblit/models/UserModel.java
@@ -37,6 +37,8 @@ public class UserModel implements Principal, Serializable, Comparable<UserModel>
        // field names are reflectively mapped in EditUser page\r
        public String username;\r
        public String password;\r
+       public String displayName;\r
+       public String emailAddress;\r
        public boolean canAdmin;\r
        public boolean excludeFromFederation;\r
        public final Set<String> repositories = new HashSet<String>();\r

diff --git a/tests/com/gitblit/tests/LdapUserServiceTest.java b/tests/com/gitblit/tests/LdapUserServiceTest.java
index 43af24f26c0fb91567d2f5a4577abf8259a7479f..c7e95c369a4b5159fc18c6562756353a520e514c 100644 (file)
--- a/tests/com/gitblit/tests/LdapUserServiceTest.java
+++ b/tests/com/gitblit/tests/LdapUserServiceTest.java
@@ -16,12 +16,16 @@
  */
 package com.gitblit.tests;
 
-import static org.junit.Assert.*;
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertNotNull;
+import static org.junit.Assert.assertNull;
+import static org.junit.Assert.assertTrue;
 
 import java.util.HashMap;
 import java.util.Map;
 
 import org.junit.Before;
+import org.junit.BeforeClass;
 import org.junit.Test;
 
 import com.gitblit.LdapUserService;
@@ -30,7 +34,6 @@ 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.LDAPConnection;
 import com.unboundid.ldif.LDIFReader;
 
 /**
@@ -44,20 +47,25 @@ public class LdapUserServiceTest {
        
        private LdapUserService ldapUserService;
        
-       @Before
-       public void createInMemoryLdapServer() throws Exception {
+       @BeforeClass
+       public static void createInMemoryLdapServer() throws Exception {
                InMemoryDirectoryServerConfig config = new InMemoryDirectoryServerConfig("dc=MyDomain");
                config.addAdditionalBindCredentials("cn=Directory Manager", "password");
                config.setListenerConfigs(InMemoryListenerConfig.createLDAPConfig("default", 389));
                config.setSchema(null);
                
                InMemoryDirectoryServer ds = new InMemoryDirectoryServer(config);
-               ds.importFromLDIF(true, new LDIFReader(this.getClass().getResourceAsStream("resources/ldapUserServiceSampleData.ldif")));
+               ds.importFromLDIF(true, new LDIFReader(LdapUserServiceTest.class.getResourceAsStream("resources/ldapUserServiceSampleData.ldif")));
                ds.startListening();
        }
        
        @Before
        public void createLdapUserService() {
+               ldapUserService = new LdapUserService();
+               ldapUserService.setup(getSettings());
+       }
+       
+       private MemorySettings getSettings() {
                Map<Object, Object> backingMap = new HashMap<Object, Object>();
                backingMap.put("realm.ldap.server", "ldap://localhost:389");
                backingMap.put("realm.ldap.domain", "");
@@ -70,11 +78,11 @@ public class LdapUserServiceTest {
                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");
                
                MemorySettings ms = new MemorySettings(backingMap);
-               
-               ldapUserService = new LdapUserService();
-               ldapUserService.setup(ms);
+               return ms;
        }
        
        @Test
@@ -101,5 +109,48 @@ public class LdapUserServiceTest {
                assertNull(userThreeModel.getTeam("git_admins"));
                assertTrue(userThreeModel.canAdmin);
        }
+       
+       @Test
+       public void testDisplayName() {
+               UserModel userOneModel = ldapUserService.authenticate("UserOne", "userOnePassword".toCharArray());
+               assertNotNull(userOneModel);
+               assertEquals("User One", userOneModel.displayName);
+               
+               // Test more complicated scenarios - concat
+               MemorySettings ms = getSettings();
+               ms.put("realm.ldap.displayName", "${personalTitle}. ${givenName} ${surname}");
+               ldapUserService = new LdapUserService();
+               ldapUserService.setup(ms);
+               
+               userOneModel = ldapUserService.authenticate("UserOne", "userOnePassword".toCharArray());
+               assertNotNull(userOneModel);
+               assertEquals("Mr. User One", userOneModel.displayName);
+       }
+       
+       @Test
+       public void testEmail() {
+               UserModel userOneModel = ldapUserService.authenticate("UserOne", "userOnePassword".toCharArray());
+               assertNotNull(userOneModel);
+               assertEquals("userone@gitblit.com", userOneModel.emailAddress);
+               
+               // Test more complicated scenarios - concat
+               MemorySettings ms = getSettings();
+               ms.put("realm.ldap.email", "${givenName}.${surname}@gitblit.com");
+               ldapUserService = new LdapUserService();
+               ldapUserService.setup(ms);
+               
+               userOneModel = ldapUserService.authenticate("UserOne", "userOnePassword".toCharArray());
+               assertNotNull(userOneModel);
+               assertEquals("User.One@gitblit.com", userOneModel.emailAddress);
+       }
+       
+       @Test
+       public void testLdapInjection() {
+               // Inject so "(&(objectClass=person)(sAMAccountName=${username}))" becomes "(&(objectClass=person)(sAMAccountName=*)(userPassword=userOnePassword))"
+               // Thus searching by password
+               
+               UserModel userOneModel = ldapUserService.authenticate("*)(userPassword=userOnePassword", "userOnePassword".toCharArray());
+               assertNull(userOneModel);
+       }
 
 }

diff --git a/tests/com/gitblit/tests/resources/ldapUserServiceSampleData.ldif b/tests/com/gitblit/tests/resources/ldapUserServiceSampleData.ldif
index 84ee243e71107d29bd206c6163e87670ac206f0c..df79333e96ec608d4645b11ed66248d8943771de 100644 (file)
--- a/tests/com/gitblit/tests/resources/ldapUserServiceSampleData.ldif
+++ b/tests/com/gitblit/tests/resources/ldapUserServiceSampleData.ldif
@@ -62,6 +62,11 @@ objectClass: user
 objectClass: person
 sAMAccountName: UserOne
 userPassword: userOnePassword
+displayName: User One
+givenName: User
+surname: One
+personalTitle: Mr
+email: userone@gitblit.com
 memberOf: CN=Git_Admins,OU=Groups,OU=UserControl,OU=MyOrganization,DC=MyDomain
 memberOf: CN=Git_Users,OU=Groups,OU=UserControl,OU=MyOrganization,DC=MyDomain
 
@@ -70,6 +75,11 @@ objectClass: user
 objectClass: person
 sAMAccountName: UserTwo
 userPassword: userTwoPassword
+displayName: User Two
+givenName: User
+surname: Two
+personalTitle: Mr
+email: usertwo@gitblit.com
 memberOf: CN=Git_Users,OU=Groups,OU=UserControl,OU=MyOrganization,DC=MyDomain
 memberOf: CN=Git Admins,OU=Groups,OU=UserControl,OU=MyOrganization,DC=MyDomain
 
@@ -78,6 +88,11 @@ objectClass: user
 objectClass: person
 sAMAccountName: UserThree
 userPassword: userThreePassword
+displayName: User Three
+givenName: User
+surname: Three
+personalTitle: Mrs
+email: userthree@gitblit.com
 memberOf: CN=Git_Users,OU=Groups,OU=UserControl,OU=MyOrganization,DC=MyDomain
 
 dn: CN=UserFour,OU=Canada,OU=Users,OU=UserControl,OU=MyOrganization,DC=MyDomain
@@ -85,4 +100,9 @@ objectClass: user
 objectClass: person
 sAMAccountName: UserFour
 userPassword: userFourPassword
+displayName: User Four
+givenName: User
+surname: Four
+personalTitle: Miss
+email: userfour@gitblit.com
 memberOf: CN=Git_Users,OU=Groups,OU=UserControl,OU=MyOrganization,DC=MyDomain
\ No newline at end of file