From 7c605fc2f94c5cc6cfaf007ded18632b3f2757fe Mon Sep 17 00:00:00 2001 From: bosiraphael <71827178+bosiraphael@users.noreply.github.com> Date: Tue, 30 Apr 2024 14:36:33 +0200 Subject: [PATCH] 4002 prevent user from creating twice the same blocklist item (#5213) Closes #4002 --- .../SettingsAccountsEmailsBlocklistInput.tsx | 45 +++--- ...SettingsAccountsEmailsBlocklistSection.tsx | 1 + .../workspace-pre-query-hook.config.ts | 4 + .../workspace-query-runner.service.ts | 18 ++- .../blocklist-create-many.pre-query.hook.ts | 44 ++---- .../blocklist-update-many.pre-query.hook.ts | 12 ++ .../blocklist-update-one.pre-query.hook.ts | 28 ++++ .../connected-account-query-hook.module.ts | 13 +- .../repositories/blocklist.repository.ts | 17 ++ .../blocklist/blocklist-validation.module.ts | 18 +++ .../blocklist/blocklist-validation.service.ts | 146 ++++++++++++++++++ .../message-find-many.pre-query.hook.ts | 4 +- 12 files changed, 297 insertions(+), 53 deletions(-) create mode 100644 packages/twenty-server/src/modules/connected-account/query-hooks/blocklist/blocklist-update-many.pre-query.hook.ts create mode 100644 packages/twenty-server/src/modules/connected-account/query-hooks/blocklist/blocklist-update-one.pre-query.hook.ts create mode 100644 packages/twenty-server/src/modules/connected-account/services/blocklist/blocklist-validation.module.ts create mode 100644 packages/twenty-server/src/modules/connected-account/services/blocklist/blocklist-validation.service.ts diff --git a/packages/twenty-front/src/modules/settings/accounts/components/SettingsAccountsEmailsBlocklistInput.tsx b/packages/twenty-front/src/modules/settings/accounts/components/SettingsAccountsEmailsBlocklistInput.tsx index 87160ac12..1794eb1d4 100644 --- a/packages/twenty-front/src/modules/settings/accounts/components/SettingsAccountsEmailsBlocklistInput.tsx +++ b/packages/twenty-front/src/modules/settings/accounts/components/SettingsAccountsEmailsBlocklistInput.tsx @@ -21,33 +21,42 @@ const StyledLinkContainer = styled.div` type SettingsAccountsEmailsBlocklistInputProps = { updateBlockedEmailList: (email: string) => void; + blockedEmailOrDomainList: string[]; }; -const validationSchema = z - .object({ - emailOrDomain: z - .string() - .trim() - .email('Invalid email or domain') - .or( - z - .string() - .refine( - (value) => value.startsWith('@') && isDomain(value.slice(1)), - 'Invalid email or domain', - ), - ), - }) - .required(); +const validationSchema = (blockedEmailOrDomainList: string[]) => + z + .object({ + emailOrDomain: z + .string() + .trim() + .email('Invalid email or domain') + .or( + z + .string() + .refine( + (value) => value.startsWith('@') && isDomain(value.slice(1)), + 'Invalid email or domain', + ), + ) + .refine( + (value) => !blockedEmailOrDomainList.includes(value), + 'Email or domain is already in blocklist', + ), + }) + .required(); -type FormInput = z.infer; +type FormInput = { + emailOrDomain: string; +}; export const SettingsAccountsEmailsBlocklistInput = ({ updateBlockedEmailList, + blockedEmailOrDomainList, }: SettingsAccountsEmailsBlocklistInputProps) => { const { reset, handleSubmit, control, formState } = useForm({ mode: 'onSubmit', - resolver: zodResolver(validationSchema), + resolver: zodResolver(validationSchema(blockedEmailOrDomainList)), defaultValues: { emailOrDomain: '', }, diff --git a/packages/twenty-front/src/modules/settings/accounts/components/SettingsAccountsEmailsBlocklistSection.tsx b/packages/twenty-front/src/modules/settings/accounts/components/SettingsAccountsEmailsBlocklistSection.tsx index 28646b9d8..aa32c6ee9 100644 --- a/packages/twenty-front/src/modules/settings/accounts/components/SettingsAccountsEmailsBlocklistSection.tsx +++ b/packages/twenty-front/src/modules/settings/accounts/components/SettingsAccountsEmailsBlocklistSection.tsx @@ -45,6 +45,7 @@ export const SettingsAccountsEmailsBlocklistSection = () => { description="Exclude the following people and domains from my email sync" /> item.handle)} updateBlockedEmailList={updateBlockedEmailList} /> , options: WorkspaceQueryRunnerOptions, ): Promise { - const { workspaceId, objectMetadataItem } = options; + const { userId, workspaceId, objectMetadataItem } = options; assertMutationNotOnRemoteObject(objectMetadataItem); assertIsValidUuid(args.data.id); @@ -376,6 +384,14 @@ export class WorkspaceQueryRunnerService { atMost: maximumRecordAffected, }); + await this.workspacePreQueryHookService.executePreHooks( + userId, + workspaceId, + objectMetadataItem.nameSingular, + 'updateMany', + args, + ); + const result = await this.execute(query, workspaceId); const parsedResults = ( diff --git a/packages/twenty-server/src/modules/connected-account/query-hooks/blocklist/blocklist-create-many.pre-query.hook.ts b/packages/twenty-server/src/modules/connected-account/query-hooks/blocklist/blocklist-create-many.pre-query.hook.ts index ef5654bc8..30e025de1 100644 --- a/packages/twenty-server/src/modules/connected-account/query-hooks/blocklist/blocklist-create-many.pre-query.hook.ts +++ b/packages/twenty-server/src/modules/connected-account/query-hooks/blocklist/blocklist-create-many.pre-query.hook.ts @@ -1,46 +1,28 @@ import { Injectable } from '@nestjs/common'; -import z from 'zod'; - import { WorkspacePreQueryHook } from 'src/engine/api/graphql/workspace-query-runner/workspace-pre-query-hook/interfaces/workspace-pre-query-hook.interface'; import { CreateManyResolverArgs } from 'src/engine/api/graphql/workspace-resolver-builder/interfaces/workspace-resolvers-builder.interface'; -import { isDomain } from 'src/engine/utils/is-domain'; -import { BlocklistObjectMetadata } from 'src/modules/connected-account/standard-objects/blocklist.object-metadata'; +import { + BlocklistItem, + BlocklistValidationService, +} from 'src/modules/connected-account/services/blocklist/blocklist-validation.service'; @Injectable() export class BlocklistCreateManyPreQueryHook implements WorkspacePreQueryHook { - constructor() {} + constructor( + private readonly blocklistValidationService: BlocklistValidationService, + ) {} async execute( userId: string, workspaceId: string, - payload: CreateManyResolverArgs< - Omit & { - createdAt: string; - updatedAt: string; - } - >, + payload: CreateManyResolverArgs, ): Promise { - const emailOrDomainSchema = z - .string() - .trim() - .email('Invalid email or domain') - .or( - z - .string() - .refine( - (value) => value.startsWith('@') && isDomain(value.slice(1)), - 'Invalid email or domain', - ), - ); - - for (const { handle } of payload.data) { - if (!handle) { - throw new Error('Handle is required'); - } - - emailOrDomainSchema.parse(handle); - } + await this.blocklistValidationService.validateBlocklistForCreateMany( + payload, + userId, + workspaceId, + ); } } diff --git a/packages/twenty-server/src/modules/connected-account/query-hooks/blocklist/blocklist-update-many.pre-query.hook.ts b/packages/twenty-server/src/modules/connected-account/query-hooks/blocklist/blocklist-update-many.pre-query.hook.ts new file mode 100644 index 000000000..b36ff3e96 --- /dev/null +++ b/packages/twenty-server/src/modules/connected-account/query-hooks/blocklist/blocklist-update-many.pre-query.hook.ts @@ -0,0 +1,12 @@ +import { Injectable } from '@nestjs/common'; + +import { WorkspacePreQueryHook } from 'src/engine/api/graphql/workspace-query-runner/workspace-pre-query-hook/interfaces/workspace-pre-query-hook.interface'; + +@Injectable() +export class BlocklistUpdateManyPreQueryHook implements WorkspacePreQueryHook { + constructor() {} + + async execute(): Promise { + throw new Error('Method not implemented.'); + } +} diff --git a/packages/twenty-server/src/modules/connected-account/query-hooks/blocklist/blocklist-update-one.pre-query.hook.ts b/packages/twenty-server/src/modules/connected-account/query-hooks/blocklist/blocklist-update-one.pre-query.hook.ts new file mode 100644 index 000000000..28c39a43b --- /dev/null +++ b/packages/twenty-server/src/modules/connected-account/query-hooks/blocklist/blocklist-update-one.pre-query.hook.ts @@ -0,0 +1,28 @@ +import { Injectable } from '@nestjs/common'; + +import { WorkspacePreQueryHook } from 'src/engine/api/graphql/workspace-query-runner/workspace-pre-query-hook/interfaces/workspace-pre-query-hook.interface'; +import { UpdateOneResolverArgs } from 'src/engine/api/graphql/workspace-resolver-builder/interfaces/workspace-resolvers-builder.interface'; + +import { + BlocklistItem, + BlocklistValidationService, +} from 'src/modules/connected-account/services/blocklist/blocklist-validation.service'; + +@Injectable() +export class BlocklistUpdateOnePreQueryHook implements WorkspacePreQueryHook { + constructor( + private readonly blocklistValidationService: BlocklistValidationService, + ) {} + + async execute( + userId: string, + workspaceId: string, + payload: UpdateOneResolverArgs, + ): Promise { + await this.blocklistValidationService.validateBlocklistForUpdateOne( + payload, + userId, + workspaceId, + ); + } +} diff --git a/packages/twenty-server/src/modules/connected-account/query-hooks/connected-account-query-hook.module.ts b/packages/twenty-server/src/modules/connected-account/query-hooks/connected-account-query-hook.module.ts index 3a841653b..c711b4b56 100644 --- a/packages/twenty-server/src/modules/connected-account/query-hooks/connected-account-query-hook.module.ts +++ b/packages/twenty-server/src/modules/connected-account/query-hooks/connected-account-query-hook.module.ts @@ -1,14 +1,25 @@ import { Module } from '@nestjs/common'; import { BlocklistCreateManyPreQueryHook } from 'src/modules/connected-account/query-hooks/blocklist/blocklist-create-many.pre-query.hook'; +import { BlocklistUpdateManyPreQueryHook } from 'src/modules/connected-account/query-hooks/blocklist/blocklist-update-many.pre-query.hook'; +import { BlocklistUpdateOnePreQueryHook } from 'src/modules/connected-account/query-hooks/blocklist/blocklist-update-one.pre-query.hook'; +import { BlocklistValidationModule } from 'src/modules/connected-account/services/blocklist/blocklist-validation.module'; @Module({ - imports: [], + imports: [BlocklistValidationModule], providers: [ { provide: BlocklistCreateManyPreQueryHook.name, useClass: BlocklistCreateManyPreQueryHook, }, + { + provide: BlocklistUpdateManyPreQueryHook.name, + useClass: BlocklistUpdateManyPreQueryHook, + }, + { + provide: BlocklistUpdateOnePreQueryHook.name, + useClass: BlocklistUpdateOnePreQueryHook, + }, ], }) export class ConnectedAccountQueryHookModule {} diff --git a/packages/twenty-server/src/modules/connected-account/repositories/blocklist.repository.ts b/packages/twenty-server/src/modules/connected-account/repositories/blocklist.repository.ts index 537e225fa..c68419652 100644 --- a/packages/twenty-server/src/modules/connected-account/repositories/blocklist.repository.ts +++ b/packages/twenty-server/src/modules/connected-account/repositories/blocklist.repository.ts @@ -50,4 +50,21 @@ export class BlocklistRepository { transactionManager, ); } + + public async getByWorkspaceMemberIdAndHandle( + workspaceMemberId: string, + handle: string, + workspaceId: string, + transactionManager?: EntityManager, + ): Promise[]> { + const dataSourceSchema = + this.workspaceDataSourceService.getSchemaName(workspaceId); + + return await this.workspaceDataSourceService.executeRawQuery( + `SELECT * FROM ${dataSourceSchema}."blocklist" WHERE "workspaceMemberId" = $1 AND "handle" = $2`, + [workspaceMemberId, handle], + workspaceId, + transactionManager, + ); + } } diff --git a/packages/twenty-server/src/modules/connected-account/services/blocklist/blocklist-validation.module.ts b/packages/twenty-server/src/modules/connected-account/services/blocklist/blocklist-validation.module.ts new file mode 100644 index 000000000..156513f73 --- /dev/null +++ b/packages/twenty-server/src/modules/connected-account/services/blocklist/blocklist-validation.module.ts @@ -0,0 +1,18 @@ +import { Module } from '@nestjs/common'; + +import { ObjectMetadataRepositoryModule } from 'src/engine/object-metadata-repository/object-metadata-repository.module'; +import { BlocklistValidationService } from 'src/modules/connected-account/services/blocklist/blocklist-validation.service'; +import { BlocklistObjectMetadata } from 'src/modules/connected-account/standard-objects/blocklist.object-metadata'; +import { WorkspaceMemberObjectMetadata } from 'src/modules/workspace-member/standard-objects/workspace-member.object-metadata'; + +@Module({ + imports: [ + ObjectMetadataRepositoryModule.forFeature([ + BlocklistObjectMetadata, + WorkspaceMemberObjectMetadata, + ]), + ], + providers: [BlocklistValidationService], + exports: [BlocklistValidationService], +}) +export class BlocklistValidationModule {} diff --git a/packages/twenty-server/src/modules/connected-account/services/blocklist/blocklist-validation.service.ts b/packages/twenty-server/src/modules/connected-account/services/blocklist/blocklist-validation.service.ts new file mode 100644 index 000000000..54f301098 --- /dev/null +++ b/packages/twenty-server/src/modules/connected-account/services/blocklist/blocklist-validation.service.ts @@ -0,0 +1,146 @@ +import { BadRequestException, Injectable } from '@nestjs/common'; + +import { z } from 'zod'; + +import { + CreateManyResolverArgs, + UpdateOneResolverArgs, +} from 'src/engine/api/graphql/workspace-resolver-builder/interfaces/workspace-resolvers-builder.interface'; + +import { InjectObjectMetadataRepository } from 'src/engine/object-metadata-repository/object-metadata-repository.decorator'; +import { isDomain } from 'src/engine/utils/is-domain'; +import { BlocklistRepository } from 'src/modules/connected-account/repositories/blocklist.repository'; +import { BlocklistObjectMetadata } from 'src/modules/connected-account/standard-objects/blocklist.object-metadata'; +import { WorkspaceMemberRepository } from 'src/modules/workspace-member/repositories/workspace-member.repository'; +import { WorkspaceMemberObjectMetadata } from 'src/modules/workspace-member/standard-objects/workspace-member.object-metadata'; + +export type BlocklistItem = Omit< + BlocklistObjectMetadata, + 'createdAt' | 'updatedAt' | 'workspaceMember' +> & { + createdAt: string; + updatedAt: string; + workspaceMemberId: string; +}; + +@Injectable() +export class BlocklistValidationService { + constructor( + @InjectObjectMetadataRepository(BlocklistObjectMetadata) + private readonly blocklistRepository: BlocklistRepository, + @InjectObjectMetadataRepository(WorkspaceMemberObjectMetadata) + private readonly workspaceMemberRepository: WorkspaceMemberRepository, + ) {} + + public async validateBlocklistForCreateMany( + payload: CreateManyResolverArgs, + userId: string, + workspaceId: string, + ) { + await this.validateSchema(payload.data); + await this.validateUniquenessForCreateMany(payload, userId, workspaceId); + } + + public async validateBlocklistForUpdateOne( + payload: UpdateOneResolverArgs, + userId: string, + workspaceId: string, + ) { + if (payload.data.handle) { + await this.validateSchema([payload.data]); + } + await this.validateUniquenessForUpdateOne(payload, userId, workspaceId); + } + + public async validateSchema(blocklist: BlocklistItem[]) { + const emailOrDomainSchema = z + .string() + .trim() + .email('Invalid email or domain') + .or( + z + .string() + .refine( + (value) => value.startsWith('@') && isDomain(value.slice(1)), + 'Invalid email or domain', + ), + ); + + for (const handle of blocklist.map((item) => item.handle)) { + if (!handle) { + throw new BadRequestException('Blocklist handle is required'); + } + + const result = emailOrDomainSchema.safeParse(handle); + + if (!result.success) { + throw new BadRequestException(result.error.errors[0].message); + } + } + } + + public async validateUniquenessForCreateMany( + payload: CreateManyResolverArgs, + userId: string, + workspaceId: string, + ) { + const currentWorkspaceMember = + await this.workspaceMemberRepository.getByIdOrFail(userId, workspaceId); + + const currentBlocklist = + await this.blocklistRepository.getByWorkspaceMemberId( + currentWorkspaceMember.id, + workspaceId, + ); + + const currentBlocklistHandles = currentBlocklist.map( + (blocklist) => blocklist.handle, + ); + + if ( + payload.data.some((item) => currentBlocklistHandles.includes(item.handle)) + ) { + throw new BadRequestException('Blocklist handle already exists'); + } + } + + public async validateUniquenessForUpdateOne( + payload: UpdateOneResolverArgs, + userId: string, + workspaceId: string, + ) { + const existingRecord = await this.blocklistRepository.getById( + payload.id, + workspaceId, + ); + + if (!existingRecord) { + throw new BadRequestException('Blocklist item not found'); + } + + if (existingRecord.workspaceMemberId !== payload.data.workspaceMemberId) { + throw new BadRequestException('Workspace member cannot be updated'); + } + + if (existingRecord.handle === payload.data.handle) { + return; + } + + const currentWorkspaceMember = + await this.workspaceMemberRepository.getByIdOrFail(userId, workspaceId); + + const currentBlocklist = + await this.blocklistRepository.getByWorkspaceMemberId( + currentWorkspaceMember.id, + workspaceId, + ); + + const currentBlocklistHandles = currentBlocklist + .filter((blocklist) => blocklist.id !== payload.id) + .map((blocklist) => blocklist.handle); + + if (currentBlocklistHandles.includes(payload.data.handle)) { + throw new BadRequestException('Blocklist handle already exists'); + } + } +} diff --git a/packages/twenty-server/src/modules/messaging/query-hooks/message/message-find-many.pre-query.hook.ts b/packages/twenty-server/src/modules/messaging/query-hooks/message/message-find-many.pre-query.hook.ts index 5824e536b..0e4cdd216 100644 --- a/packages/twenty-server/src/modules/messaging/query-hooks/message/message-find-many.pre-query.hook.ts +++ b/packages/twenty-server/src/modules/messaging/query-hooks/message/message-find-many.pre-query.hook.ts @@ -32,7 +32,7 @@ export class MessageFindManyPreQueryHook implements WorkspacePreQueryHook { @InjectObjectMetadataRepository(ConnectedAccountObjectMetadata) private readonly connectedAccountRepository: ConnectedAccountRepository, @InjectObjectMetadataRepository(WorkspaceMemberObjectMetadata) - private readonly workspaceMemberService: WorkspaceMemberRepository, + private readonly workspaceMemberRepository: WorkspaceMemberRepository, ) {} async execute( @@ -83,7 +83,7 @@ export class MessageFindManyPreQueryHook implements WorkspacePreQueryHook { } const currentWorkspaceMember = - await this.workspaceMemberService.getByIdOrFail(userId, workspaceId); + await this.workspaceMemberRepository.getByIdOrFail(userId, workspaceId); const messageChannelsConnectedAccounts = await this.connectedAccountRepository.getByIds(