flatpressblog / flatpress

FlatPress is a lightweight, easy-to-set-up flat-file blogging engine.
https://flatpress.org
GNU General Public License v2.0
181 stars 58 forks source link

There should be a fatal error message while trying to run this without HTTPS (info inside) #388

Closed 0xjams closed 3 months ago

0xjams commented 3 months ago

There's a super useful default configuration on this file fp-includes/core/core.cookie.php. It forces cookies to have secure defaults, among them the secure flag.

This is great. However, if someone is testing the app via HTTP and is not aware of this, they might end up in a situation where logging in is not possible.

My suggestion is to add a step that inspects the URL and if it's HTTP either warns about this or aborts execution altogether presenting an error message.

Fraenkiman commented 3 months ago

Hello @0xjams,

thank you for the hint and the suggestion. It should no longer occur in the current master branch that the secure flag is set for a non-HTTPS connection, among other things. We have recently closed the issue #371.

Do you still have problems with the login? If so, under what conditions?

Can you create a pull request for us?

With best regards Frank

@azett, you could insert a trigger_error. What do you think?

trigger_error("Attention, an HTTP connection is used", E_USER_ERROR);

https://github.com/flatpressblog/flatpress/blob/beb6172a7e98e3597574aec652dbb654f7bc3a5a/defaults.php#L194

0xjams commented 3 months ago

I'm sorry I was reading issue 371. I was using the app from the releases zip, I didn't clone the repo. If needed, I could trigger an error and send a pull request :)

0xjams commented 3 months ago

I checked the code on the master branch, and since this function exists:

function is_https() {
    // HTTPS called web server
    if (isset($_SERVER ['HTTPS']) && !empty($_SERVER ['HTTPS'])) {
        return true;
    }
    // HTTPS called reverse proxy / load balancer 
    if (!empty($_SERVER ['HTTP_X_FORWARDED_PROTO']) && $_SERVER ['HTTP_X_FORWARDED_PROTO'] == 'https' || !empty($_SERVER ['HTTP_X_FORWARDED_SSL']) && $_SERVER ['HTTP_X_FORWARDED_SSL'] == 'on') {
        return true;
    }
    // none of the above: must be HTTP
    return false;
}

If someone uses HTTP, they won't have any issues at all because of this:

    if (!defined('COOKIE_SECURE'))
        define('COOKIE_SECURE', is_https());

I don't think the app needs to raise a warning if this is the case.

azett commented 3 months ago

Jorge, welcome to FlatPress and thank you very much for your input :) The problem you describe has been fixed in the issue371_httphttps branch just recently. I plan to release it as version 1.3.1 tomorrow.

Calling with unencrypted HTTP is of course insecure, but there are good reasons not to display an error because of this. Not all hosters (especially the free ones) offer suitable TLS certificates. Also think about internal use and testing purposes. We all agree that HTTPS is the better choice - but we don't want to dictate this to the FlatPress users.

Please feel free to test the 371 branch (or version 1.3.1 as soon as it's released) and let us know if everything works fine. Thanks :)