[8.2] Resolves UI Breaks from Malformed Roles (elastic#131915) (elast…

…ic#132148)

* Resolves UI Breaks from Malformed Roles (elastic#131915)

* Adds catch of exceptions from PrivilegeSerializer deserialize methods to role transform function.
Resolves 124808 where malformed Elasticsearch roles cause Kibana users and roles UIs to not display correctly.

* Adds logger to role transform functions

* File accidentally not saved prior to last commit

(cherry picked from commit f4eb311)

# Conflicts:
#	x-pack/plugins/security/server/authorization/roles/elasticsearch_role.test.ts
#	x-pack/plugins/security/server/authorization/roles/elasticsearch_role.ts

* [CI] Auto-commit changed files from 'node scripts/eslint --no-cache --fix'

* Fix import and linting errors.

Co-authored-by: kibanamachine <42973632+kibanamachine@users.noreply.github.com>

x-pack/plugins/security/server/authorization/roles/elasticsearch_role.test.ts

@@ -6,6 +6,8 @@
  */
 import { omit, pick } from 'lodash';
 
+import { loggerMock } from '@kbn/logging-mocks';
+
 import { KibanaFeature } from '../../../../features/server';
 import { transformElasticsearchRoleToRole } from './elasticsearch_role';
 import type { ElasticsearchRole } from './elasticsearch_role';
@@ -79,6 +81,23 @@ const roles = [
       enabled: true,
     },
   },
+  {
+    name: 'global-malformed',
+    cluster: [],
+    indices: [],
+    applications: [
+      {
+        application: 'kibana-.kibana',
+        privileges: ['feature_securitySolutionCases.a;;'],
+        resources: ['*'],
+      },
+    ],
+    run_as: [],
+    metadata: {},
+    transient_metadata: {
+      enabled: true,
+    },
+  },
   {
     name: 'default-base-all',
     cluster: [],
@@ -147,6 +166,23 @@ const roles = [
       enabled: true,
     },
   },
+  {
+    name: 'default-malformed',
+    cluster: [],
+    indices: [],
+    applications: [
+      {
+        application: 'kibana-.kibana',
+        privileges: ['feature_securitySolutionCases.a;;'],
+        resources: ['space:default'],
+      },
+    ],
+    run_as: [],
+    metadata: {},
+    transient_metadata: {
+      enabled: true,
+    },
+  },
 ];
 
 function testRoles(
@@ -160,7 +196,8 @@ function testRoles(
       features,
       omit(role, 'name'),
       role.name,
-      'kibana-.kibana'
+      'kibana-.kibana',
+      loggerMock.create()
     );
     return pick(transformedRole, ['name', '_transform_error']);
   });
@@ -227,10 +264,12 @@ describe('#transformElasticsearchRoleToRole', () => {
     { name: 'global-base-read', _transform_error: [] },
     { name: 'global-foo-all', _transform_error: [] },
     { name: 'global-foo-read', _transform_error: [] },
+    { name: 'global-malformed', _transform_error: ['kibana'] },
     { name: 'default-base-all', _transform_error: [] },
     { name: 'default-base-read', _transform_error: [] },
     { name: 'default-foo-all', _transform_error: ['kibana'] },
     { name: 'default-foo-read', _transform_error: [] },
+    { name: 'default-malformed', _transform_error: ['kibana'] },
   ]);
 
   testRoles(
@@ -242,10 +281,12 @@ describe('#transformElasticsearchRoleToRole', () => {
       { name: 'global-base-read', _transform_error: [] },
       { name: 'global-foo-all', _transform_error: [] },
       { name: 'global-foo-read', _transform_error: ['kibana'] },
+      { name: 'global-malformed', _transform_error: ['kibana'] },
       { name: 'default-base-all', _transform_error: [] },
       { name: 'default-base-read', _transform_error: [] },
       { name: 'default-foo-all', _transform_error: [] },
       { name: 'default-foo-read', _transform_error: ['kibana'] },
+      { name: 'default-malformed', _transform_error: ['kibana'] },
     ]
   );
 });

x-pack/plugins/security/server/authorization/roles/elasticsearch_role.ts

@@ -5,12 +5,14 @@
  * 2.0.
  */
 
+import type { Logger } from '../../../../../../src/core/server';
 import type { KibanaFeature } from '../../../../features/common';
 import {
   GLOBAL_RESOURCE,
   RESERVED_PRIVILEGES_APPLICATION_WILDCARD,
 } from '../../../common/constants';
 import type { Role, RoleKibanaPrivilege } from '../../../common/model';
+import { getDetailedErrorMessage } from '../../errors';
 import { PrivilegeSerializer } from '../privilege_serializer';
 import { ResourceSerializer } from '../resource_serializer';
 
