diff --git a/config/src/main/java/org/springframework/security/config/http/HeadersBeanDefinitionParser.java b/config/src/main/java/org/springframework/security/config/http/HeadersBeanDefinitionParser.java index b012187746..f2708527a8 100644 --- a/config/src/main/java/org/springframework/security/config/http/HeadersBeanDefinitionParser.java +++ b/config/src/main/java/org/springframework/security/config/http/HeadersBeanDefinitionParser.java @@ -16,17 +16,22 @@ package org.springframework.security.config.http; import org.springframework.beans.factory.config.BeanDefinition; +import org.springframework.beans.factory.config.RuntimeBeanReference; import org.springframework.beans.factory.support.BeanDefinitionBuilder; +import org.springframework.beans.factory.support.ManagedList; import org.springframework.beans.factory.xml.BeanDefinitionParser; import org.springframework.beans.factory.xml.ParserContext; import org.springframework.security.web.headers.HeadersFilter; +import org.springframework.security.web.headers.StaticHeaderFactory; +import org.springframework.security.web.headers.frameoptions.*; import org.springframework.util.StringUtils; import org.springframework.util.xml.DomUtils; import org.w3c.dom.Element; -import java.util.HashMap; +import java.net.URI; +import java.net.URISyntaxException; import java.util.List; -import java.util.Map; +import java.util.regex.PatternSyntaxException; /** * Parser for the {@code HeadersFilter}. @@ -40,10 +45,12 @@ public class HeadersBeanDefinitionParser implements BeanDefinitionParser { private static final String ATT_BLOCK = "block"; private static final String ATT_POLICY = "policy"; - private static final String ATT_ORIGIN = "origin"; + private static final String ATT_STRATEGY = "strategy"; + private static final String ATT_FROM_PARAMETER = "from-parameter"; private static final String ATT_NAME = "name"; private static final String ATT_VALUE = "value"; + private static final String ATT_REF = "ref"; private static final String XSS_ELEMENT = "xss-protection"; private static final String CONTENT_TYPE_ELEMENT = "content-type-options"; @@ -51,55 +58,107 @@ public class HeadersBeanDefinitionParser implements BeanDefinitionParser { private static final String GENERIC_HEADER_ELEMENT = "header"; private static final String XSS_PROTECTION_HEADER = "X-XSS-Protection"; - private static final String FRAME_OPTIONS_HEADER = "X-Frame-Options"; private static final String CONTENT_TYPE_OPTIONS_HEADER = "X-Content-Type-Options"; private static final String ALLOW_FROM = "ALLOW-FROM"; + private ManagedList headerFactories; + public BeanDefinition parse(Element element, ParserContext parserContext) { + headerFactories = new ManagedList(); BeanDefinitionBuilder builder = BeanDefinitionBuilder.rootBeanDefinition(HeadersFilter.class); - final Map headers = new HashMap(); - parseXssElement(element, parserContext, headers); - parseFrameOptionsElement(element, parserContext, headers); - parseContentTypeOptionsElement(element, headers); + parseXssElement(element, parserContext); + parseFrameOptionsElement(element, parserContext); + parseContentTypeOptionsElement(element); - parseHeaderElements(element, headers); + parseHeaderElements(element); - builder.addPropertyValue("headers", headers); + builder.addConstructorArgValue(headerFactories); return builder.getBeanDefinition(); } - private void parseHeaderElements(Element element, Map headers) { - List headerEtls = DomUtils.getChildElementsByTagName(element, GENERIC_HEADER_ELEMENT); - for (Element headerEtl : headerEtls) { - headers.put(headerEtl.getAttribute(ATT_NAME), headerEtl.getAttribute(ATT_VALUE)); + private void parseHeaderElements(Element element) { + List headerElts = DomUtils.getChildElementsByTagName(element, GENERIC_HEADER_ELEMENT); + for (Element headerElt : headerElts) { + String headerFactoryRef = headerElt.getAttribute(ATT_REF); + if (StringUtils.hasText(headerFactoryRef)) { + headerFactories.add(new RuntimeBeanReference(headerFactoryRef)); + } else { + BeanDefinitionBuilder builder = BeanDefinitionBuilder.genericBeanDefinition(StaticHeaderFactory.class); + builder.addConstructorArgValue(headerElt.getAttribute(ATT_NAME)); + builder.addConstructorArgValue(headerElt.getAttribute(ATT_VALUE)); + headerFactories.add(builder.getBeanDefinition()); + } } } - private void parseContentTypeOptionsElement(Element element, Map headers) { + private void parseContentTypeOptionsElement(Element element) { Element contentTypeElt = DomUtils.getChildElementByTagName(element, CONTENT_TYPE_ELEMENT); if (contentTypeElt != null) { - headers.put(CONTENT_TYPE_OPTIONS_HEADER, "nosniff"); + BeanDefinitionBuilder builder = BeanDefinitionBuilder.genericBeanDefinition(StaticHeaderFactory.class); + builder.addConstructorArgValue(CONTENT_TYPE_OPTIONS_HEADER); + builder.addConstructorArgValue("nosniff"); + headerFactories.add(builder.getBeanDefinition()); } } - private void parseFrameOptionsElement(Element element, ParserContext parserContext, Map headers) { + private void parseFrameOptionsElement(Element element, ParserContext parserContext) { + BeanDefinitionBuilder builder = BeanDefinitionBuilder.genericBeanDefinition(FrameOptionsHeaderFactory.class); + Element frameElt = DomUtils.getChildElementByTagName(element, FRAME_OPTIONS_ELEMENT); if (frameElt != null) { String header = getAttribute(frameElt, ATT_POLICY, "DENY"); + builder.addConstructorArgValue(header); if (ALLOW_FROM.equals(header) ) { - String origin = frameElt.getAttribute(ATT_ORIGIN); - if (!StringUtils.hasText(origin) ) { - parserContext.getReaderContext().error(" requires a non-empty string value for the origin attribute to be specified.", frameElt); + String strategyRef = getAttribute(frameElt, ATT_REF, null); + String strategy = getAttribute(frameElt, ATT_STRATEGY, null); + + if (StringUtils.hasText(strategy) && StringUtils.hasText(strategyRef)) { + parserContext.getReaderContext().error("Only one of 'strategy' or 'strategy-ref' can be set.", + frameElt); + } else if (strategyRef != null) { + builder.addConstructorArgReference(strategyRef); + } else if (strategy != null) { + String value = getAttribute(frameElt, ATT_VALUE, null); + if (!StringUtils.hasText(value)) { + parserContext.getReaderContext().error("Strategy requires a 'value' to be set.", frameElt); + } + // static, whitelist, regexp + if ("static".equals(strategy)) { + try { + builder.addConstructorArgValue(new StaticAllowFromStrategy(new URI(value))); + } catch (URISyntaxException e) { + parserContext.getReaderContext().error( + "'value' attribute doesn't represent a valid URI.", frameElt, e); + } + } else { + RequestParameterAllowFromStrategy allowFromStrategy = null; + if ("whitelist".equals(strategy)) { + allowFromStrategy = new WhiteListedAllowFromStrategy( + StringUtils.commaDelimitedListToSet(value)); + } else { + try { + allowFromStrategy = new RegExpAllowFromStrategy(value); + } catch (PatternSyntaxException e) { + parserContext.getReaderContext().error( + "'value' attribute doesn't represent a valid regular expression.", frameElt, e); + } + } + String fromParameter = getAttribute(frameElt, ATT_FROM_PARAMETER, "from"); + allowFromStrategy.setParameterName(fromParameter); + builder.addConstructorArgValue(allowFromStrategy); + } + } else { + parserContext.getReaderContext().error("One of 'strategy' and 'strategy-ref' must be set.", + frameElt); } - header += " " + origin; } - headers.put(FRAME_OPTIONS_HEADER, header); + headerFactories.add(builder.getBeanDefinition()); } } - private void parseXssElement(Element element, ParserContext parserContext, Map headers) { + private void parseXssElement(Element element, ParserContext parserContext) { Element xssElt = DomUtils.getChildElementByTagName(element, XSS_ELEMENT); if (xssElt != null) { boolean enabled = Boolean.valueOf(getAttribute(xssElt, ATT_ENABLED, "true")); @@ -109,9 +168,12 @@ public class HeadersBeanDefinitionParser implements BeanDefinitionParser { if (enabled && block) { value += "; mode=block"; } else if (!enabled && block) { - parserContext.getReaderContext().error(" does not allow for the block=\"true\".", xssElt); + parserContext.getReaderContext().error(" does not allow block=\"true\".", xssElt); } - headers.put(XSS_PROTECTION_HEADER, value); + BeanDefinitionBuilder builder = BeanDefinitionBuilder.genericBeanDefinition(StaticHeaderFactory.class); + builder.addConstructorArgValue(XSS_PROTECTION_HEADER); + builder.addConstructorArgValue(value); + headerFactories.add(builder.getBeanDefinition()); } } diff --git a/config/src/main/resources/org/springframework/security/config/spring-security-3.2.rnc b/config/src/main/resources/org/springframework/security/config/spring-security-3.2.rnc index 7cfa8c16c3..6a6276077e 100644 --- a/config/src/main/resources/org/springframework/security/config/spring-security-3.2.rnc +++ b/config/src/main/resources/org/springframework/security/config/spring-security-3.2.rnc @@ -729,8 +729,18 @@ frame-options.attlist &= ## Specify the policy to use for the X-Frame-Options-Header. attribute policy {"DENY","SAMEORIGIN","ALLOW-FROM"}? frame-options.attlist &= - ## Specify the origin to use when ALLOW-FROM is chosen. - attribute origin {xsd:token}? + ## Specify the strategy to use when ALLOW-FROM is chosen. + attribute strategy {"static","whitelist","regexp"}? +frame-options.attlist &= + ## Specify the a reference to the custom AllowFromStrategy to use when ALLOW-FROM is chosen. + ref? +frame-options.attlist &= + ## Specify the a value to use for the chosen strategy. + attribute value {xsd:string}? +frame-options.attlist &= + ## Specify the request parameter to use for the origin when using a 'whitelist' or 'regexp' based strategy. Default is 'from'. + attribute from-parameter {xsd:string}? + xss-protection = ## Enable basic XSS browser protection, supported by newer browsers (IE8+), will set the X-XSS-Protection header. @@ -751,10 +761,13 @@ header= element header {header.attlist} header.attlist &= ## The name of the header to add. - attribute name {xsd:token} + attribute name {xsd:token}? header.attlist &= ## The value for the header. - attribute value {xsd:token} + attribute value {xsd:token}? +header.attlist &= + ## Reference to a custom HeaderFactory implementation. + ref? any-user-service = user-service | jdbc-user-service | ldap-user-service diff --git a/config/src/main/resources/org/springframework/security/config/spring-security-3.2.xsd b/config/src/main/resources/org/springframework/security/config/spring-security-3.2.xsd index c9c232588b..f9b6007a6d 100644 --- a/config/src/main/resources/org/springframework/security/config/spring-security-3.2.xsd +++ b/config/src/main/resources/org/springframework/security/config/spring-security-3.2.xsd @@ -2271,9 +2271,35 @@ - + - Specify the origin to use when ALLOW-FROM is chosen. + Specify the strategy to use when ALLOW-FROM is chosen. + + + + + + + + + + + + + Defines a reference to a Spring bean Id. + + + + + + Specify the a value to use for the chosen strategy. + + + + + + Specify the request parameter to use for the origin when using a 'whitelist' or 'regexp' + based strategy. Default is 'from'. @@ -2319,18 +2345,24 @@ - + The name of the header to add. - + The value for the header. + + + Defines a reference to a Spring bean Id. + + + diff --git a/config/src/test/groovy/org/springframework/security/config/http/HttpHeadersConfigTests.groovy b/config/src/test/groovy/org/springframework/security/config/http/HttpHeadersConfigTests.groovy index 553c23f6ec..062c1ede90 100644 --- a/config/src/test/groovy/org/springframework/security/config/http/HttpHeadersConfigTests.groovy +++ b/config/src/test/groovy/org/springframework/security/config/http/HttpHeadersConfigTests.groovy @@ -12,6 +12,8 @@ */ package org.springframework.security.config.http +import org.springframework.security.util.FieldUtils + import javax.servlet.Filter import javax.servlet.http.HttpServletRequest @@ -54,10 +56,12 @@ class HttpHeadersConfigTests extends AbstractHttpConfigTests { createAppContext() def hf = getFilter(HeadersFilter) + MockHttpServletResponse response = new MockHttpServletResponse(); + hf.doFilter(new MockHttpServletRequest(), response); expect: hf - hf.headers.isEmpty() + response.headers.isEmpty() } def 'http headers content-type-options'() { @@ -69,10 +73,11 @@ class HttpHeadersConfigTests extends AbstractHttpConfigTests { createAppContext() def hf = getFilter(HeadersFilter) - + MockHttpServletResponse response = new MockHttpServletResponse(); + hf.doFilter(new MockHttpServletRequest(), response); expect: hf - hf.headers == ['X-Content-Type-Options':'nosniff'] + response.headers == ['X-Content-Type-Options':'nosniff'] } def 'http headers frame-options defaults to DENY'() { @@ -288,6 +293,6 @@ class HttpHeadersConfigTests extends AbstractHttpConfigTests { then: BeanDefinitionParsingException e = thrown() - e.message.contains ' does not allow for the block="true".' + e.message.contains ' does not allow block="true".' } } diff --git a/docs/manual/src/docbook/appendix-namespace.xml b/docs/manual/src/docbook/appendix-namespace.xml index f61c134b37..f01b39a1d2 100644 --- a/docs/manual/src/docbook/appendix-namespace.xml +++ b/docs/manual/src/docbook/appendix-namespace.xml @@ -319,9 +319,44 @@ including it in a frame it is the same as the one serving the page. -
- <literal>frame-options-origin</literal> - The origin +
+ <literal>frame-options-strategy</literal> + + Select the AllowFromStrategy to use when using the ALLOW-FROM policy. + + static Use a single static ALLOW-FROM value. The value can be set + through the value attribute. + + regexp Use a regelur expression to validate incoming requests and + if they are allowed. The regular expression can be set through the value + attribute. The request parameter used to retrieve the value to validate can be specified + using the from-parameter. + + whitelistA comma-seperated list containing the allowed domains. + The comma-seperated list can be set through the value + attribute. The request parameter used to retrieve the value to validate can be specified + using the from-parameter. + + + +
+
+ <literal>frame-options-ref</literal> + + Instead of using one of the predefined strategies it is also possible to use a custom AllowFromStrategy. + The reference to this bean can be specified through this ref attribute. + +
+
+ <literal>frame-options-value</literal> + The value to use when ALLOW-FROM is used a strategy. +
+
+ <literal>frame-options-from-parameter</literal> + + Specify the name of the request parameter to use when using regexp or whitelist for the ALLOW-FROM + strategy. +
@@ -381,6 +416,10 @@ <literal>header-value</literal> The value of the header to add.
+
+ <literal>header-ref</literal> + Reference to a custom implementation of the HeaderFactory interface. +
Parent Elements of <literal><header></literal> diff --git a/web/src/main/java/org/springframework/security/web/headers/Header.java b/web/src/main/java/org/springframework/security/web/headers/Header.java new file mode 100644 index 0000000000..28abb127b6 --- /dev/null +++ b/web/src/main/java/org/springframework/security/web/headers/Header.java @@ -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)+"]"; + } +} diff --git a/web/src/main/java/org/springframework/security/web/headers/HeaderFactory.java b/web/src/main/java/org/springframework/security/web/headers/HeaderFactory.java new file mode 100644 index 0000000000..5a7ac79b6a --- /dev/null +++ b/web/src/main/java/org/springframework/security/web/headers/HeaderFactory.java @@ -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 null + */ + Header create(HttpServletRequest request, HttpServletResponse response); +} diff --git a/web/src/main/java/org/springframework/security/web/headers/HeadersFilter.java b/web/src/main/java/org/springframework/security/web/headers/HeadersFilter.java index 4863d85046..faf09cdcf4 100644 --- a/web/src/main/java/org/springframework/security/web/headers/HeadersFilter.java +++ b/web/src/main/java/org/springframework/security/web/headers/HeadersFilter.java @@ -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 headers = new HashMap(); + /** Collection of HeaderFactory instances to produce Headers. */ + private final List factories; + + public HeadersFilter(List factories) { + this.factories = factories; + } + @Override protected void doFilterInternal(HttpServletRequest request, HttpServletResponse response, FilterChain filterChain) throws ServletException, IOException { - for (Map.Entry 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 headers) { - this.headers.clear(); - this.headers.putAll(headers); - } - - public void addHeader(String name, String value) { - headers.put(name, value); - } } diff --git a/web/src/main/java/org/springframework/security/web/headers/StaticHeaderFactory.java b/web/src/main/java/org/springframework/security/web/headers/StaticHeaderFactory.java new file mode 100644 index 0000000000..db9dd95a8d --- /dev/null +++ b/web/src/main/java/org/springframework/security/web/headers/StaticHeaderFactory.java @@ -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; + } +} diff --git a/web/src/main/java/org/springframework/security/web/headers/frameoptions/AllowFromStrategy.java b/web/src/main/java/org/springframework/security/web/headers/frameoptions/AllowFromStrategy.java new file mode 100644 index 0000000000..c4d0200fc5 --- /dev/null +++ b/web/src/main/java/org/springframework/security/web/headers/frameoptions/AllowFromStrategy.java @@ -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); +} diff --git a/web/src/main/java/org/springframework/security/web/headers/frameoptions/FrameOptionsHeaderFactory.java b/web/src/main/java/org/springframework/security/web/headers/frameoptions/FrameOptionsHeaderFactory.java new file mode 100644 index 0000000000..ca6d8e7b40 --- /dev/null +++ b/web/src/main/java/org/springframework/security/web/headers/frameoptions/FrameOptionsHeaderFactory.java @@ -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); + } + } + +} diff --git a/web/src/main/java/org/springframework/security/web/headers/frameoptions/NullAllowFromStrategy.java b/web/src/main/java/org/springframework/security/web/headers/frameoptions/NullAllowFromStrategy.java new file mode 100644 index 0000000000..d039833ddf --- /dev/null +++ b/web/src/main/java/org/springframework/security/web/headers/frameoptions/NullAllowFromStrategy.java @@ -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; + } +} diff --git a/web/src/main/java/org/springframework/security/web/headers/frameoptions/RegExpAllowFromStrategy.java b/web/src/main/java/org/springframework/security/web/headers/frameoptions/RegExpAllowFromStrategy.java new file mode 100644 index 0000000000..79dc7fbe02 --- /dev/null +++ b/web/src/main/java/org/springframework/security/web/headers/frameoptions/RegExpAllowFromStrategy.java @@ -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(); + } +} diff --git a/web/src/main/java/org/springframework/security/web/headers/frameoptions/RequestParameterAllowFromStrategy.java b/web/src/main/java/org/springframework/security/web/headers/frameoptions/RequestParameterAllowFromStrategy.java new file mode 100644 index 0000000000..f36cc9277d --- /dev/null +++ b/web/src/main/java/org/springframework/security/web/headers/frameoptions/RequestParameterAllowFromStrategy.java @@ -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 from 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 true if the supplied origin is allowed. + */ + protected abstract boolean allowed(String from); +} diff --git a/web/src/main/java/org/springframework/security/web/headers/frameoptions/StaticAllowFromStrategy.java b/web/src/main/java/org/springframework/security/web/headers/frameoptions/StaticAllowFromStrategy.java new file mode 100644 index 0000000000..47108c4e11 --- /dev/null +++ b/web/src/main/java/org/springframework/security/web/headers/frameoptions/StaticAllowFromStrategy.java @@ -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(); + } +} diff --git a/web/src/main/java/org/springframework/security/web/headers/frameoptions/WhiteListedAllowFromStrategy.java b/web/src/main/java/org/springframework/security/web/headers/frameoptions/WhiteListedAllowFromStrategy.java new file mode 100644 index 0000000000..4bee097b0a --- /dev/null +++ b/web/src/main/java/org/springframework/security/web/headers/frameoptions/WhiteListedAllowFromStrategy.java @@ -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 allowed; + + public WhiteListedAllowFromStrategy(Collection allowed) { + Assert.notEmpty(allowed, "Allowed origins cannot be empty."); + this.allowed = allowed; + } + + @Override + protected boolean allowed(String from) { + return allowed.contains(from); + } +} diff --git a/web/src/test/java/org/springframework/security/web/headers/HeadersFilterTest.java b/web/src/test/java/org/springframework/security/web/headers/HeadersFilterTest.java index bb135eadf1..d916452430 100644 --- a/web/src/test/java/org/springframework/security/web/headers/HeadersFilterTest.java +++ b/web/src/test/java/org/springframework/security/web/headers/HeadersFilterTest.java @@ -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 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 headers = new HashMap(); - headers.put("X-Header1", "foo"); - headers.put("X-Header2", "bar"); - filter.setHeaders(headers); + List 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; + } + + } } diff --git a/web/src/test/java/org/springframework/security/web/headers/StaticHeaderFactoryTest.java b/web/src/test/java/org/springframework/security/web/headers/StaticHeaderFactoryTest.java new file mode 100644 index 0000000000..e1c1cbde2c --- /dev/null +++ b/web/src/test/java/org/springframework/security/web/headers/StaticHeaderFactoryTest.java @@ -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)); + } +} diff --git a/web/src/test/java/org/springframework/security/web/headers/frameoptions/RegExpAllowFromStrategyTest.java b/web/src/test/java/org/springframework/security/web/headers/frameoptions/RegExpAllowFromStrategyTest.java new file mode 100644 index 0000000000..599ed58d1e --- /dev/null +++ b/web/src/test/java/org/springframework/security/web/headers/frameoptions/RegExpAllowFromStrategyTest.java @@ -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()); + } + +} diff --git a/web/src/test/java/org/springframework/security/web/headers/frameoptions/StaticAllowFromStrategyTest.java b/web/src/test/java/org/springframework/security/web/headers/frameoptions/StaticAllowFromStrategyTest.java new file mode 100644 index 0000000000..83e9093cd0 --- /dev/null +++ b/web/src/test/java/org/springframework/security/web/headers/frameoptions/StaticAllowFromStrategyTest.java @@ -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())); + } +} diff --git a/web/src/test/java/org/springframework/security/web/headers/frameoptions/WhiteListedAllowFromStrategyTest.java b/web/src/test/java/org/springframework/security/web/headers/frameoptions/WhiteListedAllowFromStrategyTest.java new file mode 100644 index 0000000000..816f568956 --- /dev/null +++ b/web/src/test/java/org/springframework/security/web/headers/frameoptions/WhiteListedAllowFromStrategyTest.java @@ -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()); + } + + @Test(expected = IllegalArgumentException.class) + public void nullListShouldThrowException() { + new WhiteListedAllowFromStrategy(null); + } + + @Test + public void listWithSingleElementShouldMatch() { + List allowed = new ArrayList(); + 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 allowed = new ArrayList(); + 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 allowed = new ArrayList(); + 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 allowed = new ArrayList(); + allowed.add("http://www.test.com"); + WhiteListedAllowFromStrategy strategy = new WhiteListedAllowFromStrategy(allowed); + MockHttpServletRequest request = new MockHttpServletRequest(); + + String result = strategy.apply(request); + assertThat(result, is("DENY")); + + } + + +}