twilio / authy-devise

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

Is it possible to disable 2FA for another user, other than yourself (`current_user`)? #159

Closed dvjones89 closed 3 years ago

dvjones89 commented 3 years ago

Hi there,

Firstly, I apologise if this is a daft question and I've totally misunderstood/overlooked something simple, though I'm hoping someone can help point me in the right direction.

I'm currently trying to implement an (admin-only) feature, allowing Authy 2FA to be disabled for a particular user.

Initially I made use of the user_disable_authy_path route helper, without supplying any additional arguments:

 <%= link_to "Disable two-factor authentication", user_disable_authy_path, method: 'POST', class: "btn btn-outline-danger" %>

During testing, however, I realised that this wants to disable 2FA for the current_user, rather than the user instance being viewed by the admin.

After searching in GitHub to see how other open-source repos are using the user_disable_authy_path path, I saw a handful of people passing in an id parameter, so I gave that a try:

<%= link_to "Disable two-factor authentication", user_disable_authy_path(id: @user.id), method: 'POST', class: "btn btn-outline-danger" %>

Unfortunately, as far as I can tell, 2FA is still being disabled on current_user, rather than @user.

From reading the authy-devise source code, I think I've found the line where resource is being consistently set to current_user: https://github.com/twilio/authy-devise/blob/c5955b88dfd561180eecd9d93b708bd76839539a/app/controllers/devise/devise_authy_controller.rb#L184

Is there a way to override this behaviour such that resource can be set to an arbitrary userrecord?

Thanks in advance for any help πŸ™ Dave J

philnash commented 3 years ago

You are right in that the resource is the current_user. But, given the authorisation on the POST_disable_authy action also only checks that they are the current user, you wouldn't want that action to be able to disable 2FA for an arbitrary user, because any user could disable 2FA for any other user.

Instead, I would build your own action, within your admin section, that could disable 2FA for a user, but is also only available to admins.

Thanks for the kind mention on Twitter earlier btw! Its been too long!

dvjones89 commented 3 years ago

Hi @philnash πŸ‘‹

Thanks for your speedy reply!

Thanks, the existing behaviour makes complete sense and, as you suggest, I'll build something custom for this admin functionality.

I'd say let's meet up for beer/coffee once the COVID restrictions lift, though since you've moved over to Australia, that might be easier said than done πŸ˜‚.

Take care and keep up the great work πŸ™πŸΌ

philnash commented 3 years ago

Awesome, let me know if there's anything else I can help with. And yes, a beer/coffee would be great were I not now the other side of the world! πŸ˜„