default authentication handler should always be the first in the chain

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<AuthenticationHandler> authenticationHandlerSet,
     EncryptionHandler encryptionHandler, CacheManager cacheManager,
     Set<AuthenticationListener> 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<AuthenticationHandler> sort(UserManager userManager,
+    Set<AuthenticationHandler> authenticationHandlerSet)
+  {
+    List<AuthenticationHandler> 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<AuthenticationHandler> authenticationHandlerSet;
+  private List<AuthenticationHandler> authenticationHandlers;
 
   /** Field description */
   private Cache<String, AuthenticationCacheValue> cache;

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.MapCacheManager;
-import sonia.scm.cache.EhCacheManager;
 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<AuthenticationHandler> handlerSet =
-      new HashSet<AuthenticationHandler>();
-
-    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<AuthenticationHandler> 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;