From 668ae87ac5228930e643f09cfa49bacc12ffa056 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Viktor=20Sz=C3=A9pe?= Date: Mon, 15 Jul 2024 03:11:32 +0000 Subject: [PATCH 1/5] Fix errors reported by PHPStan --- class-two-factor-core.php | 1 + providers/class-two-factor-backup-codes.php | 2 +- providers/class-two-factor-dummy.php | 2 +- providers/class-two-factor-email.php | 2 +- providers/class-two-factor-fido-u2f-admin.php | 4 ++-- providers/class-two-factor-fido-u2f.php | 2 +- providers/class-two-factor-totp.php | 6 +++--- 7 files changed, 10 insertions(+), 9 deletions(-) diff --git a/class-two-factor-core.php b/class-two-factor-core.php index 5114d280..c6f28145 100644 --- a/class-two-factor-core.php +++ b/class-two-factor-core.php @@ -1161,6 +1161,7 @@ public static function current_user_can_update_two_factor_options( $context = 'd * Return a falsey value (false, 0) if you wish to never require revalidation. * * @param int $two_factor_revalidate_time The grace time between last validation time and when it'll be accepted. Default 10 minutes (in seconds). + * @param int $user_id The user ID. * @param string $context The context in use, 'display' or 'save'. Save has twice the grace time. */ $two_factor_revalidate_time = apply_filters( 'two_factor_revalidate_time', 10 * MINUTE_IN_SECONDS, $user_id, $context ); diff --git a/providers/class-two-factor-backup-codes.php b/providers/class-two-factor-backup-codes.php index c53a8448..6b017de4 100644 --- a/providers/class-two-factor-backup-codes.php +++ b/providers/class-two-factor-backup-codes.php @@ -40,7 +40,7 @@ protected function __construct() { add_action( 'two_factor_user_options_' . __CLASS__, array( $this, 'user_options' ) ); add_action( 'admin_notices', array( $this, 'admin_notices' ) ); - return parent::__construct(); + parent::__construct(); } /** diff --git a/providers/class-two-factor-dummy.php b/providers/class-two-factor-dummy.php index 34c6b57d..cc9a6119 100644 --- a/providers/class-two-factor-dummy.php +++ b/providers/class-two-factor-dummy.php @@ -21,7 +21,7 @@ class Two_Factor_Dummy extends Two_Factor_Provider { */ protected function __construct() { add_action( 'two_factor_user_options_' . __CLASS__, array( $this, 'user_options' ) ); - return parent::__construct(); + parent::__construct(); } /** diff --git a/providers/class-two-factor-email.php b/providers/class-two-factor-email.php index c6f5881f..b21f1391 100644 --- a/providers/class-two-factor-email.php +++ b/providers/class-two-factor-email.php @@ -42,7 +42,7 @@ class Two_Factor_Email extends Two_Factor_Provider { */ protected function __construct() { add_action( 'two_factor_user_options_' . __CLASS__, array( $this, 'user_options' ) ); - return parent::__construct(); + parent::__construct(); } /** diff --git a/providers/class-two-factor-fido-u2f-admin.php b/providers/class-two-factor-fido-u2f-admin.php index 23530370..d5412089 100644 --- a/providers/class-two-factor-fido-u2f-admin.php +++ b/providers/class-two-factor-fido-u2f-admin.php @@ -230,7 +230,7 @@ public static function show_user_profile( $user ) { * @static * * @param int $user_id User ID. - * @return false + * @return void|never */ public static function catch_submission( $user_id ) { if ( ! empty( $_REQUEST['do_new_security_key'] ) ) { @@ -243,7 +243,7 @@ public static function catch_submission( $user_id ) { Two_Factor_FIDO_U2F::add_security_key( $user_id, $reg ); } catch ( Exception $e ) { - return false; + return; } delete_user_meta( $user_id, self::REGISTER_DATA_USER_META_KEY ); diff --git a/providers/class-two-factor-fido-u2f.php b/providers/class-two-factor-fido-u2f.php index 52d28f57..f775224c 100644 --- a/providers/class-two-factor-fido-u2f.php +++ b/providers/class-two-factor-fido-u2f.php @@ -65,7 +65,7 @@ protected function __construct() { add_action( 'two_factor_user_options_' . __CLASS__, array( $this, 'user_options' ) ); - return parent::__construct(); + parent::__construct(); } /** diff --git a/providers/class-two-factor-totp.php b/providers/class-two-factor-totp.php index 4b075495..a90e4a8f 100644 --- a/providers/class-two-factor-totp.php +++ b/providers/class-two-factor-totp.php @@ -48,7 +48,7 @@ protected function __construct() { add_action( 'wp_enqueue_scripts', array( $this, 'enqueue_assets' ) ); add_action( 'two_factor_user_options_' . __CLASS__, array( $this, 'user_two_factor_options' ) ); - return parent::__construct(); + parent::__construct(); } /** @@ -260,13 +260,13 @@ public static function generate_qr_code_url( $user, $secret_key ) { * Display TOTP options on the user settings page. * * @param WP_User $user The current user being edited. - * @return false + * @return void * * @codeCoverageIgnore */ public function user_two_factor_options( $user ) { if ( ! isset( $user->ID ) ) { - return false; + return; } $key = $this->get_user_totp_key( $user->ID ); From 9ac7c2d52e2ef7281947b3ed655faa6ac542ba36 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Viktor=20Sz=C3=A9pe?= Date: Mon, 15 Jul 2024 03:28:58 +0000 Subject: [PATCH 2/5] More error fixes on level 2 --- providers/class-two-factor-fido-u2f.php | 4 ++-- providers/class-two-factor-totp.php | 8 ++++---- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/providers/class-two-factor-fido-u2f.php b/providers/class-two-factor-fido-u2f.php index f775224c..c90cf6b3 100644 --- a/providers/class-two-factor-fido-u2f.php +++ b/providers/class-two-factor-fido-u2f.php @@ -143,7 +143,7 @@ public static function enqueue_scripts() { * @since 0.1-dev * * @param WP_User $user WP_User object of the logged-in user. - * @return null + * @return void */ public function authentication_page( $user ) { require_once ABSPATH . '/wp-admin/includes/template.php'; @@ -165,7 +165,7 @@ public function authentication_page( $user ) { ?>

Date: Tue, 17 Sep 2024 15:01:27 +1000 Subject: [PATCH 3/5] Replace the h3 with a h2, Security keys nests under this. --- 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 62e4019f..82b42d96 100644 --- a/class-two-factor-core.php +++ b/class-two-factor-core.php @@ -1721,7 +1721,7 @@ public static function user_two_factor_options( $user ) { wp_nonce_field( 'user_two_factor_options', '_nonce_user_two_factor_options', false ); ?> -

+

From bb048cbb94cdb5bb666cd02d0ed2503a6b0a8273 Mon Sep 17 00:00:00 2001 From: Dion Date: Tue, 17 Sep 2024 15:14:44 +1000 Subject: [PATCH 4/5] TOTP: Add accessibility for the QR Code, marking it as an image and giving it a proper title. --- providers/class-two-factor-totp.php | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/providers/class-two-factor-totp.php b/providers/class-two-factor-totp.php index 4b075495..88f743db 100644 --- a/providers/class-two-factor-totp.php +++ b/providers/class-two-factor-totp.php @@ -318,6 +318,15 @@ public function user_two_factor_options( $user ) { qr.make(); document.querySelector( '#two-factor-qr-code a' ).innerHTML = qr.createSvgTag( 5 ); + + // For accessibility, markup the SVG with a title and role. + var svg = document.querySelector( '#two-factor-qr-code a svg' ), + title = document.createElement( 'title' ); + + svg.role = 'image'; + svg.ariaLabel = ; + title.innerText = svg.ariaLabel; + svg.appendChild( title ); }; // Run now if the document is loaded, otherwise on DOMContentLoaded. From d59daca076d1555c330af7142cd8068f3cf8ba4b Mon Sep 17 00:00:00 2001 From: Kaspars Dambis Date: Wed, 18 Sep 2024 18:04:41 +0300 Subject: [PATCH 5/5] This test no longer makes sense --- tests/providers/class-two-factor-totp.php | 9 --------- 1 file changed, 9 deletions(-) diff --git a/tests/providers/class-two-factor-totp.php b/tests/providers/class-two-factor-totp.php index 8c961f12..52aa6090 100644 --- a/tests/providers/class-two-factor-totp.php +++ b/tests/providers/class-two-factor-totp.php @@ -61,15 +61,6 @@ public function test_get_label() { $this->assertStringContainsString( 'Authenticator app', $this->provider->get_label() ); } - /** - * Verify the options list is empty. - * - * @covers Two_Factor_Totp::user_two_factor_options - */ - public function test_user_two_factor_options_empty() { - $this->assertFalse( $this->provider->user_two_factor_options( get_current_user() ) ); - } - /** * Verify getting user options creates a key. *