diff --git a/CRM/Contribute/Form/ContributionBase.php b/CRM/Contribute/Form/ContributionBase.php index 82767a072cd1..b423a4b41ca0 100644 --- a/CRM/Contribute/Form/ContributionBase.php +++ b/CRM/Contribute/Form/ContributionBase.php @@ -138,8 +138,6 @@ class CRM_Contribute_Form_ContributionBase extends CRM_Core_Form { */ public $_contactID; - protected $_userID; - /** * The Membership ID for membership renewal * @@ -293,9 +291,6 @@ public function preProcess() { } $this->_emailExists = $this->get('emailExists'); - // this was used prior to the cleverer this_>getContactID - unsure now - $this->_userID = CRM_Core_Session::getLoggedInContactID(); - $this->_contactID = $this->_membershipContactID = $this->getContactID(); $this->getRenewalMembershipID(); @@ -307,7 +302,7 @@ public function preProcess() { $this->_defaultMemTypeId = $membership->membership_type_id; if ($membership->contact_id != $this->_contactID) { $validMembership = FALSE; - $organizations = CRM_Contact_BAO_Relationship::getPermissionedContacts($this->_userID, NULL, NULL, 'Organization'); + $organizations = CRM_Contact_BAO_Relationship::getPermissionedContacts($this->getAuthenticatedContactID(), NULL, NULL, 'Organization'); if (!empty($organizations) && array_key_exists($membership->contact_id, $organizations)) { $this->_membershipContactID = $membership->contact_id; $this->assign('membershipContactID', $this->_membershipContactID); @@ -321,7 +316,7 @@ public function preProcess() { // CRM-14051 - membership_type.relationship_type_id is a CTRL-A padded string w one or more ID values. // Convert to comma separated list. $inheritedRelTypes = implode(',', CRM_Utils_Array::explodePadded($membershipType->relationship_type_id)); - $permContacts = CRM_Contact_BAO_Relationship::getPermissionedContacts($this->_userID, $membershipType->relationship_type_id); + $permContacts = CRM_Contact_BAO_Relationship::getPermissionedContacts($this->getAuthenticatedContactID(), $membershipType->relationship_type_id); if (array_key_exists($membership->contact_id, $permContacts)) { $this->_membershipContactID = $membership->contact_id; $validMembership = TRUE; @@ -1122,8 +1117,11 @@ private function authenticatePledgeUser(): void { ]; $validUser = FALSE; - if ($this->_userID && - $this->_userID == $pledgeValues['contact_id'] + // @todo - override getRequestedContactID to add in checking pledge values, then + // getContactID will do all this. + $userID = CRM_Core_Session::getLoggedInContactID(); + if ($userID && + $userID == $pledgeValues['contact_id'] ) { //check for authenticated user. $validUser = TRUE; diff --git a/CRM/Core/Form.php b/CRM/Core/Form.php index e671b3f8b6db..eac61b71b1cd 100644 --- a/CRM/Core/Form.php +++ b/CRM/Core/Form.php @@ -198,6 +198,16 @@ class CRM_Core_Form extends HTML_QuickForm_Page { */ protected $exportedValues = []; + /** + * The contact ID that has been authenticated and can be used for checking permissions. + * + * It could be a combination of cid in the url plus a checksum or the logged in user. + * Importantly it can be used to run permission checks on. + * + * @var int + */ + private $authenticatedContactID; + /** * @return string */ @@ -2388,66 +2398,115 @@ public static function validateMandatoryFields($fields, $values, &$errors) { } /** - * Get contact if for a form object. Prioritise - * - cid in URL if 0 (on behalf on someoneelse) - * (@todo consider setting a variable if onbehalf for clarity of downstream 'if's - * - logged in user id if it matches the one in the cid in the URL - * - contact id validated from a checksum from a checksum - * - cid from the url if the caller has ACL permission to view - * - fallback is logged in user (or ? NULL if no logged in user) (@todo wouldn't 0 be more intuitive?) + * Get contact iD for a form object. + * + * This checks the requestedContactID and returns it if + * - it is the number 0 (relevant for online contribution & event forms). + * - it is the logged in user + * - it is validated by a checksum in the url. + * - it is a contact that the logged in user has permission to view + * + * Failing that it returns the logged in user, if any. This is may be useful + * for users taking actions from their contact dashboard (although usually one + * of the variants above would be hit). * * @return NULL|int + * + * @throws \CRM_Core_Exception */ protected function setContactID() { - $tempID = CRM_Utils_Request::retrieve('cid', 'Positive', $this); - if (isset($this->_params) && !empty($this->_params['select_contact_id'])) { - $tempID = $this->_params['select_contact_id']; - } - if (isset($this->_params, $this->_params[0]) && !empty($this->_params[0]['select_contact_id'])) { - // event form stores as an indexed array, contribution form not so much... - $tempID = $this->_params[0]['select_contact_id']; - } + $requestedContactID = $this->getRequestedContactID(); // force to ignore the authenticated user - if ($tempID === '0' || $tempID === 0) { + if ($requestedContactID === 0) { // we set the cid on the form so that this will be retained for the Confirm page // in the multi-page form & prevent us returning the $userID when this is called // from that page // we don't really need to set it when $tempID is set because the params have that stored $this->set('cid', 0); - CRM_Core_Resources::singleton()->addVars('coreForm', ['contact_id' => (int) $tempID]); - return (int) $tempID; + CRM_Core_Resources::singleton()->addVars('coreForm', ['contact_id' => $requestedContactID]); + return (int) $requestedContactID; } - $userID = CRM_Core_Session::getLoggedInContactID(); - - if (!is_null($tempID) && $tempID === $userID) { - CRM_Core_Resources::singleton()->addVars('coreForm', ['contact_id' => (int) $tempID]); - return (int) $userID; - } - - //check if this is a checksum authentication - $userChecksum = CRM_Utils_Request::retrieve('cs', 'String', $this); - if ($userChecksum) { - //check for anonymous user. - $validUser = CRM_Contact_BAO_Contact_Utils::validChecksum($tempID, $userChecksum); - if ($validUser) { - CRM_Core_Resources::singleton()->addVars('coreForm', ['contact_id' => (int) $tempID]); - CRM_Core_Resources::singleton()->addVars('coreForm', ['checksum' => $userChecksum]); - return $tempID; + if ($requestedContactID === $this->getAuthenticatedContactID()) { + CRM_Core_Resources::singleton()->addVars('coreForm', ['contact_id' => $requestedContactID]); + // Check if this is a checksum authentication. + if ($this->getAuthenticatedCheckSumContactID()) { + CRM_Core_Resources::singleton()->addVars('coreForm', ['checksum' => CRM_Utils_Request::retrieve('cs', 'String', $this)]); } + return $requestedContactID; } + // check if user has permission, CRM-12062 - elseif ($tempID && CRM_Contact_BAO_Contact_Permission::allow($tempID)) { - CRM_Core_Resources::singleton()->addVars('coreForm', ['contact_id' => (int) $tempID]); - return $tempID; + if ($requestedContactID && CRM_Contact_BAO_Contact_Permission::allow($requestedContactID)) { + CRM_Core_Resources::singleton()->addVars('coreForm', ['contact_id' => (int) $requestedContactID]); + return $requestedContactID; } + $userID = CRM_Core_Session::getLoggedInContactID(); if (is_numeric($userID)) { CRM_Core_Resources::singleton()->addVars('coreForm', ['contact_id' => (int) $userID]); } return is_numeric($userID) ? $userID : NULL; } + /** + * Get the contact ID that has been requested (via url or form value). + * + * Ideally the forms would override this so only the cid in the url + * would be checked in the shared form function. + * + * @return int + * @throws \CRM_Core_Exception + */ + public function getRequestedContactID(): ?int { + if (isset($this->_params) && !empty($this->_params['select_contact_id'])) { + return (int) $this->_params['select_contact_id']; + } + if (isset($this->_params, $this->_params[0]) && !empty($this->_params[0]['select_contact_id'])) { + // Event form stores as an indexed array, contribution form not so much... + return (int) $this->_params[0]['select_contact_id']; + } + $urlContactID = CRM_Utils_Request::retrieve('cid', 'Positive', $this); + return is_numeric($urlContactID) ? (int) $urlContactID : NULL; + } + + /** + * Get the authenticated contact ID. + * + * This is either + * - a contact ID authenticated by checksum + * - the logged in user + * - 0 for none. + * + * @return int + * + * @throws \CRM_Core_Exception + */ + public function getAuthenticatedContactID() : int { + if ($this->authenticatedContactID === NULL) { + $this->authenticatedContactID = $this->getAuthenticatedCheckSumContactID(); + if (!$this->authenticatedContactID) { + $this->authenticatedContactID = (int) CRM_Core_Session::getLoggedInContactID(); + } + } + return $this->authenticatedContactID; + } + + /** + * Get the contact ID authenticated as a valid by checksum. + * + * @return int + * @throws \CRM_Core_Exception + */ + protected function getAuthenticatedCheckSumContactID(): int { + $requestedContactID = $this->getRequestedContactID(); + $userChecksum = CRM_Utils_Request::retrieve('cs', 'String', $this); + if ($userChecksum && CRM_Contact_BAO_Contact_Utils::validChecksum($requestedContactID, $userChecksum)) { + return $requestedContactID; + } + return 0; + } + /** * Get the contact id that the form is being submitted for. *