diff --git a/lib/base.php b/lib/base.php index 34cbfe3066..2cace2a0a0 100644 --- a/lib/base.php +++ b/lib/base.php @@ -442,7 +442,7 @@ class OC { if (!$session->exists('SID_CREATED')) { $session->set('SID_CREATED', time()); } else if (time() - $session->get('SID_CREATED') > $sessionLifeTime / 2) { - session_regenerate_id(true); + $session->regenerateId(); $session->set('SID_CREATED', time()); } diff --git a/lib/private/session/cryptosessiondata.php b/lib/private/session/cryptosessiondata.php index dcae1648fe..b600874412 100644 --- a/lib/private/session/cryptosessiondata.php +++ b/lib/private/session/cryptosessiondata.php @@ -131,6 +131,16 @@ class CryptoSessionData implements \ArrayAccess, ISession { $this->session->clear(); } + /** + * Wrapper around session_regenerate_id + * + * @param bool $deleteOldSession Whether to delete the old associated session file or not. + * @return void + */ + public function regenerateId($deleteOldSession = true) { + $this->session->regenerateId($deleteOldSession); + } + /** * Close the session and release the lock, also writes all changed data in batch */ diff --git a/lib/private/session/internal.php b/lib/private/session/internal.php index 0b6152acf1..8be3356c6d 100644 --- a/lib/private/session/internal.php +++ b/lib/private/session/internal.php @@ -89,10 +89,9 @@ class Internal extends Session { } } - public function clear() { session_unset(); - @session_regenerate_id(true); + $this->regenerateId(); @session_start(); $_SESSION = array(); } @@ -102,14 +101,35 @@ class Internal extends Session { parent::close(); } - public function reopen() { - throw new \Exception('The session cannot be reopened - reopen() is ony to be used in unit testing.'); - } + /** + * Wrapper around session_regenerate_id + * + * @param bool $deleteOldSession Whether to delete the old associated session file or not. + * @return void + */ + public function regenerateId($deleteOldSession = true) { + @session_regenerate_id($deleteOldSession); + } + /** + * @throws \Exception + */ + public function reopen() { + throw new \Exception('The session cannot be reopened - reopen() is ony to be used in unit testing.'); + } + + /** + * @param int $errorNumber + * @param string $errorString + * @throws \ErrorException + */ public function trapError($errorNumber, $errorString) { throw new \ErrorException($errorString); } + /** + * @throws \Exception + */ private function validateSession() { if ($this->sessionClosed) { throw new \Exception('Session has been closed - no further changes to the session are allowed'); diff --git a/lib/private/session/memory.php b/lib/private/session/memory.php index ff95efc534..c609008745 100644 --- a/lib/private/session/memory.php +++ b/lib/private/session/memory.php @@ -80,6 +80,13 @@ class Memory extends Session { $this->data = array(); } + /** + * Stub since the session ID does not need to get regenerated for the cache + * + * @param bool $deleteOldSession + */ + public function regenerateId($deleteOldSession = true) {} + /** * Helper function for PHPUnit execution - don't use in non-test code */ diff --git a/lib/private/user.php b/lib/private/user.php index cfa60d675f..fa1cea9072 100644 --- a/lib/private/user.php +++ b/lib/private/user.php @@ -162,7 +162,6 @@ class OC_User { * Log in a user and regenerate a new session - if the password is ok */ public static function login($loginname, $password) { - session_regenerate_id(true); $result = self::getUserSession()->login($loginname, $password); if ($result) { //we need to pass the user name, which may differ from login name diff --git a/lib/private/user/session.php b/lib/private/user/session.php index ba702c9f36..be38b1b1d8 100644 --- a/lib/private/user/session.php +++ b/lib/private/user/session.php @@ -213,6 +213,7 @@ class Session implements IUserSession, Emitter { * @throws LoginException */ public function login($uid, $password) { + $this->session->regenerateId(); $this->manager->emit('\OC\User', 'preLogin', array($uid, $password)); $user = $this->manager->checkPassword($uid, $password); if ($user !== false) { @@ -243,6 +244,7 @@ class Session implements IUserSession, Emitter { * @return bool */ public function loginWithCookie($uid, $currentToken) { + $this->session->regenerateId(); $this->manager->emit('\OC\User', 'preRememberedLogin', array($uid)); $user = $this->manager->get($uid); if (is_null($user)) { diff --git a/lib/public/isession.php b/lib/public/isession.php index aee635d7a9..89a181ad0f 100644 --- a/lib/public/isession.php +++ b/lib/public/isession.php @@ -86,4 +86,12 @@ interface ISession { */ public function close(); + /** + * Wrapper around session_regenerate_id + * + * @param bool $deleteOldSession Whether to delete the old associated session file or not. + * @return void + * @since 9.0.0 + */ + public function regenerateId($deleteOldSession = true); } diff --git a/tests/lib/user/session.php b/tests/lib/user/session.php index d9dace2ef0..ffd4f96d80 100644 --- a/tests/lib/user/session.php +++ b/tests/lib/user/session.php @@ -95,6 +95,8 @@ class Session extends \Test\TestCase { public function testLoginValidPasswordEnabled() { $session = $this->getMock('\OC\Session\Memory', array(), array('')); + $session->expects($this->once()) + ->method('regenerateId'); $session->expects($this->exactly(2)) ->method('set') ->with($this->callback(function ($key) { @@ -148,6 +150,8 @@ class Session extends \Test\TestCase { $session = $this->getMock('\OC\Session\Memory', array(), array('')); $session->expects($this->never()) ->method('set'); + $session->expects($this->once()) + ->method('regenerateId'); $managerMethods = get_class_methods('\OC\User\Manager'); //keep following methods intact in order to ensure hooks are @@ -179,10 +183,12 @@ class Session extends \Test\TestCase { $userSession->login('foo', 'bar'); } - public function testLoginInValidPassword() { + public function testLoginInvalidPassword() { $session = $this->getMock('\OC\Session\Memory', array(), array('')); $session->expects($this->never()) ->method('set'); + $session->expects($this->once()) + ->method('regenerateId'); $managerMethods = get_class_methods('\OC\User\Manager'); //keep following methods intact in order to ensure hooks are @@ -217,6 +223,8 @@ class Session extends \Test\TestCase { $session = $this->getMock('\OC\Session\Memory', array(), array('')); $session->expects($this->never()) ->method('set'); + $session->expects($this->once()) + ->method('regenerateId'); $manager = $this->getMock('\OC\User\Manager'); @@ -244,6 +252,8 @@ class Session extends \Test\TestCase { } }, 'foo')); + $session->expects($this->once()) + ->method('regenerateId'); $managerMethods = get_class_methods('\OC\User\Manager'); //keep following methods intact in order to ensure hooks are @@ -292,6 +302,8 @@ class Session extends \Test\TestCase { $session = $this->getMock('\OC\Session\Memory', array(), array('')); $session->expects($this->never()) ->method('set'); + $session->expects($this->once()) + ->method('regenerateId'); $managerMethods = get_class_methods('\OC\User\Manager'); //keep following methods intact in order to ensure hooks are @@ -334,6 +346,8 @@ class Session extends \Test\TestCase { $session = $this->getMock('\OC\Session\Memory', array(), array('')); $session->expects($this->never()) ->method('set'); + $session->expects($this->once()) + ->method('regenerateId'); $managerMethods = get_class_methods('\OC\User\Manager'); //keep following methods intact in order to ensure hooks are