Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Bug: Super Admins cannot setup Time Based One-Time Password as first option Two Factor #4409

Closed
spenserhale opened this issue Apr 28, 2023 · 3 comments

Comments

@spenserhale
Copy link
Contributor

Expected Behavior

GIVEN a multisite WordPress and a super_admin user without any two-factor options and wpcom_vip_is_two_factor_forced is true

WHEN the same super_admin logs in and submits an authentication code to setup two-factor time-based one-time password

THEN Two_Factor_Totp::rest_setup_totp is called to validate the code and return a success or invalid code response.

Actual Behavior

THEN the user always receives 'Sorry, you are not allowed to do that.' permission error

Cause

\wpcom_vip_two_factor_filter_caps() is not setting 'edit_user' capability because Two_Factor_Totp configuration is converting user_id argument to a float, so $user_id === $args[0] does not pass when it should.

Detailed Trace

An example where 2 === 2.0 fails. We can see JS Posted a "2", but REST Controller changed to (float) 2.0, but in capability checks, user_id is an int.

two-factor.php:194, wpcom_vip_two_factor_filter_caps()

$user_id = 2
$args = [
    0 => (float) 2.0
]

capabilities.php:873, current_user_can()

$capability = 'edit_user'
$args = [
    0 => (float) 2.0
]

class-two-factor-totp.php:94, Two_Factor_Totp->{closure:/wp/wp-content/mu-plugins/shared-plugins/two-factor/providers/class-two-factor-totp.php:93-95}
class-wp-rest-server.php:1018, WP_REST_Server->dispatch()

$request: WP_REST_Request {
    "params" => [
        "POST" => [
            "user_id" => (float) 2.0,
            "key" => "key-string",
            "code" => "123"
        ]
    ]
}

index.php:17

$_POST = [
    "user_id" => "2",
    "key" => "key-string",
    "code" => "123"
]

Stack Trace

two-factor.php:194, wpcom_vip_two_factor_filter_caps()
class-wp-hook.php:308, WP_Hook->apply_filters()
plugin.php:205, apply_filters()
capabilities.php:838, map_meta_cap()
class-wp-user.php:778, WP_User->has_cap()
capabilities.php:981, user_can()
capabilities.php:873, current_user_can()
class-two-factor-totp.php:94, Two_Factor_Totp->{closure:/wp/wp-content/mu-plugins/shared-plugins/two-factor/providers/class-two-factor-totp.php:93-95}()
class-wp-rest-server.php:1138, WP_REST_Server->respond_to_request()
class-wp-rest-server.php:1018, WP_REST_Server->dispatch()
class-wp-rest-server.php:442, WP_REST_Server->serve_request()
rest-api.php:410, rest_api_loaded()
class-wp-hook.php:308, WP_Hook->apply_filters()
class-wp-hook.php:332, WP_Hook->do_action()
plugin.php:565, do_action_ref_array()
class-wp.php:399, WP->parse_request()
class-wp.php:780, WP->main()
functions.php:1332, wp()
wp-blog-header.php:16, require()
index.php:17, {main}()

Steps to Reproduce the Problem

  1. setup a multisite and create a new user that is a super_admin
  2. log in as the new super_admin and go to the edit profile page
  3. submit an authentication code

Additional Information

Blame log: 1c99d6b
Take 2: Update Two-Factor plugin to 0.8.1  (#4322)

However, there was no git history for the file, but the commit message referenced the 0.8.1 version, so the bug is likely coming from the original source of this class/ plugin. Or it is not a problem on wordpress.com because the checks are not strict (== instead of ===).

User Workarounds

  • Require another user to setup up the user
  • Setup another 2fa option first, such as email
@spenserhale
Copy link
Contributor Author

Tagging @rebeccahum

@rebeccahum
Copy link
Contributor

Please open upstream in https://github.com/WordPress/two-factor. Thank you!

@spenserhale
Copy link
Contributor Author

Created issue upstream: WordPress/two-factor#559

rinatkhaziev pushed a commit to spenserhale/vip-go-mu-plugins that referenced this issue Jul 13, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants