From 8cc5f6036f6ff1377077da0eed1cf4350db4b7e6 Mon Sep 17 00:00:00 2001 From: Christoph Wurst Date: Thu, 28 Apr 2016 10:52:28 +0200 Subject: [PATCH] Fix existing tests --- lib/private/Files/Filesystem.php | 2 +- lib/private/User/Session.php | 44 ++++--- tests/lib/user/session.php | 214 +++++++++++++++++-------------- 3 files changed, 140 insertions(+), 120 deletions(-) diff --git a/lib/private/Files/Filesystem.php b/lib/private/Files/Filesystem.php index 99c123ad1a..89b8547aa5 100644 --- a/lib/private/Files/Filesystem.php +++ b/lib/private/Files/Filesystem.php @@ -404,7 +404,7 @@ class Filesystem { if (is_null($userObject)) { \OCP\Util::writeLog('files', ' Backends provided no user object for ' . $user, \OCP\Util::ERROR); - throw new \OC\User\NoUserException('Backends provided no user object for ' . $user); + throw new \OC\User\NoUserException('Backend provided no user object for ' . $user); } self::$usersSetup[$user] = true; diff --git a/lib/private/User/Session.php b/lib/private/User/Session.php index 262174ab17..972f59fc00 100644 --- a/lib/private/User/Session.php +++ b/lib/private/User/Session.php @@ -184,30 +184,27 @@ class Session implements IUserSession, Emitter { if (OC_User::isIncognitoMode()) { return null; } - if ($this->activeUser) { - return $this->activeUser; - } else { + if (is_null($this->activeUser)) { $uid = $this->session->get('user_id'); - if ($uid !== null && $this->isValidSession($uid)) { - return $this->activeUser; - } else { + if (is_null($uid)) { return null; } + $this->activeUser = $this->manager->get($uid); + if (is_null($this->activeUser)) { + return null; + } + $this->validateSession($this->activeUser); } + return $this->activeUser; } - private function isValidSession($uid) { - $this->activeUser = $this->manager->get($uid); - if (is_null($this->activeUser)) { - // User does not exist - return false; - } + protected function validateSession(IUser $user) { // TODO: use ISession::getId(), https://github.com/owncloud/core/pull/24229 $sessionId = session_id(); try { $token = $this->tokenProvider->getToken($sessionId); } catch (InvalidTokenException $ex) { - // Session was inalidated + // Session was invalidated $this->logout(); return false; } @@ -217,7 +214,7 @@ class Session implements IUserSession, Emitter { $lastCheck = $this->session->get('last_login_check') ? : 0; if ($lastCheck < (time() - 60 * 5)) { $pwd = $this->tokenProvider->getPassword($token, $sessionId); - if ($this->manager->checkPassword($uid, $pwd) === false) { + if ($this->manager->checkPassword($user->getUID(), $pwd) === false) { // Password has changed -> log user out $this->logout(); return false; @@ -303,13 +300,7 @@ class Session implements IUserSession, Emitter { $this->setLoginName($uid); $this->manager->emit('\OC\User', 'postLogin', array($user, $password)); if ($this->isLoggedIn()) { - // Refresh the token - \OC::$server->getCsrfTokenManager()->refreshToken(); - //we need to pass the user name, which may differ from login name - $user = $this->getUser()->getUID(); - \OC_Util::setupFS($user); - //trigger creation of user home and /files folder - \OC::$server->getUserFolder($user); + $this->prepareUserLogin(); return true; } else { // injecting l10n does not work - there is a circular dependency between session and \OCP\L10N\IFactory @@ -326,6 +317,17 @@ class Session implements IUserSession, Emitter { return false; } + protected function prepareUserLogin() { + // TODO: mock/inject/use non-static + // Refresh the token + \OC::$server->getCsrfTokenManager()->refreshToken(); + //we need to pass the user name, which may differ from login name + $user = $this->getUser()->getUID(); + \OC_Util::setupFS($user); + //trigger creation of user home and /files folder + \OC::$server->getUserFolder($user); + } + /** * Tries to login the user with HTTP Basic Authentication * @return boolean if the login was successful diff --git a/tests/lib/user/session.php b/tests/lib/user/session.php index 5a8ea57cb8..ca03e62c3a 100644 --- a/tests/lib/user/session.php +++ b/tests/lib/user/session.php @@ -11,107 +11,127 @@ namespace Test\User; use OC\Session\Memory; use OC\User\User; -use OCP\ISession; -use OCP\IUserManager; -use OCP\UserInterface; -use Test\TestCase; /** * @group DB * @package Test\User */ -class Session extends TestCase { +class Session extends \Test\TestCase { + + /** @var \OC\Authentication\Token\DefaultTokenProvider */ + protected $defaultProvider; + + protected function setUp() { + parent::setUp(); + + $this->defaultProvider = $this->getMockBuilder('\OC\Authentication\Token\DefaultTokenProvider') + ->disableOriginalConstructor() + ->getMock(); + } + public function testGetUser() { - /** @var ISession | \PHPUnit_Framework_MockObject_MockObject $session */ + $token = new \OC\Authentication\Token\DefaultToken(); + + $expectedUser = new User('foo', null); $session = $this->getMock('\OC\Session\Memory', array(), array('')); - $session->expects($this->once()) + $session->expects($this->at(0)) ->method('get') ->with('user_id') - ->will($this->returnValue('foo')); + ->will($this->returnValue($expectedUser->getUID())); - /** @var UserInterface | \PHPUnit_Framework_MockObject_MockObject $backend */ - $backend = $this->getMock('\Test\Util\User\Dummy'); - $backend->expects($this->once()) - ->method('userExists') - ->with('foo') + $manager = $this->getMockBuilder('\OC\User\Manager') + ->disableOriginalConstructor() + ->getMock(); + $this->defaultProvider->expects($this->once()) + ->method('getToken') + ->will($this->returnValue($token)); + // TODO: check passed session id once it's mockable + $session->expects($this->at(1)) + ->method('last_login_check') + ->will($this->returnValue(null)); // No check has been run yet + $this->defaultProvider->expects($this->once()) + ->method('getPassword') + // TODO: check passed UID and session id once it's mockable + ->will($this->returnValue('password123')); + $manager->expects($this->once()) + ->method('checkPassword') + ->with($expectedUser->getUID(), 'password123') ->will($this->returnValue(true)); + $session->expects($this->at(2)) + ->method('set') + ->with('last_login_check', time()); - $manager = new \OC\User\Manager(); - $manager->registerBackend($backend); + $session->expects($this->at(3)) + ->method('get') + ->with('last_token_update') + ->will($this->returnValue(null)); // No check run so far + $this->defaultProvider->expects($this->once()) + ->method('updateToken') + ->with($token); + $session->expects($this->at(4)) + ->method('set') + ->with('last_token_update', time()); - $userSession = new \OC\User\Session($manager, $session); + $manager->expects($this->any()) + ->method('get') + ->with($expectedUser->getUID()) + ->will($this->returnValue($expectedUser)); + + $userSession = new \OC\User\Session($manager, $session, $this->defaultProvider, [$this->defaultProvider]); $user = $userSession->getUser(); - $this->assertEquals('foo', $user->getUID()); + $this->assertSame($expectedUser, $user); } - public function testIsLoggedIn() { - /** @var ISession | \PHPUnit_Framework_MockObject_MockObject $session */ - $session = $this->getMock('\OC\Session\Memory', array(), array('')); - $session->expects($this->once()) - ->method('get') - ->with('user_id') - ->will($this->returnValue('foo')); - - /** @var UserInterface | \PHPUnit_Framework_MockObject_MockObject $backend */ - $backend = $this->getMock('\Test\Util\User\Dummy'); - $backend->expects($this->once()) - ->method('userExists') - ->with('foo') - ->will($this->returnValue(true)); - - $manager = new \OC\User\Manager(); - $manager->registerBackend($backend); - - $userSession = new \OC\User\Session($manager, $session); - $isLoggedIn = $userSession->isLoggedIn(); - $this->assertTrue($isLoggedIn); + public function isLoggedInData() { + return [ + [true], + [false], + ]; } - public function testNotLoggedIn() { - /** @var ISession | \PHPUnit_Framework_MockObject_MockObject $session */ + /** + * @dataProvider isLoggedInData + */ + public function testIsLoggedIn($isLoggedIn) { $session = $this->getMock('\OC\Session\Memory', array(), array('')); - $session->expects($this->once()) - ->method('get') - ->with('user_id') - ->will($this->returnValue(null)); - /** @var UserInterface | \PHPUnit_Framework_MockObject_MockObject $backend */ - $backend = $this->getMock('\Test\Util\User\Dummy'); - $backend->expects($this->never()) - ->method('userExists'); + $manager = $this->getMockBuilder('\OC\User\Manager') + ->disableOriginalConstructor() + ->getMock(); - $manager = new \OC\User\Manager(); - $manager->registerBackend($backend); - - $userSession = new \OC\User\Session($manager, $session); - $isLoggedIn = $userSession->isLoggedIn(); - $this->assertFalse($isLoggedIn); + $userSession = $this->getMockBuilder('\OC\User\Session') + ->setConstructorArgs([$manager, $session, $this->defaultProvider, [$this->defaultProvider]]) + ->setMethods([ + 'getUser' + ]) + ->getMock(); + $user = new User('sepp', null); + $userSession->expects($this->once()) + ->method('getUser') + ->will($this->returnValue($isLoggedIn ? $user : null)); + $this->assertEquals($isLoggedIn, $userSession->isLoggedIn()); } public function testSetUser() { - /** @var ISession | \PHPUnit_Framework_MockObject_MockObject $session */ $session = $this->getMock('\OC\Session\Memory', array(), array('')); $session->expects($this->once()) ->method('set') ->with('user_id', 'foo'); - /** @var IUserManager | \PHPUnit_Framework_MockObject_MockObject $manager */ $manager = $this->getMock('\OC\User\Manager'); $backend = $this->getMock('\Test\Util\User\Dummy'); - /** @var User | \PHPUnit_Framework_MockObject_MockObject $user */ $user = $this->getMock('\OC\User\User', array(), array('foo', $backend)); $user->expects($this->once()) ->method('getUID') ->will($this->returnValue('foo')); - $userSession = new \OC\User\Session($manager, $session); + $userSession = new \OC\User\Session($manager, $session, $this->defaultProvider, [$this->defaultProvider]); $userSession->setUser($user); } public function testLoginValidPasswordEnabled() { - /** @var ISession | \PHPUnit_Framework_MockObject_MockObject $session */ $session = $this->getMock('\OC\Session\Memory', array(), array('')); $session->expects($this->once()) ->method('regenerateId'); @@ -127,8 +147,7 @@ class Session extends TestCase { return false; break; } - }, - 'foo')); + }, 'foo')); $managerMethods = get_class_methods('\OC\User\Manager'); //keep following methods intact in order to ensure hooks are @@ -140,13 +159,12 @@ class Session extends TestCase { unset($managerMethods[$i]); } } - /** @var IUserManager | \PHPUnit_Framework_MockObject_MockObject $manager */ $manager = $this->getMock('\OC\User\Manager', $managerMethods, array()); $backend = $this->getMock('\Test\Util\User\Dummy'); $user = $this->getMock('\OC\User\User', array(), array('foo', $backend)); - $user->expects($this->exactly(2)) + $user->expects($this->once()) ->method('isEnabled') ->will($this->returnValue(true)); $user->expects($this->any()) @@ -160,22 +178,24 @@ class Session extends TestCase { ->with('foo', 'bar') ->will($this->returnValue($user)); - $userSession = new \OC\User\Session($manager, $session); + $userSession = $this->getMockBuilder('\OC\User\Session') + ->setConstructorArgs([$manager, $session, $this->defaultProvider, [$this->defaultProvider]]) + ->setMethods([ + 'prepareUserLogin' + ]) + ->getMock(); + $userSession->expects($this->once()) + ->method('prepareUserLogin'); $userSession->login('foo', 'bar'); $this->assertEquals($user, $userSession->getUser()); } - /** - * @expectedException \OC\User\LoginException - * @expectedExceptionMessage User disabled - */ public function testLoginValidPasswordDisabled() { - /** @var ISession | \PHPUnit_Framework_MockObject_MockObject $session */ $session = $this->getMock('\OC\Session\Memory', array(), array('')); $session->expects($this->never()) ->method('set'); $session->expects($this->once()) - ->method('regenerateId'); + ->method('regenerateId'); $managerMethods = get_class_methods('\OC\User\Manager'); //keep following methods intact in order to ensure hooks are @@ -187,7 +207,6 @@ class Session extends TestCase { unset($managerMethods[$i]); } } - /** @var IUserManager | \PHPUnit_Framework_MockObject_MockObject $manager */ $manager = $this->getMock('\OC\User\Manager', $managerMethods, array()); $backend = $this->getMock('\Test\Util\User\Dummy'); @@ -204,17 +223,16 @@ class Session extends TestCase { ->with('foo', 'bar') ->will($this->returnValue($user)); - $userSession = new \OC\User\Session($manager, $session); + $userSession = new \OC\User\Session($manager, $session, $this->defaultProvider, [$this->defaultProvider]); $userSession->login('foo', 'bar'); } public function testLoginInvalidPassword() { - /** @var ISession | \PHPUnit_Framework_MockObject_MockObject $session */ $session = $this->getMock('\OC\Session\Memory', array(), array('')); $session->expects($this->never()) ->method('set'); $session->expects($this->once()) - ->method('regenerateId'); + ->method('regenerateId'); $managerMethods = get_class_methods('\OC\User\Manager'); //keep following methods intact in order to ensure hooks are @@ -226,7 +244,6 @@ class Session extends TestCase { unset($managerMethods[$i]); } } - /** @var IUserManager | \PHPUnit_Framework_MockObject_MockObject $manager */ $manager = $this->getMock('\OC\User\Manager', $managerMethods, array()); $backend = $this->getMock('\Test\Util\User\Dummy'); @@ -242,32 +259,31 @@ class Session extends TestCase { ->with('foo', 'bar') ->will($this->returnValue(false)); - $userSession = new \OC\User\Session($manager, $session); + $userSession = new \OC\User\Session($manager, $session, $this->defaultProvider, [$this->defaultProvider]); $userSession->login('foo', 'bar'); } public function testLoginNonExisting() { - /** @var ISession | \PHPUnit_Framework_MockObject_MockObject $session */ $session = $this->getMock('\OC\Session\Memory', array(), array('')); $session->expects($this->never()) ->method('set'); $session->expects($this->once()) - ->method('regenerateId'); + ->method('regenerateId'); - /** @var IUserManager | \PHPUnit_Framework_MockObject_MockObject $manager */ $manager = $this->getMock('\OC\User\Manager'); + $backend = $this->getMock('\Test\Util\User\Dummy'); + $manager->expects($this->once()) ->method('checkPassword') ->with('foo', 'bar') ->will($this->returnValue(false)); - $userSession = new \OC\User\Session($manager, $session); + $userSession = new \OC\User\Session($manager, $session, $this->defaultProvider, [$this->defaultProvider]); $userSession->login('foo', 'bar'); } public function testRememberLoginValidToken() { - /** @var ISession | \PHPUnit_Framework_MockObject_MockObject $session */ $session = $this->getMock('\OC\Session\Memory', array(), array('')); $session->expects($this->exactly(1)) ->method('set') @@ -278,10 +294,9 @@ class Session extends TestCase { default: return false; } - }, - 'foo')); + }, 'foo')); $session->expects($this->once()) - ->method('regenerateId'); + ->method('regenerateId'); $managerMethods = get_class_methods('\OC\User\Manager'); //keep following methods intact in order to ensure hooks are @@ -314,13 +329,12 @@ class Session extends TestCase { $token = 'goodToken'; \OC::$server->getConfig()->setUserValue('foo', 'login_token', $token, time()); - /** @var \OC\User\Session $userSession */ $userSession = $this->getMock( '\OC\User\Session', //override, otherwise tests will fail because of setcookie() array('setMagicInCookie'), //there are passed as parameters to the constructor - array($manager, $session)); + array($manager, $session, $this->defaultProvider, [$this->defaultProvider])); $granted = $userSession->loginWithCookie('foo', $token); @@ -328,12 +342,11 @@ class Session extends TestCase { } public function testRememberLoginInvalidToken() { - /** @var ISession | \PHPUnit_Framework_MockObject_MockObject $session */ $session = $this->getMock('\OC\Session\Memory', array(), array('')); $session->expects($this->never()) ->method('set'); $session->expects($this->once()) - ->method('regenerateId'); + ->method('regenerateId'); $managerMethods = get_class_methods('\OC\User\Manager'); //keep following methods intact in order to ensure hooks are @@ -345,7 +358,6 @@ class Session extends TestCase { unset($managerMethods[$i]); } } - /** @var IUserManager | \PHPUnit_Framework_MockObject_MockObject $manager */ $manager = $this->getMock('\OC\User\Manager', $managerMethods, array()); $backend = $this->getMock('\Test\Util\User\Dummy'); @@ -367,19 +379,18 @@ class Session extends TestCase { $token = 'goodToken'; \OC::$server->getConfig()->setUserValue('foo', 'login_token', $token, time()); - $userSession = new \OC\User\Session($manager, $session); + $userSession = new \OC\User\Session($manager, $session, $this->defaultProvider, [$this->defaultProvider]); $granted = $userSession->loginWithCookie('foo', 'badToken'); $this->assertSame($granted, false); } public function testRememberLoginInvalidUser() { - /** @var ISession | \PHPUnit_Framework_MockObject_MockObject $session */ $session = $this->getMock('\OC\Session\Memory', array(), array('')); $session->expects($this->never()) ->method('set'); $session->expects($this->once()) - ->method('regenerateId'); + ->method('regenerateId'); $managerMethods = get_class_methods('\OC\User\Manager'); //keep following methods intact in order to ensure hooks are @@ -391,7 +402,6 @@ class Session extends TestCase { unset($managerMethods[$i]); } } - /** @var IUserManager | \PHPUnit_Framework_MockObject_MockObject $manager */ $manager = $this->getMock('\OC\User\Manager', $managerMethods, array()); $backend = $this->getMock('\Test\Util\User\Dummy'); @@ -412,7 +422,7 @@ class Session extends TestCase { $token = 'goodToken'; \OC::$server->getConfig()->setUserValue('foo', 'login_token', $token, time()); - $userSession = new \OC\User\Session($manager, $session); + $userSession = new \OC\User\Session($manager, $session, $this->defaultProvider, [$this->defaultProvider]); $granted = $userSession->loginWithCookie('foo', $token); $this->assertSame($granted, false); @@ -424,7 +434,6 @@ class Session extends TestCase { 'bar' => new User('bar', null) ); - /** @var IUserManager | \PHPUnit_Framework_MockObject_MockObject $manager */ $manager = $this->getMockBuilder('\OC\User\Manager') ->disableOriginalConstructor() ->getMock(); @@ -432,12 +441,20 @@ class Session extends TestCase { $manager->expects($this->any()) ->method('get') ->will($this->returnCallback(function ($uid) use ($users) { - return $users[$uid]; - })); + return $users[$uid]; + })); $session = new Memory(''); $session->set('user_id', 'foo'); - $userSession = new \OC\User\Session($manager, $session); + $userSession = $this->getMockBuilder('\OC\User\Session') + ->setConstructorArgs([$manager, $session, $this->defaultProvider, [$this->defaultProvider]]) + ->setMethods([ + 'validateSession' + ]) + ->getMock(); + $userSession->expects($this->any()) + ->method('validateSession'); + $this->assertEquals($users['foo'], $userSession->getUser()); $session2 = new Memory(''); @@ -445,4 +462,5 @@ class Session extends TestCase { $userSession->setSession($session2); $this->assertEquals($users['bar'], $userSession->getUser()); } + }