two-factor
two-factor copied to clipboard
Prevent other plugins from accidentally circumventing two factor authentication
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.
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.)
Although dropping the 'wp_login' hook priority can workaround the issue in the short-term, I think #406 would be better to implement.
🤔 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:
- When a user submits the username/password form, we
set_transient( '_two_factor_step_1_user_' . $user_id, true, 0 ). - When their 2nd factor is authenticated, we
delete_transient( '_two_factor_step_1_user_' . $user_id ). - When wp-admin loads, we check if
_two_factor_step_1_user_ . $user_idexists. If it does, we display anadmin_notice, or email the site admin, etc. Related: #459, #462, #476
I think this was fixed by #502. Is that right, @dd32 ? If not, please reopen it.