From 67dfc627005625328a2a3d3a9b46407763acb617 Mon Sep 17 00:00:00 2001 From: Christoph Wurst Date: Tue, 6 Sep 2016 21:41:15 +0200 Subject: [PATCH] bring back old remember-me code Signed-off-by: Christoph Wurst --- core/Controller/LoginController.php | 6 + lib/base.php | 3 + lib/private/User/Session.php | 31 ++-- lib/public/IRequest.php | 2 +- tests/lib/User/SessionTest.php | 229 ++++++++++++---------------- 5 files changed, 121 insertions(+), 150 deletions(-) diff --git a/core/Controller/LoginController.php b/core/Controller/LoginController.php index 884eea8869ef3..cac85bbbf3c83 100644 --- a/core/Controller/LoginController.php +++ b/core/Controller/LoginController.php @@ -238,6 +238,12 @@ public function tryLogin($user, $password, $redirect_url) { $this->userSession->login($user, $password); $this->userSession->createSessionToken($this->request, $loginResult->getUID(), $user, $password); + if (true) { + $token = \OC::$server->getSecureRandom()->generate(32); + $this->config->setUserValue($originalUser, 'login_token', $token, time()); + $this->userSession->setMagicInCookie($originalUser, $token); + } + // User has successfully logged in, now remove the password reset link, when it is available $this->config->deleteUserValue($loginResult->getUID(), 'core', 'lostpassword'); diff --git a/lib/base.php b/lib/base.php index 7d86245818df0..e73c35ebec356 100644 --- a/lib/base.php +++ b/lib/base.php @@ -1039,6 +1039,9 @@ static function handleLogin(OCP\IRequest $request) { if ($userSession->tryTokenLogin($request)) { return true; } + if ($userSession->loginWithCookie($_COOKIE['nc_username'], $_COOKIE['nc_token'])) { + return true; + } if ($userSession->tryBasicAuthLogin($request, \OC::$server->getBruteForceThrottler())) { return true; } diff --git a/lib/private/User/Session.php b/lib/private/User/Session.php index dec959820f868..860f46b7fed0e 100644 --- a/lib/private/User/Session.php +++ b/lib/private/User/Session.php @@ -691,15 +691,15 @@ public function loginWithCookie($uid, $currentToken) { } // get stored tokens - $tokens = OC::$server->getConfig()->getUserKeys($uid, 'login_token'); + $tokens = $this->config->getUserKeys($uid, 'login_token'); // test cookies token against stored tokens if (!in_array($currentToken, $tokens, true)) { return false; } // replace successfully used token with a new one - OC::$server->getConfig()->deleteUserValue($uid, 'login_token', $currentToken); + $this->config->deleteUserValue($uid, 'login_token', $currentToken); $newToken = OC::$server->getSecureRandom()->generate(32); - OC::$server->getConfig()->setUserValue($uid, 'login_token', $newToken, time()); + $this->config->setUserValue($uid, 'login_token', $newToken, $this->timeFacory->getTime()); $this->setMagicInCookie($user->getUID(), $newToken); //login @@ -736,9 +736,9 @@ public function logout() { public function setMagicInCookie($username, $token) { $secureCookie = OC::$server->getRequest()->getServerProtocol() === 'https'; $expires = time() + OC::$server->getConfig()->getSystemValue('remember_login_cookie_lifetime', 60 * 60 * 24 * 15); - setcookie('oc_username', $username, $expires, OC::$WEBROOT, '', $secureCookie, true); - setcookie('oc_token', $token, $expires, OC::$WEBROOT, '', $secureCookie, true); - setcookie('oc_remember_login', '1', $expires, OC::$WEBROOT, '', $secureCookie, true); + setcookie('nc_username', $username, $expires, OC::$WEBROOT, '', $secureCookie, true); + setcookie('nc_token', $token, $expires, OC::$WEBROOT, '', $secureCookie, true); + setcookie('nc_remember_login', '1', $expires, OC::$WEBROOT, '', $secureCookie, true); } /** @@ -748,17 +748,17 @@ public function unsetMagicInCookie() { //TODO: DI for cookies and IRequest $secureCookie = OC::$server->getRequest()->getServerProtocol() === 'https'; - unset($_COOKIE['oc_username']); //TODO: DI - unset($_COOKIE['oc_token']); - unset($_COOKIE['oc_remember_login']); - setcookie('oc_username', '', time() - 3600, OC::$WEBROOT, '', $secureCookie, true); - setcookie('oc_token', '', time() - 3600, OC::$WEBROOT, '', $secureCookie, true); - setcookie('oc_remember_login', '', time() - 3600, OC::$WEBROOT, '', $secureCookie, true); + unset($_COOKIE['nc_username']); //TODO: DI + unset($_COOKIE['nc_token']); + unset($_COOKIE['nc_remember_login']); + setcookie('nc_username', '', time() - 3600, OC::$WEBROOT, '', $secureCookie, true); + setcookie('nc_token', '', time() - 3600, OC::$WEBROOT, '', $secureCookie, true); + setcookie('nc_remember_login', '', time() - 3600, OC::$WEBROOT, '', $secureCookie, true); // old cookies might be stored under /webroot/ instead of /webroot // and Firefox doesn't like it! - setcookie('oc_username', '', time() - 3600, OC::$WEBROOT . '/', '', $secureCookie, true); - setcookie('oc_token', '', time() - 3600, OC::$WEBROOT . '/', '', $secureCookie, true); - setcookie('oc_remember_login', '', time() - 3600, OC::$WEBROOT . '/', '', $secureCookie, true); + setcookie('nc_username', '', time() - 3600, OC::$WEBROOT . '/', '', $secureCookie, true); + setcookie('nc_token', '', time() - 3600, OC::$WEBROOT . '/', '', $secureCookie, true); + setcookie('nc_remember_login', '', time() - 3600, OC::$WEBROOT . '/', '', $secureCookie, true); } /** @@ -778,4 +778,5 @@ public function updateSessionTokenPassword($password) { } } + } diff --git a/lib/public/IRequest.php b/lib/public/IRequest.php index 11242c481f0ef..b36a934b0c21b 100644 --- a/lib/public/IRequest.php +++ b/lib/public/IRequest.php @@ -145,7 +145,7 @@ public function getEnv($key); * Shortcut for getting cookie variables * * @param string $key the key that will be taken from the $_COOKIE array - * @return string the value in the $_COOKIE element + * @return string|null the value in the $_COOKIE element * @since 6.0.0 */ public function getCookie($key); diff --git a/tests/lib/User/SessionTest.php b/tests/lib/User/SessionTest.php index 2cd6b9b3bedd4..d58a23e681399 100644 --- a/tests/lib/User/SessionTest.php +++ b/tests/lib/User/SessionTest.php @@ -48,6 +48,8 @@ protected function setUp() { $this->tokenProvider = $this->createMock(IProvider::class); $this->config = $this->createMock(IConfig::class); $this->throttler = $this->createMock(Throttler::class); + + \OC_User::setIncognitoMode(false); } public function testGetUser() { @@ -96,7 +98,7 @@ public function testGetUser() { ->method('updateTokenActivity') ->with($token); - $manager->expects($this->any()) + $manager->expects($this->once()) ->method('get') ->with($expectedUser->getUID()) ->will($this->returnValue($expectedUser)); @@ -178,17 +180,10 @@ public function testLoginValidPasswordEnabled() { } }, 'foo')); - $managerMethods = get_class_methods('\OC\User\Manager'); - //keep following methods intact in order to ensure hooks are - //working - $doNotMock = array('__construct', 'emit', 'listen'); - foreach ($doNotMock as $methodName) { - $i = array_search($methodName, $managerMethods, true); - if ($i !== false) { - unset($managerMethods[$i]); - } - } - $manager = $this->getMockBuilder(Manager::class)->setMethods($managerMethods)->getMock(); + $managerMethods = get_class_methods(\OC\User\Manager::class); + //keep following methods intact in order to ensure hooks are working + $mockedManagerMethods = array_diff($managerMethods, ['__construct', 'emit', 'listen']); + $manager = $this->getMockBuilder(Manager::class)->setMethods($mockedManagerMethods)->getMock(); $backend = $this->createMock(\Test\Util\User\Dummy::class); @@ -233,17 +228,10 @@ public function testLoginValidPasswordDisabled() { ->with('bar') ->will($this->throwException(new \OC\Authentication\Exceptions\InvalidTokenException())); - $managerMethods = get_class_methods('\OC\User\Manager'); - //keep following methods intact in order to ensure hooks are - //working - $doNotMock = array('__construct', 'emit', 'listen'); - foreach ($doNotMock as $methodName) { - $i = array_search($methodName, $managerMethods, true); - if ($i !== false) { - unset($managerMethods[$i]); - } - } - $manager = $this->getMockBuilder(Manager::class)->setMethods($managerMethods)->getMock(); + $managerMethods = get_class_methods(\OC\User\Manager::class); + //keep following methods intact in order to ensure hooks are working + $mockedManagerMethods = array_diff($managerMethods, ['__construct', 'emit', 'listen']); + $manager = $this->getMockBuilder(Manager::class)->setMethods($mockedManagerMethods)->getMock(); $backend = $this->createMock(\Test\Util\User\Dummy::class); @@ -265,17 +253,10 @@ public function testLoginValidPasswordDisabled() { public function testLoginInvalidPassword() { $session = $this->getMockBuilder(Memory::class)->setConstructorArgs([''])->getMock(); - $managerMethods = get_class_methods('\OC\User\Manager'); - //keep following methods intact in order to ensure hooks are - //working - $doNotMock = array('__construct', 'emit', 'listen'); - foreach ($doNotMock as $methodName) { - $i = array_search($methodName, $managerMethods, true); - if ($i !== false) { - unset($managerMethods[$i]); - } - } - $manager = $this->getMockBuilder(Manager::class)->setMethods($managerMethods)->getMock(); + $managerMethods = get_class_methods(\OC\User\Manager::class); + //keep following methods intact in order to ensure hooks are working + $mockedManagerMethods = array_diff($managerMethods, ['__construct', 'emit', 'listen']); + $manager = $this->getMockBuilder(Manager::class)->setMethods($mockedManagerMethods)->getMock(); $backend = $this->createMock(\Test\Util\User\Dummy::class); $userSession = new \OC\User\Session($manager, $session, $this->timeFactory, $this->tokenProvider, $this->config); @@ -487,147 +468,127 @@ public function testLogClientInNoTokenPasswordNo2fa() { public function testRememberLoginValidToken() { $session = $this->getMockBuilder(Memory::class)->setConstructorArgs([''])->getMock(); - $session->expects($this->exactly(1)) - ->method('set') - ->with($this->callback(function ($key) { - switch ($key) { - case 'user_id': - return true; - default: - return false; - } - }, 'foo')); - $session->expects($this->once()) - ->method('regenerateId'); + $managerMethods = get_class_methods(\OC\User\Manager::class); + //keep following methods intact in order to ensure hooks are working + $mockedManagerMethods = array_diff($managerMethods, ['__construct', 'emit', 'listen']); + $manager = $this->getMockBuilder(Manager::class)->setMethods($mockedManagerMethods)->getMock(); + $userSession = $this->getMockBuilder(Session::class) + //override, otherwise tests will fail because of setcookie() + ->setMethods(['setMagicInCookie']) + ->setConstructorArgs([$manager, $session, $this->timeFactory, $this->tokenProvider, $this->config]) + ->getMock(); - $managerMethods = get_class_methods('\OC\User\Manager'); - //keep following methods intact in order to ensure hooks are - //working - $doNotMock = array('__construct', 'emit', 'listen'); - foreach ($doNotMock as $methodName) { - $i = array_search($methodName, $managerMethods, true); - if ($i !== false) { - unset($managerMethods[$i]); - } - } - $manager = $this->getMockBuilder(Manager::class)->setMethods($managerMethods)->getMock(); + $user = $this->createMock(IUser::class); + $token = 'goodToken'; - $backend = $this->createMock(\Test\Util\User\Dummy::class); - $user = $this->getMockBuilder(User::class)->setConstructorArgs(['foo', $backend])->getMock(); + $session->expects($this->once()) + ->method('regenerateId'); + $manager->expects($this->once()) + ->method('get') + ->with('foo') + ->will($this->returnValue($user)); + $this->config->expects($this->once()) + ->method('getUserKeys') + ->with('foo', 'login_token') + ->will($this->returnValue([$token])); + $this->config->expects($this->once()) + ->method('deleteUserValue') + ->with('foo', 'login_token', $token); + $this->config->expects($this->once()) + ->method('setUserValue'); // TODO: mock new random value $user->expects($this->any()) ->method('getUID') ->will($this->returnValue('foo')); + $userSession->expects($this->once()) + ->method('setMagicInCookie'); $user->expects($this->once()) ->method('updateLastLoginTimestamp'); + $session->expects($this->once()) + ->method('set') + ->with('user_id', 'foo'); - $manager->expects($this->once()) - ->method('get') - ->with('foo') - ->will($this->returnValue($user)); + $granted = $userSession->loginWithCookie('foo', $token); - //prepare login token - $token = 'goodToken'; - \OC::$server->getConfig()->setUserValue('foo', 'login_token', $token, time()); + $this->assertTrue($granted); + } + public function testRememberLoginInvalidToken() { + $session = $this->getMockBuilder(Memory::class)->setConstructorArgs([''])->getMock(); + $managerMethods = get_class_methods(\OC\User\Manager::class); + //keep following methods intact in order to ensure hooks are working + $mockedManagerMethods = array_diff($managerMethods, ['__construct', 'emit', 'listen']); + $manager = $this->getMockBuilder(Manager::class)->setMethods($mockedManagerMethods)->getMock(); $userSession = $this->getMockBuilder(Session::class) //override, otherwise tests will fail because of setcookie() ->setMethods(['setMagicInCookie']) - //there are passed as parameters to the constructor ->setConstructorArgs([$manager, $session, $this->timeFactory, $this->tokenProvider, $this->config]) ->getMock(); - $granted = $userSession->loginWithCookie('foo', $token); - - $this->assertSame($granted, true); - } + $user = $this->createMock(IUser::class); + $token = 'goodToken'; - public function testRememberLoginInvalidToken() { - $session = $this->getMockBuilder(Memory::class)->setConstructorArgs([''])->getMock(); - $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 - //working - $doNotMock = array('__construct', 'emit', 'listen'); - foreach ($doNotMock as $methodName) { - $i = array_search($methodName, $managerMethods, true); - if ($i !== false) { - unset($managerMethods[$i]); - } - } - $manager = $this->getMockBuilder(Manager::class)->setMethods($managerMethods)->getMock(); - - $backend = $this->createMock(\Test\Util\User\Dummy::class); - - $user = $this->getMockBuilder(User::class)->setConstructorArgs(['foo', $backend])->getMock(); - - $user->expects($this->any()) - ->method('getUID') - ->will($this->returnValue('foo')); - $user->expects($this->never()) - ->method('updateLastLoginTimestamp'); - $manager->expects($this->once()) ->method('get') ->with('foo') ->will($this->returnValue($user)); + $this->config->expects($this->once()) + ->method('getUserKeys') + ->with('foo', 'login_token') + ->will($this->returnValue(['anothertoken'])); + $this->config->expects($this->never()) + ->method('deleteUserValue') + ->with('foo', 'login_token', $token); + + $userSession->expects($this->never()) + ->method('setMagicInCookie'); + $user->expects($this->never()) + ->method('updateLastLoginTimestamp'); + $session->expects($this->never()) + ->method('set') + ->with('user_id', 'foo'); - //prepare login token - $token = 'goodToken'; - \OC::$server->getConfig()->setUserValue('foo', 'login_token', $token, time()); - - $userSession = new \OC\User\Session($manager, $session, $this->timeFactory, $this->tokenProvider, $this->config); - $granted = $userSession->loginWithCookie('foo', 'badToken'); + $granted = $userSession->loginWithCookie('foo', $token); - $this->assertSame($granted, false); + $this->assertFalse($granted); } public function testRememberLoginInvalidUser() { $session = $this->getMockBuilder(Memory::class)->setConstructorArgs([''])->getMock(); - $session->expects($this->never()) - ->method('set'); + $managerMethods = get_class_methods(\OC\User\Manager::class); + //keep following methods intact in order to ensure hooks are working + $mockedManagerMethods = array_diff($managerMethods, ['__construct', 'emit', 'listen']); + $manager = $this->getMockBuilder(Manager::class)->setMethods($mockedManagerMethods)->getMock(); + $userSession = $this->getMockBuilder(Session::class) + //override, otherwise tests will fail because of setcookie() + ->setMethods(['setMagicInCookie']) + ->setConstructorArgs([$manager, $session, $this->timeFactory, $this->tokenProvider, $this->config]) + ->getMock(); + $token = 'goodToken'; + $session->expects($this->once()) ->method('regenerateId'); - - $managerMethods = get_class_methods('\OC\User\Manager'); - //keep following methods intact in order to ensure hooks are - //working - $doNotMock = array('__construct', 'emit', 'listen'); - foreach ($doNotMock as $methodName) { - $i = array_search($methodName, $managerMethods, true); - if ($i !== false) { - unset($managerMethods[$i]); - } - } - $manager = $this->getMockBuilder(Manager::class)->setMethods($managerMethods)->getMock(); - - $backend = $this->createMock(\Test\Util\User\Dummy::class); - - $user = $this->getMockBuilder(User::class)->setConstructorArgs(['foo', $backend])->getMock(); - - $user->expects($this->never()) - ->method('getUID'); - $user->expects($this->never()) - ->method('updateLastLoginTimestamp'); - $manager->expects($this->once()) ->method('get') ->with('foo') ->will($this->returnValue(null)); + $this->config->expects($this->never()) + ->method('getUserKeys') + ->with('foo', 'login_token') + ->will($this->returnValue(['anothertoken'])); - //prepare login token - $token = 'goodToken'; - \OC::$server->getConfig()->setUserValue('foo', 'login_token', $token, time()); + $userSession->expects($this->never()) + ->method('setMagicInCookie'); + $session->expects($this->never()) + ->method('set') + ->with('user_id', 'foo'); - $userSession = new \OC\User\Session($manager, $session, $this->timeFactory, $this->tokenProvider, $this->config); $granted = $userSession->loginWithCookie('foo', $token); - $this->assertSame($granted, false); + $this->assertFalse($granted); } public function testActiveUserAfterSetSession() {