diff --git a/apps/api/src/modules/auth/infrastructure/__tests__/local.strategy.spec.ts b/apps/api/src/modules/auth/infrastructure/__tests__/local.strategy.spec.ts index 5f44c32..24b6245 100644 --- a/apps/api/src/modules/auth/infrastructure/__tests__/local.strategy.spec.ts +++ b/apps/api/src/modules/auth/infrastructure/__tests__/local.strategy.spec.ts @@ -18,16 +18,36 @@ vi.mock('@nestjs/passport', () => { }; }); +vi.mock('@nestjs/common', async (importOriginal) => { + const actual = (await importOriginal()) as any; + return { + ...actual, + Logger: class MockLogger { + error = vi.fn(); + warn = vi.fn(); + log = vi.fn(); + }, + }; +}); + vi.mock('@modules/shared', () => { - class UnauthorizedException extends Error { - constructor(message: string) { + class DomainException extends Error { + constructor(public errorCode: string, message: string) { super(message); + this.name = 'DomainException'; + } + } + class UnauthorizedException extends DomainException { + constructor(message: string) { + super('UNAUTHORIZED', message); this.name = 'UnauthorizedException'; } } return { + DomainException, UnauthorizedException, normalizeVietnamPhone: (phone: string) => { + if (!phone) return null; if (phone.startsWith('+84') && phone.length === 12) return phone; if (phone.startsWith('0') && phone.length === 10) return '+84' + phone.slice(1); return null; @@ -121,4 +141,44 @@ describe('LocalStrategy', () => { role: 'BUYER', }); }); + + it('throws UnauthorizedException when phone is undefined (empty body)', async () => { + await expect(strategy.validate(undefined as any, undefined as any)).rejects.toThrow( + 'Số điện thoại hoặc mật khẩu không đúng', + ); + }); + + it('throws UnauthorizedException when phone is null', async () => { + await expect(strategy.validate(null as any, 'password')).rejects.toThrow( + 'Số điện thoại hoặc mật khẩu không đúng', + ); + }); + + it('throws UnauthorizedException when password is empty string', async () => { + await expect(strategy.validate('0912345678', '')).rejects.toThrow( + 'Số điện thoại hoặc mật khẩu không đúng', + ); + }); + + it('wraps unexpected DB errors as UnauthorizedException', async () => { + mockUserRepo.findByPhone.mockRejectedValue(new Error('DB connection refused')); + + await expect(strategy.validate('0912345678', 'password')).rejects.toThrow( + 'Số điện thoại hoặc mật khẩu không đúng', + ); + }); + + it('wraps unexpected bcrypt errors as UnauthorizedException', async () => { + mockUserRepo.findByPhone.mockResolvedValue({ + id: 'user-1', + passwordHash: { compare: vi.fn().mockRejectedValue(new Error('bcrypt internal error')) }, + isActive: true, + phone: { value: '+84912345678' }, + role: 'BUYER', + }); + + await expect(strategy.validate('0912345678', 'password')).rejects.toThrow( + 'Số điện thoại hoặc mật khẩu không đúng', + ); + }); }); diff --git a/apps/api/src/modules/auth/infrastructure/strategies/local.strategy.ts b/apps/api/src/modules/auth/infrastructure/strategies/local.strategy.ts index 6f16191..9aa3ab9 100644 --- a/apps/api/src/modules/auth/infrastructure/strategies/local.strategy.ts +++ b/apps/api/src/modules/auth/infrastructure/strategies/local.strategy.ts @@ -1,11 +1,13 @@ -import { Inject, Injectable } from '@nestjs/common'; +import { Inject, Injectable, Logger } from '@nestjs/common'; import { PassportStrategy } from '@nestjs/passport'; import { Strategy } from 'passport-local'; -import { normalizeVietnamPhone, UnauthorizedException } from '@modules/shared'; +import { DomainException, normalizeVietnamPhone, UnauthorizedException } from '@modules/shared'; import { USER_REPOSITORY, type IUserRepository } from '../../domain/repositories/user.repository'; @Injectable() export class LocalStrategy extends PassportStrategy(Strategy) { + private readonly logger = new Logger(LocalStrategy.name); + constructor( @Inject(USER_REPOSITORY) private readonly userRepo: IUserRepository, @@ -14,25 +16,38 @@ export class LocalStrategy extends PassportStrategy(Strategy) { } async validate(phone: string, password: string): Promise<{ id: string; phone: string; role: string }> { - const normalizedPhone = normalizeVietnamPhone(phone); - if (!normalizedPhone) { - throw new UnauthorizedException('Số điện thoại không hợp lệ'); - } - const user = await this.userRepo.findByPhone(normalizedPhone); + try { + if (!phone || !password) { + throw new UnauthorizedException('Số điện thoại hoặc mật khẩu không đúng'); + } - if (!user || !user.passwordHash) { + const normalizedPhone = normalizeVietnamPhone(phone); + if (!normalizedPhone) { + throw new UnauthorizedException('Số điện thoại không hợp lệ'); + } + const user = await this.userRepo.findByPhone(normalizedPhone); + + if (!user || !user.passwordHash) { + throw new UnauthorizedException('Số điện thoại hoặc mật khẩu không đúng'); + } + + if (!user.isActive) { + throw new UnauthorizedException('Tài khoản đã bị vô hiệu hóa'); + } + + const isValid = await user.passwordHash.compare(password); + if (!isValid) { + throw new UnauthorizedException('Số điện thoại hoặc mật khẩu không đúng'); + } + + return { id: user.id, phone: user.phone.value, role: user.role }; + } catch (error) { + if (error instanceof DomainException) throw error; + this.logger.error( + `Authentication failed: ${error instanceof Error ? error.message : error}`, + error instanceof Error ? error.stack : undefined, + ); throw new UnauthorizedException('Số điện thoại hoặc mật khẩu không đúng'); } - - if (!user.isActive) { - throw new UnauthorizedException('Tài khoản đã bị vô hiệu hóa'); - } - - const isValid = await user.passwordHash.compare(password); - if (!isValid) { - throw new UnauthorizedException('Số điện thoại hoặc mật khẩu không đúng'); - } - - return { id: user.id, phone: user.phone.value, role: user.role }; } } diff --git a/apps/api/src/modules/auth/presentation/__tests__/local-auth.guard.spec.ts b/apps/api/src/modules/auth/presentation/__tests__/local-auth.guard.spec.ts index e71353c..74221ef 100644 --- a/apps/api/src/modules/auth/presentation/__tests__/local-auth.guard.spec.ts +++ b/apps/api/src/modules/auth/presentation/__tests__/local-auth.guard.spec.ts @@ -1,6 +1,30 @@ -import { describe, it, expect } from 'vitest'; +import { HttpException, HttpStatus } from '@nestjs/common'; +import { describe, it, expect, vi, beforeEach } from 'vitest'; import { LocalAuthGuard } from '../guards/local-auth.guard'; +vi.mock('@nestjs/common', async (importOriginal) => { + const actual = (await importOriginal()) as any; + return { + ...actual, + Logger: class MockLogger { + error = vi.fn(); + warn = vi.fn(); + log = vi.fn(); + }, + }; +}); + +vi.mock('@modules/shared', async () => { + const { HttpException: HE, HttpStatus: HS } = await import('@nestjs/common'); + class UnauthorizedException extends HE { + constructor(message: string) { + super(message, HS.UNAUTHORIZED); + this.name = 'UnauthorizedException'; + } + } + return { UnauthorizedException }; +}); + describe('LocalAuthGuard', () => { let guard: LocalAuthGuard; @@ -14,9 +38,24 @@ describe('LocalAuthGuard', () => { expect(result).toEqual(user); }); - it('throws error when error is provided', () => { - const error = new Error('Strategy error'); - expect(() => guard.handleRequest(error, null, undefined, {} as any)).toThrow('Strategy error'); + it('re-throws HttpException errors directly', () => { + const httpError = new HttpException('Bad Request', HttpStatus.BAD_REQUEST); + expect(() => guard.handleRequest(httpError, null, undefined, {} as any)).toThrow(HttpException); + expect(() => guard.handleRequest(httpError, null, undefined, {} as any)).toThrow('Bad Request'); + }); + + it('wraps non-HttpException errors as UnauthorizedException', () => { + const dbError = new Error('DB connection refused'); + expect(() => guard.handleRequest(dbError, null, undefined, {} as any)).toThrow( + 'Số điện thoại hoặc mật khẩu không đúng', + ); + }); + + it('wraps TypeError (e.g. from null/undefined) as UnauthorizedException', () => { + const typeError = new TypeError("Cannot read properties of undefined (reading 'replace')"); + expect(() => guard.handleRequest(typeError, null, undefined, {} as any)).toThrow( + 'Số điện thoại hoặc mật khẩu không đúng', + ); }); it('throws UnauthorizedException when no user and no error', () => { @@ -24,9 +63,4 @@ describe('LocalAuthGuard', () => { 'Số điện thoại hoặc mật khẩu không đúng', ); }); - - it('re-throws the original error type', () => { - const customError = new TypeError('Custom type error'); - expect(() => guard.handleRequest(customError, null, undefined, {} as any)).toThrow(TypeError); - }); }); diff --git a/apps/api/src/modules/auth/presentation/dto/login.dto.ts b/apps/api/src/modules/auth/presentation/dto/login.dto.ts index 627d187..8d21870 100644 --- a/apps/api/src/modules/auth/presentation/dto/login.dto.ts +++ b/apps/api/src/modules/auth/presentation/dto/login.dto.ts @@ -1,12 +1,14 @@ import { ApiProperty } from '@nestjs/swagger'; -import { IsString } from 'class-validator'; +import { IsNotEmpty, IsString } from 'class-validator'; export class LoginDto { @ApiProperty({ example: '0901234567' }) @IsString() + @IsNotEmpty({ message: 'Số điện thoại không được để trống' }) phone!: string; @ApiProperty({ example: 'P@ssw0rd!' }) @IsString() + @IsNotEmpty({ message: 'Mật khẩu không được để trống' }) password!: string; } diff --git a/apps/api/src/modules/auth/presentation/guards/local-auth.guard.ts b/apps/api/src/modules/auth/presentation/guards/local-auth.guard.ts index 1141dae..9b1bee7 100644 --- a/apps/api/src/modules/auth/presentation/guards/local-auth.guard.ts +++ b/apps/api/src/modules/auth/presentation/guards/local-auth.guard.ts @@ -1,14 +1,25 @@ -import { type ExecutionContext, Injectable } from '@nestjs/common'; +import { type ExecutionContext, HttpException, Injectable, Logger } from '@nestjs/common'; import { AuthGuard } from '@nestjs/passport'; import { UnauthorizedException } from '@modules/shared'; @Injectable() export class LocalAuthGuard extends AuthGuard('local') { + private readonly logger = new Logger(LocalAuthGuard.name); + override handleRequest(err: Error | null, user: T, _info: unknown, _context: ExecutionContext): T { - // If the strategy threw a DomainException (e.g. our custom UnauthorizedException), - // re-throw it directly so GlobalExceptionFilter produces the structured error format. if (err) { - throw err; + // If the strategy threw a DomainException or HttpException, + // re-throw it directly so GlobalExceptionFilter produces the structured error format. + if (err instanceof HttpException) { + throw err; + } + // Unexpected infrastructure errors (e.g. DB failure, bcrypt crash) — log and + // wrap as 401 to avoid leaking a 500 on the login endpoint. + this.logger.error( + `Unexpected auth error: ${err.message}`, + err.stack, + ); + throw new UnauthorizedException('Số điện thoại hoặc mật khẩu không đúng'); } if (!user) { throw new UnauthorizedException('Số điện thoại hoặc mật khẩu không đúng');