Houdini / two_factor_authentication

Two factor authentication extension for Devise
MIT License
400 stars 267 forks source link

How to handle when users are recoverable #29

Open jonuhal opened 10 years ago

jonuhal commented 10 years ago

When I add recoverable to my user model and try to have a user go through the Devise password recovery workflow, the after_authentication callback is never executed. It seems like this is how Devise decides to handle the login callbacks. But that means that when I recover a user password, the user does not get the code.

This is very similar to my situation, but in more generic terms:

https://groups.google.com/forum/#!topic/plataformatec-devise/30oxTAWHdSs

Any thoughts?

Houdini commented 10 years ago

Hello, you showed nice user case.

As for now we never ask code during password recovery. It make sense, if user forgot his password, he lost his shared secret for sure. Separate email is like second_factor_authentication.

On other hand it would be useful to have separate option to tune this.

What is the right behaviour? What do you think?

jonuhal commented 10 years ago

So I should explain a bit more. I am running 0.2 because trying to upgrade to 1.0 was not a simple task for me. For now, I'm stuck on 0.2.

I have my ApplicationController with the following method:

def after_sign_in_path_for(resource) if resource.valid_mfa_cookie?(request) request.cookie_jar.signed[:_return_to] if request and request.cookie_jar super else flash.clear two_factor_authentication_path_for(resource) end end

I did this to be able to store an encrypted version of the two factor auth token on the client side cookies so they could use the same location to login for however long I set the cookie expiration. The issue is that this relies on lib/two_factor_authentication/hooks/two_factor_authenticatable.rb running the appropriate callback for:

Warden::Manager.after_authentication do |user, auth, options|

Since this callback doesn't get fired when a user logs in through the password recovery (see my original post for a detailed explanation on why that doesn't happen), the code in the user model is never executed and the password is never sent. As far as I can see with 1.0, this is still the same case. In my case, with my "after_sign_in_path_for" override, I am getting redirected to the two_factor_authentication page after a successful password recovery, but since the Warden::Manager.after_authentication callback does not get run, no auth code is generated and sent.

In my opinion, I would assume one of two things happened after a successful password recovery. 1) If a user needs two factor authentication, they should get the two factor auth code entry page and a code is sent to them. 2) The user is not auto-logged in to the application, but is instead kept logged out and forwarded to a log in page that would then properly trigger the two factor code generation callback.

I can see positives and negatives to both approaches.

jonuhal commented 10 years ago

Wrong button. Definitely want to keep this open for now.

docstun commented 10 years ago

I think the 2fa callback should be triggered also when the user enters a new password in passwords#update. 2fa is a mechanism to also prevent social engineering attacks and should also protect the user account when the email account is compromised. Any plans to add this?

monfresh commented 8 years ago

I'm running into the same issue, and in my case, I'm overriding after_resetting_password_path_for(resource) in the Devise passwords_controller to take the user to the show action of the two_factor_authentication_controller. However, I have modified the show action to check if the user is already fully authenticated, and if so, they get redirected somewhere else.

The problem is that after the user resets their password is_fully_authenticated? is returning true. I don't understand why. That seems like a bug, no? When a user signs in via the Devise sessions_controller, is_fully_authenticated? is returning false as expected, and the user is prompted to enter their OTP. Why is the same behavior not happening after Devise signs the user in after they reset their password?

I don't understand the code enough to know what I need to modify to make this work.

Thanks for any help anyone can provide.

benjaminwols commented 7 years ago

To force a user to enter his tfa code after recovering account, you have to add the following line the password_controller update method:

warden.session(resource_name)[TwoFactorAuthentication::NEED_AUTHENTICATION] = true

This cookie in the warden session is used in is_fully_authenticated? to determine if the user needs to enter his tfa code

monfresh commented 7 years ago

What we ended up doing is setting config.sign_in_after_reset_password = false in the Devise config. This means the user does not get automatically signed in after resetting their password. They have to sign in again manually, and when they do, they will be prompted for their 2FA code.