SEC-1631: Reduced use of reflection in DefaultAuthenticationEventPublisher and added tests.

core/src/main/java/org/springframework/security/authentication/AuthenticationServiceException.java

@@ -18,15 +18,16 @@ package org.springframework.security.authentication;
 import org.springframework.security.core.AuthenticationException;
 
 /**
- * Thrown if an authentication request could not be processed due to a system problem.<p>This might be thrown if a
+ * Thrown if an authentication request could not be processed due to a system problem.
- * backend authentication repository is  unavailable.</p>
+ * <p>
+ * This might be thrown if a backend authentication repository is  unavailable, for example.
  *
  * @author Ben Alex
  */
 public class AuthenticationServiceException extends AuthenticationException {
     //~ Constructors ===================================================================================================
 
-/**
+    /**
      * Constructs an <code>AuthenticationServiceException</code> with the
      * specified message.
      *
@@ -36,7 +37,7 @@ public class AuthenticationServiceException extends AuthenticationException {
         super(msg);
     }
 
-/**
+    /**
      * Constructs an <code>AuthenticationServiceException</code> with the
      * specified message and root cause.
      *

core/src/main/java/org/springframework/security/authentication/DefaultAuthenticationEventPublisher.java

@@ -2,13 +2,14 @@ package org.springframework.security.authentication;
 
 import java.lang.reflect.Constructor;
 import java.lang.reflect.InvocationTargetException;
-import java.util.Properties;
+import java.util.*;
 
 import org.apache.commons.logging.Log;
 import org.apache.commons.logging.LogFactory;
 import org.springframework.context.ApplicationEventPublisher;
 import org.springframework.context.ApplicationEventPublisherAware;
 import org.springframework.security.authentication.event.AbstractAuthenticationEvent;
+import org.springframework.security.authentication.event.AbstractAuthenticationFailureEvent;
 import org.springframework.security.authentication.event.AuthenticationFailureBadCredentialsEvent;
 import org.springframework.security.authentication.event.AuthenticationFailureCredentialsExpiredEvent;
 import org.springframework.security.authentication.event.AuthenticationFailureDisabledEvent;
@@ -44,7 +45,8 @@ public class DefaultAuthenticationEventPublisher implements AuthenticationEventP
     private final Log logger = LogFactory.getLog(getClass());
 
     private ApplicationEventPublisher applicationEventPublisher;
-    private final Properties exceptionMappings;
+    private final HashMap<String,Constructor<? extends AbstractAuthenticationEvent>> exceptionMappings
+            = new HashMap<String,Constructor<? extends AbstractAuthenticationEvent>>();
 
     public DefaultAuthenticationEventPublisher() {
         this(null);
@@ -52,25 +54,17 @@ public class DefaultAuthenticationEventPublisher implements AuthenticationEventP
 
     public DefaultAuthenticationEventPublisher(ApplicationEventPublisher applicationEventPublisher) {
         this.applicationEventPublisher = applicationEventPublisher;
-        exceptionMappings = new Properties();
+
-        exceptionMappings.put(AccountExpiredException.class.getName(),
+        addMapping(BadCredentialsException.class.getName(), AuthenticationFailureBadCredentialsEvent.class);
-                AuthenticationFailureExpiredEvent.class.getName());
+        addMapping(UsernameNotFoundException.class.getName(), AuthenticationFailureBadCredentialsEvent.class);
-        exceptionMappings.put(AuthenticationServiceException.class.getName(),
+        addMapping(AccountExpiredException.class.getName(), AuthenticationFailureExpiredEvent.class);
-                AuthenticationFailureServiceExceptionEvent.class.getName());
+        addMapping(ProviderNotFoundException.class.getName(), AuthenticationFailureProviderNotFoundEvent.class);
-        exceptionMappings.put(LockedException.class.getName(),
+        addMapping(DisabledException.class.getName(), AuthenticationFailureDisabledEvent.class);
-                AuthenticationFailureLockedEvent.class.getName());
+        addMapping(LockedException.class.getName(), AuthenticationFailureLockedEvent.class);
-        exceptionMappings.put(CredentialsExpiredException.class.getName(),
+        addMapping(AuthenticationServiceException.class.getName(), AuthenticationFailureServiceExceptionEvent.class);
-                AuthenticationFailureCredentialsExpiredEvent.class.getName());
+        addMapping(CredentialsExpiredException.class.getName(), AuthenticationFailureCredentialsExpiredEvent.class);
-        exceptionMappings.put(DisabledException.class.getName(),
+        addMapping("org.springframework.security.authentication.cas.ProxyUntrustedException",
-                AuthenticationFailureDisabledEvent.class.getName());
+                AuthenticationFailureProxyUntrustedEvent.class);
-        exceptionMappings.put(BadCredentialsException.class.getName(),
-                AuthenticationFailureBadCredentialsEvent.class.getName());
-        exceptionMappings.put(UsernameNotFoundException.class.getName(),
-                AuthenticationFailureBadCredentialsEvent.class.getName());
-        exceptionMappings.put(ProviderNotFoundException.class.getName(),
-                AuthenticationFailureProviderNotFoundEvent.class.getName());
-        exceptionMappings.put("org.springframework.security.authentication.cas.ProxyUntrustedException",
-                AuthenticationFailureProxyUntrustedEvent.class.getName());
     }
 
     public void publishAuthenticationSuccess(Authentication authentication) {
@@ -79,23 +73,14 @@ public class DefaultAuthenticationEventPublisher implements AuthenticationEventP
         }
     }
 
-    public void publishAuthenticationFailure(AuthenticationException exception,
+    public void publishAuthenticationFailure(AuthenticationException exception, Authentication authentication) {
-            Authentication authentication) {
+        Constructor<? extends AbstractAuthenticationEvent> constructor = exceptionMappings.get(exception.getClass().getName());
-        String className = exceptionMappings.getProperty(exception.getClass().getName());
         AbstractAuthenticationEvent event = null;
 
-        if (className != null) {
+        if (constructor != null) {
             try {
-                Class<?> clazz = getClass().getClassLoader().loadClass(className);
+                event = constructor.newInstance(authentication, exception);
-                Constructor<?> constructor = clazz.getConstructor(new Class[] {
+            } catch (IllegalAccessException ignored) {}
-                            Authentication.class, AuthenticationException.class
-                        });
-                Object obj = constructor.newInstance(authentication, exception);
-                Assert.isInstanceOf(AbstractAuthenticationEvent.class, obj, "Must be an AbstractAuthenticationEvent");
-                event = (AbstractAuthenticationEvent) obj;
-            } catch (ClassNotFoundException ignored) {}
-            catch (NoSuchMethodException ignored) {}
-            catch (IllegalAccessException ignored) {}
             catch (InstantiationException ignored) {}
             catch (InvocationTargetException ignored) {}
         }
@@ -122,8 +107,29 @@ public class DefaultAuthenticationEventPublisher implements AuthenticationEventP
      * @param additionalExceptionMappings where keys are the fully-qualified string name of the exception class and the
      * values are the fully-qualified string name of the event class to fire.
      */
+    @SuppressWarnings({"unchecked"})
     public void setAdditionalExceptionMappings(Properties additionalExceptionMappings) {
         Assert.notNull(additionalExceptionMappings, "The exceptionMappings object must not be null");
-        exceptionMappings.putAll(additionalExceptionMappings);
+        for(Object exceptionClass : additionalExceptionMappings.keySet()) {
+            String eventClass = (String) additionalExceptionMappings.get(exceptionClass);
+            try {
+                Class<?> clazz = getClass().getClassLoader().loadClass(eventClass);
+                Assert.isAssignable(AbstractAuthenticationFailureEvent.class, clazz);
+                addMapping((String) exceptionClass, (Class<? extends AbstractAuthenticationFailureEvent>) clazz);
+            } catch (ClassNotFoundException e) {
+                throw new RuntimeException("Failed to load authentication event class " + eventClass);
+            }
+        }
+    }
+
+    private void addMapping(String exceptionClass,
+                            Class<? extends AbstractAuthenticationFailureEvent> eventClass) {
+        try {
+            Constructor<? extends AbstractAuthenticationEvent> constructor =
+                    eventClass.getConstructor(Authentication.class, AuthenticationException.class);
+            exceptionMappings.put(exceptionClass, constructor);
+        } catch (NoSuchMethodException e) {
+            throw new RuntimeException("Authentication event class " + eventClass.getName() + " has no suitable constructor");
+        }
     }
 }

