Create secure request when site port is 443 or SECURE_SSL_REDIRECT is set to True #25
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
loicteixeira
commented
Feb 3, 2018
| @@ -88,8 +88,9 @@ def build_object(self, obj): | |||
| """ | |||
| site = obj.get_site() | |||
| logger.debug("Building %s" % obj) | |||
| secure_request = site.port == 443 or getattr(settings, 'SECURE_SSL_REDIRECT', False) | |||
There was a problem hiding this comment.
I decided to also check for the site port. I'll add some tests later if you agree with the change.
There was a problem hiding this comment.
Eh.. It appears to be breaking some tests, but only for some versions of Python.. Anyway, I'll wait for your feedback before looking at it.
There was a problem hiding this comment.
Thanks for the notice @loicteixeira . In which case are you trying to build pages via ssl? Almost every site runs in http modus, as ssl offloading is done on LB level or page building is done on local machine. In this really necessary?
There was a problem hiding this comment.
The app is hosted on Heroku and as per their requirement, the SSL redirection needs to be performed at the application level.
There was a problem hiding this comment.
@robmoorman Any further thoughts on the approach then? Maybe you would prefer it to be a BAKERY_SECURE_REQUESTS setting which defaults to False instead of the automatic detection?
|
I'll use a custom view on the app instead. Thanks. |
stevejalim
pushed a commit
to mdn/developer-portal
that referenced
this pull request
Sep 23, 2019
…bakes out empty HTML This commit subclasses wagtail-bakery's `AllPublishedPagesView` in a way that detects application-level SSL redirection in order to avoid an issue where rendered pages end up being 0 bytes. See wagtail-nest/wagtail-bakery#24 for confirmation of the issue and the discussion on wagtail-nest/wagtail-bakery#25 that points to a custom view being the (current) workaround. Ideally we'll be able to replace this when that issue is resolved. The code in this commit is basically taken from that closed PR, which adds the `secure_request` variable. Hat-tip to @loicteixeira - thanks! No unit test added, but manually tested locally to confirm this does indeed fix the static build while `SECURE_SSL_REDIRECT` is `True`
stevejalim
pushed a commit
to mdn/developer-portal
that referenced
this pull request
Sep 25, 2019
* 22: Enable recommended-by-Django security settings as defaults The base settings now follow all-but-one of the recommendations from manage.py `check --deploy`. The only one that's not been done in this changeset is SECURE_HSTS_SECONDS, because of the risk of "serious, irreversible problems". That needs to be planned in to enable it properly. Note that we're explicitly setting what HTTP header to look for to detect the SSL-forwarded header, which should stop the redirect loop in production. Development settings turn off SSL redirect, because the local build isn't set to use HTTPS. (It could be tweaked to use HTTPS, too - but that's out of scope for this piece) * 22: Address issue with wagtail-bakery where SECURE_SSL_REDIRECT=True bakes out empty HTML This commit subclasses wagtail-bakery's `AllPublishedPagesView` in a way that detects application-level SSL redirection in order to avoid an issue where rendered pages end up being 0 bytes. See wagtail-nest/wagtail-bakery#24 for confirmation of the issue and the discussion on wagtail-nest/wagtail-bakery#25 that points to a custom view being the (current) workaround. Ideally we'll be able to replace this when that issue is resolved. The code in this commit is basically taken from that closed PR, which adds the `secure_request` variable. Hat-tip to @loicteixeira - thanks! No unit test added, but manually tested locally to confirm this does indeed fix the static build while `SECURE_SSL_REDIRECT` is `True`
William-Blackie
pushed a commit
to William-Blackie/Wagtail-Backery-Squash-POC
that referenced
this pull request
Aug 3, 2020
Amending initial settings
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fix #24 (Generated pages are blank when SECURE_SSL_REDIRECT is enabled).