Improvements from code review

glpi-project · Sep 13, 2024 · eb5880d · eb5880d
1 parent 64c5b43
commit eb5880d
Show file tree

Hide file tree

Showing 3 changed files with 51 additions and 40 deletions.
diff --git a/front/inventory.conf.php b/front/inventory.conf.php
@@ -40,32 +40,19 @@
 Html::header(__('Inventory'), $_SERVER['PHP_SELF'], "admin", "glpi\inventory\inventory");
 
 $conf = new Conf();
-$glpikey = new GLPIKey();
 
 if (isset($_FILES['inventory_files'])) {
     $conf->displayImportFiles($_FILES);
 } elseif (isset($_POST['update'])) {
     unset($_POST['update']);
-    if (
-        (
-            !$_POST['basic_auth_password'] ||
-            !$_POST['basic_auth_login']
-        ) && $_POST['auth_required'] === Conf::BASIC_AUTH
-    ) {
+    $conf_is_success = $conf->saveConf($_POST);
+    if ($conf_is_success) {
         Session::addMessageAfterRedirect(
-            "Basic Authentication is active. The login and/or password fields are missing.",
+            __s('Configuration has been updated'),
             false,
-            ERROR
+            INFO
         );
-        Html::back();
     }
-    $_POST['basic_auth_password'] = $glpikey->encrypt($_POST['basic_auth_password']);
-    $conf->saveConf($_POST);
-    Session::addMessageAfterRedirect(
-        __s('Configuration has been updated'),
-        false,
-        INFO
-    );
     Html::back();
 } else {
     $conf->display(['id' => 1]);

diff --git a/src/Glpi/Agent/Communication/AbstractRequest.php b/src/Glpi/Agent/Communication/AbstractRequest.php
@@ -226,24 +226,32 @@ public function handleRequest($data): bool
         }
 
         if ($auth_required === Conf::BASIC_AUTH) {
-            $authorization = $this->headers->getHeader('Authorization');
-            if (!$authorization) {
+            $authorization_header = $this->headers->getHeader('Authorization');
+            if (is_null($authorization_header)) {
                 $this->setMode(self::JSON_MODE);
                 $this->headers->setHeader("www-authenticate", 'Basic realm="basic"');
                 $this->addError('Authorization header required to send an inventory', 401);
                 return false;
             } else {
-                $glpikey = new GLPIKey();
-                $loginAgent = \Config::getConfigurationValue('inventory', 'basic_auth_login');
-                $passwordAgent = \Config::getConfigurationValue('inventory', 'basic_auth_password');
-                $authData = base64_decode(substr($authorization, 6));
-                list($loginUser, $passwordUser) = explode(':', $authData, 2);
-                if (
-                    $loginUser !== $loginAgent ||
-                    $passwordUser !== $glpikey->decrypt($passwordAgent)
-                ) {
+                $allowed = false;
+                // if Authorization start with 'Basic'
+                if (preg_match('/^Basic\s+(.*)$/i', $authorization_header, $matches)) {
+                    $inventory_login = \Config::getConfigurationValue('inventory', 'basic_auth_login');
+                    $inventory_password = (new GLPIKey())
+                        ->decrypt(\Config::getConfigurationValue('inventory', 'basic_auth_password'));
+                    $agent_credential = base64_decode($matches[1]);
+                    list($agent_login, $agent_password) = explode(':', $agent_credential, 2);
+                    if (
+                        $inventory_login == $agent_login &&
+                        $inventory_password == $agent_password
+                    ) {
+                        $allowed = true;
+                    }
+                }
+                if (!$allowed) {
                     $this->setMode(self::JSON_MODE);
-                    $this->addError('Acces denied. Login or password is wrong.', 401);
+                    $this->addError('Access denied. Wrong login or password for basic authentication.', 401);
+                    return false;
                 }
             }
         }

diff --git a/src/Glpi/Inventory/Conf.php b/src/Glpi/Inventory/Conf.php
@@ -349,9 +349,7 @@ public function showConfigForm()
          */
         global $CFG_GLPI, $PLUGIN_HOOKS;
 
-        $glpikey = new GLPIKey();
         $config = \Config::getConfigurationValues('inventory');
-        $config['basic_auth_password'] = $glpikey->decrypt($config['basic_auth_password']);
         $canedit = \Config::canUpdate();
         $rand = mt_rand();
 
@@ -406,10 +404,10 @@ public function showConfigForm()
             echo "</td></tr>";
             echo "<tr class='tab_bg_1' id='basic_auth_login_row'>";
             echo "<td>";
-            echo "<label for='basic_auth_login'>"
-                . __('Login') .
-                "<span class='required'>*</span>
-                </label>";
+            echo "<label for='basic_auth_login'>";
+            echo __s('Login');
+            echo "<span class='required'>*</span>";
+            echo "</label>";
             echo "</td>";
             echo "<td>";
             echo Html::input("basic_auth_login", [
@@ -419,14 +417,14 @@ public function showConfigForm()
             echo "</tr>";
             echo "<tr class='tab_bg_1' id='basic_auth_password_row'>";
             echo "<td>";
-            echo "<label for='basic_auth_password'>"
-                . __('Password') .
-                "<span class='required'>*</span>
-                </label>";
+            echo "<label for='basic_auth_password'>";
+            echo __s('Password');
+            echo "<span class='required'>*</span>";
+            echo "</label>";
             echo "</td>";
             echo "<td>";
             echo Html::input("basic_auth_password", [
-                "value" => $config["basic_auth_password"],
+                "value" => (new GLPIKey())->decrypt($config['basic_auth_password']),
                 "type" => "password",
             ]);
             echo "</td>";
@@ -1152,6 +1150,24 @@ public function saveConf(array $values)
             $values['stale_agents_status_condition'] = ['all'];
         }
 
+        if (
+            (
+                !$values['basic_auth_password'] ||
+                !$values['basic_auth_login']
+            ) && $values['auth_required'] === Conf::BASIC_AUTH
+        ) {
+            Session::addMessageAfterRedirect(
+                __s("Basic Authentication is active. The login and/or password fields are missing."),
+                false,
+                ERROR
+            );
+            return false;
+        }
+
+        if (!is_null($values['basic_auth_password'])) {
+            $values['basic_auth_password'] = (new GLPIKey())->encrypt($values['basic_auth_password']);
+        }
+
         $to_process = [];
         foreach ($defaults as $prop => $default_value) {
             $to_process[$prop] = $values[$prop] ?? $default_value;