-
-
Notifications
You must be signed in to change notification settings - Fork 57
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
[RTM] Distinguish between error 401 and 403 #1381
Conversation
| @@ -79,7 +81,7 @@ public function loginAction(): Response | |||
| { | |||
| $this->get('contao.framework')->initialize(); | |||
|
|
|||
| if (!isset($GLOBALS['TL_PTY']['error_403']) || !class_exists($GLOBALS['TL_PTY']['error_403'])) { | |||
| if (!isset($GLOBALS['TL_PTY']['error_401']) || !class_exists($GLOBALS['TL_PTY']['error_401'])) { | |||
| return $this->redirectToRoute('contao_root'); | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You should throw the UnauthorizedHttpException here as well
| if ($objPage->protected) | ||
| { | ||
| if (!\System::getContainer()->get('contao.security.token_checker')->hasFrontendUser()) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This service should not be used in this case. It's only meant to check users across firewalls (because it unserialized the token from the session instead of using the token storage).
| public static function find401ByPid($intPid, array $arrOptions=array()) | ||
| { | ||
| $t = static::$strTable; | ||
| $arrColumns = array("$t.pid=? AND $t.type='error_401'"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
so this only works if the page is a direct child of the root page? Is this already the case for 403?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes.
| */ | ||
| protected function prepare($objRootPage=null) | ||
| { | ||
| // Use the given root page object if available (thanks to Andreas Schempp) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think you can remove that 😂
This PR implements #1274.