core/src/main/java/org/springframework/security/authentication/ProviderNotFoundException.java

@@ -37,15 +37,4 @@ public class ProviderNotFoundException extends AuthenticationException {
     public ProviderNotFoundException(String msg) {
         super(msg);
     }
-
-    /**
-     * Constructs a <code>ProviderNotFoundException</code> with the specified
-     * message and root cause.
-     *
-     * @param msg the detail message
-     * @param t root cause
-     */
-    public ProviderNotFoundException(String msg, Throwable t) {
-        super(msg, t);
-    }
 }

core/src/test/java/org/springframework/security/authentication/DefaultAuthenticationEventPublisherTests.java

@@ -0,0 +1,123 @@
+package org.springframework.security.authentication;
+
+import static org.mockito.Matchers.any;
+import static org.mockito.Mockito.*;
+
+import org.junit.*;
+import org.springframework.context.ApplicationEventPublisher;
+import org.springframework.security.authentication.event.AuthenticationFailureBadCredentialsEvent;
+import org.springframework.security.authentication.event.AuthenticationFailureCredentialsExpiredEvent;
+import org.springframework.security.authentication.event.AuthenticationFailureDisabledEvent;
+import org.springframework.security.authentication.event.AuthenticationFailureExpiredEvent;
+import org.springframework.security.authentication.event.AuthenticationFailureLockedEvent;
+import org.springframework.security.authentication.event.AuthenticationFailureProviderNotFoundEvent;
+import org.springframework.security.authentication.event.AuthenticationFailureServiceExceptionEvent;
+import org.springframework.security.authentication.event.AuthenticationSuccessEvent;
+import org.springframework.security.core.Authentication;
+import org.springframework.security.core.AuthenticationException;
+import org.springframework.security.core.userdetails.UsernameNotFoundException;
+
+import java.util.*;
+
+/**
+ * @author Luke Taylor
+ */
+public class DefaultAuthenticationEventPublisherTests {
+    DefaultAuthenticationEventPublisher publisher;
+
+    @Test
+    public void expectedDefaultMappingsAreSatisfied() throws Exception {
+        publisher = new DefaultAuthenticationEventPublisher();
+        ApplicationEventPublisher appPublisher = mock(ApplicationEventPublisher.class);
+        publisher.setApplicationEventPublisher(appPublisher);
+        Authentication a = mock(Authentication.class);
+
+        Exception cause = new Exception();
+        Object extraInfo = new Object();
+        publisher.publishAuthenticationFailure(new BadCredentialsException(""), a);
+        publisher.publishAuthenticationFailure(new BadCredentialsException("", extraInfo), a);
+        publisher.publishAuthenticationFailure(new BadCredentialsException("", cause), a);
+        verify(appPublisher, times(3)).publishEvent(isA(AuthenticationFailureBadCredentialsEvent.class));
+        reset(appPublisher);
+        publisher.publishAuthenticationFailure(new UsernameNotFoundException(""), a);
+        publisher.publishAuthenticationFailure(new UsernameNotFoundException("", extraInfo), a);
+        publisher.publishAuthenticationFailure(new UsernameNotFoundException("", cause), a);
+        publisher.publishAuthenticationFailure(new AccountExpiredException(""), a);
+        publisher.publishAuthenticationFailure(new AccountExpiredException("", extraInfo), a);
+        publisher.publishAuthenticationFailure(new AccountExpiredException("", cause), a);
+        publisher.publishAuthenticationFailure(new ProviderNotFoundException(""), a);
+        publisher.publishAuthenticationFailure(new DisabledException(""), a);
+        publisher.publishAuthenticationFailure(new DisabledException("", extraInfo), a);
+        publisher.publishAuthenticationFailure(new DisabledException("", cause), a);
+        publisher.publishAuthenticationFailure(new LockedException(""), a);
+        publisher.publishAuthenticationFailure(new LockedException("", extraInfo), a);
+        publisher.publishAuthenticationFailure(new LockedException("", cause), a);
+        publisher.publishAuthenticationFailure(new AuthenticationServiceException(""), a);
+        publisher.publishAuthenticationFailure(new AuthenticationServiceException("",cause), a);
+        publisher.publishAuthenticationFailure(new CredentialsExpiredException(""), a);
+        publisher.publishAuthenticationFailure(new CredentialsExpiredException("", extraInfo), a);
+        publisher.publishAuthenticationFailure(new CredentialsExpiredException("", cause), a);
+        verify(appPublisher, times(3)).publishEvent(isA(AuthenticationFailureBadCredentialsEvent.class));
+        verify(appPublisher, times(3)).publishEvent(isA(AuthenticationFailureExpiredEvent.class));
+        verify(appPublisher).publishEvent(isA(AuthenticationFailureProviderNotFoundEvent.class));
+        verify(appPublisher, times(3)).publishEvent(isA(AuthenticationFailureDisabledEvent.class));
+        verify(appPublisher, times(3)).publishEvent(isA(AuthenticationFailureLockedEvent.class));
+        verify(appPublisher, times(2)).publishEvent(isA(AuthenticationFailureServiceExceptionEvent.class));
+        verify(appPublisher, times(3)).publishEvent(isA(AuthenticationFailureCredentialsExpiredEvent.class));
+        verifyNoMoreInteractions(appPublisher);
+    }
+
+    @Test
+    public void authenticationSuccessIsPublished() {
+        publisher = new DefaultAuthenticationEventPublisher();
+        ApplicationEventPublisher appPublisher = mock(ApplicationEventPublisher.class);
+        publisher.setApplicationEventPublisher(appPublisher);
+        publisher.publishAuthenticationSuccess(mock(Authentication.class));
+        verify(appPublisher).publishEvent(isA(AuthenticationSuccessEvent.class));
+
+        publisher.setApplicationEventPublisher(null);
+        // Should be ignored with null app publisher
+        publisher.publishAuthenticationSuccess(mock(Authentication.class));
+    }
+
+    @Test
+    public void additionalExceptionMappingsAreSupported() {
+        publisher = new DefaultAuthenticationEventPublisher();
+        Properties p = new Properties();
+        p.put(MockAuthenticationException.class.getName(), AuthenticationFailureDisabledEvent.class.getName());
+        publisher.setAdditionalExceptionMappings(p);
+        ApplicationEventPublisher appPublisher = mock(ApplicationEventPublisher.class);
+
+        publisher.setApplicationEventPublisher(appPublisher);
+        publisher.publishAuthenticationFailure(new MockAuthenticationException("test"), mock(Authentication.class));
+        verify(appPublisher).publishEvent(isA(AuthenticationFailureDisabledEvent.class));
+    }
+
+    @Test(expected=RuntimeException.class)
+    public void missingEventClassExceptionCausesException() {
+        publisher = new DefaultAuthenticationEventPublisher();
+        Properties p = new Properties();
+        p.put(MockAuthenticationException.class.getName(), "NoSuchClass");
+        publisher.setAdditionalExceptionMappings(p);
+    }
+
+    @Test
+    public void unknownFailureExceptionIsIgnored() throws Exception {
+        publisher = new DefaultAuthenticationEventPublisher();
+        Properties p = new Properties();
+        p.put(MockAuthenticationException.class.getName(), AuthenticationFailureDisabledEvent.class.getName());
+        publisher.setAdditionalExceptionMappings(p);
+        ApplicationEventPublisher appPublisher = mock(ApplicationEventPublisher.class);
+
+        publisher.setApplicationEventPublisher(appPublisher);
+        publisher.publishAuthenticationFailure(new AuthenticationException("") {}, mock(Authentication.class));
+        verifyZeroInteractions(appPublisher);
+    }
+
+    private static final class MockAuthenticationException extends AuthenticationException {
+        public MockAuthenticationException(String msg) {
+            super(msg);
+        }
+    }
+
+}

core/src/test/java/org/springframework/security/authentication/ProviderManagerTests.java

@@ -296,13 +296,9 @@ public class ProviderManagerTests {
             }
         }
 
-        public boolean supports(Class<? extends Object> authentication) {
+        public boolean supports(Class<?> authentication) {
-            if (TestingAuthenticationToken.class.isAssignableFrom(authentication) ||
+            return TestingAuthenticationToken.class.isAssignableFrom(authentication) ||
-                    UsernamePasswordAuthenticationToken.class.isAssignableFrom(authentication)) {
+                    UsernamePasswordAuthenticationToken.class.isAssignableFrom(authentication);
-                return true;
-            } else {
-                return false;
-            }
         }
     }
 }