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

Feature request: Force SSL #889

Closed
Sommerregen opened this issue Jun 9, 2016 · 15 comments
Closed

Feature request: Force SSL #889

Sommerregen opened this issue Jun 9, 2016 · 15 comments

Comments

@Sommerregen
Copy link
Contributor

Unless changing .htaccess currently there is no option in Grav to force redirecting the user to a secured site.

@jjui
Copy link

jjui commented Jun 13, 2016

Did it from htaccess but forcing ssl from within grav would be great.

@Sogl
Copy link
Contributor

Sogl commented Jun 13, 2016

+1

@rhukster
Copy link
Member

In Grav 1.1 you can already force ssl on at the page level. Just by setting ssl: true in the page frontmatter. I don't think this has made it to the admin yet. For this to work you need to have absolute_urls: true in the system.yaml otherwise links will not have the protocol (https://) prepended.

What is currently missing is a site-wide setting however.

@jjui
Copy link

jjui commented Jun 13, 2016

i just added redirect to https in htaccess, and ssl: true is not present in admin, tho very handy

@flaviocopes
Copy link
Contributor

Good idea to have a global one

@Sommerregen
Copy link
Contributor Author

Sommerregen commented Jun 14, 2016

I was already aware of having secured pages with the ssl: true page header ;-). However, I really mean to have a global one, which can be changed in the admin system settings tab.

The global setting further has the advantage, that it doesn't need absolute_urls set to true, since all pages have the same protocol https://. We can maybe enhanced the ssl page header in this way, that if a global setting is set to true ssl: false for a specific page takes care that (i) absolute_urls: true and (ii) the specific page is not secured and vice-verse for a global setting set to false.

@rhukster
Copy link
Member

PRs welcome :) I'm supposed to be on holiday!

@flaviocopes
Copy link
Contributor

Started working on this PR: #899

Please test and report if you got any issue.

Currently does not take into consideration custom ports, something it should most probably do.

@Sommerregen
Copy link
Contributor Author

@flaviocopes It does work. However instead of checking $_SERVER['HTTPS'] I would use the $uri class. The same goes for building the URL. Even a custom port shouldn't be a problem then. Don't know if I should start with a PR, too, or not. Unfortunately I'm involved in another project atm. With the end of the week I could provide my solution. Please let me know your thoughts on that.

@flaviocopes
Copy link
Contributor

Yes you can do that too, just another approach. If you want to improve it, feel free! 👍

Anyway this remains a rather slow way of redirecting users, since you first need to instantiate Grav.
I'd always use a .htaccess based rule (if using Apache, other solutions if another webserver) if possibile.

@Sommerregen
Copy link
Contributor Author

Ok, great. I'll think about it and provide a PR (most likely next week). I don't care if the first call is a slow process. It fortunately happens only once. Although .htaccess is the usual way to go, I think (and that is the reason why I opened the issue) having a switch directly in Grav making it more powerful due to easier editing.

@OleVik
Copy link
Contributor

OleVik commented Jun 17, 2016

Correct me if I'm wrong, but .htaccess will mainly redirect the page rather than enforce https on assets? And as Sommeregen says, a .htaccess rule will do this on every instantiation of Grav, whilst a mechanism internal to Grav will only run on every recache.

@flaviocopes
Copy link
Contributor

The PR ensures that if you access a page via http, but you force https, it will issue a redirect to the https page. The browser is redirected, it's not an internal Grav thing. Same goes for assets if called through a page route, but not if accessed directly.

@flaviocopes
Copy link
Contributor

So, .htaccess does the same, but at a lower level, and faster

@w00fz
Copy link
Member

w00fz commented Sep 8, 2016

#899 has been merged so I'm going to close this issue and add my 2 cents on this matter.

Even though the option is there, I would highly suggest to stick to the .htaccess (or NGINX equivalent) method, if you have access to it. As @flaviocopes said above, it is a much faster, optimized and more reliable way to enforce https.

I actually don't quite like the implementation of this option with #899, .htaccess (or equivalent) let your server skip all the loading of Grav and get to the HTTPS before anything has happened yet, as all it has to deal with is the HEAD request, Grav is far far away from it still.

With the system.force_ssl option, however, Grav has to work a lot (initialize and go through its life cycle) just to finally realize it has to reload to the HTTPS protocol and then redo it all again. In the meantime, nothing has been served to the client and it's been quite a waste of resources, even though Grav is pretty fast, there is no reason nor benefit in doing this.

So, I really hope this explanation really discourages each and everyone of you in using the system.force_ssl option. If you can choose between the two, go for the .htaccess method without thinking.

@w00fz w00fz closed this as completed Sep 8, 2016
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

7 participants