From 5d6cf64890500a9ea9fb6391c52703e43a9cb5ae Mon Sep 17 00:00:00 2001 From: Jamie McClelland Date: Wed, 28 Mar 2018 14:55:50 -0400 Subject: [PATCH] dev/core#35 Avoid variable leakage on recurring contribution receipts. This covers pcpParams (which are tested & sets up the data set to extend to honor params). See https://lab.civicrm.org/dev/core/issues/35 --- CRM/Contribute/BAO/Contribution.php | 25 +++++++++++++------ CRM/Core/BAO/UFJoin.php | 2 +- .../CRM/Core/Payment/AuthorizeNetIPNTest.php | 21 ++++++++++++++++ tests/phpunit/CiviTest/CiviUnitTestCase.php | 13 +++++++--- 4 files changed, 48 insertions(+), 13 deletions(-) diff --git a/CRM/Contribute/BAO/Contribution.php b/CRM/Contribute/BAO/Contribution.php index 974b2e6945..80c5cfab1a 100644 --- a/CRM/Contribute/BAO/Contribution.php +++ b/CRM/Contribute/BAO/Contribution.php @@ -2895,6 +2895,10 @@ INNER JOIN civicrm_activity ON civicrm_activity_contact.activity_id = civicrm_ac * @return mixed */ public function _assignMessageVariablesToTemplate(&$values, $input, $returnMessageText = TRUE) { + // @todo - this should have a better separation of concerns - ie. + // gatherMessageValues should build an array of values to be assigned to the template + // and this function should assign them (assigning null if not set). + // the way the pcpParams & honor Params section works is a baby-step towards this. $template = CRM_Core_Smarty::singleton(); $template->assign('first_name', $this->_relatedObjects['contact']->first_name); $template->assign('last_name', $this->_relatedObjects['contact']->last_name); @@ -2907,6 +2911,7 @@ INNER JOIN civicrm_activity ON civicrm_activity_contact.activity_id = civicrm_ac //assign honor information to receipt message $softRecord = CRM_Contribute_BAO_ContributionSoft::getSoftContribution($this->id); + $honorParams = ['soft_credit_type' => NULL, 'honor_block_is_active' => NULL]; if (isset($softRecord['soft_credit'])) { //if id of contribution page is present if (!empty($values['id'])) { @@ -2916,8 +2921,8 @@ INNER JOIN civicrm_activity ON civicrm_activity_contact.activity_id = civicrm_ac 'honor_id' => $softRecord['soft_credit'][1]['contact_id'], ); - $template->assign('soft_credit_type', $softRecord['soft_credit'][1]['soft_credit_type_label']); - $template->assign('honor_block_is_active', CRM_Core_DAO::getFieldValue('CRM_Core_DAO_UFJoin', $values['id'], 'is_active', 'entity_id')); + $honorParams['soft_credit_type'] = $softRecord['soft_credit'][1]['soft_credit_type_label']; + $honorParams['honor_block_is_active'] = CRM_Core_DAO::getFieldValue('CRM_Core_DAO_UFJoin', $values['id'], 'is_active', 'entity_id'); } else { //offline contribution @@ -2954,25 +2959,29 @@ INNER JOIN civicrm_activity ON civicrm_activity_contact.activity_id = civicrm_ac $values['amount'] = $this->total_amount; } - // add the new contribution values + $pcpParams = ['pcpBlock' => NULL, 'pcp_display_in_roll' => NULL, 'pcp_roll_nickname' => NULL, 'pcp_personal_note' => NULL, 'title' => NULL]; + if (strtolower($this->_component) == 'contribute') { //PCP Info $softDAO = new CRM_Contribute_DAO_ContributionSoft(); $softDAO->contribution_id = $this->id; if ($softDAO->find(TRUE)) { - $template->assign('pcpBlock', TRUE); - $template->assign('pcp_display_in_roll', $softDAO->pcp_display_in_roll); - $template->assign('pcp_roll_nickname', $softDAO->pcp_roll_nickname); - $template->assign('pcp_personal_note', $softDAO->pcp_personal_note); + $pcpParams['pcpBlock'] = TRUE; + $pcpParams['pcp_display_in_roll'] = $softDAO->pcp_display_in_roll; + $pcpParams['pcp_roll_nickname'] = $softDAO->pcp_roll_nickname; + $pcpParams['pcp_personal_note'] = $softDAO->pcp_personal_note; //assign the pcp page title for email subject $pcpDAO = new CRM_PCP_DAO_PCP(); $pcpDAO->id = $softDAO->pcp_id; if ($pcpDAO->find(TRUE)) { - $template->assign('title', $pcpDAO->title); + $pcpParams['title'] = $pcpDAO->title; } } } + foreach (array_merge($honorParams, $pcpParams) as $templateKey => $templateValue) { + $template->assign($templateKey, $templateValue); + } if ($this->financial_type_id) { $values['financial_type_id'] = $this->financial_type_id; diff --git a/CRM/Core/BAO/UFJoin.php b/CRM/Core/BAO/UFJoin.php index f6df703442..b5442b60fa 100644 --- a/CRM/Core/BAO/UFJoin.php +++ b/CRM/Core/BAO/UFJoin.php @@ -54,7 +54,7 @@ class CRM_Core_BAO_UFJoin extends CRM_Core_DAO_UFJoin { } $dao = new CRM_Core_DAO_UFJoin(); - $dao->copyValues($params); + $dao->copyValues($params, TRUE); if ($params['uf_group_id']) { $dao->save(); } diff --git a/tests/phpunit/CRM/Core/Payment/AuthorizeNetIPNTest.php b/tests/phpunit/CRM/Core/Payment/AuthorizeNetIPNTest.php index 09385e4db9..300f6324ff 100644 --- a/tests/phpunit/CRM/Core/Payment/AuthorizeNetIPNTest.php +++ b/tests/phpunit/CRM/Core/Payment/AuthorizeNetIPNTest.php @@ -224,6 +224,15 @@ class CRM_Core_Payment_AuthorizeNetIPNTest extends CiviUnitTestCase { $mut = new CiviMailUtils($this, TRUE); $this->setupMembershipRecurringPaymentProcessorTransaction(array('is_email_receipt' => TRUE)); $this->addProfile('supporter_profile', $this->_contributionPageID); + $this->addProfile('honoree_individual', $this->_contributionPageID, 'soft_credit'); + + $this->callAPISuccess('ContributionSoft', 'create', [ + 'contact_id' => $this->individualCreate(), + 'contribution_id' => $this->_contributionID, + 'soft_credit_type_id' => 'in_memory_of', + 'amount' => 200, + ]); + $IPN = new CRM_Core_Payment_AuthorizeNetIPN($this->getRecurTransaction()); $IPN->main(); $mut->checkAllMailLog(array( @@ -235,14 +244,18 @@ class CRM_Core_Payment_AuthorizeNetIPNTest extends CiviUnitTestCase { 'First Name: Anthony', 'Last Name: Anderson', 'Email Address: anthony_anderson@civicrm.org', + 'Honor', 'This membership will be automatically renewed every', 'Dear Mr. Anthony Anderson II', 'Thanks for your auto renew membership sign-up', + 'In Memory of', )); $mut->clearMessages(); $this->_contactID = $this->individualCreate(array('first_name' => 'Antonia', 'prefix_id' => 'Mrs.', 'email' => 'antonia_anderson@civicrm.org')); $this->_invoiceID = uniqid(); + // Note, the second contribution is not in honor of anyone and the + // receipt should not mention honor at all. $this->setupMembershipRecurringPaymentProcessorTransaction(array('is_email_receipt' => TRUE)); $IPN = new CRM_Core_Payment_AuthorizeNetIPN($this->getRecurTransaction(array('x_trans_id' => 'hers'))); $IPN->main(); @@ -263,6 +276,14 @@ class CRM_Core_Payment_AuthorizeNetIPNTest extends CiviUnitTestCase { 'Thanks for your auto renew membership sign-up', )); + $shouldNotBeInMailing = array( + 'Honor', + 'In Memory of', + ); + $mails = $mut->getAllMessages('raw'); + foreach ($mails as $mail) { + $mut->checkMailForStrings(array(), $shouldNotBeInMailing, '', $mail); + } $mut->stop(); $mut->clearMessages(); } diff --git a/tests/phpunit/CiviTest/CiviUnitTestCase.php b/tests/phpunit/CiviTest/CiviUnitTestCase.php index e7cfbbaf19..b1be120c16 100644 --- a/tests/phpunit/CiviTest/CiviUnitTestCase.php +++ b/tests/phpunit/CiviTest/CiviUnitTestCase.php @@ -3004,15 +3004,20 @@ AND ( TABLE_NAME LIKE 'civicrm_value_%' ) * * @param string $name * @param int $contributionPageID + * @param string $module */ - protected function addProfile($name, $contributionPageID) { - $this->callAPISuccess('UFJoin', 'create', array( + protected function addProfile($name, $contributionPageID, $module = 'CiviContribute') { + $params = [ 'uf_group_id' => $name, - 'module' => 'CiviContribute', + 'module' => $module, 'entity_table' => 'civicrm_contribution_page', 'entity_id' => $contributionPageID, 'weight' => 1, - )); + ]; + if ($module !== 'CiviContribute') { + $params['module_data'] = [$module => []]; + } + $this->callAPISuccess('UFJoin', 'create', $params); } /** -- 2.25.1