Details
Major
Description
Please log hours on this issue to http://dev.nysenate.gov/issues/7723
======================================================
1. Create a new column called hash in civicrm_mailing. Copy the format of the civicrm_contact hash column
a. All new mailing creation populates this hash column. The hash should follow the same pattern of generating the contact hash, but is restricted to 16 characters. Please check and centralize this code (send in a parameter for the hash size)
b. Updates of Mailing record should check and populate the hash column if null.
c. The upgrade script keeps this column null
2. Create a new setting called:
"Hashed Mailing URL's" in the civimail settings screen (http://drupal.demo.civicrm.org/civicrm/admin/mail?reset=1)
Setting is BOOLEAN, setting field is a checkbox. The default value for this setting is FALSE.
3. When user changes value of this field to CHECKED, display jScript confirmation dialog:
"WARNING: If you enable hashed mailing URLs, users will no longer be able to view new or updated mailings using the mailing ID In the URL. If you want users to 'view this mailing in a browser', then use the mailing.viewURL token to include links with a valid hashed URL in your mailings."
4. If this setting is False, we retain current behavior
5. If this setting is True, we modify:
a. The mailing.viewURL token to now uses hash value instead of the mailing ID in the id= key of the querystring (the mailing/view url stays the same, but the value of the 'id' key is the hash).
b. In the mailing view page code, if the setting is enabled, use the hash to find the relevant mailing id. If not found, throw a permission denied error. If found, render the mailing view.
NOTE: In order to accommodate mailings sent prior to the setting being enabled, we will render mailing view via ID for mailings where the hash is NULL. If SELECT via id=hash returns empty set, then query on id=id. If we find a mailing and mailing.hash is NULL then render the mailing view.
Attachments
Activity
The spec'ed pattern is similar to the contact checksum and hence i modelled it that way. It also avoids the use of a DB column
In the create view URL case, we have id and secret_key and we generate the hash
In the view mailing case, we get the id and hash from the url and regenerate the hash and check for a match
Not sure that masking the mailingID improves security, but it does avoid introducing a new column 
Submitted PR : https://github.com/civicrm/civicrm-core/pull/2861
just a few things:
- I think the setting should be in the CiviMail component settings page, not the mailer page. it has more to do with the component config than the mailer itself. and we already have options related to tokens in that page, so it fits better.
- along with the setting we should add some inline help text explaining the option.
other than that, it looks good. I tested it with a new email for which the hash was constructed, and also with an old email, which didn't have a hash. the id correctly works for the old email but not the new one.
Hi Brian,done modifications you suggested https://github.com/pratik-joshi/civicrm-core/commit/19eb83615b602cfc63035ef00065d65a641e574c (they are contained in the same PR #2861)
this looks good. but let's change the help text to the following:
If enabled, a randomized hash key will be used to reference the mailing URL in the mailing.viewUrl token, instead of the mailing ID.
I think you need to modify this file as well:
https://github.com/civicrm/civicrm-core/blob/4.4/settings/Mailing.setting.php
done. updated PR #2861 .
just ran into an unexpected issue –
the mailing/view url is used on the mailing approval page. but the code to retrieve it there still assumes the use of the id. if hash url is enabled, it fails to load on that page.
we may want to do a grep to see if the mailing/view is used in any other places as well.
I found three places we need to implement the alternate hash url:
CRM_Mailing_BAO_Mailing::getContactMailingSelector
CRM_Mailing_Form_Approve::buildQuickForm
CRM_Mailing_Form_Schedule::buildQuickForm
Hi Brian, done with above fixes, also includes one more place inside CRM_Mailing_BAO_Mailing. PR #2861 updated.
another issue –
the
token retrieves the content of the email. it's used by the rules integration by running CRM_Mailing_Page_View -> run() and passing the mailing ID.
in that method, we check to see if the mailing ID passed to the object is_numeric. if it is numeric, and the hash_mailing_url setting is enabled, we exit with a permission error.
that's the correct behavior in most circumstances – we want to prevent access via id if the hash option is enabled. but in this case, we either need to override that behavior and allow the id to be passed (perhaps a fourth parameter for that method that allows an ID override), or we need to pass the ID as a $_REQUEST variable rather than via a parameter (not great in my opinion).
here is a suggested fix:
https://github.com/nysenate/Bluebird-CRM/commit/e323ab5715f5ff4953aa547c59dbe986c1c60639
submitted a new PR : https://github.com/civicrm/civicrm-core/pull/2934 , this also consists patching suggested fix by Brian for #comment-59808 fix.
you're missing the && at the end of this line:
https://github.com/civicrm/civicrm-core/pull/2934/files#diff-32c20a7977f94eb51b6eb6f0da74153cR111
sorry for such a silly mistake, missed this commit to push.
updated PR.
could we alter the specs a bit, so that the hash is a replacement for the mailing id, not in addition to? that way the mailing ID is completely masked.
so the url to view the mailing would be identical, except with the hash enabled, the &id=VALUE would have the hash instead of the id.
structurally I'm thinking that in the _mailing table we add a column for the hash, and so that is available anytime we retrieve the mailing DAO, and the queries to retrieve the mailing details for the view page will just condition on the id column or the hash column (the hash column can be a second unique index).
I'm looking to make this as simple and streamlined as possible.