Skip to content

Fixes CORS headers needed by Elastic clients #85791

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.

Sign up for GitHub

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

Merged
merged 3 commits into from
Feb 9, 2023
Merged

Fixes CORS headers needed by Elastic clients #85791

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
d622da0
Fixes CORS headers needed by Elastic clients
swallez Apr 11, 2022
c6acd46
Update docs/changelog/85791.yaml
swallez Apr 11, 2022
8a53e3c
Merge branch 'main' into fix-cors-for-clients
swallez Jan 24, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions docs/changelog/85791.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
pr: 85791
summary: Fixes CORS headers needed by Elastic clients
area: Infra/REST API
type: bug
issues: []
9 changes: 8 additions & 1 deletion docs/reference/modules/http.asciidoc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -119,9 +119,16 @@ Which methods to allow. Defaults to `OPTIONS, HEAD, GET, POST, PUT, DELETE`.
// tag::http-cors-allow-headers-tag[]
`http.cors.allow-headers` {ess-icon}::
(<<static-cluster-setting,Static>>, string)
Which headers to allow. Defaults to `X-Requested-With, Content-Type, Content-Length`.
Which headers to allow. Defaults to `X-Requested-With, Content-Type, Content-Length, Authorization, Accept, User-Agent, X-Elastic-Client-Meta`.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I find it strange that we default access-control-allow-headers to include Authorization but access-control-allow-credentials to false?

Is that intentional?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, it is intentional:

  • access-control-allow-headers, in a response to a preflight request, defines what headers (outside of the safelist) are allowed in CORS requests. Since clients set the Authorization header in their requests, it has to be allowed by the server in preflight responses.
  • Defaulting access-control-allow-credentials to false forbids the use of browser-level credentials by clients, and thus requires clients to be provided with credential information that is then set in the Authorization header.

CORS is sometimes confusing 😅

// end::http-cors-allow-headers-tag[]

[[http-cors-expose-headers]]
// tag::http-cors-expose-headers-tag[]
`http.cors.expose-headers` {ess-icon}::
(<<static-cluster-setting,Static>>)
Which response headers to expose in the client. Defaults to `X-elastic-product`.
// end::http-cors-expose-headers-tag[]

[[http-cors-allow-credentials]]
// tag::http-cors-allow-credentials-tag[]
`http.cors.allow-credentials` {ess-icon}::
Expand Down
20 changes: 20 additions & 0 deletions server/src/main/java/org/elasticsearch/http/CorsHandler.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,7 @@
import static org.elasticsearch.http.HttpTransportSettings.SETTING_CORS_ALLOW_METHODS;
import static org.elasticsearch.http.HttpTransportSettings.SETTING_CORS_ALLOW_ORIGIN;
import static org.elasticsearch.http.HttpTransportSettings.SETTING_CORS_ENABLED;
import static org.elasticsearch.http.HttpTransportSettings.SETTING_CORS_EXPOSE_HEADERS;
import static org.elasticsearch.http.HttpTransportSettings.SETTING_CORS_MAX_AGE;

