Small fixes

- throw an error on initialization if the audit trail is configured with an unknown output
- removed unnecessary null checks

Original commit: elastic/x-pack-elasticsearch@8d26723553

src/main/java/org/elasticsearch/shield/audit/AuditTrailModule.java

@@ -5,6 +5,7 @@
  */
 package org.elasticsearch.shield.audit;
 
+import org.elasticsearch.ElasticsearchException;
 import org.elasticsearch.common.collect.Sets;
 import org.elasticsearch.common.inject.AbstractModule;
 import org.elasticsearch.common.inject.multibindings.Multibinder;
@@ -44,6 +45,8 @@ public class AuditTrailModule extends AbstractModule {
                 case LoggingAuditTrail.NAME:
                     binder.addBinding().to(LoggingAuditTrail.class);
                     break;
+                default:
+                    throw new ElasticsearchException("Unknown audit trail output [" + output + "]");
             }
         }
     }

src/main/java/org/elasticsearch/shield/authc/esusers/FileUserPasswdStore.java

@@ -81,7 +81,11 @@ public class FileUserPasswdStore extends AbstractComponent implements UserPasswd
         return Paths.get(location);
     }
 
-    public static Map<String, char[]> parseFile(Path path, @Nullable ESLogger logger) {
+    /**
+     * parses the esusers file. Should never return {@code null}, if the file doesn't exist an
+     * empty map is returned
+     */
+    public static ImmutableMap<String, char[]> parseFile(Path path, @Nullable ESLogger logger) {
         if (!Files.exists(path)) {
             return ImmutableMap.of();
         }

src/main/java/org/elasticsearch/shield/authc/esusers/FileUserRolesStore.java

@@ -54,7 +54,7 @@ public class FileUserRolesStore extends AbstractComponent implements UserRolesSt
     FileUserRolesStore(Settings settings, Environment env, ResourceWatcherService watcherService, Listener listener) {
         super(settings);
         file = resolveFile(settings, env);
-        userRoles = ImmutableMap.copyOf(parseFile(file, logger));
+        userRoles = parseFile(file, logger);
         FileWatcher watcher = new FileWatcher(file.getParent().toFile());
         watcher.addListener(new FileListener());
         watcherService.add(watcher);
@@ -74,7 +74,11 @@ public class FileUserRolesStore extends AbstractComponent implements UserRolesSt
         return Paths.get(location);
     }
 
-    public static Map<String, String[]> parseFile(Path path, @Nullable ESLogger logger) {
+    /**
+     * parses the users_roles file. Should never return return {@code null}, if the file doesn't exist
+     * an empty map is returned
+     */
+    public static ImmutableMap<String, String[]> parseFile(Path path, @Nullable ESLogger logger) {
         if (!Files.exists(path)) {
             return ImmutableMap.of();
         }

src/main/java/org/elasticsearch/shield/authc/esusers/tool/ESUsersTool.java

@@ -11,16 +11,14 @@ import org.elasticsearch.common.cli.CliTool;
 import org.elasticsearch.common.cli.CliToolConfig;
 import org.elasticsearch.common.cli.Terminal;
 import org.elasticsearch.common.cli.commons.CommandLine;
-import org.elasticsearch.common.collect.Lists;
+import org.elasticsearch.common.collect.*;
-import org.elasticsearch.common.collect.Maps;
-import org.elasticsearch.common.collect.ObjectArrays;
-import org.elasticsearch.common.collect.Sets;
 import org.elasticsearch.common.settings.Settings;
 import org.elasticsearch.env.Environment;
 import org.elasticsearch.shield.authc.esusers.FileUserPasswdStore;
 import org.elasticsearch.shield.authc.esusers.FileUserRolesStore;
 import org.elasticsearch.shield.authc.support.Hasher;
 
+import java.nio.file.Files;
 import java.nio.file.Path;
 import java.util.*;
 import java.util.regex.Pattern;
@@ -117,10 +115,6 @@ public class ESUsersTool extends CliTool {
         public ExitStatus execute(Settings settings, Environment env) throws Exception {
             Path file = FileUserPasswdStore.resolveFile(settings, env);
             Map<String, char[]> users = new HashMap<>(FileUserPasswdStore.parseFile(file, null));
-            if (users == null) {
-                // file doesn't exist so we just create a new file
-                users = new HashMap<>();
-            }
             if (users.containsKey(username)) {
                 terminal.println("User [%s] already exists", username);
                 return ExitStatus.CODE_ERROR;
@@ -132,10 +126,6 @@ public class ESUsersTool extends CliTool {
 
             file = FileUserRolesStore.resolveFile(settings, env);
             Map<String, String[]> userRoles = new HashMap<>(FileUserRolesStore.parseFile(file, null));
-            if (userRoles == null) {
-                // file doesn't exist, so we just create a new file
-                userRoles = new HashMap<>();
-            }
             userRoles.put(username, roles);
             FileUserRolesStore.writeFile(userRoles, file);
             return ExitStatus.OK;
@@ -168,7 +158,7 @@ public class ESUsersTool extends CliTool {
         public ExitStatus execute(Settings settings, Environment env) throws Exception {
             Path file = FileUserPasswdStore.resolveFile(settings, env);
             Map<String, char[]> users = new HashMap<>(FileUserPasswdStore.parseFile(file, null));
-            if (users != null) {
+            if (Files.exists(file)) {
                 char[] passwd = users.remove(username);
                 if (passwd != null) {
                     FileUserPasswdStore.writeFile(users, file);
@@ -179,7 +169,7 @@ public class ESUsersTool extends CliTool {
 
             file = FileUserRolesStore.resolveFile(settings, env);
             Map<String, String[]> userRoles = new HashMap<>(FileUserRolesStore.parseFile(file, null));
-            if (userRoles != null) {
+            if (Files.exists(file)) {
                 String[] roles = userRoles.remove(username);
                 if (roles != null) {
                     FileUserRolesStore.writeFile(userRoles, file);
@@ -234,10 +224,6 @@ public class ESUsersTool extends CliTool {
         public ExitStatus execute(Settings settings, Environment env) throws Exception {
             Path file = FileUserPasswdStore.resolveFile(settings, env);
             Map<String, char[]> users = new HashMap<>(FileUserPasswdStore.parseFile(file, null));
-            if (users == null) {
-                // file doesn't exist so we just create a new file
-                users = new HashMap<>();
-            }
             if (!users.containsKey(username)) {
                 terminal.println("User [%s] doesn't exist", username);
                 return ExitStatus.NO_USER;

src/test/java/org/elasticsearch/shield/audit/AuditTrailModuleTests.java

@@ -53,4 +53,18 @@ public class AuditTrailModuleTests extends ElasticsearchTestCase {
         assertThat(service.auditTrails[0], instanceOf(LoggingAuditTrail.class));
     }
 
+    @Test
+    public void testUnknownOutput() throws Exception {
+        Settings settings = ImmutableSettings.builder()
+                .put("shield.audit.enabled", true)
+                .put("shield.audit.outputs" , "foo")
+                .build();
+        try {
+            Guice.createInjector(new SettingsModule(settings), new AuditTrailModule(settings));
+            fail("Expect initialization to fail when an unknown audit trail output is configured");
+        } catch (Throwable t) {
+            // expected
+        }
+    }
+
 }