two-factor icon indicating copy to clipboard operation
two-factor copied to clipboard

Prevent other plugins from accidentally circumventing two factor authentication

Open kkmuffme opened this issue 5 years ago • 2 comments

Since two-factor relies on the default wp_login action, other plugins which use the same action could use an earlier priority, thus leading to two-factor getting circumvented - this results in an unwanted security risk for most WP site owners.

in class-two-factor-core.php add_action( 'wp_login', array( __CLASS__, 'wp_login' ), 10, 2 );

Since 10 is WP's default priority, various plugins and themes use this action with a priority between -10 to 9 to add a feature - this however means that two-factor might get circumvented by those, which is a major security issue.

I think we should change the priority 10, to -9999, thus ensuring that only devs who actively want to circumvent two-factor authentication also do so. From a end user's (= WP website owner) perspective this makes the most sense, because: I want to make sure that two-factor is used, no matter which theme/plugins I install. Those should not reduce my site's security by accident.

Theoretically we could also change it to PHP_INT_MIN which would make sure that two-factor is never circumvented - I think this is not a good idea though, as there may be cases where a plugin may want to circumvent two-factor.

kkmuffme avatar Sep 30 '20 10:09 kkmuffme

Yes, please! We've noticed that if we use ManageWP's automatic login feature (clicking inside ManageWP to open the site's WordPress Dashboard) it does indeed circumvent Two-Factor.

(We're already authorized via ManageWP -- and use their 2FA method on our logins there -- but 2FA also isn't mandatory by default in ManageWP.)

blogtutor avatar Jul 20 '21 13:07 blogtutor

Although dropping the 'wp_login' hook priority can workaround the issue in the short-term, I think #406 would be better to implement.

r-a-y avatar Jan 21 '22 22:01 r-a-y

🤔 I wonder if there's a way that this plugin could automatically detect if someone is logged in through the UI w/out 2FA being triggered?

If there's a reliable way to do that, it could be more effective than bumping the wp_login priority. Or we could do both for extra layers of protection.

Here's a very rough idea:

  1. When a user submits the username/password form, we set_transient( '_two_factor_step_1_user_' . $user_id, true, 0 ).
  2. When their 2nd factor is authenticated, we delete_transient( '_two_factor_step_1_user_' . $user_id ).
  3. When wp-admin loads, we check if _two_factor_step_1_user_ . $user_id exists. If it does, we display an admin_notice, or email the site admin, etc. Related: #459, #462, #476

iandunn avatar Dec 08 '22 19:12 iandunn

I think this was fixed by #502. Is that right, @dd32 ? If not, please reopen it.

iandunn avatar Feb 06 '23 21:02 iandunn