From ffadc2e1878ce448127055e98299218574b103cf Mon Sep 17 00:00:00 2001 From: Florian Zschocke Date: Wed, 9 Nov 2016 22:04:00 +0100 Subject: [PATCH] Extend LDAP tests to use LDAP servers with access restrictions. Add access restrictions to the LDAP test server instances. New modes used a test parameters are ANONYMOUS, DS_MANAGER and USR_MANAGER. ANONYMOUS can bind anonymously and access users and groups. In DS_MANAGER the server requires authentication and will only allow the DIRECTORY_MANAGER user to search for users and groups. In USR_MANAGER only the user can search groups, the USER_MANAGER, which is used to bind in this mode, can not. A third server instance is created because I did fear side effects should the tests be run in parallel, had I tried to configure the access restriction in Before. --- .../gitblit/tests/LdapAuthenticationTest.java | 302 +++++++++++++++--- 1 file changed, 257 insertions(+), 45 deletions(-) diff --git a/src/test/java/com/gitblit/tests/LdapAuthenticationTest.java b/src/test/java/com/gitblit/tests/LdapAuthenticationTest.java index cea8a4b1..2ade6819 100644 --- a/src/test/java/com/gitblit/tests/LdapAuthenticationTest.java +++ b/src/test/java/com/gitblit/tests/LdapAuthenticationTest.java @@ -16,6 +16,8 @@ */ package com.gitblit.tests; +import static org.junit.Assume.*; + import java.io.File; import java.util.Arrays; import java.util.Collection; @@ -24,6 +26,7 @@ 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; @@ -51,9 +54,22 @@ 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; /** @@ -66,28 +82,68 @@ import com.unboundid.ldif.LDIFReader; @RunWith(Parameterized.class) public class LdapAuthenticationTest extends GitblitUnitTest { - public enum ServerMode { ANONYMOUS, AUTHENTICATED }; + 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); + } + }; - @Rule - public TemporaryFolder folder = new TemporaryFolder(); - private static final String RESOURCE_DIR = "src/test/resources/ldap/"; @Parameter - public ServerMode serverMode; + public AuthMode authMode; - private File usersConf; + @Rule + public TemporaryFolder folder = new TemporaryFolder(); - private LdapAuthProvider ldap; + private File usersConf; - private static int ldapPort = 1389; - private static int ldapAuthedPort = 2389; - private static InMemoryDirectoryServer ds; - private static InMemoryDirectoryServerSnapshot dsAnonSnapshot; - private static InMemoryDirectoryServer dsAuthed; - private static InMemoryDirectoryServerSnapshot dsAuthedSnapshot; + private LdapAuthProvider ldap; private IUserManager userManager; @@ -96,30 +152,57 @@ public class LdapAuthenticationTest extends GitblitUnitTest { private MemorySettings settings; - + /** + * Run the tests with each authentication scenario once. + */ @Parameters(name = "{0}") public static Collection data() { - return Arrays.asList(new Object[][] { {ServerMode.ANONYMOUS}, {ServerMode.AUTHENTICATED} }); + 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 init() throws Exception { - InMemoryDirectoryServerConfig config = createInMemoryLdapServerConfig(); - config.setListenerConfigs(InMemoryListenerConfig.createLDAPConfig("default", ldapPort)); + 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); - dsAnonSnapshot = ds.createSnapshot(); + AuthMode.DS_MANAGER.setDS(ds); - config = createInMemoryLdapServerConfig(); - config.setListenerConfigs(InMemoryListenerConfig.createLDAPConfig("default", ldapAuthedPort)); + config = createInMemoryLdapServerConfig(AuthMode.USR_MANAGER); + config.setListenerConfigs(InMemoryListenerConfig.createLDAPConfig("default", AuthMode.USR_MANAGER.ldapPort())); config.setAuthenticationRequiredOperationTypes(EnumSet.allOf(OperationType.class)); - dsAuthed = createInMemoryLdapServer(config); - dsAuthedSnapshot = ds.createSnapshot(); + 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"); @@ -127,10 +210,14 @@ public class LdapAuthenticationTest extends GitblitUnitTest { return imds; } - public static InMemoryDirectoryServerConfig createInMemoryLdapServerConfig() throws Exception { + public static InMemoryDirectoryServerConfig createInMemoryLdapServerConfig(AuthMode authMode) throws Exception { InMemoryDirectoryServerConfig config = new InMemoryDirectoryServerConfig("dc=MyDomain"); - config.addAdditionalBindCredentials("cn=Directory Manager", "password"); + config.addAdditionalBindCredentials(DIRECTORY_MANAGER, "password"); + config.addAdditionalBindCredentials(USER_MANAGER, "passwd"); config.setSchema(null); + + config.addInMemoryOperationInterceptor(new AccessInterceptor(authMode)); + return config; } @@ -138,10 +225,7 @@ public class LdapAuthenticationTest extends GitblitUnitTest { @Before public void setup() throws Exception { - ds.restoreSnapshot(dsAnonSnapshot); - dsAuthed.restoreSnapshot(dsAuthedSnapshot); - - System.out.println("Before with server mode " + serverMode); + authMode.restoreSnapshot(); usersConf = folder.newFile("users.conf"); FileUtils.copyFile(new File(RESOURCE_DIR + "users.conf"), usersConf); @@ -170,19 +254,30 @@ public class LdapAuthenticationTest extends GitblitUnitTest { private MemorySettings getSettings() { Map backingMap = new HashMap(); backingMap.put(Keys.realm.userService, usersConf.getAbsolutePath()); - if (ServerMode.ANONYMOUS == serverMode) { - backingMap.put(Keys.realm.ldap.server, "ldap://localhost:" + ldapPort); + 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, ""); - } else { - backingMap.put(Keys.realm.ldap.server, "ldap://localhost:" + ldapAuthedPort); - backingMap.put(Keys.realm.ldap.username, "cn=Directory Manager"); + 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"); @@ -270,7 +365,7 @@ 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()); } @@ -298,6 +393,9 @@ public class LdapAuthenticationTest extends GitblitUnitTest { @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"); getDS().addEntries(LDIFReader.readEntries(RESOURCE_DIR + "addgroup.ldif")); ldap.sync(); @@ -338,7 +436,10 @@ public class LdapAuthenticationTest extends GitblitUnitTest { @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, ""); @@ -350,16 +451,11 @@ public class LdapAuthenticationTest extends GitblitUnitTest { } - - private InMemoryDirectoryServer getDS() { - if (ServerMode.ANONYMOUS == serverMode) { - return ds; - } else { - return dsAuthed; - } + private InMemoryDirectoryServer getDS() + { + return authMode.getDS(); } - private int countLdapUsersInUserManager() { int ldapAccountCount = 0; for (UserModel userModel : userManager.getAllUsers()) { @@ -380,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 lastSuccessfulBindDN = new HashMap<>(); + Map 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; + } + } + } -- 2.39.5