Browse Source

Capture anonymous roles when creating API keys (#81427)

API keys capture the owner's roles at creation time. If anonymous access
is enabled, the roles should include roles granted by the anonymous
access. In extreme cases, the API key creation privilege may be granted
by the anonymous roles. Therefore these roles should be captured when
creating API keys (along with other roles the user might have).

Note this only applies to regular users, Not service accounts or API
keys (derived keys).

Resolves: #81024
Yang Wang 3 years ago
parent
commit
20e24c3db6

+ 54 - 0
x-pack/plugin/security/src/internalClusterTest/java/org/elasticsearch/xpack/security/user/AnonymousUserIntegTests.java

@@ -7,16 +7,30 @@
 package org.elasticsearch.xpack.security.user;
 
 import org.apache.http.util.EntityUtils;
+import org.elasticsearch.action.get.GetAction;
+import org.elasticsearch.action.get.GetRequest;
+import org.elasticsearch.action.get.GetResponse;
 import org.elasticsearch.client.Request;
 import org.elasticsearch.client.Response;
 import org.elasticsearch.client.ResponseException;
 import org.elasticsearch.common.settings.Settings;
 import org.elasticsearch.test.SecurityIntegTestCase;
+import org.elasticsearch.xpack.core.security.action.CreateApiKeyAction;
+import org.elasticsearch.xpack.core.security.action.CreateApiKeyRequest;
+import org.elasticsearch.xpack.core.security.action.CreateApiKeyResponse;
+import org.elasticsearch.xpack.core.security.action.service.CreateServiceAccountTokenAction;
+import org.elasticsearch.xpack.core.security.action.service.CreateServiceAccountTokenRequest;
+import org.elasticsearch.xpack.core.security.action.service.CreateServiceAccountTokenResponse;
 import org.elasticsearch.xpack.core.security.user.AnonymousUser;
 import org.elasticsearch.xpack.security.authz.AuthorizationService;
 
+import java.io.IOException;
+import java.util.Map;
+
 import static org.hamcrest.Matchers.containsString;
+import static org.hamcrest.Matchers.hasKey;
 import static org.hamcrest.Matchers.is;
+import static org.hamcrest.Matchers.not;
 import static org.hamcrest.Matchers.notNullValue;
 import static org.hamcrest.Matchers.nullValue;
 
@@ -61,4 +75,44 @@ public class AnonymousUserIntegTests extends SecurityIntegTestCase {
             }
         }
     }
+
+    public void testAnonymousRoleShouldBeCaptureWhenCreatingApiKey() throws IOException {
+        final CreateApiKeyResponse createApiKeyResponse = client().execute(
+            CreateApiKeyAction.INSTANCE,
+            new CreateApiKeyRequest(randomAlphaOfLength(8), null, null)
+        ).actionGet();
+
+        final Map<String, Object> apiKeyDocument = getApiKeyDocument(createApiKeyResponse.getId());
+
+        @SuppressWarnings("unchecked")
+        final Map<String, Object> limitedByRoleDescriptors = (Map<String, Object>) apiKeyDocument.get("limited_by_role_descriptors");
+        assertThat(limitedByRoleDescriptors, hasKey("anonymous"));
+    }
+
+    public void testAnonymousRoleShouldNotBeCapturedWhenCreatingApiKeyWithServiceAccount() {
+        final CreateServiceAccountTokenRequest createServiceAccountTokenRequest = new CreateServiceAccountTokenRequest(
+            "elastic",
+            "fleet-server",
+            randomAlphaOfLength(8)
+        );
+        final CreateServiceAccountTokenResponse createServiceAccountTokenResponse = client().execute(
+            CreateServiceAccountTokenAction.INSTANCE,
+            createServiceAccountTokenRequest
+        ).actionGet();
+
+        final CreateApiKeyResponse createApiKeyResponse = client().filterWithHeader(
+            Map.of("Authorization", "Bearer " + createServiceAccountTokenResponse.getValue())
+        ).execute(CreateApiKeyAction.INSTANCE, new CreateApiKeyRequest(randomAlphaOfLength(8), null, null)).actionGet();
+
+        final Map<String, Object> apiKeyDocument = getApiKeyDocument(createApiKeyResponse.getId());
+
+        @SuppressWarnings("unchecked")
+        final Map<String, Object> limitedByRoleDescriptors = (Map<String, Object>) apiKeyDocument.get("limited_by_role_descriptors");
+        assertThat(limitedByRoleDescriptors, not(hasKey("anonymous")));
+    }
+
+    private Map<String, Object> getApiKeyDocument(String apiKeyId) {
+        final GetResponse getResponse = client().execute(GetAction.INSTANCE, new GetRequest(".security-7", apiKeyId)).actionGet();
+        return getResponse.getSource();
+    }
 }

