Revert "chore: improve access service (#4689)"

This reverts commit 2186e2b.

src/lib/db/access-store.ts

@@ -20,11 +20,7 @@ import {
     ROOT_PERMISSION_TYPE,
 } from '../util/constants';
 import { Db } from './db';
-import {
-    IdPermissionRef,
-    NamePermissionRef,
-    PermissionRef,
-} from 'lib/services/access-service';
+import { IdPermissionRef } from 'lib/services/access-service';
 
 const T = {
     ROLE_USER: 'role_user',
@@ -50,12 +46,6 @@ interface IPermissionRow {
     role_id: number;
 }
 
-type ResolvedPermission = {
-    id: number;
-    name?: string;
-    environment?: string;
-};
-
 export class AccessStore implements IAccessStore {
     private logger: Logger;
 
@@ -73,75 +63,6 @@ export class AccessStore implements IAccessStore {
             });
     }
 
-    private permissionHasId = (permission: PermissionRef): boolean => {
-        return (permission as IdPermissionRef).id !== undefined;
-    };
-
-    private permissionNamesToIds = async (
-        permissions: NamePermissionRef[],
-    ): Promise<ResolvedPermission[]> => {
-        const permissionNames = (permissions ?? [])
-            .filter((p) => p.name !== undefined)
-            .map((p) => p.name);
-
-        if (permissionNames.length === 0) {
-            return [];
-        }
-
-        const stopTimer = this.timer('permissionNamesToIds');
-
-        const rows = await this.db
-            .select('id', 'permission')
-            .from(T.PERMISSIONS)
-            .whereIn('permission', permissionNames);
-
-        const rowByPermissionName = rows.reduce((acc, row) => {
-            acc[row.permission] = row;
-            return acc;
-        }, {} as Map<string, IPermissionRow>);
-
-        const permissionsWithIds = permissions.map((permission) => ({
-            id: rowByPermissionName[permission.name].id,
-            ...permission,
-        }));
-
-        stopTimer();
-        return permissionsWithIds;
-    };
-
-    resolvePermissions = async (
-        permissions: PermissionRef[],
-    ): Promise<ResolvedPermission[]> => {
-        if (permissions === undefined || permissions.length === 0) {
-            return [];
-        }
-        // permissions without ids (just names)
-        const permissionsWithoutIds = permissions.filter(
-            (p) => !this.permissionHasId(p),
-        ) as NamePermissionRef[];
-        const idPermissionsFromNamed = await this.permissionNamesToIds(
-            permissionsWithoutIds,
-        );
-
-        if (permissionsWithoutIds.length === permissions.length) {
-            // all named permissions without ids
-            return idPermissionsFromNamed;
-        } else if (permissionsWithoutIds.length === 0) {
-            // all permissions have ids
-            return permissions as ResolvedPermission[];
-        }
-        // some permissions have ids, some don't (should not happen!)
-        return permissions.map((permission) => {
-            if (this.permissionHasId(permission)) {
-                return permission as ResolvedPermission;
-            } else {
-                return idPermissionsFromNamed.find(
-                    (p) => p.name === (permission as NamePermissionRef).name,
-                )!;
-            }
-        });
-    };
-
     async delete(key: number): Promise<void> {
         await this.db(T.ROLES).where({ id: key }).del();
     }
@@ -301,11 +222,9 @@ export class AccessStore implements IAccessStore {
 
     async addEnvironmentPermissionsToRole(
         role_id: number,
-        permissions: PermissionRef[],
+        permissions: IdPermissionRef[],
     ): Promise<void> {
-        const resolvedPermission = await this.resolvePermissions(permissions);
-
-        const rows = resolvedPermission.map((permission) => {
+        const rows = permissions.map((permission) => {
             return {
                 role_id,
                 permission_id: permission.id,
@@ -781,16 +700,18 @@ export class AccessStore implements IAccessStore {
 
     async addPermissionsToRole(
         role_id: number,
-        permissions: PermissionRef[],
+        permissions: string[],
         environment?: string,
     ): Promise<void> {
-        // no need to pass down the environment in this particular case because it'll be overriden
-        const permissionsWithIds = await this.resolvePermissions(permissions);
+        const rows = await this.db
+            .select('id as permissionId')
+            .from<number>(T.PERMISSIONS)
+            .whereIn('permission', permissions);
 
-        const newRoles = permissionsWithIds.map((p) => ({
+        const newRoles = rows.map((row) => ({
             role_id,
             environment,
-            permission_id: p.id,
+            permission_id: row.permissionId,
         }));
 
         return this.db.batchInsert(T.ROLE_PERMISSION, newRoles);

src/lib/schema/role-schema.test.ts

@@ -38,7 +38,7 @@ test('role schema rejects a role with a broken permission list', async () => {
         await roleSchema.validateAsync(role);
     } catch (error) {
         expect(error.details[0].message).toBe(
-            '"permissions[0]" must contain at least one of [id, name]',
+            '"permissions[0].id" is required',
         );
     }
 });

src/lib/schema/role-schema.ts

@@ -3,11 +3,9 @@ import joi from 'joi';
 export const permissionRoleSchema = joi
     .object()
     .keys({
-        id: joi.number(),
-        name: joi.string(),
+        id: joi.number().required(),
         environment: joi.string().optional().allow('').allow(null).default(''),
     })
-    .or('id', 'name')
     .options({ stripUnknown: true, allowUnknown: false, abortEarly: false });
 
 export const roleSchema = joi

src/lib/services/access-service.test.ts

@@ -154,7 +154,6 @@ test('should be able to validate and cleanup with additional properties', async
         permissions: [
             {
                 id: 1,
-                name: 'name',
                 environment: 'development',
             },
         ],

src/lib/services/access-service.ts

@@ -70,7 +70,7 @@ export interface IRoleValidation {
     permissions?: PermissionRef[];
 }
 
-export interface IRoleUpdate {
+interface IRoleUpdate {
     id: number;
     name: string;
     description: string;
@@ -406,7 +406,7 @@ export class AccessService {
         }
         return this.store.addPermissionsToRole(
             roleId,
-            [{ name: permission }],
+            [permission],
             environment,
         );
     }
@@ -630,13 +630,11 @@ export class AccessService {
         const newRole = await this.roleStore.create(baseRole);
         if (rolePermissions) {
             if (roleType === CUSTOM_ROOT_ROLE_TYPE) {
-                // this branch uses named permissions
                 await this.store.addPermissionsToRole(
                     newRole.id,
-                    rolePermissions,
+                    rolePermissions.map((p: NamePermissionRef) => p.name),
                 );
             } else {
-                // this branch uses id permissions
                 await this.store.addEnvironmentPermissionsToRole(
                     newRole.id,
                     rolePermissions,
@@ -675,7 +673,7 @@ export class AccessService {
             if (roleType === CUSTOM_ROOT_ROLE_TYPE) {
                 await this.store.addPermissionsToRole(
                     newRole.id,
-                    rolePermissions,
+                    rolePermissions.map((p: NamePermissionRef) => p.name),
                 );
             } else {
                 await this.store.addEnvironmentPermissionsToRole(

src/lib/types/stores/access-store.ts

@@ -164,7 +164,7 @@ export interface IAccessStore extends Store<IRole, number> {
 
     addPermissionsToRole(
         role_id: number,
-        permissions: PermissionRef[],
+        permissions: string[],
         environment?: string,
     ): Promise<void>;