User Profile - GetProfile API nows supports multiple UIDs (#89023)

This PR expands the existing GetProfile API to support getting multiple profiles by IDs. As a result, the response format is also changed to align with the latest version of API design guideline. Concretely, this means moving the profiles as an array inside a top level "profiles" field so that (1) does not mix dynamic fields (uid) with static fields and (2) enforcing an order in the response which is desirable for clients. The change also reports any error encounter in the retrieving process in a top level "errors" field. Relates: #81910
elastic · Aug 10, 2022 · d663231 · d663231
1 parent 895baf0
commit d663231
Show file tree

Hide file tree

Showing 26 changed files with 951 additions and 366 deletions.
diff --git a/docs/changelog/89023.yaml b/docs/changelog/89023.yaml
@@ -0,0 +1,5 @@
+pr: 89023
+summary: User Profile - `GetProfile` API nows supports multiple UIDs
+area: Security
+type: enhancement
+issues: []
diff --git a/rest-api-spec/src/main/resources/rest-api-spec/api/security.get_user_profile.json b/rest-api-spec/src/main/resources/rest-api-spec/api/security.get_user_profile.json
@@ -2,7 +2,7 @@
   "security.get_user_profile":{
     "documentation":{
       "url":"https://www.elastic.co/guide/en/elasticsearch/reference/current/security-api-get-user-profile.html",
-      "description":"Retrieves user profile for the given unique ID."
+      "description":"Retrieves user profiles for the given unique ID(s)."
     },
     "stability":"experimental",
     "visibility":"private",
@@ -18,8 +18,8 @@
           ],
           "parts":{
             "uid":{
-              "type":"string",
-              "description":"An unique identifier of the user profile"
+              "type":"list",
+              "description":"A comma-separated list of unique identifier for user profiles"
             }
           }
         }

diff --git a/x-pack/docs/en/rest-api/security/get-user-profile.asciidoc b/x-pack/docs/en/rest-api/security/get-user-profile.asciidoc
@@ -1,13 +1,13 @@
 [role="xpack"]
 [[security-api-get-user-profile]]
-=== Get user profile API
+=== Get user profiles API
 ++++
-<titleabbrev>Get user profile</titleabbrev>
+<titleabbrev>Get user profiles</titleabbrev>
 ++++
 
 beta::[]
 
-Retrieves a user's profile using the unique profile ID.
+Retrieves user profiles using a list of unique profile ID.
 
 [[security-api-get-user-profile-request]]
 ==== {api-request-title}
@@ -31,7 +31,8 @@ The get user profile API returns the user profile document matching a specified
 ==== {api-path-parms-title}
 
 `uid`::
-(Required, string) A unique identifier for the user profile.
+(Required, string) The unique identifier for the user profile. You can specify multiple IDs as
+a comma-separated list.
 
 [[security-api-get-user-profile-query-params]]
 ==== {api-query-parms-title}
