From 8a3930e673b14ab61a64dd7cdf019c853178d24f Mon Sep 17 00:00:00 2001
From: Luke Taylor <luke.taylor@springsource.com>
Date: Wed, 8 Jul 2009 23:20:46 +0000
Subject: [PATCH] Refactoring of ProviderManager to ensure that any
 AuthenticationException from the ConcurrentSessionController will prevent
 further polling of providers.

---
 .../authentication/ProviderManager.java       | 35 +++++++++++--------
 .../authentication/ProviderManagerTests.java  | 15 ++++----
 2 files changed, 29 insertions(+), 21 deletions(-)

diff --git a/core/src/main/java/org/springframework/security/authentication/ProviderManager.java b/core/src/main/java/org/springframework/security/authentication/ProviderManager.java
index 4a607a14ca..19d7f72338 100644
--- a/core/src/main/java/org/springframework/security/authentication/ProviderManager.java
+++ b/core/src/main/java/org/springframework/security/authentication/ProviderManager.java
@@ -170,27 +170,34 @@ public class ProviderManager extends AbstractAuthenticationManager implements In
             try {
                 result = provider.authenticate(authentication);
 
-                if (result != null) {
-                    copyDetails(authentication, result);
-                    sessionController.checkAuthenticationAllowed(result);
+                if (result == null) {
+                    continue;
                 }
-            } catch (AuthenticationException ae) {
-                lastException = ae;
-                result = null;
+            } catch (AccountStatusException e) {
+                // SEC-546: Avoid polling additional providers if auth failure is due to invalid account status
+                lastException = e;
+                break;
+            } catch (AuthenticationException e) {
+                lastException = e;
+                continue;
             }
 
-            // SEC-546: Avoid polling additional providers if auth failure is due to invalid account status or
-            // disallowed concurrent login.
-            if (lastException instanceof AccountStatusException || lastException instanceof ConcurrentLoginException) {
+            assert result != null;
+
+            copyDetails(authentication, result);
+
+            try {
+                sessionController.checkAuthenticationAllowed(result);
+            } catch (AuthenticationException e) {
+             // SEC-546: Avoid polling additional providers if concurrent login check fails
+                lastException = e;
                 break;
             }
 
-            if (result != null) {
-                sessionController.registerSuccessfulAuthentication(result);
-                publishEvent(new AuthenticationSuccessEvent(result));
+            sessionController.registerSuccessfulAuthentication(result);
+            publishEvent(new AuthenticationSuccessEvent(result));
 
-                return result;
-            }
+            return result;
         }
 
         if (lastException == null) {
diff --git a/core/src/test/java/org/springframework/security/authentication/ProviderManagerTests.java b/core/src/test/java/org/springframework/security/authentication/ProviderManagerTests.java
index e53108a613..3ded575ecb 100644
--- a/core/src/test/java/org/springframework/security/authentication/ProviderManagerTests.java
+++ b/core/src/test/java/org/springframework/security/authentication/ProviderManagerTests.java
@@ -16,7 +16,7 @@
 package org.springframework.security.authentication;
 
 import static org.junit.Assert.*;
-import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.*;
 
 import java.util.ArrayList;
 import java.util.Arrays;
@@ -45,8 +45,6 @@ import org.springframework.security.core.authority.AuthorityUtils;
  */
 public class ProviderManagerTests {
 
-    //~ Methods ========================================================================================================
-
     @Test(expected=ProviderNotFoundException.class)
     public void authenticationFailsWithUnsupportedToken() throws Exception {
         UsernamePasswordAuthenticationToken token = new UsernamePasswordAuthenticationToken("Test", "Password",
@@ -185,11 +183,14 @@ public class ProviderManagerTests {
     @Test(expected=ConcurrentLoginException.class)
     public void concurrentLoginExceptionPreventsCallsToSubsequentProviders() throws Exception {
         ProviderManager authMgr = makeProviderManager();
+        // Two providers so if the second is polled it will throw an AccountStatusException
+        authMgr.setProviders(Arrays.asList(new MockProvider(), new MockProviderWhichThrowsAccountStatusException()) );
+        TestingAuthenticationToken request = createAuthenticationToken();
+        ConcurrentSessionController ctrlr = mock(ConcurrentSessionController.class);
+        doThrow(new ConcurrentLoginException("mocked")).when(ctrlr).checkAuthenticationAllowed(request);
+        authMgr.setSessionController(ctrlr);
 
-        authMgr.setProviders(Arrays.asList(new MockProviderWhichThrowsConcurrentLoginException(),
-                new MockProviderWhichThrowsAccountStatusException()) );
-
-        authMgr.authenticate(createAuthenticationToken());
+        authMgr.authenticate(request);
     }
 
     private TestingAuthenticationToken createAuthenticationToken() {