From 1cbc7f31f64da350d67f6fdfee564c90781b7c84 Mon Sep 17 00:00:00 2001 From: Mohammad Hosseini Date: Thu, 3 Sep 2026 22:46:18 +0330 Subject: [PATCH 1/5] Refactor `codes` and `utilities`: extract secure code generators - Add common/utils/codes.ts: generateSecureOtp (6-digit zero-padded crypto-random), generateUniqueCode, generateTripCode and generateTrackingCode (base32) - Keep legacy generateCode as a deprecated export (trip delivery code migrates in Phase 4) - utilities.ts re-exports all generators for backward compatibility --- src/common/utilities.ts | 15 +++--- src/common/utils/codes.spec.ts | 86 ++++++++++++++++++++++++++++++++++ src/common/utils/codes.ts | 63 +++++++++++++++++++++++++ 3 files changed, 156 insertions(+), 8 deletions(-) create mode 100644 src/common/utils/codes.spec.ts create mode 100644 src/common/utils/codes.ts diff --git a/src/common/utilities.ts b/src/common/utilities.ts index c138e45..1ade6b6 100644 --- a/src/common/utilities.ts +++ b/src/common/utilities.ts @@ -1,4 +1,3 @@ -import * as crypto from 'crypto'; import { Prisma } from '../../generated/prisma'; import { BadRequestException, @@ -16,13 +15,13 @@ import { } from 'class-validator'; import { differenceInCalendarMonths, isAfter, isValid } from 'date-fns'; -export const generateCode = () => { - return crypto.randomInt(11_111, 99_999); -}; - -export const generateUniqueCode = () => { - return Date.now().toString() + crypto.randomInt(1_111_111, 9_999_999); -}; +export { + generateCode, + generateSecureOtp, + generateUniqueCode, + generateTripCode, + generateTrackingCode, +} from './utils/codes'; export const formatPrismaError = (error: Error): never => { if (process.env.NODE_ENV === 'development') console.error(error); diff --git a/src/common/utils/codes.spec.ts b/src/common/utils/codes.spec.ts new file mode 100644 index 0000000..7daf024 --- /dev/null +++ b/src/common/utils/codes.spec.ts @@ -0,0 +1,86 @@ +import * as crypto from 'crypto'; +import { + generateCode, + generateSecureOtp, + generateUniqueCode, + generateTripCode, + generateTrackingCode, +} from './codes'; + +describe('codes', () => { + describe('generateSecureOtp', () => { + it('should return a zero-padded 6-digit string', () => { + const code = generateSecureOtp(); + expect(code).toMatch(/^\d{6}$/); + expect(code).toHaveLength(6); + }); + + it('should zero-pad values below 100000', () => { + const spy = jest.spyOn(crypto, 'randomInt') as unknown as jest.Mock; + spy.mockReturnValue(5); + expect(generateSecureOtp()).toBe('000005'); + spy.mockRestore(); + }); + + it('should cover the full 0–999999 space', () => { + const spy = jest.spyOn(crypto, 'randomInt') as unknown as jest.Mock; + spy.mockReturnValueOnce(999_999).mockReturnValueOnce(0); + expect(generateSecureOtp()).toBe('999999'); + expect(generateSecureOtp()).toBe('000000'); + spy.mockRestore(); + }); + + it('should generate unique codes across a large sample', () => { + let counter = 0; + const spy = jest.spyOn(crypto, 'randomInt') as unknown as jest.Mock; + spy.mockImplementation(() => counter++ % 1_000_000); + const sample = Array.from({ length: 10_000 }, () => generateSecureOtp()); + expect(new Set(sample).size).toBe(10_000); + spy.mockRestore(); + }); + }); + + describe('generateUniqueCode', () => { + it('should keep the legacy timestamp+random format', () => { + const code = generateUniqueCode(); + expect(typeof code).toBe('string'); + expect(code).toMatch(/^\d+$/); + expect(code.length).toBeGreaterThan(13); + }); + }); + + describe('generateTripCode / generateTrackingCode', () => { + it('should return 8-character base32 codes', () => { + const trip = generateTripCode(); + const tracking = generateTrackingCode(); + expect(trip).toMatch(/^[A-Z2-7]{8}$/); + expect(tracking).toMatch(/^[A-Z2-7]{8}$/); + }); + + it('should be unique across a large sample', () => { + const sample = Array.from({ length: 10_000 }, () => generateTripCode()); + expect(new Set(sample).size).toBe(10_000); + }); + + it('should not include excluded base32 characters (0,1,8,9)', () => { + const spy = jest.spyOn(crypto, 'randomInt') as unknown as jest.Mock; + spy + .mockReturnValueOnce(0) + .mockReturnValueOnce(1) + .mockReturnValueOnce(8) + .mockReturnValueOnce(9); + const trip = generateTripCode(); + expect(trip).toHaveLength(8); + expect(trip).toMatch(/^[A-Z2-7]{8}$/); + spy.mockRestore(); + }); + }); + + describe('generateCode (deprecated legacy)', () => { + it('should return a 5-digit number in the legacy range', () => { + const code = generateCode(); + expect(code).toBeGreaterThanOrEqual(11_111); + expect(code).toBeLessThan(99_999); + }); + }); +}); \ No newline at end of file diff --git a/src/common/utils/codes.ts b/src/common/utils/codes.ts new file mode 100644 index 0000000..5ab20ef --- /dev/null +++ b/src/common/utils/codes.ts @@ -0,0 +1,63 @@ +import * as crypto from 'crypto'; + +const BASE32_ALPHABET = 'ABCDEFGHIJKLMNOPQRSTUVWXYZ234567'; + +/** + * Legacy 5-digit OTP generator (biased range, not zero-padded). + * + * @deprecated Use generateSecureOtp() instead. Kept only because the trip + * delivery code (trip.service.ts) still relies on this range/format; + * migrated in Phase 4. + */ +export function generateCode(): number { + return crypto.randomInt(11_111, 99_999); +} + +/** + * Generate a cryptographically secure 6-digit OTP code, zero-padded. + * Uniform distribution over the full 000000–999999 space. + * + * BREAKING: previously this was a biased 5-digit number (11111–99998). + * Now returns a zero-padded 6-digit string. + */ +export function generateSecureOtp(): string { + return crypto.randomInt(0, 1_000_000).toString().padStart(6, '0'); +} + +/** + * Legacy unique code generator. Format: timestamp + random int. + * + * NOTE: uniqueness is NOT guaranteed — the timestamp prefix makes this + * deterministic within the same millisecond. For new code, prefer + * generateTripCode() / generateTrackingCode() with a DB unique column + * constraint and P2002 retry (Phase 4 TransactionRunner). + */ +export function generateUniqueCode(): string { + return Date.now().toString() + crypto.randomInt(1_111_111, 9_999_999); +} + +/** + * Generate a crypto-random 8-character base32 code. + * Uniqueness is enforced by the database unique column (P2002 retry via + * TransactionRunner in Phase 4) — the code itself carries no timestamp. + */ +export function generateTripCode(): string { + return generateBase32Code(8); +} + +/** + * Generate a crypto-random 8-character base32 code. + * Uniqueness is enforced by the database unique column (P2002 retry via + * TransactionRunner in Phase 4) — the code itself carries no timestamp. + */ +export function generateTrackingCode(): string { + return generateBase32Code(8); +} + +function generateBase32Code(length: number): string { + let result = ''; + for (let i = 0; i < length; i++) { + result += BASE32_ALPHABET[crypto.randomInt(0, BASE32_ALPHABET.length)]; + } + return result; +} From 31e667c0080d02203401dedabc04064bbd7a3450 Mon Sep 17 00:00:00 2001 From: Mohammad Hosseini Date: Thu, 3 Sep 2026 22:46:36 +0330 Subject: [PATCH 2/5] Refactor `TokenService`: public typed API with config-driven expiries - Make generateAccessToken/generateTempToken/generateProgressToken public with dedicated payload types (token.types.ts); keep verifyToken unchanged - Read JWT_*_EXPIRES_IN via ConfigService with defaults 20d/20m/1d (fixes C-2: no hardcoded expiries); register optional keys in config-names + env validation - Regroup private helpers at the end of the class --- src/common/config/config-names.ts | 3 + src/common/config/env.validation.ts | 4 + src/modules/token/token.service.spec.ts | 167 ++++++++++++++++++++++-- src/modules/token/token.service.ts | 50 ++++--- src/modules/token/token.types.ts | 13 ++ 5 files changed, 209 insertions(+), 28 deletions(-) create mode 100644 src/modules/token/token.types.ts diff --git a/src/common/config/config-names.ts b/src/common/config/config-names.ts index a57ddf2..ba49421 100644 --- a/src/common/config/config-names.ts +++ b/src/common/config/config-names.ts @@ -29,6 +29,9 @@ export const ConfigKey = { JwtAccessSecret: 'JWT_ACCESS_SECRET_KEY', JwtTempSecret: 'JWT_TEMP_SECRET_KEY', JwtProgressSecret: 'JWT_PROGRESS_SECRET_KEY', + JwtAccessExpiresIn: 'JWT_ACCESS_EXPIRES_IN', + JwtTempExpiresIn: 'JWT_TEMP_EXPIRES_IN', + JwtProgressExpiresIn: 'JWT_PROGRESS_EXPIRES_IN', }, Pricing: { BasePrice: 'PRICING_BASE_PRICE', diff --git a/src/common/config/env.validation.ts b/src/common/config/env.validation.ts index 1edf30f..387f55a 100644 --- a/src/common/config/env.validation.ts +++ b/src/common/config/env.validation.ts @@ -38,6 +38,10 @@ export class EnvSchema { @IsString() @MinLength(16) JWT_TEMP_SECRET_KEY!: string; @IsString() @MinLength(16) JWT_PROGRESS_SECRET_KEY!: string; + @IsOptional() @IsString() JWT_ACCESS_EXPIRES_IN?: string; + @IsOptional() @IsString() JWT_TEMP_EXPIRES_IN?: string; + @IsOptional() @IsString() JWT_PROGRESS_EXPIRES_IN?: string; + @IsString() AWS_ACCESS_KEY!: string; @IsString() AWS_SECRET_KEY!: string; @IsString() AWS_BUCKET_NAME!: string; diff --git a/src/modules/token/token.service.spec.ts b/src/modules/token/token.service.spec.ts index aca4f15..41e2075 100644 --- a/src/modules/token/token.service.spec.ts +++ b/src/modules/token/token.service.spec.ts @@ -2,22 +2,38 @@ import { Test, TestingModule } from '@nestjs/testing'; import { TokenService } from './token.service'; import { DeepMockProxy, mockDeep } from 'jest-mock-extended'; import { ConfigService } from '@nestjs/config'; +import * as jwt from 'jsonwebtoken'; +import { UnauthorizedException } from '@nestjs/common'; +import { AuthTokens } from '../../common/enums/auth.enum'; +import { AuthMessages } from '../../common/enums/messages.enum'; describe('TokenService', () => { let service: TokenService; let configService: DeepMockProxy; + const SECRETS: Record = { + JWT_ACCESS_SECRET_KEY: 'access-secret-abcdefgh-1234567890', + JWT_TEMP_SECRET_KEY: 'temp-secret-abcdefgh-1234567890', + JWT_PROGRESS_SECRET_KEY: 'progress-secret-abcdefgh-1234567890', + }; + + const EXPIRIES: Record = { + JWT_ACCESS_EXPIRES_IN: '20d', + JWT_TEMP_EXPIRES_IN: '20m', + JWT_PROGRESS_EXPIRES_IN: '1d', + }; + + const now = (): number => Math.floor(Date.now() / 1000); + beforeEach(async () => { configService = mockDeep(); - - configService.get.mockImplementation((key: string, defaultValue?: any) => { - const config = { - JWT_ACCESS_SECRET_KEY: 'key', - JWT_TEMP_SECRET_KEY: 'key', - JWT_PROGRESS_SECRET_KEY: 'key', - }; - return config[key] || defaultValue; - }); + configService.getOrThrow.mockImplementation( + (key: string) => SECRETS[key], + ); + configService.get.mockImplementation( + (key: string, defaultValue?: unknown) => + EXPIRIES[key] ?? defaultValue, + ); const module: TestingModule = await Test.createTestingModule({ providers: [ @@ -29,7 +45,136 @@ describe('TokenService', () => { service = module.get(TokenService); }); - it('should be defined', () => { - expect(service).toBeDefined(); + afterEach(() => { + jest.restoreAllMocks(); + jest.useRealTimers(); + }); + + describe('generateAccessToken', () => { + it('should produce an access JWT with the right payload and ~20d expiry', () => { + const token = service.generateAccessToken({ + sub: 'user-1', + phoneNumber: '+989123456789', + }); + + const decoded = jwt.verify( + token, + SECRETS.JWT_ACCESS_SECRET_KEY, + ) as jwt.JwtPayload; + expect(decoded.sub).toBe('user-1'); + expect(decoded.phoneNumber).toBe('+989123456789'); + expect(decoded.exp).toBeGreaterThan(now() + 20 * 24 * 3600 - 60); + expect(decoded.exp).toBeLessThanOrEqual(now() + 20 * 24 * 3600 + 60); + }); + }); + + describe('generateTempToken', () => { + it('should produce a temp JWT with ~20m expiry', () => { + const token = service.generateTempToken({ + phoneNumber: '+989123456789', + }); + + const decoded = jwt.verify( + token, + SECRETS.JWT_TEMP_SECRET_KEY, + ) as jwt.JwtPayload; + expect(decoded.phoneNumber).toBe('+989123456789'); + expect(decoded.sub).toBeUndefined(); + expect(decoded.exp).toBeGreaterThan(now() + 20 * 60 - 5); + expect(decoded.exp).toBeLessThanOrEqual(now() + 20 * 60 + 5); + }); + }); + + describe('generateProgressToken', () => { + it('should produce a progress JWT with ~1d expiry', () => { + const token = service.generateProgressToken({ + sub: 'user-2', + phoneNumber: '+989876543210', + }); + + const decoded = jwt.verify( + token, + SECRETS.JWT_PROGRESS_SECRET_KEY, + ) as jwt.JwtPayload; + expect(decoded.sub).toBe('user-2'); + expect(decoded.exp).toBeGreaterThan(now() + 24 * 3600 - 10); + expect(decoded.exp).toBeLessThanOrEqual(now() + 24 * 3600 + 10); + }); + }); + + describe('config-driven expiry', () => { + it('should honor a JWT_ACCESS_EXPIRES_IN env override', () => { + EXPIRIES.JWT_ACCESS_EXPIRES_IN = '5s'; + const token = service.generateAccessToken({ + sub: 'user-1', + phoneNumber: '+989123456789', + }); + + const decoded = jwt.verify( + token, + SECRETS.JWT_ACCESS_SECRET_KEY, + ) as jwt.JwtPayload; + expect(decoded.exp).toBeLessThanOrEqual(now() + 10); + }); + }); + + describe('verifyToken', () => { + it('should throw InvalidToken for empty input', () => { + expect(() => service.verifyToken('', AuthTokens.Access)).toThrow( + UnauthorizedException, + ); + expect(() => service.verifyToken('', AuthTokens.Access)).toThrow( + AuthMessages.InvalidToken, + ); + }); + + it('should throw InvalidToken for non-string input', () => { + expect(() => + service.verifyToken(undefined as any, AuthTokens.Access), + ).toThrow(UnauthorizedException); + }); + + it('should report expired tokens per type', () => { + jest.useFakeTimers(); + const token = service.generateAccessToken({ + sub: 'user-1', + phoneNumber: '+989123456789', + }); + + jest.advanceTimersByTime(20 * 24 * 3600 * 1000 + 1000); + + expect(() => service.verifyToken(token, AuthTokens.Access)).toThrow( + 'access token has expired', + ); + }); + + it('should report tampered tokens per type', () => { + const token = jwt.sign( + { sub: 'user-1', phoneNumber: '+989123456789' }, + 'completely-wrong-secret-1234567890', + { expiresIn: '1h' }, + ); + + expect(() => service.verifyToken(token, AuthTokens.Progress)).toThrow( + 'progress invalid token', + ); + }); + + it('should report unexpected errors per type', () => { + const verifySpy = jest + .spyOn(jwt, 'verify') + .mockImplementation(() => { + throw new Error('boom'); + }); + + const token = service.generateAccessToken({ + sub: 'user-1', + phoneNumber: '+989123456789', + }); + expect(() => service.verifyToken(token, AuthTokens.Temporary)).toThrow( + 'temp verification failed', + ); + verifySpy.mockRestore(); + }); }); }); diff --git a/src/modules/token/token.service.ts b/src/modules/token/token.service.ts index 9be6ad1..f408fd4 100644 --- a/src/modules/token/token.service.ts +++ b/src/modules/token/token.service.ts @@ -3,14 +3,21 @@ import { ConfigService } from '@nestjs/config'; import * as jwt from 'jsonwebtoken'; import { AuthTokens } from '../../common/enums/auth.enum'; import { AuthMessages } from '../../common/enums/messages.enum'; +import { + AccessTokenPayload, + ProgressTokenPayload, + TemporaryTokenPayload, +} from './token.types'; @Injectable() export class TokenService { private accessSecretKey: string; private tempSecretKey: string; private progressSecretKey: string; + private config: ConfigService; constructor(config: ConfigService) { + this.config = config; this.accessSecretKey = config.getOrThrow('JWT_ACCESS_SECRET_KEY'); this.tempSecretKey = config.getOrThrow('JWT_TEMP_SECRET_KEY'); this.progressSecretKey = config.getOrThrow( @@ -18,27 +25,28 @@ export class TokenService { ); } - private generateToken( - payload: jwt.JwtPayload, - token: string, - { expiresIn }: jwt.SignOptions, - ) { - return jwt.sign(payload, token, { expiresIn }); - } - - private generateAccessToken(payload: jwt.JwtPayload) { - const accessSecretKey = this.accessSecretKey; - return this.generateToken(payload, accessSecretKey, { expiresIn: '20d' }); + generateAccessToken(payload: AccessTokenPayload): string { + const expiresIn = this.config.get( + 'JWT_ACCESS_EXPIRES_IN', + '20d', + ); + return this.generateToken(payload, this.accessSecretKey, { expiresIn }); } - private generateTempToken(payload: jwt.JwtPayload) { - const tempSecretKey = this.tempSecretKey; - return this.generateToken(payload, tempSecretKey, { expiresIn: '20m' }); + generateTempToken(payload: TemporaryTokenPayload): string { + const expiresIn = this.config.get( + 'JWT_TEMP_EXPIRES_IN', + '20m', + ); + return this.generateToken(payload, this.tempSecretKey, { expiresIn }); } - private generateProgressToken(payload: jwt.JwtPayload) { - const authSecretKey = this.progressSecretKey; - return this.generateToken(payload, authSecretKey, { expiresIn: '1d' }); + generateProgressToken(payload: ProgressTokenPayload): string { + const expiresIn = this.config.get( + 'JWT_PROGRESS_EXPIRES_IN', + '1d', + ); + return this.generateToken(payload, this.progressSecretKey, { expiresIn }); } verifyToken(token: string, type: AuthTokens) { @@ -73,4 +81,12 @@ export class TokenService { throw new UnauthorizedException(`${type} verification failed`); } } + + private generateToken( + payload: jwt.JwtPayload, + secret: string, + { expiresIn }: jwt.SignOptions, + ) { + return jwt.sign(payload, secret, { expiresIn }); + } } diff --git a/src/modules/token/token.types.ts b/src/modules/token/token.types.ts new file mode 100644 index 0000000..eb0aec1 --- /dev/null +++ b/src/modules/token/token.types.ts @@ -0,0 +1,13 @@ +export interface AccessTokenPayload { + sub: string; + phoneNumber: string; +} + +export interface TemporaryTokenPayload { + phoneNumber: string; +} + +export interface ProgressTokenPayload { + sub: string; + phoneNumber: string; +} From b5286dbe384cbf1b59c104496f12338180046cb0 Mon Sep 17 00:00:00 2001 From: Mohammad Hosseini Date: Thu, 3 Sep 2026 22:46:51 +0330 Subject: [PATCH 3/5] Refactor `UserService`: inject S3StoragePort instead of S3Service - Define S3StoragePort (interim port, full StoragePort lands in Phase 5) and inject it via the S3_STORAGE_PORT symbol - Bind the port to S3Service with useExisting in user.module.ts - Keep presigned-URL building behavior identical --- src/modules/user/user.module.ts | 11 +++++++++-- src/modules/user/user.service.spec.ts | 19 +++++++++---------- src/modules/user/user.service.ts | 25 +++++++++++++++++-------- 3 files changed, 35 insertions(+), 20 deletions(-) diff --git a/src/modules/user/user.module.ts b/src/modules/user/user.module.ts index d589968..34ddf4d 100644 --- a/src/modules/user/user.module.ts +++ b/src/modules/user/user.module.ts @@ -1,14 +1,21 @@ import { Module } from '@nestjs/common'; import { UserController } from './user.controller'; -import { UserService } from './user.service'; +import { UserService, S3_STORAGE_PORT } from './user.service'; import { PrismaModule } from '../prisma/prisma.module'; import { TokenModule } from '../token/token.module'; import { S3Module } from '../s3/s3.module'; +import { S3Service } from '../s3/s3.service'; @Module({ imports: [PrismaModule, TokenModule, S3Module], controllers: [UserController], - providers: [UserService], + providers: [ + UserService, + { + provide: S3_STORAGE_PORT, + useExisting: S3Service, + }, + ], exports: [UserService], }) export class UserModule {} diff --git a/src/modules/user/user.service.spec.ts b/src/modules/user/user.service.spec.ts index 722371b..ae5faea 100644 --- a/src/modules/user/user.service.spec.ts +++ b/src/modules/user/user.service.spec.ts @@ -1,8 +1,7 @@ import { Test, TestingModule } from '@nestjs/testing'; import { DeepMockProxy, mockDeep } from 'jest-mock-extended'; -import { UserService } from './user.service'; +import { UserService, S3StoragePort, S3_STORAGE_PORT } from './user.service'; import { PrismaService } from '../prisma/prisma.service'; -import { S3Service } from '../s3/s3.service'; import { PrismaClient, RolesEnum, @@ -19,7 +18,7 @@ jest.mock('../../common/utilities', () => ({ describe('UserService', () => { let service: UserService; let prismaService: DeepMockProxy; - let s3Service: DeepMockProxy; + let storage: DeepMockProxy; const mockUser = { id: 'user-123', @@ -91,13 +90,13 @@ describe('UserService', () => { jest.resetAllMocks(); prismaService = mockDeep(); - s3Service = mockDeep(); + storage = mockDeep(); const module: TestingModule = await Test.createTestingModule({ providers: [ UserService, { provide: PrismaService, useValue: prismaService }, - { provide: S3Service, useValue: s3Service }, + { provide: S3_STORAGE_PORT, useValue: storage }, ], }).compile(); @@ -149,7 +148,7 @@ describe('UserService', () => { describe('getProfile', () => { it('should get user profile with transporter data successfully', async () => { prismaService.user.findUniqueOrThrow.mockResolvedValue(mockProfile); - s3Service.generateGetPresignedUrl.mockResolvedValue( + storage.generateGetPresignedUrl.mockResolvedValue( 'https://presigned-url.com', ); @@ -183,7 +182,7 @@ describe('UserService', () => { prismaService.user.findUniqueOrThrow.mockResolvedValue( userWithoutTransporter, ); - s3Service.generateGetPresignedUrl.mockResolvedValue( + storage.generateGetPresignedUrl.mockResolvedValue( 'https://presigned-url.com', ); @@ -198,7 +197,7 @@ describe('UserService', () => { it('should handle S3 URL generation errors gracefully', async () => { prismaService.user.findUniqueOrThrow.mockResolvedValue(mockProfile); - s3Service.generateGetPresignedUrl.mockResolvedValue( + storage.generateGetPresignedUrl.mockResolvedValue( 'https://presigned-url.com', ); @@ -446,9 +445,9 @@ describe('UserService', () => { }); describe('Error scenarios', () => { - it('should handle S3 service errors in getProfile', async () => { + it('should handle storage errors in getProfile', async () => { prismaService.user.findUniqueOrThrow.mockResolvedValue(mockProfile); - s3Service.generateGetPresignedUrl.mockResolvedValue(''); + storage.generateGetPresignedUrl.mockResolvedValue(''); const result = await service.getProfile('user-123'); expect(result).toBeDefined(); diff --git a/src/modules/user/user.service.ts b/src/modules/user/user.service.ts index 303460a..84e0f91 100644 --- a/src/modules/user/user.service.ts +++ b/src/modules/user/user.service.ts @@ -1,17 +1,26 @@ -import { Injectable } from '@nestjs/common'; +import { Inject, Injectable } from '@nestjs/common'; import { PrismaService } from '../prisma/prisma.service'; import { Prisma, User } from '../../../generated/prisma'; import { UpdateTransporterDto } from './dto/update-transporter.dto'; import { PrismaTransaction } from '../prisma/prisma.types'; -import { formatPrismaError } from '../../common/utilities'; import { UpdateUserDto } from './dto/update-user.dto'; -import { S3Service } from '../s3/s3.service'; +import { formatPrismaError } from '../../common/utilities'; + +export const S3_STORAGE_PORT = Symbol('S3_STORAGE_PORT'); + +/** + * Minimal interim storage port so UserService stays testable without a + * real S3 client. Phase 5 replaces this with the full StoragePort. + */ +export interface S3StoragePort { + generateGetPresignedUrl(key: string | undefined | null): Promise; +} @Injectable() export class UserService { constructor( - private prisma: PrismaService, - private s3Service: S3Service, + private readonly prisma: PrismaService, + @Inject(S3_STORAGE_PORT) private readonly storage: S3StoragePort, ) {} async get( @@ -50,13 +59,13 @@ export class UserService { ...profile, transporter: { ...profile.transporter, - profilePictureUrl: await this.s3Service.generateGetPresignedUrl( + profilePictureUrl: await this.storage.generateGetPresignedUrl( profile.transporter?.profilePictureKey, ), - licenseDocumentUrl: await this.s3Service.generateGetPresignedUrl( + licenseDocumentUrl: await this.storage.generateGetPresignedUrl( profile.transporter?.licenseDocumentKey, ), - nationalIdDocumentUrl: await this.s3Service.generateGetPresignedUrl( + nationalIdDocumentUrl: await this.storage.generateGetPresignedUrl( profile.transporter?.nationalIdDocumentKey, ), }, From f962548ba55f784f322641f643cedc98d6f4062c Mon Sep 17 00:00:00 2001 From: Mohammad Hosseini Date: Thu, 3 Sep 2026 22:47:21 +0330 Subject: [PATCH 4/5] Refactor `AuthService`: extract OTP, state machine and signup services - Extract OtpService (domain): owns send/verify, attempt tracking, blocking and the send window. Reads limits through an injected OTP_CONFIG value object so only the canonical MAX_SEND_ATTEMPTS key is used (fixes C-1), and accesses the OTP cache through the named OTP_CACHE provider (fixes C-3) - Extract AuthStateMachine (domain): pure transporter auth-state computation - Extract TransporterSignupService (application): sender + transporter stage-1 signup orchestration - Extract session-utils: setUserState/setSessionAuth/clearAuthData session helpers - Includes unit specs for each extracted unit (fake timers for OTP, mocked cache) --- .../transporter-signup.service.spec.ts | 161 +++++++++++ .../application/transporter-signup.service.ts | 104 +++++++ .../auth/domain/auth-state.machine.spec.ts | 102 +++++++ src/modules/auth/domain/auth-state.machine.ts | 68 +++++ src/modules/auth/domain/otp.service.spec.ts | 260 ++++++++++++++++++ src/modules/auth/domain/otp.service.ts | 173 ++++++++++++ src/modules/auth/domain/session-utils.spec.ts | 46 ++++ src/modules/auth/domain/session-utils.ts | 29 ++ 8 files changed, 943 insertions(+) create mode 100644 src/modules/auth/application/transporter-signup.service.spec.ts create mode 100644 src/modules/auth/application/transporter-signup.service.ts create mode 100644 src/modules/auth/domain/auth-state.machine.spec.ts create mode 100644 src/modules/auth/domain/auth-state.machine.ts create mode 100644 src/modules/auth/domain/otp.service.spec.ts create mode 100644 src/modules/auth/domain/otp.service.ts create mode 100644 src/modules/auth/domain/session-utils.spec.ts create mode 100644 src/modules/auth/domain/session-utils.ts diff --git a/src/modules/auth/application/transporter-signup.service.spec.ts b/src/modules/auth/application/transporter-signup.service.spec.ts new file mode 100644 index 0000000..9ab7c25 --- /dev/null +++ b/src/modules/auth/application/transporter-signup.service.spec.ts @@ -0,0 +1,161 @@ +import { Test, TestingModule } from '@nestjs/testing'; +import { DeepMockProxy, mockDeep } from 'jest-mock-extended'; +import { TransporterSignupService } from './transporter-signup.service'; +import { PrismaService } from '../../prisma/prisma.service'; +import { TokenService } from '../../token/token.service'; +import { NotificationService } from '../../notification/notification.service'; +import { PrismaClient, RolesEnum, GendersEnum, LicenseTypeEnum } from '../../../../generated/prisma'; +import { NotificationMessages } from '../../notification/notification-messages'; + +describe('TransporterSignupService', () => { + let service: TransporterSignupService; + let prismaService: DeepMockProxy; + let tokenService: DeepMockProxy; + let notificationService: DeepMockProxy; + + const senderDto = { + firstName: 'احمد', + lastName: 'محمدی', + phoneNumber: '+989123456789', + gender: GendersEnum.male, + }; + + const transporterDto = { + firstName: 'احمد', + lastName: 'محمدی', + phoneNumber: '+989123456789', + gender: GendersEnum.male, + birthDate: new Date('1990-01-01'), + nationalId: '1234567890', + licenseNumber: '0987654321', + licenseType: LicenseTypeEnum.grade_one, + licenseExpiryDate: new Date('2030-01-01'), + }; + + const mockSender = { + id: 'user-1', + phoneNumber: '+989123456789', + role: RolesEnum.sender, + }; + + const mockTransporterUser = { + id: 'user-2', + phoneNumber: '+989123456789', + role: RolesEnum.transporter, + transporter: { + id: 'trans-2', + nationalId: '1234567890', + }, + }; + + beforeEach(async () => { + jest.resetAllMocks(); + + prismaService = mockDeep(); + tokenService = mockDeep(); + notificationService = mockDeep(); + notificationService.create.mockResolvedValue({} as any); + + prismaService.$transaction.mockImplementation( + async (callback) => callback(prismaService), + ); + + const module: TestingModule = await Test.createTestingModule({ + providers: [ + TransporterSignupService, + { provide: PrismaService, useValue: prismaService }, + { provide: TokenService, useValue: tokenService }, + { provide: NotificationService, useValue: notificationService }, + ], + }).compile(); + + service = module.get(TransporterSignupService); + }); + + describe('signupSender', () => { + it('should create the sender with a wallet, issue an access token and welcome notification', async () => { + prismaService.user.create.mockResolvedValue(mockSender as any); + tokenService.generateAccessToken.mockReturnValue('access-token'); + + const result = await service.signupSender(senderDto); + + expect(prismaService.user.create).toHaveBeenCalledWith( + expect.objectContaining({ + data: expect.objectContaining({ + role: RolesEnum.sender, + wallet: { create: {} }, + phoneVerifiedAt: expect.any(Date), + }), + }), + ); + expect(tokenService.generateAccessToken).toHaveBeenCalledWith({ + sub: 'user-1', + phoneNumber: '+989123456789', + }); + expect(notificationService.create).toHaveBeenCalledWith( + 'user-1', + { content: NotificationMessages.Welcome }, + prismaService, + ); + expect(result).toEqual({ + sender: mockSender, + accessToken: 'access-token', + }); + }); + + it('should propagate constraint failures without wrapping', async () => { + const error = new Error('P2002 unique constraint'); + prismaService.user.create.mockRejectedValue(error); + + await expect(service.signupSender(senderDto)).rejects.toThrow( + 'P2002 unique constraint', + ); + }); + }); + + describe('signupTransporter', () => { + it('should create the transporter with statuses, issue a progress token and welcome notification', async () => { + prismaService.user.create.mockResolvedValue(mockTransporterUser as any); + tokenService.generateProgressToken.mockReturnValue('progress-token'); + + const result = await service.signupTransporter(transporterDto); + + expect(prismaService.user.create).toHaveBeenCalledWith( + expect.objectContaining({ + data: expect.objectContaining({ + role: RolesEnum.transporter, + transporter: { + create: expect.objectContaining({ + nationalId: '1234567890', + nationalIdStatus: { create: {} }, + licenseStatus: { create: {} }, + verificationStatus: { create: {} }, + }), + }, + }), + }), + ); + expect(tokenService.generateProgressToken).toHaveBeenCalledWith({ + sub: 'user-2', + phoneNumber: '+989123456789', + }); + expect(notificationService.create).toHaveBeenCalledWith( + 'user-2', + { content: NotificationMessages.Welcome }, + prismaService, + ); + expect(result.progressToken).toBe('progress-token'); + expect(result.transporter.id).toBe('trans-2'); + expect(result.transporter.nationalId).toBe('1234567890'); + }); + + it('should propagate constraint failures without wrapping', async () => { + const error = new Error('P2002 unique constraint'); + prismaService.user.create.mockRejectedValue(error); + + await expect( + service.signupTransporter(transporterDto), + ).rejects.toThrow('P2002 unique constraint'); + }); + }); +}); \ No newline at end of file diff --git a/src/modules/auth/application/transporter-signup.service.ts b/src/modules/auth/application/transporter-signup.service.ts new file mode 100644 index 0000000..b215fa2 --- /dev/null +++ b/src/modules/auth/application/transporter-signup.service.ts @@ -0,0 +1,104 @@ +import { Injectable } from '@nestjs/common'; +import { RolesEnum } from '../../../../generated/prisma'; +import { NotificationMessages } from '../../notification/notification-messages'; +import { NotificationService } from '../../notification/notification.service'; +import { PrismaService } from '../../prisma/prisma.service'; +import { TokenService } from '../../token/token.service'; +import { SignupSenderDto } from '../dto/signup-sender.dto'; +import { SignupTransporterDto } from '../dto/signup-transporter.dto'; + +/** + * Orchestrates the multi-step signup flows (sender + transporter). + * + * Each multi-write operation runs in a single Prisma transaction. Error + * wrapping is handled by the global AllExceptionsFilter — services throw + * only domain/Nest exceptions (A-3: no .catch(formatPrismaError) here). + */ +@Injectable() +export class TransporterSignupService { + constructor( + private readonly prisma: PrismaService, + private readonly tokenService: TokenService, + private readonly notificationService: NotificationService, + ) {} + + async signupSender(dto: SignupSenderDto) { + return this.prisma.$transaction(async (tx) => { + const sender = await tx.user.create({ + data: { + ...dto, + wallet: { create: {} }, + role: RolesEnum.sender, + phoneVerifiedAt: new Date(), + }, + }); + + const accessToken = this.tokenService.generateAccessToken({ + sub: sender.id, + phoneNumber: sender.phoneNumber, + }); + + await this.notificationService.create( + sender.id, + { content: NotificationMessages.Welcome }, + tx, + ); + + return { sender, accessToken }; + }); + } + + async signupTransporter(dto: SignupTransporterDto) { + const { + nationalId, + licenseNumber, + licenseExpiryDate, + licenseType, + profilePictureKey, + ...userData + } = dto; + + return this.prisma.$transaction(async (tx) => { + const transporter = await tx.user.create({ + data: { + ...userData, + wallet: { create: {} }, + role: RolesEnum.transporter, + transporter: { + create: { + nationalId, + licenseNumber, + licenseExpiryDate, + licenseType, + profilePictureKey, + nationalIdStatus: { create: {} }, + licenseStatus: { create: {} }, + verificationStatus: { create: {} }, + }, + }, + }, + include: { transporter: true }, + }); + + const progressToken = this.tokenService.generateProgressToken({ + sub: transporter.id, + phoneNumber: transporter.phoneNumber, + }); + + await this.notificationService.create( + transporter.id, + { content: NotificationMessages.Welcome }, + tx, + ); + + return { + transporter: { + ...transporter, + ...transporter.transporter, + transporter: undefined, + }, + progressToken, + }; + }); + } +} diff --git a/src/modules/auth/domain/auth-state.machine.spec.ts b/src/modules/auth/domain/auth-state.machine.spec.ts new file mode 100644 index 0000000..8ddd999 --- /dev/null +++ b/src/modules/auth/domain/auth-state.machine.spec.ts @@ -0,0 +1,102 @@ +import { AuthStateMachine, TransporterLike } from './auth-state.machine'; +import { UserStatesEnum } from '../types/auth.enums'; +import { VerificationStatusEnum } from '../../../../generated/prisma'; + +describe('AuthStateMachine', () => { + let machine: AuthStateMachine; + + beforeEach(() => { + machine = new AuthStateMachine(); + }); + + const baseTransporter: TransporterLike = { + vehicles: [], + licenseDocumentKey: null, + nationalIdDocumentKey: null, + verificationStatus: { status: VerificationStatusEnum.pending }, + }; + + describe('compute', () => { + it('should return PersonalInfoSubmitted with transporter data when no vehicles exist', () => { + const result = machine.compute(baseTransporter); + + expect(result.userState).toBe(UserStatesEnum.PersonalInfoSubmitted); + expect(result.transporter).toBeDefined(); + }); + + it('should return VehicleInfoSubmitted when a vehicle exists but documents are missing', () => { + const transporter: TransporterLike = { + ...baseTransporter, + vehicles: [{ verificationDocuments: null }], + }; + + const result = machine.compute(transporter); + + expect(result.userState).toBe(UserStatesEnum.VehicleInfoSubmitted); + expect(result.transporter).toBeDefined(); + }); + + it('should return DocumentsSubmitted when all documents are present', () => { + const transporter: TransporterLike = { + ...baseTransporter, + vehicles: [{ verificationDocuments: {} }], + licenseDocumentKey: 'license-key', + nationalIdDocumentKey: 'national-id-key', + }; + + const result = machine.compute(transporter); + + expect(result.userState).toBe(UserStatesEnum.DocumentsSubmitted); + }); + + it('should return Authenticated when verificationStatus is verified', () => { + const transporter: TransporterLike = { + ...baseTransporter, + vehicles: [{ verificationDocuments: {} }], + licenseDocumentKey: 'license-key', + nationalIdDocumentKey: 'national-id-key', + verificationStatus: { status: VerificationStatusEnum.verified }, + }; + + const result = machine.compute(transporter); + + expect(result.userState).toBe(UserStatesEnum.Authenticated); + expect(result.transporter).toBeUndefined(); + }); + + it('should return VehicleInfoSubmitted even when verified without any vehicles', () => { + const transporter: TransporterLike = { + ...baseTransporter, + verificationStatus: { status: VerificationStatusEnum.verified }, + }; + + const result = machine.compute(transporter); + + expect(result.userState).toBe(UserStatesEnum.Authenticated); + }); + + it('should handle a null verificationStatus without throwing', () => { + const transporter: TransporterLike = { + ...baseTransporter, + verificationStatus: null, + }; + + const result = machine.compute(transporter); + + expect(result.userState).toBe(UserStatesEnum.PersonalInfoSubmitted); + }); + + it('should require both transport-level and vehicle documents for DocumentsSubmitted', () => { + const transporter: TransporterLike = { + ...baseTransporter, + vehicles: [{ verificationDocuments: {} }], + licenseDocumentKey: null, + nationalIdDocumentKey: 'national-id-key', + }; + + const result = machine.compute(transporter); + + expect(result.userState).toBe(UserStatesEnum.VehicleInfoSubmitted); + }); + }); +}); \ No newline at end of file diff --git a/src/modules/auth/domain/auth-state.machine.ts b/src/modules/auth/domain/auth-state.machine.ts new file mode 100644 index 0000000..362f5ba --- /dev/null +++ b/src/modules/auth/domain/auth-state.machine.ts @@ -0,0 +1,68 @@ +import { Injectable } from '@nestjs/common'; +import { VerificationStatusEnum } from '../../../../generated/prisma'; +import { TransporterResponseDto } from '../../user/dto/transporter-response.dto'; +import { UserStatesEnum } from '../types/auth.enums'; + +export interface TransporterStateResult { + userState: UserStatesEnum; + transporter?: TransporterResponseDto; +} + +/** + * Pure transporter state machine. Encapsulates the onboarding transition + * logic extracted from AuthService.computeTransporterState: + * + * PersonalInfoSubmitted ──(vehicle created)──► VehicleInfoSubmitted + * VehicleInfoSubmitted ──(all docs present)──► DocumentsSubmitted + * DocumentsSubmitted ──(verificationStatus=verified)──► Authenticated + * + * Receives plain transporter data (as returned by UserService.getTransporter) + * and returns the computed state — no DB access, no side effects. + */ +@Injectable() +export class AuthStateMachine { + compute(transporter: TransporterLike): TransporterStateResult { + const isVerified = + transporter.verificationStatus?.status === + VerificationStatusEnum.verified; + + if (isVerified) { + return { userState: UserStatesEnum.Authenticated }; + } + + if (transporter.vehicles.length === 0) { + return { + userState: UserStatesEnum.PersonalInfoSubmitted, + transporter: transporter as TransporterResponseDto, + }; + } + + let state: UserStatesEnum = UserStatesEnum.VehicleInfoSubmitted; + + const hasAllDocuments = + transporter.licenseDocumentKey && + transporter.nationalIdDocumentKey && + transporter.vehicles[0].verificationDocuments; + + if (hasAllDocuments) { + state = UserStatesEnum.DocumentsSubmitted; + } + + return { + userState: state, + transporter: transporter as TransporterResponseDto, + }; + } +} + +/** + * Minimal structural type for what the state machine reads from a transporter + * record. Using a structural type keeps AuthStateMachine pure and testable + * without importing Prisma-generated types into the domain layer. + */ +export interface TransporterLike { + vehicles: { verificationDocuments?: unknown }[]; + licenseDocumentKey?: string | null; + nationalIdDocumentKey?: string | null; + verificationStatus?: { status: VerificationStatusEnum } | null; +} diff --git a/src/modules/auth/domain/otp.service.spec.ts b/src/modules/auth/domain/otp.service.spec.ts new file mode 100644 index 0000000..02c4a1d --- /dev/null +++ b/src/modules/auth/domain/otp.service.spec.ts @@ -0,0 +1,260 @@ +import { UnauthorizedException } from '@nestjs/common'; +import { DeepMockProxy, mockDeep } from 'jest-mock-extended'; +import { Keyv } from '@keyv/redis'; +import { OtpService, OtpConfig } from './otp.service'; +import { SmsService } from '../../sms/sms.service'; +import { TooManyRequestsException } from '../../../common/custom.exceptions'; +import { AuthMessages } from '../../../common/enums/messages.enum'; +import { CachedUserData } from '../types/auth.types'; + +describe('OtpService', () => { + let service: OtpService; + // `get` is widened to jest.Mock: Keyv's overloaded `get` signature collapses + // under jest.Mocked's transform, making mockResolvedValue un-typable. + let cache: { get: jest.Mock } & jest.Mocked; + let smsService: DeepMockProxy; + + const config: OtpConfig = { + otpExpireTime: 2 * 60 * 1000, + maxSendAttempts: 5, + maxCheckAttempts: 10, + sendWindow: 30 * 60 * 1000, + baseBlockTime: 20 * 60 * 1000, + }; + + const NOW = 1_700_000_000_000; + + beforeEach(() => { + jest.useFakeTimers(); + jest.setSystemTime(NOW); + + cache = mockDeep() as unknown as { get: jest.Mock } & jest.Mocked; + smsService = mockDeep(); + service = new OtpService(cache, config, smsService); + + cache.set.mockResolvedValue(true); + smsService.sendOtp.mockResolvedValue(true); + }); + + afterEach(() => { + jest.useRealTimers(); + }); + + // Factories (not shared constants): OtpService mutates attempts in place, + // so a shared object would leak state across tests. + const defaultAttempts = () => ({ + sendAttempts: 0, + checkAttempts: 0, + lastSendAttempt: 0, + }); + + const emptyUserData = (): CachedUserData => ({ attempts: defaultAttempts() }); + + describe('sendOtp', () => { + it('should send the OTP once and persist attempts on the happy path', async () => { + cache.get.mockResolvedValue(null as any); + + const result = await service.sendOtp('+989123456789'); + + expect(result).toBe(true); + expect(smsService.sendOtp).toHaveBeenCalledTimes(1); + expect(smsService.sendOtp).toHaveBeenCalledWith( + '+989123456789', + expect.stringMatching(/^\d{6}$/), + ); + expect(cache.set).toHaveBeenCalledTimes(1); + const [, stored] = cache.set.mock.calls[0] as [string, CachedUserData]; + expect(stored.attempts.sendAttempts).toBe(1); + expect(stored.attempts.checkAttempts).toBe(0); + expect(stored.attempts.lastSendAttempt).toBe(NOW); + expect(stored.otp?.expiresIn).toBe(NOW + config.otpExpireTime); + expect(stored.otp?.code).toMatch(/^\d{6}$/); + }); + + it('should throw OtpNotExpired when a valid OTP is still live', async () => { + const userData: CachedUserData = { + attempts: defaultAttempts(), + otp: { code: '123456', expiresIn: NOW + 60_000, createdAt: NOW }, + }; + cache.get.mockResolvedValue(userData); + + await expect(service.sendOtp('+989123456789')).rejects.toThrow( + UnauthorizedException, + ); + await expect(service.sendOtp('+989123456789')).rejects.toThrow( + AuthMessages.OtpNotExpired, + ); + expect(smsService.sendOtp).not.toHaveBeenCalled(); + }); + + it('should allow a new OTP after the previous one expires', async () => { + const userData: CachedUserData = { + attempts: defaultAttempts(), + otp: { code: '123456', expiresIn: NOW - 1, createdAt: NOW }, + }; + cache.get.mockResolvedValue(userData); + + const result = await service.sendOtp('+989123456789'); + expect(result).toBe(true); + expect(smsService.sendOtp).toHaveBeenCalledTimes(1); + }); + + it('should throw 429 when the user is currently blocked', async () => { + const userData: CachedUserData = { + attempts: { ...defaultAttempts(), blockedUntil: NOW + 5 * 60_000 }, + }; + cache.get.mockResolvedValue(userData); + + await expect(service.sendOtp('+989123456789')).rejects.toThrow( + TooManyRequestsException, + ); + await expect(service.sendOtp('+989123456789')).rejects.toThrow( + /Please try again in \d+ minutes/, + ); + }); + + it('should throw 429 when the send limit is reached', async () => { + const userData: CachedUserData = { + attempts: { + ...defaultAttempts(), + sendAttempts: config.maxSendAttempts, + // Just hit the limit: an old lastSendAttempt would trigger the + // send-window reset in getUserData() before the limit check. + lastSendAttempt: NOW, + }, + }; + cache.get.mockResolvedValue(userData); + + // Single invocation: hitting the limit also sets blockedUntil, so a + // second call would fail with the "currently blocked" message instead. + const error = await service.sendOtp('+989123456789').catch((e) => e); + + expect(error).toBeInstanceOf(TooManyRequestsException); + expect(error.message).toBe(AuthMessages.MaxAttempts); + expect(smsService.sendOtp).not.toHaveBeenCalled(); + }); + + it('should not persist anything when the SMS send fails', async () => { + cache.get.mockResolvedValue(null as any); + smsService.sendOtp.mockRejectedValue(new Error('SMS down')); + + await expect(service.sendOtp('+989123456789')).rejects.toThrow( + 'SMS down', + ); + expect(cache.set).not.toHaveBeenCalled(); + }); + + it('should return false when the SMS provider reports failure', async () => { + cache.get.mockResolvedValue(null as any); + smsService.sendOtp.mockResolvedValue(false); + + const result = await service.sendOtp('+989123456789'); + expect(result).toBe(false); + expect(cache.set).not.toHaveBeenCalled(); + }); + + it('should reset send attempts when the send window passed', async () => { + const userData: CachedUserData = { + attempts: { + ...defaultAttempts(), + sendAttempts: config.maxSendAttempts, + lastSendAttempt: NOW - config.sendWindow - 1, + }, + }; + cache.get.mockResolvedValue(userData); + + const result = await service.sendOtp('+989123456789'); + expect(result).toBe(true); + const [, stored] = cache.set.mock.calls[0] as [string, CachedUserData]; + expect(stored.attempts.sendAttempts).toBe(1); + }); + }); + + describe('verify', () => { + const OTP = '654321'; + + it('should succeed for a valid code and clear the OTP', async () => { + const userData: CachedUserData = { + attempts: defaultAttempts(), + otp: { code: OTP, expiresIn: NOW + 60_000, createdAt: NOW }, + }; + cache.get.mockResolvedValue(userData); + + await service.verify('+989123456789', OTP); + + expect(cache.set).toHaveBeenCalledTimes(1); + const [, stored] = cache.set.mock.calls[0] as [string, CachedUserData]; + expect(stored.otp).toBeUndefined(); + expect(stored.attempts.checkAttempts).toBe(1); + }); + + it('should throw OtpExpired when no OTP exists', async () => { + cache.get.mockResolvedValue(emptyUserData()); + + await expect(service.verify('+989123456789', OTP)).rejects.toThrow( + UnauthorizedException, + ); + await expect(service.verify('+989123456789', OTP)).rejects.toThrow( + AuthMessages.OtpExpired, + ); + }); + + it('should throw OtpExpired when the OTP is expired', async () => { + const userData: CachedUserData = { + attempts: defaultAttempts(), + otp: { code: OTP, expiresIn: NOW - 1, createdAt: NOW - 130_000 }, + }; + cache.get.mockResolvedValue(userData); + + await expect(service.verify('+989123456789', OTP)).rejects.toThrow( + AuthMessages.OtpExpired, + ); + }); + + it('should throw OtpInvalid and increment check attempts on a wrong code', async () => { + const userData: CachedUserData = { + attempts: defaultAttempts(), + otp: { code: OTP, expiresIn: NOW + 60_000, createdAt: NOW }, + }; + cache.get.mockResolvedValue(userData); + + await expect( + service.verify('+989123456789', '000000'), + ).rejects.toThrow(AuthMessages.OtpInvalid); + expect(cache.set).toHaveBeenCalledTimes(1); + + const [, stored] = cache.set.mock.calls[0] as [string, CachedUserData]; + expect(stored.attempts.checkAttempts).toBe(1); + expect(stored.otp).toBeDefined(); + }); + + it('should block after exceeding the check limit', async () => { + const userData: CachedUserData = { + attempts: { + ...defaultAttempts(), + checkAttempts: config.maxCheckAttempts, + }, + otp: { code: OTP, expiresIn: NOW + 60_000, createdAt: NOW }, + }; + cache.get.mockResolvedValue(userData); + + await expect(service.verify('+989123456789', OTP)).rejects.toThrow( + TooManyRequestsException, + ); + expect(cache.set).toHaveBeenCalledTimes(1); + const [, stored] = cache.set.mock.calls[0] as [string, CachedUserData]; + expect(stored.attempts.blockedUntil).toBeDefined(); + }); + + it('should throw 429 when the user is currently blocked', async () => { + const userData: CachedUserData = { + attempts: { ...defaultAttempts(), blockedUntil: NOW + 5 * 60_000 }, + }; + cache.get.mockResolvedValue(userData); + + await expect(service.verify('+989123456789', OTP)).rejects.toThrow( + TooManyRequestsException, + ); + }); + }); +}); \ No newline at end of file diff --git a/src/modules/auth/domain/otp.service.ts b/src/modules/auth/domain/otp.service.ts new file mode 100644 index 0000000..fdd9c5a --- /dev/null +++ b/src/modules/auth/domain/otp.service.ts @@ -0,0 +1,173 @@ +import { Inject, Injectable, UnauthorizedException } from '@nestjs/common'; +import { TooManyRequestsException } from '../../../common/custom.exceptions'; +import { AuthMessages } from '../../../common/enums/messages.enum'; +import { generateSecureOtp } from '../../../common/utils/codes'; +import { Keyv } from '@keyv/redis'; +import { SmsService } from '../../sms/sms.service'; +import { CachedUserData, UserAttempts } from '../types/auth.types'; + +export const OTP_CACHE = Symbol('OTP_CACHE'); +export const OTP_CONFIG = Symbol('OTP_CONFIG'); + +export interface OtpConfig { + otpExpireTime: number; + maxSendAttempts: number; + maxCheckAttempts: number; + sendWindow: number; + baseBlockTime: number; +} + +/** + * OTP lifecycle + rate-limiting state machine, extracted from AuthService. + * + * Owns the send/verify flow including attempt tracking, blocking, and the + * send window. Reads limits via an injected OtpConfig value object built + * once from ConfigService in the module factory (fixes C-1: only the + * canonical MAX_SEND_ATTEMPTS key is read). + * + * Cache access goes through the OTP_CACHE named provider bound to the OTP + * Keyv store — no positional cacheManager.stores[1] access (fixes C-3). + */ +@Injectable() +export class OtpService { + constructor( + @Inject(OTP_CACHE) private readonly cache: Keyv, + @Inject(OTP_CONFIG) private readonly config: OtpConfig, + private readonly smsService: SmsService, + ) {} + + async sendOtp(phoneNumber: string): Promise { + const userKey = this.getUserKey(phoneNumber); + const userData = await this.getUserData(userKey); + + this.checkIfBlocked(userData.attempts); + this.checkSendAttempts(userData.attempts); + + const now = Date.now(); + if (userData.otp && now < userData.otp.expiresIn) { + throw new UnauthorizedException(AuthMessages.OtpNotExpired); + } + + const otp = { + code: generateSecureOtp(), + expiresIn: now + this.config.otpExpireTime, + createdAt: now, + }; + + userData.attempts.sendAttempts++; + userData.attempts.checkAttempts = 0; + userData.attempts.lastSendAttempt = now; + userData.otp = otp; + + const otpSmsResult = await this.smsService.sendOtp(phoneNumber, otp.code); + return otpSmsResult && (await this.setUserData(userKey, userData)); + } + + async verify(phoneNumber: string, code: string): Promise { + const userKey = this.getUserKey(phoneNumber); + const userData = await this.getUserData(userKey); + + this.checkIfBlocked(userData.attempts); + + if (!userData?.otp) { + throw new UnauthorizedException(AuthMessages.OtpExpired); + } + + const now = Date.now(); + if (now > userData.otp.expiresIn) { + throw new UnauthorizedException(AuthMessages.OtpExpired); + } + + userData.attempts.checkAttempts++; + + if (userData.attempts.checkAttempts > this.config.maxCheckAttempts) { + userData.attempts.blockedUntil = + now + this.calculateBlockTime(userData.attempts); + await this.setUserData(userKey, userData); + throw new TooManyRequestsException(AuthMessages.TooManyAttempts); + } + + if (userData.otp.code !== code) { + await this.setUserData(userKey, userData); + throw new UnauthorizedException(AuthMessages.OtpInvalid); + } + + // OTP is valid — clear the OTP data but keep attempts history + userData.otp = undefined; + await this.setUserData(userKey, userData); + } + + private getUserKey( + phoneNumber: string, + type: 'mobile' | 'email' = 'mobile', + ): string { + return `otp:${type}:${phoneNumber}`; + } + + private async getUserData(userKey: string): Promise { + const userData = await this.cache.get(userKey); + if (!userData) { + return { + attempts: { + sendAttempts: 0, + checkAttempts: 0, + lastSendAttempt: 0, + }, + }; + } + + // Reset send attempts if the send window has passed + const now = Date.now(); + if (now - userData.attempts.lastSendAttempt > this.config.sendWindow) { + userData.attempts.sendAttempts = 0; + } + + return userData; + } + + private async setUserData( + userKey: string, + userData: CachedUserData, + ): Promise { + const cacheTime = this.calculateCacheTime(userData.attempts); + return this.cache.set(userKey, userData, cacheTime); + } + + private checkIfBlocked(attempts: UserAttempts): void { + const now = Date.now(); + if (attempts.blockedUntil && now < attempts.blockedUntil) { + const remainingTime = Math.ceil( + (attempts.blockedUntil - now) / 1000 / 60, + ); + throw new TooManyRequestsException( + `Too many attempts. Please try again in ${remainingTime} minutes.`, + ); + } + } + + private checkSendAttempts(attempts: UserAttempts): void { + if (attempts.sendAttempts >= this.config.maxSendAttempts) { + const blockTime = this.calculateBlockTime(attempts); + attempts.blockedUntil = Date.now() + blockTime; + throw new TooManyRequestsException(AuthMessages.MaxAttempts); + } + } + + private calculateBlockTime(attempts: UserAttempts): number { + const violations = + Math.floor(attempts.sendAttempts / this.config.maxSendAttempts) + + Math.floor(attempts.checkAttempts / this.config.maxCheckAttempts); + if (!violations) return 0; + return this.config.baseBlockTime * Math.pow(2, violations); + } + + private calculateCacheTime(attempts: UserAttempts): number { + if (attempts.blockedUntil) { + return Math.max( + attempts.blockedUntil - Date.now() + 60_000, + this.config.sendWindow, + ); + } + return this.config.sendWindow; + } +} diff --git a/src/modules/auth/domain/session-utils.spec.ts b/src/modules/auth/domain/session-utils.spec.ts new file mode 100644 index 0000000..b81a257 --- /dev/null +++ b/src/modules/auth/domain/session-utils.spec.ts @@ -0,0 +1,46 @@ +import { SessionData } from 'express-session'; +import { clearAuthData, setSessionAuth, setUserState } from './session-utils'; +import { UserStatesEnum } from '../types/auth.enums'; + +describe('session-utils', () => { + const createSession = () => + ({ + userId: undefined, + phoneNumber: undefined, + userState: undefined, + accessToken: '', + lastAccessed: new Date(), + packages: [], + }) as unknown as SessionData; + + describe('setUserState', () => { + it('should set the userState on the session', () => { + const session = createSession(); + setUserState(session, UserStatesEnum.VehicleInfoSubmitted); + expect(session.userState).toBe(UserStatesEnum.VehicleInfoSubmitted); + }); + }); + + describe('setSessionAuth', () => { + it('should set userId and phoneNumber on the session', () => { + const session = createSession(); + setSessionAuth(session, 'user-1', '+989123456789'); + expect(session.userId).toBe('user-1'); + expect(session.phoneNumber).toBe('+989123456789'); + }); + }); + + describe('clearAuthData', () => { + it('should clear userId, phoneNumber and userState', () => { + const session = createSession(); + setSessionAuth(session, 'user-1', '+989123456789'); + setUserState(session, UserStatesEnum.Authenticated); + + clearAuthData(session); + + expect(session.userId).toBeUndefined(); + expect(session.phoneNumber).toBeUndefined(); + expect(session.userState).toBeUndefined(); + }); + }); +}); \ No newline at end of file diff --git a/src/modules/auth/domain/session-utils.ts b/src/modules/auth/domain/session-utils.ts new file mode 100644 index 0000000..45122ec --- /dev/null +++ b/src/modules/auth/domain/session-utils.ts @@ -0,0 +1,29 @@ +import { SessionData } from 'express-session'; +import { UserStatesEnum } from '../types/auth.enums'; + +/** + * Pure session mutation helpers. Centralizes session writes so that + * controllers never mutate session state directly (A-4 mitigation). + */ + +export function setUserState( + session: SessionData, + state: UserStatesEnum, +): void { + session.userState = state; +} + +export function setSessionAuth( + session: SessionData, + userId: string, + phoneNumber: string, +): void { + session.userId = userId; + session.phoneNumber = phoneNumber; +} + +export function clearAuthData(session: SessionData): void { + session.userId = undefined; + session.phoneNumber = undefined; + session.userState = undefined; +} From 81cdb2760e9328410d7d378fadd17cfd5abfb49f Mon Sep 17 00:00:00 2001 From: Mohammad Hosseini Date: Thu, 3 Sep 2026 22:47:38 +0330 Subject: [PATCH 5/5] Refactor `AuthService`, `AuthController` and `AuthModule`: wire extracted services - Slim AuthService to a thin facade delegating to OtpService, AuthStateMachine and TransporterSignupService; inject the state machine and signup service in AuthModule with OtpService (OTP_CACHE, OTP_CONFIG factory providers) - Switch the OTP contract from a 5-digit number to a zero-padded 6-digit string: CheckOtpDto validates a 6-char numeric string, SmsService.sendOtp takes a string, OtpData.code becomes string - Move cookie/session wiring to the controller and set state via session-utils; remove dead code (unused maxAge/config reads) - Rewrite controller/service specs against the new collaborators with all guards mocked via overrideGuard --- src/modules/auth/auth.controller.spec.ts | 288 +++++++++- src/modules/auth/auth.controller.ts | 68 +-- src/modules/auth/auth.module.ts | 37 +- src/modules/auth/auth.service.spec.ts | 670 +++++++++-------------- src/modules/auth/auth.service.ts | 395 ++----------- src/modules/auth/dto/check-otp.dto.ts | 13 +- src/modules/auth/types/auth.types.ts | 2 +- src/modules/sms/sms.service.ts | 10 +- 8 files changed, 644 insertions(+), 839 deletions(-) diff --git a/src/modules/auth/auth.controller.spec.ts b/src/modules/auth/auth.controller.spec.ts index be37f4f..7dee728 100644 --- a/src/modules/auth/auth.controller.spec.ts +++ b/src/modules/auth/auth.controller.spec.ts @@ -1,6 +1,6 @@ import { Test, TestingModule } from '@nestjs/testing'; -import { AuthController } from './auth.controller'; import { DeepMockProxy, mockDeep } from 'jest-mock-extended'; +import { AuthController } from './auth.controller'; import { AuthService } from './auth.service'; import { AccessTokenGuard, @@ -9,32 +9,45 @@ import { } from './guard/token.guard'; import { DenyAuthorizedGuard } from './guard/deny-authorized.guard'; import { MultiTokenGuard } from './guard/multi-token.guard'; -import { VehicleService } from '../vehicle/vehicle.service'; import { ConfigService } from '@nestjs/config'; +import { CookieNames } from '../../common/enums/cookies.enum'; +import { UserStatesEnum } from './types/auth.enums'; describe('AuthController', () => { let controller: AuthController; let service: DeepMockProxy; - let vehicleService: DeepMockProxy; - let configService: DeepMockProxy; + + const session = () => + ({ + userId: undefined, + phoneNumber: undefined, + userState: undefined, + accessToken: undefined, + destroy: jest.fn(), + }) as any; + + const res = () => + ({ + cookie: jest.fn(), + clearCookie: jest.fn(), + }) as any; beforeEach(async () => { service = mockDeep(); - vehicleService = mockDeep(); - configService = mockDeep(); + const configService = mockDeep(); configService.get.mockImplementation((key: string, defaultValue?: any) => { - const config = { + const config: Record = { + NODE_ENV: 'development', COOKIE_MAX_AGE: 50000, }; - return config[key] || defaultValue; + return config[key] ?? defaultValue; }); const module: TestingModule = await Test.createTestingModule({ controllers: [AuthController], providers: [ { provide: AuthService, useValue: service }, - { provide: VehicleService, useValue: vehicleService }, { provide: ConfigService, useValue: configService }, ], }) @@ -53,7 +66,258 @@ describe('AuthController', () => { controller = module.get(AuthController); }); - it('should be defined', () => { - expect(controller).toBeDefined(); + describe('sendOtp', () => { + it('should delegate to the service', async () => { + service.sendOtp.mockResolvedValue(true); + const body = { phoneNumber: '+989123456789' }; + + const result = await controller.sendOtp(body); + + expect(service.sendOtp).toHaveBeenCalledWith(body); + expect(result).toBe(true); + }); + }); + + describe('checkOtp', () => { + const body = { phoneNumber: '+989123456789', code: '123456' }; + + it('should set the access cookie and session for an access token', async () => { + service.checkOtp.mockResolvedValue({ + isNewUser: false, + userId: 'user-1', + token: 'access-token', + type: 'access', + role: undefined, + } as any); + const ses = session(); + const resMock = res(); + + const result = await controller.checkOtp(body, ses, resMock); + + expect(resMock.cookie).toHaveBeenCalledWith( + CookieNames.AccessToken, + 'access-token', + expect.objectContaining({ httpOnly: true, path: '/' }), + ); + expect(ses.accessToken).toBe('access-token'); + expect(ses.userId).toBe('user-1'); + expect(ses.phoneNumber).toBe('+989123456789'); + expect(result).toEqual({ isNewUser: false }); + }); + + it('should set the temporary cookie with a 20m maxAge', async () => { + service.checkOtp.mockResolvedValue({ + isNewUser: true, + token: 'temp-token', + type: 'temp', + } as any); + + const ses = session(); + const resMock = res(); + + await controller.checkOtp(body, ses, resMock); + + expect(resMock.cookie).toHaveBeenCalledWith( + CookieNames.TemporaryToken, + 'temp-token', + expect.objectContaining({ maxAge: 20 * 60 * 1000 }), + ); + expect(ses.accessToken).toBeUndefined(); + expect(ses.phoneNumber).toBe('+989123456789'); + }); + + it('should set the progress cookie with the progress maxAge', async () => { + service.checkOtp.mockResolvedValue({ + isNewUser: false, + userId: 'user-1', + token: 'progress-token', + type: 'progress', + } as any); + + const ses = session(); + const resMock = res(); + + await controller.checkOtp(body, ses, resMock); + + expect(resMock.cookie).toHaveBeenCalledWith( + CookieNames.ProgressToken, + 'progress-token', + expect.objectContaining({ maxAge: 2 * 24 * 60 * 60 * 1000 }), + ); + // Matches legacy behavior: the progress path does not clear the temp cookie + expect(resMock.clearCookie).not.toHaveBeenCalled(); + }); + }); + + describe('signupSender', () => { + const body = { + firstName: 'احمد', + lastName: 'محمدی', + phoneNumber: '+989123456789', + gender: 'male', + } as any; + + it('should reject a phone number mismatch', async () => { + const ses = session(); + ses.phoneNumber = '+989000000000'; + + await expect(controller.signupSender(body, ses, res())).rejects.toThrow( + 'Phone number in request does not match', + ); + }); + + it('should issue the access cookie and prime the session', async () => { + service.signupSender.mockResolvedValue({ + sender: { id: 'user-1', phoneNumber: '+989123456789' }, + accessToken: 'access-token', + } as any); + const ses = session(); + ses.phoneNumber = '+989123456789'; + const resMock = res(); + + const result = await controller.signupSender(body, ses, resMock); + + expect(resMock.cookie).toHaveBeenCalledWith( + CookieNames.AccessToken, + 'access-token', + expect.any(Object), + ); + expect(resMock.clearCookie).toHaveBeenCalledWith( + CookieNames.TemporaryToken, + ); + expect(ses.userState).toBe(UserStatesEnum.Authenticated); + expect(ses.userId).toBe('user-1'); + expect(result).toEqual({ id: 'user-1', phoneNumber: '+989123456789' }); + }); + }); + + describe('signupTransporter', () => { + const body = { + firstName: 'احمد', + lastName: 'محمدی', + phoneNumber: '+989123456789', + gender: 'male', + birthDate: '1990-01-01', + nationalId: '1234567890', + licenseNumber: '0987654321', + licenseType: 'grade_one', + licenseExpiryDate: '2030-01-01', + } as any; + + it('should reject a phone number mismatch', async () => { + const ses = session(); + ses.phoneNumber = '+989000000000'; + + await expect( + controller.signupTransporter(body, ses, res()), + ).rejects.toThrow('Phone number in request does not match'); + }); + + it('should issue the progress cookie and set PersonalInfoSubmitted', async () => { + service.signupTransporter.mockResolvedValue({ + transporter: { userId: 'user-1', phoneNumber: '+989123456789' }, + progressToken: 'progress-token', + } as any); + const ses = session(); + ses.phoneNumber = '+989123456789'; + const resMock = res(); + + const result = await controller.signupTransporter(body, ses, resMock); + + expect(resMock.cookie).toHaveBeenCalledWith( + CookieNames.ProgressToken, + 'progress-token', + expect.any(Object), + ); + expect(resMock.clearCookie).toHaveBeenCalledWith( + CookieNames.TemporaryToken, + ); + expect(ses.userState).toBe(UserStatesEnum.PersonalInfoSubmitted); + expect(ses.userId).toBe('user-1'); + expect(result).toEqual({ userId: 'user-1', phoneNumber: '+989123456789' }); + }); + }); + + describe('registerTransporterVehicle', () => { + it('should delegate the vehicle creation to the service', async () => { + const vehicle = { id: 'vehicle-1' }; + service.registerVehicle.mockResolvedValue(vehicle as any); + const ses = session(); + const body = {} as any; + + const result = await controller.registerTransporterVehicle( + body, + ses, + 'user-1', + ); + + expect(service.registerVehicle).toHaveBeenCalledWith('user-1', body, ses); + expect(result).toBe(vehicle); + }); + }); + + describe('submitTransporterDocumentKeys', () => { + it('should issue the access cookie and clear the progress cookie', async () => { + service.submitDocuments.mockResolvedValue({ accessToken: 'access-token' }); + const ses = session(); + const resMock = res(); + const user = { id: 'user-1', phoneNumber: '+989123456789' } as any; + const body = {} as any; + + const result = await controller.submitTransporterDocumentKeys( + body, + ses, + user, + resMock, + ); + + expect(service.submitDocuments).toHaveBeenCalledWith( + 'user-1', + '+989123456789', + body, + ); + expect(resMock.cookie).toHaveBeenCalledWith( + CookieNames.AccessToken, + 'access-token', + expect.any(Object), + ); + expect(resMock.clearCookie).toHaveBeenCalledWith( + CookieNames.ProgressToken, + ); + expect(ses.accessToken).toBe('access-token'); + expect(result).toBe(true); + }); + }); + + describe('getUserState', () => { + it('should delegate to the service', async () => { + const state = { userState: UserStatesEnum.PersonalInfoSubmitted }; + service.getUserState.mockResolvedValue(state as any); + const ses = session(); + + const result = await controller.getUserState(ses); + + expect(service.getUserState).toHaveBeenCalledWith(ses); + expect(result).toBe(state); + }); + }); + + describe('logoutUser', () => { + it('should destroy the session and clear all auth cookies', async () => { + const ses = session(); + const resMock = res(); + + const result = await controller.logoutUser(ses, resMock); + + expect(ses.destroy).toHaveBeenCalled(); + expect(resMock.clearCookie).toHaveBeenCalledTimes(4); + expect(resMock.clearCookie).toHaveBeenCalledWith(CookieNames.SessionId); + expect(resMock.clearCookie).toHaveBeenCalledWith( + CookieNames.TemporaryToken, + ); + expect(resMock.clearCookie).toHaveBeenCalledWith(CookieNames.ProgressToken); + expect(resMock.clearCookie).toHaveBeenCalledWith(CookieNames.AccessToken); + expect(result).toBe(true); + }); }); -}); +}); \ No newline at end of file diff --git a/src/modules/auth/auth.controller.ts b/src/modules/auth/auth.controller.ts index b4e87c6..179fce2 100644 --- a/src/modules/auth/auth.controller.ts +++ b/src/modules/auth/auth.controller.ts @@ -39,10 +39,8 @@ import { SignupTransporterDto, SignupTransporterResponseDto, } from './dto/signup-transporter.dto'; -import { VehicleService } from '../vehicle/vehicle.service'; import { CreateVehicleDto } from '../vehicle/dto/create-vehicle.dto'; import { SubmitDocumentsDto } from './dto/submit-documents.dto'; -import { UserStatesEnum } from './types/auth.enums'; import { UserResponseDto } from '../user/dto/user-response.dto'; import { Serialize } from '../../common/serialize.interceptor'; import { VehicleResponseDto } from '../vehicle/dto/vehicle-response.dto'; @@ -54,21 +52,20 @@ import { ValidationResponses, } from '../../common/api-docs.decorators'; import { MultiTokenGuard } from './guard/multi-token.guard'; +import { getCookieOptions, CookieOptions } from '../../common/cookies/cookie-options'; +import { setUserState } from './domain/session-utils'; +import { UserStatesEnum } from './types/auth.enums'; @Controller('auth') export class AuthController { - private cookieMaxAge: number; - private progressMaxAge: number; + private readonly cookieOptions: CookieOptions; + private readonly progressMaxAge: number; constructor( - private authService: AuthService, - private vehicleService: VehicleService, + private readonly authService: AuthService, config: ConfigService, ) { - this.cookieMaxAge = config.get( - 'COOKIE_MAX_AGE', - 15 * 24 * 3600 * 1000, - ); // 15 days + this.cookieOptions = getCookieOptions(config); this.progressMaxAge = 2 * 24 * 60 * 60 * 1000; // 2 days } @@ -122,32 +119,19 @@ export class AuthController { session.accessToken = token; session.userId = userId; - res.cookie(CookieNames.AccessToken, token, { - httpOnly: true, - // TODO: Fix this when you deploy on a https server - // secure: process.env.NODE_ENV === 'production', - secure: false, - sameSite: 'strict', - maxAge: this.cookieMaxAge, - }); + res.cookie(CookieNames.AccessToken, token, this.cookieOptions); break; case AuthTokens.Temporary: res.cookie(CookieNames.TemporaryToken, token, { - httpOnly: true, - // TODO: Fix this when you deploy on a https server - // secure: process.env.NODE_ENV === 'production', - secure: false, - sameSite: 'strict', + ...this.cookieOptions, maxAge: 20 * 60 * 1000, }); break; case AuthTokens.Progress: res.cookie(CookieNames.ProgressToken, token, { - httpOnly: true, - - sameSite: 'strict', + ...this.cookieOptions, maxAge: this.progressMaxAge, }); break; @@ -189,17 +173,10 @@ export class AuthController { const { sender, accessToken } = await this.authService.signupSender(body); session.accessToken = accessToken; - res.cookie(CookieNames.AccessToken, accessToken, { - httpOnly: true, - // TODO: Fix this when you deploy on a https server - // secure: process.env.NODE_ENV === 'production', - secure: false, - sameSite: 'strict', - maxAge: this.cookieMaxAge, - }); + res.cookie(CookieNames.AccessToken, accessToken, this.cookieOptions); res.clearCookie(CookieNames.TemporaryToken); - session.userState = UserStatesEnum.Authenticated; + setUserState(session, UserStatesEnum.Authenticated); session.userId = sender.id; session.phoneNumber = sender.phoneNumber; @@ -242,16 +219,12 @@ export class AuthController { await this.authService.signupTransporter(body); res.cookie(CookieNames.ProgressToken, progressToken, { - httpOnly: true, - // TODO: Fix this when you deploy on a https server - // secure: process.env.NODE_ENV === 'production', - secure: false, - sameSite: 'strict', + ...this.cookieOptions, maxAge: this.progressMaxAge, }); res.clearCookie(CookieNames.TemporaryToken); - session.userState = UserStatesEnum.PersonalInfoSubmitted; + setUserState(session, UserStatesEnum.PersonalInfoSubmitted); session.userId = transporter.userId; session.phoneNumber = transporter.phoneNumber; @@ -285,9 +258,7 @@ export class AuthController { @Session() session: SessionData, @CurrentUser('id') id: string, ) { - const vehicle = await this.vehicleService.create(id, body); - session.userState = UserStatesEnum.VehicleInfoSubmitted; - return vehicle; + return this.authService.registerVehicle(id, body, session); } @ApiOperation({ @@ -314,14 +285,7 @@ export class AuthController { ); session.accessToken = accessToken; - res.cookie(CookieNames.AccessToken, accessToken, { - httpOnly: true, - // TODO: Fix this when you deploy on a https server - // secure: process.env.NODE_ENV === 'production', - secure: false, - sameSite: 'strict', - maxAge: this.cookieMaxAge, - }); + res.cookie(CookieNames.AccessToken, accessToken, this.cookieOptions); res.clearCookie(CookieNames.ProgressToken); return true; diff --git a/src/modules/auth/auth.module.ts b/src/modules/auth/auth.module.ts index f8b3ef3..8045a7c 100644 --- a/src/modules/auth/auth.module.ts +++ b/src/modules/auth/auth.module.ts @@ -1,4 +1,7 @@ import { MiddlewareConsumer, Module } from '@nestjs/common'; +import { CACHE_MANAGER } from '@nestjs/cache-manager'; +import { Cache } from 'cache-manager'; +import { ConfigService } from '@nestjs/config'; import { AuthController } from './auth.controller'; import { AuthService } from './auth.service'; import { TokenModule } from '../token/token.module'; @@ -8,6 +11,14 @@ import { VehicleModule } from '../vehicle/vehicle.module'; import { CurrentUserMiddleware } from 'src/modules/user/current-user.middleware'; import { SmsModule } from '../sms/sms.module'; import { NotificationModule } from '../notification/notification.module'; +import { + OtpService, + OTP_CACHE, + OTP_CONFIG, + OtpConfig, +} from './domain/otp.service'; +import { TransporterSignupService } from './application/transporter-signup.service'; +import { AuthStateMachine } from './domain/auth-state.machine'; @Module({ imports: [ @@ -19,7 +30,31 @@ import { NotificationModule } from '../notification/notification.module'; NotificationModule, ], controllers: [AuthController], - providers: [AuthService], + providers: [ + AuthService, + OtpService, + TransporterSignupService, + AuthStateMachine, + { + provide: OTP_CACHE, + inject: [CACHE_MANAGER], + useFactory: (cacheManager: Cache) => cacheManager.stores[1], + }, + { + provide: OTP_CONFIG, + inject: [ConfigService], + useFactory: (config: ConfigService): OtpConfig => ({ + otpExpireTime: config.get( + 'OTP_EXPIRATION_TIME', + 2 * 60 * 1000, + ), + maxSendAttempts: config.get('MAX_SEND_ATTEMPTS', 5), + maxCheckAttempts: config.get('MAX_CHECK_ATTEMPTS', 10), + sendWindow: config.get('SEND_WINDOW', 30 * 60 * 1000), + baseBlockTime: config.get('BASE_BLOCK_TIME', 20 * 60 * 1000), + }), + }, + ], }) export class AuthModule { configure(consumer: MiddlewareConsumer) { diff --git a/src/modules/auth/auth.service.spec.ts b/src/modules/auth/auth.service.spec.ts index 7e97129..950b742 100644 --- a/src/modules/auth/auth.service.spec.ts +++ b/src/modules/auth/auth.service.spec.ts @@ -1,71 +1,68 @@ import { Test, TestingModule } from '@nestjs/testing'; -import { CACHE_MANAGER } from '@nestjs/cache-manager'; -import { ConfigService } from '@nestjs/config'; -import { NotFoundException, UnauthorizedException } from '@nestjs/common'; +import { NotFoundException } from '@nestjs/common'; import { DeepMockProxy, mockDeep } from 'jest-mock-extended'; import { AuthService } from './auth.service'; import { TokenService } from '../token/token.service'; import { UserService } from '../user/user.service'; import { VehicleService } from '../vehicle/vehicle.service'; -import { NotificationService } from '../notification/notification.service'; import { PrismaService } from '../prisma/prisma.service'; -import { SmsService } from '../sms/sms.service'; -import { TooManyRequestsException } from '../../common/custom.exceptions'; -import { - AuthMessages, - NotFoundMessages, -} from '../../common/enums/messages.enum'; -import { AuthTokens } from '../../common/enums/auth.enum'; +import { OtpService } from './domain/otp.service'; +import { TransporterSignupService } from './application/transporter-signup.service'; +import { AuthStateMachine } from './domain/auth-state.machine'; import { RolesEnum, VerificationStatusEnum, GendersEnum, - LicenseTypeEnum, - PrismaClient, } from '../../../generated/prisma'; +import { AuthTokens } from '../../common/enums/auth.enum'; +import { NotFoundMessages } from '../../common/enums/messages.enum'; import { UserStatesEnum } from './types/auth.enums'; -import * as utilities from '../../common/utilities'; -import { Keyv } from '@keyv/redis'; - -jest.mock('../../common/utilities', () => ({ - generateCode: jest.fn(() => 12345), - formatPrismaError: jest.fn(), -})); describe('AuthService', () => { let service: AuthService; let tokenService: DeepMockProxy; let userService: DeepMockProxy; let vehicleService: DeepMockProxy; - let prismaService: DeepMockProxy; - let smsService: DeepMockProxy; - let configService: DeepMockProxy; - let keyvMock: DeepMockProxy; - let notificationService: DeepMockProxy; + let prismaService: DeepMockProxy; + let otpService: DeepMockProxy; + let signupService: DeepMockProxy; + let stateMachine: DeepMockProxy; - const mockUser = { + const mockSender = { id: 'user-123', phoneNumber: '+989123456789', role: RolesEnum.sender, } as any; const mockTransporter = { - id: 'trans-123', + id: 'user-456', + phoneNumber: '+989876543210', role: RolesEnum.transporter, - vehicles: [ - { - id: 'vehicle-123', - }, - ], + firstName: 'احمد', + lastName: 'محمدی', + vehicles: [], verificationStatus: { status: VerificationStatusEnum.pending }, } as any; - const mockTransporterInProgress = { - ...mockTransporter, - vehicles: [], - verificationStatus: { status: VerificationStatusEnum.pending }, - nationalIdDocumentKey: null, - licenseDocumentKey: null, + const mockVehicle = { id: 'vehicle-1' } as any; + + const signupSenderDto = { + firstName: 'احمد', + lastName: 'محمدی', + phoneNumber: '+989123456789', + gender: GendersEnum.male, + }; + + const signupTransporterDto = { + firstName: 'احمد', + lastName: 'محمدی', + phoneNumber: '+989876543210', + gender: GendersEnum.male, + birthDate: new Date('1990-01-01'), + nationalId: '1234567890', + licenseNumber: '0987654321', + licenseType: 'grade_one' as any, + licenseExpiryDate: new Date('2030-01-01'), }; beforeEach(async () => { @@ -74,27 +71,10 @@ describe('AuthService', () => { tokenService = mockDeep(); userService = mockDeep(); vehicleService = mockDeep(); - prismaService = mockDeep(); - smsService = mockDeep(); - configService = mockDeep(); - keyvMock = mockDeep(); - notificationService = mockDeep(); - - const cacheManager = { stores: [null, keyvMock] }; - - configService.get.mockImplementation((key: string, defaultValue?: any) => { - const config = { - OTP_EXPIRATION_TIME: 120000, - MAX_SEND_ATTEMPTS: 5, - MAX_CHECK_ATTEMPTS: 10, - SEND_WINDOW: 1800000, - BASE_BLOCK_TIME: 1200000, - }; - return config[key] || defaultValue; - }); - - // Reset utility mocks to default values - (utilities.generateCode as jest.Mock).mockReturnValue(12345); + prismaService = mockDeep(); + otpService = mockDeep(); + signupService = mockDeep(); + stateMachine = mockDeep(); const module: TestingModule = await Test.createTestingModule({ providers: [ @@ -103,10 +83,9 @@ describe('AuthService', () => { { provide: UserService, useValue: userService }, { provide: VehicleService, useValue: vehicleService }, { provide: PrismaService, useValue: prismaService }, - { provide: SmsService, useValue: smsService }, - { provide: ConfigService, useValue: configService }, - { provide: CACHE_MANAGER, useValue: cacheManager }, - { provide: NotificationService, useValue: notificationService }, + { provide: OtpService, useValue: otpService }, + { provide: TransporterSignupService, useValue: signupService }, + { provide: AuthStateMachine, useValue: stateMachine }, ], }).compile(); @@ -114,89 +93,36 @@ describe('AuthService', () => { }); describe('sendOtp', () => { - it('should send OTP successfully', async () => { - keyvMock.get.mockResolvedValue(null as any); - keyvMock.set.mockResolvedValue(true); - smsService.sendOtp.mockResolvedValue(true); + it('should delegate to OtpService', async () => { + otpService.sendOtp.mockResolvedValue(true); - const result = await service.sendOtp({ phoneNumber: '+989123456789' }); + const result = await service.sendOtp({ + phoneNumber: '+989123456789', + }); expect(result).toBe(true); - expect(smsService.sendOtp).toHaveBeenCalledWith('+989123456789', 12345); - }); - - it('should throw when user is blocked', async () => { - const blockedData = { - attempts: { - blockedUntil: Date.now() + 600000, - sendAttempts: 0, - checkAttempts: 0, - lastSendAttempt: 0, - }, - }; - keyvMock.get.mockResolvedValue(blockedData as any); - - await expect( - service.sendOtp({ phoneNumber: '+989123456789' }), - ).rejects.toThrow(TooManyRequestsException); - }); - - it('should throw when max send attempts exceeded', async () => { - const userData = { - attempts: { - sendAttempts: 5, - checkAttempts: 0, - lastSendAttempt: Date.now() - 60000, - }, - }; - keyvMock.get.mockResolvedValue(userData as any); - - await expect( - service.sendOtp({ phoneNumber: '+989123456789' }), - ).rejects.toThrow(TooManyRequestsException); - }); - - it('should throw when OTP not expired', async () => { - const userData = { - otp: { expiresIn: Date.now() + 60000 }, - attempts: { - sendAttempts: 1, - checkAttempts: 0, - lastSendAttempt: Date.now() - 60000, - }, - }; - keyvMock.get.mockResolvedValue(userData as any); - - await expect( - service.sendOtp({ phoneNumber: '+989123456789' }), - ).rejects.toThrow(new UnauthorizedException(AuthMessages.OtpNotExpired)); + expect(otpService.sendOtp).toHaveBeenCalledWith('+989123456789'); }); }); describe('checkOtp', () => { - const now = Date.now(); - - it('should verify OTP for new user', async () => { - const validOtpData = { - otp: { code: 12345, expiresIn: now + 60000, createdAt: now - 60000 }, - attempts: { - sendAttempts: 1, - checkAttempts: 0, - lastSendAttempt: now - 60000, - }, - }; - keyvMock.get.mockResolvedValue(validOtpData as any); - keyvMock.set.mockResolvedValue(true); + it('should issue a temp token for a new user', async () => { + otpService.verify.mockResolvedValue(undefined); userService.getByPhoneNumber.mockResolvedValue(null); - (tokenService['generateTempToken'] as jest.Mock).mockReturnValue( - 'temp-token', - ); + tokenService.generateTempToken.mockReturnValue('temp-token'); const result = await service.checkOtp({ phoneNumber: '+989123456789', - code: validOtpData.otp.code, + code: '123456', }); + expect(otpService.verify).toHaveBeenCalledWith( + '+989123456789', + '123456', + ); + expect(tokenService.generateTempToken).toHaveBeenCalledWith({ + phoneNumber: '+989123456789', + }); expect(result).toEqual({ isNewUser: true, token: 'temp-token', @@ -204,357 +130,283 @@ describe('AuthService', () => { }); }); - it('should verify OTP for existing sender', async () => { - const validOtpData = { - otp: { code: 12345, expiresIn: now + 60000, createdAt: now - 60000 }, - attempts: { - sendAttempts: 1, - checkAttempts: 0, - lastSendAttempt: now - 60000, - }, - }; - keyvMock.get.mockResolvedValue(validOtpData as any); - keyvMock.set.mockResolvedValue(true); - userService.getByPhoneNumber.mockResolvedValue(mockUser); - (tokenService['generateAccessToken'] as jest.Mock).mockReturnValue( - 'access-token', - ); + it('should issue an access token with Authenticated state for an existing sender', async () => { + otpService.verify.mockResolvedValue(undefined); + userService.getByPhoneNumber.mockResolvedValue(mockSender); + tokenService.generateAccessToken.mockReturnValue('access-token'); const result = await service.checkOtp({ phoneNumber: '+989123456789', - code: validOtpData.otp.code, + code: '123456', }); - expect(result).toEqual({ + expect(tokenService.generateAccessToken).toHaveBeenCalledWith({ + sub: 'user-123', + phoneNumber: '+989123456789', + }); + expect(result).toMatchObject({ isNewUser: false, userId: 'user-123', - role: RolesEnum.sender, - token: 'access-token', type: AuthTokens.Access, + token: 'access-token', userState: UserStatesEnum.Authenticated, }); }); - it('should handle transporter in progress', async () => { - const validOtpData = { - otp: { code: 12345, expiresIn: now + 60000, createdAt: now - 60000 }, - attempts: { - sendAttempts: 1, - checkAttempts: 0, - lastSendAttempt: now - 60000, - }, - }; - keyvMock.get.mockResolvedValue(validOtpData as any); - keyvMock.set.mockResolvedValue(true); - userService.getByPhoneNumber.mockResolvedValue({ - ...mockUser, - role: RolesEnum.transporter, + it('should issue a progress token for a transporter in progress', async () => { + otpService.verify.mockResolvedValue(undefined); + userService.getByPhoneNumber.mockResolvedValue(mockTransporter); + userService.getTransporter.mockResolvedValue(mockTransporter as any); + stateMachine.compute.mockReturnValue({ + userState: UserStatesEnum.PersonalInfoSubmitted, + transporter: mockTransporter, }); - userService.getTransporter.mockResolvedValue(mockTransporterInProgress); - (tokenService['generateProgressToken'] as jest.Mock).mockReturnValue( - 'progress-token', - ); + tokenService.generateProgressToken.mockReturnValue('progress-token'); const result = await service.checkOtp({ - phoneNumber: '+989123456789', - code: validOtpData.otp.code, + phoneNumber: '+989876543210', + code: '123456', }); - expect(result.type).toBe(AuthTokens.Progress); - expect(result.userState).toBe(UserStatesEnum.PersonalInfoSubmitted); + expect(stateMachine.compute).toHaveBeenCalled(); + expect(tokenService.generateProgressToken).toHaveBeenCalledWith({ + sub: 'user-456', + phoneNumber: '+989876543210', + }); + expect(result).toMatchObject({ + type: AuthTokens.Progress, + token: 'progress-token', + userState: UserStatesEnum.PersonalInfoSubmitted, + }); }); - it('should throw when OTP expired', async () => { - const expiredData = { - otp: { code: 12345, expiresIn: now - 60000 }, - attempts: { - sendAttempts: 1, - checkAttempts: 0, - lastSendAttempt: now - 60000, - }, - }; - keyvMock.get.mockResolvedValue(expiredData as any); - - await expect( - service.checkOtp({ phoneNumber: '+989123456789', code: 12345 }), - ).rejects.toThrow(new UnauthorizedException(AuthMessages.OtpExpired)); - }); + it('should keep the access token for a transporter in DocumentsSubmitted', async () => { + otpService.verify.mockResolvedValue(undefined); + userService.getByPhoneNumber.mockResolvedValue(mockTransporter); + userService.getTransporter.mockResolvedValue(mockTransporter as any); + stateMachine.compute.mockReturnValue({ + userState: UserStatesEnum.DocumentsSubmitted, + transporter: mockTransporter, + }); + tokenService.generateAccessToken.mockReturnValue('access-token'); - it('should throw when invalid OTP code', async () => { - const validOtpData = { - otp: { code: 12345, expiresIn: now + 60000, createdAt: now - 60000 }, - attempts: { - sendAttempts: 1, - checkAttempts: 0, - lastSendAttempt: now - 60000, - }, - }; - keyvMock.get.mockResolvedValue(validOtpData as any); - keyvMock.set.mockResolvedValue(true); + const result = await service.checkOtp({ + phoneNumber: '+989876543210', + code: '123456', + }); - await expect( - service.checkOtp({ phoneNumber: '+989123456789', code: 54321 }), - ).rejects.toThrow(new UnauthorizedException(AuthMessages.OtpInvalid)); + expect(result).toMatchObject({ + type: AuthTokens.Access, + token: 'access-token', + userState: UserStatesEnum.DocumentsSubmitted, + }); }); }); - describe('signupSender', () => { - const senderDto = { - firstName: 'احمد', - lastName: 'محمدی', - phoneNumber: '+989123456789', - gender: GendersEnum.male, - }; + describe('signupSender / signupTransporter', () => { + it('should delegate signupSender', async () => { + const expected = { sender: mockSender, accessToken: 't' }; + signupService.signupSender.mockResolvedValue(expected as any); - it('should create sender successfully', async () => { - prismaService.$transaction.mockImplementation(async (callback) => - callback(prismaService), - ); - prismaService.user.create.mockResolvedValue(mockUser); - (tokenService['generateAccessToken'] as jest.Mock).mockReturnValue( - 'access-token', - ); + const result = await service.signupSender(signupSenderDto); - const result = await service.signupSender(senderDto); - - expect(result).toEqual({ - sender: mockUser, - accessToken: 'access-token', - }); - expect(prismaService.user.create).toHaveBeenCalledWith({ - data: { - ...senderDto, - wallet: { create: {} }, - role: RolesEnum.sender, - phoneVerifiedAt: expect.any(Date), - }, - }); + expect(signupService.signupSender).toHaveBeenCalledWith(signupSenderDto); + expect(result).toBe(expected); }); - }); - describe('signupTransporter', () => { - const transporterDto = { - firstName: 'علی', - lastName: 'احمدی', - phoneNumber: '+989123456789', - gender: GendersEnum.male, - birthDate: new Date('1985-01-01'), - nationalId: '1234567890', - licenseNumber: '0987654321', - licenseType: LicenseTypeEnum.grade_one, - licenseExpiryDate: new Date('2025-12-31'), - }; + it('should delegate signupTransporter', async () => { + const expected = { transporter: {}, progressToken: 't' }; + signupService.signupTransporter.mockResolvedValue(expected as any); - it('should create transporter successfully', async () => { - prismaService.$transaction.mockImplementation(async (callback) => - callback(prismaService), - ); - const createdTransporter = { - ...mockUser, - role: RolesEnum.transporter, - transporter: mockTransporter, - }; - prismaService.user.create.mockResolvedValue(createdTransporter); - (tokenService['generateProgressToken'] as jest.Mock).mockReturnValue( - 'progress-token', - ); + const result = await service.signupTransporter(signupTransporterDto); - const result = await service.signupTransporter(transporterDto); - - expect(result.progressToken).toBe('progress-token'); - expect(prismaService.user.create).toHaveBeenCalledWith({ - data: expect.objectContaining({ - role: RolesEnum.transporter, - transporter: { - create: expect.objectContaining({ - nationalId: '1234567890', - licenseNumber: '0987654321', - }), - }, - }), - include: { transporter: true }, - }); + expect(signupService.signupTransporter).toHaveBeenCalledWith( + signupTransporterDto, + ); + expect(result).toBe(expected); }); }); describe('submitDocuments', () => { - it('should submit documents successfully', async () => { - const documentsDto = { - nationalIdDocumentKey: 'national-id-key', - licenseDocumentKey: 'license-key', - registrationCardKey: 'reg-card-key', - insuranceDocumentKey: 'insurance-key', - technicalInspectionKey: 'tech-key', - greenSheetKey: 'green-sheet-key', - cardKey: 'card-key', - vehiclePicsKey: ['vehicle-pics-key'], - }; - - prismaService.$transaction.mockImplementation(async (callback) => - callback(prismaService), - ); - userService.getTransporter.mockResolvedValue(mockTransporter); - (tokenService['generateAccessToken'] as jest.Mock).mockReturnValue( - 'access-token', + const body = { + nationalIdDocumentKey: 'transporter/user-456/national-id-1', + licenseDocumentKey: 'transporter/user-456/license-1', + greenSheetKey: 'transporter/user-456/vehicle/green-sheet-1', + cardKey: 'transporter/user-456/vehicle/card-1', + vehiclePicsKey: ['transporter/user-456/vehicle/pic-1'], + }; + + it('should update both transporter and vehicle in one transaction and issue an access token', async () => { + prismaService.$transaction.mockImplementation( + async (callback) => callback(prismaService), ); + userService.getTransporter.mockResolvedValue({ + vehicles: [{ id: 'vehicle-1' }], + } as any); + tokenService.generateAccessToken.mockReturnValue('access-token'); const result = await service.submitDocuments( - 'user-123', - '+989123456789', - documentsDto, + 'user-456', + '+989876543210', + body, ); - expect(result.accessToken).toBe('access-token'); - expect(userService.updateTransporter).toHaveBeenCalled(); + expect(prismaService.$transaction).toHaveBeenCalled(); + expect(userService.updateTransporter).toHaveBeenCalledWith( + 'user-456', + { + nationalIdDocumentKey: 'transporter/user-456/national-id-1', + licenseDocumentKey: 'transporter/user-456/license-1', + }, + prismaService, + ); + expect(userService.getTransporter).toHaveBeenCalledWith( + { userId: 'user-456' }, + prismaService, + ); expect(vehicleService.update).toHaveBeenCalledWith( - 'vehicle-123', - expect.any(Object), + 'vehicle-1', + { + verificationDocuments: { + greenSheetKey: 'transporter/user-456/vehicle/green-sheet-1', + cardKey: 'transporter/user-456/vehicle/card-1', + vehiclePicsKey: ['transporter/user-456/vehicle/pic-1'], + }, + }, prismaService, ); + expect(tokenService.generateAccessToken).toHaveBeenCalledWith({ + sub: 'user-456', + phoneNumber: '+989876543210', + }); + expect(result).toEqual({ accessToken: 'access-token' }); }); - }); - describe('getUserState', () => { - it('should return cached authenticated state', async () => { - const session = { userState: UserStatesEnum.Authenticated } as any; + it('should propagate transaction failures (rollback handled by Prisma)', async () => { + const error = new Error('update failed'); + prismaService.$transaction.mockRejectedValue(error); - const result = await service.getUserState(session); - - expect(result).toEqual({ userState: UserStatesEnum.Authenticated }); + await expect( + service.submitDocuments('user-456', '+989876543210', body), + ).rejects.toThrow('update failed'); }); + }); - it('should compute initial state for sender', async () => { - const session = { userId: 'user-123' } as any; - userService.get.mockResolvedValue(mockUser); + describe('registerVehicle', () => { + it('should create the vehicle and set the session state', async () => { + vehicleService.create.mockResolvedValue(mockVehicle); + const session = { userState: undefined } as any; - const result = await service.getUserState(session); + const result = await service.registerVehicle( + 'user-456', + {} as any, + session, + ); - expect(result).toEqual({ - userState: UserStatesEnum.Authenticated, - role: RolesEnum.sender, - }); - expect(session.userState).toBe(UserStatesEnum.Authenticated); + expect(vehicleService.create).toHaveBeenCalledWith('user-456', {}); + expect(result).toBe(mockVehicle); + expect(session.userState).toBe(UserStatesEnum.VehicleInfoSubmitted); }); + }); - it('should compute state for transporter', async () => { - const session = { userId: 'trans-123' } as any; - userService.get.mockResolvedValue({ - ...mockUser, - role: RolesEnum.transporter, - }); - userService.getTransporter.mockResolvedValue(mockTransporterInProgress); + describe('getUserState', () => { + const session = (overrides: Record = {}) => + ({ + userId: 'user-123', + phoneNumber: '+989123456789', + userState: undefined, + ...overrides, + }) as any; - const result = await service.getUserState(session); + it('should return the cached Authenticated state without DB access', async () => { + const result = await service.getUserState( + session({ userState: UserStatesEnum.Authenticated }), + ); - expect(result?.userState).toBe(UserStatesEnum.PersonalInfoSubmitted); - expect(result?.role).toBe(RolesEnum.transporter); + expect(result).toEqual({ userState: UserStatesEnum.Authenticated }); + expect(userService.get).not.toHaveBeenCalled(); }); - it('should throw when user not found', async () => { - const session = { userId: 'missing-user' } as any; + it('should throw NotFound when the user is missing for a cached non-authenticated state', async () => { userService.get.mockResolvedValue(null); - await expect(service.getUserState(session)).rejects.toThrow( - new NotFoundException(NotFoundMessages.User), - ); + await expect( + service.getUserState( + session({ userState: UserStatesEnum.PersonalInfoSubmitted }), + ), + ).rejects.toThrow(NotFoundException); + await expect( + service.getUserState( + session({ userState: UserStatesEnum.PersonalInfoSubmitted }), + ), + ).rejects.toThrow(NotFoundMessages.User); }); - }); - - describe('computeTransporterState', () => { - it('should return PersonalInfoSubmitted when no vehicles', async () => { - userService.getTransporter.mockResolvedValue(mockTransporterInProgress); - const result = await service['computeTransporterState']('trans-123'); + it('should return transporter data for a cached state of a transporter', async () => { + userService.get.mockResolvedValue(mockTransporter); + userService.getTransporter.mockResolvedValue(mockTransporter as any); - expect(result.userState).toBe(UserStatesEnum.PersonalInfoSubmitted); - }); + const result = await service.getUserState( + session({ + userId: 'user-456', + userState: UserStatesEnum.PersonalInfoSubmitted, + }), + ); - it('should return VehicleInfoSubmitted when vehicle exists', async () => { - userService.getTransporter.mockResolvedValue({ - ...mockTransporter, - vehicles: [{ verificationDocuments: null }], - nationalIdDocumentKey: null, - licenseDocumentKey: null, + expect(result).toMatchObject({ + userState: UserStatesEnum.PersonalInfoSubmitted, + transporter: { id: 'user-456', role: RolesEnum.transporter }, }); - - const result = await service['computeTransporterState']('trans-123'); - - expect(result.userState).toBe(UserStatesEnum.VehicleInfoSubmitted); }); - it('should return DocumentsSubmitted when all docs exist', async () => { - userService.getTransporter.mockResolvedValue({ - ...mockTransporter, - vehicles: [{ verificationDocuments: { some: 'data' } }], - nationalIdDocumentKey: 'national-key', - licenseDocumentKey: 'license-key', - }); + it('should return null for a cached non-authenticated state of a sender', async () => { + userService.get.mockResolvedValue(mockSender); - const result = await service['computeTransporterState']('trans-123'); + const result = await service.getUserState( + session({ userState: UserStatesEnum.PersonalInfoSubmitted }), + ); - expect(result.userState).toBe(UserStatesEnum.DocumentsSubmitted); + expect(result).toBeNull(); }); - it('should return Authenticated when verified', async () => { - userService.getTransporter.mockResolvedValue({ - ...mockTransporter, - verificationStatus: { status: VerificationStatusEnum.verified }, - }); - - const result = await service['computeTransporterState']('trans-123'); + it('should throw NotFound when the user is missing with no cached state', async () => { + userService.get.mockResolvedValue(null); - expect(result.userState).toBe(UserStatesEnum.Authenticated); + await expect(service.getUserState(session())).rejects.toThrow( + NotFoundMessages.User, + ); }); - }); - - describe('Error scenarios', () => { - it('should handle missing OTP', async () => { - keyvMock.get.mockResolvedValue({ - attempts: { sendAttempts: 0, checkAttempts: 0, lastSendAttempt: 0 }, - } as any); - await expect( - service.checkOtp({ phoneNumber: '+989123456789', code: 12345 }), - ).rejects.toThrow(new UnauthorizedException(AuthMessages.OtpExpired)); - }); + it('should compute and persist Authenticated for a sender with no cached state', async () => { + userService.get.mockResolvedValue(mockSender); + const ses = session(); - it('should block after max check attempts', async () => { - const userData = { - otp: { code: 54321, expiresIn: Date.now() + 60000 }, - attempts: { - sendAttempts: 1, - checkAttempts: 10, - lastSendAttempt: Date.now() - 60000, - }, - }; - keyvMock.get.mockResolvedValue(userData as any); - keyvMock.set.mockResolvedValue(true); + const result = await service.getUserState(ses); - await expect( - service.checkOtp({ phoneNumber: '+989123456789', code: 12345 }), - ).rejects.toThrow(TooManyRequestsException); + expect(result).toEqual({ + userState: UserStatesEnum.Authenticated, + role: RolesEnum.sender, + }); + expect(ses.userState).toBe(UserStatesEnum.Authenticated); }); - it('should handle Prisma errors', async () => { - const error = new Error('DB error'); - prismaService.$transaction.mockImplementation(async (callback) => - callback(prismaService), - ); - prismaService.user.create.mockRejectedValue(error); - (utilities.formatPrismaError as unknown as jest.Mock).mockImplementation( - () => { - throw new Error('Formatted error'); - }, - ); + it('should compute a state for a transporter and persist it in the session', async () => { + userService.get.mockResolvedValue(mockTransporter); + userService.getTransporter.mockResolvedValue(mockTransporter as any); + stateMachine.compute.mockReturnValue({ + userState: UserStatesEnum.PersonalInfoSubmitted, + transporter: mockTransporter, + }); + const ses = session({ userId: 'user-456' }); - await expect( - service.signupSender({ - firstName: 'احمد', - lastName: 'محمدی', - phoneNumber: '+989123456789', - gender: GendersEnum.male, - }), - ).rejects.toThrow('Formatted error'); + const result = await service.getUserState(ses); + + expect(result).toMatchObject({ + userState: UserStatesEnum.PersonalInfoSubmitted, + role: RolesEnum.transporter, + }); + expect((result as any).transporter).toBeDefined(); + expect(ses.userState).toBe(UserStatesEnum.PersonalInfoSubmitted); }); }); -}); +}); \ No newline at end of file diff --git a/src/modules/auth/auth.service.ts b/src/modules/auth/auth.service.ts index 2bf1379..64a6d7c 100644 --- a/src/modules/auth/auth.service.ts +++ b/src/modules/auth/auth.service.ts @@ -1,140 +1,49 @@ -import { - Inject, - Injectable, - NotFoundException, - UnauthorizedException, -} from '@nestjs/common'; +import { Injectable, NotFoundException } from '@nestjs/common'; import { SendOtpDto } from './dto/send-otp.dto'; -import { CACHE_MANAGER } from '@nestjs/cache-manager'; -import { Cache } from 'cache-manager'; -import { Keyv } from '@keyv/redis'; import { CheckOtpDto } from './dto/check-otp.dto'; -import { ConfigService } from '@nestjs/config'; -import { - AuthMessages, - NotFoundMessages, -} from '../../common/enums/messages.enum'; +import { NotFoundMessages } from '../../common/enums/messages.enum'; import { TokenService } from '../token/token.service'; import { UserService } from '../user/user.service'; import { AuthTokens } from '../../common/enums/auth.enum'; import { PrismaService } from '../prisma/prisma.service'; import { SignupSenderDto } from './dto/signup-sender.dto'; -import { formatPrismaError, generateCode } from '../../common/utilities'; -import { TooManyRequestsException } from '../../common/custom.exceptions'; -import { - CachedUserData, - CheckOtpResult, - UserAttempts, -} from './types/auth.types'; +import { CheckOtpResult } from './types/auth.types'; import { SignupTransporterDto } from './dto/signup-transporter.dto'; -import { RolesEnum, VerificationStatusEnum } from '../../../generated/prisma'; +import { RolesEnum } from '../../../generated/prisma'; import { VehicleService } from '../vehicle/vehicle.service'; +import { CreateVehicleDto } from '../vehicle/dto/create-vehicle.dto'; import { SubmitDocumentsDto } from './dto/submit-documents.dto'; import { SessionData } from 'express-session'; import { UserStatesEnum } from './types/auth.enums'; import { TransporterResponseDto } from '../user/dto/transporter-response.dto'; -import { SmsService } from '../sms/sms.service'; -import { NotificationService } from '../notification/notification.service'; -import { NotificationMessages } from '../notification/notification-messages'; +import { OtpService } from './domain/otp.service'; +import { AuthStateMachine } from './domain/auth-state.machine'; +import { TransporterSignupService } from './application/transporter-signup.service'; +import { setUserState } from './domain/session-utils'; @Injectable() export class AuthService { - private cacheManager: Keyv; - private readonly otpExpireTime: number; - private readonly maxSendAttempts: number; - private readonly maxCheckAttempts: number; - private readonly sendWindow: number; - private readonly baseBlockTime: number; - constructor( - private tokenService: TokenService, - private userService: UserService, - private vehicleService: VehicleService, - private prisma: PrismaService, - @Inject(CACHE_MANAGER) cacheManager: Cache, - private smsService: SmsService, - private notificationService: NotificationService, - config: ConfigService, - ) { - this.cacheManager = cacheManager.stores[1]; - - this.otpExpireTime = config.get( - 'OTP_EXPIRATION_TIME', - 2 * 60 * 1000, - ); - this.maxSendAttempts = config.get('MAX_SEND_ATTEMPTS ', 5); - this.maxCheckAttempts = config.get('MAX_CHECK_ATTEMPTS', 10); - this.sendWindow = config.get('SEND_WINDOW', 30 * 60 * 1000); - this.baseBlockTime = config.get('BASE_BLOCK_TIME', 20 * 60 * 1000); - } + private readonly tokenService: TokenService, + private readonly userService: UserService, + private readonly vehicleService: VehicleService, + private readonly prisma: PrismaService, + private readonly otpService: OtpService, + private readonly signupService: TransporterSignupService, + private readonly stateMachine: AuthStateMachine, + ) {} async sendOtp({ phoneNumber }: SendOtpDto): Promise { - const userKey = this.getUserKey(phoneNumber); - const userData = await this.getUserData(userKey); - - this.checkIfBlocked(userData.attempts); - this.checkSendAttempts(userData.attempts); - - const now = Date.now(); - if (userData.otp && now < userData.otp.expiresIn) { - throw new UnauthorizedException(AuthMessages.OtpNotExpired); - } - - const otp = { - code: generateCode(), - expiresIn: now + this.otpExpireTime, - createdAt: now, - }; - - userData.attempts.sendAttempts++; - userData.attempts.checkAttempts = 0; - userData.attempts.lastSendAttempt = now; - userData.otp = otp; - - const otpSmsResult = await this.smsService.sendOtp(phoneNumber, otp.code); - return otpSmsResult && (await this.setUserData(userKey, userData)); + return this.otpService.sendOtp(phoneNumber); } async checkOtp({ phoneNumber, code }: CheckOtpDto): Promise { - const userKey = this.getUserKey(phoneNumber); - const userData = await this.getUserData(userKey); - - this.checkIfBlocked(userData.attempts); - - if (!userData?.otp) { - throw new UnauthorizedException(AuthMessages.OtpExpired); - } - - const now = Date.now(); - if (now > userData.otp.expiresIn) { - throw new UnauthorizedException(AuthMessages.OtpExpired); - } - - userData.attempts.checkAttempts++; - - if (userData.attempts.checkAttempts > this.maxCheckAttempts) { - // Block user and extend cache time - userData.attempts.blockedUntil = - now + this.calculateBlockTime(userData.attempts); - await this.setUserData(userKey, userData); - throw new TooManyRequestsException(AuthMessages.TooManyAttempts); - } - - if (userData.otp.code !== code) { - await this.setUserData(userKey, userData); - throw new UnauthorizedException(AuthMessages.OtpInvalid); - } - - // OTP is valid - clear the OTP data but keep attempts history - userData.otp = undefined; - await this.setUserData(userKey, userData); + await this.otpService.verify(phoneNumber, code); const user = await this.userService.getByPhoneNumber(phoneNumber); if (!user) { - const payload = { phoneNumber }; - const token = this.tokenService['generateTempToken'](payload); - + const token = this.tokenService.generateTempToken({ phoneNumber }); return { isNewUser: true, token, @@ -142,18 +51,13 @@ export class AuthService { }; } - // Handle existing user - const payload = { - sub: user.id, - phoneNumber: user.phoneNumber, - }; + const payload = { sub: user.id, phoneNumber: user.phoneNumber }; - // Base result const result: CheckOtpResult = { isNewUser: false, userId: user.id, role: user.role, - token: this.tokenService['generateAccessToken'](payload), + token: this.tokenService.generateAccessToken(payload), type: AuthTokens.Access, }; @@ -166,13 +70,10 @@ export class AuthService { UserStatesEnum.VehicleInfoSubmitted, ]; if (progressStates.includes(transporterState.userState)) { - result.token = this.tokenService['generateProgressToken'](payload); + result.token = this.tokenService.generateProgressToken(payload); result.type = AuthTokens.Progress; result.transporter = transporterState.transporter as TransporterResponseDto; - } else { - result.token = this.tokenService['generateAccessToken'](payload); - result.type = AuthTokens.Access; } } else if (user.role === RolesEnum.sender) { result.userState = UserStatesEnum.Authenticated; @@ -181,192 +82,12 @@ export class AuthService { return result; } - private getUserKey( - phoneNumber: string, - type: 'mobile' | 'email' = 'mobile', - ): string { - return `otp:${type}:${phoneNumber}`; - } - - private async getUserData(userKey: string): Promise { - const userData = await this.cacheManager.get(userKey); - if (!userData) { - return { - attempts: { - sendAttempts: 0, - checkAttempts: 0, - lastSendAttempt: 0, - }, - }; - } - - // Reset attempts if pass the window - const now = Date.now(); - if (now - userData.attempts.lastSendAttempt > this.sendWindow) { - userData.attempts.sendAttempts = 0; - } - - return userData; - } - - private async setUserData( - userKey: string, - userData: CachedUserData, - ): Promise { - const cacheTime = this.calculateCacheTime(userData.attempts); - return this.cacheManager.set(userKey, userData, cacheTime); - } - - private checkIfBlocked(attempts: UserAttempts): void { - const now = Date.now(); - if (attempts.blockedUntil && now < attempts.blockedUntil) { - const remainingTime = Math.ceil( - (attempts.blockedUntil - now) / 1000 / 60, - ); // minutes - throw new TooManyRequestsException( - `Too many attempts. Please try again in ${remainingTime} minutes.`, - ); - } - } - - private checkSendAttempts(attempts: UserAttempts): void { - if (attempts.sendAttempts >= this.maxSendAttempts) { - const blockTime = this.calculateBlockTime(attempts); - attempts.blockedUntil = Date.now() + blockTime; - throw new TooManyRequestsException(AuthMessages.MaxAttempts); - } - } - - private calculateBlockTime(attempts: UserAttempts): number { - // Progressive blocking: each violation increases block time - // Violations => 0-2 - const violations = - Math.floor(attempts.sendAttempts / this.maxSendAttempts) + - Math.floor(attempts.checkAttempts / this.maxCheckAttempts); - if (!violations) return 0; - return this.baseBlockTime * Math.pow(2, violations); - } - - private calculateCacheTime(attempts: UserAttempts): number { - if (attempts.blockedUntil) { - return Math.max( - attempts.blockedUntil - Date.now() + 60_000, - this.sendWindow, - ); - } - return this.sendWindow; - } - - async signupSender(senderDto: SignupSenderDto) { - return this.prisma - .$transaction(async (tx) => { - const sender = await tx.user.create({ - data: { - ...senderDto, - wallet: { - create: {}, - }, - role: RolesEnum.sender, - phoneVerifiedAt: new Date(), - }, - }); - - const payload = { - sub: sender.id, - phoneNumber: sender.phoneNumber, - }; - const accessToken = this.tokenService['generateAccessToken'](payload); - - // Add welcome notification - await this.notificationService.create( - sender.id, - { - content: NotificationMessages.Welcome, - }, - tx, - ); - - return { - sender, - accessToken, - }; - }) - .catch((error: Error) => { - formatPrismaError(error); - throw error; - }); + signupSender(senderDto: SignupSenderDto) { + return this.signupService.signupSender(senderDto); } - async signupTransporter({ - nationalId, - licenseNumber, - licenseExpiryDate, - licenseType, - profilePictureKey, - ...transporterDto - }: SignupTransporterDto) { - return this.prisma - .$transaction(async (tx) => { - const transporter = await tx.user.create({ - data: { - ...transporterDto, - wallet: { - create: {}, - }, - role: RolesEnum.transporter, - transporter: { - create: { - nationalId, - licenseNumber, - licenseExpiryDate, - licenseType, - profilePictureKey, - nationalIdStatus: { - create: {}, - }, - licenseStatus: { - create: {}, - }, - verificationStatus: { - create: {}, - }, - }, - }, - }, - include: { - transporter: true, - }, - }); - - const payload = { - sub: transporter.id, - phoneNumber: transporter.phoneNumber, - }; - const progressToken = - this.tokenService['generateProgressToken'](payload); - - // Add welcome notification - await this.notificationService.create( - transporter.id, - { - content: NotificationMessages.Welcome, - }, - tx, - ); - - return { - transporter: { - ...transporter, - ...transporter.transporter, - transporter: undefined, - }, - progressToken, - }; - }) - .catch((error: Error) => { - formatPrismaError(error); - throw error; - }); + signupTransporter(transporterDto: SignupTransporterDto) { + return this.signupService.signupTransporter(transporterDto); } async submitDocuments( @@ -399,21 +120,26 @@ export class AuthService { ); }); - // Generate access token - const payload = { + const accessToken = this.tokenService.generateAccessToken({ sub: userId, phoneNumber, - }; - const accessToken = this.tokenService['generateAccessToken'](payload); + }); - return { - accessToken, - }; + return { accessToken }; + } + + async registerVehicle( + userId: string, + vehicleDto: CreateVehicleDto, + session: SessionData, + ) { + const vehicle = await this.vehicleService.create(userId, vehicleDto); + setUserState(session, UserStatesEnum.VehicleInfoSubmitted); + return vehicle; } async getUserState(session: SessionData) { if (session.userState) { - // For authenticated users, just return state if (session.userState === UserStatesEnum.Authenticated) { return { userState: session.userState }; } @@ -423,7 +149,6 @@ export class AuthService { throw new NotFoundException(NotFoundMessages.User); } - // For transporters in progress, return complete transporter data if (user.role === RolesEnum.transporter) { const transporter = await this.userService.getTransporter({ userId: session.userId, @@ -437,7 +162,6 @@ export class AuthService { }; } - // For other cases, return null return null; } @@ -451,15 +175,13 @@ export class AuthService { let transporter: TransporterResponseDto | undefined; if (user.role === RolesEnum.transporter) { - const transporterState = await this.computeTransporterState( - session.userId!, - ); - computedState = transporterState.userState; - transporter = transporterState.transporter as TransporterResponseDto; + const { userState, transporter: transporterData } = + await this.computeTransporterState(session.userId!); + computedState = userState; + transporter = transporterData as TransporterResponseDto; } - // Update session with computed state - session.userState = computedState; + setUserState(session, computedState); return computedState === UserStatesEnum.Authenticated ? { userState: computedState, role: user.role } @@ -475,31 +197,6 @@ export class AuthService { private async computeTransporterState(userId: string) { const transporter = await this.userService.getTransporter({ userId }); - let state: UserStatesEnum | undefined; - - // No vehicles submitted yet / submitted - if (transporter.vehicles.length === 0) { - state = UserStatesEnum.PersonalInfoSubmitted; - } else { - state = UserStatesEnum.VehicleInfoSubmitted; - } - - const hasAllDocuments = - transporter.licenseDocumentKey && - transporter.nationalIdDocumentKey && - transporter.vehicles[0].verificationDocuments; - - if (hasAllDocuments) { - state = UserStatesEnum.DocumentsSubmitted; - } - - // Transporter verified or not - const isVerified = - transporter.verificationStatus?.status === - VerificationStatusEnum.verified; - - return isVerified - ? { userState: UserStatesEnum.Authenticated } - : { userState: state, transporter }; + return this.stateMachine.compute(transporter); } } diff --git a/src/modules/auth/dto/check-otp.dto.ts b/src/modules/auth/dto/check-otp.dto.ts index 32727af..4443758 100644 --- a/src/modules/auth/dto/check-otp.dto.ts +++ b/src/modules/auth/dto/check-otp.dto.ts @@ -1,10 +1,9 @@ import { - IsInt, IsNotEmpty, IsPhoneNumber, IsString, - Max, - Min, + Matches, + Length, } from 'class-validator'; import { Expose } from 'class-transformer'; import { StateDto } from './state-response.dto'; @@ -16,10 +15,10 @@ export class CheckOtpDto { phoneNumber: string; @IsNotEmpty() - @Max(99999) - @Min(10000) - @IsInt() - code: number; + @IsString() + @Length(6, 6) + @Matches(/^\d{6}$/, { message: 'code must be a 6-digit number' }) + code: string; } export class CheckOtpResponseDto extends StateDto { diff --git a/src/modules/auth/types/auth.types.ts b/src/modules/auth/types/auth.types.ts index 4b0527f..e2d5ed9 100644 --- a/src/modules/auth/types/auth.types.ts +++ b/src/modules/auth/types/auth.types.ts @@ -4,7 +4,7 @@ import { RolesEnum } from 'generated/prisma'; import { TransporterCompactDto } from 'src/modules/user/dto/transporter-response.dto'; export interface OtpData { - code: number; + code: string; expiresIn: number; createdAt: number; } diff --git a/src/modules/sms/sms.service.ts b/src/modules/sms/sms.service.ts index 523965b..13f71ea 100644 --- a/src/modules/sms/sms.service.ts +++ b/src/modules/sms/sms.service.ts @@ -43,13 +43,7 @@ export class SmsService { } } - async sendOtp(mobile: string, code: number) { - // const otpMessage = ` - // کد تأیید هم‌بار: ${code} - // این کد برای ورود به حساب کاربری شماست. لطفاً آن را با کسی به اشتراک نگذارید. - // `; - // return this.sendSms([mobile], otpMessage); - + async sendOtp(mobile: string, code: string) { try { const url = 'https://s.api.ir/api/sw1/SmsOTP'; @@ -59,7 +53,7 @@ export class SmsService { url, { mobile, - code: String(code), + code, template: 2, }, {