WordPress / two-factor

Two-Factor Authentication for WordPress.
https://wordpress.org/plugins/two-factor/
GNU General Public License v2.0
724 stars 151 forks source link

Revalidate two factor settings prior to allowing any two-factor changes to an account. #529

Closed dd32 closed 1 year ago

dd32 commented 1 year ago

What?

This PR requires that the user "revalidate" their two-factor details prior to changing their two-factor settings. The grace-time from last-2fa-validation to changes is 10 minutes (Twice this for the save, so you can start editing at minute 9, and save at 18 minutes).

Why?

It's generally accepted that a user should not be able to change their 2FA settings without validating that the user currently has their 2FA token. For example, consider an account left logged in on a public computer (or left unattended in an office), a malicious user should not be able to add a new key or create new backup codes.

How?

This works by building off #528 to track when the user last validated their 2FA tokens, and preventing updates unless the session is 2fa'd recently.

Testing Instructions

  1. Login
  2. Validate you can change 2FA settings
  3. Wait 15 minutes (Or change the grace time via two_factor_revalidate_time to a lower time)
  4. Validate you can't change 2FA settings after the grace-time is expired
  5. Revalidate your 2FA, check you can change 2FA settings again.

Screenshots or screencast

Changelog Entry

Added Security - Validate the user has access to their 2FA keys prior to changing 2FA details.

Tickets

Fixes #484.

527 and #528 have been split out of this, but are included in the git commit history of this branch.

dd32 commented 1 year ago

Examples of this PR:

Example
Just logged in Screenshot 2023-03-27 at 1 09 31 pm
Past 2FA grace period Screenshot 2023-03-27 at 1 46 15 pm
revalidate flow

The UI is "ugly" in that it's just an alert box that sends you off to a wp-login page, it's not a pretty inline thickbox iframe or anything. This fulfils the basic requirements of the issue, but obviously needs UX, design, and other design work done on it. Before that can start though, I think it needs some technical agreement on the direction.

StevenDufresne commented 1 year ago

Based on my testing, provided the page is not refreshed, actions can still be carried out, is that correct?

dd32 commented 1 year ago

@StevenDufresne I'm not sure I follow, you'll need to explain it better.

StevenDufresne commented 1 year ago

Here's what I did.

  1. Sign in, visit wp-admin/profile.php
  2. Wait (15 mins although i updated this locally to 1min)
  3. Click "Generate Validation Codes"
  4. Codes are generated
  5. Reload the page
  6. See "revalidate" notice.

So if a user leaves their profile page open, a third party could modify data provided they don't reload the page?

dd32 commented 1 year ago

So if a user leaves their profile page open, a third party could modify data provided they don't reload the page?

Ahh, gotcha.

I suspect this is that you were still within the grace-period, The notice shows up at $time but you can save forms for $time * 2.

* 2 is possibly too long, and maybe it should be simply + MINUTE_IN_SECONDS, or + 2 * MINUTE_IN_SECONDS. https://github.com/WordPress/two-factor/pull/529/files#diff-ee04f1d323104504c6bfa38dd11ef43e78d1dbac2883293af0b5c310d16e9519R1133-R1143

The reasoning for that is that you should be able to view the form at $expiry - 1 second and save the form. $time * 2 is something that WordPress has in a few places as the grace between generation and save.

iandunn commented 1 year ago

I suspect this is that you were still within the grace-period

I tested this a bit and that's what it seems like to me too.

dd32 commented 1 year ago

Changes in the commits above that are notable since reviews: