FilesUserRolesStore to return an empty array when there's no roles for a user
This prevents us from spreading the null invariant all over the place ending up with causing NPEs. Closes elastic/elasticsearch#147 Original commit: elastic/x-pack-elasticsearch@3d5adf94ec
This commit is contained in:
parent
6173496a52
commit
f69c1c616a
|
@ -62,7 +62,11 @@ public class FileUserRolesStore extends AbstractComponent implements UserRolesSt
|
||||||
}
|
}
|
||||||
|
|
||||||
public String[] roles(String username) {
|
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) {
|
public static Path resolveFile(Settings settings, Environment env) {
|
||||||
|
@ -86,7 +90,7 @@ public class FileUserRolesStore extends AbstractComponent implements UserRolesSt
|
||||||
return ImmutableMap.of();
|
return ImmutableMap.of();
|
||||||
}
|
}
|
||||||
|
|
||||||
List<String> lines = null;
|
List<String> lines;
|
||||||
try {
|
try {
|
||||||
lines = Files.readAllLines(path, Charsets.UTF_8);
|
lines = Files.readAllLines(path, Charsets.UTF_8);
|
||||||
} catch (IOException ioe) {
|
} catch (IOException ioe) {
|
||||||
|
|
|
@ -6,6 +6,7 @@
|
||||||
package org.elasticsearch.shield.authc.esusers;
|
package org.elasticsearch.shield.authc.esusers;
|
||||||
|
|
||||||
import com.carrotsearch.ant.tasks.junit4.dependencies.com.google.common.base.Charsets;
|
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.ImmutableSettings;
|
||||||
import org.elasticsearch.common.settings.Settings;
|
import org.elasticsearch.common.settings.Settings;
|
||||||
import org.elasticsearch.env.Environment;
|
import org.elasticsearch.env.Environment;
|
||||||
|
@ -83,7 +84,7 @@ public class FileUserRolesStoreTests extends ElasticsearchTestCase {
|
||||||
assertThat(roles, notNullValue());
|
assertThat(roles, notNullValue());
|
||||||
assertThat(roles.length, is(3));
|
assertThat(roles.length, is(3));
|
||||||
assertThat(roles, arrayContaining("role1", "role2", "role3"));
|
assertThat(roles, arrayContaining("role1", "role2", "role3"));
|
||||||
assertThat(store.roles("user4"), nullValue());
|
assertThat(store.roles("user4"), equalTo(Strings.EMPTY_ARRAY));
|
||||||
|
|
||||||
watcherService.start();
|
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
|
@Test
|
||||||
public void testThatEmptyFileIsParsed() throws Exception {
|
public void testThatEmptyFileIsParsed() throws Exception {
|
||||||
assertInvalidInputIsSilentlyIgnored("");
|
assertInvalidInputIsSilentlyIgnored("");
|
||||||
|
@ -130,6 +152,12 @@ public class FileUserRolesStoreTests extends ElasticsearchTestCase {
|
||||||
assertInvalidInputIsSilentlyIgnored("user: , ");
|
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 {
|
private void assertInvalidInputIsSilentlyIgnored(String input) throws Exception {
|
||||||
File file = tempFolder.newFile();
|
File file = tempFolder.newFile();
|
||||||
com.google.common.io.Files.write(input.getBytes(Charsets.UTF_8), file);
|
com.google.common.io.Files.write(input.getBytes(Charsets.UTF_8), file);
|
||||||
|
|
Loading…
Reference in New Issue