diff --git a/elasticsearch/x-pack/security/src/main/java/org/elasticsearch/xpack/security/Security.java b/elasticsearch/x-pack/security/src/main/java/org/elasticsearch/xpack/security/Security.java index ca73c3c8174..eb5b9c27cab 100644 --- a/elasticsearch/x-pack/security/src/main/java/org/elasticsearch/xpack/security/Security.java +++ b/elasticsearch/x-pack/security/src/main/java/org/elasticsearch/xpack/security/Security.java @@ -113,9 +113,7 @@ import org.elasticsearch.xpack.security.rest.action.user.RestPutUserAction; import org.elasticsearch.xpack.security.ssl.SSLConfigurationReloader; import org.elasticsearch.xpack.security.ssl.SSLService; import org.elasticsearch.xpack.security.support.OptionalSettings; -import org.elasticsearch.xpack.security.transport.SecurityClientTransportService; import org.elasticsearch.xpack.security.transport.SecurityServerTransportService; -import org.elasticsearch.xpack.security.transport.SecurityTransportModule; import org.elasticsearch.xpack.security.transport.filter.IPFilter; import org.elasticsearch.xpack.security.transport.netty3.SecurityNetty3HttpServerTransport; import org.elasticsearch.xpack.security.transport.netty3.SecurityNetty3Transport; @@ -172,12 +170,14 @@ public class Security implements ActionPlugin, IngestPlugin { public Collection nodeModules() { List modules = new ArrayList<>(); + if (enabled == false || transportClientMode) { + modules.add(b -> b.bind(IPFilter.class).toProvider(Providers.of(null))); + } if (transportClientMode) { if (enabled == false) { return modules; } - modules.add(new SecurityTransportModule(settings)); modules.add(b -> { // for transport client we still must inject these ssl classes with guice b.bind(SSLService.class).toInstance(new SSLService(settings, null)); @@ -186,6 +186,7 @@ public class Security implements ActionPlugin, IngestPlugin { return modules; } modules.add(b -> XPackPlugin.bindFeatureSet(b, SecurityFeatureSet.class)); + if (enabled == false) { modules.add(b -> { @@ -195,7 +196,6 @@ public class Security implements ActionPlugin, IngestPlugin { b.bind(AuditTrailService.class) .toInstance(new AuditTrailService(settings, Collections.emptyList(), licenseState)); }); - modules.add(new SecurityTransportModule(settings)); return modules; } @@ -210,7 +210,6 @@ public class Security implements ActionPlugin, IngestPlugin { }); modules.add(new SecurityRestModule(settings)); modules.add(new SecurityActionModule(settings)); - modules.add(new SecurityTransportModule(settings)); return modules; } @@ -317,6 +316,10 @@ public class Security implements ActionPlugin, IngestPlugin { components.add(new SecurityLifecycleService(settings, clusterService, threadPool, indexAuditTrail, nativeUsersStore, nativeRolesStore, client)); + if (IPFilter.IP_FILTER_ENABLED_SETTING.get(settings)) { + components.add(new IPFilter(settings, auditTrailService, clusterService.getClusterSettings(), licenseState)); + } + return components; } @@ -325,11 +328,11 @@ public class Security implements ActionPlugin, IngestPlugin { return Settings.EMPTY; } - return additionalSettings(settings); + return additionalSettings(settings, transportClientMode); } // visible for tests - static Settings additionalSettings(Settings settings) { + static Settings additionalSettings(Settings settings, boolean transportClientMode) { final Settings.Builder settingsBuilder = Settings.builder(); if (NetworkModule.TRANSPORT_TYPE_SETTING.exists(settings)) { @@ -359,13 +362,15 @@ public class Security implements ActionPlugin, IngestPlugin { SecurityNetty4HttpServerTransport.overrideSettings(settingsBuilder, settings); } - settingsBuilder.put(NetworkModule.TRANSPORT_SERVICE_TYPE_KEY, XPackPlugin.SECURITY); + if (transportClientMode == false) { + settingsBuilder.put(NetworkModule.TRANSPORT_SERVICE_TYPE_KEY, XPackPlugin.SECURITY); + } addUserSettings(settings, settingsBuilder); addTribeSettings(settings, settingsBuilder); return settingsBuilder.build(); } - public List> getSettings() { + public static List> getSettings(boolean transportClientMode) { List> settingsList = new ArrayList<>(); // always register for both client and node modes settingsList.add(USER_SETTING); @@ -510,7 +515,6 @@ public class Security implements ActionPlugin, IngestPlugin { if (enabled) { module.registerTransport(Security.NAME3, SecurityNetty3Transport.class); module.registerTransport(Security.NAME4, SecurityNetty4Transport.class); - module.registerTransportService(XPackPlugin.SECURITY, SecurityClientTransportService.class); } return; } diff --git a/elasticsearch/x-pack/security/src/main/java/org/elasticsearch/xpack/security/transport/ClientTransportFilter.java b/elasticsearch/x-pack/security/src/main/java/org/elasticsearch/xpack/security/transport/ClientTransportFilter.java deleted file mode 100644 index 1c72af62d3e..00000000000 --- a/elasticsearch/x-pack/security/src/main/java/org/elasticsearch/xpack/security/transport/ClientTransportFilter.java +++ /dev/null @@ -1,62 +0,0 @@ -/* - * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one - * or more contributor license agreements. Licensed under the Elastic License; - * you may not use this file except in compliance with the Elastic License. - */ -package org.elasticsearch.xpack.security.transport; - -import org.elasticsearch.common.inject.Inject; -import org.elasticsearch.xpack.security.authc.AuthenticationService; -import org.elasticsearch.xpack.security.user.SystemUser; -import org.elasticsearch.transport.TransportRequest; - -import java.io.IOException; - -/** - * This interface allows clients, that connect to an elasticsearch cluster, to execute - * additional logic before an operation is sent. - * - * This interface only applies to outgoing messages - */ -public interface ClientTransportFilter { - - /** - * Called just before the given request is sent by the transport. Any exception - * thrown by this method will stop the request from being sent and the error will - * be sent back to the sender. - */ - void outbound(String action, TransportRequest request) throws IOException; - - /** - * The client transport filter that should be used in transport clients - */ - public static class TransportClient implements ClientTransportFilter { - - @Override - public void outbound(String action, TransportRequest request) { - } - } - - /** - * The client transport filter that should be used in nodes - */ - public static class Node implements ClientTransportFilter { - - private final AuthenticationService authcService; - - @Inject - public Node(AuthenticationService authcService) { - this.authcService = authcService; - } - - @Override - public void outbound(String action, TransportRequest request) throws IOException { - /** - this will check if there's a user associated with the request. If there isn't, - the system user will be attached. There cannot be a request outgoing from this - node that is not associated with a user. - */ - authcService.attachUserIfMissing(SystemUser.INSTANCE); - } - } -} diff --git a/elasticsearch/x-pack/security/src/main/java/org/elasticsearch/xpack/security/transport/SecurityClientTransportService.java b/elasticsearch/x-pack/security/src/main/java/org/elasticsearch/xpack/security/transport/SecurityClientTransportService.java deleted file mode 100644 index 350e1292afc..00000000000 --- a/elasticsearch/x-pack/security/src/main/java/org/elasticsearch/xpack/security/transport/SecurityClientTransportService.java +++ /dev/null @@ -1,42 +0,0 @@ -/* - * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one - * or more contributor license agreements. Licensed under the Elastic License; - * you may not use this file except in compliance with the Elastic License. - */ -package org.elasticsearch.xpack.security.transport; - -import org.elasticsearch.cluster.node.DiscoveryNode; -import org.elasticsearch.common.inject.Inject; -import org.elasticsearch.common.settings.Settings; -import org.elasticsearch.threadpool.ThreadPool; -import org.elasticsearch.transport.Transport; -import org.elasticsearch.transport.TransportException; -import org.elasticsearch.transport.TransportRequest; -import org.elasticsearch.transport.TransportRequestOptions; -import org.elasticsearch.transport.TransportResponse; -import org.elasticsearch.transport.TransportResponseHandler; -import org.elasticsearch.transport.TransportService; - -public class SecurityClientTransportService extends TransportService { - - private final ClientTransportFilter clientFilter; - - @Inject - public SecurityClientTransportService(Settings settings, Transport transport, ThreadPool threadPool, - ClientTransportFilter clientFilter) { - super(settings, transport, threadPool); - this.clientFilter = clientFilter; - } - - @Override - public void sendRequest(DiscoveryNode node, String action, TransportRequest request, - TransportRequestOptions options, TransportResponseHandler handler) { - try { - clientFilter.outbound(action, request); - super.sendRequest(node, action, request, options, handler); - } catch (Exception e) { - handler.handleException(new TransportException("failed sending request", e)); - } - } - -} diff --git a/elasticsearch/x-pack/security/src/main/java/org/elasticsearch/xpack/security/transport/SecurityServerTransportService.java b/elasticsearch/x-pack/security/src/main/java/org/elasticsearch/xpack/security/transport/SecurityServerTransportService.java index b47c4198651..2090ae6b246 100644 --- a/elasticsearch/x-pack/security/src/main/java/org/elasticsearch/xpack/security/transport/SecurityServerTransportService.java +++ b/elasticsearch/x-pack/security/src/main/java/org/elasticsearch/xpack/security/transport/SecurityServerTransportService.java @@ -29,6 +29,7 @@ import org.elasticsearch.transport.TransportResponseHandler; import org.elasticsearch.transport.TransportService; import org.elasticsearch.transport.TransportSettings; import org.elasticsearch.xpack.security.transport.netty4.SecurityNetty4Transport; +import org.elasticsearch.xpack.security.user.SystemUser; import java.util.Collections; import java.util.HashMap; @@ -46,7 +47,6 @@ public class SecurityServerTransportService extends TransportService { protected final AuthenticationService authcService; protected final AuthorizationService authzService; protected final SecurityActionMapper actionMapper; - protected final ClientTransportFilter clientFilter; protected final XPackLicenseState licenseState; protected final Map profileFilters; @@ -56,13 +56,11 @@ public class SecurityServerTransportService extends TransportService { AuthenticationService authcService, AuthorizationService authzService, SecurityActionMapper actionMapper, - ClientTransportFilter clientTransportFilter, XPackLicenseState licenseState) { super(settings, transport, threadPool); this.authcService = authcService; this.authzService = authzService; this.actionMapper = actionMapper; - this.clientFilter = clientTransportFilter; this.licenseState = licenseState; this.profileFilters = initializeProfileFilters(); } @@ -72,24 +70,26 @@ public class SecurityServerTransportService extends TransportService { TransportRequestOptions options, TransportResponseHandler handler) { // Sometimes a system action gets executed like a internal create index request or update mappings request // which means that the user is copied over to system actions so we need to change the user - if ((clientFilter instanceof ClientTransportFilter.Node) && - AuthorizationUtils.shouldReplaceUserWithSystem(threadPool.getThreadContext(), action)) { - final ThreadContext.StoredContext original = threadPool.getThreadContext().newStoredContext(); + if (AuthorizationUtils.shouldReplaceUserWithSystem(threadPool.getThreadContext(), action)) { try (ThreadContext.StoredContext ctx = threadPool.getThreadContext().stashContext()) { - try { - clientFilter.outbound(action, request); - super.sendRequest(node, action, request, options, new ContextRestoreResponseHandler<>(original, handler)); - } catch (Exception e) { - handler.handleException(new TransportException("failed sending request", e)); - } + final ThreadContext.StoredContext original = threadPool.getThreadContext().newStoredContext(); + sendWithUser(node, action, request, options, new ContextRestoreResponseHandler<>(original, handler)); } } else { - try { - clientFilter.outbound(action, request); - super.sendRequest(node, action, request, options, handler); - } catch (Exception e) { - handler.handleException(new TransportException("failed sending request", e)); - } + sendWithUser(node, action, request, options, handler); + } + } + + private void sendWithUser(DiscoveryNode node, String action, TransportRequest request, + TransportRequestOptions options, TransportResponseHandler handler) { + try { + // this will check if there's a user associated with the request. If there isn't, + // the system user will be attached. There cannot be a request outgoing from this + // node that is not associated with a user. + authcService.attachUserIfMissing(SystemUser.INSTANCE); + super.sendRequest(node, action, request, options, handler); + } catch (Exception e) { + handler.handleException(new TransportException("failed sending request", e)); } } diff --git a/elasticsearch/x-pack/security/src/main/java/org/elasticsearch/xpack/security/transport/SecurityTransportModule.java b/elasticsearch/x-pack/security/src/main/java/org/elasticsearch/xpack/security/transport/SecurityTransportModule.java deleted file mode 100644 index 7ab781dac0c..00000000000 --- a/elasticsearch/x-pack/security/src/main/java/org/elasticsearch/xpack/security/transport/SecurityTransportModule.java +++ /dev/null @@ -1,40 +0,0 @@ -/* - * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one - * or more contributor license agreements. Licensed under the Elastic License; - * you may not use this file except in compliance with the Elastic License. - */ -package org.elasticsearch.xpack.security.transport; - -import org.elasticsearch.common.inject.util.Providers; -import org.elasticsearch.common.settings.Settings; -import org.elasticsearch.xpack.security.support.AbstractSecurityModule; -import org.elasticsearch.xpack.security.transport.filter.IPFilter; - -/** - * - */ -public class SecurityTransportModule extends AbstractSecurityModule { - - public SecurityTransportModule(Settings settings) { - super(settings); - } - - @Override - protected void configure(boolean clientMode) { - if (securityEnabled == false) { - bind(IPFilter.class).toProvider(Providers.of(null)); - return; - } - - if (clientMode) { - // no ip filtering on the client - bind(IPFilter.class).toProvider(Providers.of(null)); - bind(ClientTransportFilter.class).to(ClientTransportFilter.TransportClient.class).asEagerSingleton(); - } else { - bind(ClientTransportFilter.class).to(ClientTransportFilter.Node.class).asEagerSingleton(); - if (IPFilter.IP_FILTER_ENABLED_SETTING.get(settings)) { - bind(IPFilter.class).asEagerSingleton(); - } - } - } -} diff --git a/elasticsearch/x-pack/security/src/main/java/org/elasticsearch/xpack/security/transport/ServerTransportFilter.java b/elasticsearch/x-pack/security/src/main/java/org/elasticsearch/xpack/security/transport/ServerTransportFilter.java index 308aa454679..6b0c4a98342 100644 --- a/elasticsearch/x-pack/security/src/main/java/org/elasticsearch/xpack/security/transport/ServerTransportFilter.java +++ b/elasticsearch/x-pack/security/src/main/java/org/elasticsearch/xpack/security/transport/ServerTransportFilter.java @@ -70,10 +70,9 @@ public interface ServerTransportFilter { /* here we don't have a fallback user, as all incoming request are expected to have a user attached (either in headers or in context) - We can make this assumption because in nodes we also have the - {@link ClientTransportFilter.Node} that makes sure all outgoing requsts - from all the nodes are attached with a user (either a serialize user - an authentication token + We can make this assumption because in nodes we make sure all outgoing + requests from all the nodes are attached with a user (either a serialize + user an authentication token */ String securityAction = actionMapper.action(action, request); diff --git a/elasticsearch/x-pack/security/src/main/java/org/elasticsearch/xpack/security/transport/filter/IPFilter.java b/elasticsearch/x-pack/security/src/main/java/org/elasticsearch/xpack/security/transport/filter/IPFilter.java index 114ea5742bc..6587e907030 100644 --- a/elasticsearch/x-pack/security/src/main/java/org/elasticsearch/xpack/security/transport/filter/IPFilter.java +++ b/elasticsearch/x-pack/security/src/main/java/org/elasticsearch/xpack/security/transport/filter/IPFilter.java @@ -109,7 +109,6 @@ public class IPFilter { private final SetOnce boundHttpTransportAddress = new SetOnce<>(); private final SetOnce> profileBoundAddress = new SetOnce<>(); - @Inject public IPFilter(final Settings settings, AuditTrailService auditTrail, ClusterSettings clusterSettings, XPackLicenseState licenseState) { this.logger = Loggers.getLogger(getClass(), settings); diff --git a/elasticsearch/x-pack/security/src/test/java/org/elasticsearch/xpack/security/SecuritySettingsTests.java b/elasticsearch/x-pack/security/src/test/java/org/elasticsearch/xpack/security/SecuritySettingsTests.java index c8bc275bf2b..03f5dadf044 100644 --- a/elasticsearch/x-pack/security/src/test/java/org/elasticsearch/xpack/security/SecuritySettingsTests.java +++ b/elasticsearch/x-pack/security/src/test/java/org/elasticsearch/xpack/security/SecuritySettingsTests.java @@ -31,7 +31,7 @@ public class SecuritySettingsTests extends ESTestCase { Settings settings = Settings.builder().put("tribe.t1.cluster.name", "non_existing") .put("tribe.t2.cluster.name", "non_existing").build(); - Settings additionalSettings = Security.additionalSettings(settings); + Settings additionalSettings = Security.additionalSettings(settings, false); assertThat(additionalSettings.getAsArray("tribe.t1.plugin.mandatory", null), arrayContaining(XPackPlugin.NAME)); @@ -44,7 +44,7 @@ public class SecuritySettingsTests extends ESTestCase { //simulate what PluginsService#updatedSettings does to make sure we don't override existing mandatory plugins try { - Settings.builder().put(settings).put(Security.additionalSettings(settings)).build(); + Settings.builder().put(settings).put(Security.additionalSettings(settings, false)).build(); fail("security cannot change the value of a setting that is already defined, so a exception should be thrown"); } catch (IllegalStateException e) { assertThat(e.getMessage(), containsString(XPackPlugin.NAME)); @@ -57,7 +57,7 @@ public class SecuritySettingsTests extends ESTestCase { .putArray("tribe.t1.plugin.mandatory", "test_plugin", XPackPlugin.NAME).build(); //simulate what PluginsService#updatedSettings does to make sure we don't override existing mandatory plugins - Settings finalSettings = Settings.builder().put(settings).put(Security.additionalSettings(settings)).build(); + Settings finalSettings = Settings.builder().put(settings).put(Security.additionalSettings(settings, false)).build(); String[] finalMandatoryPlugins = finalSettings.getAsArray("tribe.t1.plugin.mandatory", null); assertThat(finalMandatoryPlugins, notNullValue()); @@ -70,7 +70,7 @@ public class SecuritySettingsTests extends ESTestCase { Settings settings = Settings.builder().put("tribe.t1.cluster.name", "non_existing") .put("tribe.t2.cluster.name", "non_existing").build(); - Settings additionalSettings = Security.additionalSettings(settings); + Settings additionalSettings = Security.additionalSettings(settings, false); assertThat(additionalSettings.getAsBoolean(TRIBE_T1_SECURITY_ENABLED, null), equalTo(true)); assertThat(additionalSettings.getAsBoolean(TRIBE_T2_SECURITY_ENABLED, null), equalTo(true)); @@ -82,7 +82,7 @@ public class SecuritySettingsTests extends ESTestCase { .put("tribe.t2.cluster.name", "non_existing").build(); try { - Security.additionalSettings(settings); + Security.additionalSettings(settings, false); fail("security cannot change the value of a setting that is already defined, so a exception should be thrown"); } catch (IllegalStateException e) { assertThat(e.getMessage(), containsString(TRIBE_T1_SECURITY_ENABLED)); @@ -96,7 +96,7 @@ public class SecuritySettingsTests extends ESTestCase { .putArray("tribe.t1.plugin.mandatory", "test_plugin", XPackPlugin.NAME).build(); try { - Security.additionalSettings(settings); + Security.additionalSettings(settings, false); fail("security cannot change the value of a setting that is already defined, so a exception should be thrown"); } catch (IllegalStateException e) { assertThat(e.getMessage(), containsString(TRIBE_T1_SECURITY_ENABLED)); @@ -112,7 +112,7 @@ public class SecuritySettingsTests extends ESTestCase { .putArray("xpack.security.something.else.here", new String[] { "foo", "bar" }) .build(); - Settings additionalSettings = Security.additionalSettings(settings); + Settings additionalSettings = Security.additionalSettings(settings, false); assertThat(additionalSettings.get("xpack.security.foo"), nullValue()); assertThat(additionalSettings.get("xpack.security.bar"), nullValue()); diff --git a/elasticsearch/x-pack/security/src/test/java/org/elasticsearch/xpack/security/SecurityTests.java b/elasticsearch/x-pack/security/src/test/java/org/elasticsearch/xpack/security/SecurityTests.java index 3697024f9aa..b7a9f195cc8 100644 --- a/elasticsearch/x-pack/security/src/test/java/org/elasticsearch/xpack/security/SecurityTests.java +++ b/elasticsearch/x-pack/security/src/test/java/org/elasticsearch/xpack/security/SecurityTests.java @@ -9,10 +9,15 @@ import java.io.IOException; import java.util.Arrays; import java.util.Collection; import java.util.Collections; +import java.util.HashSet; +import java.util.List; import java.util.Map; +import java.util.Set; import org.elasticsearch.cluster.service.ClusterService; 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.env.Environment; import org.elasticsearch.license.XPackLicenseState; @@ -29,6 +34,7 @@ import org.elasticsearch.xpack.security.authc.file.FileRealm; import static org.hamcrest.Matchers.containsString; import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; public class SecurityTests extends ESTestCase { @@ -58,6 +64,11 @@ public class SecurityTests extends ESTestCase { Security security = new Security(settings, env, new XPackLicenseState()); ThreadPool threadPool = mock(ThreadPool.class); ClusterService clusterService = mock(ClusterService.class); + settings = Security.additionalSettings(settings, false); + Set> allowedSettings = new HashSet<>(Security.getSettings(false)); + allowedSettings.addAll(ClusterSettings.BUILT_IN_CLUSTER_SETTINGS); + ClusterSettings clusterSettings = new ClusterSettings(settings, allowedSettings); + when(clusterService.getClusterSettings()).thenReturn(clusterSettings); return security.createComponents(null, threadPool, clusterService, null, Arrays.asList(extensions)); } @@ -130,46 +141,56 @@ public class SecurityTests extends ESTestCase { assertEquals("Unknown audit trail output [foo]", e.getMessage()); } - public void testTransportTypeSetting() throws Exception { - Settings defaultSettings = Security.additionalSettings(Settings.EMPTY); + public void testTransportSettingDefaults() throws Exception { + Settings defaultSettings = Security.additionalSettings(Settings.EMPTY, false); assertEquals(Security.NAME4, NetworkModule.TRANSPORT_TYPE_SETTING.get(defaultSettings)); assertEquals(Security.NAME4, NetworkModule.HTTP_TYPE_SETTING.get(defaultSettings)); + } - // set transport back to security3 - Settings transport3 = Security.additionalSettings(Settings.builder().put(NetworkModule.TRANSPORT_TYPE_KEY, Security.NAME3).build()); + public void testTransportSettingNetty3Transport() { + Settings baseSettings = Settings.builder().put(NetworkModule.TRANSPORT_TYPE_KEY, Security.NAME3).build(); + Settings transport3 = Security.additionalSettings(baseSettings, false); assertFalse(NetworkModule.TRANSPORT_TYPE_SETTING.exists(transport3)); assertEquals(Security.NAME4, NetworkModule.HTTP_TYPE_SETTING.get(transport3)); + } - // set http back to security3 - Settings http3 = Security.additionalSettings(Settings.builder().put(NetworkModule.HTTP_TYPE_KEY, Security.NAME3).build()); + public void testTransportSettingNetty3Http() { + Settings baseSettings = Settings.builder().put(NetworkModule.HTTP_TYPE_KEY, Security.NAME3).build(); + Settings http3 = Security.additionalSettings(baseSettings, false); assertEquals(Security.NAME4, NetworkModule.TRANSPORT_TYPE_SETTING.get(http3)); assertFalse(NetworkModule.HTTP_TYPE_SETTING.exists(http3)); + } - // set both to security3 + public void testTransportSettingNetty3Both() { Settings both3 = Security.additionalSettings(Settings.builder() - .put(NetworkModule.TRANSPORT_TYPE_KEY, Security.NAME3) - .put(NetworkModule.HTTP_TYPE_KEY, Security.NAME3) - .build()); + .put(NetworkModule.TRANSPORT_TYPE_KEY, Security.NAME3) + .put(NetworkModule.HTTP_TYPE_KEY, Security.NAME3) + .build(), false); assertFalse(NetworkModule.TRANSPORT_TYPE_SETTING.exists(both3)); assertFalse(NetworkModule.HTTP_TYPE_SETTING.exists(both3)); + } - // set both to 4 + public void testTransportSettingNetty4Both() { Settings both4 = Security.additionalSettings(Settings.builder() - .put(NetworkModule.TRANSPORT_TYPE_KEY, Security.NAME4) - .put(NetworkModule.HTTP_TYPE_KEY, Security.NAME4) - .build()); + .put(NetworkModule.TRANSPORT_TYPE_KEY, Security.NAME4) + .put(NetworkModule.HTTP_TYPE_KEY, Security.NAME4) + .build(), false); assertFalse(NetworkModule.TRANSPORT_TYPE_SETTING.exists(both4)); assertFalse(NetworkModule.HTTP_TYPE_SETTING.exists(both4)); + } + public void testTransportSettingValidation() { final String badType = randomFrom("netty3", "netty4", "other", "security1"); + Settings settingsTransport = Settings.builder().put(NetworkModule.TRANSPORT_TYPE_KEY, badType).build(); IllegalArgumentException badTransport = expectThrows(IllegalArgumentException.class, - () -> Security.additionalSettings(Settings.builder().put(NetworkModule.TRANSPORT_TYPE_KEY, badType).build())); + () -> Security.additionalSettings(settingsTransport, false)); assertThat(badTransport.getMessage(), containsString(Security.NAME3)); assertThat(badTransport.getMessage(), containsString(Security.NAME4)); assertThat(badTransport.getMessage(), containsString(NetworkModule.TRANSPORT_TYPE_KEY)); + Settings settingsHttp = Settings.builder().put(NetworkModule.HTTP_TYPE_KEY, badType).build(); IllegalArgumentException badHttp = expectThrows(IllegalArgumentException.class, - () -> Security.additionalSettings(Settings.builder().put(NetworkModule.HTTP_TYPE_KEY, badType).build())); + () -> Security.additionalSettings(settingsHttp, false)); assertThat(badHttp.getMessage(), containsString(Security.NAME3)); assertThat(badHttp.getMessage(), containsString(Security.NAME4)); assertThat(badHttp.getMessage(), containsString(NetworkModule.HTTP_TYPE_KEY)); diff --git a/elasticsearch/x-pack/security/src/test/java/org/elasticsearch/xpack/security/transport/ClientTransportFilterTests.java b/elasticsearch/x-pack/security/src/test/java/org/elasticsearch/xpack/security/transport/ClientTransportFilterTests.java deleted file mode 100644 index 15951ec09a7..00000000000 --- a/elasticsearch/x-pack/security/src/test/java/org/elasticsearch/xpack/security/transport/ClientTransportFilterTests.java +++ /dev/null @@ -1,35 +0,0 @@ -/* - * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one - * or more contributor license agreements. Licensed under the Elastic License; - * you may not use this file except in compliance with the Elastic License. - */ -package org.elasticsearch.xpack.security.transport; - -import org.elasticsearch.xpack.security.authc.AuthenticationService; -import org.elasticsearch.xpack.security.user.SystemUser; -import org.elasticsearch.test.ESTestCase; -import org.elasticsearch.transport.TransportRequest; -import org.junit.Before; - -import static org.mockito.Mockito.mock; -import static org.mockito.Mockito.verify; - -/** - * - */ -public class ClientTransportFilterTests extends ESTestCase { - private AuthenticationService authcService; - private ClientTransportFilter filter; - - @Before - public void init() throws Exception { - authcService = mock(AuthenticationService.class); - filter = new ClientTransportFilter.Node(authcService); - } - - public void testOutbound() throws Exception { - TransportRequest request = mock(TransportRequest.class); - filter.outbound("_action", request); - verify(authcService).attachUserIfMissing(SystemUser.INSTANCE); - } -} diff --git a/elasticsearch/x-pack/security/src/test/java/org/elasticsearch/xpack/security/transport/TransportFilterTests.java b/elasticsearch/x-pack/security/src/test/java/org/elasticsearch/xpack/security/transport/TransportFilterTests.java index 37efd75510f..1697eda141f 100644 --- a/elasticsearch/x-pack/security/src/test/java/org/elasticsearch/xpack/security/transport/TransportFilterTests.java +++ b/elasticsearch/x-pack/security/src/test/java/org/elasticsearch/xpack/security/transport/TransportFilterTests.java @@ -32,6 +32,7 @@ import org.elasticsearch.transport.TransportResponse; import org.elasticsearch.transport.TransportResponseHandler; import org.elasticsearch.transport.TransportService; import org.elasticsearch.transport.TransportSettings; +import org.elasticsearch.xpack.security.user.SystemUser; import org.mockito.InOrder; import java.io.IOException; @@ -100,13 +101,13 @@ public class TransportFilterTests extends ESIntegTestCase { ServerTransportFilter targetServerFilter = ((InternalPluginServerTransportService) targetService).transportFilter(TransportSettings.DEFAULT_PROFILE); - ClientTransportFilter sourceClientFilter = internalCluster().getInstance(ClientTransportFilter.class, source); - ClientTransportFilter targetClientFilter = internalCluster().getInstance(ClientTransportFilter.class, target); + AuthenticationService sourceAuth = internalCluster().getInstance(AuthenticationService.class, source); + AuthenticationService targetAuth = internalCluster().getInstance(AuthenticationService.class, target); - InOrder inOrder = inOrder(sourceClientFilter, targetServerFilter, targetClientFilter, sourceServerFilter); - inOrder.verify(sourceClientFilter).outbound("_action", new Request("src_to_trgt")); + InOrder inOrder = inOrder(sourceAuth, targetServerFilter, targetAuth, sourceServerFilter); + inOrder.verify(sourceAuth).attachUserIfMissing(SystemUser.INSTANCE); inOrder.verify(targetServerFilter).inbound(eq("_action"), eq(new Request("src_to_trgt")), isA(TransportChannel.class)); - inOrder.verify(targetClientFilter).outbound("_action", new Request("trgt_to_src")); + inOrder.verify(targetAuth).attachUserIfMissing(SystemUser.INSTANCE); inOrder.verify(sourceServerFilter).inbound(eq("_action"), eq(new Request("trgt_to_src")), isA(TransportChannel.class)); } @@ -120,7 +121,6 @@ public class TransportFilterTests extends ESIntegTestCase { public static class TestTransportFilterModule extends AbstractModule { @Override protected void configure() { - bind(ClientTransportFilter.class).toInstance(mock(ClientTransportFilter.class)); bind(AuthenticationService.class).toInstance(mock(AuthenticationService.class)); bind(AuthorizationService.class).toInstance(mock(AuthorizationService.class)); } @@ -286,9 +286,8 @@ public class TransportFilterTests extends ESIntegTestCase { @Inject public InternalPluginServerTransportService(Settings settings, Transport transport, ThreadPool threadPool, AuthenticationService authcService, AuthorizationService authzService, - SecurityActionMapper actionMapper, ClientTransportFilter clientTransportFilter) { - super(settings, transport, threadPool, authcService, authzService, actionMapper, clientTransportFilter, - mock(XPackLicenseState.class)); + SecurityActionMapper actionMapper) { + super(settings, transport, threadPool, authcService, authzService, actionMapper, mock(XPackLicenseState.class)); when(licenseState.isAuthAllowed()).thenReturn(true); } diff --git a/elasticsearch/x-pack/src/main/java/org/elasticsearch/xpack/XPackPlugin.java b/elasticsearch/x-pack/src/main/java/org/elasticsearch/xpack/XPackPlugin.java index aedafa3abfd..bb77061c008 100644 --- a/elasticsearch/x-pack/src/main/java/org/elasticsearch/xpack/XPackPlugin.java +++ b/elasticsearch/x-pack/src/main/java/org/elasticsearch/xpack/XPackPlugin.java @@ -285,7 +285,7 @@ public class XPackPlugin extends Plugin implements ScriptPlugin, ActionPlugin, I @Override public List> getSettings() { ArrayList> settings = new ArrayList<>(); - settings.addAll(security.getSettings()); + settings.addAll(Security.getSettings(transportClientMode)); settings.addAll(MonitoringSettings.getSettings()); settings.addAll(watcher.getSettings()); settings.addAll(licensing.getSettings());