skwasjer / IbanNet

C# .NET IBAN validator, parser, builder and generator
Apache License 2.0
119 stars 31 forks source link

[Question:] How to validate IBANs strict? #93

Closed kimpenhaus closed 1 year ago

kimpenhaus commented 1 year ago

Hi @skwasjer,

thanks for implementing and maintaining this lib. I saw #24 and was wondering how to validate IBANs strict as you've mentioned in that ticket?

Inside Validate function there is always a NormalizeOrNull done:

https://github.com/skwasjer/IbanNet/blob/a1f7e1528ad033d1b8cd4b34bab79c4ca69259fb/src/IbanNet/IbanValidator.cs#L80

which converts to upper-casing:

https://github.com/skwasjer/IbanNet/blob/a1f7e1528ad033d1b8cd4b34bab79c4ca69259fb/src/IbanNet/Iban.cs#L258-L259

So I don't actually see any chance to validate strict (also validate if the casing is correct) unless to provide an additional Ruleto the ValidationOptions. (which feels like some sort of duplicating)

Maybe you have a better approach? Would there be a strict Flag in the Options possible?

Thanks you 😀

skwasjer commented 1 year ago

From what I remember, even though SWIFT has a distinction between upper vs lower case, there is currently no country that has an IBAN format which require 'only' lowercase in certain positions (but perhaps I remember wrong) and thus technically all letters are satisfying an uppercase check (even if they are lowercase).

In major versions prior to v4 (IIRC), this library would indeed check explicitly for lower and uppercase but that was 'negated' by the introduction of the normalize function as you mentioned in subsequent versions and I considered it fine because of that. It is not entirely correct from an implementation point of view though, you are correct there.

What is your use case for requiring support for this, for my understanding?

Because it could be somewhat impactful, notably the Iban type its formatting functions, equality checks, hash code, etc.

skwasjer commented 1 year ago

Btw, the strict vs loose validation debate back then is not entirely applicable anymore. In older versions you could choose either mode, where loose would skip certain checks (for performance). That was deprecated, as performance was improved and skipping certain checks provided little gains.

skwasjer commented 1 year ago

And thinking some more, I think I may have misunderstood what you want to achieve, so do correct me. Do you just want to fail validation when the input has lowercase but the IBAN pattern requires an upper case character?

kimpenhaus commented 1 year ago

@skwasjer thanks for your detailed explanation - indeed what I am trying to achieve is what you've asked in your last message:

Do you just want to fail validation when the input has lowercase but the IBAN pattern requires an upper case character

I need to do this because of a validation in a backend system which would fail if it is called with lower-case but upper-case is required.

skwasjer commented 1 year ago

In that case I would probably recommend (for now) using the IbanParser and Iban type in your domain/business logic. Internally it still uses the validator, but it allows you to make use of the strong typed API to ensure sanization/normalization and formatting:

var myInput = "  nl91 ABna 0417 1643 00  ";

var ibanParser = new IbanParser(IbanRegistry.Default);

// Is valid?
if (ibanParser.TryParse(myInput, out Iban iban))
{
    // Pass to backend system/DB/API, ensuring IBAN is formatted correctly, without whitespace, etc.
    string formattedIbanStr = iban.ToString(IbanFormat.Electronic); // "NL91ABNA0417164300"
}

I do think your proposal/request makes sense, but would have to investigate further if the sanitation logic can be kept solely in the parser and remove it from the validator (making it strict), without causing (breaking) side effects to the Iban type as mentioned previously.

kimpenhaus commented 1 year ago

@skwasjer thanks for your suggestion but that would force us to manually "translate" the values each time.

assume following infrastructure: we have a validation component which prevents invalid data to be placed into a queue. at the end of the queue sits a processor which forwards some of the payload to a thrid party component (which validates the iban (again)). actually we are not intercepting this process but with the conversion we have to (every time accessing and forwarding this attribute/property). second is, that with the validation sitting in front we could prevent invalid data coming into the system we have to deal with then. once it is in our system we have to handle all faults occuring. one more thing to consider - we actually don't won't to change the payload on our side - pretty much the same as we are responsible for any errors occuring in that step.

it would be great to decouple validation from parsing and have the possibility to validate more detailed. I think it makes absolutly sense once validation (weaker or stricter) is done and the iban is valid to transform in a more technical representation which gives the abilities it has nowadays with your library.

thanks!

skwasjer commented 1 year ago

Moving the sanitation to the parser is trivial, it is the breaking change/side effect that either needs to be documented well, or we have to figure out if it can be made opt-in as per your OP. To make matters more complicated, the side effects unfortunately would also affect for example the FluentValidator/DataAnnotations extensions library. I will give it some thought, and come back to you.

skwasjer commented 1 year ago

I've addressed your request in #95. Since it causes a behavior change, I will push it in next major soon-ish after I update docs and address some other things I want to improve.

kimpenhaus commented 1 year ago

Good morning Martijn,

that is great to hear - thank you so much for all your effort you put into this library 🙏🏼

skwasjer commented 1 year ago

v5.7.0

kimpenhaus commented 1 year ago

works perfectly - thanks Martijn 🙏🏼