From 6aa13a08ce3b0fb91d424e86319e3cc26cf93771 Mon Sep 17 00:00:00 2001 From: Dion Hulse Date: Thu, 2 Feb 2023 16:06:24 +1000 Subject: [PATCH 01/15] Add a rate limit between two-factor login attempts. --- class-two-factor-core.php | 60 +++++++++++++++++++++++++++++++++++++++ 1 file changed, 60 insertions(+) diff --git a/class-two-factor-core.php b/class-two-factor-core.php index b70d619d..2d98cf98 100644 --- a/class-two-factor-core.php +++ b/class-two-factor-core.php @@ -35,6 +35,13 @@ class Two_Factor_Core { */ const USER_META_NONCE_KEY = '_two_factor_nonce'; + /** + * The user meta key to store the last failed timestamp. + * + * @type string + */ + const USER_RATE_LIMIT_KEY = '_two_factor_failure'; + /** * URL query paramater used for our custom actions. * @@ -857,6 +864,41 @@ public static function verify_login_nonce( $user_id, $nonce ) { return false; } + /** + * Enforce a time delay between user two factor login attempts. + * + * @since 0.1-dev + * + * @param WP_User $user The User. + * @return bool True if rate limit is okay, false if not. + */ + public static function check_user_rate_limit( $user ) { + /** + * Filter the minimum time duration between two factor attempts. + * + * @param int $rate_limit The number of seconds between two factor attempts. + * @param WP_User $user The user attempting to login. + */ + $rate_limit = apply_filters( 'two_factor_rate_limit', 5, $user ); + $last_failed = get_user_meta( $user->ID, self::USER_RATE_LIMIT_KEY, true ); + + $rate_limited = false; + if ( $last_failed && $last_failed + $rate_limit > time() ) { + $rate_limited = true; + } + + /** + * Filter whether this login attempt is rate limited or not. + * + * This allows for dedicated plugins to rate limit two factor login attempts + * based on their own rules. + * + * @param bool $rate_limited Whether the user login is rate limited. + * @param WP_User $user The user attempting to login. + */ + return ! apply_filters( 'two_factor_is_user_rate_limited', $rate_limited, $user ); + } + /** * Login form validation. * @@ -903,10 +945,27 @@ public static function login_form_validate_2fa() { exit; } + // Rate limit two factor authentication attempts. + if ( true !== self::check_user_rate_limit( $user ) ) { + $error = new WP_Error( 'two_factor_too_fast', __( 'ERROR: Whoa there, wait a few seconds before trying again, ok?', 'two-factor' ) ); + + do_action( 'wp_login_failed', $user->user_login, $error ); + + $login_nonce = self::create_login_nonce( $user->ID ); + if ( ! $login_nonce ) { + wp_die( esc_html__( 'Failed to create a login nonce.', 'two-factor' ) ); + } + + self::login_html( $user, $login_nonce['key'], $_REQUEST['redirect_to'], esc_html( $error->get_error_message() ), $provider ); + exit; + } + // Ask the provider to verify the second factor. if ( true !== $provider->validate_authentication( $user ) ) { do_action( 'wp_login_failed', $user->user_login, new WP_Error( 'two_factor_invalid', __( 'ERROR: Invalid verification code.', 'two-factor' ) ) ); + update_user_meta( $user->ID, self::USER_RATE_LIMIT_KEY, time() ); + $login_nonce = self::create_login_nonce( $user->ID ); if ( ! $login_nonce ) { wp_die( esc_html__( 'Failed to create a login nonce.', 'two-factor' ) ); @@ -917,6 +976,7 @@ public static function login_form_validate_2fa() { } self::delete_login_nonce( $user->ID ); + delete_user_meta( $user->ID, self::USER_RATE_LIMIT_KEY ); $rememberme = false; if ( isset( $_REQUEST['rememberme'] ) && $_REQUEST['rememberme'] ) { From 64e6af12eb11167990bc408716a87bd54809381f Mon Sep 17 00:00:00 2001 From: Dion Hulse Date: Thu, 2 Feb 2023 16:06:45 +1000 Subject: [PATCH 02/15] Add a warning upon login that the user previously failed to complete the two-factor prompt. --- class-two-factor-core.php | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/class-two-factor-core.php b/class-two-factor-core.php index 2d98cf98..a6f71a0a 100644 --- a/class-two-factor-core.php +++ b/class-two-factor-core.php @@ -635,6 +635,16 @@ public static function login_html( $user, $login_nonce, $redirect_to, $error_msg if ( ! empty( $error_msg ) ) { echo '
' . esc_html( $error_msg ) . '
'; + } else { + $last_failed_two_factor_login = get_user_meta( $user->ID, self::USER_RATE_LIMIT_KEY, true ); + if ( $last_failed_two_factor_login && $last_failed_two_factor_login < time() - 5 * MINUTE_IN_SECONDS ) { + echo '
'; + printf( + esc_html__( 'WARNING: Your account attempted to login at %s and failed to provide a valid two factor response. If this was you, you can ignore this message, otherwise you should reset your password after logging in.' ), + date_i18n( 'r', $last_failed_two_factor_login ) // TODO: better date format + ); + echo '
'; + } } ?> From 7f0a14c744d86bff31e29e67d238988b62f41118 Mon Sep 17 00:00:00 2001 From: Dion Hulse Date: Thu, 2 Feb 2023 16:52:34 +1000 Subject: [PATCH 03/15] Implement an exponential backoff between failed two-factor login attempts, capping at 1 hour. --- class-two-factor-core.php | 78 ++++++++++++++++++++++++++++++++++----- 1 file changed, 68 insertions(+), 10 deletions(-) diff --git a/class-two-factor-core.php b/class-two-factor-core.php index a6f71a0a..8a7fa259 100644 --- a/class-two-factor-core.php +++ b/class-two-factor-core.php @@ -42,6 +42,13 @@ class Two_Factor_Core { */ const USER_RATE_LIMIT_KEY = '_two_factor_failure'; + /** + * The user meta key to store the number of failed login attempts. + * + * @var string + */ + const USER_FAILED_LOGIN_ATTEMPTS = '_two_factor_failed_login_attempts'; + /** * URL query paramater used for our custom actions. * @@ -636,11 +643,20 @@ public static function login_html( $user, $login_nonce, $redirect_to, $error_msg if ( ! empty( $error_msg ) ) { echo '
' . esc_html( $error_msg ) . '
'; } else { - $last_failed_two_factor_login = get_user_meta( $user->ID, self::USER_RATE_LIMIT_KEY, true ); - if ( $last_failed_two_factor_login && $last_failed_two_factor_login < time() - 5 * MINUTE_IN_SECONDS ) { + $last_failed_two_factor_login = (int) get_user_meta( $user->ID, self::USER_RATE_LIMIT_KEY, true ); + $failed_login_count = (int) get_user_meta( $user->ID, self::USER_FAILED_LOGIN_ATTEMPTS, true ); + + if ( $last_failed_two_factor_login ) { echo '
'; printf( - esc_html__( 'WARNING: Your account attempted to login at %s and failed to provide a valid two factor response. If this was you, you can ignore this message, otherwise you should reset your password after logging in.' ), + _n( + + 'WARNING: Your account has attempted to login without providing a valid two factor token. The last failed login occured at %2$s. If this wasn\'t you, you should reset your password.', + 'WARNING: Your account has attempted to login %1$s times without providing a valid two factor token. The last failed login occured at %2$s. If this wasn\'t you, you should reset your password.', + $failed_login_count, + 'two-factor' + ), + number_format_i18n( $failed_login_count ), date_i18n( 'r', $last_failed_two_factor_login ) // TODO: better date format ); echo '
'; @@ -875,21 +891,49 @@ public static function verify_login_nonce( $user_id, $nonce ) { } /** - * Enforce a time delay between user two factor login attempts. + * Determine the minimum wait between two factor attempts for a user. * - * @since 0.1-dev + * This implements an increasing backoff, requiring an attacker to wait longer + * each time to attempt to brute-force the login. * - * @param WP_User $user The User. - * @return bool True if rate limit is okay, false if not. + * @param WP_User $user The user being operated upon. + * @return int Time delay in seconds between login attempts. */ - public static function check_user_rate_limit( $user ) { + public static function get_user_time_delay( $user ) { /** * Filter the minimum time duration between two factor attempts. * * @param int $rate_limit The number of seconds between two factor attempts. + */ + $rate_limit = apply_filters( 'two_factor_rate_limit', 5 ); + + $user_failed_logins = get_user_meta( $user->ID, self::USER_FAILED_LOGIN_ATTEMPTS, true ); + if ( $user_failed_logins ) { + $rate_limit = pow( 2, $user_failed_logins ) * $rate_limit; + + // Limit to 1 hour maximum time delay. + $rate_limit = min( HOUR_IN_SECONDS, $rate_limit ); + } + + /** + * Filters the per-user time duration between two factor login attempts. + * + * @param int $rate_limit The number of seconds between two factor attempts. * @param WP_User $user The user attempting to login. */ - $rate_limit = apply_filters( 'two_factor_rate_limit', 5, $user ); + return apply_filters( 'two_factor_user_rate_limit', $rate_limit, $user ); + } + + /** + * Enforce a time delay between user two factor login attempts. + * + * @since 0.1-dev + * + * @param WP_User $user The User. + * @return bool True if rate limit is okay, false if not. + */ + public static function check_user_rate_limit( $user ) { + $rate_limit = self::get_user_time_delay( $user ); $last_failed = get_user_meta( $user->ID, self::USER_RATE_LIMIT_KEY, true ); $rate_limited = false; @@ -957,7 +1001,16 @@ public static function login_form_validate_2fa() { // Rate limit two factor authentication attempts. if ( true !== self::check_user_rate_limit( $user ) ) { - $error = new WP_Error( 'two_factor_too_fast', __( 'ERROR: Whoa there, wait a few seconds before trying again, ok?', 'two-factor' ) ); + $time_delay = self::get_user_time_delay( $user ); + $last_login = get_user_meta( $user->ID, self::USER_RATE_LIMIT_KEY, true ); + + $error = new WP_Error( + 'two_factor_too_fast', + sprintf( + __( 'ERROR: Whoa there, slow down! You can try again in %s.', 'two-factor' ), + human_time_diff( $last_login + $time_delay ) + ) + ); do_action( 'wp_login_failed', $user->user_login, $error ); @@ -974,8 +1027,12 @@ public static function login_form_validate_2fa() { if ( true !== $provider->validate_authentication( $user ) ) { do_action( 'wp_login_failed', $user->user_login, new WP_Error( 'two_factor_invalid', __( 'ERROR: Invalid verification code.', 'two-factor' ) ) ); + // Store the last time a failed login occured. update_user_meta( $user->ID, self::USER_RATE_LIMIT_KEY, time() ); + // Store the number of failed login attempts. + update_user_meta( $user->ID, self::USER_FAILED_LOGIN_ATTEMPTS, 1 + (int) get_user_meta( $user->ID, self::USER_FAILED_LOGIN_ATTEMPTS, true ) ); + $login_nonce = self::create_login_nonce( $user->ID ); if ( ! $login_nonce ) { wp_die( esc_html__( 'Failed to create a login nonce.', 'two-factor' ) ); @@ -987,6 +1044,7 @@ public static function login_form_validate_2fa() { self::delete_login_nonce( $user->ID ); delete_user_meta( $user->ID, self::USER_RATE_LIMIT_KEY ); + delete_user_meta( $user->ID, self::USER_FAILED_LOGIN_ATTEMPTS_KEY ); $rememberme = false; if ( isset( $_REQUEST['rememberme'] ) && $_REQUEST['rememberme'] ) { From d5f00f7a6be16b2bb1ae419ee14b16a20aebd367 Mon Sep 17 00:00:00 2001 From: Dion Hulse Date: Thu, 2 Feb 2023 16:52:51 +1000 Subject: [PATCH 04/15] Move the failed-login notice to a new method. --- class-two-factor-core.php | 44 +++++++++++++++++++++++---------------- 1 file changed, 26 insertions(+), 18 deletions(-) diff --git a/class-two-factor-core.php b/class-two-factor-core.php index 8a7fa259..810ba08e 100644 --- a/class-two-factor-core.php +++ b/class-two-factor-core.php @@ -607,6 +607,31 @@ public static function backup_2fa() { exit; } + /** + * Displays a message informing the user that their account has had failed login attempts. + * + * @param WP_User $user WP_User object of the logged-in user. + */ + public static function maybe_show_last_login_failure_notice( $user ) { + $last_failed_two_factor_login = (int) get_user_meta( $user->ID, self::USER_RATE_LIMIT_KEY, true ); + $failed_login_count = (int) get_user_meta( $user->ID, self::USER_FAILED_LOGIN_ATTEMPTS, true ); + + if ( $last_failed_two_factor_login ) { + echo '
'; + printf( + _n( + 'WARNING: Your account has attempted to login without providing a valid two factor token. The last failed login occured at %2$s. If this wasn\'t you, you should reset your password.', + 'WARNING: Your account has attempted to login %1$s times without providing a valid two factor token. The last failed login occured at %2$s. If this wasn\'t you, you should reset your password.', + $failed_login_count, + 'two-factor' + ), + number_format_i18n( $failed_login_count ), + date_i18n( 'r', $last_failed_two_factor_login ) // TODO: better date format + ); + echo '
'; + } + } + /** * Generates the html form for the second step of the authentication process. * @@ -643,24 +668,7 @@ public static function login_html( $user, $login_nonce, $redirect_to, $error_msg if ( ! empty( $error_msg ) ) { echo '
' . esc_html( $error_msg ) . '
'; } else { - $last_failed_two_factor_login = (int) get_user_meta( $user->ID, self::USER_RATE_LIMIT_KEY, true ); - $failed_login_count = (int) get_user_meta( $user->ID, self::USER_FAILED_LOGIN_ATTEMPTS, true ); - - if ( $last_failed_two_factor_login ) { - echo '
'; - printf( - _n( - - 'WARNING: Your account has attempted to login without providing a valid two factor token. The last failed login occured at %2$s. If this wasn\'t you, you should reset your password.', - 'WARNING: Your account has attempted to login %1$s times without providing a valid two factor token. The last failed login occured at %2$s. If this wasn\'t you, you should reset your password.', - $failed_login_count, - 'two-factor' - ), - number_format_i18n( $failed_login_count ), - date_i18n( 'r', $last_failed_two_factor_login ) // TODO: better date format - ); - echo '
'; - } + self::maybe_show_last_login_failure_notice( $user ); } ?> From 9196d2b3d45543ee82f1d0fdd09819e0ecbe6a6d Mon Sep 17 00:00:00 2001 From: Dion Hulse Date: Thu, 2 Feb 2023 16:56:01 +1000 Subject: [PATCH 05/15] Use a time diff rather than absolute time. --- class-two-factor-core.php | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/class-two-factor-core.php b/class-two-factor-core.php index 810ba08e..35755a8f 100644 --- a/class-two-factor-core.php +++ b/class-two-factor-core.php @@ -620,13 +620,13 @@ public static function maybe_show_last_login_failure_notice( $user ) { echo '
'; printf( _n( - 'WARNING: Your account has attempted to login without providing a valid two factor token. The last failed login occured at %2$s. If this wasn\'t you, you should reset your password.', - 'WARNING: Your account has attempted to login %1$s times without providing a valid two factor token. The last failed login occured at %2$s. If this wasn\'t you, you should reset your password.', + 'WARNING: Your account has attempted to login without providing a valid two factor token. The last failed login occured %2$s ago. If this wasn\'t you, you should reset your password.', + 'WARNING: Your account has attempted to login %1$s times without providing a valid two factor token. The last failed login occured %2$s ago. If this wasn\'t you, you should reset your password.', $failed_login_count, 'two-factor' ), number_format_i18n( $failed_login_count ), - date_i18n( 'r', $last_failed_two_factor_login ) // TODO: better date format + human_time_diff( $last_failed_two_factor_login, time() ) ); echo '
'; } From 5cf2bd389f6216b90fcaa5432a17e7647b114eb2 Mon Sep 17 00:00:00 2001 From: Dion Hulse Date: Thu, 2 Feb 2023 17:15:16 +1000 Subject: [PATCH 06/15] Rename metakey and constant names --- class-two-factor-core.php | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/class-two-factor-core.php b/class-two-factor-core.php index 35755a8f..3190e155 100644 --- a/class-two-factor-core.php +++ b/class-two-factor-core.php @@ -40,14 +40,14 @@ class Two_Factor_Core { * * @type string */ - const USER_RATE_LIMIT_KEY = '_two_factor_failure'; + const USER_RATE_LIMIT_KEY = '_two_factor_last_login_failure'; /** * The user meta key to store the number of failed login attempts. * * @var string */ - const USER_FAILED_LOGIN_ATTEMPTS = '_two_factor_failed_login_attempts'; + const USER_FAILED_LOGIN_ATTEMPTS_KEY = '_two_factor_failed_login_attempts'; /** * URL query paramater used for our custom actions. @@ -614,7 +614,7 @@ public static function backup_2fa() { */ public static function maybe_show_last_login_failure_notice( $user ) { $last_failed_two_factor_login = (int) get_user_meta( $user->ID, self::USER_RATE_LIMIT_KEY, true ); - $failed_login_count = (int) get_user_meta( $user->ID, self::USER_FAILED_LOGIN_ATTEMPTS, true ); + $failed_login_count = (int) get_user_meta( $user->ID, self::USER_FAILED_LOGIN_ATTEMPTS_KEY, true ); if ( $last_failed_two_factor_login ) { echo '
'; @@ -915,7 +915,7 @@ public static function get_user_time_delay( $user ) { */ $rate_limit = apply_filters( 'two_factor_rate_limit', 5 ); - $user_failed_logins = get_user_meta( $user->ID, self::USER_FAILED_LOGIN_ATTEMPTS, true ); + $user_failed_logins = get_user_meta( $user->ID, self::USER_FAILED_LOGIN_ATTEMPTS_KEY, true ); if ( $user_failed_logins ) { $rate_limit = pow( 2, $user_failed_logins ) * $rate_limit; @@ -1039,7 +1039,7 @@ public static function login_form_validate_2fa() { update_user_meta( $user->ID, self::USER_RATE_LIMIT_KEY, time() ); // Store the number of failed login attempts. - update_user_meta( $user->ID, self::USER_FAILED_LOGIN_ATTEMPTS, 1 + (int) get_user_meta( $user->ID, self::USER_FAILED_LOGIN_ATTEMPTS, true ) ); + update_user_meta( $user->ID, self::USER_FAILED_LOGIN_ATTEMPTS_KEY, 1 + (int) get_user_meta( $user->ID, self::USER_FAILED_LOGIN_ATTEMPTS_KEY, true ) ); $login_nonce = self::create_login_nonce( $user->ID ); if ( ! $login_nonce ) { From 8baec519d747ff667642954e377e510a24d05795 Mon Sep 17 00:00:00 2001 From: Ian Dunn Date: Mon, 6 Feb 2023 13:13:40 -0800 Subject: [PATCH 07/15] Convert Backup Codes AJAX to REST API --- class-two-factor-core.php | 42 ++++ providers/class-two-factor-backup-codes.php | 115 +++++++---- .../class-two-factor-backup-codes-ajax.php | 110 ---------- ...class-two-factor-backup-codes-rest-api.php | 193 ++++++++++++++++++ .../class-two-factor-backup-codes.php | 6 +- 5 files changed, 311 insertions(+), 155 deletions(-) delete mode 100644 tests/providers/class-two-factor-backup-codes-ajax.php create mode 100644 tests/providers/class-two-factor-backup-codes-rest-api.php diff --git a/class-two-factor-core.php b/class-two-factor-core.php index b70d619d..ee15420a 100644 --- a/class-two-factor-core.php +++ b/class-two-factor-core.php @@ -49,6 +49,13 @@ class Two_Factor_Core { */ const USER_SETTINGS_ACTION_NONCE_QUERY_ARG = '_two_factor_action_nonce'; + /** + * Namespace for plugin rest api endpoints. + * + * @var string + */ + const REST_NAMESPACE = 'two-factor/1.0'; + /** * Keep track of all the password-based authentication sessions that * need to invalidated before the second factor authentication. @@ -1076,6 +1083,41 @@ public static function user_two_factor_options( $user ) { do_action( 'show_user_security_settings', $user ); } + /** + * Enable a provider for a user. + * + * @param int $user_id The ID of the user. + * @param string $new_provider The name of the provider class. + * + * @return bool True if the provider was enabled, false otherwise. + */ + public static function enable_provider_for_user( $user_id, $new_provider ) { + $available_providers = self::get_providers(); + + if ( ! array_key_exists( $new_provider, $available_providers ) ) { + return false; + } + + $user = get_userdata( $user_id ); + $enabled_providers = self::get_enabled_providers_for_user( $user ); + + if ( in_array( $new_provider, $enabled_providers ) ) { + return true; + } + + $enabled_providers[] = $new_provider; + $enabled = update_user_meta( $user_id, self::ENABLED_PROVIDERS_USER_META_KEY, $enabled_providers ); + + // Primary provider must be enabled. + $has_primary = is_object( self::get_primary_provider_for_user( $user_id ) ); + + if ( ! $has_primary ) { + $has_primary = update_user_meta( $user_id, self::PROVIDER_USER_META_KEY, $new_provider ); + } + + return $enabled && $has_primary; + } + /** * Update the user meta value. * diff --git a/providers/class-two-factor-backup-codes.php b/providers/class-two-factor-backup-codes.php index 62707f2f..7a8da399 100644 --- a/providers/class-two-factor-backup-codes.php +++ b/providers/class-two-factor-backup-codes.php @@ -48,13 +48,49 @@ public static function get_instance() { * @since 0.1-dev */ protected function __construct() { + add_action( 'rest_api_init', array( $this, 'register_rest_routes' ) ); add_action( 'two_factor_user_options_' . __CLASS__, array( $this, 'user_options' ) ); add_action( 'admin_notices', array( $this, 'admin_notices' ) ); - add_action( 'wp_ajax_two_factor_backup_codes_generate', array( $this, 'ajax_generate_json' ) ); return parent::__construct(); } + /** + * Register the rest-api endpoints required for this provider. + */ + public function register_rest_routes() { + register_rest_route( + Two_Factor_Core::REST_NAMESPACE, + '/generate-backup-codes', + array( + 'methods' => WP_REST_Server::CREATABLE, + 'callback' => array( $this, 'rest_generate_codes' ), + 'permission_callback' => function( $request ) { + return current_user_can( 'edit_user', $request['user_id'] ); + }, + 'args' => array( + 'user_id' => array( + 'required' => true, + 'type' => 'number', + ), + 'number' => array( + 'type' => 'number', + 'default' => self::NUMBER_OF_CODES, + ), + 'append' => array( + 'type' => 'boolean', + 'default' => false, + ), + 'enable_provider' => array( + 'required' => false, + 'type' => 'boolean', + 'default' => false, + ), + ), + ) + ); + } + /** * Displays an admin notice when backup codes have run out. * @@ -125,8 +161,7 @@ public function is_available_for_user( $user ) { * @param WP_User $user WP_User object of the logged-in user. */ public function user_options( $user ) { - $ajax_nonce = wp_create_nonce( 'two-factor-backup-codes-generate-json-' . $user->ID ); - $count = self::codes_remaining_for_user( $user ); + $count = self::codes_remaining_for_user( $user ); ?>

is_valid_key( $key ) ) { - if ( $this->is_valid_authcode( $key, $authcode ) ) { - if ( ! $this->set_user_totp_key( $user_id, $key ) ) { - $errors[] = __( 'Unable to save Two Factor Authentication code. Please re-scan the QR code and enter the code provided by your application.', 'two-factor' ); - } - } else { - $errors[] = __( 'Invalid Two Factor Authentication code.', 'two-factor' ); - } - } else { - $errors[] = __( 'Invalid Two Factor Authentication secret key.', 'two-factor' ); - } - } - - if ( ! empty( $errors ) ) { - $notices['error'] = $errors; - } - - if ( ! empty( $notices ) ) { - update_user_meta( $user_id, self::NOTICES_META_KEY, $notices ); - } - } - } - /** * Get the TOTP secret key for a user. * @@ -328,39 +424,6 @@ public function is_valid_key( $key ) { return false; } - /** - * Display any available admin notices. - * - * @param integer $user_id User ID. - * - * @return void - * - * @codeCoverageIgnore - */ - public function admin_notices( $user_id ) { - $notices = get_user_meta( $user_id, self::NOTICES_META_KEY, true ); - - if ( ! empty( $notices ) ) { - delete_user_meta( $user_id, self::NOTICES_META_KEY ); - - foreach ( $notices as $class => $messages ) { - ?> -
- -

- -

- -
- user->create( + array( + 'role' => 'administrator', + ) + ); + + self::$editor_id = $factory->user->create( + array( + 'role' => 'editor', + ) + ); + + self::$provider = Two_Factor_Totp::get_instance(); + } + + public static function wpTearDownAfterClass() { + self::delete_user( self::$admin_id ); + self::delete_user( self::$editor_id ); + } + + /** + * Verify setting up TOTP with a bad key code. + * + * @covers Two_Factor_Totp::rest_setup_totp + * @covers Two_Factor_Totp::is_available_for_user + */ + public function test_user_two_factor_rest_key_bad_auth_code() { + wp_set_current_user( self::$admin_id ); + + $request = new WP_REST_Request( 'POST', '/' . Two_Factor_Core::REST_NAMESPACE . '/totp' ); + $request->set_body_params( + array( + 'user_id' => self::$admin_id, + 'key' => 'abcdef' + ) + ); + + $response = rest_do_request( $request ); + + $this->assertErrorResponse( 'invalid_key', $response, 400 ); + + $this->assertFalse( self::$provider->is_available_for_user( wp_get_current_user() ) ); + } + + /** + * Verify setting up TOTP without an authcode. + * + * @covers Two_Factor_Totp::rest_setup_totp + * @covers Two_Factor_Totp::is_available_for_user + */ + public function test_user_two_factor_rest_set_key_no_authcode() { + wp_set_current_user( self::$admin_id ); + + $key = self::$provider->generate_key(); + + $request = new WP_REST_Request( 'POST', '/' . Two_Factor_Core::REST_NAMESPACE . '/totp' ); + $request->set_body_params( + array( + 'user_id' => self::$admin_id, + 'key' => $key, + ) + ); + + $response = rest_do_request( $request ); + + $this->assertErrorResponse( 'invalid_key_code', $response, 400 ); + + $this->assertFalse( self::$provider->is_available_for_user( wp_get_current_user() ) ); + } + + + /** + * Verify setting up TOTP with a bad authcode. + * + * @covers Two_Factor_Totp::rest_setup_totp + * @covers Two_Factor_Totp::is_available_for_user + */ + public function test_user_two_factor_rest_set_key_bad_auth_code() { + wp_set_current_user( self::$admin_id ); + + $key = self::$provider->generate_key(); + + $request = new WP_REST_Request( 'POST', '/' . Two_Factor_Core::REST_NAMESPACE . '/totp' ); + $request->set_body_params( + array( + 'user_id' => self::$admin_id, + 'key' => $key, + 'code' => 'abcdef' + ) + ); + + $response = rest_do_request( $request ); + + $this->assertErrorResponse( 'invalid_key_code', $response, 400 ); + + $this->assertFalse( self::$provider->is_available_for_user( wp_get_current_user() ) ); + } + + /** + * Verify setting up TOTP with an authcode. + * + * @covers Two_Factor_Totp::rest_setup_totp + * @covers Two_Factor_Totp::is_available_for_user + */ + public function test_user_two_factor_rest_update_set_key() { + wp_set_current_user( self::$admin_id ); + + $key = self::$provider->generate_key(); + $code = self::$provider->calc_totp( $key ); + + $request = new WP_REST_Request( 'POST', '/' . Two_Factor_Core::REST_NAMESPACE . '/totp' ); + $request->set_body_params( + array( + 'user_id' => self::$admin_id, + 'key' => $key, + 'code' => $code, + ) + ); + + $response = rest_do_request( $request ); + $data = $response->get_data(); + + $this->assertEquals( 200, $response->get_status() ); + + $this->assertTrue( $data['success'] ); + + $this->assertTrue( self::$provider->is_available_for_user( wp_get_current_user() ) ); + } + + /** + * Verify secret deletion via REST API. + * + * @covers Two_Factor_Totp::rest_delete_totp + */ + public function test_user_can_delete_secret() { + wp_set_current_user( self::$admin_id ); + + $user = wp_get_current_user(); + $key = self::$provider->generate_key(); + + // Configure secret for the user. + self::$provider->set_user_totp_key( $user->ID, $key ); + + $this->assertEquals( + $key, + self::$provider->get_user_totp_key( $user->ID ), + 'Secret was stored and can be fetched' + ); + + $request = new WP_REST_Request( 'DELETE', '/' . Two_Factor_Core::REST_NAMESPACE . '/totp' ); + $request->set_body_params( + array( + 'user_id' => self::$admin_id, + ) + ); + $response = rest_do_request( $request ); + + $this->assertEquals( 200, $response->get_status() ); + + $this->assertEquals( + '', + self::$provider->get_user_totp_key( $user->ID ), + 'Secret has been deleted' + ); + } + + /** + * Verify secret deletion via REST API. + * + * @covers Two_Factor_Totp::rest_delete_totp + */ + public function test_admin_can_delete_secret_for_others() { + wp_set_current_user( self::$admin_id ); + + $key = self::$provider->generate_key(); + + // Configure secret for the user. + self::$provider->set_user_totp_key( self::$editor_id, $key ); + + $this->assertEquals( + $key, + self::$provider->get_user_totp_key( self::$editor_id ), + 'Secret was stored and can be fetched' + ); + + $request = new WP_REST_Request( 'DELETE', '/' . Two_Factor_Core::REST_NAMESPACE . '/totp' ); + $request->set_body_params( + array( + 'user_id' => self::$editor_id, + ) + ); + $response = rest_do_request( $request ); + + $this->assertEquals( 200, $response->get_status() ); + + $this->assertEquals( + '', + self::$provider->get_user_totp_key( self::$editor_id ), + 'Secret has been deleted' + ); + } + + /** + * Verify secret deletion via REST API denied for other users. + * + * @covers Two_Factor_Totp::rest_delete_totp + */ + public function test_user_cannot_delete_secret_for_others() { + wp_set_current_user( self::$editor_id ); + + $user = get_user_by( 'id', self::$admin_id ); + $key = self::$provider->generate_key(); + + // Configure secret for the user. + self::$provider->set_user_totp_key( $user->ID, $key ); + + $this->assertEquals( + $key, + self::$provider->get_user_totp_key( $user->ID ), + 'Secret was stored and can be fetched' + ); + + $request = new WP_REST_Request( 'DELETE', '/' . Two_Factor_Core::REST_NAMESPACE . '/totp' ); + $request->set_body_params( + array( + 'user_id' => self::$admin_id, + ) + ); + $response = rest_do_request( $request ); + + $this->assertErrorResponse( 'rest_forbidden', $response, 403 ); + + $this->assertEquals( + $key, + self::$provider->get_user_totp_key( $user->ID ), + 'Secret has not been deleted' + ); + } + +} diff --git a/tests/providers/class-two-factor-totp.php b/tests/providers/class-two-factor-totp.php index d13611ae..7b11611d 100644 --- a/tests/providers/class-two-factor-totp.php +++ b/tests/providers/class-two-factor-totp.php @@ -85,95 +85,6 @@ public function test_user_two_factor_options_generates_key() { $this->assertStringContainsString( __( 'Authentication Code:', 'two-factor' ), $content ); } - /** - * Verify updating user options without an authcode. - * - * @covers Two_Factor_Totp::user_two_factor_options_update - * @covers Two_Factor_Totp::is_available_for_user - */ - public function test_user_two_factor_options_update_set_key_no_authcode() { - $user = new WP_User( self::factory()->user->create() ); - - $request_key = '_nonce_user_two_factor_totp_options'; - $nonce = wp_create_nonce( 'user_two_factor_totp_options' ); - $_POST[ $request_key ] = $nonce; - $_REQUEST[ $request_key ] = $nonce; - - $_POST['two-factor-totp-key'] = $this->provider->generate_key(); - - ob_start(); - $this->provider->user_two_factor_options_update( $user->ID ); - $content = ob_get_clean(); - - unset( $_POST['two-factor-totp-key'] ); - - unset( $_REQUEST[ $request_key ] ); - unset( $_POST[ $request_key ] ); - - $this->assertFalse( $this->provider->is_available_for_user( $user ) ); - } - - /** - * Verify updating user options with a bad authcode. - * - * @covers Two_Factor_Totp::user_two_factor_options_update - * @covers Two_Factor_Totp::is_available_for_user - */ - public function test_user_two_factor_options_update_set_key_bad_auth_code() { - $user = new WP_User( self::factory()->user->create() ); - - $request_key = '_nonce_user_two_factor_totp_options'; - $nonce = wp_create_nonce( 'user_two_factor_totp_options' ); - $_POST[ $request_key ] = $nonce; - $_REQUEST[ $request_key ] = $nonce; - - $_POST['two-factor-totp-key'] = $this->provider->generate_key(); - $_POST['two-factor-totp-authcode'] = 'bad_test_authcode'; - - ob_start(); - $this->provider->user_two_factor_options_update( $user->ID ); - $content = ob_get_clean(); - - unset( $_POST['two-factor-totp-authcode'] ); - unset( $_POST['two-factor-totp-key'] ); - - unset( $_REQUEST[ $request_key ] ); - unset( $_POST[ $request_key ] ); - - $this->assertFalse( $this->provider->is_available_for_user( $user ) ); - } - - /** - * Verify updating user options with an authcode. - * - * @covers Two_Factor_Totp::user_two_factor_options_update - * @covers Two_Factor_Totp::is_available_for_user - */ - public function test_user_two_factor_options_update_set_key() { - $user = new WP_User( self::factory()->user->create() ); - - $request_key = '_nonce_user_two_factor_totp_options'; - $nonce = wp_create_nonce( 'user_two_factor_totp_options' ); - $_POST[ $request_key ] = $nonce; - $_REQUEST[ $request_key ] = $nonce; - - $key = $this->provider->generate_key(); - $_POST['two-factor-totp-key'] = $key; - $_POST['two-factor-totp-authcode'] = $this->provider->calc_totp( $key ); - - ob_start(); - $this->provider->user_two_factor_options_update( $user->ID ); - $content = ob_get_clean(); - - unset( $_POST['two-factor-totp-authcode'] ); - unset( $_POST['two-factor-totp-key'] ); - - unset( $_REQUEST[ $request_key ] ); - unset( $_POST[ $request_key ] ); - - $this->assertTrue( $this->provider->is_available_for_user( $user ) ); - } - /** * Verify base32 encoding. * From e10cc5c6fcc8208e3a8429c24b6218720892276e Mon Sep 17 00:00:00 2001 From: Dion Hulse Date: Wed, 8 Feb 2023 11:11:49 +1000 Subject: [PATCH 09/15] Apply suggestions from Ians code review Co-authored-by: Ian Dunn --- class-two-factor-core.php | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/class-two-factor-core.php b/class-two-factor-core.php index 3190e155..4e9da034 100644 --- a/class-two-factor-core.php +++ b/class-two-factor-core.php @@ -913,7 +913,7 @@ public static function get_user_time_delay( $user ) { * * @param int $rate_limit The number of seconds between two factor attempts. */ - $rate_limit = apply_filters( 'two_factor_rate_limit', 5 ); + $rate_limit = apply_filters( 'two_factor_rate_limit', 1 ); $user_failed_logins = get_user_meta( $user->ID, self::USER_FAILED_LOGIN_ATTEMPTS_KEY, true ); if ( $user_failed_logins ) { @@ -933,9 +933,9 @@ public static function get_user_time_delay( $user ) { } /** - * Enforce a time delay between user two factor login attempts. + * Determine if a time delay between user two factor login attempts should be triggered. * - * @since 0.1-dev + * @since 0.8.0 * * @param WP_User $user The User. * @return bool True if rate limit is okay, false if not. From 06d9aa195195ccb7f5868a4bdbe5dad9f74f9fa4 Mon Sep 17 00:00:00 2001 From: Dion Hulse Date: Wed, 8 Feb 2023 12:41:18 +1000 Subject: [PATCH 10/15] Rename 'check_user_rate_limit' to 'is_user_rate_limited' --- class-two-factor-core.php | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/class-two-factor-core.php b/class-two-factor-core.php index 7a4d5e46..a11ba206 100644 --- a/class-two-factor-core.php +++ b/class-two-factor-core.php @@ -947,7 +947,7 @@ public static function get_user_time_delay( $user ) { * @param WP_User $user The User. * @return bool True if rate limit is okay, false if not. */ - public static function check_user_rate_limit( $user ) { + public static function is_user_rate_limited( $user ) { $rate_limit = self::get_user_time_delay( $user ); $last_failed = get_user_meta( $user->ID, self::USER_RATE_LIMIT_KEY, true ); @@ -965,7 +965,7 @@ public static function check_user_rate_limit( $user ) { * @param bool $rate_limited Whether the user login is rate limited. * @param WP_User $user The user attempting to login. */ - return ! apply_filters( 'two_factor_is_user_rate_limited', $rate_limited, $user ); + return apply_filters( 'two_factor_is_user_rate_limited', $rate_limited, $user ); } /** @@ -1015,7 +1015,7 @@ public static function login_form_validate_2fa() { } // Rate limit two factor authentication attempts. - if ( true !== self::check_user_rate_limit( $user ) ) { + if ( true === self::is_user_rate_limited( $user ) ) { $time_delay = self::get_user_time_delay( $user ); $last_login = get_user_meta( $user->ID, self::USER_RATE_LIMIT_KEY, true ); From 3b4b4f650192a98dbf6a927f987be75b3cb44d82 Mon Sep 17 00:00:00 2001 From: Dion Hulse Date: Wed, 8 Feb 2023 12:44:37 +1000 Subject: [PATCH 11/15] Lower the maximum time duration a user may be locked out for to 15 minutes, add a filter on that value. --- class-two-factor-core.php | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/class-two-factor-core.php b/class-two-factor-core.php index a11ba206..08b6a292 100644 --- a/class-two-factor-core.php +++ b/class-two-factor-core.php @@ -918,16 +918,22 @@ public static function get_user_time_delay( $user ) { /** * Filter the minimum time duration between two factor attempts. * - * @param int $rate_limit The number of seconds between two factor attempts. + * @param int $rate_limit The number of seconds between two factor attempts. */ - $rate_limit = apply_filters( 'two_factor_rate_limit', 1 ); + $rate_limit = apply_filters( 'two_factor_rate_limit', 1 ); $user_failed_logins = get_user_meta( $user->ID, self::USER_FAILED_LOGIN_ATTEMPTS_KEY, true ); if ( $user_failed_logins ) { $rate_limit = pow( 2, $user_failed_logins ) * $rate_limit; - // Limit to 1 hour maximum time delay. - $rate_limit = min( HOUR_IN_SECONDS, $rate_limit ); + /** + * Filter the maximum time duration a user may be locked out from retrying two factor authentications. + * + * @param int $max_rate_limit The maximum number of seconds a user might be locked out for. Default 15 minutes. + */ + $max_rate_limit = apply_filters( 'two_factor_max_rate_limit', 15 * MINUTE_IN_SECONDS ); + + $rate_limit = min( $max_rate_limit, $rate_limit ); } /** From 0b3a8c0d12e1f129911ac810740c4c3841c5e08b Mon Sep 17 00:00:00 2001 From: Dion Hulse Date: Wed, 8 Feb 2023 13:09:24 +1000 Subject: [PATCH 12/15] Add tests --- tests/class-two-factor-core.php | 50 +++++++++++++++++++++++++++++++++ 1 file changed, 50 insertions(+) diff --git a/tests/class-two-factor-core.php b/tests/class-two-factor-core.php index 5986d766..ca10bb85 100644 --- a/tests/class-two-factor-core.php +++ b/tests/class-two-factor-core.php @@ -472,4 +472,54 @@ public function test_invalid_nonce_deletes_valid_nonce() { ); } + /** + * Test that the lockout time delay for two factor attempts is respected. + * + * @covers Two_Factor_Core::get_user_time_delay() + */ + public function test_get_user_time_delay() { + $user = $this->get_dummy_user(); + + // Default values, sans filters. + $rate_limit = 1; + $max_rate_limit = 15 * MINUTE_IN_SECONDS; + + // User has never logged in, validate the minimum time delay is in play. + $this->assertEquals( $rate_limit, Two_Factor_Core::get_user_time_delay( $user ) ); + + // Simulate 5 failed login attempts, and validate that the lockout is as expected. + update_user_meta( $user->ID, Two_Factor_Core::USER_FAILED_LOGIN_ATTEMPTS_KEY, 5 ); + $this->assertEquals( pow( 2, 5 ) * $rate_limit, Two_Factor_Core::get_user_time_delay( $user ) ); + + // Simulate 100 failed login attempts, validate that the lockout is < $max_rate_limit + update_user_meta( $user->ID, Two_Factor_Core::USER_FAILED_LOGIN_ATTEMPTS_KEY, 100 ); + $this->assertEquals( $max_rate_limit, Two_Factor_Core::get_user_time_delay( $user ) ); + } + + /** + * Test that the user rate limit functions return as expected. + * + * @covers Two_Factor_Core::is_user_rate_limited() + */ + public function test_is_user_rate_limited() { + $user = $this->get_dummy_user(); + + // User has never logged in, validate they're not rate limited. + $this->assertFalse( Two_Factor_Core::is_user_rate_limited( $user ) ); + + // Failed login attempt at time(), user should be rate limited. + update_user_meta( $user->ID, Two_Factor_Core::USER_FAILED_LOGIN_ATTEMPTS_KEY, 1 ); + update_user_meta( $user->ID, Two_Factor_Core::USER_RATE_LIMIT_KEY, time() ); + $this->assertTrue( Two_Factor_Core::is_user_rate_limited( $user ) ); + + // 8 failed logins a minite ago, user should be rate limited. + update_user_meta( $user->ID, Two_Factor_Core::USER_FAILED_LOGIN_ATTEMPTS_KEY, 8 ); + update_user_meta( $user->ID, Two_Factor_Core::USER_RATE_LIMIT_KEY, time() - MINUTE_IN_SECONDS ); + $this->assertTrue( Two_Factor_Core::is_user_rate_limited( $user ) ); + + // 8 failed logins an hour ago, user should not be rate limited. + update_user_meta( $user->ID, Two_Factor_Core::USER_FAILED_LOGIN_ATTEMPTS_KEY, 8 ); + update_user_meta( $user->ID, Two_Factor_Core::USER_RATE_LIMIT_KEY, time() - HOUR_IN_SECONDS ); + $this->assertFalse( Two_Factor_Core::is_user_rate_limited( $user ) ); + } } From 7cb38bb133ba4d3be0c6ef1a52e9185df70db14f Mon Sep 17 00:00:00 2001 From: Dion Hulse Date: Wed, 8 Feb 2023 13:35:52 +1000 Subject: [PATCH 13/15] Fix test comment. --- tests/class-two-factor-core.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/class-two-factor-core.php b/tests/class-two-factor-core.php index ca10bb85..916d594c 100644 --- a/tests/class-two-factor-core.php +++ b/tests/class-two-factor-core.php @@ -491,7 +491,7 @@ public function test_get_user_time_delay() { update_user_meta( $user->ID, Two_Factor_Core::USER_FAILED_LOGIN_ATTEMPTS_KEY, 5 ); $this->assertEquals( pow( 2, 5 ) * $rate_limit, Two_Factor_Core::get_user_time_delay( $user ) ); - // Simulate 100 failed login attempts, validate that the lockout is < $max_rate_limit + // Simulate 100 failed login attempts, validate that the lockout is not greater than $max_rate_limit update_user_meta( $user->ID, Two_Factor_Core::USER_FAILED_LOGIN_ATTEMPTS_KEY, 100 ); $this->assertEquals( $max_rate_limit, Two_Factor_Core::get_user_time_delay( $user ) ); } From 766f227e74e388a9ef0348b2cae51d59f89d8301 Mon Sep 17 00:00:00 2001 From: Dion Hulse Date: Fri, 10 Feb 2023 10:29:19 +1000 Subject: [PATCH 14/15] Use a more direct error message when rate limits are exceeded to explain why it's limiting the user. Co-authored-by: Ian Dunn --- class-two-factor-core.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/class-two-factor-core.php b/class-two-factor-core.php index 08b6a292..7d8caf00 100644 --- a/class-two-factor-core.php +++ b/class-two-factor-core.php @@ -1028,7 +1028,7 @@ public static function login_form_validate_2fa() { $error = new WP_Error( 'two_factor_too_fast', sprintf( - __( 'ERROR: Whoa there, slow down! You can try again in %s.', 'two-factor' ), + __( 'ERROR: Too many invalid verification codes, you can try again in %s. This limit protects your account against automated attacks.', 'two-factor' ), human_time_diff( $last_login + $time_delay ) ) ); From 8520a17d05ea582a3a1b74cd32c9111c9b58e4f4 Mon Sep 17 00:00:00 2001 From: Dion Hulse Date: Fri, 10 Feb 2023 11:05:05 +1000 Subject: [PATCH 15/15] Tests: Add a test for the login notice of 'invalid login attempts have occurred'. --- tests/class-two-factor-core.php | 40 +++++++++++++++++++++++++++++++++ 1 file changed, 40 insertions(+) diff --git a/tests/class-two-factor-core.php b/tests/class-two-factor-core.php index 916d594c..1ee3dc5a 100644 --- a/tests/class-two-factor-core.php +++ b/tests/class-two-factor-core.php @@ -522,4 +522,44 @@ public function test_is_user_rate_limited() { update_user_meta( $user->ID, Two_Factor_Core::USER_RATE_LIMIT_KEY, time() - HOUR_IN_SECONDS ); $this->assertFalse( Two_Factor_Core::is_user_rate_limited( $user ) ); } + + /** + * Test that the "invalid login attempts have occurred" login notice works as expected. + * + * @covers Two_Factor_Core::maybe_show_last_login_failure_notice() + */ + public function test_maybe_show_last_login_failure_notice() { + $user = $this->get_dummy_user(); + + // User has never logged in, validate they're not rate limited. + ob_start(); + Two_Factor_Core::maybe_show_last_login_failure_notice( $user ); + $contents = ob_get_clean(); + + $this->assertEmpty( $contents ); + + // A failed login attempts 5 seconds ago. + // Should throw a notice, even though it's the current user, it will only be displayed if there's no other 2FA errors. + update_user_meta( $user->ID, Two_Factor_Core::USER_FAILED_LOGIN_ATTEMPTS_KEY, 1 ); + update_user_meta( $user->ID, Two_Factor_Core::USER_RATE_LIMIT_KEY, time() - 5 ); + ob_start(); + Two_Factor_Core::maybe_show_last_login_failure_notice( $user ); + $contents = ob_get_clean(); + + $this->assertNotEmpty( $contents ); + $this->assertStringNotContainsString( '1 times', $contents ); + $this->assertStringContainsString( 'login without providing a valid two factor token', $contents ); + + // 5 failed login attempts 5 hours ago - User should be informed. + $five_hours_ago = time() - 5 * HOUR_IN_SECONDS; + update_user_meta( $user->ID, Two_Factor_Core::USER_FAILED_LOGIN_ATTEMPTS_KEY, 5 ); + update_user_meta( $user->ID, Two_Factor_Core::USER_RATE_LIMIT_KEY, $five_hours_ago ); + ob_start(); + Two_Factor_Core::maybe_show_last_login_failure_notice( $user ); + $contents = ob_get_clean(); + + $this->assertNotEmpty( $contents ); + $this->assertStringContainsString( '5 times', $contents ); + $this->assertStringContainsString( human_time_diff( $five_hours_ago ), $contents ); + } }