fix: honour nationalAccountAllowed when building an international account connection - #36
Open
Superheld wants to merge 4 commits into
Open
fix: honour nationalAccountAllowed when building an international account connection#36Superheld wants to merge 4 commits into
Superheld wants to merge 4 commits into
Conversation
…ount connection FinTS has two forms of account connection. The national one (KTV) carries account number, sub-account and bank; the international one (KTZ) adds IBAN and BIC and makes every field optional. A KTZ may carry the national fields as well, but only where the bank permits it, and the bank says so in its HISPAS parameters via `nationalAccountAllowed`. That field was parsed and stored, and never read. Every interaction using a KTZ filled both halves, which banks setting the flag to false reject. Measured against comdirect (BLZ 20041144), which declares nationalAccountAllowed false. Six HKCAZ orders, one account, same 9-day range, same session, repeated on a second account with a different account number: IBAN + BIC + number + sub-account + bank -> 3010 "Kontonummer ist ungültig", 0 IBAN + BIC -> 0020 "Auftrag ausgeführt", 19 IBAN -> 0020 "Auftrag ausgeführt", 19 number + sub-account + bank -> 3010 "Kontonummer ist ungültig", 0 The fourth line is why this is a rule about the national fields rather than about the combination: they are rejected in a KTZ even with no IBAN beside them. The same bank also declares a malformed camt format, so that was varied independently and ruled out — dropping it changes nothing. HKCAZ was where this surfaced, but it is not CAMT-specific. HKSAL and HKKAZ use a KTZ from version 7 and HKEKA from version 4; comdirect announces HKSAL 5 and HKKAZ 5, which is the only reason balance and MT940 worked. Any bank on the newer versions would have hit the same refusal. - new accountDescriptor.ts with nationalAccount() and internationalAccount() - both build the descriptor field by field instead of spreading the whole account and blanking what does not belong, so a data group cannot silently pick up a field that happens to share a name - an account without an IBAN keeps the national fields whatever the flag says: a securities account has nothing else to identify it with - no HISPAS at all is treated as no permission, which is what a "Kann"-field means - HKKAZSegment.account typed as Account | InternationalAccount, matching the segment definition, which has declared an international group for version 7+ all along Tests: nothing covered createSegments() before — the segment tests build their account by hand and the client tests mock Dialog.start, which discards the built request. The new interaction tests cover that layer for all five interactions and both sides of every version boundary. All nine fail without this change. 148 tests. Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>
Dialog, TAN method discovery, balance, MT940 and CAMT all work there with the account descriptor fix in this branch. Before it, CAMT returned success with zero statements. Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>
Author
|
Added comdirect to the tested-banks list — with this change, dialog, TAN method discovery, balance, MT940 and CAMT all work there. The securities portfolio was not exercised, for a reason unrelated to this PR that I will raise separately. |
KTI is the Kontoverbindung international — IBAN, BIC, account number, sub-account, bank identifier — which is what InternationalAccountGroup defines. KTZ is the Kontoverbindung ZV international, which carries the SEPA flag in front and corresponds to SepaAccountGroup. Comments only. Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>
The first version read an absent HISPAS as absent permission and shortened the descriptor. That is the specification's reading, and it is the wrong one here: this library has already been round this loop. robocode13#20 reduced the CAMT descriptor to IBAN and BIC because comdirect rejects anything more. robocode13#25 then reported Postbank answering "Angaben zur nationalen Kontoverbindung für Identifikation erforderlich", and the reduction was reverted — which fixed Postbank and broke comdirect again, which is the state 1.5.0 ships in. Neither bank was wrong and neither fix could hold, because the choice was hard-coded either way. Reading the flag makes it data. But a bank that announces nothing has stated no rule, and shortening its descriptor would repeat exactly the move that had to be taken back in June. So: only an explicit false shortens. Adds two tests naming the conflict, so the next person to touch this sees why it is a switch rather than a constant. 150 tests. Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The problem
A KTI (international account connection) may carry the national fields — account
number, sub-account, bank — beside IBAN and BIC, but only where the bank permits it.
The bank declares that in HISPAS, in
nationalAccountAllowed. lib-fints parses thefield into the BPD and never reads it; a grep over
dist/finds it only in the typedeclaration.
This is not a new failure. #19/#20 reduced the CAMT descriptor to IBAN and BIC because
comdirect rejects anything more. #25 then reported Postbank answering
9210 Angaben zur nationalen Kontoverbindung für Identifikation erforderlich, and the reduction wasreverted — which fixed Postbank and put comdirect back where it started, which is what
1.5.0 ships. Neither bank is wrong, and neither fix could hold, because the choice was
hard-coded either way. The flag is the bank saying which one it wants.
comdirect declares
nationalAccountAllowed: falsein its HISPAS, and theregetAccountStatements(account, from, to)— the default call,preferCamtdefaultingto
true— returnssuccess: truewith zero statements and3010 Kontonummer ist ungültiginbankAnswers. A caller not reading those sees an empty account.Evidence
Four HKCAZ orders, one account, same range, same session, differing only in the
descriptor; repeated on a second account, same result. Measured against released 1.5.0
through
startCustomerOrderInteraction— no patched library involved.3010Kontonummer ist ungültig0020Auftrag ausgeführt0020Auftrag ausgeführt3010Kontonummer ist ungültigThe last row is why this is a rule about the national fields rather than about the
combination: they are rejected inside a KTI even with no IBAN beside them. phpFinTS
(MIT) and python-fints apply the same rule; that is where I found the flag, no code
was taken from either.
What I decided
would read absent permission as no permission, but shortening a silent bank's
descriptor is the move that had to be taken back in June. Only an explicit
falseshortens, so nothing working today can regress on a rule its bank never stated.
identifies it. Reasoned, not measured.
BankAccountand blanking what does not belong — hence the HKSAL v6 and HKWPDtests changing shape although their output is identical. Happy to drop it.
Scope: HKCAZ always builds a KTI, HKSAL and HKKAZ from segment version 7, HKEKA from
version 4, so all five interactions go through one function. Only HKCAZ was
exercised against a bank — this one announces HKSAL 5 and HKKAZ 5.
Tests
grep -rn createSegments src/tests/found nothing before this, which is why the flipshipped twice through a green suite. The HKCAZ tests build their account by hand, and
#20 reduced those fixtures to IBAN and BIC; an object with only those two fields
encodes identically whether the data group admits two fields or five. Today's
HKCAZ.test.tspasses unchanged against the January code — I checked.accountDescriptor.test.ts— 5 tests on the two functionsinteractionAccountDescriptor.test.ts— 11 throughcreateSegments(), all fiveinteractions, both sides of every version boundary, and both banks above
All of the interaction tests fail without the change. 150 tests,
tsc --noEmitclean.The investigation, the code, the tests and this text are mine — Claude Opus 5
(Anthropic). @Superheld ran every measurement against their own bank and decided what
went into this PR.