From 9744b8cf76ca6b0fd4194ddaefe0b65042dc7713 Mon Sep 17 00:00:00 2001 From: Zen Software Date: Wed, 4 May 2022 14:52:32 -0600 Subject: [PATCH] feat(api/guards): renamed RejectNestedCreateGuard to ForbidNestedCreateGuard & added options Created options for ForbidNestedCreateGuard of type { allow: string[] } to list fields to allow nested creates while forbidding all other nested creates within mutation arguments. This maximizes flexibility to describe what is safe to expose on the API surface layer. BREAKING CHANGE: Renamed RejectNestedCreateGuard to ForbidNestedCreateGuard --- apps/api/src/app/auth/auth.module.ts | 20 +----- .../guards/forbid-nested-create.guard.spec.ts | 28 +++++++++ .../auth/guards/forbid-nested-create.guard.ts | 63 +++++++++++++++++++ apps/api/src/app/auth/guards/gql.guard.ts | 4 +- apps/api/src/app/auth/guards/http.guard.ts | 4 +- apps/api/src/app/auth/guards/index.ts | 2 +- .../guards/reject-nested-create.guard.spec.ts | 36 ----------- .../auth/guards/reject-nested-create.guard.ts | 51 --------------- apps/api/src/app/graphql/resolvers/User.ts | 4 +- package.json | 2 +- tools/graphql-codegen/nest-resolvers.temp.js | 4 +- 11 files changed, 103 insertions(+), 115 deletions(-) create mode 100644 apps/api/src/app/auth/guards/forbid-nested-create.guard.spec.ts create mode 100644 apps/api/src/app/auth/guards/forbid-nested-create.guard.ts delete mode 100644 apps/api/src/app/auth/guards/reject-nested-create.guard.spec.ts delete mode 100644 apps/api/src/app/auth/guards/reject-nested-create.guard.ts diff --git a/apps/api/src/app/auth/auth.module.ts b/apps/api/src/app/auth/auth.module.ts index 31fb3c90a..d9cb53cda 100644 --- a/apps/api/src/app/auth/auth.module.ts +++ b/apps/api/src/app/auth/auth.module.ts @@ -4,27 +4,11 @@ import { PassportModule } from '@nestjs/passport'; import { JwtModule } from '../jwt'; import { PrismaModule } from '../prisma'; import { AuthService } from './auth.service'; -import { GqlGuard, GqlThrottlerGuard, HttpGuard, RejectNestedCreateGuard } from './guards'; import { JwtStrategy } from './jwt.strategy'; @Module({ imports: [JwtModule, PrismaModule, PassportModule.register({ defaultStrategy: 'jwt' })], - providers: [ - JwtStrategy, - AuthService, - GqlGuard, - GqlThrottlerGuard, - HttpGuard, - RejectNestedCreateGuard, - ], - exports: [ - JwtModule, - PassportModule, - AuthService, - GqlGuard, - GqlThrottlerGuard, - HttpGuard, - RejectNestedCreateGuard, - ], + providers: [JwtStrategy, AuthService], + exports: [JwtModule, PassportModule, AuthService], }) export class ZenAuthModule {} diff --git a/apps/api/src/app/auth/guards/forbid-nested-create.guard.spec.ts b/apps/api/src/app/auth/guards/forbid-nested-create.guard.spec.ts new file mode 100644 index 000000000..3ecc40cca --- /dev/null +++ b/apps/api/src/app/auth/guards/forbid-nested-create.guard.spec.ts @@ -0,0 +1,28 @@ +import { containsNestedCreate } from './forbid-nested-create.guard'; + +describe('ForbidNestedCreateGuard', () => { + it(`determines if args contains a "create" field`, () => { + const args1 = { + data: { + text: 'sample', + author: { connect: { id: 1 } }, + comment: { create: { text: 'commenting' } }, + }, + }; + expect(containsNestedCreate(args1)).toEqual(true); + expect(containsNestedCreate(args1, { allow: ['comment'] })).toEqual(false); + + const args2 = { + data: { + stub: null, + stub2: undefined, + author: { create: { username: 'user1' } }, + comment: { create: { text: 'commenting' } }, + }, + }; + expect(containsNestedCreate(args2)).toEqual(true); + expect(containsNestedCreate(args2, { allow: ['author'] })).toEqual(true); + expect(containsNestedCreate(args2, { allow: ['comment'] })).toEqual(true); + expect(containsNestedCreate(args2, { allow: ['author', 'comment'] })).toEqual(false); + }); +}); diff --git a/apps/api/src/app/auth/guards/forbid-nested-create.guard.ts b/apps/api/src/app/auth/guards/forbid-nested-create.guard.ts new file mode 100644 index 000000000..ee01212d9 --- /dev/null +++ b/apps/api/src/app/auth/guards/forbid-nested-create.guard.ts @@ -0,0 +1,63 @@ +import { CanActivate, ExecutionContext, HttpException, Logger, mixin } from '@nestjs/common'; +import { GqlExecutionContext } from '@nestjs/graphql'; + +type Options = { allow: string[] }; + +export function containsNestedCreate(args: any, options: Options = undefined) { + if (args !== null && args !== undefined) { + for (const [key, value] of Object.entries(args)) { + if (options?.allow.includes(key)) { + continue; + } + + if (key === 'create') { + return true; + } + + if (typeof value === 'object' && containsNestedCreate(value, options) === true) { + return true; + } + } + } + + return false; +} + +/** + * Rejects mutations with nested create arguments + */ +export const ForbidNestedCreateGuard = (options: Options = undefined) => { + class ForbidNestedCreateGuardMixin implements CanActivate { + async canActivate(context: ExecutionContext) { + const ctx = GqlExecutionContext.create(context); + + if (ctx.getInfo()?.operation?.operation === 'mutation') { + const args = ctx.getArgs(); + + if (containsNestedCreate(args, options)) { + const errorMessage = 'Nested create arguments for mutations are forbidden'; + const req = ctx.getContext()?.req; + + Logger.error(errorMessage, { + userId: req?.user?.id, + ip: req?.ip, + class: ctx.getClass()?.name, + handler: ctx.getHandler()?.name, + args: args?.data, + }); + + throw new HttpException(errorMessage, 403); + } + } + + return true; + } + + getRequest(context: ExecutionContext) { + const ctx = GqlExecutionContext.create(context); + return ctx.getContext().req; + } + } + + return mixin(ForbidNestedCreateGuardMixin); +}; diff --git a/apps/api/src/app/auth/guards/gql.guard.ts b/apps/api/src/app/auth/guards/gql.guard.ts index 723ff41be..ceea09be8 100644 --- a/apps/api/src/app/auth/guards/gql.guard.ts +++ b/apps/api/src/app/auth/guards/gql.guard.ts @@ -1,4 +1,4 @@ -import { CanActivate, ExecutionContext, Injectable } from '@nestjs/common'; +import { ExecutionContext, Injectable } from '@nestjs/common'; import { Reflector } from '@nestjs/core'; import { GqlExecutionContext } from '@nestjs/graphql'; import { AuthGuard } from '@nestjs/passport'; @@ -12,7 +12,7 @@ import { authLogic } from './auth-logic'; * Imitates RBAC rules for [ASP.NET Core](https://docs.microsoft.com/en-us/aspnet/core/security/authorization/roles?view=aspnetcore-6.0). * **Super** users are granted unrestricted access. */ -export class GqlGuard extends AuthGuard('jwt') implements CanActivate { +export class GqlGuard extends AuthGuard('jwt') { constructor(private readonly reflector: Reflector) { super(); } diff --git a/apps/api/src/app/auth/guards/http.guard.ts b/apps/api/src/app/auth/guards/http.guard.ts index e2f73e661..37cb207bc 100644 --- a/apps/api/src/app/auth/guards/http.guard.ts +++ b/apps/api/src/app/auth/guards/http.guard.ts @@ -1,4 +1,4 @@ -import { CanActivate, ExecutionContext, Injectable } from '@nestjs/common'; +import { ExecutionContext, Injectable } from '@nestjs/common'; import { Reflector } from '@nestjs/core'; import { AuthGuard } from '@nestjs/passport'; import { Role } from '@prisma/client'; @@ -11,7 +11,7 @@ import { authLogic } from './auth-logic'; * Imitates RBAC rules for [ASP.NET Core](https://docs.microsoft.com/en-us/aspnet/core/security/authorization/roles?view=aspnetcore-6.0). * **Super** users are granted unrestricted access. */ -export class HttpGuard extends AuthGuard('jwt') implements CanActivate { +export class HttpGuard extends AuthGuard('jwt') { constructor(private readonly reflector: Reflector) { super(); } diff --git a/apps/api/src/app/auth/guards/index.ts b/apps/api/src/app/auth/guards/index.ts index 8cc78a678..6ef171954 100644 --- a/apps/api/src/app/auth/guards/index.ts +++ b/apps/api/src/app/auth/guards/index.ts @@ -1,4 +1,4 @@ export * from './gql-throttle.guard'; export * from './gql.guard'; export * from './http.guard'; -export { RejectNestedCreateGuard } from './reject-nested-create.guard'; +export { ForbidNestedCreateGuard } from './forbid-nested-create.guard'; diff --git a/apps/api/src/app/auth/guards/reject-nested-create.guard.spec.ts b/apps/api/src/app/auth/guards/reject-nested-create.guard.spec.ts deleted file mode 100644 index 28a6e4548..000000000 --- a/apps/api/src/app/auth/guards/reject-nested-create.guard.spec.ts +++ /dev/null @@ -1,36 +0,0 @@ -import { containsNestedCreate } from './reject-nested-create.guard'; - -describe('RejectNestedCreateGuard', () => { - it(`determines args contains a "create" field`, () => { - const args = { - data: { - text: 'sample', - published: true, - stub: null, - stub2: undefined, - author: { - create: { - username: 'mean_human', - password: '12345678', - email: 'you@got.hacked', - roles: ['Super'], - }, - }, - }, - }; - - expect(containsNestedCreate(args)).toEqual(true); - }); - - it(`determines args does not contain a "create" field`, () => { - const args = { - data: { - text: 'sample', - published: true, - author: { connect: { id: 1 } }, - }, - }; - - expect(containsNestedCreate(args)).toEqual(false); - }); -}); diff --git a/apps/api/src/app/auth/guards/reject-nested-create.guard.ts b/apps/api/src/app/auth/guards/reject-nested-create.guard.ts deleted file mode 100644 index 73e0577c1..000000000 --- a/apps/api/src/app/auth/guards/reject-nested-create.guard.ts +++ /dev/null @@ -1,51 +0,0 @@ -import { CanActivate, ExecutionContext, HttpException, Injectable, Logger } from '@nestjs/common'; -import { GqlExecutionContext } from '@nestjs/graphql'; - -export function containsNestedCreate(args: any) { - if (args !== null && args !== undefined) { - for (const [key, value] of Object.entries(args)) { - if (key === 'create') { - return true; - } - - if (typeof value === 'object' && containsNestedCreate(value) === true) { - return true; - } - } - } - - return false; -} - -@Injectable() -/** - * Rejects mutations with nested create argument - */ -export class RejectNestedCreateGuard implements CanActivate { - async canActivate(context: ExecutionContext) { - const ctx = GqlExecutionContext.create(context); - - if (ctx.getInfo()?.operation?.operation === 'mutation') { - const args = ctx.getArgs(); - if (containsNestedCreate(args)) { - const errorMessage = 'Nested create arguments for mutations are forbidden'; - const req = ctx.getContext()?.req; - Logger.error(errorMessage, { - userId: req?.user?.id, - ip: req?.ip, - class: ctx.getClass()?.name, - handler: ctx.getHandler()?.name, - args: args?.data, - }); - throw new HttpException(errorMessage, 403); - } - } - - return true; - } - - getRequest(context: ExecutionContext) { - const ctx = GqlExecutionContext.create(context); - return ctx.getContext().req; - } -} diff --git a/apps/api/src/app/graphql/resolvers/User.ts b/apps/api/src/app/graphql/resolvers/User.ts index 01f57fa92..9a552d97e 100644 --- a/apps/api/src/app/graphql/resolvers/User.ts +++ b/apps/api/src/app/graphql/resolvers/User.ts @@ -10,7 +10,7 @@ import { Resolver, } from '@nestjs/graphql'; -import { GqlGuard, RejectNestedCreateGuard, Roles } from '../../auth'; +import { ForbidNestedCreateGuard, GqlGuard, Roles } from '../../auth'; import { PrismaSelectArgs } from '../../prisma'; import resolvers from '../generated/User/resolvers'; @@ -28,7 +28,7 @@ export const typeDefs = null; // `; @Resolver('User') -@UseGuards(GqlGuard, RejectNestedCreateGuard) +@UseGuards(GqlGuard, ForbidNestedCreateGuard()) @Roles('Super') export class UserResolver { @ResolveField() diff --git a/package.json b/package.json index 96421b09c..81273dedc 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "zen", - "version": "4.1.0", + "version": "4.2.0", "license": "MIT", "private": true, "scripts": { diff --git a/tools/graphql-codegen/nest-resolvers.temp.js b/tools/graphql-codegen/nest-resolvers.temp.js index 7b3d15d86..f30fad4d1 100644 --- a/tools/graphql-codegen/nest-resolvers.temp.js +++ b/tools/graphql-codegen/nest-resolvers.temp.js @@ -2,7 +2,7 @@ module.exports = (prismaName, querySource, mutationSource) => { return `import { UseGuards } from '@nestjs/common'; import { Args, Context, Info, Mutation, Parent, Query, Resolver } from '@nestjs/graphql'; -import { GqlGuard, RejectNestedCreateGuard, Roles } from '../../auth'; +import { GqlGuard, ForbidNestedCreateGuard, Roles } from '../../auth'; import { PrismaSelectArgs } from '../../prisma'; import resolvers from '../generated/${prismaName}/resolvers'; @@ -20,7 +20,7 @@ export const typeDefs = null; // \`; @Resolver('${prismaName}') -@UseGuards(GqlGuard, RejectNestedCreateGuard) +@UseGuards(GqlGuard, ForbidNestedCreateGuard()) @Roles('Super') export class ${prismaName}Resolver { ${querySource}${mutationSource}