twilio / authy-devise

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

Prevents to not break rails apps when Generic authenticator tokens are still not enabled at Authy Twilio #161

Closed taylorrf closed 2 years ago

taylorrf commented 3 years ago

While using the Authy Twilio integration with my rails application, I found when we enable config.authy_enable_qr_code at the initializers/devise.rb and still does not had enabled the Generic authenticator tokens at the Twilio Authy application it will break the rails application when trying to access the response.qr_code method:

auth_manager_authy_error

Since we have many environments, in order to protect our applications from a breakdown (in production for example) we decided to overwrite internally the GET_verify_authy_installation as this PR suggests to be considered by.

Contributing to Twilio

All third-party contributors acknowledge that any contributions they provide will be made under the same open-source license that the open-source project is provided under.

philnash commented 3 years ago

Thanks for raising this. I am intrigued though. This gem throws an error when your application doesn't have Generic Tokens enabled in order to ensure that you enable Generic Tokens before you put it into production. It also means that there is no confusion where you turn on the setting in the application, but still don't see a QR code in the application.

I don't think silently swallowing the issue is the right response here. Though I do understand you want to avoid errors in production.

What do you think about trying make the request for the QR code, rescuing the error and logging the reason it wasn't a success? Or making it a config option not to throw on errors?

taylorrf commented 3 years ago

Thank you for the feedback Phil! :)

Your suggestion to keep raising the error and only silence it when the new config option was set makes much more sense to me, that's the way I'm working to update it now. Something like config.authy_raise_errors = false (default: true) sounds good?

PS: I was intrigued looking at the build errors, and I opened this other PR to fix it.

philnash commented 3 years ago

Sounds good. Can you make the config value config.authy_raise_qr_code_errors so that it is obvious what it refers to?

Thanks for looking at the build errors too, I've not seen those before!

taylorrf commented 3 years ago

@philnash Hey Phil! Just added as suggested the new config authy_raise_qr_code_errors to control this edge case. Please let me know what your think about it.

philnash commented 2 years ago

This library is no longer actively maintained. The Authy API has been replaced with the Twilio Verify API. Twilio will support the Authy API through November 1, 2022 for SMS/Voice. After this date, we’ll start to deprecate the service for SMS/Voice. Any requests sent to the API after May 1, 2023, will automatically receive an error. Push and TOTP will continue to be supported through July 2023.

Learn more about migrating from Authy to Verify.

Please visit the Twilio Docs for:

Please direct any questions to Twilio Support. Thank you!