From 9583d1cb66c4d096218c51515289d338c53cf239 Mon Sep 17 00:00:00 2001 From: Ho Ngoc Hai Date: Wed, 8 Apr 2026 06:18:26 +0700 Subject: [PATCH] fix(payments): harden payment flow with idempotency keys, amount validation, and magic byte file validation - Add dedicated idempotencyKey column with unique constraint (userId, provider, idempotencyKey) to prevent duplicate payments at DB level - Add @Min(1) @Max(100B) validators on amountVND in CreatePaymentDto to reject invalid amounts at API boundary - Replace read-check-write callback handler with atomic updateIfStatus to eliminate race condition on concurrent callbacks - Add magic byte verification in FileValidationPipe to validate file content matches declared MIME type server-side Co-Authored-By: Paperclip --- .../handle-callback.handler.ts | 59 +++++++++++-------- .../domain/entities/payment.entity.ts | 14 +++++ .../domain/repositories/payment.repository.ts | 2 + .../repositories/prisma-payment.repository.ts | 39 +++++++++--- .../controllers/payments.controller.ts | 2 +- .../presentation/dto/create-payment.dto.ts | 16 ++++- .../pipes/file-validation.pipe.ts | 44 +++++++++++++- .../migration.sql | 5 ++ prisma/schema.prisma | 10 ++-- 9 files changed, 148 insertions(+), 43 deletions(-) create mode 100644 prisma/migrations/20260408000000_add_idempotency_key_to_payment/migration.sql diff --git a/apps/api/src/modules/payments/application/commands/handle-callback/handle-callback.handler.ts b/apps/api/src/modules/payments/application/commands/handle-callback/handle-callback.handler.ts index c497fe4..cc7c592 100644 --- a/apps/api/src/modules/payments/application/commands/handle-callback/handle-callback.handler.ts +++ b/apps/api/src/modules/payments/application/commands/handle-callback/handle-callback.handler.ts @@ -48,50 +48,59 @@ export class HandleCallbackHandler implements ICommandHandler { ); } + /** Emit completed event without modifying state (used when DB was already updated atomically). */ + emitCompleted(): void { + this.addDomainEvent( + new PaymentCompletedEvent(this.id, this._userId, this._provider, this._amount.value), + ); + } + + /** Emit failed event without modifying state (used when DB was already updated atomically). */ + emitFailed(): void { + this.addDomainEvent( + new PaymentFailedEvent(this.id, this._userId, this._provider), + ); + } + markRefunded(): void { if (this._status !== 'COMPLETED') { throw new Error('Chỉ có thể hoàn tiền cho thanh toán đã hoàn tất'); diff --git a/apps/api/src/modules/payments/domain/repositories/payment.repository.ts b/apps/api/src/modules/payments/domain/repositories/payment.repository.ts index 9d53bfc..aa8dee9 100644 --- a/apps/api/src/modules/payments/domain/repositories/payment.repository.ts +++ b/apps/api/src/modules/payments/domain/repositories/payment.repository.ts @@ -14,4 +14,6 @@ export interface IPaymentRepository { }): Promise<{ items: PaymentEntity[]; total: number }>; save(payment: PaymentEntity): Promise; update(payment: PaymentEntity): Promise; + /** Atomically update payment status only if it is currently in one of the expected statuses. Returns null if no matching row. */ + updateIfStatus(id: string, expectedStatuses: PaymentStatus[], data: { status: PaymentStatus; providerTxId?: string; callbackData?: unknown }): Promise; } diff --git a/apps/api/src/modules/payments/infrastructure/repositories/prisma-payment.repository.ts b/apps/api/src/modules/payments/infrastructure/repositories/prisma-payment.repository.ts index 6fb2eac..08f26b0 100644 --- a/apps/api/src/modules/payments/infrastructure/repositories/prisma-payment.repository.ts +++ b/apps/api/src/modules/payments/infrastructure/repositories/prisma-payment.repository.ts @@ -1,6 +1,6 @@ import { Injectable } from '@nestjs/common'; import { PrismaService } from '@modules/shared/infrastructure/prisma.service'; -import { type Payment as PrismaPayment, type PaymentStatus } from '@prisma/client'; +import { Prisma, type Payment as PrismaPayment, type PaymentStatus } from '@prisma/client'; import { type IPaymentRepository } from '../../domain/repositories/payment.repository'; import { PaymentEntity, type PaymentProps } from '../../domain/entities/payment.entity'; import { Money } from '../../domain/value-objects/money.vo'; @@ -23,12 +23,7 @@ export class PrismaPaymentRepository implements IPaymentRepository { async findByIdempotencyKey(key: string): Promise { const payment = await this.prisma.payment.findFirst({ - where: { - callbackData: { - path: ['idempotencyKey'], - equals: key, - }, - }, + where: { idempotencyKey: key }, }); return payment ? this.toDomain(payment) : null; } @@ -70,6 +65,7 @@ export class PrismaPaymentRepository implements IPaymentRepository { status: entity.status, providerTxId: entity.providerTxId, callbackData: entity.callbackData as any, + idempotencyKey: entity.idempotencyKey, }, }); } @@ -85,6 +81,33 @@ export class PrismaPaymentRepository implements IPaymentRepository { }); } + async updateIfStatus( + id: string, + expectedStatuses: PaymentStatus[], + data: { status: PaymentStatus; providerTxId?: string; callbackData?: unknown }, + ): Promise { + try { + const updated = await this.prisma.payment.update({ + where: { + id, + status: { in: expectedStatuses }, + }, + data: { + status: data.status, + ...(data.providerTxId !== undefined ? { providerTxId: data.providerTxId } : {}), + ...(data.callbackData !== undefined ? { callbackData: data.callbackData as any } : {}), + }, + }); + return this.toDomain(updated); + } catch (error) { + // P2025: Record not found (status didn't match or ID doesn't exist) + if (error instanceof Prisma.PrismaClientKnownRequestError && error.code === 'P2025') { + return null; + } + throw error; + } + } + private toDomain(raw: PrismaPayment): PaymentEntity { const amount = Money.create(raw.amountVND).unwrap(); @@ -97,7 +120,7 @@ export class PrismaPaymentRepository implements IPaymentRepository { status: raw.status, providerTxId: raw.providerTxId, callbackData: raw.callbackData, - idempotencyKey: (raw.callbackData as any)?.idempotencyKey ?? null, + idempotencyKey: raw.idempotencyKey ?? null, }; return new PaymentEntity(raw.id, props, raw.createdAt, raw.updatedAt); diff --git a/apps/api/src/modules/payments/presentation/controllers/payments.controller.ts b/apps/api/src/modules/payments/presentation/controllers/payments.controller.ts index 0f082a8..e080c72 100644 --- a/apps/api/src/modules/payments/presentation/controllers/payments.controller.ts +++ b/apps/api/src/modules/payments/presentation/controllers/payments.controller.ts @@ -62,7 +62,7 @@ export class PaymentsController { user.sub, dto.provider, dto.type, - dto.amountVND, + BigInt(dto.amountVND), dto.description, dto.returnUrl, ip || '127.0.0.1', diff --git a/apps/api/src/modules/payments/presentation/dto/create-payment.dto.ts b/apps/api/src/modules/payments/presentation/dto/create-payment.dto.ts index bc27c38..a2700a3 100644 --- a/apps/api/src/modules/payments/presentation/dto/create-payment.dto.ts +++ b/apps/api/src/modules/payments/presentation/dto/create-payment.dto.ts @@ -1,9 +1,12 @@ import { IsEnum, IsNotEmpty, + IsNumber, IsOptional, IsString, IsUrl, + Max, + Min, MinLength, } from 'class-validator'; import { Transform } from 'class-transformer'; @@ -19,10 +22,17 @@ export class CreatePaymentDto { @IsEnum(PaymentType) type!: PaymentType; - @ApiProperty({ type: Number, description: 'Amount in VND', example: 500000 }) + @ApiProperty({ type: Number, description: 'Amount in VND (1 – 100,000,000,000)', example: 500000 }) @IsNotEmpty() - @Transform(({ value }) => BigInt(value)) - amountVND!: bigint; + @IsNumber() + @Min(1, { message: 'Số tiền phải lớn hơn 0' }) + @Max(100_000_000_000, { message: 'Số tiền vượt quá giới hạn cho phép (100 tỷ VND)' }) + @Transform(({ value }) => { + const num = Number(value); + if (!Number.isFinite(num) || !Number.isInteger(num)) return value; + return num; + }, { toClassOnly: true }) + amountVND!: number; @ApiProperty({ description: 'Payment description', example: 'Listing fee' }) @IsString() diff --git a/apps/api/src/modules/shared/infrastructure/pipes/file-validation.pipe.ts b/apps/api/src/modules/shared/infrastructure/pipes/file-validation.pipe.ts index d62c9d8..50ee19b 100644 --- a/apps/api/src/modules/shared/infrastructure/pipes/file-validation.pipe.ts +++ b/apps/api/src/modules/shared/infrastructure/pipes/file-validation.pipe.ts @@ -14,6 +14,8 @@ export interface FileValidationOptions { maxSizeBytes?: number; /** Allowed MIME types. Default: common image types + PDF */ allowedMimeTypes?: string[]; + /** Whether to verify file content matches declared MIME type via magic bytes. Default: true */ + verifyMagicBytes?: boolean; } const DEFAULT_MAX_SIZE = 5 * 1024 * 1024; // 5 MB @@ -25,18 +27,33 @@ const DEFAULT_ALLOWED_MIMES = [ 'application/pdf', ]; +/** Magic byte signatures for supported file types. */ +const MAGIC_BYTES: Record = { + 'image/jpeg': [{ offset: 0, bytes: [0xFF, 0xD8, 0xFF] }], + 'image/png': [{ offset: 0, bytes: [0x89, 0x50, 0x4E, 0x47, 0x0D, 0x0A, 0x1A, 0x0A] }], + 'image/webp': [{ offset: 0, bytes: [0x52, 0x49, 0x46, 0x46] }], // "RIFF" header + 'image/gif': [ + { offset: 0, bytes: [0x47, 0x49, 0x46, 0x38, 0x37, 0x61] }, // GIF87a + { offset: 0, bytes: [0x47, 0x49, 0x46, 0x38, 0x39, 0x61] }, // GIF89a + ], + 'application/pdf': [{ offset: 0, bytes: [0x25, 0x50, 0x44, 0x46] }], // %PDF + 'video/mp4': [{ offset: 4, bytes: [0x66, 0x74, 0x79, 0x70] }], // "ftyp" at offset 4 +}; + /** - * Validates uploaded files for size and MIME type to prevent - * malicious file uploads and resource exhaustion. + * Validates uploaded files for size, MIME type, and file content + * (magic bytes) to prevent malicious file uploads and resource exhaustion. */ @Injectable() export class FileValidationPipe implements PipeTransform { private readonly maxSize: number; private readonly allowedMimes: string[]; + private readonly verifyMagicBytes: boolean; constructor(options?: FileValidationOptions) { this.maxSize = options?.maxSizeBytes ?? DEFAULT_MAX_SIZE; this.allowedMimes = options?.allowedMimeTypes ?? DEFAULT_ALLOWED_MIMES; + this.verifyMagicBytes = options?.verifyMagicBytes ?? true; } transform(file: UploadedFile): UploadedFile { @@ -56,6 +73,29 @@ export class FileValidationPipe implements PipeTransform { ); } + if (this.verifyMagicBytes) { + this.validateMagicBytes(file); + } + return file; } + + private validateMagicBytes(file: UploadedFile): void { + const signatures = MAGIC_BYTES[file.mimetype]; + if (!signatures) { + // No known signature for this type — skip magic byte check + return; + } + + const matches = signatures.some((sig) => { + if (file.buffer.length < sig.offset + sig.bytes.length) return false; + return sig.bytes.every((byte, i) => file.buffer[sig.offset + i] === byte); + }); + + if (!matches) { + throw new BadRequestException( + `File content does not match declared type '${file.mimetype}'. The file may be corrupted or mislabeled.`, + ); + } + } } diff --git a/prisma/migrations/20260408000000_add_idempotency_key_to_payment/migration.sql b/prisma/migrations/20260408000000_add_idempotency_key_to_payment/migration.sql new file mode 100644 index 0000000..963d8dc --- /dev/null +++ b/prisma/migrations/20260408000000_add_idempotency_key_to_payment/migration.sql @@ -0,0 +1,5 @@ +-- AlterTable +ALTER TABLE "Payment" ADD COLUMN "idempotencyKey" TEXT; + +-- CreateIndex +CREATE UNIQUE INDEX "Payment_idempotency_unique" ON "Payment"("userId", "provider", "idempotencyKey"); diff --git a/prisma/schema.prisma b/prisma/schema.prisma index e1f2726..2b4b837 100644 --- a/prisma/schema.prisma +++ b/prisma/schema.prisma @@ -368,11 +368,13 @@ model Payment { type PaymentType amountVND BigInt status PaymentStatus @default(PENDING) - providerTxId String? - callbackData Json? - createdAt DateTime @default(now()) - updatedAt DateTime @updatedAt + providerTxId String? + callbackData Json? + idempotencyKey String? + createdAt DateTime @default(now()) + updatedAt DateTime @updatedAt + @@unique([userId, provider, idempotencyKey], name: "Payment_idempotency_unique") @@index([userId]) @@index([transactionId]) @@index([status])