From 78377c7cd2552ba62ff7aa5833e2a48f6c23c8ea Mon Sep 17 00:00:00 2001 From: uboness Date: Mon, 13 Oct 2014 23:06:59 +0200 Subject: [PATCH] Change users_roles format to be keyed by roles Having roles as the keys is more aligned with the LDAP role_mapping file and with linux's group file (where the groups serve as the keys) Also added support for comment lines (starting with `#`) in `.users` and `.users_roles` files Original commit: elastic/x-pack-elasticsearch@60faf7330f3ee3ad1c640c34fc23fdfaf1a0b458 --- .../authc/esusers/FileUserPasswdStore.java | 3 + .../authc/esusers/FileUserRolesStore.java | 69 +++++++++++++------ .../esusers/FileUserRolesStoreTests.java | 12 ++-- .../authc/esusers/tool/ESUsersToolTests.java | 49 +++++++------ .../shield/test/ShieldIntegrationTest.java | 2 +- .../elasticsearch/shield/authc/esusers/users | 2 + .../shield/authc/esusers/users_roles | 9 ++- 7 files changed, 91 insertions(+), 55 deletions(-) diff --git a/src/main/java/org/elasticsearch/shield/authc/esusers/FileUserPasswdStore.java b/src/main/java/org/elasticsearch/shield/authc/esusers/FileUserPasswdStore.java index 8bd49ae52d6..3c05c9e3a25 100644 --- a/src/main/java/org/elasticsearch/shield/authc/esusers/FileUserPasswdStore.java +++ b/src/main/java/org/elasticsearch/shield/authc/esusers/FileUserPasswdStore.java @@ -105,6 +105,9 @@ public class FileUserPasswdStore extends AbstractComponent implements UserPasswd int lineNr = 0; for (String line : lines) { lineNr++; + if (line.startsWith("#")) { // comment + continue; + } int i = line.indexOf(":"); if (i <= 0 || i == line.length() - 1) { if (logger != null) { diff --git a/src/main/java/org/elasticsearch/shield/authc/esusers/FileUserRolesStore.java b/src/main/java/org/elasticsearch/shield/authc/esusers/FileUserRolesStore.java index 9e1f5f1e5e1..2329b350b8a 100644 --- a/src/main/java/org/elasticsearch/shield/authc/esusers/FileUserRolesStore.java +++ b/src/main/java/org/elasticsearch/shield/authc/esusers/FileUserRolesStore.java @@ -28,9 +28,7 @@ import java.nio.file.Files; import java.nio.file.Path; import java.nio.file.Paths; import java.nio.file.StandardOpenOption; -import java.util.List; -import java.util.Locale; -import java.util.Map; +import java.util.*; import java.util.regex.Pattern; /** @@ -38,7 +36,7 @@ import java.util.regex.Pattern; */ public class FileUserRolesStore extends AbstractComponent implements UserRolesStore { - private static final Pattern ROLES_DELIM = Pattern.compile("\\s*,\\s*"); + private static final Pattern USERS_DELIM = Pattern.compile("\\s*,\\s*"); private final Path file; @@ -79,7 +77,8 @@ public class FileUserRolesStore extends AbstractComponent implements UserRolesSt /** * parses the users_roles file. Should never return return {@code null}, if the file doesn't exist - * an empty map is returned + * an empty map is returned. The read file holds a mapping per line of the form "role -> users" while the returned + * map holds entries of the form "user -> roles". */ public static ImmutableMap parseFile(Path path, @Nullable ESLogger logger) { if (logger != null) { @@ -97,49 +96,79 @@ public class FileUserRolesStore extends AbstractComponent implements UserRolesSt throw new ElasticsearchException("Could not read users file [" + path.toAbsolutePath() + "]", ioe); } - ImmutableMap.Builder usersRoles = ImmutableMap.builder(); + Map> userToRoles = new HashMap<>(); int lineNr = 0; for (String line : lines) { lineNr++; + if (line.startsWith("#")) { //comment + continue; + } int i = line.indexOf(":"); if (i <= 0 || i == line.length() - 1) { if (logger != null) { - logger.error("Invalid entry in users file [" + path.toAbsolutePath() + "], line [" + lineNr + "]. Skipping..."); + logger.error("Invalid entry in users_roles file [" + path.toAbsolutePath() + "], line [" + lineNr + "]. Skipping..."); } continue; } - String username = line.substring(0, i).trim(); - if (Strings.isEmpty(username)) { + String role = line.substring(0, i).trim(); + if (Strings.isEmpty(role)) { if (logger != null) { - logger.error("Invalid username entry in users file [" + path.toAbsolutePath() + "], line [" + lineNr + "]. Skipping..."); + logger.error("Invalid username entry in users_roles file [" + path.toAbsolutePath() + "], line [" + lineNr + "]. Skipping..."); } continue; } - String rolesStr = line.substring(i + 1).trim(); - if (Strings.isEmpty(rolesStr)) { + String usersStr = line.substring(i + 1).trim(); + if (Strings.isEmpty(usersStr)) { if (logger != null) { - logger.error("Invalid roles entry in users file [" + path.toAbsolutePath() + "], line [" + lineNr + "]. Skipping..."); + logger.error("Invalid roles entry in users_roles file [" + path.toAbsolutePath() + "], line [" + lineNr + "]. Skipping..."); } continue; } - String[] roles = ROLES_DELIM.split(rolesStr); - if (roles.length == 0) { + String[] roleUsers = USERS_DELIM.split(usersStr); + if (roleUsers.length == 0) { if (logger != null) { - logger.error("Invalid roles entry in users file [" + path.toAbsolutePath() + "], line [" + lineNr + "]. Skipping..."); + logger.error("Invalid roles entry in users_roles file [" + path.toAbsolutePath() + "], line [" + lineNr + "]. Skipping..."); } continue; } - usersRoles.put(username, roles); + + for (String user : roleUsers) { + List roles = userToRoles.get(user); + if (roles == null) { + roles = new ArrayList<>(); + userToRoles.put(user, roles); + } + roles.add(role); + } + } + + ImmutableMap.Builder usersRoles = ImmutableMap.builder(); + for (Map.Entry> entry : userToRoles.entrySet()) { + usersRoles.put(entry.getKey(), entry.getValue().toArray(new String[entry.getValue().size()])); } return usersRoles.build(); } - public static void writeFile(Map userRoles, Path path) { + /** + * Accepts a mapping of user -> list of roles + */ + public static void writeFile(Map userToRoles, Path path) { + HashMap> roleToUsers = new HashMap<>(); + for (Map.Entry entry : userToRoles.entrySet()) { + for (String role : entry.getValue()) { + List users = roleToUsers.get(role); + if (users == null) { + users = new ArrayList<>(); + roleToUsers.put(role, users); + } + users.add(entry.getKey()); + } + } try (PrintWriter writer = new PrintWriter(Files.newBufferedWriter(path, Charsets.UTF_8, StandardOpenOption.TRUNCATE_EXISTING, StandardOpenOption.CREATE, StandardOpenOption.WRITE))) { - for (Map.Entry entry : userRoles.entrySet()) { - writer.printf(Locale.ROOT, "%s:%s%s", entry.getKey(), Strings.arrayToCommaDelimitedString(entry.getValue()), System.lineSeparator()); + for (Map.Entry> entry : roleToUsers.entrySet()) { + writer.printf(Locale.ROOT, "%s:%s%s", entry.getKey(), Strings.collectionToCommaDelimitedString(entry.getValue()), System.lineSeparator()); } } catch (IOException ioe) { throw new ElasticsearchException("Could not write users file [" + path.toAbsolutePath() + "], please check file permissions"); diff --git a/src/test/java/org/elasticsearch/shield/authc/esusers/FileUserRolesStoreTests.java b/src/test/java/org/elasticsearch/shield/authc/esusers/FileUserRolesStoreTests.java index 171ce3f4462..7e429820979 100644 --- a/src/test/java/org/elasticsearch/shield/authc/esusers/FileUserRolesStoreTests.java +++ b/src/test/java/org/elasticsearch/shield/authc/esusers/FileUserRolesStoreTests.java @@ -90,7 +90,7 @@ public class FileUserRolesStoreTests extends ElasticsearchTestCase { try (BufferedWriter writer = Files.newBufferedWriter(tmp, Charsets.UTF_8, StandardOpenOption.APPEND)) { writer.newLine(); - writer.append("user4:role4,role5"); + writer.append("role4:user4\nrole5:user4\n"); } if (!latch.await(5, TimeUnit.SECONDS)) { @@ -141,15 +141,15 @@ public class FileUserRolesStoreTests extends ElasticsearchTestCase { @Test public void testThatEmptyUserNameDoesNotThrowException() throws Exception { - assertInvalidInputIsSilentlyIgnored(":role1,role2"); - assertInvalidInputIsSilentlyIgnored(" :role1,role2"); + assertInvalidInputIsSilentlyIgnored(":user1,user2"); + assertInvalidInputIsSilentlyIgnored(" :user1,user2"); } @Test public void testThatEmptyRoleDoesNotThrowException() throws Exception { - assertInvalidInputIsSilentlyIgnored("user:"); - assertInvalidInputIsSilentlyIgnored("user: "); - assertInvalidInputIsSilentlyIgnored("user: , "); + assertInvalidInputIsSilentlyIgnored("role:"); + assertInvalidInputIsSilentlyIgnored("role: "); + assertInvalidInputIsSilentlyIgnored("role: , "); } private File writeUsersRoles(String input) throws Exception { 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 f4bb5fe5544..c6e02e8711d 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 @@ -102,15 +102,14 @@ public class ESUsersToolTests extends CliToolTestCase { assertFileExists(userRolesFile); lines = Files.readLines(userRolesFile, Charsets.UTF_8); - assertThat(lines.size(), is(1)); - line = lines.get(0); - assertThat(line, equalTo("user1:r1,r2")); + assertThat(lines, hasSize(2)); + assertThat(lines, containsInAnyOrder("r1:user1", "r2:user1")); } @Test public void testUseradd_Cmd_Append() throws Exception { File userFile = writeFile("user2:hash2"); - File userRolesFile = writeFile("user2:r3,r4"); + File userRolesFile = writeFile("r3:user2\nr4:user2"); Settings settings = ImmutableSettings.builder() .put("shield.authc.esusers.files.users", userFile) .put("shield.authc.esusers.files.users_roles", userRolesFile) @@ -138,14 +137,14 @@ public class ESUsersToolTests extends CliToolTestCase { assertFileExists(userRolesFile); lines = Files.readLines(userRolesFile, Charsets.UTF_8); - assertThat(lines, hasSize(2)); - assertThat(lines, containsInAnyOrder("user2:r3,r4", "user1:r1,r2")); + assertThat(lines, hasSize(4)); + assertThat(lines, containsInAnyOrder("r1:user1", "r2:user1", "r3:user2", "r4:user2")); } @Test public void testUseradd_Cmd_AddingUserWithoutRolesDoesNotAddEmptyRole() throws Exception { File userFile = writeFile("user2:hash2"); - File userRolesFile = writeFile("user2:r3,r4"); + File userRolesFile = writeFile("r3:user2\nr4:user2"); Settings settings = ImmutableSettings.builder() .put("shield.authc.esusers.files.users", userFile) .put("shield.authc.esusers.files.users_roles", userRolesFile) @@ -158,8 +157,8 @@ public class ESUsersToolTests extends CliToolTestCase { assertFileExists(userRolesFile); List lines = Files.readLines(userRolesFile, Charsets.UTF_8); - assertThat(lines, hasSize(1)); - assertThat(lines, not(hasItem(startsWith("user1")))); + assertThat(lines, hasSize(2)); + assertThat(lines, not(hasItem(containsString("user1")))); } @Test @@ -198,7 +197,7 @@ public class ESUsersToolTests extends CliToolTestCase { @Test public void testUserdel_Cmd() throws Exception { File userFile = writeFile("user1:hash2"); - File userRolesFile = writeFile("user1:r3,r4"); + File userRolesFile = writeFile("r3:user1\nr4:user1"); Settings settings = ImmutableSettings.builder() .put("shield.authc.esusers.files.users", userFile) .put("shield.authc.esusers.files.users_roles", userRolesFile) @@ -221,7 +220,7 @@ public class ESUsersToolTests extends CliToolTestCase { @Test public void testUserdel_Cmd_MissingUser() throws Exception { File userFile = writeFile("user1:hash2"); - File userRolesFile = writeFile("user1:r3,r4"); + File userRolesFile = writeFile("r3:user1\nr4:user1"); Settings settings = ImmutableSettings.builder() .put("shield.authc.esusers.files.users", userFile) .put("shield.authc.esusers.files.users_roles", userRolesFile) @@ -238,7 +237,7 @@ public class ESUsersToolTests extends CliToolTestCase { assertFileExists(userRolesFile); lines = Files.readLines(userRolesFile, Charsets.UTF_8); - assertThat(lines.size(), is(1)); + assertThat(lines, hasSize(2)); } @Test @@ -397,13 +396,13 @@ public class ESUsersToolTests extends CliToolTestCase { assertThat(userRoles.keySet(), hasSize(2)); assertThat(userRoles.keySet(), hasItems("admin", "user")); assertThat(userRoles.get("admin"), arrayContaining("admin")); - assertThat(userRoles.get("user"), arrayContaining("user", "foo")); + assertThat(userRoles.get("user"), arrayContainingInAnyOrder("user", "foo")); } @Test public void testRoles_Cmd_removingRoleWorks() throws Exception { File usersFile = writeFile("admin:hash\nuser:hash"); - File usersRoleFile = writeFile("admin: admin\nuser: user,foo,bar\n"); + File usersRoleFile = writeFile("admin: admin\nuser: user\nfoo: user\nbar: user\n"); Settings settings = ImmutableSettings.builder() .put("shield.authc.esusers.files.users", usersFile) .put("shield.authc.esusers.files.users_roles", usersRoleFile) @@ -418,13 +417,13 @@ public class ESUsersToolTests extends CliToolTestCase { assertThat(userRoles.keySet(), hasSize(2)); assertThat(userRoles.keySet(), hasItems("admin", "user")); assertThat(userRoles.get("admin"), arrayContaining("admin")); - assertThat(userRoles.get("user"), arrayContaining("user", "bar")); + assertThat(userRoles.get("user"), arrayContainingInAnyOrder("user", "bar")); } @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"); + File usersRoleFile = writeFile("admin: admin\nuser:user\nfoo:user\nbar:user\n"); Settings settings = ImmutableSettings.builder() .put("shield.authc.esusers.files.users", usersFile) .put("shield.authc.esusers.files.users_roles", usersRoleFile) @@ -439,13 +438,13 @@ public class ESUsersToolTests extends CliToolTestCase { assertThat(userRoles.keySet(), hasSize(2)); assertThat(userRoles.keySet(), hasItems("admin", "user")); assertThat(userRoles.get("admin"), arrayContaining("admin")); - assertThat(userRoles.get("user"), arrayContaining("user", "bar", "newrole")); + assertThat(userRoles.get("user"), arrayContainingInAnyOrder("user", "bar", "newrole")); } @Test public void testRoles_Cmd_removingLastRoleRemovesEntryFromRolesFile() throws Exception { File usersFile = writeFile("admin:hash\nuser:hash"); - File usersRoleFile = writeFile("admin: admin\nuser:user,foo,bar\n"); + File usersRoleFile = writeFile("admin: admin\nuser:user\nfoo:user\nbar:user\n"); Settings settings = ImmutableSettings.builder() .put("shield.authc.esusers.files.users", usersFile) .put("shield.authc.esusers.files.users_roles", usersRoleFile) @@ -457,13 +456,13 @@ public class ESUsersToolTests extends CliToolTestCase { assertThat(status, is(CliTool.ExitStatus.OK)); List usersRoleFileLines = Files.readLines(usersRoleFile, Charsets.UTF_8); - assertThat(usersRoleFileLines, not(hasItem(startsWith("user:")))); + assertThat(usersRoleFileLines, not(hasItem(containsString("user")))); } @Test public void testRoles_Cmd_userNotFound() throws Exception { File usersFile = writeFile("admin:hash\nuser:hash"); - File usersRoleFile = writeFile("admin: admin\nuser: user,foo,bar\n"); + File usersRoleFile = writeFile("admin: admin\nuser: user\nfoo:user\nbar:user\n"); Settings settings = ImmutableSettings.builder() .put("shield.authc.esusers.files.users", usersFile) .put("shield.authc.esusers.files.users_roles", usersRoleFile) @@ -478,7 +477,7 @@ public class ESUsersToolTests extends CliToolTestCase { @Test public void testRoles_Cmd_testNotAddingOrRemovingRolesShowsListingOfRoles() throws Exception { File usersFile = writeFile("admin:hash\nuser:hash"); - File usersRoleFile = writeFile("admin: admin\nuser:user,foo,bar\n"); + File usersRoleFile = writeFile("admin: admin\nuser:user\nfoo:user\nbar:user\n"); Settings settings = ImmutableSettings.builder() .put("shield.authc.esusers.files.users", usersFile) .put("shield.authc.esusers.files.users_roles", usersRoleFile) @@ -523,7 +522,7 @@ public class ESUsersToolTests extends CliToolTestCase { @Test public void testListUsersAndRoles_Cmd_listAllUsers() throws Exception { - File usersRoleFile = writeFile("admin: admin\nuser: user,foo,bar\n"); + File usersRoleFile = writeFile("admin: admin\nuser: user\nfoo:user\nbar:user\n"); Settings settings = ImmutableSettings.builder() .put("shield.authc.esusers.files.users_roles", usersRoleFile) .build(); @@ -540,7 +539,7 @@ public class ESUsersToolTests extends CliToolTestCase { @Test public void testListUsersAndRoles_Cmd_listSingleUser() throws Exception { - File usersRoleFile = writeFile("admin: admin\nuser: user,foo,bar\n"); + File usersRoleFile = writeFile("admin: admin\nuser: user\nfoo:user\nbar:user\n"); File usersFile = writeFile("admin:{plain}changeme\nuser:{plain}changeme\nno-roles-user:{plain}changeme\n"); Settings settings = ImmutableSettings.builder() .put("shield.authc.esusers.files.users_roles", usersRoleFile) @@ -559,7 +558,7 @@ public class ESUsersToolTests extends CliToolTestCase { @Test public void testListUsersAndRoles_Cmd_listSingleUserNotFound() throws Exception { - File usersRoleFile = writeFile("admin: admin\nuser: user,foo,bar\n"); + File usersRoleFile = writeFile("admin: admin\nuser: user\nfoo:user\nbar:user\n"); Settings settings = ImmutableSettings.builder() .put("shield.authc.esusers.files.users_roles", usersRoleFile) .build(); @@ -574,7 +573,7 @@ public class ESUsersToolTests extends CliToolTestCase { @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("admin: admin\nuser: user,foo,bar\n"); + File usersRoleFile = writeFile("admin: admin\nuser: user\nfoo:user\nbar:user\n"); Settings settings = ImmutableSettings.builder() .put("shield.authc.esusers.files.users_roles", usersRoleFile) .put("shield.authc.esusers.files.users", usersFile) diff --git a/src/test/java/org/elasticsearch/shield/test/ShieldIntegrationTest.java b/src/test/java/org/elasticsearch/shield/test/ShieldIntegrationTest.java index cfa15ae0fba..0c6ade17c9c 100644 --- a/src/test/java/org/elasticsearch/shield/test/ShieldIntegrationTest.java +++ b/src/test/java/org/elasticsearch/shield/test/ShieldIntegrationTest.java @@ -49,7 +49,7 @@ public abstract class ShieldIntegrationTest extends ElasticsearchIntegrationTest public static final String CONFIG_IPFILTER_ALLOW_ALL = "allow: all\n"; public static final String CONFIG_STANDARD_USER = DEFAULT_USER_NAME + ":{plain}" + DEFAULT_PASSWORD + "\n"; - public static final String CONFIG_STANDARD_USER_ROLES = DEFAULT_USER_NAME + ":" + DEFAULT_ROLE + "\n"; + public static final String CONFIG_STANDARD_USER_ROLES = DEFAULT_ROLE + ":" + DEFAULT_USER_NAME+ "\n"; public static final String CONFIG_ROLE_ALLOW_ALL = DEFAULT_ROLE + ":\n" + " cluster: ALL\n" + " indices:\n" + diff --git a/src/test/resources/org/elasticsearch/shield/authc/esusers/users b/src/test/resources/org/elasticsearch/shield/authc/esusers/users index 1387b6d40ef..51469265909 100644 --- a/src/test/resources/org/elasticsearch/shield/authc/esusers/users +++ b/src/test/resources/org/elasticsearch/shield/authc/esusers/users @@ -3,4 +3,6 @@ md5: $apr1$R3DdqiAZ$aljIkaIVPSarmDMlJUBBP. crypt: hsP1PYSLsEEvs plain: {plain}test123 sha:{SHA}cojt0Pw//L6ToM8G41aOKFIWh7w= +# this is a comment line +# another comment line bcrypt10: $2y$10$FMhmFjwU5.qxQ/BsEciS9OqcJVkFMgXMo4uH5CelOR1j4N9zIv67e \ No newline at end of file diff --git a/src/test/resources/org/elasticsearch/shield/authc/esusers/users_roles b/src/test/resources/org/elasticsearch/shield/authc/esusers/users_roles index 5b86eb573ed..f8acc608145 100644 --- a/src/test/resources/org/elasticsearch/shield/authc/esusers/users_roles +++ b/src/test/resources/org/elasticsearch/shield/authc/esusers/users_roles @@ -1,3 +1,6 @@ -user1:role1,role2,role3 -user2 : role2 , role3 -user3:role3 \ No newline at end of file +role1:user1 +role2: user1,user2 +# this is a comment line +role3: user1, user2 , user3 +# another comment line +# and another one \ No newline at end of file