From 3e13db6ddd1275dc885a01fb4f53b1eb01e9d8e0 Mon Sep 17 00:00:00 2001 From: Seamus Lee Date: Wed, 11 Dec 2019 07:07:57 +1100 Subject: [PATCH] security/core#10 Ensure there is CSRF Protection when running Scheduled Jobs from the Admin scheduled jobs UI --- CRM/Admin/Form/Job.php | 25 +++++++++++++++++++++++++ CRM/Admin/Page/Job.php | 26 +++----------------------- templates/CRM/Admin/Form/Job.tpl | 4 ++-- templates/CRM/Admin/Page/Job.tpl | 2 +- 4 files changed, 31 insertions(+), 26 deletions(-) diff --git a/CRM/Admin/Form/Job.php b/CRM/Admin/Form/Job.php index 3009a5b730..1a1c4fb88b 100644 --- a/CRM/Admin/Form/Job.php +++ b/CRM/Admin/Form/Job.php @@ -55,6 +55,22 @@ class CRM_Admin_Form_Job extends CRM_Admin_Form { return; } + if ($this->_action & CRM_Core_Action::VIEW) { + $this->addButtons([ + [ + 'type' => 'submit', + 'name' => ts('Execute'), + 'isDefault' => TRUE, + ], + [ + 'type' => 'cancel', + 'name' => ts('Cancel'), + ], + ]); + return; + } + + $attributes = CRM_Core_DAO::getAttribute('CRM_Core_DAO_Job'); $this->add('text', 'name', ts('Name'), @@ -172,6 +188,15 @@ class CRM_Admin_Form_Job extends CRM_Admin_Form { return; } + // using View action for Execute. Doh. + if ($this->_action & CRM_Core_Action::VIEW) { + $jm = new CRM_Core_JobManager(); + $jm->executeJobById($this->_id); + CRM_Core_Session::setStatus(ts('Selected Scheduled Job has been executed. See the log for details.'), ts("Executed"), "success"); + CRM_Utils_System::redirect(CRM_Utils_System::url('civicrm/admin/job', 'reset=1')); + return; + } + $values = $this->controller->exportValues($this->_name); $domainID = CRM_Core_Config::domainID(); diff --git a/CRM/Admin/Page/Job.php b/CRM/Admin/Page/Job.php index d2c1f0113c..2ef02df9c6 100644 --- a/CRM/Admin/Page/Job.php +++ b/CRM/Admin/Page/Job.php @@ -58,10 +58,10 @@ class CRM_Admin_Page_Job extends CRM_Core_Page_Basic { 'qs' => 'action=update&id=%%id%%&reset=1', 'title' => ts('Edit Scheduled Job'), ), - CRM_Core_Action::EXPORT => array( + CRM_Core_Action::VIEW => array( 'name' => ts('Execute Now'), 'url' => 'civicrm/admin/job', - 'qs' => 'action=export&id=%%id%%&reset=1', + 'qs' => 'action=view&id=%%id%%&reset=1', 'title' => ts('Execute Scheduled Job Now'), ), CRM_Core_Action::DISABLE => array( @@ -111,19 +111,6 @@ class CRM_Admin_Page_Job extends CRM_Core_Page_Basic { ); CRM_Utils_System::appendBreadCrumb($breadCrumb); - $this->_id = CRM_Utils_Request::retrieve('id', 'String', - $this, FALSE, 0 - ); - $this->_action = CRM_Utils_Request::retrieve('action', 'String', - $this, FALSE, 0 - ); - - // FIXME: Why are we comparing an integer with a string here? - if ($this->_action == 'export') { - $session = CRM_Core_Session::singleton(); - $session->pushUserContext(CRM_Utils_System::url('civicrm/admin/job', 'reset=1')); - } - if (($this->_action & CRM_Core_Action::COPY) && (!empty($this->_id))) { try { $jobResult = civicrm_api3('Job', 'clone', array('id' => $this->_id)); @@ -151,14 +138,6 @@ class CRM_Admin_Page_Job extends CRM_Core_Page_Basic { CRM_Core_Session::setStatus(ts('Execution of scheduled jobs has been turned off by default since this is a non-production environment. You can override this for particular jobs by adding runInNonProductionEnvironment=TRUE as a parameter.'), ts("Non-production Environment"), "warning", array('expires' => 0)); } - // using Export action for Execute. Doh. - if ($this->_action & CRM_Core_Action::EXPORT) { - $jm = new CRM_Core_JobManager(); - $jm->executeJobById($this->_id); - - CRM_Core_Session::setStatus(ts('Selected Scheduled Job has been executed. See the log for details.'), ts("Executed"), "success"); - } - $sj = new CRM_Core_JobManager(); $rows = $temp = array(); foreach ($sj->jobs as $job) { @@ -174,6 +153,7 @@ class CRM_Admin_Page_Job extends CRM_Core_Page_Basic { $action -= CRM_Core_Action::ENABLE; } else { + $action -= CRM_Core_Action::VIEW; $action -= CRM_Core_Action::DISABLE; } diff --git a/templates/CRM/Admin/Form/Job.tpl b/templates/CRM/Admin/Form/Job.tpl index 3c319aba56..98c05f30b5 100644 --- a/templates/CRM/Admin/Form/Job.tpl +++ b/templates/CRM/Admin/Form/Job.tpl @@ -8,7 +8,7 @@ +--------------------------------------------------------------------+ *} {* This template is used for adding/configuring Scheduled Jobs. *} -

{if $action eq 1}{ts}New Scheduled Job{/ts}{elseif $action eq 2}{ts}Edit Scheduled Job{/ts}{elseif $action eq 128}{ts}Execute Scheduled Job{/ts}{else}{ts}Delete Scheduled Job{/ts}{/if}

+

{if $action eq 1}{ts}New Scheduled Job{/ts}{elseif $action eq 2}{ts}Edit Scheduled Job{/ts}{elseif $action eq 4}{ts}Execute Scheduled Job{/ts}{else}{ts}Delete Scheduled Job{/ts}{/if}

{include file="CRM/common/formButtons.tpl" location="top"}
@@ -17,7 +17,7 @@
{ts}WARNING: Deleting this Scheduled Job will cause some important site functionality to stop working.{/ts} {ts}Do you want to continue?{/ts}
-{elseif $action eq 128} +{elseif $action eq 4}
{ts}Are you sure you would like to execute this job?{/ts} diff --git a/templates/CRM/Admin/Page/Job.tpl b/templates/CRM/Admin/Page/Job.tpl index 94856e34ae..c0812feff1 100644 --- a/templates/CRM/Admin/Page/Job.tpl +++ b/templates/CRM/Admin/Page/Job.tpl @@ -12,7 +12,7 @@ {ts 1=$runAllURL}You can configure scheduled jobs (cron tasks) for your CiviCRM installation. For most sites, your system administrator should set up one or more 'cron' tasks to run the enabled jobs. However, you can also run all scheduled jobs manually, or run specific jobs from this screen (click 'more' and then 'Execute Now').{/ts} {docURL page="sysadmin/setup/jobs" text="(Job parameters and command line syntax documentation...)"}
-{if $action eq 1 or $action eq 2 or $action eq 8} +{if $action eq 1 or $action eq 2 or $action eq 8 or $action eq 4} {include file="CRM/Admin/Form/Job.tpl"} {else} -- 2.25.1