User APIs allow use of internal usernames (#86398)

This PR removes API restrictions that prevent user-related actions
(PutUser, DeleteUser, etc.) to be performed on users with names
clashing with internal users (e.g., _system). Internal users are used
for issuing requests within the cluster and not accessible at the REST
layer; they are conceptually and practically separate from users
created through the API (they do not belong to the same realm and
follow different authentication flows). As such it's safe to remove the
current restriction. By removing, we are more consistent with how
usernames are treated across other realms (name re-use is allowed), and
ease evolving internal users without interfering with end-user
experience.

Closes #86326

docs/changelog/86398.yaml

@@ -0,0 +1,6 @@
+pr: 86398
+summary: Relax username restrictions for User APIs
+area: Security
+type: enhancement
+issues:
+ - 86326

test/framework/src/main/java/org/elasticsearch/test/ESTestCase.java

@@ -1205,13 +1205,20 @@ public static <T> List<T> randomSubsetOf(Collection<T> collection) {
         return randomSubsetOf(randomInt(collection.size()), collection);
     }
 
+    public static <T> List<T> randomNonEmptySubsetOf(Collection<T> collection) {
+        if (collection.isEmpty()) {
+            throw new IllegalArgumentException("Can't pick non-empty subset of an empty collection");
+        }
+        return randomSubsetOf(randomIntBetween(1, collection.size()), collection);
+    }
+
     /**
      * Returns size random values
      */
     public static <T> List<T> randomSubsetOf(int size, Collection<T> collection) {
         if (size > collection.size()) {
             throw new IllegalArgumentException(
-                "Can\'t pick " + size + " random objects from a collection of " + collection.size() + " objects"
+                "Can't pick " + size + " random objects from a collection of " + collection.size() + " objects"
             );
         }
         List<T> tempList = new ArrayList<>(collection);

test/framework/src/test/java/org/elasticsearch/test/test/ESTestCaseTests.java

@@ -160,6 +160,11 @@ public void testRandomUniqueNormalUsageAlwayMoreThanOne() {
         assertThat(randomUnique(() -> randomAlphaOfLengthBetween(1, 20), 10), hasSize(greaterThan(0)));
     }
 
+    public void testRandomNonEmptySubsetOfThrowsOnEmptyCollection() {
+        final var ex = expectThrows(IllegalArgumentException.class, () -> randomNonEmptySubsetOf(Collections.emptySet()));
+        assertThat(ex.getMessage(), equalTo("Can't pick non-empty subset of an empty collection"));
+    }
+
     public void testRandomValueOtherThan() {
         // "normal" way of calling where the value is not null
         int bad = randomInt();

x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authc/Authentication.java

@@ -957,18 +957,18 @@ public static User readUserFrom(StreamInput input) throws IOException {
             final boolean isInternalUser = input.readBoolean();
             final String username = input.readString();
             if (isInternalUser) {
-                if (SystemUser.is(username)) {
+                if (SystemUser.NAME.equals(username)) {
                     return SystemUser.INSTANCE;
-                } else if (XPackUser.is(username)) {
+                } else if (XPackUser.NAME.equals(username)) {
                     return XPackUser.INSTANCE;
-                } else if (XPackSecurityUser.is(username)) {
+                } else if (XPackSecurityUser.NAME.equals(username)) {
                     return XPackSecurityUser.INSTANCE;
-                } else if (SecurityProfileUser.is(username)) {
+                } else if (SecurityProfileUser.NAME.equals(username)) {
                     return SecurityProfileUser.INSTANCE;
-                } else if (AsyncSearchUser.is(username)) {
+                } else if (AsyncSearchUser.NAME.equals(username)) {
                     return AsyncSearchUser.INSTANCE;
                 }
-                throw new IllegalStateException("user [" + username + "] is not an internal user");
+                throw new IllegalStateException("username [" + username + "] does not match any internal user");
             }
             return partialReadUserFrom(username, input);
         }

x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/user/AsyncSearchUser.java

@@ -53,7 +53,4 @@ public static boolean is(User user) {
         return INSTANCE.equals(user);
     }
 
-    public static boolean is(String principal) {
-        return NAME.equals(principal);
-    }
 }

x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/user/SecurityProfileUser.java

@@ -57,7 +57,4 @@ public static boolean is(User user) {
         return INSTANCE.equals(user);
     }
 
-    public static boolean is(String principal) {
-        return NAME.equals(principal);
-    }
 }

x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/user/SystemUser.java

@@ -44,10 +44,6 @@ public static boolean is(User user) {
         return INSTANCE.equals(user);
     }
 
-    public static boolean is(String principal) {
-        return NAME.equals(principal);
-    }
-
     public static boolean isAuthorized(String action) {
         return PREDICATE.test(action);
     }

x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/user/User.java

@@ -151,14 +151,6 @@ public static boolean isInternal(User user) {
             || AsyncSearchUser.is(user);
     }
 
-    public static boolean isInternalUsername(String username) {
-        return SystemUser.NAME.equals(username)
-            || XPackUser.NAME.equals(username)
-            || XPackSecurityUser.NAME.equals(username)
-            || SecurityProfileUser.NAME.equals(username)
-            || AsyncSearchUser.NAME.equals(username);
-    }
-
     /** Write the given {@link User} */
     public static void writeUser(User user, StreamOutput output) throws IOException {
         output.writeBoolean(false); // not a system user

x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/user/XPackSecurityUser.java

@@ -53,7 +53,4 @@ public static boolean is(User user) {
         return INSTANCE.equals(user);
     }
 
-    public static boolean is(String principal) {
-        return NAME.equals(principal);
-    }
 }

x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/user/XPackUser.java

@@ -54,7 +54,4 @@ public static boolean is(User user) {
         return INSTANCE.equals(user);
     }
 
-    public static boolean is(String principal) {
-        return NAME.equals(principal);
-    }
 }

x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/security/authc/AuthenticationTestHelper.java

@@ -70,6 +70,14 @@ public class AuthenticationTestHelper {
         ServiceAccountSettings.REALM_TYPE
     );
 
+    private static final Set<User> INTERNAL_USERS = Set.of(
+        SystemUser.INSTANCE,
+        XPackUser.INSTANCE,
+        XPackSecurityUser.INSTANCE,
+        AsyncSearchUser.INSTANCE,
+        SecurityProfileUser.INSTANCE
+    );
+
     public static AuthenticationTestBuilder builder() {
         return new AuthenticationTestBuilder();
     }
@@ -161,6 +169,17 @@ private static User stripRoles(User user) {
         }
     }
 
