Restructured and cleaned up guice modules

- ShieldModule loads the SecuredTransportModule which is in turn responsible for loading the required transport related sub-modules
- Removed n2n module and incorporated n2n into the transport module
- Updated the docs to reflect the new settings for n2n ip filtering

Original commit: elastic/x-pack-elasticsearch@dd82306a04

src/main/java/org/elasticsearch/shield/SecurityModule.java

@@ -15,10 +15,7 @@ import org.elasticsearch.common.settings.Settings;
 import org.elasticsearch.shield.audit.AuditTrailModule;
 import org.elasticsearch.shield.authc.AuthenticationModule;
 import org.elasticsearch.shield.authz.AuthorizationModule;
-import org.elasticsearch.shield.n2n.N2NAuthModule;
 import org.elasticsearch.shield.transport.SecuredTransportModule;
-import org.elasticsearch.shield.transport.netty.NettySecuredHttpServerTransportModule;
-import org.elasticsearch.shield.transport.netty.NettySecuredTransportModule;
 
 /**
  *
@@ -32,7 +29,7 @@ public class SecurityModule extends AbstractModule implements SpawnModules, PreP
     public SecurityModule(Settings settings) {
         this.settings = settings;
         this.isClient = settings.getAsBoolean("node.client", false);
-        this.isShieldEnabled = settings.getComponentSettings(SecurityModule.class).getAsBoolean("enabled", true);
+        this.isShieldEnabled = settings.getAsBoolean("shield.enabled", true);
     }
 
     @Override
@@ -51,19 +48,13 @@ public class SecurityModule extends AbstractModule implements SpawnModules, PreP
 
         // spawn needed parts in client mode
         if (isClient) {
-            return ImmutableList.of(
-                    new N2NAuthModule(),
-                    new SecuredTransportModule()
-            );
+            return ImmutableList.of(new SecuredTransportModule());
         }
 
         return ImmutableList.of(
                 new AuthenticationModule(settings),
                 new AuthorizationModule(),
                 new AuditTrailModule(settings),
-                new N2NAuthModule(),
-                new NettySecuredHttpServerTransportModule(),
-                new NettySecuredTransportModule(),
                 new SecuredTransportModule());
     }
 

src/main/java/org/elasticsearch/shield/authz/AuthorizationModule.java

@@ -16,7 +16,6 @@ public class AuthorizationModule extends AbstractModule {
 
     @Override
     protected void configure() {
-
         bind(RolesStore.class).to(FileRolesStore.class);
         bind(AuthorizationService.class).to(InternalAuthorizationService.class);
     }

src/main/java/org/elasticsearch/shield/n2n/N2NAuthModule.java

@@ -1,21 +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.shield.n2n;
-
-import org.elasticsearch.common.inject.AbstractModule;
-import org.elasticsearch.common.settings.Settings;
-
-/**
- *
- */
-public class N2NAuthModule extends AbstractModule {
-
-    @Override
-    protected void configure() {
-        bind(IPFilteringN2NAuthenticator.class).asEagerSingleton();
-        bind(N2NNettyUpstreamHandler.class).asEagerSingleton();
-    }
-}

src/main/java/org/elasticsearch/shield/transport/SecuredTransportModule.java

@@ -5,16 +5,44 @@
  */
 package org.elasticsearch.shield.transport;
 
+import org.elasticsearch.common.collect.ImmutableList;
 import org.elasticsearch.common.inject.AbstractModule;
+import org.elasticsearch.common.inject.Module;
+import org.elasticsearch.common.inject.PreProcessModule;
+import org.elasticsearch.common.inject.SpawnModules;
 import org.elasticsearch.shield.SecurityFilter;
+import org.elasticsearch.shield.plugin.SecurityPlugin;
+import org.elasticsearch.shield.transport.n2n.IPFilteringN2NAuthenticator;
+import org.elasticsearch.shield.transport.netty.N2NNettyUpstreamHandler;
+import org.elasticsearch.shield.transport.netty.NettySecuredHttpServerTransportModule;
+import org.elasticsearch.shield.transport.netty.NettySecuredTransportModule;
+import org.elasticsearch.transport.TransportModule;
 
 /**
  *
  */
