Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

drush civicrm-install now accepts optional site_key #97

Open
wants to merge 5 commits into
base: 1.x-master
Choose a base branch
from
Open

drush civicrm-install now accepts optional site_key #97

Changes from 1 commit
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
06b23ef
drush civicrm-install now accepts optional site_key
tabroughton Aug 20, 2019
5d008dc
now the same sitekey gen function as is used in civicrm_setup
tabroughton Aug 21, 2019
bb268aa
removed white space
tabroughton Aug 21, 2019
7508675
reduced indent
tabroughton Aug 21, 2019
ed45887
removed white space at end of line
tabroughton Aug 21, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 6 additions & 1 deletion drush/civicrm.drush.inc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,7 @@ function civicrm_drush_command() {
'site_url' => 'Base Url for your backdrop/CiviCRM website without http (e.g. mysite.com)',
'ssl' => 'Using ssl for your backdrop/CiviCRM website if set to on (e.g. --ssl=on)',
'load_generated_data' => 'Loads the demo generated data. Defaults to FALSE.',
'site_key' => 'Define a site key, suggest md5 hash. Otional, install will generate if not present',
),
'aliases' => array('cvi'),
);
Expand Down Expand Up @@ -512,6 +513,10 @@ function _civicrm_generate_settings_file($dbuser, $dbpass, $dbhost, $dbname, $mo
$cms = $v['cms'];
}

if (!$sitekey = drush_get_option('site_key', FALSE)) {
$sitekey = md5(uniqid('', TRUE) . $baseUrl);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is tangential, but reading this code reminds me... uniqid() function is not a great source of entropy. I think this snippet of code probably originated in a much earlier iteration of civicrm-core:install/civicrm.php. That was a changed a while back (a few years ago?), so I'd suggest copying a more recent snippet, i.e.

Suggested change
$sitekey = md5(uniqid('', TRUE) . $baseUrl);
$sitekey = md5(rand() . mt_rand() . rand() . uniqid('', TRUE) . $baseUrl);

(Long term, we should swap this over to civicrm-setup so that code is not replicated as much; and we should update the shared code to prioritize better sources of entropy when available. But that's a bigger scope. The suggestion here is just sync-up with the current install/civicrm.php.)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes I agree longer term this would be good to reuse code rather than replicate it, I'd noted that when I was looking at some other issues a few weeks ago (it puzzled me slightly). I'll add the code snippet you suggested and fix the few code style issues. The function I used was the one already in the drush script. I think an overhaul of this file would be useful at some stage, it's quite messy and will be difficult to maintain.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've added the new sitekey gen function (copied from civicrm_setup as suggested) - I should have commited the suggestion above but didn't see that button until afterwards.

I've run tests without the parameter, with the parameter but no value and with the parameter and a value and they all work as expected.

}

$params = array(
'crmRoot' => $crmPath,
'templateCompileDir' => "$backdropRoot/$siteRoot/files/civicrm/templates_c",
Expand All @@ -526,7 +531,7 @@ function _civicrm_generate_settings_file($dbuser, $dbpass, $dbhost, $dbname, $mo
'CMSdbPass' => $db_spec['password'],
'CMSdbHost' => $db_spec['host'],
'CMSdbName' => $db_spec['database'],
'siteKey' => md5(uniqid('', TRUE) . $baseUrl),
'siteKey' => $sitekey,
);
$str = file_get_contents($settingsTplFile);
foreach ($params as $key => $value) {
Expand Down