feat(auth): validate KYC URLs belong to user namespace (TEC-2750)
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 `/<bucket>/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 <noreply@anthropic.com>
This commit is contained in:
@@ -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', () => {
|
||||
|
||||
@@ -60,6 +60,23 @@ export class SubmitKycHandler implements ICommandHandler<SubmitKycCommand> {
|
||||
`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<SubmitKycCommand> {
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* 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 `/<bucket>/kyc/{userId}/<object>` (production format)
|
||||
* or `/kyc/{userId}/<object>` (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,
|
||||
|
||||
Reference in New Issue
Block a user