From c0f5230667f74cf386ad9708ce9565ffc3da20d3 Mon Sep 17 00:00:00 2001 From: Luke Taylor Date: Tue, 11 Sep 2007 17:29:47 +0000 Subject: [PATCH] SEC-302: Add rolePrefix property to SecurityContextHolderAwareRequestFilter. --- .../wrapper/SavedRequestAwareWrapper.java | 65 +++++-------------- ...curityContextHolderAwareRequestFilter.java | 11 +++- ...urityContextHolderAwareRequestWrapper.java | 19 +++++- ...yContextHolderAwareRequestFilterTests.java | 9 --- ...ContextHolderAwareRequestWrapperTests.java | 31 ++++++--- 5 files changed, 64 insertions(+), 71 deletions(-) diff --git a/core/src/main/java/org/acegisecurity/wrapper/SavedRequestAwareWrapper.java b/core/src/main/java/org/acegisecurity/wrapper/SavedRequestAwareWrapper.java index c541e9a0c0..d5a4567443 100644 --- a/core/src/main/java/org/acegisecurity/wrapper/SavedRequestAwareWrapper.java +++ b/core/src/main/java/org/acegisecurity/wrapper/SavedRequestAwareWrapper.java @@ -41,11 +41,16 @@ import javax.servlet.http.HttpSession; /** - * Provides request parameters, headers and cookies from either an original request or a saved request.

Note that - * not all request parameters in the original request are emulated by this wrapper. Nevertheless, the important data - * from the original request is emulated and this should prove adequate for most purposes (in particular standard HTTP - * GET and POST operations).

- *

Added into a request by {@link org.acegisecurity.wrapper.SecurityContextHolderAwareRequestFilter}.

+ * Provides request parameters, headers and cookies from either an original request or a saved request. + * + *

Note that not all request parameters in the original request are emulated by this wrapper. + * Nevertheless, the important data from the original request is emulated and this should prove + * adequate for most purposes (in particular standard HTTP GET and POST operations).

+ * + *

Added into a request by {@link org.acegisecurity.wrapper.SecurityContextHolderAwareRequestFilter}.

+ * + * + * @see SecurityContextHolderAwareRequestFilter * * @author Andrey Grebnev * @author Ben Alex @@ -72,8 +77,8 @@ public class SavedRequestAwareWrapper extends SecurityContextHolderAwareRequestW //~ Constructors =================================================================================================== - public SavedRequestAwareWrapper(HttpServletRequest request, PortResolver portResolver) { - super(request,portResolver); + public SavedRequestAwareWrapper(HttpServletRequest request, PortResolver portResolver, String rolePrefix) { + super(request, portResolver, rolePrefix); HttpSession session = request.getSession(false); @@ -113,8 +118,6 @@ public class SavedRequestAwareWrapper extends SecurityContextHolderAwareRequestW /** * The default behavior of this method is to return getCookies() on the wrapped request object. - * - * @return DOCUMENT ME! */ public Cookie[] getCookies() { if (savedRequest == null) { @@ -129,12 +132,6 @@ public class SavedRequestAwareWrapper extends SecurityContextHolderAwareRequestW /** * The default behavior of this method is to return getDateHeader(String name) on the wrapped request * object. - * - * @param name DOCUMENT ME! - * - * @return DOCUMENT ME! - * - * @throws IllegalArgumentException DOCUMENT ME! */ public long getDateHeader(String name) { if (savedRequest == null) { @@ -143,13 +140,13 @@ public class SavedRequestAwareWrapper extends SecurityContextHolderAwareRequestW String value = getHeader(name); if (value == null) { - return (-1L); + return -1L; } // Attempt to convert the date header in a variety of formats long result = FastHttpDateFormat.parseDate(value, formats); - if (result != (-1L)) { + if (result != -1L) { return result; } @@ -159,10 +156,6 @@ public class SavedRequestAwareWrapper extends SecurityContextHolderAwareRequestW /** * The default behavior of this method is to return getHeader(String name) on the wrapped request object. - * - * @param name DOCUMENT ME! - * - * @return DOCUMENT ME! */ public String getHeader(String name) { if (savedRequest == null) { @@ -183,8 +176,6 @@ public class SavedRequestAwareWrapper extends SecurityContextHolderAwareRequestW /** * The default behavior of this method is to return getHeaderNames() on the wrapped request object. - * - * @return DOCUMENT ME! */ public Enumeration getHeaderNames() { if (savedRequest == null) { @@ -196,10 +187,6 @@ public class SavedRequestAwareWrapper extends SecurityContextHolderAwareRequestW /** * The default behavior of this method is to return getHeaders(String name) on the wrapped request object. - * - * @param name DOCUMENT ME! - * - * @return DOCUMENT ME! */ public Enumeration getHeaders(String name) { if (savedRequest == null) { @@ -212,10 +199,6 @@ public class SavedRequestAwareWrapper extends SecurityContextHolderAwareRequestW /** * The default behavior of this method is to return getIntHeader(String name) on the wrapped request * object. - * - * @param name DOCUMENT ME! - * - * @return DOCUMENT ME! */ public int getIntHeader(String name) { if (savedRequest == null) { @@ -224,17 +207,15 @@ public class SavedRequestAwareWrapper extends SecurityContextHolderAwareRequestW String value = getHeader(name); if (value == null) { - return (-1); + return -1; } else { - return (Integer.parseInt(value)); + return Integer.parseInt(value); } } } /** * The default behavior of this method is to return getLocale() on the wrapped request object. - * - * @return DOCUMENT ME! */ public Locale getLocale() { if (savedRequest == null) { @@ -260,7 +241,6 @@ public class SavedRequestAwareWrapper extends SecurityContextHolderAwareRequestW /** * The default behavior of this method is to return getLocales() on the wrapped request object. * - * @return DOCUMENT ME! */ public Enumeration getLocales() { if (savedRequest == null) { @@ -282,7 +262,6 @@ public class SavedRequestAwareWrapper extends SecurityContextHolderAwareRequestW /** * The default behavior of this method is to return getMethod() on the wrapped request object. * - * @return DOCUMENT ME! */ public String getMethod() { if (savedRequest == null) { @@ -295,10 +274,6 @@ public class SavedRequestAwareWrapper extends SecurityContextHolderAwareRequestW /** * The default behavior of this method is to return getParameter(String name) on the wrapped request * object. - * - * @param name DOCUMENT ME! - * - * @return DOCUMENT ME! */ public String getParameter(String name) { /* @@ -342,8 +317,6 @@ public class SavedRequestAwareWrapper extends SecurityContextHolderAwareRequestW /** * The default behavior of this method is to return getParameterMap() on the wrapped request object. - * - * @return DOCUMENT ME! */ public Map getParameterMap() { if (savedRequest == null) { @@ -355,8 +328,6 @@ public class SavedRequestAwareWrapper extends SecurityContextHolderAwareRequestW /** * The default behavior of this method is to return getParameterNames() on the wrapped request object. - * - * @return DOCUMENT ME! */ public Enumeration getParameterNames() { if (savedRequest == null) { @@ -369,10 +340,6 @@ public class SavedRequestAwareWrapper extends SecurityContextHolderAwareRequestW /** * The default behavior of this method is to return getParameterValues(String name) on the wrapped request * object. - * - * @param name DOCUMENT ME! - * - * @return DOCUMENT ME! */ public String[] getParameterValues(String name) { if (savedRequest == null) { diff --git a/core/src/main/java/org/acegisecurity/wrapper/SecurityContextHolderAwareRequestFilter.java b/core/src/main/java/org/acegisecurity/wrapper/SecurityContextHolderAwareRequestFilter.java index 96d1edce1d..e62c2c5037 100644 --- a/core/src/main/java/org/acegisecurity/wrapper/SecurityContextHolderAwareRequestFilter.java +++ b/core/src/main/java/org/acegisecurity/wrapper/SecurityContextHolderAwareRequestFilter.java @@ -53,6 +53,7 @@ public class SecurityContextHolderAwareRequestFilter implements Filter { private Class wrapperClass = SavedRequestAwareWrapper.class; private Constructor constructor; private PortResolver portResolver = new PortResolverImpl(); + private String rolePrefix; //~ Methods ======================================================================================================== @@ -65,14 +66,15 @@ public class SecurityContextHolderAwareRequestFilter implements Filter { if (!wrapperClass.isAssignableFrom(request.getClass())) { if (constructor == null) { try { - constructor = wrapperClass.getConstructor(new Class[] {HttpServletRequest.class, PortResolver.class}); + constructor = wrapperClass.getConstructor( + new Class[] {HttpServletRequest.class, PortResolver.class, String.class}); } catch (Exception ex) { ReflectionUtils.handleReflectionException(ex); } } try { - request = (HttpServletRequest) constructor.newInstance(new Object[] {request, portResolver}); + request = (HttpServletRequest) constructor.newInstance(new Object[] {request, portResolver, rolePrefix}); } catch (Exception ex) { ReflectionUtils.handleReflectionException(ex); } @@ -93,4 +95,9 @@ public class SecurityContextHolderAwareRequestFilter implements Filter { Assert.isTrue(HttpServletRequest.class.isAssignableFrom(wrapperClass), "Wrapper must be a HttpServletRequest"); this.wrapperClass = wrapperClass; } + + public void setRolePrefix(String rolePrefix) { + Assert.notNull(rolePrefix, "Role prefix must not be null"); + this.rolePrefix = rolePrefix.trim(); + } } diff --git a/core/src/main/java/org/acegisecurity/wrapper/SecurityContextHolderAwareRequestWrapper.java b/core/src/main/java/org/acegisecurity/wrapper/SecurityContextHolderAwareRequestWrapper.java index e5ec1f1b96..9fbc169b75 100644 --- a/core/src/main/java/org/acegisecurity/wrapper/SecurityContextHolderAwareRequestWrapper.java +++ b/core/src/main/java/org/acegisecurity/wrapper/SecurityContextHolderAwareRequestWrapper.java @@ -36,6 +36,8 @@ import javax.servlet.http.HttpServletRequestWrapper; * SecurityContextHolderAwareRequestWrapper#isUserInRole(java.lang.String)} and {@link * javax.servlet.http.HttpServletRequestWrapper#getRemoteUser()} responses. * + * @see SecurityContextHolderAwareRequestFilter + * * @author Orlando Garcia Carmona * @author Ben Alex * @version $Id$ @@ -45,10 +47,21 @@ public class SecurityContextHolderAwareRequestWrapper extends HttpServletRequest private AuthenticationTrustResolver authenticationTrustResolver = new AuthenticationTrustResolverImpl(); + /** + * The prefix passed by the filter. It will be prepended to any supplied role values before + * comparing it with the roles obtained from the security context. + */ + private String rolePrefix; + //~ Constructors =================================================================================================== - public SecurityContextHolderAwareRequestWrapper(HttpServletRequest request, PortResolver portResolver) { + public SecurityContextHolderAwareRequestWrapper( + HttpServletRequest request, + PortResolver portResolver, + String rolePrefix) { super(request); + + this.rolePrefix = rolePrefix; } //~ Methods ======================================================================================================== @@ -107,6 +120,10 @@ public class SecurityContextHolderAwareRequestWrapper extends HttpServletRequest private boolean isGranted(String role) { Authentication auth = getAuthentication(); + if( rolePrefix != null ) { + role = rolePrefix + role; + } + if ((auth == null) || (auth.getPrincipal() == null) || (auth.getAuthorities() == null)) { return false; } diff --git a/core/src/test/java/org/acegisecurity/wrapper/SecurityContextHolderAwareRequestFilterTests.java b/core/src/test/java/org/acegisecurity/wrapper/SecurityContextHolderAwareRequestFilterTests.java index 5cb2c3d8c5..902b834b7c 100644 --- a/core/src/test/java/org/acegisecurity/wrapper/SecurityContextHolderAwareRequestFilterTests.java +++ b/core/src/test/java/org/acegisecurity/wrapper/SecurityContextHolderAwareRequestFilterTests.java @@ -39,7 +39,6 @@ public class SecurityContextHolderAwareRequestFilterTests extends TestCase { //~ Constructors =================================================================================================== public SecurityContextHolderAwareRequestFilterTests() { - super(); } public SecurityContextHolderAwareRequestFilterTests(String arg0) { @@ -48,10 +47,6 @@ public class SecurityContextHolderAwareRequestFilterTests extends TestCase { //~ Methods ======================================================================================================== - public static void main(String[] args) { - junit.textui.TestRunner.run(SecurityContextHolderAwareRequestFilterTests.class); - } - public final void setUp() throws Exception { super.setUp(); } @@ -78,10 +73,6 @@ public class SecurityContextHolderAwareRequestFilterTests extends TestCase { this.expectedServletRequest = expectedServletRequest; } - private MockFilterChain() { - super(); - } - public void doFilter(ServletRequest request, ServletResponse response) throws IOException, ServletException { if (request.getClass().isAssignableFrom(expectedServletRequest)) { diff --git a/core/src/test/java/org/acegisecurity/wrapper/SecurityContextHolderAwareRequestWrapperTests.java b/core/src/test/java/org/acegisecurity/wrapper/SecurityContextHolderAwareRequestWrapperTests.java index e83899d4be..180b714c44 100644 --- a/core/src/test/java/org/acegisecurity/wrapper/SecurityContextHolderAwareRequestWrapperTests.java +++ b/core/src/test/java/org/acegisecurity/wrapper/SecurityContextHolderAwareRequestWrapperTests.java @@ -50,8 +50,7 @@ public class SecurityContextHolderAwareRequestWrapperTests extends TestCase { SecurityContextHolder.clearContext(); } - public void testCorrectOperationWithStringBasedPrincipal() - throws Exception { + public void testCorrectOperationWithStringBasedPrincipal() throws Exception { Authentication auth = new TestingAuthenticationToken("marissa", "koala", new GrantedAuthority[] {new GrantedAuthorityImpl("ROLE_FOO")}); SecurityContextHolder.getContext().setAuthentication(auth); @@ -59,7 +58,7 @@ public class SecurityContextHolderAwareRequestWrapperTests extends TestCase { MockHttpServletRequest request = new MockHttpServletRequest(); request.setRequestURI("/"); - SecurityContextHolderAwareRequestWrapper wrapper = new SecurityContextHolderAwareRequestWrapper(request, new PortResolverImpl()); + SecurityContextHolderAwareRequestWrapper wrapper = new SecurityContextHolderAwareRequestWrapper(request, new PortResolverImpl(), ""); assertEquals("marissa", wrapper.getRemoteUser()); assertTrue(wrapper.isUserInRole("ROLE_FOO")); @@ -67,8 +66,20 @@ public class SecurityContextHolderAwareRequestWrapperTests extends TestCase { assertEquals(auth, wrapper.getUserPrincipal()); } - public void testCorrectOperationWithUserDetailsBasedPrincipal() - throws Exception { + public void testUseOfRolePrefixMeansItIsntNeededWhenCallngIsUserInRole() { + Authentication auth = new TestingAuthenticationToken("marissa", "koala", + new GrantedAuthority[] {new GrantedAuthorityImpl("ROLE_FOO")}); + SecurityContextHolder.getContext().setAuthentication(auth); + + MockHttpServletRequest request = new MockHttpServletRequest(); + request.setRequestURI("/"); + + SecurityContextHolderAwareRequestWrapper wrapper = new SecurityContextHolderAwareRequestWrapper(request, new PortResolverImpl(), "ROLE_"); + + assertTrue(wrapper.isUserInRole("FOO")); + } + + public void testCorrectOperationWithUserDetailsBasedPrincipal() throws Exception { Authentication auth = new TestingAuthenticationToken(new User("marissaAsUserDetails", "koala", true, true, true, true, new GrantedAuthority[] {}), "koala", new GrantedAuthority[] {new GrantedAuthorityImpl("ROLE_HELLO"), new GrantedAuthorityImpl("ROLE_FOOBAR")}); @@ -77,7 +88,7 @@ public class SecurityContextHolderAwareRequestWrapperTests extends TestCase { MockHttpServletRequest request = new MockHttpServletRequest(); request.setRequestURI("/"); - SecurityContextHolderAwareRequestWrapper wrapper = new SecurityContextHolderAwareRequestWrapper(request, new PortResolverImpl()); + SecurityContextHolderAwareRequestWrapper wrapper = new SecurityContextHolderAwareRequestWrapper(request, new PortResolverImpl(), ""); assertEquals("marissaAsUserDetails", wrapper.getRemoteUser()); assertFalse(wrapper.isUserInRole("ROLE_FOO")); @@ -87,19 +98,19 @@ public class SecurityContextHolderAwareRequestWrapperTests extends TestCase { assertEquals(auth, wrapper.getUserPrincipal()); } - public void testNullAuthenticationHandling() throws Exception { + public void testRoleIsntHeldIfAuthenticationIsNull() throws Exception { SecurityContextHolder.getContext().setAuthentication(null); MockHttpServletRequest request = new MockHttpServletRequest(); request.setRequestURI("/"); - SecurityContextHolderAwareRequestWrapper wrapper = new SecurityContextHolderAwareRequestWrapper(request,new PortResolverImpl()); + SecurityContextHolderAwareRequestWrapper wrapper = new SecurityContextHolderAwareRequestWrapper(request,new PortResolverImpl(), ""); assertNull(wrapper.getRemoteUser()); assertFalse(wrapper.isUserInRole("ROLE_ANY")); assertNull(wrapper.getUserPrincipal()); } - public void testNullPrincipalHandling() throws Exception { + public void testRolesArentHeldIfAuthenticationPrincipalIsNull() throws Exception { Authentication auth = new TestingAuthenticationToken(null, "koala", new GrantedAuthority[] {new GrantedAuthorityImpl("ROLE_HELLO"), new GrantedAuthorityImpl("ROLE_FOOBAR")}); SecurityContextHolder.getContext().setAuthentication(auth); @@ -107,7 +118,7 @@ public class SecurityContextHolderAwareRequestWrapperTests extends TestCase { MockHttpServletRequest request = new MockHttpServletRequest(); request.setRequestURI("/"); - SecurityContextHolderAwareRequestWrapper wrapper = new SecurityContextHolderAwareRequestWrapper(request, new PortResolverImpl()); + SecurityContextHolderAwareRequestWrapper wrapper = new SecurityContextHolderAwareRequestWrapper(request, new PortResolverImpl(), ""); assertNull(wrapper.getRemoteUser()); assertFalse(wrapper.isUserInRole("ROLE_HELLO")); // principal is null, so reject