From fc1f3e6dc265f5617c44b49ac80fb149f151c43a Mon Sep 17 00:00:00 2001 From: John Oleksowicz Date: Fri, 25 Mar 2022 12:01:48 -0500 Subject: [PATCH] Inline generateToken for clarity The only thing Security::generateToken was providing was a default value for the $action, which created a pattern of using the same $action everywhere, which may not be the best way to go. Since it was essentially a wrapper for WP's built-in nonce functions, it seemed clearer to use those functions directly to be more explicit about how we're handling tokens. [MAILPOET-2030] --- mailpoet/lib/API/JSON/API.php | 11 +++-------- mailpoet/lib/Form/DisplayFormInWPContent.php | 3 +-- mailpoet/lib/Form/Widget.php | 3 +-- mailpoet/lib/Util/Security.php | 5 ----- mailpoet/tests/integration/API/JSON/APITest.php | 6 +++++- mailpoet/tests/integration/Subscription/FormTest.php | 3 +-- 6 files changed, 11 insertions(+), 20 deletions(-) diff --git a/mailpoet/lib/API/JSON/API.php b/mailpoet/lib/API/JSON/API.php index 0993884b6e..fab25dc0e2 100644 --- a/mailpoet/lib/API/JSON/API.php +++ b/mailpoet/lib/API/JSON/API.php @@ -9,7 +9,6 @@ use MailPoet\Subscription\Captcha; use MailPoet\Tracy\ApiPanel\ApiPanel; use MailPoet\Tracy\DIPanel\DIPanel; use MailPoet\Util\Helpers; -use MailPoet\Util\Security; use MailPoet\WP\Functions as WPFunctions; use MailPoetVendor\Psr\Container\ContainerInterface; use Throwable; @@ -106,7 +105,7 @@ class API { $this->requestMethod === 'subscribe' ); - if (!$ignoreToken && $this->checkToken() === false) { + if (!$ignoreToken && $this->wp->wpVerifyNonce($this->requestToken, 'mailpoet_token') === false) { $errorMessage = WPFunctions::get()->__("Sorry, but we couldn't connect to the MailPoet server. Please refresh the web page and try again.", 'mailpoet'); $errorResponse = $this->createErrorResponse(Error::UNAUTHORIZED, $errorMessage, Response::STATUS_UNAUTHORIZED); return $errorResponse->send(); @@ -228,23 +227,19 @@ class API { $this->accessControl->validatePermission($permissions['global']); } - public function checkToken() { - return WPFunctions::get()->wpVerifyNonce($this->requestToken, 'mailpoet_token'); - } - public function setTokenAndAPIVersion() { echo sprintf( '', - esc_js(Security::generateToken()), + esc_js($this->wp->wpCreateNonce('mailpoet_token')), esc_js(self::CURRENT_VERSION) ); } public function addTokenToHeartbeatResponse($response) { - $response['mailpoet_token'] = Security::generateToken(); + $response['mailpoet_token'] = $this->wp->wpCreateNonce('mailpoet_token'); return $response; } diff --git a/mailpoet/lib/Form/DisplayFormInWPContent.php b/mailpoet/lib/Form/DisplayFormInWPContent.php index 537d56d632..152b83029e 100644 --- a/mailpoet/lib/Form/DisplayFormInWPContent.php +++ b/mailpoet/lib/Form/DisplayFormInWPContent.php @@ -7,7 +7,6 @@ use MailPoet\Config\Renderer as TemplateRenderer; use MailPoet\Entities\FormEntity; use MailPoet\Subscribers\SubscribersRepository; use MailPoet\Subscribers\SubscriberSubscribeController; -use MailPoet\Util\Security; use MailPoet\WP\Functions as WPFunctions; class DisplayFormInWPContent { @@ -188,7 +187,7 @@ class DisplayFormInWPContent { } // generate security token - $templateData['token'] = Security::generateToken(); + $templateData['token'] = $this->wp->wpCreateNonce('mailpoet_token'); // add API version $templateData['api_version'] = API::CURRENT_VERSION; diff --git a/mailpoet/lib/Form/Widget.php b/mailpoet/lib/Form/Widget.php index 2efa2797e0..b107c1d5e2 100644 --- a/mailpoet/lib/Form/Widget.php +++ b/mailpoet/lib/Form/Widget.php @@ -9,7 +9,6 @@ use MailPoet\Entities\FormEntity; use MailPoet\Form\Renderer as FormRenderer; use MailPoet\Form\Util\CustomFonts; use MailPoet\Settings\SettingsController; -use MailPoet\Util\Security; use MailPoet\WP\Functions as WPFunctions; // phpcs:disable Generic.Files.InlineHTML @@ -249,7 +248,7 @@ class Widget extends \WP_Widget { ); // generate security token - $data['token'] = Security::generateToken(); + $data['token'] = $this->wp->wpCreateNonce('mailpoet_token'); // add API version $data['api_version'] = API::CURRENT_VERSION; diff --git a/mailpoet/lib/Util/Security.php b/mailpoet/lib/Util/Security.php index e240c28856..fdfe195fc5 100644 --- a/mailpoet/lib/Util/Security.php +++ b/mailpoet/lib/Util/Security.php @@ -7,7 +7,6 @@ use MailPoet\Entities\NewsletterEntity; use MailPoet\Entities\SubscriberEntity; use MailPoet\Newsletter\NewslettersRepository; use MailPoet\Subscribers\SubscribersRepository; -use MailPoet\WP\Functions as WPFunctions; class Security { const HASH_LENGTH = 12; @@ -27,10 +26,6 @@ class Security { $this->subscribersRepository = $subscribersRepository; } - public static function generateToken($action = 'mailpoet_token') { - return WPFunctions::get()->wpCreateNonce($action); - } - /** * Generate random lowercase alphanumeric string. * 1 lowercase alphanumeric character = 6 bits (because log2(36) = 5.17) diff --git a/mailpoet/tests/integration/API/JSON/APITest.php b/mailpoet/tests/integration/API/JSON/APITest.php index a04c4e9e52..d21760ef6b 100644 --- a/mailpoet/tests/integration/API/JSON/APITest.php +++ b/mailpoet/tests/integration/API/JSON/APITest.php @@ -73,11 +73,15 @@ class APITest extends \MailPoetTest { expect($api instanceof JSONAPI)->true(); } ); + $wpStub = Stub::make(new WPFunctions, [ + 'wpVerifyNonce' => asCallable(function() { + return true; + })]); $api = Stub::makeEmptyExcept( $this->api, 'setupAjax', [ - 'wp' => new WPFunctions, + 'wp' => $wpStub, 'processRoute' => Stub::makeEmpty(new SuccessResponse), 'settings' => $this->container->get(SettingsController::class), ] diff --git a/mailpoet/tests/integration/Subscription/FormTest.php b/mailpoet/tests/integration/Subscription/FormTest.php index 235e2baf3b..5c0c5c784a 100644 --- a/mailpoet/tests/integration/Subscription/FormTest.php +++ b/mailpoet/tests/integration/Subscription/FormTest.php @@ -14,7 +14,6 @@ use MailPoet\Models\Subscriber as SubscriberModel; use MailPoet\Settings\SettingsController; use MailPoet\Settings\SettingsRepository; use MailPoet\Subscription\Form; -use MailPoet\Util\Security; use MailPoet\Util\Url as UrlHelper; use MailPoet\WP\Functions as WPFunctions; use MailPoetVendor\Idiorm\ORM; @@ -62,7 +61,7 @@ class FormTest extends \MailPoetTest { 'form_id' => $this->form->getId(), $obfuscatedEmail => $this->testEmail, ], - 'token' => Security::generateToken(), + 'token' => WPFunctions::get()->wpCreateNonce('mailpoet_token'), 'api_version' => 'v1', 'endpoint' => 'subscribers', 'mailpoet_method' => 'subscribe',