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

Cannot deploy CWP 2.0 to test environments #98

Closed
robbieaverill opened this issue Jun 13, 2018 · 4 comments
Closed

Cannot deploy CWP 2.0 to test environments #98

robbieaverill opened this issue Jun 13, 2018 · 4 comments

Comments

@robbieaverill
Copy link
Contributor

robbieaverill commented Jun 13, 2018

The introduction of silverstripe/cwp-core@cf330de means that dev/build redirects to HTTPS, which prevents it from working.

Reproduce:

  • Set SS_ENVIRONMENT_TYPE to test
  • Run vendor/bin/sake dev/build

Fix options:

  1. Disable YAML configuration introduces in the commit above for test environments
  2. Release BUG Prevent canonical URL causing a redirect on CLI unless explicitly enabled silverstripe-framework#8158 in CWP 2.0.1
  3. Overload CanonicalURLMiddleware in user code and forward port this fix

The most ideal option here (option 2) is to release CWP 2.0.1 with framework 4.1.2 in it, but this will also bring other changes with it, unless we want to cherry pick and only release this single commit - this isn't a sustainable option though, we already did that for 4.1.1.

Note that we could take option 3 and overload the middleware in CWP only to forward port this fix. This would then be deprecated in CWP 2.1 and configuration for it removed - we could also mark the API as internal very explicitly and then remove it again in CWP 2.1 without as much of a strict semver obligation...

cc @jakedaleweb @chillu @tractorcow

@maxime-rainville
Copy link
Contributor

Silly idea here, but could we just disable ForceSSLPatterns if run from the CLI?

@robbieaverill
Copy link
Contributor Author

@maxime-rainville yeah that's done in silverstripe/silverstripe-framework#8158 but requires a new recipe to release

@robbieaverill
Copy link
Contributor Author

Ok have spoken on Slack with @chillu and @sminnee and agreed that the best solution for now is to overload CanonicalURLMiddleware in the cwp/cwp-core module and include the fix in silverstripe/silverstripe-framework#8158, then we can release a CWP 2.0.1 recipe which only differs in in that it contains this fix as well. This means we don't need to wait for a new SS 4.1.x recipe to be released, and don't introduce risk of commits that aren't security tested.

@robbieaverill
Copy link
Contributor Author

Fixed with silverstripe/cwp-core#36, we'll release a 2.0.1 recipe tomorrow morning

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

No branches or pull requests

4 participants