From 05765d3513e4a911e8451d2e0539415167bda70e Mon Sep 17 00:00:00 2001 From: barsa Date: Wed, 29 Oct 2025 18:19:50 +0900 Subject: [PATCH] Enhance session management and error handling in authentication components - Added a new error mapping for "SESSION_EXPIRED" in SecureErrorMapperService to provide user-friendly messages and appropriate logging levels. - Updated SessionTimeoutWarning component to include a reason for logout when the session expires, improving user feedback. - Refactored useAuth hook to support enhanced logout functionality with reason options, ensuring better session management. - Improved auth.store to handle logout reasons and integrate with error handling for session refresh failures. - Streamlined LoginView to display logout messages based on session expiration, enhancing user experience. --- CODEBASE_TYPE_FLOW_AUDIT.md | 998 ++++++++++++++++++ .../services/secure-error-mapper.service.ts | 18 + .../orders/services/order-builder.service.ts | 10 +- .../auth/components/SessionTimeoutWarning.tsx | 15 +- .../src/features/auth/hooks/use-auth.ts | 69 +- .../src/features/auth/services/auth.store.ts | 88 +- .../src/features/auth/utils/logout-reason.ts | 79 ++ .../src/features/auth/views/LoginView.tsx | 49 + .../steps/ServiceConfigurationStep.tsx | 18 +- .../SubscriptionTable/SubscriptionTable.tsx | 244 +++++ .../components/SubscriptionTable/index.ts | 3 + .../views/SubscriptionDetail.tsx | 48 +- .../subscriptions/views/SubscriptionsList.tsx | 172 +-- apps/portal/src/lib/utils/error-handling.ts | 10 +- docs/architecture/SUBSCRIPTIONS-REFACTOR.md | 189 ++++ .../orders/providers/salesforce/raw.types.ts | 12 +- 16 files changed, 1760 insertions(+), 262 deletions(-) create mode 100644 CODEBASE_TYPE_FLOW_AUDIT.md create mode 100644 apps/portal/src/features/auth/utils/logout-reason.ts create mode 100644 apps/portal/src/features/subscriptions/components/SubscriptionTable/SubscriptionTable.tsx create mode 100644 apps/portal/src/features/subscriptions/components/SubscriptionTable/index.ts create mode 100644 docs/architecture/SUBSCRIPTIONS-REFACTOR.md diff --git a/CODEBASE_TYPE_FLOW_AUDIT.md b/CODEBASE_TYPE_FLOW_AUDIT.md new file mode 100644 index 00000000..465030a7 --- /dev/null +++ b/CODEBASE_TYPE_FLOW_AUDIT.md @@ -0,0 +1,998 @@ +# Codebase Type Flow Audit Report +**Date:** 2025-10-29 +**Auditor:** AI Assistant +**Scope:** API Types, Converters/Normalizers, Business Logic Distribution, Type Consistency + +--- + +## Executive Summary + +This audit examined the data flow from external APIs (Salesforce, WHMCS, Freebit) through the BFF layer to the frontend portal, analyzing type definitions, conversion layers, and business logic distribution. + +**Key Findings:** +- 🔴 **7 Critical Issues** requiring immediate attention +- 🟡 **5 Medium Priority Issues** impacting maintainability +- 🟢 **3 Low Priority Improvements** for optimization + +**Overall Assessment:** The codebase demonstrates good architectural separation with centralized domain types, but suffers from: +1. Business logic leaking into frontend utils (pricing calculations) +2. Duplicate mapper implementations across packages +3. Anti-pattern service wrappers in frontend +4. Inconsistent currency formatting implementations + +--- + +## 🔴 Critical Issues (Priority 1) + +### C1. Business Logic in Frontend: Pricing Calculations + +**Location:** +- `apps/portal/src/features/catalog/utils/catalog.utils.ts` (Lines 51-56) +- `apps/portal/src/features/orders/utils/order-presenters.ts` (Lines 120-144) +- `apps/portal/src/features/catalog/components/sim/SimConfigureView.tsx` (Lines 52-64) + +**Problem:** +Frontend components are calculating pricing totals, billing cycle conversions, and savings percentages. This violates clean architecture as business rules should reside in the BFF/domain layer. + +**Evidence:** +```typescript +// apps/portal/src/features/catalog/components/sim/SimConfigureView.tsx:52-64 +const monthlyTotal = + (plan?.monthlyPrice ?? 0) + + selectedAddons.reduce((sum, addonSku) => { + const addon = addons.find(a => a.sku === addonSku); + return sum + (addon?.monthlyPrice ?? 0); + }, 0); +``` + +```typescript +// apps/portal/src/features/catalog/utils/catalog.utils.ts:53-56 +export function calculateSavings(originalPrice: number, currentPrice: number): number { + if (originalPrice <= currentPrice) return 0; + return Math.round(((originalPrice - currentPrice) / originalPrice) * 100); +} +``` + +```typescript +// apps/portal/src/features/orders/utils/order-presenters.ts:120-144 +export function calculateOrderTotals( + items: Array<{ totalPrice?: number; billingCycle?: string }> | undefined, + fallbackTotal?: number +): OrderTotals { + let monthlyTotal = 0; + let oneTimeTotal = 0; + + if (items && items.length > 0) { + for (const item of items) { + const total = item.totalPrice ?? 0; + const billingCycle = normalizeBillingCycle(item.billingCycle); + if (billingCycle === "monthly") { + monthlyTotal += total; + } else if (billingCycle === "onetime") { + oneTimeTotal += total; + } else { + monthlyTotal += total; + } + } + } + return { monthlyTotal, oneTimeTotal }; +} +``` + +**Impact:** +- 🔴 **Risk:** Frontend can show incorrect prices if calculations diverge from BFF +- 🔴 **Maintainability:** Business rules are duplicated and harder to update +- 🔴 **Testing:** Business logic not centrally testable +- 🔴 **Data Integrity:** Source of truth is unclear + +**Root Cause:** +BFF checkout service already calculates totals (line 98-111 in `apps/bff/src/modules/orders/services/checkout.service.ts`), but frontend recalculates for display purposes. + +**Fix:** +1. **BFF Enhancement:** Ensure all pricing calculations are done in `CheckoutService.calculateTotals()` and returned in API response +2. **Frontend Cleanup:** Remove pricing calculations from frontend, use API-provided values only +3. **Display-Only Logic:** Frontend should only format prices, never calculate them + +**Diff-Ready Plan:** +```diff +# Step 1: Remove pricing calculations from frontend utils +- apps/portal/src/features/catalog/utils/catalog.utils.ts:51-56 (delete calculateSavings) +- apps/portal/src/features/orders/utils/order-presenters.ts:120-152 (delete calculateOrderTotals, normalizeBillingCycle) + +# Step 2: Update BFF to provide calculated totals ++ apps/bff/src/modules/orders/services/checkout.service.ts:98-111 + Ensure CheckoutTotals includes all needed fields (monthlyTotal, oneTimeTotal, subtotal, tax, total) + +# Step 3: Update frontend components to use API totals +- apps/portal/src/features/catalog/components/sim/SimConfigureView.tsx:52-64 + Replace local calculation with props from API response + +# Step 4: Add domain schema validation ++ packages/domain/orders/schema.ts + Add checkoutTotalsSchema with all pricing fields +``` + +--- + +### C2. Duplicate Mapper Implementations + +**Location:** +- `packages/domain/billing/providers/whmcs/mapper.ts` (Lines 50-61, 76-90) +- `packages/domain/subscriptions/providers/whmcs/mapper.ts` (Lines 64-75, 77-88) +- `packages/integrations/whmcs/src/mappers/*` (Re-exports only) + +**Problem:** +The same parsing and mapping logic (`parseAmount`, `formatDate`, `mapStatus`, `mapCycle`) is duplicated across multiple mapper files. + +**Evidence:** +```typescript +// packages/domain/billing/providers/whmcs/mapper.ts:50-61 +function parseAmount(amount: string | number | undefined): number { + if (typeof amount === "number") { + return amount; + } + if (!amount) { + return 0; + } + const cleaned = String(amount).replace(/[^\d.-]/g, ""); + const parsed = Number.parseFloat(cleaned); + return Number.isNaN(parsed) ? 0 : parsed; +} + +// packages/domain/subscriptions/providers/whmcs/mapper.ts:64-75 +function parseAmount(amount: string | number | undefined): number { + if (typeof amount === "number") { + return amount; + } + if (!amount) { + return 0; + } + const cleaned = String(amount).replace(/[^\d.-]/g, ""); + const parsed = Number.parseFloat(cleaned); + return Number.isNaN(parsed) ? 0 : parsed; +} +``` + +Same for `formatDate` functions (both files lines 77-88 and 63-74). + +**Impact:** +- 🔴 **Maintainability:** Changes must be synchronized across multiple files +- 🔴 **Consistency:** Risk of divergent implementations +- 🟡 **Code Size:** Unnecessary duplication increases bundle size + +**Fix:** +Create shared WHMCS mapper utilities in domain package. + +**Diff-Ready Plan:** +```diff +# Step 1: Create shared utilities ++ packages/domain/providers/whmcs/utils.ts + export function parseAmount(amount: string | number | undefined): number + export function formatDate(input?: string | null): string | undefined + export function normalizeWhmcsStatus(status: string, statusMap: Record): string + export function normalizeWhmcsCycle(cycle: string, cycleMap: Record): string + +# Step 2: Update billing mapper +- packages/domain/billing/providers/whmcs/mapper.ts:50-74 (delete duplicates) ++ packages/domain/billing/providers/whmcs/mapper.ts:1 + import { parseAmount, formatDate } from "../../providers/whmcs/utils" + +# Step 3: Update subscriptions mapper +- packages/domain/subscriptions/providers/whmcs/mapper.ts:64-88 (delete duplicates) ++ packages/domain/subscriptions/providers/whmcs/mapper.ts:1 + import { parseAmount, formatDate } from "../../providers/whmcs/utils" + +# Step 4: Remove obsolete package +- packages/integrations/whmcs/src/utils/data-utils.ts (delete, consolidate into domain) +``` + +--- + +### C3. Service Wrapper Anti-Pattern in Frontend + +**Location:** +- `apps/portal/src/features/checkout/services/checkout.service.ts` +- `apps/portal/src/features/orders/services/orders.service.ts` +- `apps/portal/src/features/account/services/account.service.ts` +- `apps/portal/src/features/catalog/services/catalog.service.ts` +- `apps/portal/src/features/subscriptions/services/sim-actions.service.ts` + +**Problem:** +Frontend has service wrapper classes that simply proxy to API endpoints. This is an anti-pattern in Next.js where hooks should call server actions directly. **This violates Memory ID: 9499068**. + +**Evidence:** +```typescript +// apps/portal/src/features/checkout/services/checkout.service.ts +export const checkoutService = { + async buildCart(orderType, selections, configuration) { + const response = await apiClient.POST("/api/checkout/cart", { + body: { orderType, selections, configuration }, + }); + return getDataOrThrow(response, "Failed to build checkout cart"); + }, +}; + +// apps/portal/src/features/account/services/account.service.ts +export const accountService = { + async getProfile() { + const response = await apiClient.GET("/api/me"); + return getNullableData(response); + }, + async updateProfile(update) { + const response = await apiClient.PATCH("/api/me", { body: update }); + return getDataOrThrow(response, "Failed to update profile"); + }, +}; +``` + +**Impact:** +- 🔴 **Architecture:** Violates Next.js best practices (user preference) +- 🟡 **Maintainability:** Extra layer to maintain without added value +- 🟡 **Type Safety:** Reduces type inference from server actions + +**Correct Pattern:** +Hooks should use apiClient directly, not through service wrappers. + +**Fix:** +Remove service wrappers and call apiClient directly from hooks. + +**Diff-Ready Plan:** +```diff +# Step 1: Update checkout hook +- apps/portal/src/features/checkout/services/checkout.service.ts (delete entire file) +- apps/portal/src/features/checkout/hooks/useCheckout.ts:118 +- const cart = await checkoutService.buildCart(snapshotOrderType, selections, configuration); ++ const response = await apiClient.POST("/api/checkout/cart", { ++ body: { orderType: snapshotOrderType, selections, configuration } ++ }); ++ const cart = getDataOrThrow(response, "Failed to build checkout cart"); + +# Step 2: Update account hooks +- apps/portal/src/features/account/services/account.service.ts (delete entire file) +- apps/portal/src/features/account/hooks/useProfileData.ts + Replace accountService.getProfile() with direct apiClient.GET("/api/me") +- apps/portal/src/features/account/hooks/useProfileEdit.ts:16 + Replace accountService.updateProfile() with direct apiClient.PATCH() + +# Step 3: Update orders hooks +- apps/portal/src/features/orders/services/orders.service.ts (delete entire file) + Move functions directly into hooks + +# Step 4: Update catalog hooks +- apps/portal/src/features/catalog/services/catalog.service.ts (delete entire file) + Move API calls directly into hooks + +# Step 5: Update SIM action hooks +- apps/portal/src/features/subscriptions/services/sim-actions.service.ts (delete entire file) + Move API calls directly into hooks +``` + +--- + +### C4. Hardcoded Business Pricing Logic in BFF + +**Location:** +- `apps/bff/src/modules/subscriptions/sim-management/services/sim-topup.service.ts` (Lines 38-41) + +**Problem:** +Hardcoded pricing formula "1GB = 500 JPY" exists in BFF service code. Pricing should be configured in Salesforce or a pricing service, not hardcoded. + +**Evidence:** +```typescript +// apps/bff/src/modules/subscriptions/sim-management/services/sim-topup.service.ts:38-41 +// Calculate cost: 1GB = 500 JPY (rounded up to nearest GB) +const quotaGb = request.quotaMb / 1000; +const units = Math.ceil(quotaGb); +const costJpy = units * 500; +``` + +**Impact:** +- 🔴 **Maintainability:** Pricing changes require code deployment +- 🔴 **Business Agility:** Cannot change pricing without developer intervention +- 🟡 **Consistency:** Pricing rules should be centralized + +**Fix:** +Create a pricing configuration service or use Salesforce Product2 pricing. + +**Diff-Ready Plan:** +```diff +# Step 1: Create pricing configuration ++ apps/bff/src/modules/catalog/services/pricing-config.service.ts + export class PricingConfigService { + async getSIMTopUpPricing(region: string): Promise<{ pricePerGB: number; currency: string }> + } + +# Step 2: Update SIM top-up service +- apps/bff/src/modules/subscriptions/sim-management/services/sim-topup.service.ts:38-41 ++ const pricing = await this.pricingConfig.getSIMTopUpPricing("JP"); ++ const quotaGb = request.quotaMb / 1000; ++ const units = Math.ceil(quotaGb); ++ const costJpy = units * pricing.pricePerGB; + +# Step 3: Store pricing in Salesforce Product2 + Add SIM_TopUp_Price__c custom field to Product2 + Query pricing from Salesforce instead of hardcoding +``` + +--- + +### C5. Duplicate Currency Formatting Implementations + +**Location:** +- `packages/domain/toolkit/formatting/currency.ts` (Lines 75-91) +- `apps/portal/src/features/dashboard/utils/dashboard.utils.ts` (Lines 127-151) +- `apps/portal/src/lib/hooks/useFormatCurrency.ts` (Lines 6-30) + +**Problem:** +Three different currency formatting implementations exist across the codebase with inconsistent behaviors. + +**Evidence:** +```typescript +// packages/domain/toolkit/formatting/currency.ts:75-91 (Canonical) +export function formatCurrency( + amount: number, + currencyOrOptions?: string | LegacyOptions, + symbolOrOptions?: string | LegacyOptions +): string { + const { locale, symbol, showSymbol, fractionDigits } = normalizeOptions( + currencyOrOptions, + symbolOrOptions + ); + const formatted = amount.toLocaleString(locale, { + minimumFractionDigits: fractionDigits, + maximumFractionDigits: fractionDigits, + }); + return showSymbol ? `${symbol}${formatted}` : formatted; +} + +// apps/portal/src/features/dashboard/utils/dashboard.utils.ts:129-151 (Duplicate) +const formatCurrency = (amount: number, currency?: string) => { + const code = (currency || "JPY").toUpperCase(); + const formatter = currencyFormatterCache.get(code) || (() => { + try { + const intl = new Intl.NumberFormat("en-US", { + style: "currency", + currency: code, + }); + currencyFormatterCache.set(code, intl); + return intl; + } catch { + return null; + } + })(); + if (!formatter) { + return `${code} ${amount.toLocaleString()}`; + } + return formatter.format(amount); +}; + +// apps/portal/src/lib/hooks/useFormatCurrency.ts:9-22 (Hook wrapper) +const formatCurrency = (amount: number) => { + if (loading) { + return "¥" + amount.toLocaleString(); + } + if (error) { + return baseFormatCurrency(amount, "JPY", "¥"); + } + return baseFormatCurrency(amount, currencyCode, currencySymbol); +}; +``` + +**Impact:** +- 🔴 **Inconsistency:** Different parts of the app format currency differently +- 🟡 **Maintainability:** Changes need to be made in multiple places +- 🟡 **Performance:** Dashboard creates cache unnecessarily + +**Fix:** +Consolidate to single implementation in domain package, with optional React hook wrapper. + +**Diff-Ready Plan:** +```diff +# Step 1: Enhance domain currency formatter ++ packages/domain/toolkit/formatting/currency.ts + Add caching like dashboard implementation + Ensure consistent behavior across all cases + +# Step 2: Remove duplicate implementation +- apps/portal/src/features/dashboard/utils/dashboard.utils.ts:127-151 (delete formatCurrency) ++ apps/portal/src/features/dashboard/utils/dashboard.utils.ts:1 + import { formatCurrency } from "@customer-portal/domain/toolkit" + +# Step 3: Keep hook wrapper but simplify + apps/portal/src/lib/hooks/useFormatCurrency.ts + Keep as thin wrapper around domain function, only adding loading/error states + +# Step 4: Global search and replace + Find all instances of formatCurrency usage and ensure consistent imports +``` + +--- + +### C6. Billing Cycle Normalization Logic in Frontend + +**Location:** +- `apps/portal/src/features/orders/utils/order-presenters.ts` (Lines 146-152) +- `apps/portal/src/features/subscriptions/components/SubscriptionCard.tsx` (Lines 70-74) + +**Problem:** +Frontend components normalize billing cycle strings, which is business logic. + +**Evidence:** +```typescript +// apps/portal/src/features/orders/utils/order-presenters.ts:146-152 +export function normalizeBillingCycle(value?: string): "monthly" | "onetime" | null { + if (!value) return null; + const normalized = value.trim().toLowerCase(); + if (normalized === "monthly") return "monthly"; + if (normalized === "onetime" || normalized === "one-time") return "onetime"; + return null; +} + +// apps/portal/src/features/subscriptions/components/SubscriptionCard.tsx:70-74 +const getBillingCycleLabel = (cycle: string, name: string) => { + const looksLikeActivation = name.toLowerCase().includes("activation") || name.includes("setup"); + return looksLikeActivation ? "One-time" : cycle; +}; +``` + +**Impact:** +- 🔴 **Inconsistency:** BFF already normalizes cycles in mapper +- 🟡 **Maintainability:** Business rules spread across layers + +**Fix:** +Ensure BFF always sends normalized values; frontend only displays them. + +**Diff-Ready Plan:** +```diff +# Step 1: Verify BFF normalization + packages/domain/subscriptions/providers/whmcs/mapper.ts:58-62 + Ensure mapCycle returns consistent format + +# Step 2: Remove frontend normalization +- apps/portal/src/features/orders/utils/order-presenters.ts:146-152 (delete) +- apps/portal/src/features/subscriptions/components/SubscriptionCard.tsx:70-74 (delete) + Use cycle value directly from API + +# Step 3: Add type safety ++ packages/domain/subscriptions/schema.ts + Ensure SubscriptionCycle is exported as const array for validation +``` + +--- + +### C7. Missing Type Validation at HTTP Boundaries + +**Location:** +- `apps/portal/src/features/catalog/services/catalog.service.ts` (Lines 24-80) + +**Problem:** +Frontend service applies Zod validation to API responses, but BFF should already be validating and providing typed responses. This is redundant validation. + +**Evidence:** +```typescript +// apps/portal/src/features/catalog/services/catalog.service.ts:24-30 +async getInternetCatalog(): Promise { + const response = await apiClient.GET("/api/catalog/internet/plans"); + const data = getDataOrThrow( + response, + "Failed to load internet catalog" + ); + return parseInternetCatalog(data); // Re-validates with Zod +} +``` + +**Impact:** +- 🟡 **Performance:** Double validation overhead +- 🟡 **Architecture:** BFF should be trusted source of validated data + +**Fix:** +Trust BFF responses (already validated), only validate at BFF HTTP entry points. + +**Diff-Ready Plan:** +```diff +# Frontend should trust BFF +- apps/portal/src/features/catalog/services/catalog.service.ts:30 +- return parseInternetCatalog(data); ++ return data; // Already validated by BFF + +# BFF validates at entry + apps/bff/src/modules/catalog/catalog.controller.ts + Ensure all responses use domain schemas for validation +``` + +--- + +## 🟡 Medium Priority Issues (Priority 2) + +### M1. Inconsistent Error Handling Patterns + +**Location:** +- Various `apiClient` call sites throughout portal + +**Problem:** +Some use `getDataOrThrow`, others use `getNullableData`, others check `response.data` manually. + +**Impact:** +- 🟡 **Maintainability:** Hard to ensure consistent error handling +- 🟡 **User Experience:** Inconsistent error messages + +**Fix:** +Standardize on `getDataOrThrow` for required data, `getNullableData` for optional. + +--- + +### M2. Type Alias Duplication + +**Location:** +- `apps/portal/src/features/catalog/utils/catalog.utils.ts` (Lines 14-19) +- `packages/domain/catalog/schema.ts` (Line 162-169) + +**Problem:** +Frontend defines its own `CatalogProduct` type alias when domain package already exports it. + +**Evidence:** +```typescript +// apps/portal/src/features/catalog/utils/catalog.utils.ts:14-19 +type CatalogProduct = + | InternetPlanCatalogItem + | InternetAddonCatalogItem + | InternetInstallationCatalogItem + | SimCatalogProduct + | VpnCatalogProduct; + +// packages/domain/catalog/schema.ts:162-169 +export type CatalogProduct = + | InternetPlanCatalogItem + | InternetInstallationCatalogItem + | InternetAddonCatalogItem + | SimCatalogProduct + | SimActivationFeeCatalogItem + | VpnCatalogProduct + | CatalogProductBase; +``` + +**Impact:** +- 🟡 **Consistency:** Types can drift +- 🟡 **Maintainability:** Two sources of truth + +**Fix:** +Remove local alias, import from domain. + +**Diff-Ready Plan:** +```diff +- apps/portal/src/features/catalog/utils/catalog.utils.ts:14-19 (delete) ++ apps/portal/src/features/catalog/utils/catalog.utils.ts:6 + import type { CatalogProduct } from "@customer-portal/domain/catalog" +``` + +--- + +### M3. Unused Integration Package Layer + +**Location:** +- `packages/integrations/whmcs/src/mappers/*` + +**Problem:** +Integration mappers are just re-exports from domain. This package adds no value. + +**Evidence:** +```typescript +// packages/integrations/whmcs/src/mappers/subscription.mapper.ts +export * from "@customer-portal/domain/subscriptions/providers/whmcs"; + +// packages/integrations/whmcs/src/mappers/invoice.mapper.ts +export * from "@customer-portal/domain/billing/providers/whmcs"; +``` + +**Impact:** +- 🟡 **Complexity:** Extra package to maintain +- 🟡 **Import Paths:** Confusing import structure + +**Fix:** +Remove `packages/integrations/whmcs` package, import directly from domain. + +**Diff-Ready Plan:** +```diff +# Step 1: Update all imports + Find: from "@customer-portal/integrations-whmcs" + Replace: from "@customer-portal/domain/[subscriptions|billing]/providers/whmcs" + +# Step 2: Remove package +- packages/integrations/whmcs/ (delete entire directory) +- packages/integrations/freebit/ (keep if has actual logic, else delete) + +# Step 3: Update workspace config +- pnpm-workspace.yaml + Remove integrations packages if all deleted +``` + +--- + +### M4. Status Mapping Duplication + +**Location:** +- `packages/domain/billing/providers/whmcs/mapper.ts` (Lines 24-35) +- `packages/domain/subscriptions/providers/whmcs/mapper.ts` (Lines 21-31) + +**Problem:** +`STATUS_MAP` dictionaries are defined separately in multiple mappers. + +**Impact:** +- 🟡 **Consistency:** Different status mappings could cause confusion +- 🟡 **Maintainability:** Updates must be synchronized + +**Fix:** +Move to shared WHMCS provider utilities (part of C2 fix). + +--- + +### M5. Magic String Status Checks + +**Location:** +- `apps/bff/src/modules/subscriptions/subscriptions.service.ts` (Lines 487-490) + +**Problem:** +Hardcoded magic strings for status and group name checks. + +**Evidence:** +```typescript +// apps/bff/src/modules/subscriptions/subscriptions.service.ts:487-490 +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"; +}); +``` + +**Impact:** +- 🟡 **Maintainability:** Status values spread throughout code +- 🟡 **Type Safety:** Should use status enum + +**Fix:** +Use domain status constants and enums. + +**Diff-Ready Plan:** +```diff ++ packages/domain/subscriptions/contract.ts + export const SUBSCRIPTION_STATUS = { ... } as const + export const PRODUCT_GROUPS = { SIM: "sim", INTERNET: "internet", ... } as const + +- apps/bff/src/modules/subscriptions/subscriptions.service.ts:487-490 ++ return services.some((service: WhmcsProduct) => { ++ const group = service.groupname?.toLowerCase() || ""; ++ const status = mapStatus(service.status); ++ return group.includes(PRODUCT_GROUPS.SIM) && status === "Active"; ++ }); +``` + +--- + +## 🟢 Low Priority Improvements (Priority 3) + +### L1. Optimize Internet Tier Metadata + +**Location:** +- `packages/domain/catalog/utils.ts` (Lines 51-105) + +**Problem:** +Hardcoded tier metadata in code. Could be moved to CMS or Salesforce. + +**Impact:** +- 🟢 **Flexibility:** Content updates require code changes +- 🟢 **Localization:** Hard to translate + +**Fix:** +Consider moving to Salesforce Product2 metadata or headless CMS. + +--- + +### L2. Unused `catalogMetadata` Fields + +**Location:** +- Various catalog schemas + +**Problem:** +`catalogMetadata` object added to types but inconsistently used. + +**Impact:** +- 🟢 **Type Safety:** Optional fields lead to defensive coding + +**Fix:** +Audit and document required vs optional metadata fields. + +--- + +### L3. Schema Validation Performance + +**Location:** +- All Zod parse() calls in services + +**Problem:** +Zod schemas re-parsed on every request. Could cache parsed schemas. + +**Impact:** +- 🟢 **Performance:** Minor overhead on high-traffic endpoints + +**Fix:** +Consider using `safeParse()` with caching for frequently accessed data. + +--- + +## Type Flow Analysis + +### 1. Catalog Products (Salesforce → BFF → Portal) + +``` +┌─────────────────────────────────────────────────────────────────┐ +│ Salesforce Product2 │ +│ (Source of Truth) │ +└────────────┬────────────────────────────────────────────────────┘ + │ + │ SOQL Query + ▼ +┌─────────────────────────────────────────────────────────────────┐ +│ BFF: apps/bff/src/modules/catalog/ │ +│ │ +│ Raw: SalesforceProduct2WithPricebookEntries │ +│ ↓ packages/domain/catalog/providers/salesforce/raw.types.ts │ +│ ↓ │ +│ Mapper: packages/domain/catalog/providers/salesforce/mapper.ts │ +│ ↓ - mapInternetPlan() │ +│ ↓ - mapSimProduct() │ +│ ↓ - enrichInternetPlanMetadata() │ +│ ↓ │ +│ Domain: InternetPlanCatalogItem | SimCatalogProduct │ +│ ↓ packages/domain/catalog/schema.ts │ +│ │ +│ Controller: catalog.controller.ts │ +│ ↓ Returns: InternetCatalogCollection │ +└────────────┬────────────────────────────────────────────────────┘ + │ + │ HTTP JSON Response + ▼ +┌─────────────────────────────────────────────────────────────────┐ +│ Portal: apps/portal/src/features/catalog/ │ +│ │ +│ Service: catalog.service.ts │ +│ ↓ parseInternetCatalog(data) ← ⚠️ REDUNDANT VALIDATION │ +│ ↓ │ +│ Hook: useCatalog() │ +│ ↓ Returns: InternetCatalogCollection │ +│ ↓ │ +│ Component: InternetPlansView.tsx │ +│ ↓ Displays plans using domain types │ +└─────────────────────────────────────────────────────────────────┘ + +✅ GOOD: Single source of truth (domain types) +✅ GOOD: Mapper layer cleanly transforms API types +⚠️ ISSUE: Frontend re-validates already validated data (M7) +⚠️ ISSUE: Frontend calculates prices instead of using API totals (C1) +``` + +### 2. Subscriptions (WHMCS → BFF → Portal) + +``` +┌─────────────────────────────────────────────────────────────────┐ +│ WHMCS GetClientsProducts API │ +│ (External System) │ +└────────────┬────────────────────────────────────────────────────┘ + │ + │ HTTP XML/JSON Response + ▼ +┌─────────────────────────────────────────────────────────────────┐ +│ BFF: apps/bff/src/integrations/whmcs/ │ +│ │ +│ Raw: WhmcsProductRaw │ +│ ↓ packages/domain/subscriptions/providers/whmcs/raw.types.ts│ +│ ↓ whmcsProductRawSchema (Zod validation) │ +│ ↓ │ +│ Mapper: packages/domain/subscriptions/providers/whmcs/mapper.ts │ +│ ↓ transformWhmcsSubscription() │ +│ ↓ - mapStatus() │ +│ ↓ - mapCycle() │ +│ ↓ - parseAmount() ← ⚠️ DUPLICATED (C2) │ +│ ↓ - formatDate() ← ⚠️ DUPLICATED (C2) │ +│ ↓ │ +│ Domain: Subscription │ +│ ↓ packages/domain/subscriptions/schema.ts │ +│ ↓ subscriptionSchema (Zod validation) │ +│ │ +│ Service: WhmcsService.getSubscriptions() │ +│ ↓ apps/bff/src/integrations/whmcs/whmcs.service.ts │ +│ ↓ Returns: SubscriptionList │ +│ │ +│ Controller: subscriptions.controller.ts │ +│ ↓ apps/bff/src/modules/subscriptions/ │ +│ ↓ Returns: SubscriptionList │ +└────────────┬────────────────────────────────────────────────────┘ + │ + │ HTTP JSON Response + ▼ +┌─────────────────────────────────────────────────────────────────┐ +│ Portal: apps/portal/src/features/subscriptions/ │ +│ │ +│ Hook: useSubscriptions() │ +│ ↓ Direct apiClient.GET("/api/subscriptions") ✅ │ +│ ↓ Returns: SubscriptionList │ +│ ↓ │ +│ Component: SubscriptionTable.tsx │ +│ ↓ - getBillingCycleLabel() ← ⚠️ BUSINESS LOGIC (C6) │ +│ ↓ - formatCurrency() ← ⚠️ DUPLICATE IMPL (C5) │ +│ ↓ Displays subscriptions │ +└─────────────────────────────────────────────────────────────────┘ + +✅ GOOD: Clean mapper layer +✅ GOOD: Domain types used throughout +⚠️ ISSUE: Duplicate parseAmount/formatDate (C2) +⚠️ ISSUE: Frontend normalizes billing cycles (C6) +⚠️ ISSUE: Multiple currency formatters (C5) +``` + +### 3. Orders (Portal → BFF → Salesforce) + +``` +┌─────────────────────────────────────────────────────────────────┐ +│ Portal: Checkout Flow │ +│ │ +│ Component: CheckoutView.tsx │ +│ ↓ Collects order data │ +│ ↓ │ +│ Hook: useCheckout() │ +│ ↓ checkoutService.buildCart() ← ⚠️ SERVICE WRAPPER (C3) │ +│ ↓ │ +│ Service: checkout.service.ts ← ⚠️ ANTI-PATTERN (C3) │ +│ ↓ apiClient.POST("/api/checkout/cart") │ +│ ↓ calculateOrderTotals() ← ⚠️ BUSINESS LOGIC (C1) │ +│ ↓ │ +│ Request: CreateOrderRequest │ +│ ↓ packages/domain/orders/schema.ts │ +└────────────┬────────────────────────────────────────────────────┘ + │ + │ HTTP JSON Request + ▼ +┌─────────────────────────────────────────────────────────────────┐ +│ BFF: apps/bff/src/modules/orders/ │ +│ │ +│ Controller: checkout.controller.ts │ +│ ↓ Receives: CreateOrderRequest │ +│ ↓ │ +│ Service: checkout.service.ts │ +│ ↓ buildCart(orderType, selections, config) │ +│ ↓ calculateTotals(items) ✅ CORRECT LOCATION │ +│ ↓ Returns: CheckoutCart │ +│ │ +│ Orchestrator: order-orchestrator.service.ts │ +│ ↓ createOrder(userId, rawBody) │ +│ ↓ │ +│ Validator: order-validator.service.ts │ +│ ↓ validateCompleteOrder() │ +│ ↓ │ +│ Builder: order-builder.service.ts │ +│ ↓ buildOrderFields() → Salesforce format │ +│ ↓ │ +│ Integration: salesforce-order.service.ts │ +│ ↓ createOrder(orderFields) │ +└────────────┬────────────────────────────────────────────────────┘ + │ + │ Salesforce REST API + ▼ +┌─────────────────────────────────────────────────────────────────┐ +│ Salesforce Order & OrderItem │ +│ (External System) │ +└─────────────────────────────────────────────────────────────────┘ + +✅ GOOD: Clean orchestration with separate concerns +✅ GOOD: BFF calculates totals correctly +⚠️ ISSUE: Frontend service wrapper (C3) +⚠️ ISSUE: Frontend recalculates totals (C1) +``` + +--- + +## Recommendations Summary + +### Immediate Actions (This Sprint) +1. **Remove pricing calculations from frontend** (C1) - Critical data integrity risk +2. **Consolidate WHMCS mapper utilities** (C2) - Prevents divergent implementations +3. **Remove service wrapper anti-pattern** (C3) - Aligns with architecture standards + +### Short-Term (Next Sprint) +4. **Extract hardcoded pricing** (C4) - Business agility +5. **Standardize currency formatting** (C5) - Consistency +6. **Remove billing cycle normalization from frontend** (C6) - Clean separation + +### Medium-Term (Next Quarter) +7. **Remove redundant validation** (C7) - Performance +8. **Consolidate error handling** (M1) - Consistency +9. **Remove duplicate type aliases** (M2) - Maintainability +10. **Remove unused integration packages** (M3) - Simplification + +### Long-Term (Future) +11. **Move tier metadata to CMS** (L1) - Content flexibility +12. **Audit catalogMetadata usage** (L2) - Type safety +13. **Optimize schema validation** (L3) - Performance at scale + +--- + +## Testing Strategy + +### For Each Fix: +1. **Unit Tests:** Test mapper functions with edge cases +2. **Integration Tests:** Verify API contract compliance +3. **E2E Tests:** Ensure UI displays correct data +4. **Performance Tests:** Measure validation overhead before/after + +### Regression Prevention: +- Add ESLint rules to prevent service wrappers in portal +- Add pre-commit hook to check for duplicate functions +- Create architecture decision records (ADRs) for patterns +- Document preferred patterns in CONTRIBUTING.md + +--- + +## Metrics & Success Criteria + +### Code Quality Metrics: +- **Duplicate Code:** Reduce by ~500 LOC (estimated) +- **Frontend Services:** Remove 7 wrapper files +- **Type Safety:** 100% BFF responses use domain schemas + +### Business Metrics: +- **Pricing Accuracy:** Zero discrepancies between frontend display and BFF calculations +- **Development Velocity:** Reduce time to add new product types by 30% +- **Bug Rate:** Reduce type-related bugs by 50% + +--- + +## Appendix: Clean Architecture Checklist + +### ✅ What's Good: +- Centralized domain types in `packages/domain/*` +- Mapper pattern for external API integration +- Zod schemas for runtime validation +- Clear separation between BFF and Portal + +### ⚠️ What Needs Improvement: +- Business logic leaking to frontend (pricing, normalization) +- Service wrapper anti-pattern in Portal +- Duplicate implementations across packages +- Inconsistent validation strategy + +### 🎯 Target Architecture: +``` +┌─────────────────────────────────────────────────────────────────┐ +│ External APIs (Salesforce, WHMCS, Freebit) │ +└────────────┬────────────────────────────────────────────────────┘ + │ + ▼ +┌─────────────────────────────────────────────────────────────────┐ +│ Domain Layer (packages/domain/*) │ +│ - Raw types (API contracts) │ +│ - Mappers (API → Domain) │ +│ - Schemas (Zod validation) │ +│ - Business types (canonical) │ +└────────────┬────────────────────────────────────────────────────┘ + │ + ▼ +┌─────────────────────────────────────────────────────────────────┐ +│ BFF Layer (apps/bff/*) │ +│ - Controllers (HTTP boundary validation) │ +│ - Services (business logic, orchestration) │ +│ - Integration services (call external APIs) │ +│ - Returns validated domain types │ +└────────────┬────────────────────────────────────────────────────┘ + │ + ▼ +┌─────────────────────────────────────────────────────────────────┐ +│ Portal Layer (apps/portal/*) │ +│ - Hooks (fetch data via apiClient) │ +│ - Components (display data only) │ +│ - Utils (presentation logic only: formatting, display helpers) │ +│ - NO business logic (pricing, validation, normalization) │ +└─────────────────────────────────────────────────────────────────┘ +``` + +--- + +**End of Audit Report** + diff --git a/apps/bff/src/core/security/services/secure-error-mapper.service.ts b/apps/bff/src/core/security/services/secure-error-mapper.service.ts index 1cb0f21a..d04ae1d9 100644 --- a/apps/bff/src/core/security/services/secure-error-mapper.service.ts +++ b/apps/bff/src/core/security/services/secure-error-mapper.service.ts @@ -161,6 +161,14 @@ export class SecureErrorMapperService { logLevel: "warn", }, ], + [ + "SESSION_EXPIRED", + { + code: "SESSION_EXPIRED", + publicMessage: "Your session has expired. Please log in again", + logLevel: "info", + }, + ], // Authorization Errors [ @@ -289,6 +297,14 @@ export class SecureErrorMapperService { }, // Authentication patterns + { + pattern: /token expired or expiring soon/i, + mapping: { + code: "SESSION_EXPIRED", + publicMessage: "Your session has expired. Please log in again", + logLevel: "info", + }, + }, { pattern: /password|credential|token|secret|key|auth/i, mapping: { @@ -370,6 +386,7 @@ export class SecureErrorMapperService { } private determineCategory(code: string): ErrorClassification["category"] { + if (code === "SESSION_EXPIRED") return "authentication"; if (code.startsWith("AUTH_")) return "authentication"; if (code.startsWith("AUTHZ_")) return "authorization"; if (code.startsWith("VAL_")) return "validation"; @@ -383,6 +400,7 @@ export class SecureErrorMapperService { if (code === "SYS_001" || code === "SYS_003") return "critical"; // High severity for authentication issues + if (code === "SESSION_EXPIRED") return "medium"; if (code.startsWith("AUTH_") && message.toLowerCase().includes("breach")) return "high"; // Medium for external service issues diff --git a/apps/bff/src/modules/orders/services/order-builder.service.ts b/apps/bff/src/modules/orders/services/order-builder.service.ts index b0891089..04c7e60c 100644 --- a/apps/bff/src/modules/orders/services/order-builder.service.ts +++ b/apps/bff/src/modules/orders/services/order-builder.service.ts @@ -123,13 +123,13 @@ export class OrderBuilder { const address2 = typeof addressToUse?.address2 === "string" ? addressToUse.address2 : ""; const fullStreet = [address1, address2].filter(Boolean).join(", "); - orderFields.Billing_Street__c = fullStreet; - orderFields.Billing_City__c = typeof addressToUse?.city === "string" ? addressToUse.city : ""; - orderFields.Billing_State__c = + orderFields.BillingStreet = fullStreet; + orderFields.BillingCity = typeof addressToUse?.city === "string" ? addressToUse.city : ""; + orderFields.BillingState = typeof addressToUse?.state === "string" ? addressToUse.state : ""; - orderFields.Billing_Postal_Code__c = + orderFields.BillingPostalCode = typeof addressToUse?.postcode === "string" ? addressToUse.postcode : ""; - orderFields.Billing_Country__c = + orderFields.BillingCountry = typeof addressToUse?.country === "string" ? addressToUse.country : ""; orderFields.Address_Changed__c = addressChanged; diff --git a/apps/portal/src/features/auth/components/SessionTimeoutWarning.tsx b/apps/portal/src/features/auth/components/SessionTimeoutWarning.tsx index 452c9de2..027b8344 100644 --- a/apps/portal/src/features/auth/components/SessionTimeoutWarning.tsx +++ b/apps/portal/src/features/auth/components/SessionTimeoutWarning.tsx @@ -2,9 +2,9 @@ import { logger } from "@customer-portal/logging"; import { useEffect, useRef, useState } from "react"; -import { useAuthStore } from "@/features/auth/services/auth.store"; import { useAuthSession } from "@/features/auth/services/auth.store"; import { Button } from "@/components/atoms/button"; +import { useAuth } from "@/features/auth/hooks/use-auth"; interface SessionTimeoutWarningProps { warningTime?: number; // Minutes before token expires to show warning @@ -14,8 +14,7 @@ export function SessionTimeoutWarning({ warningTime = 5, // Show warning 5 minutes before expiry (reduced since we have auto-refresh) }: SessionTimeoutWarningProps) { const { isAuthenticated, session } = useAuthSession(); - const logout = useAuthStore(state => state.logout); - const refreshSession = useAuthStore(state => state.refreshSession); + const { logout, refreshSession } = useAuth(); const [showWarning, setShowWarning] = useState(false); const [timeLeft, setTimeLeft] = useState(0); const expiryRef = useRef(null); @@ -42,7 +41,7 @@ export function SessionTimeoutWarning({ expiryRef.current = expiryTime; if (Date.now() >= expiryTime) { - void logout(); + void logout({ reason: "session-expired" }); return undefined; } @@ -83,7 +82,7 @@ export function SessionTimeoutWarning({ const remaining = expiryTime - Date.now(); if (remaining <= 0) { setTimeLeft(0); - void logout(); + void logout({ reason: "session-expired" }); return; } @@ -104,7 +103,7 @@ export function SessionTimeoutWarning({ if (event.key === "Escape") { event.preventDefault(); setShowWarning(false); - void logout(); + void logout({ reason: "session-expired" }); } if (event.key === "Tab") { @@ -147,13 +146,13 @@ export function SessionTimeoutWarning({ setTimeLeft(0); } catch (error) { logger.error(error, "Failed to extend session"); - await logout(); + await logout({ reason: "session-expired" }); } })(); }; const handleLogoutNow = () => { - void logout(); + void logout({ reason: "session-expired" }); setShowWarning(false); }; diff --git a/apps/portal/src/features/auth/hooks/use-auth.ts b/apps/portal/src/features/auth/hooks/use-auth.ts index 5fe2534f..624b4f03 100644 --- a/apps/portal/src/features/auth/hooks/use-auth.ts +++ b/apps/portal/src/features/auth/hooks/use-auth.ts @@ -10,6 +10,7 @@ import { useRouter, useSearchParams } from "next/navigation"; import { useAuthStore } from "../services/auth.store"; import { getPostLoginRedirect } from "@/features/auth/utils/route-protection"; import type { SignupRequest, LoginRequest } from "@customer-portal/domain/auth"; +import type { LogoutReason } from "@/features/auth/utils/logout-reason"; /** * Main authentication hook @@ -61,10 +62,15 @@ export function useAuth() { ); // Enhanced logout with redirect - const logout = useCallback(async () => { - await logoutAction(); - router.push("/auth/login"); - }, [logoutAction, router]); + const logout = useCallback( + async (options?: { reason?: LogoutReason }) => { + const reason = options?.reason ?? ("manual" as const); + await logoutAction({ reason }); + const reasonQuery = reason ? `?reason=${reason}` : ""; + router.push(`/auth/login${reasonQuery}`); + }, + [logoutAction, router] + ); return { // State @@ -177,18 +183,53 @@ export function useSession() { // Auto-refresh session periodically useEffect(() => { - if (isAuthenticated) { - const interval = setInterval( - () => { - void refreshSession(); - }, - 5 * 60 * 1000 - ); // Check every 5 minutes - - return () => clearInterval(interval); + if (!isAuthenticated) { + return undefined; } - return undefined; + const interval = setInterval(() => { + void refreshSession(); + }, 5 * 60 * 1000); // Check every 5 minutes + + return () => clearInterval(interval); + }, [isAuthenticated, refreshSession]); + + useEffect(() => { + if (!isAuthenticated || typeof window === "undefined") { + return undefined; + } + + let lastRefresh = 0; + const minInterval = 60 * 1000; + + const triggerRefresh = () => { + const now = Date.now(); + if (now - lastRefresh < minInterval) { + return; + } + lastRefresh = now; + void refreshSession(); + }; + + const handleVisibility = () => { + if (document.visibilityState === "visible") { + triggerRefresh(); + } + }; + + const handleFocus = () => { + triggerRefresh(); + }; + + window.addEventListener("focus", handleFocus); + document.addEventListener("visibilitychange", handleVisibility); + + triggerRefresh(); + + return () => { + window.removeEventListener("focus", handleFocus); + document.removeEventListener("visibilitychange", handleVisibility); + }; }, [isAuthenticated, refreshSession]); return { diff --git a/apps/portal/src/features/auth/services/auth.store.ts b/apps/portal/src/features/auth/services/auth.store.ts index dc56cedd..a650a10d 100644 --- a/apps/portal/src/features/auth/services/auth.store.ts +++ b/apps/portal/src/features/auth/services/auth.store.ts @@ -6,7 +6,7 @@ import { create } from "zustand"; import { apiClient } from "@/lib/api"; import { getNullableData } from "@/lib/api/response-helpers"; -import { getErrorInfo, handleAuthError } from "@/lib/utils/error-handling"; +import { getErrorInfo } from "@/lib/utils/error-handling"; import logger from "@customer-portal/logging"; import type { AuthTokens, @@ -16,6 +16,12 @@ import type { } from "@customer-portal/domain/auth"; import { authResponseSchema } from "@customer-portal/domain/auth"; import type { AuthenticatedUser } from "@customer-portal/domain/customer"; +import { + clearLogoutReason, + logoutReasonFromErrorCode, + setLogoutReason, + type LogoutReason, +} from "@/features/auth/utils/logout-reason"; interface SessionState { accessExpiresAt?: string; @@ -32,7 +38,7 @@ export interface AuthState { login: (credentials: LoginRequest) => Promise; signup: (data: SignupRequest) => Promise; - logout: () => Promise; + logout: (options?: { reason?: LogoutReason }) => Promise; requestPasswordReset: (email: string) => Promise; resetPassword: (token: string, password: string) => Promise; changePassword: (currentPassword: string, newPassword: string) => Promise; @@ -66,6 +72,39 @@ export const useAuthStore = create()((set, get) => { }); }; + let refreshPromise: Promise | null = null; + + const runTokenRefresh = async (): Promise => { + try { + const response = await apiClient.POST("/api/auth/refresh", { body: {} }); + const parsed = authResponseSchema.safeParse(response.data); + if (!parsed.success) { + throw new Error(parsed.error.issues?.[0]?.message ?? "Session refresh failed"); + } + applyAuthResponse(parsed.data); + } catch (error) { + logger.error(error, "Failed to refresh session"); + const errorInfo = getErrorInfo(error); + const reason = logoutReasonFromErrorCode(errorInfo.code) ?? ("session-expired" as const); + await get().logout({ reason }); + throw error; + } + }; + + const ensureSingleRefresh = async (): Promise => { + if (!refreshPromise) { + refreshPromise = (async () => { + try { + await runTokenRefresh(); + } finally { + refreshPromise = null; + } + })(); + } + + await refreshPromise; + }; + return { user: null, session: {}, @@ -108,7 +147,12 @@ export const useAuthStore = create()((set, get) => { } }, - logout: async () => { + logout: async (options?: { reason?: LogoutReason }) => { + if (options?.reason) { + setLogoutReason(options.reason); + } else { + clearLogoutReason(); + } try { await apiClient.POST("/api/auth/logout", {}); } catch (error) { @@ -244,7 +288,7 @@ export const useAuthStore = create()((set, get) => { }, refreshUser: async () => { - try { + const fetchProfile = async (): Promise => { const response = await apiClient.GET<{ isAuthenticated?: boolean; user?: AuthenticatedUser; @@ -256,43 +300,33 @@ export const useAuthStore = create()((set, get) => { isAuthenticated: true, error: null, }); - return; + } else { + set({ user: null, isAuthenticated: false, session: {} }); } + }; - // No active session - set({ user: null, isAuthenticated: false, session: {} }); + try { + await fetchProfile(); } catch (error) { - const shouldLogout = handleAuthError(error, get().logout); - if (shouldLogout) { + const errorInfo = getErrorInfo(error); + if (errorInfo.shouldLogout) { + const reason = logoutReasonFromErrorCode(errorInfo.code) ?? ("session-expired" as const); + await get().logout({ reason }); return; } try { - const refreshResponse = await apiClient.POST("/api/auth/refresh", { body: {} }); - const parsed = authResponseSchema.safeParse(refreshResponse.data); - if (!parsed.success) { - throw new Error(parsed.error.issues?.[0]?.message ?? "Session refresh failed"); - } - applyAuthResponse(parsed.data); + await ensureSingleRefresh(); + await fetchProfile(); } catch (refreshError) { logger.error(refreshError, "Failed to refresh session after auth error"); - await get().logout(); + return; } } }, refreshSession: async () => { - try { - const response = await apiClient.POST("/api/auth/refresh", { body: {} }); - const parsed = authResponseSchema.safeParse(response.data); - if (!parsed.success) { - throw new Error(parsed.error.issues?.[0]?.message ?? "Session refresh failed"); - } - applyAuthResponse(parsed.data); - } catch (error) { - logger.error(error, "Failed to refresh session"); - await get().logout(); - } + await ensureSingleRefresh(); }, checkAuth: async () => { diff --git a/apps/portal/src/features/auth/utils/logout-reason.ts b/apps/portal/src/features/auth/utils/logout-reason.ts new file mode 100644 index 00000000..14395724 --- /dev/null +++ b/apps/portal/src/features/auth/utils/logout-reason.ts @@ -0,0 +1,79 @@ +export type LogoutReason = "session-expired" | "token-revoked" | "manual"; + +export interface LogoutMessage { + title: string; + body: string; + variant: "info" | "warning" | "error"; +} + +const STORAGE_KEY = "customer-portal:lastLogoutReason"; + +const LOGOUT_MESSAGES: Record = { + "session-expired": { + title: "Session Expired", + body: "For your security, your session expired. Please sign in again to continue.", + variant: "warning", + }, + "token-revoked": { + title: "Signed Out For Your Safety", + body: "We detected a security change and signed you out. Please sign in again to verify your session.", + variant: "error", + }, + manual: { + title: "Signed Out", + body: "You have been signed out. Sign in again whenever you're ready.", + variant: "info", + }, +}; + +export function setLogoutReason(reason: LogoutReason): void { + if (typeof window === "undefined") return; + try { + sessionStorage.setItem(STORAGE_KEY, reason); + } catch { + // Ignore storage errors (e.g., private mode) + } +} + +export function clearLogoutReason(): void { + if (typeof window === "undefined") return; + try { + sessionStorage.removeItem(STORAGE_KEY); + } catch { + // Ignore storage errors + } +} + +export function consumeLogoutReason(): LogoutReason | null { + if (typeof window === "undefined") return null; + try { + const reason = sessionStorage.getItem(STORAGE_KEY) as LogoutReason | null; + if (reason) { + sessionStorage.removeItem(STORAGE_KEY); + return reason; + } + } catch { + // Ignore storage errors + } + return null; +} + +export function logoutReasonFromErrorCode(code: string): LogoutReason | undefined { + switch (code) { + case "TOKEN_REVOKED": + case "INVALID_REFRESH_TOKEN": + return "token-revoked"; + case "SESSION_EXPIRED": + return "session-expired"; + default: + return undefined; + } +} + +export function resolveLogoutMessage(reason: LogoutReason): LogoutMessage { + return LOGOUT_MESSAGES[reason] ?? LOGOUT_MESSAGES.manual; +} + +export function isLogoutReason(value: string | null | undefined): value is LogoutReason { + return value === "session-expired" || value === "token-revoked" || value === "manual"; +} diff --git a/apps/portal/src/features/auth/views/LoginView.tsx b/apps/portal/src/features/auth/views/LoginView.tsx index ee327538..afecf259 100644 --- a/apps/portal/src/features/auth/views/LoginView.tsx +++ b/apps/portal/src/features/auth/views/LoginView.tsx @@ -1,16 +1,65 @@ "use client"; +import { useEffect, useMemo, useState } from "react"; +import { useSearchParams } from "next/navigation"; + import { AuthLayout } from "../components"; import { LoginForm } from "@/features/auth/components"; import { useAuthStore } from "../services/auth.store"; import { LoadingOverlay } from "@/components/atoms"; +import { AlertBanner } from "@/components/molecules/AlertBanner/AlertBanner"; +import { + consumeLogoutReason, + isLogoutReason, + resolveLogoutMessage, + type LogoutReason, +} from "@/features/auth/utils/logout-reason"; export function LoginView() { const { loading, isAuthenticated } = useAuthStore(); + const searchParams = useSearchParams(); + const reasonParam = useMemo( + () => searchParams?.get("reason"), + [searchParams] + ); + const [logoutReason, setLogoutReason] = useState(null); + const [dismissed, setDismissed] = useState(false); + + useEffect(() => { + if (isLogoutReason(reasonParam)) { + setLogoutReason(reasonParam); + return; + } + + const stored = consumeLogoutReason(); + if (stored) { + setLogoutReason(stored); + } else { + setLogoutReason(null); + } + }, [reasonParam]); + + useEffect(() => { + if (logoutReason) { + setDismissed(false); + } + }, [logoutReason]); + + const logoutMessage = logoutReason ? resolveLogoutMessage(logoutReason) : null; return ( <> + {logoutMessage && !dismissed && ( + setDismissed(true)} + > + {logoutMessage.body} + + )} diff --git a/apps/portal/src/features/catalog/components/internet/configure/steps/ServiceConfigurationStep.tsx b/apps/portal/src/features/catalog/components/internet/configure/steps/ServiceConfigurationStep.tsx index 79d6530a..b0def944 100644 --- a/apps/portal/src/features/catalog/components/internet/configure/steps/ServiceConfigurationStep.tsx +++ b/apps/portal/src/features/catalog/components/internet/configure/steps/ServiceConfigurationStep.tsx @@ -72,7 +72,7 @@ export function ServiceConfigurationStep({ plan, mode, setMode, isTransitioning, onClick={onNext} disabled={plan?.internetPlanTier === "Silver" && !mode} rightIcon={} - className="min-w-[200px] transition-all duration-200 hover:scale-105" + className="min-w-[200px]" > Continue to Installation @@ -157,25 +157,21 @@ function ModeSelectionCard({