search

silverstripe / silverstripe-framework

Silverstripe Framework, the MVC framework that powers Silverstripe CMS
https://www.silverstripe.org
BSD 3-Clause "New" or "Revised" License
722 stars 821 forks source link

ForceSSL in `cwpsecurity` yaml block interferes with custom use of `Director::forceSSL()` #8925

Open jakedaleweb opened 5 years ago

jakedaleweb commented 5 years ago

When using this part of the recipe on a cwp site, if a user sets Director::forceSSL() in their _config.php, for example, their site will not redirect to https on all URLs.

This is because this yaml block https://github.com/silverstripe/cwp-core/blob/master/_config/security.yml#L25 sets ForceSSLPatterns. In order to force SSL with Director a CWP user would have to set the following yaml:

Name: whatever
After: cwpsecurity
---
SilverStripe\Core\Injector\Injector:
  SilverStripe\Control\Middleware\CanonicalURLMiddleware:
    properties:
      ForceSSL: true
      ForceSSLPatterns: null

This isn't necessarily a bug, because the code is doing what it should, but the documentation probably needs to be updated to clarify why Director::forceSSL() doesn't work in _config.php as people would usually expect.

robbieaverill commented 5 years ago

Ideally we'd support it there as well, but encourage people to configure their SSL rules in YAML (or htaccess if using Apache). This could probably be moved to a framework issue, since CanonicalURLMiddleware appears not to be compatible with Director::forceSSL().

scott1702 commented 5 years ago

Might also be best to update the docs to opt for the yaml configuration instead of a change in php

NightJar commented 5 years ago

That's the goal that's spurred the investigation resulting in this issue @scott1702 :)