Skip to content

Commit

Permalink
Sessions should be invalidated if password was changed #1238
Browse files Browse the repository at this point in the history
  • Loading branch information
@Fasse
Fasse committed Mar 13, 2022
1 parent c9b08f0 commit e84e472
Show file tree
Hide file tree
Showing 2 changed files with 85 additions and 44 deletions.
45 changes: 28 additions & 17 deletions adm_program/system/classes/Session.php
View file
Expand Up @@ -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);
}
Expand Down Expand Up @@ -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
{
Expand All @@ -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.
*/
Expand Down Expand Up @@ -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()
{
Expand All @@ -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;
Expand Down Expand Up @@ -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();

Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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']);
Expand Down Expand Up @@ -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();

Expand Down Expand Up @@ -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)
{
Expand All @@ -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';
Expand Down Expand Up @@ -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.'
Expand Down
84 changes: 57 additions & 27 deletions adm_program/system/classes/User.php
View file
Expand Up @@ -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));
}
Expand All @@ -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.'
Expand All @@ -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**
Expand All @@ -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');

Expand All @@ -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,
Expand All @@ -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'),
Expand All @@ -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');
}
Expand All @@ -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');
}
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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;
Expand All @@ -429,38 +428,38 @@ 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;
}
}
$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);
}
Expand Down Expand Up @@ -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**
Expand Down Expand Up @@ -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;
}

/**
Expand Down Expand Up @@ -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<ProfileFields> All profile fields of the user
*/
public function getProfileFieldsData()
{
Expand Down

0 comments on commit e84e472

Please sign in to comment.