Revert "Resolve anonymous roles and deduplicate roles during authentication (#53453) (#55995)" (#57858)

This reverts commit 84a2f1a.

Author: ywangd

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

@@ -46,4 +46,4 @@ public void writeTo(StreamOutput out) throws IOException {
         }
     }
 
-}
+    }

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

@@ -119,10 +119,6 @@ public boolean isRunAs() {
         return authenticatedUser != null;
     }
 
-    public User withRoles(String[] newRoles) {
-        return new User(username, newRoles, fullName, email, metadata, enabled);
-    }
-
     @Override
     public String toString() {
         StringBuilder sb = new StringBuilder();

x-pack/plugin/security/qa/basic-enable-security/src/test/java/org/elasticsearch/xpack/security/EnableSecurityOnBasicLicenseIT.java

@@ -24,7 +24,7 @@
 import java.util.Map;
 
 import static org.elasticsearch.xpack.core.security.authc.support.UsernamePasswordToken.basicAuthHeaderValue;
-import static org.hamcrest.Matchers.containsInAnyOrder;
+import static org.hamcrest.Matchers.contains;
 import static org.hamcrest.Matchers.containsString;
 import static org.hamcrest.Matchers.equalTo;
 import static org.hamcrest.Matchers.notNullValue;
@@ -126,7 +126,7 @@ private void checkAuthentication() throws IOException {
         final Map<String, Object> auth = getAsMap("/_security/_authenticate");
         // From file realm, configured in build.gradle
         assertThat(ObjectPath.evaluate(auth, "username"), equalTo("security_test_user"));
-        assertThat(ObjectPath.evaluate(auth, "roles"), containsInAnyOrder("security_test_role", "anonymous"));
+        assertThat(ObjectPath.evaluate(auth, "roles"), contains("security_test_role"));
     }
 
     private void checkAllowedWrite(String indexName) throws IOException {

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

@@ -49,14 +49,11 @@
 import org.elasticsearch.xpack.security.support.SecurityIndexManager;
 
 import java.util.ArrayList;
-import java.util.Arrays;
 import java.util.Collections;
 import java.util.LinkedHashMap;
-import java.util.LinkedHashSet;
 import java.util.List;
 import java.util.Map;
 import java.util.Objects;
-import java.util.Set;
 import java.util.concurrent.atomic.AtomicLong;
 import java.util.function.BiConsumer;
 import java.util.function.Consumer;
@@ -669,8 +666,7 @@ void finishAuthentication(User finalUser) {
                 logger.debug("user [{}] is disabled. failing authentication", finalUser);
                 listener.onFailure(request.authenticationFailed(authenticationToken));
             } else {
-                final Authentication finalAuth = new Authentication(
-                    maybeConsolidateRolesForUser(finalUser), authenticatedBy, lookedupBy);
+                final Authentication finalAuth = new Authentication(finalUser, authenticatedBy, lookedupBy);
                 writeAuthToContext(finalAuth);
             }
         }
@@ -703,33 +699,6 @@ void writeAuthToContext(Authentication authentication) {
         private void authenticateToken(AuthenticationToken token) {
             this.consumeToken(token);
         }
-
-        private User maybeConsolidateRolesForUser(User user) {
-            if (User.isInternal(user)) {
-                return user;
-            } else if (isAnonymousUserEnabled && anonymousUser.equals(user) == false) {
-                if (anonymousUser.roles().length == 0) {
-                    throw new IllegalStateException("anonymous is only enabled when the anonymous user has roles");
-                }
-                User userWithMergedRoles = user.withRoles(mergeRoles(user.roles(), anonymousUser.roles()));
-                if (user.isRunAs()) {
-                    final User authUserWithMergedRoles = user.authenticatedUser().withRoles(
-                        mergeRoles(user.authenticatedUser().roles(), anonymousUser.roles()));
-                    userWithMergedRoles = new User(userWithMergedRoles, authUserWithMergedRoles);
-                }
-                return userWithMergedRoles;
-            } else {
-                return user;
-            }
-        }
-
-        private String[] mergeRoles(String[] existingRoles, String[] otherRoles) {
-            Set<String> roles = new LinkedHashSet<>(Arrays.asList(existingRoles));
-            if (otherRoles != null) {
-                Collections.addAll(roles, otherRoles);
-            }
-            return roles.toArray(new String[0]);
-        }
     }
 
     abstract static class AuditableRequest {

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

@@ -24,21 +24,21 @@
  */
 public class NativeRealm extends CachingUsernamePasswordRealm {
 
-    private final NativeUsersStore usersStore;
+    private final NativeUsersStore userStore;
 
     public NativeRealm(RealmConfig config, NativeUsersStore usersStore, ThreadPool threadPool) {
         super(config, threadPool);
-        this.usersStore = usersStore;
+        this.userStore = usersStore;
     }
 
     @Override
     protected void doLookupUser(String username, ActionListener<User> listener) {
-        usersStore.getUser(username, listener);
+        userStore.getUser(username, listener);
     }
 
     @Override
     protected void doAuthenticate(UsernamePasswordToken token, ActionListener<AuthenticationResult> listener) {
-        usersStore.verifyPassword(token.principal(), token.credentials(), listener);
+        userStore.verifyPassword(token.principal(), token.credentials(), listener);
     }
 
     public void onSecurityIndexStateChange(SecurityIndexManager.State previousState, SecurityIndexManager.State currentState) {
@@ -50,7 +50,7 @@ public void onSecurityIndexStateChange(SecurityIndexManager.State previousState,
     @Override
     public void usageStats(ActionListener<Map<String, Object>> listener) {
         super.usageStats(ActionListener.wrap(stats ->
-            usersStore.getUserCount(ActionListener.wrap(size -> {
+            userStore.getUserCount(ActionListener.wrap(size -> {
                 stats.put("size", size);
                 listener.onResponse(stats);
             }, listener::onFailure))

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

@@ -57,7 +57,6 @@
 import java.util.Collection;
 import java.util.Collections;
 import java.util.HashMap;
-import java.util.LinkedHashSet;
 import java.util.List;
 import java.util.Map;
 import java.util.function.Consumer;
@@ -647,7 +646,7 @@ private UserAndPassword transformUser(final String id, final Map<String, Object>
         final String username = id.substring(USER_DOC_TYPE.length() + 1);
         try {
             String password = (String) sourceMap.get(Fields.PASSWORD.getPreferredName());
-            String[] roles = new LinkedHashSet<>((List<String>)sourceMap.get(Fields.ROLES.getPreferredName())).toArray(Strings.EMPTY_ARRAY);
+            String[] roles = ((List<String>) sourceMap.get(Fields.ROLES.getPreferredName())).toArray(Strings.EMPTY_ARRAY);
             String fullName = (String) sourceMap.get(Fields.FULL_NAME.getPreferredName());
             String email = (String) sourceMap.get(Fields.EMAIL.getPreferredName());
             Boolean enabled = (Boolean) sourceMap.get(Fields.ENABLED.getPreferredName());

x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authc/file/FileUserRolesStore.java

@@ -30,7 +30,6 @@
 import java.util.ArrayList;
 import java.util.Collections;
 import java.util.HashMap;
-import java.util.LinkedHashSet;
 import java.util.List;
 import java.util.Locale;
 import java.util.Map;
@@ -165,7 +164,7 @@ public static Map<String, String[]> parseFile(Path path, @Nullable Logger logger
 
         Map<String, String[]> usersRoles = new HashMap<>();
         for (Map.Entry<String, List<String>> entry : userToRoles.entrySet()) {
-            usersRoles.put(entry.getKey(), new LinkedHashSet<>(entry.getValue()).toArray(new String[0]));
+            usersRoles.put(entry.getKey(), entry.getValue().toArray(new String[entry.getValue().size()]));
         }
 
         logger.debug("parsed [{}] user to role mappings from file [{}]", usersRoles.size(), path.toAbsolutePath());

x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authz/AuthorizationService.java

@@ -60,8 +60,11 @@
 import org.elasticsearch.xpack.core.security.authz.privilege.ClusterPrivilegeResolver;
 import org.elasticsearch.xpack.core.security.authz.privilege.IndexPrivilege;
 import org.elasticsearch.xpack.core.security.user.AnonymousUser;
+import org.elasticsearch.xpack.core.security.user.AsyncSearchUser;
 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.audit.AuditLevel;
 import org.elasticsearch.xpack.security.audit.AuditTrail;
 import org.elasticsearch.xpack.security.audit.AuditTrailService;
@@ -170,7 +173,7 @@ public void authorize(final Authentication authentication, final String action,
         if (auditId == null) {
             // We would like to assert that there is an existing request-id, but if this is a system action, then that might not be
             // true because the request-id is generated during authentication
-            if (User.isInternal(authentication.getUser()) != false) {
+            if (isInternalUser(authentication.getUser()) != false) {
                 auditId = AuditUtil.getOrGenerateRequestId(threadContext);
             } else {
                 auditTrailService.get().tamperedRequest(null, authentication.getUser(), action, originalRequest);
@@ -365,7 +368,7 @@ AuthorizationEngine getAuthorizationEngine(final Authentication authentication)
     private AuthorizationEngine getAuthorizationEngineForUser(final User user) {
         if (rbacEngine != authorizationEngine && licenseState.isSecurityEnabled() &&
             licenseState.isAllowed(Feature.SECURITY_AUTHORIZATION_ENGINE)) {
-            if (ClientReservedRealm.isReserved(user.principal(), settings) || User.isInternal(user)) {
+            if (ClientReservedRealm.isReserved(user.principal(), settings) || isInternalUser(user)) {
                 return rbacEngine;
             } else {
                 return authorizationEngine;
@@ -416,6 +419,10 @@ private TransportRequest maybeUnwrapRequest(Authentication authentication, Trans
         return request;
     }
 
+    private boolean isInternalUser(User user) {
+        return SystemUser.is(user) || XPackUser.is(user) || XPackSecurityUser.is(user) || AsyncSearchUser.is(user);
+    }
+
     private void authorizeRunAs(final RequestInfo requestInfo, final AuthorizationInfo authzInfo,
                                 final ActionListener<AuthorizationResult> listener) {
         final Authentication authentication = requestInfo.getAuthentication();

x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authz/store/CompositeRolesStore.java

@@ -44,6 +44,7 @@
 import org.elasticsearch.xpack.core.security.authz.store.RoleRetrievalResult;
 import org.elasticsearch.xpack.core.security.support.CacheIteratorHelper;
 import org.elasticsearch.xpack.core.security.support.MetadataUtils;
+import org.elasticsearch.xpack.core.security.user.AnonymousUser;
 import org.elasticsearch.xpack.core.security.user.AsyncSearchUser;
 import org.elasticsearch.xpack.core.security.user.SystemUser;
 import org.elasticsearch.xpack.core.security.user.User;
@@ -100,7 +101,9 @@ public class CompositeRolesStore {
     private final DocumentSubsetBitsetCache dlsBitsetCache;
     private final ThreadContext threadContext;
     private final AtomicLong numInvalidation = new AtomicLong();
+    private final AnonymousUser anonymousUser;
     private final ApiKeyService apiKeyService;
+    private final boolean isAnonymousEnabled;
     private final List<BiConsumer<Set<String>, ActionListener<RoleRetrievalResult>>> builtInRoleProviders;
     private final List<BiConsumer<Set<String>, ActionListener<RoleRetrievalResult>>> allRoleProviders;
 
@@ -143,6 +146,8 @@ public CompositeRolesStore(Settings settings, FileRolesStore fileRolesStore, Nat
             allList.addAll(rolesProviders);
             this.allRoleProviders = Collections.unmodifiableList(allList);
         }
+        this.anonymousUser = new AnonymousUser(settings);
+        this.isAnonymousEnabled = AnonymousUser.isAnonymousEnabled(settings);
     }
 
     public void roles(Set<String> roleNames, ActionListener<Role> roleActionListener) {
@@ -232,6 +237,13 @@ public void getRoles(User user, Authentication authentication, ActionListener<Ro
             }, roleActionListener::onFailure));
         } else {
             Set<String> roleNames = new HashSet<>(Arrays.asList(user.roles()));
+            if (isAnonymousEnabled && anonymousUser.equals(user) == false) {
+                if (anonymousUser.roles().length == 0) {
+                    throw new IllegalStateException("anonymous is only enabled when the anonymous user has roles");
+                }
+                Collections.addAll(roleNames, anonymousUser.roles());
+            }
+
             if (roleNames.isEmpty()) {
                 roleActionListener.onResponse(Role.EMPTY);
             } else if (roleNames.contains(ReservedRolesStore.SUPERUSER_ROLE_DESCRIPTOR.getName())) {

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

@@ -74,11 +74,8 @@
 import org.elasticsearch.xpack.core.security.authz.AuthorizationEngine.EmptyAuthorizationInfo;
 import org.elasticsearch.xpack.core.security.index.RestrictedIndicesNames;
 import org.elasticsearch.xpack.core.security.user.AnonymousUser;
-import org.elasticsearch.xpack.core.security.user.AsyncSearchUser;
 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.audit.AuditTrail;
 import org.elasticsearch.xpack.security.audit.AuditTrailService;
 import org.elasticsearch.xpack.security.audit.AuditUtil;
@@ -111,7 +108,6 @@
 import static org.elasticsearch.xpack.core.security.support.Exceptions.authenticationError;
 import static org.elasticsearch.xpack.security.authc.TokenServiceTests.mockGetTokenFromId;
 import static org.hamcrest.Matchers.arrayContaining;
-import static org.hamcrest.Matchers.arrayContainingInAnyOrder;
 import static org.hamcrest.Matchers.contains;
 import static org.hamcrest.Matchers.containsString;
 import static org.hamcrest.Matchers.emptyOrNullString;
@@ -906,48 +902,6 @@ public void testAnonymousUserTransportWithDefaultUser() throws Exception {
         assertThreadContextContainsAuthentication(result);
     }
 
-    public void testInheritAnonymousUserRoles() {
-        Settings settings = Settings.builder()
-            .putList(AnonymousUser.ROLES_SETTING.getKey(), "r3", "r4", "r5")
-            .build();
-        final AnonymousUser anonymousUser = new AnonymousUser(settings);
-        service = new AuthenticationService(settings, realms, auditTrailService,
-            new DefaultAuthenticationFailureHandler(Collections.emptyMap()),
-            threadPool, anonymousUser, tokenService, apiKeyService);
-        User user1 = new User("username", "r1", "r2", "r3");
-        when(firstRealm.token(threadContext)).thenReturn(token);
-        when(firstRealm.supports(token)).thenReturn(true);
-        mockAuthenticate(firstRealm, token, user1);
-        // this call does not actually go async
-        final AtomicBoolean completed = new AtomicBoolean(false);
-        service.authenticate(restRequest, true, ActionListener.wrap(authentication -> {
-            assertThat(authentication.getUser().roles(), arrayContainingInAnyOrder("r1", "r2", "r3", "r4", "r5"));
-            setCompletedToTrue(completed);
-        }, this::logAndFail));
-        assertTrue(completed.get());
-    }
-
-    public void testSystemUsersDoNotInheritAnonymousRoles() {
-        Settings settings = Settings.builder()
-            .putList(AnonymousUser.ROLES_SETTING.getKey(), "r3", "r4", "r5")
-            .build();
-        final AnonymousUser anonymousUser = new AnonymousUser(settings);
-        service = new AuthenticationService(settings, realms, auditTrailService,
-            new DefaultAuthenticationFailureHandler(Collections.emptyMap()),
-            threadPool, anonymousUser, tokenService, apiKeyService);
-        when(firstRealm.token(threadContext)).thenReturn(token);
-        when(firstRealm.supports(token)).thenReturn(true);
-        final User sysUser = randomFrom(SystemUser.INSTANCE, XPackUser.INSTANCE, XPackSecurityUser.INSTANCE, AsyncSearchUser.INSTANCE);
-        mockAuthenticate(firstRealm, token, sysUser);
-        // this call does not actually go async
-        final AtomicBoolean completed = new AtomicBoolean(false);
-        service.authenticate(restRequest, true, ActionListener.wrap(authentication -> {
-            assertThat(authentication.getUser().roles(), equalTo(sysUser.roles()));
-            setCompletedToTrue(completed);
-        }, this::logAndFail));
-        assertTrue(completed.get());
-    }
-
     public void testRealmTokenThrowingException() throws Exception {
         final String reqId = AuditUtil.getOrGenerateRequestId(threadContext);
         when(firstRealm.token(threadContext)).thenThrow(authenticationError("realm doesn't like tokens"));

x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authc/esnative/NativeUsersStoreTests.java

@@ -53,7 +53,6 @@
 import static org.hamcrest.CoreMatchers.equalTo;
 import static org.hamcrest.CoreMatchers.notNullValue;
 import static org.hamcrest.CoreMatchers.nullValue;
-import static org.hamcrest.Matchers.arrayContainingInAnyOrder;
 import static org.mockito.Matchers.any;
 import static org.mockito.Mockito.doAnswer;
 import static org.mockito.Mockito.mock;
@@ -157,21 +156,6 @@ public void testVerifyUserWithCorrectPassword() throws Exception {
     }
 
     public void testVerifyUserWithIncorrectPassword() throws Exception {
-        final NativeUsersStore nativeUsersStore = startNativeUsersStore();
-        final String username = randomAlphaOfLengthBetween(4, 12);
-        final SecureString password = new SecureString(randomAlphaOfLengthBetween(12, 16).toCharArray());
-        final List<String> roles = randomList(1, 4, () -> randomAlphaOfLength(12));
-        roles.add(randomIntBetween(0, roles.size()), roles.get(0));
-
-        final PlainActionFuture<AuthenticationResult> future = new PlainActionFuture<>();
-        nativeUsersStore.verifyPassword(username, password, future);
-        respondToGetUserRequest(username, password, roles.toArray(new String[0]));
-
-        final AuthenticationResult result = future.get();
-        assertThat(result.getUser().roles(), arrayContainingInAnyOrder(roles.stream().distinct().toArray()));
-    }
-
-    public void testDeduplicateUserRoles() throws Exception {
         final NativeUsersStore nativeUsersStore = startNativeUsersStore();
         final String username = randomAlphaOfLengthBetween(4, 12);
         final SecureString correctPassword = new SecureString(randomAlphaOfLengthBetween(12, 16).toCharArray());