Merge pull request #4379 from nilsteampassnet/review

Code review fixes
nilsteampassnet · Sep 30, 2024 · 2f2bbe8 · 2f2bbe8
2 parents 912a165 + ed4f43e
commit 2f2bbe8
Show file tree

Hide file tree

Showing 8 changed files with 105 additions and 44 deletions.
diff --git a/docs/install/installation.md b/docs/install/installation.md
@@ -9,7 +9,12 @@ The easiest way to install Teampass is to install LAMP dedicated to the GNU/Linu
 
 This document highlights a basic setup, but you can refer to many other existing tutorials to install Apache, MariaDB (or mySQL) and PHP.
 
-> :bulb: **Note:**  Teampass requires at least PHP 7.4 version.
+> :bulb: **Note:**  Teampass should be installed using the most recent PHP version.
+>The branch `master` is the living one that is improved and comes with new features.
+>It requires __at least__ `PHP 8.1` installed on the server.
+>Nevertheless, Teampass can be used with PHP 7.4 version.
+>The Github Teampass project has a dedicated branch called `PHP_7.4` for this version.
+>Notice that only bug fixing will be performed on this branch.
 
 ### Install the Apache web server and the required PHP extensions
 

diff --git a/includes/config/include.php b/includes/config/include.php
@@ -28,7 +28,7 @@
 
 define('TP_VERSION', '3.1.2');
 define("UPGRADE_MIN_DATE", "1727110744");
-define('TP_VERSION_MINOR', '126');
+define('TP_VERSION_MINOR', '130');
 define('TP_TOOL_NAME', 'Teampass');
 define('TP_ONE_DAY_SECONDS', 86400);
 define('TP_ONE_WEEK_SECONDS', 604800);

diff --git a/includes/tables_integrity.json b/includes/tables_integrity.json
@@ -13,11 +13,11 @@
     },
     {
         "table_name": "background_tasks",
-        "structure_hash": "019216c55451e2995810d42c1dfd236a3bd57993a963e6d93468ac65fa802529"
+        "structure_hash": "c3b96e3d6b07ca079266f59370af356e84848c6863aaa3662f06ffaf42b65b55"
     },
     {
         "table_name": "background_tasks_logs",
-        "structure_hash": "8f780290562b44d9d4f369afedfbfb15321a14ffe2203a76309751d80c6bfb4c"
+        "structure_hash": "b1b6b00ee651771e9085d2f15f0ab6ca2a093555a40be7b24a4b84b2dad623fd"
     },
     {
         "table_name": "cache",
@@ -101,15 +101,15 @@
     },
     {
         "table_name": "log_system",
-        "structure_hash": "1e6bc407e3d9084514392f7aee11af12344d6c86c735ec613319cbbf0444bd52"
+        "structure_hash": "0495909d3975e4a801010849068ce29bcefcd5a6ac0de46de8f5c2d1d7361b12"
     },
     {
         "table_name": "misc",
         "structure_hash": "3cc8939148fb17fabdabcea7eeef550f261c1b62bcbf863e5f9cb5984ad448d1"
     },
     {
         "table_name": "nested_tree",
-        "structure_hash": "43c41856e67406da11202578e7275cb7927d8a8cad833999e5588128ebba6ea6"
+        "structure_hash": "bd4056f24f5dc53535872c6b6821c03ab3191ea9ce0680c6050ae01fa2cd751d"
     },
     {
         "table_name": "notification",

diff --git a/pages/users.js.php b/pages/users.js.php
@@ -1599,6 +1599,9 @@ function(teampassApplication) {
                                 timeOut: 1000
                             }
                         );
+
+                        // Rrefresh list of users in Teampass
+                        oTable.ajax.reload();
                     }
                 }
             );

diff --git a/sources/identify.php b/sources/identify.php
@@ -392,10 +392,10 @@ function identifyUser(string $sentData, array $SETTINGS): bool
         );
         return false;
     }
