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 062c1ede90..82179e3c14 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
@@ -17,6 +17,7 @@ import org.springframework.security.util.FieldUtils
import javax.servlet.Filter
import javax.servlet.http.HttpServletRequest
+import org.springframework.beans.factory.BeanCreationException;
import org.springframework.beans.factory.parsing.BeanDefinitionParsingException
import org.springframework.beans.factory.xml.XmlBeanDefinitionStoreException;
import org.springframework.mock.web.MockFilterChain
@@ -56,11 +57,10 @@ class HttpHeadersConfigTests extends AbstractHttpConfigTests {
createAppContext()
def hf = getFilter(HeadersFilter)
- MockHttpServletResponse response = new MockHttpServletResponse();
- hf.doFilter(new MockHttpServletRequest(), response);
+ MockHttpServletResponse response = new MockHttpServletResponse()
+ hf.doFilter(new MockHttpServletRequest(), response, new MockFilterChain())
expect:
- hf
response.headers.isEmpty()
}
@@ -73,11 +73,11 @@ class HttpHeadersConfigTests extends AbstractHttpConfigTests {
createAppContext()
def hf = getFilter(HeadersFilter)
- MockHttpServletResponse response = new MockHttpServletResponse();
- hf.doFilter(new MockHttpServletRequest(), response);
+ MockHttpServletResponse response = new MockHttpServletResponse()
+ hf.doFilter(new MockHttpServletRequest(), response, new MockFilterChain())
+
expect:
- hf
- response.headers == ['X-Content-Type-Options':'nosniff']
+ assertHeaders(response, ['X-Content-Type-Options':'nosniff'])
}
def 'http headers frame-options defaults to DENY'() {
@@ -89,10 +89,11 @@ class HttpHeadersConfigTests extends AbstractHttpConfigTests {
createAppContext()
def hf = getFilter(HeadersFilter)
+ MockHttpServletResponse response = new MockHttpServletResponse()
+ hf.doFilter(new MockHttpServletRequest(), response, new MockFilterChain())
expect:
- hf
- hf.headers == ['X-Frame-Options':'DENY']
+ assertHeaders(response, ['X-Frame-Options':'DENY'])
}
def 'http headers frame-options DENY'() {
@@ -104,10 +105,11 @@ class HttpHeadersConfigTests extends AbstractHttpConfigTests {
createAppContext()
def hf = getFilter(HeadersFilter)
+ MockHttpServletResponse response = new MockHttpServletResponse()
+ hf.doFilter(new MockHttpServletRequest(), response, new MockFilterChain())
expect:
- hf
- hf.headers == ['X-Frame-Options':'DENY']
+ assertHeaders(response, ['X-Frame-Options':'DENY'])
}
def 'http headers frame-options SAMEORIGIN'() {
@@ -119,17 +121,18 @@ class HttpHeadersConfigTests extends AbstractHttpConfigTests {
createAppContext()
def hf = getFilter(HeadersFilter)
+ MockHttpServletResponse response = new MockHttpServletResponse()
+ hf.doFilter(new MockHttpServletRequest(), response, new MockFilterChain())
expect:
- hf
- hf.headers == ['X-Frame-Options':'SAMEORIGIN']
+ assertHeaders(response, ['X-Frame-Options':'SAMEORIGIN'])
}
def 'http headers frame-options ALLOW-FROM no origin reports error'() {
when:
httpAutoConfig {
'headers'() {
- 'frame-options'(policy : 'ALLOW-FROM')
+ 'frame-options'(policy : 'ALLOW-FROM', strategy : 'static')
}
}
createAppContext()
@@ -138,14 +141,14 @@ class HttpHeadersConfigTests extends AbstractHttpConfigTests {
then:
BeanDefinitionParsingException e = thrown()
- e.message.contains ' requires a non-empty string value for the origin attribute to be specified.'
+ e.message.contains "Strategy requires a 'value' to be set." // FIME better error message?
}
def 'http headers frame-options ALLOW-FROM spaces only origin reports error'() {
when:
httpAutoConfig {
'headers'() {
- 'frame-options'(policy : 'ALLOW-FROM', origin : ' ')
+ 'frame-options'(policy : 'ALLOW-FROM', strategy: 'static', value : ' ')
}
}
createAppContext()
@@ -154,23 +157,24 @@ class HttpHeadersConfigTests extends AbstractHttpConfigTests {
then:
BeanDefinitionParsingException e = thrown()
- e.message.contains ' requires a non-empty string value for the origin attribute to be specified.'
+ e.message.contains "Strategy requires a 'value' to be set." // FIME better error message?
}
def 'http headers frame-options ALLOW-FROM'() {
when:
httpAutoConfig {
'headers'() {
- 'frame-options'(policy : 'ALLOW-FROM', origin : 'https://example.com')
+ 'frame-options'(policy : 'ALLOW-FROM', strategy: 'static', value : 'https://example.com')
}
}
createAppContext()
def hf = getFilter(HeadersFilter)
+ MockHttpServletResponse response = new MockHttpServletResponse()
+ hf.doFilter(new MockHttpServletRequest(), response, new MockFilterChain())
then:
- hf
- hf.headers == ['X-Frame-Options':'ALLOW-FROM https://example.com']
+ assertHeaders(response, ['X-Frame-Options':'ALLOW-FROM https://example.com'])
}
def 'http headers header a=b'() {
@@ -183,10 +187,11 @@ class HttpHeadersConfigTests extends AbstractHttpConfigTests {
createAppContext()
def hf = getFilter(HeadersFilter)
+ MockHttpServletResponse response = new MockHttpServletResponse()
+ hf.doFilter(new MockHttpServletRequest(), response, new MockFilterChain())
then:
- hf
- hf.headers == ['a':'b']
+ assertHeaders(response, ['a':'b'])
}
def 'http headers header a=b and c=d'() {
@@ -200,10 +205,11 @@ class HttpHeadersConfigTests extends AbstractHttpConfigTests {
createAppContext()
def hf = getFilter(HeadersFilter)
+ MockHttpServletResponse response = new MockHttpServletResponse()
+ hf.doFilter(new MockHttpServletRequest(), response, new MockFilterChain())
then:
- hf
- hf.headers.sort() == ['a':'b', 'c':'d'].sort()
+ assertHeaders(response , ['a':'b', 'c':'d'])
}
def 'http headers header no name produces error'() {
@@ -216,7 +222,7 @@ class HttpHeadersConfigTests extends AbstractHttpConfigTests {
createAppContext()
then:
- thrown(XmlBeanDefinitionStoreException)
+ thrown(BeanCreationException)
}
def 'http headers header no value produces error'() {
@@ -229,7 +235,7 @@ class HttpHeadersConfigTests extends AbstractHttpConfigTests {
createAppContext()
then:
- thrown(XmlBeanDefinitionStoreException)
+ thrown(BeanCreationException)
}
def 'http headers xss-protection defaults'() {
@@ -242,10 +248,11 @@ class HttpHeadersConfigTests extends AbstractHttpConfigTests {
createAppContext()
def hf = getFilter(HeadersFilter)
+ MockHttpServletResponse response = new MockHttpServletResponse()
+ hf.doFilter(new MockHttpServletRequest(), response, new MockFilterChain())
then:
- hf
- hf.headers == ['X-XSS-Protection':'1; mode=block']
+ assertHeaders(response, ['X-XSS-Protection':'1; mode=block'])
}
def 'http headers xss-protection enabled=true'() {
@@ -258,10 +265,11 @@ class HttpHeadersConfigTests extends AbstractHttpConfigTests {
createAppContext()
def hf = getFilter(HeadersFilter)
+ MockHttpServletResponse response = new MockHttpServletResponse()
+ hf.doFilter(new MockHttpServletRequest(), response, new MockFilterChain())
then:
- hf
- hf.headers == ['X-XSS-Protection':'1; mode=block']
+ assertHeaders(response, ['X-XSS-Protection':'1; mode=block'])
}
def 'http headers xss-protection enabled=false'() {
@@ -274,10 +282,11 @@ class HttpHeadersConfigTests extends AbstractHttpConfigTests {
createAppContext()
def hf = getFilter(HeadersFilter)
+ MockHttpServletResponse response = new MockHttpServletResponse()
+ hf.doFilter(new MockHttpServletRequest(), response, new MockFilterChain())
then:
- hf
- hf.headers == ['X-XSS-Protection':'0']
+ assertHeaders(response, ['X-XSS-Protection':'0'])
}
def 'http headers xss-protection enabled=false and block=true produces error'() {
@@ -295,4 +304,11 @@ class HttpHeadersConfigTests extends AbstractHttpConfigTests {
BeanDefinitionParsingException e = thrown()
e.message.contains ' does not allow block="true".'
}
+
+ def assertHeaders(MockHttpServletResponse response, Map expected) {
+ assert response.headerNames == expected.keySet()
+ expected.each { headerName, value ->
+ assert response.getHeaderValues(headerName) == [value]
+ }
+ }
}
diff --git a/docs/manual/src/docbook/appendix-namespace.xml b/docs/manual/src/docbook/appendix-namespace.xml
index f01b39a1d2..96d9c8afb9 100644
--- a/docs/manual/src/docbook/appendix-namespace.xml
+++ b/docs/manual/src/docbook/appendix-namespace.xml
@@ -319,7 +319,7 @@
including it in a frame it is the same as the one serving the page.
-
+
<literal>frame-options-strategy</literal>
Select the AllowFromStrategy to use when using the ALLOW-FROM policy.
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
index db9dd95a8d..c78e908b45 100644
--- a/web/src/main/java/org/springframework/security/web/headers/StaticHeaderFactory.java
+++ b/web/src/main/java/org/springframework/security/web/headers/StaticHeaderFactory.java
@@ -3,6 +3,8 @@ package org.springframework.security.web.headers;
import javax.servlet.http.HttpServletRequest;
import javax.servlet.http.HttpServletResponse;
+import org.springframework.util.Assert;
+
/**
* {@code HeaderFactory} implementation which returns the same {@code Header} instance.
*
@@ -14,6 +16,9 @@ public class StaticHeaderFactory implements HeaderFactory {
private final Header header;
public StaticHeaderFactory(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);
}
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
index ca6d8e7b40..65ff4c6285 100644
--- 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
@@ -33,11 +33,10 @@ public class FrameOptionsHeaderFactory implements HeaderFactory {
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);
+ return new Header(FRAME_OPTIONS_HEADER, ALLOW_FROM + " " + 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
index d039833ddf..98ee4a720a 100644
--- 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
@@ -10,7 +10,6 @@ import javax.servlet.http.HttpServletRequest;
* 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/RequestParameterAllowFromStrategy.java b/web/src/main/java/org/springframework/security/web/headers/frameoptions/RequestParameterAllowFromStrategy.java
index f36cc9277d..f7697ef17f 100644
--- 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
@@ -24,7 +24,6 @@ public abstract class RequestParameterAllowFromStrategy implements AllowFromStra
protected final Log log = LogFactory.getLog(getClass());
- @Override
public String apply(HttpServletRequest request) {
String from = request.getParameter(parameter);
if (log.isDebugEnabled()) {
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 47108c4e11..39be7fff41 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,6 @@ public class StaticAllowFromStrategy implements AllowFromStrategy {
this.uri=uri;
}
- @Override
public String apply(HttpServletRequest request) {
return uri.toString();
}
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 d916452430..0eb856fd71 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
@@ -84,7 +84,6 @@ public class HeadersFilterTest {
private String name;
private String value;
- @Override
public Header create(HttpServletRequest request, HttpServletResponse response) {
return new Header(name, value);
}