-public class SecuredTransportModule extends AbstractModule {
+public class SecuredTransportModule extends AbstractModule implements SpawnModules, PreProcessModule {
+
+    @Override
+    public Iterable<? extends Module> spawnModules() {
+
+        //todo we only need to spawn http module if we're not within the transport client context
+        return ImmutableList.of(
+                new NettySecuredHttpServerTransportModule(),
+                new NettySecuredTransportModule());
+    }
+
+    @Override
+    public void processModule(Module module) {
+        if (module instanceof TransportModule) {
+            ((TransportModule) module).setTransportService(SecuredTransportService.class, SecurityPlugin.NAME);
+        }
+    }
 
     @Override
     protected void configure() {
         bind(TransportFilter.class).to(SecurityFilter.Transport.class).asEagerSingleton();
+        bind(IPFilteringN2NAuthenticator.class).asEagerSingleton();
+        bind(N2NNettyUpstreamHandler.class).asEagerSingleton();
     }
 }

src/main/java/org/elasticsearch/shield/transport/n2n/IPFilteringN2NAuthenticator.java

@@ -3,7 +3,7 @@
  * 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.shield.n2n;
+package org.elasticsearch.shield.transport.n2n;
 
 import org.elasticsearch.ElasticsearchParseException;
 import org.elasticsearch.common.Nullable;
@@ -44,7 +44,6 @@ public class IPFilteringN2NAuthenticator extends AbstractComponent implements N2
     private static final IpFilterRule[] NO_RULES = new IpFilterRule[0];
 
     private final Path file;
-    private final FileWatcher watcher;
 
     private volatile IpFilterRule[] rules = NO_RULES;
 
@@ -53,13 +52,13 @@ public class IPFilteringN2NAuthenticator extends AbstractComponent implements N2
         super(settings);
         file = resolveFile(componentSettings, env);
         rules = parseFile(file, logger);
-        watcher = new FileWatcher(file.getParent().toFile());
+        FileWatcher watcher = new FileWatcher(file.getParent().toFile());
         watcher.addListener(new FileListener());
         watcherService.add(watcher);
     }
 
     private Path resolveFile(Settings settings, Environment env) {
-        String location = settings.get("file");
+        String location = settings.get("ip_filter.file");
         if (location == null) {
             File shieldDirectory = new File(env.configFile(), SecurityPlugin.NAME);
             return shieldDirectory.toPath().resolve(DEFAULT_FILE);
@@ -75,40 +74,35 @@ public class IPFilteringN2NAuthenticator extends AbstractComponent implements N2
 
         List<IpFilterRule> rules = new ArrayList<>();
 
-        byte[] content;
-        try {
-            content = Files.readAllBytes(path);
-            try (XContentParser parser = YamlXContent.yamlXContent.createParser(content)) {
-                XContentParser.Token token;
-                String currentFieldName = null;
-                while ((token = parser.nextToken()) != XContentParser.Token.END_OBJECT && token != null) {
-                    if (token == XContentParser.Token.FIELD_NAME) {
-                        currentFieldName = parser.currentName();
-                        if (!"allow".equals(currentFieldName) && !"deny".equals(currentFieldName)) {
-                            throw new ElasticsearchParseException("Field name [" + currentFieldName + "] not valid. Must be [allow] or [deny]");
-                        }
-                    } else if (token == XContentParser.Token.VALUE_STRING && currentFieldName != null) {
-                        String value = parser.text();
-                        if (!Strings.hasLength(value)) {
-                            throw new ElasticsearchParseException("Field value for fieldname [" + currentFieldName + "] must not be empty");
-                        }
-
-                        boolean isAllowRule = currentFieldName.equals("allow");
-
-                        if (value.contains(",")) {
-                            for (String rule : COMMA_DELIM.split(parser.text().trim())) {
-                                rules.add(getRule(isAllowRule, rule));
-                            }
-                        } else {
-                            rules.add(getRule(isAllowRule, value));
-                        }
-
+        try (XContentParser parser = YamlXContent.yamlXContent.createParser(Files.newInputStream(path))) {
+            XContentParser.Token token;
+            String currentFieldName = null;
+            while ((token = parser.nextToken()) != XContentParser.Token.END_OBJECT && token != null) {
+                if (token == XContentParser.Token.FIELD_NAME) {
+                    currentFieldName = parser.currentName();
+                    if (!"allow".equals(currentFieldName) && !"deny".equals(currentFieldName)) {
+                        throw new ElasticsearchParseException("Field name [" + currentFieldName + "] not valid. Must be [allow] or [deny]");
                     }
+                } else if (token == XContentParser.Token.VALUE_STRING && currentFieldName != null) {
+                    String value = parser.text();
+                    if (!Strings.hasLength(value)) {
+                        throw new ElasticsearchParseException("Field value for fieldname [" + currentFieldName + "] must not be empty");
+                    }
+
+                    boolean isAllowRule = currentFieldName.equals("allow");
+
+                    if (value.contains(",")) {
+                        for (String rule : COMMA_DELIM.split(parser.text().trim())) {
+                            rules.add(getRule(isAllowRule, rule));
+                        }
+                    } else {
+                        rules.add(getRule(isAllowRule, value));
+                    }
+
                 }
             }
-
-        } catch (YAMLException|IOException ioe) {
-            throw new ElasticsearchParseException("Failed to read & parse host access file [" + path.toAbsolutePath() + "]", ioe);
+        } catch (IOException | YAMLException e) {
+            throw new ElasticsearchParseException("Failed to read & parse host access file [" + path.toAbsolutePath() + "]", e);
         }
 
         if (rules.size() == 0) {
@@ -134,9 +128,9 @@ public class IPFilteringN2NAuthenticator extends AbstractComponent implements N2
 
     @Override
     public boolean authenticate(@Nullable Principal peerPrincipal, InetAddress peerAddress, int peerPort) {
-        for (int i = 0; i < rules.length; i++) {
-            if (rules[i].contains(peerAddress)) {
-                boolean isAllowed =  rules[i].isAllowRule();
+        for (IpFilterRule rule : rules) {
+            if (rule.contains(peerAddress)) {
+                boolean isAllowed =  rule.isAllowRule();
                 logger.trace("Authentication rule matched for host [{}]: {}", peerAddress, isAllowed);
                 return isAllowed;
             }

src/main/java/org/elasticsearch/shield/transport/n2n/N2NAuthenticator.java

@@ -3,7 +3,7 @@
  * 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.shield.n2n;
+package org.elasticsearch.shield.transport.n2n;
 
 import org.elasticsearch.common.Nullable;
 

src/main/java/org/elasticsearch/shield/transport/netty/N2NNettyUpstreamHandler.java

@@ -3,13 +3,14 @@
  * 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.shield.n2n;
+package org.elasticsearch.shield.transport.netty;
 
 import org.elasticsearch.common.inject.Inject;
 import org.elasticsearch.common.netty.channel.ChannelEvent;
 import org.elasticsearch.common.netty.channel.ChannelHandler;
 import org.elasticsearch.common.netty.channel.ChannelHandlerContext;
 import org.elasticsearch.common.netty.handler.ipfilter.IpFilteringHandlerImpl;
+import org.elasticsearch.shield.transport.n2n.IPFilteringN2NAuthenticator;
 
 import java.net.InetSocketAddress;
 

src/main/java/org/elasticsearch/shield/transport/netty/NettySecuredHttpServerTransport.java

@@ -13,7 +13,6 @@ import org.elasticsearch.common.network.NetworkService;
 import org.elasticsearch.common.settings.Settings;
 import org.elasticsearch.common.util.BigArrays;
 import org.elasticsearch.http.netty.NettyHttpServerTransport;
-import org.elasticsearch.shield.n2n.N2NNettyUpstreamHandler;
 import org.elasticsearch.shield.transport.ssl.SSLConfig;
 
 import javax.net.ssl.SSLEngine;
@@ -57,7 +56,7 @@ public class NettySecuredHttpServerTransport extends NettyHttpServerTransport {
         @Override
         public ChannelPipeline getPipeline() throws Exception {
             ChannelPipeline pipeline = super.getPipeline();
-            if (settings.getAsBoolean("shield.n2n.enabled", true)) {
+            if (settings.getAsBoolean("shield.transport.n2n.ip_filter.enabled", true)) {
                 pipeline.addFirst("ipfilter", shieldUpstreamHandler);
             }
             if (ssl) {

src/main/java/org/elasticsearch/shield/transport/netty/NettySecuredHttpServerTransportModule.java

@@ -19,7 +19,7 @@ public class NettySecuredHttpServerTransportModule extends AbstractModule implem
     @Override
     public void processModule(Module module) {
         if (module instanceof HttpServerModule) {
-            ((HttpServerModule)module).setHttpServerTransport(NettySecuredHttpServerTransport.class, SecurityPlugin.NAME);
+            ((HttpServerModule) module).setHttpServerTransport(NettySecuredHttpServerTransport.class, SecurityPlugin.NAME);
         }
     }
 

src/main/java/org/elasticsearch/shield/transport/netty/NettySecuredTransport.java

@@ -13,7 +13,6 @@ import org.elasticsearch.common.netty.handler.ssl.SslHandler;
 import org.elasticsearch.common.network.NetworkService;
 import org.elasticsearch.common.settings.Settings;
 import org.elasticsearch.common.util.BigArrays;
-import org.elasticsearch.shield.n2n.N2NNettyUpstreamHandler;
 import org.elasticsearch.shield.transport.ssl.SSLConfig;
 import org.elasticsearch.threadpool.ThreadPool;
 import org.elasticsearch.transport.netty.NettyTransport;
@@ -64,7 +63,7 @@ public class NettySecuredTransport extends NettyTransport {
         @Override
         public ChannelPipeline getPipeline() throws Exception {
             ChannelPipeline pipeline = super.getPipeline();
-            if (settings.getAsBoolean("shield.n2n.enabled", true)) {
+            if (settings.getAsBoolean("shield.transport.n2n.ip_filter.enabled", true)) {
                 pipeline.addFirst("ipfilter", shieldUpstreamHandler);
             }
             if (ssl) {

src/main/java/org/elasticsearch/shield/transport/netty/NettySecuredTransportModule.java

@@ -19,7 +19,7 @@ public class NettySecuredTransportModule extends AbstractModule implements PrePr
     @Override
     public void processModule(Module module) {
         if (module instanceof TransportModule) {
-            ((TransportModule)module).setTransport(NettySecuredTransport.class, SecurityPlugin.NAME);
+            ((TransportModule) module).setTransport(NettySecuredTransport.class, SecurityPlugin.NAME);
         }
     }
 

src/test/java/org/elasticsearch/shield/n2n/N2NPlugin.java

@@ -1,32 +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.shield.n2n;
-
-import com.google.common.collect.ImmutableSet;
-import org.elasticsearch.common.inject.Module;
-import org.elasticsearch.plugins.AbstractPlugin;
-
-import java.util.Collection;
-
-/**
- * a plugin that just loads the N2NModule (required for transport integration tests)
- */
-public class N2NPlugin extends AbstractPlugin {
-    @Override
-    public String name() {
-        return "test-n2n-plugin";
-    }
-
-    @Override
-    public String description() {
-        return "";
-    }
-
-    @Override
-    public Collection<Class<? extends Module>> modules() {
-        return ImmutableSet.<Class<? extends Module>>of(N2NAuthModule.class);
-    }
-}

src/test/java/org/elasticsearch/shield/test/ShieldIntegrationTest.java

@@ -69,7 +69,7 @@ public abstract class ShieldIntegrationTest extends ElasticsearchIntegrationTest
                 .put("shield.authc.esusers.files.users", writeFile(folder, "users", CONFIG_STANDARD_USER))
                 .put("shield.authc.esusers.files.users_roles", writeFile(folder, "users_roles", CONFIG_STANDARD_USER_ROLES))
                 .put("shield.authz.store.files.roles", writeFile(folder, "roles.yml", CONFIG_ROLE_ALLOW_ALL))
-                .put("shield.n2n.file", writeFile(folder, "ip_filter.yml", CONFIG_IPFILTER_ALLOW_ALL))
+                .put("shield.transport.n2n.ip_filter.file", writeFile(folder, "ip_filter.yml", CONFIG_IPFILTER_ALLOW_ALL))
                 .put(getSSLSettingsForStore("/org/elasticsearch/shield/transport/ssl/certs/simple/testnode.jks", "testnode"))
                 .put("shield.audit.enabled", true);
 

src/test/java/org/elasticsearch/shield/transport/n2n/IPFilteringN2NAuthenticatorTests.java

@@ -3,7 +3,7 @@
  * 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.shield.n2n;
+package org.elasticsearch.shield.transport.n2n;
 
 import com.google.common.io.Files;
 import org.elasticsearch.ElasticsearchParseException;
@@ -145,7 +145,7 @@ public class IPFilteringN2NAuthenticatorTests extends ElasticsearchTestCase {
     private void writeConfigFile(String data) throws IOException {
         Files.write(data.getBytes(Charsets.UTF_8), configFile);
         resourceWatcherService = new ResourceWatcherService(resourceWatcherServiceSettings, new ThreadPool("resourceWatcher")).start();
-        settings = settingsBuilder().put("shield.n2n.file", configFile.getPath()).build();
+        settings = settingsBuilder().put("shield.transport.n2n.ip_filter.file", configFile.getPath()).build();
         ipFilteringN2NAuthenticator = new IPFilteringN2NAuthenticator(settings, new Environment(), resourceWatcherService);
     }
 

src/test/java/org/elasticsearch/shield/transport/n2n/IpFilteringIntegrationTests.java

@@ -3,7 +3,7 @@
  * 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.shield.n2n;
+package org.elasticsearch.shield.transport.n2n;
 
 import com.google.common.base.Charsets;
 import org.elasticsearch.common.settings.ImmutableSettings;
@@ -40,9 +40,9 @@ public class IpFilteringIntegrationTests extends ShieldIntegrationTest {
         // either deny all or do not have a configuration file, as this denies by default
         if (getRandom().nextBoolean()) {
             File folder = newFolder();
-            builder.put("shield.n2n.file", writeFile(folder, "ip_filter.yml", CONFIG_IPFILTER_DENY_ALL));
+            builder.put("shield.transport.n2n.ip_filter.file", writeFile(folder, "ip_filter.yml", CONFIG_IPFILTER_DENY_ALL));
         } else {
-            builder.remove("shield.n2n.file");
+            builder.remove("shield.transport.n2n.ip_filter.file");
         }
 
         return builder.build();

src/test/java/org/elasticsearch/shield/transport/netty/N2NNettyUpstreamHandlerTests.java

@@ -3,7 +3,7 @@
  * 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.shield.n2n;
+package org.elasticsearch.shield.transport.netty;
 
 import com.google.common.base.Charsets;
 import com.google.common.io.Files;
@@ -12,6 +12,7 @@ import org.elasticsearch.common.netty.channel.*;
 import org.elasticsearch.common.settings.ImmutableSettings;
 import org.elasticsearch.common.settings.Settings;
 import org.elasticsearch.env.Environment;
+import org.elasticsearch.shield.transport.n2n.IPFilteringN2NAuthenticator;
 import org.elasticsearch.test.ElasticsearchTestCase;
 import org.elasticsearch.threadpool.ThreadPool;
 import org.elasticsearch.watcher.ResourceWatcherService;
@@ -45,7 +46,7 @@ public class N2NNettyUpstreamHandlerTests extends ElasticsearchTestCase {
         String testData = "allow: 127.0.0.1\ndeny: 10.0.0.0/8";
         Files.write(testData.getBytes(Charsets.UTF_8), configFile);
 
-        Settings settings = settingsBuilder().put("shield.n2n.file", configFile.getPath()).build();
+        Settings settings = settingsBuilder().put("shield.transport.n2n.ip_filter.file", configFile.getPath()).build();
         IPFilteringN2NAuthenticator ipFilteringN2NAuthenticator = new IPFilteringN2NAuthenticator(settings, new Environment(), resourceWatcherService);
 
         nettyUpstreamHandler = new N2NNettyUpstreamHandler(ipFilteringN2NAuthenticator);

src/test/java/org/elasticsearch/shield/transport/ssl/SslIntegrationTests.java

@@ -117,7 +117,7 @@ public class SslIntegrationTests extends ShieldIntegrationTest {
                 .put("request.headers.Authorization", basicAuthHeaderValue(getClientUsername(), getClientPassword().toCharArray()))
                 .put(TransportModule.TRANSPORT_TYPE_KEY, NettySecuredTransport.class.getName())
                 .put("plugins." + PluginsService.LOAD_PLUGIN_FROM_CLASSPATH, false)
-                .put("shield.n2n.file", writeFile(newFolder(), "ip_filter.yml", ShieldIntegrationTest.CONFIG_IPFILTER_ALLOW_ALL))
+                .put("shield.transport.n2n.ip_filter.file", writeFile(newFolder(), "ip_filter.yml", ShieldIntegrationTest.CONFIG_IPFILTER_ALLOW_ALL))
 
                 .put(getSSLSettingsForStore("certs/simple/testclient.jks", "testclient"))
                 .build();