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

Signed-off-by: Louis Chemineau <louis@chmn.me>
This commit is contained in:
Louis Chemineau 2024-09-20 11:26:22 +02:00 committed by Louis
parent ba41f32e3e
commit 22d5d29c01
8 changed files with 68 additions and 39 deletions

View file

@ -16,6 +16,7 @@ use OCP\EventDispatcher\IEventDispatcher;
use OCP\ICacheFactory;
use OCP\IConfig;
use OCP\IServerContainer;
use Psr\Log\LoggerInterface;
/**
* Class LDAPProviderTest
@ -54,6 +55,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())

View file

@ -36,9 +36,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() {

View file

@ -68,6 +68,7 @@ class Manager extends PublicEmitter implements IUserManager {
private IConfig $config,
ICacheFactory $cacheFactory,
private IEventDispatcher $eventDispatcher,
private LoggerInterface $logger,
) {
$this->cache = new WithLocalCache($cacheFactory->createDistributed('user_backend_map'));
$this->listen('\OC\User', 'postDelete', function (IUser $user): void {
@ -201,7 +202,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;
@ -319,11 +320,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);

View file

@ -62,7 +62,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', '');

View file

@ -137,7 +137,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())
@ -577,7 +578,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,
@ -659,7 +661,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();

View file

@ -13,6 +13,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';
@ -57,7 +58,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())

View file

@ -16,6 +16,7 @@ use OCP\ICache;
use OCP\ICacheFactory;
use OCP\IConfig;
use OCP\IUser;
use Psr\Log\LoggerInterface;
use Test\TestCase;
/**
@ -34,6 +35,8 @@ class ManagerTest extends TestCase {
private $cacheFactory;
/** @var ICache */
private $cache;
/** @var LoggerInterface */
private $logger;
protected function setUp(): void {
parent::setUp();
@ -42,6 +45,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);
@ -49,7 +53,7 @@ class ManagerTest extends TestCase {
public function testGetBackends(): void {
$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);
@ -68,7 +72,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'));
@ -84,14 +88,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(): void {
$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'));
}
@ -115,7 +119,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);
@ -139,7 +143,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);
@ -166,7 +170,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');
@ -185,7 +189,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'));
@ -203,7 +207,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());
@ -219,7 +223,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'));
@ -237,7 +241,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());
@ -255,7 +259,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');
@ -289,7 +293,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);
@ -343,7 +347,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);
@ -370,7 +374,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');
@ -397,7 +401,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');
@ -418,14 +422,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(): void {
$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'));
}
@ -445,7 +449,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);
}
@ -485,7 +489,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);
@ -493,7 +497,7 @@ class ManagerTest extends TestCase {
}
public function testCountUsersNoBackend(): void {
$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));
@ -518,7 +522,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();
@ -559,7 +563,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);
@ -672,7 +676,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);
@ -706,7 +710,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');

View file

@ -181,6 +181,7 @@ class SessionTest extends \Test\TestCase {
$this->config,
$this->createMock(ICacheFactory::class),
$this->createMock(IEventDispatcher::class),
$this->createMock(LoggerInterface::class),
])
->getMock();
@ -247,6 +248,7 @@ class SessionTest extends \Test\TestCase {
$this->config,
$this->createMock(ICacheFactory::class),
$this->createMock(IEventDispatcher::class),
$this->createMock(LoggerInterface::class),
])
->getMock();
@ -280,6 +282,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);
@ -323,6 +326,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);
@ -360,6 +364,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);
@ -617,6 +622,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)
@ -706,6 +712,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)
@ -770,6 +777,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)
@ -822,6 +830,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)