From e84e472ebe517e2ff5795c46dc10b5f49dc4d46a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Markus=20Fa=C3=9Fbender?= Date: Sun, 13 Mar 2022 09:26:54 +0100 Subject: [PATCH] Sessions should be invalidated if password was changed #1238 --- adm_program/system/classes/Session.php | 45 ++++++++------ adm_program/system/classes/User.php | 84 +++++++++++++++++--------- 2 files changed, 85 insertions(+), 44 deletions(-) diff --git a/adm_program/system/classes/Session.php b/adm_program/system/classes/Session.php index 7e50e4deb4..80d2aa82c0 100644 --- a/adm_program/system/classes/Session.php +++ b/adm_program/system/classes/Session.php @@ -74,7 +74,7 @@ public function __construct(Database $database, $cookiePrefix = '') $this->readDataByColumns(array('ses_session_id' => $sessionId)); if ($this->newRecord) { - // if PHP session id was commited then store them in that field + // if PHP session id was committed then store them in that field $this->setValue('ses_session_id', $sessionId); $this->setValue('ses_timestamp', DATETIME_NOW); } @@ -117,9 +117,11 @@ protected function clearUserData() * Returns a CSRF token from the session. If no CSRF token exists a new one will be * generated and stored within the session. The next call of the method will than * return the existing token. The CSRF token has 30 characters. A new token could - * be forced by the paramter **$newToken** + * be forced by the parameter **$newToken** * @param bool $newToken If set to true, always a new token will be generated. * @return string Returns the CSRF token + * @throws AdmException + * @throws AdmException */ public function getCsrfToken(bool $newToken = false): string { @@ -132,7 +134,7 @@ public function getCsrfToken(bool $newToken = false): string /** * Returns a reference of an object that is stored in the session. - * This is necessary because the old database connection is not longer valid. + * This is necessary because the old database connection is not valid anymore. * @param string $objectName Internal unique name of the object. The name was set with the method **addObject** * @return object|false Returns the reference to the object or false if the object was not found. */ @@ -174,8 +176,8 @@ public function hasObject(string $objectName): bool } /** - * Initialize the array with all objects except the gNavigation object. If the session got an refresh - * the existing navigation should still be stored in the refreshed sesssion. + * Initialize the array with all objects except the gNavigation object. If the session got a refresh + * the existing navigation should still be stored in the refreshed session. */ public function initializeObjects() { @@ -201,7 +203,7 @@ public function isValidLogin(int $userId): bool // session has a user assigned -> check if login is still valid $timeGap = time() - strtotime($this->getValue('ses_timestamp', 'Y-m-d H:i:s')); - // Check how long the user was inactive. If time range is to long -> logout + // Check how long the user was inactive. If time range is too long -> logout // if user has auto login than session is also valid if ($this->mAutoLogin instanceof AutoLogin || $timeGap < $gSettingsManager->getInt('logout_minutes') * 60) { return true; @@ -261,8 +263,8 @@ public function refreshAutoLogin() $this->setValue('ses_usr_id', (int) $this->mAutoLogin->getValue('atl_usr_id')); // save cookie for autologin - $currDateTime = new \DateTime(); - $oneYearDateInterval = new \DateInterval('P1Y'); + $currDateTime = new DateTime(); + $oneYearDateInterval = new DateInterval('P1Y'); $oneYearAfterDateTime = $currDateTime->add($oneYearDateInterval); $timestampExpired = $oneYearAfterDateTime->getTimestamp(); @@ -297,8 +299,13 @@ public function refreshAutoLogin() */ public function refresh() { - // read session data from database to update the renew flag - $this->readDataById((int) $this->getValue('ses_id')); + // read session data from database to update the reload flag + if(!$this->readDataById((int) $this->getValue('ses_id'))) { + // if session was not found than destroy session object + unset($_SESSION['gCurrentSession']); + $this->initializeObjects(); + $this->clear(); + } // check if current connection has same ip address as of session initialization // if config parameter $gCheckIpAddress = 0 then don't check ip address @@ -370,9 +377,11 @@ public function reload(int $userId) */ public function save($updateFingerPrint = true): bool { + global $gCurrentOrgId; + if ($this->newRecord) { // Insert - $this->setValue('ses_org_id', $GLOBALS['gCurrentOrgId']); + $this->setValue('ses_org_id', $gCurrentOrgId); $this->setValue('ses_begin', DATETIME_NOW); // remove the last part of the IP because of privacy (GDPR) $ip = preg_replace(array('/\.\d+$/', '/[\da-f]*:[\da-f]*$/'), array('.XXX', 'XXXX:XXXX'), $_SERVER['REMOTE_ADDR']); @@ -403,8 +412,8 @@ public function setAutoLogin() $this->mAutoLogin->save(); // save cookie for autologin - $currDateTime = new \DateTime(); - $oneYearDateInterval = new \DateInterval('P1Y'); + $currDateTime = new DateTime(); + $oneYearDateInterval = new DateInterval('P1Y'); $oneYearAfterDateTime = $currDateTime->add($oneYearDateInterval); $timestampExpired = $oneYearAfterDateTime->getTimestamp(); @@ -478,7 +487,7 @@ public static function setCookie( * @param bool|null $secure If "true" cookie is only set if connection is HTTPS. Default is an auto detection. * @param bool $httpOnly If "true" cookie is accessible only via HTTP. * Set to "false" to allow access for JavaScript. (Possible XSS security leak) - * @throws \RuntimeException + * @throws RuntimeException */ public static function start(string $cookiePrefix, int $limit = 0, string $path = '', string $domain = '', bool $secure = null, bool $httpOnly = true) { @@ -488,7 +497,7 @@ public static function start(string $cookiePrefix, int $limit = 0, string $path $message = 'HTTP-Headers already sent!'; $gLogger->alert($message); - throw new \RuntimeException($message); + throw new RuntimeException($message); } $sessionName = $cookiePrefix . '_SESSION_ID'; @@ -543,11 +552,13 @@ public static function start(string $cookiePrefix, int $limit = 0, string $path /** * Deletes all sessions in table admSessions that are inactive since **$maxInactiveTime** minutes.. * @param int $maxInactiveMinutes Time in Minutes after that a session will be deleted. + * @throws Exception + * @throws Exception */ public function tableCleanup(int $maxInactiveMinutes = 30) { - $now = new \DateTime(); - $minutesBack = new \DateInterval('PT' . $maxInactiveMinutes . 'M'); + $now = new DateTime(); + $minutesBack = new DateInterval('PT' . $maxInactiveMinutes . 'M'); $timestamp = $now->sub($minutesBack)->format('Y-m-d H:i:s'); $sql = 'DELETE FROM '.TBL_SESSIONS.' diff --git a/adm_program/system/classes/User.php b/adm_program/system/classes/User.php index b2e104b2df..78419ba5c9 100644 --- a/adm_program/system/classes/User.php +++ b/adm_program/system/classes/User.php @@ -109,7 +109,7 @@ public function allowedEditProfileField(self $user, $fieldNameIntern) * @param string $fieldNameIntern Expects the **usf_name_intern** of the field that should be checked. * @return bool Return true if the current user is allowed to view this profile field of **$user**. */ - public function allowedViewProfileField(self $user, $fieldNameIntern) + public function allowedViewProfileField(self $user, string $fieldNameIntern) { return $user->mProfileFieldsData->isVisible($fieldNameIntern, $this->hasRightEditProfile($user)); } @@ -125,7 +125,7 @@ public function assignDefaultRoles() $this->db->startTransaction(); // every user will get the default roles for registration, if the current user has the right to assign roles - // than the roles assignment dialog will be shown + // than the role assignment dialog will be shown $sql = 'SELECT rol_id FROM '.TBL_ROLES.' INNER JOIN '.TBL_CATEGORIES.' @@ -149,7 +149,7 @@ public function assignDefaultRoles() /** * @param string $mode 'set' or 'edit' - * @param int $id Id of the role for which the membership should be set, + * @param int $id ID of the role for which the membership should be set, * or id of the current membership that should be edited. * @param string $startDate New start date of the membership. Default will be **DATE_NOW**. * @param string $endDate New end date of the membership. Default will be **31.12.9999** @@ -169,7 +169,7 @@ private function changeRoleMembership($mode, $id, $startDate, $endDate, $leader) $maxEndDate = $endDate; if ($mode === 'set') { - // subtract 1 day from start date so that we find memberships that ends yesterday + // subtract 1 day from start date so that we find memberships that end yesterday // these memberships can be continued with new date $oneDayOffset = new \DateInterval('P1D'); @@ -192,7 +192,7 @@ private function changeRoleMembership($mode, $id, $startDate, $endDate, $leader) AND mem_usr_id = ? -- $usrId AND mem_begin <= ? -- $endDate AND mem_end >= ? -- $startDate - ORDER BY mem_begin ASC'; + ORDER BY mem_begin'; $queryParams = array( $id, $usrId, @@ -209,7 +209,7 @@ private function changeRoleMembership($mode, $id, $startDate, $endDate, $leader) AND mem_usr_id = ? -- $usrId AND mem_begin <= ? -- $endDate AND mem_end >= ? -- $startDate - ORDER BY mem_begin ASC'; + ORDER BY mem_begin'; $queryParams = array( $id, $member->getValue('mem_rol_id'), @@ -234,14 +234,14 @@ private function changeRoleMembership($mode, $id, $startDate, $endDate, $leader) } if ($mode === 'set') { - // save new end date if an later date exists - // but only if end date is greater than the begin date otherwise the membership should be deleted + // save new end date if a later date exists + // but only if end date is greater than the beginn date otherwise the membership should be deleted if (strcmp($member->getValue('mem_end', 'Y-m-d'), $maxEndDate) > 0 && strcmp($member->getValue('mem_begin', 'Y-m-d'), $maxEndDate) < 0) { $maxEndDate = $member->getValue('mem_end', 'Y-m-d'); } } else { - // save new end date if an later date exists + // save new end date if a later date exists if (strcmp($member->getValue('mem_end', 'Y-m-d'), $maxEndDate) > 0) { $maxEndDate = $member->getValue('mem_end', 'Y-m-d'); } @@ -257,7 +257,7 @@ private function changeRoleMembership($mode, $id, $startDate, $endDate, $leader) $minStartDate = $member->getValue('mem_begin', 'Y-m-d'); } - // save new end date if an later date exists + // save new end date if a later date exists if (strcmp($member->getValue('mem_end', 'Y-m-d'), $maxEndDate) > 0) { $maxEndDate = $member->getValue('mem_end', 'Y-m-d'); } @@ -296,7 +296,7 @@ private function changeRoleMembership($mode, $id, $startDate, $endDate, $leader) } /** - * Method reads all relationships of the user and will store them in an array. Also the + * Method reads all relationships of the user and will store them in an array. The * relationship property if the user can edit the profile of the other user will be stored * for later checks within this class. * @return bool Return true if relationships could be checked. @@ -336,8 +336,8 @@ private function checkRelationshipsRights() /** * The method reads all roles where this user has a valid membership and checks the rights of * those roles. It stores all rights that the user get at last through one role in an array. - * In addition the method checks which roles lists the user could see in an separate array. - * Also an array with all roles where the user has the right to write an email will be stored. + * The method checks which role lists the user could see in a separate array. + * An array with all roles where the user has the right to write an email will be stored. * The method considered the role leader rights of each role if this is set and the current * user is a leader in a role. * @param string $right The database column name of the right that should be checked. If this param @@ -409,8 +409,7 @@ public function checkRolesRight($right = null) // add role to membership array $this->rolesMembership[] = $rolId; - // Rechte der Rollen in das Array uebertragen, - // falls diese noch nicht durch andere Rollen gesetzt wurden + // Transfer the rights of the roles into the array, if these have not yet been set by other roles foreach ($tmpRolesRights as $key => &$value) { if (!$value && $row[$key] == '1') { $value = true; @@ -429,25 +428,25 @@ public function checkRolesRight($right = null) } } - // Listenansichtseinstellung merken - // Leiter duerfen die Rolle sehen + // Remember list view setting + // leaders are allowed to see the role if ($row['mem_usr_id'] > 0 && ($row['rol_this_list_view'] > 0 || $memLeader)) { - // Mitgliedschaft bei der Rolle und diese nicht gesperrt, dann anschauen + // Membership to the role and this is not locked, then look at it $this->listViewRights[$rolId] = true; } elseif ((int) $row['rol_this_list_view'] === 2) { - // andere Rollen anschauen, wenn jeder sie sehen darf + // look at other roles when everyone is allowed to see them $this->listViewRights[$rolId] = true; } else { $this->listViewRights[$rolId] = false; } - // Mailrechte setzen - // Leiter duerfen der Rolle Mails schreiben + // Set mail permissions + // Leaders are allowed to write mails to the role if ($row['mem_usr_id'] > 0 && ($row['rol_mail_this_role'] > 0 || $memLeader)) { - // Mitgliedschaft bei der Rolle und diese nicht gesperrt, dann anschauen + // Membership to the role and this is not locked, then look at it $this->listMailRights[$rolId] = true; } elseif ($row['rol_mail_this_role'] >= 2) { - // andere Rollen anschauen, wenn jeder sie sehen darf + // look at other roles when everyone is allowed to see them $this->listMailRights[$rolId] = true; } else { $this->listMailRights[$rolId] = false; @@ -455,12 +454,12 @@ public function checkRolesRight($right = null) } $this->rolesRights = $tmpRolesRights; - // ist das Recht 'alle Listen einsehen' gesetzt, dann dies auch im Array bei allen Rollen setzen + // if the right 'view all lists' is set, then set this also in the array for all roles if ($this->rolesRights['rol_all_lists_view']) { $this->listViewRights = array_fill_keys(array_keys($this->listViewRights), true); } - // ist das Recht 'allen Rollen EMails schreiben' gesetzt, dann dies auch im Array bei allen Rollen setzen + // if the right 'write emails to all roles' is set, then set this also in the array for all roles if ($this->rolesRights['rol_mail_to_all']) { $this->listMailRights = array_fill_keys(array_keys($this->listMailRights), true); } @@ -1422,6 +1421,32 @@ private function handleIncorrectPasswordLogin() return 'SYS_LOGIN_USERNAME_PASSWORD_INCORRECT'; } + /** + * Deletes all other sessions of the current user except the current session. Also all auto logins of the user + * will be removed. This method is useful if the user changed his password or if unusual activities within + * the user account are noticed. + * @return bool Returns true if all things could be done. Otherwise false is returned. + */ + public function invalidateAllOtherLogins() + { + global $gCurrentUserId, $gCurrentSession; + + // remove all sessions of the current user except the current session + $sql = 'DELETE FROM ' . TBL_SESSIONS . ' + WHERE ses_usr_id = ? -- $gCurrentUserId + AND ses_id <> ? -- $gCurrentSession->getValue(\'ses_id\') '; + $queryParams = array($gCurrentUserId, $gCurrentSession->getValue('ses_id')); + $this->db->queryPrepared($sql, $queryParams); + + // remove all auto logins of the current user + $sql = 'DELETE FROM ' . TBL_AUTO_LOGIN . ' + WHERE atl_usr_id = ? -- $gCurrentUserId '; + $queryParams = array($gCurrentUserId); + $this->db->queryPrepared($sql, $queryParams); + + return true; + } + /** * Checks if the user is assigned to the role **Administrator** * @return bool Returns **true** if the user is a member of the role **Administrator** @@ -1765,7 +1790,12 @@ public function setPassword($newPassword, $doHashing = true) $this ); } - return parent::setValue('usr_password', $newPasswordHash, false); + if(parent::setValue('usr_password', $newPasswordHash, false)) { + // for security reasons remove all sessions and auto login of the user + return $this->invalidateAllOtherLogins(); + } + + return false; } /** @@ -2029,7 +2059,7 @@ public function editWeblinksRight() /** * Return the (internal) representation of this user's profile fields - * @return array All profile fields of the user + * @return object All profile fields of the user */ public function getProfileFieldsData() {