From 10673780dbdeb1e192b93ca02fceead4d58dec7c Mon Sep 17 00:00:00 2001 From: Luke Taylor Date: Mon, 13 Apr 2009 14:56:49 +0000 Subject: [PATCH] OPEN - issue SEC-1136: Removed SpringSecurityException. Introduced new AclException as base class for Acl module. Refactored JAAS authentication to map to AuthenticationExcpetions rather than SpringSecurityException. Modified ExceptionTranslationFilter to look explicitly for AuthenticationException or AccessDeniedException (which it should do since these are the only two it handles). --- .../security/acls/AclException.java | 34 +++++++++++++++++++ .../security/acls/AlreadyExistsException.java | 5 +-- .../security/acls/ChildrenExistException.java | 5 +-- .../acls/IdentityUnavailableException.java | 5 +-- .../security/acls/NotFoundException.java | 5 +-- .../security/acls/UnloadedSidException.java | 5 +-- .../SecurityConfigurationException.java | 5 ++- .../access/AccessDeniedException.java | 4 +-- .../BadCredentialsException.java | 3 -- .../jaas/DefaultLoginExceptionResolver.java | 4 +-- .../jaas/JaasAuthenticationProvider.java | 4 +-- .../jaas/LoginExceptionResolver.java | 10 +++--- .../rcp/RemoteAuthenticationException.java | 15 ++++---- .../core/AuthenticationException.java | 4 ++- .../UsernameNotFoundException.java | 4 +-- .../security/util/EncryptionUtils.java | 4 +-- .../jaas/JaasAuthenticationProviderTests.java | 3 +- .../LdapAuthenticationProviderTests.java | 12 ++----- .../web/ExceptionTranslationFilter.java | 16 +++++---- 19 files changed, 80 insertions(+), 67 deletions(-) create mode 100644 acl/src/main/java/org/springframework/security/acls/AclException.java diff --git a/acl/src/main/java/org/springframework/security/acls/AclException.java b/acl/src/main/java/org/springframework/security/acls/AclException.java new file mode 100644 index 0000000000..6b2c1dcd29 --- /dev/null +++ b/acl/src/main/java/org/springframework/security/acls/AclException.java @@ -0,0 +1,34 @@ +package org.springframework.security.acls; + +import org.springframework.core.NestedRuntimeException; + +/** + * Abstract superclass for all exceptions thrown in the acls package and subpackages. + * + * @author Luke Taylor + * @version $Id$ + * @since 2.5 + */ +public abstract class AclException extends NestedRuntimeException { + + /** + * Constructs an AclException with the specified + * message and root cause. + * + * @param msg the detail message + * @param t the root cause + */ + public AclException(String msg, Throwable cause) { + super(msg, cause); + } + + /** + * Constructs an AclException with the specified + * message and no root cause. + * + * @param msg the detail message + */ + public AclException(String msg) { + super(msg); + } +} diff --git a/acl/src/main/java/org/springframework/security/acls/AlreadyExistsException.java b/acl/src/main/java/org/springframework/security/acls/AlreadyExistsException.java index ef3423025a..16a54afa65 100644 --- a/acl/src/main/java/org/springframework/security/acls/AlreadyExistsException.java +++ b/acl/src/main/java/org/springframework/security/acls/AlreadyExistsException.java @@ -14,16 +14,13 @@ */ package org.springframework.security.acls; -import org.springframework.security.core.SpringSecurityException; - - /** * Thrown if an Acl entry already exists for the object. * * @author Ben Alex * @version $Id$ */ -public class AlreadyExistsException extends SpringSecurityException { +public class AlreadyExistsException extends AclException { //~ Constructors =================================================================================================== /** diff --git a/acl/src/main/java/org/springframework/security/acls/ChildrenExistException.java b/acl/src/main/java/org/springframework/security/acls/ChildrenExistException.java index 72bfc03844..52bd9dd55f 100644 --- a/acl/src/main/java/org/springframework/security/acls/ChildrenExistException.java +++ b/acl/src/main/java/org/springframework/security/acls/ChildrenExistException.java @@ -14,16 +14,13 @@ */ package org.springframework.security.acls; -import org.springframework.security.core.SpringSecurityException; - - /** * Thrown if an {@link Acl} cannot be deleted because children Acls exist. * * @author Ben Alex * @version $Id$ */ -public class ChildrenExistException extends SpringSecurityException { +public class ChildrenExistException extends AclException { //~ Constructors =================================================================================================== /** diff --git a/acl/src/main/java/org/springframework/security/acls/IdentityUnavailableException.java b/acl/src/main/java/org/springframework/security/acls/IdentityUnavailableException.java index 156c552f90..4c0cf4aeff 100644 --- a/acl/src/main/java/org/springframework/security/acls/IdentityUnavailableException.java +++ b/acl/src/main/java/org/springframework/security/acls/IdentityUnavailableException.java @@ -14,16 +14,13 @@ */ package org.springframework.security.acls; -import org.springframework.security.core.SpringSecurityException; - - /** * Thrown if an ACL identity could not be extracted from an object. * * @author Ben Alex * @version $Id$ */ -public class IdentityUnavailableException extends SpringSecurityException { +public class IdentityUnavailableException extends AclException { //~ Constructors =================================================================================================== /** diff --git a/acl/src/main/java/org/springframework/security/acls/NotFoundException.java b/acl/src/main/java/org/springframework/security/acls/NotFoundException.java index 07a635c593..20dc4b87d3 100644 --- a/acl/src/main/java/org/springframework/security/acls/NotFoundException.java +++ b/acl/src/main/java/org/springframework/security/acls/NotFoundException.java @@ -14,16 +14,13 @@ */ package org.springframework.security.acls; -import org.springframework.security.core.SpringSecurityException; - - /** * Thrown if an ACL-related object cannot be found. * * @author Ben Alex * @version $Id$ */ -public class NotFoundException extends SpringSecurityException { +public class NotFoundException extends AclException { //~ Constructors =================================================================================================== /** diff --git a/acl/src/main/java/org/springframework/security/acls/UnloadedSidException.java b/acl/src/main/java/org/springframework/security/acls/UnloadedSidException.java index e6e2fea05b..451a55dbc4 100644 --- a/acl/src/main/java/org/springframework/security/acls/UnloadedSidException.java +++ b/acl/src/main/java/org/springframework/security/acls/UnloadedSidException.java @@ -14,9 +14,6 @@ */ package org.springframework.security.acls; -import org.springframework.security.core.SpringSecurityException; - - /** * Thrown if an {@link Acl} cannot perform an operation because it only loaded a subset of Sids and * the caller has requested details for an unloaded Sid. @@ -24,7 +21,7 @@ import org.springframework.security.core.SpringSecurityException; * @author Ben Alex * @version $Id$ */ -public class UnloadedSidException extends SpringSecurityException { +public class UnloadedSidException extends AclException { //~ Constructors =================================================================================================== /** diff --git a/config/src/main/java/org/springframework/security/config/SecurityConfigurationException.java b/config/src/main/java/org/springframework/security/config/SecurityConfigurationException.java index 3c50ff0633..bc4e03b579 100644 --- a/config/src/main/java/org/springframework/security/config/SecurityConfigurationException.java +++ b/config/src/main/java/org/springframework/security/config/SecurityConfigurationException.java @@ -1,14 +1,13 @@ package org.springframework.security.config; -import org.springframework.security.core.SpringSecurityException; - +import org.springframework.core.NestedRuntimeException; /** * @author Luke Taylor * @author Ben Alex * @version $Id$ */ -public class SecurityConfigurationException extends SpringSecurityException { +public class SecurityConfigurationException extends NestedRuntimeException { public SecurityConfigurationException(String s) { super(s); } diff --git a/core/src/main/java/org/springframework/security/access/AccessDeniedException.java b/core/src/main/java/org/springframework/security/access/AccessDeniedException.java index 5953494744..dd63585c4c 100644 --- a/core/src/main/java/org/springframework/security/access/AccessDeniedException.java +++ b/core/src/main/java/org/springframework/security/access/AccessDeniedException.java @@ -15,8 +15,8 @@ package org.springframework.security.access; +import org.springframework.core.NestedRuntimeException; import org.springframework.security.core.Authentication; -import org.springframework.security.core.SpringSecurityException; /** * Thrown if an {@link Authentication} object does not hold a required authority. @@ -24,7 +24,7 @@ import org.springframework.security.core.SpringSecurityException; * @author Ben Alex * @version $Id$ */ -public class AccessDeniedException extends SpringSecurityException { +public class AccessDeniedException extends NestedRuntimeException { //~ Constructors =================================================================================================== /** diff --git a/core/src/main/java/org/springframework/security/authentication/BadCredentialsException.java b/core/src/main/java/org/springframework/security/authentication/BadCredentialsException.java index 044b1fb43b..8ad0760593 100644 --- a/core/src/main/java/org/springframework/security/authentication/BadCredentialsException.java +++ b/core/src/main/java/org/springframework/security/authentication/BadCredentialsException.java @@ -51,7 +51,4 @@ public class BadCredentialsException extends AuthenticationException { public BadCredentialsException(String msg, Throwable t) { super(msg, t); } - - //~ Methods ======================================================================================================== - } diff --git a/core/src/main/java/org/springframework/security/authentication/jaas/DefaultLoginExceptionResolver.java b/core/src/main/java/org/springframework/security/authentication/jaas/DefaultLoginExceptionResolver.java index 17dc8c7014..cdad0ba216 100644 --- a/core/src/main/java/org/springframework/security/authentication/jaas/DefaultLoginExceptionResolver.java +++ b/core/src/main/java/org/springframework/security/authentication/jaas/DefaultLoginExceptionResolver.java @@ -16,7 +16,7 @@ package org.springframework.security.authentication.jaas; import org.springframework.security.authentication.AuthenticationServiceException; -import org.springframework.security.core.SpringSecurityException; +import org.springframework.security.core.AuthenticationException; import javax.security.auth.login.LoginException; @@ -30,7 +30,7 @@ import javax.security.auth.login.LoginException; public class DefaultLoginExceptionResolver implements LoginExceptionResolver { //~ Methods ======================================================================================================== - public SpringSecurityException resolveException(LoginException e) { + public AuthenticationException resolveException(LoginException e) { return new AuthenticationServiceException(e.getMessage(), e); } } diff --git a/core/src/main/java/org/springframework/security/authentication/jaas/JaasAuthenticationProvider.java b/core/src/main/java/org/springframework/security/authentication/jaas/JaasAuthenticationProvider.java index 87aa6315ea..3550ac5f6d 100644 --- a/core/src/main/java/org/springframework/security/authentication/jaas/JaasAuthenticationProvider.java +++ b/core/src/main/java/org/springframework/security/authentication/jaas/JaasAuthenticationProvider.java @@ -218,7 +218,7 @@ public class JaasAuthenticationProvider implements AuthenticationProvider, Appli return result; } catch (LoginException loginException) { - SpringSecurityException ase = loginExceptionResolver.resolveException(loginException); + AuthenticationException ase = loginExceptionResolver.resolveException(loginException); publishFailureEvent(request, ase); throw ase; @@ -354,7 +354,7 @@ public class JaasAuthenticationProvider implements AuthenticationProvider, Appli * @param token The {@link UsernamePasswordAuthenticationToken} being processed * @param ase The {@link SpringSecurityException} that caused the failure */ - protected void publishFailureEvent(UsernamePasswordAuthenticationToken token, SpringSecurityException ase) { + protected void publishFailureEvent(UsernamePasswordAuthenticationToken token, AuthenticationException ase) { applicationEventPublisher.publishEvent(new JaasAuthenticationFailedEvent(token, ase)); } diff --git a/core/src/main/java/org/springframework/security/authentication/jaas/LoginExceptionResolver.java b/core/src/main/java/org/springframework/security/authentication/jaas/LoginExceptionResolver.java index d67815a9ef..e1fb1ab075 100644 --- a/core/src/main/java/org/springframework/security/authentication/jaas/LoginExceptionResolver.java +++ b/core/src/main/java/org/springframework/security/authentication/jaas/LoginExceptionResolver.java @@ -15,15 +15,15 @@ package org.springframework.security.authentication.jaas; -import org.springframework.security.core.SpringSecurityException; +import org.springframework.security.core.AuthenticationException; import javax.security.auth.login.LoginException; /** * The JaasAuthenticationProvider takes an instance of LoginExceptionResolver - * to resolve LoginModule specific exceptions to Spring Security exceptions. For - * instance, a configured login module could throw a + * to resolve LoginModule specific exceptions to Spring Security AuthenticationExceptions. + * For instance, a configured login module could throw a * ScrewedUpPasswordException that extends LoginException, in this instance * the LoginExceptionResolver implementation would return a {@link * org.springframework.security.authentication.BadCredentialsException}. @@ -39,7 +39,7 @@ public interface LoginExceptionResolver { * * @param e The LoginException thrown by the configured LoginModule. * - * @return The SpringSecurityException that the JaasAuthenticationProvider should throw. + * @return The AuthenticationException that the JaasAuthenticationProvider should throw. */ - SpringSecurityException resolveException(LoginException e); + AuthenticationException resolveException(LoginException e); } diff --git a/core/src/main/java/org/springframework/security/authentication/rcp/RemoteAuthenticationException.java b/core/src/main/java/org/springframework/security/authentication/rcp/RemoteAuthenticationException.java index ec31bc440c..4102edb6f8 100644 --- a/core/src/main/java/org/springframework/security/authentication/rcp/RemoteAuthenticationException.java +++ b/core/src/main/java/org/springframework/security/authentication/rcp/RemoteAuthenticationException.java @@ -15,21 +15,22 @@ package org.springframework.security.authentication.rcp; -import org.springframework.security.core.SpringSecurityException; - +import org.springframework.core.NestedRuntimeException; /** - * Thrown if a RemoteAuthenticationManager cannot validate the presented authentication request.

This - * is thrown rather than the normal AuthenticationException because AuthenticationException - * contains additional properties which may cause issues for the remoting protocol.

+ * Thrown if a RemoteAuthenticationManager cannot validate the presented authentication request. + *

+ * This is thrown rather than the normal AuthenticationException because + * AuthenticationException contains additional properties which may cause issues for + * the remoting protocol. * * @author Ben Alex * @version $Id$ */ -public class RemoteAuthenticationException extends SpringSecurityException { +public class RemoteAuthenticationException extends NestedRuntimeException { //~ Constructors =================================================================================================== -/** + /** * Constructs a RemoteAuthenticationException with the * specified message and no root cause. * diff --git a/core/src/main/java/org/springframework/security/core/AuthenticationException.java b/core/src/main/java/org/springframework/security/core/AuthenticationException.java index 7cafee34cc..18e1fe00b9 100644 --- a/core/src/main/java/org/springframework/security/core/AuthenticationException.java +++ b/core/src/main/java/org/springframework/security/core/AuthenticationException.java @@ -15,6 +15,8 @@ package org.springframework.security.core; +import org.springframework.core.NestedRuntimeException; + /** * Abstract superclass for all exceptions related to an {@link Authentication} object being invalid for whatever @@ -23,7 +25,7 @@ package org.springframework.security.core; * @author Ben Alex * @version $Id$ */ -public abstract class AuthenticationException extends SpringSecurityException { +public abstract class AuthenticationException extends NestedRuntimeException { //~ Instance fields ================================================================================================ private Authentication authentication; diff --git a/core/src/main/java/org/springframework/security/userdetails/UsernameNotFoundException.java b/core/src/main/java/org/springframework/security/userdetails/UsernameNotFoundException.java index 1f8775852d..a8b4cf218b 100644 --- a/core/src/main/java/org/springframework/security/userdetails/UsernameNotFoundException.java +++ b/core/src/main/java/org/springframework/security/userdetails/UsernameNotFoundException.java @@ -15,7 +15,7 @@ package org.springframework.security.userdetails; -import org.springframework.security.authentication.BadCredentialsException; +import org.springframework.security.core.AuthenticationException; /** @@ -24,7 +24,7 @@ import org.springframework.security.authentication.BadCredentialsException; * @author Ben Alex * @version $Id$ */ -public class UsernameNotFoundException extends BadCredentialsException { +public class UsernameNotFoundException extends AuthenticationException { //~ Constructors =================================================================================================== /** diff --git a/core/src/main/java/org/springframework/security/util/EncryptionUtils.java b/core/src/main/java/org/springframework/security/util/EncryptionUtils.java index c0fc048b76..c6602f143d 100644 --- a/core/src/main/java/org/springframework/security/util/EncryptionUtils.java +++ b/core/src/main/java/org/springframework/security/util/EncryptionUtils.java @@ -23,8 +23,8 @@ import javax.crypto.SecretKey; import javax.crypto.SecretKeyFactory; import javax.crypto.spec.DESedeKeySpec; -import org.springframework.security.core.SpringSecurityException; import org.apache.commons.codec.binary.Base64; +import org.springframework.core.NestedRuntimeException; import org.springframework.util.Assert; /** @@ -150,7 +150,7 @@ public final class EncryptionUtils { Assert.isTrue(key.length() >= 24, "Key must be at least 24 characters long"); } - public static class EncryptionException extends SpringSecurityException { + public static class EncryptionException extends NestedRuntimeException { private static final long serialVersionUID = 1L; public EncryptionException(String message, Throwable t) { diff --git a/core/src/test/java/org/springframework/security/authentication/jaas/JaasAuthenticationProviderTests.java b/core/src/test/java/org/springframework/security/authentication/jaas/JaasAuthenticationProviderTests.java index 46988c276e..5eb9efad32 100644 --- a/core/src/test/java/org/springframework/security/authentication/jaas/JaasAuthenticationProviderTests.java +++ b/core/src/test/java/org/springframework/security/authentication/jaas/JaasAuthenticationProviderTests.java @@ -41,7 +41,6 @@ import org.springframework.security.core.AuthorityUtils; import org.springframework.security.core.GrantedAuthority; import org.springframework.security.core.GrantedAuthorityImpl; import org.springframework.security.core.SessionDestroyedEvent; -import org.springframework.security.core.SpringSecurityException; import org.springframework.security.core.context.SecurityContextImpl; @@ -187,7 +186,7 @@ public class JaasAuthenticationProviderTests extends TestCase { public void testLoginExceptionResolver() { assertNotNull(jaasProvider.getLoginExceptionResolver()); jaasProvider.setLoginExceptionResolver(new LoginExceptionResolver() { - public SpringSecurityException resolveException(LoginException e) { + public AuthenticationException resolveException(LoginException e) { return new LockedException("This is just a test!"); } }); 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 35670ba1e7..d84ad8b8e4 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 @@ -34,7 +34,6 @@ import org.springframework.security.core.AuthorityUtils; import org.springframework.security.core.GrantedAuthority; import org.springframework.security.ldap.LdapAuthenticator; import org.springframework.security.ldap.LdapAuthoritiesPopulator; -import org.springframework.security.ldap.authentication.LdapAuthenticationProvider; import org.springframework.security.ldap.userdetails.LdapUserDetailsMapper; import org.springframework.security.userdetails.UserDetails; import org.springframework.security.userdetails.UsernameNotFoundException; @@ -89,7 +88,7 @@ public class LdapAuthenticationProviderTests { ldapProvider.authenticate(new UsernamePasswordAuthenticationToken("jen", "")); } - @Test + @Test(expected=BadCredentialsException.class) public void usernameNotFoundExceptionIsHiddenByDefault() { final LdapAuthenticator authenticator = jmock.mock(LdapAuthenticator.class); final UsernamePasswordAuthenticationToken joe = new UsernamePasswordAuthenticationToken("joe", "password"); @@ -98,14 +97,7 @@ public class LdapAuthenticationProviderTests { }}); LdapAuthenticationProvider provider = new LdapAuthenticationProvider(authenticator); - try { - provider.authenticate(joe); - fail(); - } catch (BadCredentialsException expected) { - if (expected instanceof UsernameNotFoundException) { - fail("Exception should have been hidden"); - } - } + provider.authenticate(joe); } @Test(expected=UsernameNotFoundException.class) diff --git a/web/src/main/java/org/springframework/security/web/ExceptionTranslationFilter.java b/web/src/main/java/org/springframework/security/web/ExceptionTranslationFilter.java index f1fe454b4b..ccf0b36001 100644 --- a/web/src/main/java/org/springframework/security/web/ExceptionTranslationFilter.java +++ b/web/src/main/java/org/springframework/security/web/ExceptionTranslationFilter.java @@ -26,6 +26,7 @@ import org.springframework.security.util.ThrowableAnalyzer; import org.springframework.security.util.ThrowableCauseExtractor; import org.springframework.security.web.savedrequest.SavedRequest; import org.springframework.beans.factory.InitializingBean; +import org.springframework.core.NestedRuntimeException; import org.springframework.util.Assert; @@ -102,14 +103,17 @@ public class ExceptionTranslationFilter extends SpringSecurityFilter implements } catch (Exception ex) { // Try to extract a SpringSecurityException from the stacktrace - Throwable[] causeChain = this.throwableAnalyzer.determineCauseChain(ex); - SpringSecurityException ase = (SpringSecurityException) - this.throwableAnalyzer.getFirstThrowableOfType(SpringSecurityException.class, causeChain); + Throwable[] causeChain = throwableAnalyzer.determineCauseChain(ex); + NestedRuntimeException ase = (NestedRuntimeException) + throwableAnalyzer.getFirstThrowableOfType(AuthenticationException.class, causeChain); + + if (ase == null) { + ase = (NestedRuntimeException)throwableAnalyzer.getFirstThrowableOfType(AccessDeniedException.class, causeChain); + } if (ase != null) { handleException(request, response, chain, ase); - } - else { + } else { // Rethrow ServletExceptions and RuntimeExceptions as-is if (ex instanceof ServletException) { throw (ServletException) ex; @@ -137,7 +141,7 @@ public class ExceptionTranslationFilter extends SpringSecurityFilter implements } private void handleException(HttpServletRequest request, HttpServletResponse response, FilterChain chain, - SpringSecurityException exception) throws IOException, ServletException { + NestedRuntimeException exception) throws IOException, ServletException { if (exception instanceof AuthenticationException) { if (logger.isDebugEnabled()) { logger.debug("Authentication exception occurred; redirecting to authentication entry point", exception);