diff --git a/core/src/main/java/org/springframework/security/authentication/AuthenticationServiceException.java b/core/src/main/java/org/springframework/security/authentication/AuthenticationServiceException.java index 6a6de158ed..950fd4586f 100644 --- a/core/src/main/java/org/springframework/security/authentication/AuthenticationServiceException.java +++ b/core/src/main/java/org/springframework/security/authentication/AuthenticationServiceException.java @@ -23,6 +23,7 @@ import org.springframework.security.core.AuthenticationException; * This might be thrown if a backend authentication repository is unavailable, for example. * * @author Ben Alex + * @see InternalAuthenticationServiceException */ public class AuthenticationServiceException extends AuthenticationException { //~ Constructors =================================================================================================== diff --git a/core/src/main/java/org/springframework/security/authentication/InternalAuthenticationServiceException.java b/core/src/main/java/org/springframework/security/authentication/InternalAuthenticationServiceException.java new file mode 100644 index 0000000000..648f3b5291 --- /dev/null +++ b/core/src/main/java/org/springframework/security/authentication/InternalAuthenticationServiceException.java @@ -0,0 +1,40 @@ +/* + * Copyright 2002-2012 the original author or authors. + * + * Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software distributed under the License is distributed on + * an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the License for the + * specific language governing permissions and limitations under the License. + */ +package org.springframework.security.authentication; + +/** + *
+ * Thrown if an authentication request could not be processed due to a system problem that occurred internally. It + * differs from {@link AuthenticationServiceException} in that it would not be thrown if an external system has an + * internal error or failure. This ensures that we can handle errors that are within our control distinctly from errors + * of other systems. The advantage to this distinction is that the unrusted external system should not be able to fill + * up logs and cause excessive IO. However, an internal system should report errors. + *
+ *+ * This might be thrown if a backend authentication repository is unavailable, for example. However, it would not be + * thrown in the event that an error occurred when validating an OpenID response with an OpenID Provider. + *
+ * + * @author Rob Winch + * + */ +public class InternalAuthenticationServiceException extends AuthenticationServiceException { + + public InternalAuthenticationServiceException(String message, Throwable cause) { + super(message, cause); + } + + public InternalAuthenticationServiceException(String message) { + super(message); + } +} \ No newline at end of file diff --git a/ldap/src/main/java/org/springframework/security/ldap/authentication/LdapAuthenticationProvider.java b/ldap/src/main/java/org/springframework/security/ldap/authentication/LdapAuthenticationProvider.java index e205c85964..10f5976690 100644 --- a/ldap/src/main/java/org/springframework/security/ldap/authentication/LdapAuthenticationProvider.java +++ b/ldap/src/main/java/org/springframework/security/ldap/authentication/LdapAuthenticationProvider.java @@ -17,8 +17,8 @@ package org.springframework.security.ldap.authentication; import org.springframework.ldap.NamingException; import org.springframework.ldap.core.DirContextOperations; -import org.springframework.security.authentication.AuthenticationServiceException; import org.springframework.security.authentication.BadCredentialsException; +import org.springframework.security.authentication.InternalAuthenticationServiceException; import org.springframework.security.authentication.LockedException; import org.springframework.security.authentication.UsernamePasswordAuthenticationToken; import org.springframework.security.core.GrantedAuthority; @@ -188,7 +188,7 @@ public class LdapAuthenticationProvider extends AbstractLdapAuthenticationProvid throw notFound; } } catch (NamingException ldapAccessFailure) { - throw new AuthenticationServiceException(ldapAccessFailure.getMessage(), ldapAccessFailure); + throw new InternalAuthenticationServiceException(ldapAccessFailure.getMessage(), ldapAccessFailure); } } diff --git a/ldap/src/test/java/org/springframework/security/ldap/authentication/LdapAuthenticationProviderTests.java b/ldap/src/test/java/org/springframework/security/ldap/authentication/LdapAuthenticationProviderTests.java index 63aa441a23..8ec29e2940 100644 --- a/ldap/src/test/java/org/springframework/security/ldap/authentication/LdapAuthenticationProviderTests.java +++ b/ldap/src/test/java/org/springframework/security/ldap/authentication/LdapAuthenticationProviderTests.java @@ -21,10 +21,12 @@ import static org.mockito.Mockito.*; import java.util.*; import org.junit.Test; +import org.springframework.ldap.CommunicationException; import org.springframework.ldap.core.DirContextAdapter; import org.springframework.ldap.core.DirContextOperations; import org.springframework.ldap.core.DistinguishedName; import org.springframework.security.authentication.BadCredentialsException; +import org.springframework.security.authentication.InternalAuthenticationServiceException; import org.springframework.security.authentication.UsernamePasswordAuthenticationToken; import org.springframework.security.core.Authentication; import org.springframework.security.core.GrantedAuthority; @@ -39,6 +41,7 @@ import org.springframework.security.ldap.userdetails.LdapUserDetailsMapper; * Tests {@link LdapAuthenticationProvider}. * * @author Luke Taylor + * @author Rob Winch */ public class LdapAuthenticationProviderTests { @@ -147,6 +150,22 @@ public class LdapAuthenticationProviderTests { assertTrue(AuthorityUtils.authorityListToSet(user.getAuthorities()).contains("ROLE_FROM_ENTRY")); } + @Test + public void authenticateWithNamingException() { + UsernamePasswordAuthenticationToken authRequest = new UsernamePasswordAuthenticationToken("ben", "benspassword"); + LdapAuthenticator mockAuthenticator = mock(LdapAuthenticator.class); + CommunicationException expectedCause = new CommunicationException(new javax.naming.CommunicationException()); + when(mockAuthenticator.authenticate(authRequest)).thenThrow(expectedCause); + + LdapAuthenticationProvider ldapProvider = new LdapAuthenticationProvider(mockAuthenticator); + try { + ldapProvider.authenticate(authRequest); + fail("Expected Exception"); + } catch(InternalAuthenticationServiceException success) { + assertSame(expectedCause, success.getCause()); + } + } + //~ Inner Classes ================================================================================================== class MockAuthenticator implements LdapAuthenticator { diff --git a/web/src/main/java/org/springframework/security/web/authentication/AbstractAuthenticationProcessingFilter.java b/web/src/main/java/org/springframework/security/web/authentication/AbstractAuthenticationProcessingFilter.java index 1f94305519..a2db490781 100644 --- a/web/src/main/java/org/springframework/security/web/authentication/AbstractAuthenticationProcessingFilter.java +++ b/web/src/main/java/org/springframework/security/web/authentication/AbstractAuthenticationProcessingFilter.java @@ -31,6 +31,7 @@ import org.springframework.context.MessageSourceAware; import org.springframework.context.support.MessageSourceAccessor; import org.springframework.security.authentication.AuthenticationDetailsSource; import org.springframework.security.authentication.AuthenticationManager; +import org.springframework.security.authentication.InternalAuthenticationServiceException; import org.springframework.security.authentication.event.InteractiveAuthenticationSuccessEvent; import org.springframework.security.core.Authentication; import org.springframework.security.core.AuthenticationException; @@ -197,6 +198,11 @@ public abstract class AbstractAuthenticationProcessingFilter extends GenericFilt return; } sessionStrategy.onAuthentication(authResult, request, response); + } catch(InternalAuthenticationServiceException failed) { + logger.error("An internal error occurred while trying to authenticate the user.", failed); + unsuccessfulAuthentication(request, response, failed); + + return; } catch (AuthenticationException failed) { // Authentication failed diff --git a/web/src/test/java/org/springframework/security/web/authentication/AbstractAuthenticationProcessingFilterTests.java b/web/src/test/java/org/springframework/security/web/authentication/AbstractAuthenticationProcessingFilterTests.java index 693a24d1cb..21be453b4e 100644 --- a/web/src/test/java/org/springframework/security/web/authentication/AbstractAuthenticationProcessingFilterTests.java +++ b/web/src/test/java/org/springframework/security/web/authentication/AbstractAuthenticationProcessingFilterTests.java @@ -18,6 +18,7 @@ package org.springframework.security.web.authentication; import static org.junit.Assert.*; import static org.mockito.Mockito.*; +import org.apache.commons.logging.Log; import org.junit.After; import org.junit.Before; import org.junit.Test; @@ -26,6 +27,7 @@ import org.springframework.mock.web.MockHttpServletRequest; import org.springframework.mock.web.MockHttpServletResponse; import org.springframework.security.authentication.AuthenticationManager; import org.springframework.security.authentication.BadCredentialsException; +import org.springframework.security.authentication.InternalAuthenticationServiceException; import org.springframework.security.authentication.UsernamePasswordAuthenticationToken; import org.springframework.security.core.Authentication; import org.springframework.security.core.AuthenticationException; @@ -33,6 +35,7 @@ import org.springframework.security.core.authority.AuthorityUtils; import org.springframework.security.core.context.SecurityContextHolder; import org.springframework.security.web.authentication.rememberme.TokenBasedRememberMeServices; import org.springframework.security.web.authentication.session.SessionAuthenticationStrategy; +import org.springframework.test.util.ReflectionTestUtils; import javax.servlet.FilterChain; import javax.servlet.ServletException; @@ -49,6 +52,7 @@ import java.io.IOException; * * @author Ben Alex * @author Luke Taylor + * @author Rob Winch */ @SuppressWarnings("deprecation") public class AbstractAuthenticationProcessingFilterTests { @@ -352,6 +356,28 @@ public class AbstractAuthenticationProcessingFilterTests { assertEquals(HttpServletResponse.SC_UNAUTHORIZED, response.getStatus()); } + /** + * SEC-1919 + */ + @Test + public void loginErrorWithInternAuthenticationServiceExceptionLogsError() throws Exception { + MockHttpServletRequest request = createMockAuthenticationRequest(); + + MockFilterChain chain = new MockFilterChain(true); + MockHttpServletResponse response = new MockHttpServletResponse(); + + Log logger = mock(Log.class); + MockAuthenticationFilter filter = new MockAuthenticationFilter(false); + ReflectionTestUtils.setField(filter, "logger", logger); + filter.exceptionToThrow = new InternalAuthenticationServiceException("Mock requested to do so"); + successHandler.setDefaultTargetUrl("http://monkeymachine.co.uk/"); + filter.setAuthenticationSuccessHandler(successHandler); + + filter.doFilter(request, response, chain); + + verify(logger).error(anyString(), eq(filter.exceptionToThrow)); + assertEquals(HttpServletResponse.SC_UNAUTHORIZED, response.getStatus()); + } //~ Inner Classes ==================================================================================================