From 7472b5014423e3b9bbaac944941e7857088b2bce Mon Sep 17 00:00:00 2001
From: c-a-m <cam.morris@owasp.org>
Date: Mon, 10 Nov 2014 11:59:48 -0700
Subject: [PATCH] Fix listing of users without roles

When no users have roles, esusers will now correctly list all users.

Fixes https://github.com/elasticsearch/elasticsearch-shield/issues/315

Original commit: elastic/x-pack-elasticsearch@12f4c171d8a7316a98e85af35c61f9adba0ffbd6
---
 .../authc/esusers/tool/ESUsersTool.java       |  8 +++----
 .../authc/esusers/tool/ESUsersToolTests.java  | 24 ++++++++++++++++++-
 2 files changed, 27 insertions(+), 5 deletions(-)

diff --git a/src/main/java/org/elasticsearch/shield/authc/esusers/tool/ESUsersTool.java b/src/main/java/org/elasticsearch/shield/authc/esusers/tool/ESUsersTool.java
index d1609c3a97e..935a6ad9c15 100644
--- a/src/main/java/org/elasticsearch/shield/authc/esusers/tool/ESUsersTool.java
+++ b/src/main/java/org/elasticsearch/shield/authc/esusers/tool/ESUsersTool.java
@@ -385,11 +385,11 @@ public class ESUsersTool extends CliTool {
                 }
                 // list users without roles
                 Set<String> usersWithoutRoles = Sets.newHashSet(users);
-                if (usersWithoutRoles.removeAll(userRoles.keySet())) {
-                    for (String user : usersWithoutRoles) {
-                        terminal.println("%-15s: -", user);
-                    }
+                usersWithoutRoles.removeAll(userRoles.keySet());
+                for (String user : usersWithoutRoles) {
+                    terminal.println("%-15s: -", user);
                 }
+
                 if (unknownRolesFound) {
                     // at least one role is marked... so printing the legend
                     Path rolesFile = FileRolesStore.resolveFile(settings, env).toAbsolutePath();
diff --git a/src/test/java/org/elasticsearch/shield/authc/esusers/tool/ESUsersToolTests.java b/src/test/java/org/elasticsearch/shield/authc/esusers/tool/ESUsersToolTests.java
index 1f05fa6ee73..cecea1eaea0 100644
--- a/src/test/java/org/elasticsearch/shield/authc/esusers/tool/ESUsersToolTests.java
+++ b/src/test/java/org/elasticsearch/shield/authc/esusers/tool/ESUsersToolTests.java
@@ -598,7 +598,7 @@ public class ESUsersToolTests extends CliToolTestCase {
     }
 
     @Test
-    public void testListUsersAndRoles_Cmd_testThatUsersWithoutRolesAreListed() throws Exception {
+    public void testListUsersAndRoles_Cmd_testThatUsersWithAndWithoutRolesAreListed() throws Exception {
         File usersFile = writeFile("admin:{plain}changeme\nuser:{plain}changeme\nno-roles-user:{plain}changeme\n");
         File usersRoleFile = writeFile("admin: admin\nuser: user\nfoo:user\nbar:user\n");
         File rolesFile = writeFile("admin:\n  cluster: all\n\nuser:\n  cluster: all\n\nfoo:\n  cluster: all\n\nbar:\n  cluster: all");
@@ -619,6 +619,28 @@ public class ESUsersToolTests extends CliToolTestCase {
         assertThat(catchTerminalOutput.getTerminalOutput(), hasItem(allOf(containsString("no-roles-user"), containsString("-"))));
     }
 
+    @Test
+    public void testListUsersAndRoles_Cmd_testThatUsersWithoutRolesAreListed() throws Exception {
+        File usersFile = writeFile("admin:{plain}changeme\nuser:{plain}changeme\nno-roles-user:{plain}changeme\n");
+        File usersRoleFile = writeFile("");
+        File rolesFile = writeFile("admin:\n  cluster: all\n\nuser:\n  cluster: all\n\nfoo:\n  cluster: all\n\nbar:\n  cluster: all");
+        Settings settings = ImmutableSettings.builder()
+                .put("shield.authc.esusers.files.users_roles", usersRoleFile)
+                .put("shield.authc.esusers.files.users", usersFile)
+                .put("shield.authz.store.files.roles", rolesFile)
+                .build();
+
+        CaptureOutputTerminal catchTerminalOutput = new CaptureOutputTerminal();
+        ESUsersTool.ListUsersAndRoles cmd = new ESUsersTool.ListUsersAndRoles(catchTerminalOutput, null);
+        CliTool.ExitStatus status = execute(cmd, settings);
+
+        assertThat(status, is(CliTool.ExitStatus.OK));
+        assertThat(catchTerminalOutput.getTerminalOutput(), hasSize(greaterThanOrEqualTo(3)));
+        assertThat(catchTerminalOutput.getTerminalOutput(), hasItem(allOf(containsString("admin"), containsString("-"))));
+        assertThat(catchTerminalOutput.getTerminalOutput(), hasItem(allOf(containsString("user"), containsString("-"))));
+        assertThat(catchTerminalOutput.getTerminalOutput(), hasItem(allOf(containsString("no-roles-user"), containsString("-"))));
+    }
+
     @Test
     public void testListUsersAndRoles_Cmd_testThatUsersWithoutRolesAreListedForSingleUser() throws Exception {
         File usersFile = writeFile("admin:{plain}changeme");