diff --git a/lib/Cron/Workers/SendingQueue/Tasks/Links.php b/lib/Cron/Workers/SendingQueue/Tasks/Links.php index 0ec700feb7..7c4976049a 100644 --- a/lib/Cron/Workers/SendingQueue/Tasks/Links.php +++ b/lib/Cron/Workers/SendingQueue/Tasks/Links.php @@ -47,7 +47,7 @@ class Links { } $data = NewsletterLinks::createUrlDataObject( $subscriber->id, - $subscriber->email, + $subscriber->getLinkToken(), $queue->id, $link_hash->hash, false diff --git a/lib/Models/Subscriber.php b/lib/Models/Subscriber.php index 10e567db1e..255eb8146e 100644 --- a/lib/Models/Subscriber.php +++ b/lib/Models/Subscriber.php @@ -20,6 +20,7 @@ use MailPoet\Util\Security; * @property string|null $last_subscribed_at * @property string|null $deleted_at * @property string|null $source + * @property string|null $link_token * @property int $count_confirmations * @property int $wp_user_id * @property array $segments @@ -102,6 +103,14 @@ class Subscriber extends Model { return (bool)$this->is_woocommerce_user; } + function getLinkToken() { + if ($this->link_token === null) { + $this->link_token = self::generateToken($this->email); + $this->save(); + } + return $this->link_token; + } + static function getCurrentWPUser() { $wp_user = WPFunctions::get()->wpGetCurrentUser(); if (empty($wp_user->ID)) { @@ -121,10 +130,10 @@ class Subscriber extends Model { return false; } - static function verifyToken($email, $token) { + function verifyToken($token) { return call_user_func( 'hash_equals', - self::generateToken($email, strlen($token)), + $this->getLinkToken(), $token ); } diff --git a/lib/Newsletter/Links/Links.php b/lib/Newsletter/Links/Links.php index ca5b2ca804..ac7624be8b 100644 --- a/lib/Newsletter/Links/Links.php +++ b/lib/Newsletter/Links/Links.php @@ -136,7 +136,7 @@ class Links { } $data = self::createUrlDataObject( $subscriber->id, - $subscriber->email, + $subscriber->getLinkToken(), $queue_id, $hash, $preview @@ -197,11 +197,11 @@ class Links { } static function createUrlDataObject( - $subscriber_id, $subscriber_email, $queue_id, $link_hash, $preview + $subscriber_id, $subscriber_link_token, $queue_id, $link_hash, $preview ) { return [ $subscriber_id, - Subscriber::generateToken($subscriber_email), + $subscriber_link_token, $queue_id, $link_hash, $preview, diff --git a/lib/Newsletter/Url.php b/lib/Newsletter/Url.php index 0c8a356a8c..87a242d8c8 100644 --- a/lib/Newsletter/Url.php +++ b/lib/Newsletter/Url.php @@ -17,7 +17,7 @@ class Url { $preview = false ) { if ($subscriber instanceof SubscriberModel) { - $subscriber->token = SubscriberModel::generateToken($subscriber->email); + $subscriber->token = $subscriber->getLinkToken(); } switch ($type) { case self::TYPE_ARCHIVE: diff --git a/lib/Router/Endpoints/Track.php b/lib/Router/Endpoints/Track.php index 4827649ed2..5a403a7476 100644 --- a/lib/Router/Endpoints/Track.php +++ b/lib/Router/Endpoints/Track.php @@ -70,8 +70,7 @@ class Track { function _validateTrackData($data) { if (!$data->subscriber || !$data->queue || !$data->newsletter) return false; - $subscriber_token_match = - Subscriber::verifyToken($data->subscriber->email, $data->subscriber_token); + $subscriber_token_match = $data->subscriber->verifyToken($data->subscriber_token); if (!$subscriber_token_match) { $this->terminate(403); } diff --git a/lib/Router/Endpoints/ViewInBrowser.php b/lib/Router/Endpoints/ViewInBrowser.php index 6fb94a7500..4283d84ac0 100644 --- a/lib/Router/Endpoints/ViewInBrowser.php +++ b/lib/Router/Endpoints/ViewInBrowser.php @@ -60,7 +60,7 @@ class ViewInBrowser { false; if ($data->subscriber) { if (empty($data->subscriber_token) || - !Subscriber::verifyToken($data->subscriber->email, $data->subscriber_token) + !$data->subscriber->verifyToken($data->subscriber_token) ) return false; } else if (!$data->subscriber && !empty($data->preview)) { // if this is a preview and subscriber does not exist, diff --git a/lib/Subscription/Manage.php b/lib/Subscription/Manage.php index 876520fb65..c03c9d70af 100644 --- a/lib/Subscription/Manage.php +++ b/lib/Subscription/Manage.php @@ -30,10 +30,13 @@ class Manage { $subscriber_data = $_POST['data']; $subscriber_data = $this->field_name_obfuscator->deobfuscateFormPayload($subscriber_data); - if (!empty($subscriber_data['email']) && Subscriber::verifyToken($subscriber_data['email'], $token)) { - if ($subscriber_data['email'] !== Pages::DEMO_EMAIL) { - $subscriber = Subscriber::createOrUpdate($this->filterOutEmptyMandatoryFields($subscriber_data)); - $subscriber->getErrors(); + if (!empty($subscriber_data['email'])) { + $subscriber = Subscriber::where('email', $subscriber_data['email'])->findOne(); + if ($subscriber && $subscriber->verifyToken($token)) { + if ($subscriber_data['email'] !== Pages::DEMO_EMAIL) { + $subscriber = Subscriber::createOrUpdate($this->filterOutEmptyMandatoryFields($subscriber_data)); + $subscriber->getErrors(); + } } } diff --git a/lib/Subscription/Pages.php b/lib/Subscription/Pages.php index 2de5e398d0..e50c11b50e 100644 --- a/lib/Subscription/Pages.php +++ b/lib/Subscription/Pages.php @@ -103,9 +103,8 @@ class Pages { return false; } - return (Subscriber::verifyToken($email, $token)) ? - Subscriber::findOne($email) : - false; + $subscriber = Subscriber::where('email', $email)->findOne(); + return ($subscriber && $subscriber->verifyToken($token)) ? $subscriber : false; } function confirm() { @@ -283,6 +282,7 @@ class Pages { 'email' => self::DEMO_EMAIL, 'first_name' => 'John', 'last_name' => 'Doe', + 'link_token' => Subscriber::generateToken(self::DEMO_EMAIL), ]); } else if ($this->subscriber !== false) { $subscriber = $this->subscriber @@ -438,7 +438,7 @@ class Pages { $subscriber->email . '" />'; $form_html .= ''; $form_html .= '
'; diff --git a/lib/Subscription/Url.php b/lib/Subscription/Url.php index 00f35f7c01..fe17bfe2df 100644 --- a/lib/Subscription/Url.php +++ b/lib/Subscription/Url.php @@ -46,7 +46,7 @@ class Url { if ($subscriber !== null) { $data = [ - 'token' => Subscriber::generateToken($subscriber->email), + 'token' => $subscriber->getLinkToken(), 'email' => $subscriber->email, ]; } elseif (is_null($data)) { diff --git a/tests/integration/Cron/Workers/SendingQueue/SendingQueueTest.php b/tests/integration/Cron/Workers/SendingQueue/SendingQueueTest.php index 791ad13ceb..e5234a125b 100644 --- a/tests/integration/Cron/Workers/SendingQueue/SendingQueueTest.php +++ b/tests/integration/Cron/Workers/SendingQueue/SendingQueueTest.php @@ -99,7 +99,7 @@ class SendingQueueTest extends \MailPoetTest { private function getTrackedUnsubscribeURL() { $data = Links::createUrlDataObject( $this->subscriber->id, - $this->subscriber->email, + $this->subscriber->getLinkToken(), $this->queue->id, $this->newsletter_link->hash, false diff --git a/tests/integration/Models/SubscriberTest.php b/tests/integration/Models/SubscriberTest.php index 779a823ebd..8608af7b63 100644 --- a/tests/integration/Models/SubscriberTest.php +++ b/tests/integration/Models/SubscriberTest.php @@ -678,14 +678,12 @@ class SubscriberTest extends \MailPoetTest { } function testItVerifiesSubscriberToken() { - $token = Subscriber::generateToken($this->test_data['email']); - expect(Subscriber::verifyToken($this->test_data['email'], $token))->true(); - expect(Subscriber::verifyToken('fake@email.com', $token))->false(); - } - - function testItVerifiesTokensOfDifferentLengths() { - $token = Subscriber::generateToken($this->test_data['email'], 6); - expect(Subscriber::verifyToken($this->test_data['email'], $token))->true(); + $subscriber = Subscriber::createOrUpdate([ + 'email' => $this->test_data['email'], + ]); + $token = $subscriber->getLinkToken(); + expect($subscriber->verifyToken($token))->true(); + expect($subscriber->verifyToken('faketoken'))->false(); } function testItBulkDeletesSubscribers() { diff --git a/tests/integration/Newsletter/Links/LinksTest.php b/tests/integration/Newsletter/Links/LinksTest.php index 7c51f831c6..5538d5d7ea 100644 --- a/tests/integration/Newsletter/Links/LinksTest.php +++ b/tests/integration/Newsletter/Links/LinksTest.php @@ -126,7 +126,7 @@ class LinksTest extends \MailPoetTest { ]; $url_data_object = Links::createUrlDataObject( $data['subscriber_id'], - $subscriber_email, + $data['subscriber_token'], $data['queue_id'], $data['link_hash'], $data['preview'] diff --git a/tests/integration/Router/Endpoints/TrackTest.php b/tests/integration/Router/Endpoints/TrackTest.php index 5029e1ba25..fa18b7fdec 100644 --- a/tests/integration/Router/Endpoints/TrackTest.php +++ b/tests/integration/Router/Endpoints/TrackTest.php @@ -46,7 +46,7 @@ class TrackTest extends \MailPoetTest { 'queue_id' => $queue->id, 'subscriber_id' => $subscriber->id, 'newsletter_id' => $newsletter->id, - 'subscriber_token' => Subscriber::generateToken($subscriber->email), + 'subscriber_token' => $subscriber->getLinkToken(), 'link_hash' => $link->hash, 'preview' => false, ]; diff --git a/tests/integration/Router/Endpoints/ViewInBrowserTest.php b/tests/integration/Router/Endpoints/ViewInBrowserTest.php index 4f16bf9e1f..3e8469ef05 100644 --- a/tests/integration/Router/Endpoints/ViewInBrowserTest.php +++ b/tests/integration/Router/Endpoints/ViewInBrowserTest.php @@ -38,7 +38,7 @@ class ViewInBrowserTest extends \MailPoetTest { 'queue_id' => $queue->id, 'subscriber_id' => $subscriber->id, 'newsletter_id' => $newsletter->id, - 'subscriber_token' => Subscriber::generateToken($subscriber->email), + 'subscriber_token' => $subscriber->getLinkToken(), 'preview' => false, ]; // instantiate class diff --git a/tests/integration/Statistics/Track/ClicksTest.php b/tests/integration/Statistics/Track/ClicksTest.php index 07a18a463a..7d31055df5 100644 --- a/tests/integration/Statistics/Track/ClicksTest.php +++ b/tests/integration/Statistics/Track/ClicksTest.php @@ -53,7 +53,7 @@ class ClicksTest extends \MailPoetTest { 'queue' => $queue, 'subscriber' => $subscriber, 'newsletter' => $newsletter, - 'subscriber_token' => Subscriber::generateToken($subscriber->email), + 'subscriber_token' => $subscriber->getLinkToken(), 'link' => $link, 'preview' => false, ]; diff --git a/tests/integration/Statistics/Track/OpensTest.php b/tests/integration/Statistics/Track/OpensTest.php index 7de87b5823..d6aaeec61c 100644 --- a/tests/integration/Statistics/Track/OpensTest.php +++ b/tests/integration/Statistics/Track/OpensTest.php @@ -35,7 +35,7 @@ class OpensTest extends \MailPoetTest { 'queue' => $queue, 'subscriber' => $subscriber, 'newsletter' => $newsletter, - 'subscriber_token' => Subscriber::generateToken($subscriber->email), + 'subscriber_token' => $subscriber->getLinkToken(), 'preview' => false, ]; // instantiate class diff --git a/tests/integration/Subscription/PagesTest.php b/tests/integration/Subscription/PagesTest.php index 2c7e81b95e..b82a95b308 100644 --- a/tests/integration/Subscription/PagesTest.php +++ b/tests/integration/Subscription/PagesTest.php @@ -30,7 +30,7 @@ class PagesTest extends \MailPoetTest { $this->subscriber->save(); expect($this->subscriber->getErrors())->false(); $this->test_data['email'] = $this->subscriber->email; - $this->test_data['token'] = Subscriber::generateToken($this->subscriber->email); + $this->test_data['token'] = $this->subscriber->getLinkToken(); $this->pages = ContainerWrapper::getInstance()->get(Pages::class); }