feat(auth): validate KYC image URL hosts match MinIO bucket
Closes TEC-2725. Backend KYC presign + submit endpoints already landed in 8f8e20f; this adds the remaining acceptance criterion — host validation on presigned URLs accepted via /auth/kyc/submit. - Add IMediaStorageService.isTrustedUrl(url) — host+bucket check, supports MINIO_TRUSTED_HOSTS for CDN aliases - SubmitKycHandler rejects imageUrls pointing outside our MinIO bucket - Update handler specs with mock + new untrusted-host test Co-Authored-By: Paperclip <noreply@paperclip.ing>
This commit is contained in:
@@ -1,8 +1,8 @@
|
|||||||
|
import { type IMediaStorageService } from '../../../../listings/infrastructure/services/media-storage.service';
|
||||||
import { UserEntity } from '../../domain/entities/user.entity';
|
import { UserEntity } from '../../domain/entities/user.entity';
|
||||||
import { type IUserRepository } from '../../domain/repositories/user.repository';
|
import { type IUserRepository } from '../../domain/repositories/user.repository';
|
||||||
import { type HashedPassword } from '../../domain/value-objects/hashed-password.vo';
|
import { type HashedPassword } from '../../domain/value-objects/hashed-password.vo';
|
||||||
import { Phone } from '../../domain/value-objects/phone.vo';
|
import { Phone } from '../../domain/value-objects/phone.vo';
|
||||||
import { type IMediaStorageService } from '../../../../listings/infrastructure/services/media-storage.service';
|
|
||||||
import { GenerateKycUploadUrlsCommand } from '../commands/generate-kyc-upload-urls/generate-kyc-upload-urls.command';
|
import { GenerateKycUploadUrlsCommand } from '../commands/generate-kyc-upload-urls/generate-kyc-upload-urls.command';
|
||||||
import { GenerateKycUploadUrlsHandler } from '../commands/generate-kyc-upload-urls/generate-kyc-upload-urls.handler';
|
import { GenerateKycUploadUrlsHandler } from '../commands/generate-kyc-upload-urls/generate-kyc-upload-urls.handler';
|
||||||
|
|
||||||
@@ -42,6 +42,7 @@ describe('GenerateKycUploadUrlsHandler', () => {
|
|||||||
getPresignedUploadUrl: vi.fn(),
|
getPresignedUploadUrl: vi.fn(),
|
||||||
generatePresignedUpload: vi.fn(),
|
generatePresignedUpload: vi.fn(),
|
||||||
getPublicUrl: vi.fn(),
|
getPublicUrl: vi.fn(),
|
||||||
|
isTrustedUrl: vi.fn().mockReturnValue(true),
|
||||||
};
|
};
|
||||||
mockLogger = {
|
mockLogger = {
|
||||||
error: vi.fn(),
|
error: vi.fn(),
|
||||||
|
|||||||
@@ -1,8 +1,8 @@
|
|||||||
|
import { type IMediaStorageService } from '../../../../listings/infrastructure/services/media-storage.service';
|
||||||
import { UserEntity } from '../../domain/entities/user.entity';
|
import { UserEntity } from '../../domain/entities/user.entity';
|
||||||
import { type IUserRepository } from '../../domain/repositories/user.repository';
|
import { type IUserRepository } from '../../domain/repositories/user.repository';
|
||||||
import { type HashedPassword } from '../../domain/value-objects/hashed-password.vo';
|
import { type HashedPassword } from '../../domain/value-objects/hashed-password.vo';
|
||||||
import { Phone } from '../../domain/value-objects/phone.vo';
|
import { Phone } from '../../domain/value-objects/phone.vo';
|
||||||
import { type IMediaStorageService } from '../../../../listings/infrastructure/services/media-storage.service';
|
|
||||||
import { SubmitKycCommand } from '../commands/submit-kyc/submit-kyc.command';
|
import { SubmitKycCommand } from '../commands/submit-kyc/submit-kyc.command';
|
||||||
import { SubmitKycHandler } from '../commands/submit-kyc/submit-kyc.handler';
|
import { SubmitKycHandler } from '../commands/submit-kyc/submit-kyc.handler';
|
||||||
|
|
||||||
@@ -43,6 +43,7 @@ describe('SubmitKycHandler', () => {
|
|||||||
getPresignedUploadUrl: vi.fn(),
|
getPresignedUploadUrl: vi.fn(),
|
||||||
generatePresignedUpload: vi.fn(),
|
generatePresignedUpload: vi.fn(),
|
||||||
getPublicUrl: vi.fn(),
|
getPublicUrl: vi.fn(),
|
||||||
|
isTrustedUrl: vi.fn().mockReturnValue(true),
|
||||||
};
|
};
|
||||||
mockCache = {
|
mockCache = {
|
||||||
invalidate: vi.fn().mockResolvedValue(undefined),
|
invalidate: vi.fn().mockResolvedValue(undefined),
|
||||||
@@ -137,6 +138,27 @@ describe('SubmitKycHandler', () => {
|
|||||||
expect(result.message).toBeTruthy();
|
expect(result.message).toBeTruthy();
|
||||||
expect(user.kycStatus).toBe('PENDING');
|
expect(user.kycStatus).toBe('PENDING');
|
||||||
});
|
});
|
||||||
|
|
||||||
|
it('rejects untrusted image URL hosts', async () => {
|
||||||
|
const user = createTestUser();
|
||||||
|
mockUserRepo.findById.mockResolvedValue(user);
|
||||||
|
mockMediaStorage.isTrustedUrl.mockImplementation((url: string) =>
|
||||||
|
url.startsWith('https://minio/'),
|
||||||
|
);
|
||||||
|
|
||||||
|
const command = new SubmitKycCommand(
|
||||||
|
'user-1',
|
||||||
|
'CCCD',
|
||||||
|
'012345678901',
|
||||||
|
undefined,
|
||||||
|
undefined,
|
||||||
|
undefined,
|
||||||
|
{ frontImageUrl: 'https://evil.example.com/kyc/front.jpg' },
|
||||||
|
);
|
||||||
|
|
||||||
|
await expect(handler.execute(command)).rejects.toThrow();
|
||||||
|
expect(mockUserRepo.update).not.toHaveBeenCalled();
|
||||||
|
});
|
||||||
});
|
});
|
||||||
|
|
||||||
describe('legacy file upload flow', () => {
|
describe('legacy file upload flow', () => {
|
||||||
|
|||||||
@@ -49,6 +49,17 @@ export class SubmitKycHandler implements ICommandHandler<SubmitKycCommand> {
|
|||||||
frontImageUrl = command.imageUrls.frontImageUrl;
|
frontImageUrl = command.imageUrls.frontImageUrl;
|
||||||
backImageUrl = command.imageUrls.backImageUrl ?? null;
|
backImageUrl = command.imageUrls.backImageUrl ?? null;
|
||||||
selfieUrl = command.imageUrls.selfieUrl ?? null;
|
selfieUrl = command.imageUrls.selfieUrl ?? null;
|
||||||
|
|
||||||
|
// Validate URL hosts match our MinIO bucket (reject SSRF / tampering)
|
||||||
|
const untrusted: string[] = [];
|
||||||
|
if (!this.mediaStorage.isTrustedUrl(frontImageUrl)) untrusted.push('frontImageUrl');
|
||||||
|
if (backImageUrl && !this.mediaStorage.isTrustedUrl(backImageUrl)) untrusted.push('backImageUrl');
|
||||||
|
if (selfieUrl && !this.mediaStorage.isTrustedUrl(selfieUrl)) untrusted.push('selfieUrl');
|
||||||
|
if (untrusted.length > 0) {
|
||||||
|
throw new ValidationException(
|
||||||
|
`URL khong hop le (${untrusted.join(', ')}): chi chap nhan URL tu MinIO bucket cua he thong`,
|
||||||
|
);
|
||||||
|
}
|
||||||
} else if (command.frontImage) {
|
} else if (command.frontImage) {
|
||||||
// Legacy file upload flow: upload buffers server-side
|
// Legacy file upload flow: upload buffers server-side
|
||||||
const folder = `${KYC_FOLDER}/${command.userId}`;
|
const folder = `${KYC_FOLDER}/${command.userId}`;
|
||||||
|
|||||||
@@ -30,6 +30,7 @@ export interface IMediaStorageService {
|
|||||||
expiresInSeconds?: number,
|
expiresInSeconds?: number,
|
||||||
): Promise<PresignedUploadResult>;
|
): Promise<PresignedUploadResult>;
|
||||||
getPublicUrl(objectKey: string): string;
|
getPublicUrl(objectKey: string): string;
|
||||||
|
isTrustedUrl(url: string): boolean;
|
||||||
}
|
}
|
||||||
|
|
||||||
function requireEnv(key: string): string {
|
function requireEnv(key: string): string {
|
||||||
@@ -151,6 +152,45 @@ export class MinioMediaStorageService implements IMediaStorageService, OnModuleI
|
|||||||
return `${protocol}://${this.endpoint}:${this.port}/${this.bucket}/${objectKey}`;
|
return `${protocol}://${this.endpoint}:${this.port}/${this.bucket}/${objectKey}`;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Validates that a URL points to our configured MinIO bucket.
|
||||||
|
* Accepts the primary endpoint, plus an optional comma-separated list of
|
||||||
|
* additional trusted hosts via `MINIO_TRUSTED_HOSTS` (e.g. public CDN domains).
|
||||||
|
* Also enforces the bucket is the first path segment.
|
||||||
|
*/
|
||||||
|
isTrustedUrl(url: string): boolean {
|
||||||
|
if (!url || typeof url !== 'string') {
|
||||||
|
return false;
|
||||||
|
}
|
||||||
|
let parsed: URL;
|
||||||
|
try {
|
||||||
|
parsed = new URL(url);
|
||||||
|
} catch {
|
||||||
|
return false;
|
||||||
|
}
|
||||||
|
|
||||||
|
const allowedHosts = new Set<string>();
|
||||||
|
allowedHosts.add(this.endpoint.toLowerCase());
|
||||||
|
allowedHosts.add(`${this.endpoint.toLowerCase()}:${this.port}`);
|
||||||
|
|
||||||
|
const extra = process.env['MINIO_TRUSTED_HOSTS'];
|
||||||
|
if (extra) {
|
||||||
|
for (const h of extra.split(',')) {
|
||||||
|
const trimmed = h.trim().toLowerCase();
|
||||||
|
if (trimmed) allowedHosts.add(trimmed);
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
const host = parsed.host.toLowerCase();
|
||||||
|
if (!allowedHosts.has(host) && !allowedHosts.has(parsed.hostname.toLowerCase())) {
|
||||||
|
return false;
|
||||||
|
}
|
||||||
|
|
||||||
|
// Path must start with /<bucket>/
|
||||||
|
const expectedPrefix = `/${this.bucket}/`;
|
||||||
|
return parsed.pathname.startsWith(expectedPrefix) && parsed.pathname.length > expectedPrefix.length;
|
||||||
|
}
|
||||||
|
|
||||||
async delete(fileUrl: string): Promise<void> {
|
async delete(fileUrl: string): Promise<void> {
|
||||||
try {
|
try {
|
||||||
const urlObj = new URL(fileUrl);
|
const urlObj = new URL(fileUrl);
|
||||||
|
|||||||
Reference in New Issue
Block a user