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 02e978e528..dba6fc71cc 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 @@ -62,10 +62,10 @@ public class HeadersBeanDefinitionParser implements BeanDefinitionParser { private static final String ALLOW_FROM = "ALLOW-FROM"; - private ManagedList headerFactories; + private ManagedList headerWriters; public BeanDefinition parse(Element element, ParserContext parserContext) { - headerFactories = new ManagedList(); + headerWriters = new ManagedList(); BeanDefinitionBuilder builder = BeanDefinitionBuilder.rootBeanDefinition(HeadersFilter.class); parseXssElement(element, parserContext); @@ -74,7 +74,11 @@ public class HeadersBeanDefinitionParser implements BeanDefinitionParser { parseHeaderElements(element); - builder.addConstructorArgValue(headerFactories); + if(headerWriters.isEmpty()) { + parserContext.getReaderContext().error("At least one type of header must be specified when using ", + element); + } + builder.addConstructorArgValue(headerWriters); return builder.getBeanDefinition(); } @@ -83,12 +87,12 @@ public class HeadersBeanDefinitionParser implements BeanDefinitionParser { for (Element headerElt : headerElts) { String headerFactoryRef = headerElt.getAttribute(ATT_REF); if (StringUtils.hasText(headerFactoryRef)) { - headerFactories.add(new RuntimeBeanReference(headerFactoryRef)); + headerWriters.add(new RuntimeBeanReference(headerFactoryRef)); } else { BeanDefinitionBuilder builder = BeanDefinitionBuilder.genericBeanDefinition(StaticHeadersWriter.class); builder.addConstructorArgValue(headerElt.getAttribute(ATT_NAME)); builder.addConstructorArgValue(headerElt.getAttribute(ATT_VALUE)); - headerFactories.add(builder.getBeanDefinition()); + headerWriters.add(builder.getBeanDefinition()); } } } @@ -99,17 +103,16 @@ public class HeadersBeanDefinitionParser implements BeanDefinitionParser { BeanDefinitionBuilder builder = BeanDefinitionBuilder.genericBeanDefinition(StaticHeadersWriter.class); builder.addConstructorArgValue(CONTENT_TYPE_OPTIONS_HEADER); builder.addConstructorArgValue("nosniff"); - headerFactories.add(builder.getBeanDefinition()); + headerWriters.add(builder.getBeanDefinition()); } } private void parseFrameOptionsElement(Element element, ParserContext parserContext) { - BeanDefinitionBuilder builder = BeanDefinitionBuilder.genericBeanDefinition(FrameOptionsHeaderWriter.class); + BeanDefinitionBuilder builder = BeanDefinitionBuilder.genericBeanDefinition(XFrameOptionsHeaderWriter.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 strategyRef = getAttribute(frameElt, ATT_REF, null); String strategy = getAttribute(frameElt, ATT_STRATEGY, null); @@ -133,7 +136,7 @@ public class HeadersBeanDefinitionParser implements BeanDefinitionParser { "'value' attribute doesn't represent a valid URI.", frameElt, e); } } else { - RequestParameterAllowFromStrategy allowFromStrategy = null; + AbstractRequestParameterAllowFromStrategy allowFromStrategy = null; if ("whitelist".equals(strategy)) { allowFromStrategy = new WhiteListedAllowFromStrategy( StringUtils.commaDelimitedListToSet(value)); @@ -146,15 +149,17 @@ public class HeadersBeanDefinitionParser implements BeanDefinitionParser { } } String fromParameter = getAttribute(frameElt, ATT_FROM_PARAMETER, "from"); - allowFromStrategy.setParameterName(fromParameter); + allowFromStrategy.setAllowFromParameterName(fromParameter); builder.addConstructorArgValue(allowFromStrategy); } } else { parserContext.getReaderContext().error("One of 'strategy' and 'strategy-ref' must be set.", frameElt); } + } else { + builder.addConstructorArgValue(header); } - headerFactories.add(builder.getBeanDefinition()); + headerWriters.add(builder.getBeanDefinition()); } } @@ -173,7 +178,7 @@ public class HeadersBeanDefinitionParser implements BeanDefinitionParser { BeanDefinitionBuilder builder = BeanDefinitionBuilder.genericBeanDefinition(StaticHeadersWriter.class); builder.addConstructorArgValue(XSS_PROTECTION_HEADER); builder.addConstructorArgValue(value); - headerFactories.add(builder.getBeanDefinition()); + headerWriters.add(builder.getBeanDefinition()); } } 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 82179e3c14..90eecc156a 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 @@ -32,6 +32,8 @@ import org.springframework.security.web.access.ExceptionTranslationFilter import org.springframework.security.web.authentication.rememberme.AbstractRememberMeServices import org.springframework.security.web.authentication.ui.DefaultLoginPageGeneratingFilter import org.springframework.security.web.headers.HeadersFilter +import org.springframework.security.web.headers.StaticHeadersWriter; +import org.springframework.security.web.headers.frameoptions.StaticAllowFromStrategy; /** * @@ -51,17 +53,14 @@ class HttpHeadersConfigTests extends AbstractHttpConfigTests { } def 'http headers with empty headers'() { - httpAutoConfig { - 'headers'() - } - createAppContext() - - def hf = getFilter(HeadersFilter) - MockHttpServletResponse response = new MockHttpServletResponse() - hf.doFilter(new MockHttpServletRequest(), response, new MockFilterChain()) - - expect: - response.headers.isEmpty() + when: + httpAutoConfig { + 'headers'() + } + createAppContext() + then: + BeanDefinitionParsingException success = thrown() + success.message.contains "At least one type of header must be specified when using " } def 'http headers content-type-options'() { @@ -212,6 +211,26 @@ class HttpHeadersConfigTests extends AbstractHttpConfigTests { assertHeaders(response , ['a':'b', 'c':'d']) } + def 'http headers with ref'() { + setup: + httpAutoConfig { + 'headers'() { + 'header'(ref:'headerWriter') + } + } + xml.'b:bean'(id: 'headerWriter', 'class': StaticHeadersWriter.name) { + 'b:constructor-arg'(value:'abc') {} + 'b:constructor-arg'(value:'def') {} + } + createAppContext() + when: + def hf = getFilter(HeadersFilter) + MockHttpServletResponse response = new MockHttpServletResponse() + hf.doFilter(new MockHttpServletRequest(), response, new MockFilterChain()) + then: + assertHeaders(response, ['abc':'def']) + } + def 'http headers header no name produces error'() { when: httpAutoConfig { diff --git a/docs/manual/src/docbook/namespace-config.xml b/docs/manual/src/docbook/namespace-config.xml index 81e11adb5b..323e0893cd 100644 --- a/docs/manual/src/docbook/namespace-config.xml +++ b/docs/manual/src/docbook/namespace-config.xml @@ -617,14 +617,19 @@ List<OpenIDAttribute> attributes = token.getAttributes();The + + + +
]]> + For additional information refer to headers section of the Security Namespace appendix.
Adding in Your Own Filters 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 index 28abb127b6..503fd01c61 100644 --- a/web/src/main/java/org/springframework/security/web/headers/Header.java +++ b/web/src/main/java/org/springframework/security/web/headers/Header.java @@ -1,37 +1,56 @@ package org.springframework.security.web.headers; import java.util.Arrays; +import java.util.List; + +import javax.servlet.http.HttpServletResponse; + +import org.springframework.util.Assert; /** - * Created with IntelliJ IDEA. - * User: marten - * Date: 29-01-13 - * Time: 20:26 - * To change this template use File | Settings | File Templates. + * Represents a Header to be added to the {@link HttpServletResponse} */ -public final class Header { +final class Header { - private final String name; - private final String[] values; + private final String headerName; + private final List headerValues; - public Header(String name, String... values) { - this.name = name; - this.values = values; + /** + * Creates a new instance + * @param headerName the name of the header + * @param headerValues the values of the header + */ + public Header(String headerName, String... headerValues) { + Assert.hasText(headerName, "headerName is required"); + Assert.notEmpty(headerValues, "headerValues cannot be null or empty"); + Assert.noNullElements(headerValues, "headerValues cannot contain null values"); + this.headerName = headerName; + this.headerValues = Arrays.asList(headerValues); } + /** + * Gets the name of the header. Cannot be null. + * @return the name of the header. + */ public String getName() { - return this.name; + return this.headerName; } - public String[] getValues() { - return this.values; + /** + * Gets the values of the header. Cannot be null, empty, or contain null + * values. + * + * @return the values of the header + */ + public List getValues() { + return this.headerValues; } public int hashCode() { - return name.hashCode() + Arrays.hashCode(values); + return headerName.hashCode() + headerValues.hashCode(); } public String toString() { - return "Header [name: " + name + ", values: " + Arrays.toString(values)+"]"; + return "Header [name: " + headerName + ", values: " + headerValues +"]"; } } diff --git a/web/src/main/java/org/springframework/security/web/headers/HeaderWriter.java b/web/src/main/java/org/springframework/security/web/headers/HeaderWriter.java index 1ddedd3439..aa672686ee 100644 --- a/web/src/main/java/org/springframework/security/web/headers/HeaderWriter.java +++ b/web/src/main/java/org/springframework/security/web/headers/HeaderWriter.java @@ -19,7 +19,7 @@ import javax.servlet.http.HttpServletRequest; import javax.servlet.http.HttpServletResponse; /** - * Contract for a factory that creates {@code Header} instances. + * Contract for writing headers to a {@link HttpServletResponse} * * @see HeadersFilter * 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 46d47e5443..27e4f072fe 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 @@ -15,6 +15,7 @@ */ package org.springframework.security.web.headers; +import org.springframework.util.Assert; import org.springframework.web.filter.OncePerRequestFilter; import javax.servlet.FilterChain; @@ -35,17 +36,22 @@ import java.util.*; public class HeadersFilter extends OncePerRequestFilter { /** Collection of {@link HeaderWriter} instances to write out the headers to the response . */ - private final List factories; + private final List headerWriters; - public HeadersFilter(List factories) { - this.factories = factories; + /** + * Creates a new instance. + * + * @param headerWriters the {@link HeaderWriter} instances to write out headers to the {@link HttpServletResponse}. + */ + public HeadersFilter(List headerWriters) { + Assert.notEmpty(headerWriters, "headerWriters cannot be null"); + this.headerWriters = headerWriters; } - @Override protected void doFilterInternal(HttpServletRequest request, HttpServletResponse response, FilterChain filterChain) throws ServletException, IOException { - for (HeaderWriter factory : factories) { + for (HeaderWriter factory : headerWriters) { factory.writeHeaders(request, response); } filterChain.doFilter(request, response); diff --git a/web/src/main/java/org/springframework/security/web/headers/StaticHeadersWriter.java b/web/src/main/java/org/springframework/security/web/headers/StaticHeadersWriter.java index f576631537..51bb3711aa 100644 --- a/web/src/main/java/org/springframework/security/web/headers/StaticHeadersWriter.java +++ b/web/src/main/java/org/springframework/security/web/headers/StaticHeadersWriter.java @@ -3,8 +3,6 @@ package org.springframework.security.web.headers; import javax.servlet.http.HttpServletRequest; import javax.servlet.http.HttpServletResponse; -import org.springframework.util.Assert; - /** * {@code HeaderWriter} implementation which writes the same {@code Header} instance. * @@ -15,11 +13,13 @@ public class StaticHeadersWriter implements HeaderWriter { private final Header header; - public StaticHeadersWriter(String name, String... values) { - Assert.hasText(name, "Header name is required"); - Assert.notEmpty(values, "Header values cannot be null or empty"); - Assert.noNullElements(values, "Header values cannot contain null values"); - header = new Header(name, values); + /** + * Creates a new instance + * @param headerName the name of the header + * @param headerValues the values for the header + */ + public StaticHeadersWriter(String headerName, String... headerValues) { + header = new Header(headerName, headerValues); } public void writeHeaders(HttpServletRequest request, HttpServletResponse response) { @@ -27,4 +27,4 @@ public class StaticHeadersWriter implements HeaderWriter { response.addHeader(header.getName(), value); } } -} +} \ No newline at end of file diff --git a/web/src/main/java/org/springframework/security/web/headers/frameoptions/AbstractRequestParameterAllowFromStrategy.java b/web/src/main/java/org/springframework/security/web/headers/frameoptions/AbstractRequestParameterAllowFromStrategy.java new file mode 100644 index 0000000000..91cfb64e6b --- /dev/null +++ b/web/src/main/java/org/springframework/security/web/headers/frameoptions/AbstractRequestParameterAllowFromStrategy.java @@ -0,0 +1,58 @@ +package org.springframework.security.web.headers.frameoptions; + +import org.apache.commons.logging.Log; +import org.apache.commons.logging.LogFactory; +import org.springframework.util.Assert; +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 x-frames-allow-from is read from the request. + * + * @author Marten Deinum + * @since 3.2 + */ +public abstract class AbstractRequestParameterAllowFromStrategy implements AllowFromStrategy { + + private static final String DEFAULT_ORIGIN_REQUEST_PARAMETER = "x-frames-allow-from"; + + private String allowFromParameterName = DEFAULT_ORIGIN_REQUEST_PARAMETER; + + /** Logger for use by subclasses */ + protected final Log log = LogFactory.getLog(getClass()); + + + public String getAllowFromValue(HttpServletRequest request) { + String allowFromOrigin = request.getParameter(allowFromParameterName); + if (log.isDebugEnabled()) { + log.debug("Supplied origin '"+allowFromOrigin+"'"); + } + if (StringUtils.hasText(allowFromOrigin) && allowed(allowFromOrigin)) { + return "ALLOW-FROM " + allowFromOrigin; + } else { + return "DENY"; + } + } + + /** + * Sets the HTTP parameter used to retrieve the value for the origin that is + * allowed from. The value of the parameter should be a valid URL. The + * default parameter name is "x-frames-allow-from". + * + * @param allowFromParameterName the name of the HTTP parameter to + */ + public void setAllowFromParameterName(String allowFromParameterName) { + Assert.notNull(allowFromParameterName, "allowFromParameterName cannot be null"); + this.allowFromParameterName = allowFromParameterName; + } + + /** + * Method to be implemented by base classes, used to determine if the supplied origin is allowed. + * + * @param allowFromOrigin the supplied origin + * @return true if the supplied origin is allowed. + */ + protected abstract boolean allowed(String allowFromOrigin); +} 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 index 755d2c1608..be62b49be2 100644 --- 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 @@ -11,5 +11,12 @@ import javax.servlet.http.HttpServletRequest; */ public interface AllowFromStrategy { - String apply(HttpServletRequest request); + /** + * Gets the value for ALLOW-FROM excluding the ALLOW-FROM. For example, the + * result might be "https://example.com/". + * + * @param request the {@link HttpServletRequest} + * @return the value for ALLOW-FROM or null if no header should be added for this request. + */ + String getAllowFromValue(HttpServletRequest request); } diff --git a/web/src/main/java/org/springframework/security/web/headers/frameoptions/FrameOptionsHeaderWriter.java b/web/src/main/java/org/springframework/security/web/headers/frameoptions/FrameOptionsHeaderWriter.java deleted file mode 100644 index 64ee69d848..0000000000 --- a/web/src/main/java/org/springframework/security/web/headers/frameoptions/FrameOptionsHeaderWriter.java +++ /dev/null @@ -1,44 +0,0 @@ -package org.springframework.security.web.headers.frameoptions; - -import org.springframework.security.web.headers.HeaderWriter; - -import javax.servlet.http.HttpServletRequest; -import javax.servlet.http.HttpServletResponse; - -/** - * {@code HeaderWriter} 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 FrameOptionsHeaderWriter implements HeaderWriter { - - 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 FrameOptionsHeaderWriter(String mode) { - this(mode, new NullAllowFromStrategy()); - } - - public FrameOptionsHeaderWriter(String mode, AllowFromStrategy allowFromStrategy) { - this.mode=mode; - this.allowFromStrategy=allowFromStrategy; - } - - public void writeHeaders(HttpServletRequest request, HttpServletResponse response) { - if (ALLOW_FROM.equals(mode)) { - String value = allowFromStrategy.apply(request); - response.addHeader(FRAME_OPTIONS_HEADER, ALLOW_FROM + " " + value); - } else { - response.addHeader(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 deleted file mode 100644 index 98ee4a720a..0000000000 --- a/web/src/main/java/org/springframework/security/web/headers/frameoptions/NullAllowFromStrategy.java +++ /dev/null @@ -1,16 +0,0 @@ -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 { - 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 index 79dc7fbe02..dd93c20fa4 100644 --- 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 @@ -5,22 +5,31 @@ import org.springframework.util.Assert; import java.util.regex.Pattern; /** - * Implementation which uses a regular expression to validate the supplied origin. + * Implementation which uses a regular expression to validate the supplied + * origin. If the value of the HTTP parameter matches the pattern, then the the + * result will be ALLOW-FROM . * * @author Marten Deinum * @since 3.2 */ -public class RegExpAllowFromStrategy extends RequestParameterAllowFromStrategy { +public class RegExpAllowFromStrategy extends AbstractRequestParameterAllowFromStrategy { private final Pattern pattern; + /** + * Creates a new instance + * + * @param pattern + * the Pattern to compare against the HTTP parameter value. If + * the pattern matches, the domain will be allowed, else denied. + */ 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(); + protected boolean allowed(String allowFromOrigin) { + return pattern.matcher(allowFromOrigin).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 deleted file mode 100644 index f7697ef17f..0000000000 --- a/web/src/main/java/org/springframework/security/web/headers/frameoptions/RequestParameterAllowFromStrategy.java +++ /dev/null @@ -1,50 +0,0 @@ -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()); - - - 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 index 39be7fff41..8cfc4e5dfb 100644 --- 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 @@ -14,7 +14,7 @@ public class StaticAllowFromStrategy implements AllowFromStrategy { this.uri=uri; } - public String apply(HttpServletRequest request) { + public String getAllowFromValue(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 index 4bee097b0a..5289e752f1 100644 --- 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 @@ -1,9 +1,8 @@ package org.springframework.security.web.headers.frameoptions; -import org.springframework.util.Assert; - import java.util.Collection; -import java.util.List; + +import org.springframework.util.Assert; /** * Implementation which checks the supplied origin against a list of allowed origins. @@ -11,17 +10,21 @@ import java.util.List; * @author Marten Deinum * @since 3.2 */ -public class WhiteListedAllowFromStrategy extends RequestParameterAllowFromStrategy { +public class WhiteListedAllowFromStrategy extends AbstractRequestParameterAllowFromStrategy { private final Collection allowed; + /** + * Creates a new instance + * @param allowed the origins that are 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); + protected boolean allowed(String allowFromOrigin) { + return allowed.contains(allowFromOrigin); } } diff --git a/web/src/main/java/org/springframework/security/web/headers/frameoptions/XFrameOptionsHeaderWriter.java b/web/src/main/java/org/springframework/security/web/headers/frameoptions/XFrameOptionsHeaderWriter.java new file mode 100644 index 0000000000..326135f60c --- /dev/null +++ b/web/src/main/java/org/springframework/security/web/headers/frameoptions/XFrameOptionsHeaderWriter.java @@ -0,0 +1,110 @@ +/* + * Copyright 2002-2013 the original author or authors. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.springframework.security.web.headers.frameoptions; + +import org.springframework.security.web.headers.HeaderWriter; +import org.springframework.util.Assert; + +import javax.servlet.http.HttpServletRequest; +import javax.servlet.http.HttpServletResponse; + +/** + * {@code HeaderWriter} 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 + * @author Rob Winch + * @since 3.2 + * + * @see AllowFromStrategy + */ +public class XFrameOptionsHeaderWriter implements HeaderWriter { + + public static final String XFRAME_OPTIONS_HEADER = "X-Frame-Options"; + + private final AllowFromStrategy allowFromStrategy; + private final XFrameOptionsMode frameOptionsMode; + + /** + * Creates a new instance + * + * @param frameOptionsMode + * the {@link XFrameOptionsMode} to use. If using + * {@link XFrameOptionsMode#ALLOW_FROM}, use + * {@link #FrameOptionsHeaderWriter(AllowFromStrategy)} + * instead. + */ + public XFrameOptionsHeaderWriter(XFrameOptionsMode frameOptionsMode) { + Assert.notNull(frameOptionsMode, "frameOptionsMode cannot be null"); + if(XFrameOptionsMode.ALLOW_FROM.equals(frameOptionsMode)) { + throw new IllegalArgumentException( + "ALLOW_FROM requires an AllowFromStrategy. Please use FrameOptionsHeaderWriter(AllowFromStrategy allowFromStrategy) instead"); + } + this.frameOptionsMode = frameOptionsMode; + this.allowFromStrategy = null; + } + + /** + * Creates a new instance with {@link XFrameOptionsMode#ALLOW_FROM}. + * + * @param allowFromStrategy + * the strategy for determining what the value for ALLOW_FROM is. + */ + public XFrameOptionsHeaderWriter(AllowFromStrategy allowFromStrategy) { + Assert.notNull(allowFromStrategy, "allowFromStrategy cannot be null"); + this.frameOptionsMode = XFrameOptionsMode.ALLOW_FROM; + this.allowFromStrategy = allowFromStrategy; + } + + public void writeHeaders(HttpServletRequest request, HttpServletResponse response) { + if (XFrameOptionsMode.ALLOW_FROM.equals(frameOptionsMode)) { + String allowFromValue = allowFromStrategy.getAllowFromValue(request); + if(allowFromValue != null) { + response.addHeader(XFRAME_OPTIONS_HEADER, XFrameOptionsMode.ALLOW_FROM.getMode() + " " + allowFromValue); + } + } else { + response.addHeader(XFRAME_OPTIONS_HEADER, frameOptionsMode.getMode()); + } + } + + /** + * The possible values for the X-Frame-Options header. + * + * @author Rob Winch + * @since 3.2 + */ + public enum XFrameOptionsMode { + DENY("DENY"), + SAMEORIGIN("SAMEORIGIN"), + ALLOW_FROM("ALLOW-FROM"); + + private String mode; + + private XFrameOptionsMode(String mode) { + this.mode = mode; + } + + /** + * Gets the mode for the X-Frame-Options header value. For example, + * DENY, SAMEORIGIN, ALLOW-FROM. Cannot be null. + * + * @return the mode for the X-Frame-Options header value. + */ + public String getMode() { + return mode; + } + } +} diff --git a/web/src/test/java/org/springframework/security/web/headers/HeadersFilterTest.java b/web/src/test/java/org/springframework/security/web/headers/HeadersFilterTests.java similarity index 81% rename from web/src/test/java/org/springframework/security/web/headers/HeadersFilterTest.java rename to web/src/test/java/org/springframework/security/web/headers/HeadersFilterTests.java index e7ab1a1b79..c0411bf3c1 100644 --- a/web/src/test/java/org/springframework/security/web/headers/HeadersFilterTest.java +++ b/web/src/test/java/org/springframework/security/web/headers/HeadersFilterTests.java @@ -15,7 +15,7 @@ */ package org.springframework.security.web.headers; -import static org.junit.Assert.assertTrue; +import static org.fest.assertions.Assertions.assertThat; import static org.mockito.Mockito.verify; import java.util.ArrayList; @@ -33,27 +33,26 @@ import org.springframework.mock.web.MockHttpServletResponse; * Tests for the {@code HeadersFilter} * * @author Marten Deinum + * @author Rob Winch * @since 3.2 */ @RunWith(MockitoJUnitRunner.class) -public class HeadersFilterTest { +public class HeadersFilterTests { @Mock private HeaderWriter writer1; @Mock private HeaderWriter writer2; - @Test + @Test(expected = IllegalArgumentException.class) public void noHeadersConfigured() throws Exception { List headerWriters = new ArrayList(); - HeadersFilter filter = new HeadersFilter(headerWriters); - MockHttpServletRequest request = new MockHttpServletRequest(); - MockHttpServletResponse response = new MockHttpServletResponse(); - MockFilterChain filterChain = new MockFilterChain(); + new HeadersFilter(headerWriters); + } - filter.doFilter(request, response, filterChain); - - assertTrue(response.getHeaderNames().isEmpty()); + @Test(expected = IllegalArgumentException.class) + public void constructorNullWriters() throws Exception { + new HeadersFilter(null); } @Test @@ -72,5 +71,6 @@ public class HeadersFilterTest { verify(writer1).writeHeaders(request, response); verify(writer2).writeHeaders(request, response); + assertThat(filterChain.getRequest()).isEqualTo(request); // verify the filterChain continued } } diff --git a/web/src/test/java/org/springframework/security/web/headers/StaticHeaderWriterTests.java b/web/src/test/java/org/springframework/security/web/headers/StaticHeaderWriterTests.java index 6ce7320af8..eaa5b5b25f 100644 --- a/web/src/test/java/org/springframework/security/web/headers/StaticHeaderWriterTests.java +++ b/web/src/test/java/org/springframework/security/web/headers/StaticHeaderWriterTests.java @@ -1,3 +1,18 @@ +/* + * Copyright 2002-2013 the original author or authors. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ package org.springframework.security.web.headers; import static org.fest.assertions.Assertions.assertThat; @@ -13,6 +28,7 @@ import org.springframework.mock.web.MockHttpServletResponse; * Test for the {@code StaticHeadersWriter} * * @author Marten Deinum + * @author Rob Winch * @since 3.2 */ public class StaticHeaderWriterTests { @@ -25,6 +41,21 @@ public class StaticHeaderWriterTests { response = new MockHttpServletResponse(); } + @Test(expected = IllegalArgumentException.class) + public void constructorNullHeaderName() { + new StaticHeadersWriter(null, "value1"); + } + + @Test(expected = IllegalArgumentException.class) + public void constructorNullHeaderValues() { + new StaticHeadersWriter("name", (String[]) null); + } + + @Test(expected = IllegalArgumentException.class) + public void constructorContainsNullHeaderValue() { + new StaticHeadersWriter("name", "value1", null); + } + @Test public void sameHeaderShouldBeReturned() { String headerName = "X-header"; diff --git a/web/src/test/java/org/springframework/security/web/headers/frameoptions/AbstractRequestParameterAllowFromStrategyTests.java b/web/src/test/java/org/springframework/security/web/headers/frameoptions/AbstractRequestParameterAllowFromStrategyTests.java new file mode 100644 index 0000000000..8c541c1ee7 --- /dev/null +++ b/web/src/test/java/org/springframework/security/web/headers/frameoptions/AbstractRequestParameterAllowFromStrategyTests.java @@ -0,0 +1,101 @@ +/* + * Copyright 2002-2013 the original author or authors. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.springframework.security.web.headers.frameoptions; + +import static org.fest.assertions.Assertions.assertThat; + +import org.junit.Before; +import org.junit.Test; +import org.springframework.mock.web.MockHttpServletRequest; + +/** + * @author Rob Winch + * + */ +public class AbstractRequestParameterAllowFromStrategyTests { + private MockHttpServletRequest request; + + @Before + public void setup() { + request = new MockHttpServletRequest(); + } + + @Test + public void nullAllowFromParameterValue() { + RequestParameterAllowFromStrategyStub strategy = new RequestParameterAllowFromStrategyStub(true); + + assertThat( + strategy + .getAllowFromValue(request)).isEqualTo("DENY"); + } + + @Test + public void emptyAllowFromParameterValue() { + request.setParameter("x-frames-allow-from", ""); + RequestParameterAllowFromStrategyStub strategy = new RequestParameterAllowFromStrategyStub(true); + + assertThat( + strategy + .getAllowFromValue(request)).isEqualTo("DENY"); + } + + @Test + public void emptyAllowFromCustomParameterValue() { + String customParam = "custom"; + request.setParameter(customParam, ""); + RequestParameterAllowFromStrategyStub strategy = new RequestParameterAllowFromStrategyStub(true); + strategy.setAllowFromParameterName(customParam); + + assertThat( + strategy + .getAllowFromValue(request)).isEqualTo("DENY"); + } + + @Test + public void allowFromParameterValueAllowed() { + String value = "https://example.com"; + request.setParameter("x-frames-allow-from", value); + RequestParameterAllowFromStrategyStub strategy = new RequestParameterAllowFromStrategyStub(true); + + assertThat( + strategy + .getAllowFromValue(request)).isEqualTo("ALLOW-FROM "+value); + } + + @Test + public void allowFromParameterValueDenied() { + String value = "https://example.com"; + request.setParameter("x-frames-allow-from", value); + RequestParameterAllowFromStrategyStub strategy = new RequestParameterAllowFromStrategyStub(false); + + assertThat( + strategy + .getAllowFromValue(request)).isEqualTo("DENY"); + } + + private static class RequestParameterAllowFromStrategyStub extends AbstractRequestParameterAllowFromStrategy { + private boolean match; + + RequestParameterAllowFromStrategyStub(boolean match) { + this.match = match; + } + + @Override + protected boolean allowed(String allowFromOrigin) { + return match; + } + } +} diff --git a/web/src/test/java/org/springframework/security/web/headers/frameoptions/FrameOptionsHeaderWriterTests.java b/web/src/test/java/org/springframework/security/web/headers/frameoptions/FrameOptionsHeaderWriterTests.java new file mode 100644 index 0000000000..ae6e704fdc --- /dev/null +++ b/web/src/test/java/org/springframework/security/web/headers/frameoptions/FrameOptionsHeaderWriterTests.java @@ -0,0 +1,109 @@ +/* + * Copyright 2002-2013 the original author or authors. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.springframework.security.web.headers.frameoptions; + +import static org.fest.assertions.Assertions.assertThat; +import static org.mockito.Mockito.when; + +import java.util.Collections; + +import org.junit.Before; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.mockito.Mock; +import org.mockito.runners.MockitoJUnitRunner; +import org.springframework.mock.web.MockHttpServletRequest; +import org.springframework.mock.web.MockHttpServletResponse; +import org.springframework.security.web.headers.frameoptions.XFrameOptionsHeaderWriter.XFrameOptionsMode; + +/** + * @author Rob Winch + * + */ +@RunWith(MockitoJUnitRunner.class) +public class FrameOptionsHeaderWriterTests { + @Mock + private AllowFromStrategy strategy; + + private MockHttpServletResponse response; + + private MockHttpServletRequest request; + + private XFrameOptionsHeaderWriter writer; + + @Before + public void setup() { + request = new MockHttpServletRequest(); + response = new MockHttpServletResponse(); + } + + @Test(expected = IllegalArgumentException.class) + public void constructorNullMode() { + new XFrameOptionsHeaderWriter((XFrameOptionsMode)null); + } + + @Test(expected = IllegalArgumentException.class) + public void constructorAllowFromNoAllowFromStrategy() { + new XFrameOptionsHeaderWriter(XFrameOptionsMode.ALLOW_FROM); + } + + @Test(expected = IllegalArgumentException.class) + public void constructorNullAllowFromStrategy() { + new XFrameOptionsHeaderWriter((AllowFromStrategy)null); + } + + @Test + public void writeHeadersAllowFromReturnsNull() { + writer = new XFrameOptionsHeaderWriter(strategy); + + writer.writeHeaders(request, response); + + assertThat(response.getHeaderNames().isEmpty()).isTrue(); + } + + @Test + public void writeHeadersAllowFrom() { + String allowFromValue = "https://example.com/"; + when(strategy.getAllowFromValue(request)).thenReturn(allowFromValue); + writer = new XFrameOptionsHeaderWriter(strategy); + + writer.writeHeaders(request, response); + + assertThat(response.getHeaderNames().size()).isEqualTo(1); + assertThat(response.getHeader(XFrameOptionsHeaderWriter.XFRAME_OPTIONS_HEADER)).isEqualTo("ALLOW-FROM " + allowFromValue); + } + + @Test + public void writeHeadersDeny() { + writer = new XFrameOptionsHeaderWriter(XFrameOptionsMode.DENY); + + writer.writeHeaders(request, response); + + assertThat(response.getHeaderNames().size()).isEqualTo(1); + assertThat(response.getHeader(XFrameOptionsHeaderWriter.XFRAME_OPTIONS_HEADER)).isEqualTo("DENY"); + } + + + @Test + public void writeHeadersSameOrigin() { + writer = new XFrameOptionsHeaderWriter(XFrameOptionsMode.SAMEORIGIN); + + writer.writeHeaders(request, response); + + assertThat(response.getHeaderNames().size()).isEqualTo(1); + assertThat(response.getHeader(XFrameOptionsHeaderWriter.XFRAME_OPTIONS_HEADER)).isEqualTo("SAMEORIGIN"); + } +} 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/RegExpAllowFromStrategyTests.java similarity index 68% rename from web/src/test/java/org/springframework/security/web/headers/frameoptions/RegExpAllowFromStrategyTest.java rename to web/src/test/java/org/springframework/security/web/headers/frameoptions/RegExpAllowFromStrategyTests.java index 599ed58d1e..45119b9e50 100644 --- a/web/src/test/java/org/springframework/security/web/headers/frameoptions/RegExpAllowFromStrategyTest.java +++ b/web/src/test/java/org/springframework/security/web/headers/frameoptions/RegExpAllowFromStrategyTests.java @@ -1,23 +1,18 @@ package org.springframework.security.web.headers.frameoptions; +import static org.hamcrest.CoreMatchers.is; +import static org.junit.Assert.assertThat; + +import java.util.regex.PatternSyntaxException; + 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. + * + * @author Marten Deinum */ -public class RegExpAllowFromStrategyTest { +public class RegExpAllowFromStrategyTests { @Test(expected = PatternSyntaxException.class) public void invalidRegularExpressionShouldLeadToException() { @@ -32,18 +27,19 @@ public class RegExpAllowFromStrategyTest { @Test public void subdomainMatchingRegularExpression() { RegExpAllowFromStrategy strategy = new RegExpAllowFromStrategy("^http://([a-z0-9]*?\\.)test\\.com"); + strategy.setAllowFromParameterName("from"); MockHttpServletRequest request = new MockHttpServletRequest(); request.setParameter("from", "http://abc.test.com"); - String result1 = strategy.apply(request); + String result1 = strategy.getAllowFromValue(request); assertThat(result1, is("ALLOW-FROM http://abc.test.com")); request.setParameter("from", "http://foo.test.com"); - String result2 = strategy.apply(request); + String result2 = strategy.getAllowFromValue(request); assertThat(result2, is("ALLOW-FROM http://foo.test.com")); request.setParameter("from", "http://test.foobar.com"); - String result3 = strategy.apply(request); + String result3 = strategy.getAllowFromValue(request); assertThat(result3, is("DENY")); } @@ -51,16 +47,7 @@ public class RegExpAllowFromStrategyTest { public void noParameterShouldDeny() { RegExpAllowFromStrategy strategy = new RegExpAllowFromStrategy("^http://([a-z0-9]*?\\.)test\\.com"); MockHttpServletRequest request = new MockHttpServletRequest(); - String result1 = strategy.apply(request); + String result1 = strategy.getAllowFromValue(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/StaticAllowFromStrategyTests.java similarity index 79% rename from web/src/test/java/org/springframework/security/web/headers/frameoptions/StaticAllowFromStrategyTest.java rename to web/src/test/java/org/springframework/security/web/headers/frameoptions/StaticAllowFromStrategyTests.java index 83e9093cd0..bb941db474 100644 --- a/web/src/test/java/org/springframework/security/web/headers/frameoptions/StaticAllowFromStrategyTest.java +++ b/web/src/test/java/org/springframework/security/web/headers/frameoptions/StaticAllowFromStrategyTests.java @@ -13,12 +13,12 @@ import static org.junit.Assert.assertEquals; * @author Marten Deinum * @since 3.2 */ -public class StaticAllowFromStrategyTest { +public class StaticAllowFromStrategyTests { @Test public void shouldReturnUri() { String uri = "http://www.test.com"; StaticAllowFromStrategy strategy = new StaticAllowFromStrategy(URI.create(uri)); - assertEquals(uri, strategy.apply(new MockHttpServletRequest())); + assertEquals(uri, strategy.getAllowFromValue(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/WhiteListedAllowFromStrategyTests.java similarity index 83% rename from web/src/test/java/org/springframework/security/web/headers/frameoptions/WhiteListedAllowFromStrategyTest.java rename to web/src/test/java/org/springframework/security/web/headers/frameoptions/WhiteListedAllowFromStrategyTests.java index 816f568956..4f3f93db9c 100644 --- a/web/src/test/java/org/springframework/security/web/headers/frameoptions/WhiteListedAllowFromStrategyTest.java +++ b/web/src/test/java/org/springframework/security/web/headers/frameoptions/WhiteListedAllowFromStrategyTests.java @@ -15,7 +15,7 @@ import static org.springframework.test.util.MatcherAssertionErrors.assertThat; * @author Marten Deinum * @since 3.2 */ -public class WhiteListedAllowFromStrategyTest { +public class WhiteListedAllowFromStrategyTests { @Test(expected = IllegalArgumentException.class) public void emptyListShouldThrowException() { @@ -32,10 +32,11 @@ public class WhiteListedAllowFromStrategyTest { List allowed = new ArrayList(); allowed.add("http://www.test.com"); WhiteListedAllowFromStrategy strategy = new WhiteListedAllowFromStrategy(allowed); + strategy.setAllowFromParameterName("from"); MockHttpServletRequest request = new MockHttpServletRequest(); request.setParameter("from", "http://www.test.com"); - String result = strategy.apply(request); + String result = strategy.getAllowFromValue(request); assertThat(result, is("ALLOW-FROM http://www.test.com")); } @@ -45,10 +46,11 @@ public class WhiteListedAllowFromStrategyTest { allowed.add("http://www.test.com"); allowed.add("http://www.springsource.org"); WhiteListedAllowFromStrategy strategy = new WhiteListedAllowFromStrategy(allowed); + strategy.setAllowFromParameterName("from"); MockHttpServletRequest request = new MockHttpServletRequest(); request.setParameter("from", "http://www.test.com"); - String result = strategy.apply(request); + String result = strategy.getAllowFromValue(request); assertThat(result, is("ALLOW-FROM http://www.test.com")); } @@ -57,10 +59,11 @@ public class WhiteListedAllowFromStrategyTest { List allowed = new ArrayList(); allowed.add("http://www.test.com"); WhiteListedAllowFromStrategy strategy = new WhiteListedAllowFromStrategy(allowed); + strategy.setAllowFromParameterName("from"); MockHttpServletRequest request = new MockHttpServletRequest(); request.setParameter("from", "http://www.test123.com"); - String result = strategy.apply(request); + String result = strategy.getAllowFromValue(request); assertThat(result, is("DENY")); } @@ -69,9 +72,10 @@ public class WhiteListedAllowFromStrategyTest { List allowed = new ArrayList(); allowed.add("http://www.test.com"); WhiteListedAllowFromStrategy strategy = new WhiteListedAllowFromStrategy(allowed); + strategy.setAllowFromParameterName("from"); MockHttpServletRequest request = new MockHttpServletRequest(); - String result = strategy.apply(request); + String result = strategy.getAllowFromValue(request); assertThat(result, is("DENY")); }