Adding support for echoing the request origin for CORS.

Motivation:
When CORS has been configured to allow "*" origin, and at the same time
is allowing credentials/cookies, this causes an error from the browser
because when the response 'Access-Control-Allow-Credentials' header
is true, the 'Access-Control-Allow-Origin' must be an actual origin.

Modifications:
Changed CorsHandler setOrigin method to check for the combination of "*"
origin and allowCredentials, and if the check matches echo the CORS
request's 'Origin' value.

Result:
This addition enables the echoing of the request 'Origin' value as the
'Access-Control-Allow-Origin' value when the server has been configured
to allow any origin in combination with allowCredentials.

This allows client requests to succeed when expecting the server to
be able to handle "*" origin and at the same time be able to send cookies
by setting 'xhr.withCredentials=true'. A concrete example of this is
the SockJS protocol which expects behaviour.
This commit is contained in:
Daniel Bevenius 2014-04-25 13:26:41 +02:00 committed by Norman Maurer
parent b4bc47e280
commit 3f03612241
2 changed files with 38 additions and 4 deletions

View File

@ -89,11 +89,17 @@ public class CorsHandler extends ChannelDuplexHandler {
return true; return true;
} }
if (config.isAnyOriginSupported()) { if (config.isAnyOriginSupported()) {
setAnyOrigin(response); if (config.isCredentialsAllowed()) {
echoRequestOrigin(response);
setVaryHeader(response);
} else {
setAnyOrigin(response);
}
return true; return true;
} }
if (config.origins().contains(origin)) { if (config.origins().contains(origin)) {
response.headers().set(ACCESS_CONTROL_ALLOW_ORIGIN, origin); setOrigin(response, origin);
setVaryHeader(response);
return true; return true;
} }
logger.debug("Request origin [" + origin + "] was not among the configured origins " + config.origins()); logger.debug("Request origin [" + origin + "] was not among the configured origins " + config.origins());
@ -101,8 +107,20 @@ public class CorsHandler extends ChannelDuplexHandler {
return false; return false;
} }
private static void setAnyOrigin(final HttpResponse response) { private void echoRequestOrigin(final HttpResponse response) {
response.headers().set(ACCESS_CONTROL_ALLOW_ORIGIN, "*"); setOrigin(response, request.headers().get(ORIGIN));
}
private void setVaryHeader(final HttpResponse response) {
response.headers().set(VARY, ORIGIN);
}
private void setAnyOrigin(final HttpResponse response) {
setOrigin(response, "*");
}
private void setOrigin(final HttpResponse response, final String origin) {
response.headers().set(ACCESS_CONTROL_ALLOW_ORIGIN, origin);
} }
private void setAllowCredentials(final HttpResponse response) { private void setAllowCredentials(final HttpResponse response) {

View File

@ -82,6 +82,7 @@ public class CorsHandlerTest {
final HttpResponse response = preflightRequest(config, "http://localhost:8888", "content-type, xheader1"); final HttpResponse response = preflightRequest(config, "http://localhost:8888", "content-type, xheader1");
assertThat(response.headers().get(ACCESS_CONTROL_ALLOW_ORIGIN), is("http://localhost:8888")); assertThat(response.headers().get(ACCESS_CONTROL_ALLOW_ORIGIN), is("http://localhost:8888"));
assertThat(response.headers().getAll(ACCESS_CONTROL_ALLOW_METHODS), hasItems("GET", "DELETE")); assertThat(response.headers().getAll(ACCESS_CONTROL_ALLOW_METHODS), hasItems("GET", "DELETE"));
assertThat(response.headers().get(VARY), equalTo(ORIGIN.toString()));
} }
@Test @Test
@ -94,6 +95,7 @@ public class CorsHandlerTest {
assertThat(response.headers().get(ACCESS_CONTROL_ALLOW_ORIGIN), is("http://localhost:8888")); assertThat(response.headers().get(ACCESS_CONTROL_ALLOW_ORIGIN), is("http://localhost:8888"));
assertThat(response.headers().getAll(ACCESS_CONTROL_ALLOW_METHODS), hasItems("OPTIONS", "GET")); assertThat(response.headers().getAll(ACCESS_CONTROL_ALLOW_METHODS), hasItems("OPTIONS", "GET"));
assertThat(response.headers().getAll(ACCESS_CONTROL_ALLOW_HEADERS), hasItems("content-type", "xheader1")); assertThat(response.headers().getAll(ACCESS_CONTROL_ALLOW_HEADERS), hasItems("content-type", "xheader1"));
assertThat(response.headers().get(VARY), equalTo(ORIGIN.toString()));
} }
@Test @Test
@ -102,6 +104,7 @@ public class CorsHandlerTest {
final HttpResponse response = preflightRequest(config, "http://localhost:8888", "content-type, xheader1"); final HttpResponse response = preflightRequest(config, "http://localhost:8888", "content-type, xheader1");
assertThat(response.headers().get(CONTENT_LENGTH), is("0")); assertThat(response.headers().get(CONTENT_LENGTH), is("0"));
assertThat(response.headers().get(DATE), is(notNullValue())); assertThat(response.headers().get(DATE), is(notNullValue()));
assertThat(response.headers().get(VARY), equalTo(ORIGIN.toString()));
} }
@Test @Test
@ -111,6 +114,7 @@ public class CorsHandlerTest {
.build(); .build();
final HttpResponse response = preflightRequest(config, "http://localhost:8888", "content-type, xheader1"); final HttpResponse response = preflightRequest(config, "http://localhost:8888", "content-type, xheader1");
assertThat(response.headers().get("CustomHeader"), equalTo("somevalue")); assertThat(response.headers().get("CustomHeader"), equalTo("somevalue"));
assertThat(response.headers().get(VARY), equalTo(ORIGIN.toString()));
} }
@Test @Test
@ -120,6 +124,7 @@ public class CorsHandlerTest {
.build(); .build();
final HttpResponse response = preflightRequest(config, "http://localhost:8888", "content-type, xheader1"); final HttpResponse response = preflightRequest(config, "http://localhost:8888", "content-type, xheader1");
assertThat(response.headers().getAll("CustomHeader"), hasItems("value1", "value2")); assertThat(response.headers().getAll("CustomHeader"), hasItems("value1", "value2"));
assertThat(response.headers().get(VARY), equalTo(ORIGIN.toString()));
} }
@Test @Test
@ -129,6 +134,7 @@ public class CorsHandlerTest {
.build(); .build();
final HttpResponse response = preflightRequest(config, "http://localhost:8888", "content-type, xheader1"); final HttpResponse response = preflightRequest(config, "http://localhost:8888", "content-type, xheader1");
assertThat(response.headers().getAll("CustomHeader"), hasItems("value1", "value2")); assertThat(response.headers().getAll("CustomHeader"), hasItems("value1", "value2"));
assertThat(response.headers().get(VARY), equalTo(ORIGIN.toString()));
} }
@Test @Test
@ -142,6 +148,7 @@ public class CorsHandlerTest {
}).build(); }).build();
final HttpResponse response = preflightRequest(config, "http://localhost:8888", "content-type, xheader1"); final HttpResponse response = preflightRequest(config, "http://localhost:8888", "content-type, xheader1");
assertThat(response.headers().get("GenHeader"), equalTo("generatedValue")); assertThat(response.headers().get("GenHeader"), equalTo("generatedValue"));
assertThat(response.headers().get(VARY), equalTo(ORIGIN.toString()));
} }
@Test @Test
@ -190,6 +197,15 @@ public class CorsHandlerTest {
assertThat(response.headers().contains(ACCESS_CONTROL_ALLOW_CREDENTIALS), is(false)); assertThat(response.headers().contains(ACCESS_CONTROL_ALLOW_CREDENTIALS), is(false));
} }
@Test
public void anyOriginAndAllowCredentialsShouldEchoRequestOrigin() {
final CorsConfig config = CorsConfig.withAnyOrigin().allowCredentials().build();
final HttpResponse response = simpleRequest(config, "http://localhost:7777", "");
assertThat(response.headers().get(ACCESS_CONTROL_ALLOW_CREDENTIALS), equalTo("true"));
assertThat(response.headers().get(ACCESS_CONTROL_ALLOW_ORIGIN), equalTo("http://localhost:7777"));
assertThat(response.headers().get(VARY), equalTo(ORIGIN.toString()));
}
@Test @Test
public void simpleRequestExposeHeaders() { public void simpleRequestExposeHeaders() {
final CorsConfig config = CorsConfig.withAnyOrigin().exposeHeaders("one", "two").build(); final CorsConfig config = CorsConfig.withAnyOrigin().exposeHeaders("one", "two").build();