do not attempt to resolve permissions in the esusers tool

The esusers tool reads the list of roles to provide validation feedback to the user,
however since we have added custom roles the tool doesn't know about these roles
as they come from outside of Shield. When a custom role was found, a warning
was printed that can be confusing to users.

Now when validating roles, we only read the names from the roles.yml file.

Closes elastic/elasticsearch#835

Original commit: elastic/x-pack-elasticsearch@89d0e3efce

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

@@ -21,7 +21,6 @@ import org.elasticsearch.shield.authc.esusers.FileUserPasswdStore;
 import org.elasticsearch.shield.authc.esusers.FileUserRolesStore;
 import org.elasticsearch.shield.authc.support.Hasher;
 import org.elasticsearch.shield.authc.support.SecuredString;
-import org.elasticsearch.shield.authz.Permission;
 import org.elasticsearch.shield.authz.store.FileRolesStore;
 import org.elasticsearch.shield.support.Validation;
 
@@ -430,7 +429,7 @@ public class ESUsersTool extends CliTool {
         @Override
         public ExitStatus execute(Settings settings, Environment env) throws Exception {
             Settings esusersSettings = Realms.internalRealmSettings(settings, ESUsersRealm.TYPE);
-            ImmutableMap<String, Permission.Global.Role> knownRoles = loadRoles(terminal, settings, env);
+            ImmutableSet<String> knownRoles = loadRoleNames(terminal, settings, env);
             Path userRolesFilePath = FileUserRolesStore.resolveFile(esusersSettings, env);
             Map<String, String[]> userRoles = FileUserRolesStore.parseFile(userRolesFilePath, null);
             Path userFilePath = FileUserPasswdStore.resolveFile(esusersSettings, env);
@@ -444,7 +443,7 @@ public class ESUsersTool extends CliTool {
 
                 if (userRoles.containsKey(username)) {
                     String[] roles = userRoles.get(username);
-                    Set<String> unknownRoles = Sets.difference(Sets.newHashSet(roles), knownRoles.keySet());
+                    Set<String> unknownRoles = Sets.difference(Sets.newHashSet(roles), knownRoles);
                     String[] markedRoles = markUnknownRoles(roles, unknownRoles);
                     terminal.println("%-15s: %s", username, Joiner.on(",").useForNull("-").join(markedRoles));
                     if (!unknownRoles.isEmpty()) {
@@ -461,7 +460,7 @@ public class ESUsersTool extends CliTool {
                 boolean usersExist = false;
                 for (Map.Entry<String, String[]> entry : userRoles.entrySet()) {
                     String[] roles = entry.getValue();
-                    Set<String> unknownRoles = Sets.difference(Sets.newHashSet(roles), knownRoles.keySet());
+                    Set<String> unknownRoles = Sets.difference(Sets.newHashSet(roles), knownRoles);
                     String[] markedRoles = markUnknownRoles(roles, unknownRoles);
                     terminal.println("%-15s: %s", entry.getKey(), Joiner.on(",").join(markedRoles));
                     unknownRolesFound = unknownRolesFound || !unknownRoles.isEmpty();
@@ -492,10 +491,10 @@ public class ESUsersTool extends CliTool {
         }
     }
 
-    private static ImmutableMap<String, Permission.Global.Role> loadRoles(Terminal terminal, Settings settings, Environment env) {
+    private static ImmutableSet<String> loadRoleNames(Terminal terminal, Settings settings, Environment env) {
         Path rolesFile = FileRolesStore.resolveFile(settings, env);
         try {
-            return FileRolesStore.parseFile(rolesFile, null);
+            return FileRolesStore.parseFileForRoleNames(rolesFile, null);
         } catch (Throwable t) {
             // if for some reason, parsing fails (malformatted perhaps) we just warn
             terminal.println("Warning:  Could not parse [%s] for roles verification. Please revise and fix it. Nonetheless, the user will still be associated with all specified roles", rolesFile.toAbsolutePath());
@@ -519,11 +518,8 @@ public class ESUsersTool extends CliTool {
     }
 
     private static void verifyRoles(Terminal terminal, Settings settings, Environment env, String[] roles) {
-        ImmutableMap<String, Permission.Global.Role> knownRoles = loadRoles(terminal, settings, env);
+        ImmutableSet<String> knownRoles = loadRoleNames(terminal, settings, env);
-        if (knownRoles == null) {
+        Set<String> unknownRoles = Sets.difference(Sets.newHashSet(roles), knownRoles);
-            return;
-        }
-        Set<String> unknownRoles = Sets.difference(Sets.newHashSet(roles), knownRoles.keySet());
         if (!unknownRoles.isEmpty()) {
             Path rolesFile = FileRolesStore.resolveFile(settings, env);
             terminal.println("Warning: The following roles [%s] are unknown. Make sure to add them to the [%s] file. " +

src/main/java/org/elasticsearch/shield/authz/store/FileRolesStore.java

@@ -100,11 +100,19 @@ public class FileRolesStore extends AbstractLifecycleComponent<RolesStore> imple
         return env.homeFile().resolve(location);
     }
 
-    public static ImmutableMap<String, Permission.Global.Role> parseFile(Path path, ESLogger logger) {
+    public static ImmutableSet<String> parseFileForRoleNames(Path path, ESLogger logger) {
-        return parseFile(path, Collections.<Permission.Global.Role>emptySet(), logger);
+        ImmutableMap<String, Permission.Global.Role> roleMap = parseFile(path, Collections.<Permission.Global.Role>emptySet(), logger, false);
+        if (roleMap == null) {
+            return ImmutableSet.<String>builder().build();
+        }
+        return roleMap.keySet();
     }
 
     public static ImmutableMap<String, Permission.Global.Role> parseFile(Path path, Set<Permission.Global.Role> reservedRoles, ESLogger logger) {
+        return parseFile(path, reservedRoles, logger, true);
+    }
+
+    public static ImmutableMap<String, Permission.Global.Role> parseFile(Path path, Set<Permission.Global.Role> reservedRoles, ESLogger logger, boolean resolvePermission) {
         if (logger == null) {
             logger = NoOpLogger.INSTANCE;
         }
@@ -121,7 +129,7 @@ public class FileRolesStore extends AbstractLifecycleComponent<RolesStore> imple
 
             List<String> roleSegments = roleSegments(path);
             for (String segment : roleSegments) {
-                Permission.Global.Role role = parseRole(segment, path, logger);
+                Permission.Global.Role role = parseRole(segment, path, logger, resolvePermission);
                 if (role != null) {
                     if (SystemRole.NAME.equals(role.name())) {
                         logger.warn("role [{}] is reserved to the system. the relevant role definition in the mapping file will be ignored", SystemRole.NAME);
@@ -146,7 +154,7 @@ public class FileRolesStore extends AbstractLifecycleComponent<RolesStore> imple
         return ImmutableMap.copyOf(roles);
     }
 
-    private static Permission.Global.Role parseRole(String segment, Path path, ESLogger logger) {
+    private static Permission.Global.Role parseRole(String segment, Path path, ESLogger logger, boolean resolvePermissions) {
         String roleName = null;
         try {
             XContentParser parser = YamlXContent.yamlXContent.createParser(segment);
@@ -160,7 +168,12 @@ public class FileRolesStore extends AbstractLifecycleComponent<RolesStore> imple
                         logger.error("invalid role definition [{}] in roles file [{}]. invalid role name - {}. skipping role... ", roleName, path.toAbsolutePath(), validationError);
                         return null;
                     }
+
                     Permission.Global.Role.Builder permission = Permission.Global.Role.builder(roleName);
+                    if (resolvePermissions == false) {
+                        return permission.build();
+                    }
+
                     token = parser.nextToken();
                     if (token == XContentParser.Token.START_OBJECT) {
                         String currentFieldName = null;

src/test/java/org/elasticsearch/shield/authc/esusers/tool/ESUsersToolTests.java

@@ -213,6 +213,52 @@ public class ESUsersToolTests extends CliToolTestCase {
         assertThat(status, is(CliTool.ExitStatus.CODE_ERROR));
     }
 
+
+    @Test
+    public void testUseradd_CustomRole() throws Exception {
+        Path usersFile = createTempFile();
+        Path userRolesFile = createTempFile();
+        Path rolesFile = writeFile("plugin_admin:\n" +
+                "  manage_plugin");
+        Settings settings = Settings.builder()
+                .put("shield.authc.realms.esusers.type", "esusers")
+                .put("shield.authc.realms.esusers.files.users", usersFile)
+                .put("shield.authc.realms.esusers.files.users_roles", userRolesFile)
+                .put("shield.authz.store.files.roles", rolesFile)
+                .put("path.home", createTempDir())
+                .build();
+
+        final CaptureOutputTerminal terminal = new CaptureOutputTerminal();
+        ESUsersTool.Useradd cmd = new ESUsersTool.Useradd(terminal, "user1", SecuredStringTests.build("changeme"), "plugin_admin");
+
+        CliTool.ExitStatus status = execute(cmd, settings);
+        assertThat(status, is(CliTool.ExitStatus.OK));
+        assertThat(terminal.getTerminalOutput(), hasSize(0));
+    }
+
+    @Test
+    public void testUseradd_NonExistantRole() throws Exception {
+        Path usersFile = createTempFile();
+        Path userRolesFile = createTempFile();
+        Path rolesFile = writeFile("plugin_admin:\n" +
+                "  manage_plugin");
+        Settings settings = Settings.builder()
+                .put("shield.authc.realms.esusers.type", "esusers")
+                .put("shield.authc.realms.esusers.files.users", usersFile)
+                .put("shield.authc.realms.esusers.files.users_roles", userRolesFile)
+                .put("shield.authz.store.files.roles", rolesFile)
+                .put("path.home", createTempDir())
+                .build();
+
+        final CaptureOutputTerminal terminal = new CaptureOutputTerminal();
+        ESUsersTool.Useradd cmd = new ESUsersTool.Useradd(terminal, "user1", SecuredStringTests.build("changeme"), "plugin_admin_2");
+
+        CliTool.ExitStatus status = execute(cmd, settings);
+        assertThat(status, is(CliTool.ExitStatus.OK));
+        assertThat(terminal.getTerminalOutput(), hasSize(1));
+        assertThat(terminal.getTerminalOutput().get(0), containsString("[plugin_admin_2]"));
+    }
+
     @Test
     public void testUserdel_Parse() throws Exception {
         ESUsersTool tool = new ESUsersTool();

src/test/java/org/elasticsearch/shield/authz/PermissionTests.java

@@ -15,9 +15,7 @@ import org.junit.Test;
 import java.util.Iterator;
 
 import static org.elasticsearch.shield.authz.Privilege.Index.*;
-import static org.hamcrest.Matchers.equalTo;
+import static org.hamcrest.Matchers.*;
-import static org.hamcrest.Matchers.instanceOf;
-import static org.hamcrest.Matchers.is;
 
 /**
  *
@@ -64,6 +62,13 @@ public class PermissionTests extends ElasticsearchTestCase {
         assertThat(count, is(equalTo(permission.indices().groups().length)));
     }
 
+    @Test
+    public void buildEmptyRole() {
+        Permission.Global.Role.Builder permission = Permission.Global.Role.builder("some_role");
+        Permission.Global.Role role = permission.build();
+        assertThat(role, notNullValue());
+    }
+
     // "baz_*foo", "/fool.*bar/"
     private void testAllowedIndicesMatcher(Predicate<String> indicesMatcher) {
         assertThat(indicesMatcher.apply("foobar"), is(false));

src/test/java/org/elasticsearch/shield/authz/store/FileRolesStoreTests.java

@@ -41,7 +41,7 @@ public class FileRolesStoreTests extends ElasticsearchTestCase {
     @Test
     public void testParseFile() throws Exception {
         Path path = getDataPath("roles.yml");
-        Map<String, Permission.Global.Role> roles = FileRolesStore.parseFile(path, logger);
+        Map<String, Permission.Global.Role> roles = FileRolesStore.parseFile(path, Collections.<Permission.Global.Role>emptySet(), logger);
         assertThat(roles, notNullValue());
         assertThat(roles.size(), is(4));
 
@@ -107,7 +107,7 @@ public class FileRolesStoreTests extends ElasticsearchTestCase {
     @Test
     public void testDefaultRolesFile() throws Exception {
         Path path = getDataPath("default_roles.yml");
-        Map<String, Permission.Global.Role> roles = FileRolesStore.parseFile(path, logger);
+        Map<String, Permission.Global.Role> roles = FileRolesStore.parseFile(path, Collections.<Permission.Global.Role>emptySet(), logger);
         assertThat(roles, notNullValue());
         assertThat(roles.size(), is(8));
 
@@ -187,7 +187,7 @@ public class FileRolesStoreTests extends ElasticsearchTestCase {
     public void testThatEmptyFileDoesNotResultInLoop() throws Exception {
         Path file = createTempFile();
         Files.write(file, ImmutableList.of("#"), Charsets.UTF_8);
-        Map<String, Permission.Global.Role> roles = FileRolesStore.parseFile(file, logger);
+        Map<String, Permission.Global.Role> roles = FileRolesStore.parseFile(file, Collections.<Permission.Global.Role>emptySet(), logger);
         assertThat(roles.keySet(), is(empty()));
     }
 
@@ -195,7 +195,7 @@ public class FileRolesStoreTests extends ElasticsearchTestCase {
     public void testThatInvalidRoleDefinitions() throws Exception {
         Path path = getDataPath("invalid_roles.yml");
         CapturingLogger logger = new CapturingLogger(CapturingLogger.Level.ERROR);
-        Map<String, Permission.Global.Role> roles = FileRolesStore.parseFile(path, logger);
+        Map<String, Permission.Global.Role> roles = FileRolesStore.parseFile(path, Collections.<Permission.Global.Role>emptySet(), logger);
         assertThat(roles.size(), is(1));
         assertThat(roles, hasKey("valid_role"));
         Permission.Global.Role role = roles.get("valid_role");
@@ -211,6 +211,19 @@ public class FileRolesStoreTests extends ElasticsearchTestCase {
         assertThat(entries.get(4).text, startsWith("invalid role definition [role4] in roles file [" + path.toAbsolutePath() + "]. could not resolve indices privileges [al;kjdlkj;lkj]"));
     }
 
+    @Test
+    public void testThatRoleNamesDoesNotResolvePermissions() throws Exception {
+        Path path = getDataPath("invalid_roles.yml");
+        CapturingLogger logger = new CapturingLogger(CapturingLogger.Level.ERROR);
+        ImmutableSet<String> roleNames = FileRolesStore.parseFileForRoleNames(path, logger);
+        assertThat(roleNames.size(), is(5));
+        assertThat(roleNames, containsInAnyOrder("valid_role", "role1", "role2", "role3", "role4"));
+
+        List<CapturingLogger.Msg> entries = logger.output(CapturingLogger.Level.ERROR);
+        assertThat(entries, hasSize(1));
+        assertThat(entries.get(0).text, startsWith("invalid role definition [$dlk39] in roles file [" + path.toAbsolutePath() + "]. invalid role name"));
+    }
+
     @Test
     public void testReservedRoles() throws Exception {
         Set<Permission.Global.Role> reservedRoles = ImmutableSet.<Permission.Global.Role>builder()