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.

Sign up for GitHub

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

Enforce 2FA for super admins #17

Merged
merged 1 commit into from
Nov 18, 2022
Merged

Enforce 2FA for super admins #17

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions tests/bootstrap.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
* PHPUnit bootstrap file
*
* phpcs:disable WordPress.Security.EscapeOutput.OutputNotEscaped -- This is a shell script.
* phpcs:disable WordPress.WP.GlobalVariablesOverride.Prohibited -- This is intentional and necessary.
*/

$_tests_dir = getenv( 'WP_TESTS_DIR' );
Expand All @@ -23,6 +24,10 @@
* Manually load the plugin being tested.
*/
function _manually_load_plugin() {
// Mimic w.org capes.php.
$GLOBALS['supes'] = array();
$GLOBALS['super_admins'] = array();

require dirname( __DIR__, 2 ) . '/two-factor/two-factor.php';
require dirname( __DIR__ ) . '/wporg-two-factor.php';
}
Expand Down
189 changes: 187 additions & 2 deletions tests/test-wporg-two-factor.php
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,11 +1,39 @@
<?php

//use function WordPressdotorg\Two_Factor\{};
use function WordPressdotorg\Two_Factor\{ user_requires_2fa };

defined( 'WPINC' ) || die();

class Test_WPorg_Two_Factor extends WP_UnitTestCase {
public function test_two_factor_providers() {
protected static WP_User $privileged_user;
protected static WP_User $regular_user;

/**
* Initialize things when class loads.
*/
public static function wpSetUpBeforeClass( WP_UnitTest_Factory $factory ) : void {
// Roles, etc will be assigned dynamically by individual tests.
self::$privileged_user = $factory->user->create_and_get( array( 'user_login' => 'privileged_user' ) );

self::$regular_user = $factory->user->create_and_get( array(
'user_login' => 'regular_user',
'role' => 'contributor',
) );
}

/**
* Reset things that aren't automatically reset by Core. Runs after each test.
*/
public function tear_down() : void {
parent::tear_down();

$GLOBALS['super_admins'] = array();
}

/**
* @covers WordPressdotorg\Two_Factor\two_factor_providers
*/
public function test_two_factor_providers() : void {
$actual = Two_Factor_Core::get_providers();

$this->assertArrayHasKey( 'Two_Factor_Totp', $actual );
Expand All @@ -16,4 +44,161 @@ public function test_two_factor_providers() {
$this->assertArrayNotHasKey( 'Two_Factor_Email', $actual );
$this->assertArrayNotHasKey( 'Two_Factor_Dummy', $actual );
}

/**
* Enable a 2FA provider on the given user.
*/
protected function enable_2fa_for_user( int $user_id ) : void {
// This should start counting at one instead of zero, to match `Two_Factor_Core`.
update_user_meta( $user_id, Two_Factor_Core::ENABLED_PROVIDERS_USER_META_KEY, array( 1 => 'Two_Factor_Totp' ) );
update_user_meta( $user_id, Two_Factor_Core::PROVIDER_USER_META_KEY, 'Two_Factor_Totp' );
update_user_meta( $user_id, Two_Factor_Totp::SECRET_META_KEY, 'foo bar bax quiz' );

$this->assertTrue( Two_Factor_Core::is_user_using_two_factor( $user_id ) );
}

/**
* @covers WordPressdotorg\Two_Factor\remove_super_admins_until_2fa_enabled
*/
public function test_super_admin_removed_when_2fa_not_enabled() : void {
global $supes, $super_admins;
$supes[] = self::$privileged_user->user_login;
$super_admins[] = self::$privileged_user->user_login;

$this->assertTrue( is_super_admin( self::$privileged_user->ID ) );
$this->assertTrue( user_requires_2fa( self::$privileged_user ) );
wp_set_current_user( self::$privileged_user->ID, self::$privileged_user->user_login ); // Triggers remove_super_admins_until_2fa_enabled().
$this->assertFalse( is_super_admin( self::$privileged_user->ID ) );
}

/**
* @covers WordPressdotorg\Two_Factor\remove_super_admins_until_2fa_enabled
*/
public function test_super_admin_maintained_when_2fa_enabled() : void {
global $supes, $super_admins;
$supes[] = self::$privileged_user->user_login;
$super_admins[] = self::$privileged_user->user_login;

$this->assertTrue( is_super_admin( self::$privileged_user->ID ) );
$this->assertTrue( user_requires_2fa( self::$privileged_user ) );
self::enable_2fa_for_user( self::$privileged_user->ID );
wp_set_current_user( self::$privileged_user->ID, self::$privileged_user->user_login ); // Triggers remove_super_admins_until_2fa_enabled().
$this->assertTrue( is_super_admin( self::$privileged_user->ID ) );
}

/**
* @covers WordPressdotorg\Two_Factor\remove_capabilities_until_2fa_enabled
*/
public function test_caps_removed_when_2fa_not_enabled() : void {
global $supes, $super_admins;
$supes[] = self::$privileged_user->user_login;
$super_admins[] = self::$privileged_user->user_login;

$this->assertTrue( is_super_admin( self::$privileged_user->ID ) );
$this->assertTrue( user_requires_2fa( self::$privileged_user ) );
$this->assertFalse( Two_Factor_Core::is_user_using_two_factor( self::$privileged_user->ID ) );

wp_set_current_user( self::$privileged_user->ID ); // Triggers `remove_super_admins_until_2fa_enabled()`.
$this->assertFalse( is_super_admin( self::$privileged_user->ID ) );
$this->assertFalse( user_can( self::$privileged_user, 'manage_network' ) ); // Triggers `remove_capabilities_until_2fa_enabled()`.
}

/**
* @covers WordPressdotorg\Two_Factor\remove_capabilities_until_2fa_enabled
*/
public function test_caps_maintained_when_2fa_enabled() : void {
global $supes, $super_admins;
$supes[] = self::$privileged_user->user_login;
$super_admins[] = self::$privileged_user->user_login;

self::enable_2fa_for_user( self::$privileged_user->ID );
$this->assertTrue( is_super_admin( self::$privileged_user->ID ) );
$this->assertTrue( user_requires_2fa( self::$privileged_user ) );

wp_set_current_user( self::$privileged_user->ID ); // Triggers `remove_super_admins_until_2fa_enabled()`.
$this->assertTrue( user_can( self::$privileged_user, 'manage_network' ) ); // Triggers `remove_capabilities_until_2fa_enabled()`.
}

/**
* @covers WordPressdotorg\Two_Factor\user_requires_2fa
*/
public function test_user_requires_2fa() : void {
$cases = $this->data_user_requires_2fa();

foreach ( $cases as $case ) {
$GLOBALS[ $case['global_name'] ] = $case['global_value'];

$this->assertTrue( user_requires_2fa( self::$privileged_user ) );
$this->assertFalse( user_requires_2fa( self::$regular_user ) );

$GLOBALS[ $case['global_name'] ] = array();
}
}

/**
* This isn't a formal `@dataProvider` because those are executed before `wpSetUpBeforeClass()`,
* but this needs to access variables created during that method.
*
* @link https://stackoverflow.com/a/42161440/450127
*/
public function data_user_requires_2fa() : array {
return array(
'supes' => array(
'global_name' => 'supes',
'global_value' => array( self::$privileged_user->user_login ),
),

'wordcamp trusted deputies' => array(
'global_name' => 'trusted_deputies',
'global_value' => array( self::$privileged_user->ID ),
),

'wordcamp subroles' => array(
'global_name' => 'wcorg_subroles',
'global_value' => array(
self::$privileged_user->ID => array( 'wordcamp_wrangler' ),
),
),
);
}

/**
* @covers WordPressdotorg\Two_Factor\redirect_to_2fa_settings
*/
public function test_redirected_when_2fa_needed() {
global $supes, $super_admins;
$supes[] = self::$privileged_user->user_login;
$super_admins[] = self::$privileged_user->user_login;

wp_set_current_user( self::$privileged_user->ID, self::$privileged_user->user_login );
$expected = admin_url( 'profile.php' );
$actual = apply_filters( 'login_redirect', admin_url(), admin_url(), self::$privileged_user );

$this->assertTrue( user_requires_2fa( self::$privileged_user ) );
$this->assertFalse( Two_Factor_Core::is_user_using_two_factor( self::$privileged_user->ID ) );
$this->assertSame( $expected, $actual );
}

/**
* @covers WordPressdotorg\Two_Factor\redirect_to_2fa_settings
*/
public function test_not_redirected_when_2fa_not_needed() {
global $supes, $super_admins;

$expected = admin_url();

$actual = apply_filters( 'login_redirect', $expected, $expected, new WP_Error() );
$this->assertSame( $expected, $actual );

$actual = apply_filters( 'login_redirect', $expected, $expected, self::$regular_user );
$this->assertSame( $expected, $actual );

// User requires 2fa and has it enabled.
$supes[] = self::$privileged_user->user_login;
$super_admins[] = self::$privileged_user->user_login;
$this->enable_2fa_for_user( self::$privileged_user->ID );
wp_set_current_user( self::$privileged_user->ID, self::$privileged_user->user_login );
$actual = apply_filters( 'login_redirect', $expected, $expected, self::$privileged_user);
$this->assertSame( $expected, $actual );
}
}
124 changes: 123 additions & 1 deletion wporg-two-factor.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,9 @@
*/

namespace WordPressdotorg\Two_Factor;
use Two_Factor_Core;
use WP_User;

defined( 'WPINC' ) || die();

// Disabled until ready for launch.
Expand All @@ -18,7 +21,10 @@
}


add_filter( 'two_factor_providers', __NAMESPACE__ . '\two_factor_providers', 99 ); // Must run after all other plugins.
add_filter( 'two_factor_providers', __NAMESPACE__ . '\two_factor_providers', 99 ); // Must run _after_ all other plugins.
add_action( 'set_current_user', __NAMESPACE__ . '\remove_super_admins_until_2fa_enabled', 1 ); // Must run _before_ all other plugins.
add_action( 'login_redirect', __NAMESPACE__ . '\redirect_to_2fa_settings', 105, 3 ); // After `wporg_remember_where_user_came_from_redirect()`, before `WP_WPorg_SSO::redirect_to_policy_update()`.
add_action( 'user_has_cap', __NAMESPACE__ . '\remove_capabilities_until_2fa_enabled', 99, 4 ); // Must run _after_ all other plugins.

/**
* Determine which providers should be available to users.
Expand All @@ -33,3 +39,119 @@ function two_factor_providers( array $providers ) : array {

return array_intersect_key( $providers, $desired_providers );
}


/**
* Remove a user's Super Admins status if they don't have 2FA enabled.
*
* This is needed in addition to `remove_capabilities_until_2fa_enabled()` for two reasons:
* 1: To protect against code that calls `is_super_admin()` directly, instead of checking capabilities.
* 2: To avoid the code in `has_cap()` that allows Super Admins to do anything unless `do_not_allow` is set.
* That would interfere with reducing their capabilities to a Subscriber in `remove_capabilities_until_2fa_enabled()`.
*/
function remove_super_admins_until_2fa_enabled() : void {
global $super_admins;

$user = wp_get_current_user();
$position = array_search( $user->user_login, $super_admins, true );

if ( false === $position ) {
return;
}

if ( user_requires_2fa( $user ) && ! Two_Factor_Core::is_user_using_two_factor( $user->ID ) ) {
unset( $super_admins[ $position ] );
}
}

/**
* Remove capabilities when a user with elevated privileges hasn't enabled 2FA.
*
* That is necessary even though we'll redirect all requests to their profile, because otherwise they could still
* perform privileged actions on the front end, via the REST API, etc.
*/
function remove_capabilities_until_2fa_enabled( array $allcaps, array $caps, array $args, WP_User $user ) : array {
if ( 0 === $user->ID || ! user_requires_2fa( $user ) ) {
return $allcaps;
}

if ( ! Two_Factor_Core::is_user_using_two_factor( $user->ID ) ) {
// This also relies on `remove_super_admins_until_2fa_enabled()`, see notes in that function.
$allcaps = array(
'subscriber' => true,
'read' => true,
);
Comment on lines +80 to +83
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Another option here I think is to return the do_not_allow cap when the requested cap is anything other than the whitelisted caps.

I believe that would override the super-admin always-truthful caps too, but remove_super_admins_until_2fa_enabled() would be best kept still then too.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, that's a good point 👍🏻

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Er, nevermind, it looks like user_has_cap doesn't even get called in that case.

https://github.com/WordPress/wordpress-develop/blob/a71d7373ad6ce0a8f252717832f42aebc3e08854/src/wp-includes/class-wp-user.php#L780-L809


add_action( 'admin_notices', __NAMESPACE__ . '\render_enable_2fa_notice' );
}

return $allcaps;
}

/**
* Check if the user has enough elevated privileges to require 2FA.
*/
function user_requires_2fa( WP_User $user ) : bool {
global $supes, $trusted_deputies, $wcorg_subroles;

$required = false;

// Only checking `$supes` because a user should never be in `$super_admins` without first being in `$supes`.
// A user still requires 2FA even if `remove_super_admins_until_2fa_enabled()` has removed them from `$GLOBALS['super_admins']`.
if ( $supes && in_array( $user->user_login, $supes, true ) ) {
$required = true;
} elseif ( $trusted_deputies && in_array( $user->ID, $trusted_deputies, true ) ) {
$required = true;
} elseif ( $wcorg_subroles && array_key_exists( $user->ID, $wcorg_subroles ) ) {
$required = true;
}

return $required;
}

/**
* Redirect a user to their 2FA settings if they need to enable it.
*
* This isn't usually necessary, since WordPress will prevent Subscribers from visiting other Core screens, but
* sometimes plugins add screens that are available to Subscribers (either intentionally or not).
*/
function redirect_to_2fa_settings( string $redirect_to, string $requested_redirect_to, $user ) : string {
if ( is_wp_error( $user ) ) {
return $redirect_to;
}

if ( ! user_requires_2fa( $user ) || Two_Factor_Core::is_user_using_two_factor( $user->ID ) ) {
return $redirect_to;
}

$primary_blog_id = (int) get_user_meta( $user->ID, 'primary_blog', true );
$primary_site = get_site( $primary_blog_id );

// todo Change this to match the front-end URL once that's implemented.
return 'https://' . $primary_site->domain . trailingslashit( $primary_site->path ) . 'wp-admin/profile.php';
}

/**
* Inform the user that they need to enable 2FA.
*
* @codeCoverageIgnore
*/
function render_enable_2fa_notice() : void {
// @todo change this to use front-end URL/styles when 2FA settings are moved there.

?>

<div class="notice notice-error">
<p>
<?php echo wp_kses_data( sprintf(
__(
'Your account requires two-factor authentication, which adds an extra layer of protection against hackers. You cannot make any changes to the site until you <a href="%s">enable it</a>.',
'wporg'
),
esc_url( admin_url( 'profile.php' ) ) . '#two-factor-options'
) ); ?>
</p>
</div>

<?php
}