Changing the structure of the error message that is received from ES …

…when checking privileges during `suggest` user profiles (elastic#144050)

* Changing the structure of the error message that is received from ES and logging the additional data

* Adding property description for error

x-pack/plugins/security/server/authorization/check_privileges.test.ts

@@ -3109,14 +3109,25 @@ describe('#checkUserProfilesPrivileges.atSpace', () => {
         ],
         esHasPrivilegesResponse: Promise.resolve({
           has_privilege_uids: ['uid-1', 'uid-2'],
-          error_uids: ['uid-3'],
+          errors: {
+            count: 1,
+            details: {
+              'uid-3': { type: 'Not Found', reason: 'UID not found' },
+            },
+          },
         }),
       })
     ).resolves.toMatchInlineSnapshot(`
         Object {
-          "errorUids": Array [
-            "uid-3",
-          ],
+          "errors": Object {
+            "count": 1,
+            "details": Object {
+              "uid-3": Object {
+                "reason": "UID not found",
+                "type": "Not Found",
+              },
+            },
+          },
           "hasPrivilegeUids": Array [
             "uid-1",
             "uid-2",

x-pack/plugins/security/server/authorization/check_privileges.ts

@@ -78,7 +78,10 @@ export function checkPrivilegesFactory(
 
       const response = await clusterClient.asInternalUser.transport.request<{
         has_privilege_uids: string[];
-        error_uids?: string[];
+        errors: {
+          count: number;
+          details: Record<string, { type: string; reason: string }>;
+        };
       }>({
         method: 'POST',
         path: '_security/profile/_has_privileges',
@@ -90,7 +93,7 @@ export function checkPrivilegesFactory(
 
       return {
         hasPrivilegeUids: response.has_privilege_uids,
-        errorUids: response.error_uids ?? [],
+        ...(response.errors && { errors: response.errors }),
       };
     };
 

x-pack/plugins/security/server/authorization/types.ts

@@ -139,9 +139,14 @@ export interface CheckUserProfilesPrivilegesResponse {
    * The subset of the requested profile IDs of the users that have all the requested privileges.
    */
   hasPrivilegeUids: string[];
+
   /**
-   * 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.
+   * An errors object that may be returned from ES that contains a `count` of UIDs that have errors in the `details` property.
+   *
+   * Each entry in `details` will contain an error `type`, e.g 'resource_not_found_exception', and a `reason` message, e.g. 'profile document not found'
    */
-  errorUids: string[];
+  errors?: {
+    count: number;
+    details: Record<string, { type: string; reason: string }>;
+  };
 }

x-pack/plugins/security/server/user_profile/user_profile_service.test.ts

@@ -816,7 +816,6 @@ describe('UserProfileService', () => {
       const mockAtSpacePrivilegeCheck = { atSpace: jest.fn() };
       mockAtSpacePrivilegeCheck.atSpace.mockResolvedValue({
         hasPrivilegeUids: ['UID-0', 'UID-1', 'UID-8'],
-        errorUids: [],
       });
       mockAuthz.checkUserProfilesPrivileges.mockReturnValue(mockAtSpacePrivilegeCheck);
 
@@ -911,15 +910,12 @@ describe('UserProfileService', () => {
       mockAtSpacePrivilegeCheck.atSpace
         .mockResolvedValueOnce({
           hasPrivilegeUids: ['UID-0'],
-          errorUids: [],
         })
         .mockResolvedValueOnce({
           hasPrivilegeUids: ['UID-20'],
-          errorUids: [],
         })
         .mockResolvedValueOnce({
           hasPrivilegeUids: [],
-          errorUids: [],
         });
       mockAuthz.checkUserProfilesPrivileges.mockReturnValue(mockAtSpacePrivilegeCheck);
 
@@ -1020,7 +1016,6 @@ describe('UserProfileService', () => {
       const mockAtSpacePrivilegeCheck = { atSpace: jest.fn() };
       mockAtSpacePrivilegeCheck.atSpace.mockResolvedValue({
         hasPrivilegeUids: ['UID-0', 'UID-1', 'UID-8'],
-        errorUids: [],
       });
       mockAuthz.checkUserProfilesPrivileges.mockReturnValue(mockAtSpacePrivilegeCheck);
 
@@ -1084,6 +1079,109 @@ describe('UserProfileService', () => {
         kibana: ['privilege-1', 'privilege-2'],
       });
     });
+
+    it('properly handles privileges checks and logs errors when errors with reasons are returned from the privilege check', async () => {
+      // In this test we'd like to simulate the following case:
+      // 1. User requests 2 results with privileges check
+      // 2. Kibana will fetch 10 (min batch) results
+      // 3. Only UID-0, UID-1 and UID-8 profiles will have necessary privileges
+      mockStartParams.clusterClient.asInternalUser.security.suggestUserProfiles.mockResolvedValue({
+        profiles: Array.from({ length: 10 }).map((_, index) =>
+          userProfileMock.createWithSecurity({
+            uid: `UID-${index}`,
+            data: { some: 'data', kibana: { some: `kibana-data-${index}` } },
+          })
+        ),
+      } as unknown as SecuritySuggestUserProfilesResponse);
+
+      const mockAtSpacePrivilegeCheck = { atSpace: jest.fn() };
+
+      mockAtSpacePrivilegeCheck.atSpace.mockResolvedValue({
+        hasPrivilegeUids: ['UID-0', 'UID-1', 'UID-8'],
+        errors: {
+          count: 2,
+          details: {
+            'UID-3': { type: 'some type 3', reason: 'some reason 3' },
+            'UID-4': { type: 'some type 4', reason: 'some reason 4' },
+          },
+        },
+      });
+
+      mockAuthz.checkUserProfilesPrivileges.mockReturnValue(mockAtSpacePrivilegeCheck);
+
+      const startContract = userProfileService.start(mockStartParams);
+
+      await expect(
+        startContract.suggest({
+          name: 'some',
+          size: 2,
+          dataPath: 'one,two',
+          requiredPrivileges: {
+            spaceId: 'some-space',
+            privileges: { kibana: ['privilege-1', 'privilege-2'] },
+          },
+        })
+      ).resolves.toMatchInlineSnapshot(`
+              Array [
+                Object {
+                  "data": Object {
+                    "some": "kibana-data-0",
+                  },
+                  "enabled": true,
+                  "uid": "UID-0",
+                  "user": Object {
+                    "email": "some@email",
+                    "full_name": undefined,
+                    "username": "some-username",
+                  },
+                },
+                Object {
+                  "data": Object {
+                    "some": "kibana-data-1",
+                  },
+                  "enabled": true,
+                  "uid": "UID-1",
+                  "user": Object {
+                    "email": "some@email",
+                    "full_name": undefined,
+                    "username": "some-username",
+                  },
+                },
+              ]
+            `);
+
+      expect(
+        mockStartParams.clusterClient.asInternalUser.security.suggestUserProfiles
+      ).toHaveBeenCalledTimes(1);
+
+      expect(
+        mockStartParams.clusterClient.asInternalUser.security.suggestUserProfiles
+      ).toHaveBeenCalledWith({
+        name: 'some',
+        size: 10,
+        data: 'kibana.one,kibana.two',
+      });
+
+      expect(mockAuthz.checkUserProfilesPrivileges).toHaveBeenCalledTimes(1);
+
+      expect(mockAuthz.checkUserProfilesPrivileges).toHaveBeenCalledWith(
+        new Set(Array.from({ length: 10 }).map((_, index) => `UID-${index}`))
+      );
+
+      expect(mockAtSpacePrivilegeCheck.atSpace).toHaveBeenCalledTimes(1);
+
+      expect(mockAtSpacePrivilegeCheck.atSpace).toHaveBeenCalledWith('some-space', {
+        kibana: ['privilege-1', 'privilege-2'],
+      });
+
+      expect(logger.error).toHaveBeenCalledWith(
+        'Privileges check API failed for UID UID-3 because some reason 3.'
+      );
+
+      expect(logger.error).toHaveBeenCalledWith(
+        'Privileges check API failed for UID UID-4 because some reason 4.'
+      );
+    });
   });
 });
 

x-pack/plugins/security/server/user_profile/user_profile_service.ts

@@ -506,11 +506,15 @@ export class UserProfileService {
         this.logger.error(`Privileges check API returned unknown profile UIDs: ${unknownUids}.`);
       }
 
-      // Log profile UIDs for which an error was encountered.
-      if (response.errorUids.length > 0) {
-        this.logger.error(
-          `Privileges check API failed for the following user profiles: ${response.errorUids}.`
-        );
+      // Log profile UIDs and reason for which an error was encountered.
+      if (response.errors?.count) {
+        const uids = Object.keys(response.errors.details);
+
+        for (const uid of uids) {
+          this.logger.error(
+            `Privileges check API failed for UID ${uid} because ${response.errors.details[uid].reason}.`
+          );
+        }
       }
     }