diff options
author | Alexander Shubin <aleksandrs.subins@zabbix.com> | 2020-10-09 14:15:11 +0300 |
---|---|---|
committer | Alexander Shubin <aleksandrs.subins@zabbix.com> | 2020-10-09 14:15:11 +0300 |
commit | 4c5f84c992c75350d099f5831d0857ae10ab36df (patch) | |
tree | d0e47aaab6318c78dd66bff788a02aad4615bdfe /ui | |
parent | fa75e55994e01183840adc297004f91507175404 (diff) |
..F....... [ZBXNEXT-5965] updated session id generation
Diffstat (limited to 'ui')
-rw-r--r-- | ui/include/classes/api/CApiService.php | 7 | ||||
-rw-r--r-- | ui/include/classes/api/services/CUser.php | 24 | ||||
-rw-r--r-- | ui/include/classes/api/wrappers/CApiWrapper.php | 9 | ||||
-rw-r--r-- | ui/include/classes/api/wrappers/CFrontendApiWrapper.php | 2 | ||||
-rw-r--r-- | ui/include/classes/core/CCookieSession.php | 119 | ||||
-rw-r--r-- | ui/include/classes/core/CEncryptedCookieSession.php | 66 | ||||
-rw-r--r-- | ui/include/classes/core/ZBase.php | 34 | ||||
-rw-r--r-- | ui/include/classes/user/CWebUser.php | 4 | ||||
-rw-r--r-- | ui/index.php | 2 |
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'], |