ActiveLogin / ActiveLogin.Identity

Parsing and validation of Swedish identities such as Personal Identity Number (svenskt personnummer) in .NET.
https://activelogin.net
MIT License
56 stars 9 forks source link

Data validation in forms, data imports or similar #140

Closed pontusdacke closed 3 years ago

pontusdacke commented 3 years ago

Is your feature request related to a problem? Please describe. It is not possible to use this plugin for reliable data validation in forms, data imports, or other places when the parsing is this generous with the inputs.

This "guid" will be correctly parsed as an identity number.

var ssn = ActiveLogin.Identity.Swedish.SwedishPersonalIdentityNumber.Parse("7d0aabeb-0be8-bbb08-beee-eeeeeeee9281");

Example based on a real experience

What area is it related to PersonalIdentityNumber

Describe the solution you'd like

I believe there should be a standard set of allowed formats that the tool will accept by default (10/12 with/without dash) and then users could be able to add options that allows to modify what formats it can allow with the current implementation being "allow all".

viktorvan commented 3 years ago

Excellent point, (that is a "tricky" Guid, by the way šŸ˜‰ ).

Let's look into adding configuration for how strict the parser should be.

What allowed formats do you think is needed? 10-digits 12-digits 10 or 12 digits (either of above) Allow all (current implementation, any of the above and input will be cleaned of any additional characters)

Is it of value to be able to specify if a delimiter is allowed or not? For a 10-digit number we could as default interpret a missing delimiter as a dash, which I think is what the current implementation does. For a 12-digit number it never makes sense to have a delimiter, since it has no meaning, the default would be to just remove it.

Or is it a requirement for you @pontusdacke to be able to get a parsing error if a 10-digit number does not have a delimiter, or if a 12-digit number does have a delimiter?

@PeterOrneholm any additional thoughts?

pontusdacke commented 3 years ago

What allowed formats do you think is needed?

The formats you suggested will cover the needs we have that I can think of at the moment.

Is it of value to be able to specify if a delimiter is allowed or not?

Probably not! Always allowing it on 10-digits, and always ignoring it on 12 digits seems very reasonable when I think about it.

Is it a requirement to be able to get a parsing error

Nope!

Thanks for the quick feedback šŸ‘

SpaceOgre commented 3 years ago

@viktorvan Any progress on this? I just started using the attribute for validation in my API and ran into this problem.

viktorvan commented 3 years ago

Yes, I have added new test cases for the stricter parsing. I will try to schedule some more time to work on this for this week.

PeterOrneholm commented 3 years ago

Awesome work @viktorvan!

Iā€™m not sure, but maybe we should include this pattern in some semi strict mode: YYYYMMDD-BBBC

I know that it is not a valid PIN, but I think Iā€™ve seen large websites ā€promotingā€ that pattern.

viktorvan commented 3 years ago

I donā€™t like it šŸ˜

In that case you have the option of using StrictMode.Off, so we are only talking about the edge case where someone wants to use ā€˜strictā€™ parsing, but also allow an invalid hyphen in a 12-digit identity number. Semi-strict indeed.

I can maybe be convinced otherwise, but my reasoning is that this library parses (valid) identity numbers. If you need to parse ā€œcustomā€ versions of identity numbers, then you can handle that in your client code before calling our parser.

Itā€™s not hard for a client to strip the delimiter from 12-digit string before passing it to the Parse-function. But if we allow it in our library itā€™s yet another scenario that needs to be tested and maintained.

Then again I also have seen that this pattern is common.

viktorvan commented 3 years ago

Maybe it could be itā€™s own setting? Instead of just passing StrictMode we can have a config- or settings-object with a StrictMode and AllowHyphen properties?

PeterOrneholm commented 3 years ago

You are right. It should be either correct, or loose. Do you want semi-strict you can remove all dashes.

Yes, for next major release I'd vote setting default to StrictMode.TenOrTwelveDigits.

Speaking of that naming. I would say that in 99.99% of the cases you should use StrictMode.TenOrTwelveDigits, as a consumer of the service I expect to be able to use them both (10 or 12 digits). Can we make it clear that that is the prefered option? An alias `StrictMode.AllValid? It will be solved when strict mode is the default. Maybe a new major version could be released now?

viktorvan commented 3 years ago

Let's just do a major release and use StrictMode.TenOrTwelveDigits as the default.

I think the suggested name StrictMode.AllValid is maybe a bit confusing; it makes me wonder if I can have invalid parsing?

viktorvan commented 3 years ago

@pontusdacke @SpaceOgre there is a new beta-release available now that supports StrictMode that you can try out. https://www.nuget.org/packages/ActiveLogin.Identity.Swedish/3.0.0-beta-1