From 73b62497a3b8335802956f453fa72c43b73e67f1 Mon Sep 17 00:00:00 2001 From: Luke Taylor Date: Sun, 7 Feb 2010 00:04:57 +0000 Subject: [PATCH] SEC-1493: Added CredentialsContainer interface and implemented it in User, AbstractAuthenticationToken and UsernamePasswordAuthenticationToken. ProviderManager makes use of this to erase the credentials of the returned Authentication object (and its contents) if configured to do so by setting the 'eraseCredentialsAfterAuthentication' property. --- .../AbstractAuthenticationToken.java | 21 ++++++++++-- .../authentication/ProviderManager.java | 33 ++++++++++++++++++- .../RememberMeAuthenticationToken.java | 4 +-- .../UsernamePasswordAuthenticationToken.java | 12 +++++-- .../security/core/CredentialsContainer.java | 17 ++++++++++ .../security/core/userdetails/User.java | 15 ++++++--- .../authentication/ProviderManagerTests.java | 28 +++++++++++++--- 7 files changed, 113 insertions(+), 17 deletions(-) create mode 100644 core/src/main/java/org/springframework/security/core/CredentialsContainer.java diff --git a/core/src/main/java/org/springframework/security/authentication/AbstractAuthenticationToken.java b/core/src/main/java/org/springframework/security/authentication/AbstractAuthenticationToken.java index 19fd525ce4..0450e366cf 100644 --- a/core/src/main/java/org/springframework/security/authentication/AbstractAuthenticationToken.java +++ b/core/src/main/java/org/springframework/security/authentication/AbstractAuthenticationToken.java @@ -22,6 +22,7 @@ import java.util.Collections; import org.springframework.security.core.Authentication; import org.springframework.security.core.GrantedAuthority; +import org.springframework.security.core.CredentialsContainer; import org.springframework.security.core.authority.AuthorityUtils; import org.springframework.security.core.userdetails.UserDetails; @@ -34,7 +35,7 @@ import org.springframework.security.core.userdetails.UserDetails; * @author Ben Alex * @author Luke Taylor */ -public abstract class AbstractAuthenticationToken implements Authentication { +public abstract class AbstractAuthenticationToken implements Authentication, CredentialsContainer { //~ Instance fields ================================================================================================ private Object details; @@ -99,6 +100,22 @@ public abstract class AbstractAuthenticationToken implements Authentication { this.details = details; } + /** + * Checks the {@code credentials}, {@code principal} and {@code details} objects, invoking the + * {@code eraseCredentials} method on any which implement {@link CredentialsContainer}. + */ + public void eraseCredentials() { + eraseSecret(getCredentials()); + eraseSecret(getPrincipal()); + eraseSecret(details); + } + + private void eraseSecret(Object secret) { + if (secret instanceof CredentialsContainer) { + ((CredentialsContainer)secret).eraseCredentials(); + } + } + @Override public boolean equals(Object obj) { if (!(obj instanceof AbstractAuthenticationToken)) { @@ -174,7 +191,7 @@ public abstract class AbstractAuthenticationToken implements Authentication { StringBuilder sb = new StringBuilder(); sb.append(super.toString()).append(": "); sb.append("Principal: ").append(this.getPrincipal()).append("; "); - sb.append("Password: [PROTECTED]; "); + sb.append("Credentials: [PROTECTED]; "); sb.append("Authenticated: ").append(this.isAuthenticated()).append("; "); sb.append("Details: ").append(this.getDetails()).append("; "); 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 9117bb1109..e969c2ca19 100644 --- a/core/src/main/java/org/springframework/security/authentication/ProviderManager.java +++ b/core/src/main/java/org/springframework/security/authentication/ProviderManager.java @@ -26,6 +26,7 @@ import org.springframework.context.MessageSourceAware; import org.springframework.context.support.MessageSourceAccessor; import org.springframework.security.core.Authentication; import org.springframework.security.core.AuthenticationException; +import org.springframework.security.core.CredentialsContainer; import org.springframework.security.core.SpringSecurityMessageSource; import org.springframework.util.Assert; @@ -42,10 +43,17 @@ import org.springframework.util.Assert; * AuthenticationException, the last AuthenticationException received will be used. * If no provider returns a non-null response, or indicates it can even process an Authentication, * the ProviderManager will throw a ProviderNotFoundException. + * A parent {@code AuthenticationManager} can also be set, and this will also be tried if none of the configured + * providers can perform the authentication. This is intended to support namespace configuration options though and + * is not a feature that should normally be required. *

* The exception to this process is when a provider throws an {@link AccountStatusException}, in which case no * further providers in the list will be queried. * + * Post-authentication, the credentials will be cleared from the returned {@code Authentication} object, if it + * implements the {@link CredentialsContainer} interface. This behaviour can be controlled by modifying the + * {@link #setEraseCredentialsAfterAuthentication(boolean) eraseCredentialsAfterAuthentication} property. + * *

Event Publishing

*

* Authentication event publishing is delegated to the configured {@link AuthenticationEventPublisher} which defaults @@ -57,9 +65,10 @@ import org.springframework.util.Assert; * the <http> configuration, so you will receive events from the web part of your application automatically. *

* Note that the implementation also publishes authentication failure events when it obtains an authentication result - * (or an exception) from the "parent" AuthenticationManager if one has been set. So in this situation, the + * (or an exception) from the "parent" {@code AuthenticationManager} if one has been set. So in this situation, the * parent should not generally be configured to publish events or there will be duplicates. * + * * @author Ben Alex * @author Luke Taylor * @@ -76,6 +85,7 @@ public class ProviderManager extends AbstractAuthenticationManager implements Me private List providers = Collections.emptyList(); protected MessageSourceAccessor messages = SpringSecurityMessageSource.getAccessor(); private AuthenticationManager parent; + private boolean eraseCredentialsAfterAuthentication = false; //~ Methods ======================================================================================================== @@ -145,6 +155,11 @@ public class ProviderManager extends AbstractAuthenticationManager implements Me } if (result != null) { + if (eraseCredentialsAfterAuthentication && (result instanceof CredentialsContainer)) { + // Authentication is complete. Remove credentials and other secret data from authentication + ((CredentialsContainer)result).eraseCredentials(); + } + eventPublisher.publishAuthenticationSuccess(result); return result; } @@ -193,6 +208,22 @@ public class ProviderManager extends AbstractAuthenticationManager implements Me this.eventPublisher = eventPublisher; } + /** + * If set to, a resulting {@code Authentication} which implements the {@code CredentialsContainer} interface + * will have its {@link CredentialsContainer#eraseCredentials() eraseCredentials} method called before it is returned + * from the {@code authenticate()} method. + * + * @param eraseSecretData set to {@literal false} to retain the credentials data in memory. + * Defaults to {@literal true}. + */ + public void setEraseCredentialsAfterAuthentication(boolean eraseSecretData) { + this.eraseCredentialsAfterAuthentication = eraseSecretData; + } + + public boolean isEraseCredentialsAfterAuthentication() { + return eraseCredentialsAfterAuthentication; + } + /** * Sets the {@link AuthenticationProvider} objects to be used for authentication. * diff --git a/core/src/main/java/org/springframework/security/authentication/RememberMeAuthenticationToken.java b/core/src/main/java/org/springframework/security/authentication/RememberMeAuthenticationToken.java index d133bd185c..a720829a82 100644 --- a/core/src/main/java/org/springframework/security/authentication/RememberMeAuthenticationToken.java +++ b/core/src/main/java/org/springframework/security/authentication/RememberMeAuthenticationToken.java @@ -15,7 +15,6 @@ package org.springframework.security.authentication; -import java.io.Serializable; import java.util.Collection; import org.springframework.security.core.GrantedAuthority; @@ -28,8 +27,9 @@ import org.springframework.security.core.GrantedAuthority; * GrantedAuthoritys that apply. * * @author Ben Alex + * @author Luke Taylor */ -public class RememberMeAuthenticationToken extends AbstractAuthenticationToken implements Serializable { +public class RememberMeAuthenticationToken extends AbstractAuthenticationToken { //~ Instance fields ================================================================================================ private final Object principal; diff --git a/core/src/main/java/org/springframework/security/authentication/UsernamePasswordAuthenticationToken.java b/core/src/main/java/org/springframework/security/authentication/UsernamePasswordAuthenticationToken.java index 727eb7f11e..75fa959a24 100644 --- a/core/src/main/java/org/springframework/security/authentication/UsernamePasswordAuthenticationToken.java +++ b/core/src/main/java/org/springframework/security/authentication/UsernamePasswordAuthenticationToken.java @@ -22,8 +22,8 @@ import org.springframework.security.core.GrantedAuthority; /** - * An {@link org.springframework.security.core.Authentication} implementation that is designed for simple presentation of a - * username and password. + * An {@link org.springframework.security.core.Authentication} implementation that is designed for simple presentation + * of a username and password. *

* The principal and credentials should be set with an Object that provides * the respective property via its Object.toString() method. The simplest such Object to use @@ -34,8 +34,8 @@ import org.springframework.security.core.GrantedAuthority; public class UsernamePasswordAuthenticationToken extends AbstractAuthenticationToken { //~ Instance fields ================================================================================================ - private final Object credentials; private final Object principal; + private Object credentials; //~ Constructors =================================================================================================== @@ -94,4 +94,10 @@ public class UsernamePasswordAuthenticationToken extends AbstractAuthenticationT super.setAuthenticated(false); } + + @Override + public void eraseCredentials() { + super.eraseCredentials(); + credentials = null; + } } diff --git a/core/src/main/java/org/springframework/security/core/CredentialsContainer.java b/core/src/main/java/org/springframework/security/core/CredentialsContainer.java new file mode 100644 index 0000000000..cb78215144 --- /dev/null +++ b/core/src/main/java/org/springframework/security/core/CredentialsContainer.java @@ -0,0 +1,17 @@ +package org.springframework.security.core; + +/** + * Indicates that the implementing object contains sensitive data, which can be erased using the + * {@code eraseCredentials} method. Implementations are expected to invoke the method on any internal objects + * which may also implement this interface. + *

+ * For internal framework use only. Users who are writing their own {@code AuthenticationProvider} implementations + * should create and return an appropriate {@code Authentication} object there, minus any sensitive data, + * rather than using this interface. + * + * @author Luke Taylor + * @since 3.0.3 + */ +public interface CredentialsContainer { + void eraseCredentials(); +} diff --git a/core/src/main/java/org/springframework/security/core/userdetails/User.java b/core/src/main/java/org/springframework/security/core/userdetails/User.java index 9b3f813c44..dd40ac11b8 100644 --- a/core/src/main/java/org/springframework/security/core/userdetails/User.java +++ b/core/src/main/java/org/springframework/security/core/userdetails/User.java @@ -25,6 +25,7 @@ import java.util.SortedSet; import java.util.TreeSet; import org.springframework.security.core.GrantedAuthority; +import org.springframework.security.core.CredentialsContainer; import org.springframework.util.Assert; /** @@ -42,9 +43,9 @@ import org.springframework.util.Assert; * @author Ben Alex * @author Luke Taylor */ -public class User implements UserDetails { +public class User implements UserDetails, CredentialsContainer { //~ Instance fields ================================================================================================ - private final String password; + private String password; private final String username; private final Set authorities; private final boolean accountNonExpired; @@ -116,20 +117,24 @@ public class User implements UserDetails { return username; } + public boolean isEnabled() { + return enabled; + } + public boolean isAccountNonExpired() { return accountNonExpired; } public boolean isAccountNonLocked() { - return this.accountNonLocked; + return accountNonLocked; } public boolean isCredentialsNonExpired() { return credentialsNonExpired; } - public boolean isEnabled() { - return enabled; + public void eraseCredentials() { + password = null; } private static SortedSet sortAuthorities(Collection authorities) { 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 a8ec61c75f..0113f60aa1 100644 --- a/core/src/test/java/org/springframework/security/authentication/ProviderManagerTests.java +++ b/core/src/test/java/org/springframework/security/authentication/ProviderManagerTests.java @@ -28,7 +28,6 @@ import org.springframework.context.MessageSource; import org.springframework.security.core.Authentication; import org.springframework.security.core.AuthenticationException; import org.springframework.security.core.GrantedAuthority; -import org.springframework.security.core.authority.AuthorityUtils; /** * Tests {@link ProviderManager}. @@ -40,14 +39,33 @@ public class ProviderManagerTests { @Test(expected=ProviderNotFoundException.class) public void authenticationFailsWithUnsupportedToken() throws Exception { - UsernamePasswordAuthenticationToken token = new UsernamePasswordAuthenticationToken("Test", "Password", - AuthorityUtils.createAuthorityList("ROLE_ONE", "ROLE_TWO")); + Authentication token = new AbstractAuthenticationToken (null) { + public Object getCredentials() { + return ""; + } + public Object getPrincipal() { + return ""; + } + }; ProviderManager mgr = makeProviderManager(); mgr.setMessageSource(mock(MessageSource.class)); mgr.authenticate(token); } + @Test + public void credentialsAreClearedByDefault() throws Exception { + UsernamePasswordAuthenticationToken token = new UsernamePasswordAuthenticationToken("Test", "Password"); + ProviderManager mgr = makeProviderManager(); + Authentication result = mgr.authenticate(token); + assertNull(result.getCredentials()); + + mgr.setEraseCredentialsAfterAuthentication(false); + token = new UsernamePasswordAuthenticationToken("Test", "Password"); + result = mgr.authenticate(token); + assertNotNull(result.getCredentials()); + } + @Test public void authenticationSucceedsWithSupportedTokenAndReturnsExpectedObject() throws Exception { final Authentication a = mock(Authentication.class); @@ -126,6 +144,7 @@ public class ProviderManagerTests { request.setDetails(details); Authentication result = authMgr.authenticate(request); + assertNotNull(result.getCredentials()); assertSame(details, result.getDetails()); } @@ -278,7 +297,8 @@ public class ProviderManagerTests { } public boolean supports(Class authentication) { - if (TestingAuthenticationToken.class.isAssignableFrom(authentication)) { + if (TestingAuthenticationToken.class.isAssignableFrom(authentication) || + UsernamePasswordAuthenticationToken.class.isAssignableFrom(authentication)) { return true; } else { return false;