Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

French iban has invalid checksum #2588

Closed
elkesrio opened this issue Oct 11, 2022 · 12 comments · Fixed by #2625
Closed

French iban has invalid checksum #2588

elkesrio opened this issue Oct 11, 2022 · 12 comments · Fixed by #2625
Assignees

Comments

@elkesrio
Copy link

Describe the bug

Some french ibans have invalid checksums

To Reproduce

Using Faker version 2.15.1:

Faker::Bank.iban(country_code: "fr") # => "FR006769721996MP541QY956313"

This iban has an invalid checksum (https://www.ibancalculator.com/iban_validieren.html)

@fbuys
Copy link
Contributor

fbuys commented Oct 11, 2022

Hey @elkesrio, I am looking at this link, to better understand the structure of a french iban. Do you know how the check digits for the iban is calculated?

@elkesrio
Copy link
Author

Hey @fbuys, I have no idea. You can check ibandit gem for better understanding

@fbuys
Copy link
Contributor

fbuys commented Oct 11, 2022

I'll do some more digging but is seems like this part defines how the check digit is calculated. Might be all we need to create the correct check digits - but not sure about that yet.

@fbuys
Copy link
Contributor

fbuys commented Oct 11, 2022

@stefannibrasil I wonder if we can add the hactoberfest label here too?

@thdaraujo
Copy link
Contributor

@stefannibrasil I wonder if we can add the hactoberfest label here too?

done, thanks for the help!

@stefannibrasil
Copy link
Contributor

hi, sorry I missed this! Thanks for reporting this issue @elkesrio and @fbuys for the help!

I haven't had the time to reproduce this yet. I am not sure this is happening for all country codes or "fr" only.

It would be helpful to have a reproduction script for this issue. If anyone could do that before me, that would be great!

@elkesrio
Copy link
Author

Hi @stefannibrasil, you can reproduce it using the ibandit gem

require 'ibandit'
while true do
  iban = Faker::Bank.iban(country_code: "fr")
  raise StandardError, iban unless Ibandit::IBAN.new(iban).valid?
end

# StandardError: FR007626128613KJOM8X38HG512

@srcoley
Copy link
Contributor

srcoley commented Oct 14, 2022

The issue can occur with multiple country codes. In addition to the IBAN check digit, some countries also make use of a "national check digit". France is one such country that not only makes use of a national check digit, but also uses their own variation of ISO 7064 MOD-97-10. For more information, read here.

I believe I have this issue solved for France specifically - currently generating 100k valid French IBANs. While my solution is not yet generic enough to drop-in national check digits for other countries, I'd like to continue working on this so others can add them later more easily.

@srcoley
Copy link
Contributor

srcoley commented Oct 16, 2022

I've opened a pull request to solve the issue of some generated IBANs not being valid according to ibandit. While this change would solve the issue with ibandit specifically it does not guarantee the IBANs are actually valid. ibandits validation is rudimentary and does not take into account the national check digit that some countries use. This means that a Faker generated IBAN might be valid according to ibandit, but invalid to more strict validators such as https://www.iban.com/iban-checker.

More detail on this in description of the pull request tagged above.

@stefannibrasil
Copy link
Contributor

hi @srcoley thank you for opening a PR! What is the status of this issue? Are there next steps? Thank you!

@srcoley
Copy link
Contributor

srcoley commented Nov 1, 2022

@stefannibrasil #2591 partially resolves the issue and has been merged into main.

What it solves

Invalid IBAN checksums

What it does not solve

Invalid national checksums

Next steps

Resolving invalid national checksums will take more effort because the portion of countries that implement them use varying algorithms for generation and validation. Check this table to see what I mean.

Additionally, ibandit(and no other library I could find) does not properly validate national checksums. The lack of an "authoritative" validator means several dozen would need to be written for Faker in order to test that the generators are working properly.

In regard to the web validator mentioned in the original issue(https://www.ibancalculator.com/iban_validieren.html), even if the national checksum issue is resolved, validation will likely still fail because it also checks that the bank and branch codes actually exist.

I'll leave the maintainers to decide what level of IBAN validity they think is acceptable for Faker's goals.

@stefannibrasil
Copy link
Contributor

@srcoley Thank you so much for your detailed work and response! I haven't had the time to get more context about this until now. The details you provided are constructive.

I will say that fixing invalid IBAN checksums being generated is enough. Although we would like to support all cases and have higher accuracy, it would require too much effort to accommodate.

Adding to the docs that Faker only generates valid IBAN check digits but does not take national check digits used within the BBAN is enough.

If ibandit or other similar libraries can't guarantee to validate national checksums for all edge cases, and if this behavior is critical for an app, I recommend users generate their valid IBAN digits to guarantee their business logic relies on checks they own.

IMHO, the fix you added is enough, and adding variants for all edge cases is too much for Faker. I'd rather have something that serves basic needs well (without national check digits) than provide inaccurate information to users.

The next step to fix this issue would be to update the docs with this information.

@vbrazo @thdaraujo, what do you think?

stefannibrasil pushed a commit to hexdevs/faker that referenced this issue Nov 12, 2022
Although we would like to support all cases and have higher accuracy, it would require too much effort to accommodate.

Faker generates valid IBAN check digits but does not support national check digits used within the BBAN.

If this behavior is critical for an app, it's recommend to generate its own IBAN digits validity check.

See [faker-ruby#2588](faker-ruby#2588) for more details.
thdaraujo pushed a commit that referenced this issue Nov 18, 2022
Although we would like to support all cases and have higher accuracy, it would require too much effort to accommodate.

Faker generates valid IBAN check digits but does not support national check digits used within the BBAN.

If this behavior is critical for an app, it's recommend to generate its own IBAN digits validity check.

See [#2588](#2588) for more details.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants