From 1bea27cfa697a57451cf941dac7c98198b9ad151 Mon Sep 17 00:00:00 2001 From: TaprootFreak <142087526+TaprootFreak@users.noreply.github.com> Date: Sat, 25 Jul 2026 18:46:13 +0200 Subject: [PATCH 01/11] feat(bank): add a receive-IBAN check for the support form The support form's "Receiver IBAN" field is a required dropdown filled from GET /bank, which lists only the shared bank accounts. Customers who deposit through a personal IBAN (virtual_iban) can never find their own receiving IBAN there, so today they are forced to pick a shared account they never transferred to - the ticket then carries a wrong IBAN that reads like a statement from the customer. The field is being replaced by free text, and this endpoint is what lets the frontend tell the customer straight away whether the IBAN they typed is one DFX receives money on. PUT /bank/receive-iban takes the IBAN in the body - never in the URL, so it stays out of access logs - and answers with one of four states: DfxIban, InvalidIban, UnknownIban, or LoginRequired. Two filters are deliberately absent. The bank lookup ignores `receive`, because a missing transfer is by nature an old one and a hit on a retired account is still money that reached DFX; filtering would tell a real customer their IBAN does not belong to DFX. The personal-IBAN lookup ignores the lifecycle state for the same reason - an expired or deactivated personal IBAN was still a real receiving IBAN. Personal IBANs are matched only against the requesting account. The guard is optional, so a global lookup would turn this into an unauthenticated oracle over customer-bound IBANs. That is why LoginRequired exists as its own state: without a login the personal IBANs stay unchecked, and answering UnknownIban there would be a false statement to a customer whose IBAN does exist. The endpoint is an input aid only - it enforces nothing, and issue creation still accepts any free text. Comparison runs on the normalised electronic format, since both the stored values and customer input carry arbitrary grouping spaces and casing. BankService takes the VirtualIbanRepository rather than the VirtualIbanService, because that service already depends on BankService and both live in the same module. --- .../bank/__tests__/bank.controller.spec.ts | 40 ++++++ .../bank/bank/__tests__/bank.service.spec.ts | 131 ++++++++++++++++++ .../supporting/bank/bank/bank.controller.ts | 20 ++- .../supporting/bank/bank/bank.service.ts | 48 ++++++- .../bank/bank/dto/receive-iban.dto.ts | 18 +++ .../bank/bank/dto/receive-iban.enum.ts | 6 + 6 files changed, 260 insertions(+), 3 deletions(-) create mode 100644 src/subdomains/supporting/bank/bank/__tests__/bank.controller.spec.ts create mode 100644 src/subdomains/supporting/bank/bank/dto/receive-iban.dto.ts create mode 100644 src/subdomains/supporting/bank/bank/dto/receive-iban.enum.ts diff --git a/src/subdomains/supporting/bank/bank/__tests__/bank.controller.spec.ts b/src/subdomains/supporting/bank/bank/__tests__/bank.controller.spec.ts new file mode 100644 index 0000000000..e32a56ab1a --- /dev/null +++ b/src/subdomains/supporting/bank/bank/__tests__/bank.controller.spec.ts @@ -0,0 +1,40 @@ +import { createMock, DeepMocked } from '@golevelup/ts-jest'; +import { JwtPayload } from 'src/shared/auth/jwt-payload.interface'; +import { UserRole } from 'src/shared/auth/user-role.enum'; +import { olkyEUR } from '../__mocks__/bank.entity.mock'; +import { BankController } from '../bank.controller'; +import { BankService } from '../bank.service'; +import { CheckReceiveIbanDto } from '../dto/receive-iban.dto'; +import { ReceiveIbanStatus } from '../dto/receive-iban.enum'; + +// The receive-iban check runs behind an optional guard, so the controller must forward the account of a +// present JWT and undefined otherwise - that distinction is what makes the service answer LoginRequired. +describe('BankController.checkReceiveIban', () => { + let controller: BankController; + let service: DeepMocked; + + const dto: CheckReceiveIbanDto = { iban: olkyEUR.iban }; + + beforeEach(() => { + service = createMock(); + controller = new BankController(service); + }); + + it('forwards the account of the authenticated customer and wraps the status', async () => { + service.getReceiveIbanStatus.mockResolvedValue(ReceiveIbanStatus.DFX_IBAN); + + const result = await controller.checkReceiveIban({ account: 42, role: UserRole.USER } as JwtPayload, dto); + + expect(service.getReceiveIbanStatus).toHaveBeenCalledWith(dto.iban, 42); + expect(result).toEqual({ status: ReceiveIbanStatus.DFX_IBAN }); + }); + + it('forwards undefined without a JWT', async () => { + service.getReceiveIbanStatus.mockResolvedValue(ReceiveIbanStatus.LOGIN_REQUIRED); + + const result = await controller.checkReceiveIban(undefined, dto); + + expect(service.getReceiveIbanStatus).toHaveBeenCalledWith(dto.iban, undefined); + expect(result).toEqual({ status: ReceiveIbanStatus.LOGIN_REQUIRED }); + }); +}); diff --git a/src/subdomains/supporting/bank/bank/__tests__/bank.service.spec.ts b/src/subdomains/supporting/bank/bank/__tests__/bank.service.spec.ts index 64667c5b93..0abf1f692a 100644 --- a/src/subdomains/supporting/bank/bank/__tests__/bank.service.spec.ts +++ b/src/subdomains/supporting/bank/bank/__tests__/bank.service.spec.ts @@ -12,8 +12,12 @@ import { createDefaultUserData } from 'src/subdomains/generic/user/models/user-d import { UserData } from 'src/subdomains/generic/user/models/user-data/user-data.entity'; import { UserService } from 'src/subdomains/generic/user/models/user/user.service'; import { BankAccountService } from 'src/subdomains/supporting/bank/bank-account/bank-account.service'; +import { createCustomVirtualIban } from 'src/subdomains/supporting/bank/virtual-iban/__mocks__/virtual-iban.entity.mock'; +import { VirtualIban, VirtualIbanStatus } from 'src/subdomains/supporting/bank/virtual-iban/virtual-iban.entity'; +import { VirtualIbanRepository } from 'src/subdomains/supporting/bank/virtual-iban/virtual-iban.repository'; import { FiatPaymentMethod } from 'src/subdomains/supporting/payment/dto/payment-method.enum'; import { + createCustomBank, createDefaultBanks, createDefaultDisabledBanks, yapealCHF, @@ -26,6 +30,7 @@ import { Bank } from '../bank.entity'; import { BankRepository } from '../bank.repository'; import { BankSelectorInput, BankService } from '../bank.service'; import { IbanBankName } from '../dto/bank.dto'; +import { ReceiveIbanStatus } from '../dto/receive-iban.enum'; function createBankSelectorInput( currency = 'EUR', @@ -69,6 +74,7 @@ describe('BankService', () => { { provide: FiatService, useValue: fiatService }, { provide: CountryService, useValue: countryService }, { provide: BankAccountService, useValue: bankAccountService }, + { provide: VirtualIbanRepository, useValue: createMock() }, TestUtil.provideConfig(), ], }).compile(); @@ -225,6 +231,7 @@ describe('Bank (name, currency) collision tie-break', () => { { provide: FiatService, useValue: createMock() }, { provide: CountryService, useValue: createMock() }, { provide: BankAccountService, useValue: createMock() }, + { provide: VirtualIbanRepository, useValue: createMock() }, TestUtil.provideConfig(), ], }).compile(); @@ -369,3 +376,127 @@ describe('Bank (name, currency) collision tie-break', () => { expect(BankService.isBankMatching(asset, 'YAPEAL-UNBOUND-NEWER-IBAN')).toBe(false); }); }); + +describe('BankService.getReceiveIbanStatus', () => { + const accountId = 42; + const otherAccountId = 43; + + // A retired collective account: same IBAN a customer may have transferred to years ago, but receive=false today. + const retiredCollectiveAccount = createCustomBank({ iban: 'CH5604835012345678009', receive: false, send: false }); + const personalIban = 'DE89370400440532013000'; + const expiredPersonalIban = 'AT483200000012345864'; + const foreignPersonalIban = 'CH4431999123000889012'; + + let service: BankService; + let bankRepo: BankRepository; + let virtualIbanRepo: VirtualIbanRepository; + + beforeEach(async () => { + bankRepo = createMock(); + virtualIbanRepo = createMock(); + + const module: TestingModule = await Test.createTestingModule({ + imports: [TestSharedModule], + providers: [ + BankService, + { provide: BankRepository, useValue: bankRepo }, + { provide: VirtualIbanRepository, useValue: virtualIbanRepo }, + TestUtil.provideConfig(), + ], + }).compile(); + + service = module.get(BankService); + }); + + function setup(banks: Bank[], virtualIbansByAccount: Map = new Map()) { + jest.spyOn(bankRepo, 'findCached').mockResolvedValue(banks); + jest + .spyOn(virtualIbanRepo, 'findCachedBy') + .mockImplementation(async (_key: string, where: any) => virtualIbansByAccount.get(where.userData.id) ?? []); + } + + it('reports a collective account IBAN as a DFX IBAN', async () => { + setup(createDefaultBanks()); + + await expect(service.getReceiveIbanStatus(olkyEUR.iban, accountId)).resolves.toBe(ReceiveIbanStatus.DFX_IBAN); + }); + + it('reports a collective account IBAN with receive=false as a DFX IBAN', async () => { + // The customer reports a missing, often old transfer - a retired or closed account still received DFX money. + setup([retiredCollectiveAccount]); + + await expect(service.getReceiveIbanStatus(retiredCollectiveAccount.iban, accountId)).resolves.toBe( + ReceiveIbanStatus.DFX_IBAN, + ); + }); + + it('reports a personal IBAN of the requesting account as a DFX IBAN', async () => { + setup(createDefaultBanks(), new Map([[accountId, [createCustomVirtualIban({ iban: personalIban })]]])); + + await expect(service.getReceiveIbanStatus(personalIban, accountId)).resolves.toBe(ReceiveIbanStatus.DFX_IBAN); + expect(virtualIbanRepo.findCachedBy).toHaveBeenCalledWith(`user-${accountId}`, { userData: { id: accountId } }); + }); + + it.each([VirtualIbanStatus.EXPIRED, VirtualIbanStatus.DEACTIVATED, VirtualIbanStatus.RESERVED])( + 'reports a personal IBAN with status %s as a DFX IBAN', + async (status) => { + // An expired personal IBAN was still a real receiving IBAN, so no lifecycle state may be filtered out. + setup( + createDefaultBanks(), + new Map([[accountId, [createCustomVirtualIban({ iban: expiredPersonalIban, active: false, status })]]]), + ); + + await expect(service.getReceiveIbanStatus(expiredPersonalIban, accountId)).resolves.toBe( + ReceiveIbanStatus.DFX_IBAN, + ); + }, + ); + + it('reports a formally invalid IBAN as invalid, without querying any IBAN', async () => { + setup(createDefaultBanks()); + + await expect(service.getReceiveIbanStatus('DE123456', accountId)).resolves.toBe(ReceiveIbanStatus.INVALID_IBAN); + expect(bankRepo.findCached).not.toHaveBeenCalled(); + expect(virtualIbanRepo.findCachedBy).not.toHaveBeenCalled(); + }); + + it('reports a valid IBAN that belongs to neither list as unknown when the customer is logged in', async () => { + setup(createDefaultBanks()); + + await expect(service.getReceiveIbanStatus(foreignPersonalIban, accountId)).resolves.toBe( + ReceiveIbanStatus.UNKNOWN_IBAN, + ); + }); + + it('requires a login for a valid unknown IBAN, because personal IBANs stay unchecked without one', async () => { + setup(createDefaultBanks()); + + await expect(service.getReceiveIbanStatus(foreignPersonalIban)).resolves.toBe(ReceiveIbanStatus.LOGIN_REQUIRED); + expect(virtualIbanRepo.findCachedBy).not.toHaveBeenCalled(); + }); + + it('recognizes the same IBAN written with grouping spaces and in lower case', async () => { + setup(createDefaultBanks(), new Map([[accountId, [createCustomVirtualIban({ iban: personalIban })]]])); + + await expect(service.getReceiveIbanStatus('lu11 6060 0020 0000 5040', accountId)).resolves.toBe( + ReceiveIbanStatus.DFX_IBAN, + ); + await expect(service.getReceiveIbanStatus('de89 3704 0044 0532 0130 00', accountId)).resolves.toBe( + ReceiveIbanStatus.DFX_IBAN, + ); + }); + + it('never reports a personal IBAN of another account as a DFX IBAN', async () => { + setup( + createDefaultBanks(), + new Map([ + [accountId, [createCustomVirtualIban({ iban: personalIban })]], + [otherAccountId, [createCustomVirtualIban({ iban: foreignPersonalIban })]], + ]), + ); + + await expect(service.getReceiveIbanStatus(foreignPersonalIban, accountId)).resolves.toBe( + ReceiveIbanStatus.UNKNOWN_IBAN, + ); + }); +}); diff --git a/src/subdomains/supporting/bank/bank/bank.controller.ts b/src/subdomains/supporting/bank/bank/bank.controller.ts index 2e974f332c..4368e71f0a 100644 --- a/src/subdomains/supporting/bank/bank/bank.controller.ts +++ b/src/subdomains/supporting/bank/bank/bank.controller.ts @@ -1,8 +1,12 @@ -import { Controller, Get } from '@nestjs/common'; -import { ApiOkResponse, ApiTags } from '@nestjs/swagger'; +import { Body, Controller, Get, Put, UseGuards } from '@nestjs/common'; +import { ApiBearerAuth, ApiOkResponse, ApiTags } from '@nestjs/swagger'; +import { GetJwt } from 'src/shared/auth/get-jwt.decorator'; +import { JwtPayload } from 'src/shared/auth/jwt-payload.interface'; +import { OptionalJwtAuthGuard } from 'src/shared/auth/optional.guard'; import { BankService } from './bank.service'; import { BankDto } from './dto/bank.dto'; import { BankMapper } from './dto/bank.mapper'; +import { CheckReceiveIbanDto, ReceiveIbanDto } from './dto/receive-iban.dto'; @ApiTags('Bank') @Controller('bank') @@ -16,4 +20,16 @@ export class BankController { return banks.map(BankMapper.toDto); } + + // PUT because the IBAN to check belongs in the body, never in the URL - this is a read, it changes nothing. + @Put('receive-iban') + @ApiBearerAuth() + @UseGuards(OptionalJwtAuthGuard) + @ApiOkResponse({ type: ReceiveIbanDto }) + async checkReceiveIban( + @GetJwt() jwt: JwtPayload | undefined, + @Body() dto: CheckReceiveIbanDto, + ): Promise { + return { status: await this.bankService.getReceiveIbanStatus(dto.iban, jwt?.account) }; + } } diff --git a/src/subdomains/supporting/bank/bank/bank.service.ts b/src/subdomains/supporting/bank/bank/bank.service.ts index ae5215727d..bd45805403 100644 --- a/src/subdomains/supporting/bank/bank/bank.service.ts +++ b/src/subdomains/supporting/bank/bank/bank.service.ts @@ -1,13 +1,16 @@ import { Injectable, OnModuleInit } from '@nestjs/common'; +import * as IbanTools from 'ibantools'; import { Blockchain } from 'src/integration/blockchain/shared/enums/blockchain.enum'; import { Asset } from 'src/shared/models/asset/asset.entity'; import { DfxLogger } from 'src/shared/services/dfx-logger'; import { Util } from 'src/shared/utils/util'; import { UserData } from 'src/subdomains/generic/user/models/user-data/user-data.entity'; +import { VirtualIbanRepository } from 'src/subdomains/supporting/bank/virtual-iban/virtual-iban.repository'; import { FiatPaymentMethod } from '../../payment/dto/payment-method.enum'; import { Bank } from './bank.entity'; import { BankRepository } from './bank.repository'; import { IbanBankName } from './dto/bank.dto'; +import { ReceiveIbanStatus } from './dto/receive-iban.enum'; export interface BankSelectorInput { amount?: number; @@ -21,7 +24,12 @@ export class BankService implements OnModuleInit { private readonly logger = new DfxLogger(BankService); private static ibanCache: Map = new Map(); // key: "bankName-currency", value: iban - constructor(private bankRepo: BankRepository) {} + // The VirtualIbanRepository is injected instead of the VirtualIbanService: that service depends on this + // one, and both live in BankModule, so the service-level dependency would close a provider cycle. + constructor( + private bankRepo: BankRepository, + private readonly virtualIbanRepo: VirtualIbanRepository, + ) {} onModuleInit() { void this.loadIbanCache(); @@ -110,6 +118,38 @@ export class BankService implements OnModuleInit { ); } + // --- RECEIVE IBAN CHECK --- // + + // Tells the client whether an IBAN typed in by a customer is one DFX receives customer money on. Pure + // input aid for the support form: it enforces nothing, it only lets the frontend phrase a helpful hint. + async getReceiveIbanStatus(iban: string, userDataId?: number): Promise { + const normalizedIban = BankService.normalizeIban(iban); + + if (!IbanTools.validateIBAN(normalizedIban).valid) return ReceiveIbanStatus.INVALID_IBAN; + + // Deliberately not filtered by `receive`: the customer is reporting a missing, often old transfer, so a + // hit on a retired or closed account is still money that went to DFX. A receive=true filter would tell a + // real customer that their IBAN does not belong to DFX. + const banks = await this.getAllBanks(); + if (banks.some((b) => BankService.normalizeIban(b.iban) === normalizedIban)) return ReceiveIbanStatus.DFX_IBAN; + + // Personal IBANs are only ever checked for the requesting account. The guard is optional, so a global + // lookup would turn this endpoint into an unauthenticated oracle over customer-bound IBANs. Without a + // login the personal IBANs stay unchecked, hence the answer must never be UNKNOWN_IBAN here. + if (!userDataId) return ReceiveIbanStatus.LOGIN_REQUIRED; + + // No lifecycle filter either (active=false, status Expired/Deactivated/Reserved all count): an expired + // personal IBAN was still a real receiving IBAN. Same cache key and filter as + // VirtualIbanService.getVirtualIbansForAccount, so both paths share the cached list. + const virtualIbans = await this.virtualIbanRepo.findCachedBy(`user-${userDataId}`, { + userData: { id: userDataId }, + }); + if (virtualIbans.some((v) => BankService.normalizeIban(v.iban) === normalizedIban)) + return ReceiveIbanStatus.DFX_IBAN; + + return ReceiveIbanStatus.UNKNOWN_IBAN; + } + static isBankMatching(asset: Asset, accountIban: string): boolean { const bankName = this.blockchainToBankName(asset.blockchain); if (!bankName) return false; @@ -120,6 +160,12 @@ export class BankService implements OnModuleInit { // --- HELPER METHODS --- // + // IBANs are stored and typed in with arbitrary grouping spaces and casing (the bank table itself holds + // both formats), so every comparison runs on the electronic format. + private static normalizeIban(iban: string): string { + return iban.replace(/\s/g, '').toUpperCase(); + } + // Picks the bank row that owns attribution for a single (name, currency) key. `banks` must already // be sorted by id descending (newest first). Prefer a row linked to an asset: that binding is the // basis of every per-asset match (isBankMatching) and of the IBAN already present on booked diff --git a/src/subdomains/supporting/bank/bank/dto/receive-iban.dto.ts b/src/subdomains/supporting/bank/bank/dto/receive-iban.dto.ts new file mode 100644 index 0000000000..f88ce7d8a8 --- /dev/null +++ b/src/subdomains/supporting/bank/bank/dto/receive-iban.dto.ts @@ -0,0 +1,18 @@ +import { ApiProperty } from '@nestjs/swagger'; +import { Transform } from 'class-transformer'; +import { IsNotEmpty, IsString } from 'class-validator'; +import { Util } from 'src/shared/utils/util'; +import { ReceiveIbanStatus } from './receive-iban.enum'; + +export class CheckReceiveIbanDto { + @ApiProperty() + @IsNotEmpty() + @IsString() + @Transform(Util.sanitize) + iban: string; +} + +export class ReceiveIbanDto { + @ApiProperty({ enum: ReceiveIbanStatus }) + status: ReceiveIbanStatus; +} diff --git a/src/subdomains/supporting/bank/bank/dto/receive-iban.enum.ts b/src/subdomains/supporting/bank/bank/dto/receive-iban.enum.ts new file mode 100644 index 0000000000..e836a713e8 --- /dev/null +++ b/src/subdomains/supporting/bank/bank/dto/receive-iban.enum.ts @@ -0,0 +1,6 @@ +export enum ReceiveIbanStatus { + DFX_IBAN = 'DfxIban', + UNKNOWN_IBAN = 'UnknownIban', + INVALID_IBAN = 'InvalidIban', + LOGIN_REQUIRED = 'LoginRequired', +} From e2037dec63d722f6ca4f8a047d4072ac141264b5 Mon Sep 17 00:00:00 2001 From: TaprootFreak <142087526+TaprootFreak@users.noreply.github.com> Date: Sat, 25 Jul 2026 19:15:17 +0200 Subject: [PATCH 02/11] fix(bank): correct IBAN normalization and input handling in the receive-IBAN check Review turned up four real defects in the first commit. Normalization stripped only whitespace, so a customer pasting an IBAN with hyphen grouping got InvalidIban for a perfectly valid IBAN. ibantools ships electronicFormatIBAN, which strips both spaces and hyphens and uppercases, so the hand-rolled helper is gone in favour of the library. It also returns null for a non-string, which is now treated like any other unusable input - and it makes the stored side null-safe, where the old regex would have thrown. The DTO no longer carries @Transform(Util.sanitize). That transform runs before @IsString, and Util.sanitizeString calls value.trim() unguarded, so a body such as {"iban": 123} threw a TypeError that the exception filter turned into a 500 - on an endpoint that is reachable without a login. HTML sanitizing is pointless for an IBAN that is structurally validated and normalized anyway. @IsString now rejects a non-string with a clean 400. A comment records why the transform must not come back. UnknownIban was renamed to NotMatched, because the old name asserted more than the check knows. For an authenticated caller the state means "could not attribute this", not "does not belong to DFX": personal IBANs of other accounts are deliberately never checked, and mergeUserData does not move virtual_iban rows to the master, so a customer's own older personal IBAN can land there too. Every state is now documented in the enum, including that NotMatched makes no claim about DFX ownership. The endpoint is reachable unauthenticated, so it now runs behind RateLimitGuard, placed first as in the existing public endpoints. Two tests were added: hyphen-grouped input resolves to DfxIban, and unusable input yields InvalidIban instead of throwing. --- .../bank/bank/__tests__/bank.service.spec.ts | 31 ++++++++++++++++--- .../supporting/bank/bank/bank.controller.ts | 4 ++- .../supporting/bank/bank/bank.service.ts | 22 ++++++------- .../bank/bank/dto/receive-iban.dto.ts | 6 ++-- .../bank/bank/dto/receive-iban.enum.ts | 14 ++++++++- 5 files changed, 55 insertions(+), 22 deletions(-) diff --git a/src/subdomains/supporting/bank/bank/__tests__/bank.service.spec.ts b/src/subdomains/supporting/bank/bank/__tests__/bank.service.spec.ts index 0abf1f692a..0d2ef77b19 100644 --- a/src/subdomains/supporting/bank/bank/__tests__/bank.service.spec.ts +++ b/src/subdomains/supporting/bank/bank/__tests__/bank.service.spec.ts @@ -460,15 +460,27 @@ describe('BankService.getReceiveIbanStatus', () => { expect(virtualIbanRepo.findCachedBy).not.toHaveBeenCalled(); }); - it('reports a valid IBAN that belongs to neither list as unknown when the customer is logged in', async () => { + it.each([undefined, null, '', ' '])( + 'reports an unusable input (%p) as invalid instead of throwing', + async (input) => { + // electronicFormatIBAN returns null for a non-string and an empty string for blank input. + setup(createDefaultBanks()); + + await expect(service.getReceiveIbanStatus(input as unknown as string, accountId)).resolves.toBe( + ReceiveIbanStatus.INVALID_IBAN, + ); + }, + ); + + it('reports a valid IBAN that matched neither list as not matched when the customer is logged in', async () => { setup(createDefaultBanks()); await expect(service.getReceiveIbanStatus(foreignPersonalIban, accountId)).resolves.toBe( - ReceiveIbanStatus.UNKNOWN_IBAN, + ReceiveIbanStatus.NOT_MATCHED, ); }); - it('requires a login for a valid unknown IBAN, because personal IBANs stay unchecked without one', async () => { + it('requires a login for a valid unmatched IBAN, because personal IBANs stay unchecked without one', async () => { setup(createDefaultBanks()); await expect(service.getReceiveIbanStatus(foreignPersonalIban)).resolves.toBe(ReceiveIbanStatus.LOGIN_REQUIRED); @@ -486,6 +498,17 @@ describe('BankService.getReceiveIbanStatus', () => { ); }); + it('recognizes the same IBAN written with hyphen grouping', async () => { + setup([frickEUR], new Map([[accountId, [createCustomVirtualIban({ iban: personalIban })]]])); + + await expect(service.getReceiveIbanStatus('LI75-0881-1010-5923-K000E', accountId)).resolves.toBe( + ReceiveIbanStatus.DFX_IBAN, + ); + await expect(service.getReceiveIbanStatus('de89-3704-0044-0532-0130-00', accountId)).resolves.toBe( + ReceiveIbanStatus.DFX_IBAN, + ); + }); + it('never reports a personal IBAN of another account as a DFX IBAN', async () => { setup( createDefaultBanks(), @@ -496,7 +519,7 @@ describe('BankService.getReceiveIbanStatus', () => { ); await expect(service.getReceiveIbanStatus(foreignPersonalIban, accountId)).resolves.toBe( - ReceiveIbanStatus.UNKNOWN_IBAN, + ReceiveIbanStatus.NOT_MATCHED, ); }); }); diff --git a/src/subdomains/supporting/bank/bank/bank.controller.ts b/src/subdomains/supporting/bank/bank/bank.controller.ts index 4368e71f0a..9df29c531c 100644 --- a/src/subdomains/supporting/bank/bank/bank.controller.ts +++ b/src/subdomains/supporting/bank/bank/bank.controller.ts @@ -3,6 +3,7 @@ import { ApiBearerAuth, ApiOkResponse, ApiTags } from '@nestjs/swagger'; import { GetJwt } from 'src/shared/auth/get-jwt.decorator'; import { JwtPayload } from 'src/shared/auth/jwt-payload.interface'; import { OptionalJwtAuthGuard } from 'src/shared/auth/optional.guard'; +import { RateLimitGuard } from 'src/shared/auth/rate-limit.guard'; import { BankService } from './bank.service'; import { BankDto } from './dto/bank.dto'; import { BankMapper } from './dto/bank.mapper'; @@ -24,7 +25,8 @@ export class BankController { // PUT because the IBAN to check belongs in the body, never in the URL - this is a read, it changes nothing. @Put('receive-iban') @ApiBearerAuth() - @UseGuards(OptionalJwtAuthGuard) + // RateLimitGuard first: the JWT is optional, so this endpoint is reachable unauthenticated. + @UseGuards(RateLimitGuard, OptionalJwtAuthGuard) @ApiOkResponse({ type: ReceiveIbanDto }) async checkReceiveIban( @GetJwt() jwt: JwtPayload | undefined, diff --git a/src/subdomains/supporting/bank/bank/bank.service.ts b/src/subdomains/supporting/bank/bank/bank.service.ts index bd45805403..db2d49b54f 100644 --- a/src/subdomains/supporting/bank/bank/bank.service.ts +++ b/src/subdomains/supporting/bank/bank/bank.service.ts @@ -123,19 +123,21 @@ export class BankService implements OnModuleInit { // Tells the client whether an IBAN typed in by a customer is one DFX receives customer money on. Pure // input aid for the support form: it enforces nothing, it only lets the frontend phrase a helpful hint. async getReceiveIbanStatus(iban: string, userDataId?: number): Promise { - const normalizedIban = BankService.normalizeIban(iban); - - if (!IbanTools.validateIBAN(normalizedIban).valid) return ReceiveIbanStatus.INVALID_IBAN; + // electronicFormatIBAN strips grouping spaces *and* hyphens and uppercases, so a customer pasting + // `LI75-0881-1010-5923-K000E` is understood. It returns null for a non-string input; treat that and an + // empty result like any other unusable input. Both sides of every comparison run through it. + const normalizedIban = IbanTools.electronicFormatIBAN(iban); + if (!normalizedIban || !IbanTools.validateIBAN(normalizedIban).valid) return ReceiveIbanStatus.INVALID_IBAN; // Deliberately not filtered by `receive`: the customer is reporting a missing, often old transfer, so a // hit on a retired or closed account is still money that went to DFX. A receive=true filter would tell a // real customer that their IBAN does not belong to DFX. const banks = await this.getAllBanks(); - if (banks.some((b) => BankService.normalizeIban(b.iban) === normalizedIban)) return ReceiveIbanStatus.DFX_IBAN; + if (banks.some((b) => IbanTools.electronicFormatIBAN(b.iban) === normalizedIban)) return ReceiveIbanStatus.DFX_IBAN; // Personal IBANs are only ever checked for the requesting account. The guard is optional, so a global // lookup would turn this endpoint into an unauthenticated oracle over customer-bound IBANs. Without a - // login the personal IBANs stay unchecked, hence the answer must never be UNKNOWN_IBAN here. + // login the personal IBANs stay unchecked, hence the answer must never be NOT_MATCHED here. if (!userDataId) return ReceiveIbanStatus.LOGIN_REQUIRED; // No lifecycle filter either (active=false, status Expired/Deactivated/Reserved all count): an expired @@ -144,10 +146,10 @@ export class BankService implements OnModuleInit { const virtualIbans = await this.virtualIbanRepo.findCachedBy(`user-${userDataId}`, { userData: { id: userDataId }, }); - if (virtualIbans.some((v) => BankService.normalizeIban(v.iban) === normalizedIban)) + if (virtualIbans.some((v) => IbanTools.electronicFormatIBAN(v.iban) === normalizedIban)) return ReceiveIbanStatus.DFX_IBAN; - return ReceiveIbanStatus.UNKNOWN_IBAN; + return ReceiveIbanStatus.NOT_MATCHED; } static isBankMatching(asset: Asset, accountIban: string): boolean { @@ -160,12 +162,6 @@ export class BankService implements OnModuleInit { // --- HELPER METHODS --- // - // IBANs are stored and typed in with arbitrary grouping spaces and casing (the bank table itself holds - // both formats), so every comparison runs on the electronic format. - private static normalizeIban(iban: string): string { - return iban.replace(/\s/g, '').toUpperCase(); - } - // Picks the bank row that owns attribution for a single (name, currency) key. `banks` must already // be sorted by id descending (newest first). Prefer a row linked to an asset: that binding is the // basis of every per-asset match (isBankMatching) and of the IBAN already present on booked diff --git a/src/subdomains/supporting/bank/bank/dto/receive-iban.dto.ts b/src/subdomains/supporting/bank/bank/dto/receive-iban.dto.ts index f88ce7d8a8..ffad168d54 100644 --- a/src/subdomains/supporting/bank/bank/dto/receive-iban.dto.ts +++ b/src/subdomains/supporting/bank/bank/dto/receive-iban.dto.ts @@ -1,14 +1,14 @@ import { ApiProperty } from '@nestjs/swagger'; -import { Transform } from 'class-transformer'; import { IsNotEmpty, IsString } from 'class-validator'; -import { Util } from 'src/shared/utils/util'; import { ReceiveIbanStatus } from './receive-iban.enum'; +// No @Transform(Util.sanitize) here: it would run before @IsString and throw on a non-string body, which the +// exception filter turns into a 500 on this unauthenticated endpoint. HTML sanitizing is pointless for an IBAN +// anyway - the service normalizes and structurally validates it. export class CheckReceiveIbanDto { @ApiProperty() @IsNotEmpty() @IsString() - @Transform(Util.sanitize) iban: string; } diff --git a/src/subdomains/supporting/bank/bank/dto/receive-iban.enum.ts b/src/subdomains/supporting/bank/bank/dto/receive-iban.enum.ts index e836a713e8..b231b35d99 100644 --- a/src/subdomains/supporting/bank/bank/dto/receive-iban.enum.ts +++ b/src/subdomains/supporting/bank/bank/dto/receive-iban.enum.ts @@ -1,6 +1,18 @@ export enum ReceiveIbanStatus { + // An IBAN DFX receives customer money on: either a collective account from the bank table, or a personal + // deposit IBAN of the requesting account. Lifecycle state is irrelevant - a retired collective account and + // an expired personal IBAN both received real customer money. DFX_IBAN = 'DfxIban', - UNKNOWN_IBAN = 'UnknownIban', + + // The IBAN could not be attributed for this caller. This does NOT claim that the IBAN does not belong to + // DFX: personal IBANs of other accounts are deliberately never checked, and an account merge leaves the + // virtual_iban rows on the former account, so a customer's own older personal IBAN can land here as well. + NOT_MATCHED = 'NotMatched', + + // The input is not a structurally valid IBAN (country, length or checksum), so there is nothing to look up. INVALID_IBAN = 'InvalidIban', + + // No collective account matched, and personal IBANs are only ever checked for the authenticated account, so + // without a login the check stays incomplete. Never answered as NOT_MATCHED, which would overstate it. LOGIN_REQUIRED = 'LoginRequired', } From dc488bf8d45aafef7b8b804b661336d52ff337fb Mon Sep 17 00:00:00 2001 From: TaprootFreak <142087526+TaprootFreak@users.noreply.github.com> Date: Sat, 25 Jul 2026 20:12:36 +0200 Subject: [PATCH 03/11] fix(bank): accept every whitespace style when normalizing an IBAN Switching to electronicFormatIBAN in the previous commit traded one gap for another. The library strips only ASCII spaces and hyphens; the hand-rolled helper it replaced stripped every kind of whitespace but no hyphens. Measured against ibantools 4.5.1 with a valid IBAN in different groupings, the library alone rejects a non-breaking space, a narrow non-breaking space, a tab and a line break, while accepting ASCII spaces and hyphens. That is not an edge case for this endpoint. An IBAN pasted out of a PDF statement or off a web page very often carries a non-breaking space as its grouping character, and the whole point of the free-text field is that customers paste what they have. Such a customer would have been told their perfectly valid IBAN is invalid. The normalization helper is back, now doing both: it strips all whitespace itself and then hands the value to electronicFormatIBAN, which removes hyphens, uppercases, and returns null for anything unusable. A typeof guard keeps that null-safety for stored values as well. All three comparison sites - the input, bank.iban and virtualIban.iban - run through it. The comment that implied the library normalized completely is corrected. Four cases were added covering non-breaking space, narrow non-breaking space, tab and line break, for a collective account and for a personal IBAN. The separators are written as escape sequences so no invisible character can be lost while editing. The guard was checked by reverting the helper to the library-only form: exactly those four cases fail, and pass again once it is restored. --- .../bank/bank/__tests__/bank.service.spec.ts | 23 +++++++++++++++++++ .../supporting/bank/bank/bank.service.ts | 18 ++++++++++----- 2 files changed, 35 insertions(+), 6 deletions(-) diff --git a/src/subdomains/supporting/bank/bank/__tests__/bank.service.spec.ts b/src/subdomains/supporting/bank/bank/__tests__/bank.service.spec.ts index 0d2ef77b19..f04f32fd2d 100644 --- a/src/subdomains/supporting/bank/bank/__tests__/bank.service.spec.ts +++ b/src/subdomains/supporting/bank/bank/__tests__/bank.service.spec.ts @@ -509,6 +509,29 @@ describe('BankService.getReceiveIbanStatus', () => { ); }); + // Separators written as escape sequences on purpose: as literal characters they are invisible and an editor + // or a copy-paste would silently turn them back into ordinary spaces, which would void these cases. + it.each([ + ['a non-breaking space', '\u00a0'], + ['a narrow non-breaking space', '\u202f'], + ['a tab', '\t'], + ['a line break', '\n'], + ])( + 'recognizes an IBAN grouped with %s, which the ibantools formatter alone does not strip', + async (_name, separator) => { + setup([frickEUR], new Map([[accountId, [createCustomVirtualIban({ iban: personalIban })]]])); + + const group = (iban: string) => (iban.match(/.{1,4}/g) ?? []).join(separator); + + await expect(service.getReceiveIbanStatus(group(frickEUR.iban), accountId)).resolves.toBe( + ReceiveIbanStatus.DFX_IBAN, + ); + await expect(service.getReceiveIbanStatus(group(personalIban), accountId)).resolves.toBe( + ReceiveIbanStatus.DFX_IBAN, + ); + }, + ); + it('never reports a personal IBAN of another account as a DFX IBAN', async () => { setup( createDefaultBanks(), diff --git a/src/subdomains/supporting/bank/bank/bank.service.ts b/src/subdomains/supporting/bank/bank/bank.service.ts index db2d49b54f..0319e6fda6 100644 --- a/src/subdomains/supporting/bank/bank/bank.service.ts +++ b/src/subdomains/supporting/bank/bank/bank.service.ts @@ -123,17 +123,16 @@ export class BankService implements OnModuleInit { // Tells the client whether an IBAN typed in by a customer is one DFX receives customer money on. Pure // input aid for the support form: it enforces nothing, it only lets the frontend phrase a helpful hint. async getReceiveIbanStatus(iban: string, userDataId?: number): Promise { - // electronicFormatIBAN strips grouping spaces *and* hyphens and uppercases, so a customer pasting - // `LI75-0881-1010-5923-K000E` is understood. It returns null for a non-string input; treat that and an - // empty result like any other unusable input. Both sides of every comparison run through it. - const normalizedIban = IbanTools.electronicFormatIBAN(iban); + // normalizeIban absorbs every grouping style a customer may paste and yields null for unusable input; + // treat null and an empty result alike. Both sides of every comparison run through it. + const normalizedIban = BankService.normalizeIban(iban); if (!normalizedIban || !IbanTools.validateIBAN(normalizedIban).valid) return ReceiveIbanStatus.INVALID_IBAN; // Deliberately not filtered by `receive`: the customer is reporting a missing, often old transfer, so a // hit on a retired or closed account is still money that went to DFX. A receive=true filter would tell a // real customer that their IBAN does not belong to DFX. const banks = await this.getAllBanks(); - if (banks.some((b) => IbanTools.electronicFormatIBAN(b.iban) === normalizedIban)) return ReceiveIbanStatus.DFX_IBAN; + if (banks.some((b) => BankService.normalizeIban(b.iban) === normalizedIban)) return ReceiveIbanStatus.DFX_IBAN; // Personal IBANs are only ever checked for the requesting account. The guard is optional, so a global // lookup would turn this endpoint into an unauthenticated oracle over customer-bound IBANs. Without a @@ -146,7 +145,7 @@ export class BankService implements OnModuleInit { const virtualIbans = await this.virtualIbanRepo.findCachedBy(`user-${userDataId}`, { userData: { id: userDataId }, }); - if (virtualIbans.some((v) => IbanTools.electronicFormatIBAN(v.iban) === normalizedIban)) + if (virtualIbans.some((v) => BankService.normalizeIban(v.iban) === normalizedIban)) return ReceiveIbanStatus.DFX_IBAN; return ReceiveIbanStatus.NOT_MATCHED; @@ -162,6 +161,13 @@ export class BankService implements OnModuleInit { // --- HELPER METHODS --- // + // Strip every kind of whitespace ourselves - electronicFormatIBAN only removes ASCII spaces and hyphens, + // so a value pasted from a statement or a web page (NBSP, narrow NBSP, tab) would otherwise fail + // validation. The library call then handles hyphens, casing, and a non-string input by returning null. + private static normalizeIban(iban: string): string | null { + return typeof iban === 'string' ? IbanTools.electronicFormatIBAN(iban.replace(/\s/g, '')) : null; + } + // Picks the bank row that owns attribution for a single (name, currency) key. `banks` must already // be sorted by id descending (newest first). Prefer a row linked to an asset: that binding is the // basis of every per-asset match (isBankMatching) and of the IBAN already present on booked From ce14473559acbe4b33a284629b2168ade2687efb Mon Sep 17 00:00:00 2001 From: TaprootFreak <142087526+TaprootFreak@users.noreply.github.com> Date: Sat, 25 Jul 2026 20:46:48 +0200 Subject: [PATCH 04/11] fix(bank): normalize an IBAN by allow-list and close the untested branches Three safeguards of the receive-IBAN check turned out to be unpinned: inverting them left every test green. The branch order that answers a logged-out customer with DfxIban on a collective account - the single most common path through this endpoint - could be swapped for LoginRequired unnoticed. Checksum validation could be reduced to a shape check, which would answer a customer who mistyped a digit with NotMatched, telling them their IBAN is not ours instead of that it has a typo. And normalization could be dropped from the stored side of either comparison, which matters for personal IBANs because those values are persisted straight from the provider response without validation. One test each now pins these, and each mutation is caught by exactly its own test. Normalization changes approach rather than gaining another character. Stripping whitespace missed the zero-width family (U+200B, U+200D, U+2060, the direction marks) and the soft hyphen, none of which JavaScript counts as whitespace, plus dots, slashes and quotes. That was the third defect in the same function, so the deny-list of separators is replaced by an allow-list of what an IBAN may contain: letters and digits, nothing else. That is complete by construction. An IBAN: prefix stays invalid, which is correct - it is not a separator problem. Three comments overstated what they knew and are corrected. The normalization comment no longer promises to absorb every pasted form. The rate-limit comment no longer promises protection: ThrottlerModule.forRoot() is called without options, so limit is undefined and the guard's comparison is always false - the guard is inert until that is fixed separately, and the comment now only explains the ordering. The DTO comment now names the actual cause of the 500 it avoids, an unguarded value.trim() in Util.sanitizeString, rather than arguing that HTML sanitizing is pointless for an IBAN. Also moved the section header so it no longer encloses the unrelated isBankMatching, and made the constructor uniform by adding the missing readonly. --- .../bank/bank/__tests__/bank.service.spec.ts | 98 ++++++++++++++----- .../supporting/bank/bank/bank.controller.ts | 2 +- .../supporting/bank/bank/bank.service.ts | 35 ++++--- .../bank/bank/dto/receive-iban.dto.ts | 7 +- 4 files changed, 99 insertions(+), 43 deletions(-) diff --git a/src/subdomains/supporting/bank/bank/__tests__/bank.service.spec.ts b/src/subdomains/supporting/bank/bank/__tests__/bank.service.spec.ts index f04f32fd2d..6c1c55debe 100644 --- a/src/subdomains/supporting/bank/bank/__tests__/bank.service.spec.ts +++ b/src/subdomains/supporting/bank/bank/__tests__/bank.service.spec.ts @@ -421,6 +421,34 @@ describe('BankService.getReceiveIbanStatus', () => { await expect(service.getReceiveIbanStatus(olkyEUR.iban, accountId)).resolves.toBe(ReceiveIbanStatus.DFX_IBAN); }); + it('reports a collective account IBAN as a DFX IBAN without a login, before ever asking for personal IBANs', async () => { + // The most common case by far: a logged-out customer types a collective account IBAN. The bank check must + // run before the login check, otherwise this answers LoginRequired for an IBAN we can already confirm. + setup(createDefaultBanks()); + + await expect(service.getReceiveIbanStatus(olkyEUR.iban)).resolves.toBe(ReceiveIbanStatus.DFX_IBAN); + expect(virtualIbanRepo.findCachedBy).not.toHaveBeenCalled(); + }); + + it('reports a collective account IBAN stored in paper format as a DFX IBAN', async () => { + // The stored side is normalized too - bank.iban is maintained by hand and holds grouped values. + setup([createCustomBank({ iban: 'LU11 6060 0020 0000 5040' })]); + + await expect(service.getReceiveIbanStatus('LU116060002000005040', accountId)).resolves.toBe( + ReceiveIbanStatus.DFX_IBAN, + ); + }); + + it('reports a personal IBAN stored in paper format as a DFX IBAN', async () => { + // virtual_iban values arrive unvalidated from provider.reserveViban(), so their format is not guaranteed. + setup( + createDefaultBanks(), + new Map([[accountId, [createCustomVirtualIban({ iban: 'de89 3704 0044 0532 0130 00' })]]]), + ); + + await expect(service.getReceiveIbanStatus(personalIban, accountId)).resolves.toBe(ReceiveIbanStatus.DFX_IBAN); + }); + it('reports a collective account IBAN with receive=false as a DFX IBAN', async () => { // The customer reports a missing, often old transfer - a retired or closed account still received DFX money. setup([retiredCollectiveAccount]); @@ -460,10 +488,22 @@ describe('BankService.getReceiveIbanStatus', () => { expect(virtualIbanRepo.findCachedBy).not.toHaveBeenCalled(); }); + it('reports a correctly shaped IBAN with a wrong checksum as invalid, not as unmatched', async () => { + // A transposed digit keeps the country and length intact, so only the checksum catches it. Answering + // NotMatched here would send a customer looking for a transfer that never left with a typo in the IBAN. + setup(createDefaultBanks()); + + await expect(service.getReceiveIbanStatus('DE89370400440532013001', accountId)).resolves.toBe( + ReceiveIbanStatus.INVALID_IBAN, + ); + }); + it.each([undefined, null, '', ' '])( 'reports an unusable input (%p) as invalid instead of throwing', async (input) => { - // electronicFormatIBAN returns null for a non-string and an empty string for blank input. + // Defensive only: @IsString/@IsNotEmpty reject undefined, null and '' with a 400 before the service is + // reached, so of these only ' ' can actually arrive. The typeof guard in normalizeIban short-circuits + // the non-string cases, and an all-separator string normalizes to '' and is returned as null. setup(createDefaultBanks()); await expect(service.getReceiveIbanStatus(input as unknown as string, accountId)).resolves.toBe( @@ -498,39 +538,49 @@ describe('BankService.getReceiveIbanStatus', () => { ); }); - it('recognizes the same IBAN written with hyphen grouping', async () => { + // The invisible separators are written as escape sequences on purpose: as literal characters an editor or a + // copy-paste would silently turn them back into ordinary spaces, which would void exactly those cases. + it.each([ + ['an ASCII space', ' '], + ['a hyphen', '-'], + ['a dot', '.'], + ['a slash', '/'], + ['a non-breaking space', '\u00a0'], + ['a narrow non-breaking space', '\u202f'], + ['a zero-width space', '\u200b'], + ['a soft hyphen', '\u00ad'], + ['a tab', '\t'], + ['a line break', '\n'], + ])('recognizes an IBAN grouped with %s', async (_name, separator) => { setup([frickEUR], new Map([[accountId, [createCustomVirtualIban({ iban: personalIban })]]])); - await expect(service.getReceiveIbanStatus('LI75-0881-1010-5923-K000E', accountId)).resolves.toBe( + const group = (iban: string) => (iban.match(/.{1,4}/g) ?? []).join(separator); + + await expect(service.getReceiveIbanStatus(group(frickEUR.iban), accountId)).resolves.toBe( ReceiveIbanStatus.DFX_IBAN, ); - await expect(service.getReceiveIbanStatus('de89-3704-0044-0532-0130-00', accountId)).resolves.toBe( + await expect(service.getReceiveIbanStatus(group(personalIban), accountId)).resolves.toBe( ReceiveIbanStatus.DFX_IBAN, ); }); - // Separators written as escape sequences on purpose: as literal characters they are invisible and an editor - // or a copy-paste would silently turn them back into ordinary spaces, which would void these cases. - it.each([ - ['a non-breaking space', '\u00a0'], - ['a narrow non-breaking space', '\u202f'], - ['a tab', '\t'], - ['a line break', '\n'], - ])( - 'recognizes an IBAN grouped with %s, which the ibantools formatter alone does not strip', - async (_name, separator) => { - setup([frickEUR], new Map([[accountId, [createCustomVirtualIban({ iban: personalIban })]]])); + it('recognizes an IBAN pasted with surrounding quotes', async () => { + setup([frickEUR]); + + await expect(service.getReceiveIbanStatus('"LI75 0881 1010 5923 K000E"', accountId)).resolves.toBe( + ReceiveIbanStatus.DFX_IBAN, + ); + }); - const group = (iban: string) => (iban.match(/.{1,4}/g) ?? []).join(separator); + it('does not extract an IBAN out of surrounding words', async () => { + // The allow-list strips separators, it does not salvage an IBAN from a labelled value - and it must not, + // because a prefix is indistinguishable from extra characters that make the IBAN wrong. + setup([frickEUR]); - await expect(service.getReceiveIbanStatus(group(frickEUR.iban), accountId)).resolves.toBe( - ReceiveIbanStatus.DFX_IBAN, - ); - await expect(service.getReceiveIbanStatus(group(personalIban), accountId)).resolves.toBe( - ReceiveIbanStatus.DFX_IBAN, - ); - }, - ); + await expect(service.getReceiveIbanStatus('IBAN: LI75 0881 1010 5923 K000E', accountId)).resolves.toBe( + ReceiveIbanStatus.INVALID_IBAN, + ); + }); it('never reports a personal IBAN of another account as a DFX IBAN', async () => { setup( diff --git a/src/subdomains/supporting/bank/bank/bank.controller.ts b/src/subdomains/supporting/bank/bank/bank.controller.ts index 9df29c531c..efd38e0341 100644 --- a/src/subdomains/supporting/bank/bank/bank.controller.ts +++ b/src/subdomains/supporting/bank/bank/bank.controller.ts @@ -25,7 +25,7 @@ export class BankController { // PUT because the IBAN to check belongs in the body, never in the URL - this is a read, it changes nothing. @Put('receive-iban') @ApiBearerAuth() - // RateLimitGuard first: the JWT is optional, so this endpoint is reachable unauthenticated. + // RateLimitGuard before the auth guard so it also sees requests that arrive without a JWT. @UseGuards(RateLimitGuard, OptionalJwtAuthGuard) @ApiOkResponse({ type: ReceiveIbanDto }) async checkReceiveIban( diff --git a/src/subdomains/supporting/bank/bank/bank.service.ts b/src/subdomains/supporting/bank/bank/bank.service.ts index 0319e6fda6..c3eeb13f2a 100644 --- a/src/subdomains/supporting/bank/bank/bank.service.ts +++ b/src/subdomains/supporting/bank/bank/bank.service.ts @@ -27,7 +27,7 @@ export class BankService implements OnModuleInit { // The VirtualIbanRepository is injected instead of the VirtualIbanService: that service depends on this // one, and both live in BankModule, so the service-level dependency would close a provider cycle. constructor( - private bankRepo: BankRepository, + private readonly bankRepo: BankRepository, private readonly virtualIbanRepo: VirtualIbanRepository, ) {} @@ -118,13 +118,22 @@ export class BankService implements OnModuleInit { ); } + static isBankMatching(asset: Asset, accountIban: string): boolean { + const bankName = this.blockchainToBankName(asset.blockchain); + if (!bankName) return false; + + const expectedIban = this.ibanCache.get(`${bankName}-${asset.dexName}`); + return expectedIban === accountIban; + } + // --- RECEIVE IBAN CHECK --- // // Tells the client whether an IBAN typed in by a customer is one DFX receives customer money on. Pure // input aid for the support form: it enforces nothing, it only lets the frontend phrase a helpful hint. async getReceiveIbanStatus(iban: string, userDataId?: number): Promise { - // normalizeIban absorbs every grouping style a customer may paste and yields null for unusable input; - // treat null and an empty result alike. Both sides of every comparison run through it. + // normalizeIban strips separator characters and yields null for input that cannot hold an IBAN at all; + // it does not rescue every conceivable input (a `IBAN:` prefix stays invalid, correctly). Both sides of + // every comparison run through it, so a stored value in paper format matches too. const normalizedIban = BankService.normalizeIban(iban); if (!normalizedIban || !IbanTools.validateIBAN(normalizedIban).valid) return ReceiveIbanStatus.INVALID_IBAN; @@ -151,21 +160,17 @@ export class BankService implements OnModuleInit { return ReceiveIbanStatus.NOT_MATCHED; } - static isBankMatching(asset: Asset, accountIban: string): boolean { - const bankName = this.blockchainToBankName(asset.blockchain); - if (!bankName) return false; - - const expectedIban = this.ibanCache.get(`${bankName}-${asset.dexName}`); - return expectedIban === accountIban; - } - // --- HELPER METHODS --- // - // Strip every kind of whitespace ourselves - electronicFormatIBAN only removes ASCII spaces and hyphens, - // so a value pasted from a statement or a web page (NBSP, narrow NBSP, tab) would otherwise fail - // validation. The library call then handles hyphens, casing, and a non-string input by returning null. + // An IBAN is alphanumeric only, so everything else is separator noise: grouping spaces of any kind, + // hyphens, dots, slashes, quotes, and the invisible formatting characters that come along when a value + // is pasted out of a statement PDF or an HTML mail. Stripping by an allow-list of what an IBAN may + // contain is complete by construction, where chasing a deny-list of separators was not - ibantools' + // own electronicFormatIBAN only removes ASCII spaces and hyphens, and \s misses the zero-width family. private static normalizeIban(iban: string): string | null { - return typeof iban === 'string' ? IbanTools.electronicFormatIBAN(iban.replace(/\s/g, '')) : null; + if (typeof iban !== 'string') return null; + + return iban.replace(/[^A-Za-z0-9]/g, '').toUpperCase() || null; } // Picks the bank row that owns attribution for a single (name, currency) key. `banks` must already diff --git a/src/subdomains/supporting/bank/bank/dto/receive-iban.dto.ts b/src/subdomains/supporting/bank/bank/dto/receive-iban.dto.ts index ffad168d54..d5b3f9a3d3 100644 --- a/src/subdomains/supporting/bank/bank/dto/receive-iban.dto.ts +++ b/src/subdomains/supporting/bank/bank/dto/receive-iban.dto.ts @@ -2,9 +2,10 @@ import { ApiProperty } from '@nestjs/swagger'; import { IsNotEmpty, IsString } from 'class-validator'; import { ReceiveIbanStatus } from './receive-iban.enum'; -// No @Transform(Util.sanitize) here: it would run before @IsString and throw on a non-string body, which the -// exception filter turns into a 500 on this unauthenticated endpoint. HTML sanitizing is pointless for an IBAN -// anyway - the service normalizes and structurally validates it. +// No @Transform(Util.sanitize) here: Util.sanitizeString calls value.trim() without a type check (util.ts, +// same in Util.trim and Util.trimAll), and @Transform runs before @IsString - so a non-string body would +// throw a TypeError that the exception filter reports as a 500 instead of a 400, on an endpoint reachable +// without a JWT. Validation alone rejects it cleanly; the service normalizes the string afterwards. export class CheckReceiveIbanDto { @ApiProperty() @IsNotEmpty() From 5f9a72863ba4a8a447da05c6e0ca0305cf943078 Mon Sep 17 00:00:00 2001 From: TaprootFreak <142087526+TaprootFreak@users.noreply.github.com> Date: Sat, 25 Jul 2026 22:48:58 +0200 Subject: [PATCH 05/11] test(bank): pin the guards, the throttle and the validation boundary Mutation testing found three safeguards that could be removed without a single test noticing. Deleting @UseGuards entirely, or swapping OptionalJwtAuthGuard for a hard AuthGuard, left all tests green. Both are severe: without the optional guard req.user is never populated and every authenticated customer is told LoginRequired, while a hard guard turns anonymous callers away with a 401. The controller spec now asserts the route metadata the way ledger.controller.spec.ts does - path, method, and the guard list including its order - so both failure modes are caught. Removing the DTO validators, or "harmonizing" them with the @Transform(Util.trimAll) that every other IBAN DTO in the project carries, also went unnoticed. The second one is the realistic edit, and it would turn the deliberate 400 back into a 500 on a route reachable without a login. The DTO is now driven through a ValidationPipe built from the exact options in main.ts, asserting a BadRequestException for a number, an array, an object, null, undefined and an empty string, plus a positive case proving a plain string arrives unchanged - which is what catches a transform being added. The throttle is raised from 10 to 60 per minute. RateLimitGuard buckets IPv4 callers by /24, so a whole company network shares one counter, and unlike the one-shot precedents it was copied from - 2FA verification, mail login - an IBAN field gets re-checked while a customer corrects a typo. Ten would have meant two colleagues in one office locking each other out of the form they opened because something already went wrong. Two comments claimed more than the code delivers. The normalization is complete only for ASCII: non-ASCII letters and digits are stripped as separators too, so a label in a non-Latin script can be dropped where an ASCII one is not. That cannot produce a different valid IBAN, but the comment and the test name now say what actually holds. And DfxIban is phrased as belonging rather than as an invitation to pay in, because most matching rows are retired accounts. --- .../bank/__tests__/bank.controller.spec.ts | 57 +++++++++++++++++++ .../bank/bank/__tests__/bank.service.spec.ts | 12 ++-- .../supporting/bank/bank/bank.controller.ts | 7 ++- .../supporting/bank/bank/bank.service.ts | 15 +++-- .../bank/bank/dto/receive-iban.dto.ts | 9 +-- .../bank/bank/dto/receive-iban.enum.ts | 8 ++- 6 files changed, 91 insertions(+), 17 deletions(-) diff --git a/src/subdomains/supporting/bank/bank/__tests__/bank.controller.spec.ts b/src/subdomains/supporting/bank/bank/__tests__/bank.controller.spec.ts index e32a56ab1a..018a313858 100644 --- a/src/subdomains/supporting/bank/bank/__tests__/bank.controller.spec.ts +++ b/src/subdomains/supporting/bank/bank/__tests__/bank.controller.spec.ts @@ -1,5 +1,11 @@ import { createMock, DeepMocked } from '@golevelup/ts-jest'; +import { BadRequestException, ValidationPipe } from '@nestjs/common'; +import { GUARDS_METADATA, METHOD_METADATA, PATH_METADATA } from '@nestjs/common/constants'; +import { RequestMethod } from '@nestjs/common/enums'; +import { THROTTLER_LIMIT, THROTTLER_TTL } from '@nestjs/throttler/dist/throttler.constants'; import { JwtPayload } from 'src/shared/auth/jwt-payload.interface'; +import { OptionalJwtAuthGuard } from 'src/shared/auth/optional.guard'; +import { RateLimitGuard } from 'src/shared/auth/rate-limit.guard'; import { UserRole } from 'src/shared/auth/user-role.enum'; import { olkyEUR } from '../__mocks__/bank.entity.mock'; import { BankController } from '../bank.controller'; @@ -38,3 +44,54 @@ describe('BankController.checkReceiveIban', () => { expect(result).toEqual({ status: ReceiveIbanStatus.LOGIN_REQUIRED }); }); }); + +// The two tests above pass the JWT in directly, so they cannot see the decorators that decide whether a JWT +// is ever attached. Removing @UseGuards entirely leaves them green while every logged-in customer would get +// LoginRequired (req.user never set), and swapping in a hard AuthGuard() would 401 every anonymous customer. +describe('BankController.checkReceiveIban routing & security metadata', () => { + const handler = BankController.prototype.checkReceiveIban; + + it('is mounted as PUT bank/receive-iban', () => { + expect(Reflect.getMetadata(PATH_METADATA, BankController)).toBe('bank'); + expect(Reflect.getMetadata(PATH_METADATA, handler)).toBe('receive-iban'); + expect(Reflect.getMetadata(METHOD_METADATA, handler)).toBe(RequestMethod.PUT); + }); + + it('guards the route with RateLimitGuard before OptionalJwtAuthGuard', () => { + const guards = Reflect.getMetadata(GUARDS_METADATA, handler) as unknown[]; + + // Order matters as documented on the route; the optional guard must be the auth guard, because a hard + // AuthGuard() would reject exactly the anonymous callers this endpoint exists to serve. + expect(guards).toEqual([RateLimitGuard, OptionalJwtAuthGuard]); + }); + + it('carries a route-level throttle, which is what gives the guard a limit at all', () => { + // RateLimitGuard resolves `routeOrClassLimit || this.options.limit`, and ThrottlerModule.forRoot() is + // registered without options - so without this decorator nothing would be throttled. + expect(Reflect.getMetadata(THROTTLER_LIMIT, handler)).toBe(60); + expect(Reflect.getMetadata(THROTTLER_TTL, handler)).toBe(60); + }); +}); + +// CheckReceiveIbanDto deliberately carries no @Transform. Every other IBAN DTO in the project uses +// @Transform(Util.trimAll), so "aligning" this one is a natural-looking edit - and it would turn the clean +// 400 below into a 500 on a route reachable without a JWT, because the Util transforms call string methods +// on the raw value. These cases pin the boundary behaviour, not the absence of a decorator. +describe('CheckReceiveIbanDto validation boundary', () => { + // Same configuration as the global pipe in main.ts. + const pipe = new ValidationPipe({ whitelist: true, transformOptions: { exposeUnsetFields: false } }); + const metadata = { type: 'body' as const, metatype: CheckReceiveIbanDto }; + + it.each([123, [], {}, null, undefined, ''])( + 'rejects %p with a BadRequestException, never a TypeError', + async (iban) => { + await expect(pipe.transform({ iban }, metadata)).rejects.toBeInstanceOf(BadRequestException); + }, + ); + + it('passes a plain string through unchanged, leaving normalization to the service', async () => { + await expect(pipe.transform({ iban: ' LI75-0881-1010-5923-K000E ' }, metadata)).resolves.toEqual({ + iban: ' LI75-0881-1010-5923-K000E ', + }); + }); +}); diff --git a/src/subdomains/supporting/bank/bank/__tests__/bank.service.spec.ts b/src/subdomains/supporting/bank/bank/__tests__/bank.service.spec.ts index 6c1c55debe..962590d489 100644 --- a/src/subdomains/supporting/bank/bank/__tests__/bank.service.spec.ts +++ b/src/subdomains/supporting/bank/bank/__tests__/bank.service.spec.ts @@ -504,9 +504,11 @@ describe('BankService.getReceiveIbanStatus', () => { // Defensive only: @IsString/@IsNotEmpty reject undefined, null and '' with a 400 before the service is // reached, so of these only ' ' can actually arrive. The typeof guard in normalizeIban short-circuits // the non-string cases, and an all-separator string normalizes to '' and is returned as null. + // The cast stays because getReceiveIbanStatus itself declares `iban: string`; it is what lets the test + // reach the guard from outside the type system, which is exactly the situation the guard exists for. setup(createDefaultBanks()); - await expect(service.getReceiveIbanStatus(input as unknown as string, accountId)).resolves.toBe( + await expect(service.getReceiveIbanStatus(input as string, accountId)).resolves.toBe( ReceiveIbanStatus.INVALID_IBAN, ); }, @@ -572,9 +574,11 @@ describe('BankService.getReceiveIbanStatus', () => { ); }); - it('does not extract an IBAN out of surrounding words', async () => { - // The allow-list strips separators, it does not salvage an IBAN from a labelled value - and it must not, - // because a prefix is indistinguishable from extra characters that make the IBAN wrong. + it('does not extract an IBAN out of surrounding ASCII words', async () => { + // Separators are stripped, an ASCII label is not: it survives normalization and makes the value invalid, + // which is what we want - a prefix is indistinguishable from extra characters that corrupt the IBAN. + // The guarantee is ASCII-only by construction: a label in a non-Latin script is stripped like a + // separator and the IBAN is accepted. Harmless, but the reason this test says "ASCII". setup([frickEUR]); await expect(service.getReceiveIbanStatus('IBAN: LI75 0881 1010 5923 K000E', accountId)).resolves.toBe( diff --git a/src/subdomains/supporting/bank/bank/bank.controller.ts b/src/subdomains/supporting/bank/bank/bank.controller.ts index efd38e0341..bd4669a4a2 100644 --- a/src/subdomains/supporting/bank/bank/bank.controller.ts +++ b/src/subdomains/supporting/bank/bank/bank.controller.ts @@ -1,5 +1,6 @@ import { Body, Controller, Get, Put, UseGuards } from '@nestjs/common'; import { ApiBearerAuth, ApiOkResponse, ApiTags } from '@nestjs/swagger'; +import { Throttle } from '@nestjs/throttler'; import { GetJwt } from 'src/shared/auth/get-jwt.decorator'; import { JwtPayload } from 'src/shared/auth/jwt-payload.interface'; import { OptionalJwtAuthGuard } from 'src/shared/auth/optional.guard'; @@ -25,8 +26,12 @@ export class BankController { // PUT because the IBAN to check belongs in the body, never in the URL - this is a read, it changes nothing. @Put('receive-iban') @ApiBearerAuth() - // RateLimitGuard before the auth guard so it also sees requests that arrive without a JWT. + // Rate limit first, matching the guard order of the existing public endpoints. The route-level @Throttle is + // what actually sets the limit. Deliberately more generous than the 10/60 of the one-shot endpoints (2FA + // verify, mail login): RateLimitGuard buckets IPv4 callers by /24, so everyone behind one company NAT + // shares this counter, and an IBAN field is re-checked several times while a customer corrects a typo. @UseGuards(RateLimitGuard, OptionalJwtAuthGuard) + @Throttle(60, 60) @ApiOkResponse({ type: ReceiveIbanDto }) async checkReceiveIban( @GetJwt() jwt: JwtPayload | undefined, diff --git a/src/subdomains/supporting/bank/bank/bank.service.ts b/src/subdomains/supporting/bank/bank/bank.service.ts index c3eeb13f2a..fd7aa86ebf 100644 --- a/src/subdomains/supporting/bank/bank/bank.service.ts +++ b/src/subdomains/supporting/bank/bank/bank.service.ts @@ -162,12 +162,17 @@ export class BankService implements OnModuleInit { // --- HELPER METHODS --- // - // An IBAN is alphanumeric only, so everything else is separator noise: grouping spaces of any kind, + // An IBAN is ASCII alphanumeric only, so everything else is separator noise: grouping spaces of any kind, // hyphens, dots, slashes, quotes, and the invisible formatting characters that come along when a value - // is pasted out of a statement PDF or an HTML mail. Stripping by an allow-list of what an IBAN may - // contain is complete by construction, where chasing a deny-list of separators was not - ibantools' - // own electronicFormatIBAN only removes ASCII spaces and hyphens, and \s misses the zero-width family. - private static normalizeIban(iban: string): string | null { + // is pasted out of a statement PDF or an HTML mail. Removing everything that is not ASCII alphanumeric + // covers every separator by construction, where chasing a deny-list did not - ibantools' own + // electronicFormatIBAN only removes ASCII spaces and hyphens, and \s misses the zero-width family. + // Note this also drops non-ASCII letters and digits, so a label in a non-Latin script is silently + // stripped rather than making the value invalid. Harmless (it can never produce a *different* valid + // IBAN), but it means only ASCII surroundings are reliably rejected. + // The parameter is widened past the callers' types on purpose: this sits on the trust boundary between a + // request body and the comparison, so it answers for anything the type system cannot actually guarantee. + private static normalizeIban(iban: string | null | undefined): string | null { if (typeof iban !== 'string') return null; return iban.replace(/[^A-Za-z0-9]/g, '').toUpperCase() || null; diff --git a/src/subdomains/supporting/bank/bank/dto/receive-iban.dto.ts b/src/subdomains/supporting/bank/bank/dto/receive-iban.dto.ts index d5b3f9a3d3..2d557f5941 100644 --- a/src/subdomains/supporting/bank/bank/dto/receive-iban.dto.ts +++ b/src/subdomains/supporting/bank/bank/dto/receive-iban.dto.ts @@ -2,10 +2,11 @@ import { ApiProperty } from '@nestjs/swagger'; import { IsNotEmpty, IsString } from 'class-validator'; import { ReceiveIbanStatus } from './receive-iban.enum'; -// No @Transform(Util.sanitize) here: Util.sanitizeString calls value.trim() without a type check (util.ts, -// same in Util.trim and Util.trimAll), and @Transform runs before @IsString - so a non-string body would -// throw a TypeError that the exception filter reports as a 500 instead of a 400, on an endpoint reachable -// without a JWT. Validation alone rejects it cleanly; the service normalizes the string afterwards. +// No @Transform(Util.sanitize) here: Util.sanitizeString calls value.trim() behind a mere truthiness check, +// and @Transform runs before @IsString - so a non-string body would throw a TypeError that the exception +// filter reports as a 500 instead of a 400, on an endpoint reachable without a JWT. The same defect class +// (an unguarded string method on an untyped transform value) sits in Util.trim and Util.trimAll too. +// Validation alone rejects a non-string cleanly; the service normalizes the string afterwards. export class CheckReceiveIbanDto { @ApiProperty() @IsNotEmpty() diff --git a/src/subdomains/supporting/bank/bank/dto/receive-iban.enum.ts b/src/subdomains/supporting/bank/bank/dto/receive-iban.enum.ts index b231b35d99..03f7e7b775 100644 --- a/src/subdomains/supporting/bank/bank/dto/receive-iban.enum.ts +++ b/src/subdomains/supporting/bank/bank/dto/receive-iban.enum.ts @@ -1,7 +1,7 @@ export enum ReceiveIbanStatus { - // An IBAN DFX receives customer money on: either a collective account from the bank table, or a personal - // deposit IBAN of the requesting account. Lifecycle state is irrelevant - a retired collective account and - // an expired personal IBAN both received real customer money. + // An IBAN that belongs to DFX: either a collective account from the bank table, or a personal deposit IBAN + // of the requesting account. It does not say the account still accepts money - most bank rows are retired, + // and an expired personal IBAN matches too. Phrase the hint as "belongs to us", never as "pay in here". DFX_IBAN = 'DfxIban', // The IBAN could not be attributed for this caller. This does NOT claim that the IBAN does not belong to @@ -14,5 +14,7 @@ export enum ReceiveIbanStatus { // No collective account matched, and personal IBANs are only ever checked for the authenticated account, so // without a login the check stays incomplete. Never answered as NOT_MATCHED, which would overstate it. + // Tokens that carry no `account` claim get this too even though they are authenticated - company tokens + // (generateCompanyToken) are wallet-scoped, not account-scoped. Not a case the support form produces. LOGIN_REQUIRED = 'LoginRequired', } From 88f5e8fb301d2a23e7e6a29cc4256452ad7f8d0e Mon Sep 17 00:00:00 2001 From: TaprootFreak <142087526+TaprootFreak@users.noreply.github.com> Date: Sat, 25 Jul 2026 23:22:47 +0200 Subject: [PATCH 06/11] refactor(bank): use a camelCase route and correct two overstated comments CONTRIBUTING documents camelCase for URL routes, and the closest sibling for this concept - GET /buy/personalIban - follows it, as do the two read-with-a-body precedents this endpoint was modelled on. The hyphenated route was an unjustified deviation, and renaming it is free right now: the endpoint is unreleased and has no consumer in production. Once the client library ships the call, the same rename would break everyone using it. Only the route literal and the two metadata assertions change; filenames, the enum, the DTO and the method names stay. Two comments claimed more than had been checked. The first said every other IBAN DTO in the project carries @Transform(Util.trimAll); three admin DTOs do not - update-bank-tx and create/update-fiat-output. Narrowed to customer-facing IBAN input DTOs, which is both true and the sharper form of the argument, since those are what someone would align against. The second said the bank table holds grouped IBAN values. Production stores all eighteen rows compact and uppercase; the only grouped value is a test fixture. What actually holds is the invariant behind it: no service writes bank.iban - rows arrive through migrations or by hand - and nothing normalizes the column on write, so a row can carry a grouped value at any time. The test that covers it was right and is unchanged. --- .../bank/bank/__tests__/bank.controller.spec.ts | 13 +++++++------ .../bank/bank/__tests__/bank.service.spec.ts | 3 ++- .../supporting/bank/bank/bank.controller.ts | 2 +- 3 files changed, 10 insertions(+), 8 deletions(-) diff --git a/src/subdomains/supporting/bank/bank/__tests__/bank.controller.spec.ts b/src/subdomains/supporting/bank/bank/__tests__/bank.controller.spec.ts index 018a313858..90a822b184 100644 --- a/src/subdomains/supporting/bank/bank/__tests__/bank.controller.spec.ts +++ b/src/subdomains/supporting/bank/bank/__tests__/bank.controller.spec.ts @@ -51,9 +51,9 @@ describe('BankController.checkReceiveIban', () => { describe('BankController.checkReceiveIban routing & security metadata', () => { const handler = BankController.prototype.checkReceiveIban; - it('is mounted as PUT bank/receive-iban', () => { + it('is mounted as PUT bank/receiveIban', () => { expect(Reflect.getMetadata(PATH_METADATA, BankController)).toBe('bank'); - expect(Reflect.getMetadata(PATH_METADATA, handler)).toBe('receive-iban'); + expect(Reflect.getMetadata(PATH_METADATA, handler)).toBe('receiveIban'); expect(Reflect.getMetadata(METHOD_METADATA, handler)).toBe(RequestMethod.PUT); }); @@ -73,10 +73,11 @@ describe('BankController.checkReceiveIban routing & security metadata', () => { }); }); -// CheckReceiveIbanDto deliberately carries no @Transform. Every other IBAN DTO in the project uses -// @Transform(Util.trimAll), so "aligning" this one is a natural-looking edit - and it would turn the clean -// 400 below into a 500 on a route reachable without a JWT, because the Util transforms call string methods -// on the raw value. These cases pin the boundary behaviour, not the absence of a decorator. +// CheckReceiveIbanDto deliberately carries no @Transform. Every other customer-facing IBAN input DTO uses +// @Transform(Util.trimAll) - the admin-side ones (update-bank-tx, create/update-fiat-output) do not - so +// "aligning" this one is a natural-looking edit, and it would turn the clean 400 below into a 500 on a route +// reachable without a JWT, because the Util transforms call string methods on the raw value. These cases pin +// the boundary behaviour, not the absence of a decorator. describe('CheckReceiveIbanDto validation boundary', () => { // Same configuration as the global pipe in main.ts. const pipe = new ValidationPipe({ whitelist: true, transformOptions: { exposeUnsetFields: false } }); diff --git a/src/subdomains/supporting/bank/bank/__tests__/bank.service.spec.ts b/src/subdomains/supporting/bank/bank/__tests__/bank.service.spec.ts index 962590d489..8e529b18c6 100644 --- a/src/subdomains/supporting/bank/bank/__tests__/bank.service.spec.ts +++ b/src/subdomains/supporting/bank/bank/__tests__/bank.service.spec.ts @@ -431,7 +431,8 @@ describe('BankService.getReceiveIbanStatus', () => { }); it('reports a collective account IBAN stored in paper format as a DFX IBAN', async () => { - // The stored side is normalized too - bank.iban is maintained by hand and holds grouped values. + // The stored side is normalized too. No service writes bank.iban - rows arrive via migrations or by hand + // - and nothing normalizes the column on write, so a row can carry a grouped value at any time. setup([createCustomBank({ iban: 'LU11 6060 0020 0000 5040' })]); await expect(service.getReceiveIbanStatus('LU116060002000005040', accountId)).resolves.toBe( diff --git a/src/subdomains/supporting/bank/bank/bank.controller.ts b/src/subdomains/supporting/bank/bank/bank.controller.ts index bd4669a4a2..d13b0fbdf1 100644 --- a/src/subdomains/supporting/bank/bank/bank.controller.ts +++ b/src/subdomains/supporting/bank/bank/bank.controller.ts @@ -24,7 +24,7 @@ export class BankController { } // PUT because the IBAN to check belongs in the body, never in the URL - this is a read, it changes nothing. - @Put('receive-iban') + @Put('receiveIban') @ApiBearerAuth() // Rate limit first, matching the guard order of the existing public endpoints. The route-level @Throttle is // what actually sets the limit. Deliberately more generous than the 10/60 of the one-shot endpoints (2FA From 0d1b102575c6473d5029fb5aa0f84b80980a477e Mon Sep 17 00:00:00 2001 From: TaprootFreak <142087526+TaprootFreak@users.noreply.github.com> Date: Sat, 25 Jul 2026 23:42:11 +0200 Subject: [PATCH 07/11] docs(bank): argue the missing transform from mechanism, not from a census Three attempts at one comment, each narrowing a claim about what every other IBAN DTO carries, and each still wrong - the last counterexample being create-support-issue, which is customer-facing, login-optional and uses Util.sanitize rather than trimAll. The lesson is not a fourth narrowing. Any claim of the form "every other DTO does X" is either already false or becomes false with the next DTO, and it was never load-bearing: the argument works from mechanism alone. The Util helpers call string methods on the raw value, @Transform runs before @IsString, and a non-string body therefore becomes a TypeError that the exception filter turns into a 500 on a route reachable without a login. That transforms exist on other IBAN fields is enough to explain why adding one here would look like tidying up; how many and which is irrelevant. I swept the remaining comments in the diff for the same shape. Everything else states either this code's own behaviour or a fact measured directly, so no claim now depends on an inventory that can drift. Also names the seed CSV as a third way rows reach the bank table, alongside migrations and manual inserts. --- .../bank/bank/__tests__/bank.controller.spec.ts | 11 ++++++----- .../bank/bank/__tests__/bank.service.spec.ts | 4 ++-- 2 files changed, 8 insertions(+), 7 deletions(-) diff --git a/src/subdomains/supporting/bank/bank/__tests__/bank.controller.spec.ts b/src/subdomains/supporting/bank/bank/__tests__/bank.controller.spec.ts index 90a822b184..768e6efbfd 100644 --- a/src/subdomains/supporting/bank/bank/__tests__/bank.controller.spec.ts +++ b/src/subdomains/supporting/bank/bank/__tests__/bank.controller.spec.ts @@ -73,11 +73,12 @@ describe('BankController.checkReceiveIban routing & security metadata', () => { }); }); -// CheckReceiveIbanDto deliberately carries no @Transform. Every other customer-facing IBAN input DTO uses -// @Transform(Util.trimAll) - the admin-side ones (update-bank-tx, create/update-fiat-output) do not - so -// "aligning" this one is a natural-looking edit, and it would turn the clean 400 below into a 500 on a route -// reachable without a JWT, because the Util transforms call string methods on the raw value. These cases pin -// the boundary behaviour, not the absence of a decorator. +// CheckReceiveIbanDto deliberately carries no @Transform. The Util transform helpers call string methods on +// the raw value - sanitizeString does value.trim() behind a bare truthiness check, trimAll does +// value?.replace(...) - and @Transform runs before @IsString, so a non-string body throws a TypeError that +// the exception filter turns into a 500 on a route reachable without a login. Transforms do sit on other +// IBAN fields, so adding one here would look like tidying up; these cases pin the boundary behaviour rather +// than the absence of a decorator. describe('CheckReceiveIbanDto validation boundary', () => { // Same configuration as the global pipe in main.ts. const pipe = new ValidationPipe({ whitelist: true, transformOptions: { exposeUnsetFields: false } }); diff --git a/src/subdomains/supporting/bank/bank/__tests__/bank.service.spec.ts b/src/subdomains/supporting/bank/bank/__tests__/bank.service.spec.ts index 8e529b18c6..75a4696de0 100644 --- a/src/subdomains/supporting/bank/bank/__tests__/bank.service.spec.ts +++ b/src/subdomains/supporting/bank/bank/__tests__/bank.service.spec.ts @@ -431,8 +431,8 @@ describe('BankService.getReceiveIbanStatus', () => { }); it('reports a collective account IBAN stored in paper format as a DFX IBAN', async () => { - // The stored side is normalized too. No service writes bank.iban - rows arrive via migrations or by hand - // - and nothing normalizes the column on write, so a row can carry a grouped value at any time. + // The stored side is normalized too. No service writes bank.iban - rows arrive via migrations, the seed + // CSV or by hand - and nothing normalizes the column on write, so a row can carry a grouped value. setup([createCustomBank({ iban: 'LU11 6060 0020 0000 5040' })]); await expect(service.getReceiveIbanStatus('LU116060002000005040', accountId)).resolves.toBe( From 3fb9690ab93743a224052db0653cf961da4d45e2 Mon Sep 17 00:00:00 2001 From: TaprootFreak <142087526+TaprootFreak@users.noreply.github.com> Date: Sun, 26 Jul 2026 00:01:10 +0200 Subject: [PATCH 08/11] docs(bank): drop every comment claim that depends on an unnamed inventory A comment may describe this code, or name a location a reader can open. It must not quantify over an unnamed set of other files or over production data, because such a claim goes stale invisibly. Three statements still did, and the pattern across the previous rounds was to weaken the quantifier rather than remove the dependency. Removed: that transforms sit on other IBAN fields, which was the fourth variant of the same sentence and carried nothing the mechanism argument had not already established; that no service writes bank.iban, a universal negative over present and future services whose load-bearing half was only ever the column; and that the guard order matches the existing public endpoints. The 60/60 rationale stays, because it names the two endpoints it compares against. The enum lost "most bank rows are retired" for the same reason - it quantifies over the contents of the production table and cannot be checked from the repository. The warning it carried is now grounded in the method instead: the check ignores the receive flag and every lifecycle state, so a long-closed account matches just as well. The merge caveat stays, rewritten to name mergeUserData, since a named function is re-checkable in one grep and the caveat is one of the two reasons NotMatched must not be read as "not a DFX IBAN". The audit also caught the method summary still describing the endpoint in the present tense as reporting an IBAN DFX receives money on, the same framing corrected in the enum a round earlier, and one stale route spelling in a test comment. --- .../bank/bank/__tests__/bank.controller.spec.ts | 7 +++---- .../supporting/bank/bank/__tests__/bank.service.spec.ts | 4 ++-- src/subdomains/supporting/bank/bank/bank.controller.ts | 8 ++++---- src/subdomains/supporting/bank/bank/bank.service.ts | 5 +++-- .../supporting/bank/bank/dto/receive-iban.enum.ts | 9 +++++---- 5 files changed, 17 insertions(+), 16 deletions(-) diff --git a/src/subdomains/supporting/bank/bank/__tests__/bank.controller.spec.ts b/src/subdomains/supporting/bank/bank/__tests__/bank.controller.spec.ts index 768e6efbfd..2eb0d3dcca 100644 --- a/src/subdomains/supporting/bank/bank/__tests__/bank.controller.spec.ts +++ b/src/subdomains/supporting/bank/bank/__tests__/bank.controller.spec.ts @@ -13,7 +13,7 @@ import { BankService } from '../bank.service'; import { CheckReceiveIbanDto } from '../dto/receive-iban.dto'; import { ReceiveIbanStatus } from '../dto/receive-iban.enum'; -// The receive-iban check runs behind an optional guard, so the controller must forward the account of a +// The receiveIban check runs behind an optional guard, so the controller must forward the account of a // present JWT and undefined otherwise - that distinction is what makes the service answer LoginRequired. describe('BankController.checkReceiveIban', () => { let controller: BankController; @@ -76,9 +76,8 @@ describe('BankController.checkReceiveIban routing & security metadata', () => { // CheckReceiveIbanDto deliberately carries no @Transform. The Util transform helpers call string methods on // the raw value - sanitizeString does value.trim() behind a bare truthiness check, trimAll does // value?.replace(...) - and @Transform runs before @IsString, so a non-string body throws a TypeError that -// the exception filter turns into a 500 on a route reachable without a login. Transforms do sit on other -// IBAN fields, so adding one here would look like tidying up; these cases pin the boundary behaviour rather -// than the absence of a decorator. +// the exception filter turns into a 500 on a route reachable without a login. Adding a transform here would +// look like tidying up; these cases pin the boundary behaviour rather than the absence of a decorator. describe('CheckReceiveIbanDto validation boundary', () => { // Same configuration as the global pipe in main.ts. const pipe = new ValidationPipe({ whitelist: true, transformOptions: { exposeUnsetFields: false } }); diff --git a/src/subdomains/supporting/bank/bank/__tests__/bank.service.spec.ts b/src/subdomains/supporting/bank/bank/__tests__/bank.service.spec.ts index 75a4696de0..fd6fc53131 100644 --- a/src/subdomains/supporting/bank/bank/__tests__/bank.service.spec.ts +++ b/src/subdomains/supporting/bank/bank/__tests__/bank.service.spec.ts @@ -431,8 +431,8 @@ describe('BankService.getReceiveIbanStatus', () => { }); it('reports a collective account IBAN stored in paper format as a DFX IBAN', async () => { - // The stored side is normalized too. No service writes bank.iban - rows arrive via migrations, the seed - // CSV or by hand - and nothing normalizes the column on write, so a row can carry a grouped value. + // The stored side is normalized too: nothing normalizes bank.iban on write, so a stored row can carry a + // grouped value. setup([createCustomBank({ iban: 'LU11 6060 0020 0000 5040' })]); await expect(service.getReceiveIbanStatus('LU116060002000005040', accountId)).resolves.toBe( diff --git a/src/subdomains/supporting/bank/bank/bank.controller.ts b/src/subdomains/supporting/bank/bank/bank.controller.ts index d13b0fbdf1..442956314e 100644 --- a/src/subdomains/supporting/bank/bank/bank.controller.ts +++ b/src/subdomains/supporting/bank/bank/bank.controller.ts @@ -26,10 +26,10 @@ export class BankController { // PUT because the IBAN to check belongs in the body, never in the URL - this is a read, it changes nothing. @Put('receiveIban') @ApiBearerAuth() - // Rate limit first, matching the guard order of the existing public endpoints. The route-level @Throttle is - // what actually sets the limit. Deliberately more generous than the 10/60 of the one-shot endpoints (2FA - // verify, mail login): RateLimitGuard buckets IPv4 callers by /24, so everyone behind one company NAT - // shares this counter, and an IBAN field is re-checked several times while a customer corrects a typo. + // RateLimitGuard first; the route-level @Throttle below is what sets the limit. Deliberately more generous + // than the 10/60 on the one-shot endpoints (kyc 2fa/verify, auth mail login): RateLimitGuard buckets IPv4 + // callers by /24, so everyone behind one company NAT shares this counter, and an IBAN field is re-checked + // several times while a customer corrects a typo. @UseGuards(RateLimitGuard, OptionalJwtAuthGuard) @Throttle(60, 60) @ApiOkResponse({ type: ReceiveIbanDto }) diff --git a/src/subdomains/supporting/bank/bank/bank.service.ts b/src/subdomains/supporting/bank/bank/bank.service.ts index fd7aa86ebf..0eb71025e0 100644 --- a/src/subdomains/supporting/bank/bank/bank.service.ts +++ b/src/subdomains/supporting/bank/bank/bank.service.ts @@ -128,8 +128,9 @@ export class BankService implements OnModuleInit { // --- RECEIVE IBAN CHECK --- // - // Tells the client whether an IBAN typed in by a customer is one DFX receives customer money on. Pure - // input aid for the support form: it enforces nothing, it only lets the frontend phrase a helpful hint. + // Tells the client whether an IBAN typed in by a customer is one that belongs to DFX - not whether it still + // accepts money. Pure input aid for the support form: it enforces nothing, it only lets the frontend phrase + // a helpful hint. async getReceiveIbanStatus(iban: string, userDataId?: number): Promise { // normalizeIban strips separator characters and yields null for input that cannot hold an IBAN at all; // it does not rescue every conceivable input (a `IBAN:` prefix stays invalid, correctly). Both sides of diff --git a/src/subdomains/supporting/bank/bank/dto/receive-iban.enum.ts b/src/subdomains/supporting/bank/bank/dto/receive-iban.enum.ts index 03f7e7b775..c4ceb81a22 100644 --- a/src/subdomains/supporting/bank/bank/dto/receive-iban.enum.ts +++ b/src/subdomains/supporting/bank/bank/dto/receive-iban.enum.ts @@ -1,12 +1,13 @@ export enum ReceiveIbanStatus { // An IBAN that belongs to DFX: either a collective account from the bank table, or a personal deposit IBAN - // of the requesting account. It does not say the account still accepts money - most bank rows are retired, - // and an expired personal IBAN matches too. Phrase the hint as "belongs to us", never as "pay in here". + // of the requesting account. It does not say the account still accepts money - getReceiveIbanStatus ignores + // the bank `receive` flag and every virtual_iban lifecycle state, so a long-closed account matches just as + // well. Phrase the hint as "belongs to us", never as "pay in here". DFX_IBAN = 'DfxIban', // The IBAN could not be attributed for this caller. This does NOT claim that the IBAN does not belong to - // DFX: personal IBANs of other accounts are deliberately never checked, and an account merge leaves the - // virtual_iban rows on the former account, so a customer's own older personal IBAN can land here as well. + // DFX: personal IBANs of other accounts are deliberately never checked, and mergeUserData does not move + // virtual_iban rows to the master, so a customer's own older personal IBAN can land here as well. NOT_MATCHED = 'NotMatched', // The input is not a structurally valid IBAN (country, length or checksum), so there is nothing to look up. From d7ebc5789d6de2b9720494b3fd3bfdd2e96509f0 Mon Sep 17 00:00:00 2001 From: TaprootFreak <142087526+TaprootFreak@users.noreply.github.com> Date: Sun, 26 Jul 2026 00:20:05 +0200 Subject: [PATCH 09/11] docs(bank): drop the last comment clause that quantifies over write paths The previous commit removed one half of this sentence and kept the other, which was the same shape with the verb swapped: "nothing normalizes bank.iban on write" is a universal negative over an unnamed set of write paths, and it goes stale the moment one appears. It was also never the point of the comment - what the test demonstrates is that the comparison normalizes the stored side, which is a statement about this code and needs no inventory at all. --- .../supporting/bank/bank/__tests__/bank.service.spec.ts | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/subdomains/supporting/bank/bank/__tests__/bank.service.spec.ts b/src/subdomains/supporting/bank/bank/__tests__/bank.service.spec.ts index fd6fc53131..1fe57b1aa4 100644 --- a/src/subdomains/supporting/bank/bank/__tests__/bank.service.spec.ts +++ b/src/subdomains/supporting/bank/bank/__tests__/bank.service.spec.ts @@ -431,8 +431,7 @@ describe('BankService.getReceiveIbanStatus', () => { }); it('reports a collective account IBAN stored in paper format as a DFX IBAN', async () => { - // The stored side is normalized too: nothing normalizes bank.iban on write, so a stored row can carry a - // grouped value. + // The stored side is normalized too, so a row that carries a grouped value still matches. setup([createCustomBank({ iban: 'LU11 6060 0020 0000 5040' })]); await expect(service.getReceiveIbanStatus('LU116060002000005040', accountId)).resolves.toBe( From 8e4b37b6830c1b899bb1e99a36ec0d47d3e42d9b Mon Sep 17 00:00:00 2001 From: TaprootFreak <142087526+TaprootFreak@users.noreply.github.com> Date: Mon, 27 Jul 2026 11:13:51 +0200 Subject: [PATCH 10/11] test(bank): type the spec helpers and pin the short-circuit for a logged-in caller Two independent review lanes on the final state turned up eight items. The ordering of the checks was only pinned for an anonymous caller. A logged-in caller whose IBAN matches a collective account must also skip the account-scoped lookup, and nothing asserted that - running the personal lookup eagerly would have passed. That test now asserts it, and inverting the order fails exactly it. The spec helpers carried an untyped mock argument and no return types. The filter shape is now a named local type, so the mock is typed without any. Five comments claimed more than was checked. Two stated frequencies over customer behaviour - that a missing transfer is often old, and that the logged-out collective case is the most common by far. Neither is knowable from here, and both rationales stand without the quantifier. One claimed personal IBANs arrive unvalidated from the provider, which is false for one of the two providers; it now speaks only about this comparison normalizing stored values. One called a fixture a transposed digit where it substitutes one. One asserted that editors convert invisible separators, which is not generally true - the honest reason for escape sequences is that they make the characters visible in review, and that writing them literally went wrong twice here. And the throttle rationale described a consumer re-checking the field in the present tense, for a consumer that does not exist yet. --- .../bank/bank/__tests__/bank.service.spec.ts | 30 ++++++++++++------- .../supporting/bank/bank/bank.controller.ts | 4 +-- .../supporting/bank/bank/bank.service.ts | 6 ++-- 3 files changed, 24 insertions(+), 16 deletions(-) diff --git a/src/subdomains/supporting/bank/bank/__tests__/bank.service.spec.ts b/src/subdomains/supporting/bank/bank/__tests__/bank.service.spec.ts index 1fe57b1aa4..5ec9049a8b 100644 --- a/src/subdomains/supporting/bank/bank/__tests__/bank.service.spec.ts +++ b/src/subdomains/supporting/bank/bank/__tests__/bank.service.spec.ts @@ -408,22 +408,29 @@ describe('BankService.getReceiveIbanStatus', () => { service = module.get(BankService); }); - function setup(banks: Bank[], virtualIbansByAccount: Map = new Map()) { + // The only shape getReceiveIbanStatus passes to findCachedBy; keeps the mock typed without `any`. + type AccountScopedWhere = { userData: { id: number } }; + + function setup(banks: Bank[], virtualIbansByAccount: Map = new Map()): void { jest.spyOn(bankRepo, 'findCached').mockResolvedValue(banks); jest .spyOn(virtualIbanRepo, 'findCachedBy') - .mockImplementation(async (_key: string, where: any) => virtualIbansByAccount.get(where.userData.id) ?? []); + .mockImplementation( + async (_key: string | number, where: AccountScopedWhere) => virtualIbansByAccount.get(where.userData.id) ?? [], + ); } - it('reports a collective account IBAN as a DFX IBAN', async () => { + it('reports a collective account IBAN as a DFX IBAN, without asking for personal IBANs', async () => { + // A collective account hit short-circuits for a logged-in caller too - no account-scoped lookup happens. setup(createDefaultBanks()); await expect(service.getReceiveIbanStatus(olkyEUR.iban, accountId)).resolves.toBe(ReceiveIbanStatus.DFX_IBAN); + expect(virtualIbanRepo.findCachedBy).not.toHaveBeenCalled(); }); it('reports a collective account IBAN as a DFX IBAN without a login, before ever asking for personal IBANs', async () => { - // The most common case by far: a logged-out customer types a collective account IBAN. The bank check must - // run before the login check, otherwise this answers LoginRequired for an IBAN we can already confirm. + // The bank check must run before the login check, otherwise a logged-out customer gets LoginRequired for + // an IBAN we can already confirm. setup(createDefaultBanks()); await expect(service.getReceiveIbanStatus(olkyEUR.iban)).resolves.toBe(ReceiveIbanStatus.DFX_IBAN); @@ -440,7 +447,7 @@ describe('BankService.getReceiveIbanStatus', () => { }); it('reports a personal IBAN stored in paper format as a DFX IBAN', async () => { - // virtual_iban values arrive unvalidated from provider.reserveViban(), so their format is not guaranteed. + // The comparison normalizes stored virtual_iban values as well, so their format need not be guaranteed. setup( createDefaultBanks(), new Map([[accountId, [createCustomVirtualIban({ iban: 'de89 3704 0044 0532 0130 00' })]]]), @@ -450,7 +457,7 @@ describe('BankService.getReceiveIbanStatus', () => { }); it('reports a collective account IBAN with receive=false as a DFX IBAN', async () => { - // The customer reports a missing, often old transfer - a retired or closed account still received DFX money. + // A retired or closed account still received DFX money, and a missing transfer can predate it being stood down. setup([retiredCollectiveAccount]); await expect(service.getReceiveIbanStatus(retiredCollectiveAccount.iban, accountId)).resolves.toBe( @@ -489,7 +496,7 @@ describe('BankService.getReceiveIbanStatus', () => { }); it('reports a correctly shaped IBAN with a wrong checksum as invalid, not as unmatched', async () => { - // A transposed digit keeps the country and length intact, so only the checksum catches it. Answering + // A changed digit keeps the country and length intact, so only the checksum catches it. Answering // NotMatched here would send a customer looking for a transfer that never left with a typo in the IBAN. setup(createDefaultBanks()); @@ -540,8 +547,9 @@ describe('BankService.getReceiveIbanStatus', () => { ); }); - // The invisible separators are written as escape sequences on purpose: as literal characters an editor or a - // copy-paste would silently turn them back into ordinary spaces, which would void exactly those cases. + // The invisible separators are written as escape sequences on purpose: it makes them visible in review and + // lowers the risk of an edit or a copy-paste quietly normalizing them into ordinary spaces, which would + // void exactly those cases. Writing them literally already went wrong twice here. it.each([ ['an ASCII space', ' '], ['a hyphen', '-'], @@ -556,7 +564,7 @@ describe('BankService.getReceiveIbanStatus', () => { ])('recognizes an IBAN grouped with %s', async (_name, separator) => { setup([frickEUR], new Map([[accountId, [createCustomVirtualIban({ iban: personalIban })]]])); - const group = (iban: string) => (iban.match(/.{1,4}/g) ?? []).join(separator); + const group = (iban: string): string => (iban.match(/.{1,4}/g) ?? []).join(separator); await expect(service.getReceiveIbanStatus(group(frickEUR.iban), accountId)).resolves.toBe( ReceiveIbanStatus.DFX_IBAN, diff --git a/src/subdomains/supporting/bank/bank/bank.controller.ts b/src/subdomains/supporting/bank/bank/bank.controller.ts index 442956314e..ab4b35682a 100644 --- a/src/subdomains/supporting/bank/bank/bank.controller.ts +++ b/src/subdomains/supporting/bank/bank/bank.controller.ts @@ -28,8 +28,8 @@ export class BankController { @ApiBearerAuth() // RateLimitGuard first; the route-level @Throttle below is what sets the limit. Deliberately more generous // than the 10/60 on the one-shot endpoints (kyc 2fa/verify, auth mail login): RateLimitGuard buckets IPv4 - // callers by /24, so everyone behind one company NAT shares this counter, and an IBAN field is re-checked - // several times while a customer corrects a typo. + // callers by /24, so everyone behind one company NAT shares this counter, and the intended consumer is an + // input field meant to be re-checked while a customer corrects a typo. @UseGuards(RateLimitGuard, OptionalJwtAuthGuard) @Throttle(60, 60) @ApiOkResponse({ type: ReceiveIbanDto }) diff --git a/src/subdomains/supporting/bank/bank/bank.service.ts b/src/subdomains/supporting/bank/bank/bank.service.ts index 0eb71025e0..f93cd8379d 100644 --- a/src/subdomains/supporting/bank/bank/bank.service.ts +++ b/src/subdomains/supporting/bank/bank/bank.service.ts @@ -138,9 +138,9 @@ export class BankService implements OnModuleInit { const normalizedIban = BankService.normalizeIban(iban); if (!normalizedIban || !IbanTools.validateIBAN(normalizedIban).valid) return ReceiveIbanStatus.INVALID_IBAN; - // Deliberately not filtered by `receive`: the customer is reporting a missing, often old transfer, so a - // hit on a retired or closed account is still money that went to DFX. A receive=true filter would tell a - // real customer that their IBAN does not belong to DFX. + // Deliberately not filtered by `receive`: a hit on a retired or closed account is still money that went + // to DFX, and a missing transfer can predate the account being stood down. A receive=true filter would + // tell a real customer that their IBAN does not belong to DFX. const banks = await this.getAllBanks(); if (banks.some((b) => BankService.normalizeIban(b.iban) === normalizedIban)) return ReceiveIbanStatus.DFX_IBAN; From 521afcd289073668bedb73930ccc2b7f8e756d32 Mon Sep 17 00:00:00 2001 From: TaprootFreak <142087526+TaprootFreak@users.noreply.github.com> Date: Mon, 27 Jul 2026 11:26:22 +0200 Subject: [PATCH 11/11] test(bank): pin the four status strings as the wire contract The enum values are what three repositories agree on: the client library carries its own copy, and the support form derives its wording from them. Every test so far compared enum members, so renaming a value would have passed here and broken at runtime in a customer's browser instead. The literals are now asserted; changing one fails exactly that test. One comment also went out claiming the separator cases had been written literally twice by mistake. That history is not in the repository - the cases arrived escaped and stayed escaped - so for any reader it is unverifiable. The verifiable half of the rationale stands on its own: escape sequences make the characters visible in review and lower the risk of an edit normalizing them away. --- .../bank/bank/__tests__/bank.controller.spec.ts | 12 ++++++++++++ .../bank/bank/__tests__/bank.service.spec.ts | 2 +- 2 files changed, 13 insertions(+), 1 deletion(-) diff --git a/src/subdomains/supporting/bank/bank/__tests__/bank.controller.spec.ts b/src/subdomains/supporting/bank/bank/__tests__/bank.controller.spec.ts index 2eb0d3dcca..59db5b0fef 100644 --- a/src/subdomains/supporting/bank/bank/__tests__/bank.controller.spec.ts +++ b/src/subdomains/supporting/bank/bank/__tests__/bank.controller.spec.ts @@ -96,3 +96,15 @@ describe('CheckReceiveIbanDto validation boundary', () => { }); }); }); + +// These four strings are the wire contract: the client library carries its own copy of this enum, and the +// support form derives its wording from it. Comparing enum members would let a renamed value pass here and +// break at runtime in the browser instead, so the literals are asserted. +describe('ReceiveIbanStatus wire values', () => { + it('serializes to the strings the consumers expect', () => { + expect(ReceiveIbanStatus.DFX_IBAN).toBe('DfxIban'); + expect(ReceiveIbanStatus.NOT_MATCHED).toBe('NotMatched'); + expect(ReceiveIbanStatus.INVALID_IBAN).toBe('InvalidIban'); + expect(ReceiveIbanStatus.LOGIN_REQUIRED).toBe('LoginRequired'); + }); +}); diff --git a/src/subdomains/supporting/bank/bank/__tests__/bank.service.spec.ts b/src/subdomains/supporting/bank/bank/__tests__/bank.service.spec.ts index 5ec9049a8b..53d11e3571 100644 --- a/src/subdomains/supporting/bank/bank/__tests__/bank.service.spec.ts +++ b/src/subdomains/supporting/bank/bank/__tests__/bank.service.spec.ts @@ -549,7 +549,7 @@ describe('BankService.getReceiveIbanStatus', () => { // The invisible separators are written as escape sequences on purpose: it makes them visible in review and // lowers the risk of an edit or a copy-paste quietly normalizing them into ordinary spaces, which would - // void exactly those cases. Writing them literally already went wrong twice here. + // void exactly those cases. it.each([ ['an ASCII space', ' '], ['a hyphen', '-'],