twilio / authy-devise

Authy Devise plugin to add Two-Factor Authentication
MIT License
201 stars 84 forks source link

Remember device with OneTouch login #127

Closed ruralopez closed 4 years ago

ruralopez commented 4 years ago

Hi! Is there any reason to avoid remember device on OneTouch login flow? I noticed that remember_device helper is not used anywhere in OneTouch login process, and this could be a very useful usability improvement.

I could submit a PR if necessary, but I want to be sure this is not the intended behavior cause some security issue.

Thanks!

philnash commented 4 years ago

I don't believe there is a security reason to not include it. Though perhaps there is a UX question. That is, at which stage of the process do you include the check box to remember the device when you are authenticating via OneTouch?

I'm absolutely for including this, if we can find a good way to include it in the UI.

Thanks for bringing it up and let me know if you need any pointers around the repo.

ruralopez commented 4 years ago

Thanks for your answer @philnash, maybe we can use the same checkbox currently used in verify-token form. Just retrieving state on onetouch-status success trigger and sending the variable as a query string to the server could be enough (like onetouch_uuid). Then, in devise_authy_controller.rb we can replicate remember_device helper in 'approved' case for GET_authy_onetouch_status.

philnash commented 4 years ago

Sounds good! Give it a go and we will see how it comes together.

ruralopez commented 4 years ago

Hi @philnash, I already created a Pull Request with a possible solution for this Issue. Is there anything else I should do? Please if you can review my code would be awesome. Thanks!

philnash commented 4 years ago

Thanks for the PR! I will take a look soon :)