Skip to content

Commit

Permalink
[7.17] Resolves UI Breaks from Malformed Roles (#131915) (#132149)
Browse files Browse the repository at this point in the history
* Resolves UI Breaks from Malformed Roles (#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
#	x-pack/plugins/security/server/routes/authorization/roles/get.ts
#	x-pack/plugins/security/server/routes/authorization/roles/get_all.ts

* Fixed import and test issues.
  • Loading branch information
jeramysoucy authored May 16, 2022
1 parent 17ad098 commit 551257f
Show file tree
Hide file tree
Showing 4 changed files with 31 additions and 13 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -5,11 +5,13 @@
* 2.0.
*/

import type { Logger } from '../../../../../../src/core/server';
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';

Expand All @@ -27,11 +29,13 @@ export type ElasticsearchRole = Pick<Role, 'name' | 'metadata' | 'transient_meta
export function transformElasticsearchRoleToRole(
elasticsearchRole: Omit<ElasticsearchRole, 'name'>,
name: string,
application: string
application: string,
logger: Logger
): Role {
const kibanaTransformResult = transformRoleApplicationsToKibanaPrivileges(
elasticsearchRole.applications,
application
application,
logger
);

return {
Expand All @@ -54,7 +58,8 @@ export function transformElasticsearchRoleToRole(

function transformRoleApplicationsToKibanaPrivileges(
roleApplications: ElasticsearchRole['applications'],
application: string
application: string,
logger: Logger
) {
const roleKibanaApplications = roleApplications.filter(
(roleApplication) =>
Expand Down Expand Up @@ -184,9 +189,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) =>
Expand Down Expand Up @@ -246,8 +251,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 = (
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,8 @@ export const getPrivilegeDeprecationsService = (
// @ts-expect-error `SecurityIndicesPrivileges.names` expected to be `string[]`
elasticsearchRole,
roleName,
authz.applicationName
authz.applicationName,
logger
)
);
} catch (e) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ import { wrapIntoCustomErrorResponse } from '../../../errors';
import { createLicensedRouteHandler } from '../../licensed_route_handler';
import { transformElasticsearchRoleToRole } from './model';

export function defineGetRolesRoutes({ router, authz }: RouteDefinitionParams) {
export function defineGetRolesRoutes({ router, authz, logger }: RouteDefinitionParams) {
router.get(
{
path: '/api/security/role/{name}',
Expand All @@ -34,7 +34,8 @@ export function defineGetRolesRoutes({ router, authz }: RouteDefinitionParams) {
// @ts-expect-error `SecurityIndicesPrivileges.names` expected to be `string[]`
elasticsearchRole,
request.params.name,
authz.applicationName
authz.applicationName,
logger
),
});
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ import { createLicensedRouteHandler } from '../../licensed_route_handler';
import type { ElasticsearchRole } from './model';
import { transformElasticsearchRoleToRole } from './model';

export function defineGetAllRolesRoutes({ router, authz }: RouteDefinitionParams) {
export function defineGetAllRolesRoutes({ router, authz, logger }: RouteDefinitionParams) {
router.get(
{ path: '/api/security/role', validate: false },
createLicensedRouteHandler(async (context, request, response) => {
Expand All @@ -29,7 +29,8 @@ export function defineGetAllRolesRoutes({ router, authz }: RouteDefinitionParams
// @ts-expect-error @elastic/elasticsearch SecurityIndicesPrivileges.names expected to be string[]
elasticsearchRole,
roleName,
authz.applicationName
authz.applicationName,
logger
)
)
.sort((roleA, roleB) => {
Expand Down

0 comments on commit 551257f

Please sign in to comment.