From 712f1770d94d43ce64f0df30a38c35eaeea4cf60 Mon Sep 17 00:00:00 2001 From: Luke Taylor Date: Thu, 13 Mar 2008 16:03:18 +0000 Subject: [PATCH] SEC-714: Refactor PreAuthenticatedGrantedAuthoritiesSetter and PreAuthenticatedGrantedAuthoritiesRetriever http://jira.springframework.org/browse/SEC-714 --- .../security/GrantedAuthoritiesContainer.java | 15 +++++++ .../MutableGrantedAuthoritiesContainer.java | 18 ++++++++ ...henticatedGrantedAuthoritiesRetriever.java | 18 -------- ...AuthenticatedGrantedAuthoritiesSetter.java | 21 ---------- ...dGrantedAuthoritiesUserDetailsService.java | 15 ++++--- ...edAuthoritiesWebAuthenticationDetails.java | 41 +++++++------------ ...ticatedWebAuthenticationDetailsSource.java | 10 ++--- ...tedAuthoritiesUserDetailsServiceTests.java | 5 ++- ...horitiesWebAuthenticationDetailsTests.java | 15 +++---- ...edWebAuthenticationDetailsSourceTests.java | 3 +- 10 files changed, 69 insertions(+), 92 deletions(-) create mode 100644 core/src/main/java/org/springframework/security/GrantedAuthoritiesContainer.java create mode 100644 core/src/main/java/org/springframework/security/MutableGrantedAuthoritiesContainer.java delete mode 100755 core/src/main/java/org/springframework/security/providers/preauth/PreAuthenticatedGrantedAuthoritiesRetriever.java delete mode 100755 core/src/main/java/org/springframework/security/providers/preauth/PreAuthenticatedGrantedAuthoritiesSetter.java diff --git a/core/src/main/java/org/springframework/security/GrantedAuthoritiesContainer.java b/core/src/main/java/org/springframework/security/GrantedAuthoritiesContainer.java new file mode 100644 index 0000000000..21cc1d2228 --- /dev/null +++ b/core/src/main/java/org/springframework/security/GrantedAuthoritiesContainer.java @@ -0,0 +1,15 @@ +package org.springframework.security; + +/** + * Indicates that a object stores GrantedAuthority objects. + *