+ 14 - 21
x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authc/support/ApiKeyGenerator.java

@@ -14,16 +14,13 @@ import org.elasticsearch.xcontent.NamedXContentRegistry;
 import org.elasticsearch.xpack.core.security.action.CreateApiKeyRequest;
 import org.elasticsearch.xpack.core.security.action.CreateApiKeyResponse;
 import org.elasticsearch.xpack.core.security.authc.Authentication;
-import org.elasticsearch.xpack.core.security.authc.service.ServiceAccountSettings;
+import org.elasticsearch.xpack.core.security.authc.AuthenticationContext;
+import org.elasticsearch.xpack.core.security.authc.Subject;
 import org.elasticsearch.xpack.core.security.authz.RoleDescriptor;
 import org.elasticsearch.xpack.core.security.authz.support.DLSRoleQueryValidator;
 import org.elasticsearch.xpack.security.authc.ApiKeyService;
-import org.elasticsearch.xpack.security.authc.service.ServiceAccount;
-import org.elasticsearch.xpack.security.authc.service.ServiceAccountService;
 import org.elasticsearch.xpack.security.authz.store.CompositeRolesStore;
 
-import java.util.Arrays;
-import java.util.HashSet;
 import java.util.Set;
 
 public class ApiKeyGenerator {
@@ -57,22 +54,18 @@ public class ApiKeyGenerator {
             apiKeyService.createApiKey(authentication, request, roleDescriptors, listener);
         }, listener::onFailure);
 
-        if (ServiceAccountSettings.REALM_NAME.equals(authentication.getSourceRealm().getName())) {
-            final ServiceAccount serviceAccount = ServiceAccountService.getServiceAccounts().get(authentication.getUser().principal());
-            if (serviceAccount == null) {
-                roleDescriptorsListener.onFailure(
-                    new ElasticsearchSecurityException(
-                        "the authentication is created by a service account that does not exist: ["
-                            + authentication.getUser().principal()
-                            + "]"
-                    )
-                );
-            } else {
-                roleDescriptorsListener.onResponse(Set.of(serviceAccount.roleDescriptor()));
-            }
-        } else {
-            rolesStore.getRoleDescriptors(new HashSet<>(Arrays.asList(authentication.getUser().roles())), roleDescriptorsListener);
+        final Subject effectiveSubject = AuthenticationContext.fromAuthentication(authentication).getEffectiveSubject();
+
+        // Retain current behaviour that User of an API key authentication has no roles
+        if (effectiveSubject.getType() == Subject.Type.API_KEY) {
+            roleDescriptorsListener.onResponse(Set.of());
+            return;
         }
-    }
 
+        rolesStore.getRoleDescriptorsList(effectiveSubject, ActionListener.wrap(roleDescriptorsList -> {
+            assert roleDescriptorsList.size() == 1;
+            roleDescriptorsListener.onResponse(roleDescriptorsList.iterator().next());
+
+        }, roleDescriptorsListener::onFailure));
+    }
 }

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

@@ -10,7 +10,9 @@ import org.apache.logging.log4j.LogManager;
 import org.apache.logging.log4j.Logger;
 import org.apache.logging.log4j.message.ParameterizedMessage;
 import org.apache.lucene.util.automaton.Automaton;
+import org.elasticsearch.ElasticsearchException;
 import org.elasticsearch.action.ActionListener;
+import org.elasticsearch.action.support.GroupedActionListener;
 import org.elasticsearch.cluster.metadata.IndexNameExpressionResolver;
 import org.elasticsearch.common.Strings;
 import org.elasticsearch.common.bytes.BytesReference;
@@ -344,9 +346,22 @@ public class CompositeRolesStore {
         );
     }
 
