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 41fb59e52e7..9e1f5f1e5e1 100644 --- a/src/main/java/org/elasticsearch/shield/authc/esusers/FileUserRolesStore.java +++ b/src/main/java/org/elasticsearch/shield/authc/esusers/FileUserRolesStore.java @@ -62,7 +62,11 @@ public class FileUserRolesStore extends AbstractComponent implements UserRolesSt } public String[] roles(String username) { - return userRoles != null ? userRoles.get(username) : null; + if (userRoles == null) { + return Strings.EMPTY_ARRAY; + } + String[] roles = userRoles.get(username); + return roles == null ? Strings.EMPTY_ARRAY : userRoles.get(username); } public static Path resolveFile(Settings settings, Environment env) { @@ -86,7 +90,7 @@ public class FileUserRolesStore extends AbstractComponent implements UserRolesSt return ImmutableMap.of(); } - List lines = null; + List lines; try { lines = Files.readAllLines(path, Charsets.UTF_8); } catch (IOException ioe) { 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 ffcf6849bb7..171ce3f4462 100644 --- a/src/test/java/org/elasticsearch/shield/authc/esusers/FileUserRolesStoreTests.java +++ b/src/test/java/org/elasticsearch/shield/authc/esusers/FileUserRolesStoreTests.java @@ -6,6 +6,7 @@ package org.elasticsearch.shield.authc.esusers; import com.carrotsearch.ant.tasks.junit4.dependencies.com.google.common.base.Charsets; +import org.elasticsearch.common.Strings; import org.elasticsearch.common.settings.ImmutableSettings; import org.elasticsearch.common.settings.Settings; import org.elasticsearch.env.Environment; @@ -83,7 +84,7 @@ public class FileUserRolesStoreTests extends ElasticsearchTestCase { assertThat(roles, notNullValue()); assertThat(roles.length, is(3)); assertThat(roles, arrayContaining("role1", "role2", "role3")); - assertThat(store.roles("user4"), nullValue()); + assertThat(store.roles("user4"), equalTo(Strings.EMPTY_ARRAY)); watcherService.start(); @@ -111,6 +112,27 @@ public class FileUserRolesStoreTests extends ElasticsearchTestCase { } } + @Test + public void testThatEmptyRolesDoesNotCauseNPE() throws Exception { + ThreadPool threadPool = null; + try { + threadPool = new ThreadPool("test"); + File usersRoles = writeUsersRoles("admin:role1"); + Settings settings = ImmutableSettings.builder() + .put("watcher.enabled", "false") + .put("shield.authc.esusers.files.users_roles", usersRoles.toPath().toAbsolutePath()) + .build(); + Environment env = new Environment(settings); + ResourceWatcherService watcherService = new ResourceWatcherService(settings, threadPool); + FileUserRolesStore store = new FileUserRolesStore(settings, env, watcherService); + assertThat(store.roles("user"), equalTo(Strings.EMPTY_ARRAY)); + } finally { + if (threadPool != null) { + threadPool.shutdownNow(); + } + } + } + @Test public void testThatEmptyFileIsParsed() throws Exception { assertInvalidInputIsSilentlyIgnored(""); @@ -130,6 +152,12 @@ public class FileUserRolesStoreTests extends ElasticsearchTestCase { assertInvalidInputIsSilentlyIgnored("user: , "); } + private File writeUsersRoles(String input) throws Exception { + File file = tempFolder.newFile(); + com.google.common.io.Files.write(input.getBytes(Charsets.UTF_8), file); + return file; + } + private void assertInvalidInputIsSilentlyIgnored(String input) throws Exception { File file = tempFolder.newFile(); com.google.common.io.Files.write(input.getBytes(Charsets.UTF_8), file);