Fixes #3255: On Acquia Cloud, acquia_config.php should better handle multisite

[anavarre]

Fixes #3255

Changes proposed

• Place the multisite-friendly code right where it belongs, under elseif (getenv('AH_SITE_ENVIRONMENT'))

Steps to replicate the issue

1. It's not easy to replicate the issue. In multisite envs, there are instances where port 443 will be missing from the constructed URL and give a permission denied without any easy way to get to the bottom of the issue. When this happens you need to know about and leverage the SAML Chrome Panel extension to find out port 443 isn't being passed, thus causing the issue.

Previous behavior (before applying PR)

Port 443 could be missing from the constructed URL.

Expected behavior (after applying PR)

Port 443 is always being passed in the constructed URL.

Additional details

Please review carefully before merging. There might be a need to simply duplicate the baseurlpath config override in and out of the AH_SITE_ENVIRONMENT if statement to account for non-Acquia hosted users.

[danepowell]

@lcatlett or @shanefjordan can you please review? Would you consider this best practice for multisites using SAML?

[danepowell]

This needs to be removed or it will clobber what you've defined above, right?

[anavarre]

I'm not sure I understand what you mean. I've just moved around the $config['baseurlpath'] = $protocol . $_SERVER['HTTP_HOST'] . $port . '/simplesaml/'; line which fixed the missing port 443 in the URL for a multisite we have.

I'm not saying this is best practice or the right fix for BLT, but it's what worked for us. It was really not obvious to understand it had to be within the elseif (getenv('AH_SITE_ENVIRONMENT')) block thus why putting it here - assuming it's where it should belong by default - looked best to prevent others from running into the issue in the future.

[danepowell]

I'm talking about this line, three lines below the code that you inserted: $config['baseurlpath'] = 'simplesaml/';

That will clobber the value of baseurlpath that you just assigned.

[lcatlett]

Port 443 should only be explicitly required in this config when deliberately manipulating server variables for HTTPS and SERVER_PORT when SSL is terminated at the ELB/balancer - this should already be done in BLT https://github.com/acquia/blt/blob/10.x/settings/simplesamlphp.settings.php#L15-L17 so it seems that something else is off with this config.

I think it is more likely that host forwarding is misconfigured in the BLT config which is causing issues on both multisite and single site implementations, it may just be more obvious on multisite since the uri of an individual multisite is usually different than the absolute url of the saml SP. This should likely be tested/validated in the BLT Travis build, as it currently only tests that the saml config files were copied to the correct place rather than if the saml settings being used in BLT actually work... @danepowell do you agree?

[timcosgrove]

Noting here that $protocol and $port are not defined unless the code block above is uncommented. This was also true of the code prior to this PR.

[danepowell]

As far as I can tell this will have no functional change because $config['baseurlpath'] gets overwritten to be /simplesaml regardless. So if people find that this is somehow more helpful I guess I'm fine with that.