..F....... [ZBXNEXT-5965] updated session id generation

9 files changed, 124 insertions, 143 deletions

diff --git a/ui/include/classes/api/CApiService.php b/ui/include/classes/api/CApiService.php
index fd4a72fd6ba..1283c42e92a 100644
--- a/ui/include/classes/api/CApiService.php
+++ b/ui/include/classes/api/CApiService.php
@@ -22,13 +22,6 @@
 class CApiService {
 
 	/**
-	 * Authentication token.
-	 *
-	 * @var string
-	 */
-	public static $auth;
-
-	/**
 	 * Authorized user data.
 	 *
 	 * @var array
diff --git a/ui/include/classes/api/services/CUser.php b/ui/include/classes/api/services/CUser.php
index 3a494bf21b8..54f004cc2a4 100644
--- a/ui/include/classes/api/services/CUser.php
+++ b/ui/include/classes/api/services/CUser.php
@@ -1172,10 +1172,12 @@ class CUser extends CApiService {
 			self::exception(ZBX_API_ERROR_PARAMETERS, $error);
 		}
 
+		$sessionid = self::$userData['sessionid'];
+
 		$db_sessions = DB::select('sessions', [
 			'output' => ['userid'],
 			'filter' => [
-				'sessionid' => self::$auth,
+				'sessionid' => $sessionid,
 				'status' => ZBX_SESSION_ACTIVE
 			],
 			'limit' => 1
@@ -1191,7 +1193,7 @@ class CUser extends CApiService {
 		]);
 		DB::update('sessions', [
 			'values' => ['status' => ZBX_SESSION_PASSIVE],
-			'where' => ['sessionid' => self::$auth]
+			'where' => ['sessionid' => $sessionid]
 		]);
 
 		$this->addAuditDetails(AUDIT_ACTION_LOGOUT, AUDIT_RESOURCE_USER);
@@ -1367,10 +1369,10 @@ class CUser extends CApiService {
 			self::exception(ZBX_API_ERROR_PARAMETERS, $error);
 		}
 
-		self::$auth = $session['sessionid'];
+		$sessionid = $session['sessionid'];
 
 		// access DB only once per page load
-		if (self::$userData !== null && self::$userData['sessionid'] === self::$auth) {
+		if (self::$userData !== null && self::$userData['sessionid'] === $sessionid) {
 			return self::$userData;
 		}
 
@@ -1378,7 +1380,7 @@ class CUser extends CApiService {
 
 		$db_sessions = DB::select('sessions', [
 			'output' => ['userid', 'lastaccess', 'status'],
-			'sessionids' => self::$auth
+			'sessionids' => $sessionid
 		]);
 
 		// If session not created.
@@ -1409,7 +1411,7 @@ class CUser extends CApiService {
 
 		$usrgrps = $this->getUserGroupsData($db_user['userid']);
 
-		$db_user['sessionid'] = self::$auth;
+		$db_user['sessionid'] = $sessionid;
 		$db_user['debug_mode'] = $usrgrps['debug_mode'];
 		$db_user['userip'] = $usrgrps['userip'];
 		$db_user['gui_access'] = $usrgrps['gui_access'];
@@ -1435,7 +1437,7 @@ class CUser extends CApiService {
 			]);
 			DB::update('sessions', [
 				'values' => ['status' => ZBX_SESSION_PASSIVE],
-				'where' => ['sessionid' => self::$auth]
+				'where' => ['sessionid' => $sessionid]
 			]);
 
 			self::exception(ZBX_API_ERROR_PARAMETERS, _('Session terminated, re-login, please.'));
@@ -1444,7 +1446,7 @@ class CUser extends CApiService {
 		if ($session['extend'] && $time != $db_session['lastaccess']) {
 			DB::update('sessions', [
 				'values' => ['lastaccess' => $time],
-				'where' => ['sessionid' => self::$auth]
+				'where' => ['sessionid' => $sessionid]
 			]);
 		}
 
@@ -1559,11 +1561,7 @@ class CUser extends CApiService {
 	 * @return array
 	 */
 	private function createSession(array $db_user): array {
-		$db_user['sessionid'] = self::$auth;
-
-		DB::delete('sessions', [
-			'sessionid' => $db_user['sessionid']
-		]);
+		$db_user['sessionid'] = CEncryptHelper::generateKey();
 
 		DB::insert('sessions', [[
 			'sessionid' => $db_user['sessionid'],
diff --git a/ui/include/classes/api/wrappers/CApiWrapper.php b/ui/include/classes/api/wrappers/CApiWrapper.php
index d43e0779008..7b87cb777e3 100644
--- a/ui/include/classes/api/wrappers/CApiWrapper.php
+++ b/ui/include/classes/api/wrappers/CApiWrapper.php
@@ -32,6 +32,13 @@ class CApiWrapper {
 	public $api;
 
 	/**
+	 * Authentication token.
+	 *
+	 * @var string
+	 */
+	public $auth;
+
+	/**
 	 * Current API client.
 	 *
 	 * @var CApiClient
@@ -96,6 +103,6 @@ class CApiWrapper {
 	 * @return CApiClientResponse
 	 */
 	protected function callClientMethod($method, $params) {
-		return $this->client->callMethod($this->api, $method, $params, CApiService::$auth);
+		return $this->client->callMethod($this->api, $method, $params, $this->auth);
 	}
 }
diff --git a/ui/include/classes/api/wrappers/CFrontendApiWrapper.php b/ui/include/classes/api/wrappers/CFrontendApiWrapper.php
index 09b639c0768..e2c524675d7 100644
--- a/ui/include/classes/api/wrappers/CFrontendApiWrapper.php
+++ b/ui/include/classes/api/wrappers/CFrontendApiWrapper.php
@@ -92,7 +92,7 @@ class CFrontendApiWrapper extends CApiWrapper {
 	 * @return CApiClientResponse
 	 */
 	protected function callClientMethod($method, $params) {
-		$auth = ($this->requiresAuthentication($this->api, $method)) ? CApiService::$auth : null;
+		$auth = ($this->requiresAuthentication($this->api, $method)) ? $this->auth : null;
 
 		return $this->client->callMethod($this->api, $method, $params, $auth);
 	}
diff --git a/ui/include/classes/core/CCookieSession.php b/ui/include/classes/core/CCookieSession.php
index c07f0cf0d56..7ca4f323c12 100644
--- a/ui/include/classes/core/CCookieSession.php
+++ b/ui/include/classes/core/CCookieSession.php
@@ -22,7 +22,7 @@
 /**
  * Session wrapper uses cookie for session store.
  */
-class CCookieSession implements \SessionHandlerInterface {
+class CCookieSession implements SessionHandlerInterface {
 
 	/**
 	 * Cookie name.
@@ -43,35 +43,30 @@ class CCookieSession implements \SessionHandlerInterface {
 			session_set_save_handler([$this, 'open'], [$this, 'close'], [$this, 'read'],
 				[$this, 'write'], [$this, 'destroy'], [$this, 'gc']
 			);
-
-			if (!$this->session_start()) {
-				throw new \Exception(_('Session initialization error.'));
-			}
-
-			CSessionHelper::set('sessionid', CSessionHelper::getId());
 		}
 	}
 
 	/**
 	 * @inheritDoc
 	 *
+	 * @param string $save_path
+	 * @param string $session_name
+	 *
 	 * @return boolean
 	 */
-	public function close() {
-		echo ob_get_clean();
+	public function open($save_path, $session_name) {
+		ob_start();
 
-		return true;
+		return session_status() === PHP_SESSION_ACTIVE;
 	}
 
 	/**
 	 * @inheritDoc
 	 *
-	 * @param string $session_id
-	 *
 	 * @return boolean
 	 */
-	public function destroy($session_id) {
-		CCookieHelper::unset(self::COOKIE_NAME);
+	public function close() {
+		echo ob_get_clean();
 
 		return true;
 	}
@@ -79,26 +74,30 @@ class CCookieSession implements \SessionHandlerInterface {
 	/**
 	 * @inheritDoc
 	 *
-	 * @param integer $maxlifetime
+	 * @param string $session_id
 	 *
-	 * @return integer
+	 * @return string
 	 */
-	public function gc($maxlifetime) {
-		return true;
+	public function read($session_id) {
+		return $this->parseData();
 	}
 
 	/**
 	 * @inheritDoc
 	 *
-	 * @param string $save_path
-	 * @param string $session_name
+	 * @param string $session_id
+	 * @param string $session_data
 	 *
 	 * @return boolean
 	 */
-	public function open($save_path, $session_name) {
-		ob_start();
+	public function write($session_id, $session_data) {
+		$session_data = $this->prepareData($session_data);
 
-		return session_status() === PHP_SESSION_ACTIVE;
+		if (!CCookieHelper::set(self::COOKIE_NAME, $session_data, 0)) {
+			throw new \Exception(_('Cannot set session cookie.'));
+		}
+
+		return true;
 	}
 
 	/**
@@ -106,44 +105,34 @@ class CCookieSession implements \SessionHandlerInterface {
 	 *
 	 * @param string $session_id
 	 *
-	 * @return string
+	 * @return boolean
 	 */
-	public function read($session_id) {
-		return $this->parseData();
+	public function destroy($session_id) {
+		CCookieHelper::unset(self::COOKIE_NAME);
+
+		return true;
 	}
 
 	/**
 	 * @inheritDoc
 	 *
-	 * @param string $session_id
-	 * @param string $session_data
+	 * @param integer $maxlifetime
 	 *
-	 * @return boolean
+	 * @return integer
 	 */
-	public function write($session_id, $session_data) {
-		$session_data = $this->prepareData($session_data);
-
-		if (!CCookieHelper::set(self::COOKIE_NAME, $session_data, 0)) {
-			throw new \Exception(_('Cannot set session cookie.'));
-		}
-
+	public function gc($maxlifetime) {
 		return true;
 	}
 
 	/**
 	 * Run session_start.
 	 *
+	 * @param string $sessionid
+	 *
 	 * @return boolean
 	 */
-	protected function session_start(): bool {
-		$session_data = $this->parseData();
-
-		if (mb_strlen($session_data) === 0) {
-			return session_start();
-		}
-
-		$sessionid = $this->extractSessionId($session_data);
-		session_id($sessionid ?: CEncryptHelper::generateKey());
+	public function session_start(string $sessionid): bool {
+		session_id($sessionid);
 
 		return session_start();
 	}
@@ -151,31 +140,22 @@ class CCookieSession implements \SessionHandlerInterface {
 	/**
 	 * Extract session id from session data.
 	 *
-	 * @param string $session_data
-	 *
 	 * @return string|null
 	 */
-	protected function extractSessionId(string $session_data): ?string {
-		$session_data = unserialize($session_data);
+	public function extractSessionId(): ?string {
+		$session_data = $this->parseData();
 
-		if (array_key_exists('sessionid', $session_data)) {
-			return $session_data['sessionid'];
+		if ($session_data === '') {
+			return null;
 		}
 
-		return null;
-	}
+		$session_data = unserialize($session_data);
 
-	/**
-	 * Prepare session data.
-	 *
-	 * @param string $data
-	 *
-	 * @return string
-	 */
-	protected function prepareData(string $data): string {
-		$data = unserialize($data);
+		if (!array_key_exists('sessionid', $session_data)) {
+			return null;
+		}
 
-		return base64_encode(serialize($data));
+		return $session_data['sessionid'];
 	}
 
 	/**
@@ -190,4 +170,17 @@ class CCookieSession implements \SessionHandlerInterface {
 
 		return '';
 	}
+
+	/**
+	 * Prepare session data.
+	 *
+	 * @param string $data
+	 *
+	 * @return string
+	 */
+	protected function prepareData(string $data): string {
+		$data = unserialize($data);
+
+		return base64_encode(serialize($data));
+	}
 }
diff --git a/ui/include/classes/core/CEncryptedCookieSession.php b/ui/include/classes/core/CEncryptedCookieSession.php
index 2be9bfe829b..73746478636 100644
--- a/ui/include/classes/core/CEncryptedCookieSession.php
+++ b/ui/include/classes/core/CEncryptedCookieSession.php
@@ -25,19 +25,28 @@
 class CEncryptedCookieSession extends CCookieSession {
 
 	/**
-	 * Prepare data and check sign.
-	 *
-	 * @param string $data
+	 * @inheritDoc
 	 *
-	 * @return boolean
+	 * @return string|null
 	 */
-	protected function checkSign(string $data): bool {
-		$data = unserialize($data);
-		$session_sign = $data['sign'];
-		unset($data['sign']);
-		$sign = CEncryptHelper::sign(serialize($data));
+	public function extractSessionId(): ?string {
+		if (CSettingsHelper::getGlobal(CSettingsHelper::SESSION_KEY) === '') {
+			CEncryptHelper::updateKey(CEncryptHelper::generateKey());
+		}
 
-		return $session_sign && $sign && CEncryptHelper::checkSign($session_sign, $sign);
+		$session_data = $this->parseData();
+
+		if ($session_data === '' || !$this->checkSign($session_data)) {
+			return null;
+		}
+
+		$session_data = unserialize($session_data);
+
+		if (!array_key_exists('sessionid', $session_data)) {
+			return null;
+		}
+
+		return $session_data['sessionid'];
 	}
 
 	/**
@@ -60,39 +69,18 @@ class CEncryptedCookieSession extends CCookieSession {
 	}
 
 	/**
-	 * @inheritDoc
-	 *
-	 * @return boolean
-	 */
-	protected function session_start(): bool {
-		if (!$this->checkSessionKey()) {
-			CEncryptHelper::updateKey(CEncryptHelper::generateKey());
-		}
-
-		$session_data = $this->parseData();
-
-		if (mb_strlen($session_data) === 0 || !$this->checkSign($session_data)) {
-			return session_start();
-		}
-
-		$sessionid = $this->extractSessionId($session_data);
-		session_id($sessionid ?: CEncryptHelper::generateKey());
-
-		return session_start();
-	}
-
-	/**
-	 * Check exist secret session key.
+	 * Prepare data and check sign.
 	 *
-	 * @throws \Exception
+	 * @param string $data
 	 *
 	 * @return boolean
 	 */
-	private function checkSessionKey(): bool {
-		if (CSettingsHelper::getGlobal(CSettingsHelper::SESSION_KEY) === '') {
-			return false;
-		}
+	protected function checkSign(string $data): bool {
+		$data = unserialize($data);
+		$session_sign = $data['sign'];
+		unset($data['sign']);
+		$sign = CEncryptHelper::sign(serialize($data));
 
-		return true;
+		return $session_sign && $sign && CEncryptHelper::checkSign($session_sign, $sign);
 	}
 }
diff --git a/ui/include/classes/core/ZBase.php b/ui/include/classes/core/ZBase.php
index ea1e8e0787f..ab2065915fa 100644
--- a/ui/include/classes/core/ZBase.php
+++ b/ui/include/classes/core/ZBase.php
@@ -154,9 +154,10 @@ class ZBase {
 	/**
 	 * Initializes the application.
 	 *
-	 * @param string $mode  Application initialization mode.
+	 * @param string $mode Application initialization mode.
 	 *
 	 * @throws DBException
+	 * @throws Exception
 	 */
 	public function run($mode) {
 		$this->init();
@@ -168,15 +169,9 @@ class ZBase {
 
 		switch ($mode) {
 			case self::EXEC_MODE_DEFAULT:
-
 				$this->loadConfigFile();
 				$this->initDB();
-
 				$this->initLocales(CSettingsHelper::getGlobal(CSettingsHelper::DEFAULT_LANG));
-
-				// Start sesion only after DB initialized.
-				new CEncryptedCookieSession();
-
 				$this->authenticateUser();
 
 				if (CWebUser::$data['lang'] !== CSettingsHelper::get(CSettingsHelper::DEFAULT_LANG)) {
@@ -207,9 +202,6 @@ class ZBase {
 
 			case self::EXEC_MODE_API:
 				$this->loadConfigFile();
-
-				CUser::$auth = CEncryptHelper::generateKey();
-
 				$this->initDB();
 				$this->initLocales('en_gb');
 				break;
@@ -219,12 +211,9 @@ class ZBase {
 					// try to load config file, if it exists we need to init db and authenticate user to check permissions
 					$this->loadConfigFile();
 					$this->initDB();
-
-					new CEncryptedCookieSession();
-
 					$this->authenticateUser();
-					$this->initComponents();
 					$this->initLocales(CWebUser::$data['lang']);
+					$this->initComponents();
 				}
 				catch (ConfigFileException $e) {
 					if ($e->getCode() == CConfigFile::CONFIG_VAULT_ERROR) {
@@ -459,13 +448,26 @@ class ZBase {
 
 	/**
 	 * Authenticate user.
+	 *
+	 * @throws Exception
 	 */
 	protected function authenticateUser(): void {
-		if (!CWebUser::checkAuthentication(CSessionHelper::getId())) {
+		$session = new CEncryptedCookieSession();
+
+		if (!CWebUser::checkAuthentication($session->extractSessionId())) {
 			CWebUser::setDefault();
 		}
 
-		// enable debug mode in the API
+		if (!$session->session_start(CWebUser::$data['sessionid'])) {
+			throw new Exception(_('Session initialization error.'));
+		}
+
+		CSessionHelper::set('sessionid', CWebUser::$data['sessionid']);
+
+		// Set the authentication token for the API.
+		API::getWrapper()->auth = CWebUser::$data['sessionid'];
+
+		// Enable debug mode in the API.
 		API::getWrapper()->debug = CWebUser::getDebugMode();
 	}
 
diff --git a/ui/include/classes/user/CWebUser.php b/ui/include/classes/user/CWebUser.php
index 21788370253..eb6c6d69045 100644
--- a/ui/include/classes/user/CWebUser.php
+++ b/ui/include/classes/user/CWebUser.php
@@ -84,15 +84,13 @@ class CWebUser {
 	 * Log-out the current user.
 	 */
 	public static function logout(): void {
-		self::$data['sessionid'] = CSessionHelper::getId();
-
 		if (API::User()->logout([])) {
 			self::$data = null;
 			session_destroy();
 		}
 	}
 
-	public static function checkAuthentication(string $sessionid): bool {
+	public static function checkAuthentication(?string $sessionid): bool {
 		try {
 			self::$data = API::User()->checkAuthentication([
 				'sessionid' => $sessionid,
diff --git a/ui/index.php b/ui/index.php
index 69cdc1705c0..9050d3dbb64 100644
--- a/ui/index.php
+++ b/ui/index.php
@@ -71,6 +71,8 @@ if (!hasRequest('form') && CAuthenticationHelper::get(CAuthenticationHelper::HTT
 
 // login via form
 if (hasRequest('enter') && CWebUser::login(getRequest('name', ZBX_GUEST_USER), getRequest('password', ''))) {
+	CSessionHelper::set('sessionid', CWebUser::$data['sessionid']);
+
 	if (CWebUser::$data['autologin'] != $autologin) {
 		API::User()->update([
 			'userid' => CWebUser::$data['userid'],