Merge pull request #133 from Yoast/132-whip-php-81-improvements

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 );
+	}
 }

src/Whip_WPMessageDismissListener.php

@@ -39,10 +39,13 @@ 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 ) ) {
+		// 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;
+		// 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();
 		}
 	}

tests/WPMessageDismissListenerTest.php

@@ -0,0 +1,86 @@
+<?php
+/**
+ * WHIP libary test file.
+ *
+ * @package Yoast\WHIP
+ */
+
+/**
+ * Message Dismiss Listener unit tests.
+ *
+ * @coversDefaultClass Whip_WPMessageDismissListener
+ */
+class WPMessageDismissListener extends Whip_TestCase {
+
+	/**
+	 * Tests the listen method.
+	 *
+	 * @covers ::listen
+	 *
+	 * @dataProvider listenProvider
+	 *
+	 * @param string $action The action to test.
+	 * @param string $nonce The nonce to test.
+	 * @param int    $verifyNonceTimes The times to call wp_verify_nonce.
+	 * @param bool   $isCorrectNonce Whether the nonce is correct.
+	 * @param bool   $dismissTimes The times to call dismiss.
+	 */
+	public function testDismiss( $action, $nonce, $verifyNonceTimes, $isCorrectNonce, $dismissTimes ) {
+		$dismisser = $this->getMockBuilder( 'Whip_MessageDismisser' )
+			->disableOriginalConstructor()
+			->getMock();
+
+		$instance = new Whip_WPMessageDismissListener( $dismisser );
+
+		$_GET['action'] = $action;
+		$_GET['nonce']  = $nonce;
+
+		$dismisser->expects( $this->exactly( $verifyNonceTimes ) )
+				->method( 'verifyNonce' )
+				->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,
+			),
+		);
+	}
+}

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;
+}