From 640a4e1094e8dbb15cd2487a1d3c68273ef2b233 Mon Sep 17 00:00:00 2001 From: barsa Date: Thu, 25 Sep 2025 13:32:16 +0900 Subject: [PATCH] Refactor various services to improve code organization and error handling. Streamline provider declarations in Salesforce module, enhance WHMCS service with new client product retrieval method, and update pagination logic in AuthAdminController. Clean up unused imports and improve type handling across multiple modules, ensuring better maintainability and consistency. --- .../salesforce/salesforce.module.ts | 6 +- .../src/integrations/whmcs/whmcs.service.ts | 8 ++ .../src/modules/auth/auth-admin.controller.ts | 2 - .../src/modules/auth/auth-zod.controller.ts | 2 - apps/bff/src/modules/auth/auth.service.ts | 10 +- .../modules/auth/services/token.service.ts | 102 ++++++++++++++---- .../catalog/services/base-catalog.service.ts | 14 ++- .../catalog/services/sim-catalog.service.ts | 1 - .../modules/id-mappings/mappings.service.ts | 30 +++--- .../order-fulfillment-orchestrator.service.ts | 20 +++- .../services/order-orchestrator.service.ts | 8 +- .../subscriptions/subscriptions.service.ts | 30 +++--- apps/bff/src/modules/users/users.service.ts | 6 +- 13 files changed, 154 insertions(+), 85 deletions(-) diff --git a/apps/bff/src/integrations/salesforce/salesforce.module.ts b/apps/bff/src/integrations/salesforce/salesforce.module.ts index 2e6fb1a8..0f1c7f2d 100644 --- a/apps/bff/src/integrations/salesforce/salesforce.module.ts +++ b/apps/bff/src/integrations/salesforce/salesforce.module.ts @@ -4,11 +4,7 @@ import { SalesforceConnection } from "./services/salesforce-connection.service"; import { SalesforceAccountService } from "./services/salesforce-account.service"; @Module({ - providers: [ - SalesforceConnection, - SalesforceAccountService, - SalesforceService, - ], + providers: [SalesforceConnection, SalesforceAccountService, SalesforceService], exports: [SalesforceService, SalesforceConnection], }) export class SalesforceModule {} diff --git a/apps/bff/src/integrations/whmcs/whmcs.service.ts b/apps/bff/src/integrations/whmcs/whmcs.service.ts index aa798f2e..89c8a463 100644 --- a/apps/bff/src/integrations/whmcs/whmcs.service.ts +++ b/apps/bff/src/integrations/whmcs/whmcs.service.ts @@ -23,6 +23,8 @@ import { WhmcsAddClientParams, WhmcsClientResponse, WhmcsCatalogProductsResponse, + WhmcsGetClientsProductsParams, + WhmcsProductsResponse, } from "./types/whmcs-api.types"; import { Logger } from "nestjs-pino"; @@ -337,6 +339,12 @@ export class WhmcsService { return this.connectionService.getSystemInfo(); } + async getClientsProducts( + params: WhmcsGetClientsProductsParams + ): Promise { + return this.connectionService.getClientsProducts(params); + } + // ========================================== // INVOICE CREATION AND PAYMENT OPERATIONS // ========================================== diff --git a/apps/bff/src/modules/auth/auth-admin.controller.ts b/apps/bff/src/modules/auth/auth-admin.controller.ts index 6b901522..97aa553e 100644 --- a/apps/bff/src/modules/auth/auth-admin.controller.ts +++ b/apps/bff/src/modules/auth/auth-admin.controller.ts @@ -33,8 +33,6 @@ export class AuthAdminController { ) { const pageNum = parseInt(page, 10); const limitNum = parseInt(limit, 10); - const skip = (pageNum - 1) * limitNum; - if (Number.isNaN(pageNum) || Number.isNaN(limitNum) || pageNum < 1 || limitNum < 1) { throw new BadRequestException("Invalid pagination parameters"); } diff --git a/apps/bff/src/modules/auth/auth-zod.controller.ts b/apps/bff/src/modules/auth/auth-zod.controller.ts index a76e2f69..ae12a638 100644 --- a/apps/bff/src/modules/auth/auth-zod.controller.ts +++ b/apps/bff/src/modules/auth/auth-zod.controller.ts @@ -11,7 +11,6 @@ import { ZodValidationPipe } from "@bff/core/validation"; // Import Zod schemas from domain import { signupRequestSchema, - loginRequestSchema, passwordResetRequestSchema, passwordResetSchema, setPasswordRequestSchema, @@ -22,7 +21,6 @@ import { ssoLinkRequestSchema, checkPasswordNeededRequestSchema, type SignupRequestInput, - type LoginRequestInput, type PasswordResetRequestInput, type PasswordResetInput, type SetPasswordRequestInput, diff --git a/apps/bff/src/modules/auth/auth.service.ts b/apps/bff/src/modules/auth/auth.service.ts index 4fc80925..a2b78705 100644 --- a/apps/bff/src/modules/auth/auth.service.ts +++ b/apps/bff/src/modules/auth/auth.service.ts @@ -1,10 +1,4 @@ -import { - Injectable, - UnauthorizedException, - ConflictException, - BadRequestException, - Inject, -} from "@nestjs/common"; +import { Injectable, UnauthorizedException, BadRequestException, Inject } from "@nestjs/common"; import { JwtService } from "@nestjs/jwt"; import { ConfigService } from "@nestjs/config"; import * as bcrypt from "bcrypt"; @@ -18,8 +12,6 @@ import { getErrorMessage } from "@bff/core/utils/error.util"; import { Logger } from "nestjs-pino"; import { sanitizeWhmcsRedirectPath } from "@bff/core/utils/sso.util"; import { - authResponseSchema, - type AuthResponse, type SignupRequestInput, type ValidateSignupRequestInput, type LinkWhmcsRequestInput, diff --git a/apps/bff/src/modules/auth/services/token.service.ts b/apps/bff/src/modules/auth/services/token.service.ts index 685f0265..632c85b1 100644 --- a/apps/bff/src/modules/auth/services/token.service.ts +++ b/apps/bff/src/modules/auth/services/token.service.ts @@ -188,13 +188,21 @@ export class AuthTokenService { throw new UnauthorizedException("Invalid refresh token"); } - const tokenData = JSON.parse(storedToken); - if (!tokenData.valid) { + const tokenRecord = this.parseRefreshTokenRecord(storedToken); + if (!tokenRecord) { + this.logger.warn("Stored refresh token payload was invalid JSON", { + tokenHash: refreshTokenHash.slice(0, 8), + }); + await this.redis.del(`${this.REFRESH_TOKEN_PREFIX}${refreshTokenHash}`); + throw new UnauthorizedException("Invalid refresh token"); + } + + if (!tokenRecord.valid) { this.logger.warn("Refresh token marked as invalid", { tokenHash: refreshTokenHash.slice(0, 8), }); // Invalidate entire token family on reuse attempt - await this.invalidateTokenFamily(tokenData.familyId); + await this.invalidateTokenFamily(tokenRecord.familyId); throw new UnauthorizedException("Invalid refresh token"); } @@ -228,13 +236,13 @@ export class AuthTokenService { if (this.redis.status !== "ready") { this.logger.warn("Redis unavailable during token refresh; issuing fallback token pair"); - const fallbackPayload = this.jwtService.decode(refreshToken) as - | RefreshTokenPayload - | null; + const fallbackDecoded = this.jwtService.decode(refreshToken); + const fallbackUserId = + fallbackDecoded && typeof fallbackDecoded === "object" + ? (fallbackDecoded as { userId?: unknown }).userId + : undefined; - const fallbackUserId = fallbackPayload?.userId; - - if (fallbackUserId) { + if (typeof fallbackUserId === "string") { const fallbackUser = await this.usersService .findByIdInternal(fallbackUserId) .catch(() => null); @@ -265,9 +273,12 @@ export class AuthTokenService { const storedToken = await this.redis.get(`${this.REFRESH_TOKEN_PREFIX}${refreshTokenHash}`); if (storedToken) { - const tokenData = JSON.parse(storedToken); + const tokenRecord = this.parseRefreshTokenRecord(storedToken); await this.redis.del(`${this.REFRESH_TOKEN_PREFIX}${refreshTokenHash}`); - await this.redis.del(`${this.REFRESH_TOKEN_FAMILY_PREFIX}${tokenData.familyId}`); + + if (tokenRecord) { + await this.redis.del(`${this.REFRESH_TOKEN_FAMILY_PREFIX}${tokenRecord.familyId}`); + } this.logger.debug("Revoked refresh token", { tokenHash: refreshTokenHash.slice(0, 8) }); } @@ -289,8 +300,8 @@ export class AuthTokenService { for (const key of keys) { const data = await this.redis.get(key); if (data) { - const family = JSON.parse(data); - if (family.userId === userId) { + const family = this.parseRefreshTokenFamilyRecord(data); + if (family && family.userId === userId) { await this.redis.del(key); await this.redis.del(`${this.REFRESH_TOKEN_PREFIX}${family.tokenHash}`); } @@ -309,15 +320,17 @@ export class AuthTokenService { try { const familyData = await this.redis.get(`${this.REFRESH_TOKEN_FAMILY_PREFIX}${familyId}`); if (familyData) { - const family = JSON.parse(familyData); - + const family = this.parseRefreshTokenFamilyRecord(familyData); await this.redis.del(`${this.REFRESH_TOKEN_FAMILY_PREFIX}${familyId}`); - await this.redis.del(`${this.REFRESH_TOKEN_PREFIX}${family.tokenHash}`); - this.logger.warn("Invalidated token family due to security concern", { - familyId: familyId.slice(0, 8), - userId: family.userId, - }); + if (family) { + await this.redis.del(`${this.REFRESH_TOKEN_PREFIX}${family.tokenHash}`); + + this.logger.warn("Invalidated token family due to security concern", { + familyId: familyId.slice(0, 8), + userId: family.userId, + }); + } } } catch (error) { this.logger.error("Failed to invalidate token family", { @@ -334,6 +347,55 @@ export class AuthTokenService { return createHash("sha256").update(token).digest("hex"); } + private parseRefreshTokenRecord(value: string): StoredRefreshToken | null { + try { + const parsed = JSON.parse(value) as Partial; + if ( + parsed && + typeof parsed === "object" && + typeof parsed.familyId === "string" && + typeof parsed.userId === "string" && + typeof parsed.valid === "boolean" + ) { + return { + familyId: parsed.familyId, + userId: parsed.userId, + valid: parsed.valid, + }; + } + } catch (error) { + this.logger.warn("Failed to parse refresh token record", { + error: error instanceof Error ? error.message : String(error), + }); + } + return null; + } + + private parseRefreshTokenFamilyRecord(value: string): StoredRefreshTokenFamily | null { + try { + const parsed = JSON.parse(value) as Partial; + if ( + parsed && + typeof parsed === "object" && + typeof parsed.userId === "string" && + typeof parsed.tokenHash === "string" + ) { + return { + userId: parsed.userId, + tokenHash: parsed.tokenHash, + deviceId: typeof parsed.deviceId === "string" ? parsed.deviceId : undefined, + userAgent: typeof parsed.userAgent === "string" ? parsed.userAgent : undefined, + createdAt: typeof parsed.createdAt === "string" ? parsed.createdAt : undefined, + }; + } + } catch (error) { + this.logger.warn("Failed to parse refresh token family record", { + error: error instanceof Error ? error.message : String(error), + }); + } + return null; + } + private parseExpiryToMs(expiry: string): number { const unit = expiry.slice(-1); const value = parseInt(expiry.slice(0, -1)); diff --git a/apps/bff/src/modules/catalog/services/base-catalog.service.ts b/apps/bff/src/modules/catalog/services/base-catalog.service.ts index 16f20680..ecf9c36a 100644 --- a/apps/bff/src/modules/catalog/services/base-catalog.service.ts +++ b/apps/bff/src/modules/catalog/services/base-catalog.service.ts @@ -2,7 +2,11 @@ import { Injectable, Inject } from "@nestjs/common"; import { Logger } from "nestjs-pino"; import { SalesforceConnection } from "@bff/integrations/salesforce/services/salesforce-connection.service"; import { getSalesforceFieldMap } from "@bff/core/config/field-map"; -import { assertSalesforceId, sanitizeSoqlLiteral } from "@bff/integrations/salesforce/utils/soql.util"; +import { + assertSalesforceId, + sanitizeSoqlLiteral, +} from "@bff/integrations/salesforce/utils/soql.util"; +import { getErrorMessage } from "@bff/core/utils/error.util"; import type { SalesforceProduct2WithPricebookEntries, SalesforceQueryResult, @@ -31,8 +35,12 @@ export class BaseCatalogService { try { const res = (await this.sf.query(soql)) as SalesforceQueryResult; return res.records ?? []; - } catch (error) { - this.logger.error({ error, soql, context }, `Query failed: ${context}`); + } catch (error: unknown) { + this.logger.error(`Query failed: ${context}`, { + error: getErrorMessage(error), + soql, + context, + }); return []; } } diff --git a/apps/bff/src/modules/catalog/services/sim-catalog.service.ts b/apps/bff/src/modules/catalog/services/sim-catalog.service.ts index 25b2f5f7..0386829e 100644 --- a/apps/bff/src/modules/catalog/services/sim-catalog.service.ts +++ b/apps/bff/src/modules/catalog/services/sim-catalog.service.ts @@ -50,7 +50,6 @@ export class SimCatalogService extends BaseCatalogService { } async getActivationFees(): Promise { - const fields = this.getFields(); const soql = this.buildProductQuery("SIM", "Activation", []); const records = await this.executeQuery( soql, diff --git a/apps/bff/src/modules/id-mappings/mappings.service.ts b/apps/bff/src/modules/id-mappings/mappings.service.ts index e33f992b..fdfcff8c 100644 --- a/apps/bff/src/modules/id-mappings/mappings.service.ts +++ b/apps/bff/src/modules/id-mappings/mappings.service.ts @@ -299,22 +299,24 @@ export class MappingsService { async getMappingStats(): Promise { try { - const [totalCount, whmcsCount, sfCount, completeCount] = await Promise.all([ - this.prisma.idMapping.count(), - this.prisma.idMapping.count({ where: { whmcsClientId: { not: null } } }), - this.prisma.idMapping.count({ where: { sfAccountId: { not: null } } }), - this.prisma.idMapping.count({ where: { whmcsClientId: { not: null }, sfAccountId: { not: null } } }), - ]); + const [totalCount, whmcsCount, sfCount, completeCount] = await Promise.all([ + this.prisma.idMapping.count(), + this.prisma.idMapping.count({ where: { whmcsClientId: { not: null } } }), + this.prisma.idMapping.count({ where: { sfAccountId: { not: null } } }), + this.prisma.idMapping.count({ + where: { whmcsClientId: { not: null }, sfAccountId: { not: null } }, + }), + ]); - const orphanedMappings = whmcsCount - completeCount; + const orphanedMappings = whmcsCount - completeCount; - const stats: MappingStats = { - totalMappings: totalCount, - whmcsMappings: whmcsCount, - salesforceMappings: sfCount, - completeMappings: completeCount, - orphanedMappings: orphanedMappings < 0 ? 0 : orphanedMappings, - }; + const stats: MappingStats = { + totalMappings: totalCount, + whmcsMappings: whmcsCount, + salesforceMappings: sfCount, + completeMappings: completeCount, + orphanedMappings: orphanedMappings < 0 ? 0 : orphanedMappings, + }; this.logger.debug("Generated mapping statistics", stats); return stats; } catch (error) { diff --git a/apps/bff/src/modules/orders/services/order-fulfillment-orchestrator.service.ts b/apps/bff/src/modules/orders/services/order-fulfillment-orchestrator.service.ts index c8bf3969..0353850b 100644 --- a/apps/bff/src/modules/orders/services/order-fulfillment-orchestrator.service.ts +++ b/apps/bff/src/modules/orders/services/order-fulfillment-orchestrator.service.ts @@ -28,7 +28,7 @@ export interface OrderFulfillmentStep { export interface OrderFulfillmentContext { sfOrderId: string; idempotencyKey: string; - validation: OrderFulfillmentValidationResult; + validation: OrderFulfillmentValidationResult | null; orderDetails?: OrderDetailsResponse; mappingResult?: OrderItemMappingResult; whmcsResult?: WhmcsOrderResult; @@ -63,8 +63,10 @@ export class OrderFulfillmentOrchestrator { const context: OrderFulfillmentContext = { sfOrderId, idempotencyKey, - validation: {} as OrderFulfillmentValidationResult, - steps: this.initializeSteps(payload.orderType as string), + validation: null, + steps: this.initializeSteps( + typeof payload.orderType === "string" ? payload.orderType : "Unknown" + ), }; this.logger.log("Starting fulfillment orchestration", { @@ -81,6 +83,10 @@ export class OrderFulfillmentOrchestrator { ); }); + if (!context.validation) { + throw new Error("Fulfillment validation did not complete successfully"); + } + // If already provisioned, return early if (context.validation.isAlreadyProvisioned) { this.markStepCompleted(context, "validation"); @@ -136,6 +142,10 @@ export class OrderFulfillmentOrchestrator { // Step 5: Create order in WHMCS await this.executeStep(context, "whmcs_create", async () => { + if (!context.validation) { + throw new Error("Validation context is missing"); + } + const orderNotes = this.orderWhmcsMapper.createOrderNotes( sfOrderId, `Provisioned from Salesforce Order ${sfOrderId}` @@ -157,7 +167,7 @@ export class OrderFulfillmentOrchestrator { context.whmcsResult = { orderId: createResult.orderId, serviceIds: [], - } as unknown as { orderId: number; serviceIds: number[] }; + }; }); // Step 6: Accept/provision order in WHMCS @@ -373,7 +383,7 @@ export class OrderFulfillmentOrchestrator { const isSuccess = context.steps.every((s: OrderFulfillmentStep) => s.status === "completed"); const failedStep = context.steps.find((s: OrderFulfillmentStep) => s.status === "failed"); - if (context.validation.isAlreadyProvisioned) { + if (context.validation?.isAlreadyProvisioned) { return { success: true, status: "Already Fulfilled", diff --git a/apps/bff/src/modules/orders/services/order-orchestrator.service.ts b/apps/bff/src/modules/orders/services/order-orchestrator.service.ts index fddc6b3e..a77b7941 100644 --- a/apps/bff/src/modules/orders/services/order-orchestrator.service.ts +++ b/apps/bff/src/modules/orders/services/order-orchestrator.service.ts @@ -20,6 +20,7 @@ import { getOrderItemProduct2Select, } from "@bff/core/config/field-map"; import { assertSalesforceId, buildInClause } from "@bff/integrations/salesforce/utils/soql.util"; +import { getErrorMessage } from "@bff/core/utils/error.util"; const fieldMap = getSalesforceFieldMap(); @@ -262,8 +263,11 @@ export class OrderOrchestrator { }, })), }); - } catch (error) { - this.logger.error({ error, orderId }, "Failed to fetch order with items"); + } catch (error: unknown) { + this.logger.error("Failed to fetch order with items", { + error: getErrorMessage(error), + orderId, + }); throw error; } } diff --git a/apps/bff/src/modules/subscriptions/subscriptions.service.ts b/apps/bff/src/modules/subscriptions/subscriptions.service.ts index fc51e218..543cc806 100644 --- a/apps/bff/src/modules/subscriptions/subscriptions.service.ts +++ b/apps/bff/src/modules/subscriptions/subscriptions.service.ts @@ -1,11 +1,6 @@ import { getErrorMessage } from "@bff/core/utils/error.util"; import { Injectable, NotFoundException, Inject } from "@nestjs/common"; -import { - Subscription, - SubscriptionList, - InvoiceList, - invoiceListSchema, -} from "@customer-portal/domain"; +import { Subscription, SubscriptionList, InvoiceList } from "@customer-portal/domain"; import type { Invoice, InvoiceItem } from "@customer-portal/domain"; import { WhmcsService } from "@bff/integrations/whmcs/whmcs.service"; import { MappingsService } from "@bff/modules/id-mappings/mappings.service"; @@ -15,6 +10,7 @@ import { subscriptionSchema, type SubscriptionSchema, } from "@customer-portal/domain/validation/shared/entities"; +import type { WhmcsProduct, WhmcsProductsResponse } from "@bff/integrations/whmcs/types/whmcs-api.types"; export interface GetSubscriptionsOptions { status?: string; @@ -515,20 +511,20 @@ export class SubscriptionsService { return false; } - const products = await this.whmcsService.getClientsProducts({ + const productsResponse: WhmcsProductsResponse = await this.whmcsService.getClientsProducts({ clientid: mapping.whmcsClientId, }); - const services = products?.products?.product ?? []; + const services = productsResponse.products?.product ?? []; - return services.some( - service => - typeof service.groupname === "string" && - service.groupname.toLowerCase().includes("sim") && - typeof service.status === "string" && - service.status.toLowerCase() === "active" - ); - } catch (error) { - this.logger.warn(`Failed to check existing SIM for user ${userId}`, error); + return services.some((service: WhmcsProduct) => { + const group = typeof service.groupname === "string" ? service.groupname.toLowerCase() : ""; + const status = typeof service.status === "string" ? service.status.toLowerCase() : ""; + return group.includes("sim") && status === "active"; + }); + } catch (error: unknown) { + this.logger.warn(`Failed to check existing SIM for user ${userId}`, { + error: getErrorMessage(error), + }); return false; } } diff --git a/apps/bff/src/modules/users/users.service.ts b/apps/bff/src/modules/users/users.service.ts index 26f8a151..aff82ea8 100644 --- a/apps/bff/src/modules/users/users.service.ts +++ b/apps/bff/src/modules/users/users.service.ts @@ -1,10 +1,6 @@ import { getErrorMessage } from "@bff/core/utils/error.util"; import { normalizeAndValidateEmail, validateUuidV4OrThrow } from "@bff/core/utils/validation.util"; -import { - mapPrismaUserToSharedUser, - mapPrismaUserToEnhancedBase, -} from "@bff/infra/utils/user-mapper.util"; -import type { UpdateAddressRequest, SalesforceContactRecord } from "@customer-portal/domain"; +import type { UpdateAddressRequest } from "@customer-portal/domain"; import { Injectable, Inject, NotFoundException, BadRequestException } from "@nestjs/common"; import { Logger } from "nestjs-pino"; import { PrismaService } from "@bff/infra/database/prisma.service";