Logged-Out-User & Routes Issue ( UF ver 4.6.6 )

[jonsq]

*Note: I'm not sure if this affects previous versions of UF.

If you are not signed in and you try to access the settings page, a "ForbiddenException" is thrown and an error is displayed.

I feel the better solution here is to redirect the user to the sign in page.

To do this, simply edit the routes file "routes\ConfigManager.php" so that the group declaration ends with ->add('authGuard')

Example of the modified code:

$app->group('/settings', function () {
    $this->get('', 'UserFrosting\Sprinkle\ConfigManager\Controller\ConfigManagerController:displayMain')
         ->setName('ConfigManager');

    $this->post('/{schema}', 'UserFrosting\Sprinkle\ConfigManager\Controller\ConfigManagerController:update')
         ->setName('ConfigManager.save');
})->add('authGuard');

[lcharette]

UF_ConfigManager/src/Controller/ConfigManagerController.php

Lines 71 to 73 in f726855

	if (!$authorizer->checkAccess($currentUser, 'update_site_config')) {
	throw new ForbiddenException();
	}

It's a design choice. Forbidden exception will be shown for any logged in user that don't have the update_site_config permission.

Any "guest" user (aka non logged in) won't have this permission, so it does make sense to add authGuard as no guest user can access this page. However, any guest redirected to the login page will be redirected to the setting page upon login. If this (logged in) user don't have permission, they will be still be shown the Forbidden exception... I'm not closed to the idea of adding authGuard in a Pull Request, but at the same time don't think it's 100% useful. This page is supposed to be "known" for admin only, not everyday users. Either way, it would be even more secure to not acknowledge it exist if user don't have permission to access it maybe ?

[jonsq]

You make several good points. Honestly I'm not certain what the best choice would be for the majority of users.

I found this purely by accident when I had a tab opened to the settings page overnight and the next morning (when my login had expired), I refreshed the settings page tab and saw an error page that did not reference any sort of Forbidden Exception (Screenshot shown below).

It took me a while to figure out what was wrong. Ended up temporarily modifing the filp/whoops PlainTextHandler.php file to fix it's error, (Screenshot shown below), so that I could see the true problem of Forbidden access.

[lcharette]

That error should be fixed by updating composer. It was a bug in flip/whoops

[jonsq]

I was running composer version 2.5.4 when I installed this Sprinkle. That was the latest version available at the time. Is there something I may have missed or done wrong?

[jonsq]

I see that the whoops bug you're referring to was fixed in whoops release version 2.15.1, which is included with UF 4.6.6.

However, reading up on that whoops bug pointed me in the right direction.

UF 4.6.6's "core" sprinkle has the same bug in "app\sprinkles\core\src\Error\Renderer\WhoopsRenderer.php". This file is missing the setRun(..) statement after line # 295 (example of missing statement shown below).

$plainTextHandler->setRun($this->getRun());

I don't see any mention of this problem on UF's GitHub. Perhaps it takes a very specific scenario to raise this bug and not enough users have ran into it yet?

[lcharette]

I see how this could indeed be an issue on UF4 code. A PR would be welcomed.

[jonsq]

I would if I thought I could. However I don't see any usage of the whoops' "RunInterface" in UF's code and I honestly wouldn't know where to begin to implement it.

For the time being, I'm satisfied with the add('authGuard') "solution" I provided in my original comment.