thingco / shared-frontend-libs

0 stars 0 forks source link

Feat/split pin stages #73

Closed ThingCoDanW closed 3 years ago

ThingCoDanW commented 3 years ago

Added new validatingPin stage. Success will move machine into changingPin stage.

TODO:

DanCouper commented 3 years ago

May need to handle incorrect pin attempts, if we care about that here?

This I'm not sure about -- I don't know if there's any benefit. It's easy enough to do what we do for OTP input and fail them after (say) the third try, but what do we do if that happens? Could completely log them out, but there are only two situations where they can be at this point in the authentication:

So I don't think we need any kind of handling for incorrect PIN attempts. They can always hit cancel if they've managed to forget what PIN they entered within a minute or so of entering it, and next time they open the app they can hit "forgot PIN" and reset it immediately by logging back in, so I don't think there's any security benefit to be gained (the whole PIN thing is security theatre anyway IME 🤷 )

DanCouper commented 3 years ago

~Add tests.~ Double check my tests

Tests look fine. I'll do a quick call to walk through my thinking, sorry for it not being totally clear. The lib has been rewritten specifically to be as easily testable as possible, but it needed quite a bit of faffing on to get to the point where I had a way to quickly build out the number of tests I needed to cover as much as possible, and in turn that thinking hasn't been communicated at all as well as it could have been.

It needs a full integration test doing -- like E2E but can just use react-testing-library to render it using JSDOM rather than a Puppeteer test, and it'll look like the model tests for the main auth-system.ts file. This is required because atm there aren't any tests checking the other values returned from the hooks.