fix(users): Don't crash if disabled user is missing in the database

Signed-off-by: Louis Chemineau <louis@chmn.me>

diff --git a/apps/user_ldap/tests/LDAPProviderTest.php b/apps/user_ldap/tests/LDAPProviderTest.php
index 3e34ce7bdbb1408ea6cbb38c92a66c515cf7895f..3c80eb2f41f547899acf50534c0c39e426d75b20 100644 (file)
--- a/apps/user_ldap/tests/LDAPProviderTest.php
+++ b/apps/user_ldap/tests/LDAPProviderTest.php
@@ -39,6 +39,7 @@ use OCP\EventDispatcher\IEventDispatcher;
 use OCP\ICacheFactory;
 use OCP\IConfig;
 use OCP\IServerContainer;
+use Psr\Log\LoggerInterface;
 
 /**
  * Class LDAPProviderTest
@@ -77,6 +78,7 @@ class LDAPProviderTest extends \Test\TestCase {
                                $this->createMock(IConfig::class),
                                $this->createMock(ICacheFactory::class),
                                $this->createMock(IEventDispatcher::class),
+                               $this->createMock(LoggerInterface::class),
                        ])
                        ->getMock();
                $userManager->expects($this->any())

diff --git a/lib/private/User/LazyUser.php b/lib/private/User/LazyUser.php
index 396d3c252f11d4cc21e75656aeaa87985763aafa..704075c1b9287ca241617329acf5f476c3052e6c 100644 (file)
--- a/lib/private/User/LazyUser.php
+++ b/lib/private/User/LazyUser.php
@@ -51,9 +51,12 @@ class LazyUser implements IUser {
                                $this->user = $this->userManager->get($this->uid);
                        }
                }
-               /** @var IUser */
-               $user = $this->user;
-               return $user;
+
+               if ($this->user === null) {
+                       throw new NoUserException('User not found in backend');
+               }
+
+               return $this->user;
        }
 
        public function getUID() {

diff --git a/lib/private/User/Manager.php b/lib/private/User/Manager.php
index f0d5c7c1e4a96aef437dda9c34301b5316242501..7067afc273b9073a2b434ef261ae0cb6fd678a7e 100644 (file)
--- a/lib/private/User/Manager.php
+++ b/lib/private/User/Manager.php
@@ -99,7 +99,8 @@ class Manager extends PublicEmitter implements IUserManager {
 
        public function __construct(IConfig $config,
                ICacheFactory $cacheFactory,
-               IEventDispatcher $eventDispatcher) {
+               IEventDispatcher $eventDispatcher,
+               private LoggerInterface $logger) {
                $this->config = $config;
                $this->cache = new WithLocalCache($cacheFactory->createDistributed('user_backend_map'));
                $cachedUsers = &$this->cachedUsers;
@@ -236,7 +237,7 @@ class Manager extends PublicEmitter implements IUserManager {
                $result = $this->checkPasswordNoLogging($loginName, $password);
 
                if ($result === false) {
-                       \OCP\Server::get(LoggerInterface::class)->warning('Login failed: \''. $loginName .'\' (Remote IP: \''. \OC::$server->getRequest()->getRemoteAddress(). '\')', ['app' => 'core']);
+                       $this->logger->warning('Login failed: \''. $loginName .'\' (Remote IP: \''. \OC::$server->getRequest()->getRemoteAddress(). '\')', ['app' => 'core']);
                }
 
                return $result;
@@ -354,11 +355,16 @@ class Manager extends PublicEmitter implements IUserManager {
                if ($search !== '') {
                        $users = array_filter(
                                $users,
-                               fn (IUser $user): bool =>
-                                       mb_stripos($user->getUID(), $search) !== false ||
-                                       mb_stripos($user->getDisplayName(), $search) !== false ||
-                                       mb_stripos($user->getEMailAddress() ?? '', $search) !== false,
-                       );
+                               function (IUser $user) use ($search): bool {
+                                       try {
+                                               return mb_stripos($user->getUID(), $search) !== false ||
+                                               mb_stripos($user->getDisplayName(), $search) !== false ||
+                                               mb_stripos($user->getEMailAddress() ?? '', $search) !== false;
+                                       } catch (NoUserException $ex) {
+                                               $this->logger->error('Error while filtering disabled users', ['exception' => $ex, 'userUID' => $user->getUID()]);
+                                               return false;
+                                       }
+                               });
                }
 
                $tempLimit = ($limit === null ? null : $limit + $offset);

diff --git a/tests/lib/Files/Config/UserMountCacheTest.php b/tests/lib/Files/Config/UserMountCacheTest.php
index 9e910f4f47f5b986409e61184b048e956ebb74d8..9aebe26178fac733b317d9f19a2485b5aeff9450 100644 (file)
--- a/tests/lib/Files/Config/UserMountCacheTest.php
+++ b/tests/lib/Files/Config/UserMountCacheTest.php
@@ -61,7 +61,7 @@ class UserMountCacheTest extends TestCase {
                        ->expects($this->any())
                        ->method('getAppValue')
                        ->willReturnArgument(2);
-               $this->userManager = new Manager($config, $this->createMock(ICacheFactory::class), $this->createMock(IEventDispatcher::class));
+               $this->userManager = new Manager($config, $this->createMock(ICacheFactory::class), $this->createMock(IEventDispatcher::class), $this->createMock(LoggerInterface::class));
                $userBackend = new Dummy();
                $userBackend->createUser('u1', '');
                $userBackend->createUser('u2', '');

diff --git a/tests/lib/Files/Storage/Wrapper/EncryptionTest.php b/tests/lib/Files/Storage/Wrapper/EncryptionTest.php
index be5f191c6eb1480116a63d5907e123eb4b284a7a..9888d837f171dd33a0b0b4b6f4bd8dbb9398f662 100644 (file)
--- a/tests/lib/Files/Storage/Wrapper/EncryptionTest.php
+++ b/tests/lib/Files/Storage/Wrapper/EncryptionTest.php
@@ -133,7 +133,8 @@ class EncryptionTest extends Storage {
                        ->setConstructorArgs([new View(), new Manager(
                                $this->config,
                                $this->createMock(ICacheFactory::class),
-                               $this->createMock(IEventDispatcher::class)
+                               $this->createMock(IEventDispatcher::class),
+                               $this->createMock(LoggerInterface::class),
                        ), $this->groupManager, $this->config, $this->arrayCache])
                        ->getMock();
                $this->util->expects($this->any())
@@ -573,7 +574,8 @@ class EncryptionTest extends Storage {
                                        new Manager(
                                                $this->config,
                                                $this->createMock(ICacheFactory::class),
-                                               $this->createMock(IEventDispatcher::class)
+                                               $this->createMock(IEventDispatcher::class),
+                                               $this->createMock(LoggerInterface::class),
                                        ),
                                        $this->groupManager,
                                        $this->config,
@@ -655,7 +657,8 @@ class EncryptionTest extends Storage {
                        ->setConstructorArgs([new View(), new Manager(
                                $this->config,
                                $this->createMock(ICacheFactory::class),
-                               $this->createMock(IEventDispatcher::class)
+                               $this->createMock(IEventDispatcher::class),
+                               $this->createMock(LoggerInterface::class),
                        ), $this->groupManager, $this->config, $this->arrayCache])
                        ->getMock();
 

diff --git a/tests/lib/Files/Stream/EncryptionTest.php b/tests/lib/Files/Stream/EncryptionTest.php
index 3d45ff2fc3cedd6a14a7284c5cd57a73be48039e..7a2e1534c9ba560abab565a7c486156d814e7fcb 100644 (file)
--- a/tests/lib/Files/Stream/EncryptionTest.php
+++ b/tests/lib/Files/Stream/EncryptionTest.php
@@ -9,6 +9,7 @@ use OCP\EventDispatcher\IEventDispatcher;
 use OCP\Files\Cache\ICache;
 use OCP\ICacheFactory;
 use OCP\IConfig;
+use Psr\Log\LoggerInterface;
 
 class EncryptionTest extends \Test\TestCase {
        public const DEFAULT_WRAPPER = '\OC\Files\Stream\Encryption';
@@ -53,7 +54,8 @@ class EncryptionTest extends \Test\TestCase {
                        ->setConstructorArgs([new View(), new \OC\User\Manager(
                                $config,
                                $this->createMock(ICacheFactory::class),
-                               $this->createMock(IEventDispatcher::class)
+                               $this->createMock(IEventDispatcher::class),
+                               $this->createMock(LoggerInterface::class),
                        ), $groupManager, $config, $arrayCache])
                        ->getMock();
                $util->expects($this->any())

diff --git a/tests/lib/User/ManagerTest.php b/tests/lib/User/ManagerTest.php
index 5e8c2ed713176baeac6772e3ab137ce847b227b1..ce408894ec50b56d5307d3dddd0c26d2bbf964f7 100644 (file)
--- a/tests/lib/User/ManagerTest.php
+++ b/tests/lib/User/ManagerTest.php
@@ -17,6 +17,7 @@ use OCP\ICache;
 use OCP\ICacheFactory;
 use OCP\IConfig;
 use OCP\IUser;
+use Psr\Log\LoggerInterface;
 use Test\TestCase;
 
 /**
@@ -35,6 +36,8 @@ class ManagerTest extends TestCase {
        private $cacheFactory;
        /** @var ICache */
        private $cache;
+       /** @var LoggerInterface */
+       private $logger;
 
        protected function setUp(): void {
                parent::setUp();
@@ -43,6 +46,7 @@ class ManagerTest extends TestCase {
                $this->eventDispatcher = $this->createMock(IEventDispatcher::class);
                $this->cacheFactory = $this->createMock(ICacheFactory::class);
                $this->cache = $this->createMock(ICache::class);
+               $this->logger = $this->createMock(LoggerInterface::class);
 
                $this->cacheFactory->method('createDistributed')
                        ->willReturn($this->cache);
@@ -50,7 +54,7 @@ class ManagerTest extends TestCase {
 
        public function testGetBackends() {
                $userDummyBackend = $this->createMock(\Test\Util\User\Dummy::class);
-               $manager = new \OC\User\Manager($this->config, $this->cacheFactory, $this->eventDispatcher);
+               $manager = new \OC\User\Manager($this->config, $this->cacheFactory, $this->eventDispatcher, $this->logger);
                $manager->registerBackend($userDummyBackend);
                $this->assertEquals([$userDummyBackend], $manager->getBackends());
                $dummyDatabaseBackend = $this->createMock(Database::class);
@@ -69,7 +73,7 @@ class ManagerTest extends TestCase {
                        ->with($this->equalTo('foo'))
                        ->willReturn(true);
 
-               $manager = new \OC\User\Manager($this->config, $this->cacheFactory, $this->eventDispatcher);
+               $manager = new \OC\User\Manager($this->config, $this->cacheFactory, $this->eventDispatcher, $this->logger);
                $manager->registerBackend($backend);
 
                $this->assertTrue($manager->userExists('foo'));
@@ -85,14 +89,14 @@ class ManagerTest extends TestCase {
                        ->with($this->equalTo('foo'))
                        ->willReturn(false);
 
-               $manager = new \OC\User\Manager($this->config, $this->cacheFactory, $this->eventDispatcher);
+               $manager = new \OC\User\Manager($this->config, $this->cacheFactory, $this->eventDispatcher, $this->logger);
                $manager->registerBackend($backend);
 
                $this->assertFalse($manager->userExists('foo'));
        }
 
        public function testUserExistsNoBackends() {
-               $manager = new \OC\User\Manager($this->config, $this->cacheFactory, $this->eventDispatcher);
+               $manager = new \OC\User\Manager($this->config, $this->cacheFactory, $this->eventDispatcher, $this->logger);
 
                $this->assertFalse($manager->userExists('foo'));
        }
@@ -116,7 +120,7 @@ class ManagerTest extends TestCase {
                        ->with($this->equalTo('foo'))
                        ->willReturn(true);
 
-               $manager = new \OC\User\Manager($this->config, $this->cacheFactory, $this->eventDispatcher);
+               $manager = new \OC\User\Manager($this->config, $this->cacheFactory, $this->eventDispatcher, $this->logger);
                $manager->registerBackend($backend1);
                $manager->registerBackend($backend2);
 
@@ -140,7 +144,7 @@ class ManagerTest extends TestCase {
                $backend2->expects($this->never())
                        ->method('userExists');
 
-               $manager = new \OC\User\Manager($this->config, $this->cacheFactory, $this->eventDispatcher);
+               $manager = new \OC\User\Manager($this->config, $this->cacheFactory, $this->eventDispatcher, $this->logger);
                $manager->registerBackend($backend1);
                $manager->registerBackend($backend2);
 
@@ -167,7 +171,7 @@ class ManagerTest extends TestCase {
                                }
                        });
 
-               $manager = new \OC\User\Manager($this->config, $this->cacheFactory, $this->eventDispatcher);
+               $manager = new \OC\User\Manager($this->config, $this->cacheFactory, $this->eventDispatcher, $this->logger);
                $manager->registerBackend($backend);
 
                $user = $manager->checkPassword('foo', 'bar');
@@ -186,7 +190,7 @@ class ManagerTest extends TestCase {
                        ->method('implementsActions')
                        ->willReturn(false);
 
-               $manager = new \OC\User\Manager($this->config, $this->cacheFactory, $this->eventDispatcher);
+               $manager = new \OC\User\Manager($this->config, $this->cacheFactory, $this->eventDispatcher, $this->logger);
                $manager->registerBackend($backend);
 
                $this->assertFalse($manager->checkPassword('foo', 'bar'));
@@ -204,7 +208,7 @@ class ManagerTest extends TestCase {
                $backend->expects($this->never())
                        ->method('loginName2UserName');
 
-               $manager = new \OC\User\Manager($this->config, $this->cacheFactory, $this->eventDispatcher);
+               $manager = new \OC\User\Manager($this->config, $this->cacheFactory, $this->eventDispatcher, $this->logger);
                $manager->registerBackend($backend);
 
                $this->assertEquals('foo', $manager->get('foo')->getUID());
@@ -220,7 +224,7 @@ class ManagerTest extends TestCase {
                        ->with($this->equalTo('foo'))
                        ->willReturn(false);
 
-               $manager = new \OC\User\Manager($this->config, $this->cacheFactory, $this->eventDispatcher);
+               $manager = new \OC\User\Manager($this->config, $this->cacheFactory, $this->eventDispatcher, $this->logger);
                $manager->registerBackend($backend);
 
                $this->assertEquals(null, $manager->get('foo'));
@@ -238,7 +242,7 @@ class ManagerTest extends TestCase {
                $backend->expects($this->never())
                        ->method('loginName2UserName');
 
-               $manager = new \OC\User\Manager($this->config, $this->cacheFactory, $this->eventDispatcher);
+               $manager = new \OC\User\Manager($this->config, $this->cacheFactory, $this->eventDispatcher, $this->logger);
                $manager->registerBackend($backend);
 
                $this->assertEquals('bLeNdEr', $manager->get('bLeNdEr')->getUID());
@@ -256,7 +260,7 @@ class ManagerTest extends TestCase {
                $backend->expects($this->never())
                        ->method('loginName2UserName');
 
-               $manager = new \OC\User\Manager($this->config, $this->cacheFactory, $this->eventDispatcher);
+               $manager = new \OC\User\Manager($this->config, $this->cacheFactory, $this->eventDispatcher, $this->logger);
                $manager->registerBackend($backend);
 
                $result = $manager->search('fo');
@@ -290,7 +294,7 @@ class ManagerTest extends TestCase {
                $backend2->expects($this->never())
                        ->method('loginName2UserName');
 
-               $manager = new \OC\User\Manager($this->config, $this->cacheFactory, $this->eventDispatcher);
+               $manager = new \OC\User\Manager($this->config, $this->cacheFactory, $this->eventDispatcher, $this->logger);
                $manager->registerBackend($backend1);
                $manager->registerBackend($backend2);
 
@@ -344,7 +348,7 @@ class ManagerTest extends TestCase {
                        ->willReturn(true);
 
 
-               $manager = new \OC\User\Manager($this->config, $this->cacheFactory, $this->eventDispatcher);
+               $manager = new \OC\User\Manager($this->config, $this->cacheFactory, $this->eventDispatcher, $this->logger);
                $manager->registerBackend($backend);
 
                $this->expectException(\InvalidArgumentException::class, $exception);
@@ -371,7 +375,7 @@ class ManagerTest extends TestCase {
                $backend->expects($this->never())
                        ->method('loginName2UserName');
 
-               $manager = new \OC\User\Manager($this->config, $this->cacheFactory, $this->eventDispatcher);
+               $manager = new \OC\User\Manager($this->config, $this->cacheFactory, $this->eventDispatcher, $this->logger);
                $manager->registerBackend($backend);
 
                $user = $manager->createUser('foo', 'bar');
@@ -398,7 +402,7 @@ class ManagerTest extends TestCase {
                        ->with($this->equalTo('foo'))
                        ->willReturn(true);
 
-               $manager = new \OC\User\Manager($this->config, $this->cacheFactory, $this->eventDispatcher);
+               $manager = new \OC\User\Manager($this->config, $this->cacheFactory, $this->eventDispatcher, $this->logger);
                $manager->registerBackend($backend);
 
                $manager->createUser('foo', 'bar');
@@ -419,14 +423,14 @@ class ManagerTest extends TestCase {
                $backend->expects($this->never())
                        ->method('userExists');
 
-               $manager = new \OC\User\Manager($this->config, $this->cacheFactory, $this->eventDispatcher);
+               $manager = new \OC\User\Manager($this->config, $this->cacheFactory, $this->eventDispatcher, $this->logger);
                $manager->registerBackend($backend);
 
                $this->assertFalse($manager->createUser('foo', 'bar'));
        }
 
        public function testCreateUserNoBackends() {
-               $manager = new \OC\User\Manager($this->config, $this->cacheFactory, $this->eventDispatcher);
+               $manager = new \OC\User\Manager($this->config, $this->cacheFactory, $this->eventDispatcher, $this->logger);
 
                $this->assertFalse($manager->createUser('foo', 'bar'));
        }
@@ -446,7 +450,7 @@ class ManagerTest extends TestCase {
                        ->with('MyUid', 'MyPassword')
                        ->willReturn(false);
 
-               $manager = new Manager($this->config, $this->cacheFactory, $this->eventDispatcher);
+               $manager = new Manager($this->config, $this->cacheFactory, $this->eventDispatcher, $this->logger);
                $manager->createUserFromBackend('MyUid', 'MyPassword', $backend);
        }
 
@@ -486,7 +490,7 @@ class ManagerTest extends TestCase {
                        ->with($this->equalTo('foo'))
                        ->willReturn(true);
 
-               $manager = new \OC\User\Manager($this->config, $this->cacheFactory, $this->eventDispatcher);
+               $manager = new \OC\User\Manager($this->config, $this->cacheFactory, $this->eventDispatcher, $this->logger);
                $manager->registerBackend($backend1);
                $manager->registerBackend($backend2);
 
@@ -494,7 +498,7 @@ class ManagerTest extends TestCase {
        }
 
        public function testCountUsersNoBackend() {
-               $manager = new \OC\User\Manager($this->config, $this->cacheFactory, $this->eventDispatcher);
+               $manager = new \OC\User\Manager($this->config, $this->cacheFactory, $this->eventDispatcher, $this->logger);
 
                $result = $manager->countUsers();
                $this->assertTrue(is_array($result));
@@ -519,7 +523,7 @@ class ManagerTest extends TestCase {
                        ->method('getBackendName')
                        ->willReturn('Mock_Test_Util_User_Dummy');
 
-               $manager = new \OC\User\Manager($this->config, $this->cacheFactory, $this->eventDispatcher);
+               $manager = new \OC\User\Manager($this->config, $this->cacheFactory, $this->eventDispatcher, $this->logger);
                $manager->registerBackend($backend);
 
                $result = $manager->countUsers();
@@ -560,7 +564,7 @@ class ManagerTest extends TestCase {
                        ->method('getBackendName')
                        ->willReturn('Mock_Test_Util_User_Dummy');
 
-               $manager = new \OC\User\Manager($this->config, $this->cacheFactory, $this->eventDispatcher);
+               $manager = new \OC\User\Manager($this->config, $this->cacheFactory, $this->eventDispatcher, $this->logger);
                $manager->registerBackend($backend1);
                $manager->registerBackend($backend2);
 
@@ -673,7 +677,7 @@ class ManagerTest extends TestCase {
                        ->method('getAppValue')
                        ->willReturnArgument(2);
 
-               $manager = new \OC\User\Manager($config, $this->cacheFactory, $this->eventDispatcher);
+               $manager = new \OC\User\Manager($config, $this->cacheFactory, $this->eventDispatcher, $this->logger);
                $backend = new \Test\Util\User\Dummy();
 
                $manager->registerBackend($backend);
@@ -707,7 +711,7 @@ class ManagerTest extends TestCase {
                                true
                        );
 
-               $manager = new \OC\User\Manager($config, $this->cacheFactory, $this->eventDispatcher);
+               $manager = new \OC\User\Manager($config, $this->cacheFactory, $this->eventDispatcher, $this->logger);
                $manager->registerBackend($backend);
 
                $users = $manager->getByEmail('test@example.com');

diff --git a/tests/lib/User/SessionTest.php b/tests/lib/User/SessionTest.php
index ad4eaccd9de5bb002db89eb29907833b498f9e5b..5b5afbe618a09397b6acc813f67a796569a3106f 100644 (file)
--- a/tests/lib/User/SessionTest.php
+++ b/tests/lib/User/SessionTest.php
@@ -182,6 +182,7 @@ class SessionTest extends \Test\TestCase {
                                $this->config,
                                $this->createMock(ICacheFactory::class),
                                $this->createMock(IEventDispatcher::class),
+                               $this->createMock(LoggerInterface::class),
                        ])
                        ->getMock();
 
@@ -248,6 +249,7 @@ class SessionTest extends \Test\TestCase {
                                $this->config,
                                $this->createMock(ICacheFactory::class),
                                $this->createMock(IEventDispatcher::class),
+                               $this->createMock(LoggerInterface::class),
                        ])
                        ->getMock();
 
@@ -281,6 +283,7 @@ class SessionTest extends \Test\TestCase {
                                $this->config,
                                $this->createMock(ICacheFactory::class),
                                $this->createMock(IEventDispatcher::class),
+                               $this->createMock(LoggerInterface::class),
                        ])
                        ->getMock();
                $backend = $this->createMock(\Test\Util\User\Dummy::class);
@@ -324,6 +327,7 @@ class SessionTest extends \Test\TestCase {
                                $this->config,
                                $this->createMock(ICacheFactory::class),
                                $this->createMock(IEventDispatcher::class),
+                               $this->createMock(LoggerInterface::class),
                        ])
                        ->getMock();
                $userSession = new Session($manager, $session, $this->timeFactory, $this->tokenProvider, $this->config, $this->random, $this->lockdownManager, $this->logger, $this->dispatcher);
@@ -361,6 +365,7 @@ class SessionTest extends \Test\TestCase {
                                $this->config,
                                $this->createMock(ICacheFactory::class),
                                $this->createMock(IEventDispatcher::class),
+                               $this->createMock(LoggerInterface::class),
                        ])
                        ->getMock();
                $userSession = new Session($manager, $session, $this->timeFactory, $this->tokenProvider, $this->config, $this->random, $this->lockdownManager, $this->logger, $this->dispatcher);
@@ -618,6 +623,7 @@ class SessionTest extends \Test\TestCase {
                                $this->config,
                                $this->createMock(ICacheFactory::class),
                                $this->createMock(IEventDispatcher::class),
+                               $this->createMock(LoggerInterface::class),
                        ])
                        ->getMock();
                $userSession = $this->getMockBuilder(Session::class)
@@ -707,6 +713,7 @@ class SessionTest extends \Test\TestCase {
                                $this->config,
                                $this->createMock(ICacheFactory::class),
                                $this->createMock(IEventDispatcher::class),
+                               $this->createMock(LoggerInterface::class),
                        ])
                        ->getMock();
                $userSession = $this->getMockBuilder(Session::class)
@@ -771,6 +778,7 @@ class SessionTest extends \Test\TestCase {
                                $this->config,
                                $this->createMock(ICacheFactory::class),
                                $this->createMock(IEventDispatcher::class),
+                               $this->createMock(LoggerInterface::class),
                        ])
                        ->getMock();
                $userSession = $this->getMockBuilder(Session::class)
@@ -823,6 +831,7 @@ class SessionTest extends \Test\TestCase {
                                $this->config,
                                $this->createMock(ICacheFactory::class),
                                $this->createMock(IEventDispatcher::class),
+                               $this->createMock(LoggerInterface::class),
                        ])
                        ->getMock();
                $userSession = $this->getMockBuilder(Session::class)