Avoid NPE if users_roles file does not exist (elastic#109606)

In `elasticsearch-users` gracefully handle the case where the `users` file exists, but the `users_roles` file does not.
arteam · Jun 13, 2024 · b4fdfbb · b4fdfbb
1 parent 60a34f2
commit b4fdfbb
Show file tree

Hide file tree

Showing 3 changed files with 38 additions and 1 deletion.
diff --git a/docs/changelog/109606.yaml b/docs/changelog/109606.yaml
@@ -0,0 +1,5 @@
+pr: 109606
+summary: Avoid NPE if `users_roles` file does not exist
+area: Authentication
+type: bug
+issues: []
diff --git a/...in/security/src/main/java/org/elasticsearch/xpack/security/authc/file/tool/UsersTool.java b/...in/security/src/main/java/org/elasticsearch/xpack/security/authc/file/tool/UsersTool.java
@@ -134,7 +134,13 @@ public void execute(Terminal terminal, OptionSet options, Environment env, Proce
             FileUserPasswdStore.writeFile(users, passwordFile);
 
             if (roles.length > 0) {
-                Map<String, String[]> userRoles = new HashMap<>(FileUserRolesStore.parseFile(rolesFile, null));
+                final Map<String, String[]> userRoles;
+                if (Files.exists(rolesFile)) {
+                    userRoles = new HashMap<>(FileUserRolesStore.parseFile(rolesFile, null));
+                } else {
+                    terminal.println("Roles file [" + rolesFile + "] does not exist, will attempt to create it");
+                    userRoles = new HashMap<>();
+                }
                 userRoles.put(username, roles);
                 FileUserRolesStore.writeFile(userRoles, rolesFile);
             }

diff --git a/...-tests/src/test/java/org/elasticsearch/xpack/security/authc/file/tool/UsersToolTests.java b/...-tests/src/test/java/org/elasticsearch/xpack/security/authc/file/tool/UsersToolTests.java
@@ -23,6 +23,7 @@
 import org.elasticsearch.core.PathUtilsForTesting;
 import org.elasticsearch.env.Environment;
 import org.elasticsearch.env.TestEnvironment;
+import org.elasticsearch.test.FileMatchers;
 import org.elasticsearch.test.SecuritySettingsSourceField;
 import org.elasticsearch.xpack.core.XPackSettings;
 import org.elasticsearch.xpack.core.security.authc.support.Hasher;
@@ -45,6 +46,7 @@
 
 import static org.elasticsearch.test.SecurityIntegTestCase.getFastStoredHashAlgoForTests;
 import static org.hamcrest.Matchers.containsString;
+import static org.hamcrest.Matchers.hasSize;
 
 public class UsersToolTests extends CommandTestCase {
 
@@ -368,6 +370,30 @@ public void testUseraddNoRoles() throws Exception {
         assertTrue(lines.toString(), lines.isEmpty());
     }
 
+    public void testUseraddRolesFileDoesNotExist() throws Exception {
+        final Path rolesFilePath = confDir.resolve("users_roles");
+        Files.delete(rolesFilePath);
+        var output = execute(
+            "useradd",
+            pathHomeParameter,
+            fileOrderParameter,
+            "trevor.slattery",
+            "-p",
+            SecuritySettingsSourceField.TEST_PASSWORD,
+            "-r",
+            "mandarin"
+        );
+        assertThat(output, containsString("does not exist"));
+        assertThat(output, containsString(rolesFilePath + "]"));
+        assertThat(output, containsString("attempt to create"));
+        assertThat(rolesFilePath, FileMatchers.pathExists());
+
+        List<String> lines = Files.readAllLines(rolesFilePath, StandardCharsets.UTF_8);
+        assertThat(lines, hasSize(1));
+        assertThat(lines.get(0), containsString("trevor.slattery"));
+        assertThat(lines.get(0), containsString("mandarin"));
+    }
+
     public void testAddUserWithInvalidHashingAlgorithmInFips() throws Exception {
         settings = Settings.builder()
             .put(settings)