From db95916a9c43348f7bd3a188befe75a3e763eacf Mon Sep 17 00:00:00 2001 From: Biswajeet Das Date: Thu, 27 Feb 2025 21:57:23 +0530 Subject: [PATCH] feat(api): enhance recipient validation with dry run feature and update subscriber ID regex --- .../parse-event-request.usecase.ts | 130 +++++++++++++++--- .../shared/src/consts/subscriberIdRegex.ts | 2 +- packages/shared/src/types/feature-flags.ts | 1 + 3 files changed, 111 insertions(+), 22 deletions(-) diff --git a/apps/api/src/app/events/usecases/parse-event-request/parse-event-request.usecase.ts b/apps/api/src/app/events/usecases/parse-event-request/parse-event-request.usecase.ts index 5d2f926352b..a06bcb82453 100644 --- a/apps/api/src/app/events/usecases/parse-event-request/parse-event-request.usecase.ts +++ b/apps/api/src/app/events/usecases/parse-event-request/parse-event-request.usecase.ts @@ -9,6 +9,7 @@ import { ExecuteBridgeRequest, ExecuteBridgeRequestCommand, ExecuteBridgeRequestDto, + FeatureFlagsService, Instrument, InstrumentUsecase, IWorkflowDataDto, @@ -20,8 +21,10 @@ import { EnvironmentRepository, NotificationTemplateEntity, NotificationTemplateRepository, + OrganizationEntity, TenantEntity, TenantRepository, + UserEntity, WorkflowOverrideEntity, WorkflowOverrideRepository, } from '@novu/dal'; @@ -36,6 +39,7 @@ import { WorkflowOriginEnum, SUBSCRIBER_ID_REGEX, TriggerRecipient, + FeatureFlagsKeysEnum, } from '@novu/shared'; import { ApiException } from '../../../shared/exceptions/api.exception'; @@ -59,6 +63,7 @@ export class ParseEventRequest { private tenantRepository: TenantRepository, private workflowOverrideRepository: WorkflowOverrideRepository, private executeBridgeRequest: ExecuteBridgeRequest, + private featureFlagService: FeatureFlagsService, protected moduleRef: ModuleRef ) {} @@ -200,17 +205,35 @@ export class ParseEventRequest { ...command, }; + const isDryRun = await this.featureFlagService.getFlag({ + environment: { _id: command.environmentId } as EnvironmentEntity, + organization: { _id: command.organizationId } as OrganizationEntity, + user: { _id: command.userId } as UserEntity, + key: FeatureFlagsKeysEnum.IS_SUBSCRIBER_ID_VALIDATION_DRY_RUN_ENABLED, + defaultValue: false, + }); + if ('to' in commandArgs) { - const validSubscribers = this.removeInvalidRecipients(commandArgs.to); + const { validSubscribers, inValidSubscribers } = this.separateRecipients(commandArgs.to); - if (!validSubscribers) { + if (inValidSubscribers.length > 0 && isDryRun) { + Logger.warn( + `[Dry run] Invalid recipients: ${inValidSubscribers.map((recipient) => JSON.stringify(recipient)).join(', ')}`, + 'ParseEventRequest' + ); + } + + if (!validSubscribers && !isDryRun) { return { acknowledged: true, status: TriggerEventStatusEnum.INVALID_RECIPIENTS, transactionId, }; } - commandArgs.to = validSubscribers; + + if (!isDryRun && validSubscribers) { + commandArgs.to = validSubscribers; + } } const jobData: IWorkflowDataDto = { @@ -305,43 +328,108 @@ export class ParseEventRequest { return reservedVariables?.map((reservedVariable) => reservedVariable.type) || []; } - private isValidId(subscriberId: string) { - if (subscriberId?.trim().match(SUBSCRIBER_ID_REGEX)) { - return subscriberId.trim(); - } + private isValidSubscriberId(subscriberId: string) { + return Boolean(subscriberId?.trim().match(SUBSCRIBER_ID_REGEX)); } - private removeInvalidRecipients(payload: TriggerRecipientsPayload): TriggerRecipientsPayload | null { - if (!payload) return null; + /** + * Separates valid and invalid recipients from the given payload. + * + * @param payload - The payload containing recipients to be validated. + * @returns An object containing valid subscribers and invalid subscribers. + * - `validSubscribers`: An array of valid recipients or null if none are valid. + * - `inValidSubscribers`: An array of invalid recipients. + */ + private separateRecipients(payload: TriggerRecipientsPayload): { + validSubscribers: TriggerRecipientsPayload | null; + inValidSubscribers: TriggerRecipientsPayload[]; + } { + if (!payload) + return { + validSubscribers: null, + inValidSubscribers: [], + }; if (!Array.isArray(payload)) { - return this.filterValidRecipient(payload) as TriggerRecipientsPayload; + const { invalid, valid } = this.filterValidRecipient(payload); + + return { + validSubscribers: valid as TriggerRecipientsPayload, + inValidSubscribers: invalid ? ([invalid] as TriggerRecipientsPayload[]) : [], + }; } - const filteredRecipients: TriggerRecipients = payload - .map((subscriber) => this.filterValidRecipient(subscriber)) - .filter((subscriber): subscriber is TriggerRecipient => subscriber !== null); + const invalidRecipients: TriggerRecipientsPayload[] = []; + const recipients: TriggerRecipients = payload + .map((subscriber) => { + const { invalid, valid } = this.filterValidRecipient(subscriber); - return filteredRecipients.length > 0 ? filteredRecipients : null; + if (invalid) { + invalidRecipients.push(invalid as TriggerRecipientsPayload); + } + + return valid; + }) + .filter((recipient) => recipient !== null); + + return { validSubscribers: recipients.length > 0 ? recipients : null, inValidSubscribers: invalidRecipients }; } - private filterValidRecipient(subscriber: TriggerRecipient): TriggerRecipient | null { + /** + * Filters a given subscriber and determines if it is valid or invalid. + * + * @param subscriber - The subscriber to be validated. It can be a string or an object. + * + * @returns An object containing: + * - `valid`: The valid subscriber if the input is valid, otherwise `null`. + * - `invalid`: The invalid subscriber if the input is invalid, otherwise `null`. + * + * The function performs the following checks: + * - If the subscriber is a string, it trims the string and checks if it is a valid subscriber ID. + * - If the subscriber is an object, it checks if it contains a `topicKey` or a `subscriberId`. + * - If it contains a `topicKey`, then we do nothing and return it as it is. + * - If it contains a `subscriberId`, it trims the `subscriberId` and checks if it is valid. + * + * If the subscriber does not meet any of the above conditions, it is considered invalid. + */ + private filterValidRecipient(subscriber: TriggerRecipient): { + valid: TriggerRecipient | null; + invalid: TriggerRecipient | null; + } { if (typeof subscriber === 'string') { - return this.isValidId(subscriber) ? subscriber : null; + const trimmedSubscriber = subscriber.trim(); + + return this.isValidSubscriberId(subscriber) + ? { + valid: trimmedSubscriber, + invalid: null, + } + : { + valid: null, + invalid: subscriber, + }; } if (typeof subscriber === 'object' && subscriber !== null) { if ('topicKey' in subscriber) { - return subscriber; + return { valid: subscriber, invalid: null }; } if ('subscriberId' in subscriber) { - const subscriberId = this.isValidId(subscriber.subscriberId); - - return subscriberId ? { ...subscriber, subscriberId } : null; + const isValidSubscriberId = this.isValidSubscriberId(subscriber.subscriberId); + + return isValidSubscriberId + ? { valid: { ...subscriber, subscriberId: subscriber.subscriberId.trim() }, invalid: null } + : { + valid: null, + invalid: subscriber, + }; } } - return null; + return { + valid: null, + invalid: subscriber, + }; } } diff --git a/packages/shared/src/consts/subscriberIdRegex.ts b/packages/shared/src/consts/subscriberIdRegex.ts index e951efebcfd..7f83e7ae214 100644 --- a/packages/shared/src/consts/subscriberIdRegex.ts +++ b/packages/shared/src/consts/subscriberIdRegex.ts @@ -1 +1 @@ -export const SUBSCRIBER_ID_REGEX = /^[a-zA-Z0-9@._-]+$/; +export const SUBSCRIBER_ID_REGEX = /^[a-zA-Z0-9!#$%&'*+=?^_`{|}~.-]+$/; diff --git a/packages/shared/src/types/feature-flags.ts b/packages/shared/src/types/feature-flags.ts index 32452bda90c..d3a25500ed2 100644 --- a/packages/shared/src/types/feature-flags.ts +++ b/packages/shared/src/types/feature-flags.ts @@ -50,6 +50,7 @@ export enum FeatureFlagsKeysEnum { IS_WORKFLOW_LIMIT_ENABLED = 'IS_WORKFLOW_LIMIT_ENABLED', IS_MAX_WORKFLOW_LIMIT_ENABLED = 'IS_MAX_WORKFLOW_LIMIT_ENABLED', IS_MAX_STEPS_PER_WORKFLOW_ENABLED = 'IS_MAX_STEPS_PER_WORKFLOW_ENABLED', + IS_SUBSCRIBER_ID_VALIDATION_DRY_RUN_ENABLED = 'IS_SUBSCRIBER_ID_VALIDATION_DRY_RUN_ENABLED', // Numeric flags MAX_WORKFLOW_LIMIT_NUMBER = 'MAX_WORKFLOW_LIMIT_NUMBER',