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.

Sign up for GitHub

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

Create secure request when site port is 443 or SECURE_SSL_REDIRECT is set to True #25

Closed
wants to merge 1 commit into from
Closed

Create secure request when site port is 443 or SECURE_SSL_REDIRECT is set to True #25

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion src/wagtailbakery/views.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I decided to also check for the site port. I'll add some tests later if you agree with the change.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The app is hosted on Heroku and as per their requirement, the SSL redirection needs to be performed at the application level.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@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?

self.request = RequestFactory(
SERVER_NAME=site.hostname).get(self.get_url(obj))
SERVER_NAME=site.hostname).get(self.get_url(obj), secure=secure_request)
self.set_kwargs(obj)
path = self.get_build_path(obj)
self.build_file(path, self.get_content(obj))
Expand Down