From 3cb91ad05af569d798d48f1a8958b3ca0f7ee686 Mon Sep 17 00:00:00 2001
From: Prasad <prasad@vtiger.com>
Date: Mon, 6 May 2024 13:03:03 +0530
Subject: [PATCH] Refactored Users save action validation

---
 modules/Users/actions/Save.php     | 12 ++++++++----
 modules/Users/actions/SaveAjax.php | 13 +++++++++++++
 modules/Users/models/Record.php    | 10 ++++++++--
 3 files changed, 29 insertions(+), 6 deletions(-)

diff --git a/modules/Users/actions/Save.php b/modules/Users/actions/Save.php
index 378a09d58..e0155d7b8 100644
--- a/modules/Users/actions/Save.php
+++ b/modules/Users/actions/Save.php
@@ -109,6 +109,13 @@ class Users_Save_Action extends Vtiger_Save_Action {
 		return $recordModel;
 	}
 
+	protected function checkRestrictedValueChange(Vtiger_Request $request) {
+		if ($request->has('user_name') || $request->has('user_password') || $request->has('accesskey') ) {
+			// should use separate actions.
+			throw new AppException(vtranslate('LBL_PERMISSION_DENIED', $module));
+		}
+	}
+
 	public function process(Vtiger_Request $request) {
 		$result = Vtiger_Util_Helper::transformUploadedFiles($_FILES, true);
 		$_FILES = $result['imagename'];
@@ -123,10 +130,7 @@ class Users_Save_Action extends Vtiger_Save_Action {
 				throw new AppException(vtranslate('LBL_DUPLICATE_USER_EXISTS', $module));
 			}
 		} else {
-			if ($request->has('user_name') || $request->has('user_password') || $request->has('accesskey') ) {
-				// should use separate actions.
-				throw new AppException(vtranslate('LBL_PERMISSION_DENIED', $module));
-			}
+			$this->checkRestrictedValueChange($request);
 		}
 
 		$recordModel = $this->saveRecord($request);
diff --git a/modules/Users/actions/SaveAjax.php b/modules/Users/actions/SaveAjax.php
index 4c242a029..e136a1c8a 100644
--- a/modules/Users/actions/SaveAjax.php
+++ b/modules/Users/actions/SaveAjax.php
@@ -43,12 +43,25 @@ class Users_SaveAjax_Action extends Vtiger_SaveAjax_Action {
 		}
 	}
 
+	protected function checkRestrictedValueChange(Vtiger_Request $request) {
+		if ($request->has('user_name') || $request->has('user_password') || $request->has('accesskey') ) {
+			// should use separate actions.
+			throw new AppException(vtranslate('LBL_PERMISSION_DENIED', $module));
+		}
+		if ($request->has('field') && in_array($request->get('field'), array('user_name', 'user_password', 'accesskey'))) {
+			// should use separate actions.
+			throw new AppException(vtranslate('LBL_PERMISSION_DENIED', $module));
+		}
+	}
+	
 	public function process(Vtiger_Request $request) {
 
 		$mode = $request->get('mode');
 		if (!empty($mode)) {
 			$this->invokeExposedMethod($mode, $request);
 			return;
+		} else {
+			$this->checkRestrictedValueChange($request);
 		}
 
 		$recordModel = $this->saveRecord($request);
diff --git a/modules/Users/models/Record.php b/modules/Users/models/Record.php
index 06fc11f1d..2133ea354 100644
--- a/modules/Users/models/Record.php
+++ b/modules/Users/models/Record.php
@@ -880,10 +880,16 @@ class Users_Record_Model extends Vtiger_Record_Model {
 	 */
 	public static function changeUsername($newUsername,$newpassword,$oldPassword,$forUserId) {
 		$response = array('success'=> false,'message' => 'error');
-		$record = self::getInstanceFromPreferenceFile($forUserId);
-		$moduleName = $record->getModuleName();
+		$moduleName = "Users";
 		$currentUserModel = static::getCurrentUserModel();
 		
+		if (empty($oldPassword) || empty($newpassword) || empty($forUserId)) {
+			$response['message'] = vtranslate('ERROR_CHANGE_USERNAME', $moduleName);
+			return $response;
+		}
+
+		$record = self::getInstanceFromPreferenceFile($forUserId);
+
 		if($currentUserModel->getId() == $forUserId || !Users_Privileges_Model::isPermittedToChangeUsername($forUserId)) {
 			$response['message'] = vtranslate('LBL_PERMISSION_DENIED', $moduleName);
 			return $response;
-- 
GitLab