-    // TODO: Temporary to fill the gap
-    public void getRoleDescriptors(Set<String> roleNames, ActionListener<Set<RoleDescriptor>> listener) {
-        roleReferenceResolver.getRoleDescriptors(roleNames, listener);
+    public void getRoleDescriptorsList(Subject subject, ActionListener<Collection<Set<RoleDescriptor>>> listener) {
+        final List<RoleReference> roleReferences = subject.getRoleReferenceIntersection(anonymousUser).getRoleReferences();
+        final GroupedActionListener<Set<RoleDescriptor>> groupedActionListener = new GroupedActionListener<>(
+            listener,
+            roleReferences.size()
+        );
+
+        roleReferences.forEach(roleReference -> {
+            roleReference.resolve(roleReferenceResolver, ActionListener.wrap(rolesRetrievalResult -> {
+                if (rolesRetrievalResult.isSuccess()) {
+                    groupedActionListener.onResponse(rolesRetrievalResult.getRoleDescriptors());
+                } else {
+                    groupedActionListener.onFailure(new ElasticsearchException("role retrieval had one or more failures"));
+                }
+            }, groupedActionListener::onFailure));
+        });
     }
 
     public static void buildRoleFromDescriptors(

+ 0 - 13
x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authz/store/RoleDescriptorStore.java

@@ -10,7 +10,6 @@ package org.elasticsearch.xpack.security.authz.store;
 import org.apache.logging.log4j.LogManager;
 import org.apache.logging.log4j.Logger;
 import org.apache.logging.log4j.message.ParameterizedMessage;
-import org.elasticsearch.ElasticsearchException;
 import org.elasticsearch.action.ActionListener;
 import org.elasticsearch.action.support.ContextPreservingActionListener;
 import org.elasticsearch.common.cache.Cache;
@@ -82,10 +81,8 @@ public class RoleDescriptorStore implements RoleReferenceResolver {
     ) {
         final Set<String> roleNames = Set.copyOf(new HashSet<>(List.of(namedRoleReference.getRoleNames())));
         if (roleNames.isEmpty()) {
-            assert false : "empty role names should have short circuited earlier";
             listener.onResponse(RolesRetrievalResult.EMPTY);
         } else if (roleNames.equals(Set.of(ReservedRolesStore.SUPERUSER_ROLE_DESCRIPTOR.getName()))) {
-            assert false : "superuser role should have short circuited earlier";
             listener.onResponse(RolesRetrievalResult.SUPERUSER);
         } else {
             resolveRoleNames(roleNames, listener);
@@ -172,16 +169,6 @@ public class RoleDescriptorStore implements RoleReferenceResolver {
         }, listener::onFailure));
     }
 
-    public void getRoleDescriptors(Set<String> roleNames, ActionListener<Set<RoleDescriptor>> listener) {
-        roleDescriptors(roleNames, ActionListener.wrap(rolesRetrievalResult -> {
-            if (rolesRetrievalResult.isSuccess()) {
-                listener.onResponse(rolesRetrievalResult.getRoleDescriptors());
-            } else {
-                listener.onFailure(new ElasticsearchException("role retrieval had one or more failures"));
-            }
-        }, listener::onFailure));
-    }
-
     private void roleDescriptors(Set<String> roleNames, ActionListener<RolesRetrievalResult> rolesResultListener) {
         final Set<String> filteredRoleNames = roleNames.stream().filter((s) -> {
             if (negativeLookupCache.get(s) != null) {

+ 11 - 5
x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authc/support/ApiKeyGeneratorTests.java

@@ -16,16 +16,20 @@ import org.elasticsearch.xcontent.NamedXContentRegistry;
 import org.elasticsearch.xpack.core.security.action.CreateApiKeyRequest;
 import org.elasticsearch.xpack.core.security.action.CreateApiKeyResponse;
 import org.elasticsearch.xpack.core.security.authc.Authentication;
+import org.elasticsearch.xpack.core.security.authc.Subject;
 import org.elasticsearch.xpack.core.security.authz.RoleDescriptor;
 import org.elasticsearch.xpack.core.security.user.User;
 import org.elasticsearch.xpack.security.authc.ApiKeyService;
 import org.elasticsearch.xpack.security.authz.store.CompositeRolesStore;
 
+import java.util.Collection;
+import java.util.List;
 import java.util.Set;
 import java.util.stream.Collectors;
 
 import static org.hamcrest.Matchers.arrayWithSize;
 import static org.hamcrest.Matchers.equalTo;
+import static org.hamcrest.Matchers.is;
 import static org.hamcrest.Matchers.sameInstance;
 import static org.mockito.ArgumentMatchers.any;
 import static org.mockito.ArgumentMatchers.anySet;
@@ -56,13 +60,15 @@ public class ApiKeyGeneratorTests extends ESTestCase {
             final Object[] args = inv.getArguments();
             assertThat(args, arrayWithSize(2));
 
-            Set<String> roleNames = (Set<String>) args[0];
-            assertThat(roleNames, equalTo(userRoleNames));
+            Subject subject = (Subject) args[0];
+            assertThat(subject.getType(), is(Subject.Type.USER));
+            assertThat(Set.of(subject.getUser().roles()), equalTo(userRoleNames));
 
-            ActionListener<Set<RoleDescriptor>> listener = (ActionListener<Set<RoleDescriptor>>) args[args.length - 1];
-            listener.onResponse(roleDescriptors);
+            ActionListener<Collection<Set<RoleDescriptor>>> listener = (ActionListener<Collection<Set<RoleDescriptor>>>) args[args.length
+                - 1];
+            listener.onResponse(List.of(roleDescriptors));
             return null;
-        }).when(rolesStore).getRoleDescriptors(anySet(), any(ActionListener.class));
+        }).when(rolesStore).getRoleDescriptorsList(any(Subject.class), any(ActionListener.class));
 
         CreateApiKeyResponse response = new CreateApiKeyResponse(
             "name",