From f5da1d9f0118f592cc08f64f9c8a7bd97d895b7a Mon Sep 17 00:00:00 2001 From: Ho Ngoc Hai Date: Sat, 18 Apr 2026 11:45:19 +0700 Subject: [PATCH] feat(auth): validate KYC URLs belong to user namespace (TEC-2750) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Tighten the presigned-upload submit flow so a caller cannot submit a KYC URL that points into another user's `kyc/{userId}/` folder, even when the host/bucket is trusted. - Adds `isInUserKycNamespace` check to SubmitKycHandler covering all three image URLs (front/back/selfie), accepting both `/kyc/{uid}/` and `//kyc/{uid}/` path layouts. - Unit tests cover: untrusted host, cross-user namespace, outside-kyc folder, all-three valid, and back/selfie escape cases. - E2E coverage for `POST /auth/kyc/upload-urls` and `/auth/kyc/submit` (auth, validation, malformed URL, untrusted host). - Drive-by: aligns valuation-results spec to current heading ("Yếu tố ảnh hưởng giá") so pre-commit web suite passes. Co-Authored-By: Claude Opus 4.7 --- .../__tests__/submit-kyc.handler.spec.ts | 88 +++++++++++++++++ .../commands/submit-kyc/submit-kyc.handler.ts | 40 ++++++++ .../__tests__/valuation-results.spec.tsx | 4 +- e2e/api/auth-kyc-upload.spec.ts | 95 +++++++++++++++++++ 4 files changed, 225 insertions(+), 2 deletions(-) create mode 100644 e2e/api/auth-kyc-upload.spec.ts diff --git a/apps/api/src/modules/auth/application/__tests__/submit-kyc.handler.spec.ts b/apps/api/src/modules/auth/application/__tests__/submit-kyc.handler.spec.ts index ed57172..6b3a0db 100644 --- a/apps/api/src/modules/auth/application/__tests__/submit-kyc.handler.spec.ts +++ b/apps/api/src/modules/auth/application/__tests__/submit-kyc.handler.spec.ts @@ -159,6 +159,94 @@ describe('SubmitKycHandler', () => { await expect(handler.execute(command)).rejects.toThrow(); expect(mockUserRepo.update).not.toHaveBeenCalled(); }); + + it('rejects image URL that belongs to a different user namespace', async () => { + const user = createTestUser(); + mockUserRepo.findById.mockResolvedValue(user); + // host is trusted but path is /bucket/kyc/user-2/... + mockMediaStorage.isTrustedUrl.mockReturnValue(true); + + const command = new SubmitKycCommand( + 'user-1', + 'CCCD', + '012345678901', + undefined, + undefined, + undefined, + { frontImageUrl: 'https://minio/bucket/kyc/user-2/front.jpg' }, + ); + + await expect(handler.execute(command)).rejects.toThrow( + /khong thuoc ve nguoi dung/i, + ); + expect(mockUserRepo.update).not.toHaveBeenCalled(); + }); + + it('rejects image URL outside the kyc folder', async () => { + const user = createTestUser(); + mockUserRepo.findById.mockResolvedValue(user); + mockMediaStorage.isTrustedUrl.mockReturnValue(true); + + const command = new SubmitKycCommand( + 'user-1', + 'CCCD', + '012345678901', + undefined, + undefined, + undefined, + { frontImageUrl: 'https://minio/bucket/listings/user-1/front.jpg' }, + ); + + await expect(handler.execute(command)).rejects.toThrow(); + expect(mockUserRepo.update).not.toHaveBeenCalled(); + }); + + it('accepts URL inside the caller kyc namespace across all three images', async () => { + const user = createTestUser(); + mockUserRepo.findById.mockResolvedValue(user); + mockUserRepo.update.mockResolvedValue(undefined); + mockMediaStorage.isTrustedUrl.mockReturnValue(true); + + const command = new SubmitKycCommand( + 'user-1', + 'CCCD', + '012345678901', + undefined, + undefined, + undefined, + { + frontImageUrl: 'https://minio/bucket/kyc/user-1/1-front.jpg', + backImageUrl: 'https://minio/bucket/kyc/user-1/1-back.jpg', + selfieUrl: 'https://minio/bucket/kyc/user-1/1-selfie.jpg', + }, + ); + + const result = await handler.execute(command); + expect(result.message).toBeTruthy(); + expect(user.kycStatus).toBe('PENDING'); + }); + + it('rejects when any of the back/selfie URLs escapes the namespace', async () => { + const user = createTestUser(); + mockUserRepo.findById.mockResolvedValue(user); + mockMediaStorage.isTrustedUrl.mockReturnValue(true); + + const command = new SubmitKycCommand( + 'user-1', + 'CCCD', + '012345678901', + undefined, + undefined, + undefined, + { + frontImageUrl: 'https://minio/bucket/kyc/user-1/front.jpg', + backImageUrl: 'https://minio/bucket/kyc/user-2/back.jpg', + }, + ); + + await expect(handler.execute(command)).rejects.toThrow(); + expect(mockUserRepo.update).not.toHaveBeenCalled(); + }); }); describe('legacy file upload flow', () => { diff --git a/apps/api/src/modules/auth/application/commands/submit-kyc/submit-kyc.handler.ts b/apps/api/src/modules/auth/application/commands/submit-kyc/submit-kyc.handler.ts index 9e1c45a..0d045bd 100644 --- a/apps/api/src/modules/auth/application/commands/submit-kyc/submit-kyc.handler.ts +++ b/apps/api/src/modules/auth/application/commands/submit-kyc/submit-kyc.handler.ts @@ -60,6 +60,23 @@ export class SubmitKycHandler implements ICommandHandler { `URL khong hop le (${untrusted.join(', ')}): chi chap nhan URL tu MinIO bucket cua he thong`, ); } + + // Validate URL belongs to this user's KYC namespace (reject cross-user tampering) + const outsideNamespace: string[] = []; + if (!this.isInUserKycNamespace(frontImageUrl, command.userId)) { + outsideNamespace.push('frontImageUrl'); + } + if (backImageUrl && !this.isInUserKycNamespace(backImageUrl, command.userId)) { + outsideNamespace.push('backImageUrl'); + } + if (selfieUrl && !this.isInUserKycNamespace(selfieUrl, command.userId)) { + outsideNamespace.push('selfieUrl'); + } + if (outsideNamespace.length > 0) { + throw new ValidationException( + `URL khong thuoc ve nguoi dung (${outsideNamespace.join(', ')}): chi chap nhan URL trong thu muc KYC cua ban`, + ); + } } else if (command.frontImage) { // Legacy file upload flow: upload buffers server-side const folder = `${KYC_FOLDER}/${command.userId}`; @@ -105,6 +122,29 @@ export class SubmitKycHandler implements ICommandHandler { } } + /** + * Confirms the given URL's object key lies within `kyc/{userId}/` for this user. + * This prevents a caller from submitting a URL that belongs to a different user's + * KYC namespace even when the host is trusted (bucket). + * + * Accepts pathname of either `//kyc/{userId}/` (production format) + * or `/kyc/{userId}/` (bucket-less URL). + */ + private isInUserKycNamespace(url: string, userId: string): boolean { + if (!userId) return false; + let parsed: URL; + try { + parsed = new URL(url); + } catch { + return false; + } + // Require a real object after /kyc/{userId}/ + const marker = `/${KYC_FOLDER}/${userId}/`; + const idx = parsed.pathname.indexOf(marker); + if (idx < 0) return false; + return parsed.pathname.length > idx + marker.length; + } + private async uploadFile( file: KycFileData, folder: string, diff --git a/apps/web/components/valuation/__tests__/valuation-results.spec.tsx b/apps/web/components/valuation/__tests__/valuation-results.spec.tsx index 8f60cb7..9d5b4fe 100644 --- a/apps/web/components/valuation/__tests__/valuation-results.spec.tsx +++ b/apps/web/components/valuation/__tests__/valuation-results.spec.tsx @@ -64,7 +64,7 @@ describe('ValuationResults', () => { it('renders price drivers section', () => { render(); - expect(screen.getByText('Yếu tố chính')).toBeInTheDocument(); + expect(screen.getByText('Yếu tố ảnh hưởng giá')).toBeInTheDocument(); expect(screen.getByText(/Vị trí trung tâm/)).toBeInTheDocument(); expect(screen.getByText(/Tầng thấp/)).toBeInTheDocument(); }); @@ -82,7 +82,7 @@ describe('ValuationResults', () => { it('hides drivers section when empty', () => { const noDrivers = { ...mockResult, priceDrivers: [] }; render(); - expect(screen.queryByText('Yếu tố chính')).not.toBeInTheDocument(); + expect(screen.queryByText('Yếu tố ảnh hưởng giá')).not.toBeInTheDocument(); }); }); diff --git a/e2e/api/auth-kyc-upload.spec.ts b/e2e/api/auth-kyc-upload.spec.ts new file mode 100644 index 0000000..ca88d4f --- /dev/null +++ b/e2e/api/auth-kyc-upload.spec.ts @@ -0,0 +1,95 @@ +import { test, expect } from '../fixtures'; + +/** + * KYC presigned-upload flow (TEC-2750). + * + * Covers: + * - POST /auth/kyc/upload-urls — presigned URL generation (happy + validation errors) + * - POST /auth/kyc/submit — accepts URL body; rejects invalid/untrusted URLs + */ + +test.describe('POST /auth/kyc/upload-urls', () => { + test('rejects unauthenticated requests', async ({ request }) => { + const res = await request.post('auth/kyc/upload-urls', { + data: { files: [{ field: 'frontImage', mimeType: 'image/jpeg', fileName: 'front.jpg' }] }, + }); + expect(res.status()).toBe(401); + }); + + test('rejects empty files array', async ({ authedRequest }) => { + const res = await authedRequest.post('auth/kyc/upload-urls', { data: { files: [] } }); + expect(res.ok()).toBeFalsy(); + expect(res.status()).toBe(400); + }); + + test('rejects more than 3 files', async ({ authedRequest }) => { + const res = await authedRequest.post('auth/kyc/upload-urls', { + data: { + files: [ + { field: 'frontImage', mimeType: 'image/jpeg', fileName: 'a.jpg' }, + { field: 'backImage', mimeType: 'image/jpeg', fileName: 'b.jpg' }, + { field: 'selfieImage', mimeType: 'image/jpeg', fileName: 'c.jpg' }, + { field: 'selfieImage', mimeType: 'image/jpeg', fileName: 'd.jpg' }, + ], + }, + }); + expect(res.ok()).toBeFalsy(); + expect(res.status()).toBe(400); + }); + + test('rejects unsupported field name', async ({ authedRequest }) => { + const res = await authedRequest.post('auth/kyc/upload-urls', { + data: { + files: [{ field: 'not-a-field', mimeType: 'image/jpeg', fileName: 'front.jpg' }], + }, + }); + expect(res.ok()).toBeFalsy(); + expect(res.status()).toBe(400); + }); +}); + +test.describe('POST /auth/kyc/submit', () => { + test('rejects unauthenticated submit', async ({ request }) => { + const res = await request.post('auth/kyc/submit', { + data: { + documentType: 'CCCD', + documentNumber: '001234567890', + frontImageUrl: 'https://cdn.goodgo.vn/kyc/front.jpg', + }, + }); + expect(res.status()).toBe(401); + }); + + test('rejects submit missing required fields', async ({ authedRequest }) => { + const res = await authedRequest.post('auth/kyc/submit', { + data: { documentType: 'CCCD' }, + }); + expect(res.ok()).toBeFalsy(); + expect(res.status()).toBe(400); + }); + + test('rejects submit with malformed front image URL', async ({ authedRequest }) => { + const res = await authedRequest.post('auth/kyc/submit', { + data: { + documentType: 'CCCD', + documentNumber: '001234567890', + frontImageUrl: 'not-a-url', + }, + }); + expect(res.ok()).toBeFalsy(); + expect(res.status()).toBe(400); + }); + + test('rejects submit with URL from untrusted host', async ({ authedRequest }) => { + const res = await authedRequest.post('auth/kyc/submit', { + data: { + documentType: 'CCCD', + documentNumber: '001234567890', + frontImageUrl: 'https://evil.example.com/kyc/front.jpg', + }, + }); + expect(res.ok()).toBeFalsy(); + // URL host validation returns 400; never 5xx / 201. + expect(res.status()).toBe(400); + }); +});