From d2b45783eaae2255c48311e31fb796e7b200c36d Mon Sep 17 00:00:00 2001 From: fengmk2 Date: Thu, 6 Mar 2025 10:40:00 +0000 Subject: [PATCH] feat(server): use zod parse to impl input validation (#10566) close CLOUD-124 --- .../__tests__/nestjs/error-handler.spec.ts | 113 ++++++++++++++++++ packages/backend/server/src/base/error/def.ts | 6 + .../server/src/base/error/errors.gen.ts | 13 +- .../server/src/base/nestjs/exception.ts | 6 + .../server/src/models/common/schema.ts | 3 + packages/backend/server/src/schema.gql | 1 + 6 files changed, 141 insertions(+), 1 deletion(-) create mode 100644 packages/backend/server/src/models/common/schema.ts diff --git a/packages/backend/server/src/__tests__/nestjs/error-handler.spec.ts b/packages/backend/server/src/__tests__/nestjs/error-handler.spec.ts index 934473503d..b19ce6e0e3 100644 --- a/packages/backend/server/src/__tests__/nestjs/error-handler.spec.ts +++ b/packages/backend/server/src/__tests__/nestjs/error-handler.spec.ts @@ -1,20 +1,24 @@ import { applyDecorators, + Body, Controller, Get, HttpStatus, INestApplication, Logger, LoggerService, + Post, } from '@nestjs/common'; import { Args, Mutation, Query, Resolver } from '@nestjs/graphql'; import { + MessageBody, SubscribeMessage as RawSubscribeMessage, WebSocketGateway, } from '@nestjs/websockets'; import testFn, { TestFn } from 'ava'; import Sinon from 'sinon'; import request from 'supertest'; +import { z } from 'zod'; import { AccessDenied, @@ -22,8 +26,14 @@ import { UserFriendlyError, } from '../../base'; import { Public } from '../../core/auth'; +import { EmailSchema } from '../../models/common/schema'; import { createTestingApp } from '../utils'; +const TestSchema = z.object({ + email: EmailSchema, + foo: z.string().trim().min(1).optional(), +}); + @Public() @Resolver(() => String) class TestResolver { @@ -40,6 +50,12 @@ class TestResolver { return this.greating; } + @Mutation(() => String) + validate(@Args('email') email: string) { + const input = TestSchema.parse({ email }); + return input.email; + } + @Query(() => String) errorQuery() { throw new AccessDenied(); @@ -68,6 +84,12 @@ class TestController { throwUnknownError() { throw new Error('Unknown error'); } + + @Post('/validate') + validate(@Body() body: { email: string }) { + const input = TestSchema.parse(body); + return input; + } } const SubscribeMessage = (event: string) => @@ -91,6 +113,12 @@ class TestGateway { async throwUnknownError() { throw new Error('Unknown error'); } + + @SubscribeMessage('event:validate') + async validate(@MessageBody() body: { email: string }) { + const input = TestSchema.parse(body); + return input; + } } const test = testFn as TestFn<{ @@ -147,6 +175,30 @@ test('should be able to handle unknown internal error in graphql query', async t t.true(t.context.logger.error.calledOnceWith('internal_server_error')); }); +test('should be able to handle validation error in graphql query', async t => { + const res = await gql( + t.context.app, + `mutation { validate(email: "invalid-email") }` + ); + const err = res.body.errors[0]; + t.is( + err.message, + `Validation error, errors: [ + { + "validation": "email", + "code": "invalid_string", + "message": "Invalid email", + "path": [ + "email" + ] + } +]` + ); + t.is(err.extensions.status, HttpStatus.BAD_REQUEST); + t.is(err.extensions.name, 'VALIDATION_ERROR'); + t.true(t.context.logger.error.notCalled); +}); + test('should be able to respond request', async t => { const res = await request(t.context.app.getHttpServer()) .get('/ok') @@ -179,6 +231,42 @@ test('should be able to handle unknown internal error in http request', async t ); }); +test('should be able to handle validation error in http request', async t => { + const res = await request(t.context.app.getHttpServer()) + .post('/validate') + .send({ email: 'invalid-email', foo: '' }) + .expect(HttpStatus.BAD_REQUEST); + t.is( + res.body.message, + `Validation error, errors: [ + { + "validation": "email", + "code": "invalid_string", + "message": "Invalid email", + "path": [ + "email" + ] + }, + { + "code": "too_small", + "minimum": 1, + "type": "string", + "inclusive": true, + "exact": false, + "message": "String must contain at least 1 character(s)", + "path": [ + "foo" + ] + } +]` + ); + t.is(res.body.name, 'VALIDATION_ERROR'); + t.is(res.body.type, 'INVALID_INPUT'); + t.is(res.body.code, 'Bad Request'); + t.truthy(res.body.data.errors); + t.true(t.context.logger.error.notCalled); +}); + // Hard to test through websocket, will call event handler directly test('should be able to response websocket event', async t => { const gateway = t.context.app.get(TestGateway); @@ -208,3 +296,28 @@ test('should be able to handle unknown internal error in websocket event', async t.is(error.name, 'INTERNAL_SERVER_ERROR'); t.true(t.context.logger.error.calledOnceWith('internal_server_error')); }); + +test('should be able to handle validation error in graphql mutation', async t => { + const gateway = t.context.app.get(TestGateway); + + const { error } = (await gateway.validate({ + email: 'invalid-email', + })) as unknown as { + error: UserFriendlyError; + }; + t.is( + error.message, + `Validation error, errors: [ + { + "validation": "email", + "code": "invalid_string", + "message": "Invalid email", + "path": [ + "email" + ] + } +]` + ); + t.is(error.name, 'VALIDATION_ERROR'); + t.true(t.context.logger.error.notCalled); +}); diff --git a/packages/backend/server/src/base/error/def.ts b/packages/backend/server/src/base/error/def.ts index 1dc178c0b9..75fd8968c1 100644 --- a/packages/backend/server/src/base/error/def.ts +++ b/packages/backend/server/src/base/error/def.ts @@ -265,6 +265,12 @@ export const USER_FRIENDLY_ERRORS = { message: ({ max }) => `Query is too long, max length is ${max}.`, }, + validation_error: { + type: 'invalid_input', + args: { errors: 'string' }, + message: ({ errors }) => `Validation error, errors: ${errors}`, + }, + // User Errors user_not_found: { type: 'resource_not_found', diff --git a/packages/backend/server/src/base/error/errors.gen.ts b/packages/backend/server/src/base/error/errors.gen.ts index 860debf2ce..bd26f0903d 100644 --- a/packages/backend/server/src/base/error/errors.gen.ts +++ b/packages/backend/server/src/base/error/errors.gen.ts @@ -48,6 +48,16 @@ export class QueryTooLong extends UserFriendlyError { super('invalid_input', 'query_too_long', message, args); } } +@ObjectType() +class ValidationErrorDataType { + @Field() errors!: string +} + +export class ValidationError extends UserFriendlyError { + constructor(args: ValidationErrorDataType, message?: string | ((args: ValidationErrorDataType) => string)) { + super('invalid_input', 'validation_error', message, args); + } +} export class UserNotFound extends UserFriendlyError { constructor(message?: string) { @@ -846,6 +856,7 @@ export enum ErrorNames { BAD_REQUEST, GRAPHQL_BAD_REQUEST, QUERY_TOO_LONG, + VALIDATION_ERROR, USER_NOT_FOUND, USER_AVATAR_NOT_FOUND, EMAIL_ALREADY_USED, @@ -954,5 +965,5 @@ registerEnumType(ErrorNames, { export const ErrorDataUnionType = createUnionType({ name: 'ErrorDataUnion', types: () => - [GraphqlBadRequestDataType, QueryTooLongDataType, WrongSignInCredentialsDataType, UnknownOauthProviderDataType, InvalidOauthCallbackCodeDataType, MissingOauthQueryParameterDataType, InvalidEmailDataType, InvalidPasswordLengthDataType, WorkspacePermissionNotFoundDataType, SpaceNotFoundDataType, MemberNotFoundInSpaceDataType, NotInSpaceDataType, AlreadyInSpaceDataType, SpaceAccessDeniedDataType, SpaceOwnerNotFoundDataType, SpaceShouldHaveOnlyOneOwnerDataType, DocNotFoundDataType, DocActionDeniedDataType, DocUpdateBlockedDataType, VersionRejectedDataType, InvalidHistoryTimestampDataType, DocHistoryNotFoundDataType, BlobNotFoundDataType, ExpectToGrantDocUserRolesDataType, ExpectToRevokeDocUserRolesDataType, ExpectToUpdateDocUserRoleDataType, UnsupportedSubscriptionPlanDataType, SubscriptionAlreadyExistsDataType, SubscriptionNotExistsDataType, SameSubscriptionRecurringDataType, SubscriptionPlanNotFoundDataType, CopilotDocNotFoundDataType, CopilotMessageNotFoundDataType, CopilotPromptNotFoundDataType, CopilotProviderSideErrorDataType, CopilotInvalidContextDataType, CopilotContextFileNotSupportedDataType, CopilotFailedToModifyContextDataType, CopilotFailedToMatchContextDataType, RuntimeConfigNotFoundDataType, InvalidRuntimeConfigTypeDataType, InvalidLicenseUpdateParamsDataType, WorkspaceMembersExceedLimitToDowngradeDataType, UnsupportedClientVersionDataType] as const, + [GraphqlBadRequestDataType, QueryTooLongDataType, ValidationErrorDataType, WrongSignInCredentialsDataType, UnknownOauthProviderDataType, InvalidOauthCallbackCodeDataType, MissingOauthQueryParameterDataType, InvalidEmailDataType, InvalidPasswordLengthDataType, WorkspacePermissionNotFoundDataType, SpaceNotFoundDataType, MemberNotFoundInSpaceDataType, NotInSpaceDataType, AlreadyInSpaceDataType, SpaceAccessDeniedDataType, SpaceOwnerNotFoundDataType, SpaceShouldHaveOnlyOneOwnerDataType, DocNotFoundDataType, DocActionDeniedDataType, DocUpdateBlockedDataType, VersionRejectedDataType, InvalidHistoryTimestampDataType, DocHistoryNotFoundDataType, BlobNotFoundDataType, ExpectToGrantDocUserRolesDataType, ExpectToRevokeDocUserRolesDataType, ExpectToUpdateDocUserRoleDataType, UnsupportedSubscriptionPlanDataType, SubscriptionAlreadyExistsDataType, SubscriptionNotExistsDataType, SameSubscriptionRecurringDataType, SubscriptionPlanNotFoundDataType, CopilotDocNotFoundDataType, CopilotMessageNotFoundDataType, CopilotPromptNotFoundDataType, CopilotProviderSideErrorDataType, CopilotInvalidContextDataType, CopilotContextFileNotSupportedDataType, CopilotFailedToModifyContextDataType, CopilotFailedToMatchContextDataType, RuntimeConfigNotFoundDataType, InvalidRuntimeConfigTypeDataType, InvalidLicenseUpdateParamsDataType, WorkspaceMembersExceedLimitToDowngradeDataType, UnsupportedClientVersionDataType] as const, }); diff --git a/packages/backend/server/src/base/nestjs/exception.ts b/packages/backend/server/src/base/nestjs/exception.ts index 8baa67a35a..1d9606b173 100644 --- a/packages/backend/server/src/base/nestjs/exception.ts +++ b/packages/backend/server/src/base/nestjs/exception.ts @@ -13,6 +13,7 @@ import { Response } from 'express'; import { GraphQLError } from 'graphql'; import { of } from 'rxjs'; import { Socket } from 'socket.io'; +import { ZodError } from 'zod'; import { GraphqlBadRequest, @@ -20,6 +21,7 @@ import { NotFound, TooManyRequest, UserFriendlyError, + ValidationError, } from '../error'; import { metrics } from '../metrics'; import { getRequestIdFromHost } from '../utils'; @@ -52,6 +54,10 @@ export function mapAnyError(error: any): UserFriendlyError { return new TooManyRequest(); } else if (error instanceof NotFoundException) { return new NotFound(); + } else if (error instanceof ZodError) { + return new ValidationError({ + errors: error.message, + }); } else { const e = new InternalServerError(); e.cause = error; diff --git a/packages/backend/server/src/models/common/schema.ts b/packages/backend/server/src/models/common/schema.ts new file mode 100644 index 0000000000..6bbeae9b5d --- /dev/null +++ b/packages/backend/server/src/models/common/schema.ts @@ -0,0 +1,3 @@ +import { z } from 'zod'; + +export const EmailSchema = z.string().trim().email(); diff --git a/packages/backend/server/src/schema.gql b/packages/backend/server/src/schema.gql index 5400fdeaa4..c9268e6381 100644 --- a/packages/backend/server/src/schema.gql +++ b/packages/backend/server/src/schema.gql @@ -431,6 +431,7 @@ enum ErrorNames { UNSUPPORTED_SUBSCRIPTION_PLAN USER_AVATAR_NOT_FOUND USER_NOT_FOUND + VALIDATION_ERROR VERSION_REJECTED WORKSPACE_ID_REQUIRED_FOR_TEAM_SUBSCRIPTION WORKSPACE_ID_REQUIRED_TO_UPDATE_TEAM_SUBSCRIPTION