Mandate X-Pack REST handler installed

server/src/main/java/org/elasticsearch/action/ActionModule.java

@@ -235,8 +235,6 @@
 import org.elasticsearch.common.inject.AbstractModule;
 import org.elasticsearch.common.inject.TypeLiteral;
 import org.elasticsearch.common.inject.multibindings.MapBinder;
-import org.elasticsearch.common.logging.DeprecationCategory;
-import org.elasticsearch.common.logging.DeprecationLogger;
 import org.elasticsearch.common.settings.ClusterSettings;
 import org.elasticsearch.common.settings.IndexScopedSettings;
 import org.elasticsearch.common.settings.Settings;
@@ -403,7 +401,6 @@
 public class ActionModule extends AbstractModule {
 
     private static final Logger logger = LogManager.getLogger(ActionModule.class);
-    private static final DeprecationLogger deprecationLogger = DeprecationLogger.getLogger(logger.getName());
 
     private final Settings settings;
     private final IndexNameExpressionResolver indexNameExpressionResolver;
@@ -444,17 +441,15 @@ public ActionModule(Settings settings, IndexNameExpressionResolver indexNameExpr
             UnaryOperator<RestHandler> newRestWrapper = plugin.getRestHandlerWrapper(threadPool.getThreadContext());
             if (newRestWrapper != null) {
                 logger.debug("Using REST wrapper from plugin " + plugin.getClass().getName());
+                if (plugin.getClass().getCanonicalName() == null ||
+                    plugin.getClass().getCanonicalName().startsWith("org.elasticsearch.xpack") == false) {
+                    throw new IllegalArgumentException("The " + plugin.getClass().getName() + " plugin tried to install a custom REST " +
+                        "wrapper. This functionality is not available anymore.");
+                }
                 if (restWrapper != null) {
                     throw new IllegalArgumentException("Cannot have more than one plugin implementing a REST wrapper");
                 }
                 restWrapper = newRestWrapper;
-                if (restWrapper.getClass().getCanonicalName() == null ||
-                    restWrapper.getClass().getCanonicalName().startsWith("org.elasticsearch") == false) {
-                    deprecationLogger.deprecate(DeprecationCategory.PLUGINS, "3rd_party_rest_deprecation", "The " +
-                        plugin.getClass().getName() + "plugin installs a custom REST wrapper. This functionality is deprecated and will " +
-                        "not be possible in Elasticsearch 8.0. If this plugin is intended to provide security features for Elasticsearch " +
-                        "then you should switch to using the built-in Elasticsearch features instead.");
-                }
             }
         }
         mappingRequestValidators = new RequestValidators<>(

server/src/test/java/org/elasticsearch/action/ActionModuleTests.java

@@ -20,6 +20,7 @@
 import org.elasticsearch.common.settings.Settings;
 import org.elasticsearch.common.settings.SettingsFilter;
 import org.elasticsearch.common.settings.SettingsModule;
+import org.elasticsearch.common.util.concurrent.ThreadContext;
 import org.elasticsearch.indices.TestIndexNameExpressionResolver;
 import org.elasticsearch.plugins.ActionPlugin;
 import org.elasticsearch.plugins.ActionPlugin.ActionHandler;
@@ -34,10 +35,13 @@
 import org.elasticsearch.threadpool.TestThreadPool;
 import org.elasticsearch.threadpool.ThreadPool;
 import org.elasticsearch.usage.UsageService;
+import org.hamcrest.Matchers;
 
 import java.io.IOException;
+import java.util.Arrays;
 import java.util.List;
 import java.util.function.Supplier;
+import java.util.function.UnaryOperator;
 
 import static java.util.Collections.emptyList;
 import static java.util.Collections.singletonList;
@@ -194,4 +198,47 @@ public List<Route> routes() {
             threadPool.shutdown();
         }
     }
+
+    public void test3rdPartyHandlerIsNotInstalled() {
+        Settings settings = Settings.builder()
+            .put("xpack.security.enabled", false)
+            .put("path.home", createTempDir())
+            .build();
+
+        SettingsModule settingsModule = new SettingsModule(Settings.EMPTY);
+        ThreadPool threadPool = new TestThreadPool(getTestName());
+        ActionPlugin secPlugin = new SecPlugin();
+        try {
+            UsageService usageService = new UsageService();
+
+            Exception e = expectThrows(IllegalArgumentException.class, () ->
+                new ActionModule(settingsModule.getSettings(),
+                    TestIndexNameExpressionResolver.newInstance(threadPool.getThreadContext()),
+                    settingsModule.getIndexScopedSettings(), settingsModule.getClusterSettings(), settingsModule.getSettingsFilter(),
+                    threadPool, Arrays.asList(secPlugin), null, null, usageService, null)
+            );
+            assertThat(e.getMessage(), Matchers.equalTo("The org.elasticsearch.action.ActionModuleTests$SecPlugin plugin tried to " +
+                "install a custom REST wrapper. This functionality is not available anymore."));
+        } finally {
+            threadPool.shutdown();
+        }
+    }
+
+    class FakeHandler implements RestHandler {
+        @Override
+        public List<Route> routes() {
+            return singletonList(new Route(GET, "/_dummy"));
+        }
+
+        @Override
+        public void handleRequest(RestRequest request, RestChannel channel, NodeClient client) throws Exception {
+        }
+    }
+
+    class SecPlugin implements ActionPlugin {
+        @Override
+        public UnaryOperator<RestHandler> getRestHandlerWrapper(ThreadContext threadContext) {
+            return handler -> new FakeHandler();
+        }
+    };
 }

