search

HermannBjorgvin / Kennitala

Icelandic national ID (kennitölur) utilities for servers and clients.
MIT License
36 stars 10 forks source link

Random numbers from `kennitala.generatePerson` is not following all the rules #6

Closed MiniGod closed 5 years ago

MiniGod commented 6 years ago

According to skra.is, the twoRandomDigits should be from 20 to 99:

Næstu tveir tölustafir hafa enga merkingu og er þeim úthlutað í röð frá og með 20

However the numbers generated in kennitala.generatePerson are:

var twoRandomDigits = "" + Math.floor(Math.random() * 10) + Math.floor(Math.random() * 10)

This rule is also not followed in evaluate.

HermannBjorgvin commented 6 years ago

@MiniGod thanks for bringing that to my attention, I'll hopefully have a patch released tonight along with updated unit tests for this.

This also means kennitala.generateCompany is broken currently.

Will patch for all versions.

HermannBjorgvin commented 6 years ago

@MiniGod my unit tests started failing when I implemented this. I have the old Plain Vanilla SSN as a test case, which has a 7-8th digit below 20. I'm going to wait until I have a definitive answer on whether this applies to companies or not. Will give an update soon.

See failing testcase: https://www.rsk.is/fyrirtaekjaskra/leit/kennitala/6010100890

HermannBjorgvin commented 6 years ago

I spoke with a person at Þjóðskrá who handles the stored procedure that generates kennitölur. So two things, company kennitölur are generated by Ríkisskattstjóri, and they seem to just have the 7-8th numbers as random or incrementing from 00-99, they are not generated by Þjóðskrá. And the 2nd thing is that individuals are indeed incrementing from 20-99.

We could change the evalute function to check for 20-99 for individuals, or we could allow for it assuming that in the future if Þjóðskrá starts generating kennitölur at 00-99 they will pass verification. A good point can be made that it makes the library more future proof since the rules now at first glance seem less strict for the 7-8th numbers since RSK can generate ones with 00-99.

I'm going to patch the generate person function tonight, I'm going to keep the numbers randomly generated though just between 20-99. The way þjóðskrá does it is generate a set of all possible kennitölur for a given day and then finds the lowest unique kennitala. So while the function only returns one value I think it's better to give a random value than always give 20 or 21 or 22 for all kennitölur.

Maybe it would be useful to alter the function to take a second parameter and return a set of all kennitölur for a given day, something to think about.

Thank you for a good issue!

HermannBjorgvin commented 5 years ago

After a bit of a struggle with learning how git tags work and how to patch older minor versions I can say that this issue is closed.

kennitala.generatePerson now selects a random number between 20-99

I chose not to invalidate persons with a 7-8th digits under 20 because the rules do not seem strict enough to eliminate the possibility of a future change to the way kennitölur are generated.