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.

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

Fix generating civicrm.settings.php by ensuring that CMSdbSSL and dbS… #63

Merged

Conversation

seamuslee001
Copy link
Contributor

…SL variables are properly removed from the DSNs

Doing a recent upgrade of a Joomla site I found that the %%CMSdbSSL%% and %%dbSSL%% was still printed at the end of the DSNs

This should remove it and there doesn't seem to be a way to determine if we are using ssl in Joomla but this should be a reasonable intermediate step

ping @lcdservices @totten @demeritcowboy

…SL variables are properly removed from the DSNs
@lcdservices
Copy link
Contributor

Tested the patch and it works as expected -- the placeholder values are no longer appended to the DSN when Civi is installed.

Re: determining if Joomla is using SSL -- Joomla's configuration.php file has a variable $force_ssl that indicates if the site should be forced to used SSL. A value of 1 or 2 would indicate it is. However -- that setting affects the application server, and we can't assume it also applies to the DB server (in cases where they are different). I'm not aware of anything that would let us know with certainty if the DB server is running over SSL.

@demeritcowboy
Copy link
Contributor

Thanks. It seems when I added the dbSSL stuff for the newer civicrm-setup installer it also causes the same variable to be left unreplaced when using the "legacy" installer for other CMSes.
So this makes sense and I'll also put up a patch for the "legacy" installer which should be something similar.

Regarding detecting using SSL for mysql, the way the civicrm-setup installer does it is to "guess" if Drupal is already doing it based on Drupal's config, and it's only implemented for Drupal/Backdrop. The guess was just meant as a convenience because depending on what type of certificates you're using you might have to do some manual config anyway. So I think leaving out any guessing here is fine.

@totten
Copy link
Member

totten commented Oct 1, 2021

when I added the dbSSL stuff for the newer civicrm-setup installer it also causes the same variable to be left unreplaced when using the "legacy" installer

Yeah, it's annoying to track down and test each of them. Relatedly, that's why the %%signKeys%% and %%credKeys%% go through this funny dance: https://github.com/civicrm/civicrm-core/blob/dedd04aad15342fa14fb6052bfbed32fc2269321/templates/CRM/common/civicrm.settings.php.template#L319-L320

@demeritcowboy
Copy link
Contributor

Yeah ok maybe doing it in the template isn't too messy. Will take a look.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants