From 3e14b6bd11d91ba5e6a7fd94ecae6ecc05cf22bc Mon Sep 17 00:00:00 2001 From: Vraja Das Date: Thu, 27 Jul 2023 11:01:21 +0200 Subject: [PATCH 1/7] php 8.1 improvements Replaced filter_var wrapped wp_verify_nonce for better testing and used here --- src/Whip_WPMessageDismissListener.php | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/src/Whip_WPMessageDismissListener.php b/src/Whip_WPMessageDismissListener.php index a387e90..dc3f292 100644 --- a/src/Whip_WPMessageDismissListener.php +++ b/src/Whip_WPMessageDismissListener.php @@ -39,10 +39,11 @@ public function __construct( Whip_MessageDismisser $dismisser ) { * @return void */ public function listen() { - $action = filter_input( INPUT_GET, 'action' ); - $nonce = filter_input( INPUT_GET, 'nonce' ); - if ( $action === self::ACTION_NAME && wp_verify_nonce( $nonce, self::ACTION_NAME ) ) { + $action = ( isset( $_GET['action'] ) && is_string( $_GET['action'] ) ) ? sanitize_text_field( wp_unslash( $_GET['action'] ) ) : null; + $nonce = ( isset( $_GET['nonce'] ) && is_string( $_GET['nonce'] ) ) ? sanitize_text_field( wp_unslash( $_GET['nonce'] ) ) : null; + + if ( $action === self::ACTION_NAME && $this->dismisser->verify_nonce( $nonce, self::ACTION_NAME ) ) { $this->dismisser->dismiss(); } } From dc4184e04a7410281ca9ef3982f05117fd195edf Mon Sep 17 00:00:00 2001 From: Vraja Das Date: Thu, 27 Jul 2023 11:03:10 +0200 Subject: [PATCH 2/7] php cs fix --- src/Whip_WPMessageDismissListener.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Whip_WPMessageDismissListener.php b/src/Whip_WPMessageDismissListener.php index dc3f292..4747057 100644 --- a/src/Whip_WPMessageDismissListener.php +++ b/src/Whip_WPMessageDismissListener.php @@ -43,7 +43,7 @@ public function listen() { $action = ( isset( $_GET['action'] ) && is_string( $_GET['action'] ) ) ? sanitize_text_field( wp_unslash( $_GET['action'] ) ) : null; $nonce = ( isset( $_GET['nonce'] ) && is_string( $_GET['nonce'] ) ) ? sanitize_text_field( wp_unslash( $_GET['nonce'] ) ) : null; - if ( $action === self::ACTION_NAME && $this->dismisser->verify_nonce( $nonce, self::ACTION_NAME ) ) { + if ( $action === self::ACTION_NAME && $this->dismisser->verifyNonce( $nonce, self::ACTION_NAME ) ) { $this->dismisser->dismiss(); } } From 93c4bae10a9dccfe83771e28ce6ef1753bbdba8e Mon Sep 17 00:00:00 2001 From: Vraja Das Date: Thu, 27 Jul 2023 11:03:47 +0200 Subject: [PATCH 3/7] Wrapped wp_verify_nonce for testing --- src/Whip_MessageDismisser.php | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/src/Whip_MessageDismisser.php b/src/Whip_MessageDismisser.php index ed9c178..881a7f2 100644 --- a/src/Whip_MessageDismisser.php +++ b/src/Whip_MessageDismisser.php @@ -59,4 +59,16 @@ public function dismiss() { public function isDismissed() { return ( $this->currentTime <= ( $this->storage->get() + $this->threshold ) ); } + + /** + * Checks the nonce. + * + * @param string $nonce The nonce to check. + * @param string $action The action to check. + * + * @return bool True when the nonce is valid. + */ + public function verifyNonce( $nonce, $action ) { + return wp_verify_nonce( $nonce, $action ); + } } From c50aec242fc5e18dc7417858dcaaead417e8be12 Mon Sep 17 00:00:00 2001 From: Vraja Das Date: Thu, 27 Jul 2023 11:07:33 +0200 Subject: [PATCH 4/7] add mock methods --- tests/doubles/WPCoreFunctionsMock.php | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+) diff --git a/tests/doubles/WPCoreFunctionsMock.php b/tests/doubles/WPCoreFunctionsMock.php index 65b278c..950d2c3 100644 --- a/tests/doubles/WPCoreFunctionsMock.php +++ b/tests/doubles/WPCoreFunctionsMock.php @@ -30,3 +30,25 @@ function __( $message ) { function esc_url( $url ) { return $url; } + +/** + * Mock for sanitize_text_field. + * + * @param string $text The text to be sanitize. + * + * @return string The text that was sanitized. + */ +function sanitize_text_field( $text ) { + return $text; +} + +/** + * Mock for wp_unslash. + * + * @param string $string The string to be wp_unslash. + * + * @return string The string that was unslashed. + */ +function wp_unslash( $string ) { + return $string; +} From 75dfc00c0eb0957419d26911c6439825e00af6b2 Mon Sep 17 00:00:00 2001 From: Vraja Das Date: Thu, 27 Jul 2023 11:08:05 +0200 Subject: [PATCH 5/7] unit test for WPMessageDismissListener --- tests/WPMessageDismissListenerTest.php | 86 ++++++++++++++++++++++++++ 1 file changed, 86 insertions(+) create mode 100644 tests/WPMessageDismissListenerTest.php diff --git a/tests/WPMessageDismissListenerTest.php b/tests/WPMessageDismissListenerTest.php new file mode 100644 index 0000000..84fcb35 --- /dev/null +++ b/tests/WPMessageDismissListenerTest.php @@ -0,0 +1,86 @@ +getMockBuilder( 'Whip_MessageDismisser' ) + ->disableOriginalConstructor() + ->getMock(); + + $instance = new Whip_WPMessageDismissListener( $dismisser ); + + $_GET['action'] = $action; + $_GET['nonce'] = $nonce; + + $dismisser->expects( $this->exactly( $verifyNonceTimes ) ) + ->method( 'verify_nonce' ) + ->with( $nonce, $action ) + ->willReturn( $isCorrectNonce ); + + $dismisser->expects( $this->exactly( $dismissTimes ) ) + ->method( 'dismiss' ); + + $instance->listen(); + } + + /** + * Data provider for testDismiss. + * + * @return array + */ + public function listenProvider() { + return array( + 'correct action and nonce' => array( + 'action' => Whip_WPMessageDismissListener::ACTION_NAME, + 'nonce' => 'the_right_nonce', + 'verifyNonceTimes' => 1, + 'isCorrectNonce' => true, + 'dismissTimes' => 1, + ), + 'incorrect action correct nonce' => array( + 'action' => 'wrong_action', + 'nonce' => 'the_right_nonce', + 'verifyNonceTimes' => 0, + 'isCorrectNonce' => false, + 'dismissTimes' => 0, + ), + 'correct action incorrect nonce' => array( + 'action' => Whip_WPMessageDismissListener::ACTION_NAME, + 'nonce' => 'wrong_nonce', + 'verifyNonceTimes' => 1, + 'isCorrectNonce' => false, + 'dismissTimes' => 0, + ), + 'incorrect action and nonce' => array( + 'action' => 'wrong_action', + 'nonce' => 'wrong_nonce', + 'verifyNonceTimes' => 0, + 'isCorrectNonce' => false, + 'dismissTimes' => 0, + ), + ); + } +} From 84a3978d3d7ff8b1e02a8e6155477ccaed674b8d Mon Sep 17 00:00:00 2001 From: Vraja Das Date: Thu, 27 Jul 2023 11:26:26 +0200 Subject: [PATCH 6/7] php cs fix - comments --- src/Whip_WPMessageDismissListener.php | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/Whip_WPMessageDismissListener.php b/src/Whip_WPMessageDismissListener.php index 4747057..44835c2 100644 --- a/src/Whip_WPMessageDismissListener.php +++ b/src/Whip_WPMessageDismissListener.php @@ -40,8 +40,10 @@ public function __construct( Whip_MessageDismisser $dismisser ) { */ public function listen() { + // phpcs:ignore WordPress.Security.NonceVerification.Recommended -- Nonce is verified in the dismisser. $action = ( isset( $_GET['action'] ) && is_string( $_GET['action'] ) ) ? sanitize_text_field( wp_unslash( $_GET['action'] ) ) : null; - $nonce = ( isset( $_GET['nonce'] ) && is_string( $_GET['nonce'] ) ) ? sanitize_text_field( wp_unslash( $_GET['nonce'] ) ) : null; + // phpcs:ignore WordPress.Security.NonceVerification.Recommended -- Nonce is verified in the dismisser. + $nonce = ( isset( $_GET['nonce'] ) && is_string( $_GET['nonce'] ) ) ? sanitize_text_field( wp_unslash( $_GET['nonce'] ) ) : null; if ( $action === self::ACTION_NAME && $this->dismisser->verifyNonce( $nonce, self::ACTION_NAME ) ) { $this->dismisser->dismiss(); From feb1abe591dc78acd5604e4fcf959a8631566662 Mon Sep 17 00:00:00 2001 From: Vraja Das Date: Thu, 27 Jul 2023 11:28:20 +0200 Subject: [PATCH 7/7] refactor naming --- tests/WPMessageDismissListenerTest.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/WPMessageDismissListenerTest.php b/tests/WPMessageDismissListenerTest.php index 84fcb35..58f8f52 100644 --- a/tests/WPMessageDismissListenerTest.php +++ b/tests/WPMessageDismissListenerTest.php @@ -36,7 +36,7 @@ public function testDismiss( $action, $nonce, $verifyNonceTimes, $isCorrectNonce $_GET['nonce'] = $nonce; $dismisser->expects( $this->exactly( $verifyNonceTimes ) ) - ->method( 'verify_nonce' ) + ->method( 'verifyNonce' ) ->with( $nonce, $action ) ->willReturn( $isCorrectNonce );