fix(core): Admin can only read Roles at or below their permission level

Fixes vendure-ecommerce#2492
alexisvigoureux · Jan 12, 2024 · fc5d981 · fc5d981
1 parent 65aced6
commit fc5d981
Show file tree

Hide file tree

Showing 2 changed files with 51 additions and 5 deletions.
diff --git a/packages/core/e2e/role.e2e-spec.ts b/packages/core/e2e/role.e2e-spec.ts
@@ -510,6 +510,24 @@ describe('Role resolver', () => {
             await adminClient.asUserWithCredentials(limitedAdmin.emailAddress, 'test');
         });
 
+        it('limited admin cannot view Roles which require permissions they do not have', async () => {
+            const result = await adminClient.query<Codegen.GetRolesQuery, Codegen.GetRolesQueryVariables>(
+                GET_ROLES,
+            );
+
+            const roleCodes = result.roles.items.map(r => r.code);
+            expect(roleCodes).toEqual(['second-channel-admin-manager']);
+        });
+
+        it('limited admin cannot view Role which requires permissions they do not have', async () => {
+            const result = await adminClient.query<Codegen.GetRoleQuery, Codegen.GetRoleQueryVariables>(
+                GET_ROLE,
+                { id: orderReaderRole.id },
+            );
+
+            expect(result.role).toBeNull();
+        });
+
         it(
             'limited admin cannot create Role with SuperAdmin permission',
             assertThrowsWithMessage(async () => {

diff --git a/packages/core/src/service/services/role.service.ts b/packages/core/src/service/services/role.service.ts
@@ -72,10 +72,19 @@ export class RoleService {
         return this.listQueryBuilder
             .build(Role, options, { relations: relations ?? ['channels'], ctx })
             .getManyAndCount()
-            .then(([items, totalItems]) => ({
-                items,
-                totalItems,
-            }));
+            .then(async ([items, totalItems]) => {
+                const visibleRoles: Role[] = [];
+                for (const item of items) {
+                    const canRead = await this.activeUserCanReadRole(ctx, item);
+                    if (canRead) {
+                        visibleRoles.push(item);
+                    }
+                }
+                return {
+                    items: visibleRoles,
+                    totalItems,
+                };
+            });
     }
 
     findOne(ctx: RequestContext, roleId: ID, relations?: RelationPaths<Role>): Promise<Role | undefined> {
@@ -85,7 +94,11 @@ export class RoleService {
                 where: { id: roleId },
                 relations: relations ?? ['channels'],
             })
-            .then(result => result ?? undefined);
+            .then(async result => {
+                if (result && (await this.activeUserCanReadRole(ctx, result))) {
+                    return result;
+                }
+            });
     }
 
     getChannelsForRole(ctx: RequestContext, roleId: ID): Promise<Channel[]> {
@@ -156,6 +169,21 @@ export class RoleService {
         return false;
     }
 
+    private async activeUserCanReadRole(ctx: RequestContext, role: Role): Promise<boolean> {
+        const permissionsRequired = getChannelPermissions([role]);
+        for (const channelPermissions of permissionsRequired) {
+            const activeUserHasRequiredPermissions = await this.userHasAllPermissionsOnChannel(
+                ctx,
+                channelPermissions.id,
+                channelPermissions.permissions,
+            );
+            if (!activeUserHasRequiredPermissions) {
+                return false;
+            }
+        }
+        return true;
+    }
+
     /**
      * @description
      * Returns true if the User has all the specified permissions on that Channel