- Created HeaderFactory abstraction

- Implemented different ALLOW-FROM strategies as specified in the proposal. Conflicts: config/src/main/java/org/springframework/security/config/http/HeadersBeanDefinitionParser.java config/src/test/groovy/org/springframework/security/config/http/HttpHeadersConfigTests.groovy
2026-08-03 00:37:03 +00:00 · 2013-02-22 20:43:29 +01:00
parent a63baa8391
commit d0b40cd2ae
21 changed files with 740 additions and 67 deletions
@@ -0,0 +1,37 @@
+package org.springframework.security.web.headers;
+
+import java.util.Arrays;
+
+/**
+ * Created with IntelliJ IDEA.
+ * User: marten
+ * Date: 29-01-13
+ * Time: 20:26
+ * To change this template use File | Settings | File Templates.
+ */
+public final class Header {
+
+    private final String name;
+    private final String[] values;
+
+    public Header(String name, String... values) {
+        this.name = name;
+        this.values = values;
+    }
+
+    public String getName() {
+        return this.name;
+    }
+
+    public String[] getValues() {
+        return this.values;
+    }
+
+    public int hashCode() {
+        return name.hashCode() + Arrays.hashCode(values);
+    }
+
+    public String toString() {
+        return "Header [name: " + name + ", values: " + Arrays.toString(values)+"]";
+    }
+}
@@ -0,0 +1,23 @@
+package org.springframework.security.web.headers;
+
+import javax.servlet.http.HttpServletRequest;
+import javax.servlet.http.HttpServletResponse;
+
+/**
+ * Contract for a factory that creates {@code Header} instances.
+ *
+ * @author Marten Deinum
+ * @since 3.2
+ * @see HeadersFilter
+ */
+public interface HeaderFactory {
+
+    /**
+     * Create a {@code Header} instance.
+     *
+     * @param request the request
+     * @param response the response
+     * @return the created Header or <code>null</code>
+     */
+    Header create(HttpServletRequest request, HttpServletResponse response);
+}
@@ -22,8 +22,7 @@ import javax.servlet.ServletException;
 import javax.servlet.http.HttpServletRequest;
 import javax.servlet.http.HttpServletResponse;
 import java.io.IOException;
