dev-xo / remix-auth-totp

A Time-Based One-Time Password (TOTP) Authentication Strategy for Remix-Auth.
https://totp.fly.dev
MIT License
418 stars 28 forks source link

Can `maxAttempts` be optional in `TOTPGenerationOptions` interface? #25

Closed jacobiajohnson closed 1 year ago

jacobiajohnson commented 1 year ago

First of all, thanks for your work on this project!

I am setting a different period for my TOTP generation, and noticed that doing so also required me to set the maxAttempts field. Is there a reason we have to manually set this field when overriding other fields in the TOTPGenerationOptions?

If there's a good reason that's fine! I will just continue to set to the default maxAttempts in that case.

https://github.com/dev-xo/remix-auth-totp/blob/1011fea3677f30d6c0738ad14b7ec171d08aec5a/src/index.ts#L19-L57

dev-xo commented 1 year ago

I think you are right @jacobiajohnson.

That was a last moment addition in order to add more security into the Strategy and remove some client-side work for the developer. I will look into it, but I think we could simply set it as optional.

Gonna leave this Issue open until I figure it out! Will look into it as soon as possible. Thanks for noticing!

dev-xo commented 1 year ago

Issue addressed on here @jacobiajohnson. Feel free to give it a try on v1.1.1.

Closing this for now, thanks for noticing!