Skip to content

Commit

Permalink
CRM-21753 add pass-through support for criteria in urls on dedupe pages
Browse files Browse the repository at this point in the history
This permits urls such as
 civicrm/contact/dedupefind?reset=1&rgid=4&gid=&limit=&criteria={"contact":{"contact_id":205}}&action=update

The contents of $criteria['contact'] are passed to the contact api to get a list of contacts to find possible
matches for (acls are applied when generating the match list to both the matched & match list in the next step).

Note that the urls are urlencoded & hence will have %22 rather than a quote so that feels xss safe but I added some
validation before I figured that part out & I think it still makes sense to keep it.

civicrm#11658

Change-Id: I22129a0c63da4bdcb0151501e10244762651de95
  • Loading branch information
eileenmcnaughton committed Mar 20, 2018
1 parent 5b04c99 commit 021a48b
Show file tree
Hide file tree
Showing 7 changed files with 83 additions and 14 deletions.
14 changes: 10 additions & 4 deletions CRM/Contact/Form/Merge.php
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,11 @@ class CRM_Contact_Form_Merge extends CRM_Core_Form {

var $_contactType = NULL;

/**
* @var array
*/
public $criteria = array();

/**
* Query limit to be retained in the urls.
*
Expand Down Expand Up @@ -74,8 +79,9 @@ public function preProcess() {
$this->_gid = $gid = CRM_Utils_Request::retrieve('gid', 'Positive', $this, FALSE);
$this->_mergeId = CRM_Utils_Request::retrieve('mergeId', 'Positive', $this, FALSE);
$this->limit = CRM_Utils_Request::retrieve('limit', 'Positive', $this, FALSE);
$this->criteria = CRM_Utils_Request::retrieve('criteria', 'Json', $this, FALSE, '{}');

$urlParams = ['reset' => 1, 'rgid' => $this->_rgid, 'gid' => $this->_gid, 'limit' => $this->limit];
$urlParams = ['reset' => 1, 'rgid' => $this->_rgid, 'gid' => $this->_gid, 'limit' => $this->limit, 'criteria' => $this->criteria];

$this->bounceIfInvalid($this->_cid, $this->_oid);

Expand All @@ -100,7 +106,7 @@ public function preProcess() {
CRM_Core_Session::singleton()->pushUserContext($browseUrl);
}

$cacheKey = CRM_Dedupe_Merger::getMergeCacheKeyString($this->_rgid, $gid);
$cacheKey = CRM_Dedupe_Merger::getMergeCacheKeyString($this->_rgid, $gid, json_decode($this->criteria, TRUE));

$join = CRM_Dedupe_Merger::getJoinOnDedupeTable();
$where = "de.id IS NULL";
Expand Down Expand Up @@ -300,7 +306,7 @@ public function postProcess() {
$message = '<ul><li>' . ts('%1 has been updated.', array(1 => $name)) . '</li><li>' . ts('Contact ID %1 has been deleted.', array(1 => $this->_oid)) . '</li></ul>';
CRM_Core_Session::setStatus($message, ts('Contacts Merged'), 'success');

$urlParams = ['reset' => 1, 'cid' => $this->_cid, 'rgid' => $this->_rgid, 'gid' => $this->_gid, 'limit' => $this->limit];
$urlParams = ['reset' => 1, 'cid' => $this->_cid, 'rgid' => $this->_rgid, 'gid' => $this->_gid, 'limit' => $this->limit, 'criteria' => $this->criteria];
$contactViewUrl = CRM_Utils_System::url('civicrm/contact/view', ['reset' => 1, 'cid' => $this->_cid]);

if (!empty($formValues['_qf_Merge_submit'])) {
Expand All @@ -314,7 +320,7 @@ public function postProcess() {
}

if ($this->next && $this->_mergeId) {
$cacheKey = CRM_Dedupe_Merger::getMergeCacheKeyString($this->_rgid, $this->_gid);
$cacheKey = CRM_Dedupe_Merger::getMergeCacheKeyString($this->_rgid, $this->_gid, json_decode($this->criteria, TRUE));

$join = CRM_Dedupe_Merger::getJoinOnDedupeTable();
$where = "de.id IS NULL";
Expand Down
9 changes: 7 additions & 2 deletions CRM/Contact/Page/AJAX.php
Original file line number Diff line number Diff line change
Expand Up @@ -646,13 +646,16 @@ public static function getDedupes() {

$gid = CRM_Utils_Request::retrieve('gid', 'Positive');
$rgid = CRM_Utils_Request::retrieve('rgid', 'Positive');
$null = NULL;
$criteria = CRM_Utils_Request::retrieve('criteria', 'Json', $null, FALSE, '{}');
$selected = isset($_REQUEST['selected']) ? CRM_Utils_Type::escape($_REQUEST['selected'], 'Integer') : 0;
if ($rowCount < 0) {
$rowCount = 0;
}

$whereClause = $orderByClause = '';
$cacheKeyString = CRM_Dedupe_Merger::getMergeCacheKeyString($rgid, $gid);
$cacheKeyString = CRM_Dedupe_Merger::getMergeCacheKeyString($rgid, $gid, json_decode($criteria, TRUE));

$searchRows = array();

$searchParams = self::getSearchOptionsFromRequest();
Expand Down Expand Up @@ -814,6 +817,7 @@ public static function getDedupes() {
'oid' => $pairInfo['entity_id2'],
'action' => 'update',
'rgid' => $rgid,
'criteria' => $criteria,
'limit' => CRM_Utils_Request::retrieve('limit', 'Integer'),
];
if ($gid) {
Expand Down Expand Up @@ -1014,8 +1018,9 @@ public static function toggleDedupeSelect() {
$gid = CRM_Utils_Type::escape($_REQUEST['gid'], 'Integer');
$pnid = $_REQUEST['pnid'];
$isSelected = CRM_Utils_Type::escape($_REQUEST['is_selected'], 'Boolean');
$criteria = CRM_Utils_Request::retrieve('criteria', 'Json', $null, FALSE, '{}');

$cacheKeyString = CRM_Dedupe_Merger::getMergeCacheKeyString($rgid, $gid);
$cacheKeyString = CRM_Dedupe_Merger::getMergeCacheKeyString($rgid, $gid, json_decode($criteria, TRUE));

$params = array(
1 => array($isSelected, 'Boolean'),
Expand Down
8 changes: 6 additions & 2 deletions CRM/Contact/Page/DedupeFind.php
Original file line number Diff line number Diff line change
Expand Up @@ -63,8 +63,10 @@ public function run() {
$limit = CRM_Utils_Request::retrieve('limit', 'Integer', $this);
$rgid = CRM_Utils_Request::retrieve('rgid', 'Positive', $this);
$cid = CRM_Utils_Request::retrieve('cid', 'Positive', $this, FALSE, 0);
// Using a placeholder for criteria as it is intended to be able to pass this later.
$criteria = array();

$criteria = CRM_Utils_Request::retrieve('criteria', 'Json', $this, FALSE, '{}');
$this->assign('criteria', $criteria);

$isConflictMode = ($context == 'conflicts');
if ($cid) {
$this->_cid = $cid;
Expand All @@ -79,8 +81,10 @@ public function run() {
'rgid' => $rgid,
'gid' => $gid,
'limit' => $limit,
'criteria' => $criteria,
);
$this->assign('urlQuery', CRM_Utils_System::makeQueryString($urlQry));
$criteria = json_decode($criteria, TRUE);

if ($context == 'search') {
$context = 'search';
Expand Down
15 changes: 10 additions & 5 deletions CRM/Contact/Page/DedupeMerge.php
Original file line number Diff line number Diff line change
Expand Up @@ -52,22 +52,26 @@ public function run() {
* Build a queue of tasks by dividing dupe pairs in batches.
*/
public static function getRunner() {

$rgid = CRM_Utils_Request::retrieveValue('rgid', 'Positive');
$gid = CRM_Utils_Request::retrieveValue('gid', 'Positive');
$limit = CRM_Utils_Request::retrieveValue('limit', 'Positive');
$action = CRM_Utils_Request::retrieveValue('action', 'String');
$mode = CRM_Utils_Request::retrieveValue('mode', 'String', 'safe');

$cacheKeyString = CRM_Dedupe_Merger::getMergeCacheKeyString($rgid, $gid);
$criteria = CRM_Utils_Request::retrieve('criteria', 'Json', $null, FALSE, '{}');

$urlQry = array(
'reset' => 1,
'action' => 'update',
'rgid' => $rgid,
'gid' => $gid,
'limit' => $limit,
'criteria' => $criteria,
);

$criteria = json_decode($criteria, TRUE);
$cacheKeyString = CRM_Dedupe_Merger::getMergeCacheKeyString($rgid, $gid, $criteria);

if ($mode == 'aggressive' && !CRM_Core_Permission::check('force merge duplicate contacts')) {
CRM_Core_Session::setStatus(ts('You do not have permission to force merge duplicate contact records'), ts('Permission Denied'), 'error');
CRM_Utils_System::redirect(CRM_Utils_System::url('civicrm/contact/dedupefind', $urlQry));
Expand Down Expand Up @@ -101,7 +105,7 @@ public static function getRunner() {
for ($i = 1; $i <= ceil($total / self::BATCHLIMIT); $i++) {
$task = new CRM_Queue_Task(
array('CRM_Contact_Page_DedupeMerge', 'callBatchMerge'),
array($rgid, $gid, $mode, self::BATCHLIMIT, $isSelected),
array($rgid, $gid, $mode, self::BATCHLIMIT, $isSelected, $criteria),
"Processed " . $i * self::BATCHLIMIT . " pair of duplicates out of " . $total
);

Expand Down Expand Up @@ -131,11 +135,12 @@ public static function getRunner() {
* 'safe' mode or 'force' mode.
* @param int $batchLimit
* @param int $isSelected
* @param array $criteria
*
* @return int
*/
public static function callBatchMerge(CRM_Queue_TaskContext $ctx, $rgid, $gid, $mode = 'safe', $batchLimit, $isSelected) {
CRM_Dedupe_Merger::batchMerge($rgid, $gid, $mode, $batchLimit, $isSelected);
public static function callBatchMerge(CRM_Queue_TaskContext $ctx, $rgid, $gid, $mode = 'safe', $batchLimit, $isSelected, $criteria) {
CRM_Dedupe_Merger::batchMerge($rgid, $gid, $mode, $batchLimit, $isSelected, $criteria);
return CRM_Queue_Task::TASK_SUCCESS;
}

Expand Down
39 changes: 39 additions & 0 deletions CRM/Utils/Rule.php
Original file line number Diff line number Diff line change
Expand Up @@ -813,6 +813,25 @@ public static function xssString($value) {
}
}

/**
* Validate json string for xss
*
* @param string $value
*
* @return bool
* False if invalid, true if valid / safe.
*/
public static function json($value) {
if (!self::xssString($value)) {
return FALSE;
}
$array = json_decode($value, TRUE);
if (!$array || !is_array($array)) {
return FALSE;
}
return self::arrayValue($array);
}

/**
* @param $path
*
Expand Down Expand Up @@ -942,4 +961,24 @@ public static function checkExtesnionKeyIsValid($key = NULL) {
return TRUE;
}

/**
* Validate array recursively checking keys and values.
*
* @param array $array
* @return bool
*/
protected static function arrayValue($array) {
foreach ($array as $key => $item) {
if (is_array($item)) {
if (!self::xssString($key) || !self::arrayValue($item)) {
return FALSE;
}
}
if (!self::xssString($key) || !self::xssString($item)) {
return FALSE;
}
}
return TRUE;
}

}
7 changes: 7 additions & 0 deletions CRM/Utils/Type.php
Original file line number Diff line number Diff line change
Expand Up @@ -422,6 +422,7 @@ public static function validate($data, $type, $abort = TRUE, $name = 'One of par
'MysqlOrderByDirection',
'MysqlOrderBy',
'ExtensionKey',
'Json',
);
if (!in_array($type, $possibleTypes)) {
if ($isThrowException) {
Expand Down Expand Up @@ -530,6 +531,12 @@ public static function validate($data, $type, $abort = TRUE, $name = 'One of par
return $data;
}
break;

case 'Json':
if (CRM_Utils_Rule::json($data)) {
return $data;
}
break;
}

if ($abort) {
Expand Down
5 changes: 4 additions & 1 deletion templates/CRM/Contact/Page/DedupeFind.tpl
Original file line number Diff line number Diff line change
Expand Up @@ -324,14 +324,17 @@
var is_selected = CRM.$('.crm-dedupe-select-all').prop('checked') ? 1 : 0;
}
var criteria = {/literal}'{$criteria|escape}'{literal};
criteria = criteria.length > 0 ? criteria : 0;
var dataUrl = {/literal}"{crmURL p='civicrm/ajax/toggleDedupeSelect' h=0 q='snippet=4'}"{literal};
var rgid = {/literal}"{$rgid}"{literal};
var gid = {/literal}"{$gid}"{literal};
rgid = rgid.length > 0 ? rgid : 0;
gid = gid.length > 0 ? gid : 0;
CRM.$.post(dataUrl, {pnid: id, rgid: rgid, gid: gid, is_selected: is_selected}, function (data) {
CRM.$.post(dataUrl, {pnid: id, rgid: rgid, gid: gid, is_selected: is_selected, criteria : criteria}, function (data) {
// nothing to do for now
}, 'json');
}
Expand Down

0 comments on commit 021a48b

Please sign in to comment.