Resolve anonymous roles and deduplicate roles during authentication (#…

…53453)

Anonymous roles resolution and user role deduplication are now performed during authentication instead of authorization. The change ensures:

* If anonymous access is enabled, user will be able to see the anonymous roles added in the roles field in the /_security/_authenticate response.
* Any duplication in user roles are removed and will not show in the above authenticate response.
* In any other case, the response is unchanged.

It also introduces a behaviour change: the anonymous role resolution is now authentication node specific, previously it was authorization node specific. Details can be found at #47195 (comment)

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

@@ -34,4 +34,4 @@ public void writeTo(StreamOutput out) throws IOException {
         authentication.writeTo(out);
     }
 
-    }
+}

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

@@ -118,6 +118,10 @@ 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.contains;
+import static org.hamcrest.Matchers.containsInAnyOrder;
 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"), contains("security_test_role"));
+        assertThat(ObjectPath.evaluate(auth, "roles"), containsInAnyOrder("security_test_role", "anonymous"));
     }
 
     private void checkAllowedWrite(String indexName) throws IOException {

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

@@ -49,11 +49,14 @@
 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;
@@ -656,7 +659,8 @@ void finishAuthentication(User finalUser) {
                 logger.debug("user [{}] is disabled. failing authentication", finalUser);
                 listener.onFailure(request.authenticationFailed(authenticationToken));
             } else {
-                final Authentication finalAuth = new Authentication(finalUser, authenticatedBy, lookedupBy);
+                final Authentication finalAuth = new Authentication(
+                    maybeConsolidateRolesForUser(finalUser), authenticatedBy, lookedupBy);
                 writeAuthToContext(finalAuth);
             }
         }
@@ -689,6 +693,33 @@ 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 userStore;
+    private final NativeUsersStore usersStore;
 
     public NativeRealm(RealmConfig config, NativeUsersStore usersStore, ThreadPool threadPool) {
         super(config, threadPool);
-        this.userStore = usersStore;
+        this.usersStore = usersStore;
     }
 
     @Override
     protected void doLookupUser(String username, ActionListener<User> listener) {
-        userStore.getUser(username, listener);
+        usersStore.getUser(username, listener);
     }
 
     @Override
     protected void doAuthenticate(UsernamePasswordToken token, ActionListener<AuthenticationResult> listener) {
-        userStore.verifyPassword(token.principal(), token.credentials(), listener);
+        usersStore.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 ->
-            userStore.getUserCount(ActionListener.wrap(size -> {
+            usersStore.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,6 +57,7 @@
 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;
@@ -643,7 +644,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 = ((List<String>) sourceMap.get(Fields.ROLES.getPreferredName())).toArray(Strings.EMPTY_ARRAY);
+            String[] roles = new LinkedHashSet<>((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,6 +30,7 @@
 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;
@@ -164,7 +165,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(), entry.getValue().toArray(new String[entry.getValue().size()]));
+            usersRoles.put(entry.getKey(), new LinkedHashSet<>(entry.getValue()).toArray(new String[0]));
         }
 
         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,11 +60,8 @@
 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;
@@ -173,7 +170,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 (isInternalUser(authentication.getUser()) != false) {
+            if (User.isInternal(authentication.getUser()) != false) {
                 auditId = AuditUtil.getOrGenerateRequestId(threadContext);
             } else {
                 auditTrailService.get().tamperedRequest(null, authentication.getUser(), action, originalRequest);
@@ -368,7 +365,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) || isInternalUser(user)) {
+            if (ClientReservedRealm.isReserved(user.principal(), settings) || User.isInternal(user)) {
                 return rbacEngine;
             } else {
                 return authorizationEngine;
@@ -419,10 +416,6 @@ 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,7 +44,6 @@
 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;
@@ -101,9 +100,7 @@ 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;
 
@@ -146,8 +143,6 @@ 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) {
@@ -237,13 +232,6 @@ 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,8 +74,11 @@
 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;
@@ -108,6 +111,7 @@
 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;
@@ -902,6 +906,48 @@ 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

@@ -52,6 +52,7 @@
 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;
@@ -154,6 +155,21 @@ 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());