From 59651f52141dbdc355b52b3dd38b0bb9a91068d5 Mon Sep 17 00:00:00 2001 From: Luke Taylor Date: Mon, 18 Feb 2008 20:18:40 +0000 Subject: [PATCH] SEC-678: Moved extraInformation property to AuthenticationException so ti isn't only available in BadCredentialsException. Added clearExtraInformation flag to AbstractAuthenticationManager to allow the information to be removed if required before rethrowing. --- .../AbstractAuthenticationManager.java | 20 ++++++++++++++ .../security/AccountExpiredException.java | 4 +++ .../security/AccountStatusException.java | 4 +++ .../security/AuthenticationException.java | 27 ++++++++++++++++--- .../security/BadCredentialsException.java | 15 +---------- .../security/CredentialsExpiredException.java | 4 +++ .../security/DisabledException.java | 4 +++ .../security/LockedException.java | 4 +++ ...ractUserDetailsAuthenticationProvider.java | 17 +++++++----- .../dao/DaoAuthenticationProvider.java | 9 ++++++- .../AccountStatusUserDetailsChecker.java | 8 +++--- 11 files changed, 88 insertions(+), 28 deletions(-) diff --git a/core/src/main/java/org/springframework/security/AbstractAuthenticationManager.java b/core/src/main/java/org/springframework/security/AbstractAuthenticationManager.java index fe0a4a17b8..0a1d7626bb 100644 --- a/core/src/main/java/org/springframework/security/AbstractAuthenticationManager.java +++ b/core/src/main/java/org/springframework/security/AbstractAuthenticationManager.java @@ -22,6 +22,10 @@ package org.springframework.security; * @version $Id$ */ public abstract class AbstractAuthenticationManager implements AuthenticationManager { + + //~ Instance fields ================================================================================================ + private boolean clearExtraInformation = false; + //~ Methods ======================================================================================================== /** @@ -42,6 +46,11 @@ public abstract class AbstractAuthenticationManager implements AuthenticationMan return doAuthentication(authRequest); } catch (AuthenticationException e) { e.setAuthentication(authRequest); + + if (clearExtraInformation) { + e.clearExtraInformation(); + } + throw e; } } @@ -59,4 +68,15 @@ public abstract class AbstractAuthenticationManager implements AuthenticationMan * @throws AuthenticationException if authentication fails */ protected abstract Authentication doAuthentication(Authentication authentication) throws AuthenticationException; + + /** + * If set to true, the extraInformation set on an AuthenticationException will be cleared + * before rethrowing it. This is useful for use with remoting protocols where the information shouldn't + * be serialized to the client. Defaults to 'false'. + * + * @see org.springframework.security.AuthenticationException#getExtraInformation() + */ + public void setClearExtraInformation(boolean clearExtraInformation) { + this.clearExtraInformation = clearExtraInformation; + } } diff --git a/core/src/main/java/org/springframework/security/AccountExpiredException.java b/core/src/main/java/org/springframework/security/AccountExpiredException.java index 9b0eb3fab3..9bed62c7d6 100644 --- a/core/src/main/java/org/springframework/security/AccountExpiredException.java +++ b/core/src/main/java/org/springframework/security/AccountExpiredException.java @@ -45,4 +45,8 @@ public class AccountExpiredException extends AccountStatusException { public AccountExpiredException(String msg, Throwable t) { super(msg, t); } + + public AccountExpiredException(String msg, Object extraInformation) { + super(msg, extraInformation); + } } diff --git a/core/src/main/java/org/springframework/security/AccountStatusException.java b/core/src/main/java/org/springframework/security/AccountStatusException.java index 6d820bf548..484fffb856 100644 --- a/core/src/main/java/org/springframework/security/AccountStatusException.java +++ b/core/src/main/java/org/springframework/security/AccountStatusException.java @@ -15,4 +15,8 @@ public abstract class AccountStatusException extends AuthenticationException { public AccountStatusException(String msg, Throwable t) { super(msg, t); } + + protected AccountStatusException(String msg, Object extraInformation) { + super(msg, extraInformation); + } } diff --git a/core/src/main/java/org/springframework/security/AuthenticationException.java b/core/src/main/java/org/springframework/security/AuthenticationException.java index 93a9f235b7..07cf1fa73b 100644 --- a/core/src/main/java/org/springframework/security/AuthenticationException.java +++ b/core/src/main/java/org/springframework/security/AuthenticationException.java @@ -25,12 +25,12 @@ package org.springframework.security; public abstract class AuthenticationException extends SpringSecurityException { //~ Instance fields ================================================================================================ - /** The authentication that related to this exception (may be null) */ private Authentication authentication; + private Object extraInformation; //~ Constructors =================================================================================================== -/** + /** * Constructs an AuthenticationException with the specified * message and root cause. * @@ -41,7 +41,7 @@ public abstract class AuthenticationException extends SpringSecurityException { super(msg, t); } -/** + /** * Constructs an AuthenticationException with the specified * message and no root cause. * @@ -51,8 +51,16 @@ public abstract class AuthenticationException extends SpringSecurityException { super(msg); } + public AuthenticationException(String msg, Object extraInformation) { + super(msg); + this.extraInformation = extraInformation; + } + //~ Methods ======================================================================================================== + /** + * The authentication request which this exception corresponds to (may be null) + */ public Authentication getAuthentication() { return authentication; } @@ -60,4 +68,17 @@ public abstract class AuthenticationException extends SpringSecurityException { void setAuthentication(Authentication authentication) { this.authentication = authentication; } + + /** + * Any additional information about the exception. Generally a UserDetails object. + * + * @return extra information or null + */ + public Object getExtraInformation() { + return extraInformation; + } + + void clearExtraInformation() { + this.extraInformation = null; + } } diff --git a/core/src/main/java/org/springframework/security/BadCredentialsException.java b/core/src/main/java/org/springframework/security/BadCredentialsException.java index 812cb151e0..a7a39e992f 100644 --- a/core/src/main/java/org/springframework/security/BadCredentialsException.java +++ b/core/src/main/java/org/springframework/security/BadCredentialsException.java @@ -23,10 +23,6 @@ package org.springframework.security; * @version $Id$ */ public class BadCredentialsException extends AuthenticationException { - //~ Instance fields ================================================================================================ - - private Object extraInformation; - //~ Constructors =================================================================================================== /** @@ -40,8 +36,7 @@ public class BadCredentialsException extends AuthenticationException { } public BadCredentialsException(String msg, Object extraInformation) { - super(msg); - this.extraInformation = extraInformation; + super(msg, extraInformation); } /** @@ -57,12 +52,4 @@ public class BadCredentialsException extends AuthenticationException { //~ Methods ======================================================================================================== - /** - * Any additional information about the exception. Generally a UserDetails object. - * - * @return extra information or null - */ - public Object getExtraInformation() { - return extraInformation; - } } diff --git a/core/src/main/java/org/springframework/security/CredentialsExpiredException.java b/core/src/main/java/org/springframework/security/CredentialsExpiredException.java index a65396886a..8b2714218c 100644 --- a/core/src/main/java/org/springframework/security/CredentialsExpiredException.java +++ b/core/src/main/java/org/springframework/security/CredentialsExpiredException.java @@ -45,4 +45,8 @@ public class CredentialsExpiredException extends AccountStatusException { public CredentialsExpiredException(String msg, Throwable t) { super(msg, t); } + + public CredentialsExpiredException(String msg, Object extraInformation) { + super(msg, extraInformation); + } } diff --git a/core/src/main/java/org/springframework/security/DisabledException.java b/core/src/main/java/org/springframework/security/DisabledException.java index d34a80fec7..e318dd9cb2 100644 --- a/core/src/main/java/org/springframework/security/DisabledException.java +++ b/core/src/main/java/org/springframework/security/DisabledException.java @@ -44,4 +44,8 @@ public class DisabledException extends AccountStatusException { public DisabledException(String msg, Throwable t) { super(msg, t); } + + public DisabledException(String msg, Object extraInformation) { + super(msg, extraInformation); + } } diff --git a/core/src/main/java/org/springframework/security/LockedException.java b/core/src/main/java/org/springframework/security/LockedException.java index 889a48b48c..d6b32c8038 100644 --- a/core/src/main/java/org/springframework/security/LockedException.java +++ b/core/src/main/java/org/springframework/security/LockedException.java @@ -44,4 +44,8 @@ public class LockedException extends AccountStatusException { public LockedException(String msg, Throwable t) { super(msg, t); } + + public LockedException(String msg, Object extraInformation) { + super(msg, extraInformation); + } } diff --git a/core/src/main/java/org/springframework/security/providers/dao/AbstractUserDetailsAuthenticationProvider.java b/core/src/main/java/org/springframework/security/providers/dao/AbstractUserDetailsAuthenticationProvider.java index a699136ae3..8b2ae536ec 100644 --- a/core/src/main/java/org/springframework/security/providers/dao/AbstractUserDetailsAuthenticationProvider.java +++ b/core/src/main/java/org/springframework/security/providers/dao/AbstractUserDetailsAuthenticationProvider.java @@ -269,6 +269,12 @@ public abstract class AbstractUserDetailsAuthenticationProvider implements Authe return preAuthenticationChecks; } + /** + * Sets the policy will be used to verify the status of the loaded UserDetails before + * validation of the credentials takes place. + * + * @param preAuthenticationChecks strategy to be invoked prior to authentication. + */ public void setPreAuthenticationChecks(UserDetailsChecker preAuthenticationChecks) { this.preAuthenticationChecks = preAuthenticationChecks; } @@ -285,19 +291,18 @@ public abstract class AbstractUserDetailsAuthenticationProvider implements Authe public void check(UserDetails user) { if (!user.isAccountNonLocked()) { throw new LockedException(messages.getMessage("AbstractUserDetailsAuthenticationProvider.locked", - "User account is locked")); + "User account is locked"), user); } if (!user.isEnabled()) { throw new DisabledException(messages.getMessage("AbstractUserDetailsAuthenticationProvider.disabled", - "User is disabled")); + "User is disabled"), user); } if (!user.isAccountNonExpired()) { throw new AccountExpiredException(messages.getMessage("AbstractUserDetailsAuthenticationProvider.expired", - "User account has expired")); + "User account has expired"), user); } - } } @@ -305,9 +310,9 @@ public abstract class AbstractUserDetailsAuthenticationProvider implements Authe public void check(UserDetails user) { if (!user.isCredentialsNonExpired()) { throw new CredentialsExpiredException(messages.getMessage( - "AbstractUserDetailsAuthenticationProvider.credentialsExpired", "User credentials have expired")); + "AbstractUserDetailsAuthenticationProvider.credentialsExpired", + "User credentials have expired"), user); } - } } } diff --git a/core/src/main/java/org/springframework/security/providers/dao/DaoAuthenticationProvider.java b/core/src/main/java/org/springframework/security/providers/dao/DaoAuthenticationProvider.java index 28cb6744fa..1b51ccb15e 100644 --- a/core/src/main/java/org/springframework/security/providers/dao/DaoAuthenticationProvider.java +++ b/core/src/main/java/org/springframework/security/providers/dao/DaoAuthenticationProvider.java @@ -130,10 +130,17 @@ public class DaoAuthenticationProvider extends AbstractUserDetailsAuthentication return userDetailsService; } - public boolean isIncludeDetailsObject() { + protected boolean isIncludeDetailsObject() { return includeDetailsObject; } + /** + * Determines whether the UserDetails will be included in the extraInformation field of a + * thrown BadCredentialsException. Defaults to true, but can be set to false if the exception will be + * used with a remoting protocol, for example. + * + * @deprecated use {@link org.springframework.security.providers.ProviderManager#setClearExtraInformation(boolean)} + */ public void setIncludeDetailsObject(boolean includeDetailsObject) { this.includeDetailsObject = includeDetailsObject; } diff --git a/core/src/main/java/org/springframework/security/userdetails/checker/AccountStatusUserDetailsChecker.java b/core/src/main/java/org/springframework/security/userdetails/checker/AccountStatusUserDetailsChecker.java index ab08cbf628..378048b401 100644 --- a/core/src/main/java/org/springframework/security/userdetails/checker/AccountStatusUserDetailsChecker.java +++ b/core/src/main/java/org/springframework/security/userdetails/checker/AccountStatusUserDetailsChecker.java @@ -19,21 +19,21 @@ public class AccountStatusUserDetailsChecker implements UserDetailsChecker { public void check(UserDetails user) { if (!user.isAccountNonLocked()) { - throw new LockedException(messages.getMessage("UserDetailsService.locked", "User account is locked")); + throw new LockedException(messages.getMessage("UserDetailsService.locked", "User account is locked"), user); } if (!user.isEnabled()) { - throw new DisabledException(messages.getMessage("UserDetailsService.disabled", "User is disabled")); + throw new DisabledException(messages.getMessage("UserDetailsService.disabled", "User is disabled"), user); } if (!user.isAccountNonExpired()) { throw new AccountExpiredException(messages.getMessage("UserDetailsService.expired", - "User account has expired")); + "User account has expired"), user); } if (!user.isCredentialsNonExpired()) { throw new CredentialsExpiredException(messages.getMessage("UserDetailsService.credentialsExpired", - "User credentials have expired")); + "User credentials have expired"), user); } } }