Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[stable28] fix: Don't crash if disabled user is missing in the database #48283

Merged
merged 1 commit into from
Sep 24, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions apps/user_ldap/tests/LDAPProviderTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@
use OCP\ICacheFactory;
use OCP\IConfig;
use OCP\IServerContainer;
use Psr\Log\LoggerInterface;

/**
* Class LDAPProviderTest
Expand Down Expand Up @@ -77,6 +78,7 @@ private function getUserManagerMock(IUserLDAP $userBackend) {
$this->createMock(IConfig::class),
$this->createMock(ICacheFactory::class),
$this->createMock(IEventDispatcher::class),
$this->createMock(LoggerInterface::class),
])
->getMock();
$userManager->expects($this->any())
Expand Down
9 changes: 6 additions & 3 deletions lib/private/User/LazyUser.php
Original file line number Diff line number Diff line change
Expand Up @@ -51,9 +51,12 @@ private function getUser(): 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() {
Expand Down
19 changes: 13 additions & 6 deletions lib/private/User/Manager.php
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,7 @@
use OCP\User\Events\BeforeUserCreatedEvent;
use OCP\User\Events\UserCreatedEvent;
use OCP\UserInterface;
use Psr\Log\LoggerInterface;

/**
* Class Manager
Expand Down Expand Up @@ -98,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;
Expand Down Expand Up @@ -353,11 +355,16 @@ public function getDisabledUsers(?int $limit = null, int $offset = 0, string $se
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);
Expand Down
2 changes: 1 addition & 1 deletion tests/lib/Files/Config/UserMountCacheTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ protected function setUp(): void {
->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', '');
Expand Down
9 changes: 6 additions & 3 deletions tests/lib/Files/Storage/Wrapper/EncryptionTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -133,7 +133,8 @@ protected function setUp(): void {
->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())
Expand Down Expand Up @@ -573,7 +574,8 @@ public function testGetHeader($path, $strippedPathExists, $strippedPath) {
new Manager(
$this->config,
$this->createMock(ICacheFactory::class),
$this->createMock(IEventDispatcher::class)
$this->createMock(IEventDispatcher::class),
$this->createMock(LoggerInterface::class),
),
$this->groupManager,
$this->config,
Expand Down Expand Up @@ -655,7 +657,8 @@ public function testGetHeaderAddLegacyModule($header, $isEncrypted, $exists, $ex
->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();

Expand Down
4 changes: 3 additions & 1 deletion tests/lib/Files/Stream/EncryptionTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
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';
Expand Down Expand Up @@ -53,7 +54,8 @@ protected function getStream($fileName, $mode, $unencryptedSize, $wrapper = self
->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())
Expand Down
48 changes: 26 additions & 22 deletions tests/lib/User/ManagerTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
use OCP\ICacheFactory;
use OCP\IConfig;
use OCP\IUser;
use Psr\Log\LoggerInterface;
use Test\TestCase;

/**
Expand All @@ -35,6 +36,8 @@ class ManagerTest extends TestCase {
private $cacheFactory;
/** @var ICache */
private $cache;
/** @var LoggerInterface */
private $logger;

protected function setUp(): void {
parent::setUp();
Expand All @@ -43,14 +46,15 @@ protected function setUp(): void {
$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);
}

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);
Expand All @@ -69,7 +73,7 @@ public function testUserExistsSingleBackendExists() {
->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'));
Expand All @@ -85,7 +89,7 @@ public function testUserExistsSingleBackendNotExists() {
->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'));
Expand Down Expand Up @@ -116,7 +120,7 @@ public function testUserExistsTwoBackendsSecondExists() {
->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);

Expand All @@ -140,7 +144,7 @@ public function testUserExistsTwoBackendsFirstExists() {
$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);

Expand All @@ -167,7 +171,7 @@ public function testCheckPassword() {
}
});

$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');
Expand All @@ -186,7 +190,7 @@ public function testCheckPasswordNotSupported() {
->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'));
Expand All @@ -204,7 +208,7 @@ public function testGetOneBackendExists() {
$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());
Expand All @@ -220,7 +224,7 @@ public function testGetOneBackendNotExists() {
->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'));
Expand All @@ -238,7 +242,7 @@ public function testGetOneBackendDoNotTranslateLoginNames() {
$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());
Expand All @@ -256,7 +260,7 @@ public function testSearchOneBackend() {
$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');
Expand Down Expand Up @@ -290,7 +294,7 @@ public function testSearchTwoBackendLimitOffset() {
$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);

Expand Down Expand Up @@ -344,7 +348,7 @@ public function testCreateUserInvalid($uid, $password, $exception) {
->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);
Expand All @@ -371,7 +375,7 @@ public function testCreateUserSingleBackendNotExists() {
$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');
Expand All @@ -398,7 +402,7 @@ public function testCreateUserSingleBackendExists() {
->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');
Expand All @@ -419,7 +423,7 @@ public function testCreateUserSingleBackendNotSupported() {
$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'));
Expand All @@ -446,7 +450,7 @@ public function testCreateUserFromBackendWithBackendError() {
->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);
}

Expand Down Expand Up @@ -486,7 +490,7 @@ public function testCreateUserTwoBackendExists() {
->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);

Expand Down Expand Up @@ -519,7 +523,7 @@ public function testCountUsersOneBackend() {
->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();
Expand Down Expand Up @@ -560,7 +564,7 @@ public function testCountUsersTwoBackends() {
->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);

Expand Down Expand Up @@ -673,7 +677,7 @@ public function testDeleteUser() {
->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);
Expand Down Expand Up @@ -707,7 +711,7 @@ public function testGetByEmail() {
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');
Expand Down
Loading
Loading