From db913f685715f04b1a477a33fb4d76f21b819099 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 | 34 +++++++++++++++++-- .../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(+), 18 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 19fd525ce43..0450e366cfb 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 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 @@ * @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 void setDetails(Object details) { 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 String toString() { 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 55badc88797..52b95b25d3d 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.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 @@ * 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 @@ * 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,7 +85,7 @@ public class ProviderManager implements AuthenticationManager, MessageSourceAwar private List providers = Collections.emptyList(); protected MessageSourceAccessor messages = SpringSecurityMessageSource.getAccessor(); private AuthenticationManager parent; - + private boolean eraseCredentialsAfterAuthentication = true; private boolean clearExtraInformation = false; //~ Methods ======================================================================================================== @@ -150,6 +159,11 @@ public Authentication authenticate(Authentication authentication) throws Authent } 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; } @@ -207,6 +221,22 @@ public void setAuthenticationEventPublisher(AuthenticationEventPublisher eventPu 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 d133bd185cc..a720829a82b 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 @@ * 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 727eb7f11ea..75fa959a249 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 @@ /** - * 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 @@ public class UsernamePasswordAuthenticationToken extends AbstractAuthenticationToken { //~ Instance fields ================================================================================================ - private final Object credentials; private final Object principal; + private Object credentials; //~ Constructors =================================================================================================== @@ -94,4 +94,10 @@ public void setAuthenticated(boolean isAuthenticated) throws IllegalArgumentExce 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 00000000000..cb782151448 --- /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 fcc5f7f2857..d56ccd12205 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 @@ -24,6 +24,7 @@ import java.util.TreeSet; import org.springframework.security.core.GrantedAuthority; +import org.springframework.security.core.CredentialsContainer; import org.springframework.util.Assert; /** @@ -41,9 +42,9 @@ * @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; @@ -113,20 +114,24 @@ public String getUsername() { 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 a8ec61c75f6..0113f60aa1d 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.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 void detailsAreSetOnAuthenticationTokenIfNotAlreadySetByProvider() throws request.setDetails(details); Authentication result = authMgr.authenticate(request); + assertNotNull(result.getCredentials()); assertSame(details, result.getDetails()); } @@ -278,7 +297,8 @@ public Authentication authenticate(Authentication authentication) throws Authent } public boolean supports(Class authentication) { - if (TestingAuthenticationToken.class.isAssignableFrom(authentication)) { + if (TestingAuthenticationToken.class.isAssignableFrom(authentication) || + UsernamePasswordAuthenticationToken.class.isAssignableFrom(authentication)) { return true; } else { return false;