diff --git a/scm-webapp/src/main/java/sonia/scm/web/security/ChainAuthenticatonManager.java b/scm-webapp/src/main/java/sonia/scm/web/security/ChainAuthenticatonManager.java index 6ce950f52c..e7ef7c43af 100644 --- a/scm-webapp/src/main/java/sonia/scm/web/security/ChainAuthenticatonManager.java +++ b/scm-webapp/src/main/java/sonia/scm/web/security/ChainAuthenticatonManager.java @@ -35,6 +35,9 @@ package sonia.scm.web.security; //~--- non-JDK imports -------------------------------------------------------- +import com.google.common.annotations.VisibleForTesting; +import com.google.common.base.Strings; +import com.google.common.collect.Lists; import com.google.inject.Inject; import com.google.inject.Singleton; @@ -46,6 +49,7 @@ import sonia.scm.cache.Cache; import sonia.scm.cache.CacheManager; import sonia.scm.security.EncryptionHandler; import sonia.scm.user.User; +import sonia.scm.user.UserManager; import sonia.scm.util.AssertUtil; import sonia.scm.util.IOUtil; import sonia.scm.util.Util; @@ -55,6 +59,7 @@ import sonia.scm.util.Util; import java.io.IOException; import java.io.Serializable; +import java.util.List; import java.util.Set; import javax.servlet.http.HttpServletRequest; @@ -81,6 +86,8 @@ public class ChainAuthenticatonManager extends AbstractAuthenticationManager * Constructs ... * * + * + * @param userManager * @param authenticationHandlerSet * @param encryptionHandler * @param cacheManager @@ -88,14 +95,14 @@ public class ChainAuthenticatonManager extends AbstractAuthenticationManager * @param authenticationListeners */ @Inject - public ChainAuthenticatonManager( + public ChainAuthenticatonManager(UserManager userManager, Set authenticationHandlerSet, EncryptionHandler encryptionHandler, CacheManager cacheManager, Set authenticationListeners) { AssertUtil.assertIsNotEmpty(authenticationHandlerSet); AssertUtil.assertIsNotNull(cacheManager); - this.authenticationHandlerSet = authenticationHandlerSet; + this.authenticationHandlers = sort(userManager, authenticationHandlerSet); this.encryptionHandler = encryptionHandler; this.cache = cacheManager.getCache(String.class, AuthenticationCacheValue.class, CACHE_NAME); @@ -162,7 +169,7 @@ public class ChainAuthenticatonManager extends AbstractAuthenticationManager @Override public void close() throws IOException { - for (AuthenticationHandler authenticator : authenticationHandlerSet) + for (AuthenticationHandler authenticator : authenticationHandlers) { if (logger.isTraceEnabled()) { @@ -182,7 +189,7 @@ public class ChainAuthenticatonManager extends AbstractAuthenticationManager @Override public void init(SCMContextProvider context) { - for (AuthenticationHandler authenticator : authenticationHandlerSet) + for (AuthenticationHandler authenticator : authenticationHandlers) { if (logger.isTraceEnabled()) { @@ -214,7 +221,7 @@ public class ChainAuthenticatonManager extends AbstractAuthenticationManager logger.trace("start authentication chain for user {}", username); } - for (AuthenticationHandler authenticator : authenticationHandlerSet) + for (AuthenticationHandler authenticator : authenticationHandlers) { if (logger.isTraceEnabled()) { @@ -262,6 +269,39 @@ public class ChainAuthenticatonManager extends AbstractAuthenticationManager return ar; } + /** + * Method description + * + * + * @param userManager + * @param authenticationHandlerSet + * + * @return + */ + @VisibleForTesting + private List sort(UserManager userManager, + Set authenticationHandlerSet) + { + List handlers = + Lists.newArrayListWithCapacity(authenticationHandlerSet.size()); + + String first = Strings.nullToEmpty(userManager.getDefaultType()); + + for (AuthenticationHandler handler : authenticationHandlerSet) + { + if (first.equals(handler.getType())) + { + handlers.add(0, handler); + } + else + { + handlers.add(handler); + } + } + + return handlers; + } + //~--- get methods ---------------------------------------------------------- /** @@ -338,7 +378,7 @@ public class ChainAuthenticatonManager extends AbstractAuthenticationManager //~--- fields --------------------------------------------------------------- /** Field description */ - private Set authenticationHandlerSet; + private List authenticationHandlers; /** Field description */ private Cache cache; diff --git a/scm-webapp/src/test/java/sonia/scm/web/security/ChainAuthenticationManagerTest.java b/scm-webapp/src/test/java/sonia/scm/web/security/ChainAuthenticationManagerTest.java index e0badfa1cc..d35068db82 100644 --- a/scm-webapp/src/test/java/sonia/scm/web/security/ChainAuthenticationManagerTest.java +++ b/scm-webapp/src/test/java/sonia/scm/web/security/ChainAuthenticationManagerTest.java @@ -35,27 +35,28 @@ package sonia.scm.web.security; //~--- non-JDK imports -------------------------------------------------------- +import com.google.common.collect.ImmutableSet; import org.junit.Test; import sonia.scm.AbstractTestBase; import sonia.scm.SCMContextProvider; -import sonia.scm.cache.CacheManager; -import sonia.scm.cache.EhCacheManager; +import sonia.scm.cache.MapCacheManager; import sonia.scm.security.MessageDigestEncryptionHandler; import sonia.scm.user.User; +import sonia.scm.user.UserManager; import sonia.scm.user.UserTestData; import sonia.scm.util.MockUtil; import static org.junit.Assert.*; +import static org.mockito.Mockito.*; //~--- JDK imports ------------------------------------------------------------ import java.io.IOException; import java.util.Collections; -import java.util.HashSet; import java.util.Set; import javax.servlet.http.HttpServletRequest; @@ -75,6 +76,8 @@ public class ChainAuthenticationManagerTest extends AbstractTestBase @Test public void testAuthenticateFailed() { + manager = createManager(); + AuthenticationResult result = manager.authenticate(request, response, trillian.getName(), "trillian"); @@ -88,6 +91,8 @@ public class ChainAuthenticationManagerTest extends AbstractTestBase @Test public void testAuthenticateNotFound() { + manager = createManager(); + AuthenticationResult result = manager.authenticate(request, response, "dent", "trillian"); @@ -101,6 +106,8 @@ public class ChainAuthenticationManagerTest extends AbstractTestBase @Test public void testAuthenticateSuccess() { + manager = createManager(); + AuthenticationResult result = manager.authenticate(request, response, trillian.getName(), "trillian123"); @@ -114,6 +121,32 @@ public class ChainAuthenticationManagerTest extends AbstractTestBase assertEquals("perfectsType", result.getUser().getType()); } + /** + * Method description + * + */ + @Test + public void testAuthenticationOrder() + { + User trillian = UserTestData.createTrillian(); + + trillian.setPassword("trillian123"); + + SingleUserAuthenticaionHandler a1 = + new SingleUserAuthenticaionHandler("a1", trillian); + SingleUserAuthenticaionHandler a2 = + new SingleUserAuthenticaionHandler("a2", trillian); + + manager = createManager("a2", a1, a2); + + AuthenticationResult result = manager.authenticate(request, response, + trillian.getName(), "trillian123"); + + assertNotNull(result); + assertEquals(AuthenticationState.SUCCESS, result.getState()); + assertEquals("a2", result.getUser().getType()); + } + /** * Method description * @@ -123,20 +156,6 @@ public class ChainAuthenticationManagerTest extends AbstractTestBase @Override protected void postSetUp() throws Exception { - Set handlerSet = - new HashSet(); - - perfect = UserTestData.createPerfect(); - perfect.setPassword("perfect123"); - handlerSet.add(new SingleUserAuthenticaionHandler("perfectsType", perfect)); - trillian = UserTestData.createTrillian(); - trillian.setPassword("trillian123"); - handlerSet.add(new SingleUserAuthenticaionHandler("trilliansType", - trillian)); - manager = new ChainAuthenticatonManager(handlerSet, - new MessageDigestEncryptionHandler(), cacheManager, - Collections.EMPTY_SET); - manager.init(contextProvider); request = MockUtil.getHttpServletRequest(); response = MockUtil.getHttpServletResponse(); } @@ -167,6 +186,49 @@ public class ChainAuthenticationManagerTest extends AbstractTestBase assertEquals(user.getMail(), other.getMail()); } + /** + * Method description + * + * + * @return + */ + private ChainAuthenticatonManager createManager() + { + perfect = UserTestData.createPerfect(); + perfect.setPassword("perfect123"); + trillian = UserTestData.createTrillian(); + trillian.setPassword("trillian123"); + + return createManager("", + new SingleUserAuthenticaionHandler("perfectsType", perfect), + new SingleUserAuthenticaionHandler("trilliansType", trillian)); + } + + /** + * Method description + * + * + * @param defaultType + * @param handlers + * + * @return + */ + private ChainAuthenticatonManager createManager(String defaultType, + AuthenticationHandler... handlers) + { + Set handlerSet = ImmutableSet.copyOf(handlers); + + UserManager userManager = mock(UserManager.class); + + when(userManager.getDefaultType()).thenReturn(defaultType); + manager = new ChainAuthenticatonManager(userManager, handlerSet, + new MessageDigestEncryptionHandler(), new MapCacheManager(), + Collections.EMPTY_SET); + manager.init(contextProvider); + + return manager; + } + //~--- inner classes -------------------------------------------------------- /** @@ -216,7 +278,7 @@ public class ChainAuthenticationManagerTest extends AbstractTestBase { if (password.equals(user.getPassword())) { - result = new AuthenticationResult(user); + result = new AuthenticationResult(user.clone()); } else { @@ -275,10 +337,6 @@ public class ChainAuthenticationManagerTest extends AbstractTestBase //~--- fields --------------------------------------------------------------- - /** Field description */ - private CacheManager cacheManager = - new EhCacheManager(net.sf.ehcache.CacheManager.create()); - /** Field description */ private ChainAuthenticatonManager manager;