diff --git a/apps/bff/src/core/http/exception.filter.ts b/apps/bff/src/core/http/exception.filter.ts index a226314e..ad94b934 100644 --- a/apps/bff/src/core/http/exception.filter.ts +++ b/apps/bff/src/core/http/exception.filter.ts @@ -10,6 +10,7 @@ import type { ExceptionFilter, ArgumentsHost } from "@nestjs/common"; import type { Request, Response } from "express"; import { Logger } from "nestjs-pino"; import { ConfigService } from "@nestjs/config"; +import { ZodValidationException } from "nestjs-zod"; import { ErrorCode, ErrorMessages, @@ -79,16 +80,22 @@ export class UnifiedExceptionFilter implements ExceptionFilter { const errorContext = this.buildErrorContext(request); // Extract status code and error details - const { status, errorCode, originalMessage } = this.extractErrorDetails(exception); + const { status, errorCode, originalMessage, fieldErrors } = this.extractErrorDetails(exception); // Get user-friendly message (with dev details if in development) - const userMessage = this.getUserMessage(errorCode, originalMessage); + const userMessage = this.getUserMessage(errorCode, originalMessage, fieldErrors); // Log the error this.logError(errorCode, originalMessage, status, errorContext, exception); // Build and send response - const errorResponse = this.buildErrorResponse(errorCode, userMessage, status, errorContext); + const errorResponse = this.buildErrorResponse( + errorCode, + userMessage, + status, + errorContext, + fieldErrors + ); response.status(status).json(errorResponse); } @@ -99,12 +106,22 @@ export class UnifiedExceptionFilter implements ExceptionFilter { status: number; errorCode: ErrorCodeType; originalMessage: string; + fieldErrors?: Record | undefined; } { let status = HttpStatus.INTERNAL_SERVER_ERROR; let originalMessage = "An unexpected error occurred"; let explicitCode: ErrorCodeType | undefined; + let fieldErrors: Record | undefined; - if (exception instanceof HttpException) { + // ZodValidationException must be checked before HttpException + // since ZodValidationException extends BadRequestException extends HttpException + if (exception instanceof ZodValidationException) { + status = exception.getStatus(); + const extracted = this.extractZodFieldErrors(exception); + fieldErrors = extracted.fieldErrors; + originalMessage = extracted.message; + explicitCode = ErrorCode.VALIDATION_FAILED; + } else if (exception instanceof HttpException) { status = exception.getStatus(); const extracted = this.extractExceptionDetails(exception); originalMessage = extracted.message; @@ -120,7 +137,49 @@ export class UnifiedExceptionFilter implements ExceptionFilter { // Map to error code const errorCode = explicitCode ?? mapHttpStatusToErrorCode(status); - return { status, errorCode, originalMessage }; + return { status, errorCode, originalMessage, fieldErrors }; + } + + /** + * Extract field errors and a summary message from a ZodValidationException. + */ + private extractZodFieldErrors(exception: ZodValidationException): { + fieldErrors?: Record; + message: string; + } { + const zodError = exception.getZodError(); + const issues = this.extractZodIssues(zodError); + + if (issues.length === 0) { + return { message: "Validation failed" }; + } + + const fieldErrors: Record = {}; + for (const issue of issues) { + const fieldKey = issue.path.length > 0 ? issue.path.join(".") : "_form"; + if (!(fieldKey in fieldErrors)) { + fieldErrors[fieldKey] = issue.message; + } + } + + return { fieldErrors, message: issues[0]!.message }; + } + + /** + * Extract Zod issues from the unknown ZodError returned by getZodError() + */ + private extractZodIssues( + zodError: unknown + ): Array<{ path: (string | number)[]; message: string }> { + if ( + zodError && + typeof zodError === "object" && + "issues" in zodError && + Array.isArray((zodError as { issues: unknown }).issues) + ) { + return (zodError as { issues: Array<{ path: (string | number)[]; message: string }> }).issues; + } + return []; } /** @@ -193,9 +252,21 @@ export class UnifiedExceptionFilter implements ExceptionFilter { } /** - * Get user-friendly message, with dev details in development mode + * Get user-friendly message, with dev details in development mode. + * When fieldErrors is present (Zod validation), use the original Zod message + * instead of the generic canned message, since Zod messages are developer-authored + * and safe to display to users. */ - private getUserMessage(errorCode: ErrorCodeType, originalMessage: string): string { + private getUserMessage( + errorCode: ErrorCodeType, + originalMessage: string, + fieldErrors?: Record + ): string { + // For Zod validation errors, use the original message (developer-authored, safe to display) + if (fieldErrors) { + return originalMessage; + } + const userMessage = ErrorMessages[errorCode] ?? ErrorMessages[ErrorCode.UNKNOWN]; if (this.isDevelopment && originalMessage !== userMessage) { @@ -228,18 +299,19 @@ export class UnifiedExceptionFilter implements ExceptionFilter { request: Request & { user?: { id?: string }; requestId?: string } ): ErrorContext { const userAgentHeader = request.headers["user-agent"]; + const userAgent = + typeof userAgentHeader === "string" + ? userAgentHeader + : Array.isArray(userAgentHeader) + ? userAgentHeader[0] + : undefined; return { requestId: request.requestId ?? this.generateRequestId(), userId: request.user?.id, method: request.method, path: request.url, - userAgent: - typeof userAgentHeader === "string" - ? userAgentHeader - : Array.isArray(userAgentHeader) - ? userAgentHeader[0] - : undefined, + userAgent, ip: request.ip, }; } @@ -302,7 +374,8 @@ export class UnifiedExceptionFilter implements ExceptionFilter { errorCode: ErrorCodeType, message: string, status: number, - context: ErrorContext + context: ErrorContext, + fieldErrors?: Record ): ApiError { const metadata = ErrorMetadata[errorCode] ?? ErrorMetadata[ErrorCode.UNKNOWN]; @@ -317,6 +390,7 @@ export class UnifiedExceptionFilter implements ExceptionFilter { timestamp: new Date().toISOString(), path: context.path, requestId: context.requestId, + ...(fieldErrors && { fieldErrors }), }, }, }; diff --git a/apps/bff/src/infra/queue/services/whmcs-request-queue.service.ts b/apps/bff/src/infra/queue/services/whmcs-request-queue.service.ts index 2cf954e6..1a9f4302 100644 --- a/apps/bff/src/infra/queue/services/whmcs-request-queue.service.ts +++ b/apps/bff/src/infra/queue/services/whmcs-request-queue.service.ts @@ -193,7 +193,7 @@ export class WhmcsRequestQueueService implements OnModuleInit, OnModuleDestroy { this.metrics.failedRequests++; this.metrics.lastErrorTime = new Date(); - this.logger.error( + this.logger.warn( { requestId, waitTime, diff --git a/apps/bff/src/integrations/japanpost/facades/japanpost.facade.ts b/apps/bff/src/integrations/japanpost/facades/japanpost.facade.ts index b9dfd272..32053887 100644 --- a/apps/bff/src/integrations/japanpost/facades/japanpost.facade.ts +++ b/apps/bff/src/integrations/japanpost/facades/japanpost.facade.ts @@ -3,15 +3,27 @@ * * Public API for Japan Post integration. * Controllers should use this facade instead of internal services directly. + * + * Adds Redis-backed caching for postal code lookups — address data is + * stable and rarely changes, so a 24-hour TTL avoids redundant API calls. */ -import { Injectable } from "@nestjs/common"; +import { Inject, Injectable } from "@nestjs/common"; +import { Logger } from "nestjs-pino"; import { JapanPostAddressService } from "../services/japanpost-address.service.js"; +import { CacheService } from "@bff/infra/cache/cache.service.js"; import type { AddressLookupResult } from "@customer-portal/domain/address"; +const CACHE_PREFIX = "japanpost:zip:"; +const CACHE_TTL_SECONDS = 86_400; // 24 hours + @Injectable() export class JapanPostFacade { - constructor(private readonly addressService: JapanPostAddressService) {} + constructor( + private readonly addressService: JapanPostAddressService, + private readonly cache: CacheService, + @Inject(Logger) private readonly logger: Logger + ) {} /** * Lookup address by ZIP code @@ -24,7 +36,30 @@ export class JapanPostFacade { zipCode: string, clientIp: string = "127.0.0.1" ): Promise { - return this.addressService.lookupByZipCode(zipCode, clientIp); + const normalizedZip = zipCode.replace(/-/g, ""); + const cacheKey = `${CACHE_PREFIX}${normalizedZip}`; + + try { + const cached = await this.cache.get(cacheKey); + if (cached) { + return cached; + } + } catch { + // Redis unavailable — proceed without cache + } + + const result = await this.addressService.lookupByZipCode(zipCode, clientIp); + + // Cache successful lookups with results + if (result.count > 0) { + try { + await this.cache.set(cacheKey, result, CACHE_TTL_SECONDS); + } catch { + this.logger.debug({ zipCode: normalizedZip }, "Failed to cache Japan Post lookup result"); + } + } + + return result; } /** diff --git a/apps/bff/src/integrations/whmcs/connection/services/whmcs-http-client.service.ts b/apps/bff/src/integrations/whmcs/connection/services/whmcs-http-client.service.ts index 6e3f836c..53051feb 100644 --- a/apps/bff/src/integrations/whmcs/connection/services/whmcs-http-client.service.ts +++ b/apps/bff/src/integrations/whmcs/connection/services/whmcs-http-client.service.ts @@ -49,7 +49,7 @@ export class WhmcsHttpClientService { this.stats.failedRequests++; this.stats.lastErrorTime = new Date(); - this.logger.error(`WHMCS HTTP request failed [${action}]`, { + this.logger.warn(`WHMCS HTTP request failed [${action}]`, { error: extractErrorMessage(error), action, params: redactForLogs(params), diff --git a/apps/bff/src/integrations/whmcs/services/whmcs-client.service.ts b/apps/bff/src/integrations/whmcs/services/whmcs-client.service.ts index 19ce52b1..17fe0aca 100644 --- a/apps/bff/src/integrations/whmcs/services/whmcs-client.service.ts +++ b/apps/bff/src/integrations/whmcs/services/whmcs-client.service.ts @@ -18,6 +18,9 @@ import { addressSchema, type Address, type WhmcsClient } from "@customer-portal/ @Injectable() export class WhmcsClientService { + /** In-flight getClientDetails calls — deduplicates concurrent requests for the same client */ + private readonly inflightDetails = new Map>(); + constructor( private readonly connectionService: WhmcsConnectionFacade, private readonly cacheService: WhmcsCacheService, @@ -54,18 +57,35 @@ export class WhmcsClientService { } /** - * Get client details by ID - * Returns WhmcsClient (type inferred from domain mapper) + * Get client details by ID. + * Concurrent calls for the same clientId share a single API request. */ async getClientDetails(clientId: number): Promise { - try { - // Try cache first - const cached = await this.cacheService.getClientData(clientId); - if (cached) { - this.logger.debug(`Cache hit for client: ${clientId}`); - return cached; - } + // Try cache first (fast path, no dedup needed) + const cached = await this.cacheService.getClientData(clientId); + if (cached) { + this.logger.debug(`Cache hit for client: ${clientId}`); + return cached; + } + // Deduplicate: reuse in-flight request for the same clientId + const inflight = this.inflightDetails.get(clientId); + if (inflight) { + return inflight; + } + + const promise = this.fetchClientDetails(clientId); + this.inflightDetails.set(clientId, promise); + + try { + return await promise; + } finally { + this.inflightDetails.delete(clientId); + } + } + + private async fetchClientDetails(clientId: number): Promise { + try { const response = await this.connectionService.getClientDetails(clientId); if (!response || !response.client) { diff --git a/apps/bff/src/integrations/whmcs/services/whmcs-invoice.service.ts b/apps/bff/src/integrations/whmcs/services/whmcs-invoice.service.ts index b4c0eeae..85288fbd 100644 --- a/apps/bff/src/integrations/whmcs/services/whmcs-invoice.service.ts +++ b/apps/bff/src/integrations/whmcs/services/whmcs-invoice.service.ts @@ -233,7 +233,7 @@ export class WhmcsInvoiceService { limit: number ): InvoiceList { if (!response.invoices?.invoice) { - this.logger.warn(`No invoices found for client ${clientId}`); + this.logger.debug(`No invoices found for client ${clientId}`); return { invoices: [], pagination: { diff --git a/apps/bff/src/modules/address/address-writer.service.ts b/apps/bff/src/modules/address/address-writer.service.ts index b816bd92..e06c9fd0 100644 --- a/apps/bff/src/modules/address/address-writer.service.ts +++ b/apps/bff/src/modules/address/address-writer.service.ts @@ -33,6 +33,12 @@ export interface ResolveWhmcsAddressParams { @Injectable() export class AddressWriterService { + /** In-flight Japan Post lookups keyed by postcode — deduplicates concurrent calls */ + private readonly inflightLookups = new Map< + string, + Promise>> + >(); + constructor( private readonly japanPost: JapanPostFacade, private readonly salesforceFacade: SalesforceFacade, @@ -63,16 +69,13 @@ export class AddressWriterService { * Resolve English address from postal code via Japan Post API * and prepare WHMCS address fields. * - * Flow: - * 1. Call Japan Post API with postal code - * 2. Match result using townJa (if multiple matches) - * 3. Build BilingualAddress with resolved English fields - * 4. Return WhmcsAddressFields via prepareWhmcsAddressFields() + * Uses in-flight deduplication so concurrent lookups for the same + * postcode share a single API call. */ async resolveWhmcsAddress(params: ResolveWhmcsAddressParams): Promise { const { postcode, townJa, streetAddress, buildingName, roomNumber, residenceType } = params; - const lookupResult = await this.japanPost.lookupByZipCode(postcode); + const lookupResult = await this.lookupPostalCode(postcode); if (lookupResult.addresses.length === 0) { this.logger.warn({ postcode }, "Japan Post API returned no results for postal code"); @@ -113,17 +116,73 @@ export class AddressWriterService { } /** - * Convenience: resolve WHMCS address from a full BilingualAddress. - * Re-derives English fields from postal code via Japan Post API. + * Deduplicated Japan Post lookup — concurrent calls for the same postcode + * share one API request. */ - async resolveWhmcsAddressFromBilingual(address: BilingualAddress): Promise { - return this.resolveWhmcsAddress({ + private async lookupPostalCode( + postcode: string + ): Promise>> { + const inflight = this.inflightLookups.get(postcode); + if (inflight) { + return inflight; + } + + const promise = this.japanPost.lookupByZipCode(postcode); + this.inflightLookups.set(postcode, promise); + + try { + return await promise; + } finally { + this.inflightLookups.delete(postcode); + } + } + + /** + * Prepare WHMCS address fields from a full BilingualAddress. + * Uses the already-populated English fields — no API call needed. + */ + resolveWhmcsAddressFromBilingual(address: BilingualAddress): WhmcsAddressFields { + return prepareWhmcsAddressFields(address); + } + + /** + * Convert WhmcsAddressFields to the shape expected by CreateWhmcsClientStep. + */ + toWhmcsStepAddress(fields: WhmcsAddressFields): { + address1: string; + address2?: string; + city: string; + state: string; + postcode: string; + country: string; + } { + return { + address1: fields.address1 || "", + ...(fields.address2 && { address2: fields.address2 }), + city: fields.city, + state: fields.state, + postcode: fields.postcode, + country: fields.country, + }; + } + + /** + * Build case address from bilingual address. + * Maps Japanese fields to the eligibility case step's expected format. + */ + toCaseAddress(address: BilingualAddress): { + address1: string; + city: string; + state: string; + postcode: string; + country: string; + } { + return { + address1: `${address.townJa}${address.streetAddress}`, + city: address.cityJa, + state: address.prefectureJa, postcode: address.postcode, - townJa: address.townJa, - streetAddress: address.streetAddress, - buildingName: address.buildingName, - roomNumber: address.roomNumber, - residenceType: address.residenceType, - }); + country: "Japan", + }; } } diff --git a/apps/bff/src/modules/auth/infra/workflows/guest-eligibility-workflow.service.ts b/apps/bff/src/modules/auth/infra/workflows/guest-eligibility-workflow.service.ts index c53e9996..8918d30b 100644 --- a/apps/bff/src/modules/auth/infra/workflows/guest-eligibility-workflow.service.ts +++ b/apps/bff/src/modules/auth/infra/workflows/guest-eligibility-workflow.service.ts @@ -10,6 +10,7 @@ import { DistributedLockService } from "@bff/infra/cache/distributed-lock.servic import { SalesforceAccountService } from "@bff/integrations/salesforce/services/salesforce-account.service.js"; import { SalesforceFacade } from "@bff/integrations/salesforce/facades/salesforce.facade.js"; import { extractErrorMessage } from "@bff/core/utils/error.util.js"; +import { safeOperation, OperationCriticality } from "@bff/core/utils/safe-operation.util.js"; import { PORTAL_SOURCE_INTERNET_ELIGIBILITY, PORTAL_STATUS_NOT_YET, @@ -101,27 +102,20 @@ export class GuestEligibilityWorkflowService { ); } - // Write Japanese address to Salesforce - try { - await this.addressWriter.writeToSalesforce(sfAccountId, address); - } catch (addressError) { - this.logger.warn( - { error: extractErrorMessage(addressError), email: normalizedEmail }, - "SF address write failed (non-critical, continuing)" - ); - } - - // Create eligibility case via shared step - const { caseId } = await this.eligibilityCaseStep.execute({ - sfAccountId, - address: { - address1: `${address.townJa}${address.streetAddress}`, - city: address.cityJa, - state: address.prefectureJa, - postcode: address.postcode, - country: "Japan", - }, - }); + // SF address write (DEGRADABLE) + eligibility case creation — independent, run in parallel + const [, { caseId }] = await Promise.all([ + safeOperation(async () => this.addressWriter.writeToSalesforce(sfAccountId, address), { + criticality: OperationCriticality.OPTIONAL, + fallback: undefined, + context: "SF address write", + logger: this.logger, + metadata: { email: normalizedEmail }, + }), + this.eligibilityCaseStep.execute({ + sfAccountId, + address: this.addressWriter.toCaseAddress(address), + }), + ]); // If user wants to continue to account creation, generate a handoff token let handoffToken: string | undefined; diff --git a/apps/bff/src/modules/auth/infra/workflows/new-customer-signup-workflow.service.ts b/apps/bff/src/modules/auth/infra/workflows/new-customer-signup-workflow.service.ts index 2454e232..b9d443fc 100644 --- a/apps/bff/src/modules/auth/infra/workflows/new-customer-signup-workflow.service.ts +++ b/apps/bff/src/modules/auth/infra/workflows/new-customer-signup-workflow.service.ts @@ -8,8 +8,8 @@ import { type SignupWithEligibilityRequest } from "@customer-portal/domain/get-s import { DistributedLockService } from "@bff/infra/cache/distributed-lock.service.js"; import { EmailService } from "@bff/infra/email/email.service.js"; import { UsersService } from "@bff/modules/users/application/users.service.js"; -import { WhmcsAccountDiscoveryService } from "@bff/integrations/whmcs/services/whmcs-account-discovery.service.js"; import { extractErrorMessage } from "@bff/core/utils/error.util.js"; +import { safeOperation, OperationCriticality } from "@bff/core/utils/safe-operation.util.js"; import { PORTAL_SOURCE_INTERNET_ELIGIBILITY } from "@bff/modules/auth/constants/portal.constants.js"; import type { AuthResultInternal } from "@bff/modules/auth/auth.types.js"; import { AddressWriterService } from "@bff/modules/address/address-writer.service.js"; @@ -39,7 +39,6 @@ export class NewCustomerSignupWorkflowService { private readonly sessionService: GetStartedSessionService, private readonly lockService: DistributedLockService, private readonly usersService: UsersService, - private readonly whmcsDiscovery: WhmcsAccountDiscoveryService, private readonly emailService: EmailService, private readonly sfStep: ResolveSalesforceAccountStep, private readonly caseStep: CreateEligibilityCaseStep, @@ -148,39 +147,38 @@ export class NewCustomerSignupWorkflowService { updateSourceIfExists: true, }); - // Step 1.5: Write Japanese address to Salesforce (DEGRADABLE) - try { - await this.addressWriter.writeToSalesforce(sfResult.sfAccountId, address); - } catch (addressError) { - this.logger.warn( - { error: extractErrorMessage(addressError), email }, - "SF address write failed (non-critical, continuing)" - ); - } - - // Step 2: Create eligibility case (DEGRADABLE) - let eligibilityRequestId: string | undefined; - try { - const caseResult = await this.caseStep.execute({ - sfAccountId: sfResult.sfAccountId, - address: { - address1: `${address.townJa}${address.streetAddress}`, - city: address.cityJa, - state: address.prefectureJa, - postcode: address.postcode, - country: "Japan", + // Steps 1.5 + 2: SF address write + eligibility case creation (both DEGRADABLE, independent) + const [, eligibilityRequestId] = await Promise.all([ + safeOperation( + async () => this.addressWriter.writeToSalesforce(sfResult.sfAccountId, address), + { + criticality: OperationCriticality.OPTIONAL, + fallback: undefined, + context: "SF address write", + logger: this.logger, + metadata: { email }, + } + ), + safeOperation( + async () => { + const caseResult = await this.caseStep.execute({ + sfAccountId: sfResult.sfAccountId, + address: this.addressWriter.toCaseAddress(address), + }); + return caseResult.caseId; }, - }); - eligibilityRequestId = caseResult.caseId; - } catch (caseError) { - this.logger.warn( - { error: extractErrorMessage(caseError), email }, - "Eligibility case creation failed (non-critical, continuing)" - ); - } + { + criticality: OperationCriticality.OPTIONAL, + fallback: undefined as string | undefined, + context: "Eligibility case creation", + logger: this.logger, + metadata: { email }, + } + ), + ]); // Step 3: Create WHMCS client (CRITICAL, has rollback) - const whmcsAddress = await this.addressWriter.resolveWhmcsAddressFromBilingual(address); + const whmcsAddress = this.addressWriter.resolveWhmcsAddressFromBilingual(address); const whmcsResult = await this.whmcsStep.execute({ email, @@ -188,14 +186,7 @@ export class NewCustomerSignupWorkflowService { firstName, lastName, phone: phone ?? "", - address: { - address1: whmcsAddress.address1 || "", - ...(whmcsAddress.address2 && { address2: whmcsAddress.address2 }), - city: whmcsAddress.city, - state: whmcsAddress.state, - postcode: whmcsAddress.postcode, - country: whmcsAddress.country, - }, + address: this.addressWriter.toWhmcsStepAddress(whmcsAddress), customerNumber: sfResult.customerNumber ?? null, dateOfBirth, gender, @@ -217,17 +208,20 @@ export class NewCustomerSignupWorkflowService { } // Step 5: Update SF flags (DEGRADABLE) - try { - await this.sfFlagsStep.execute({ - sfAccountId: sfResult.sfAccountId, - whmcsClientId: whmcsResult.whmcsClientId, - }); - } catch (flagsError) { - this.logger.warn( - { error: extractErrorMessage(flagsError), email }, - "SF flags update failed (non-critical, continuing)" - ); - } + await safeOperation( + async () => + this.sfFlagsStep.execute({ + sfAccountId: sfResult.sfAccountId, + whmcsClientId: whmcsResult.whmcsClientId, + }), + { + criticality: OperationCriticality.OPTIONAL, + fallback: undefined, + context: "SF flags update", + logger: this.logger, + metadata: { email }, + } + ); // Step 6: Generate auth result + finalize const authResult = await this.authResultStep.execute({ @@ -255,10 +249,9 @@ export class NewCustomerSignupWorkflowService { private async checkExistingAccounts( email: string ): Promise<{ success: false; message: string } | null> { - const [portalUser, whmcsClient] = await Promise.all([ - this.usersService.findByEmailInternal(email), - this.whmcsDiscovery.findClientByEmail(email), - ]); + // Only check Portal — the verification phase already checked WHMCS, + // and the AddClient step will reject duplicates if one was created in the interim. + const portalUser = await this.usersService.findByEmailInternal(email); if (portalUser) { return { @@ -267,14 +260,6 @@ export class NewCustomerSignupWorkflowService { }; } - if (whmcsClient) { - return { - success: false, - message: - "A billing account already exists with this email. Please use account linking instead.", - }; - } - return null; } diff --git a/apps/bff/src/modules/auth/infra/workflows/sf-completion-workflow.service.ts b/apps/bff/src/modules/auth/infra/workflows/sf-completion-workflow.service.ts index 5cdfcd62..2ac71d27 100644 --- a/apps/bff/src/modules/auth/infra/workflows/sf-completion-workflow.service.ts +++ b/apps/bff/src/modules/auth/infra/workflows/sf-completion-workflow.service.ts @@ -6,8 +6,8 @@ import { type CompleteAccountRequest } from "@customer-portal/domain/get-started import { DistributedLockService } from "@bff/infra/cache/distributed-lock.service.js"; import { UsersService } from "@bff/modules/users/application/users.service.js"; -import { WhmcsAccountDiscoveryService } from "@bff/integrations/whmcs/services/whmcs-account-discovery.service.js"; import { extractErrorMessage } from "@bff/core/utils/error.util.js"; +import { safeOperation, OperationCriticality } from "@bff/core/utils/safe-operation.util.js"; import { PORTAL_SOURCE_NEW_SIGNUP } from "@bff/modules/auth/constants/portal.constants.js"; import type { AuthResultInternal } from "@bff/modules/auth/auth.types.js"; import { AddressWriterService } from "@bff/modules/address/address-writer.service.js"; @@ -35,7 +35,6 @@ export class SfCompletionWorkflowService { private readonly sessionService: GetStartedSessionService, private readonly lockService: DistributedLockService, private readonly usersService: UsersService, - private readonly whmcsDiscovery: WhmcsAccountDiscoveryService, private readonly sfStep: ResolveSalesforceAccountStep, private readonly whmcsStep: CreateWhmcsClientStep, private readonly portalUserStep: CreatePortalUserStep, @@ -135,30 +134,33 @@ export class SfCompletionWorkflowService { source: PORTAL_SOURCE_NEW_SIGNUP, }); - // Step 1.5: Write Japanese address to Salesforce for new customers (DEGRADABLE) - if (isNewCustomer && address.prefectureJa) { - try { - await this.addressWriter.writeToSalesforce( - sfResult.sfAccountId, - address as BilingualAddress - ); - } catch (addressError) { - this.logger.warn( - { error: extractErrorMessage(addressError), email: session.email }, - "SF address write failed (non-critical, continuing)" - ); - } - } - - // Step 2: Create WHMCS client (CRITICAL, has rollback) - const whmcsAddress = await this.addressWriter.resolveWhmcsAddress({ - postcode: address.postcode, - townJa: address.townJa, - streetAddress: address.streetAddress || "", - buildingName: address.buildingName, - roomNumber: address.roomNumber, - residenceType: address.residenceType || "house", - }); + // Steps 1.5 + 2: SF address write (DEGRADABLE) + WHMCS address resolve (CRITICAL) — independent + const [, whmcsAddress] = await Promise.all([ + isNewCustomer && address.prefectureJa + ? safeOperation( + async () => + this.addressWriter.writeToSalesforce( + sfResult.sfAccountId, + address as BilingualAddress + ), + { + criticality: OperationCriticality.OPTIONAL, + fallback: undefined, + context: "SF address write", + logger: this.logger, + metadata: { email: session.email }, + } + ) + : Promise.resolve(), + this.addressWriter.resolveWhmcsAddress({ + postcode: address.postcode, + townJa: address.townJa, + streetAddress: address.streetAddress || "", + buildingName: address.buildingName, + roomNumber: address.roomNumber, + residenceType: address.residenceType || "house", + }), + ]); const whmcsResult = await this.whmcsStep.execute({ firstName: finalFirstName, @@ -166,14 +168,7 @@ export class SfCompletionWorkflowService { email: session.email, password, phone: phone ?? "", - address: { - address1: whmcsAddress.address1 || "", - ...(whmcsAddress.address2 && { address2: whmcsAddress.address2 }), - city: whmcsAddress.city, - state: whmcsAddress.state, - postcode: whmcsAddress.postcode, - country: whmcsAddress.country, - }, + address: this.addressWriter.toWhmcsStepAddress(whmcsAddress), customerNumber: sfResult.customerNumber ?? null, dateOfBirth, gender, @@ -195,17 +190,20 @@ export class SfCompletionWorkflowService { } // Step 4: Update SF flags (DEGRADABLE) - try { - await this.sfFlagsStep.execute({ - sfAccountId: sfResult.sfAccountId, - whmcsClientId: whmcsResult.whmcsClientId, - }); - } catch (flagsError) { - this.logger.warn( - { error: extractErrorMessage(flagsError), email: session.email }, - "SF flags update failed (non-critical, continuing)" - ); - } + await safeOperation( + async () => + this.sfFlagsStep.execute({ + sfAccountId: sfResult.sfAccountId, + whmcsClientId: whmcsResult.whmcsClientId, + }), + { + criticality: OperationCriticality.OPTIONAL, + fallback: undefined, + context: "SF flags update", + logger: this.logger, + metadata: { email: session.email }, + } + ); // Step 5: Generate auth result const authResult = await this.authResultStep.execute({ @@ -226,16 +224,10 @@ export class SfCompletionWorkflowService { } private async ensureNoExistingAccounts(email: string): Promise { - const [portalUser, whmcsClient] = await Promise.all([ - this.usersService.findByEmailInternal(email), - this.whmcsDiscovery.findClientByEmail(email), - ]); + // Only check Portal — the verification phase already checked WHMCS, + // and the AddClient step will reject duplicates if one was created in the interim. + const portalUser = await this.usersService.findByEmailInternal(email); - if (whmcsClient) { - throw new ConflictException( - "A billing account already exists. Please use the account migration flow." - ); - } if (portalUser) { throw new ConflictException("An account already exists. Please log in."); } diff --git a/apps/portal/src/features/auth/components/LoginForm/LoginForm.tsx b/apps/portal/src/features/auth/components/LoginForm/LoginForm.tsx index cbfc38b4..1fa44d95 100644 --- a/apps/portal/src/features/auth/components/LoginForm/LoginForm.tsx +++ b/apps/portal/src/features/auth/components/LoginForm/LoginForm.tsx @@ -19,6 +19,7 @@ import { LoginOtpStep } from "../LoginOtpStep"; import { useLoginWithOtp } from "../../hooks/use-auth"; import { loginRequestSchema } from "@customer-portal/domain/auth"; import { useZodForm } from "@/shared/hooks"; +import { parseError } from "@/shared/utils"; import { z } from "zod"; import { getSafeRedirect } from "@/features/auth/utils/route-protection"; @@ -94,8 +95,7 @@ export function LoginForm({ onSuccess?.(); } } catch (err) { - const message = err instanceof Error ? err.message : "Login failed"; - onError?.(message); + onError?.(parseError(err).message); throw err; } }, @@ -112,8 +112,7 @@ export function LoginForm({ await verifyOtp(otpState.sessionToken, code, rememberDevice); onSuccess?.(); } catch (err) { - const message = err instanceof Error ? err.message : "Verification failed"; - onError?.(message); + onError?.(parseError(err).message); throw err; } }, diff --git a/apps/portal/src/features/auth/components/PasswordResetForm/PasswordResetForm.tsx b/apps/portal/src/features/auth/components/PasswordResetForm/PasswordResetForm.tsx index 41c3c9f4..49a5b064 100644 --- a/apps/portal/src/features/auth/components/PasswordResetForm/PasswordResetForm.tsx +++ b/apps/portal/src/features/auth/components/PasswordResetForm/PasswordResetForm.tsx @@ -13,6 +13,7 @@ import { PasswordRequirements, PasswordMatchIndicator } from ".."; import { usePasswordReset } from "../../hooks/use-auth"; import { usePasswordValidation } from "../../hooks/usePasswordValidation"; import { useZodForm } from "@/shared/hooks"; +import { parseError } from "@/shared/utils"; import { passwordResetRequestSchema, passwordResetSchema } from "@customer-portal/domain/auth"; import { z } from "zod"; @@ -66,8 +67,7 @@ export function PasswordResetForm({ await requestPasswordReset(data.email); onSuccess?.(); } catch (err) { - const errorMessage = err instanceof Error ? err.message : "Request failed"; - onError?.(errorMessage); + onError?.(parseError(err).message); throw err; } }, @@ -82,8 +82,7 @@ export function PasswordResetForm({ await resetPassword(data.token, data.password); onSuccess?.(); } catch (err) { - const errorMessage = err instanceof Error ? err.message : "Reset failed"; - onError?.(errorMessage); + onError?.(parseError(err).message); throw err; } }, diff --git a/apps/portal/src/features/auth/components/SetPasswordForm/SetPasswordForm.tsx b/apps/portal/src/features/auth/components/SetPasswordForm/SetPasswordForm.tsx index 513c935a..b894ecff 100644 --- a/apps/portal/src/features/auth/components/SetPasswordForm/SetPasswordForm.tsx +++ b/apps/portal/src/features/auth/components/SetPasswordForm/SetPasswordForm.tsx @@ -12,6 +12,7 @@ import { PasswordRequirements, PasswordMatchIndicator } from ".."; import { useAuth } from "../../hooks/use-auth"; import { usePasswordValidation } from "../../hooks/usePasswordValidation"; import { useZodForm } from "@/shared/hooks"; +import { parseError } from "@/shared/utils"; import { setPasswordRequestSchema, getPasswordStrengthDisplay } from "@customer-portal/domain/auth"; import { z } from "zod"; @@ -47,7 +48,7 @@ export function SetPasswordForm({ await setPassword(data.email, data.password); onSuccess?.(); } catch (err) { - onError?.(err instanceof Error ? err.message : "Failed to set password"); + onError?.(parseError(err).message); throw err; } }, diff --git a/apps/portal/src/features/auth/stores/auth.store.ts b/apps/portal/src/features/auth/stores/auth.store.ts index faf10ed0..7473fb02 100644 --- a/apps/portal/src/features/auth/stores/auth.store.ts +++ b/apps/portal/src/features/auth/stores/auth.store.ts @@ -302,10 +302,8 @@ export const useAuthStore = create()((set, get) => { } applyAuthResponse(parsed.data); } catch (error) { - set({ - loading: false, - error: error instanceof Error ? error.message : "Signup failed", - }); + const parsed = parseError(error); + set({ loading: false, error: parsed.message }); throw error; } }, @@ -340,10 +338,8 @@ export const useAuthStore = create()((set, get) => { }); set({ loading: false }); } catch (error) { - set({ - loading: false, - error: error instanceof Error ? error.message : "Password reset request failed", - }); + const parsed = parseError(error); + set({ loading: false, error: parsed.message }); throw error; } }, @@ -364,10 +360,8 @@ export const useAuthStore = create()((set, get) => { error: null, }); } catch (error) { - set({ - loading: false, - error: error instanceof Error ? error.message : "Password reset failed", - }); + const parsed = parseError(error); + set({ loading: false, error: parsed.message }); throw error; } }, @@ -384,10 +378,8 @@ export const useAuthStore = create()((set, get) => { } applyAuthResponse(parsed.data); } catch (error) { - set({ - loading: false, - error: error instanceof Error ? error.message : "Password change failed", - }); + const parsed = parseError(error); + set({ loading: false, error: parsed.message }); throw error; } }, @@ -408,10 +400,8 @@ export const useAuthStore = create()((set, get) => { set({ loading: false }); return parsed.data; } catch (error) { - set({ - loading: false, - error: error instanceof Error ? error.message : "Check failed", - }); + const parsed = parseError(error); + set({ loading: false, error: parsed.message }); throw error; } }, @@ -429,10 +419,8 @@ export const useAuthStore = create()((set, get) => { } applyAuthResponse(parsed.data); } catch (error) { - set({ - loading: false, - error: error instanceof Error ? error.message : "Set password failed", - }); + const parsed = parseError(error); + set({ loading: false, error: parsed.message }); throw error; } }, diff --git a/apps/portal/src/shared/hooks/useZodForm.ts b/apps/portal/src/shared/hooks/useZodForm.ts index ed6004b1..898d98df 100644 --- a/apps/portal/src/shared/hooks/useZodForm.ts +++ b/apps/portal/src/shared/hooks/useZodForm.ts @@ -6,6 +6,7 @@ import { useCallback, useMemo, useRef, useState } from "react"; import type { FormEvent } from "react"; import { ZodError, type ZodIssue, type ZodType } from "zod"; +import { parseError } from "@/shared/utils/error-handling"; export type FormErrors<_TValues extends Record> = Record< string, @@ -212,9 +213,23 @@ export function useZodForm>({ try { await onSubmit(values); } catch (error) { - const message = error instanceof Error ? error.message : String(error); + const parsed = parseError(error); + const message = parsed.message; setSubmitError(message); - setErrors(prev => ({ ...prev, ["_form"]: message })); + + if (parsed.fieldErrors && Object.keys(parsed.fieldErrors).length > 0) { + // Merge server field errors into form errors and mark those fields as touched + setErrors(prev => ({ ...prev, ...parsed.fieldErrors, ["_form"]: message })); + setTouchedState(prev => { + const next = { ...prev }; + for (const key of Object.keys(parsed.fieldErrors!)) { + next[key] = true; + } + return next; + }); + } else { + setErrors(prev => ({ ...prev, ["_form"]: message })); + } } finally { setIsSubmitting(false); } diff --git a/apps/portal/src/shared/utils/error-handling.ts b/apps/portal/src/shared/utils/error-handling.ts index 4ccf17b4..c1e370d5 100644 --- a/apps/portal/src/shared/utils/error-handling.ts +++ b/apps/portal/src/shared/utils/error-handling.ts @@ -23,6 +23,7 @@ export interface ParsedError { message: string; shouldLogout: boolean; shouldRetry: boolean; + fieldErrors?: Record | undefined; } // ============================================================================ @@ -78,11 +79,13 @@ function parseApiError(error: ClientApiError): ParsedError { : ErrorCode.UNKNOWN; const metadata = ErrorMetadata[resolvedCode] ?? ErrorMetadata[ErrorCode.UNKNOWN]; + const fieldErrors = extractFieldErrors(domainError.error.details); return { code: resolvedCode, message: domainError.error.message, shouldLogout: metadata.shouldLogout, shouldRetry: metadata.shouldRetry, + ...(fieldErrors && { fieldErrors }), }; } @@ -97,6 +100,26 @@ function parseApiError(error: ClientApiError): ParsedError { }; } +/** + * Extract fieldErrors from error details if present and valid. + * Validates that the value is a Record at runtime. + */ +function extractFieldErrors(details: unknown): Record | undefined { + if (!details || typeof details !== "object" || Array.isArray(details)) return undefined; + + const raw = (details as Record)["fieldErrors"]; + if (!raw || typeof raw !== "object" || Array.isArray(raw)) return undefined; + + const result: Record = {}; + for (const [key, value] of Object.entries(raw as Record)) { + if (typeof value === "string") { + result[key] = value; + } + } + + return Object.keys(result).length > 0 ? result : undefined; +} + /** * Parse native JavaScript errors (network, timeout, etc.) */ diff --git a/docs/plans/2026-03-03-signup-flow-simplification-design.md b/docs/plans/2026-03-03-signup-flow-simplification-design.md new file mode 100644 index 00000000..6b7c7050 --- /dev/null +++ b/docs/plans/2026-03-03-signup-flow-simplification-design.md @@ -0,0 +1,242 @@ +# Auth Flow Simplification Design + +**Date**: 2026-03-03 +**Status**: Approved + +## Problem + +The auth system has accumulated duplication and dead code: + +1. **Duplicate signup workflows**: `SfCompletionWorkflow` and `NewCustomerSignupWorkflow` are 85% identical (~300 lines each). The only difference is eligibility case creation and a welcome email. +2. **Confusing names**: `SfCompletionWorkflow` doesn't describe what it does (it creates full accounts, not just "completes SF"). +3. **Missing SF fields**: `Birthdate` and `Sex__c` are not set on Salesforce PersonContact during account creation. The only code that called `createContact()` lived in dead legacy code. +4. **Dead code**: `signup/` directory (6 files), `WhmcsLinkWorkflowService` — all unreferenced or superseded. +5. **Legacy duplicate**: `WhmcsLinkWorkflowService` (password-based migration via `POST /auth/migrate`) is superseded by `WhmcsMigrationWorkflowService` (OTP-based migration via get-started). +6. **Scattered shared services**: `SignupUserCreationService` and `SignupWhmcsService` live in `signup/` but are used by other workflows via `CreatePortalUserStep`. They should live alongside the steps they serve. + +## Approach — 3 Phases (can stop after any phase) + +### Phase 1: Merge signup workflows + cleanup + +Merge the two near-duplicate workflows. Move shared services. Delete dead code. Fix Birthdate/Sex\_\_c. + +### Phase 2: Remove legacy migrate endpoint + +Delete `WhmcsLinkWorkflowService` and `POST /auth/migrate`. All WHMCS migration goes through the get-started OTP-based flow. + +### Phase 3: Clean up OTP infrastructure + +Extract shared OTP email-sending pattern. Login and signup OTP stay as separate workflows (they serve different purposes: 2FA vs email verification). Keep separate sessions (different data models, TTLs, lifecycle). Improve the PORTAL_EXISTS redirect UX (prefill email on login page). + +**Why login and signup stay separate**: In the ISP/telecom industry, login (password → OTP as 2FA) and signup (OTP as email verification → form) serve fundamentally different security purposes. Forcing them into one flow either adds friction for existing users or weakens the security model. Major ISP portals keep these flows separate. + +--- + +## Current Structure + +``` +workflows/ +├── verification-workflow.service.ts # OTP + account status detection +├── guest-eligibility-workflow.service.ts # Guest ISP check (no account created) +├── sf-completion-workflow.service.ts # Creates account (misleading name, 85% overlap) +├── new-customer-signup-workflow.service.ts # Creates account + eligibility (85% overlap) +├── whmcs-migration-workflow.service.ts # OTP-based WHMCS migration (active) +├── whmcs-link-workflow.service.ts # Password-based WHMCS migration (legacy, superseded) +├── login-otp-workflow.service.ts # Login OTP 2FA +├── password-workflow.service.ts # Password operations +├── get-started-coordinator.service.ts # Get-started routing +├── workflow-error.util.ts # Error classification +├── signup/ # Mixed: some active, some dead +│ ├── signup-account-resolver.service.ts # Dead (no active caller) +│ ├── signup-validation.service.ts # Dead (no active caller) +│ ├── signup-whmcs.service.ts # Active (markClientForCleanup used by CreatePortalUserStep) +│ ├── signup-user-creation.service.ts # Active (used by CreatePortalUserStep + WhmcsMigration) +│ ├── signup.types.ts # Dead (only used by dead validation service) +│ └── index.ts # Barrel +└── steps/ # Shared step services + ├── resolve-salesforce-account.step.ts + ├── create-whmcs-client.step.ts + ├── create-portal-user.step.ts # Delegates to SignupUserCreationService + ├── create-eligibility-case.step.ts + ├── update-salesforce-flags.step.ts + ├── generate-auth-result.step.ts + └── index.ts +``` + +## New Structure (after all 3 phases) + +``` +workflows/ +├── verification-workflow.service.ts # OTP + account detection (unchanged) +├── guest-eligibility-workflow.service.ts # Guest ISP check (unchanged) +├── account-creation-workflow.service.ts # MERGED: replaces sf-completion + new-customer-signup +├── account-migration-workflow.service.ts # RENAMED: from whmcs-migration +├── login-otp-workflow.service.ts # Login OTP 2FA (Phase 3: uses shared OTP orchestration) +├── password-workflow.service.ts # Unchanged +├── get-started-coordinator.service.ts # Updated imports +├── workflow-error.util.ts # Unchanged +└── steps/ # Shared steps + moved services + ├── resolve-salesforce-account.step.ts + ├── create-whmcs-client.step.ts + ├── create-portal-user.step.ts + ├── create-eligibility-case.step.ts + ├── update-salesforce-flags.step.ts + ├── generate-auth-result.step.ts + ├── portal-user-creation.service.ts # MOVED from signup/ (renamed) + ├── whmcs-cleanup.service.ts # MOVED from signup/ (renamed, trimmed) + └── index.ts +``` + +**Deleted (all phases combined):** + +- `sf-completion-workflow.service.ts` (merged, Phase 1) +- `new-customer-signup-workflow.service.ts` (merged, Phase 1) +- `whmcs-migration-workflow.service.ts` (renamed, Phase 1) +- `whmcs-link-workflow.service.ts` (removed, Phase 2) +- `signup/` entire directory (Phase 1) + +--- + +## Phase 1: Merge signup workflows + cleanup + +### Merged Workflow: AccountCreationWorkflowService + +**API:** + +```typescript +class AccountCreationWorkflowService { + execute( + request: CompleteAccountRequest | SignupWithEligibilityRequest, + options?: { withEligibility?: boolean } + ): Promise; +} +``` + +**Routing:** + +- `POST /auth/get-started/complete-account` → `execute(request)` (no eligibility) +- `POST /auth/get-started/signup-with-eligibility` → `execute(request, { withEligibility: true })` + +**Step Sequence:** + +``` +1. Validate + check existing accounts +2. Hash password +3. Resolve address + names (from request or session prefill) +4. Step 1: Resolve SF Account (CRITICAL) +5. Step 1.5: Write SF Address (DEGRADABLE) +6. Step 1.6: Create SF Contact — Birthdate + Sex__c (DEGRADABLE) ← NEW, fixes the bug +7. [if withEligibility] Step 2: Create Eligibility Case (DEGRADABLE) +8. Step 3: Create WHMCS Client (CRITICAL, rollback) +9. Step 4: Create Portal User (CRITICAL, rollback WHMCS on fail) +10. Step 5: Update SF Flags (DEGRADABLE) +11. Step 6: Generate Auth Result +12. [if withEligibility] Send welcome email with eligibility ref (DEGRADABLE) +13. Invalidate session +``` + +**Error handling difference between paths:** + +- `completeAccount` path: throws exceptions directly (current sf-completion behavior) +- `signupWithEligibility` path: wraps in try/catch, returns `{ success: false, message, errorCategory }` using `classifyError()` + +**Birthdate/Sex\_\_c Fix:** +After resolving the SF account, call `SalesforceAccountService.createContact()` on the PersonContact: + +- `Birthdate` → `dateOfBirth` (YYYY-MM-DD) +- `Sex__c` → mapped from `gender` (`male`→`男性`, `female`→`女性`, `other`→`Other`) + This is degradable — account creation continues if it fails. + +### Rename: AccountMigrationWorkflowService + +Rename `WhmcsMigrationWorkflowService` → `AccountMigrationWorkflowService`. Refactor to use `CreatePortalUserStep` instead of calling `SignupUserCreationService` directly (reuse existing step, same as other workflows). + +### Move signup/ services into steps/ + +| Old | New | Changes | +| ---------------------------------------- | --------------------------------------- | ------------------------------------------------------------------- | +| `signup/signup-user-creation.service.ts` | `steps/portal-user-creation.service.ts` | Rename class to `PortalUserCreationService` | +| `signup/signup-whmcs.service.ts` | `steps/whmcs-cleanup.service.ts` | Rename to `WhmcsCleanupService`, keep only `markClientForCleanup()` | + +### Delete dead code + +| File | Reason | +| ------------------------------------------- | ------------------------------------ | +| `signup/signup-account-resolver.service.ts` | No active caller | +| `signup/signup-validation.service.ts` | No active caller | +| `signup/signup.types.ts` | Only used by dead validation service | +| `signup/index.ts` | Directory being removed | + +--- + +## Phase 2: Remove legacy migrate endpoint + +### Delete WhmcsLinkWorkflowService + +`WhmcsLinkWorkflowService` (`POST /auth/migrate`) is superseded by `AccountMigrationWorkflowService` (`POST /auth/get-started/migrate-whmcs-account`). The new flow uses OTP-based identity verification instead of WHMCS password validation — strictly more secure. + +**Files to modify:** + +- `apps/bff/src/modules/auth/auth.module.ts` — remove provider + import +- `apps/bff/src/modules/auth/application/auth-orchestrator.service.ts` — remove `linkWhmcsUser()` method + dependency +- `apps/bff/src/modules/auth/presentation/http/auth.controller.ts` — remove `POST /auth/migrate` endpoint + +**File to delete:** + +- `apps/bff/src/modules/auth/infra/workflows/whmcs-link-workflow.service.ts` + +--- + +## Phase 3: Clean up OTP infrastructure + +### Extract shared OTP email-sending pattern + +Both `LoginOtpWorkflowService` and `VerificationWorkflowService` implement the same pattern: + +1. Look up email template ID from config +2. If template exists, send via template with dynamic data +3. If no template, send fallback HTML + +Extract this into a shared `OtpEmailService` (or method on existing `OtpService`). + +### Improve PORTAL_EXISTS redirect UX + +When get-started verification detects `PORTAL_EXISTS`, pass the verified email to the login page so it's prefilled. This reduces friction for users who accidentally start on the wrong page. + +### What stays separate (and why) + +- **Login OTP workflow**: OTP serves as 2FA after password validation +- **Verification OTP workflow**: OTP serves as email ownership proof before signup +- **LoginSessionService**: Simple (5 fields, 10min TTL, no prefill) +- **GetStartedSessionService**: Complex (10+ fields, 1hr TTL, prefill data, handoff tokens, idempotency locking) + +These serve fundamentally different security purposes and have different data models. Forcing unification would create a confusing abstraction. + +--- + +## Module Updates (all phases) + +### GetStartedModule (Phase 1) + +- Remove: `NewCustomerSignupWorkflowService`, `SfCompletionWorkflowService`, `WhmcsMigrationWorkflowService` +- Remove: `SignupAccountResolverService`, `SignupValidationService`, `SignupWhmcsService`, `SignupUserCreationService` +- Add: `AccountCreationWorkflowService`, `AccountMigrationWorkflowService` +- Add: `PortalUserCreationService`, `WhmcsCleanupService` + +### GetStartedCoordinator (Phase 1) + +- `completeAccount()` → `accountCreation.execute(request)` +- `signupWithEligibility()` → `accountCreation.execute(request, { withEligibility: true })` +- `migrateWhmcsAccount()` → `accountMigration.execute(request)` + +### AuthModule (Phase 2) + +- Remove: `WhmcsLinkWorkflowService` from providers and imports + +### AuthOrchestrator (Phase 2) + +- Remove: `linkWhmcsUser()` method and `WhmcsLinkWorkflowService` dependency + +### AuthController (Phase 2) + +- Remove: `POST /auth/migrate` endpoint diff --git a/docs/plans/2026-03-03-signup-flow-simplification-plan.md b/docs/plans/2026-03-03-signup-flow-simplification-plan.md new file mode 100644 index 00000000..8afdb0c0 --- /dev/null +++ b/docs/plans/2026-03-03-signup-flow-simplification-plan.md @@ -0,0 +1,508 @@ +# Auth Flow Simplification Implementation Plan + +> **For Claude:** REQUIRED SUB-SKILL: Use superpowers:executing-plans to implement this plan task-by-task. + +**Goal:** Simplify auth flows: merge duplicate signup workflows, fix missing SF Birthdate/Sex\_\_c, remove legacy migrate endpoint, clean up OTP infrastructure, delete dead code. + +**Architecture:** 3 phases (can stop after any phase). Phase 1 merges signup workflows + cleans up. Phase 2 removes legacy WHMCS migration. Phase 3 extracts shared OTP pattern. + +**Tech Stack:** NestJS 11, TypeScript, Prisma, Salesforce jsforce, WHMCS API, argon2, BullMQ + +**Design doc:** `docs/plans/2026-03-03-signup-flow-simplification-design.md` + +--- + +# Phase 1: Merge signup workflows + cleanup + +## Task 1: Move signup shared services into steps/ + +Move `SignupUserCreationService` and `SignupWhmcsService` from `signup/` into `steps/`, renaming to remove the "Signup" prefix. Update all imports. + +**Files:** + +- Create: `apps/bff/src/modules/auth/infra/workflows/steps/portal-user-creation.service.ts` +- Create: `apps/bff/src/modules/auth/infra/workflows/steps/whmcs-cleanup.service.ts` +- Modify: `apps/bff/src/modules/auth/infra/workflows/steps/create-portal-user.step.ts` +- Modify: `apps/bff/src/modules/auth/infra/workflows/steps/index.ts` +- Modify: `apps/bff/src/modules/auth/infra/workflows/whmcs-migration-workflow.service.ts` +- Modify: `apps/bff/src/modules/auth/get-started/get-started.module.ts` + +**Step 1: Create `portal-user-creation.service.ts`** + +Copy `signup/signup-user-creation.service.ts` to `steps/portal-user-creation.service.ts`: + +- Rename class: `SignupUserCreationService` → `PortalUserCreationService` +- Update import: `SignupWhmcsService` → `WhmcsCleanupService` from `./whmcs-cleanup.service.js` + +**Step 2: Create `whmcs-cleanup.service.ts`** + +Copy `signup/signup-whmcs.service.ts` to `steps/whmcs-cleanup.service.ts`: + +- Rename class: `SignupWhmcsService` → `WhmcsCleanupService` +- Keep only `markClientForCleanup()` method +- Delete: `createClient()`, `checkExistingClient()`, `validateAddressData()` (unused by active code) +- Remove unused imports: `SignupRequest`, `serializeWhmcsKeyValueMap`, `DomainHttpException`, `ErrorCode`, `ConfigService`, `MappingsService`, `WhmcsAccountDiscoveryService` +- Constructor only needs: `WhmcsClientService` + `Logger` + +**Step 3: Update `create-portal-user.step.ts` import** + +``` +Line 3 — FROM: import { SignupUserCreationService } from "../signup/signup-user-creation.service.js"; +Line 3 — TO: import { PortalUserCreationService } from "./portal-user-creation.service.js"; +``` + +Update constructor param: `signupUserCreation` → `portalUserCreation`, update usage on line 36. + +**Step 4: Update `steps/index.ts` barrel** + +Add at end: + +```typescript +export { PortalUserCreationService } from "./portal-user-creation.service.js"; +export type { UserCreationParams, CreatedUserResult } from "./portal-user-creation.service.js"; +export { WhmcsCleanupService } from "./whmcs-cleanup.service.js"; +``` + +**Step 5: Update `whmcs-migration-workflow.service.ts` import** + +``` +Line 24 — FROM: import { SignupUserCreationService } from "./signup/signup-user-creation.service.js"; +Line 24 — TO: import { PortalUserCreationService } from "./steps/portal-user-creation.service.js"; +``` + +Update constructor param and usage accordingly. (Task 3 will refactor this further to use `CreatePortalUserStep`.) + +**Step 6: Update `get-started.module.ts`** + +- Remove imports: `SignupAccountResolverService`, `SignupValidationService`, `SignupWhmcsService`, `SignupUserCreationService` +- Add imports: `PortalUserCreationService`, `WhmcsCleanupService` from `../infra/workflows/steps/index.js` +- Remove from providers: `SignupAccountResolverService`, `SignupValidationService`, `SignupWhmcsService`, `SignupUserCreationService` +- Add to providers: `PortalUserCreationService`, `WhmcsCleanupService` + +**Step 7: Run type-check** + +Run: `pnpm type-check` +Expected: PASS + +**Step 8: Commit** + +```bash +git add apps/bff/src/modules/auth/infra/workflows/steps/portal-user-creation.service.ts \ + apps/bff/src/modules/auth/infra/workflows/steps/whmcs-cleanup.service.ts \ + apps/bff/src/modules/auth/infra/workflows/steps/create-portal-user.step.ts \ + apps/bff/src/modules/auth/infra/workflows/steps/index.ts \ + apps/bff/src/modules/auth/infra/workflows/whmcs-migration-workflow.service.ts \ + apps/bff/src/modules/auth/get-started/get-started.module.ts +git commit -m "refactor: move signup shared services into steps directory" +``` + +--- + +## Task 2: Create merged AccountCreationWorkflowService + +Merge `SfCompletionWorkflowService` + `NewCustomerSignupWorkflowService` into one workflow. Fix Birthdate/Sex\_\_c gap. + +**Files:** + +- Create: `apps/bff/src/modules/auth/infra/workflows/account-creation-workflow.service.ts` + +**Step 1: Create the merged workflow** + +Constructor dependencies (union of both old workflows): + +- `ConfigService` (email template IDs, APP_BASE_URL) +- `GetStartedSessionService` +- `DistributedLockService` +- `UsersService` +- `EmailService` (welcome email in eligibility path) +- `SalesforceAccountService` (for `createContact()` — Birthdate/Sex\_\_c) +- `AddressWriterService` +- `ResolveSalesforceAccountStep` +- `CreateEligibilityCaseStep` +- `CreateWhmcsClientStep` +- `CreatePortalUserStep` +- `UpdateSalesforceFlagsStep` +- `GenerateAuthResultStep` +- `Logger` + +Public API: + +```typescript +execute( + request: CompleteAccountRequest | SignupWithEligibilityRequest, + options?: { withEligibility?: boolean } +): Promise +``` + +Step sequence in `executeCreation()`: + +1. Check existing accounts (Portal user duplicate check) +2. Hash password (argon2) +3. Resolve address + names (from request or session prefill — port `resolveAddress()` and `resolveNames()` from sf-completion) +4. **Step 1: Resolve SF Account** (CRITICAL) — `source` depends on `withEligibility` +5. **Step 1.5: Write SF Address** (DEGRADABLE via `safeOperation`) +6. **Step 1.6: Create SF Contact** (DEGRADABLE via `safeOperation`) — **NEW: fixes Birthdate/Sex\_\_c** + - Call `salesforceAccountService.createContact({ accountId, firstName, lastName, email, phone, gender, dateOfBirth })` +7. **Step 2** (conditional): Create Eligibility Case (DEGRADABLE, only if `withEligibility`) +8. **Step 3: Create WHMCS Client** (CRITICAL) +9. **Step 4: Create Portal User** (CRITICAL, rollback WHMCS on fail) +10. **Step 5: Update SF Flags** (DEGRADABLE) +11. **Step 6: Generate Auth Result** +12. (conditional) Send welcome email (DEGRADABLE, only if `withEligibility`) — port `sendWelcomeWithEligibilityEmail()` from new-customer-signup +13. Invalidate session + +Error handling per path: + +- `withEligibility=true`: wrap in try/catch, return `{ success: false, errorCategory, message }` using `classifyError()` (from `workflow-error.util.ts`) +- `withEligibility=false` (default): let exceptions propagate directly (matching current sf-completion behavior) + +Session handling: + +- Use `acquireAndMarkAsUsed(sessionToken, withEligibility ? "signup_with_eligibility" : "complete_account")` +- `isNewCustomer = !session.sfAccountId` (same logic as sf-completion) +- For `isNewCustomer`: require firstName, lastName, address from request +- For SF_UNMAPPED: use session prefill data as fallback + +Port private helpers from sf-completion: + +- `validateRequest()` — checks isNewCustomer needs firstName/lastName/address +- `ensureNoExistingAccounts()` — Portal user duplicate check +- `resolveAddress()` — from request or session +- `resolveNames()` — from request or session + +**Step 2: Run type-check** + +Run: `pnpm type-check` +Expected: PASS + +**Step 3: Commit** + +```bash +git add apps/bff/src/modules/auth/infra/workflows/account-creation-workflow.service.ts +git commit -m "feat: add merged AccountCreationWorkflowService with Birthdate/Sex__c fix" +``` + +--- + +## Task 3: Rename and refactor AccountMigrationWorkflowService + +Rename `WhmcsMigrationWorkflowService` → `AccountMigrationWorkflowService`. Refactor to use `CreatePortalUserStep` instead of `SignupUserCreationService` directly. + +**Files:** + +- Create: `apps/bff/src/modules/auth/infra/workflows/account-migration-workflow.service.ts` + +**Step 1: Create renamed + refactored file** + +Copy `whmcs-migration-workflow.service.ts` → `account-migration-workflow.service.ts`: + +- Rename class: `WhmcsMigrationWorkflowService` → `AccountMigrationWorkflowService` +- Replace `PortalUserCreationService` (or `SignupUserCreationService` if Task 1 import not yet updated) with `CreatePortalUserStep` +- Constructor: remove `userCreation: PortalUserCreationService`, add `portalUserStep: CreatePortalUserStep` +- In `executeMigration()`: replace `this.userCreation.createUserWithMapping({...})` with `this.portalUserStep.execute({...})` +- Update imports accordingly + +**Step 2: Run type-check** + +Run: `pnpm type-check` +Expected: PASS + +**Step 3: Commit** + +```bash +git add apps/bff/src/modules/auth/infra/workflows/account-migration-workflow.service.ts +git commit -m "refactor: rename WhmcsMigration to AccountMigration, use CreatePortalUserStep" +``` + +--- + +## Task 4: Update coordinator and module + +Wire new services, remove old references. + +**Files:** + +- Modify: `apps/bff/src/modules/auth/infra/workflows/get-started-coordinator.service.ts` +- Modify: `apps/bff/src/modules/auth/get-started/get-started.module.ts` + +**Step 1: Update coordinator** + +Imports: + +- Remove: `NewCustomerSignupWorkflowService`, `SfCompletionWorkflowService`, `WhmcsMigrationWorkflowService` +- Add: `AccountCreationWorkflowService`, `AccountMigrationWorkflowService` + +Constructor: + +- Remove: `newCustomerSignup`, `sfCompletion`, `whmcsMigration` +- Add: `accountCreation: AccountCreationWorkflowService`, `accountMigration: AccountMigrationWorkflowService` + +Methods: + +- `completeAccount()`: `return this.accountCreation.execute(request);` +- `signupWithEligibility()`: `return this.accountCreation.execute(request, { withEligibility: true });` +- `migrateWhmcsAccount()`: `return this.accountMigration.execute(request);` + +**Step 2: Update module providers** + +Imports: + +- Remove: `NewCustomerSignupWorkflowService`, `SfCompletionWorkflowService`, `WhmcsMigrationWorkflowService` +- Add: `AccountCreationWorkflowService`, `AccountMigrationWorkflowService` + +Providers: + +- Remove: `NewCustomerSignupWorkflowService`, `SfCompletionWorkflowService`, `WhmcsMigrationWorkflowService` +- Add: `AccountCreationWorkflowService`, `AccountMigrationWorkflowService` + +**Step 3: Run type-check** + +Run: `pnpm type-check` +Expected: PASS + +**Step 4: Commit** + +```bash +git add apps/bff/src/modules/auth/infra/workflows/get-started-coordinator.service.ts \ + apps/bff/src/modules/auth/get-started/get-started.module.ts +git commit -m "refactor: wire AccountCreation and AccountMigration into coordinator and module" +``` + +--- + +## Task 5: Delete old files + +**Step 1: Delete old workflow files** + +```bash +rm apps/bff/src/modules/auth/infra/workflows/sf-completion-workflow.service.ts +rm apps/bff/src/modules/auth/infra/workflows/new-customer-signup-workflow.service.ts +rm apps/bff/src/modules/auth/infra/workflows/whmcs-migration-workflow.service.ts +``` + +**Step 2: Delete entire signup/ directory** + +```bash +rm -rf apps/bff/src/modules/auth/infra/workflows/signup/ +``` + +**Step 3: Run type-check and tests** + +Run: `pnpm type-check && pnpm --filter @customer-portal/bff test` +Expected: PASS + +**Step 4: Commit** + +```bash +git add -A +git commit -m "chore: delete merged/deprecated signup workflow files and signup/ directory" +``` + +--- + +## Task 6: Verify no stale references (Phase 1 checkpoint) + +**Step 1: Search for stale references in source code** + +Search `apps/` and `packages/` for: + +- `SfCompletionWorkflow`, `NewCustomerSignupWorkflow`, `WhmcsMigrationWorkflow` +- `SignupAccountResolver`, `SignupValidation`, `SignupWhmcsService`, `SignupUserCreation` +- `signup-account-resolver`, `signup-validation`, `signup-whmcs`, `signup-user-creation` + +Expected: No matches in `apps/` or `packages/` (docs/ matches are fine) + +**Step 2: Fix any remaining references** + +**Step 3: Run full type-check and tests** + +Run: `pnpm type-check && pnpm --filter @customer-portal/bff test` +Expected: PASS + +**Step 4: Commit if fixes were needed** + +```bash +git add -A +git commit -m "fix: update remaining references to old workflow service names" +``` + +--- + +# Phase 2: Remove legacy migrate endpoint + +## Task 7: Remove WhmcsLinkWorkflowService and /auth/migrate + +Remove the legacy password-based WHMCS migration. All WHMCS migration now goes through get-started OTP-based flow. + +**Files:** + +- Delete: `apps/bff/src/modules/auth/infra/workflows/whmcs-link-workflow.service.ts` +- Modify: `apps/bff/src/modules/auth/auth.module.ts` (remove provider + import) +- Modify: `apps/bff/src/modules/auth/application/auth-orchestrator.service.ts` (remove `linkWhmcsUser()` + dependency) +- Modify: `apps/bff/src/modules/auth/presentation/http/auth.controller.ts` (remove `POST /auth/migrate` endpoint) + +**Step 1: Update AuthOrchestrator** + +- Remove `linkWhmcsUser()` method +- Remove `WhmcsLinkWorkflowService` from constructor injection +- Remove import + +**Step 2: Update AuthController** + +- Remove the `POST /auth/migrate` endpoint handler +- Remove related imports (request DTO, etc.) + +**Step 3: Update AuthModule** + +- Remove `WhmcsLinkWorkflowService` from providers +- Remove import + +**Step 4: Delete the workflow file** + +```bash +rm apps/bff/src/modules/auth/infra/workflows/whmcs-link-workflow.service.ts +``` + +**Step 5: Search for stale references** + +Search for `WhmcsLinkWorkflow`, `linkWhmcsUser`, `/auth/migrate` in source code. + +**Step 6: Run type-check and tests** + +Run: `pnpm type-check && pnpm --filter @customer-portal/bff test` +Expected: PASS + +**Step 7: Commit** + +```bash +git add -A +git commit -m "refactor: remove legacy /auth/migrate endpoint and WhmcsLinkWorkflowService" +``` + +--- + +# Phase 3: Clean up OTP infrastructure + +## Task 8: Extract shared OTP email-sending pattern + +Both `LoginOtpWorkflowService.sendOtpEmail()` and `VerificationWorkflowService.sendOtpEmail()` implement the same pattern: look up template ID from config, send via template or fallback HTML. + +**Files:** + +- Create: `apps/bff/src/modules/auth/infra/otp/otp-email.service.ts` +- Modify: `apps/bff/src/modules/auth/infra/workflows/login-otp-workflow.service.ts` +- Modify: `apps/bff/src/modules/auth/infra/workflows/verification-workflow.service.ts` +- Modify: `apps/bff/src/modules/auth/otp/otp.module.ts` (add provider) + +**Step 1: Create `OtpEmailService`** + +Extract the shared email-sending pattern: + +```typescript +@Injectable() +export class OtpEmailService { + constructor( + private readonly config: ConfigService, + private readonly emailService: EmailService + ) {} + + async sendOtpCode(email: string, code: string, context: "login" | "verification"): Promise { + // Look up template ID based on context + // Send via template or fallback HTML + // Both current implementations do the same thing with different template IDs + } +} +``` + +**Step 2: Refactor LoginOtpWorkflowService** + +Replace inline `sendOtpEmail()` with `otpEmailService.sendOtpCode(email, code, 'login')`. + +**Step 3: Refactor VerificationWorkflowService** + +Replace inline `sendOtpEmail()` with `otpEmailService.sendOtpCode(email, code, 'verification')`. + +**Step 4: Update OtpModule** + +Add `OtpEmailService` to providers and exports. + +**Step 5: Run type-check and tests** + +Run: `pnpm type-check && pnpm --filter @customer-portal/bff test` +Expected: PASS + +**Step 6: Commit** + +```bash +git add -A +git commit -m "refactor: extract shared OTP email service from login and verification workflows" +``` + +--- + +## Task 9: Improve PORTAL_EXISTS redirect UX + +When get-started verification detects `PORTAL_EXISTS`, the frontend redirects to login without context. Improve this by passing the verified email so it can be prefilled. + +**Files:** + +- Modify: `apps/portal/src/features/get-started/machines/get-started.machine.ts` +- Potentially: login page component (to accept prefilled email from query param or state) + +**Step 1: Update get-started machine** + +When `accountStatus === PORTAL_EXISTS`, redirect to login with email as query param: + +```typescript +// Instead of: router.push('/login') +// Do: router.push(`/login?email=${encodeURIComponent(verifiedEmail)}`) +``` + +**Step 2: Update login page to accept prefilled email** + +Read `email` from URL query params and prefill the email input field. + +**Step 3: Test manually** + +1. Go to get-started with an existing account email +2. Verify via OTP +3. Should redirect to login with email prefilled + +**Step 4: Commit** + +```bash +git add -A +git commit -m "feat: prefill email on login page when redirected from get-started" +``` + +--- + +## Notes + +### Error handling difference between merged paths + +The `completeAccount` endpoint throws exceptions directly. The `signupWithEligibility` endpoint returns `{ success: false, message, errorCategory }`. The merged workflow preserves this: + +```typescript +if (options?.withEligibility) { + try { return await this.executeCreation(...); } + catch (error) { + const classified = classifyError(error); + return { success: false, errorCategory: classified.errorCategory, message: classified.message }; + } +} else { + return await this.executeCreation(...); // exceptions propagate +} +``` + +### createContact() for Birthdate/Sex\_\_c + +New degradable step using existing `SalesforceAccountService.createContact()`. Gender mapping: + +- `male` → `男性` +- `female` → `女性` +- `other` → `Other` + +### Sessions stay separate + +Login sessions (`LoginSessionService`) and get-started sessions (`GetStartedSessionService`) have fundamentally different data models, TTLs, and lifecycle requirements. Only the OTP email-sending pattern is worth extracting.