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 their first Two Factor option on WP VIP #559

Closed
spenserhale opened this issue Apr 28, 2023 · 0 comments · Fixed by #560
Labels
Backup Codes TOTP Time-based One-time Passwords
Milestone

Comments

@spenserhale
Copy link
Contributor

spenserhale commented Apr 28, 2023

Describe the bug

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}()

Notes

Similar to #557

Originally submitted to: Automattic/vip-go-mu-plugins#4409

Steps to Reproduce

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

Screenshots, screen recording, code snippet

No response

Environment information

No response

Please confirm that you have searched existing issues in this repository.

Yes

Please confirm that you have tested with all plugins deactivated except Two-Factor.

Yes

spenserhale added a commit to spenserhale/wordpress-com-two-factor that referenced this issue Apr 28, 2023
@dd32 dd32 added TOTP Time-based One-time Passwords Backup Codes labels May 2, 2023
@dd32 dd32 added this to the 0.9.0 milestone May 2, 2023
@dd32 dd32 closed this as completed in #560 May 2, 2023
dd32 pushed a commit that referenced this issue May 2, 2023
…rict user_id capability checks (#560)

* Fixing bug where Super Admins cannot setup Time Based One-Time Password as first Two Factor option on WP VIP
* refactor(Two_Factor_Backup_Codes): setting user_id type to integer instead of number to make consistent with Two_Factor_Totp

Fixes #559
Fixes #557
dd32 pushed a commit that referenced this issue May 22, 2023
…rict user_id capability checks (#560)

* Fixing bug where Super Admins cannot setup Time Based One-Time Password as first Two Factor option on WP VIP
* refactor(Two_Factor_Backup_Codes): setting user_id type to integer instead of number to make consistent with Two_Factor_Totp

Fixes #559
Fixes #557
@dd32 dd32 mentioned this issue May 22, 2023
14 tasks
@jeffpaul jeffpaul modified the milestones: 0.9.0, 0.8.2 May 24, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Backup Codes TOTP Time-based One-time Passwords
Projects
None yet
3 participants