Simplify the redirection to login form related code

glpi-project · Jan 22, 2025 · 5f8a4a1 · 5f8a4a1
1 parent 823b4eb
commit 5f8a4a1
Show file tree

Hide file tree

Showing 11 changed files with 51 additions and 153 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -529,6 +529,8 @@ The present file will list all changes made to the project; according to the
 - `Search::outputData()`
 - `Search::sylk_clean()`
 - `Session::buildSessionName()`
+- `Session::redirectIfNotLoggedIn()`
+- `Session::redirectToLogin()`
 - `SlaLevel::showForSLA()`. Replaced by `LevelAgreementLevel::showForLA()`.
 - `SLM::setTicketCalendar()`
 - `SoftwareLicense::getSonsOf()`

diff --git a/front/item_device.common.form.php b/front/item_device.common.form.php
@@ -54,8 +54,6 @@
     throw new BadRequestHttpException();
 }
 if (!$item_device->canView()) {
-   // Gestion timeout session
-    Session::redirectIfNotLoggedIn();
     throw new AccessDeniedHttpException();
 }
 

diff --git a/front/item_device.php b/front/item_device.php
@@ -47,7 +47,6 @@
 /** @var class-string $_GET['itemtype'] */
 $itemDevice = getItemForItemtype($_GET['itemtype']);
 if (!$itemDevice->canView()) {
-    Session::redirectIfNotLoggedIn();
     throw new AccessDeniedHttpException();
 }
 

