diff --git a/lib/API/Endpoints/Subscribers.php b/lib/API/Endpoints/Subscribers.php index 915ac54a7c..972ce53124 100644 --- a/lib/API/Endpoints/Subscribers.php +++ b/lib/API/Endpoints/Subscribers.php @@ -73,7 +73,7 @@ class Subscribers extends APIEndpoint { if(empty($segment_ids)) { return $this->badRequest(array( - APIError::BAD_REQUEST => __('Please select a list', 'mailpoet') + APIError::BAD_REQUEST => __('Please select a list.', 'mailpoet') )); } diff --git a/lib/Config/Migrator.php b/lib/Config/Migrator.php index 04a6faee47..36c7395105 100644 --- a/lib/Config/Migrator.php +++ b/lib/Config/Migrator.php @@ -134,6 +134,9 @@ class Migrator { 'last_name tinytext NOT NULL DEFAULT "",', 'email varchar(150) NOT NULL,', 'status varchar(12) NOT NULL DEFAULT "' . Subscriber::STATUS_UNCONFIRMED . '",', + 'ip varchar(32) NULL,', + 'confirmed_ip varchar(32) NULL,', + 'confirmed_at TIMESTAMP NULL,', 'created_at TIMESTAMP NULL,', 'updated_at TIMESTAMP NOT NULL DEFAULT CURRENT_TIMESTAMP ON UPDATE CURRENT_TIMESTAMP,', 'deleted_at TIMESTAMP NULL,', diff --git a/lib/Models/Subscriber.php b/lib/Models/Subscriber.php index 14dddeb2a5..338468c7d1 100644 --- a/lib/Models/Subscriber.php +++ b/lib/Models/Subscriber.php @@ -14,6 +14,8 @@ class Subscriber extends Model { const STATUS_UNSUBSCRIBED = 'unsubscribed'; const STATUS_UNCONFIRMED = 'unconfirmed'; + const SUBSCRIPTION_LIMIT_COOLDOWN = 60; + function __construct() { parent::__construct(); @@ -158,9 +160,27 @@ class Subscriber extends Model { 'signup_confirmation.enabled' ); + // set user ip + $subscriber_data['ip'] = (isset($_SERVER['REMOTE_ADDR'])) + ? $_SERVER['REMOTE_ADDR'] + : null; + + // make sure we don't allow too many subscriptions with the same ip address + $subscription_count = Subscriber::where('ip', $subscriber_data['ip']) + ->whereRaw( + 'TIME_TO_SEC(TIMEDIFF(NOW(), created_at)) < ?', + self::SUBSCRIPTION_LIMIT_COOLDOWN + ) + ->count(); + + if($subscription_count > 0) { + throw new \Exception(__('You need to wait before subscribing again.', 'mailpoet')); + } + $subscriber = self::findOne($subscriber_data['email']); if($subscriber === false) { + // create new subscriber $subscriber = self::createOrUpdate($subscriber_data); if($subscriber->getErrors() !== false) { diff --git a/lib/Subscription/Pages.php b/lib/Subscription/Pages.php index 30b0385296..fc9bbed56a 100644 --- a/lib/Subscription/Pages.php +++ b/lib/Subscription/Pages.php @@ -58,7 +58,12 @@ class Pages { function confirm() { if($this->subscriber !== false) { + // update status $this->subscriber->status = Subscriber::STATUS_SUBSCRIBED; + // set confirmed ip & time + $this->subscriber->confirmed_ip = $_SERVER['REMOTE_ADDR']; + $this->subscriber->setExpr('confirmed_at', 'NOW()'); + $this->subscriber->save(); } } diff --git a/tests/unit/API/Endpoints/SubscribersTest.php b/tests/unit/API/Endpoints/SubscribersTest.php index edf562c3f6..e957df7493 100644 --- a/tests/unit/API/Endpoints/SubscribersTest.php +++ b/tests/unit/API/Endpoints/SubscribersTest.php @@ -4,6 +4,7 @@ use \MailPoet\API\Endpoints\Subscribers; use \MailPoet\API\Response as APIResponse; use \MailPoet\Models\Subscriber; use \MailPoet\Models\Segment; +use \MailPoet\Models\Setting; class SubscribersTest extends MailPoetTest { function _before() { @@ -26,6 +27,12 @@ class SubscribersTest extends MailPoetTest { $this->segment_2->id ) )); + + // setup mailer + Setting::setValue('sender', array( + 'address' => 'sender@mailpoet.com', + 'name' => 'Sender' + )); } function testItCanGetASubscriber() { @@ -354,6 +361,38 @@ class SubscribersTest extends MailPoetTest { expect($response->errors[0]['message'])->contains('has no method'); } + function testItCanSubscribeAUser() { + // set ip address + $_SERVER['REMOTE_ADDR'] = '127.0.0.1'; + + $router = new Subscribers(); + + // missing segment + $response = $router->subscribe(array( + 'email' => 'toto@mailpoet.com' + )); + + expect($response->status)->equals(APIResponse::STATUS_BAD_REQUEST); + expect($response->errors[0]['message'])->equals('Please select a list.'); + + // proper subscription + $response = $router->subscribe(array( + 'email' => 'toto@mailpoet.com', + 'segments' => array($this->segment_1->id, $this->segment_2->id) + )); + expect($response->status)->equals(APIResponse::STATUS_OK); + + try { + $response = $router->subscribe(array( + 'email' => 'tata@mailpoet.com', + 'segments' => array($this->segment_1->id, $this->segment_2->id) + )); + $this->fail('It should not be possible to subscribe a second time so soon'); + } catch(\Exception $e) { + expect($e->getMessage())->equals('You need to wait before subscribing again.'); + } + } + function _after() { Segment::deleteMany(); Subscriber::deleteMany();