From 9d0ae816908466fbcd4568de5de6df623e1b7224 Mon Sep 17 00:00:00 2001 From: Yulong Ruan Date: Wed, 9 Aug 2023 14:46:16 +0800 Subject: [PATCH] refactor: simplified ACL transformPermission function (#78) refactor: simplified ACL transformPermission function --------- Signed-off-by: Yulong Ruan --- .../permission_control/acl.test.ts | 13 ++- .../saved_objects/permission_control/acl.ts | 88 ++++++++----------- .../permission_control/client.ts | 2 +- src/core/server/workspaces/routes/index.ts | 2 +- 4 files changed, 51 insertions(+), 54 deletions(-) diff --git a/src/core/server/saved_objects/permission_control/acl.test.ts b/src/core/server/saved_objects/permission_control/acl.test.ts index 95da5ca4d33b..cec90bbec891 100644 --- a/src/core/server/saved_objects/permission_control/acl.test.ts +++ b/src/core/server/saved_objects/permission_control/acl.test.ts @@ -107,8 +107,17 @@ describe('SavedObjectTypeRegistry', () => { write: principals, }; acl = new ACL(permissions); - const result = acl.transformPermissions(); - expect(result?.length).toEqual(3); + const result = acl.toFlatList(); + expect(result).toHaveLength(3); + expect(result).toEqual( + expect.arrayContaining([{ type: 'users', name: 'user1', permissions: ['read', 'write'] }]) + ); + expect(result).toEqual( + expect.arrayContaining([{ type: 'groups', name: 'group1', permissions: ['read', 'write'] }]) + ); + expect(result).toEqual( + expect.arrayContaining([{ type: 'groups', name: 'group2', permissions: ['read', 'write'] }]) + ); }); it('test generate query DSL', () => { diff --git a/src/core/server/saved_objects/permission_control/acl.ts b/src/core/server/saved_objects/permission_control/acl.ts index 2bac6823634a..1d34b9f6c401 100644 --- a/src/core/server/saved_objects/permission_control/acl.ts +++ b/src/core/server/saved_objects/permission_control/acl.ts @@ -3,7 +3,7 @@ * SPDX-License-Identifier: Apache-2.0 */ -import { PrincipalType } from '../../../../core/utils/constants'; +import { PrincipalType } from '../../../utils/constants'; export interface Principals { users?: string[]; @@ -111,7 +111,7 @@ export class ACL { return this; } - // permissions object build funciton, remove specific permission of specific principal from the object + // permissions object build function, remove specific permission of specific principal from the object public removePermission(permissionTypes: string[], principals: Principals) { if (!permissionTypes || !principals) { return this; @@ -134,62 +134,50 @@ export class ACL { return this; } - /* - transfrom permissions format - original permissions: { - read: { - users:['user1'] - }, - write:{ - groups:['group1'] - } - } - - transformed permissions: [ - {type:'users',name:'user1',permissions:['read']}, - {type:'groups',name:'group1',permissions:['write']}, - ] - */ - public transformPermissions(): TransformedPermission[] { + /** + * transform permissions format + * original permissions: { + * read: { + * users:['user1'] + * }, + * write:{ + * groups:['group1'] + * } + * } + * + * transformed permissions: [ + * {type:'users',name:'user1',permissions:['read']}, + * {type:'groups',name:'group1',permissions:['write']}, + * ] + */ + public toFlatList(): TransformedPermission[] { const result: TransformedPermission[] = []; if (!this.permissions) { return result; } - const permissionMapResult: Record> = {}; - const principalTypes = [PrincipalType.Users, PrincipalType.Groups]; for (const permissionType in this.permissions) { - if (!!permissionType) { - const value = this.permissions[permissionType]; - principalTypes.forEach((principalType) => { - if (value?.[principalType]) { - for (const principal of value[principalType]!) { - if (!permissionMapResult[principalType]) { - permissionMapResult[principalType] = {}; - } - if (!permissionMapResult[principalType][principal]) { - permissionMapResult[principalType][principal] = []; - } - permissionMapResult[principalType][principal] = [ - ...permissionMapResult[principalType][principal]!, - permissionType, - ]; - } + if (Object.prototype.hasOwnProperty.call(this.permissions, permissionType)) { + const { users = [], groups = [] } = this.permissions[permissionType] ?? {}; + users.forEach((user) => { + const found = result.find((r) => r.type === PrincipalType.Users && r.name === user); + if (found) { + found.permissions.push(permissionType); + } else { + result.push({ type: PrincipalType.Users, name: user, permissions: [permissionType] }); + } + }); + groups.forEach((group) => { + const found = result.find((r) => r.type === PrincipalType.Groups && r.name === group); + if (found) { + found.permissions.push(permissionType); + } else { + result.push({ type: PrincipalType.Groups, name: group, permissions: [permissionType] }); } }); } } - Object.entries(permissionMapResult).forEach(([type, permissionMap]) => { - Object.entries(permissionMap).forEach(([principal, permissions]) => { - result.push({ - type, - name: principal, - permissions, - }); - }); - }); - return result; } @@ -203,9 +191,9 @@ export class ACL { return this.permissions; } - /* - generate query DSL by the specific conditions, used for fetching saved objects from the saved objects index - */ + /** + * generate query DSL by the specific conditions, used for fetching saved objects from the saved objects index + */ public static genereateGetPermittedSavedObjectsQueryDSL( permissionTypes: string[], principals: Principals, diff --git a/src/core/server/saved_objects/permission_control/client.ts b/src/core/server/saved_objects/permission_control/client.ts index 0e34a4bc8cb6..e00274ce1a0c 100644 --- a/src/core/server/saved_objects/permission_control/client.ts +++ b/src/core/server/saved_objects/permission_control/client.ts @@ -116,7 +116,7 @@ export class SavedObjectsPermissionControl { return detailedSavedObjects.reduce((total, current) => { return { ...total, - [current.id]: new ACL(current.permissions).transformPermissions(), + [current.id]: new ACL(current.permissions).toFlatList(), }; }, {}); } diff --git a/src/core/server/workspaces/routes/index.ts b/src/core/server/workspaces/routes/index.ts index 6a22b6fc310e..67c3fcfd77a0 100644 --- a/src/core/server/workspaces/routes/index.ts +++ b/src/core/server/workspaces/routes/index.ts @@ -67,7 +67,7 @@ const convertToACL = ( const convertFromACL = (permissions: Permissions) => { const acl = new ACL(permissions); - return acl.transformPermissions().map(({ name, permissions: modes, type }) => ({ + return acl.toFlatList().map(({ name, permissions: modes, type }) => ({ type: type === 'users' ? 'user' : 'group', modes, ...{ [type === 'users' ? 'userId' : 'group']: name },