From 0580557f613a2985dbd9bafe7df59835a5658664 Mon Sep 17 00:00:00 2001 From: Joo-Byungho Date: Mon, 13 Jan 2025 17:17:46 +0900 Subject: [PATCH 1/9] Feat: Implement graphql query guard - Can add inaccessible field from query to block attack request --- src/common/decorators/option.decorator.ts | 2 +- .../decorators/query-guard.decorator.ts | 20 +++++++++ src/common/guards/graphql-query.guard.ts | 44 +++++++++++++++++++ src/user/user.resolver.ts | 3 ++ 4 files changed, 68 insertions(+), 1 deletion(-) create mode 100644 src/common/decorators/query-guard.decorator.ts create mode 100644 src/common/guards/graphql-query.guard.ts diff --git a/src/common/decorators/option.decorator.ts b/src/common/decorators/option.decorator.ts index cedd5ca..3e136a3 100644 --- a/src/common/decorators/option.decorator.ts +++ b/src/common/decorators/option.decorator.ts @@ -120,7 +120,7 @@ export function getOptionFromGqlQuery( ); } -const getCurrentGraphQLQuery = (ctx: GqlExecutionContext) => { +export const getCurrentGraphQLQuery = (ctx: GqlExecutionContext) => { const { fieldName, path } = ctx.getArgByIndex(3) as { fieldName: string; path: { key: string }; diff --git a/src/common/decorators/query-guard.decorator.ts b/src/common/decorators/query-guard.decorator.ts new file mode 100644 index 0000000..5b5c5d5 --- /dev/null +++ b/src/common/decorators/query-guard.decorator.ts @@ -0,0 +1,20 @@ +import { SetMetadata, UseGuards, applyDecorators } from '@nestjs/common'; + +import { FindOptionsSelect } from 'typeorm'; + +import { GraphqlQueryGuard } from '../guards/graphql-query.guard'; + +export type ClassConstructor = new (...args: unknown[]) => T; + +export const PERMISSION = Symbol('PERMISSION'); +export const INSTANCE = Symbol('INSTANCE'); + +export const UseQueryGuard = ( + instance: T, + permission: FindOptionsSelect>, +) => + applyDecorators( + SetMetadata(INSTANCE, instance), + SetMetadata(PERMISSION, permission), + UseGuards(GraphqlQueryGuard), + ); diff --git a/src/common/guards/graphql-query.guard.ts b/src/common/guards/graphql-query.guard.ts new file mode 100644 index 0000000..85919b5 --- /dev/null +++ b/src/common/guards/graphql-query.guard.ts @@ -0,0 +1,44 @@ +import { ExecutionContext, Injectable } from '@nestjs/common'; +import { Reflector } from '@nestjs/core'; +import { GqlExecutionContext } from '@nestjs/graphql'; + +import { DataSource, FindOptionsSelect } from 'typeorm'; + +import { + getCurrentGraphQLQuery, + getOptionFromGqlQuery, +} from '../decorators/option.decorator'; +import { + ClassConstructor, + INSTANCE, + PERMISSION, +} from '../decorators/query-guard.decorator'; +import { GetInfoFromQueryProps } from '../graphql/utils/types'; + +@Injectable() +export class GraphqlQueryGuard { + constructor( + private reflector: Reflector, + private readonly dataSource: DataSource, + ) {} + + canActivate(context: ExecutionContext): boolean { + const permission = this.reflector.get>( + PERMISSION, + context.getHandler(), + ); + + const entity = this.reflector.get(INSTANCE, context.getHandler()); + const repository = this.dataSource.getRepository(entity); + + const ctx = GqlExecutionContext.create(context); + const query = getCurrentGraphQLQuery(ctx); + + const { select }: GetInfoFromQueryProps = getOptionFromGqlQuery.call( + repository, + query, + ); + + return Object.keys(select).every((key) => permission[key]); + } +} diff --git a/src/user/user.resolver.ts b/src/user/user.resolver.ts index 2a51a0e..b2407df 100644 --- a/src/user/user.resolver.ts +++ b/src/user/user.resolver.ts @@ -5,6 +5,7 @@ import GraphQLJSON from 'graphql-type-json'; import { CustomCache } from 'src/cache/custom-cache.decorator'; import { UseAuthGuard } from 'src/common/decorators/auth-guard.decorator'; import { GraphQLQueryToOption } from 'src/common/decorators/option.decorator'; +import { UseQueryGuard } from 'src/common/decorators/query-guard.decorator'; import { UseRepositoryInterceptor } from 'src/common/decorators/repository-interceptor.decorator'; import { GetManyInput, GetOneInput } from 'src/common/graphql/custom.input'; import { GetInfoFromQueryProps } from 'src/common/graphql/utils/types'; @@ -20,6 +21,7 @@ export class UserResolver { @Query(() => GetUserType) @UseAuthGuard('admin') + @UseQueryGuard(User, { username: true }) @UseRepositoryInterceptor(User) @CustomCache({ logger: console.log, ttl: 1000 }) getManyUserList( @@ -63,6 +65,7 @@ export class UserResolver { @Query(() => User) @UseAuthGuard() + @UseQueryGuard(User, { username: true }) @UseRepositoryInterceptor(User) getMe( @CurrentUser() user: User, From a408b2bc7045f558d493a1c81cfdc1792a34fc2e Mon Sep 17 00:00:00 2001 From: Joo-Byungho Date: Mon, 13 Jan 2025 17:30:14 +0900 Subject: [PATCH 2/9] Feat: Remove has count type --- generator/templates/resolver.hbs | 2 +- src/common/decorators/option.decorator.ts | 14 ++++---------- src/common/graphql/utils/types.ts | 1 - src/user/user.resolver.ts | 2 +- 4 files changed, 6 insertions(+), 13 deletions(-) diff --git a/generator/templates/resolver.hbs b/generator/templates/resolver.hbs index a28e399..0cc47af 100644 --- a/generator/templates/resolver.hbs +++ b/generator/templates/resolver.hbs @@ -20,7 +20,7 @@ export class {{pascalCase tableName}}Resolver { @UseRepositoryInterceptor({{pascalCase tableName}}) getMany{{pascalCase tableName}}List( @Args({ name: 'input', nullable: true }) condition: GetManyInput<{{pascalCase tableName}}>, - @GraphQLQueryToOption<{{pascalCase tableName}}>(true) + @GraphQLQueryToOption<{{pascalCase tableName}}>() option: GetInfoFromQueryProps<{{pascalCase tableName}}>, ) { return this.{{tableName}}Service.getMany({ ...condition, ...option }); diff --git a/src/common/decorators/option.decorator.ts b/src/common/decorators/option.decorator.ts index 3e136a3..2eb1c37 100644 --- a/src/common/decorators/option.decorator.ts +++ b/src/common/decorators/option.decorator.ts @@ -21,12 +21,11 @@ const addKeyValuesInObject = ({ relations, select, expandRelation, - hasCountType, }: AddKeyValueInObjectProps): GetInfoFromQueryProps => { if (stack.length) { let stackToString = stack.join('.'); - if (hasCountType) { + if (stack.length && stack[0] === DATA) { if (stack[0] !== DATA || (stack.length === 1 && stack[0] === DATA)) { return { relations, select }; } @@ -46,7 +45,6 @@ const addKeyValuesInObject = ({ export function getOptionFromGqlQuery( this: Repository, query: string, - hasCountType?: boolean, ): GetInfoFromQueryProps { const splitted = query.split('\n'); @@ -65,7 +63,7 @@ export function getOptionFromGqlQuery( if (line.includes('{')) { stack.push(replacedLine); - const isFirstLineDataType = hasCountType && replacedLine === DATA; + const isFirstLineDataType = replacedLine === DATA; if (!isFirstLineDataType) { lastMetadata = lastMetadata.relations.find( @@ -78,11 +76,9 @@ export function getOptionFromGqlQuery( relations: acc.relations, select: acc.select, expandRelation: true, - hasCountType, }); } else if (line.includes('}')) { - const hasDataTypeInStack = - hasCountType && stack.length && stack[0] === DATA; + const hasDataTypeInStack = stack.length && stack[0] === DATA; lastMetadata = stack.length < (hasDataTypeInStack ? 3 : 2) @@ -110,7 +106,6 @@ export function getOptionFromGqlQuery( stack: addedStack, relations: acc.relations, select: acc.select, - hasCountType, }); }, { @@ -159,7 +154,7 @@ export const getCurrentGraphQLQuery = (ctx: GqlExecutionContext) => { return stack.join('\n'); }; -export const GraphQLQueryToOption = (hasCountType?: boolean) => +export const GraphQLQueryToOption = () => createParamDecorator((_: unknown, context: ExecutionContext) => { const ctx = GqlExecutionContext.create(context); const request = ctx.getContext().req; @@ -175,7 +170,6 @@ export const GraphQLQueryToOption = (hasCountType?: boolean) => const queryOption: GetInfoFromQueryProps = getOptionFromGqlQuery.call( repository, query, - hasCountType, ); return queryOption; diff --git a/src/common/graphql/utils/types.ts b/src/common/graphql/utils/types.ts index 14aa848..04cf6bb 100644 --- a/src/common/graphql/utils/types.ts +++ b/src/common/graphql/utils/types.ts @@ -115,5 +115,4 @@ export interface AddKeyValueInObjectProps extends GetInfoFromQueryProps { stack: string[]; expandRelation?: boolean; - hasCountType?: boolean; } diff --git a/src/user/user.resolver.ts b/src/user/user.resolver.ts index b2407df..c62641e 100644 --- a/src/user/user.resolver.ts +++ b/src/user/user.resolver.ts @@ -27,7 +27,7 @@ export class UserResolver { getManyUserList( @Args({ name: 'input', nullable: true }) condition: GetManyInput, - @GraphQLQueryToOption(true) + @GraphQLQueryToOption() option: GetInfoFromQueryProps, ) { return this.userService.getMany({ ...condition, ...option }); From 83623c0b6bad3da563c9bc2394a507367798dbb7 Mon Sep 17 00:00:00 2001 From: Joo-Byungho Date: Mon, 13 Jan 2025 18:31:32 +0900 Subject: [PATCH 3/9] Feat: Add check permission --- src/common/guards/graphql-query.guard.ts | 25 ++++++++++++++++++------ src/user/user.resolver.ts | 3 +-- 2 files changed, 20 insertions(+), 8 deletions(-) diff --git a/src/common/guards/graphql-query.guard.ts b/src/common/guards/graphql-query.guard.ts index 85919b5..2466283 100644 --- a/src/common/guards/graphql-query.guard.ts +++ b/src/common/guards/graphql-query.guard.ts @@ -15,6 +15,21 @@ import { } from '../decorators/query-guard.decorator'; import { GetInfoFromQueryProps } from '../graphql/utils/types'; +const checkPermission = ( + permission: FindOptionsSelect>, + select: FindOptionsSelect>, +): boolean => { + return Object.entries(permission) + .filter((v) => !!v[1]) + .every(([key, value]) => { + if (typeof value === 'boolean') { + return select[key] ? false : true; + } + + return checkPermission(value, select[key]); + }); +}; + @Injectable() export class GraphqlQueryGuard { constructor( @@ -23,7 +38,7 @@ export class GraphqlQueryGuard { ) {} canActivate(context: ExecutionContext): boolean { - const permission = this.reflector.get>( + const permission = this.reflector.get>>( PERMISSION, context.getHandler(), ); @@ -34,11 +49,9 @@ export class GraphqlQueryGuard { const ctx = GqlExecutionContext.create(context); const query = getCurrentGraphQLQuery(ctx); - const { select }: GetInfoFromQueryProps = getOptionFromGqlQuery.call( - repository, - query, - ); + const { select }: GetInfoFromQueryProps> = + getOptionFromGqlQuery.call(repository, query); - return Object.keys(select).every((key) => permission[key]); + return checkPermission(permission, select); } } diff --git a/src/user/user.resolver.ts b/src/user/user.resolver.ts index c62641e..d40913b 100644 --- a/src/user/user.resolver.ts +++ b/src/user/user.resolver.ts @@ -21,8 +21,8 @@ export class UserResolver { @Query(() => GetUserType) @UseAuthGuard('admin') - @UseQueryGuard(User, { username: true }) @UseRepositoryInterceptor(User) + @UseQueryGuard(User, { username: true }) @CustomCache({ logger: console.log, ttl: 1000 }) getManyUserList( @Args({ name: 'input', nullable: true }) @@ -65,7 +65,6 @@ export class UserResolver { @Query(() => User) @UseAuthGuard() - @UseQueryGuard(User, { username: true }) @UseRepositoryInterceptor(User) getMe( @CurrentUser() user: User, From e3c15f12d33650bb7ae33b2669b78f191931940d Mon Sep 17 00:00:00 2001 From: Joo-Byungho Date: Mon, 13 Jan 2025 18:37:03 +0900 Subject: [PATCH 4/9] Chore: Remove test code --- src/user/user.resolver.ts | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/user/user.resolver.ts b/src/user/user.resolver.ts index d40913b..64bda12 100644 --- a/src/user/user.resolver.ts +++ b/src/user/user.resolver.ts @@ -5,7 +5,6 @@ import GraphQLJSON from 'graphql-type-json'; import { CustomCache } from 'src/cache/custom-cache.decorator'; import { UseAuthGuard } from 'src/common/decorators/auth-guard.decorator'; import { GraphQLQueryToOption } from 'src/common/decorators/option.decorator'; -import { UseQueryGuard } from 'src/common/decorators/query-guard.decorator'; import { UseRepositoryInterceptor } from 'src/common/decorators/repository-interceptor.decorator'; import { GetManyInput, GetOneInput } from 'src/common/graphql/custom.input'; import { GetInfoFromQueryProps } from 'src/common/graphql/utils/types'; @@ -22,7 +21,6 @@ export class UserResolver { @Query(() => GetUserType) @UseAuthGuard('admin') @UseRepositoryInterceptor(User) - @UseQueryGuard(User, { username: true }) @CustomCache({ logger: console.log, ttl: 1000 }) getManyUserList( @Args({ name: 'input', nullable: true }) From 98e94e51cf23d5bd3e305ae18a756c81d9f2ecc5 Mon Sep 17 00:00:00 2001 From: Joo-Byungho Date: Thu, 16 Jan 2025 16:10:55 +0900 Subject: [PATCH 5/9] Chore: Move md explanation --- README.md | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/README.md b/README.md index 2f56971..4e0c432 100644 --- a/README.md +++ b/README.md @@ -61,6 +61,11 @@ You can solve them with Sending JWT token in `Http Header` with the `Authorizati } ``` +#### Example of some protected GraphQL + +- getMe (must be authenticated) +- All methods generated by the generator (must be authenticated and must be admin) + ### GraphQL Query To Select and relations #### Dynamic Query Optimization @@ -75,11 +80,6 @@ You can solve them with Sending JWT token in `Http Header` with the `Authorizati - You can find example code in [/src/user/user.resolver.ts](/src/user/user.resolver.ts) -#### Example of some protected GraphQL - -- getMe (must be authenticated) -- All methods generated by the generator (must be authenticated and must be admin) - ## License MIT From 9c9207480b330f4c8db22a4d97490a5c4053710b Mon Sep 17 00:00:00 2001 From: Joo-Byungho Date: Thu, 16 Jan 2025 16:18:31 +0900 Subject: [PATCH 6/9] Chore: Change name of class --- src/common/decorators/query-guard.decorator.ts | 6 +++--- ...hql-query.guard.ts => graphql-query-permission.guard.ts} | 2 +- 2 files changed, 4 insertions(+), 4 deletions(-) rename src/common/guards/{graphql-query.guard.ts => graphql-query-permission.guard.ts} (95%) diff --git a/src/common/decorators/query-guard.decorator.ts b/src/common/decorators/query-guard.decorator.ts index 5b5c5d5..15a2603 100644 --- a/src/common/decorators/query-guard.decorator.ts +++ b/src/common/decorators/query-guard.decorator.ts @@ -2,19 +2,19 @@ import { SetMetadata, UseGuards, applyDecorators } from '@nestjs/common'; import { FindOptionsSelect } from 'typeorm'; -import { GraphqlQueryGuard } from '../guards/graphql-query.guard'; +import { GraphqlQueryPermissionGuard } from '../guards/graphql-query-permission.guard'; export type ClassConstructor = new (...args: unknown[]) => T; export const PERMISSION = Symbol('PERMISSION'); export const INSTANCE = Symbol('INSTANCE'); -export const UseQueryGuard = ( +export const UseQueryPermissionGuard = ( instance: T, permission: FindOptionsSelect>, ) => applyDecorators( SetMetadata(INSTANCE, instance), SetMetadata(PERMISSION, permission), - UseGuards(GraphqlQueryGuard), + UseGuards(GraphqlQueryPermissionGuard), ); diff --git a/src/common/guards/graphql-query.guard.ts b/src/common/guards/graphql-query-permission.guard.ts similarity index 95% rename from src/common/guards/graphql-query.guard.ts rename to src/common/guards/graphql-query-permission.guard.ts index 2466283..048101e 100644 --- a/src/common/guards/graphql-query.guard.ts +++ b/src/common/guards/graphql-query-permission.guard.ts @@ -31,7 +31,7 @@ const checkPermission = ( }; @Injectable() -export class GraphqlQueryGuard { +export class GraphqlQueryPermissionGuard { constructor( private reflector: Reflector, private readonly dataSource: DataSource, From e39ccb088081ba6fd65334fe049b722de7705b67 Mon Sep 17 00:00:00 2001 From: Joo-Byungho Date: Thu, 16 Jan 2025 16:31:43 +0900 Subject: [PATCH 7/9] Docs: Readme.md --- README.md | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+) diff --git a/README.md b/README.md index 4e0c432..66146e2 100644 --- a/README.md +++ b/README.md @@ -80,6 +80,28 @@ You can solve them with Sending JWT token in `Http Header` with the `Authorizati - You can find example code in [/src/user/user.resolver.ts](/src/user/user.resolver.ts) +### permission + +The [permission guard](/src/common/decorators/query-guard.decorator.ts) is used to block access to specific fields in client requests. + +#### Why it was created + +- In GraphQL, clients can request any field, which could expose sensitive information. This guard ensures that sensitive fields are protected. + +- It allows controlling access to specific fields based on the server's permissions. + +#### How to use + +```ts +@Query(()=>Some) +@UseQueryPermissionGuard(Some, { something: true }) +async getManySomeList(){ + return this.someService.getMany() +} +``` + +With this API, if the client request includes the field "something," a `Forbidden` error will be triggered. + ## License MIT From 1c90a326978b056aeaeb4f604dd686c562800d42 Mon Sep 17 00:00:00 2001 From: Joo-Byungho Date: Thu, 16 Jan 2025 16:37:18 +0900 Subject: [PATCH 8/9] Docs: Add note about permission guard --- README.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/README.md b/README.md index 66146e2..cdf0334 100644 --- a/README.md +++ b/README.md @@ -102,6 +102,10 @@ async getManySomeList(){ With this API, if the client request includes the field "something," a `Forbidden` error will be triggered. +#### Note + +There might be duplicate code when using this guard alongside `other interceptors`(name: `UseRepositoryInterceptor`) in this boilerplate. In such cases, you may need to adjust the code to ensure compatibility. + ## License MIT From 7cbadcb597d9680ce5eaf353a98e9b50d10cf82c Mon Sep 17 00:00:00 2001 From: Joo-Byungho Date: Wed, 22 Jan 2025 10:32:25 +0900 Subject: [PATCH 9/9] Fix: typo --- README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/README.md b/README.md index cdf0334..ffef196 100644 --- a/README.md +++ b/README.md @@ -80,7 +80,7 @@ You can solve them with Sending JWT token in `Http Header` with the `Authorizati - You can find example code in [/src/user/user.resolver.ts](/src/user/user.resolver.ts) -### permission +### Permission for specific field The [permission guard](/src/common/decorators/query-guard.decorator.ts) is used to block access to specific fields in client requests.