diff --git a/modules/reindex/src/test/java/org/elasticsearch/index/reindex/ReindexFromRemoteWithAuthTests.java b/modules/reindex/src/test/java/org/elasticsearch/index/reindex/ReindexFromRemoteWithAuthTests.java index 9c017cb2ded..0457856f0f1 100644 --- a/modules/reindex/src/test/java/org/elasticsearch/index/reindex/ReindexFromRemoteWithAuthTests.java +++ b/modules/reindex/src/test/java/org/elasticsearch/index/reindex/ReindexFromRemoteWithAuthTests.java @@ -43,6 +43,7 @@ import org.elasticsearch.env.Environment; import org.elasticsearch.env.NodeEnvironment; import org.elasticsearch.plugins.ActionPlugin; import org.elasticsearch.plugins.Plugin; +import org.elasticsearch.rest.RestHeaderDefinition; import org.elasticsearch.rest.RestStatus; import org.elasticsearch.script.ScriptService; import org.elasticsearch.tasks.Task; @@ -161,8 +162,9 @@ public class ReindexFromRemoteWithAuthTests extends ESSingleNodeTestCase { } @Override - public Collection getRestHeaders() { - return Arrays.asList(TestFilter.AUTHORIZATION_HEADER, TestFilter.EXAMPLE_HEADER); + public Collection getRestHeaders() { + return Arrays.asList(new RestHeaderDefinition(TestFilter.AUTHORIZATION_HEADER, false), + new RestHeaderDefinition(TestFilter.EXAMPLE_HEADER, false)); } } diff --git a/qa/smoke-test-http/src/test/java/org/elasticsearch/http/ContextAndHeaderTransportIT.java b/qa/smoke-test-http/src/test/java/org/elasticsearch/http/ContextAndHeaderTransportIT.java index 04f01cf0f0e..eeec2c48059 100644 --- a/qa/smoke-test-http/src/test/java/org/elasticsearch/http/ContextAndHeaderTransportIT.java +++ b/qa/smoke-test-http/src/test/java/org/elasticsearch/http/ContextAndHeaderTransportIT.java @@ -50,6 +50,7 @@ import org.elasticsearch.index.query.TermsQueryBuilder; import org.elasticsearch.indices.TermsLookup; import org.elasticsearch.plugins.ActionPlugin; import org.elasticsearch.plugins.Plugin; +import org.elasticsearch.rest.RestHeaderDefinition; import org.elasticsearch.script.ScriptService; import org.elasticsearch.test.ESIntegTestCase.ClusterScope; import org.elasticsearch.threadpool.ThreadPool; @@ -81,7 +82,7 @@ import static org.hamcrest.Matchers.is; @ClusterScope(scope = SUITE) public class ContextAndHeaderTransportIT extends HttpSmokeTestCase { private static final List requests = new CopyOnWriteArrayList<>(); - private static final String CUSTOM_HEADER = "SomeCustomHeader"; + private static final RestHeaderDefinition CUSTOM_HEADER = new RestHeaderDefinition("SomeCustomHeader", false); private String randomHeaderValue = randomAlphaOfLength(20); private String queryIndex = "query-" + randomAlphaOfLength(10).toLowerCase(Locale.ROOT); private String lookupIndex = "lookup-" + randomAlphaOfLength(10).toLowerCase(Locale.ROOT); @@ -223,7 +224,7 @@ public class ContextAndHeaderTransportIT extends HttpSmokeTestCase { final String IRRELEVANT_HEADER = "SomeIrrelevantHeader"; Request request = new Request("GET", "/" + queryIndex + "/_search"); RequestOptions.Builder options = request.getOptions().toBuilder(); - options.addHeader(CUSTOM_HEADER, randomHeaderValue); + options.addHeader(CUSTOM_HEADER.getName(), randomHeaderValue); options.addHeader(IRRELEVANT_HEADER, randomHeaderValue); request.setOptions(options); Response response = getRestClient().performRequest(request); @@ -231,7 +232,7 @@ public class ContextAndHeaderTransportIT extends HttpSmokeTestCase { List searchRequests = getRequests(SearchRequest.class); assertThat(searchRequests, hasSize(greaterThan(0))); for (RequestAndHeaders requestAndHeaders : searchRequests) { - assertThat(requestAndHeaders.headers.containsKey(CUSTOM_HEADER), is(true)); + assertThat(requestAndHeaders.headers.containsKey(CUSTOM_HEADER.getName()), is(true)); // was not specified, thus is not included assertThat(requestAndHeaders.headers.containsKey(IRRELEVANT_HEADER), is(false)); } @@ -272,21 +273,22 @@ public class ContextAndHeaderTransportIT extends HttpSmokeTestCase { } private void assertRequestContainsHeader(ActionRequest request, Map context) { - String msg = String.format(Locale.ROOT, "Expected header %s to be in request %s", CUSTOM_HEADER, request.getClass().getName()); + String msg = String.format(Locale.ROOT, "Expected header %s to be in request %s", CUSTOM_HEADER.getName(), + request.getClass().getName()); if (request instanceof IndexRequest) { IndexRequest indexRequest = (IndexRequest) request; - msg = String.format(Locale.ROOT, "Expected header %s to be in index request %s/%s/%s", CUSTOM_HEADER, + msg = String.format(Locale.ROOT, "Expected header %s to be in index request %s/%s/%s", CUSTOM_HEADER.getName(), indexRequest.index(), indexRequest.type(), indexRequest.id()); } - assertThat(msg, context.containsKey(CUSTOM_HEADER), is(true)); - assertThat(context.get(CUSTOM_HEADER).toString(), is(randomHeaderValue)); + assertThat(msg, context.containsKey(CUSTOM_HEADER.getName()), is(true)); + assertThat(context.get(CUSTOM_HEADER.getName()).toString(), is(randomHeaderValue)); } /** * a transport client that adds our random header */ private Client transportClient() { - return internalCluster().transportClient().filterWithHeader(Collections.singletonMap(CUSTOM_HEADER, randomHeaderValue)); + return internalCluster().transportClient().filterWithHeader(Collections.singletonMap(CUSTOM_HEADER.getName(), randomHeaderValue)); } public static class ActionLoggingPlugin extends Plugin implements ActionPlugin { @@ -340,7 +342,7 @@ public class ContextAndHeaderTransportIT extends HttpSmokeTestCase { } public static class CustomHeadersPlugin extends Plugin implements ActionPlugin { - public Collection getRestHeaders() { + public Collection getRestHeaders() { return Collections.singleton(CUSTOM_HEADER); } } diff --git a/server/src/main/java/org/elasticsearch/action/ActionModule.java b/server/src/main/java/org/elasticsearch/action/ActionModule.java index d5fcc5623b8..8727179e848 100644 --- a/server/src/main/java/org/elasticsearch/action/ActionModule.java +++ b/server/src/main/java/org/elasticsearch/action/ActionModule.java @@ -227,6 +227,7 @@ import org.elasticsearch.plugins.ActionPlugin; import org.elasticsearch.plugins.ActionPlugin.ActionHandler; import org.elasticsearch.rest.RestController; import org.elasticsearch.rest.RestHandler; +import org.elasticsearch.rest.RestHeaderDefinition; import org.elasticsearch.rest.action.RestFieldCapabilitiesAction; import org.elasticsearch.rest.action.RestMainAction; import org.elasticsearch.rest.action.admin.cluster.RestAddVotingConfigExclusionAction; @@ -389,9 +390,9 @@ public class ActionModule extends AbstractModule { actionFilters = setupActionFilters(actionPlugins); autoCreateIndex = transportClient ? null : new AutoCreateIndex(settings, clusterSettings, indexNameExpressionResolver); destructiveOperations = new DestructiveOperations(settings, clusterSettings); - Set headers = Stream.concat( + Set headers = Stream.concat( actionPlugins.stream().flatMap(p -> p.getRestHeaders().stream()), - Stream.of(Task.X_OPAQUE_ID) + Stream.of(new RestHeaderDefinition(Task.X_OPAQUE_ID, false)) ).collect(Collectors.toSet()); UnaryOperator restWrapper = null; for (ActionPlugin plugin : actionPlugins) { diff --git a/server/src/main/java/org/elasticsearch/plugins/ActionPlugin.java b/server/src/main/java/org/elasticsearch/plugins/ActionPlugin.java index f440cc8c79a..f6f2a9c5064 100644 --- a/server/src/main/java/org/elasticsearch/plugins/ActionPlugin.java +++ b/server/src/main/java/org/elasticsearch/plugins/ActionPlugin.java @@ -38,6 +38,7 @@ import org.elasticsearch.common.settings.SettingsFilter; import org.elasticsearch.common.util.concurrent.ThreadContext; import org.elasticsearch.rest.RestController; import org.elasticsearch.rest.RestHandler; +import org.elasticsearch.rest.RestHeaderDefinition; import java.util.Collection; import java.util.Collections; @@ -93,7 +94,7 @@ public interface ActionPlugin { /** * Returns headers which should be copied through rest requests on to internal requests. */ - default Collection getRestHeaders() { + default Collection getRestHeaders() { return Collections.emptyList(); } diff --git a/server/src/main/java/org/elasticsearch/rest/RestController.java b/server/src/main/java/org/elasticsearch/rest/RestController.java index 96b95491319..1f29e9ed0a8 100644 --- a/server/src/main/java/org/elasticsearch/rest/RestController.java +++ b/server/src/main/java/org/elasticsearch/rest/RestController.java @@ -52,6 +52,7 @@ import java.util.Set; import java.util.concurrent.atomic.AtomicBoolean; import java.util.function.Supplier; import java.util.function.UnaryOperator; +import java.util.stream.Collectors; import static org.elasticsearch.rest.BytesRestResponse.TEXT_CONTENT_TYPE; import static org.elasticsearch.rest.RestStatus.BAD_REQUEST; @@ -73,10 +74,10 @@ public class RestController implements HttpServerTransport.Dispatcher { private final CircuitBreakerService circuitBreakerService; /** Rest headers that are copied to internal requests made during a rest request. */ - private final Set headersToCopy; + private final Set headersToCopy; private final UsageService usageService; - public RestController(Set headersToCopy, UnaryOperator handlerWrapper, + public RestController(Set headersToCopy, UnaryOperator handlerWrapper, NodeClient client, CircuitBreakerService circuitBreakerService, UsageService usageService) { this.headersToCopy = headersToCopy; this.usageService = usageService; @@ -257,10 +258,19 @@ public class RestController implements HttpServerTransport.Dispatcher { } private void tryAllHandlers(final RestRequest request, final RestChannel channel, final ThreadContext threadContext) throws Exception { - for (String key : headersToCopy) { - String httpHeader = request.header(key); - if (httpHeader != null) { - threadContext.putHeader(key, httpHeader); + for (final RestHeaderDefinition restHeader : headersToCopy) { + final String name = restHeader.getName(); + final List headerValues = request.getAllHeaderValues(name); + if (headerValues != null && headerValues.isEmpty() == false) { + final List distinctHeaderValues = headerValues.stream().distinct().collect(Collectors.toList()); + if (restHeader.isMultiValueAllowed() == false && distinctHeaderValues.size() > 1) { + channel.sendResponse( + BytesRestResponse. + createSimpleErrorResponse(channel, BAD_REQUEST, "multiple values for single-valued header [" + name + "].")); + return; + } else { + threadContext.putHeader(name, String.join(",", distinctHeaderValues)); + } } } // error_trace cannot be used when we disable detailed errors diff --git a/server/src/main/java/org/elasticsearch/rest/RestHeaderDefinition.java b/server/src/main/java/org/elasticsearch/rest/RestHeaderDefinition.java new file mode 100644 index 00000000000..6fb95ba80dd --- /dev/null +++ b/server/src/main/java/org/elasticsearch/rest/RestHeaderDefinition.java @@ -0,0 +1,46 @@ +/* + * Licensed to Elasticsearch under one or more contributor + * license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright + * ownership. Elasticsearch licenses this file to you 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.elasticsearch.rest; + +/** + * A definition for an http header that should be copied to the {@link org.elasticsearch.common.util.concurrent.ThreadContext} when + * reading the request on the rest layer. + */ +public final class RestHeaderDefinition { + private final String name; + /** + * This should be set to true only when the syntax of the value of the Header to copy is defined as a comma separated list of String + * values. + */ + private final boolean multiValueAllowed; + + public RestHeaderDefinition(String name, boolean multiValueAllowed) { + this.name = name; + this.multiValueAllowed = multiValueAllowed; + } + + public String getName() { + return name; + } + + public boolean isMultiValueAllowed() { + return multiValueAllowed; + } +} diff --git a/server/src/test/java/org/elasticsearch/rest/RestControllerTests.java b/server/src/test/java/org/elasticsearch/rest/RestControllerTests.java index d097afad005..d9e7ede5df9 100644 --- a/server/src/test/java/org/elasticsearch/rest/RestControllerTests.java +++ b/server/src/test/java/org/elasticsearch/rest/RestControllerTests.java @@ -104,7 +104,8 @@ public class RestControllerTests extends ESTestCase { public void testApplyRelevantHeaders() throws Exception { final ThreadContext threadContext = new ThreadContext(Settings.EMPTY); - Set headers = new HashSet<>(Arrays.asList("header.1", "header.2")); + Set headers = new HashSet<>(Arrays.asList(new RestHeaderDefinition("header.1", true), + new RestHeaderDefinition("header.2", true))); final RestController restController = new RestController(headers, null, null, circuitBreakerService, usageService); Map> restHeaders = new HashMap<>(); restHeaders.put("header.1", Collections.singletonList("true")); @@ -137,6 +138,40 @@ public class RestControllerTests extends ESTestCase { assertNull(threadContext.getHeader("header.3")); } + public void testRequestWithDisallowedMultiValuedHeader() { + final ThreadContext threadContext = new ThreadContext(Settings.EMPTY); + Set headers = new HashSet<>(Arrays.asList(new RestHeaderDefinition("header.1", true), + new RestHeaderDefinition("header.2", false))); + final RestController restController = new RestController(headers, null, null, circuitBreakerService, usageService); + Map> restHeaders = new HashMap<>(); + restHeaders.put("header.1", Collections.singletonList("boo")); + restHeaders.put("header.2", Arrays.asList("foo", "bar")); + RestRequest fakeRequest = new FakeRestRequest.Builder(xContentRegistry()).withHeaders(restHeaders).build(); + AssertingChannel channel = new AssertingChannel(fakeRequest, false, RestStatus.BAD_REQUEST); + restController.dispatchRequest(fakeRequest, channel, threadContext); + assertTrue(channel.getSendResponseCalled()); + } + + public void testRequestWithDisallowedMultiValuedHeaderButSameValues() { + final ThreadContext threadContext = new ThreadContext(Settings.EMPTY); + Set headers = new HashSet<>(Arrays.asList(new RestHeaderDefinition("header.1", true), + new RestHeaderDefinition("header.2", false))); + final RestController restController = new RestController(headers, null, null, circuitBreakerService, usageService); + Map> restHeaders = new HashMap<>(); + restHeaders.put("header.1", Collections.singletonList("boo")); + restHeaders.put("header.2", Arrays.asList("foo", "foo")); + RestRequest fakeRequest = new FakeRestRequest.Builder(xContentRegistry()).withHeaders(restHeaders).withPath("/bar").build(); + restController.registerHandler(RestRequest.Method.GET, "/bar", new RestHandler() { + @Override + public void handleRequest(RestRequest request, RestChannel channel, NodeClient client) throws Exception { + channel.sendResponse(new BytesRestResponse(RestStatus.OK, BytesRestResponse.TEXT_CONTENT_TYPE, BytesArray.EMPTY)); + } + }); + AssertingChannel channel = new AssertingChannel(fakeRequest, false, RestStatus.OK); + restController.dispatchRequest(fakeRequest, channel, threadContext); + assertTrue(channel.getSendResponseCalled()); + } + public void testRegisterAsDeprecatedHandler() { RestController controller = mock(RestController.class); diff --git a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/LocalStateCompositeXPackPlugin.java b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/LocalStateCompositeXPackPlugin.java index 350840883a7..96d6d80d13a 100644 --- a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/LocalStateCompositeXPackPlugin.java +++ b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/LocalStateCompositeXPackPlugin.java @@ -63,6 +63,7 @@ import org.elasticsearch.plugins.ScriptPlugin; import org.elasticsearch.repositories.Repository; import org.elasticsearch.rest.RestController; import org.elasticsearch.rest.RestHandler; +import org.elasticsearch.rest.RestHeaderDefinition; import org.elasticsearch.script.ScriptContext; import org.elasticsearch.script.ScriptService; import org.elasticsearch.threadpool.ExecutorBuilder; @@ -163,8 +164,8 @@ public class LocalStateCompositeXPackPlugin extends XPackPlugin implements Scrip } @Override - public Collection getRestHeaders() { - List headers = new ArrayList<>(); + public Collection getRestHeaders() { + List headers = new ArrayList<>(); headers.addAll(super.getRestHeaders()); filterPlugins(ActionPlugin.class).stream().forEach(p -> headers.addAll(p.getRestHeaders())); return headers; diff --git a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/Security.java b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/Security.java index 6eccfb251b9..66354186c5e 100644 --- a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/Security.java +++ b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/Security.java @@ -59,6 +59,7 @@ import org.elasticsearch.plugins.NetworkPlugin; import org.elasticsearch.plugins.Plugin; import org.elasticsearch.rest.RestController; import org.elasticsearch.rest.RestHandler; +import org.elasticsearch.rest.RestHeaderDefinition; import org.elasticsearch.script.ScriptService; import org.elasticsearch.threadpool.ExecutorBuilder; import org.elasticsearch.threadpool.FixedExecutorBuilder; @@ -670,17 +671,17 @@ public class Security extends Plugin implements ActionPlugin, IngestPlugin, Netw } @Override - public Collection getRestHeaders() { + public Collection getRestHeaders() { if (transportClientMode) { return Collections.emptyList(); } - Set headers = new HashSet<>(); - headers.add(UsernamePasswordToken.BASIC_AUTH_HEADER); + Set headers = new HashSet<>(); + headers.add(new RestHeaderDefinition(UsernamePasswordToken.BASIC_AUTH_HEADER, false)); if (XPackSettings.AUDIT_ENABLED.get(settings)) { - headers.add(AuditTrail.X_FORWARDED_FOR_HEADER); + headers.add(new RestHeaderDefinition(AuditTrail.X_FORWARDED_FOR_HEADER, true)); } if (AuthenticationServiceField.RUN_AS_ENABLED.get(settings)) { - headers.add(AuthenticationServiceField.RUN_AS_USER_HEADER); + headers.add(new RestHeaderDefinition(AuthenticationServiceField.RUN_AS_USER_HEADER, false)); } return headers; } diff --git a/x-pack/qa/security-example-spi-extension/src/main/java/org/elasticsearch/example/SpiExtensionPlugin.java b/x-pack/qa/security-example-spi-extension/src/main/java/org/elasticsearch/example/SpiExtensionPlugin.java index 9314b6a6750..eedb06f2c1b 100644 --- a/x-pack/qa/security-example-spi-extension/src/main/java/org/elasticsearch/example/SpiExtensionPlugin.java +++ b/x-pack/qa/security-example-spi-extension/src/main/java/org/elasticsearch/example/SpiExtensionPlugin.java @@ -9,6 +9,7 @@ import org.elasticsearch.common.settings.Setting; import org.elasticsearch.example.realm.CustomRealm; import org.elasticsearch.plugins.ActionPlugin; import org.elasticsearch.plugins.Plugin; +import org.elasticsearch.rest.RestHeaderDefinition; import org.elasticsearch.xpack.core.security.authc.RealmSettings; import java.util.ArrayList; @@ -22,8 +23,10 @@ import java.util.List; public class SpiExtensionPlugin extends Plugin implements ActionPlugin { @Override - public Collection getRestHeaders() { - return Arrays.asList(CustomRealm.USER_HEADER, CustomRealm.PW_HEADER); + public Collection getRestHeaders() { + return Arrays.asList( + new RestHeaderDefinition(CustomRealm.USER_HEADER, false), + new RestHeaderDefinition(CustomRealm.PW_HEADER, false)); } @Override