/**
Expand All @@ -77,6 +78,7 @@ public class CorsHandler {
public static final String ACCESS_CONTROL_ALLOW_METHODS = "access-control-allow-methods";
public static final String ACCESS_CONTROL_ALLOW_ORIGIN = "access-control-allow-origin";
public static final String ACCESS_CONTROL_MAX_AGE = "access-control-max-age";
public static final String ACCESS_CONTROL_EXPOSE_HEADERS = "access-control-expose-headers";

private static final Pattern SCHEME_PATTERN = Pattern.compile("^https?://");
private static final DateTimeFormatter dateTimeFormatter = DateTimeFormatter.ofPattern("EEE, dd MMM yyyy HH:mm:ss O", Locale.ENGLISH);
Expand Down Expand Up @@ -105,6 +107,7 @@ public void setCorsResponseHeaders(final HttpRequest httpRequest, final HttpResp
}
if (setOrigin(httpRequest, httpResponse)) {
setAllowCredentials(httpResponse);
setExposeHeaders(httpResponse);
}
}

Expand Down Expand Up @@ -228,6 +231,12 @@ private void setAllowHeaders(final HttpResponse response) {
}
}

private void setExposeHeaders(final HttpResponse response) {
for (String header : config.accessControlExposeHeaders) {
response.addHeader(ACCESS_CONTROL_EXPOSE_HEADERS, header);
}
}

private void setAllowCredentials(final HttpResponse response) {
if (config.isCredentialsAllowed()) {
response.addHeader(ACCESS_CONTROL_ALLOW_CREDENTIALS, "true");
Expand All @@ -247,6 +256,7 @@ public static class Config {
private final boolean credentialsAllowed;
private final Set<RestRequest.Method> allowedRequestMethods;
private final Set<String> allowedRequestHeaders;
private final Set<String> accessControlExposeHeaders;
private final long maxAge;

public Config(Builder builder) {
Expand All @@ -257,6 +267,7 @@ public Config(Builder builder) {
this.credentialsAllowed = builder.allowCredentials;
this.allowedRequestMethods = Collections.unmodifiableSet(builder.requestMethods);
this.allowedRequestHeaders = Collections.unmodifiableSet(builder.requestHeaders);
this.accessControlExposeHeaders = Collections.unmodifiableSet(builder.accessControlExposeHeaders);
this.maxAge = builder.maxAge;
}

Expand Down Expand Up @@ -314,6 +325,8 @@ public String toString() {
+ allowedRequestMethods
+ ", allowedRequestHeaders="
+ allowedRequestHeaders
+ ", accessControlExposeHeaders="
+ accessControlExposeHeaders
+ ", maxAge="
+ maxAge
+ '}';
Expand All @@ -329,6 +342,7 @@ private static class Builder {
long maxAge;
private final Set<RestRequest.Method> requestMethods = new HashSet<>();
private final Set<String> requestHeaders = new HashSet<>();
private final Set<String> accessControlExposeHeaders = new HashSet<>();

private Builder() {
anyOrigin = true;
Expand Down Expand Up @@ -380,6 +394,11 @@ public Builder allowedRequestHeaders(String[] headers) {
return this;
}

public Builder accessControlExposeHeaders(String[] headers) {
accessControlExposeHeaders.addAll(Arrays.asList(headers));
return this;
}

public Config build() {
return new Config(this);
}
Expand Down Expand Up @@ -427,6 +446,7 @@ public static Config buildConfig(Settings settings) {
Config config = builder.allowedRequestMethods(methods)
.maxAge(SETTING_CORS_MAX_AGE.get(settings))
.allowedRequestHeaders(Strings.tokenizeToStringArray(SETTING_CORS_ALLOW_HEADERS.get(settings), ","))
.accessControlExposeHeaders(Strings.tokenizeToStringArray(SETTING_CORS_EXPOSE_HEADERS.get(settings), ","))
.build();
return config;
}
Expand Down
8 changes: 7 additions & 1 deletion server/src/main/java/org/elasticsearch/http/HttpTransportSettings.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,13 @@ public final class HttpTransportSettings {
);
public static final Setting<String> SETTING_CORS_ALLOW_HEADERS = new Setting<>(
"http.cors.allow-headers",
"X-Requested-With,Content-Type,Content-Length",
"X-Requested-With,Content-Type,Content-Length,Authorization,Accept,User-Agent,X-Elastic-Client-Meta",
(value) -> value,
Property.NodeScope
);
public static final Setting<String> SETTING_CORS_EXPOSE_HEADERS = new Setting<>(
"http.cors.expose-headers",
"X-elastic-product",
(value) -> value,
Property.NodeScope
);
Expand Down
24 changes: 22 additions & 2 deletions server/src/test/java/org/elasticsearch/http/CorsHandlerTests.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -204,7 +204,15 @@ public void testHandleInboundPreflightWithWildcardAllowCredentials() {
assertThat(headers.get(CorsHandler.ACCESS_CONTROL_ALLOW_METHODS), containsInAnyOrder("HEAD", "OPTIONS", "GET", "DELETE", "POST"));
assertThat(
headers.get(CorsHandler.ACCESS_CONTROL_ALLOW_HEADERS),
containsInAnyOrder("X-Requested-With", "Content-Type", "Content-Length")
containsInAnyOrder(
"X-Requested-With",
"Content-Type",
"Content-Length",
"Authorization",
"Accept",
"User-Agent",
"X-Elastic-Client-Meta"
)
);
assertThat(headers.get(CorsHandler.ACCESS_CONTROL_ALLOW_CREDENTIALS), containsInAnyOrder("true"));
assertThat(headers.get(CorsHandler.ACCESS_CONTROL_MAX_AGE), containsInAnyOrder("1728000"));
Expand Down Expand Up @@ -232,7 +240,15 @@ public void testHandleInboundPreflightWithValidOriginAllowCredentials() {
assertThat(headers.get(CorsHandler.ACCESS_CONTROL_ALLOW_METHODS), containsInAnyOrder("HEAD", "OPTIONS", "GET", "DELETE", "POST"));
assertThat(
headers.get(CorsHandler.ACCESS_CONTROL_ALLOW_HEADERS),
containsInAnyOrder("X-Requested-With", "Content-Type", "Content-Length")
containsInAnyOrder(
"X-Requested-With",
"Content-Type",
"Content-Length",
"Authorization",
"Accept",
"User-Agent",
"X-Elastic-Client-Meta"
)
);
assertThat(headers.get(CorsHandler.ACCESS_CONTROL_ALLOW_CREDENTIALS), containsInAnyOrder("true"));
assertThat(headers.get(CorsHandler.ACCESS_CONTROL_MAX_AGE), containsInAnyOrder("1728000"));
Expand All @@ -254,6 +270,7 @@ public void testSetResponseNonCorsRequest() {

Map<String, List<String>> headers = response.headers();
assertNull(headers.get(CorsHandler.ACCESS_CONTROL_ALLOW_ORIGIN));
assertNull(headers.get(CorsHandler.ACCESS_CONTROL_EXPOSE_HEADERS));
}

public void testSetResponseHeadersWithWildcardOrigin() {
Expand All @@ -270,6 +287,7 @@ public void testSetResponseHeadersWithWildcardOrigin() {

Map<String, List<String>> headers = response.headers();
assertThat(headers.get(CorsHandler.ACCESS_CONTROL_ALLOW_ORIGIN), containsInAnyOrder("*"));
assertThat(headers.get(CorsHandler.ACCESS_CONTROL_EXPOSE_HEADERS), containsInAnyOrder("X-elastic-product"));
assertNull(headers.get(CorsHandler.VARY));
}

Expand All @@ -288,6 +306,7 @@ public void testSetResponseHeadersWithCredentialsWithWildcard() {

Map<String, List<String>> headers = response.headers();
assertThat(headers.get(CorsHandler.ACCESS_CONTROL_ALLOW_ORIGIN), containsInAnyOrder("valid-origin"));
assertThat(headers.get(CorsHandler.ACCESS_CONTROL_EXPOSE_HEADERS), containsInAnyOrder("X-elastic-product"));
assertThat(headers.get(CorsHandler.ACCESS_CONTROL_ALLOW_CREDENTIALS), containsInAnyOrder("true"));
assertThat(headers.get(CorsHandler.VARY), containsInAnyOrder(CorsHandler.ORIGIN));
}
Expand All @@ -308,6 +327,7 @@ public void testSetResponseHeadersWithNonWildcardOrigin() {

Map<String, List<String>> headers = response.headers();
assertThat(headers.get(CorsHandler.ACCESS_CONTROL_ALLOW_ORIGIN), containsInAnyOrder("valid-origin"));
assertThat(headers.get(CorsHandler.ACCESS_CONTROL_EXPOSE_HEADERS), containsInAnyOrder("X-elastic-product"));
assertThat(headers.get(CorsHandler.VARY), containsInAnyOrder(CorsHandler.ORIGIN));
if (allowCredentials) {
assertThat(headers.get(CorsHandler.ACCESS_CONTROL_ALLOW_CREDENTIALS), containsInAnyOrder("true"));
Expand Down