Updatable API keys - auto-update legacy superuser RDs

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

@@ -82,6 +82,7 @@
 import org.elasticsearch.xpack.core.security.authc.file.FileRealmSettings;
 import org.elasticsearch.xpack.core.security.authz.RoleDescriptor;
 import org.elasticsearch.xpack.core.security.authz.privilege.ClusterPrivilegeResolver;
+import org.elasticsearch.xpack.core.security.authz.store.ReservedRolesStore;
 import org.elasticsearch.xpack.core.security.user.User;
 import org.elasticsearch.xpack.security.authz.RoleDescriptorTests;
 import org.elasticsearch.xpack.security.transport.filter.IPFilter;
@@ -1871,6 +1872,43 @@ public void testNoopUpdateApiKey() throws ExecutionException, InterruptedExcepti
         assertTrue(response.isUpdated());
     }
 
+    public void testUpdateApiKeyAutoUpdatesLegacySuperuserRoleDescriptor() throws ExecutionException, InterruptedException, IOException {
+        final Tuple<CreateApiKeyResponse, Map<String, Object>> createdApiKey = createApiKey(TEST_USER_NAME, null);
+        final var apiKeyId = createdApiKey.v1().getId();
+        final ServiceWithNodeName serviceWithNodeName = getServiceWithNodeName();
+        final Authentication authentication = Authentication.newRealmAuthentication(
+            new User(TEST_USER_NAME, TEST_ROLE),
+            new Authentication.RealmRef("file", "file", serviceWithNodeName.nodeName())
+        );
+        final Set<RoleDescriptor> legacySuperuserRoleDescriptor = Set.of(ApiKeyService.LEGACY_SUPERUSER_ROLE_DESCRIPTOR);
+        PlainActionFuture<UpdateApiKeyResponse> listener = new PlainActionFuture<>();
+        // Force set user role descriptors to 7.x legacy superuser role descriptors
+        serviceWithNodeName.service()
+            .updateApiKey(authentication, UpdateApiKeyRequest.usingApiKeyId(apiKeyId), legacySuperuserRoleDescriptor, listener);
+        UpdateApiKeyResponse response = listener.get();
+        assertNotNull(response);
+        assertTrue(response.isUpdated());
+        expectRoleDescriptorsForApiKey("limited_by_role_descriptors", legacySuperuserRoleDescriptor, getApiKeyDocument(apiKeyId));
+
+        final Set<RoleDescriptor> currentSuperuserRoleDescriptors = Set.of(ReservedRolesStore.SUPERUSER_ROLE_DESCRIPTOR);
+        PlainActionFuture<UpdateApiKeyResponse> listener2 = new PlainActionFuture<>();
+        serviceWithNodeName.service()
+            .updateApiKey(authentication, UpdateApiKeyRequest.usingApiKeyId(apiKeyId), currentSuperuserRoleDescriptors, listener2);
+        response = listener2.get();
+        assertNotNull(response);
+        // The first request is not a noop because we are auto-updating the legacy role descriptors to 8.x role descriptors
+        assertTrue(response.isUpdated());
+        expectRoleDescriptorsForApiKey("limited_by_role_descriptors", currentSuperuserRoleDescriptors, getApiKeyDocument(apiKeyId));
+
+        PlainActionFuture<UpdateApiKeyResponse> listener3 = new PlainActionFuture<>();
+        serviceWithNodeName.service()
+            .updateApiKey(authentication, UpdateApiKeyRequest.usingApiKeyId(apiKeyId), currentSuperuserRoleDescriptors, listener3);
+        response = listener3.get();
+        assertNotNull(response);
+        // Second update is noop because role descriptors were auto-updated by the previous request
+        assertFalse(response.isUpdated());
+    }
+
     public void testUpdateApiKeyClearsApiKeyDocCache() throws IOException, ExecutionException, InterruptedException {
         final List<ServiceWithNodeName> services = Arrays.stream(internalCluster().getNodeNames())
             .map(n -> new ServiceWithNodeName(internalCluster().getInstance(ApiKeyService.class, n), n))

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

@@ -119,7 +119,6 @@
 import java.util.Collection;
 import java.util.Collections;
 import java.util.HashMap;
-import java.util.HashSet;
 import java.util.List;
 import java.util.Map;
 import java.util.Objects;
@@ -445,6 +444,7 @@ static XContentBuilder newDocument(
     /**
      * @return `null` if the update is a noop, i.e., if no changes to `currentApiKeyDoc` are required
      */
+    @Nullable
     XContentBuilder maybeBuildUpdatedDocument(
         final ApiKeyDoc currentApiKeyDoc,
         final Version targetDocVersion,
@@ -560,26 +560,21 @@ private boolean isNoop(
             final List<RoleDescriptor> currentRoleDescriptors = parseRoleDescriptorsBytes(
                 request.getId(),
                 apiKeyDoc.roleDescriptorsBytes,
-                RoleReference.ApiKeyRoleType.ASSIGNED
+                false
             );
             if (false == (newRoleDescriptors.size() == currentRoleDescriptors.size()
-                && Set.copyOf(newRoleDescriptors).containsAll(new HashSet<>(currentRoleDescriptors)))) {
+                && Set.copyOf(newRoleDescriptors).containsAll(currentRoleDescriptors))) {
                 return false;
             }
         }
 
         assert userRoleDescriptors != null;
-        // There is an edge case here when we update an 7.x API key that has a `LEGACY_SUPERUSER_ROLE_DESCRIPTOR` role descriptor:
-        // `parseRoleDescriptorsBytes` automatically transforms it to `ReservedRolesStore.SUPERUSER_ROLE_DESCRIPTOR`. As such, when we
-        // perform the noop check on `ReservedRolesStore.SUPERUSER_ROLE_DESCRIPTOR` we will treat it as a noop even though the actual
-        // role descriptor bytes on the API key are different, and correspond to `LEGACY_SUPERUSER_ROLE_DESCRIPTOR`.
-        //
-        // This does *not* present a functional issue, since whenever a `LEGACY_SUPERUSER_ROLE_DESCRIPTOR` is loaded at authentication time,
-        // it is likewise automatically transformed to `ReservedRolesStore.SUPERUSER_ROLE_DESCRIPTOR`.
         final List<RoleDescriptor> currentLimitedByRoleDescriptors = parseRoleDescriptorsBytes(
             request.getId(),
             apiKeyDoc.limitedByRoleDescriptorsBytes,
-            RoleReference.ApiKeyRoleType.LIMITED_BY
+            // We want the 7.x `LEGACY_SUPERUSER_ROLE_DESCRIPTOR` role descriptor to be returned here to auto-update
+            // `LEGACY_SUPERUSER_ROLE_DESCRIPTOR` to `ReservedRolesStore.SUPERUSER_ROLE_DESCRIPTOR`, when we update a 7.x API key.
+            false
         );
         return (userRoleDescriptors.size() == currentLimitedByRoleDescriptors.size()
             && userRoleDescriptors.containsAll(currentLimitedByRoleDescriptors));
@@ -705,6 +700,14 @@ public List<RoleDescriptor> parseRoleDescriptorsBytes(
         final String apiKeyId,
         BytesReference bytesReference,
         RoleReference.ApiKeyRoleType roleType
+    ) {
+        return parseRoleDescriptorsBytes(apiKeyId, bytesReference, roleType == RoleReference.ApiKeyRoleType.LIMITED_BY);
+    }
+
+    private List<RoleDescriptor> parseRoleDescriptorsBytes(
+        final String apiKeyId,
+        BytesReference bytesReference,
+        final boolean replaceLegacySuperuserRoleDescriptor
     ) {
         if (bytesReference == null) {
             return Collections.emptyList();
@@ -728,9 +731,7 @@ public List<RoleDescriptor> parseRoleDescriptorsBytes(
         } catch (IOException e) {
             throw new UncheckedIOException(e);
         }
-        return roleType == RoleReference.ApiKeyRoleType.LIMITED_BY
-            ? maybeReplaceSuperuserRoleDescriptor(apiKeyId, roleDescriptors)
-            : roleDescriptors;
+        return replaceLegacySuperuserRoleDescriptor ? maybeReplaceSuperuserRoleDescriptor(apiKeyId, roleDescriptors) : roleDescriptors;
     }
 
     // package private for tests