From 0c0a216f42bb004380efca1fd665711f938579d9 Mon Sep 17 00:00:00 2001 From: Christoph Wurst Date: Fri, 17 Jun 2016 13:59:15 +0200 Subject: [PATCH] store last check timestamp in token instead of session --- db_structure.xml | 9 ++ .../Authentication/Token/DefaultToken.php | 23 +++ .../Token/DefaultTokenMapper.php | 4 +- .../Token/DefaultTokenProvider.php | 27 ++-- .../Authentication/Token/IProvider.php | 14 +- lib/private/Authentication/Token/IToken.php | 14 ++ lib/private/User/Session.php | 146 +++++++++++------- version.php | 2 +- 8 files changed, 161 insertions(+), 78 deletions(-) diff --git a/db_structure.xml b/db_structure.xml index b7dacc05d9..6b91c3c4c5 100644 --- a/db_structure.xml +++ b/db_structure.xml @@ -1120,6 +1120,15 @@ 4 + + last_check + integer + 0 + true + true + 4 + + authtoken_token_index true diff --git a/lib/private/Authentication/Token/DefaultToken.php b/lib/private/Authentication/Token/DefaultToken.php index 299291e34a..79b03eed27 100644 --- a/lib/private/Authentication/Token/DefaultToken.php +++ b/lib/private/Authentication/Token/DefaultToken.php @@ -74,6 +74,11 @@ class DefaultToken extends Entity implements IToken { */ protected $lastActivity; + /** + * @var int + */ + protected $lastCheck; + public function getId() { return $this->id; } @@ -109,4 +114,22 @@ class DefaultToken extends Entity implements IToken { ]; } + /** + * Get the timestamp of the last password check + * + * @return int + */ + public function getLastCheck() { + return parent::getLastCheck(); + } + + /** + * Get the timestamp of the last password check + * + * @param int $time + */ + public function setLastCheck($time) { + return parent::setLastCheck($time); + } + } diff --git a/lib/private/Authentication/Token/DefaultTokenMapper.php b/lib/private/Authentication/Token/DefaultTokenMapper.php index c56a513b94..b02220976c 100644 --- a/lib/private/Authentication/Token/DefaultTokenMapper.php +++ b/lib/private/Authentication/Token/DefaultTokenMapper.php @@ -70,7 +70,7 @@ class DefaultTokenMapper extends Mapper { public function getToken($token) { /* @var $qb IQueryBuilder */ $qb = $this->db->getQueryBuilder(); - $result = $qb->select('id', 'uid', 'login_name', 'password', 'name', 'type', 'token', 'last_activity') + $result = $qb->select('id', 'uid', 'login_name', 'password', 'name', 'type', 'token', 'last_activity', 'last_check') ->from('authtoken') ->where($qb->expr()->eq('token', $qb->createParameter('token'))) ->setParameter('token', $token) @@ -95,7 +95,7 @@ class DefaultTokenMapper extends Mapper { public function getTokenByUser(IUser $user) { /* @var $qb IQueryBuilder */ $qb = $this->db->getQueryBuilder(); - $qb->select('id', 'uid', 'login_name', 'password', 'name', 'type', 'token', 'last_activity') + $qb->select('id', 'uid', 'login_name', 'password', 'name', 'type', 'token', 'last_activity', 'last_check') ->from('authtoken') ->where($qb->expr()->eq('uid', $qb->createNamedParameter($user->getUID()))) ->setMaxResults(1000); diff --git a/lib/private/Authentication/Token/DefaultTokenProvider.php b/lib/private/Authentication/Token/DefaultTokenProvider.php index 03b8bb5da2..2467b8d836 100644 --- a/lib/private/Authentication/Token/DefaultTokenProvider.php +++ b/lib/private/Authentication/Token/DefaultTokenProvider.php @@ -91,6 +91,18 @@ class DefaultTokenProvider implements IProvider { return $dbToken; } + /** + * Save the updated token + * + * @param IToken $token + */ + public function updateToken(IToken $token) { + if (!($token instanceof DefaultToken)) { + throw new InvalidTokenException(); + } + $this->mapper->update($token); + } + /** * Update token activity timestamp * @@ -181,21 +193,6 @@ class DefaultTokenProvider implements IProvider { $this->mapper->invalidateOld($olderThan); } - /** - * @param string $token - * @throws InvalidTokenException - * @return DefaultToken user UID - */ - public function validateToken($token) { - try { - $dbToken = $this->mapper->getToken($this->hashToken($token)); - $this->logger->debug('valid default token for ' . $dbToken->getUID()); - return $dbToken; - } catch (DoesNotExistException $ex) { - throw new InvalidTokenException(); - } - } - /** * @param string $token * @return string diff --git a/lib/private/Authentication/Token/IProvider.php b/lib/private/Authentication/Token/IProvider.php index e79ba8b30e..97f8ababbb 100644 --- a/lib/private/Authentication/Token/IProvider.php +++ b/lib/private/Authentication/Token/IProvider.php @@ -49,13 +49,6 @@ interface IProvider { */ public function getToken($tokenId) ; - /** - * @param string $token - * @throws InvalidTokenException - * @return IToken - */ - public function validateToken($token); - /** * Invalidate (delete) the given session token * @@ -71,6 +64,13 @@ interface IProvider { */ public function invalidateTokenById(IUser $user, $id); + /** + * Save the updated token + * + * @param IToken $token + */ + public function updateToken(IToken $token); + /** * Update token activity timestamp * diff --git a/lib/private/Authentication/Token/IToken.php b/lib/private/Authentication/Token/IToken.php index a34bdc2c43..096550fd09 100644 --- a/lib/private/Authentication/Token/IToken.php +++ b/lib/private/Authentication/Token/IToken.php @@ -55,4 +55,18 @@ interface IToken extends JsonSerializable { * @return string */ public function getPassword(); + + /** + * Get the timestamp of the last password check + * + * @return int + */ + public function getLastCheck(); + + /** + * Get the timestamp of the last password check + * + * @param int $time + */ + public function setLastCheck($time); } diff --git a/lib/private/User/Session.php b/lib/private/User/Session.php index 89148dcf8e..ccae72ed35 100644 --- a/lib/private/User/Session.php +++ b/lib/private/User/Session.php @@ -192,52 +192,22 @@ class Session implements IUserSession, Emitter { if (is_null($this->activeUser)) { return null; } - $this->validateSession($this->activeUser); + $this->validateSession(); } return $this->activeUser; } - protected function validateSession(IUser $user) { + protected function validateSession() { try { $sessionId = $this->session->getId(); } catch (SessionNotAvailableException $ex) { return; } - try { - $token = $this->tokenProvider->getToken($sessionId); - } catch (InvalidTokenException $ex) { + + if (!$this->validateToken($sessionId)) { // Session was invalidated $this->logout(); - return; } - - // Check whether login credentials are still valid and the user was not disabled - // This check is performed each 5 minutes - $lastCheck = $this->session->get('last_login_check') ? : 0; - $now = $this->timeFacory->getTime(); - if ($lastCheck < ($now - 60 * 5)) { - try { - $pwd = $this->tokenProvider->getPassword($token, $sessionId); - } catch (InvalidTokenException $ex) { - // An invalid token password was used -> log user out - $this->logout(); - return; - } catch (PasswordlessTokenException $ex) { - // Token has no password, nothing to check - $this->session->set('last_login_check', $now); - return; - } - - if ($this->manager->checkPassword($token->getLoginName(), $pwd) === false - || !$user->isEnabled()) { - // Password has changed or user was disabled -> log user out - $this->logout(); - return; - } - $this->session->set('last_login_check', $now); - } - - $this->tokenProvider->updateTokenActivity($token); } /** @@ -297,20 +267,22 @@ class Session implements IUserSession, Emitter { public function login($uid, $password) { $this->session->regenerateId(); if ($this->validateToken($password)) { - $user = $this->getUser(); - // When logging in with token, the password must be decrypted first before passing to login hook try { $token = $this->tokenProvider->getToken($password); try { - $password = $this->tokenProvider->getPassword($token, $password); - $this->manager->emit('\OC\User', 'preLogin', array($uid, $password)); + $loginPassword = $this->tokenProvider->getPassword($token, $password); + $this->manager->emit('\OC\User', 'preLogin', array($uid, $loginPassword)); } catch (PasswordlessTokenException $ex) { $this->manager->emit('\OC\User', 'preLogin', array($uid, '')); } } catch (InvalidTokenException $ex) { // Invalid token, nothing to do } + + $this->loginWithToken($password); + $user = $this->getUser(); + $this->tokenProvider->updateTokenActivity($token); } else { $this->manager->emit('\OC\User', 'preLogin', array($uid, $password)); $user = $this->manager->checkPassword($uid, $password); @@ -459,8 +431,21 @@ class Session implements IUserSession, Emitter { return false; } - private function loginWithToken($uid) { - // TODO: $this->manager->emit('\OC\User', 'preTokenLogin', array($uid)); + private function loginWithToken($token) { + try { + $dbToken = $this->tokenProvider->getToken($token); + } catch (InvalidTokenException $ex) { + return false; + } + $uid = $dbToken->getUID(); + + try { + $password = $this->tokenProvider->getPassword($dbToken, $token); + $this->manager->emit('\OC\User', 'preLogin', array($uid, $password)); + } catch (PasswordlessTokenException $ex) { + $this->manager->emit('\OC\User', 'preLogin', array($uid, '')); + } + $user = $this->manager->get($uid); if (is_null($user)) { // user does not exist @@ -473,7 +458,9 @@ class Session implements IUserSession, Emitter { //login $this->setUser($user); - // TODO: $this->manager->emit('\OC\User', 'postTokenLogin', array($user)); + $this->tokenProvider->updateTokenActivity($dbToken); + + $this->manager->emit('\OC\User', 'postLogin', array($user, $password)); return true; } @@ -530,24 +517,72 @@ class Session implements IUserSession, Emitter { } /** + * @param IToken $dbToken + * @param string $token + * @return boolean + */ + private function checkTokenCredentials(IToken $dbToken, $token) { + // Check whether login credentials are still valid and the user was not disabled + // This check is performed each 5 minutes + $lastCheck = $dbToken->getLastCheck() ? : 0; + $now = $this->timeFacory->getTime(); + if ($lastCheck > ($now - 60 * 5)) { + // Checked performed recently, nothing to do now + return true; + } + + try { + $pwd = $this->tokenProvider->getPassword($dbToken, $token); + } catch (InvalidTokenException $ex) { + // An invalid token password was used -> log user out + $this->logout(); + return false; + } catch (PasswordlessTokenException $ex) { + // Token has no password + + if (!is_null($this->activeUser) && !$this->activeUser->isEnabled()) { + $this->tokenProvider->invalidateToken($token); + $this->logout(); + return false; + } + + $dbToken->setLastCheck($now); + $this->tokenProvider->updateToken($dbToken); + return true; + } + + if ($this->manager->checkPassword($dbToken->getLoginName(), $pwd) === false + || (!is_null($this->activeUser) && !$this->activeUser->isEnabled())) { + $this->tokenProvider->invalidateToken($token); + // Password has changed or user was disabled -> log user out + $this->logout(); + return false; + } + $dbToken->setLastCheck($now); + $this->tokenProvider->updateToken($dbToken); + return true; + } + + /** + * Check if the given token exists and performs password/user-enabled checks + * + * Invalidates the token if checks fail + * * @param string $token * @return boolean */ private function validateToken($token) { try { - $token = $this->tokenProvider->validateToken($token); - if (!is_null($token)) { - $result = $this->loginWithToken($token->getUID()); - if ($result) { - // Login success - $this->tokenProvider->updateTokenActivity($token); - return true; - } - } + $dbToken = $this->tokenProvider->getToken($token); } catch (InvalidTokenException $ex) { - + return false; } - return false; + + if (!$this->checkTokenCredentials($dbToken, $token)) { + return false; + } + + return true; } /** @@ -562,10 +597,15 @@ class Session implements IUserSession, Emitter { // No auth header, let's try session id try { $sessionId = $this->session->getId(); - return $this->validateToken($sessionId); } catch (SessionNotAvailableException $ex) { return false; } + + if (!$this->validateToken($sessionId)) { + return false; + } + + return $this->loginWithToken($sessionId); } else { $token = substr($authHeader, 6); return $this->validateToken($token); diff --git a/version.php b/version.php index 698636a219..3015d976e5 100644 --- a/version.php +++ b/version.php @@ -25,7 +25,7 @@ // We only can count up. The 4. digit is only for the internal patchlevel to trigger DB upgrades // between betas, final and RCs. This is _not_ the public version number. Reset minor/patchlevel // when updating major/minor version number. -$OC_Version = array(9, 1, 0, 8); +$OC_Version = array(9, 1, 0, 9); // The human readable string $OC_VersionString = '9.1.0 beta 2';