getkirby / kirby

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

Panel redirect does not work with relative URLs and users with panel: false #6179

Open dgsiegel opened 9 months ago

dgsiegel commented 9 months ago

Description

I've set up a user role like this:

title: Author
description: The author can't login

permissions:
  access:
    panel: false

Then I've created a new user with this role and set a password. When trying to log in with this user, I get a dialog saying NetworkError when attempting to fetch resource.. On reload the page will break until I delete the session cookie with the browser error The page isn’t redirecting properly.

The Firefox browser console shows this:

Uncaught TypeError: document.querySelector(...) is null
    blur http://localhost:8080/media/panel/5baeb58a5608e28c5f241eee74f064b2/js/index.min.js:1
    emit http://localhost:8080/media/panel/5baeb58a5608e28c5f241eee74f064b2/js/vendor.min.js:11
    emit http://localhost:8080/media/panel/5baeb58a5608e28c5f241eee74f064b2/js/vendor.min.js:11
    click http://localhost:8080/media/panel/5baeb58a5608e28c5f241eee74f064b2/js/index.min.js:1
    subscribe http://localhost:8080/media/panel/5baeb58a5608e28c5f241eee74f064b2/js/index.min.js:1
    created http://localhost:8080/media/panel/5baeb58a5608e28c5f241eee74f064b2/js/index.min.js:1
    VueJS 16
    <anonymous> http://localhost:8080/media/panel/5baeb58a5608e28c5f241eee74f064b2/js/index.min.js:1
