From 151e12188af1884ecad33f9230115ad81142ce4f Mon Sep 17 00:00:00 2001 From: Franz Unger Date: Mon, 26 Feb 2024 16:00:07 +0100 Subject: [PATCH] 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 <48853629+johnnyomair@users.noreply.github.com> --- .changeset/flat-onions-turn.md | 5 + demo/api/src/menus/main-menu-item.resolver.ts | 3 +- .../auth/user-permissions.guard.ts | 71 +++++++------- .../user-permissions/content-scope.service.ts | 98 +++++++++---------- .../decorators/affected-entity.decorator.ts | 18 ++-- 5 files changed, 101 insertions(+), 94 deletions(-) create mode 100644 .changeset/flat-onions-turn.md diff --git a/.changeset/flat-onions-turn.md b/.changeset/flat-onions-turn.md new file mode 100644 index 0000000000..5ed59252fa --- /dev/null +++ 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 index 3f3e0a4266..f2e4c0eefa 100644 --- a/demo/api/src/menus/main-menu-item.resolver.ts +++ b/demo/api/src/menus/main-menu-item.resolver.ts @@ -17,7 +17,6 @@ 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, @@ -25,6 +24,7 @@ export class MainMenuItemResolver { ) {} @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 index 5d9631a4ec..17eec92d00 100644 --- 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 { - 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("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(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(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() === "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 index 689863142e..00cb075cbf 100644 --- 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 { + async getScopesForPermissionCheck(context: ExecutionContext): Promise { + const contentScopes: ContentScope[][] = []; const args = await this.getArgs(context); - const affectedEntity = this.reflector.getAllAndOverride("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(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("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 { + return [...new Set((await this.getScopesForPermissionCheck(context)).flat())]; + } + + private async getContentScopesFromEntity(affectedEntity: AffectedEntityMeta, args: Record): Promise { + 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("scopedEntity", [ - affectedEntity.entity as EntityClass, - ]); - 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("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 { - 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 index 6ad3e86520..5454b3782a 100644 --- 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; //TODO + entity: EntityClass; options: AffectedEntityOptions; -} +}; export const AffectedEntity = ( // eslint-disable-next-line @typescript-eslint/no-explicit-any entity: EntityName, { idArg, pageTreeNodeIdArg }: AffectedEntityOptions = { idArg: "id" }, -): CustomDecorator => { - 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; + }; };