diff --git a/lib/Config/Session.php b/lib/Config/Session.php index e1aeb6b81c..efa4c57a07 100644 --- a/lib/Config/Session.php +++ b/lib/Config/Session.php @@ -2,6 +2,7 @@ namespace MailPoet\Config; +use MailPoet\Util\Cookies; use MailPoet\Util\Security; class Session @@ -10,30 +11,41 @@ class Session const KEY_LENGTH = 32; const COOKIE_EXPIRATION = 84600; // day + /** @var Cookies */ + private $cookies; + + function __construct(Cookies $cookies) { + $this->cookies = $cookies; + } + function getId() { - return isset($_COOKIE[self::COOKIE_NAME]) ? $_COOKIE[self::COOKIE_NAME] : null; + return $this->cookies->get(self::COOKIE_NAME) ?: null; } function init() { - $id = $this->getId() ?: Security::generateRandomString(self::KEY_LENGTH); - if (!headers_sent()) { - $this->setCookie($id); + if (headers_sent()) { + return false; } + $id = $this->getId() ?: Security::generateRandomString(self::KEY_LENGTH); + $this->setCookie($id); + return true; } function destroy() { if ($this->getId() === null) { return; } - unset($_COOKIE[self::COOKIE_NAME]); + $this->cookies->delete(self::COOKIE_NAME); } private function setCookie($id) { - setcookie( + $this->cookies->set( self::COOKIE_NAME, $id, - time() + self::COOKIE_EXPIRATION, - "/" + [ + 'expires' => time() + self::COOKIE_EXPIRATION, + 'path' => '/', + ] ); } } diff --git a/lib/Subscription/Captcha.php b/lib/Subscription/Captcha.php index c2c754dc35..5e3e2e3dbd 100644 --- a/lib/Subscription/Captcha.php +++ b/lib/Subscription/Captcha.php @@ -5,6 +5,7 @@ namespace MailPoet\Subscription; use MailPoet\Config\Session; use MailPoet\Models\Subscriber; use MailPoet\Models\SubscriberIP; +use MailPoet\Util\Cookies; use MailPoet\Util\Helpers; use MailPoet\WP\Functions as WPFunctions; use MailPoetVendor\Gregwar\Captcha\CaptchaBuilder; @@ -25,7 +26,7 @@ class Captcha { $wp = new WPFunctions; } if ($captcha_session === null) { - $captcha_session = new CaptchaSession($wp, new Session()); + $captcha_session = new CaptchaSession($wp, new Session(new Cookies())); } $this->wp = $wp; $this->captcha_session = $captcha_session; diff --git a/lib/Util/Cookies.php b/lib/Util/Cookies.php index 5bf50a23e4..e3705bfee6 100644 --- a/lib/Util/Cookies.php +++ b/lib/Util/Cookies.php @@ -44,4 +44,8 @@ class Cookies { } return $value; } + + function delete($name) { + unset($_COOKIE[$name]); + } } diff --git a/tests/integration/API/JSON/v1/SubscribersTest.php b/tests/integration/API/JSON/v1/SubscribersTest.php index 424e881156..5d0327ebfa 100644 --- a/tests/integration/API/JSON/v1/SubscribersTest.php +++ b/tests/integration/API/JSON/v1/SubscribersTest.php @@ -8,6 +8,8 @@ use MailPoet\API\JSON\Response as APIResponse; use MailPoet\Config\Session; use MailPoet\DI\ContainerWrapper; use MailPoet\Form\Util\FieldNameObfuscator; +use MailPoet\Listing\BulkActionController; +use MailPoet\Listing\Handler; use MailPoet\Models\CustomField; use MailPoet\Models\Form; use MailPoet\Models\Newsletter; @@ -19,10 +21,14 @@ use MailPoet\Models\SubscriberIP; use MailPoet\Models\Segment; use MailPoet\Models\Setting; use MailPoet\Models\SubscriberSegment; +use MailPoet\Segments\SubscribersListings; use MailPoet\Settings\SettingsController; +use MailPoet\Subscribers\RequiredCustomFieldValidator; use MailPoet\Subscribers\Source; +use MailPoet\Subscribers\SubscriberActions; use MailPoet\Subscription\Captcha; use MailPoet\Subscription\CaptchaSession; +use MailPoet\Util\Cookies; use MailPoet\WP\Functions; class SubscribersTest extends \MailPoetTest { @@ -33,10 +39,29 @@ class SubscribersTest extends \MailPoetTest { /** @var SettingsController */ private $settings; + /** @var CaptchaSession */ + private $captcha_session; + function _before() { parent::_before(); $this->cleanup(); - $this->endpoint = ContainerWrapper::getInstance()->get(Subscribers::class); + $cookies_mock = $this->createMock(Cookies::class); + $cookies_mock->method('get') + ->willReturn('abcd'); + $session = new Session($cookies_mock); + $container = ContainerWrapper::getInstance(); + $this->captcha_session = new CaptchaSession($container->get(Functions::class), $session); + $this->endpoint = new Subscribers( + $container->get(BulkActionController::class), + $container->get(SubscribersListings::class), + $container->get(SubscriberActions::class), + $container->get(RequiredCustomFieldValidator::class), + $container->get(Handler::class), + $container->get(Captcha::class), + $container->get(Functions::class), + $container->get(SettingsController::class), + $this->captcha_session + ); $obfuscator = new FieldNameObfuscator(); $this->obfuscatedEmail = $obfuscator->obfuscate('email'); $this->obfuscatedSegments = $obfuscator->obfuscate('segments'); @@ -80,9 +105,6 @@ class SubscribersTest extends \MailPoetTest { 'address' => 'sender@mailpoet.com', 'name' => 'Sender', ]); - - // MAILPOET SESSION - $_COOKIE[Session::COOKIE_NAME] = 'abcd'; } function testItCanGetASubscriber() { @@ -523,8 +545,7 @@ class SubscribersTest extends \MailPoetTest { $subscriber->count_confirmations = 1; $subscriber->save(); $captcha_value = 'ihg5w'; - $captcha_session = new CaptchaSession(new Functions(), new Session()); - $captcha_session->setCaptchaHash($captcha_value); + $this->captcha_session->setCaptchaHash('ihg5w'); $response = $this->endpoint->subscribe([ $this->obfuscatedEmail => $email, 'form_id' => $this->form->id, diff --git a/tests/integration/Subscription/CaptchaTest.php b/tests/integration/Subscription/CaptchaTest.php index d7023a8a1c..4b5a13403e 100644 --- a/tests/integration/Subscription/CaptchaTest.php +++ b/tests/integration/Subscription/CaptchaTest.php @@ -8,13 +8,25 @@ use MailPoet\Models\Subscriber; use MailPoet\Models\SubscriberIP; use MailPoet\Subscription\Captcha; use MailPoet\Subscription\CaptchaSession; +use MailPoet\Util\Cookies; use MailPoet\WP\Functions as WPFunctions; use MailPoet\WP\Functions; class CaptchaTest extends \MailPoetTest { + + /** @var Captcha */ + private $captcha; + + /** @var CaptchaSession */ + private $captcha_session; + function _before() { - $this->captcha = new Captcha(new WPFunctions); + $cookies_mock = $this->createMock(Cookies::class); + $cookies_mock->method('get')->willReturn('abcd'); + $this->captcha_session = new CaptchaSession(new Functions(), new Session($cookies_mock)); + $this->captcha = new Captcha(new WPFunctions, $this->captcha_session); $_SERVER['REMOTE_ADDR'] = '127.0.0.1'; + $this->captcha_session->reset(); } function testItDoesNotRequireCaptchaForTheFirstSubscription() { @@ -42,13 +54,11 @@ class CaptchaTest extends \MailPoetTest { expect($result)->equals(true); } - function testItRendersImage() { - $_COOKIE[Session::COOKIE_NAME] = 'abcd'; - $captcha_session = new CaptchaSession(new Functions(), new Session()); - expect($captcha_session->getCaptchaHash())->false(); + function testItRendersImageAndStoresHashToSession() { + expect($this->captcha_session->getCaptchaHash())->false(); $image = $this->captcha->renderImage(null, null, true); expect($image)->notEmpty(); - expect($captcha_session->getCaptchaHash())->notEmpty(); + expect($this->captcha_session->getCaptchaHash())->notEmpty(); } function _after() { diff --git a/tests/unit/Config/SessionTest.php b/tests/unit/Config/SessionTest.php new file mode 100644 index 0000000000..68faf25c8a --- /dev/null +++ b/tests/unit/Config/SessionTest.php @@ -0,0 +1,79 @@ +cookies_mock = $this->createMock(Cookies::class); + $this->session = new Session($this->cookies_mock); + } + + function testItInitializesNewSessionCorrectly() { + $this->cookies_mock + ->expects($this->once()) + ->method('get') + ->willReturn(null); + $this->cookies_mock + ->expects($this->once()) + ->method('set') + ->with( + $this->equalTo(Session::COOKIE_NAME), + $this->isType('string'), + $this->callback(function ($options) { + if (!isset($options['expires']) || $options['expires'] < time()) { + return false; + } + if ($options['path'] !== '/') { + return false; + } + return true; + }) + ); + $this->session->init(); + } + + function testItPrologsCurrentSessionDuringInitialization() { + $session_id = 'abcd'; + $this->cookies_mock + ->expects($this->once()) + ->method('get') + ->willReturn($session_id); + $this->cookies_mock + ->expects($this->once()) + ->method('set') + ->with( + $this->equalTo(Session::COOKIE_NAME), + $this->equalTo($session_id), + $this->callback(function ($options) { + if (!isset($options['expires']) || $options['expires'] < time()) { + return false; + } + if ($options['path'] !== '/') { + return false; + } + return true; + }) + ); + $this->session->init(); + } + + function testItReturnsSessionIdCorrectly() { + $session_id = 'abcd'; + $this->cookies_mock + ->expects($this->once()) + ->method('get') + ->willReturn($session_id); + expect($this->session->getid())->equals($session_id); + } +}