+ * Typically used in a pre-authenticated scenario when an AuthenticationDetails instance may also be + * used to obtain user authorities. + * + * @author Ruud Senden + * @author Luke Taylor + * @since 2.0 + */ +public interface GrantedAuthoritiesContainer { + GrantedAuthority[] getGrantedAuthorities(); +} diff --git a/core/src/main/java/org/springframework/security/MutableGrantedAuthoritiesContainer.java b/core/src/main/java/org/springframework/security/MutableGrantedAuthoritiesContainer.java new file mode 100644 index 0000000000..61211e779f --- /dev/null +++ b/core/src/main/java/org/springframework/security/MutableGrantedAuthoritiesContainer.java @@ -0,0 +1,18 @@ +package org.springframework.security; + +/** + * Indicates that a object can be used to store and retrieve GrantedAuthority objects. + *

+ * Typically used in a pre-authenticated scenario when an AuthenticationDetails instance may also be + * used to obtain user authorities. + * + * @author Ruud Senden + * @author Luke Taylor + * @since 2.0 + */ +public interface MutableGrantedAuthoritiesContainer extends GrantedAuthoritiesContainer { + /** + * Used to store authorities in the containing object. + */ + void setGrantedAuthorities(GrantedAuthority[] authorities); +} diff --git a/core/src/main/java/org/springframework/security/providers/preauth/PreAuthenticatedGrantedAuthoritiesRetriever.java b/core/src/main/java/org/springframework/security/providers/preauth/PreAuthenticatedGrantedAuthoritiesRetriever.java deleted file mode 100755 index c78f090bc0..0000000000 --- a/core/src/main/java/org/springframework/security/providers/preauth/PreAuthenticatedGrantedAuthoritiesRetriever.java +++ /dev/null @@ -1,18 +0,0 @@ -package org.springframework.security.providers.preauth; - -import org.springframework.security.GrantedAuthority; - - -/** - * Interface that allows for retrieval of a list of pre-authenticated Granted - * Authorities. - * - * @author Ruud Senden - * @since 2.0 - */ -public interface PreAuthenticatedGrantedAuthoritiesRetriever { - /** - * @return GrantedAuthority[] List of pre-authenticated GrantedAuthorities - */ - GrantedAuthority[] getPreAuthenticatedGrantedAuthorities(); -} diff --git a/core/src/main/java/org/springframework/security/providers/preauth/PreAuthenticatedGrantedAuthoritiesSetter.java b/core/src/main/java/org/springframework/security/providers/preauth/PreAuthenticatedGrantedAuthoritiesSetter.java deleted file mode 100755 index 179d81f730..0000000000 --- a/core/src/main/java/org/springframework/security/providers/preauth/PreAuthenticatedGrantedAuthoritiesSetter.java +++ /dev/null @@ -1,21 +0,0 @@ -package org.springframework.security.providers.preauth; - -import org.springframework.security.GrantedAuthority; - -/** - * Counterpart of PreAuthenticatedGrantedAuthoritiesRetriever that allows - * setting a list of pre-authenticated GrantedAuthorities. This interface is not - * actually being used by the PreAuthenticatedAuthenticationProvider or one of - * its related classes, but may be useful for classes that also implement - * PreAuthenticatedGrantedAuthoritiesRetriever. - * - * @author Ruud Senden - * @since 2.0 - */ -public interface PreAuthenticatedGrantedAuthoritiesSetter { - /** - * @param aPreAuthenticatedGrantedAuthorities - * The pre-authenticated GrantedAuthority[] to set - */ - void setPreAuthenticatedGrantedAuthorities(GrantedAuthority[] aPreAuthenticatedGrantedAuthorities); -} diff --git a/core/src/main/java/org/springframework/security/providers/preauth/PreAuthenticatedGrantedAuthoritiesUserDetailsService.java b/core/src/main/java/org/springframework/security/providers/preauth/PreAuthenticatedGrantedAuthoritiesUserDetailsService.java index 4709c70907..ef07befde6 100755 --- a/core/src/main/java/org/springframework/security/providers/preauth/PreAuthenticatedGrantedAuthoritiesUserDetailsService.java +++ b/core/src/main/java/org/springframework/security/providers/preauth/PreAuthenticatedGrantedAuthoritiesUserDetailsService.java @@ -2,6 +2,7 @@ package org.springframework.security.providers.preauth; import org.springframework.security.userdetails.UserDetails; import org.springframework.security.userdetails.User; +import org.springframework.security.GrantedAuthoritiesContainer; import org.springframework.security.GrantedAuthority; import org.springframework.security.AuthenticationException; import org.springframework.security.Authentication; @@ -20,10 +21,8 @@ import org.springframework.util.Assert; * PreAuthenticatedAuthenticationToken.getDetails(). * *

