User Profile - Detailed errors in hasPrivileges response

docs/changelog/89224.yaml

@@ -0,0 +1,5 @@
+pr: 89224
+summary: User Profile - Detailed errors in `hasPrivileges` response
+area: Security
+type: enhancement
+issues: []

x-pack/docs/en/rest-api/security/has-privileges-user-profile.asciidoc

@@ -68,14 +68,21 @@ Note that the `privileges` section above is identical to the
 ==== {api-response-body-title}
 
 A successful has privileges user profile API call returns a JSON structure that contains
-two list fields:
+two fields:
 
 `has_privilege_uids`:: (list) The subset of the requested profile IDs of the users that have
 **all** the requested privileges.
 
-`error_uids`:: (list) The subset of the requested profile IDs for which an error was
-encountered. It does **not** include the missing profile IDs or the profile IDs of
-the users that do not have all the requested privileges. This field is absent if empty.
+`errors`:: (object) Errors encountered while fulfilling the request. This field is absent if there is no error.
+It does **not** include the profile IDs of the users that do not have all the requested privileges.
++
+.Properties of objects in `errors`
+[%collapsible%open]
+====
+`count`:: (number) Total number of errors
+
+`details`:: (object) The detailed error report with keys being profile IDs and values being the exact errors.
+====
 
 [[security-api-has-privileges-user-profile-example]]
 ==== {api-examples-title}