@@ -65,33 +66,35 @@ The API returns the following response for a `uid` matching `u_79HkWkwmnBH5gqFKw
 [source,console-result]
 ----
 {
-  "u_79HkWkwmnBH5gqFKwoxggWPjEBOur1zLPXQPEl1VBW0_0": {
-    "uid": "u_79HkWkwmnBH5gqFKwoxggWPjEBOur1zLPXQPEl1VBW0_0",
-    "enabled": true,
-    "last_synchronized": 1642650651037,
-    "user": {
-      "username": "jacknich",
-      "roles": [
-        "admin", "other_role1"
-      ],
-      "realm_name": "native",
-      "full_name": "Jack Nicholson",
-      "email": "jacknich@example.com"
-    },
-    "labels": {
-      "direction": "north"
-    },
-    "data": {}, <1>
-    "_doc": {
-      "_primary_term": 88,
-      "_seq_no": 66
+  "profiles": [
+    {
+      "uid": "u_79HkWkwmnBH5gqFKwoxggWPjEBOur1zLPXQPEl1VBW0_0",
+      "enabled": true,
+      "last_synchronized": 1642650651037,
+      "user": {
+        "username": "jacknich",
+        "roles": [
+          "admin", "other_role1"
+        ],
+        "realm_name": "native",
+        "full_name": "Jack Nicholson",
+        "email": "jacknich@example.com"
+      },
+      "labels": {
+        "direction": "north"
+      },
+      "data": {}, <1>
+      "_doc": {
+        "_primary_term": 88,
+        "_seq_no": 66
+      }
     }
-  }
+  ]
 }
 ----
-// TESTRESPONSE[s/1642650651037/$body.u_79HkWkwmnBH5gqFKwoxggWPjEBOur1zLPXQPEl1VBW0_0.last_synchronized/]
-// TESTRESPONSE[s/88/$body.u_79HkWkwmnBH5gqFKwoxggWPjEBOur1zLPXQPEl1VBW0_0._doc._primary_term/]
-// TESTRESPONSE[s/66/$body.u_79HkWkwmnBH5gqFKwoxggWPjEBOur1zLPXQPEl1VBW0_0._doc._seq_no/]
+// TESTRESPONSE[s/1642650651037/$body.profiles.0.last_synchronized/]
+// TESTRESPONSE[s/88/$body.profiles.0._doc._primary_term/]
+// TESTRESPONSE[s/66/$body.profiles.0._doc._seq_no/]
 
 <1> No content is returned in the `data` field by default.
 
@@ -107,35 +110,55 @@ GET /_security/profile/u_79HkWkwmnBH5gqFKwoxggWPjEBOur1zLPXQPEl1VBW0_0?data=app1
 [source,console-result]
 ----
 {
-  "u_79HkWkwmnBH5gqFKwoxggWPjEBOur1zLPXQPEl1VBW0_0": {
-    "uid": "u_79HkWkwmnBH5gqFKwoxggWPjEBOur1zLPXQPEl1VBW0_0",
-    "enabled": true,
-    "last_synchronized": 1642650651037,
-    "user": {
-      "username": "jacknich",
-      "roles": [
-        "admin", "other_role1"
-      ],
-      "realm_name": "native",
-      "full_name": "Jack Nicholson",
-      "email": "jacknich@example.com"
-    },
-    "labels": {
-      "direction": "north"
-    },
-    "data": {
-      "app1": {
-        "key1": "value1"
+  "profiles": [
+    {
+      "uid": "u_79HkWkwmnBH5gqFKwoxggWPjEBOur1zLPXQPEl1VBW0_0",
+      "enabled": true,
+      "last_synchronized": 1642650651037,
+      "user": {
+        "username": "jacknich",
+        "roles": [
+          "admin", "other_role1"
+        ],
+        "realm_name": "native",
+        "full_name": "Jack Nicholson",
+        "email": "jacknich@example.com"
+      },
+      "labels": {
+        "direction": "north"
+      },
+      "data": {
+        "app1": {
+          "key1": "value1"
+        }
+      },
+      "_doc": {
+        "_primary_term": 88,
+        "_seq_no": 66
       }
-    },
-    "_doc": {
-      "_primary_term": 88,
-      "_seq_no": 66
     }
-  }
+  ]
 }
 ----
-// TESTRESPONSE[s/1642650651037/$body.u_79HkWkwmnBH5gqFKwoxggWPjEBOur1zLPXQPEl1VBW0_0.last_synchronized/]
-// TESTRESPONSE[s/88/$body.u_79HkWkwmnBH5gqFKwoxggWPjEBOur1zLPXQPEl1VBW0_0._doc._primary_term/]
-// TESTRESPONSE[s/66/$body.u_79HkWkwmnBH5gqFKwoxggWPjEBOur1zLPXQPEl1VBW0_0._doc._seq_no/]
+// TESTRESPONSE[s/1642650651037/$body.profiles.0.last_synchronized/]
+// TESTRESPONSE[s/88/$body.profiles.0._doc._primary_term/]
+// TESTRESPONSE[s/66/$body.profiles.0._doc._seq_no/]
+
+If there has been any errors when retrieving the user profiles, they are returned in the `errors` field:
 
+[source,js]
+--------------------------------------------------
+{
+  "profiles": [],
+  "errors": {
+    "count": 1,
+    "details": {
+       "u_FmxQt3gr1BBH5wpnz9HkouPj3Q710XkOgg1PWkwLPBW_5": {
+         "type": "resource_not_found_exception",
+         "reason": "profile document not found"
+       }
+    }
+  }
+}
+--------------------------------------------------
+// NOTCONSOLE
diff --git a/x-pack/docs/en/rest-api/security/update-user-profile-data.asciidoc b/x-pack/docs/en/rest-api/security/update-user-profile-data.asciidoc
@@ -137,35 +137,37 @@ GET /_security/profile/u_P_0BMHgaOK3p7k-PFWUCbw9dQ-UFjt01oWJ_Dp2PmPc_0?data=*
 [source,console-result]
 ----
 {
-  "u_P_0BMHgaOK3p7k-PFWUCbw9dQ-UFjt01oWJ_Dp2PmPc_0": {
-    "uid": "u_P_0BMHgaOK3p7k-PFWUCbw9dQ-UFjt01oWJ_Dp2PmPc_0",
-    "enabled": true,
-    "last_synchronized": 1642650651037,
-    "user": {
-      "username": "jackrea",
-      "roles": [
-        "admin"
-      ],
-      "realm_name": "native",
-      "full_name": "Jack Reacher",
-      "email": "jackrea@example.com"
-    },
-    "labels": {
-      "direction": "west"
-    },
-    "data": {
-      "app1": {
-        "theme": "default",
-        "font": "large"
+  "profiles": [
+    {
+      "uid": "u_P_0BMHgaOK3p7k-PFWUCbw9dQ-UFjt01oWJ_Dp2PmPc_0",
+      "enabled": true,
+      "last_synchronized": 1642650651037,
+      "user": {
+        "username": "jackrea",
+        "roles": [
+          "admin"
+        ],
+        "realm_name": "native",
+        "full_name": "Jack Reacher",
+        "email": "jackrea@example.com"
+      },
+      "labels": {
+        "direction": "west"
+      },
+      "data": {
+        "app1": {
+          "theme": "default",
+          "font": "large"
+        }
+      },
+      "_doc": {
+        "_primary_term": 88,
+        "_seq_no": 66
       }
-    },
-    "_doc": {
-      "_primary_term": 88,
-      "_seq_no": 66
     }
-  }
+  ]
 }
 ----
-// TESTRESPONSE[s/1642650651037/$body.u_P_0BMHgaOK3p7k-PFWUCbw9dQ-UFjt01oWJ_Dp2PmPc_0.last_synchronized/]
-// TESTRESPONSE[s/88/$body.u_P_0BMHgaOK3p7k-PFWUCbw9dQ-UFjt01oWJ_Dp2PmPc_0._doc._primary_term/]
-// TESTRESPONSE[s/66/$body.u_P_0BMHgaOK3p7k-PFWUCbw9dQ-UFjt01oWJ_Dp2PmPc_0._doc._seq_no/]
+// TESTRESPONSE[s/1642650651037/$body.profiles.0.last_synchronized/]
+// TESTRESPONSE[s/88/$body.profiles.0._doc._primary_term/]
+// TESTRESPONSE[s/66/$body.profiles.0._doc._seq_no/]
diff --git a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/common/ResultsAndErrors.java b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/common/ResultsAndErrors.java
@@ -0,0 +1,33 @@
+/*
+ * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
+ * or more contributor license agreements. Licensed under the Elastic License
+ * 2.0; you may not use this file except in compliance with the Elastic License
+ * 2.0.
+ */
+
+package org.elasticsearch.xpack.core.common;
+
+import java.util.Collection;
+import java.util.List;
+import java.util.Map;
+
+/**
+ * A record class encapsulate a collection of results and associated errors. An intended usage is to model the
+ * generic MultiGetResponse to domain specific ones. The results are a collection of entity objects translated
+ * from the documents retrieved by MultiGet and the errors are a map key by IDs and any exception encountered
+ * when attempt retrieving associated documents.
+ */
+public record ResultsAndErrors<T> (Collection<T> results, Map<String, Exception> errors) {
+
+    private static final ResultsAndErrors<?> EMPTY = new ResultsAndErrors<>(List.of(), Map.of());
+
+    public boolean isEmpty() {
+        return results.isEmpty() && errors.isEmpty();
+    }
+
+    @SuppressWarnings("unchecked")
+    public static <T> ResultsAndErrors<T> empty() {
+        return (ResultsAndErrors<T>) EMPTY;
+    }
+
+}
diff --git a/...in/java/org/elasticsearch/xpack/core/security/action/apikey/BulkUpdateApiKeyResponse.java b/...in/java/org/elasticsearch/xpack/core/security/action/apikey/BulkUpdateApiKeyResponse.java
@@ -7,13 +7,13 @@
 
 package org.elasticsearch.xpack.core.security.action.apikey;
 
-import org.elasticsearch.ElasticsearchException;
 import org.elasticsearch.action.ActionResponse;
 import org.elasticsearch.common.io.stream.StreamInput;
 import org.elasticsearch.common.io.stream.StreamOutput;
 import org.elasticsearch.common.io.stream.Writeable;
 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.ArrayList;
@@ -59,20 +59,7 @@ public int getTotalResultCount() {
     @Override
     public XContentBuilder toXContent(XContentBuilder builder, Params params) throws IOException {
         builder.startObject().stringListField("updated", updated).stringListField("noops", noops);
-        if (errorDetails.isEmpty() == false) {
-            builder.startObject("errors");
-            {
-                builder.field("count", errorDetails.size());
-                builder.startObject("details");
-                for (Map.Entry<String, Exception> idWithException : errorDetails.entrySet()) {
-                    builder.startObject(idWithException.getKey());
-                    ElasticsearchException.generateThrowableXContent(builder, params, idWithException.getValue());
-                    builder.endObject();
-                }
-                builder.endObject();
-            }
-            builder.endObject();
-        }
+        XContentUtils.maybeAddErrorDetails(builder, errorDetails);
         return builder.endObject();
     }
 

diff --git a/...src/main/java/org/elasticsearch/xpack/core/security/action/profile/GetProfileRequest.java b/...src/main/java/org/elasticsearch/xpack/core/security/action/profile/GetProfileRequest.java