-
+    
     // Check user and password
     if ($userLdap['userPasswordVerified'] === false && $userOauth2['userPasswordVerified'] === false
-        && (int) checkCredentials($passwordClear, $userInfo) !== 1
+        && checkCredentials($passwordClear, $userInfo) !== true
     ) {
         echo prepareExchangedData(
             [
@@ -476,7 +476,7 @@ function identifyUser(string $sentData, array $SETTINGS): bool
             return false;
         }
     }
-    
+
     // Can connect if
     // 1- no LDAP mode + user enabled + pw ok
     // 2- LDAP mode + user enabled + ldap connection ok + user is not admin
@@ -1983,7 +1983,7 @@ function duoMFAPerform(
  *
  * @return bool
  */
-function checkCredentials($passwordClear, $userInfo)
+function checkCredentials($passwordClear, $userInfo): bool
 {
     $passwordManager = new PasswordManager();
     // Migrate password if needed
@@ -1992,7 +1992,6 @@ function checkCredentials($passwordClear, $userInfo)
         $passwordClear,
         (int) $userInfo['id']
     );
-    if (WIP === true) error_log("checkCredentials - User ".$userInfo['id']." | verify pwd: ".$passwordManager->verifyPassword($userInfo['pw'], $passwordClear));
 
     if ($passwordManager->verifyPassword($userInfo['pw'], $passwordClear) === false) {
         // password is not correct
@@ -2359,6 +2358,8 @@ function shouldUserAuthWithOauth2(
         return [
             'error' => true,
             'message' => 'user_not_allowed_to_auth_to_teampass_app',
+            'oauth2Connection' => false,
+            'userPasswordVerified' => false,
         ];
     }
 
@@ -2385,12 +2386,24 @@ function shouldUserAuthWithOauth2(
                 return [
                     'error' => false,
                     'message' => '',
+                    'oauth2Connection' => true,
+                    'userPasswordVerified' => true,
                 ];
-            } elseif ((string) $userInfo['auth_type'] !== 'oauth2') {
+            } elseif ((string) $userInfo['auth_type'] === 'oauth2') {
+                // OAuth2 login request on OAuth2 user account.
+                return [
+                    'error' => false,
+                    'message' => '',
+                    'oauth2Connection' => true,
+                    'userPasswordVerified' => true,
+                ];
+            } else {
                 // Case where auth_type is not managed
                 return [
                     'error' => true,
                     'message' => 'user_not_allowed_to_auth_to_teampass_app',
+                    'oauth2Connection' => false,
+                    'userPasswordVerified' => false,
                 ];
             }
         } else {
@@ -2400,6 +2413,8 @@ function shouldUserAuthWithOauth2(
                 return [
                     'error' => true,
                     'message' => 'user_exists_but_not_oauth2',
+                    'oauth2Connection' => false,
+                    'userPasswordVerified' => false,
                 ];
             }
         }
@@ -2409,6 +2424,8 @@ function shouldUserAuthWithOauth2(
     return [
         'error' => false,
         'message' => '',
+        'oauth2Connection' => false,
+        'userPasswordVerified' => false,
     ];
 }
 
@@ -2488,8 +2505,8 @@ function createOauth2User(
         return [
             'error' => false,
             'retExternalAD' => $userInfo,
-            'oauth2Connection' => true,
-            'userPasswordVerified' => true,
+            'oauth2Connection' => $ret['oauth2Connection'],
+            'userPasswordVerified' => $ret['userPasswordVerified'],
         ];
     }
 

diff --git a/sources/upload.attachments.php b/sources/upload.attachments.php
@@ -33,6 +33,7 @@
 use TeampassClasses\SessionManager\SessionManager;
 use TeampassClasses\Language\Language;
 use TeampassClasses\PerformChecks\PerformChecks;
+use TeampassClasses\ConfigManager\ConfigManager;
 
 
 // Load functions

diff --git a/sources/users.queries.php b/sources/users.queries.php
@@ -2078,33 +2078,60 @@
                 'decode'
             );
 
-            $post_source_id = filter_var(htmlspecialchars_decode($dataReceived['source_id']), FILTER_SANITIZE_NUMBER_INT);
-            $post_destination_ids = filter_var_array($dataReceived['destination_ids'], FILTER_SANITIZE_NUMBER_INT);
-            $post_user_functions = filter_var(htmlspecialchars_decode($dataReceived['user_functions']), FILTER_SANITIZE_FULL_SPECIAL_CHARS);
-            $post_user_managedby = filter_var(htmlspecialchars_decode($dataReceived['user_managedby']), FILTER_SANITIZE_FULL_SPECIAL_CHARS);
-            $post_user_fldallowed = filter_var(htmlspecialchars_decode($dataReceived['user_fldallowed']), FILTER_SANITIZE_FULL_SPECIAL_CHARS);
-            $post_user_fldforbid = filter_var(htmlspecialchars_decode($dataReceived['user_fldforbid']), FILTER_SANITIZE_FULL_SPECIAL_CHARS);
-            $post_user_admin = filter_var(htmlspecialchars_decode($dataReceived['user_admin']), FILTER_SANITIZE_NUMBER_INT);
-            $post_user_manager = filter_var(htmlspecialchars_decode($dataReceived['user_manager']), FILTER_SANITIZE_NUMBER_INT);
-            $post_user_hr = filter_var(htmlspecialchars_decode($dataReceived['user_hr']), FILTER_SANITIZE_NUMBER_INT);
-            $post_user_readonly = filter_var(htmlspecialchars_decode($dataReceived['user_readonly']), FILTER_SANITIZE_NUMBER_INT);
-            $post_user_personalfolder = filter_var(htmlspecialchars_decode($dataReceived['user_personalfolder']), FILTER_SANITIZE_NUMBER_INT);
-            $post_user_rootfolder = filter_var(htmlspecialchars_decode($dataReceived['user_rootfolder']), FILTER_SANITIZE_NUMBER_INT);
+            // Prepare variables
+            $data = [
+                'source_id' => isset($dataReceived['source_id']) === true ? $dataReceived['source_id'] : 0,
+                'destination_ids' => isset($dataReceived['destination_ids']) === true ? $dataReceived['destination_ids'] : 0,
+                'user_functions' => isset($dataReceived['user_functions']) === true ? $dataReceived['user_functions'] : '',
+                'user_managedby' => isset($dataReceived['user_managedby']) === true ? $dataReceived['user_managedby'] : '',
+                'user_fldallowed' => isset($dataReceived['user_fldallowed']) === true ? $dataReceived['user_fldallowed'] : '',
+                'user_fldforbid' => isset($dataReceived['user_fldforbid']) === true ? $dataReceived['user_fldforbid'] : '',
+                'user_admin' => isset($dataReceived['user_admin']) === true ? $dataReceived['user_admin'] : 0,
+                'user_manager' => isset($dataReceived['user_manager']) === true ? $dataReceived['user_manager'] : 0,
+                'user_hr' => isset($dataReceived['user_hr']) === true ? $dataReceived['user_hr'] : 0,
+                'user_readonly' => isset($dataReceived['user_readonly']) === true ? $dataReceived['user_readonly'] : 1,
+                'user_personalfolder' => isset($dataReceived['user_personalfolder']) === true ? $dataReceived['user_personalfolder'] : 0,
+                'user_rootfolder' => isset($dataReceived['user_rootfolder']) === true ? $dataReceived['user_rootfolder'] : 0,
+            ];
+
+            $filters = [
+                'source_id' => 'cast:integer',
+                'destination_ids' => 'trim|escape',
+                'user_functions' => 'trim|escape',
+                'user_managedby' => 'trim|escape',
+                'user_fldallowed' => 'trim|escape',
+                'user_fldforbid' => 'trim|escape',
+                'user_admin' => 'cast:integer',
+                'user_manager' => 'cast:integer',
+                'user_hr' => 'cast:integer',
+                'user_readonly' => 'cast:integer',
+                'user_personalfolder' => 'cast:integer',
+                'user_rootfolder' => 'cast:integer',
+            ];
+
+            $inputData = dataSanitizer(
+                $data,
+                $filters,
+                $SETTINGS['cpassman_dir']
+            );
 
             // Check send values
-            if (
-                empty($post_source_id) === true
-                || $post_destination_ids === 0
-            ) {
+            if ($inputData['source_id'] === 0 || $inputData['destination_ids'] === 0) {
                 // error
-                exit();
+                echo prepareExchangedData(
+                    array(
+                        'error' => true,
+                        'message' => $lang->get('error_not_allowed_to'),
+                    ),
+                    'encode'
+                );
             }
 
             // Get info about user
             $data_user = DB::queryfirstrow(
                 'SELECT admin, isAdministratedByRole FROM ' . prefixTable('users') . '
                 WHERE id = %i',
-                $post_source_id
+                $inputData['source_id']
             );
 
             // Is this user allowed to do this?
@@ -2113,7 +2140,7 @@
                 || (in_array($data_user['isAdministratedByRole'], $session->get('user-roles_array')))
                 || ((int) $session->get('user-can_manage_all_users') === 1 && (int) $data_user['admin'] !== 1)
             ) {
-                foreach ($post_destination_ids as $dest_user_id) {
+                foreach ($inputData['destination_ids'] as $dest_user_id) {
                     // Is this user allowed to do this?
                     if (
                         (int) $session->get('user-admin') === 1
@@ -2124,23 +2151,31 @@
                         DB::update(
                             prefixTable('users'),
                             array(
-                                'fonction_id' => $post_user_functions,
-                                'isAdministratedByRole' => $post_user_managedby,
-                                'groupes_visibles' => $post_user_fldallowed,
-                                'groupes_interdits' => $post_user_fldforbid,
-                                'gestionnaire' => $post_user_manager,
-                                'read_only' => $post_user_readonly,
-                                'can_create_root_folder' => $post_user_rootfolder,
-                                'personal_folder' => $post_user_personalfolder,
-                                'can_manage_all_users' => $post_user_hr,
-                                'admin' => $post_user_admin,
+                                'fonction_id' => str_replace(",", ";", (string) $inputData['user_functions']),
+                                'isAdministratedByRole' => $inputData['user_managedby'],
+                                'groupes_visibles' => $inputData['user_fldallowed'],
+                                'groupes_interdits' => $inputData['user_fldforbid'],
+                                'gestionnaire' => $inputData['user_manager'],
+                                'read_only' => $inputData['user_readonly'],
+                                'can_create_root_folder' => $inputData['user_rootfolder'],
+                                'personal_folder' => $inputData['user_personalfolder'],
+                                'can_manage_all_users' => $inputData['user_hr'],
+                                'admin' => $inputData['user_admin'],
                             ),
                             'id = %i',
                             $dest_user_id
                         );
                     }
                 }
             }
+
+            echo prepareExchangedData(
+                array(
+                    'error' => false,
+                ),
+                'encode'
+            );
+
             break;
 
             /*

diff --git a/vendor/composer/autoload_real.php b/vendor/composer/autoload_real.php
@@ -35,7 +35,7 @@ public static function getLoader()
         require __DIR__ . '/autoload_static.php';
         call_user_func(\Composer\Autoload\ComposerStaticInite3f3ee27f81ca21f7bd7499d7b935c11::getInitializer($loader));
 
-        $loader->setApcuPrefix('b95328986d6b35bd5725');
+        $loader->setApcuPrefix('10b5b5505b2ef65ba33c');
         $loader->register(true);
 
         $filesToLoad = \Composer\Autoload\ComposerStaticInite3f3ee27f81ca21f7bd7499d7b935c11::$files;
-Original file line number
+Diff line change
@@ Expand Up / @@ -1599,6 +1599,9 @@ function(teampassApplication) { @@
                                     timeOut: 1000
                                 }
                             );
+                            // Rrefresh list of users in Teampass
+                            oTable.ajax.reload();
                         }
                     }
                 );
@@ Expand Down @@