getkirby / kirby

Kirby's core application folder
https://getkirby.com
Other
1.27k stars 167 forks source link

Url overwrite config doesn't overwrite protocol #4234

Closed ChristophLabacher closed 2 years ago

ChristophLabacher commented 2 years ago

Description

After upgrading to 3.6(.3.1) I experience a behavior similar to the one described in https://github.com/getkirby/kirby/issues/3645, when I try to access the panel, only for me it’s not the port, but the entire domain:

DOMException: Failed to execute 'pushState' on 'History': A history state object with URL 'http://localhost/panel/site' cannot be created in a document with origin 'https://ourdomain.com' and URL 'https://ourdomain.com/panel/site'.

The frontend of our website with Kirby works as expected.

We run Kirby in a Docker setup with two containers: An nginx container that handles the incoming requests and forwards them to a php-fmt container, which runs Kirby. I assume the described behavior happens because within this setup the Kirby server is actually running on localhost (within its container), as it gets its requests directly from the nginx container.

In the frontend we observed a similar behavior, where one would be forwarded to localhost/de when loading the page, but there we could fix it simply by setting 'url' => "ourdomain.com" in config.php.

Expected behavior
The user should be forwarded to https://ourdomain.com/panel/site. Like the frontend, the Panel should prefer the domain given inconfig.php rather than try to extract the domain from the server’s information (I assume HTTP_HOST?)

Edit: While Server::get('HTTP_HOST') returns the correct host, Server::host() returns localhost.

Your setup

Kirby Version
Everything above 3.6

Console output

DOMException: Failed to execute 'pushState' on 'History': A history state object with URL 'http://localhost/panel/site' cannot be created in a document with origin 'https://ourdomain.com' and URL 'https://ourdomain.com/panel/site'.
lukasbestle commented 2 years ago

Hi Christoph, have you tested it all with the url option set to the URL string? This would be helpful to know as the behavior depends on the type of the configured option value (null, string, array or constant).

Also, can you reproduce the issue with Kirby 3.6.2? We added the new URL logic in 3.6.3.

ChristophLabacher commented 2 years ago

Hi Lukas,

We set the url option in config.php to a string: https://ourdomain.com.

After testing with 3.6.2 and confirming the issue could be reproduced, I upgraded back to 3.6.3.1 and the issue was partly resolved. Apparently, our server hadn’t pulled the latest version correctly on the last deployment and I was still running 3.6.2 even though I had updated to 3.6.3.

3.6.3.1 includes this fix https://github.com/getkirby/kirby/issues/4185.

Because of this fix, the host is now set correctly. However the protocol is wrong, leading to the same behavior I originally described. This is the new console log:

DOMException: Failed to execute 'pushState' on 'History': A history state object with URL 'http://ourdomain.com/panel/site' cannot be created in a document with origin 'https://ourdomain.com' and URL 'https://ourdomain.com'.

Notice how the Panel wants to forward to http://, even though the url option is set to https://.

I assume this is because the https() function of Server.php only looks at $_SERVER['HTTPS'] (which is false in our case, because we’re within the docker container): https://github.com/getkirby/kirby/blob/main/src/Http/Server.php#L167-L183

The url option is only used to set the hosts, not the protocol: https://github.com/getkirby/kirby/blob/main/src/Cms/Environment.php#L197-L211

lukasbestle commented 2 years ago

Thanks, the HTTPS issue is certainly a bug in the new URL implementation. I'm glad the rest works now.

bastianallgeier commented 2 years ago

@ChristophLabacher sorry for the delay. Is your container forwarding anything useful in the $_SERVER global to get the protocol from there? I.e. $_SERVER['HTTP_X_FORWARDED_PROTO']?

ChristophLabacher commented 2 years ago

@bastianallgeier No worries, I just added $_SERVER['HTTPS'] = true; to index.php and that makes things work for now.

We do set the x-forwarded headers in nginx:

        proxy_set_header X-Real-IP $remote_addr;
        proxy_set_header X-Forwarded-For $remote_addr;
        proxy_set_header X-Forwarded-Proto $scheme;

But none of these fields is set on the $_SERVER var 🤔

lukasbestle commented 2 years ago

@bastianallgeier I'm already working on it, see my assignment.

paresy commented 2 years ago

Just wanted to add a confirmation that this problem seems to be a https -> http redirect problem. The automatic detection broke after the Kirby upgrade. At the moment i have this in my config.php to get the frontend working, but the panel is stuck at the white page with only Firefox throwing the already mentioned error. (As a default Chrome user getting no error and a white page can really drive you nuts while searching what went wrong :smile)

return [
    'url' => '/',
];

Btw: This is my first Kirby project and it is currently very simple and deployed as an Azure WebApp.

bogdancondorachi commented 2 years ago

So I seem to have the same problem on my setup, returning a blank panel with the following console error DOMException: Failed to execute 'pushState' on 'History': A history state object with URL 'http://hasty.ai/panel/login' cannot be created in a document with origin 'https://hasty.ai' and URL 'https://hasty.ai/panel/login'.

Is there a work around until this get's fixed? Also posted on forum more info about my setup https://forum.getkirby.com/t/panel-appears-as-blank/25394

lukasbestle commented 2 years ago

@bogdancondorachi We are working on the fix for Kirby 3.7.0, but it is not ready yet. A workaround can be to hardcode your real protocol and port in the methods of the src/Http/Server.php class.

paresy commented 2 years ago

@bogdancondorachi Have you tried the simple workaround that was posted by ChristophLabacher ?

I just added $_SERVER['HTTPS'] = true; to index.php and that makes things work for now.

bogdancondorachi commented 2 years ago

@lukasbestle thanks for looking into this, for now I've find a solution that works for me until a fix comes around.

if (strpos($_SERVER['HTTP_X_FORWARDED_PROTO'], 'https') !== false) {
    $_SERVER['HTTPS'] = true;
}

It checks the protocol used and if it's https it will force https over kirby, might not be the best one but solves the problem for me as well as not interfiring with local dev or test server that does not uses https.

lukasbestle commented 2 years ago

One note about this: It should only be used if the input header can be trusted (i.e. if you are absolutely certain that it cannot be set by an attacker from the outside). With the protocol it probably isn't as critical as with the hostname, but better safe than sorry.

bastianallgeier commented 2 years ago