From 52ba1ceff3a7cc7e9a8fb8f3ec7a9ed4779c23f3 Mon Sep 17 00:00:00 2001 From: Alexander Reelsen Date: Thu, 28 Aug 2014 14:50:08 +0200 Subject: [PATCH] CliTool: Fixed adding of roles for existing users The roles file had been checked instead of the users file when checking if the user already exists. This lead to wrong "user not found" error messages. Original commit: elastic/x-pack-elasticsearch@6fc5646ce7801451f0c3a2318acb76af09e5bbec --- .../authc/esusers/tool/ESUsersTool.java | 16 +++++---- .../authc/esusers/tool/ESUsersToolTests.java | 34 +++++++++++++++++-- 2 files changed, 41 insertions(+), 9 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 ef1b4070d24..042f4f95b81 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 @@ -295,7 +295,6 @@ public class ESUsersTool extends CliTool { return new ListUsersAndRoles(terminal, username).execute(settings, env); } - // check for roles if they match String[] allRoles = ObjectArrays.concat(addRoles, removeRoles, String.class); for (String role : allRoles) { @@ -305,15 +304,20 @@ public class ESUsersTool extends CliTool { } } - Path file = FileUserRolesStore.resolveFile(settings, env); - Map userRoles = FileUserRolesStore.parseFile(file, null); - - if (!userRoles.containsKey(username)) { + Path path = FileUserPasswdStore.resolveFile(settings, env); + Map usersMap = FileUserPasswdStore.parseFile(path, null); + if (!usersMap.containsKey(username)) { terminal.println("User [%s] doesn't exist", username); return ExitStatus.NO_USER; } - List roles = Lists.newArrayList(userRoles.get(username));; + Path file = FileUserRolesStore.resolveFile(settings, env); + Map userRoles = FileUserRolesStore.parseFile(file, null); + + List roles = Lists.newArrayList(); + if (userRoles.get(username) != null) { + roles.addAll(Arrays.asList(userRoles.get(username))); + } roles.addAll(Arrays.asList(addRoles)); roles.removeAll(Arrays.asList(removeRoles)); 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 7735040a0d9..d35cc981a6c 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 @@ -358,8 +358,10 @@ public class ESUsersToolTests extends CliToolTestCase { @Test public void testRoles_Cmd_addingRoleWorks() throws Exception { - File usersRoleFile = writeFile("admin: admin\nuser:user\n"); + File usersFile = writeFile("admin:hash\nuser:hash"); + File usersRoleFile = writeFile("admin: admin\nuser: user\n"); Settings settings = ImmutableSettings.builder() + .put("shield.authc.esusers.files.users", usersFile) .put("shield.authc.esusers.files.users_roles", usersRoleFile) .build(); @@ -377,8 +379,10 @@ public class ESUsersToolTests extends CliToolTestCase { @Test public void testRoles_Cmd_removingRoleWorks() throws Exception { - File usersRoleFile = writeFile("admin: admin\nuser:user,foo,bar\n"); + File usersFile = writeFile("admin:hash\nuser:hash"); + File usersRoleFile = writeFile("admin: admin\nuser: user,foo,bar\n"); Settings settings = ImmutableSettings.builder() + .put("shield.authc.esusers.files.users", usersFile) .put("shield.authc.esusers.files.users_roles", usersRoleFile) .build(); @@ -396,8 +400,10 @@ public class ESUsersToolTests extends CliToolTestCase { @Test public void testRoles_Cmd_addingAndRemovingRoleWorks() throws Exception { + File usersFile = writeFile("admin:hash\nuser:hash"); File usersRoleFile = writeFile("admin: admin\nuser:user,foo,bar\n"); Settings settings = ImmutableSettings.builder() + .put("shield.authc.esusers.files.users", usersFile) .put("shield.authc.esusers.files.users_roles", usersRoleFile) .build(); @@ -415,8 +421,10 @@ public class ESUsersToolTests extends CliToolTestCase { @Test public void testRoles_Cmd_userNotFound() throws Exception { - File usersRoleFile = writeFile("admin: admin\nuser:user,foo,bar\n"); + File usersFile = writeFile("admin:hash\nuser:hash"); + File usersRoleFile = writeFile("admin: admin\nuser: user,foo,bar\n"); Settings settings = ImmutableSettings.builder() + .put("shield.authc.esusers.files.users", usersFile) .put("shield.authc.esusers.files.users_roles", usersRoleFile) .build(); @@ -443,6 +451,26 @@ public class ESUsersToolTests extends CliToolTestCase { assertThat(catchTerminalOutput.getTerminalOutput(), hasItem(allOf(containsString("user"), containsString("user,foo,bar")))); } + @Test + public void testRoles_cmd_testRoleCanBeAddedWhenUserIsNotInRolesFile() throws Exception { + File usersFile = writeFile("admin:hash\nuser:hash"); + File usersRoleFile = writeFile("admin: admin\n"); + Settings settings = ImmutableSettings.builder() + .put("shield.authc.esusers.files.users", usersFile) + .put("shield.authc.esusers.files.users_roles", usersRoleFile) + .build(); + + CaptureOutputTerminal catchTerminalOutput = new CaptureOutputTerminal(); + ESUsersTool.Roles cmd = new ESUsersTool.Roles(catchTerminalOutput, "user", new String[]{"myrole"}, Strings.EMPTY_ARRAY); + CliTool.ExitStatus status = execute(cmd, settings); + + assertThat(status, is(CliTool.ExitStatus.OK)); + Map userRoles = FileUserRolesStore.parseFile(usersRoleFile.toPath(), logger); + assertThat(userRoles.keySet(), hasSize(2)); + assertThat(userRoles.keySet(), hasItems("admin", "user")); + assertThat(userRoles.get("user"), arrayContaining("myrole")); + } + @Test public void testListUsersAndRoles_Cmd_parsingWorks() throws Exception { ESUsersTool tool = new ESUsersTool();