Handle Unauthenticated OPTIONS requests

server/src/main/java/org/elasticsearch/rest/RestController.java

@@ -263,6 +263,8 @@ private void registerHandlerNoWrap(RestRequest.Method method, String path, RestA
         if (RESERVED_PATHS.contains(path)) {
             throw new IllegalArgumentException("path [" + path + "] is a reserved path and may not be registered");
         }
+        // the HTTP OPTIONS method is treated internally, not by handlers, see {@code #handleNoHandlerFound}
+        assert method != RestRequest.Method.OPTIONS : "There should be no handlers registered for the OPTIONS HTTP method";
         handlers.insertOrUpdate(
             path,
             new MethodHandlers(path).addMethod(method, version, handler),

server/src/test/java/org/elasticsearch/rest/RestControllerTests.java

@@ -88,6 +88,7 @@ public class RestControllerTests extends ESTestCase {
     private UsageService usageService;
     private NodeClient client;
     private Tracer tracer;
+    private List<RestRequest.Method> methodList;
 
     @Before
     public void setup() {
@@ -119,6 +120,7 @@ public void setup() {
             (request, channel, client) -> { throw new IllegalArgumentException("test error"); }
         );
         httpServerTransport.start();
+        methodList = Arrays.stream(RestRequest.Method.values()).filter(m -> m != OPTIONS).toList();
     }
 
     @After
@@ -207,7 +209,7 @@ public void testRequestWithDisallowedMultiValuedHeaderButSameValues() {
     public void testRegisterAsDeprecatedHandler() {
         RestController controller = mock(RestController.class);
 
-        RestRequest.Method method = randomFrom(RestRequest.Method.values());
+        RestRequest.Method method = randomFrom(methodList);
         String path = "/_" + randomAlphaOfLengthBetween(1, 6);
         RestHandler handler = (request, channel, client) -> {};
         String deprecationMessage = randomAlphaOfLengthBetween(1, 10);
@@ -228,10 +230,10 @@ public void testRegisterAsDeprecatedHandler() {
     public void testRegisterAsReplacedHandler() {
         final RestController controller = mock(RestController.class);
 
-        final RestRequest.Method method = randomFrom(RestRequest.Method.values());
+        final RestRequest.Method method = randomFrom(methodList);
         final String path = "/_" + randomAlphaOfLengthBetween(1, 6);
         final RestHandler handler = (request, channel, client) -> {};
-        final RestRequest.Method replacedMethod = randomFrom(RestRequest.Method.values());
+        final RestRequest.Method replacedMethod = randomFrom(methodList);
         final String replacedPath = "/_" + randomAlphaOfLengthBetween(1, 6);
         final RestApiVersion current = RestApiVersion.current();
         final RestApiVersion previous = RestApiVersion.previous();
@@ -261,8 +263,8 @@ public void testRegisterAsReplacedHandler() {
     public void testRegisterSecondMethodWithDifferentNamedWildcard() {
         final RestController restController = new RestController(null, null, circuitBreakerService, usageService, tracer, false);
 
-        RestRequest.Method firstMethod = randomFrom(RestRequest.Method.values());
-        RestRequest.Method secondMethod = randomFrom(Arrays.stream(RestRequest.Method.values()).filter(m -> m != firstMethod).toList());
+        RestRequest.Method firstMethod = randomFrom(methodList);
+        RestRequest.Method secondMethod = randomFrom(methodList.stream().filter(m -> m != firstMethod).toList());
 
         final String path = "/_" + randomAlphaOfLengthBetween(1, 6);
 
@@ -582,7 +584,7 @@ public void testDispatchBadRequest() {
     }
 
     public void testDoesNotConsumeContent() throws Exception {
-        final RestRequest.Method method = randomFrom(RestRequest.Method.values());
+        final RestRequest.Method method = randomFrom(methodList);
         restController.registerHandler(new Route(method, "/notconsumed"), new RestHandler() {
             @Override
             public void handleRequest(RestRequest request, RestChannel channel, NodeClient client) throws Exception {
@@ -895,7 +897,7 @@ public void testRegisterWithReservedPath() {
         for (String path : RestController.RESERVED_PATHS) {
             IllegalArgumentException iae = expectThrows(IllegalArgumentException.class, () -> {
                 restController.registerHandler(
-                    new Route(randomFrom(RestRequest.Method.values()), path),
+                    new Route(randomFrom(methodList), path),
                     (request, channel, client) -> channel.sendResponse(
                         new RestResponse(RestStatus.OK, RestResponse.TEXT_CONTENT_TYPE, BytesArray.EMPTY)
                     )

x-pack/plugin/security/src/internalClusterTest/java/org/elasticsearch/xpack/security/authc/HttOptionsNoAuthnIntegTests.java

@@ -0,0 +1,99 @@
+/*
+ * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
+ * or more contributor license agreements. Licensed under the Elastic License
+ * 2.0; you may not use this file except in compliance with the Elastic License
+ * 2.0.
+ */
+
+package org.elasticsearch.xpack.security.authc;
+
+import org.elasticsearch.client.Request;
+import org.elasticsearch.client.RequestOptions;
+import org.elasticsearch.client.Response;
+import org.elasticsearch.common.settings.SecureString;
+import org.elasticsearch.common.settings.Settings;
+import org.elasticsearch.http.CorsHandler;
+import org.elasticsearch.http.HttpTransportSettings;
+import org.elasticsearch.test.SecurityIntegTestCase;
+import org.elasticsearch.test.SecuritySettingsSource;
+import org.elasticsearch.xpack.core.security.authc.support.UsernamePasswordToken;
+
+import java.util.List;
+
+import static org.hamcrest.Matchers.is;
+import static org.hamcrest.Matchers.notNullValue;
+
+public class HttOptionsNoAuthnIntegTests extends SecurityIntegTestCase {
+
+    @Override
+    protected boolean addMockHttpTransport() {
+        return false; // need real http
+    }
+
+    @Override
+    protected Settings nodeSettings(int nodeOrdinal, Settings otherSettings) {
+        final Settings.Builder builder = Settings.builder().put(super.nodeSettings(nodeOrdinal, otherSettings));
+        // needed to test preflight requests
+        builder.put(HttpTransportSettings.SETTING_CORS_ENABLED.getKey(), "true")
+            .put(HttpTransportSettings.SETTING_CORS_ALLOW_ORIGIN.getKey(), "*");
+        return builder.build();
+    }
+
+    public void testNoAuthnForResourceOptionsMethod() throws Exception {
+        Request requestNoCredentials = new Request(
+            "OPTIONS",
+            randomFrom("/", "/_cluster/stats", "/some-index", "/index/_stats", "/_stats/flush")
+        );
+        // no "Authorization" request header -> request is unauthenticated
+        assertThat(requestNoCredentials.getOptions().getHeaders().isEmpty(), is(true));
+        // WRONG "Authorization" request header
+        Request requestWrongCredentials = new Request(
+            "OPTIONS",
+            randomFrom("/", "/_cluster/stats", "/some-index", "/index/_stats", "/_stats/flush")
+        );
+        RequestOptions.Builder options = requestWrongCredentials.getOptions().toBuilder();
+        options.addHeader(
+            "Authorization",
+            UsernamePasswordToken.basicAuthHeaderValue(SecuritySettingsSource.TEST_USER_NAME, new SecureString("WRONG"))
+        );
+        requestWrongCredentials.setOptions(options);
+        for (Request request : List.of(requestNoCredentials, requestWrongCredentials)) {
+            Response response = getRestClient().performRequest(request);
+            assertThat(response.getStatusLine().getStatusCode(), is(200));
+            assertThat(response.getHeader("Allow"), notNullValue());
+            assertThat(response.getHeader("X-elastic-product"), is("Elasticsearch"));
+            assertThat(response.getHeader("content-length"), is("0"));
+        }
+    }
+
+    public void testNoAuthnForPreFlightRequest() throws Exception {
+        Request requestNoCredentials = new Request(
+            "OPTIONS",
+            randomFrom("/", "/_cluster/stats", "/some-index", "/index/_stats", "/_stats/flush")
+        );
+        RequestOptions.Builder options = requestNoCredentials.getOptions().toBuilder();
+        options.addHeader(CorsHandler.ORIGIN, "google.com");
+        options.addHeader(CorsHandler.ACCESS_CONTROL_REQUEST_METHOD, "GET");
+        requestNoCredentials.setOptions(options);
+        // no "Authorization" request header -> request is unauthenticated
+        Request requestWrongCredentials = new Request(
+            "OPTIONS",
+            randomFrom("/", "/_cluster/stats", "/some-index", "/index/_stats", "/_stats/flush")
+        );
+        options = requestWrongCredentials.getOptions().toBuilder();
+        // WRONG "Authorization" request header
+        options.addHeader(
+            "Authorization",
+            UsernamePasswordToken.basicAuthHeaderValue(SecuritySettingsSource.TEST_USER_NAME, new SecureString("WRONG"))
+        );
+        options.addHeader(CorsHandler.ORIGIN, "google.com");
+        options.addHeader(CorsHandler.ACCESS_CONTROL_REQUEST_METHOD, "GET");
+        requestWrongCredentials.setOptions(options);
+        for (Request request : List.of(requestWrongCredentials)) {
+            Response response = getRestClient().performRequest(request);
+            assertThat(response.getStatusLine().getStatusCode(), is(200));
+            assertThat(response.getHeader("content-length"), is("0"));
+        }
+    }
+
+}

x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/Security.java

@@ -1656,10 +1656,16 @@ public boolean test(String profile, InetSocketAddress peerAddress) {
                 RemoteHostHeader.process(channel, threadContext);
                 // step 2: Run authentication on the now properly prepared thread-context.
                 // This inspects and modifies the thread context.
-                authenticationService.authenticate(
-                    httpPreRequest,
-                    ActionListener.wrap(ignored -> listener.onResponse(null), listener::onFailure)
-                );
+                if (httpPreRequest.method() != RestRequest.Method.OPTIONS) {
+                    authenticationService.authenticate(
+                        httpPreRequest,
+                        ActionListener.wrap(ignored -> listener.onResponse(null), listener::onFailure)
+                    );
+                } else {
+                    // allow for unauthenticated OPTIONS request
+                    // this includes CORS preflight, and regular OPTIONS that return permitted methods for a given path
+                    listener.onResponse(null);
+                }
             };
             return getHttpServerTransportWithHeadersValidator(
                 settings,

x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/rest/SecurityRestFilter.java

@@ -9,6 +9,7 @@
 import org.apache.logging.log4j.LogManager;
 import org.apache.logging.log4j.Logger;
 import org.apache.logging.log4j.util.Supplier;
+import org.elasticsearch.ElasticsearchSecurityException;
 import org.elasticsearch.action.ActionListener;
 import org.elasticsearch.client.internal.node.NodeClient;
 import org.elasticsearch.common.util.concurrent.ThreadContext;
@@ -60,9 +61,14 @@ public RestHandler getConcreteRestHandler() {
 
     @Override
     public void handleRequest(RestRequest request, RestChannel channel, NodeClient client) throws Exception {
+        // requests with the OPTIONS method should be handled elsewhere, and not by calling {@code RestHandler#handleRequest}
+        // authn is bypassed for HTTP requests with the OPTIONS method, so this sanity check prevents dispatching unauthenticated requests
         if (request.method() == Method.OPTIONS) {
-            // CORS - allow for preflight unauthenticated OPTIONS request
-            restHandler.handleRequest(request, channel, client);
+            handleException(
+                request,
+                channel,
+                new ElasticsearchSecurityException("Cannot dispatch OPTIONS request, as they are not authenticated")
+            );
             return;
         }
 

x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/rest/SecurityRestFilterTests.java

@@ -22,12 +22,15 @@
 import org.elasticsearch.rest.RestHandler;
 import org.elasticsearch.rest.RestRequest;
 import org.elasticsearch.rest.RestRequestFilter;
+import org.elasticsearch.rest.RestResponse;
+import org.elasticsearch.rest.RestStatus;
 import org.elasticsearch.test.ESTestCase;
 import org.elasticsearch.test.SecuritySettingsSourceField;
 import org.elasticsearch.test.rest.FakeRestRequest;
 import org.elasticsearch.xcontent.DeprecationHandler;
 import org.elasticsearch.xcontent.NamedXContentRegistry;
 import org.elasticsearch.xcontent.XContentType;
+import org.elasticsearch.xcontent.json.JsonXContent;
 import org.elasticsearch.xpack.core.security.SecurityContext;
 import org.elasticsearch.xpack.core.security.authc.Authentication;
 import org.elasticsearch.xpack.core.security.authc.AuthenticationTestHelper;
@@ -38,6 +41,7 @@
 import org.elasticsearch.xpack.security.authc.AuthenticationService;
 import org.elasticsearch.xpack.security.authc.support.SecondaryAuthenticator;
 import org.junit.Before;
+import org.mockito.ArgumentCaptor;
 
 import java.util.Base64;
 import java.util.Collections;
@@ -47,7 +51,9 @@
 import java.util.concurrent.atomic.AtomicReference;
 
 import static org.elasticsearch.test.ActionListenerUtils.anyActionListener;
+import static org.hamcrest.Matchers.containsString;
 import static org.hamcrest.Matchers.hasItem;
+import static org.hamcrest.Matchers.is;
 import static org.hamcrest.Matchers.not;
 import static org.hamcrest.Matchers.notNullValue;
 import static org.hamcrest.Matchers.sameInstance;
@@ -149,12 +155,17 @@ public void testProcessWithSecurityDisabled() throws Exception {
     }
 
     public void testProcessOptionsMethod() throws Exception {
-        RestRequest request = mock(RestRequest.class);
-        when(request.method()).thenReturn(RestRequest.Method.OPTIONS);
+        FakeRestRequest request = new FakeRestRequest.Builder(NamedXContentRegistry.EMPTY).withMethod(RestRequest.Method.OPTIONS).build();
+        when(channel.request()).thenReturn(request);
+        when(channel.newErrorBuilder()).thenReturn(JsonXContent.contentBuilder());
         filter.handleRequest(request, channel, null);
-        verify(restHandler).handleRequest(request, channel, null);
-        verifyNoMoreInteractions(channel);
+        verifyNoMoreInteractions(restHandler);
         verifyNoMoreInteractions(authcService);
+        ArgumentCaptor<RestResponse> responseArgumentCaptor = ArgumentCaptor.forClass(RestResponse.class);
+        verify(channel).sendResponse(responseArgumentCaptor.capture());
+        RestResponse restResponse = responseArgumentCaptor.getValue();
+        assertThat(restResponse.status(), is(RestStatus.INTERNAL_SERVER_ERROR));
+        assertThat(restResponse.content().utf8ToString(), containsString("Cannot dispatch OPTIONS request, as they are not authenticated"));
     }
 
     public void testProcessFiltersBodyCorrectly() throws Exception {