-import java.util.HashMap;
-import java.util.Map;
+import java.util.*;

 /**
 * Filter implementation to add headers to the current request. Can be useful to add certain headers which enable
@@ -35,28 +34,41 @@ import java.util.Map;
 */
 public class HeadersFilter extends OncePerRequestFilter {

-    /** Map of headers to add to a response */
-    private final Map<String, String> headers = new HashMap<String, String>();
+    /** Collection of HeaderFactory instances to produce Headers. */
+    private final List<HeaderFactory> factories;
+
+    public HeadersFilter(List<HeaderFactory> factories) {
+        this.factories = factories;
+    }
+

    @Override
    protected void doFilterInternal(HttpServletRequest request, HttpServletResponse response, FilterChain filterChain) throws ServletException, IOException {
-        for (Map.Entry<String, String> header : headers.entrySet()) {
-            String name = header.getKey();
-            String value = header.getValue();
-            if (logger.isTraceEnabled()) {
-                logger.trace("Adding header '" + name + "' with value '"+value +"'");
+
+        for (HeaderFactory factory : factories) {
+            Header header = factory.create(request, response);
+            if (header != null) {
+                String name = header.getName();
+                String[] values = header.getValues();
+                boolean first = true;
+                for (String value : values) {
+                    if (logger.isDebugEnabled()) {
+                        logger.debug("Adding header '" + name + "' with value '"+value +"'");
+                    }
+                    if (first) {
+                        response.setHeader(name, value);
+                        first = false;
+                    } else {
+                        response.addHeader(name, value);
+                    }
+                }
+            } else {
+                if (logger.isDebugEnabled()) {
+                    logger.debug("Factory produced no header.");
+                }
            }
-            response.setHeader(header.getKey(), header.getValue());
        }
        filterChain.doFilter(request, response);
    }

-    public void setHeaders(Map<String, String> headers) {
-        this.headers.clear();
-        this.headers.putAll(headers);
-    }
-
-    public void addHeader(String name, String value) {
-        headers.put(name, value);
-    }
 }
@@ -0,0 +1,23 @@
+package org.springframework.security.web.headers;
+
+import javax.servlet.http.HttpServletRequest;
+import javax.servlet.http.HttpServletResponse;
+
+/**
+ * {@code HeaderFactory} implementation which returns the same {@code Header} instance.
+ *
+ * @author Marten Deinum
+ * @since 3.2
+ */
+public class StaticHeaderFactory implements HeaderFactory {
+
+    private final Header header;
+
+    public StaticHeaderFactory(String name, String... values) {
+        header = new Header(name, values);
+    }
+
+    public Header create(HttpServletRequest request, HttpServletResponse response) {
+        return header;
+    }
+}
@@ -0,0 +1,15 @@
+package org.springframework.security.web.headers.frameoptions;
+
+import javax.servlet.http.HttpServletRequest;
+
+/**
+ * Strategy interfaces used by the {@code FrameOptionsHeaderFactory} to determine the actual value to use for the
+ * X-Frame-Options header when using the ALLOW-FROM directive.
+ *
+ * @author Marten Deinum
+ * @since 3.2
+ */
+public interface AllowFromStrategy {
+
+    String apply(HttpServletRequest request);
+}
@@ -0,0 +1,46 @@
+package org.springframework.security.web.headers.frameoptions;
+
+import org.springframework.security.web.headers.Header;
+import org.springframework.security.web.headers.HeaderFactory;
+
+import javax.servlet.http.HttpServletRequest;
+import javax.servlet.http.HttpServletResponse;
+
+/**
+ * {@code HeaderFactory} implementation for the X-Frame-Options headers. When using the ALLOW-FROM directive the actual
+ * value is determined by a {@code AllowFromStrategy}.
+ *
+ * @author Marten Deinum
+ * @since 3.2
+ *
+ * @see AllowFromStrategy
+ */
+public class FrameOptionsHeaderFactory implements HeaderFactory {
+
+    public static final String FRAME_OPTIONS_HEADER = "X-Frame-Options";
+
+    private static final String ALLOW_FROM = "ALLOW-FROM";
+
+    private final AllowFromStrategy allowFromStrategy;
+    private final String mode;
+
+    public FrameOptionsHeaderFactory(String mode) {
+        this(mode, new NullAllowFromStrategy());
+    }
+
+    public FrameOptionsHeaderFactory(String mode, AllowFromStrategy allowFromStrategy) {
+        this.mode=mode;
+        this.allowFromStrategy=allowFromStrategy;
+    }
+
+    @Override
+    public Header create(HttpServletRequest request, HttpServletResponse response) {
+        if (ALLOW_FROM.equals(mode)) {
+            String value = allowFromStrategy.apply(request);
+            return new Header(FRAME_OPTIONS_HEADER, value);
+        } else {
+            return new Header(FRAME_OPTIONS_HEADER, mode);
+        }
+    }
+
+}
@@ -0,0 +1,17 @@
+package org.springframework.security.web.headers.frameoptions;
+
+import javax.servlet.http.HttpServletRequest;
+
+/**
+ * Created with IntelliJ IDEA.
+ * User: marten
+ * Date: 30-01-13
+ * Time: 11:06
+ * To change this template use File | Settings | File Templates.
+ */
+public class NullAllowFromStrategy implements AllowFromStrategy {
+    @Override
+    public String apply(HttpServletRequest request) {
+        return null;
+    }
+}
@@ -0,0 +1,26 @@
+package org.springframework.security.web.headers.frameoptions;
+
+import org.springframework.util.Assert;
+
+import java.util.regex.Pattern;
+
+/**
+ * Implementation which uses a regular expression to validate the supplied origin.
+ *
+ * @author Marten Deinum
+ * @since 3.2
+ */
+public class RegExpAllowFromStrategy extends RequestParameterAllowFromStrategy {
+
+    private final Pattern pattern;
+
+    public RegExpAllowFromStrategy(String pattern) {
+        Assert.hasText(pattern, "Pattern cannot be empty.");
+        this.pattern = Pattern.compile(pattern);
+    }
+
+    @Override
+    protected boolean allowed(String from) {
+        return pattern.matcher(from).matches();
+    }
+}
@@ -0,0 +1,51 @@
+package org.springframework.security.web.headers.frameoptions;
+
+import org.apache.commons.logging.Log;
+import org.apache.commons.logging.LogFactory;
+import org.springframework.util.StringUtils;
+
+import javax.servlet.http.HttpServletRequest;
+
+/**
+ * Base class for AllowFromStrategy implementations which use a request parameter to retrieve the origin. By default
+ * the parameter named <code>from</code> is read from the request.
+ *
+ * @author Marten Deinum
+ * @since 3.2
+ */
+public abstract class RequestParameterAllowFromStrategy implements AllowFromStrategy {
+
+
+    private static final String DEFAULT_ORIGIN_REQUEST_PARAMETER = "from";
+
+    private String parameter = DEFAULT_ORIGIN_REQUEST_PARAMETER;
+
+    /** Logger for use by subclasses */
+    protected final Log log = LogFactory.getLog(getClass());
+
+
+    @Override
+    public String apply(HttpServletRequest request) {
+        String from = request.getParameter(parameter);
+        if (log.isDebugEnabled()) {
+            log.debug("Supplied origin '"+from+"'");
+        }
+        if (StringUtils.hasText(from) && allowed(from)) {
+            return "ALLOW-FROM " + from;
+        } else {
+            return "DENY";
+        }
+    }
+
+    public void setParameterName(String parameter) {
+        this.parameter=parameter;
+    }
+
+    /**
+     * Method to be implemented by base classes, used to determine if the supplied origin is allowed.
+     *
+     * @param from the supplied origin
+     * @return <code>true</code> if the supplied origin is allowed.
+     */
+    protected abstract boolean allowed(String from);
+}
@@ -0,0 +1,21 @@
+package org.springframework.security.web.headers.frameoptions;
+
+import javax.servlet.http.HttpServletRequest;
+import java.net.URI;
+
+/**
+ * Simple implementation of the {@code AllowFromStrategy}
+ */
+public class StaticAllowFromStrategy implements AllowFromStrategy {
+
+    private final URI uri;
+
+    public StaticAllowFromStrategy(URI uri) {
+        this.uri=uri;
+    }
+
+    @Override
+    public String apply(HttpServletRequest request) {
+        return uri.toString();
+    }
+}
@@ -0,0 +1,27 @@
+package org.springframework.security.web.headers.frameoptions;
+
+import org.springframework.util.Assert;
+
+import java.util.Collection;
+import java.util.List;
+
+/**
+ * Implementation which checks the supplied origin against a list of allowed origins.
+ *
+ * @author Marten Deinum
+ * @since 3.2
+ */
+public class WhiteListedAllowFromStrategy extends RequestParameterAllowFromStrategy {
+
+    private final Collection<String> allowed;
+
+    public WhiteListedAllowFromStrategy(Collection<String> allowed) {
+        Assert.notEmpty(allowed, "Allowed origins cannot be empty.");
+        this.allowed = allowed;
+    }
+
+    @Override
+    protected boolean allowed(String from) {
+        return allowed.contains(from);
+    }
+}
@@ -20,9 +20,9 @@ import org.springframework.mock.web.MockFilterChain;
 import org.springframework.mock.web.MockHttpServletRequest;
 import org.springframework.mock.web.MockHttpServletResponse;

-import java.util.Collection;
-import java.util.HashMap;
-import java.util.Map;
+import javax.servlet.http.HttpServletRequest;
+import javax.servlet.http.HttpServletResponse;
+import java.util.*;

 import static org.hamcrest.CoreMatchers.is;
 import static org.junit.Assert.assertThat;
@@ -39,7 +39,8 @@ public class HeadersFilterTest {

    @Test
    public void noHeadersConfigured() throws Exception {
-        HeadersFilter filter = new HeadersFilter();
+        List<HeaderFactory> factories = new ArrayList();
+        HeadersFilter filter = new HeadersFilter(factories);
        MockHttpServletRequest request = new MockHttpServletRequest();
        MockHttpServletResponse response = new MockHttpServletResponse();
        MockFilterChain filterChain = new MockFilterChain();
@@ -51,11 +52,18 @@ public class HeadersFilterTest {

    @Test
    public void additionalHeadersShouldBeAddedToTheResponse() throws Exception {
-        HeadersFilter filter = new HeadersFilter();
-        Map<String, String> headers = new HashMap<String, String>();
-        headers.put("X-Header1", "foo");
-        headers.put("X-Header2", "bar");
-        filter.setHeaders(headers);
+        List<HeaderFactory> factories = new ArrayList();
+        MockHeaderFactory factory1 = new MockHeaderFactory();
+        factory1.setName("X-Header1");
+        factory1.setValue("foo");
+        MockHeaderFactory factory2 = new MockHeaderFactory();
+        factory2.setName("X-Header2");
+        factory2.setValue("bar");
+
+        factories.add(factory1);
+        factories.add(factory2);
+
+        HeadersFilter filter = new HeadersFilter(factories);

        MockHttpServletRequest request = new MockHttpServletRequest();
        MockHttpServletResponse response = new MockHttpServletResponse();
@@ -70,4 +78,24 @@ public class HeadersFilterTest {
        assertThat(response.getHeader("X-Header2"), is("bar"));

    }
+
+    private static final class MockHeaderFactory implements HeaderFactory {
+
+        private String name;
+        private String value;
+
+        @Override
+        public Header create(HttpServletRequest request, HttpServletResponse response) {
+            return new Header(name, value);
+        }
+
+        public void setName(String name) {
+            this.name=name;
+        }
+
+        public void setValue(String value) {
+            this.value=value;
+        }
+
+    }
 }
@@ -0,0 +1,26 @@
+package org.springframework.security.web.headers;
+
+import org.junit.Test;
+
+import static org.hamcrest.CoreMatchers.is;
+import static org.junit.Assert.assertSame;
+import static org.springframework.test.util.MatcherAssertionErrors.assertThat;
+
+/**
+ * Test for the {@code StaticHeaderFactory}
+ *
+ * @author Marten Deinum
+ * @since 3.2
+ */
+public class StaticHeaderFactoryTest {
+
+    @Test
+    public void sameHeaderShouldBeReturned() {
+        StaticHeaderFactory factory = new StaticHeaderFactory("X-header", "foo");
+        Header header = factory.create(null, null);
+        assertThat(header.getName(), is("X-header"));
+        assertThat(header.getValues()[0], is("foo"));
+
+        assertSame(header, factory.create(null, null));
+    }
+}
@@ -0,0 +1,66 @@
+package org.springframework.security.web.headers.frameoptions;
+
+import org.junit.Test;
+import org.springframework.mock.web.MockHttpServletRequest;
+
+import java.util.regex.Pattern;
+import java.util.regex.PatternSyntaxException;
+
+import static org.hamcrest.CoreMatchers.is;
+import static org.junit.Assert.assertThat;
+import static org.junit.Assert.assertTrue;
+
+/**
+ * Created with IntelliJ IDEA.
+ * User: marten
+ * Date: 01-02-13
+ * Time: 20:25
+ * To change this template use File | Settings | File Templates.
+ */
+public class RegExpAllowFromStrategyTest {
+
+    @Test(expected = PatternSyntaxException.class)
+    public void invalidRegularExpressionShouldLeadToException() {
+        new RegExpAllowFromStrategy("[a-z");
+    }
+
+    @Test(expected = IllegalArgumentException.class)
+    public void nullRegularExpressionShouldLeadToException() {
+        new RegExpAllowFromStrategy(null);
+    }
+
+    @Test
+    public void subdomainMatchingRegularExpression() {
+        RegExpAllowFromStrategy strategy = new RegExpAllowFromStrategy("^http://([a-z0-9]*?\\.)test\\.com");
+        MockHttpServletRequest request = new MockHttpServletRequest();
+
+        request.setParameter("from", "http://abc.test.com");
+        String result1 = strategy.apply(request);
+        assertThat(result1, is("ALLOW-FROM http://abc.test.com"));
+
+        request.setParameter("from", "http://foo.test.com");
+        String result2 = strategy.apply(request);
+        assertThat(result2, is("ALLOW-FROM http://foo.test.com"));
+
+        request.setParameter("from", "http://test.foobar.com");
+        String result3 = strategy.apply(request);
+        assertThat(result3, is("DENY"));
+    }
+
+    @Test
+    public void noParameterShouldDeny() {
+        RegExpAllowFromStrategy strategy = new RegExpAllowFromStrategy("^http://([a-z0-9]*?\\.)test\\.com");
+        MockHttpServletRequest request = new MockHttpServletRequest();
+        String result1 = strategy.apply(request);
+        assertThat(result1, is("DENY"));
+    }
+
+    @Test
+    public void test() {
+        String pattern = "^http://([a-z0-9]*?\\.)test\\.com";
+        Pattern p = Pattern.compile(pattern);
+        String url = "http://abc.test.com";
+        assertTrue(p.matcher(url).matches());
+    }
+
+}
@@ -0,0 +1,24 @@
+package org.springframework.security.web.headers.frameoptions;
+
+import org.junit.Test;
+import org.springframework.mock.web.MockHttpServletRequest;
+
+import java.net.URI;
+
+import static org.junit.Assert.assertEquals;
+
+/**
+ * Test for the StaticAllowFromStrategy.
+ *
+ * @author Marten Deinum
+ * @since 3.2
+ */
+public class StaticAllowFromStrategyTest {
+
+    @Test
+    public void shouldReturnUri() {
+        String uri = "http://www.test.com";
+        StaticAllowFromStrategy strategy = new StaticAllowFromStrategy(URI.create(uri));
+        assertEquals(uri, strategy.apply(new MockHttpServletRequest()));
+    }
+}
@@ -0,0 +1,80 @@
+package org.springframework.security.web.headers.frameoptions;
+
+import org.junit.Test;
+import org.springframework.mock.web.MockHttpServletRequest;
+
+import java.util.ArrayList;
+import java.util.List;
+
+import static org.hamcrest.CoreMatchers.is;
+import static org.springframework.test.util.MatcherAssertionErrors.assertThat;
+
+/**
+ * Test for the {@code WhiteListedAllowFromStrategy}.
+ *
+ * @author Marten Deinum
+ * @since 3.2
+ */
+public class WhiteListedAllowFromStrategyTest {
+
+    @Test(expected = IllegalArgumentException.class)
+    public void emptyListShouldThrowException() {
+        new WhiteListedAllowFromStrategy(new ArrayList<String>());
+    }
+
+    @Test(expected = IllegalArgumentException.class)
+    public void nullListShouldThrowException() {
+        new WhiteListedAllowFromStrategy(null);
+    }
+
+    @Test
+    public void listWithSingleElementShouldMatch() {
+        List<String> allowed = new ArrayList<String>();
+        allowed.add("http://www.test.com");
+        WhiteListedAllowFromStrategy strategy = new WhiteListedAllowFromStrategy(allowed);
+        MockHttpServletRequest request = new MockHttpServletRequest();
+        request.setParameter("from", "http://www.test.com");
+
+        String result = strategy.apply(request);
+        assertThat(result, is("ALLOW-FROM http://www.test.com"));
+    }
+
+    @Test
+    public void listWithMultipleElementShouldMatch() {
+        List<String> allowed = new ArrayList<String>();
+        allowed.add("http://www.test.com");
+        allowed.add("http://www.springsource.org");
+        WhiteListedAllowFromStrategy strategy = new WhiteListedAllowFromStrategy(allowed);
+        MockHttpServletRequest request = new MockHttpServletRequest();
+        request.setParameter("from", "http://www.test.com");
+
+        String result = strategy.apply(request);
+        assertThat(result, is("ALLOW-FROM http://www.test.com"));
+    }
+
+    @Test
+    public void listWithSingleElementShouldNotMatch() {
+        List<String> allowed = new ArrayList<String>();
+        allowed.add("http://www.test.com");
+        WhiteListedAllowFromStrategy strategy = new WhiteListedAllowFromStrategy(allowed);
+        MockHttpServletRequest request = new MockHttpServletRequest();
+        request.setParameter("from", "http://www.test123.com");
+
+        String result = strategy.apply(request);
+        assertThat(result, is("DENY"));
+    }
+
+    @Test
+    public void requestWithoutParameterShouldNotMatch() {
+        List<String> allowed = new ArrayList<String>();
+        allowed.add("http://www.test.com");
+        WhiteListedAllowFromStrategy strategy = new WhiteListedAllowFromStrategy(allowed);
+        MockHttpServletRequest request = new MockHttpServletRequest();
+
+        String result = strategy.apply(request);
+        assertThat(result, is("DENY"));
+
+    }
+
+
+}