From fe7d79384f2d7b9dc1a3e1ff9d7497ec44cc0a71 Mon Sep 17 00:00:00 2001
From: Alexander Reelsen <alexander@reelsen.net>
Date: Mon, 6 Oct 2014 09:09:55 +0200
Subject: [PATCH] CLI: Fix esusers tool to not create bogus role entry

If a user was created, but the user was not supplied roles on the commandline,
a bogus 'user:' was added to the roles file. This fix checks, if roles were
supplied when creating a user and only changes the roles file in that case.

Original commit: elastic/x-pack-elasticsearch@286951c0167f032fa25523584cf63edec25c4553
---
 .../authc/esusers/tool/ESUsersTool.java       | 11 +++++-----
 .../authc/esusers/tool/ESUsersToolTests.java  | 20 +++++++++++++++++++
 2 files changed, 26 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 e1d5a852c31..c9769dcf062 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
@@ -127,11 +127,12 @@ public class ESUsersTool extends CliTool {
             users.put(username, hasher.hash(passwd));
             FileUserPasswdStore.writeFile(users, file);
 
-
-            file = FileUserRolesStore.resolveFile(settings, env);
-            Map<String, String[]> userRoles = new HashMap<>(FileUserRolesStore.parseFile(file, null));
-            userRoles.put(username, roles);
-            FileUserRolesStore.writeFile(userRoles, file);
+            if (roles != null && roles.length > 0) {
+                file = FileUserRolesStore.resolveFile(settings, env);
+                Map<String, String[]> userRoles = new HashMap<>(FileUserRolesStore.parseFile(file, null));
+                userRoles.put(username, roles);
+                FileUserRolesStore.writeFile(userRoles, file);
+            }
             return ExitStatus.OK;
         }
     }
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 f993a12eea1..f4bb5fe5544 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
@@ -142,6 +142,26 @@ public class ESUsersToolTests extends CliToolTestCase {
         assertThat(lines, containsInAnyOrder("user2:r3,r4", "user1:r1,r2"));
     }
 
+    @Test
+    public void testUseradd_Cmd_AddingUserWithoutRolesDoesNotAddEmptyRole() throws Exception {
+        File userFile = writeFile("user2:hash2");
+        File userRolesFile = writeFile("user2:r3,r4");
+        Settings settings = ImmutableSettings.builder()
+                .put("shield.authc.esusers.files.users", userFile)
+                .put("shield.authc.esusers.files.users_roles", userRolesFile)
+                .build();
+
+        ESUsersTool.Useradd cmd = new ESUsersTool.Useradd(new MockTerminal(), "user1", SecuredStringTests.build("changeme"));
+
+        CliTool.ExitStatus status = execute(cmd, settings);
+        assertThat(status, is(CliTool.ExitStatus.OK));
+
+        assertFileExists(userRolesFile);
+        List<String> lines = Files.readLines(userRolesFile, Charsets.UTF_8);
+        assertThat(lines, hasSize(1));
+        assertThat(lines, not(hasItem(startsWith("user1"))));
+    }
+
     @Test
     public void testUseradd_Cmd_Append_UserAlreadyExists() throws Exception {
         File userFile = writeFile("user1:hash1");