@@ -87,7 +94,11 @@ requested set of cluster, index, and application privileges:
 --------------------------------------------------
 POST /_security/user/_has_privileges
 {
-  "uids": ["u_LQPnxDxEjIH0GOUoFkZr5Y57YUwSkL9Joiq-g4OCbPc_0", "u_rzRnxDgEHIH0GOUoFkZr5Y27YUwSk19Joiq=g4OCxxB_1"],
+  "uids": [
+    "u_LQPnxDxEjIH0GOUoFkZr5Y57YUwSkL9Joiq-g4OCbPc_0",
+    "u_rzRnxDgEHIH0GOUoFkZr5Y27YUwSk19Joiq=g4OCxxB_1",
+    "u_does-not-exist_0"
+  ],
   "cluster": [ "monitor", "create_snapshot", "manage_ml" ],
   "index" : [
     {
@@ -110,12 +121,22 @@ POST /_security/user/_has_privileges
 --------------------------------------------------
 // TEST[skip:TODO setup and tests will be possible once the profile uid is predictable]
 
-The following example output indicates that only one of the two users has all the privileges:
+The following example output indicates that only one of the three users has all the privileges
+and one of them is not found:
 
 [source,js]
 --------------------------------------------------
 {
-  "has_privilege_uids": ["u_rzRnxDgEHIH0GOUoFkZr5Y27YUwSk19Joiq=g4OCxxB_1"]
+  "has_privilege_uids": ["u_rzRnxDgEHIH0GOUoFkZr5Y27YUwSk19Joiq=g4OCxxB_1"],
+  "errors": {
+    "count": 1,
+    "details": {
+      "u_does-not-exist_0": {
+        "type": "resource_not_found_exception",
+        "reason": "profile document not found"
+      }
+    }
+  }
 }
 --------------------------------------------------
 // NOTCONSOLE

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

@@ -12,66 +12,68 @@
 import org.elasticsearch.common.io.stream.StreamOutput;
 import org.elasticsearch.xcontent.ToXContentObject;
 import org.elasticsearch.xcontent.XContentBuilder;
+import org.elasticsearch.xpack.core.security.xcontent.XContentUtils;
 
 import java.io.IOException;
+import java.util.Map;
 import java.util.Objects;
 import java.util.Set;
 
 public class ProfileHasPrivilegesResponse extends ActionResponse implements ToXContentObject {
 
     private Set<String> hasPrivilegeUids;
-    private Set<String> errorUids;
+    private final Map<String, Exception> errors;
 
     public ProfileHasPrivilegesResponse(StreamInput in) throws IOException {
         super(in);
         this.hasPrivilegeUids = in.readSet(StreamInput::readString);
-        this.errorUids = in.readSet(StreamInput::readString);
+        this.errors = in.readMap(StreamInput::readString, StreamInput::readException);
     }
 
-    public ProfileHasPrivilegesResponse(Set<String> hasPrivilegeUids, Set<String> errorUids) {
+    public ProfileHasPrivilegesResponse(Set<String> hasPrivilegeUids, Map<String, Exception> errors) {
         super();
         this.hasPrivilegeUids = Objects.requireNonNull(hasPrivilegeUids);
-        this.errorUids = Objects.requireNonNull(errorUids);
+        this.errors = Objects.requireNonNull(errors);
     }
 
     public Set<String> hasPrivilegeUids() {
         return hasPrivilegeUids;
     }
 
-    public Set<String> errorUids() {
-        return errorUids;
+    public Map<String, Exception> errors() {
+        return errors;
     }
 
     @Override
     public boolean equals(Object o) {
         if (this == o) return true;
         if (o == null || getClass() != o.getClass()) return false;
         ProfileHasPrivilegesResponse that = (ProfileHasPrivilegesResponse) o;
-        return hasPrivilegeUids.equals(that.hasPrivilegeUids) && errorUids.equals(that.errorUids);
+        // Only compare the keys (profile uids) of the errors, actual error types do not matter
+        return hasPrivilegeUids.equals(that.hasPrivilegeUids) && errors.keySet().equals(that.errors.keySet());
     }
 
     @Override
     public int hashCode() {
-        return Objects.hash(hasPrivilegeUids, errorUids);
+        // Only include the keys (profile uids) of the errors, actual error types do not matter
+        return Objects.hash(hasPrivilegeUids, errors.keySet());
     }
 
     @Override
     public XContentBuilder toXContent(XContentBuilder builder, Params params) throws IOException {
         XContentBuilder xContentBuilder = builder.startObject().stringListField("has_privilege_uids", hasPrivilegeUids);
-        if (false == errorUids.isEmpty()) {
-            xContentBuilder.stringListField("error_uids", errorUids);
-        }
+        XContentUtils.maybeAddErrorDetails(builder, errors);
         return xContentBuilder.endObject();
     }
 
     @Override
     public void writeTo(StreamOutput out) throws IOException {
         out.writeStringCollection(hasPrivilegeUids);
-        out.writeStringCollection(errorUids);
+        out.writeMap(errors, StreamOutput::writeString, StreamOutput::writeException);
     }
 
     @Override
     public String toString() {
-        return getClass().getSimpleName() + "{" + "has_privilege_uids=" + hasPrivilegeUids + ", error_uids=" + errorUids + "}";
+        return getClass().getSimpleName() + "{" + "has_privilege_uids=" + hasPrivilegeUids + ", errors=" + errors + "}";
     }
 }

x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/security/action/profile/ProfileHasPrivilegesResponseTests.java

@@ -7,22 +7,30 @@
 
 package org.elasticsearch.xpack.core.security.action.profile;
 
+import org.elasticsearch.ElasticsearchException;
+import org.elasticsearch.ResourceNotFoundException;
 import org.elasticsearch.common.bytes.BytesReference;
 import org.elasticsearch.common.io.stream.Writeable;
 import org.elasticsearch.common.xcontent.XContentHelper;
 import org.elasticsearch.test.AbstractWireSerializingTestCase;
 import org.elasticsearch.xcontent.ToXContent;
 import org.elasticsearch.xcontent.XContentBuilder;
 import org.elasticsearch.xcontent.XContentFactory;
+import org.elasticsearch.xcontent.json.JsonXContent;
 import org.elasticsearch.xpack.core.security.action.user.ProfileHasPrivilegesResponse;
 
 import java.io.IOException;
 import java.util.ArrayList;
 import java.util.HashSet;
+import java.util.List;
 import java.util.Map;
 import java.util.Set;
+import java.util.TreeMap;
+import java.util.function.Supplier;
+import java.util.stream.IntStream;
 
 import static org.hamcrest.Matchers.equalTo;
+import static org.hamcrest.Matchers.hasEntry;
 
 public class ProfileHasPrivilegesResponseTests extends AbstractWireSerializingTestCase<ProfileHasPrivilegesResponse> {
 
@@ -35,16 +43,19 @@ protected Writeable.Reader<ProfileHasPrivilegesResponse> instanceReader() {
     protected ProfileHasPrivilegesResponse createTestInstance() {
         return new ProfileHasPrivilegesResponse(
             randomUnique(() -> randomAlphaOfLengthBetween(0, 5), randomIntBetween(0, 5)),
-            randomUnique(() -> randomAlphaOfLengthBetween(0, 5), randomIntBetween(0, 5))
+            randomErrors()
         );
     }
 
     @Override
     protected ProfileHasPrivilegesResponse mutateInstance(ProfileHasPrivilegesResponse instance) throws IOException {
         return randomFrom(
-            new ProfileHasPrivilegesResponse(newMutatedSet(instance.hasPrivilegeUids()), instance.errorUids()),
-            new ProfileHasPrivilegesResponse(instance.hasPrivilegeUids(), newMutatedSet(instance.errorUids())),
-            new ProfileHasPrivilegesResponse(newMutatedSet(instance.hasPrivilegeUids()), newMutatedSet(instance.errorUids()))
+            new ProfileHasPrivilegesResponse(newMutatedSet(instance.hasPrivilegeUids()), instance.errors()),
+            new ProfileHasPrivilegesResponse(instance.hasPrivilegeUids(), randomValueOtherThan(instance.errors(), this::randomErrors)),
+            new ProfileHasPrivilegesResponse(
+                newMutatedSet(instance.hasPrivilegeUids()),
+                randomValueOtherThan(instance.errors(), this::randomErrors)
+            )
         );
     }
 
@@ -55,20 +66,47 @@ public void testToXContent() throws IOException {
         final Map<String, Object> responseMap = XContentHelper.convertToMap(BytesReference.bytes(builder), false, builder.contentType())
             .v2();
 
-        if (response.errorUids().isEmpty()) {
+        if (response.errors().isEmpty()) {
             assertThat(responseMap, equalTo(Map.of("has_privilege_uids", new ArrayList<>(response.hasPrivilegeUids()))));
         } else {
-            assertThat(
-                responseMap,
-                equalTo(
-                    Map.of(
-                        "has_privilege_uids",
-                        new ArrayList<>(response.hasPrivilegeUids()),
-                        "error_uids",
-                        new ArrayList<>(response.errorUids())
-                    )
-                )
-            );
+            assertThat(responseMap, hasEntry("has_privilege_uids", List.copyOf(response.hasPrivilegeUids())));
+            @SuppressWarnings("unchecked")
+            final Map<String, Object> errorsMap = (Map<String, Object>) responseMap.get("errors");
+            assertThat(errorsMap.get("count"), equalTo(response.errors().size()));
+            @SuppressWarnings("unchecked")
+            final Map<String, Object> detailsMap = (Map<String, Object>) errorsMap.get("details");
+            assertThat(detailsMap.keySet(), equalTo(response.errors().keySet()));
+
+            detailsMap.forEach((k, v) -> {
+                final String errorString;
+                final Exception e = response.errors().get(k);
+                if (e instanceof IllegalArgumentException illegalArgumentException) {
+                    errorString = """
+                        {
+                          "type": "illegal_argument_exception",
+                          "reason": "%s"
+                        }""".formatted(illegalArgumentException.getMessage());
+                } else if (e instanceof ResourceNotFoundException resourceNotFoundException) {
+                    errorString = """
+                        {
+                          "type": "resource_not_found_exception",
+                          "reason": "%s"
+                        }""".formatted(resourceNotFoundException.getMessage());
+                } else if (e instanceof ElasticsearchException elasticsearchException) {
+                    errorString = """
+                        {
+                          "type": "exception",
+                          "reason": "%s",
+                          "caused_by": {
+                            "type": "illegal_argument_exception",
+                            "reason": "%s"
+                          }
+                        }""".formatted(elasticsearchException.getMessage(), elasticsearchException.getCause().getMessage());
+                } else {
+                    throw new IllegalArgumentException("unknown exception type: " + e);
+                }
+                assertThat(v, equalTo(XContentHelper.convertToMap(JsonXContent.jsonXContent, errorString, false)));
+            });
         }
     }
 
@@ -86,4 +124,15 @@ private Set<String> newMutatedSet(Set<String> in) {
         }
         return mutated;
     }
+
+    private Map<String, Exception> randomErrors() {
+        final Map<String, Exception> errors = new TreeMap<>();
+        final Supplier<Exception> randomExceptionSupplier = () -> randomFrom(
+            new IllegalArgumentException(randomAlphaOfLengthBetween(0, 18)),
+            new ResourceNotFoundException(randomAlphaOfLengthBetween(0, 18)),
+            new ElasticsearchException(randomAlphaOfLengthBetween(0, 18), new IllegalArgumentException(randomAlphaOfLengthBetween(0, 18)))
+        );
+        IntStream.range(0, randomIntBetween(0, 3)).forEach(i -> errors.put(randomAlphaOfLength(20) + i, randomExceptionSupplier.get()));
+        return errors;
+    }
 }

x-pack/plugin/security/qa/profile/src/javaRestTest/java/org/elasticsearch/xpack/security/profile/ProfileIT.java

@@ -115,8 +115,19 @@ public void testProfileHasPrivileges() throws IOException {
         final Response profileHasPrivilegesResponse = adminClient().performRequest(profileHasPrivilegesRequest);
         assertOK(profileHasPrivilegesResponse);
         Map<String, Object> profileHasPrivilegesResponseMap = responseAsMap(profileHasPrivilegesResponse);
-        assertThat(profileHasPrivilegesResponseMap.keySet(), contains("has_privilege_uids"));
+        assertThat(profileHasPrivilegesResponseMap.keySet(), contains("has_privilege_uids", "errors"));
         assertThat(((List<String>) profileHasPrivilegesResponseMap.get("has_privilege_uids")), contains(profileUid));
+        assertThat(
+            profileHasPrivilegesResponseMap.get("errors"),
+            equalTo(
+                Map.of(
+                    "count",
+                    1,
+                    "details",
+                    Map.of("some_missing_profile", Map.of("type", "resource_not_found_exception", "reason", "profile document not found"))
+                )
+            )
+        );
     }
 
     public void testGetProfiles() throws IOException {

x-pack/plugin/security/src/internalClusterTest/java/org/elasticsearch/xpack/security/profile/ProfileIntegTests.java

@@ -586,7 +586,7 @@ public void testProfileAPIsWhenIndexNotCreated() {
             )
         ).actionGet();
         assertThat(profileHasPrivilegesResponse.hasPrivilegeUids(), emptyIterable());
-        assertThat(profileHasPrivilegesResponse.errorUids(), emptyIterable());
+        assertThat(profileHasPrivilegesResponse.errors(), anEmptyMap());
 
         // Ensure index does not exist
         assertThat(getProfileIndexResponse().getIndices(), not(hasItemInArray(INTERNAL_SECURITY_PROFILE_INDEX_8)));
@@ -650,7 +650,7 @@ public void testSetEnabled() {
             )
         ).actionGet();
         assertThat(profileHasPrivilegesResponse.hasPrivilegeUids(), emptyIterable());
-        assertThat(profileHasPrivilegesResponse.errorUids(), emptyIterable());
+        assertThat(profileHasPrivilegesResponse.errors(), anEmptyMap());
 
         // Enable again for search
         final SetProfileEnabledRequest setProfileEnabledRequest2 = new SetProfileEnabledRequest(

x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/action/profile/TransportProfileHasPrivilegesAction.java

@@ -36,6 +36,7 @@
 import java.util.HashSet;
 import java.util.Map;
 import java.util.Set;
+import java.util.concurrent.ConcurrentHashMap;
 import java.util.concurrent.atomic.AtomicInteger;
 import java.util.stream.Collectors;
 
@@ -75,20 +76,18 @@ public TransportProfileHasPrivilegesAction(
     protected void doExecute(Task task, ProfileHasPrivilegesRequest request, ActionListener<ProfileHasPrivilegesResponse> listener) {
         assert task instanceof CancellableTask : "task must be cancellable";
         profileService.getProfileSubjects(request.profileUids(), ActionListener.wrap(profileSubjectsAndFailures -> {
-            if (profileSubjectsAndFailures.profileUidToSubject().isEmpty()) {
-                listener.onResponse(new ProfileHasPrivilegesResponse(Set.of(), profileSubjectsAndFailures.failureProfileUids()));
+            if (profileSubjectsAndFailures.results().isEmpty()) {
+                listener.onResponse(new ProfileHasPrivilegesResponse(Set.of(), profileSubjectsAndFailures.errors()));
                 return;
             }
             final Set<String> hasPrivilegeProfiles = Collections.synchronizedSet(new HashSet<>());
-            final Set<String> errorProfiles = Collections.synchronizedSet(new HashSet<>(profileSubjectsAndFailures.failureProfileUids()));
-            final Collection<Map.Entry<String, Subject>> profileUidAndSubjects = profileSubjectsAndFailures.profileUidToSubject()
-                .entrySet();
-            final AtomicInteger counter = new AtomicInteger(profileUidAndSubjects.size());
+            final Map<String, Exception> errorProfiles = new ConcurrentHashMap<>(profileSubjectsAndFailures.errors());
+            final AtomicInteger counter = new AtomicInteger(profileSubjectsAndFailures.results().size());
             assert counter.get() > 0;
             resolveApplicationPrivileges(
                 request,
                 ActionListener.wrap(applicationPrivilegeDescriptors -> threadPool.generic().execute(() -> {
-                    for (Map.Entry<String, Subject> profileUidToSubject : profileUidAndSubjects) {
+                    for (Map.Entry<String, Subject> profileUidToSubject : profileSubjectsAndFailures.results()) {
                         // return the partial response if the "has privilege" task got cancelled in the meantime
                         if (((CancellableTask) task).isCancelled()) {
                             listener.onFailure(new TaskCancelledException("has privilege task cancelled"));
@@ -107,7 +106,7 @@ protected void doExecute(Task task, ProfileHasPrivilegesRequest request, ActionL
                                 }
                             }, checkPrivilegesException -> {
                                 logger.debug(() -> "Failed to check privileges for profile [" + profileUid + "]", checkPrivilegesException);
-                                errorProfiles.add(profileUid);
+                                errorProfiles.put(profileUid, checkPrivilegesException);
                             }), () -> {
                                 if (counter.decrementAndGet() == 0) {
                                     listener.onResponse(new ProfileHasPrivilegesResponse(hasPrivilegeProfiles, errorProfiles));