From 9f2bfd4be91abc18712b1a2e5f142ac2fd0fb778 Mon Sep 17 00:00:00 2001 From: Ian Dunn Date: Tue, 11 Oct 2022 11:42:27 -0700 Subject: [PATCH 1/3] Readme: Add missing setup instructions --- readme.md | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/readme.md b/readme.md index 9189fd25..75c8f24c 100644 --- a/readme.md +++ b/readme.md @@ -14,12 +14,18 @@ Please [report issues](https://github.com/WordPress/two-factor/issues) and [open Join the `#core-passwords` channel [on WordPress Slack](http://wordpress.slack.com) ([sign up here](http://chat.wordpress.org)). -Here is how to get started: +To use the provided development environment, you'll first need to install and launch Docker. Once it's running, the next steps are: $ git clone https://github.com/wordpress/two-factor.git + $ cd two-factor + $ composer install $ npm install + $ npm run build + $ npm run env start -Then open [a pull request](https://help.github.com/articles/creating-a-pull-request-from-a-fork/) with the suggested changes. +See `package.json` for other available scripts you might want to use during development, like linting and testing. + +When you're ready, open [a pull request](https://help.github.com/articles/creating-a-pull-request-from-a-fork/) with the suggested changes. ## Deployments From c6a6c414cf2d129fd3d269c956e9b2e9fd725586 Mon Sep 17 00:00:00 2001 From: Ian Dunn Date: Tue, 11 Oct 2022 18:22:31 -0700 Subject: [PATCH 2/3] TOTP: Ignore controller/UI code when calculating coverage These functions are either untestable in a pure unit test, or aren't worth testing. They're better suited to manual testing and e2e tests. --- providers/class-two-factor-totp.php | 22 +++++++++++++++++++++- 1 file changed, 21 insertions(+), 1 deletion(-) diff --git a/providers/class-two-factor-totp.php b/providers/class-two-factor-totp.php index 88da5ce8..e64ecd8a 100644 --- a/providers/class-two-factor-totp.php +++ b/providers/class-two-factor-totp.php @@ -38,7 +38,7 @@ class Two_Factor_Totp extends Two_Factor_Provider { const DEFAULT_TIME_STEP_ALLOWANCE = 4; /** - * Chracters used in base32 encoding. + * Characters used in base32 encoding. * * @var string */ @@ -46,6 +46,8 @@ class Two_Factor_Totp extends Two_Factor_Provider { /** * Class constructor. Sets up hooks, etc. + * + * @codeCoverageIgnore */ protected function __construct() { add_action( 'two_factor_user_options_' . __CLASS__, array( $this, 'user_two_factor_options' ) ); @@ -58,6 +60,8 @@ protected function __construct() { /** * Ensures only one instance of this class exists in memory at any one time. + * + * @codeCoverageIgnore */ public static function get_instance() { static $instance; @@ -81,6 +85,8 @@ public function get_label() { * @param string $action Action ID. * * @return void + * + * @codeCoverageIgnore */ public function user_settings_action( $user_id, $action ) { if ( self::ACTION_SECRET_DELETE === $action ) { @@ -94,6 +100,8 @@ public function user_settings_action( $user_id, $action ) { * @param integer $user_id User ID. * * @return string + * + * @codeCoverageIgnore */ protected function get_token_delete_url_for_user( $user_id ) { return Two_Factor_Core::get_user_update_action_url( $user_id, self::ACTION_SECRET_DELETE ); @@ -104,6 +112,8 @@ protected function get_token_delete_url_for_user( $user_id ) { * * @param WP_User $user The current user being edited. * @return false + * + * @codeCoverageIgnore */ public function user_two_factor_options( $user ) { if ( ! isset( $user->ID ) ) { @@ -161,6 +171,8 @@ public function user_two_factor_options( $user ) { * @param integer $user_id The user ID whose options are being updated. * * @return void + * + * @codeCoverageIgnore */ public function user_two_factor_options_update( $user_id ) { $notices = array(); @@ -255,6 +267,8 @@ public function is_valid_key( $key ) { * @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 ); @@ -286,6 +300,8 @@ public function admin_notices( $user_id ) { * @param WP_User $user WP_User object of the logged-in user. * * @return bool Whether the user gave a valid code + * + * @codeCoverageIgnore */ public function validate_authentication( $user ) { if ( ! empty( $_REQUEST['authcode'] ) ) { @@ -421,6 +437,8 @@ public static function calc_totp( $key, $step_count = false, $digits = self::DEF * @param string $title The title to display in the Authentication app. * * @return string A URL to use as an img src to display the QR code + * + * @codeCoverageIgnore */ public static function get_google_qr_code( $name, $key, $title = null ) { // Encode to support spaces, question marks and other characters. @@ -450,6 +468,8 @@ public function is_available_for_user( $user ) { * Prints the form that prompts the user to authenticate. * * @param WP_User $user WP_User object of the logged-in user. + * + * @codeCoverageIgnore */ public function authentication_page( $user ) { require_once ABSPATH . '/wp-admin/includes/template.php'; From 57506c38e8bbd0da517525d61f07b0098fef2db6 Mon Sep 17 00:00:00 2001 From: Ian Dunn Date: Tue, 11 Oct 2022 18:30:40 -0700 Subject: [PATCH 3/3] TOTP: Add tests to increase coverage --- tests/providers/class-two-factor-totp.php | 30 ++++++++++++++++++++++- 1 file changed, 29 insertions(+), 1 deletion(-) diff --git a/tests/providers/class-two-factor-totp.php b/tests/providers/class-two-factor-totp.php index be5e72c5..ccc9ab0b 100644 --- a/tests/providers/class-two-factor-totp.php +++ b/tests/providers/class-two-factor-totp.php @@ -184,18 +184,32 @@ public function test_base32_encode() { $string_base32 = 'IVLDKWCXG5KE6TBUKFEESS2CJFDVMRKVGIZUWQKGKJHEINRWJRMQ'; $this->assertEquals( $string_base32, $this->provider->base32_encode( $string ) ); + $this->assertEquals( '', $this->provider->base32_encode( '' ) ); } /** * Verify base32 decoding. * - * @covers Two_Factor_Totp::base32_encode + * @covers Two_Factor_Totp::base32_decode */ public function test_base32_decode() { $string = 'EV5XW7TOL4QHIKBIGVEU23KAFRND66LY'; $string_base32 = 'IVLDKWCXG5KE6TBUKFEESS2CJFDVMRKVGIZUWQKGKJHEINRWJRMQ'; $this->assertEquals( $string, $this->provider->base32_decode( $string_base32 ) ); + + } + + /** + * Test base32 decoding an invalid string. + * + * @covers Two_Factor_Totp::base32_decode + */ + public function test_base32_decode_exception() { + $string_base32 = 'IVLDKWCXG5KE6TBUKFEESS2CJFDVMRKVGIZUWQKGKJHEINRWJRMQ'; + + $this->expectExceptionMessage( 'Invalid characters in the base32 string.' ); + $this->provider->base32_decode( $string_base32 . '@' ); } /** @@ -204,6 +218,9 @@ public function test_base32_decode() { * @covers Two_Factor_Totp::is_valid_authcode * @covers Two_Factor_Totp::generate_key * @covers Two_Factor_Totp::calc_totp + * @covers Two_Factor_Totp::pack64 + * @covers Two_Factor_Totp::base32_decode + * @covers Two_Factor_Totp::abssort */ public function test_is_valid_authcode() { $key = $this->provider->generate_key(); @@ -212,6 +229,17 @@ public function test_is_valid_authcode() { $this->assertTrue( $this->provider->is_valid_authcode( $key, $authcode ) ); } + /** + * Verify authcode rejection. + * + * @covers Two_Factor_Totp::is_valid_authcode + */ + public function test_invalid_authcode_rejected() { + $key = $this->provider->generate_key(); + + $this->assertFalse( $this->provider->is_valid_authcode( $key, '012345' ) ); + } + /** * Check secret key CRUD operations. *