@@ -29,12 +31,14 @@ export function transformElasticsearchRoleToRole(
   features: KibanaFeature[],
   elasticsearchRole: Omit<ElasticsearchRole, 'name'>,
   name: string,
-  application: string
+  application: string,
+  logger: Logger
 ): Role {
   const kibanaTransformResult = transformRoleApplicationsToKibanaPrivileges(
     features,
     elasticsearchRole.applications,
-    application
+    application,
+    logger
   );
   return {
     name,
@@ -57,7 +61,8 @@ export function transformElasticsearchRoleToRole(
 function transformRoleApplicationsToKibanaPrivileges(
   features: KibanaFeature[],
   roleApplications: ElasticsearchRole['applications'],
-  application: string
+  application: string,
+  logger: Logger
 ) {
   const roleKibanaApplications = roleApplications.filter(
     (roleApplication) =>
@@ -225,9 +230,9 @@ function transformRoleApplicationsToKibanaPrivileges(
     };
   }
 
-  return {
-    success: true,
-    value: roleKibanaApplications.map(({ resources, privileges }) => {
+  // try/catch block ensures graceful return on deserialize exceptions
+  try {
+    const transformResult = roleKibanaApplications.map(({ resources, privileges }) => {
       // if we're dealing with a global entry, which we've ensured above is only possible if it's the only item in the array
       if (resources.length === 1 && resources[0] === GLOBAL_RESOURCE) {
         const reservedPrivileges = privileges.filter((privilege) =>
@@ -287,8 +292,18 @@ function transformRoleApplicationsToKibanaPrivileges(
         }, {} as RoleKibanaPrivilege['feature']),
         spaces: resources.map((resource) => ResourceSerializer.deserializeSpaceResource(resource)),
       };
-    }),
-  };
+    });
+
+    return {
+      success: true,
+      value: transformResult,
+    };
+  } catch (e) {
+    logger.error(`Error transforming Elasticsearch role: ${getDetailedErrorMessage(e)}`);
+    return {
+      success: false,
+    };
+  }
 }
 
 const extractUnrecognizedApplicationNames = (

x-pack/plugins/security/server/deprecations/privilege_deprecations.ts

@@ -51,7 +51,8 @@ export const getPrivilegeDeprecationsService = ({
           // @ts-expect-error `SecurityIndicesPrivileges.names` expected to be `string[]`
           elasticsearchRole,
           roleName,
-          authz.applicationName
+          authz.applicationName,
+          logger
         )
       );
     } catch (e) {

x-pack/plugins/security/server/routes/authorization/roles/get.ts

@@ -12,7 +12,12 @@ import { wrapIntoCustomErrorResponse } from '../../../errors';
 import { createLicensedRouteHandler } from '../../licensed_route_handler';
 import { transformElasticsearchRoleToRole } from './model';
 
-export function defineGetRolesRoutes({ router, authz, getFeatures }: RouteDefinitionParams) {
+export function defineGetRolesRoutes({
+  router,
+  authz,
+  getFeatures,
+  logger,
+}: RouteDefinitionParams) {
   router.get(
     {
       path: '/api/security/role/{name}',
@@ -37,7 +42,8 @@ export function defineGetRolesRoutes({ router, authz, getFeatures }: RouteDefini
               // @ts-expect-error `SecurityIndicesPrivileges.names` expected to be `string[]`
               elasticsearchRole,
               request.params.name,
-              authz.applicationName
+              authz.applicationName,
+              logger
             ),
           });
         }

x-pack/plugins/security/server/routes/authorization/roles/get_all.ts

@@ -10,7 +10,12 @@ import { wrapIntoCustomErrorResponse } from '../../../errors';
 import { createLicensedRouteHandler } from '../../licensed_route_handler';
 import { transformElasticsearchRoleToRole } from './model';
 
-export function defineGetAllRolesRoutes({ router, authz, getFeatures }: RouteDefinitionParams) {
+export function defineGetAllRolesRoutes({
+  router,
+  authz,
+  getFeatures,
+  logger,
+}: RouteDefinitionParams) {
   router.get(
     { path: '/api/security/role', validate: false },
     createLicensedRouteHandler(async (context, request, response) => {
@@ -29,7 +34,8 @@ export function defineGetAllRolesRoutes({ router, authz, getFeatures }: RouteDef
                 // @ts-expect-error @elastic/elasticsearch SecurityIndicesPrivileges.names expected to be string[]
                 elasticsearchRole,
                 roleName,
-                authz.applicationName
+                authz.applicationName,
+                logger
               )
             )
             .sort((roleA, roleB) => {