Use CaptchaSession service instead of PHP Session

[MAILPOET-2248]
This commit is contained in:
Rostislav Wolny
2019-08-05 08:55:16 +02:00
committed by Jack Kitterhing
parent 6a494a2fae
commit 8e1c3b8c03
5 changed files with 47 additions and 19 deletions

View File

@ -20,6 +20,7 @@ use MailPoet\Subscribers\RequiredCustomFieldValidator;
use MailPoet\Subscribers\Source; use MailPoet\Subscribers\Source;
use MailPoet\Subscribers\SubscriberActions; use MailPoet\Subscribers\SubscriberActions;
use MailPoet\Subscription\Captcha; use MailPoet\Subscription\Captcha;
use MailPoet\Subscription\CaptchaSession;
use MailPoet\Subscription\Throttling as SubscriptionThrottling; use MailPoet\Subscription\Throttling as SubscriptionThrottling;
use MailPoet\Subscription\Url as SubscriptionUrl; use MailPoet\Subscription\Url as SubscriptionUrl;
use MailPoet\WP\Functions as WPFunctions; use MailPoet\WP\Functions as WPFunctions;
@ -58,6 +59,9 @@ class Subscribers extends APIEndpoint {
/** @var SettingsController */ /** @var SettingsController */
private $settings; private $settings;
/** @var CaptchaSession */
private $captcha_session;
public function __construct( public function __construct(
Listing\BulkActionController $bulk_action_controller, Listing\BulkActionController $bulk_action_controller,
SubscribersListings $subscribers_listings, SubscribersListings $subscribers_listings,
@ -66,7 +70,8 @@ class Subscribers extends APIEndpoint {
Listing\Handler $listing_handler, Listing\Handler $listing_handler,
Captcha $subscription_captcha, Captcha $subscription_captcha,
WPFunctions $wp, WPFunctions $wp,
SettingsController $settings SettingsController $settings,
CaptchaSession $captcha_session
) { ) {
$this->bulk_action_controller = $bulk_action_controller; $this->bulk_action_controller = $bulk_action_controller;
$this->subscribers_listings = $subscribers_listings; $this->subscribers_listings = $subscribers_listings;
@ -76,6 +81,7 @@ class Subscribers extends APIEndpoint {
$this->subscription_captcha = $subscription_captcha; $this->subscription_captcha = $subscription_captcha;
$this->wp = $wp; $this->wp = $wp;
$this->settings = $settings; $this->settings = $settings;
$this->captcha_session = $captcha_session;
} }
function get($data = []) { function get($data = []) {
@ -164,10 +170,10 @@ class Subscribers extends APIEndpoint {
if (!empty($captcha_settings['type']) && $captcha_settings['type'] === Captcha::TYPE_BUILTIN) { if (!empty($captcha_settings['type']) && $captcha_settings['type'] === Captcha::TYPE_BUILTIN) {
if (!isset($data['captcha'])) { if (!isset($data['captcha'])) {
// Save form data to session // Save form data to session
$_SESSION[Captcha::SESSION_FORM_KEY] = array_merge($data, ['form_id' => $form_id]); $this->captcha_session->setFormData(array_merge($data, ['form_id' => $form_id]));
} elseif (!empty($_SESSION[Captcha::SESSION_FORM_KEY])) { } elseif ($this->captcha_session->getFormData()) {
// Restore form data from session // Restore form data from session
$data = array_merge($_SESSION[Captcha::SESSION_FORM_KEY], ['captcha' => $data['captcha']]); $data = array_merge($this->captcha_session->getFormData(), ['captcha' => $data['captcha']]);
} }
// Otherwise use the post data // Otherwise use the post data
} }
@ -222,8 +228,7 @@ class Subscribers extends APIEndpoint {
} else { } else {
if (!empty($captcha_settings['type']) && $captcha_settings['type'] === Captcha::TYPE_BUILTIN) { if (!empty($captcha_settings['type']) && $captcha_settings['type'] === Captcha::TYPE_BUILTIN) {
// Captcha has been verified, invalidate the session vars // Captcha has been verified, invalidate the session vars
$_SESSION[Captcha::SESSION_KEY] = null; $this->captcha_session->reset();
$_SESSION[Captcha::SESSION_FORM_KEY] = null;
} }
$meta = []; $meta = [];
@ -299,12 +304,13 @@ class Subscribers extends APIEndpoint {
]); ]);
} }
} elseif ($captcha_settings['type'] === Captcha::TYPE_BUILTIN && $is_builtin_captcha_required) { } elseif ($captcha_settings['type'] === Captcha::TYPE_BUILTIN && $is_builtin_captcha_required) {
if (empty($_SESSION[Captcha::SESSION_KEY])) { $captcha_hash = $this->captcha_session->getCaptchaHash();
if (empty($captcha_hash)) {
return $this->badRequest([ return $this->badRequest([
APIError::BAD_REQUEST => WPFunctions::get()->__('Please regenerate the CAPTCHA.', 'mailpoet'), APIError::BAD_REQUEST => WPFunctions::get()->__('Please regenerate the CAPTCHA.', 'mailpoet'),
]); ]);
} elseif (!hash_equals(strtolower($data['captcha']), $_SESSION[Captcha::SESSION_KEY])) { } elseif (!hash_equals(strtolower($data['captcha']), $captcha_hash)) {
$_SESSION[Captcha::SESSION_KEY] = null; $this->captcha_session->setCaptchaHash(null);
$meta = []; $meta = [];
$meta['refresh_captcha'] = true; $meta['refresh_captcha'] = true;
return $this->badRequest([ return $this->badRequest([

View File

@ -2,6 +2,7 @@
namespace MailPoet\Subscription; namespace MailPoet\Subscription;
use MailPoet\Config\Session;
use MailPoet\Models\Subscriber; use MailPoet\Models\Subscriber;
use MailPoet\Models\SubscriberIP; use MailPoet\Models\SubscriberIP;
use MailPoet\Util\Helpers; use MailPoet\Util\Helpers;
@ -13,17 +14,21 @@ class Captcha {
const TYPE_RECAPTCHA = 'recaptcha'; const TYPE_RECAPTCHA = 'recaptcha';
const TYPE_DISABLED = null; const TYPE_DISABLED = null;
const SESSION_KEY = 'mailpoet_captcha';
const SESSION_FORM_KEY = 'mailpoet_captcha_form';
/** @var WPFunctions */ /** @var WPFunctions */
private $wp; private $wp;
function __construct(WPFunctions $wp = null) { /** @var CaptchaSession */
private $captcha_session;
function __construct(WPFunctions $wp = null, CaptchaSession $captcha_session = null) {
if ($wp === null) { if ($wp === null) {
$wp = new WPFunctions; $wp = new WPFunctions;
} }
if ($captcha_session === null) {
$captcha_session = new CaptchaSession($wp, new Session());
}
$this->wp = $wp; $this->wp = $wp;
$this->captcha_session = $captcha_session;
} }
function isSupported() { function isSupported() {
@ -95,7 +100,7 @@ class Captcha {
->setMaxBehindLines(0) ->setMaxBehindLines(0)
->build($width ?: 220, $height ?: 60, $font); ->build($width ?: 220, $height ?: 60, $font);
$_SESSION[self::SESSION_KEY] = $builder->getPhrase(); $this->captcha_session->setCaptchaHash($builder->getPhrase());
if ($return) { if ($return) {
return $builder->get(); return $builder->get();

View File

@ -14,9 +14,13 @@ class CaptchaRenderer {
/** @var WPFunctions */ /** @var WPFunctions */
private $wp; private $wp;
function __construct(UrlHelper $url_helper, WPFunctions $wp) { /** @var CaptchaSession */
private $captcha_session;
function __construct(UrlHelper $url_helper, WPFunctions $wp, CaptchaSession $captcha_session) {
$this->url_helper = $url_helper; $this->url_helper = $url_helper;
$this->wp = $wp; $this->wp = $wp;
$this->captcha_session = $captcha_session;
} }
public function getCaptchaPageTitle() { public function getCaptchaPageTitle() {
@ -49,7 +53,8 @@ class CaptchaRenderer {
] ]
); );
$form_id = isset($_SESSION[Captcha::SESSION_FORM_KEY]['form_id']) ? (int)$_SESSION[Captcha::SESSION_FORM_KEY]['form_id'] : 0; $captcha_session_form = $this->captcha_session->getFormData();
$form_id = isset($captcha_session_form['form_id']) ? (int)$captcha_session_form['form_id'] : 0;
$form_model = FormModel::findOne($form_id); $form_model = FormModel::findOne($form_id);
if (!$form_model instanceof FormModel) { if (!$form_model instanceof FormModel) {
return false; return false;

View File

@ -5,6 +5,7 @@ use Carbon\Carbon;
use Codeception\Util\Fixtures; use Codeception\Util\Fixtures;
use MailPoet\API\JSON\v1\Subscribers; use MailPoet\API\JSON\v1\Subscribers;
use MailPoet\API\JSON\Response as APIResponse; use MailPoet\API\JSON\Response as APIResponse;
use MailPoet\Config\Session;
use MailPoet\DI\ContainerWrapper; use MailPoet\DI\ContainerWrapper;
use MailPoet\Form\Util\FieldNameObfuscator; use MailPoet\Form\Util\FieldNameObfuscator;
use MailPoet\Models\CustomField; use MailPoet\Models\CustomField;
@ -21,6 +22,8 @@ use MailPoet\Models\SubscriberSegment;
use MailPoet\Settings\SettingsController; use MailPoet\Settings\SettingsController;
use MailPoet\Subscribers\Source; use MailPoet\Subscribers\Source;
use MailPoet\Subscription\Captcha; use MailPoet\Subscription\Captcha;
use MailPoet\Subscription\CaptchaSession;
use MailPoet\WP\Functions;
class SubscribersTest extends \MailPoetTest { class SubscribersTest extends \MailPoetTest {
@ -77,6 +80,9 @@ class SubscribersTest extends \MailPoetTest {
'address' => 'sender@mailpoet.com', 'address' => 'sender@mailpoet.com',
'name' => 'Sender', 'name' => 'Sender',
]); ]);
// MAILPOET SESSION
$_COOKIE[Session::COOKIE_NAME] = 'abcd';
} }
function testItCanGetASubscriber() { function testItCanGetASubscriber() {
@ -517,7 +523,8 @@ class SubscribersTest extends \MailPoetTest {
$subscriber->count_confirmations = 1; $subscriber->count_confirmations = 1;
$subscriber->save(); $subscriber->save();
$captcha_value = 'ihg5w'; $captcha_value = 'ihg5w';
$_SESSION[Captcha::SESSION_KEY] = $captcha_value; $captcha_session = new CaptchaSession(new Functions(), new Session());
$captcha_session->setCaptchaHash($captcha_value);
$response = $this->endpoint->subscribe([ $response = $this->endpoint->subscribe([
$this->obfuscatedEmail => $email, $this->obfuscatedEmail => $email,
'form_id' => $this->form->id, 'form_id' => $this->form->id,

View File

@ -3,10 +3,13 @@ namespace MailPoet\Test\Subscription;
use Carbon\Carbon; use Carbon\Carbon;
use Codeception\Util\Fixtures; use Codeception\Util\Fixtures;
use MailPoet\Config\Session;
use MailPoet\Models\Subscriber; use MailPoet\Models\Subscriber;
use MailPoet\Models\SubscriberIP; use MailPoet\Models\SubscriberIP;
use MailPoet\Subscription\Captcha; use MailPoet\Subscription\Captcha;
use MailPoet\Subscription\CaptchaSession;
use MailPoet\WP\Functions as WPFunctions; use MailPoet\WP\Functions as WPFunctions;
use MailPoet\WP\Functions;
class CaptchaTest extends \MailPoetTest { class CaptchaTest extends \MailPoetTest {
function _before() { function _before() {
@ -40,10 +43,12 @@ class CaptchaTest extends \MailPoetTest {
} }
function testItRendersImage() { function testItRendersImage() {
expect_that(empty($_SESSION[Captcha::SESSION_KEY])); $_COOKIE[Session::COOKIE_NAME] = 'abcd';
$captcha_session = new CaptchaSession(new Functions(), new Session());
expect($captcha_session->getCaptchaHash())->false();
$image = $this->captcha->renderImage(null, null, true); $image = $this->captcha->renderImage(null, null, true);
expect($image)->notEmpty(); expect($image)->notEmpty();
expect_that(!empty($_SESSION[Captcha::SESSION_KEY])); expect($captcha_session->getCaptchaHash())->notEmpty();
} }
function _after() { function _after() {