From ec929f07f21341ed0e679ca27dc7e3cacd8b1b2d Mon Sep 17 00:00:00 2001 From: Christoph Wurst Date: Fri, 3 Jun 2016 08:55:00 +0200 Subject: [PATCH 1/2] When creating a session token, make sure it's the login password and not a device token --- lib/private/User/Session.php | 35 ++++++-- tests/lib/User/SessionTest.php | 159 ++++++++++++++++++++++++++++----- 2 files changed, 164 insertions(+), 30 deletions(-) diff --git a/lib/private/User/Session.php b/lib/private/User/Session.php index 362468d410..e5a2cf9c44 100644 --- a/lib/private/User/Session.php +++ b/lib/private/User/Session.php @@ -460,6 +460,7 @@ class Session implements IUserSession, Emitter { * @param string $uid user UID * @param string $loginName login name * @param string $password + * @throws SessionNotAvailableException * @return boolean */ public function createSessionToken(IRequest $request, $uid, $loginName, $password = null) { @@ -468,15 +469,37 @@ class Session implements IUserSession, Emitter { return false; } $name = isset($request->server['HTTP_USER_AGENT']) ? $request->server['HTTP_USER_AGENT'] : 'unknown browser'; - try { - $sessionId = $this->session->getId(); - $this->tokenProvider->generateToken($sessionId, $uid, $loginName, $password, $name); - } catch (SessionNotAvailableException $ex) { - - } + $sessionId = $this->session->getId(); + $pwd = $this->getPassword($password); + $this->tokenProvider->generateToken($sessionId, $uid, $loginName, $pwd, $name); return true; } + /** + * Checks if the given password is a token. + * If yes, the password is extracted from the token. + * If no, the same password is returned. + * + * @param string $password either the login password or a device token + * @return string|null the password or null if none was set in the token + */ + private function getPassword($password) { + if (is_null($password)) { + // This is surely no token ;-) + return null; + } + try { + $token = $this->tokenProvider->getToken($password); + try { + return $this->tokenProvider->getPassword($token, $password); + } catch (PasswordlessTokenException $ex) { + return null; + } + } catch (InvalidTokenException $ex) { + return $password; + } + } + /** * @param string $token * @return boolean diff --git a/tests/lib/User/SessionTest.php b/tests/lib/User/SessionTest.php index 36f14e8549..eac38ebba1 100644 --- a/tests/lib/User/SessionTest.php +++ b/tests/lib/User/SessionTest.php @@ -22,7 +22,7 @@ class SessionTest extends \Test\TestCase { private $timeFactory; /** @var \OC\Authentication\Token\DefaultTokenProvider */ - protected $defaultProvider; + protected $tokenProvider; /** @var \OCP\IConfig */ private $config; @@ -34,9 +34,7 @@ class SessionTest extends \Test\TestCase { $this->timeFactory->expects($this->any()) ->method('getTime') ->will($this->returnValue(10000)); - $this->defaultProvider = $this->getMockBuilder('\OC\Authentication\Token\DefaultTokenProvider') - ->disableOriginalConstructor() - ->getMock(); + $this->tokenProvider = $this->getMock('\OC\Authentication\Token\IProvider'); $this->config = $this->getMock('\OCP\IConfig'); } @@ -61,14 +59,14 @@ class SessionTest extends \Test\TestCase { $session->expects($this->once()) ->method('getId') ->will($this->returnValue($sessionId)); - $this->defaultProvider->expects($this->once()) + $this->tokenProvider->expects($this->once()) ->method('getToken') ->will($this->returnValue($token)); $session->expects($this->at(2)) ->method('get') ->with('last_login_check') ->will($this->returnValue(null)); // No check has been run yet - $this->defaultProvider->expects($this->once()) + $this->tokenProvider->expects($this->once()) ->method('getPassword') ->with($token, $sessionId) ->will($this->returnValue('password123')); @@ -87,7 +85,7 @@ class SessionTest extends \Test\TestCase { ->method('get') ->with('last_token_update') ->will($this->returnValue(null)); // No check run so far - $this->defaultProvider->expects($this->once()) + $this->tokenProvider->expects($this->once()) ->method('updateToken') ->with($token); $session->expects($this->at(5)) @@ -99,7 +97,7 @@ class SessionTest extends \Test\TestCase { ->with($expectedUser->getUID()) ->will($this->returnValue($expectedUser)); - $userSession = new \OC\User\Session($manager, $session, $this->timeFactory, $this->defaultProvider, $this->config); + $userSession = new \OC\User\Session($manager, $session, $this->timeFactory, $this->tokenProvider, $this->config); $user = $userSession->getUser(); $this->assertSame($expectedUser, $user); } @@ -122,7 +120,7 @@ class SessionTest extends \Test\TestCase { ->getMock(); $userSession = $this->getMockBuilder('\OC\User\Session') - ->setConstructorArgs([$manager, $session, $this->timeFactory, $this->defaultProvider, $this->config]) + ->setConstructorArgs([$manager, $session, $this->timeFactory, $this->tokenProvider, $this->config]) ->setMethods([ 'getUser' ]) @@ -149,7 +147,7 @@ class SessionTest extends \Test\TestCase { ->method('getUID') ->will($this->returnValue('foo')); - $userSession = new \OC\User\Session($manager, $session, $this->timeFactory, $this->defaultProvider, $this->config); + $userSession = new \OC\User\Session($manager, $session, $this->timeFactory, $this->tokenProvider, $this->config); $userSession->setUser($user); } @@ -201,7 +199,7 @@ class SessionTest extends \Test\TestCase { ->will($this->returnValue($user)); $userSession = $this->getMockBuilder('\OC\User\Session') - ->setConstructorArgs([$manager, $session, $this->timeFactory, $this->defaultProvider, $this->config]) + ->setConstructorArgs([$manager, $session, $this->timeFactory, $this->tokenProvider, $this->config]) ->setMethods([ 'prepareUserLogin' ]) @@ -248,7 +246,7 @@ class SessionTest extends \Test\TestCase { ->with('foo', 'bar') ->will($this->returnValue($user)); - $userSession = new \OC\User\Session($manager, $session, $this->timeFactory, $this->defaultProvider, $this->config); + $userSession = new \OC\User\Session($manager, $session, $this->timeFactory, $this->tokenProvider, $this->config); $userSession->login('foo', 'bar'); } @@ -284,7 +282,7 @@ class SessionTest extends \Test\TestCase { ->with('foo', 'bar') ->will($this->returnValue(false)); - $userSession = new \OC\User\Session($manager, $session, $this->timeFactory, $this->defaultProvider, $this->config); + $userSession = new \OC\User\Session($manager, $session, $this->timeFactory, $this->tokenProvider, $this->config); $userSession->login('foo', 'bar'); } @@ -304,7 +302,7 @@ class SessionTest extends \Test\TestCase { ->with('foo', 'bar') ->will($this->returnValue(false)); - $userSession = new \OC\User\Session($manager, $session, $this->timeFactory, $this->defaultProvider, $this->config); + $userSession = new \OC\User\Session($manager, $session, $this->timeFactory, $this->tokenProvider, $this->config); $userSession->login('foo', 'bar'); } @@ -316,11 +314,11 @@ class SessionTest extends \Test\TestCase { /** @var \OC\User\Session $userSession */ $userSession = $this->getMockBuilder('\OC\User\Session') - ->setConstructorArgs([$manager, $session, $this->timeFactory, $this->defaultProvider, $this->config]) + ->setConstructorArgs([$manager, $session, $this->timeFactory, $this->tokenProvider, $this->config]) ->setMethods(['login']) ->getMock(); - $this->defaultProvider->expects($this->once()) + $this->tokenProvider->expects($this->once()) ->method('getToken') ->with('doe') ->will($this->throwException(new \OC\Authentication\Exceptions\InvalidTokenException())); @@ -341,11 +339,11 @@ class SessionTest extends \Test\TestCase { /** @var \OC\User\Session $userSession */ $userSession = $this->getMockBuilder('\OC\User\Session') - ->setConstructorArgs([$manager, $session, $this->timeFactory, $this->defaultProvider, $this->config]) + ->setConstructorArgs([$manager, $session, $this->timeFactory, $this->tokenProvider, $this->config]) ->setMethods(['login', 'isTwoFactorEnforced']) ->getMock(); - $this->defaultProvider->expects($this->once()) + $this->tokenProvider->expects($this->once()) ->method('getToken') ->with('doe') ->will($this->throwException(new \OC\Authentication\Exceptions\InvalidTokenException())); @@ -413,7 +411,7 @@ class SessionTest extends \Test\TestCase { //override, otherwise tests will fail because of setcookie() array('setMagicInCookie'), //there are passed as parameters to the constructor - array($manager, $session, $this->timeFactory, $this->defaultProvider, $this->config)); + array($manager, $session, $this->timeFactory, $this->tokenProvider, $this->config)); $granted = $userSession->loginWithCookie('foo', $token); @@ -458,7 +456,7 @@ class SessionTest extends \Test\TestCase { $token = 'goodToken'; \OC::$server->getConfig()->setUserValue('foo', 'login_token', $token, time()); - $userSession = new \OC\User\Session($manager, $session, $this->timeFactory, $this->defaultProvider, $this->config); + $userSession = new \OC\User\Session($manager, $session, $this->timeFactory, $this->tokenProvider, $this->config); $granted = $userSession->loginWithCookie('foo', 'badToken'); $this->assertSame($granted, false); @@ -501,7 +499,7 @@ class SessionTest extends \Test\TestCase { $token = 'goodToken'; \OC::$server->getConfig()->setUserValue('foo', 'login_token', $token, time()); - $userSession = new \OC\User\Session($manager, $session, $this->timeFactory, $this->defaultProvider, $this->config); + $userSession = new \OC\User\Session($manager, $session, $this->timeFactory, $this->tokenProvider, $this->config); $granted = $userSession->loginWithCookie('foo', $token); $this->assertSame($granted, false); @@ -526,7 +524,7 @@ class SessionTest extends \Test\TestCase { $session = new Memory(''); $session->set('user_id', 'foo'); $userSession = $this->getMockBuilder('\OC\User\Session') - ->setConstructorArgs([$manager, $session, $this->timeFactory, $this->defaultProvider, $this->config]) + ->setConstructorArgs([$manager, $session, $this->timeFactory, $this->tokenProvider, $this->config]) ->setMethods([ 'validateSession' ]) @@ -542,6 +540,119 @@ class SessionTest extends \Test\TestCase { $this->assertEquals($users['bar'], $userSession->getUser()); } + public function testCreateSessionToken() { + $manager = $this->getMockBuilder('\OC\User\Manager') + ->disableOriginalConstructor() + ->getMock(); + $session = $this->getMock('\OCP\ISession'); + $token = $this->getMock('\OC\Authentication\Token\IToken'); + $user = $this->getMock('\OCP\IUser'); + $userSession = new \OC\User\Session($manager, $session, $this->timeFactory, $this->tokenProvider, $this->config); + + $random = $this->getMock('\OCP\Security\ISecureRandom'); + $config = $this->getMock('\OCP\IConfig'); + $csrf = $this->getMockBuilder('\OC\Security\CSRF\CsrfTokenManager') + ->disableOriginalConstructor() + ->getMock(); + $request = new \OC\AppFramework\Http\Request([ + 'server' => [ + 'HTTP_USER_AGENT' => 'Firefox', + ] + ], $random, $config, $csrf); + + $uid = 'user123'; + $loginName = 'User123'; + $password = 'passme'; + $sessionId = 'abcxyz'; + + $manager->expects($this->once()) + ->method('get') + ->with($uid) + ->will($this->returnValue($user)); + $session->expects($this->once()) + ->method('getId') + ->will($this->returnValue($sessionId)); + $this->tokenProvider->expects($this->once()) + ->method('getToken') + ->with($password) + ->will($this->throwException(new \OC\Authentication\Exceptions\InvalidTokenException())); + + $this->tokenProvider->expects($this->once()) + ->method('generateToken') + ->with($sessionId, $uid, $loginName, $password, 'Firefox'); + + $this->assertTrue($userSession->createSessionToken($request, $uid, $loginName, $password)); + } + + public function testCreateSessionTokenWithTokenPassword() { + $manager = $this->getMockBuilder('\OC\User\Manager') + ->disableOriginalConstructor() + ->getMock(); + $session = $this->getMock('\OCP\ISession'); + $token = $this->getMock('\OC\Authentication\Token\IToken'); + $user = $this->getMock('\OCP\IUser'); + $userSession = new \OC\User\Session($manager, $session, $this->timeFactory, $this->tokenProvider, $this->config); + + $random = $this->getMock('\OCP\Security\ISecureRandom'); + $config = $this->getMock('\OCP\IConfig'); + $csrf = $this->getMockBuilder('\OC\Security\CSRF\CsrfTokenManager') + ->disableOriginalConstructor() + ->getMock(); + $request = new \OC\AppFramework\Http\Request([ + 'server' => [ + 'HTTP_USER_AGENT' => 'Firefox', + ] + ], $random, $config, $csrf); + + $uid = 'user123'; + $loginName = 'User123'; + $password = 'iamatoken'; + $realPassword = 'passme'; + $sessionId = 'abcxyz'; + + $manager->expects($this->once()) + ->method('get') + ->with($uid) + ->will($this->returnValue($user)); + $session->expects($this->once()) + ->method('getId') + ->will($this->returnValue($sessionId)); + $this->tokenProvider->expects($this->once()) + ->method('getToken') + ->with($password) + ->will($this->returnValue($token)); + $this->tokenProvider->expects($this->once()) + ->method('getPassword') + ->with($token, $password) + ->will($this->returnValue($realPassword)); + + $this->tokenProvider->expects($this->once()) + ->method('generateToken') + ->with($sessionId, $uid, $loginName, $realPassword, 'Firefox'); + + $this->assertTrue($userSession->createSessionToken($request, $uid, $loginName, $password)); + } + + public function testCreateSessionTokenWithNonExistentUser() { + $manager = $this->getMockBuilder('\OC\User\Manager') + ->disableOriginalConstructor() + ->getMock(); + $session = $this->getMock('\OCP\ISession'); + $userSession = new \OC\User\Session($manager, $session, $this->timeFactory, $this->tokenProvider, $this->config); + $request = $this->getMock('\OCP\IRequest'); + + $uid = 'user123'; + $loginName = 'User123'; + $password = 'passme'; + + $manager->expects($this->once()) + ->method('get') + ->with($uid) + ->will($this->returnValue(null)); + + $this->assertFalse($userSession->createSessionToken($request, $uid, $loginName, $password)); + } + public function testTryTokenLoginWithDisabledUser() { $manager = $this->getMockBuilder('\OC\User\Manager') ->disableOriginalConstructor() @@ -549,14 +660,14 @@ class SessionTest extends \Test\TestCase { $session = new Memory(''); $token = $this->getMock('\OC\Authentication\Token\IToken'); $user = $this->getMock('\OCP\IUser'); - $userSession = new \OC\User\Session($manager, $session, $this->timeFactory, $this->defaultProvider, $this->config); + $userSession = new \OC\User\Session($manager, $session, $this->timeFactory, $this->tokenProvider, $this->config); $request = $this->getMock('\OCP\IRequest'); $request->expects($this->once()) ->method('getHeader') ->with('Authorization') ->will($this->returnValue('token xxxxx')); - $this->defaultProvider->expects($this->once()) + $this->tokenProvider->expects($this->once()) ->method('validateToken') ->with('xxxxx') ->will($this->returnValue($token)); From 46e26f6b4939759c6037fe848c5f56854a70ca3b Mon Sep 17 00:00:00 2001 From: Christoph Wurst Date: Wed, 8 Jun 2016 15:03:15 +0200 Subject: [PATCH 2/2] catch sessionnotavailable exception if memory session is used --- lib/private/User/Session.php | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-) diff --git a/lib/private/User/Session.php b/lib/private/User/Session.php index e5a2cf9c44..e1ede95e2a 100644 --- a/lib/private/User/Session.php +++ b/lib/private/User/Session.php @@ -460,7 +460,6 @@ class Session implements IUserSession, Emitter { * @param string $uid user UID * @param string $loginName login name * @param string $password - * @throws SessionNotAvailableException * @return boolean */ public function createSessionToken(IRequest $request, $uid, $loginName, $password = null) { @@ -469,10 +468,16 @@ class Session implements IUserSession, Emitter { return false; } $name = isset($request->server['HTTP_USER_AGENT']) ? $request->server['HTTP_USER_AGENT'] : 'unknown browser'; - $sessionId = $this->session->getId(); - $pwd = $this->getPassword($password); - $this->tokenProvider->generateToken($sessionId, $uid, $loginName, $pwd, $name); - return true; + try { + $sessionId = $this->session->getId(); + $pwd = $this->getPassword($password); + $this->tokenProvider->generateToken($sessionId, $uid, $loginName, $pwd, $name); + return true; + } catch (SessionNotAvailableException $ex) { + // This can happen with OCC, where a memory session is used + // if a memory session is used, we shouldn't create a session token anyway + return false; + } } /**