Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Security: Allow to configure CORS allow-credentials header to work via SSL #7059

Merged

Conversation

Projects
None yet
5 participants
@spinscale
Copy link
Member

commented Jul 28, 2014

This adds support to return the "Access-Control-Allow-Credentials" header
if needed, so CORS will work flawlessly with authenticated applications.

Closes #6380

@erikringsmuth

This comment has been minimized.

Copy link
Contributor

commented Jul 28, 2014

I pulled this code and ran it locally. I can confirm it fixes the issue. Thanks @spinscale!

@s1monw

View changes

src/main/java/org/elasticsearch/http/netty/NettyHttpChannel.java Outdated
@@ -112,6 +112,10 @@ public void sendResponse(RestResponse response) {
resp.headers().add(ACCESS_CONTROL_ALLOW_METHODS, transport.settings().get("http.cors.allow-methods", "OPTIONS, HEAD, GET, POST, PUT, DELETE"));
resp.headers().add(ACCESS_CONTROL_ALLOW_HEADERS, transport.settings().get("http.cors.allow-headers", "X-Requested-With, Content-Type, Content-Length"));
}

if (transport.settings().getAsBoolean("http.cors.allow-credentials", false)) {

This comment has been minimized.

Copy link
@s1monw

s1monw Jul 29, 2014

Contributor

can we use a constant here?

@s1monw

View changes

src/test/java/org/elasticsearch/rest/CorsRegexTests.java Outdated
@@ -48,6 +49,7 @@
protected Settings nodeSettings(int nodeOrdinal) {
return ImmutableSettings.settingsBuilder()
.put("http.cors.allow-origin", "/https?:\\/\\/localhost(:[0-9]+)?/")
.put("http.cors.allow-credentials", "true")

This comment has been minimized.

Copy link
@s1monw

s1monw Jul 29, 2014

Contributor

can you use a constant too?

@s1monw s1monw removed the review label Jul 29, 2014

@s1monw

This comment has been minimized.

Copy link
Contributor

commented Jul 29, 2014

left some minor comments

@spinscale

This comment has been minimized.

Copy link
Member Author

commented Jul 30, 2014

made all the CORS configuration options constants in NettyHttpServerTransport

@kimchy

This comment has been minimized.

Copy link
Member

commented Aug 5, 2014

LGTM

CORS: Allowed to configure allow-credentials header to work via SSL
This adds support to return the "Access-Control-Allow-Credentials" header
if needed, so CORS will work flawlessly with authenticated applications.

Closes #6380

@spinscale spinscale merged commit 35e67c8 into elastic:master Aug 5, 2014

@spinscale spinscale removed review labels Aug 5, 2014

@clintongormley clintongormley changed the title CORS: Allowed to configure allow-credentials header to work via SSL Security: Allow to configure CORS allow-credentials header to work via SSL Sep 10, 2014

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.