[index.min.js:1:385304](http://localhost:8080/media/panel/5baeb58a5608e28c5f241eee74f064b2/js/index.min.js)
Uncaught TypeError: document.querySelector(...) is null
    blur http://localhost:8080/media/panel/5baeb58a5608e28c5f241eee74f064b2/js/index.min.js:1
    emit http://localhost:8080/media/panel/5baeb58a5608e28c5f241eee74f064b2/js/vendor.min.js:11
    emit http://localhost:8080/media/panel/5baeb58a5608e28c5f241eee74f064b2/js/vendor.min.js:11
    click http://localhost:8080/media/panel/5baeb58a5608e28c5f241eee74f064b2/js/index.min.js:1
    subscribe http://localhost:8080/media/panel/5baeb58a5608e28c5f241eee74f064b2/js/index.min.js:1
    created http://localhost:8080/media/panel/5baeb58a5608e28c5f241eee74f064b2/js/index.min.js:1
    VueJS 16
    <anonymous> http://localhost:8080/media/panel/5baeb58a5608e28c5f241eee74f064b2/js/index.min.js:1
[index.min.js:1:385304](http://localhost:8080/media/panel/5baeb58a5608e28c5f241eee74f064b2/js/index.min.js)
TypeError: NetworkError when attempting to fetch resource. [index.min.js:1:391282](http://localhost:8080/media/panel/5baeb58a5608e28c5f241eee74f064b2/js/index.min.js)

Chrome shows this:

index.min.js:1 

GET http://localhost:8080/panel net::ERR_TOO_MANY_REDIRECTS index.min.js:1 
TypeError: Failed to fetch
    at Ga (index.min.js:1:368382)
    at Object.request (index.min.js:1:391879)
    at Object.get (index.min.js:1:391380)
    at Object.open (index.min.js:1:391537)
    at Object.load (index.min.js:1:373246)
    at Object.open (index.min.js:1:373400)
    at Object.reload (index.min.js:1:373955)
    at a.login (index.min.js:1:71965)
error @ index.min.js:1
open @ index.min.js:1
await in open (async)
load @ index.min.js:1
open @ index.min.js:1
reload @ index.min.js:1
login @ index.min.js:1
await in login (async)
submit @ index.min.js:1
pn @ vue.min.js:11
r @ vue.min.js:11
i._wrapper @ vue.min.js:11
index.min.js:1 Uncaught TypeError: Cannot read properties of null (reading 'contains')
    at Object.blur (index.min.js:1:385340)
    at vendor.min.js:11:21268
    at Array.map (<anonymous>)
    at Object.emit (vendor.min.js:11:21251)
    at Object.click (index.min.js:1:380310)
blur @ index.min.js:1
(anonymous) @ vendor.min.js:11
emit @ vendor.min.js:11
click @ index.min.js:1

Expected behavior
I'd expect a permission denied error or no error message.

Your setup

Kirby Version
4.0.3

afbora commented 9 months ago

I couldn't reproduce the issue on Chrome and Firefox. My steps:

  1. I've created an editor doesn't have panel access
  2. Went to /panel/login and try to login
  3. Redirected to site homepage
  4. Then tried to go to /panel or /panel/login or /panel/site manually
  5. All worked as expected for me

Do you have any plugins, custom js or hooks?

dgsiegel commented 9 months ago

Do you have any plugins, custom js or hooks?

Thanks for the hints! I should have tested this issue first with the Starterkit, as it doesn't happen there. The culprit is of course the relative URL workaround I've described here: https://github.com/getkirby/kirby/issues/6078#issuecomment-1868343757

Setting the index.php like I've described there will indeed bring up the issue. Is there a way to keep this workaround?

dgsiegel commented 9 months ago

@afbora it looks like the frontend JS doesn't behave well with relative URLs in the Location parameter in the header. If you would change this line https://github.com/getkirby/kirby/blob/main/src/Panel/Home.php#L48 to return App::instance()->url('base', true);, ie. always return a full URL, the issue goes away.

dgsiegel commented 9 months ago

@afbora on further investigation the issue looks like this:

  1. I've enabled relative URLs by modifying index.php like this:
<?php

require 'kirby/bootstrap.php';

$kirby = new Kirby([
  'urls' => [
    'index' => '/',
  ],
]);

echo $kirby->render();
  1. When logging in with a disabled user, the Panel will redirect (302) the user to the home page (https://github.com/getkirby/kirby/blob/main/src/Panel/Home.php#L203). In our case none of the checks pass and the alternative function will be called (https://github.com/getkirby/kirby/blob/main/src/Panel/Home.php#L42)
  2. As the user has no access to the panel, they will be redirected to the main/site url (https://github.com/getkirby/kirby/blob/main/src/Panel/Home.php#L48)
  3. The problem here is that in this setup, the URL will be a relative one, ie. / instead of a full URL with host
  4. Hence the Location parameter in the redirect will be set to /panel continuously and we got an infinite loop, which a browser will stop at some point with NetworkError when attempting to fetch resource or net::ERR_TOO_MANY_REDIRECTS
  5. The problem is therefore a relative URL which gets returned, and an easy fix was described in my previous comment (https://github.com/getkirby/kirby/issues/6179#issuecomment-1904593349)
  6. But I guess the actual culprit is the Panel's url function (https://github.com/getkirby/kirby/blob/main/src/Panel/Panel.php#L566) which "should" Create an absolute Panel URL independent of the Panel slug config.
  7. The problem in this function is that it will return a relative URL in this line $baseUri = new Uri($kirby->url()); (https://github.com/getkirby/kirby/blob/main/src/Panel/Panel.php#L574) with this setup
  8. The correct approach IMHO would be to use $baseUri = new Uri($kirby->url('base', true)); as that would make sure that you always return an absolute URL with host, even if no host was defined before.
dgsiegel commented 8 months ago

@afbora is there anything else you need to replicate this?

afbora commented 8 months ago

Sorry @dgsiegel I can't reproduce the issue. Is your setup in subfolder?

dgsiegel commented 8 months ago

Sorry @dgsiegel I can't reproduce the issue. Is your setup in subfolder?

@afbora, nope just a normal installation.

Here is a diff you can apply onto a plainkit setup with the following changes:

I am running the setup via php -S localhost:8080 kirby/router.php, but that shouldn't make a difference

diff -Nur '--exclude=accounts' plainkit-main/index.php plainkit-bug/index.php
--- plainkit-main/index.php 2024-03-06 12:41:59.000000000 +0100
+++ plainkit-bug/index.php  2024-03-16 13:27:27.025673095 +0100
@@ -2,4 +2,10 @@

 require 'kirby/bootstrap.php';

-echo (new Kirby)->render();
+$kirby = new Kirby([
+  'urls' => [
+    'index' => '/',
+  ],
+]);
+
+echo $kirby->render();
diff -Nur '--exclude=accounts' plainkit-main/site/blueprints/users/nologin.yml plainkit-bug/site/blueprints/users/nologin.yml
--- plainkit-main/site/blueprints/users/nologin.yml 1970-01-01 01:00:00.000000000 +0100
+++ plainkit-bug/site/blueprints/users/nologin.yml  2024-03-16 13:26:39.537982941 +0100
@@ -0,0 +1,6 @@
+title: Nologin
+description: The nologin user can't login
+
+permissions:
+  access:
+    panel: false

After installing the plainkit, create a new user with the nologin role and try to log in in a fresh browser.

distantnative commented 7 months ago

I am receiving the error screen "You are not allowed to access the panel" - which is different from the console errors you are describing. But indeed not the redirect to the site.

dgsiegel commented 7 months ago

@distantnative maybe there's a difference in how we set up our projects. I've created a zip containing all my changes:

plainkit-main.zip

After downloading,

afbora commented 6 months ago

Sorry for late. I've tested your setup (with subfolder 'index' => '/test/kirby/6179/') and works for me (redirected to the homepage) as expected.

dgsiegel commented 6 months ago

Sorry for late. I've tested your setup (with subfolder 'index' => '/test/kirby/6179/') and works for me (redirected to the homepage) as expected.

Yeah, this won't work with a subfolder as you never will get an invalid string as with /. You would need to run this directly on localhost or whatever local development domain you use.

github-actions[bot] commented 4 months ago

This issue has been marked as stale because it requires further input but has not seen activity in the past months. This is for us to prioritize issues that are still relevant and actionable. It will be closed if no further activity occurs within the next 15 days. If this issue is still relevant to you, please help us in gathering the necessary input.

dgsiegel commented 4 months ago

This issue has been marked as stale because it requires further input but has not seen activity in the past months. This is for us to prioritize issues that are still relevant and actionable. It will be closed if no further activity occurs within the next 15 days. If this issue is still relevant to you, please help us in gathering the necessary input.

@afbora @distantnative what further input do you need?

distantnative commented 3 months ago

@dgsiegel I can now reproduce the issue.

However, your suggested fix doesn't seem to work for me. I still get the network error using

$baseUri  = $kirby->url('base', true);

Did it resolve the issue on your end?

dgsiegel commented 3 months ago

Did it resolve the issue on your end?

Yes, it did fix the issue for me. But I was using

return App::instance()->url('base', true);

in https://github.com/getkirby/kirby/blob/main/src/Panel/Home.php#L48

Don't know if that makes any difference? Also, you need to clear all cookies and storage in your browser, otherwise you'll always get the network error.