Don't allow null User.principal (#51988)

Some parts of the User class (e.g. equals/hashCode) assumed that
principal could never be null, but the constructor didn't enforce
that.

This adds a null check into the constructor and fixes a few tests that
relied on being able to pass in null usernames.

Author: tvernum

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

@@ -16,6 +16,7 @@
 import java.io.IOException;
 import java.util.Arrays;
 import java.util.Map;
+import java.util.Objects;
 
 /**
  * An authenticated user
@@ -49,7 +50,7 @@ public User(String username, String[] roles, String fullName, String email, Map<
 
     private User(String username, String[] roles, String fullName, String email, Map<String, Object> metadata, boolean enabled,
                 User authenticatedUser) {
-        this.username = username;
+        this.username = Objects.requireNonNull(username);
         this.roles = roles == null ? Strings.EMPTY_ARRAY : roles;
         this.metadata = metadata == null ? Map.of() : metadata;
         this.fullName = fullName;

x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/audit/logfile/LoggingAuditTrailFilterTests.java

@@ -113,11 +113,9 @@ public void testPolicyDoesNotMatchNullValuesInEvent() throws Exception {
         // user field matches
         assertTrue("Matches the user filter predicate.", auditTrail.eventFilterPolicyRegistry.ignorePredicate().test(
                 new AuditEventMetaInfo(Optional.of(randomFrom(filteredUsers)), Optional.empty(), Optional.empty(), Optional.empty())));
-        final User unfilteredUser;
+        final User unfilteredUser = mock(User.class);
         if (randomBoolean()) {
-            unfilteredUser = new User(null);
-        } else {
-            unfilteredUser = new User(new User(null), new User(randomFrom(filteredUsers).principal()));
+            when(unfilteredUser.authenticatedUser()).thenReturn(new User(randomFrom(filteredUsernames)));
         }
         // null user field does NOT match
         assertFalse("Does not match the user filter predicate because of null username.",

x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/ingest/SetSecurityUserProcessorTests.java

@@ -13,6 +13,7 @@
 import org.elasticsearch.xpack.core.security.authc.AuthenticationField;
 import org.elasticsearch.xpack.core.security.user.User;
 import org.elasticsearch.xpack.security.ingest.SetSecurityUserProcessor.Property;
+import org.mockito.Mockito;
 
 import java.util.Arrays;
 import java.util.EnumSet;
@@ -42,15 +43,21 @@ public void testProcessor() throws Exception {
         assertThat(result.get("email"), equalTo("_email"));
         assertThat(((Map) result.get("metadata")).size(), equalTo(1));
         assertThat(((Map) result.get("metadata")).get("key"), equalTo("value"));
+    }
 
-        // test when user holds no data:
-        threadContext = new ThreadContext(Settings.EMPTY);
-        user = new User(null, null, null);
-        threadContext.putTransient(AuthenticationField.AUTHENTICATION_KEY, new Authentication(user, realmRef, null));
-        ingestDocument = new IngestDocument(new HashMap<>(), new HashMap<>());
-        processor = new SetSecurityUserProcessor("_tag", threadContext, "_field", EnumSet.allOf(Property.class));
+    public void testProcessorWithEmptyUserData() throws Exception {
+        // test when user returns null for all values (need a mock, because a real user cannot have a null username)
+        User user = Mockito.mock(User.class);
+        Authentication authentication = Mockito.mock(Authentication.class);
+        Mockito.when(authentication.getUser()).thenReturn(user);
+
+        ThreadContext threadContext = new ThreadContext(Settings.EMPTY);
+        threadContext.putTransient(AuthenticationField.AUTHENTICATION_KEY, authentication);
+
+        IngestDocument ingestDocument = new IngestDocument(new HashMap<>(), new HashMap<>());
+        SetSecurityUserProcessor processor = new SetSecurityUserProcessor("_tag", threadContext, "_field", EnumSet.allOf(Property.class));
         processor.execute(ingestDocument);
-        result = ingestDocument.getFieldValue("_field", Map.class);
+        Map result = ingestDocument.getFieldValue("_field", Map.class);
         assertThat(result.size(), equalTo(0));
     }
 
@@ -80,7 +87,7 @@ public void testUsernameProperties() throws Exception {
 
     public void testRolesProperties() throws Exception {
         ThreadContext threadContext = new ThreadContext(Settings.EMPTY);
-        User user = new User(null, "role1", "role2");
+        User user = new User(randomAlphaOfLengthBetween(4, 12), "role1", "role2");
         Authentication.RealmRef realmRef = new Authentication.RealmRef("_name", "_type", "_node_name");
         threadContext.putTransient(AuthenticationField.AUTHENTICATION_KEY, new Authentication(user, realmRef, null));
 
@@ -96,7 +103,7 @@ public void testRolesProperties() throws Exception {
 
     public void testFullNameProperties() throws Exception {
         ThreadContext threadContext = new ThreadContext(Settings.EMPTY);
-        User user = new User(null, null, "_full_name", null, Map.of(), true);
+        User user = new User(randomAlphaOfLengthBetween(4, 12), null, "_full_name", null, Map.of(), true);
         Authentication.RealmRef realmRef = new Authentication.RealmRef("_name", "_type", "_node_name");
         threadContext.putTransient(AuthenticationField.AUTHENTICATION_KEY, new Authentication(user, realmRef, null));
 
@@ -112,7 +119,7 @@ public void testFullNameProperties() throws Exception {
 
     public void testEmailProperties() throws Exception {
         ThreadContext threadContext = new ThreadContext(Settings.EMPTY);
-        User user = new User(null, null, null, "_email", Map.of(), true);
+        User user = new User(randomAlphaOfLengthBetween(4, 12), null, null, "_email", Map.of(), true);
         Authentication.RealmRef realmRef = new Authentication.RealmRef("_name", "_type", "_node_name");
         threadContext.putTransient(AuthenticationField.AUTHENTICATION_KEY, new Authentication(user, realmRef, null));
 
@@ -128,7 +135,7 @@ public void testEmailProperties() throws Exception {
 
     public void testMetadataProperties() throws Exception {
         ThreadContext threadContext = new ThreadContext(Settings.EMPTY);
-        User user = new User(null, null, null, null, Map.of("key", "value"), true);
+        User user = new User(randomAlphaOfLengthBetween(4, 12), null, null, null, Map.of("key", "value"), true);
         Authentication.RealmRef realmRef = new Authentication.RealmRef("_name", "_type", "_node_name");
         threadContext.putTransient(AuthenticationField.AUTHENTICATION_KEY, new Authentication(user, realmRef, null));