diff --git a/CRM/Admin/Form/Setting/Mail.php b/CRM/Admin/Form/Setting/Mail.php index 4b49514c34a4..358c87f81fcf 100644 --- a/CRM/Admin/Form/Setting/Mail.php +++ b/CRM/Admin/Form/Setting/Mail.php @@ -29,6 +29,7 @@ class CRM_Admin_Form_Setting_Mail extends CRM_Admin_Form_Setting { // dev/core#1768 Make this interval configurable. 'civimail_sync_interval' => CRM_Core_BAO_Setting::MAILING_PREFERENCES_NAME, 'replyTo' => CRM_Core_BAO_Setting::MAILING_PREFERENCES_NAME, + 'civimail_unsubscribe_methods' => CRM_Core_BAO_Setting::MAILING_PREFERENCES_NAME, ]; /** diff --git a/CRM/Mailing/Form/Unsubscribe.php b/CRM/Mailing/Form/Unsubscribe.php index 36fb63a1525e..bf7024c1af17 100644 --- a/CRM/Mailing/Form/Unsubscribe.php +++ b/CRM/Mailing/Form/Unsubscribe.php @@ -60,7 +60,7 @@ public function preProcess() { throw new CRM_Core_Exception(ts("There was an error in your request")); } - list($displayName, $email) = CRM_Mailing_Event_BAO_MailingEventQueue::getContactInfo($queue_id); + [$displayName, $email] = CRM_Mailing_Event_BAO_MailingEventQueue::getContactInfo($queue_id); $this->assign('display_name', $displayName); $emailMasked = CRM_Utils_String::maskEmail($email); $this->assign('email_masked', $emailMasked); diff --git a/CRM/Mailing/Page/Unsubscribe.php b/CRM/Mailing/Page/Unsubscribe.php index 349ee59d2a7a..218fe4c82873 100644 --- a/CRM/Mailing/Page/Unsubscribe.php +++ b/CRM/Mailing/Page/Unsubscribe.php @@ -14,7 +14,7 @@ * @package CRM * @copyright CiviCRM LLC https://civicrm.org/licensing */ -class CRM_Mailing_Page_Unsubscribe extends CRM_Mailing_Page_Common { +class CRM_Mailing_Page_Unsubscribe extends CRM_Core_Page { /** * Run page. @@ -25,8 +25,44 @@ class CRM_Mailing_Page_Unsubscribe extends CRM_Mailing_Page_Common { * @throws Exception */ public function run() { - $this->_type = 'unsubscribe'; - return parent::run(); + $isOneClick = ($_SERVER['REQUEST_METHOD'] === 'POST' && CRM_Utils_Request::retrieve('List-Unsubscribe', 'String') === 'One-Click'); + if ($isOneClick) { + $this->handleOneClick(); + return NULL; + } + + $wrapper = new CRM_Utils_Wrapper(); + return $wrapper->run('CRM_Mailing_Form_Unsubscribe', $this->_title); + } + + /** + * + * Pre-condition: Validated the _job_id, _queue_id, _hash. + * Post-condition: Unsubscribed + * + * @link https://datatracker.ietf.org/doc/html/rfc8058 + * @return void + */ + public function handleOneClick(): void { + $jobId = CRM_Utils_Request::retrieve('jid', 'Integer'); + $queueId = CRM_Utils_Request::retrieve('qid', 'Integer'); + $hash = CRM_Utils_Request::retrieve('h', 'String'); + + $q = CRM_Mailing_Event_BAO_MailingEventQueue::verify(NULL, $queueId, $hash); + if (!$q) { + CRM_Utils_System::sendResponse( + new \GuzzleHttp\Psr7\Response(400, [], ts("Invalid request: bad parameters")) + ); + } + + $groups = CRM_Mailing_Event_BAO_MailingEventUnsubscribe::unsub_from_mailing($jobId, $queueId, $hash); + if (!empty($groups)) { + CRM_Mailing_Event_BAO_MailingEventUnsubscribe::send_unsub_response($queueId, $groups, FALSE, $jobId); + } + + CRM_Utils_System::sendResponse( + new \GuzzleHttp\Psr7\Response(200, [], 'OK') + ); } } diff --git a/CRM/Mailing/Service/ListUnsubscribe.php b/CRM/Mailing/Service/ListUnsubscribe.php new file mode 100644 index 000000000000..77a786e1a7a8 --- /dev/null +++ b/CRM/Mailing/Service/ListUnsubscribe.php @@ -0,0 +1,82 @@ + ts('Mailto'), + 'http' => ts('HTTP(S) Web-Form'), + 'oneclick' => ts('HTTP(S) One-Click'), + ]; + } + + public static function getSubscribedEvents() { + return [ + '&hook_civicrm_alterMailParams' => ['alterMailParams', 1000], + ]; + } + + /** + * @see \CRM_Utils_Hook::alterMailParams() + */ + public function alterMailParams(&$params, $context = NULL): void { + // FIXME: Flexmailer (BasicHeaders) and BAO (getVerpAndUrlsAndHeaders) separately define + // `List-Unsubscribe: `. And they have separate invocations of alterMailParams. + // + // This code is a little ugly because it anticipates serving both code-paths. + // But the BAO path should be properly killed. Doing so will allow you cleanup this code more. + + if (!in_array($context, ['civimail', 'flexmailer'])) { + return; + } + + $methods = Civi::settings()->get('civimail_unsubscribe_methods'); + if ($methods === ['mailto']) { + return; + } + + $sep = preg_quote(Civi::settings()->get('verpSeparator'), ';'); + $regex = ";^]*u{$sep}(\d+){$sep}(\d+){$sep}(\w*)@(.+)>$;"; + if (!preg_match($regex, $params['List-Unsubscribe'], $m)) { + \Civi::log()->warning('Failed to set final value of List-Unsubscribe'); + return; + } + + if ($this->urlFlags === NULL) { + $this->urlFlags = 'a'; + if (in_array('oneclick', $methods) && empty(parse_url(CIVICRM_UF_BASEURL, PHP_URL_PORT))) { + // Yahoo etal require HTTPS for one-click URLs. Cron-runs can be a bit inconsistent wrt HTTP(S), + // so we force-SSL for most production-style sites. + $this->urlFlags .= 's'; + } + } + + $listUnsubscribe = []; + if (in_array('mailto', $methods)) { + $listUnsubscribe[] = $params['List-Unsubscribe']; + } + if (array_intersect(['http', 'oneclick'], $methods)) { + $listUnsubscribe[] = '<' . Civi::url('frontend://civicrm/mailing/unsubscribe', $this->urlFlags)->addQuery([ + 'reset' => 1, + 'jid' => $m[1], + 'qid' => $m[2], + 'h' => $m[3], + ]) . '>'; + } + + if (in_array('oneclick', $methods)) { + $params['headers']['List-Unsubscribe-Post'] = 'List-Unsubscribe=One-Click'; + } + $params['headers']['List-Unsubscribe'] = implode(', ', $listUnsubscribe); + unset($params['List-Unsubscribe']); + } + +} diff --git a/CRM/Mailing/xml/Menu/Mailing.xml b/CRM/Mailing/xml/Menu/Mailing.xml index 5e6b7b474e7d..373ee2dc1a7c 100644 --- a/CRM/Mailing/xml/Menu/Mailing.xml +++ b/CRM/Mailing/xml/Menu/Mailing.xml @@ -106,7 +106,7 @@ civicrm/mailing/unsubscribe Unsubscribe - CRM_Mailing_Form_Unsubscribe + CRM_Mailing_Page_Unsubscribe access CiviMail subscribe/unsubscribe pages true 640 diff --git a/CRM/Utils/Check/Component/Mailing.php b/CRM/Utils/Check/Component/Mailing.php new file mode 100644 index 000000000000..b7288fb8f095 --- /dev/null +++ b/CRM/Utils/Check/Component/Mailing.php @@ -0,0 +1,46 @@ +get('civimail_unsubscribe_methods'); + if (in_array('oneclick', $methods)) { + return []; + } + + // OK, all guards passed. Show message. + $message = new CRM_Utils_Check_Message( + __FUNCTION__, + '

' . ts('Beginning in 2024, some web-mail services (Google and Yahoo) will require that large mailing-lists support another unsubscribe method: "HTTP One-Click" (RFC 8058). Please review the documentation and update the settings.') . '

', + ts('CiviMail: Enable One-Click Unsubscribe'), + \Psr\Log\LogLevel::NOTICE, + 'fa-server' + ); + $message->addAction(ts('Learn more'), FALSE, 'href', ['url' => 'https://civicrm.org/redirect/unsubscribe-one-click'], 'fa-info-circle'); + $message->addAction(ts('Update settings'), FALSE, 'href', ['path' => 'civicrm/admin/mail', 'query' => 'reset=1'], 'fa-wrench'); + + return [$message]; + } + +} diff --git a/CRM/Utils/Check/Message.php b/CRM/Utils/Check/Message.php index 8aae42516f2f..deb4a4b6cbba 100644 --- a/CRM/Utils/Check/Message.php +++ b/CRM/Utils/Check/Message.php @@ -155,6 +155,9 @@ public function addHelp($help) { * Currently supports: api3 or href * @param array $params * Params to be passed to CRM.api3 or CRM.url depending on type + * Ex: ['MyApiEntity', 'MyApiAction', [...params...]] + * Ex: ['path' => 'civicrm/admin/foo', 'query' => 'reset=1'] + * Ex: ['url' => 'https://example.com/more/info'] * @param string $icon * Fa-icon class for the button */ diff --git a/CRM/Utils/System/Base.php b/CRM/Utils/System/Base.php index 72ea4f1331f7..210e573addca 100644 --- a/CRM/Utils/System/Base.php +++ b/CRM/Utils/System/Base.php @@ -927,7 +927,7 @@ public function sendResponse(\Psr\Http\Message\ResponseInterface $response) { CRM_Utils_System::setHttpHeader($name, implode(', ', (array) $values)); } echo $response->getBody(); - CRM_Utils_System::civiExit(); + CRM_Utils_System::civiExit(0, ['response' => $response]); } /** diff --git a/Civi/Test/LocalHttpClient.php b/Civi/Test/LocalHttpClient.php new file mode 100644 index 000000000000..5463935b36e0 --- /dev/null +++ b/Civi/Test/LocalHttpClient.php @@ -0,0 +1,276 @@ + FALSE]); + * $response = $c->sendRequest(new Request('GET', '/civicrm/foo?reset=1&bar=100')); + * $response = $c->sendRequest(new Request('GET', '/civicrm/whiz?reset=1&bang=200')); + * + * In theory, this could be the basis for headless HTTP testing with client-libraries like Guzzle, Mink, or BrowserKit. + * + * WHY: CiviCRM predates the PSR HTTP OOP conventions -- many things are built with $_GET, $_REQUEST, etc. + * To simulate an HTTP request to these, we swap-in and swap-out values for $_GET, $_REQUEST, etc. + * Consequently, there is some limited isolation between the parent/requester and child/requestee. + * + * NOTE: You can improve the isolation more with `reboot=>TRUE`. This will swap (and reinitialize) + * the CiviCRM runtime-config and service-container. However, there is no comprehensive option to + * swap all static properties (other classes), so some data may still leak between requester+requestee. + * + * NOTE: This is primarily intended for use in headless testing (CIVICRM_UF=UnitTests). It may + * or may not be quirky with real UFs. + * + * @link https://www.php-fig.org/psr/psr-18/ + */ +class LocalHttpClient implements ClientInterface { + + /** + * List of scopes which should be backed-up, (re)populated, (re)set for the duration of the subrequest. + * + * @var array + * Ex: ['_GET' => new SuperGlobal('_GET')] + */ + protected array $scopes; + + /** + * List of scopes which should be inherited/extended within the subrequest. + * + * @var array + * Ex: ['_COOKIE', '_SERVER'] + */ + protected array $inherit; + + /** + * Whether to generate the HTML er + * + * @var bool + */ + protected bool $htmlHeader; + + /** + * @param array $options + * - reboot (bool): TRUE if you want to re-bootstrap CiviCRM (config/container) on each request + * Default: FALSE + * - htmlHeader (bool): TRUE if you want the generated page to include the full HTML header + * This may become standard (non-optional). It's opt-out to help debug/work-around some early + * quirks when first using LocalHttpClient in CI. + * - globals (string[]): List of (super)globals that should be backed-up, populated, used, and restored. + * Default: ['_GET', '_POST', '_COOKIE', '_FILES', '_SERVER', '_REQUEST'] + * - inherit (string[]): When populating these (super)globals, build on top of the existing values. + * Default: ['_COOKIE', '_SERVER'] + */ + public function __construct(array $options = []) { + $defaultOptions = [ + 'reboot' => FALSE, + 'htmlHeader' => TRUE, + 'globals' => ['_GET', '_POST', '_COOKIE', '_FILES', '_SERVER', '_REQUEST'], + 'inherit' => ['_COOKIE', '_SERVER'], + ]; + $options = array_merge($defaultOptions, $options); + + $this->inherit = $options['inherit']; + $this->htmlHeader = $options['htmlHeader']; + $this->scopes = []; + + foreach ($options['globals'] as $scopeName) { + $this->scopes[$scopeName] = new SuperGlobal($scopeName); + } + + if ($options['reboot']) { + $classes = [ + \Civi::class, + \CRM_Core_Config::class, + \CRM_Utils_Hook::class, + \Civi\Core\Resolver::class, + \CRM_Queue_Service::class, + \CRM_Utils_System::class, + \CRM_Utils_Cache::class, + ]; + foreach ($classes as $class) { + $this->scopes[$class] = new ClassProps($class); + } + } + } + + public function sendRequest(RequestInterface $request): ResponseInterface { + $backup = $this->getAllValues(); + try { + $this->initScopes($request); + + $var = \CRM_Core_Config::singleton()->userFrameworkURLVar; + if (!isset($_GET[$var])) { + $_GET[$var] = ltrim($request->getUri()->getPath(), '/'); + } + $body = $this->invoke($_GET[$var]); + // FIXME: There's probably a way to instrument CRM_Utils_System_UnitTests to do this better. + return new Response(200, [], $body); + } + catch (\CRM_Core_Exception_PrematureExitException $e) { + if (isset($e->errorData['response'])) { + return $e->errorData['response']; + } + // FIXME: There are some things which emit PrematureExitException but don't provide the $response object. + // We should probably revise \CRM_Utils_System::redirect() and returnJsonResponse() + else { + throw $e; + } + } + finally { + $this->restoreAllValues($backup); + } + } + + protected function initScopes(RequestInterface $request) { + foreach ($this->scopes as $scopeName => $scope) { + if (!in_array($scopeName, $this->inherit)) { + $scope->unsetKeys(array_keys($scope->getValues())); + } + + $method = 'initValues' . $scopeName; + $initValues = is_callable([$this, $method]) ? $this->$method($request) : []; + $scope->setValues($initValues); + } + if (in_array('CRM_Core_Config', $this->scopes)) { + \CRM_Core_Config::singleton(); + } + } + + /** + * Map data from the request to $_GET. + * + * @param \Psr\Http\Message\RequestInterface $request + * @return array + */ + protected function initValues_GET(RequestInterface $request): array { + $result = []; + parse_str($request->getUri()->getQuery() ?: '', $result); + return $result; + } + + /** + * Map data from the request to $_POST. + * + * @param \Psr\Http\Message\RequestInterface $request + * @return array + */ + protected function initValues_POST(RequestInterface $request): array { + $result = []; + if ($request->getMethod() === 'POST') { + $contentTypes = $request->getHeader('Content-Type'); + if (in_array('application/x-www-form-urlencoded', $contentTypes) || empty($contentTypes)) { + $body = (string) $request->getBody(); + parse_str($body, $result); + } + } + return $result; + } + + /** + * Map data from the request to $_REQUEST. + * + * @param \Psr\Http\Message\RequestInterface $request + * @return array + */ + protected function initValues_REQUEST(RequestInterface $request): array { + $sources = ['g' => '_GET', 'p' => '_POST', 'c' => '_COOKIE']; + + if (ini_get('request_order')) { + $order = strtolower(ini_get('request_order')); + } + elseif (ini_get('variables_order')) { + $order = strtolower(ini_get('variables_order')); + } + else { + $order = 'gpc'; + } + + $result = []; + for ($i = 0; $i < strlen($order); $i++) { + if (isset($sources[$order[$i]])) { + $scope = $this->scopes[$sources[$order[$i]]]; + $result = array_merge($result, $scope->getValues()); + } + } + return $result; + } + + protected function initValues_SERVER(RequestInterface $request): array { + $uri = $request->getUri(); + + $result = []; + $result['REQUEST_METHOD'] = $request->getMethod(); + $result['REQUEST_URI'] = $uri->getPath(); + if ($uri->getQuery()) { + $result['REQUEST_URI'] .= '?' . $uri->getQuery(); + } + if ($uri->getHost()) { + $result['HTTP_HOST'] = $uri->getHost(); + if ($uri->getPort()) { + $result['HTTP_HOST'] .= ':' . $uri->getPort(); + $result['SERVER_PORT'] = $uri->getPort(); + } + $result['SERVER_NAME'] = $uri->getHost(); + } + $result['HTTP_USER_AGENT'] = __CLASS__; + return $result; + } + + protected function invoke(string $route): ?string { + if ($this->htmlHeader) { + \CRM_Core_Resources::singleton()->addCoreResources('html-header'); + } + + ob_start(); + try { + $pageContent = \CRM_Core_Invoke::_invoke(explode('/', $route)); + } + finally { + $printedContent = ob_get_clean(); + } + + if (empty($pageContent) && !empty($printedContent)) { + $pageContent = $printedContent; + } + + $locale = \CRM_Core_I18n::getLocale(); + $lang = substr($locale, 0, 2); + $dir = \CRM_Core_I18n::isLanguageRTL($locale) ? 'rtl' : 'ltr'; + $head = $this->htmlHeader ? \CRM_Core_Region::instance('html-header')->render('') : ''; + + return << + +$head +$pageContent + +PAGETPL; + } + + public function getAllValues(): array { + $backup = []; + foreach ($this->scopes as $scopeName => $scope) { + $backup[$scopeName] = $scope->getValues(); + } + return $backup; + } + + protected function restoreAllValues(array &$backup): void { + foreach ($this->scopes as $scopeName => $scope) { + $extraKeys = array_diff(array_keys($scope->getValues()), array_keys($backup[$scopeName])); + $scope->unsetKeys($extraKeys); + $scope->setValues($backup[$scopeName]); + } + } + +} diff --git a/Civi/Test/LocalHttpClient/ClassProps.php b/Civi/Test/LocalHttpClient/ClassProps.php new file mode 100644 index 000000000000..a894b1482ac4 --- /dev/null +++ b/Civi/Test/LocalHttpClient/ClassProps.php @@ -0,0 +1,31 @@ +class = new \ReflectionClass($class); + } + + public function getValues() { + return $this->class->getStaticProperties() ?: []; + } + + public function setValues(iterable $values): void { + foreach ($values as $key => $value) { + $this->class->setStaticPropertyValue($key, $value); + } + } + + public function unsetKeys(iterable $keys): void { + foreach ($keys as $key) { + $this->class->setStaticPropertyValue($key, NULL); + } + } + +} diff --git a/Civi/Test/LocalHttpClient/SuperGlobal.php b/Civi/Test/LocalHttpClient/SuperGlobal.php new file mode 100644 index 000000000000..15649255f438 --- /dev/null +++ b/Civi/Test/LocalHttpClient/SuperGlobal.php @@ -0,0 +1,34 @@ +name = $name; + } + + public function getValues() { + return $GLOBALS[$this->name]; + } + + public function setValues(iterable $values): void { + foreach ($values as $key => $value) { + $GLOBALS[$this->name][$key] = $value; + } + } + + public function unsetKeys(iterable $keys): void { + foreach ($keys as $key) { + unset($GLOBALS[$this->name][$key]); + } + } + +} diff --git a/ang/crmStatusPage/StatusPageCtrl.js b/ang/crmStatusPage/StatusPageCtrl.js index 5dad04fc593c..2d6d75f61ffa 100644 --- a/ang/crmStatusPage/StatusPageCtrl.js +++ b/ang/crmStatusPage/StatusPageCtrl.js @@ -51,7 +51,7 @@ function run() { switch (action.type) { case 'href': - window.location = CRM.url(action.params.path, action.params.query, action.params.mode); + window.location = action.params.url ? action.params.url : CRM.url(action.params.path, action.params.query, action.params.mode); break; case 'api3': diff --git a/ext/flexmailer/tests/phpunit/Civi/FlexMailer/FlexMailerSystemTest.php b/ext/flexmailer/tests/phpunit/Civi/FlexMailer/FlexMailerSystemTest.php index dbba1d7ab5fc..0f5b6078bd78 100644 --- a/ext/flexmailer/tests/phpunit/Civi/FlexMailer/FlexMailerSystemTest.php +++ b/ext/flexmailer/tests/phpunit/Civi/FlexMailer/FlexMailerSystemTest.php @@ -75,13 +75,13 @@ public function handleEvent(GenericHookEvent $e) { * @see CRM_Utils_Hook::alterMailParams */ public function hook_alterMailParams(&$params, $context = NULL) { - $this->counts['hook_alterMailParams'] = 1; - $this->assertEquals('flexmailer', $context); + $this->counts["hook_alterMailParams::$context"] = 1; } public function tearDown(): void { parent::tearDown(); - $this->assertNotEmpty($this->counts['hook_alterMailParams']); + $this->assertNotEmpty($this->counts['hook_alterMailParams::flexmailer']); + $this->assertEmpty($this->counts['hook_alterMailParams::civimail'] ?? NULL); foreach (FlexMailer::getEventTypes() as $event => $class) { $this->assertTrue( $this->counts[$class] > 0, diff --git a/settings/Mailing.setting.php b/settings/Mailing.setting.php index d000dc47fda6..32e6a92ef602 100644 --- a/settings/Mailing.setting.php +++ b/settings/Mailing.setting.php @@ -18,6 +18,8 @@ * Settings metadata file */ +$unsubLearnMore = '
' . ts('(Learn more)', [1 => 'href="https://civicrm.org/redirect/unsubscribe-one-click" target="_blank"']); + return [ 'profile_double_optin' => [ 'group_name' => 'Mailing Preferences', @@ -91,6 +93,28 @@ 'is_contact' => 0, 'help_text' => NULL, ], + 'civimail_unsubscribe_methods' => [ + 'group_name' => 'Mailing Preferences', + 'group' => 'mailing', + 'name' => 'civimail_unsubscribe_methods', + 'type' => 'Array', + 'quick_form_type' => 'Select', + 'html_type' => 'Select', + 'html_attributes' => [ + 'multiple' => 1, + 'class' => 'crm-select2', + ], + 'default' => version_compare(CRM_Utils_System::version(), '5.72', '<=') ? ['mailto'] : ['mailto', 'http', 'oneclick'], + 'add' => '5.70', + 'title' => ts('Unsubscribe Methods'), + 'is_domain' => 1, + 'is_contact' => 0, + 'description' => ts("These methods will be offered to email clients for semi-automated unsubscribes. Support for each depends on the recipient's email client.") . $unsubLearnMore, + 'help_text' => NULL, + 'pseudoconstant' => [ + 'callback' => 'CRM_Mailing_Service_ListUnsubscribe::getMethods', + ], + ], 'replyTo' => [ 'group_name' => 'Mailing Preferences', 'group' => 'mailing', diff --git a/tests/phpunit/CRM/Mailing/BaseMailingSystemTest.php b/tests/phpunit/CRM/Mailing/BaseMailingSystemTest.php index d4e74e96df24..8259e9ca9e45 100644 --- a/tests/phpunit/CRM/Mailing/BaseMailingSystemTest.php +++ b/tests/phpunit/CRM/Mailing/BaseMailingSystemTest.php @@ -18,6 +18,8 @@ * @copyright CiviCRM LLC https://civicrm.org/licensing */ +use GuzzleHttp\Psr7\Request; + /** * Class CRM_Mailing_MailingSystemTest * @group headless @@ -39,6 +41,7 @@ public function setUp(): void { parent::setUp(); $this->useTransaction(); CRM_Mailing_BAO_MailingJob::$mailsProcessed = 0; + CRM_Core_BAO_MailSettings::defaultDAO(TRUE); $this->_groupID = $this->groupCreate(); $this->createContactsInGroup(2, $this->_groupID); @@ -52,11 +55,13 @@ public function setUp(): void { $this->_mut = new CiviMailUtils($this, TRUE); $this->callAPISuccess('mail_settings', 'get', ['api.mail_settings.create' => ['domain' => 'chaos.org']]); + Civi::settings()->set('civimail_unsubscribe_methods', ['mailto', 'http', 'oneclick']); } /** */ public function tearDown(): void { + $this->revertSetting('civimail_unsubscribe_methods'); $this->_mut->stop(); CRM_Utils_Hook::singleton()->reset(); // DGW @@ -85,11 +90,69 @@ public function testBasicHeaders() { $this->assertRegExp('#^text/plain; charset=utf-8#', $message->headers['Content-Type']); $this->assertRegExp(';^b\.[\d\.a-f]+@chaos.org$;', $message->headers['Return-Path']); $this->assertRegExp(';^b\.[\d\.a-f]+@chaos.org$;', $message->headers['X-CiviMail-Bounce'][0]); - $this->assertRegExp(';^\$;', $message->headers['List-Unsubscribe'][0]); + $this->assertRegExp(';^\, \$;', $message->headers['List-Unsubscribe'][0]); $this->assertEquals('bulk', $message->headers['Precedence'][0]); } } + public function testHttpUnsubscribe(): void { + $client = new Civi\Test\LocalHttpClient(['reboot' => TRUE, 'htmlHeader' => FALSE]); + + $allMessages = $this->runMailingSuccess([ + 'subject' => 'Yellow Unsubmarine', + 'body_text' => 'In the {domain.address} where I was born, lived a man who sailed to sea', + ]); + + $getMembers = fn($status) => Civi\Api4\GroupContact::get(FALSE) + ->addWhere('group_id', '=', $this->_groupID) + ->addWhere('status', '=', $status) + ->execute(); + + $this->assertEquals(2, $getMembers('Added')->count()); + $this->assertEquals(0, $getMembers('Removed')->count()); + + foreach ($allMessages as $k => $message) { + $this->assertNotEmpty($message->headers['List-Unsubscribe'][0]); + $this->assertEquals('List-Unsubscribe=One-Click', $message->headers['List-Unsubscribe-Post'][0]); + + $urls = array_map( + fn($s) => trim($s, '<>'), + preg_split('/[,\s]+/', $message->headers['List-Unsubscribe'][0]) + + ); + $url = CRM_Utils_Array::first(preg_grep('/^http/', $urls)); + $this->assertMatchesRegularExpression(';civicrm/mailing/unsubscribe;', $url); + + // Older clients (RFC 2369 only): Open a browser for the unsubscribe page + // FIXME: This works locally, but in CI complains about ckeditor4. Smells like compatibility issue between LocalHttpClient and $unclear. + $get = $client->sendRequest(new Request('GET', $url)); + $this->assertEquals(200, $get->getStatusCode()); + $this->assertMatchesRegularExpression(';You are requesting to unsubscribe;', (string) $get->getBody()); + + // Newer clients (RFC 8058): Send headless HTTP POST + $post = $client->sendRequest(new Request('POST', $url, [], $message->headers['List-Unsubscribe-Post'][0])); + $this->assertEquals(200, $post->getStatusCode()); + $this->assertEquals('OK', trim((string) $post->getBody())); + } + + // The HTTP POSTs removed the members. + $this->assertEquals(0, $getMembers('Added')->count()); + $this->assertEquals(2, $getMembers('Removed')->count()); + } + + public function testHttpUnsubscribe_altVerp(): void { + CRM_Core_DAO::executeQuery('UPDATE civicrm_mail_settings SET localpart = "aeiou.aeiou-"'); + CRM_Core_BAO_MailSettings::defaultDAO(TRUE); + + try { + Civi::settings()->set('verpSeparator', '-'); + $this->testHttpUnsubscribe(); + } + finally { + $this->revertSetting('verpSeparator'); + } + } + /** * Generate a fully-formatted mailing (with body_text content). */ diff --git a/tests/phpunit/CRM/Mailing/MailingSystemTest.php b/tests/phpunit/CRM/Mailing/MailingSystemTest.php index 264ff4f6749c..23a74b33e6a5 100644 --- a/tests/phpunit/CRM/Mailing/MailingSystemTest.php +++ b/tests/phpunit/CRM/Mailing/MailingSystemTest.php @@ -221,208 +221,4 @@ public function testMailingReplyAutoRespond(): void { $this->callAPISuccess('Group', 'delete', ['id' => $group_1]); } - /** - * Data provider for testGitLabIssue1108 - * - * First we run it without multiLingual mode, then with. - * - * This is because we test table names, which may have been translated in a - * multiLingual context. - * - */ - public function multiLingual() { - return [[0], [1]]; - } - - /** - * - unsubscribe used dodgy SQL that only checked half of the polymorphic - * relationship in mailing_group, meaning it could match 'mailing 123' - * against _group_ 123. - * - * - also, an INNER JOIN on the group table hid the mailing-based - * mailing_group records. - * - * - in turn this inner join meant the query returned nothing, which then - * caused the code that is supposed to find the contact within those groups - * to basically find all the groups that the contact in or were smart groups. - * - * - in certain situations (which I have not been able to replicate in this - * test) it caused the unsubscribe to fail to find *any* groups to unsubscribe - * people from, thereby breaking the unsubscribe. - * - * @dataProvider multiLingual - * - */ - public function testGitLabIssue1108($isMultiLingual): void { - - // We need to make sure the mailing IDs are higher than the groupIDs. - // We do this by adding mailings until the mailing.id value is at least 10 - // higher than the highest group.id - // Note that creating a row in a transaction then rolling back the - // transaction still increments the AUTO_INCREMENT counter for the table. - // (If this behaviour ever changes we throw an exception.) - if ($isMultiLingual) { - $cleanup = $this->useMultilingual(['en_US' => 'fr_FR']); - } - $max_group_id = CRM_Core_DAO::singleValueQuery("SELECT MAX(id) FROM civicrm_group"); - $max_mailing_id = 0; - while ($max_mailing_id < $max_group_id + 10) { - CRM_Core_Transaction::create()->run(function($tx) use (&$max_mailing_id) { - CRM_Core_DAO::executeQuery("INSERT INTO civicrm_mailing (name) VALUES ('dummy');"); - $_ = (int) CRM_Core_DAO::singleValueQuery("SELECT MAX(id) FROM civicrm_mailing"); - if ($_ === $max_mailing_id) { - throw new RuntimeException("Expected that creating a new row would increment ID, but it did not. This could be a change in MySQL's implementation of rollback"); - } - $max_mailing_id = $_; - $tx->rollback(); - }); - } - - // Because our parent class marks the _groupID as private, we can't use that :-( - $group_1 = $this->groupCreate([ - 'name' => 'Test Group 1108.1', - 'title' => 'Test Group 1108.1', - ]); - $this->createContactsInGroup(2, $group_1); - - // Also _mut is private to the parent, so we have to make our own: - $mut = new CiviMailUtils($this, TRUE); - - // Create initial mailing to the group. - $mailingParams = [ - 'name' => 'Issue 1108: mailing 1', - 'subject' => 'Issue 1108: mailing 1', - 'created_id' => 1, - 'groups' => ['include' => [$group_1]], - 'scheduled_date' => 'now', - 'body_text' => 'Please just {action.unsubscribe}', - ]; - - // The following code is exactly the same as runMailingSuccess() except that we store the ID of the mailing. - $mailing_1 = $this->callAPISuccess('mailing', 'create', $mailingParams); - $mut->assertRecipients(array()); - $this->callAPISuccess('job', 'process_mailing', array('runInNonProductionEnvironment' => TRUE)); - - $allMessages = $mut->getAllMessages('ezc'); - // There are exactly two contacts produced by setUp(). - $this->assertEquals(2, count($allMessages)); - - // We need a new group - $group_2 = $this->groupCreate([ - 'name' => 'Test Group 1108.2', - 'title' => 'Test Group 1108.2', - ]); - - // Now create the 2nd mailing to the recipients of the first, - // excluding our new albeit empty group. - $mailingParams = [ - 'name' => 'Issue 1108: mailing 2', - 'subject' => 'Issue 1108: mailing 2', - 'created_id' => 1, - 'mailings' => ['include' => [$mailing_1['id']]], - 'groups' => ['exclude' => [$group_2]], - 'scheduled_date' => 'now', - 'body_text' => 'Please just {action.unsubscribeUrl}', - ]; - $this->callAPISuccess('mailing', 'create', $mailingParams); - $_ = $this->callAPISuccess('job', 'process_mailing', array('runInNonProductionEnvironment' => TRUE)); - - $allMessages = $mut->getAllMessages('ezc'); - // We should have 2+2 messages sent by the mail system now. - $this->assertEquals(4, count($allMessages)); - - // So far so good. - // Now extract the unsubscribe details. - $message = end($allMessages); - $this->assertTrue($message->body instanceof ezcMailText); - $this->assertEquals('plain', $message->body->subType); - $this->assertEquals(1, preg_match( - '@mailing/unsubscribe.*jid=(\d+)&qid=(\d+)&h=([0-9a-z]+)@', - $message->body->text, - $matches - )); - - // Create a group that has nothing to do with this mailing. - $group_3 = $this->groupCreate([ - 'name' => 'Test Group 1108.3', - 'title' => 'Test Group 1108.3', - ]); - // Add contacts from group 1 to group 3. - $gcQuery = new CRM_Contact_BAO_GroupContact(); - $gcQuery->group_id = $group_1; - $gcQuery->status = 'Added'; - $gcQuery->find(); - while ($gcQuery->fetch()) { - $this->callAPISuccess('group_contact', 'create', - ['group_id' => $group_3, 'contact_id' => $gcQuery->contact_id, 'status' => 'Added']); - } - - // Part of the issue is caused by the fact that (at time of writing) the - // SQL joined the mailing_group table on just the entity_id, assuming it to - // be a group, but actually it could be a mailing. - // The difficulty in testing this is that because all our IDs are very low - // and contiguous the SQL looking for a match for 'mailing 1' does match a - // group ID of '1', which is created in this class's parent's setUp(). - // Strictly speaking we don't know that it has ID 1, but as we can't access _groupID - // we'll have to assume that. - // - // So by deleting that group the SQL then matches nothing which is what we - // need for this case. - $_ = new CRM_Contact_BAO_Group(); - $_->id = 1; - $_->delete(); - - $hooks = \CRM_Utils_Hook::singleton(); - $found = []; - $hooks->setHook('civicrm_unsubscribeGroups', - function ($op, $mailingId, $contactId, &$groups, &$baseGroups) use (&$found) { - $found['groups'] = $groups; - $found['baseGroups'] = $baseGroups; - }); - - // Now test unsubscribe groups. - $groups = CRM_Mailing_Event_BAO_MailingEventUnsubscribe::unsub_from_mailing( - $matches[1], - $matches[2], - $matches[3], - TRUE - ); - - // We expect that our group_1 was found. - $this->assertEquals(['groups' => [$group_1], 'baseGroups' => []], $found); - - // We *should* get an array with just our $group_1 since this is the only group - // that we have included. - // $group_2 was only used to exclude people. - // $group_3 has nothing to do with this mailing and should not be there. - $this->assertNotEmpty($groups, "We should have received an array."); - $this->assertEquals([$group_1], array_keys($groups), - "We should have received an array with our group 1 in it."); - - if ($isMultiLingual) { - global $dbLocale; - $dbLocale = '_fr_FR'; - // Now test unsubscribe groups. - $groups = CRM_Mailing_Event_BAO_MailingEventUnsubscribe::unsub_from_mailing( - $matches[1], - $matches[2], - $matches[3], - TRUE - ); - - // We expect that our group_1 was found. - $this->assertEquals(['groups' => [$group_1], 'baseGroups' => []], $found); - - // We *should* get an array with just our $group_1 since this is the only group - // that we have included. - // $group_2 was only used to exclude people. - // $group_3 has nothing to do with this mailing and should not be there. - $this->assertNotEmpty($groups, "We should have received an array."); - $this->assertEquals([$group_1], array_keys($groups), - "We should have received an array with our group 1 in it."); - global $dbLocale; - $dbLocale = '_en_US'; - } - } - } diff --git a/tests/phpunit/CRM/Mailing/MultilingualSystemTest.php b/tests/phpunit/CRM/Mailing/MultilingualSystemTest.php new file mode 100644 index 000000000000..77b93dab1447 --- /dev/null +++ b/tests/phpunit/CRM/Mailing/MultilingualSystemTest.php @@ -0,0 +1,301 @@ +_groupID = $this->groupCreate(); + $this->createContactsInGroup(2, $this->_groupID); + + $this->_mut = new CiviMailUtils($this, TRUE); + $this->callAPISuccess('mail_settings', 'get', + ['api.mail_settings.create' => ['domain' => 'chaos.org']]); + } + + public function tearDown(): void { + $this->quickCleanup([ + 'civicrm_mailing', + 'civicrm_mailing_job', + 'civicrm_mailing_spool', + 'civicrm_mailing_group', + 'civicrm_mailing_recipients', + 'civicrm_mailing_event_queue', + 'civicrm_mailing_event_bounce', + 'civicrm_mailing_event_delivered', + 'civicrm_group', + 'civicrm_group_contact', + 'civicrm_contact', + 'civicrm_activity_contact', + 'civicrm_activity', + ]); + + global $dbLocale; + if ($dbLocale) { + $this->disableMultilingual(); + } + + $this->_mut->stop(); + CRM_Utils_Hook::singleton()->reset(); + // DGW + CRM_Mailing_BAO_MailingJob::$mailsProcessed = 0; + + parent::tearDown(); + } + + /** + * Data provider for testGitLabIssue1108 + * + * First we run it without multiLingual mode, then with. + * + * This is because we test table names, which may have been translated in a + * multiLingual context. + * + */ + public function multiLingual() { + return [[0], [1]]; + } + + /** + * - unsubscribe used dodgy SQL that only checked half of the polymorphic + * relationship in mailing_group, meaning it could match 'mailing 123' + * against _group_ 123. + * + * - also, an INNER JOIN on the group table hid the mailing-based + * mailing_group records. + * + * - in turn this inner join meant the query returned nothing, which then + * caused the code that is supposed to find the contact within those groups + * to basically find all the groups that the contact in or were smart groups. + * + * - in certain situations (which I have not been able to replicate in this + * test) it caused the unsubscribe to fail to find *any* groups to unsubscribe + * people from, thereby breaking the unsubscribe. + * + * @dataProvider multiLingual + * + */ + public function testGitLabIssue1108($isMultiLingual): void { + + // We need to make sure the mailing IDs are higher than the groupIDs. + // We do this by adding mailings until the mailing.id value is at least 10 + // higher than the highest group.id + // Note that creating a row in a transaction then rolling back the + // transaction still increments the AUTO_INCREMENT counter for the table. + // (If this behaviour ever changes we throw an exception.) + if ($isMultiLingual) { + $cleanup = $this->useMultilingual(['en_US' => 'fr_FR']); + } + $max_group_id = CRM_Core_DAO::singleValueQuery("SELECT MAX(id) FROM civicrm_group"); + $max_mailing_id = 0; + while ($max_mailing_id < $max_group_id + 10) { + CRM_Core_Transaction::create()->run(function($tx) use (&$max_mailing_id) { + CRM_Core_DAO::executeQuery("INSERT INTO civicrm_mailing (name) VALUES ('dummy');"); + $_ = (int) CRM_Core_DAO::singleValueQuery("SELECT MAX(id) FROM civicrm_mailing"); + if ($_ === $max_mailing_id) { + throw new RuntimeException("Expected that creating a new row would increment ID, but it did not. This could be a change in MySQL's implementation of rollback"); + } + $max_mailing_id = $_; + $tx->rollback(); + }); + } + + // Because our parent class marks the _groupID as private, we can't use that :-( + $group_1 = $this->groupCreate([ + 'name' => 'Test Group 1108.1', + 'title' => 'Test Group 1108.1', + ]); + $this->createContactsInGroup(2, $group_1); + + // Also _mut is private to the parent, so we have to make our own: + $mut = new CiviMailUtils($this, TRUE); + + // Create initial mailing to the group. + $mailingParams = [ + 'name' => 'Issue 1108: mailing 1', + 'subject' => 'Issue 1108: mailing 1', + 'created_id' => 1, + 'groups' => ['include' => [$group_1]], + 'scheduled_date' => 'now', + 'body_text' => 'Please just {action.unsubscribe}', + ]; + + // The following code is exactly the same as runMailingSuccess() except that we store the ID of the mailing. + $mailing_1 = $this->callAPISuccess('mailing', 'create', $mailingParams); + $mut->assertRecipients([]); + $this->callAPISuccess('job', 'process_mailing', ['runInNonProductionEnvironment' => TRUE]); + + $allMessages = $mut->getAllMessages('ezc'); + // There are exactly two contacts produced by setUp(). + $this->assertEquals(2, count($allMessages)); + + // We need a new group + $group_2 = $this->groupCreate([ + 'name' => 'Test Group 1108.2', + 'title' => 'Test Group 1108.2', + ]); + + // Now create the 2nd mailing to the recipients of the first, + // excluding our new albeit empty group. + $mailingParams = [ + 'name' => 'Issue 1108: mailing 2', + 'subject' => 'Issue 1108: mailing 2', + 'created_id' => 1, + 'mailings' => ['include' => [$mailing_1['id']]], + 'groups' => ['exclude' => [$group_2]], + 'scheduled_date' => 'now', + 'body_text' => 'Please just {action.unsubscribeUrl}', + ]; + $this->callAPISuccess('mailing', 'create', $mailingParams); + $_ = $this->callAPISuccess('job', 'process_mailing', ['runInNonProductionEnvironment' => TRUE]); + + $allMessages = $mut->getAllMessages('ezc'); + // We should have 2+2 messages sent by the mail system now. + $this->assertEquals(4, count($allMessages)); + + // So far so good. + // Now extract the unsubscribe details. + $message = end($allMessages); + $this->assertTrue($message->body instanceof ezcMailText); + $this->assertEquals('plain', $message->body->subType); + $this->assertEquals(1, preg_match( + '@mailing/unsubscribe.*jid=(\d+)&qid=(\d+)&h=([0-9a-z]+)@', + $message->body->text, + $matches + )); + + // Create a group that has nothing to do with this mailing. + $group_3 = $this->groupCreate([ + 'name' => 'Test Group 1108.3', + 'title' => 'Test Group 1108.3', + ]); + // Add contacts from group 1 to group 3. + $gcQuery = new CRM_Contact_BAO_GroupContact(); + $gcQuery->group_id = $group_1; + $gcQuery->status = 'Added'; + $gcQuery->find(); + while ($gcQuery->fetch()) { + $this->callAPISuccess('group_contact', 'create', + ['group_id' => $group_3, 'contact_id' => $gcQuery->contact_id, 'status' => 'Added']); + } + + // Part of the issue is caused by the fact that (at time of writing) the + // SQL joined the mailing_group table on just the entity_id, assuming it to + // be a group, but actually it could be a mailing. + // The difficulty in testing this is that because all our IDs are very low + // and contiguous the SQL looking for a match for 'mailing 1' does match a + // group ID of '1', which is created in this class's parent's setUp(). + // Strictly speaking we don't know that it has ID 1, but as we can't access _groupID + // we'll have to assume that. + // + // So by deleting that group the SQL then matches nothing which is what we + // need for this case. + $_ = new CRM_Contact_BAO_Group(); + $_->id = 1; + $_->delete(); + + $hooks = \CRM_Utils_Hook::singleton(); + $found = []; + $hooks->setHook('civicrm_unsubscribeGroups', + function ($op, $mailingId, $contactId, &$groups, &$baseGroups) use (&$found) { + $found['groups'] = $groups; + $found['baseGroups'] = $baseGroups; + }); + + // Now test unsubscribe groups. + $groups = CRM_Mailing_Event_BAO_MailingEventUnsubscribe::unsub_from_mailing( + $matches[1], + $matches[2], + $matches[3], + TRUE + ); + + // We expect that our group_1 was found. + $this->assertEquals(['groups' => [$group_1], 'baseGroups' => []], $found); + + // We *should* get an array with just our $group_1 since this is the only group + // that we have included. + // $group_2 was only used to exclude people. + // $group_3 has nothing to do with this mailing and should not be there. + $this->assertNotEmpty($groups, "We should have received an array."); + $this->assertEquals([$group_1], array_keys($groups), + "We should have received an array with our group 1 in it."); + + if ($isMultiLingual) { + global $dbLocale; + $dbLocale = '_fr_FR'; + // Now test unsubscribe groups. + $groups = CRM_Mailing_Event_BAO_MailingEventUnsubscribe::unsub_from_mailing( + $matches[1], + $matches[2], + $matches[3], + TRUE + ); + + // We expect that our group_1 was found. + $this->assertEquals(['groups' => [$group_1], 'baseGroups' => []], $found); + + // We *should* get an array with just our $group_1 since this is the only group + // that we have included. + // $group_2 was only used to exclude people. + // $group_3 has nothing to do with this mailing and should not be there. + $this->assertNotEmpty($groups, "We should have received an array."); + $this->assertEquals([$group_1], array_keys($groups), + "We should have received an array with our group 1 in it."); + global $dbLocale; + $dbLocale = '_en_US'; + } + } + + /** + * (FIXME) De-duplicate BaseMailingSystemTest::createContactsInGroup and MultilingualSystemTest::createContactsInGroup + * This should probably be in a trait. + * + * Create contacts in group. + * + * @param int $count + * @param int $groupID + * @param string $domain + */ + protected function createContactsInGroup( + $count, + $groupID, + $domain = 'nul.example.com' + ) { + for ($i = 1; $i <= $count; $i++) { + $contactID = $this->individualCreate([ + 'first_name' => "Foo{$i}", + 'email' => 'mail' . $i . '@' . $domain, + ]); + $this->callAPISuccess('group_contact', 'create', [ + 'contact_id' => $contactID, + 'group_id' => $groupID, + 'status' => 'Added', + ]); + } + } + +}