x-pack/plugin/security/qa/basic-enable-security/src/javaRestTest/java/org/elasticsearch/xpack/security/EnableSecurityOnBasicLicenseIT.java

@@ -8,7 +8,6 @@
 
 import org.apache.http.HttpHost;
 import org.apache.http.util.EntityUtils;
-import org.elasticsearch.Version;
 import org.elasticsearch.client.Request;
 import org.elasticsearch.client.Response;
 import org.elasticsearch.client.ResponseException;
@@ -21,7 +20,6 @@
 import org.elasticsearch.test.rest.ESRestTestCase;
 import org.elasticsearch.test.rest.yaml.ObjectPath;
 import org.elasticsearch.xpack.security.authc.InternalRealms;
-import org.hamcrest.Matchers;
 import org.junit.BeforeClass;
 
 import java.io.IOException;
@@ -106,22 +104,6 @@ public void testSecuritySetup() throws Exception {
         } else {
             checkAllowedWrite(otherIndex);
         }
-        checkSecurityDisabledWarning();
-    }
-
-    public void checkSecurityDisabledWarning() throws Exception {
-        final Request request = new Request("GET", "/_cat/indices");
-        Response response = client().performRequest(request);
-        List<String> warningHeaders = response.getWarnings();
-        if (securityExplicitlySet) {
-            assertThat (warningHeaders, Matchers.empty());
-        } else {
-            assertThat (warningHeaders, Matchers.hasSize(1));
-            assertThat (warningHeaders.get(0),
-                containsString("Elasticsearch built-in security features are not enabled. Without authentication, your cluster could be " +
-                    "accessible to anyone. See https://www.elastic.co/guide/en/elasticsearch/reference/" + Version.CURRENT.major + "." +
-                    Version.CURRENT.minor + "/security-minimal-setup.html to enable security."));
-        }
     }
 
     private String getClusterInfo() throws IOException {

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

@@ -1120,12 +1120,13 @@ public Map<String, Supplier<HttpServerTransport>> getHttpTransports(Settings set
 
     @Override
     public UnaryOperator<RestHandler> getRestHandlerWrapper(ThreadContext threadContext) {
-        if (enabled == false) {
-            return null;
+        final boolean extractClientCertificate;
+        if (enabled && HTTP_SSL_ENABLED.get(settings)) {
+            final SSLConfiguration httpSSLConfig = getSslService().getHttpTransportSSLConfiguration();
+            extractClientCertificate = getSslService().isSSLClientAuthEnabled(httpSSLConfig);
+        } else {
+            extractClientCertificate = false;
         }
-        final boolean ssl = HTTP_SSL_ENABLED.get(settings);
-        final SSLConfiguration httpSSLConfig = getSslService().getHttpTransportSSLConfiguration();
-        boolean extractClientCertificate = ssl && getSslService().isSSLClientAuthEnabled(httpSSLConfig);
         return handler -> new SecurityRestFilter(getLicenseState(), threadContext, authcService.get(), secondayAuthc.get(),
             handler, extractClientCertificate);
     }

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

@@ -11,10 +11,8 @@
 import org.apache.logging.log4j.message.ParameterizedMessage;
 import org.apache.logging.log4j.util.Supplier;
 import org.elasticsearch.ExceptionsHelper;
-import org.elasticsearch.Version;
 import org.elasticsearch.action.ActionListener;
 import org.elasticsearch.client.node.NodeClient;
-import org.elasticsearch.common.logging.HeaderWarning;
 import org.elasticsearch.common.util.Maps;
 import org.elasticsearch.common.util.concurrent.ThreadContext;
 import org.elasticsearch.common.xcontent.MediaType;
@@ -64,8 +62,13 @@ public boolean allowSystemIndexAccessByDefault() {
 
     @Override
     public void handleRequest(RestRequest request, RestChannel channel, NodeClient client) throws Exception {
-        if (licenseState.isSecurityEnabled() && request.method() != Method.OPTIONS) {
+        if (request.method() == Method.OPTIONS) {
             // CORS - allow for preflight unauthenticated OPTIONS request
+            restHandler.handleRequest(request, channel, client);
+            return;
+        }
+
+        if (licenseState.isSecurityEnabled()) {
             if (extractClientCertificate) {
                 HttpChannel httpChannel = request.getHttpChannel();
                 SSLEngineUtils.extractClientCertificates(logger, threadContext, httpChannel);
@@ -90,11 +93,6 @@ public void handleRequest(RestRequest request, RestChannel channel, NodeClient c
                         e -> handleException("Secondary authentication", request, channel, e)));
                 }, e -> handleException("Authentication", request, channel, e)));
         } else {
-            if (request.method() != Method.OPTIONS) {
-                HeaderWarning.addWarning("Elasticsearch built-in security features are not enabled. Without authentication, your cluster " +
-                    "could be accessible to anyone. See https://www.elastic.co/guide/en/elasticsearch/reference/" + Version.CURRENT.major +
-                    "." + Version.CURRENT.minor + "/security-minimal-setup.html to enable security.");
-            }
             restHandler.handleRequest(request, channel, client);
         }
     }

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

@@ -6,9 +6,13 @@
  */
 package org.elasticsearch.xpack.security;
 
+import org.apache.logging.log4j.Level;
+import org.apache.logging.log4j.LogManager;
+import org.apache.logging.log4j.Logger;
 import org.elasticsearch.ElasticsearchSecurityException;
 import org.elasticsearch.Version;
 import org.elasticsearch.action.ActionListener;
+import org.elasticsearch.action.ActionModule;
 import org.elasticsearch.client.Client;
 import org.elasticsearch.cluster.ClusterName;
 import org.elasticsearch.cluster.ClusterState;
@@ -18,10 +22,12 @@
 import org.elasticsearch.cluster.node.DiscoveryNodes;
 import org.elasticsearch.cluster.service.ClusterService;
 import org.elasticsearch.common.Strings;
+import org.elasticsearch.common.logging.Loggers;
 import org.elasticsearch.common.network.NetworkModule;
 import org.elasticsearch.common.settings.ClusterSettings;
 import org.elasticsearch.common.settings.Setting;
 import org.elasticsearch.common.settings.Settings;
+import org.elasticsearch.common.settings.SettingsModule;
 import org.elasticsearch.core.TimeValue;
 import org.elasticsearch.common.util.concurrent.ThreadContext;
 import org.elasticsearch.env.Environment;
@@ -34,9 +40,12 @@
 import org.elasticsearch.rest.RestRequest;
 import org.elasticsearch.script.ScriptService;
 import org.elasticsearch.test.ESTestCase;
+import org.elasticsearch.test.MockLogAppender;
 import org.elasticsearch.test.VersionUtils;
 import org.elasticsearch.test.rest.FakeRestRequest;
+import org.elasticsearch.threadpool.TestThreadPool;
 import org.elasticsearch.threadpool.ThreadPool;
+import org.elasticsearch.usage.UsageService;
 import org.elasticsearch.watcher.ResourceWatcherService;
 import org.elasticsearch.xpack.core.XPackField;
 import org.elasticsearch.xpack.core.XPackSettings;
@@ -514,6 +523,71 @@ public void testLicenseUpdateFailureHandlerUpdate() throws Exception {
         }
     }
 
+    public void testSecurityPluginInstallsRestHandlerWrapperEvenIfSecurityIsDisabled() throws IllegalAccessException {
+        Settings settings = Settings.builder()
+            .put("xpack.security.enabled", false)
+            .put("path.home", createTempDir())
+            .build();
+        SettingsModule settingsModule = new SettingsModule(Settings.EMPTY);
+        ThreadPool threadPool = new TestThreadPool(getTestName());
+
+        try {
+            UsageService usageService = new UsageService();
+            Security security = new Security(settings, null);
+
+            ActionModule actionModule = new ActionModule(settingsModule.getSettings(),
+                TestIndexNameExpressionResolver.newInstance(threadPool.getThreadContext()),
+                settingsModule.getIndexScopedSettings(), settingsModule.getClusterSettings(), settingsModule.getSettingsFilter(),
+                threadPool, Arrays.asList(security), null, null, usageService, null);
+            actionModule.initRestHandlers(null);
+
+            assertTrue(security.getRestHandlerWrapper(threadPool.getThreadContext()) != null);
+
+        } finally {
+            threadPool.shutdown();
+        }
+
+    }
+
+    public void testSecurityRestHandlerWrapperCanBeInstalled() throws IllegalAccessException {
+        final Logger amLogger = LogManager.getLogger(ActionModule.class);
+        Loggers.setLevel(amLogger, Level.DEBUG);
+        final MockLogAppender appender = new MockLogAppender();
+        Loggers.addAppender(amLogger, appender);
+        appender.start();
+
+        Settings settings = Settings.builder()
+            .put("xpack.security.enabled", false)
+            .put("path.home", createTempDir())
+            .build();
+        SettingsModule settingsModule = new SettingsModule(Settings.EMPTY);
+        ThreadPool threadPool = new TestThreadPool(getTestName());
+
+        try {
+            UsageService usageService = new UsageService();
+            Security security = new Security(settings, null);
+
+            // Verify Security rest wrapper is about to be installed
+            // We will throw later if another wrapper is already installed
+            appender.addExpectation(new MockLogAppender.SeenEventExpectation(
+                "Security rest wrapper", ActionModule.class.getName(), Level.DEBUG,
+                "Using REST wrapper from plugin org.elasticsearch.xpack.security.Security"
+            ));
+
+            ActionModule actionModule = new ActionModule(settingsModule.getSettings(),
+                TestIndexNameExpressionResolver.newInstance(threadPool.getThreadContext()),
+                settingsModule.getIndexScopedSettings(), settingsModule.getClusterSettings(), settingsModule.getSettingsFilter(),
+                threadPool, Arrays.asList(security), null, null, usageService, null);
+            actionModule.initRestHandlers(null);
+
+            appender.assertAllExpectationsMatched();
+        } finally {
+            threadPool.shutdown();
+            appender.stop();
+            Loggers.removeAppender(amLogger, appender);
+        }
+    }
+
     private void logAndFail(Exception e) {
         logger.error("unexpected exception", e);
         fail("unexpected exception " + e.getMessage());

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

@@ -9,7 +9,6 @@
 import com.nimbusds.jose.util.StandardCharset;
 import org.apache.lucene.util.SetOnce;
 import org.elasticsearch.ElasticsearchException;
-import org.elasticsearch.Version;
 import org.elasticsearch.action.ActionListener;
 import org.elasticsearch.client.node.NodeClient;
 import org.elasticsearch.common.bytes.BytesArray;
@@ -144,9 +143,6 @@ public void testProcessBasicLicense() throws Exception {
         RestRequest request = mock(RestRequest.class);
         when(licenseState.isSecurityEnabled()).thenReturn(false);
         filter.handleRequest(request, channel, null);
-        assertWarnings("Elasticsearch built-in security features are not enabled. Without authentication, your cluster " +
-            "could be accessible to anyone. See https://www.elastic.co/guide/en/elasticsearch/reference/" + Version.CURRENT.major + "." +
-            Version.CURRENT.minor + "/security-minimal-setup.html to enable security.");
         verify(restHandler).handleRequest(request, channel, null);
         verifyZeroInteractions(channel, authcService);
     }