Support multiple @AffectedEntity()-decorators (#1733)

Notable changes: 1. AffectedEntity()-decorator is only valid for Methods, not for classes anymore 2. Every (not at least one) affected ContentScope must apply to user-permissions (was a security hole before) 3. In entities without scope-field the @ScopedEntity-decorator is now mandatory 4. The PageTreeNode must have a scope-field when AffectedEntity is used 5. Removed equality-check when using AffectedEntity() and at the same time submitting a scope argument 6. Support array of ids submitted in args 1-4 are breaking changes, but mainly in theory. Practically no project will be affected. --------- Co-authored-by: Johannes Obermair <[email protected]>
vivid-planet · Feb 26, 2024 · 151e121 · 151e121
1 parent ad153c9
commit 151e121
Show file tree

Hide file tree

Showing 5 changed files with 101 additions and 94 deletions.
diff --git a/.changeset/flat-onions-turn.md b/.changeset/flat-onions-turn.md
@@ -0,0 +1,5 @@
+---
+"@comet/cms-api": minor
+---
+
+Support multiple `@AffectedEntity()`-decorators for a single function
diff --git a/demo/api/src/menus/main-menu-item.resolver.ts b/demo/api/src/menus/main-menu-item.resolver.ts
@@ -17,14 +17,14 @@ import { MainMenuItem } from "./entities/main-menu-item.entity";
 
 @Resolver(() => MainMenuItem)
 @RequiredPermission(["pageTree"])
-@AffectedEntity(MainMenuItem, { pageTreeNodeIdArg: "pageTreeNodeId" })
 export class MainMenuItemResolver {
     constructor(
         @InjectRepository(MainMenuItem) private readonly mainMenuItemRepository: EntityRepository<MainMenuItem>,
         private readonly pageTreeService: PageTreeService,
     ) {}
 
     @Query(() => MainMenuItem)
+    @AffectedEntity(MainMenuItem, { pageTreeNodeIdArg: "pageTreeNodeId" })
     async mainMenuItem(
         @Args("pageTreeNodeId", { type: () => ID }) pageTreeNodeId: string,
         @RequestContext() { includeInvisiblePages }: RequestContextInterface,
@@ -47,6 +47,7 @@ export class MainMenuItemResolver {
     }
 
     @Mutation(() => MainMenuItem)
+    @AffectedEntity(MainMenuItem, { pageTreeNodeIdArg: "pageTreeNodeId" })
     async updateMainMenuItem(
         @Args("pageTreeNodeId", { type: () => ID }) pageTreeNodeId: string,
         @Args("input", { type: () => MainMenuItemInput }) input: MainMenuItemInput,

diff --git a/packages/api/cms-api/src/user-permissions/auth/user-permissions.guard.ts b/packages/api/cms-api/src/user-permissions/auth/user-permissions.guard.ts
@@ -5,7 +5,6 @@ import { GqlContextType, GqlExecutionContext } from "@nestjs/graphql";
 import { ContentScopeService } from "../content-scope.service";
 import { RequiredPermissionMetadata } from "../decorators/required-permission.decorator";
 import { CurrentUser } from "../dto/current-user";
-import { ContentScope } from "../interfaces/content-scope.interface";
 import { ACCESS_CONTROL_SERVICE } from "../user-permissions.constants";
 import { AccessControlServiceInterface } from "../user-permissions.types";
 
@@ -18,52 +17,52 @@ export class UserPermissionsGuard implements CanActivate {
     ) {}
 
     async canActivate(context: ExecutionContext): Promise<boolean> {
-        if (this.reflector.getAllAndOverride("disableGlobalGuard", [context.getHandler(), context.getClass()])) {
-            return true;
-        }
+        const location = `${context.getClass().name}::${context.getHandler().name}()`;
 
-        if (this.reflector.getAllAndOverride("publicApi", [context.getHandler(), context.getClass()])) {
-            return true;
-        }
+        if (this.getDecorator(context, "disableGlobalGuard")) return true;
+        if (this.getDecorator(context, "publicApi")) return true;
 
-        const request =
-            context.getType().toString() === "graphql" ? GqlExecutionContext.create(context).getContext().req : context.switchToHttp().getRequest();
-        const user = request.user as CurrentUser | undefined;
+        const user = this.getUser(context);
         if (!user) return false;
 
-        const requiredPermission = this.reflector.getAllAndOverride<RequiredPermissionMetadata>("requiredPermission", [
-            context.getHandler(),
-            context.getClass(),
-        ]);
-        if (!requiredPermission) {
-            throw new Error(`RequiredPermission decorator is missing in ${context.getClass().name}::${context.getHandler().name}()`);
-        }
-
-        let requiredContentScopes: ContentScope[] | undefined;
-        if (!this.isResolvingGraphQLField(context) && !requiredPermission.options?.skipScopeCheck) {
-            requiredContentScopes = await this.contentScopeService.inferScopesFromExecutionContext(context);
-            if (!requiredContentScopes) {
+        const requiredPermission = this.getDecorator<RequiredPermissionMetadata>(context, "requiredPermission");
+        if (!requiredPermission) throw new Error(`RequiredPermission decorator is missing in ${location}`);
+        const requiredPermissions = requiredPermission.requiredPermission;
+        if (requiredPermissions.length === 0) throw new Error(`RequiredPermission decorator has empty permissions in ${location}`);
+        if (this.isResolvingGraphQLField(context) || requiredPermission.options?.skipScopeCheck) {
+            // At least one permission is required
+            return requiredPermissions.some((permission) => this.accessControlService.isAllowed(user, permission));
+        } else {
+            const requiredContentScopes = await this.contentScopeService.getScopesForPermissionCheck(context);
+            if (requiredContentScopes.length === 0)
                 throw new Error(
-                    `Could not get ContentScope. Either pass a scope-argument or add @AffectedEntity()-decorator or enable skipScopeCheck in @RequiredPermission() (${
-                        context.getClass().name
-                    }::${context.getHandler().name}())`,
+                    `Could not get content scope. Either pass a scope-argument or add an @AffectedEntity()-decorator or enable skipScopeCheck in the @RequiredPermission()-decorator of ${location}`,
                 );
-            }
-        }
 
-        const requiredPermissions = requiredPermission.requiredPermission;
-        if (requiredPermissions.length === 0) {
-            throw new Error(`RequiredPermission decorator has empty permissions in ${context.getClass().name}::${context.getHandler().name}()`);
+            // requiredContentScopes is an two level array of scopes
+            // The first level has to be checked with AND, the second level with OR
+            // The first level consists of submitted scopes and affected entities
+            // The only case that there is more than one scope in the second level is when the ScopedEntity returns more scopes
+            return requiredPermissions.some((permission) =>
+                requiredContentScopes.every((contentScopes) =>
+                    contentScopes.some((contentScope) => this.accessControlService.isAllowed(user, permission, contentScope)),
+                ),
+            );
         }
-        return requiredPermissions.some((permission) =>
-            requiredContentScopes
-                ? requiredContentScopes.some((contentScope) => this.accessControlService.isAllowed(user, permission, contentScope))
-                : this.accessControlService.isAllowed(user, permission),
-        );
+    }
+
+    private getUser(context: ExecutionContext): CurrentUser | undefined {
+        const request =
+            context.getType().toString() === "graphql" ? GqlExecutionContext.create(context).getContext().req : context.switchToHttp().getRequest();
+        return request.user as CurrentUser | undefined;
+    }
+
+    private getDecorator<T = object>(context: ExecutionContext, decorator: string): T {
+        return this.reflector.getAllAndOverride(decorator, [context.getClass(), context.getHandler()]);
     }
 
     // See https://docs.nestjs.com/graphql/other-features#execute-enhancers-at-the-field-resolver-level
-    isResolvingGraphQLField(context: ExecutionContext): boolean {
+    private isResolvingGraphQLField(context: ExecutionContext): boolean {
         if (context.getType<GqlContextType>() === "graphql") {
             const gqlContext = GqlExecutionContext.create(context);
             const info = gqlContext.getInfo();

diff --git a/packages/api/cms-api/src/user-permissions/content-scope.service.ts b/packages/api/cms-api/src/user-permissions/content-scope.service.ts
@@ -1,4 +1,4 @@
-import { EntityClass, MikroORM } from "@mikro-orm/core";
+import { MikroORM } from "@mikro-orm/core";
 import { ExecutionContext, Injectable, Optional } from "@nestjs/common";
 import { Reflector } from "@nestjs/core";
 import { GqlExecutionContext } from "@nestjs/graphql";
@@ -9,6 +9,7 @@ import { ScopedEntityMeta } from "../user-permissions/decorators/scoped-entity.d
 import { ContentScope } from "../user-permissions/interfaces/content-scope.interface";
 import { AffectedEntityMeta } from "./decorators/affected-entity.decorator";
 
+// TODO Remove service and move into UserPermissionsGuard once ChangesCheckerInterceptor is removed
 @Injectable()
 export class ContentScopeService {
     constructor(private reflector: Reflector, private readonly orm: MikroORM, @Optional() private readonly pageTreeService?: PageTreeService) {}
@@ -21,63 +22,60 @@ export class ContentScopeService {
         return isEqual({ ...scope1 }, { ...scope2 });
     }
 
-    async inferScopeFromExecutionContext(context: ExecutionContext): Promise<ContentScope | ContentScope[] | undefined> {
+    async getScopesForPermissionCheck(context: ExecutionContext): Promise<ContentScope[][]> {
+        const contentScopes: ContentScope[][] = [];
         const args = await this.getArgs(context);
 
-        const affectedEntity = this.reflector.getAllAndOverride<AffectedEntityMeta>("affectedEntity", [context.getHandler(), context.getClass()]);
-        if (affectedEntity) {
-            let contentScope: ContentScope | ContentScope[] | undefined;
-            // eslint-disable-next-line @typescript-eslint/no-explicit-any
-            const repo = this.orm.em.getRepository<any>(affectedEntity.entity);
-            if (affectedEntity.options.idArg) {
-                if (!args[affectedEntity.options.idArg]) {
-                    throw new Error(`${affectedEntity.options.idArg} arg not found`);
-                }
-                const row = await repo.findOneOrFail(args[affectedEntity.options.idArg]);
+        const affectedEntities = this.reflector.getAllAndOverride<AffectedEntityMeta[]>("affectedEntities", [context.getHandler()]) || [];
+        for (const affectedEntity of affectedEntities) {
+            contentScopes.push(...(await this.getContentScopesFromEntity(affectedEntity, args)));
+        }
+        if (args.scope) {
+            contentScopes.push([args.scope as ContentScope]);
+        }
+        return contentScopes;
+    }
+
+    async inferScopesFromExecutionContext(context: ExecutionContext): Promise<ContentScope[]> {
+        return [...new Set((await this.getScopesForPermissionCheck(context)).flat())];
+    }
+
+    private async getContentScopesFromEntity(affectedEntity: AffectedEntityMeta, args: Record<string, string>): Promise<ContentScope[][]> {
+        const contentScopes: ContentScope[][] = [];
+        if (affectedEntity.options.idArg) {
+            if (!args[affectedEntity.options.idArg]) throw new Error(`${affectedEntity.options.idArg} arg not found`);
+            const repo = this.orm.em.getRepository<{ scope?: ContentScope }>(affectedEntity.entity);
+            const id = args[affectedEntity.options.idArg];
+            const ids = Array.isArray(id) ? id : [id];
+            for (const id of ids) {
+                const row = await repo.findOneOrFail(id);
                 if (row.scope) {
-                    contentScope = row.scope;
+                    contentScopes.push([row.scope as ContentScope]);
                 } else {
-                    // eslint-disable-next-line @typescript-eslint/no-explicit-any
-                    const scoped = this.reflector.getAllAndOverride<ScopedEntityMeta>("scopedEntity", [
-                        affectedEntity.entity as EntityClass<unknown>,
-                    ]);
-                    if (!scoped) {
-                        return undefined;
-                    }
-                    contentScope = await scoped.fn(row);
-                }
-            } else if (affectedEntity.options.pageTreeNodeIdArg && args[affectedEntity.options.pageTreeNodeIdArg]) {
-                if (!args[affectedEntity.options.pageTreeNodeIdArg]) {
-                    throw new Error(`${affectedEntity.options.pageTreeNodeIdArg} arg not found`);
+                    const scoped = this.reflector.getAllAndOverride<ScopedEntityMeta>("scopedEntity", [affectedEntity.entity]);
+                    if (!scoped) throw new Error(`Entity ${affectedEntity.entity} is missing @ScopedEntity decorator`);
+                    const scopes = await scoped.fn(row);
+                    if (!scopes) throw new Error(`@ScopedEntity function for ${affectedEntity.entity} didn't return any scopes`);
+                    contentScopes.push(Array.isArray(scopes) ? scopes : [scopes]);
                 }
-                if (this.pageTreeService === undefined) {
-                    throw new Error("pageTreeNodeIdArg was given but no PageTreeModule is registered");
-                }
-                const node = await this.pageTreeService.createReadApi({ visibility: "all" }).getNode(args[affectedEntity.options.pageTreeNodeIdArg]);
-                if (!node) throw new Error("Can't find pageTreeNode");
-                contentScope = node.scope;
-            } else {
-                // TODO implement something more flexible that supports something like that: @AffectedEntity(Product, ProductEntityLoader)
-                throw new Error("idArg or pageTreeNodeIdArg is required");
             }
-            if (contentScope === undefined) throw new Error("Scope not found");
-            if (args.scope) {
-                // args.scope also exists, check if they match
-                if (!isEqual(args.scope, contentScope)) {
-                    throw new Error("Content Scope from arg doesn't match affectedEntity scope, usually you only need one of them");
-                }
+        } else if (affectedEntity.options.pageTreeNodeIdArg && args[affectedEntity.options.pageTreeNodeIdArg]) {
+            if (!args[affectedEntity.options.pageTreeNodeIdArg]) throw new Error(`${affectedEntity.options.pageTreeNodeIdArg} arg not found`);
+            if (this.pageTreeService === undefined) throw new Error("pageTreeNodeIdArg was given but no PageTreeModule is registered");
+            const pageTreeApi = await this.pageTreeService.createReadApi({ visibility: "all" });
+            const id = args[affectedEntity.options.pageTreeNodeIdArg];
+            const ids = Array.isArray(id) ? id : [id];
+            for (const id of ids) {
+                const node = await pageTreeApi.getNode(id);
+                if (!node) throw new Error("Can't find pageTreeNode");
+                if (!node.scope) throw new Error("PageTreeNode doesn't have a scope");
+                contentScopes.push([node.scope as ContentScope]);
             }
-            return contentScope;
-        }
-        if (args.scope) {
-            return args.scope;
+        } else {
+            // TODO implement something more flexible that supports something like that: @AffectedEntity(Product, ProductEntityLoader)
+            throw new Error("idArg or pageTreeNodeIdArg is required");
         }
-    }
-
-    async inferScopesFromExecutionContext(context: ExecutionContext): Promise<ContentScope[] | undefined> {
-        const scope = await this.inferScopeFromExecutionContext(context);
-        if (scope === undefined) return scope;
-        return Array.isArray(scope) ? scope : [scope];
+        return contentScopes;
     }
 
     // eslint-disable-next-line @typescript-eslint/no-explicit-any

diff --git a/packages/api/cms-api/src/user-permissions/decorators/affected-entity.decorator.ts b/packages/api/cms-api/src/user-permissions/decorators/affected-entity.decorator.ts
@@ -1,20 +1,24 @@
-import { EntityName } from "@mikro-orm/core";
-import { CustomDecorator, SetMetadata } from "@nestjs/common";
+import { EntityClass, EntityName } from "@mikro-orm/core";
 
 export interface AffectedEntityOptions {
     idArg?: string;
     pageTreeNodeIdArg?: string;
 }
-export interface AffectedEntityMeta {
+export type AffectedEntityMeta = {
     // eslint-disable-next-line @typescript-eslint/no-explicit-any
-    entity: EntityName<any>; //TODO
+    entity: EntityClass<object>;
     options: AffectedEntityOptions;
-}
+};
 
 export const AffectedEntity = (
     // eslint-disable-next-line @typescript-eslint/no-explicit-any
     entity: EntityName<any>,
     { idArg, pageTreeNodeIdArg }: AffectedEntityOptions = { idArg: "id" },
-): CustomDecorator<string> => {
-    return SetMetadata("affectedEntity", { entity, options: { idArg, pageTreeNodeIdArg } });
+): MethodDecorator => {
+    return (target: object, key: string | symbol, descriptor: PropertyDescriptor) => {
+        const metadata = Reflect.getOwnMetadata("affectedEntities", descriptor.value) || [];
+        metadata.push({ entity, options: { idArg, pageTreeNodeIdArg } });
+        Reflect.defineMetadata("affectedEntities", metadata, descriptor.value);
+        return descriptor;
+    };
 };