Don't confirm SMS setup during sign in

jeremyevans / rodauth

Ruby's Most Advanced Authentication Framework

http://rodauth.jeremyevans.net

MIT License

1.65k stars 95 forks source link

Don't confirm SMS setup during sign in #377

Closed Bertg closed 7 months ago

Bertg commented 7 months ago

This is a possible simple fix for #376

Introduce failing test
Fix issue where we accidentally accept a non-confirmed SMS code during sign in

jeremyevans commented 7 months ago

Thanks for this. The test is especially helpful. It passes both with the fix here and the fix I posted in https://github.com/jeremyevans/rodauth/issues/376#issuecomment-1854136723. I'm thinking of combining the two, that way we can skip the query in sms_remove_failures if the sms still needs confirmation.

Bertg commented 7 months ago

@jeremyevans Your call on how to proceed.

For me the main thing I wanted to achieve here is writing the test, as that unambiguously proved the issue. I think the quick return in sms_remove_failures is good, but can be combined with the conditional update from your patch, so that any race condition in the future would also be covered.

I'll apply your changes - as I see them - to this PR.

Bertg commented 7 months ago

@jeremyevans Updated PR is here ☝️