feat(auth): add phoneNumber to profile update with SMS OTP re-verify
TEC-2722 — PATCH /api/v1/auth/profile now accepts phoneNumber alongside fullName, avatarUrl, and email. Phone changes are deferred until the user confirms the SMS OTP via POST /api/v1/auth/profile/verify-phone, mirroring the existing email-change OTP flow. - Add PhoneChangeRequestedEvent + user.phone_change_otp SMS template - Add VerifyPhoneChangeHandler with Redis-backed 10-minute OTP - Re-check phone uniqueness at verify time to catch races - Extend unit tests for UpdateProfileHandler + add VerifyPhoneChangeHandler spec Co-Authored-By: Paperclip <noreply@paperclip.ing>
This commit is contained in:
@@ -191,4 +191,85 @@ describe('UpdateProfileHandler', () => {
|
||||
expect(mockUserRepo.update).toHaveBeenCalledWith(user);
|
||||
expect(mockCache.invalidate).toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('defers phone change via SMS OTP instead of updating directly', async () => {
|
||||
const user = createTestUser();
|
||||
mockUserRepo.findById.mockResolvedValue(user);
|
||||
mockUserRepo.findByPhone.mockResolvedValue(null);
|
||||
mockUserRepo.update.mockResolvedValue(undefined);
|
||||
|
||||
const command = new UpdateProfileCommand(
|
||||
'user-1',
|
||||
undefined,
|
||||
undefined,
|
||||
undefined,
|
||||
'0987654321',
|
||||
);
|
||||
const result = await handler.execute(command);
|
||||
|
||||
// Phone should NOT change yet — deferred pending OTP
|
||||
expect(result.phoneNumber).toBe('+84912345678');
|
||||
expect(result.phoneChangePending).toBe(true);
|
||||
|
||||
expect(mockRedis.set).toHaveBeenCalledWith(
|
||||
'auth:phone_change_otp:user-1',
|
||||
expect.stringContaining('+84987654321'),
|
||||
600,
|
||||
);
|
||||
expect(mockEventBus.publish).toHaveBeenCalledWith(
|
||||
expect.objectContaining({
|
||||
eventName: 'user.phone_change_requested',
|
||||
newPhone: '+84987654321',
|
||||
}),
|
||||
);
|
||||
});
|
||||
|
||||
it('throws ConflictException when new phone is already taken', async () => {
|
||||
const user = createTestUser();
|
||||
const otherUser = createTestUser({ id: 'user-2' });
|
||||
mockUserRepo.findById.mockResolvedValue(user);
|
||||
mockUserRepo.findByPhone.mockResolvedValue(otherUser);
|
||||
|
||||
const command = new UpdateProfileCommand(
|
||||
'user-1',
|
||||
undefined,
|
||||
undefined,
|
||||
undefined,
|
||||
'0987654321',
|
||||
);
|
||||
await expect(handler.execute(command)).rejects.toThrow('Số điện thoại đã được sử dụng');
|
||||
});
|
||||
|
||||
it('skips SMS OTP when phone is unchanged', async () => {
|
||||
const user = createTestUser();
|
||||
mockUserRepo.findById.mockResolvedValue(user);
|
||||
mockUserRepo.update.mockResolvedValue(undefined);
|
||||
|
||||
const command = new UpdateProfileCommand(
|
||||
'user-1',
|
||||
undefined,
|
||||
undefined,
|
||||
undefined,
|
||||
'0912345678',
|
||||
);
|
||||
const result = await handler.execute(command);
|
||||
|
||||
expect(mockRedis.set).not.toHaveBeenCalled();
|
||||
expect(mockEventBus.publish).not.toHaveBeenCalled();
|
||||
expect(result.phoneChangePending).toBeUndefined();
|
||||
});
|
||||
|
||||
it('throws ValidationException for invalid phone format', async () => {
|
||||
const user = createTestUser();
|
||||
mockUserRepo.findById.mockResolvedValue(user);
|
||||
|
||||
const command = new UpdateProfileCommand(
|
||||
'user-1',
|
||||
undefined,
|
||||
undefined,
|
||||
undefined,
|
||||
'not-a-phone',
|
||||
);
|
||||
await expect(handler.execute(command)).rejects.toThrow('Số điện thoại');
|
||||
});
|
||||
});
|
||||
|
||||
@@ -0,0 +1,119 @@
|
||||
import { UserEntity } from '../../domain/entities/user.entity';
|
||||
import { type IUserRepository } from '../../domain/repositories/user.repository';
|
||||
import { type HashedPassword } from '../../domain/value-objects/hashed-password.vo';
|
||||
import { Phone } from '../../domain/value-objects/phone.vo';
|
||||
import { VerifyPhoneChangeCommand } from '../commands/verify-phone-change/verify-phone-change.command';
|
||||
import { VerifyPhoneChangeHandler } from '../commands/verify-phone-change/verify-phone-change.handler';
|
||||
|
||||
function createTestUser(overrides?: Partial<{ id: string; phone: string }>): UserEntity {
|
||||
const phone = Phone.create(overrides?.phone ?? '0912345678').unwrap();
|
||||
const pw = { value: 'hashed' } as HashedPassword;
|
||||
return new UserEntity(overrides?.id ?? 'user-1', {
|
||||
email: null,
|
||||
phone,
|
||||
passwordHash: pw,
|
||||
fullName: 'Nguyen Van A',
|
||||
avatarUrl: null,
|
||||
role: 'BUYER',
|
||||
kycStatus: 'NONE',
|
||||
kycData: null,
|
||||
isActive: true,
|
||||
totpSecret: null,
|
||||
totpEnabled: false,
|
||||
totpBackupCodes: [],
|
||||
totpEnabledAt: null,
|
||||
});
|
||||
}
|
||||
|
||||
describe('VerifyPhoneChangeHandler', () => {
|
||||
let handler: VerifyPhoneChangeHandler;
|
||||
let mockUserRepo: { [K in keyof IUserRepository]: ReturnType<typeof vi.fn> };
|
||||
let mockRedis: { get: ReturnType<typeof vi.fn>; del: ReturnType<typeof vi.fn>; set: ReturnType<typeof vi.fn> };
|
||||
let mockCache: { invalidate: ReturnType<typeof vi.fn> };
|
||||
|
||||
beforeEach(() => {
|
||||
mockUserRepo = {
|
||||
findById: vi.fn(),
|
||||
findByPhone: vi.fn(),
|
||||
findByEmail: vi.fn(),
|
||||
save: vi.fn(),
|
||||
update: vi.fn(),
|
||||
updateMfaSecret: vi.fn(),
|
||||
updateMfaEnabled: vi.fn(),
|
||||
updateMfaDisabled: vi.fn(),
|
||||
updateBackupCodes: vi.fn(),
|
||||
};
|
||||
mockRedis = {
|
||||
get: vi.fn(),
|
||||
del: vi.fn().mockResolvedValue(undefined),
|
||||
set: vi.fn().mockResolvedValue(undefined),
|
||||
};
|
||||
mockCache = { invalidate: vi.fn().mockResolvedValue(undefined) };
|
||||
|
||||
handler = new VerifyPhoneChangeHandler(
|
||||
mockUserRepo as any,
|
||||
mockRedis as any,
|
||||
mockCache as any,
|
||||
{ error: vi.fn() } as any,
|
||||
);
|
||||
});
|
||||
|
||||
it('verifies SMS OTP and updates phone', async () => {
|
||||
const user = createTestUser();
|
||||
const payload = JSON.stringify({ newPhone: '+84987654321', code: '123456' });
|
||||
mockRedis.get.mockResolvedValue(payload);
|
||||
mockUserRepo.findById.mockResolvedValue(user);
|
||||
mockUserRepo.findByPhone.mockResolvedValue(null);
|
||||
mockUserRepo.update.mockResolvedValue(undefined);
|
||||
|
||||
const command = new VerifyPhoneChangeCommand('user-1', '123456');
|
||||
const result = await handler.execute(command);
|
||||
|
||||
expect(result.phoneNumber).toBe('+84987654321');
|
||||
expect(result.id).toBe('user-1');
|
||||
expect(mockRedis.del).toHaveBeenCalledWith('auth:phone_change_otp:user-1');
|
||||
expect(mockUserRepo.update).toHaveBeenCalledWith(user);
|
||||
expect(mockCache.invalidate).toHaveBeenCalledWith(
|
||||
expect.stringContaining('user-1'),
|
||||
);
|
||||
});
|
||||
|
||||
it('throws ValidationException when OTP has expired', async () => {
|
||||
mockRedis.get.mockResolvedValue(null);
|
||||
|
||||
const command = new VerifyPhoneChangeCommand('user-1', '123456');
|
||||
await expect(handler.execute(command)).rejects.toThrow('hết hạn');
|
||||
});
|
||||
|
||||
it('throws ValidationException when OTP code is wrong', async () => {
|
||||
const payload = JSON.stringify({ newPhone: '+84987654321', code: '123456' });
|
||||
mockRedis.get.mockResolvedValue(payload);
|
||||
|
||||
const command = new VerifyPhoneChangeCommand('user-1', '999999');
|
||||
await expect(handler.execute(command)).rejects.toThrow('không đúng');
|
||||
});
|
||||
|
||||
it('throws ConflictException when phone was taken since OTP was issued', async () => {
|
||||
const user = createTestUser();
|
||||
const otherUser = createTestUser({ id: 'user-2', phone: '0987654321' });
|
||||
const payload = JSON.stringify({ newPhone: '+84987654321', code: '123456' });
|
||||
mockRedis.get.mockResolvedValue(payload);
|
||||
mockUserRepo.findById.mockResolvedValue(user);
|
||||
mockUserRepo.findByPhone.mockResolvedValue(otherUser);
|
||||
|
||||
const command = new VerifyPhoneChangeCommand('user-1', '123456');
|
||||
await expect(handler.execute(command)).rejects.toThrow('Số điện thoại đã được sử dụng');
|
||||
|
||||
// OTP should be cleaned up on conflict
|
||||
expect(mockRedis.del).toHaveBeenCalledWith('auth:phone_change_otp:user-1');
|
||||
});
|
||||
|
||||
it('throws NotFoundException when user does not exist', async () => {
|
||||
const payload = JSON.stringify({ newPhone: '+84987654321', code: '123456' });
|
||||
mockRedis.get.mockResolvedValue(payload);
|
||||
mockUserRepo.findById.mockResolvedValue(null);
|
||||
|
||||
const command = new VerifyPhoneChangeCommand('user-1', '123456');
|
||||
await expect(handler.execute(command)).rejects.toThrow('Người dùng');
|
||||
});
|
||||
});
|
||||
@@ -4,5 +4,6 @@ export class UpdateProfileCommand {
|
||||
public readonly fullName?: string,
|
||||
public readonly avatarUrl?: string,
|
||||
public readonly email?: string,
|
||||
public readonly phoneNumber?: string,
|
||||
) {}
|
||||
}
|
||||
|
||||
@@ -12,8 +12,10 @@ import {
|
||||
ValidationException,
|
||||
} from '@modules/shared';
|
||||
import { EmailChangeRequestedEvent } from '../../../domain/events/email-change-requested.event';
|
||||
import { PhoneChangeRequestedEvent } from '../../../domain/events/phone-change-requested.event';
|
||||
import { type IUserRepository, USER_REPOSITORY } from '../../../domain/repositories/user.repository';
|
||||
import { Email } from '../../../domain/value-objects/email.vo';
|
||||
import { Phone } from '../../../domain/value-objects/phone.vo';
|
||||
import { UpdateProfileCommand } from './update-profile.command';
|
||||
|
||||
/** TTL for email-change OTP codes stored in Redis (10 minutes). */
|
||||
@@ -22,12 +24,20 @@ const EMAIL_CHANGE_OTP_TTL = 600;
|
||||
/** Redis key prefix for pending email-change OTP. */
|
||||
export const EMAIL_CHANGE_OTP_PREFIX = 'auth:email_change_otp';
|
||||
|
||||
/** TTL for phone-change OTP codes stored in Redis (10 minutes). */
|
||||
const PHONE_CHANGE_OTP_TTL = 600;
|
||||
|
||||
/** Redis key prefix for pending phone-change OTP. */
|
||||
export const PHONE_CHANGE_OTP_PREFIX = 'auth:phone_change_otp';
|
||||
|
||||
export interface UpdateProfileResultDto {
|
||||
id: string;
|
||||
fullName: string;
|
||||
avatarUrl: string | null;
|
||||
email: string | null;
|
||||
phoneNumber: string;
|
||||
emailChangePending?: boolean;
|
||||
phoneChangePending?: boolean;
|
||||
updatedAt: Date;
|
||||
}
|
||||
|
||||
@@ -49,6 +59,7 @@ export class UpdateProfileHandler implements ICommandHandler<UpdateProfileComman
|
||||
}
|
||||
|
||||
let emailChangePending = false;
|
||||
let phoneChangePending = false;
|
||||
|
||||
// Validate and handle email change via OTP
|
||||
if (command.email !== undefined) {
|
||||
@@ -84,7 +95,41 @@ export class UpdateProfileHandler implements ICommandHandler<UpdateProfileComman
|
||||
}
|
||||
}
|
||||
|
||||
// Apply non-email fields immediately
|
||||
// Validate and handle phone change via SMS OTP
|
||||
if (command.phoneNumber !== undefined) {
|
||||
const phoneResult = Phone.create(command.phoneNumber);
|
||||
if (phoneResult.isErr) {
|
||||
throw new ValidationException(phoneResult.unwrapErr());
|
||||
}
|
||||
const phone = phoneResult.unwrap();
|
||||
|
||||
// Check if phone is actually changing
|
||||
if (user.phone.value !== phone.value) {
|
||||
// Check uniqueness
|
||||
const existingUser = await this.userRepo.findByPhone(phone.value);
|
||||
if (existingUser && existingUser.id !== command.userId) {
|
||||
throw new ConflictException('Số điện thoại đã được sử dụng bởi tài khoản khác');
|
||||
}
|
||||
|
||||
// Generate OTP and store pending change in Redis
|
||||
const otpCode = String(randomInt(100_000, 999_999));
|
||||
const payload = JSON.stringify({ newPhone: phone.value, code: otpCode });
|
||||
await this.redis.set(
|
||||
`${PHONE_CHANGE_OTP_PREFIX}:${command.userId}`,
|
||||
payload,
|
||||
PHONE_CHANGE_OTP_TTL,
|
||||
);
|
||||
|
||||
// Emit event so notifications module can send the SMS OTP
|
||||
this.eventBus.publish(
|
||||
new PhoneChangeRequestedEvent(command.userId, phone.value, otpCode),
|
||||
);
|
||||
|
||||
phoneChangePending = true;
|
||||
}
|
||||
}
|
||||
|
||||
// Apply non-email / non-phone fields immediately
|
||||
user.updateProfile(command.fullName, command.avatarUrl, undefined);
|
||||
await this.userRepo.update(user);
|
||||
|
||||
@@ -97,7 +142,9 @@ export class UpdateProfileHandler implements ICommandHandler<UpdateProfileComman
|
||||
fullName: user.fullName,
|
||||
avatarUrl: user.avatarUrl,
|
||||
email: user.email?.value ?? null,
|
||||
phoneNumber: user.phone.value,
|
||||
...(emailChangePending ? { emailChangePending: true } : {}),
|
||||
...(phoneChangePending ? { phoneChangePending: true } : {}),
|
||||
updatedAt: user.updatedAt,
|
||||
};
|
||||
} catch (error) {
|
||||
|
||||
@@ -0,0 +1,6 @@
|
||||
export class VerifyPhoneChangeCommand {
|
||||
constructor(
|
||||
public readonly userId: string,
|
||||
public readonly code: string,
|
||||
) {}
|
||||
}
|
||||
@@ -0,0 +1,87 @@
|
||||
import { Inject, InternalServerErrorException } from '@nestjs/common';
|
||||
import { CommandHandler, type ICommandHandler } from '@nestjs/cqrs';
|
||||
import {
|
||||
CachePrefix,
|
||||
CacheService,
|
||||
ConflictException,
|
||||
DomainException,
|
||||
type LoggerService,
|
||||
NotFoundException,
|
||||
type RedisService,
|
||||
ValidationException,
|
||||
} from '@modules/shared';
|
||||
import { type IUserRepository, USER_REPOSITORY } from '../../../domain/repositories/user.repository';
|
||||
import { Phone } from '../../../domain/value-objects/phone.vo';
|
||||
import { PHONE_CHANGE_OTP_PREFIX } from '../update-profile/update-profile.handler';
|
||||
import { VerifyPhoneChangeCommand } from './verify-phone-change.command';
|
||||
|
||||
export interface VerifyPhoneChangeResultDto {
|
||||
id: string;
|
||||
phoneNumber: string;
|
||||
updatedAt: Date;
|
||||
}
|
||||
|
||||
@CommandHandler(VerifyPhoneChangeCommand)
|
||||
export class VerifyPhoneChangeHandler implements ICommandHandler<VerifyPhoneChangeCommand> {
|
||||
constructor(
|
||||
@Inject(USER_REPOSITORY) private readonly userRepo: IUserRepository,
|
||||
private readonly redis: RedisService,
|
||||
private readonly cache: CacheService,
|
||||
private readonly logger: LoggerService,
|
||||
) {}
|
||||
|
||||
async execute(command: VerifyPhoneChangeCommand): Promise<VerifyPhoneChangeResultDto> {
|
||||
try {
|
||||
const redisKey = `${PHONE_CHANGE_OTP_PREFIX}:${command.userId}`;
|
||||
const raw = await this.redis.get(redisKey);
|
||||
|
||||
if (!raw) {
|
||||
throw new ValidationException(
|
||||
'Mã xác thực đã hết hạn hoặc không tồn tại. Vui lòng yêu cầu đổi số điện thoại lại.',
|
||||
);
|
||||
}
|
||||
|
||||
const { newPhone, code } = JSON.parse(raw) as { newPhone: string; code: string };
|
||||
|
||||
if (code !== command.code) {
|
||||
throw new ValidationException('Mã xác thực không đúng');
|
||||
}
|
||||
|
||||
const user = await this.userRepo.findById(command.userId);
|
||||
if (!user) {
|
||||
throw new NotFoundException('Người dùng', command.userId);
|
||||
}
|
||||
|
||||
// Re-check phone uniqueness (may have been taken since the request)
|
||||
const existingUser = await this.userRepo.findByPhone(newPhone);
|
||||
if (existingUser && existingUser.id !== command.userId) {
|
||||
await this.redis.del(redisKey);
|
||||
throw new ConflictException('Số điện thoại đã được sử dụng bởi tài khoản khác');
|
||||
}
|
||||
|
||||
const phoneVo = Phone.create(newPhone).unwrap();
|
||||
user.updatePhone(phoneVo);
|
||||
await this.userRepo.update(user);
|
||||
|
||||
// Clean up OTP and invalidate profile cache
|
||||
await this.redis.del(redisKey);
|
||||
await this.cache.invalidate(
|
||||
CacheService.buildKey(CachePrefix.USER_PROFILE, command.userId),
|
||||
);
|
||||
|
||||
return {
|
||||
id: user.id,
|
||||
phoneNumber: phoneVo.value,
|
||||
updatedAt: user.updatedAt,
|
||||
};
|
||||
} catch (error) {
|
||||
if (error instanceof DomainException) throw error;
|
||||
this.logger.error(
|
||||
`Failed to verify phone change: ${error instanceof Error ? error.message : error}`,
|
||||
error instanceof Error ? error.stack : undefined,
|
||||
this.constructor.name,
|
||||
);
|
||||
throw new InternalServerErrorException('Không thể xác thực đổi số điện thoại');
|
||||
}
|
||||
}
|
||||
}
|
||||
Reference in New Issue
Block a user