From c3d216e7bb812c8276fe50d172bf5ea8a239bca0 Mon Sep 17 00:00:00 2001 From: Luke Taylor Date: Sat, 6 Dec 2008 17:31:53 +0000 Subject: [PATCH] SEC-1012: Minor improvements to SecurityContextHolderAwareRequestFilter and conversion to use jmock for test. --- ...curityContextHolderAwareRequestFilter.java | 44 ++++++----- ...yContextHolderAwareRequestFilterTests.java | 73 +++++++------------ 2 files changed, 46 insertions(+), 71 deletions(-) diff --git a/core/src/main/java/org/springframework/security/wrapper/SecurityContextHolderAwareRequestFilter.java b/core/src/main/java/org/springframework/security/wrapper/SecurityContextHolderAwareRequestFilter.java index fd525fcec1..c185806502 100644 --- a/core/src/main/java/org/springframework/security/wrapper/SecurityContextHolderAwareRequestFilter.java +++ b/core/src/main/java/org/springframework/security/wrapper/SecurityContextHolderAwareRequestFilter.java @@ -32,13 +32,15 @@ import org.springframework.util.ReflectionUtils; /** - * A Filter which populates the ServletRequest with a new request wrapper.

Several - * request wrappers are included with the framework. The simplest version is {@link - * SecurityContextHolderAwareRequestWrapper}. A more complex and powerful request wrapper is {@link - * org.springframework.security.wrapper.SavedRequestAwareWrapper}. The latter is also the default.

- *

To modify the wrapper used, call {@link #setWrapperClass(Class)}.

- *

Any request wrapper configured for instantiation by this class must provide a public constructor that - * accepts two arguments, being a HttpServletRequest and a PortResolver.

+ * A Filter which populates the ServletRequest with a new request wrapper. + * Several request wrappers are included with the framework. The simplest version is {@link + * SecurityContextHolderAwareRequestWrapper}. A more complex and powerful request wrapper is + * {@link SavedRequestAwareWrapper}. The latter is also the default. + *

+ * To modify the wrapper used, call {@link #setWrapperClass(Class)}. + *

+ * Any request wrapper configured for instantiation by this class must provide a public constructor that + * accepts two arguments, being a HttpServletRequest and a PortResolver. * * @author Orlando Garcia Carmona * @author Ben Alex @@ -47,8 +49,8 @@ import org.springframework.util.ReflectionUtils; public class SecurityContextHolderAwareRequestFilter extends SpringSecurityFilter { //~ Instance fields ================================================================================================ - private Class wrapperClass = SavedRequestAwareWrapper.class; - private Constructor constructor; + private Class wrapperClass = SavedRequestAwareWrapper.class; + private Constructor constructor; private PortResolver portResolver = new PortResolverImpl(); private String rolePrefix; @@ -59,6 +61,7 @@ public class SecurityContextHolderAwareRequestFilter extends SpringSecurityFilte this.portResolver = portResolver; } + @SuppressWarnings("unchecked") public void setWrapperClass(Class wrapperClass) { Assert.notNull(wrapperClass, "WrapperClass required"); Assert.isTrue(HttpServletRequest.class.isAssignableFrom(wrapperClass), "Wrapper must be a HttpServletRequest"); @@ -70,28 +73,23 @@ public class SecurityContextHolderAwareRequestFilter extends SpringSecurityFilte this.rolePrefix = rolePrefix.trim(); } - protected void doFilterHttp(HttpServletRequest request, HttpServletResponse response, FilterChain chain) throws IOException, ServletException { + protected void doFilterHttp(HttpServletRequest request, HttpServletResponse response, FilterChain chain) throws IOException, ServletException { if (!wrapperClass.isAssignableFrom(request.getClass())) { - if (constructor == null) { - try { - constructor = wrapperClass.getConstructor( - new Class[] {HttpServletRequest.class, PortResolver.class, String.class}); - } catch (Exception ex) { - ReflectionUtils.handleReflectionException(ex); + try { + if (constructor == null) { + constructor = wrapperClass.getConstructor(HttpServletRequest.class, PortResolver.class, String.class); } - } - try { - request = (HttpServletRequest) constructor.newInstance(new Object[] {request, portResolver, rolePrefix}); + request = constructor.newInstance(request, portResolver, rolePrefix); } catch (Exception ex) { ReflectionUtils.handleReflectionException(ex); } } chain.doFilter(request, response); - } + } - public int getOrder() { - return FilterChainOrder.SERVLET_API_SUPPORT_FILTER; - } + public int getOrder() { + return FilterChainOrder.SERVLET_API_SUPPORT_FILTER; + } } diff --git a/core/src/test/java/org/springframework/security/wrapper/SecurityContextHolderAwareRequestFilterTests.java b/core/src/test/java/org/springframework/security/wrapper/SecurityContextHolderAwareRequestFilterTests.java index 27cb2852b5..49009fc9b9 100644 --- a/core/src/test/java/org/springframework/security/wrapper/SecurityContextHolderAwareRequestFilterTests.java +++ b/core/src/test/java/org/springframework/security/wrapper/SecurityContextHolderAwareRequestFilterTests.java @@ -15,19 +15,17 @@ package org.springframework.security.wrapper; -import junit.framework.TestCase; - -import org.springframework.security.MockFilterConfig; +import javax.servlet.FilterChain; +import javax.servlet.FilterConfig; +import javax.servlet.http.HttpServletResponse; +import org.jmock.Expectations; +import org.jmock.Mockery; +import org.jmock.integration.junit4.JUnit4Mockery; +import org.junit.Test; import org.springframework.mock.web.MockHttpServletRequest; import org.springframework.mock.web.MockHttpServletResponse; - -import java.io.IOException; - -import javax.servlet.FilterChain; -import javax.servlet.ServletException; -import javax.servlet.ServletRequest; -import javax.servlet.ServletResponse; +import org.springframework.security.util.PortResolverImpl; /** @@ -36,51 +34,30 @@ import javax.servlet.ServletResponse; * @author Ben Alex * @version $Id$ */ -public class SecurityContextHolderAwareRequestFilterTests extends TestCase { - //~ Constructors =================================================================================================== - - public SecurityContextHolderAwareRequestFilterTests() { - } - - public SecurityContextHolderAwareRequestFilterTests(String arg0) { - super(arg0); - } +public class SecurityContextHolderAwareRequestFilterTests { + Mockery jmock = new JUnit4Mockery(); //~ Methods ======================================================================================================== - public final void setUp() throws Exception { - super.setUp(); - } - - public void testCorrectOperation() throws Exception { + @Test + public void expectedRequestWrapperClassIsUsed() throws Exception { SecurityContextHolderAwareRequestFilter filter = new SecurityContextHolderAwareRequestFilter(); - filter.init(new MockFilterConfig()); - filter.doFilter(new MockHttpServletRequest(null, null), new MockHttpServletResponse(), - new MockFilterChain(SavedRequestAwareWrapper.class)); + filter.setPortResolver(new PortResolverImpl()); + filter.setWrapperClass(SavedRequestAwareWrapper.class); + filter.setRolePrefix("ROLE_"); + filter.init(jmock.mock(FilterConfig.class)); + final FilterChain filterChain = jmock.mock(FilterChain.class); - // Now re-execute the filter, ensuring our replacement wrapper is still used - filter.doFilter(new MockHttpServletRequest(null, null), new MockHttpServletResponse(), - new MockFilterChain(SavedRequestAwareWrapper.class)); + jmock.checking(new Expectations() {{ + exactly(2).of(filterChain).doFilter( + with(aNonNull(SavedRequestAwareWrapper.class)), with(aNonNull(HttpServletResponse.class))); + }}); - filter.destroy(); - } + filter.doFilter(new MockHttpServletRequest(), new MockHttpServletResponse(), filterChain); - //~ Inner Classes ================================================================================================== - - private class MockFilterChain implements FilterChain { - private Class expectedServletRequest; - - public MockFilterChain(Class expectedServletRequest) { - this.expectedServletRequest = expectedServletRequest; - } + // Now re-execute the filter, ensuring our replacement wrapper is still used + filter.doFilter(new MockHttpServletRequest(), new MockHttpServletResponse(), filterChain); - public void doFilter(ServletRequest request, ServletResponse response) - throws IOException, ServletException { - if (request.getClass().isAssignableFrom(expectedServletRequest)) { - assertTrue(true); - } else { - fail("Expected class to be of type " + expectedServletRequest + " but was: " + request.getClass()); - } - } + filter.destroy(); } }