Merge pull request #1884 from nextcloud/downstream-26292

Fix logClientIn for non-existing users (#26292)

2 files changed, 46 insertions, 34 deletions

diff --git a/lib/private/User/Session.php b/lib/private/User/Session.php
index 4b56609ccfc..a213ee48c2a 100644
--- a/lib/private/User/Session.php
+++ b/lib/private/User/Session.php
@@ -362,6 +362,9 @@ class Session implements IUserSession, Emitter {
 		$user = $this->manager->get($username);
 		if (is_null($user)) {
 			$users = $this->manager->getByEmail($username);
+			if (empty($users)) {
+				return false;
+			}
 			if (count($users) !== 1) {
 				return true;
 			}
diff --git a/tests/lib/User/SessionTest.php b/tests/lib/User/SessionTest.php
index 21ac1b655b9..1b3d5cc4601 100644
--- a/tests/lib/User/SessionTest.php
+++ b/tests/lib/User/SessionTest.php
@@ -33,11 +33,11 @@ use OCP\Security\ISecureRandom;
  * @package Test\User
  */
 class SessionTest extends \Test\TestCase {
-	/** @var \OCP\AppFramework\Utility\ITimeFactory */
+	/** @var ITimeFactory|\PHPUnit_Framework_MockObject_MockObject */
 	private $timeFactory;
-	/** @var \OC\Authentication\Token\DefaultTokenProvider */
+	/** @var DefaultTokenProvider|\PHPUnit_Framework_MockObject_MockObject */
 	protected $tokenProvider;
-	/** @var \OCP\IConfig */
+	/** @var IConfig|\PHPUnit_Framework_MockObject_MockObject */
 	private $config;
 	/** @var Throttler */
 	private $throttler;
@@ -124,11 +124,9 @@ class SessionTest extends \Test\TestCase {
 	public function testIsLoggedIn($isLoggedIn) {
 		$session = $this->getMockBuilder(Memory::class)->setConstructorArgs([''])->getMock();
 
-		$manager = $this->getMockBuilder('\OC\User\Manager')
-			->disableOriginalConstructor()
-			->getMock();
+		$manager = $this->createMock(Manager::class);
 
-		$userSession = $this->getMockBuilder('\OC\User\Session')
+		$userSession = $this->getMockBuilder(Session::class)
 			->setConstructorArgs([$manager, $session, $this->timeFactory, $this->tokenProvider, $this->config])
 			->setMethods([
 				'getUser'
@@ -182,7 +180,7 @@ class SessionTest extends \Test\TestCase {
 				}
 			}, 'foo'));
 
-		$managerMethods = get_class_methods('\OC\User\Manager');
+		$managerMethods = get_class_methods(Manager::class);
 		//keep following methods intact in order to ensure hooks are
 		//working
 		$doNotMock = array('__construct', 'emit', 'listen');
@@ -211,7 +209,7 @@ class SessionTest extends \Test\TestCase {
 			->with('foo', 'bar')
 			->will($this->returnValue($user));
 
-		$userSession = $this->getMockBuilder('\OC\User\Session')
+		$userSession = $this->getMockBuilder(Session::class)
 			->setConstructorArgs([$manager, $session, $this->timeFactory, $this->tokenProvider, $this->config])
 			->setMethods([
 				'prepareUserLogin'
@@ -310,7 +308,6 @@ class SessionTest extends \Test\TestCase {
 	public function testLoginNonExisting() {
 		$session = $this->getMockBuilder(Memory::class)->setConstructorArgs([''])->getMock();
 		$manager = $this->createMock(Manager::class);
-		$backend = $this->createMock(\Test\Util\User\Dummy::class);
 		$userSession = new \OC\User\Session($manager, $session, $this->timeFactory, $this->tokenProvider, $this->config);
 
 		$session->expects($this->never())
@@ -337,7 +334,6 @@ class SessionTest extends \Test\TestCase {
 	public function testLoginWithDifferentTokenLoginName() {
 		$session = $this->getMockBuilder(Memory::class)->setConstructorArgs([''])->getMock();
 		$manager = $this->createMock(Manager::class);
-		$backend = $this->createMock(\Test\Util\User\Dummy::class);
 		$userSession = new \OC\User\Session($manager, $session, $this->timeFactory, $this->tokenProvider, $this->config);
 		$username = 'user123';
 		$token = new \OC\Authentication\Token\DefaultToken();
@@ -364,14 +360,12 @@ class SessionTest extends \Test\TestCase {
 	 * @expectedException \OC\Authentication\Exceptions\PasswordLoginForbiddenException
 	 */
 	public function testLogClientInNoTokenPasswordWith2fa() {
-		$manager = $this->getMockBuilder('\OC\User\Manager')
-			->disableOriginalConstructor()
-			->getMock();
+		$manager = $this->createMock(Manager::class);
 		$session = $this->createMock(ISession::class);
 		$request = $this->createMock(IRequest::class);
 
 		/** @var \OC\User\Session $userSession */
-		$userSession = $this->getMockBuilder('\OC\User\Session')
+		$userSession = $this->getMockBuilder(Session::class)
 			->setConstructorArgs([$manager, $session, $this->timeFactory, $this->tokenProvider, $this->config])
 			->setMethods(['login', 'supportsCookies', 'createSessionToken', 'getUser'])
 			->getMock();
@@ -401,15 +395,36 @@ class SessionTest extends \Test\TestCase {
 		$userSession->logClientIn('john', 'doe', $request, $this->throttler);
 	}
 
-	public function testLogClientInWithTokenPassword() {
-		$manager = $this->getMockBuilder('\OC\User\Manager')
-			->disableOriginalConstructor()
+	public function testLogClientInUnexist() {
+		$manager = $this->createMock(Manager::class);
+		$session = $this->createMock(ISession::class);
+		$request = $this->createMock(IRequest::class);
+
+		/** @var Session $userSession */
+		$userSession = $this->getMockBuilder(Session::class)
+			->setConstructorArgs([$manager, $session, $this->timeFactory, $this->tokenProvider, $this->config])
+			->setMethods(['login', 'supportsCookies', 'createSessionToken', 'getUser'])
 			->getMock();
+
+		$this->tokenProvider->expects($this->once())
+			->method('getToken')
+			->with('doe')
+			->will($this->throwException(new \OC\Authentication\Exceptions\InvalidTokenException()));
+		$this->config->expects($this->once())
+			->method('getSystemValue')
+			->with('token_auth_enforced', false)
+			->will($this->returnValue(false));
+
+		$this->assertFalse($userSession->logClientIn('unexist', 'doe', $request, $this->throttler));
+	}
+
+	public function testLogClientInWithTokenPassword() {
+		$manager = $this->createMock(Manager::class);
 		$session = $this->createMock(ISession::class);
 		$request = $this->createMock(IRequest::class);
 
 		/** @var \OC\User\Session $userSession */
-		$userSession = $this->getMockBuilder('\OC\User\Session')
+		$userSession = $this->getMockBuilder(Session::class)
 			->setConstructorArgs([$manager, $session, $this->timeFactory, $this->tokenProvider, $this->config])
 			->setMethods(['isTokenPassword', 'login', 'supportsCookies', 'createSessionToken', 'getUser'])
 			->getMock();
@@ -446,14 +461,12 @@ class SessionTest extends \Test\TestCase {
 	 * @expectedException \OC\Authentication\Exceptions\PasswordLoginForbiddenException
 	 */
 	public function testLogClientInNoTokenPasswordNo2fa() {
-		$manager = $this->getMockBuilder('\OC\User\Manager')
-			->disableOriginalConstructor()
-			->getMock();
+		$manager = $this->createMock(Manager::class);
 		$session = $this->createMock(ISession::class);
 		$request = $this->createMock(IRequest::class);
 
 		/** @var \OC\User\Session $userSession */
-		$userSession = $this->getMockBuilder('\OC\User\Session')
+		$userSession = $this->getMockBuilder(Session::class)
 			->setConstructorArgs([$manager, $session, $this->timeFactory, $this->tokenProvider, $this->config])
 			->setMethods(['login', 'isTwoFactorEnforced'])
 			->getMock();
@@ -504,7 +517,7 @@ class SessionTest extends \Test\TestCase {
 		$session->expects($this->once())
 			->method('regenerateId');
 
-		$managerMethods = get_class_methods('\OC\User\Manager');
+		$managerMethods = get_class_methods(Manager::class);
 		//keep following methods intact in order to ensure hooks are
 		//working
 		$doNotMock = array('__construct', 'emit', 'listen');
@@ -966,9 +979,7 @@ class SessionTest extends \Test\TestCase {
 	}
 
 	public function testUpdateAuthTokenLastCheck() {
-		$manager = $this->getMockBuilder('\OC\User\Manager')
-			->disableOriginalConstructor()
-			->getMock();
+		$manager = $this->createMock(Manager::class);
 		$session = $this->createMock(ISession::class);
 		$request = $this->createMock(IRequest::class);
 
@@ -981,8 +992,8 @@ class SessionTest extends \Test\TestCase {
 		$mapper = $this->getMockBuilder(DefaultTokenMapper::class)
 			->disableOriginalConstructor()
 			->getMock();
-		$crypto = $this->getMock(ICrypto::class);
-		$logger = $this->getMock(ILogger::class);
+		$crypto = $this->createMock(ICrypto::class);
+		$logger = $this->createMock(ILogger::class);
 		$tokenProvider = new DefaultTokenProvider($mapper, $crypto, $this->config, $logger, $this->timeFactory);
 
 		/** @var \OC\User\Session $userSession */
@@ -1018,9 +1029,7 @@ class SessionTest extends \Test\TestCase {
 	}
 
 	public function testNoUpdateAuthTokenLastCheckRecent() {
-		$manager = $this->getMockBuilder('\OC\User\Manager')
-			->disableOriginalConstructor()
-			->getMock();
+		$manager = $this->createMock(Manager::class);
 		$session = $this->createMock(ISession::class);
 		$request = $this->createMock(IRequest::class);
 
@@ -1033,8 +1042,8 @@ class SessionTest extends \Test\TestCase {
 		$mapper = $this->getMockBuilder(DefaultTokenMapper::class)
 			->disableOriginalConstructor()
 			->getMock();
-		$crypto = $this->getMock(ICrypto::class);
-		$logger = $this->getMock(ILogger::class);
+		$crypto = $this->createMock(ICrypto::class);
+		$logger = $this->createMock(ILogger::class);
 		$tokenProvider = new DefaultTokenProvider($mapper, $crypto, $this->config, $logger, $this->timeFactory);
 
 		/** @var \OC\User\Session $userSession */