+    public static String randomInternalUsername() {
+        return builder().internal().build(false).getUser().principal();
+    }
+
+    /**
+     * @return non-empty collection of internal usernames
+     */
+    public static List<String> randomInternalUsernames() {
+        return ESTestCase.randomNonEmptySubsetOf(INTERNAL_USERS.stream().map(User::principal).toList());
+    }
+
     public static class AuthenticationTestBuilder {
         private Version version;
         private Authentication authenticatingAuthentication;
@@ -238,15 +257,7 @@ public AuthenticationTestBuilder anonymous(User user) {
         }
 
         public AuthenticationTestBuilder internal() {
-            return internal(
-                ESTestCase.randomFrom(
-                    SystemUser.INSTANCE,
-                    XPackUser.INSTANCE,
-                    XPackSecurityUser.INSTANCE,
-                    SecurityProfileUser.INSTANCE,
-                    AsyncSearchUser.INSTANCE
-                )
-            );
+            return internal(ESTestCase.randomFrom(INTERNAL_USERS));
         }
 
         public AuthenticationTestBuilder internal(User user) {
@@ -423,13 +434,7 @@ public Authentication build(boolean runAsIfNotAlready) {
                     }
                     case INTERNAL -> {
                         if (user == null) {
-                            user = ESTestCase.randomFrom(
-                                SystemUser.INSTANCE,
-                                XPackUser.INSTANCE,
-                                XPackSecurityUser.INSTANCE,
-                                SecurityProfileUser.INSTANCE,
-                                AsyncSearchUser.INSTANCE
-                            );
+                            user = ESTestCase.randomFrom(INTERNAL_USERS);
                         }
                         assert User.isInternal(user) : "user must be internal for internal authentication";
                         assert realmRef == null : "cannot specify realm type for internal authentication";

x-pack/plugin/security/src/internalClusterTest/java/org/elasticsearch/xpack/security/authc/esnative/NativeRealmIntegTests.java

@@ -51,6 +51,7 @@
 import org.elasticsearch.xpack.core.security.action.user.PutUserRequestBuilder;
 import org.elasticsearch.xpack.core.security.action.user.SetEnabledRequestBuilder;
 import org.elasticsearch.xpack.core.security.authc.Authentication;
+import org.elasticsearch.xpack.core.security.authc.AuthenticationTestHelper;
 import org.elasticsearch.xpack.core.security.authc.support.Hasher;
 import org.elasticsearch.xpack.core.security.authz.RestrictedIndices;
 import org.elasticsearch.xpack.core.security.authz.RoleDescriptor;
@@ -59,13 +60,9 @@
 import org.elasticsearch.xpack.core.security.support.Automatons;
 import org.elasticsearch.xpack.core.security.test.TestRestrictedIndices;
 import org.elasticsearch.xpack.core.security.user.AnonymousUser;
-import org.elasticsearch.xpack.core.security.user.AsyncSearchUser;
 import org.elasticsearch.xpack.core.security.user.ElasticUser;
 import org.elasticsearch.xpack.core.security.user.KibanaUser;
-import org.elasticsearch.xpack.core.security.user.SystemUser;
 import org.elasticsearch.xpack.core.security.user.User;
-import org.elasticsearch.xpack.core.security.user.XPackSecurityUser;
-import org.elasticsearch.xpack.core.security.user.XPackUser;
 import org.elasticsearch.xpack.security.LocalStateSecurity;
 import org.elasticsearch.xpack.security.authz.store.NativeRolesStore;
 import org.junit.Before;
@@ -269,7 +266,24 @@ public void testAddAndGetRole() throws Exception {
         assertFalse("role should not exist after being deleted", resp2.hasRoles());
     }
 
-    public void testAddUserAndRoleThenAuth() throws Exception {
+    public void testAddUserAndRoleThenAuth() {
+        testAddUserAndRoleThenAuth("joe");
+    }
+
+    public void testAddUserWithInternalUsernameAndRoleThenAuth() {
+        testAddUserAndRoleThenAuth(AuthenticationTestHelper.randomInternalUsername());
+    }
+
+    public void testAuthWithInternalUsernameFailsWithoutCorrespondingUser() {
+        String token = basicAuthHeaderValue(AuthenticationTestHelper.randomInternalUsername(), new SecureString("s3krit-password"));
+        ElasticsearchSecurityException e = expectThrows(
+            ElasticsearchSecurityException.class,
+            () -> client().filterWithHeader(Collections.singletonMap("Authorization", token)).prepareSearch("idx").get()
+        );
+        assertThat(e.status(), is(RestStatus.UNAUTHORIZED));
+    }
+
+    private void testAddUserAndRoleThenAuth(String username) {
         logger.error("--> creating role");
         preparePutRole("test_role").cluster("all")
             .addIndices(
@@ -282,19 +296,19 @@ public void testAddUserAndRoleThenAuth() throws Exception {
             )
             .get();
         logger.error("--> creating user");
-        preparePutUser("joe", "s3krit-password", hasher, "test_role").get();
+        preparePutUser(username, "s3krit-password", hasher, "test_role").get();
         logger.error("--> waiting for .security index");
         ensureGreen(SECURITY_MAIN_ALIAS);
         logger.info("--> retrieving user");
-        GetUsersResponse resp = new GetUsersRequestBuilder(client()).usernames("joe").get();
+        GetUsersResponse resp = new GetUsersRequestBuilder(client()).usernames(username).get();
         assertTrue("user should exist", resp.hasUsers());
 
         createIndex("idx");
         ensureGreen("idx");
         // Index a document with the default test user
         client().prepareIndex("idx").setId("1").setSource("body", "foo").setRefreshPolicy(IMMEDIATE).get();
 
-        String token = basicAuthHeaderValue("joe", new SecureString("s3krit-password"));
+        String token = basicAuthHeaderValue(username, new SecureString("s3krit-password"));
         SearchResponse searchResp = client().filterWithHeader(Collections.singletonMap("Authorization", token)).prepareSearch("idx").get();
 
         assertEquals(1L, searchResp.getHits().getTotalHits().value);
@@ -721,19 +735,6 @@ public void testOperationsOnReservedUsers() throws Exception {
         );
         assertThat(exception.getMessage(), containsString("user [" + AnonymousUser.DEFAULT_ANONYMOUS_USERNAME + "] is anonymous"));
 
-        final String internalUser = randomFrom(SystemUser.NAME, XPackUser.NAME, XPackSecurityUser.NAME, AsyncSearchUser.NAME);
-        exception = expectThrows(IllegalArgumentException.class, () -> preparePutUser(internalUser, "foobar-password", hasher).get());
-        assertThat(exception.getMessage(), containsString("user [" + internalUser + "] is internal"));
-
-        exception = expectThrows(
-            IllegalArgumentException.class,
-            () -> new ChangePasswordRequestBuilder(client()).username(internalUser).password("foobar-password".toCharArray(), hasher).get()
-        );
-        assertThat(exception.getMessage(), containsString("user [" + internalUser + "] is internal"));
-
-        exception = expectThrows(IllegalArgumentException.class, () -> new DeleteUserRequestBuilder(client()).username(internalUser).get());
-        assertThat(exception.getMessage(), containsString("user [" + internalUser + "] is internal"));
-
         // get should work
         GetUsersResponse response = new GetUsersRequestBuilder(client()).usernames(username).get();
         assertThat(response.hasUsers(), is(true));
@@ -989,4 +990,5 @@ private PutUserRequestBuilder preparePutUser(String username, String password, H
     private PutRoleRequestBuilder preparePutRole(String name) {
         return new PutRoleRequestBuilder(client()).name(name);
     }
+
 }

x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/action/user/TransportChangePasswordAction.java

@@ -19,7 +19,6 @@
 import org.elasticsearch.xpack.core.security.action.user.ChangePasswordRequest;
 import org.elasticsearch.xpack.core.security.authc.support.Hasher;
 import org.elasticsearch.xpack.core.security.user.AnonymousUser;
-import org.elasticsearch.xpack.core.security.user.User;
 import org.elasticsearch.xpack.security.authc.esnative.NativeUsersStore;
 
 public class TransportChangePasswordAction extends HandledTransportAction<ChangePasswordRequest, ActionResponse.Empty> {
@@ -45,9 +44,6 @@ protected void doExecute(Task task, ChangePasswordRequest request, ActionListene
         if (AnonymousUser.isAnonymousUsername(username, settings)) {
             listener.onFailure(new IllegalArgumentException("user [" + username + "] is anonymous and cannot be modified via the API"));
             return;
-        } else if (User.isInternalUsername(username)) {
-            listener.onFailure(new IllegalArgumentException("user [" + username + "] is internal"));
-            return;
         }
         final String requestPwdHashAlgo = Hasher.resolveFromHash(request.passwordHash()).name();
         final String configPwdHashAlgo = Hasher.resolve(XPackSettings.PASSWORD_HASHING_ALGORITHM.get(settings)).name();

x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/action/user/TransportDeleteUserAction.java

@@ -18,7 +18,6 @@
 import org.elasticsearch.xpack.core.security.action.user.DeleteUserResponse;
 import org.elasticsearch.xpack.core.security.authc.esnative.ClientReservedRealm;
 import org.elasticsearch.xpack.core.security.user.AnonymousUser;
-import org.elasticsearch.xpack.core.security.user.User;
 import org.elasticsearch.xpack.security.authc.esnative.NativeUsersStore;
 
 public class TransportDeleteUserAction extends HandledTransportAction<DeleteUserRequest, DeleteUserResponse> {
@@ -44,13 +43,9 @@ protected void doExecute(Task task, DeleteUserRequest request, final ActionListe
         if (ClientReservedRealm.isReserved(username, settings)) {
             if (AnonymousUser.isAnonymousUsername(username, settings)) {
                 listener.onFailure(new IllegalArgumentException("user [" + username + "] is anonymous and cannot be deleted"));
-                return;
             } else {
                 listener.onFailure(new IllegalArgumentException("user [" + username + "] is reserved and cannot be deleted"));
-                return;
             }
-        } else if (User.isInternalUsername(username)) {
-            listener.onFailure(new IllegalArgumentException("user [" + username + "] is internal"));
             return;
         }
 

x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/action/user/TransportGetUsersAction.java

@@ -59,9 +59,6 @@ protected void doExecute(Task task, final GetUsersRequest request, final ActionL
             for (String username : requestedUsers) {
                 if (ClientReservedRealm.isReserved(username, settings)) {
                     realmLookup.add(username);
-                } else if (User.isInternalUsername(username)) {
-                    listener.onFailure(new IllegalArgumentException("user [" + username + "] is internal"));
-                    return;
                 } else {
                     usersToSearchFor.add(username);
                 }

x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/action/user/TransportPutUserAction.java

@@ -22,7 +22,6 @@
 import org.elasticsearch.xpack.core.security.authc.esnative.ClientReservedRealm;
 import org.elasticsearch.xpack.core.security.support.Validation;
 import org.elasticsearch.xpack.core.security.user.AnonymousUser;
-import org.elasticsearch.xpack.core.security.user.User;
 import org.elasticsearch.xpack.security.authc.esnative.NativeUsersStore;
 
 import static org.elasticsearch.action.ValidateActions.addValidationError;
@@ -85,8 +84,6 @@ private ActionRequestValidationException validateRequest(PutUserRequest request)
                     validationException
                 );
             }
-        } else if (User.isInternalUsername(username)) {
-            validationException = addValidationError("user [" + username + "] is internal", validationException);
         } else {
             Validation.Error usernameError = Validation.Users.validateUsername(username, true, settings);
             if (usernameError != null) {

x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/action/user/TransportSetEnabledAction.java

@@ -18,7 +18,6 @@
 import org.elasticsearch.xpack.core.security.action.user.SetEnabledAction;
 import org.elasticsearch.xpack.core.security.action.user.SetEnabledRequest;
 import org.elasticsearch.xpack.core.security.user.AnonymousUser;
-import org.elasticsearch.xpack.core.security.user.User;
 import org.elasticsearch.xpack.security.authc.esnative.NativeUsersStore;
 
 /**
@@ -51,9 +50,6 @@ protected void doExecute(Task task, SetEnabledRequest request, ActionListener<Ac
         if (securityContext.getUser().principal().equals(request.username())) {
             listener.onFailure(new IllegalArgumentException("users may not update the enabled status of their own account"));
             return;
-        } else if (User.isInternalUsername(username)) {
-            listener.onFailure(new IllegalArgumentException("user [" + username + "] is internal"));
-            return;
         } else if (AnonymousUser.isAnonymousUsername(username, settings)) {
             listener.onFailure(new IllegalArgumentException("user [" + username + "] is anonymous and cannot be modified using the api"));
             return;

x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authc/esnative/NativeUsersStore.java

@@ -244,7 +244,6 @@ public void onFailure(Exception t) {
      */
     public void changePassword(final ChangePasswordRequest request, final ActionListener<Void> listener) {
         final String username = request.username();
-        assert User.isInternalUsername(username) == false : username + "is internal!";
         final String docType;
         if (ClientReservedRealm.isReserved(username, settings)) {
             docType = RESERVED_USER_TYPE;