diff --git a/src/shared/models/asset/__tests__/asset.controller.spec.ts b/src/shared/models/asset/__tests__/asset.controller.spec.ts new file mode 100644 index 0000000000..5510dc072c --- /dev/null +++ b/src/shared/models/asset/__tests__/asset.controller.spec.ts @@ -0,0 +1,76 @@ +import { createMock, DeepMocked } from '@golevelup/ts-jest'; +import { ConfigService } from 'src/config/config'; +import { Blockchain } from 'src/integration/blockchain/shared/enums/blockchain.enum'; +import { + TransactionDirection, + TransactionSpecification, +} from 'src/subdomains/supporting/payment/entities/transaction-specification.entity'; +import { TransactionSpecificationRepository } from 'src/subdomains/supporting/payment/repositories/transaction-specification.repository'; +import { EntityManager } from 'typeorm'; +import { RepositoryFactory } from '../../../repositories/repository.factory'; +import { createCustomAsset } from '../__mocks__/asset.entity.mock'; +import { AssetController } from '../asset.controller'; +import { AssetService } from '../asset.service'; + +function createSpec(values: Partial): TransactionSpecification { + return Object.assign(new TransactionSpecification(), { + system: Blockchain.ETHEREUM, + minVolume: 1, + minFee: 0, + ...values, + }); +} + +// The specifications must come from the cache: find() is stubbed to reject, so an uncached read fails the +// test instead of silently passing. +describe('AssetController.getAllAsset', () => { + const usdt = createCustomAsset({ dexName: 'USDT', sellable: true, approxPriceChf: 1 }); + const specs = [createSpec({ asset: 'USDT', direction: TransactionDirection.OUT, minVolume: 4 })]; + + let controller: AssetController; + let assetService: DeepMocked; + let specRepo: TransactionSpecificationRepository; + let find: jest.SpyInstance; + let findCached: jest.SpyInstance; + + beforeAll(() => new ConfigService()); // sets module-level Config (AssetDtoMapper reads tradingLimits) + + beforeEach(() => { + assetService = createMock(); + specRepo = new TransactionSpecificationRepository(createMock()); + + find = jest.spyOn(specRepo, 'find').mockRejectedValue(new Error('uncached specification query')); + findCached = jest.spyOn(specRepo, 'findCached').mockResolvedValue(specs); + + assetService.getAllBlockchainAssets.mockResolvedValue([usdt]); + + controller = new AssetController(assetService, { + transactionSpecification: specRepo, + } as unknown as RepositoryFactory); + }); + + it('reads the specifications from the cache', async () => { + await controller.getAllAsset(undefined, { includePrivate: 'false' }); + + expect(findCached).toHaveBeenCalledTimes(1); + expect(findCached).toHaveBeenCalledWith('all'); + expect(find).not.toHaveBeenCalled(); + }); + + it('applies the cached specification to the returned limits', async () => { + const [dto] = await controller.getAllAsset(undefined, { includePrivate: 'false' }); + + expect(dto.limits.minVolume).toBe(4); + }); + + it('picks the outgoing specification of the asset', async () => { + findCached.mockResolvedValue([ + createSpec({ asset: 'USDT', direction: TransactionDirection.IN, minVolume: 222 }), + createSpec({ asset: 'USDT', direction: TransactionDirection.OUT, minVolume: 8 }), + ]); + + const [dto] = await controller.getAllAsset(undefined, { includePrivate: 'false' }); + + expect(dto.limits.minVolume).toBe(8); + }); +}); diff --git a/src/shared/models/asset/asset.controller.ts b/src/shared/models/asset/asset.controller.ts index b83479c299..5758d60562 100644 --- a/src/shared/models/asset/asset.controller.ts +++ b/src/shared/models/asset/asset.controller.ts @@ -36,7 +36,7 @@ export class AssetController { const queryBlockchains = blockchains?.split(',').map((value) => value as Blockchain); const specRepo = this.repoFactory.transactionSpecification; - const specs = await specRepo.find(); + const specs = await specRepo.findCached('all'); return this.assetService .getAllBlockchainAssets(queryBlockchains ?? jwt?.blockchains ?? [], includePrivate === 'true') diff --git a/src/subdomains/generic/kyc/controllers/kyc.controller.ts b/src/subdomains/generic/kyc/controllers/kyc.controller.ts index 67acea4473..539d1d1002 100644 --- a/src/subdomains/generic/kyc/controllers/kyc.controller.ts +++ b/src/subdomains/generic/kyc/controllers/kyc.controller.ts @@ -80,6 +80,9 @@ const MergedResponse = { type: MergedDto, }; const TfaResponse = { description: '2FA is required' }; +// Staff reaching a protected file without a completed identification; the body carries +// code STAFF_KYC_REQUIRED so clients can branch on it rather than on the message text. +const StaffKycResponse = { description: 'Staff access requires a completed identification' }; @ApiTags('KYC') @Controller({ path: 'kyc', version: [GetConfig().kycVersion] }) @@ -163,6 +166,7 @@ export class KycController { @Get('file/:id') @ApiBearerAuth() + @ApiForbiddenResponse(StaffKycResponse) @UseGuards(OptionalJwtAuthGuard) async getFile( @GetJwt() jwt: JwtPayload | undefined, diff --git a/src/subdomains/generic/kyc/services/__tests__/kyc.service.spec.ts b/src/subdomains/generic/kyc/services/__tests__/kyc.service.spec.ts index 582086c6aa..c0e621cfa7 100644 --- a/src/subdomains/generic/kyc/services/__tests__/kyc.service.spec.ts +++ b/src/subdomains/generic/kyc/services/__tests__/kyc.service.spec.ts @@ -3,6 +3,7 @@ import { ForbiddenException } from '@nestjs/common'; import { Configuration, ConfigService } from 'src/config/config'; import { BlobContent } from 'src/integration/infrastructure/storage/storage.service'; import { JwtPayload } from 'src/shared/auth/jwt-payload.interface'; +import { StaffKycRequiredException } from 'src/shared/auth/exceptions/staff-kyc-required.exception'; import { SetStaffKycClearance } from 'src/shared/auth/staff-kyc-clearance'; import { UserRole } from 'src/shared/auth/user-role.enum'; import { createCustomCountry } from 'src/shared/models/country/__mocks__/country.entity.mock'; @@ -162,6 +163,19 @@ describe('KycService getFileByUid protected-file access', () => { expect(documentService.downloadFile).not.toHaveBeenCalled(); }); + // The counterpart: a wrong role is a different problem with a different fix, so it must NOT produce + // the KYC answer — otherwise the two cases are indistinguishable again, just in the other direction. + it('answers a non-privileged role generically, not with the KYC code', async () => { + SetStaffKycClearance([]); + kycFileService.getKycFile.mockResolvedValue(kycFile()); + + const error = await service.getFileByUid('FILE-UID', jwtFor(UserRole.USER), ip).catch((e) => e); + + expect(error).toBeInstanceOf(ForbiddenException); + expect(error).not.toBeInstanceOf(StaffKycRequiredException); + expect(error.getResponse()).not.toHaveProperty('code'); + }); + it('forbids an unauthenticated request (no JWT) from a protected file', async () => { kycFileService.getKycFile.mockResolvedValue(kycFile()); @@ -177,10 +191,27 @@ describe('KycService getFileByUid protected-file access', () => { SetStaffKycClearance([]); kycFileService.getKycFile.mockResolvedValue(kycFile()); - await expect(service.getFileByUid('FILE-UID', jwtFor(role), ip)).rejects.toBeInstanceOf(ForbiddenException); + // Pins the concrete exception, not just the ForbiddenException it extends: the point of this + // answer is that the caller can tell a missing identification apart from a missing role, and an + // assertion on the base type would stay green if it fell back to the generic 403. + await expect(service.getFileByUid('FILE-UID', jwtFor(role), ip)).rejects.toBeInstanceOf( + StaffKycRequiredException, + ); expect(documentService.downloadFile).not.toHaveBeenCalled(); }); + it('answers with the machine-readable code', async () => { + SetStaffKycClearance([]); + kycFileService.getKycFile.mockResolvedValue(kycFile()); + + const error = await service.getFileByUid('FILE-UID', jwtFor(role), ip).catch((e) => e); + + expect(error.getResponse()).toEqual({ + code: 'STAFF_KYC_REQUIRED', + message: expect.stringContaining('KYC level 50'), + }); + }); + it('still serves a non-protected file', async () => { SetStaffKycClearance([]); kycFileService.getKycFile.mockResolvedValue(kycFile({ protected: false })); diff --git a/src/subdomains/supporting/payment/repositories/__tests__/transaction-specification.repository.spec.ts b/src/subdomains/supporting/payment/repositories/__tests__/transaction-specification.repository.spec.ts new file mode 100644 index 0000000000..e86e0bfb93 --- /dev/null +++ b/src/subdomains/supporting/payment/repositories/__tests__/transaction-specification.repository.spec.ts @@ -0,0 +1,99 @@ +import { createMock } from '@golevelup/ts-jest'; +import { EntityManager } from 'typeorm'; +import { TransactionDirection, TransactionSpecification } from '../../entities/transaction-specification.entity'; +import { TransactionSpecificationRepository } from '../transaction-specification.repository'; + +function createSpec(values: Partial): TransactionSpecification { + return Object.assign(new TransactionSpecification(), { system: 'Fiat', minVolume: 1, minFee: 0, ...values }); +} + +// The repository extends CachedRepository so that FiatController.getAllFiat and AssetController.getAllAsset +// stop issuing one query per request. These tests pin the caching behaviour itself, because the switch from +// find() to findCached() leaves the returned data unchanged. +describe('TransactionSpecificationRepository.findCached', () => { + const specs = [createSpec({ asset: 'EUR', direction: TransactionDirection.IN, minVolume: 5 })]; + + let repository: TransactionSpecificationRepository; + let find: jest.SpyInstance; + + beforeEach(() => { + repository = new TransactionSpecificationRepository(createMock()); + find = jest.spyOn(repository, 'find').mockResolvedValue(specs); + }); + + afterEach(() => { + jest.useRealTimers(); + }); + + it('queries the database once and serves the following calls from the cache', async () => { + await expect(repository.findCached('all')).resolves.toBe(specs); + await expect(repository.findCached('all')).resolves.toBe(specs); + await expect(repository.findCached('all')).resolves.toBe(specs); + + expect(find).toHaveBeenCalledTimes(1); + }); + + it('issues a single query for concurrent calls', async () => { + const [first, second] = await Promise.all([repository.findCached('all'), repository.findCached('all')]); + + expect(first).toBe(specs); + expect(second).toBe(specs); + expect(find).toHaveBeenCalledTimes(1); + }); + + it('queries again after invalidateCache', async () => { + await repository.findCached('all'); + + repository.invalidateCache(); + const updated = [createSpec({ asset: 'CHF', direction: TransactionDirection.IN, minVolume: 9 })]; + find.mockResolvedValue(updated); + + await expect(repository.findCached('all')).resolves.toBe(updated); + expect(find).toHaveBeenCalledTimes(2); + }); + + it('propagates a failing query instead of returning a stale or empty result', async () => { + find.mockRejectedValue(new Error('connection terminated')); + + await expect(repository.findCached('all')).rejects.toThrow('connection terminated'); + + find.mockResolvedValue(specs); + await expect(repository.findCached('all')).resolves.toBe(specs); + expect(find).toHaveBeenCalledTimes(2); + }); + + // The repository passes CacheItemResetPeriod.EVERY_HOUR explicitly. Dropping that argument would fall back + // to the CachedRepository default of 5 minutes without any other visible effect, so the window is asserted + // here: the first case fails on the default, the second one fails if the entry never expires. + it('serves the cached list well past the 5 minute default', async () => { + jest.useFakeTimers(); + + await repository.findCached('all'); + jest.advanceTimersByTime(59 * 60 * 1000); + await repository.findCached('all'); + + expect(find).toHaveBeenCalledTimes(1); + }); + + it('queries again once the hour has passed', async () => { + jest.useFakeTimers(); + + await repository.findCached('all'); + jest.advanceTimersByTime(61 * 60 * 1000); + await repository.findCached('all'); + + expect(find).toHaveBeenCalledTimes(2); + }); + + it('keeps a separate cache entry per key', async () => { + const inSpecs = [createSpec({ direction: TransactionDirection.IN })]; + const outSpecs = [createSpec({ direction: TransactionDirection.OUT })]; + find.mockResolvedValueOnce(inSpecs).mockResolvedValueOnce(outSpecs); + + await expect(repository.findCached('in')).resolves.toBe(inSpecs); + await expect(repository.findCached('out')).resolves.toBe(outSpecs); + await expect(repository.findCached('in')).resolves.toBe(inSpecs); + + expect(find).toHaveBeenCalledTimes(2); + }); +}); diff --git a/src/subdomains/supporting/payment/repositories/transaction-specification.repository.ts b/src/subdomains/supporting/payment/repositories/transaction-specification.repository.ts index d2ad59f615..c7ec06046f 100644 --- a/src/subdomains/supporting/payment/repositories/transaction-specification.repository.ts +++ b/src/subdomains/supporting/payment/repositories/transaction-specification.repository.ts @@ -1,15 +1,15 @@ import { Injectable } from '@nestjs/common'; import { Active, isAsset } from 'src/shared/models/active'; import { CachedRepository } from 'src/shared/repositories/cached.repository'; +import { CacheItemResetPeriod } from 'src/shared/utils/async-cache'; import { EntityManager } from 'typeorm'; import { TransactionDirection, TransactionSpecification } from '../entities/transaction-specification.entity'; -// Same cache duration as Fiat and Country (CachedRepository default EVERY_5_MINUTES). -// Pure reference/master data: no save/update/insert/delete path exists in the codebase. +// Cached for an hour, longer than the CachedRepository default of EVERY_5_MINUTES. @Injectable() export class TransactionSpecificationRepository extends CachedRepository { constructor(manager: EntityManager) { - super(TransactionSpecification, manager); + super(TransactionSpecification, manager, CacheItemResetPeriod.EVERY_HOUR); } getProps(param: Active): { system: string; asset: string } {