diff --git a/front/knowbaseitem.php b/front/knowbaseitem.php
@@ -39,7 +39,6 @@
 global $CFG_GLPI;
 
 if (!Session::haveRightsOr('knowbase', [READ, KnowbaseItem::READFAQ])) {
-    Session::redirectIfNotLoggedIn();
     throw new AccessDeniedHttpException();
 }
 if (isset($_GET["id"])) {

diff --git a/front/updatepassword.php b/front/updatepassword.php
@@ -33,16 +33,13 @@
  * ---------------------------------------------------------------------
  */
 
-/**
- * @var array $CFG_GLPI
- */
-global $CFG_GLPI;
+use Glpi\Exception\Http\AccessDeniedHttpException;
 
 // Cannot use `Session::checkLoginUser()` as it block users that have their password expired to be able to change it.
 // Indeed, when password expired, sessions is loaded without profiles nor rights, and `Session::checkLoginUser()`
 // considers it as an invalid session.
 if (Session::getLoginUserID() === false) {
-    Html::redirectToLogin();
+    throw new AccessDeniedHttpException();
 }
 
 switch (Session::getCurrentInterface()) {

diff --git a/src/CommonDBTM.php b/src/CommonDBTM.php
@@ -3151,13 +3151,9 @@ public function check($ID, int $right, ?array &$input = null): void
     {
         // Check item exists
         if (!$this->checkIfExistOrNew($ID)) {
-           // Gestion timeout session
-            Session::redirectIfNotLoggedIn();
             throw new NotFoundHttpException();
         } else {
             if (!$this->can($ID, $right, $input)) {
-               // Gestion timeout session
-                Session::redirectIfNotLoggedIn();
                 /** @var class-string<CommonDBTM> $itemtype */
                 $itemtype = static::getType();
                 $right_name = Session::getRightNameForError($itemtype::$rightname, $right);
@@ -3232,8 +3228,6 @@ public function checkEntity($recursive = false)
     public function checkGlobal(int $right): void
     {
         if (!$this->canGlobal($right)) {
-           // Gestion timeout session
-            Session::redirectIfNotLoggedIn();
             /** @var class-string<CommonDBTM> $itemtype */
             $itemtype = static::getType();
             $right_name = Session::getRightNameForError($itemtype::$rightname, $right);

diff --git a/src/Glpi/Controller/LegacyFileLoadController.php b/src/Glpi/Controller/LegacyFileLoadController.php
@@ -67,7 +67,6 @@ protected function setAjax(): void
         $this->getRequest()->attributes->set('_glpi_ajax', true);
 
         \Session::setAjax();
-        \Html::setAjax();
     }
 
     private function getRequest(): ?Request

diff --git a/src/Glpi/Http/Listener/AccessErrorListener.php b/src/Glpi/Http/Listener/AccessErrorListener.php
@@ -63,8 +63,8 @@ public function onKernelException(ExceptionEvent $event): void
 
         $request = $event->getRequest();
 
-        if ($request->isXmlHttpRequest()) {
-            // Do not redirect AJAX requests.
+        if ($request->isXmlHttpRequest() || $request->getPreferredFormat() !== 'html') {
+            // Do not redirect AJAX requests nor requests that expect the response to be something else than HTML.
             return;
         }
 
@@ -73,7 +73,7 @@ public function onKernelException(ExceptionEvent $event): void
         $response = null;
 
         if ($throwable instanceof SessionExpiredException) {
-            Session::destroy(); // destroy the session to prevent pesistence of unexcpected data
+            Session::destroy(); // destroy the session to prevent pesistence of unexpected data
 
             $response = new RedirectResponse(
                 sprintf(

diff --git a/src/Glpi/Http/Listener/LegacyPostRequestActionsListener.php b/src/Glpi/Http/Listener/LegacyPostRequestActionsListener.php
@@ -48,7 +48,6 @@ public static function getSubscribedEvents(): array
 
     public function onFinishRequest(): void
     {
-        \Html::resetAjaxParam();
         \Session::resetAjaxParam();
     }
 }
diff --git a/src/Html.php b/src/Html.php
@@ -56,12 +56,6 @@
  **/
 class Html
 {
-    /**
-     * Indicates whether the request is made in an AJAX context.
-     * @FIXME This flag is actually not set to true by all AJAX requests.
-     */
-    private static bool $is_ajax_request = false;
-
     /**
      * Recursivly execute html_entity_decode on an array
      *
@@ -438,43 +432,6 @@ public static function redirect($dest, $http_response_code = 302): never
         throw new RedirectException($dest, $http_response_code);
     }
 
-    /**
-     * Redirection to Login page
-     *
-     * @param string $params  param to add to URL (default '')
-     * @since 0.85
-     *
-     * @return void
-     **/
-    public static function redirectToLogin($params = '')
-    {
-        /**
-         * @var array $CFG_GLPI
-         */
-        global $CFG_GLPI;
-
-        $dest = $CFG_GLPI["root_doc"] . "/index.php";
-
-        if (!self::$is_ajax_request) {
-            $url_dest = preg_replace(
-                '/^' . preg_quote($CFG_GLPI["root_doc"], '/') . '/',
-                '',
-                $_SERVER['REQUEST_URI']
-            );
-            $dest .= "?redirect=" . rawurlencode($url_dest);
-        }
-
-        if (!empty($params)) {
-            if (str_contains($dest, '?')) {
-                $dest .= '&' . $params;
-            } else {
-                $dest .= '?' . $params;
-            }
-        }
-
-        self::redirect($dest);
-    }
-
 
     /**
      * Display common message for item not found
@@ -6586,22 +6543,6 @@ public static function timestampToRelativeStr($ts)
         }
     }
 
-    /**
-     * Indicates that the request is made in an AJAX context.
-     */
-    public static function setAjax(): void
-    {
-        self::$is_ajax_request = true;
-    }
-
-    /**
-     * Unset the flag that indicates that the request is made in an AJAX context.
-     */
-    public static function resetAjaxParam(): void
-    {
-        self::$is_ajax_request = false;
-    }
-
     /**
      * Sanitize a input name to prevent XSS.
      *

diff --git a/src/Session.php b/src/Session.php
@@ -41,6 +41,7 @@
 use Glpi\Exception\SessionExpiredException;
 use Glpi\Plugin\Hooks;
 use Glpi\Session\SessionInfo;
+use Symfony\Component\HttpFoundation\Request;
 
 /**
  * Session Class
@@ -997,22 +998,6 @@ public static function getLoginUserID($force_human = true)
         return false;
     }
 
-
-    /**
-     * Redirect User to login if not logged in
-     *
-     * @since 0.85
-     *
-     * @return void
-     **/
-    public static function redirectIfNotLoggedIn()
-    {
-
-        if (!self::isAuthenticated()) {
-            Html::redirectToLogin();
-        }
-    }
-
     /**
      * Global check of session to prevent PHP vulnerability
      *
@@ -1038,60 +1023,56 @@ public static function checkValidSessionId()
         $profile_id = $_SESSION['glpiactiveprofile']['id'] ?? null;
         $entity_id  = $_SESSION['glpiactive_entity'] ?? null;
 
-        $valid_user = true;
-
         if (!is_numeric($user_id) || $profile_id === null || $entity_id === null) {
-            $valid_user = false;
-        } else {
-            $user_table = User::getTable();
-            $pu_table   = Profile_User::getTable();
-            $profile_table = Profile::getTable();
-            $result = $DB->request(
-                [
-                    'COUNT'     => 'count',
-                    'SELECT'    => [$profile_table . '.last_rights_update'],
-                    'FROM'      => $user_table,
-                    'LEFT JOIN' => [
-                        $pu_table => [
-                            'FKEY'  => [
-                                Profile_User::getTable() => 'users_id',
-                                $user_table         => 'id'
-                            ]
-                        ],
-                        $profile_table => [
-                            'FKEY'  => [
-                                $pu_table => 'profiles_id',
-                                $profile_table => 'id'
-                            ]
+            throw new SessionExpiredException();
+        }
+
+        $user_table = User::getTable();
+        $pu_table   = Profile_User::getTable();
+        $profile_table = Profile::getTable();
+        $result = $DB->request(
+            [
+                'COUNT'     => 'count',
+                'SELECT'    => [$profile_table . '.last_rights_update'],
+                'FROM'      => $user_table,
+                'LEFT JOIN' => [
+                    $pu_table => [
+                        'FKEY'  => [
+                            Profile_User::getTable() => 'users_id',
+                            $user_table         => 'id'
                         ]
                     ],
-                    'WHERE'     => [
-                        $user_table . '.id'         => $user_id,
-                        $user_table . '.is_active'  => 1,
-                        $user_table . '.is_deleted' => 0,
-                        $pu_table . '.profiles_id'  => $profile_id,
-                    ] + getEntitiesRestrictCriteria($pu_table, 'entities_id', $entity_id, true),
-                    'GROUPBY'   => [$profile_table . '.id'],
-                ]
-            );
+                    $profile_table => [
+                        'FKEY'  => [
+                            $pu_table => 'profiles_id',
+                            $profile_table => 'id'
+                        ]
+                    ]
+                ],
+                'WHERE'     => [
+                    $user_table . '.id'         => $user_id,
+                    $user_table . '.is_active'  => 1,
+                    $user_table . '.is_deleted' => 0,
+                    $pu_table . '.profiles_id'  => $profile_id,
+                ] + getEntitiesRestrictCriteria($pu_table, 'entities_id', $entity_id, true),
+                'GROUPBY'   => [$profile_table . '.id'],
+            ]
+        );
 
-            $row = $result->current();
+        $row = $result->current();
 
-            if ($row === null || $row['count'] === 0) {
-                $valid_user = false;
-            } elseif (
-                $row['last_rights_update'] !== null
-                && $row['last_rights_update'] > ($_SESSION['glpiactiveprofile']['last_rights_update'] ?? 0)
-            ) {
-                Session::reloadCurrentProfile();
-                $_SESSION['glpiactiveprofile']['last_rights_update'] = $row['last_rights_update'];
-            }
+        if ($row === null || $row['count'] === 0) {
+            // The current profile cannot be found for the current user in the database.
+            // The session information are stale, therefore the session should be considered as expired.
+            throw new SessionExpiredException();
         }
 
-        if (!$valid_user) {
-            Session::destroy();
-            Auth::setRememberMeCookie('');
-            Html::redirectToLogin();
+        if (
+            $row['last_rights_update'] !== null
+            && $row['last_rights_update'] > ($_SESSION['glpiactiveprofile']['last_rights_update'] ?? 0)
+        ) {
+            Session::reloadCurrentProfile();
+            $_SESSION['glpiactiveprofile']['last_rights_update'] = $row['last_rights_update'];
         }
 
         return true;
@@ -1106,8 +1087,6 @@ public static function checkCentralAccess()
     {
         self::checkValidSessionId();
         if (Session::getCurrentInterface() != "central") {
-           // Gestion timeout session
-            self::redirectIfNotLoggedIn();
             throw new AccessDeniedHttpException("The current profile does not use the standard interface");
         }
     }
@@ -1141,8 +1120,6 @@ public static function checkHelpdeskAccess()
     {
         self::checkValidSessionId();
         if (Session::getCurrentInterface() != "helpdesk") {
-           // Gestion timeout session
-            self::redirectIfNotLoggedIn();
             throw new AccessDeniedHttpException("The current profile does not use the simplified interface");
         }
     }
@@ -1156,8 +1133,6 @@ public static function checkLoginUser()
     {
         self::checkValidSessionId();
         if (!isset($_SESSION["glpiname"])) {
-           // Gestion timeout session
-            self::redirectIfNotLoggedIn();
             throw new AccessDeniedHttpException("User has no valid session but seems to be logged in");
         }
     }
@@ -1230,8 +1205,6 @@ public static function checkRight($module, $right)
     {
         self::checkValidSessionId();
         if (!self::haveRight($module, $right)) {
-           // Gestion timeout session
-            self::redirectIfNotLoggedIn();
             $right_name = self::getRightNameForError($module, $right);
             throw new AccessDeniedHttpException("User is missing the $right ($right_name) right for $module");
         }
@@ -1249,7 +1222,6 @@ public static function checkRightsOr($module, $rights = [])
     {
         self::checkValidSessionId();
         if (!self::haveRightsOr($module, $rights)) {
-            self::redirectIfNotLoggedIn();
             $info = "User is missing all of the following rights: ";
             foreach ($rights as $right) {
                 $right_name = self::getRightNameForError($module, $right);
@@ -1292,8 +1264,6 @@ public static function checkSeveralRightsOr($modules)
         }
 
         if (!$valid) {
-           // Gestion timeout session
-            self::redirectIfNotLoggedIn();
             $info = "User is missing all of the following rights: ";
             foreach ($modules as $mod => $right) {
                 $right_name = self::getRightNameForError($mod, $right);