From e5639d1adbadedb40a79c5d17ef59ca7afcec3bb Mon Sep 17 00:00:00 2001 From: Pavel Dohnal Date: Tue, 21 Apr 2020 13:42:08 +0200 Subject: [PATCH] Remove SPF notice [MAILPOET-2867] --- assets/js/src/common/check_spf_record.jsx | 23 -------------- .../newsletter_editor/components/sidebar.js | 4 --- .../settings/pages/send_with/send_with.tsx | 4 +-- lib/API/JSON/v1/Services.php | 22 ------------- lib/DI/ContainerConfigurator.php | 1 - lib/Services/SPFCheck.php | 31 ------------------- .../integration/API/JSON/v1/ServicesTest.php | 28 ----------------- tests/unit/Services/SPFCheckTest.php | 25 --------------- views/layout.html | 5 --- views/newsletter/editor.html | 1 - webpack.config.js | 6 ---- 11 files changed, 1 insertion(+), 149 deletions(-) delete mode 100644 assets/js/src/common/check_spf_record.jsx delete mode 100644 lib/Services/SPFCheck.php delete mode 100644 tests/unit/Services/SPFCheckTest.php diff --git a/assets/js/src/common/check_spf_record.jsx b/assets/js/src/common/check_spf_record.jsx deleted file mode 100644 index 1eacb8c299..0000000000 --- a/assets/js/src/common/check_spf_record.jsx +++ /dev/null @@ -1,23 +0,0 @@ -import MailPoet from 'mailpoet'; -import _ from 'underscore'; - -const getErrorMessage = (senderAddress) => `

${MailPoet.I18n.t('spfCheckTitle')}

-

${MailPoet.I18n.t('spfCheckMsgWhy').replace('%s', _.escape(senderAddress))}

-

${MailPoet.I18n.t('spfCheckMsgEdit').replace('%s', 'include:spf.sendingservice.net')}

-

${MailPoet.I18n.t('spfCheckReadMore')}

`; - -const checkSPFRecord = () => MailPoet.Ajax.post({ - api_version: window.mailpoet_api_version, - endpoint: 'services', - action: 'checkSPFRecord', - data: {}, -}).fail((response) => { - if (response.meta.sender_address) { - MailPoet.Notice.system( - getErrorMessage(response.meta.sender_address), - { static: true, scroll: true, id: 'spf_check_error' } - ); - } -}); - -export default checkSPFRecord; diff --git a/assets/js/src/newsletter_editor/components/sidebar.js b/assets/js/src/newsletter_editor/components/sidebar.js index dbad329d64..3f240b1db2 100644 --- a/assets/js/src/newsletter_editor/components/sidebar.js +++ b/assets/js/src/newsletter_editor/components/sidebar.js @@ -7,7 +7,6 @@ import Marionette from 'backbone.marionette'; import SuperModel from 'backbone.supermodel'; import _ from 'underscore'; import jQuery from 'jquery'; -import checkSPFRecord from 'common/check_spf_record.jsx'; var Module = {}; var SidebarView; @@ -348,9 +347,6 @@ Module.SidebarPreviewView = Marionette.View.extend({ 'MailPoet Free version': window.mailpoet_version, 'Domain name': data.subscriber.substring(data.subscriber.indexOf('@') + 1), }); - if (App.getConfig().get('validation.validateSPFRecord')) { - checkSPFRecord(); - } }).fail(function (response) { if (response.errors.length > 0) { MailPoet.Notice.error( diff --git a/assets/js/src/settings/pages/send_with/send_with.tsx b/assets/js/src/settings/pages/send_with/send_with.tsx index 1e292d17ce..9841886fff 100644 --- a/assets/js/src/settings/pages/send_with/send_with.tsx +++ b/assets/js/src/settings/pages/send_with/send_with.tsx @@ -6,7 +6,6 @@ import { useSelector, useAction, useSetting } from 'settings/store/hooks'; import { MssStatus } from 'settings/store/types'; import { t } from 'common/functions'; import { Link } from 'react-router-dom'; -import checkSPFRecord from 'common/check_spf_record'; export default function SendWith() { const isNewUser = useSelector('isNewUser')(); @@ -23,8 +22,7 @@ export default function SendWith() { await setSetting(['mta', 'method'], 'MailPoet'); await setSetting(['mta', 'mailpoet_api_key'], key); await setSetting(['signup_confirmation', 'enabled'], '1'); - await saveSettings(); - return checkSPFRecord(); + return saveSettings(); }; return ( diff --git a/lib/API/JSON/v1/Services.php b/lib/API/JSON/v1/Services.php index 21f0ea0cc2..2e30000c94 100644 --- a/lib/API/JSON/v1/Services.php +++ b/lib/API/JSON/v1/Services.php @@ -14,7 +14,6 @@ use MailPoet\Cron\Workers\KeyCheck\SendingServiceKeyCheck; use MailPoet\Mailer\MailerLog; use MailPoet\Services\Bridge; use MailPoet\Services\CongratulatoryMssEmailController; -use MailPoet\Services\SPFCheck; use MailPoet\Settings\SettingsController; use MailPoet\WP\DateTime; use MailPoet\WP\Functions as WPFunctions; @@ -29,9 +28,6 @@ class Services extends APIEndpoint { /** @var AnalyticsHelper */ private $analytics; - /** @var SPFCheck */ - private $spfCheck; - /** @var DateTime */ public $dateTime; @@ -58,7 +54,6 @@ class Services extends APIEndpoint { Bridge $bridge, SettingsController $settings, AnalyticsHelper $analytics, - SPFCheck $spfCheck, SendingServiceKeyCheck $mssWorker, PremiumKeyCheck $premiumWorker, ServicesChecker $servicesChecker, @@ -68,7 +63,6 @@ class Services extends APIEndpoint { $this->bridge = $bridge; $this->settings = $settings; $this->analytics = $analytics; - $this->spfCheck = $spfCheck; $this->mssWorker = $mssWorker; $this->premiumWorker = $premiumWorker; $this->dateTime = new DateTime(); @@ -77,22 +71,6 @@ class Services extends APIEndpoint { $this->wp = $wp; } - public function checkSPFRecord($data = []) { - $senderAddress = $this->settings->get('sender.address'); - $domainName = mb_substr($senderAddress, mb_strpos($senderAddress, '@') + 1); - - $result = $this->spfCheck->checkSPFRecord($domainName); - - if (!$result) { - return $this->errorResponse( - [APIError::BAD_REQUEST => $this->wp->__('SPF check has failed.', 'mailpoet')], - ['sender_address' => $senderAddress] - ); - } - - return $this->successResponse(); - } - public function checkMSSKey($data = []) { $key = isset($data['key']) ? trim($data['key']) : null; diff --git a/lib/DI/ContainerConfigurator.php b/lib/DI/ContainerConfigurator.php index eb9006cbf4..63593bd319 100644 --- a/lib/DI/ContainerConfigurator.php +++ b/lib/DI/ContainerConfigurator.php @@ -239,7 +239,6 @@ class ContainerConfigurator implements IContainerConfigurator { $container->autowire(\MailPoet\Services\Bridge::class)->setPublic(true); $container->autowire(\MailPoet\Services\AuthorizedEmailsController::class); $container->autowire(\MailPoet\Services\CongratulatoryMssEmailController::class); - $container->autowire(\MailPoet\Services\SPFCheck::class)->setPublic(true); // Tasks $container->autowire(\MailPoet\Tasks\State::class); // Settings diff --git a/lib/Services/SPFCheck.php b/lib/Services/SPFCheck.php deleted file mode 100644 index 314587f614..0000000000 --- a/lib/Services/SPFCheck.php +++ /dev/null @@ -1,31 +0,0 @@ -getSPFRecord($domain); - if (empty($record)) { - return true; - } - return strpos($record, 'include:spf.sendingservice.net') !== false; - } - - private function getSPFRecord($domain) { - $records = $this->dnsGetRecord($domain, DNS_TXT); - if (empty($records[0])) { - return false; - } - foreach ($records as $record) { - if (empty($record['txt']) || !preg_match('/^v=spf1/', trim($record['txt']))) { - continue; - } - return $record['txt']; - } - return false; - } - - protected function dnsGetRecord($domain, $type) { - return dns_get_record($domain, $type); - } -} diff --git a/tests/integration/API/JSON/v1/ServicesTest.php b/tests/integration/API/JSON/v1/ServicesTest.php index fb238fe50a..a08c3289f5 100644 --- a/tests/integration/API/JSON/v1/ServicesTest.php +++ b/tests/integration/API/JSON/v1/ServicesTest.php @@ -14,7 +14,6 @@ use MailPoet\Mailer\Mailer; use MailPoet\Mailer\MailerLog; use MailPoet\Services\Bridge; use MailPoet\Services\CongratulatoryMssEmailController; -use MailPoet\Services\SPFCheck; use MailPoet\Settings\SettingsController; use MailPoet\Settings\SettingsRepository; use MailPoet\WP\Functions as WPFunctions; @@ -32,32 +31,6 @@ class ServicesTest extends \MailPoetTest { $this->settings = SettingsController::getInstance(); } - public function testItRespondsWithErrorIfSPFCheckFails() { - $email = 'spf_test@example.com'; - $this->settings->set('sender.address', $email); - - $spfCheck = $this->make( - SPFCheck::class, - ['checkSPFRecord' => false] - ); - - $servicesEndpoint = $this->createServicesEndpointWithMocks(['spfCheck' => $spfCheck]); - $response = $servicesEndpoint->checkSPFRecord([]); - expect($response->status)->equals(APIResponse::STATUS_NOT_FOUND); - expect($response->meta['sender_address'])->equals($email); - } - - public function testItRespondsWithSuccessIfSPFCheckPasses() { - $spfCheck = $this->make( - SPFCheck::class, - ['checkSPFRecord' => true] - ); - - $servicesEndpoint = $this->createServicesEndpointWithMocks(['spfCheck' => $spfCheck]); - $response = $servicesEndpoint->checkSPFRecord([]); - expect($response->status)->equals(APIResponse::STATUS_OK); - } - public function testItRespondsWithErrorIfNoMSSKeyIsGiven() { $response = $this->diContainer->get(Services::class)->checkMSSKey(['key' => '']); expect($response->status)->equals(APIResponse::STATUS_BAD_REQUEST); @@ -572,7 +545,6 @@ class ServicesTest extends \MailPoetTest { $mocks['bridge'] ?? $this->diContainer->get(Bridge::class), $this->diContainer->get(SettingsController::class), $this->diContainer->get(Analytics::class), - $mocks['spfCheck'] ?? $this->diContainer->get(SPFCheck::class), $this->diContainer->get(SendingServiceKeyCheck::class), $this->diContainer->get(PremiumKeyCheck::class), $this->diContainer->get(ServicesChecker::class), diff --git a/tests/unit/Services/SPFCheckTest.php b/tests/unit/Services/SPFCheckTest.php deleted file mode 100644 index 3149ae1a75..0000000000 --- a/tests/unit/Services/SPFCheckTest.php +++ /dev/null @@ -1,25 +0,0 @@ -make(SPFCheck::class, ['dnsGetRecord' => $response]); - expect($check->checkSPFRecord($domain))->equals(true); - // No SPF record - $response = [['txt' => '123'], ['txt' => 'abc']]; - $check = $this->make(SPFCheck::class, ['dnsGetRecord' => $response]); - expect($check->checkSPFRecord($domain))->equals(true); - // Good SPF record - $response = [['txt' => 'v=spf1 include:spf.protection.outlook.com include:sendgrid.net include:spf.sendingservice.net -all']]; - $check = $this->make(SPFCheck::class, ['dnsGetRecord' => $response]); - expect($check->checkSPFRecord($domain))->equals(true); - // Bad SPF record - $response = [['txt' => 'v=spf1 include:spf.protection.outlook.com include:sendgrid.net -all']]; - $check = $this->make(SPFCheck::class, ['dnsGetRecord' => $response]); - expect($check->checkSPFRecord($domain))->equals(false); - } -} diff --git a/views/layout.html b/views/layout.html index f33b4f02c5..413be1d326 100644 --- a/views/layout.html +++ b/views/layout.html @@ -81,11 +81,6 @@ jQuery('.toplevel_page_mailpoet-newsletters.menu-top-last') 'mailerSendingResumedNotice': __('Sending has been resumed.'), 'dismissNotice': __('Dismiss this notice.'), - 'spfCheckTitle': _x('Improve your deliverability!', 'DNS SPF Record check'), - 'spfCheckMsgWhy': _x("Your email is set to be sent from %s and we noticed that you have an SPF record for this domain. It means some subscribers may not receive your emails.", 'DNS SPF Record check'), - 'spfCheckMsgEdit': _x("Since you're sending with the MailPoet Sending Service, you need to add %s to the existing SPF entry in your DNS records. This will allow MailPoet to send on your behalf for optimal deliverability.", 'DNS SPF Record check'), - 'spfCheckReadMore': _x('Read the Guide', 'DNS SPF Record check'), - 'subscribersLimitNoticeTitle': __('Congratulations, you now have more than [subscribersLimit] subscribers!'), 'freeVersionLimit': __('Our free version is limited to [subscribersLimit] subscribers.'), 'yourPlanLimit': __('Your plan is limited to [subscribersLimit] subscribers.'), diff --git a/views/newsletter/editor.html b/views/newsletter/editor.html index bf302b98e6..20e4285132 100644 --- a/views/newsletter/editor.html +++ b/views/newsletter/editor.html @@ -1463,7 +1463,6 @@ }, validation: { validateUnsubscribeLinkPresent: <%= mss_active and is_wc_transactional_email != true ? 'true' : 'false' %>, - validateSPFRecord: <%= mss_active ? 'true' : 'false' %>, }, urls: { send: '<%= admin_url('admin.php?page=mailpoet-newsletters#/send/' ~ (params('id') | intval)) %>', diff --git a/webpack.config.js b/webpack.config.js index 295b2edbd5..65513a3595 100644 --- a/webpack.config.js +++ b/webpack.config.js @@ -110,11 +110,6 @@ const baseConfig = { include: path.resolve(__dirname, 'assets/js/src/hooks.js'), use: 'expose-loader?' + globalPrefix + '.Hooks', }, - { - // Expose for usage in the settings view inline JS - include: path.resolve(__dirname, 'assets/js/src/common/check_spf_record.jsx'), - use: 'expose-loader?' + globalPrefix + '.checkSPFRecord', - }, { test: /listing.jsx/i, use: [ @@ -203,7 +198,6 @@ const adminConfig = { 'help-tooltip.jsx', 'listing/listing.jsx', 'newsletters/badges/stats.jsx', - 'common/check_spf_record.jsx', ], admin: 'webpack_admin_index.jsx', newsletter_editor: 'newsletter_editor/webpack_index.jsx',