- * The details object as returned by - * PreAuthenticatedAuthenticationToken.getDetails() must implement the - * PreAuthenticatedGrantedAuthoritiesRetriever interface for this implementation - * to work. + * The details object as returned by PreAuthenticatedAuthenticationToken.getDetails() must implement the + * {@link GrantedAuthoritiesContainer} interface for this implementation to work. * * @author Ruud Senden * @since 2.0 @@ -32,14 +31,14 @@ public class PreAuthenticatedGrantedAuthoritiesUserDetailsService implements Aut /** * Get a UserDetails object based on the user name contained in the given * token, and the GrantedAuthorities as returned by the - * PreAuthenticatedGrantedAuthoritiesRetriever implementation as returned by + * GrantedAuthoritiesContainer implementation as returned by * the token.getDetails() method. */ public UserDetails loadUserDetails(Authentication token) throws AuthenticationException { Assert.notNull(token.getDetails()); - Assert.isInstanceOf(PreAuthenticatedGrantedAuthoritiesRetriever.class, token.getDetails()); - GrantedAuthority[] preAuthenticatedGrantedAuthorities = ((PreAuthenticatedGrantedAuthoritiesRetriever) token.getDetails()) - .getPreAuthenticatedGrantedAuthorities(); + Assert.isInstanceOf(GrantedAuthoritiesContainer.class, token.getDetails()); + GrantedAuthority[] preAuthenticatedGrantedAuthorities = ((GrantedAuthoritiesContainer) token.getDetails()) + .getGrantedAuthorities(); UserDetails ud = new User(token.getName(), "N/A", true, true, true, true, preAuthenticatedGrantedAuthorities); return ud; } diff --git a/core/src/main/java/org/springframework/security/ui/preauth/PreAuthenticatedGrantedAuthoritiesWebAuthenticationDetails.java b/core/src/main/java/org/springframework/security/ui/preauth/PreAuthenticatedGrantedAuthoritiesWebAuthenticationDetails.java index 048c1ba111..ed2b2e43af 100755 --- a/core/src/main/java/org/springframework/security/ui/preauth/PreAuthenticatedGrantedAuthoritiesWebAuthenticationDetails.java +++ b/core/src/main/java/org/springframework/security/ui/preauth/PreAuthenticatedGrantedAuthoritiesWebAuthenticationDetails.java @@ -4,10 +4,9 @@ import java.util.Arrays; import javax.servlet.http.HttpServletRequest; -import org.springframework.security.providers.preauth.PreAuthenticatedGrantedAuthoritiesRetriever; -import org.springframework.security.providers.preauth.PreAuthenticatedGrantedAuthoritiesSetter; import org.springframework.security.ui.WebAuthenticationDetails; import org.springframework.security.GrantedAuthority; +import org.springframework.security.MutableGrantedAuthoritiesContainer; import org.springframework.util.Assert; @@ -16,10 +15,11 @@ import org.springframework.util.Assert; * pre-authenticated Granted Authorities. * * @author Ruud Senden + * @author Luke Taylor * @since 2.0 */ public class PreAuthenticatedGrantedAuthoritiesWebAuthenticationDetails extends WebAuthenticationDetails implements - PreAuthenticatedGrantedAuthoritiesRetriever, PreAuthenticatedGrantedAuthoritiesSetter { + MutableGrantedAuthoritiesContainer { public static final long serialVersionUID = 1L; private GrantedAuthority[] preAuthenticatedGrantedAuthorities = null; @@ -28,35 +28,22 @@ public class PreAuthenticatedGrantedAuthoritiesWebAuthenticationDetails extends super(request); } - /** - * @return The String representation of this object. - */ - public String toString() { - StringBuffer sb = new StringBuffer(); - sb.append(super.toString() + "; "); - sb.append("preAuthenticatedGrantedAuthorities: " + Arrays.asList(preAuthenticatedGrantedAuthorities)); - return sb.toString(); - } - - /* - * (non-Javadoc) - * - * @see org.springframework.security.providers.preauth.PreAuthenticatedGrantedAuthoritiesRetriever#getPreAuthenticatedGrantedAuthorities() - */ - public GrantedAuthority[] getPreAuthenticatedGrantedAuthorities() { + public GrantedAuthority[] getGrantedAuthorities() { Assert.notNull(preAuthenticatedGrantedAuthorities, "Pre-authenticated granted authorities have not been set"); GrantedAuthority[] result = new GrantedAuthority[preAuthenticatedGrantedAuthorities.length]; System.arraycopy(preAuthenticatedGrantedAuthorities, 0, result, 0, result.length); return result; } - /* - * (non-Javadoc) - * - * @see org.springframework.security.providers.preauth.j2ee.PreAuthenticatedGrantedAuthoritiesSetter#setJ2eeBasedGrantedAuthorities() - */ - public void setPreAuthenticatedGrantedAuthorities(GrantedAuthority[] aJ2eeBasedGrantedAuthorities) { - this.preAuthenticatedGrantedAuthorities = new GrantedAuthority[aJ2eeBasedGrantedAuthorities.length]; - System.arraycopy(aJ2eeBasedGrantedAuthorities, 0, preAuthenticatedGrantedAuthorities, 0, preAuthenticatedGrantedAuthorities.length); + public void setGrantedAuthorities(GrantedAuthority[] authorities) { + this.preAuthenticatedGrantedAuthorities = new GrantedAuthority[authorities.length]; + System.arraycopy(authorities, 0, preAuthenticatedGrantedAuthorities, 0, preAuthenticatedGrantedAuthorities.length); } + + public String toString() { + StringBuffer sb = new StringBuffer(); + sb.append(super.toString() + "; "); + sb.append("preAuthenticatedGrantedAuthorities: " + Arrays.asList(preAuthenticatedGrantedAuthorities)); + return sb.toString(); + } } diff --git a/core/src/main/java/org/springframework/security/ui/preauth/j2ee/J2eeBasedPreAuthenticatedWebAuthenticationDetailsSource.java b/core/src/main/java/org/springframework/security/ui/preauth/j2ee/J2eeBasedPreAuthenticatedWebAuthenticationDetailsSource.java index 1c7030a506..1e664cd4b2 100755 --- a/core/src/main/java/org/springframework/security/ui/preauth/j2ee/J2eeBasedPreAuthenticatedWebAuthenticationDetailsSource.java +++ b/core/src/main/java/org/springframework/security/ui/preauth/j2ee/J2eeBasedPreAuthenticatedWebAuthenticationDetailsSource.java @@ -2,8 +2,8 @@ package org.springframework.security.ui.preauth.j2ee; import org.springframework.security.ui.preauth.PreAuthenticatedGrantedAuthoritiesWebAuthenticationDetails; import org.springframework.security.ui.WebAuthenticationDetailsSource; -import org.springframework.security.providers.preauth.PreAuthenticatedGrantedAuthoritiesSetter; import org.springframework.security.GrantedAuthority; +import org.springframework.security.MutableGrantedAuthoritiesContainer; import org.springframework.security.authoritymapping.Attributes2GrantedAuthoritiesMapper; import org.springframework.security.authoritymapping.MappableAttributesRetriever; @@ -48,7 +48,7 @@ public class J2eeBasedPreAuthenticatedWebAuthenticationDetailsSource extends Web /** * Build the authentication details object. If the specified authentication - * details class implements the PreAuthenticatedGrantedAuthoritiesSetter, a + * details class implements {@link MutableGrantedAuthoritiesContainer}, a * list of pre-authenticated Granted Authorities will be set based on the * J2EE roles for the current user. * @@ -56,9 +56,9 @@ public class J2eeBasedPreAuthenticatedWebAuthenticationDetailsSource extends Web */ public Object buildDetails(Object context) { Object result = super.buildDetails(context); - if (result instanceof PreAuthenticatedGrantedAuthoritiesSetter) { - ((PreAuthenticatedGrantedAuthoritiesSetter) result) - .setPreAuthenticatedGrantedAuthorities(getJ2eeBasedGrantedAuthorities((HttpServletRequest)context)); + if (result instanceof MutableGrantedAuthoritiesContainer) { + ((MutableGrantedAuthoritiesContainer) result) + .setGrantedAuthorities(getJ2eeBasedGrantedAuthorities((HttpServletRequest)context)); } return result; } diff --git a/core/src/test/java/org/springframework/security/providers/preauth/PreAuthenticatedGrantedAuthoritiesUserDetailsServiceTests.java b/core/src/test/java/org/springframework/security/providers/preauth/PreAuthenticatedGrantedAuthoritiesUserDetailsServiceTests.java index 1099d59056..ca17e5f490 100755 --- a/core/src/test/java/org/springframework/security/providers/preauth/PreAuthenticatedGrantedAuthoritiesUserDetailsServiceTests.java +++ b/core/src/test/java/org/springframework/security/providers/preauth/PreAuthenticatedGrantedAuthoritiesUserDetailsServiceTests.java @@ -1,5 +1,6 @@ package org.springframework.security.providers.preauth; +import org.springframework.security.GrantedAuthoritiesContainer; import org.springframework.security.GrantedAuthorityImpl; import org.springframework.security.GrantedAuthority; import org.springframework.security.userdetails.UserDetails; @@ -53,8 +54,8 @@ public class PreAuthenticatedGrantedAuthoritiesUserDetailsServiceTests extends T private void testGetUserDetails(final String userName, final GrantedAuthority[] gas) { PreAuthenticatedGrantedAuthoritiesUserDetailsService svc = new PreAuthenticatedGrantedAuthoritiesUserDetailsService(); PreAuthenticatedAuthenticationToken token = new PreAuthenticatedAuthenticationToken(userName, "dummy"); - token.setDetails(new PreAuthenticatedGrantedAuthoritiesRetriever() { - public GrantedAuthority[] getPreAuthenticatedGrantedAuthorities() { + token.setDetails(new GrantedAuthoritiesContainer() { + public GrantedAuthority[] getGrantedAuthorities() { return gas; } }); diff --git a/core/src/test/java/org/springframework/security/ui/preauth/PreAuthenticatedGrantedAuthoritiesWebAuthenticationDetailsTests.java b/core/src/test/java/org/springframework/security/ui/preauth/PreAuthenticatedGrantedAuthoritiesWebAuthenticationDetailsTests.java index 44bb408d02..23e437459e 100755 --- a/core/src/test/java/org/springframework/security/ui/preauth/PreAuthenticatedGrantedAuthoritiesWebAuthenticationDetailsTests.java +++ b/core/src/test/java/org/springframework/security/ui/preauth/PreAuthenticatedGrantedAuthoritiesWebAuthenticationDetailsTests.java @@ -15,9 +15,7 @@ import junit.framework.TestCase; import org.springframework.mock.web.MockHttpServletRequest; /** - * * @author TSARDD - * @since 18-okt-2007 */ public class PreAuthenticatedGrantedAuthoritiesWebAuthenticationDetailsTests extends TestCase { @@ -25,7 +23,7 @@ public class PreAuthenticatedGrantedAuthoritiesWebAuthenticationDetailsTests ext PreAuthenticatedGrantedAuthoritiesWebAuthenticationDetails details = new PreAuthenticatedGrantedAuthoritiesWebAuthenticationDetails( getRequest("testUser", new String[] {})); GrantedAuthority[] gas = new GrantedAuthority[] { new GrantedAuthorityImpl("Role1"), new GrantedAuthorityImpl("Role2") }; - details.setPreAuthenticatedGrantedAuthorities(gas); + details.setGrantedAuthorities(gas); String toString = details.toString(); assertTrue("toString should contain Role1", toString.contains("Role1")); assertTrue("toString should contain Role2", toString.contains("Role2")); @@ -37,18 +35,17 @@ public class PreAuthenticatedGrantedAuthoritiesWebAuthenticationDetailsTests ext GrantedAuthority[] gas = new GrantedAuthority[] { new GrantedAuthorityImpl("Role1"), new GrantedAuthorityImpl("Role2") }; Collection expectedGas = Arrays.asList(gas); - details.setPreAuthenticatedGrantedAuthorities(gas); - Collection returnedGas = Arrays.asList(details.getPreAuthenticatedGrantedAuthorities()); - assertTrue("Collections do not contain same elements; expected: " + expectedGas + ", returned: " + returnedGas, expectedGas - .containsAll(returnedGas) - && returnedGas.containsAll(expectedGas)); + details.setGrantedAuthorities(gas); + Collection returnedGas = Arrays.asList(details.getGrantedAuthorities()); + assertTrue("Collections do not contain same elements; expected: " + expectedGas + ", returned: " + returnedGas, + expectedGas.containsAll(returnedGas) && returnedGas.containsAll(expectedGas)); } public final void testGetWithoutSetPreAuthenticatedGrantedAuthorities() { PreAuthenticatedGrantedAuthoritiesWebAuthenticationDetails details = new PreAuthenticatedGrantedAuthoritiesWebAuthenticationDetails( getRequest("testUser", new String[] {})); try { - GrantedAuthority[] gas = details.getPreAuthenticatedGrantedAuthorities(); + GrantedAuthority[] gas = details.getGrantedAuthorities(); fail("Expected exception didn't occur"); } catch (IllegalArgumentException expected) { } catch (Exception unexpected) { diff --git a/core/src/test/java/org/springframework/security/ui/preauth/j2ee/J2eeBasedPreAuthenticatedWebAuthenticationDetailsSourceTests.java b/core/src/test/java/org/springframework/security/ui/preauth/j2ee/J2eeBasedPreAuthenticatedWebAuthenticationDetailsSourceTests.java index 1d6efa3ad8..d3c359c555 100755 --- a/core/src/test/java/org/springframework/security/ui/preauth/j2ee/J2eeBasedPreAuthenticatedWebAuthenticationDetailsSourceTests.java +++ b/core/src/test/java/org/springframework/security/ui/preauth/j2ee/J2eeBasedPreAuthenticatedWebAuthenticationDetailsSourceTests.java @@ -21,7 +21,6 @@ import org.springframework.mock.web.MockHttpServletRequest; /** * * @author TSARDD - * @since 18-okt-2007 */ public class J2eeBasedPreAuthenticatedWebAuthenticationDetailsSourceTests extends TestCase { @@ -92,7 +91,7 @@ public class J2eeBasedPreAuthenticatedWebAuthenticationDetailsSourceTests extend assertTrue("Returned object not of type PreAuthenticatedGrantedAuthoritiesWebAuthenticationDetails, actual type: " + o.getClass(), o instanceof PreAuthenticatedGrantedAuthoritiesWebAuthenticationDetails); PreAuthenticatedGrantedAuthoritiesWebAuthenticationDetails details = (PreAuthenticatedGrantedAuthoritiesWebAuthenticationDetails) o; - GrantedAuthority[] gas = details.getPreAuthenticatedGrantedAuthorities(); + GrantedAuthority[] gas = details.getGrantedAuthorities(); assertNotNull("Granted authorities should not be null", gas); assertTrue("Number of granted authorities should be " + expectedRoles.length, gas.length == expectedRoles.length);