Details
-
Type:
Bug
-
Status: Done/Fixed
-
Priority:
Trivial
-
Resolution: Won't Fix
-
Affects Version/s: Unscheduled
-
Fix Version/s: Unscheduled
-
Component/s: None
-
Labels:None
-
Versioning Impact:Patch (backwards-compatible bug fixes)
-
Documentation Required?:Developer Doc
-
Funding Source:Needs Funding
Description
We have had a request for the 'frontendpageoptions' extension ( https://github.com/eileenmcnaughton/nz.co.fuzion.frontendpageoptions ) to support tokens in the redirect url. The extension allows you to specify a url to redirect the user to when they would otherwise wind up on the CiviCRM thank you page. A common use of this is to redirect to a web-form or profile to gather additional information. However, for that to work for a non-logged in user the url needs to include a checksum & cid. Adding these seems to be more painful than it needs to be!
It seems that if the extension could call the following it would make it easier
$result = civicrm_api3('MessageTemplate', 'rendertokens', array(
'check_permissions' => 0,
'input_strings' = array($url),
'whitelist' = array('cid', 'checksum',
);
$replacedUrl = $result['values']['0]['output_strings'][0]
I did some pseudocode for this
https://github.com/eileenmcnaughton/civicrm-core/commit/57c412ccbba8ac011218345cc6c259d247b54ca7
based on https://github.com/civicrm/civicrm-core/blob/master/CRM/Core/BAO/MessageTemplate.php#L181
It did occur to me that there is a potential issue in this logic whereby we want to give a non-logged in user the ability to get some tokens rendered & I wasn't sure whether that could be exploitable. (The thankyou url is saved when configuring the contribution_page so it's not accessible to edit but it is based on user input). I added the whitelist to the example above (but not the pseudocode at this stage. Of course the whitelist imposes limitations that may not be desirable & it is no more open than the configuration of the message templates themselves so I'm not sure if it is of value or not in this example
Attachments
Activity
yeah - that is nicer! But, would we just expose that through apiv3 - or extend the 'api' to include other (class-based) interfaces. The key distinction between the api & other code is the test-contract - ie. if it's tested then the functionality, inputs & outputs won't change.
I am worried about putting more and more business logic in the API layer. All business logic should be centralized in the BAO layer, and the API (and any other piece of code) call a BAO function rather than implement their own logic. Right now there are already many places where we are doing token substitution in Civi, and it seems these have slightly different logic every time. You might just be adding yet another one.
So: yes on exposing a public interface, but the implementation should be in the BAO and the API just implement arguments checks/mapping. A sanity test for this BAO function is then if you can get any other code in CiviCRM to reuse it, therefore homogenizing & simplifying the code base.
Looks like this wound up in the too-hard basket a long time ago!
Re: pseudocode – I'm not keen to have new features based on iterative replacement via CRM_Utils_Token::replaceFoo(). Better formulation: http://wiki.civicrm.org/confluence/display/CRMDOC/Token+Reference