Fix cache invalidation on privilege modification (#102193)

Previously the application privilege cache would only be updated for
privileges that were _created_. If an existing privilge was modified,
then that would not automatically invalidate the cache.

This was because the NativePrivilegeStore.putPrivileges had a confusing
response type that replied with the set of privileges that were created,
and left the caller to assume that any other privileges in the request
were updated.

This commit changes the response type of that method to return all
privileges that were included in the request, along with their Result

Relates: #102056

docs/changelog/102193.yaml

@@ -0,0 +1,5 @@
+pr: 102193
+summary: Fix cache invalidation on privilege modification
+area: Authorization
+type: bug
+issues: []

x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/action/privilege/TransportPutPrivilegesAction.java

@@ -7,9 +7,11 @@
 package org.elasticsearch.xpack.security.action.privilege;
 
 import org.elasticsearch.action.ActionListener;
+import org.elasticsearch.action.DocWriteResponse;
 import org.elasticsearch.action.support.ActionFilters;
 import org.elasticsearch.action.support.HandledTransportAction;
 import org.elasticsearch.common.inject.Inject;
+import org.elasticsearch.common.util.Maps;
 import org.elasticsearch.common.util.concurrent.EsExecutors;
 import org.elasticsearch.tasks.Task;
 import org.elasticsearch.transport.TransportService;
@@ -19,6 +21,8 @@
 import org.elasticsearch.xpack.security.authz.store.NativePrivilegeStore;
 
 import java.util.Collections;
+import java.util.List;
+import java.util.Map;
 
 /**
  * Transport action to retrieve one or more application privileges from the security index
@@ -45,8 +49,23 @@ protected void doExecute(Task task, final PutPrivilegesRequest request, final Ac
             this.privilegeStore.putPrivileges(
                 request.getPrivileges(),
                 request.getRefreshPolicy(),
-                ActionListener.wrap(created -> listener.onResponse(new PutPrivilegesResponse(created)), listener::onFailure)
+                ActionListener.wrap(result -> listener.onResponse(buildResponse(result)), listener::onFailure)
             );
         }
     }
+
+    private static PutPrivilegesResponse buildResponse(Map<String, Map<String, DocWriteResponse.Result>> result) {
+        final Map<String, List<String>> createdPrivilegesByApplicationName = Maps.newHashMapWithExpectedSize(result.size());
+        result.forEach((appName, privileges) -> {
+            List<String> createdPrivileges = privileges.entrySet()
+                .stream()
+                .filter(e -> e.getValue() == DocWriteResponse.Result.CREATED)
+                .map(e -> e.getKey())
+                .toList();
+            if (createdPrivileges.isEmpty() == false) {
+                createdPrivilegesByApplicationName.put(appName, createdPrivileges);
+            }
+        });
+        return new PutPrivilegesResponse(createdPrivilegesByApplicationName);
+    }
 }

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

@@ -370,7 +370,7 @@ protected void cacheFetchedDescriptors(
     public void putPrivileges(
         Collection<ApplicationPrivilegeDescriptor> privileges,
         WriteRequest.RefreshPolicy refreshPolicy,
-        ActionListener<Map<String, List<String>>> listener
+        ActionListener<Map<String, Map<String, DocWriteResponse.Result>>> listener
     ) {
         if (privileges.isEmpty()) {
             listener.onResponse(Map.of());
@@ -416,9 +416,9 @@ private IndexRequest preparePutPrivilege(ApplicationPrivilegeDescriptor privileg
         }
     }
 
-    private void handleBulkResponse(BulkResponse bulkResponse, ActionListener<Map<String, List<String>>> listener) {
+    private void handleBulkResponse(BulkResponse bulkResponse, ActionListener<Map<String, Map<String, DocWriteResponse.Result>>> listener) {
         ElasticsearchException failure = null;
-        final Map<String, List<String>> createdPrivilegesByAppName = new HashMap<>();
+        final Map<String, Map<String, DocWriteResponse.Result>> privilegeResultByAppName = new HashMap<>();
         for (var item : bulkResponse.getItems()) {
             if (item.isFailed()) {
                 if (failure == null) {
@@ -427,24 +427,22 @@ private void handleBulkResponse(BulkResponse bulkResponse, ActionListener<Map<St
                     failure.addSuppressed(item.getFailure().getCause());
                 }
             } else {
-                if (item.getResponse().getResult() == DocWriteResponse.Result.CREATED) {
-                    final Tuple<String, String> name = nameFromDocId(item.getId());
-                    final String appName = name.v1();
-                    final String privilegeName = name.v2();
-
-                    List<String> createdPrivileges = createdPrivilegesByAppName.get(appName);
-                    if (createdPrivileges == null) {
-                        createdPrivileges = new ArrayList<>();
-                        createdPrivilegesByAppName.put(appName, createdPrivileges);
-                    }
-                    createdPrivileges.add(privilegeName);
+                final Tuple<String, String> name = nameFromDocId(item.getId());
+                final String appName = name.v1();
+                final String privilegeName = name.v2();
+
+                var privileges = privilegeResultByAppName.get(appName);
+                if (privileges == null) {
+                    privileges = new HashMap<>();
+                    privilegeResultByAppName.put(appName, privileges);
                 }
+                privileges.put(privilegeName, item.getResponse().getResult());
             }
         }
         if (failure != null) {
             listener.onFailure(failure);
         } else {
-            clearCaches(listener, createdPrivilegesByAppName.keySet(), createdPrivilegesByAppName);
+            clearCaches(listener, privilegeResultByAppName.keySet(), privilegeResultByAppName);
         }
     }