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

John Crygier

2012-04-25 7e0ce40e79a193426e8f7b6f22407422634dafe3

Add logic to get display name & email (Simply stored in user model for now).
Add code / test to defend against LDAP injection attacks.

 distrib/gitblit.properties

@@ -221,6 +221,28 @@
# SINCE 1.0.0

realm.ldap.admins= @Git_Admins



# Attribute(s) on the USER record that indicate their display (or full) name. Leave blank

# for no mapping available in LDAP

#

# This may be a single attribute, or a string of multiple attributes.  Examples:

#  displayName - Uses the attribute 'displayName' on the user record

#  ${personalTitle}. ${givenName} ${surname} - Will concatenate the 3 
#       attributes together, with a '.' after personalTitle 
#

# SINCE 1.0.0

realm.ldap.displayName= displayName



# Attribute(s) on the USER record that indicate their email address.  Leave blank

# for no mapping available in LDAP

#

# This may be a single attribute, or a string of multiple attributes.  Examples:

#  email - Uses the attribute 'email' on the user record

#  ${givenName}.${surname}@gitblit.com -Will concatenate the 2 attributes

#       together with a '.' and '@' creating something like first.last@gitblit.com 
#

# SINCE 1.0.0

realm.ldap.email = email



#

# Gitblit Web Settings

#


 src/com/gitblit/LdapUserService.java

@@ -137,7 +137,7 @@
            // 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}))");

            accountPattern = StringUtils.replace(accountPattern, "${username}", simpleUsername);

            accountPattern = StringUtils.replace(accountPattern, "${username}", escapeLDAPSearchFilter(simpleUsername));



            SearchResult result = doSearch(ldapConnection, accountBase, accountPattern);

            if (result != null && result.getEntryCount() == 1) {

@@ -149,15 +149,15 @@
                    

                    UserModel user = getUserModel(simpleUsername);

                    if (user == null)    // create user object for new authenticated user

                        user = createUserFromLdap(simpleUsername, loggingInUser);

                        user = new UserModel(simpleUsername);

                    

                    user.password = "StoredInLDAP";

					
                    

                    if (!supportsTeamMembershipChanges())

                        getTeamsFromLdap(ldapConnection, simpleUsername, loggingInUser, user);

                    

                    // Get Admin Attributes

                    setAdminAttribute(user);

                    // Get User Attributes

                    setUserAttributes(user, loggingInUser);



                    // Push the ldap looked up values to backing file

                    super.updateUserModel(user);

@@ -186,6 +186,37 @@
                    user.canAdmin = true;

        }

    }

	
    private void setUserAttributes(UserModel user, SearchResultEntry userEntry) {

        // Is this user an admin?

        setAdminAttribute(user);

		
        // Don't want visibility into the real password, make up a dummy

        user.password = "StoredInLDAP";

		
        // Get Attributes for full name / email

        String displayName = settings.getString(Keys.realm.ldap_displayName, "displayName");

        String email = settings.getString(Keys.realm.ldap_email, "email");



        // Replace embedded ${} with attributes

        if (displayName.contains("${")) {

            for (Attribute userAttribute : userEntry.getAttributes())

                displayName = StringUtils.replace(displayName, "${" + userAttribute.getName() + "}", userAttribute.getValue());

			
            user.displayName = displayName;

        } else {

            user.displayName = userEntry.getAttribute(displayName).getValue();

        }

		
        if (email.contains("${")) {

            for (Attribute userAttribute : userEntry.getAttributes())

                email = StringUtils.replace(email, "${" + userAttribute.getName() + "}", userAttribute.getValue());

			
            user.emailAddress = email;

        } else {

            user.emailAddress = userEntry.getAttribute(email).getValue();

        }

    }



    private void getTeamsFromLdap(LDAPConnection ldapConnection, String simpleUsername, SearchResultEntry loggingInUser, UserModel user) {

        String loggingInUserDN = loggingInUser.getDN();

@@ -194,12 +225,12 @@
        String groupBase = settings.getString(Keys.realm.ldap_groupBase, "");

        String groupMemberPattern = settings.getString(Keys.realm.ldap_groupMemberPattern, "(&(objectClass=group)(member=${dn}))");

        

        groupMemberPattern = StringUtils.replace(groupMemberPattern, "${dn}", loggingInUserDN);

        groupMemberPattern = StringUtils.replace(groupMemberPattern, "${username}", simpleUsername);

        groupMemberPattern = StringUtils.replace(groupMemberPattern, "${dn}", escapeLDAPSearchFilter(loggingInUserDN));

        groupMemberPattern = StringUtils.replace(groupMemberPattern, "${username}", escapeLDAPSearchFilter(simpleUsername));

        

        // Fill in attributes into groupMemberPattern

        for (Attribute userAttribute : loggingInUser.getAttributes())

            groupMemberPattern = StringUtils.replace(groupMemberPattern, "${" + userAttribute.getName() + "}", userAttribute.getValue());

            groupMemberPattern = StringUtils.replace(groupMemberPattern, "${" + userAttribute.getName() + "}", escapeLDAPSearchFilter(userAttribute.getValue()));

        

        SearchResult teamMembershipResult = doSearch(ldapConnection, groupBase, groupMemberPattern);

        if (teamMembershipResult != null && teamMembershipResult.getEntryCount() > 0) {

@@ -223,13 +254,6 @@
        

        return answer;        

    }

	
    private UserModel createUserFromLdap(String simpleUserName, SearchResultEntry userEntry) {

        UserModel answer = new UserModel(simpleUserName);

        //If attributes other than user name ever from from LDAP, this is where to get them

		
        return answer;

    }



    private SearchResult doSearch(LDAPConnection ldapConnection, String base, String filter) {

        try {

@@ -243,6 +267,7 @@
    

    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) {

@@ -263,6 +288,35 @@
        if (lastSlash > -1) {

            username = username.substring(lastSlash + 1);

        }

		
        return username;

    }

	
    // From: https://www.owasp.org/index.php/Preventing_LDAP_Injection_in_Java

    public static final String escapeLDAPSearchFilter(String filter) {

        StringBuilder sb = new StringBuilder();

        for (int i = 0; i < filter.length(); i++) {

            char curChar = filter.charAt(i);

            switch (curChar) {

            case '\\':

                sb.append("\\5c");

                break;

            case '*':

                sb.append("\\2a");

                break;

            case '(':

                sb.append("\\28");

                break;

            case ')':

                sb.append("\\29");

                break;

            case '\u0000': 
                sb.append("\\00"); 
                break;

            default:

                sb.append(curChar);

            }

        }

        return sb.toString();

    }

}


 src/com/gitblit/models/UserModel.java

@@ -37,6 +37,8 @@
    // field names are reflectively mapped in EditUser page

    public String username;

    public String password;

    public String displayName;

    public String emailAddress;

    public boolean canAdmin;

    public boolean excludeFromFederation;

    public final Set<String> repositories = new HashSet<String>();


 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.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 @@
    
    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 @@
        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 @@
        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);
    }

}

 tests/com/gitblit/tests/resources/ldapUserServiceSampleData.ldif

@@ -62,6 +62,11 @@
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: 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: 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: 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