Skip to content

Commit 1c90f7e

Browse files
committed
fix: Correctly filter users when the backend does not implement ISearchKnownUsersBackend
Signed-off-by: Côme Chilliet <come.chilliet@nextcloud.com>
1 parent ba46df3 commit 1c90f7e

3 files changed

Lines changed: 43 additions & 4 deletions

File tree

lib/private/User/Manager.php

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@
99
namespace OC\User;
1010

1111
use OC\Hooks\PublicEmitter;
12+
use OC\KnownUser\KnownUserService;
1213
use OC\Memcache\WithLocalCache;
1314
use OCP\Config\IUserConfig;
1415
use OCP\DB\QueryBuilder\IQueryBuilder;
@@ -83,6 +84,7 @@ public function __construct(
8384
ICacheFactory $cacheFactory,
8485
private IEventDispatcher $eventDispatcher,
8586
private LoggerInterface $logger,
87+
private KnownUserService $knownUserService,
8688
) {
8789
$this->cache = new WithLocalCache($cacheFactory->createDistributed('user_backend_map'));
8890
$this->listen('\OC\User', 'postDelete', function (IUser $user): void {
@@ -374,7 +376,12 @@ public function searchKnownUsersByDisplayName(string $searcher, string $pattern,
374376
$backendUsers = $backend->searchKnownUsersByDisplayName($searcher, $pattern, $limit, $offset);
375377
} else {
376378
// Better than nothing, but filtering after pagination can remove lots of results.
377-
$backendUsers = $backend->getDisplayNames($pattern, $limit, $offset);
379+
$backendUsers = array_filter(
380+
$backend->getDisplayNames($pattern, $limit, $offset),
381+
fn (string $uid): bool => $this->knownUserService->isKnownToUser($searcher, $uid),
382+
ARRAY_FILTER_USE_KEY,
383+
);
384+
378385
}
379386
if (is_array($backendUsers)) {
380387
foreach ($backendUsers as $uid => $displayName) {

tests/lib/User/ManagerTest.php

Lines changed: 25 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@
99
namespace Test\User;
1010

1111
use OC\AllConfig;
12+
use OC\KnownUser\KnownUserService;
1213
use OC\USER\BACKEND;
1314
use OC\User\Database;
1415
use OC\User\Manager;
@@ -34,6 +35,8 @@ class ManagerTest extends TestCase {
3435
private ICacheFactory&MockObject $cacheFactory;
3536
private ICache&MockObject $cache;
3637
private LoggerInterface&MockObject $logger;
38+
private KnownUserService&MockObject $knownUserService;
39+
3740
private IUserManager $manager;
3841

3942
#[\Override]
@@ -45,11 +48,18 @@ protected function setUp(): void {
4548
$this->cacheFactory = $this->createMock(ICacheFactory::class);
4649
$this->cache = $this->createMock(ICache::class);
4750
$this->logger = $this->createMock(LoggerInterface::class);
51+
$this->knownUserService = $this->createMock(KnownUserService::class);
4852

4953
$this->cacheFactory->method('createDistributed')
5054
->willReturn($this->cache);
5155

52-
$this->manager = new Manager($this->config, $this->cacheFactory, $this->eventDispatcher, $this->logger);
56+
$this->manager = new Manager(
57+
$this->config,
58+
$this->cacheFactory,
59+
$this->eventDispatcher,
60+
$this->logger,
61+
$this->knownUserService,
62+
);
5363
}
5464

5565
public function testGetBackends(): void {
@@ -666,7 +676,13 @@ public function testDeleteUser(): void {
666676
->method('getAppValue')
667677
->willReturnArgument(2);
668678

669-
$this->manager = new Manager($config, $this->cacheFactory, $this->eventDispatcher, $this->logger);
679+
$this->manager = new Manager(
680+
$config,
681+
$this->cacheFactory,
682+
$this->eventDispatcher,
683+
$this->logger,
684+
$this->knownUserService,
685+
);
670686
$backend = new \Test\Util\User\Dummy();
671687

672688
$this->manager->registerBackend($backend);
@@ -688,7 +704,13 @@ public function testGetByEmail(): void {
688704
});
689705

690706
$this->manager = $this->getMockBuilder(Manager::class)
691-
->setConstructorArgs([$this->config, $this->cacheFactory, $this->eventDispatcher, $this->logger])
707+
->setConstructorArgs([
708+
$this->config,
709+
$this->cacheFactory,
710+
$this->eventDispatcher,
711+
$this->logger,
712+
$this->knownUserService,
713+
])
692714
->onlyMethods(['getUserConfig', 'get'])
693715
->getMock();
694716
$this->manager->method('getUserConfig')->willReturn($userConfig);

tests/lib/User/SessionTest.php

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@
1616
use OC\Authentication\Token\IProvider;
1717
use OC\Authentication\Token\IToken;
1818
use OC\Authentication\Token\PublicKeyToken;
19+
use OC\KnownUser\KnownUserService;
1920
use OC\Security\CSRF\CsrfTokenManager;
2021
use OC\Session\Memory;
2122
use OC\User\LoginException;
@@ -183,6 +184,7 @@ public function testLoginValidPasswordEnabled(): void {
183184
$this->createMock(ICacheFactory::class),
184185
$this->createMock(IEventDispatcher::class),
185186
$this->createMock(LoggerInterface::class),
187+
$this->createMock(KnownUserService::class),
186188
])
187189
->getMock();
188190

@@ -249,6 +251,7 @@ public function testLoginValidPasswordDisabled(): void {
249251
$this->createMock(ICacheFactory::class),
250252
$this->createMock(IEventDispatcher::class),
251253
$this->createMock(LoggerInterface::class),
254+
$this->createMock(KnownUserService::class),
252255
])
253256
->getMock();
254257

@@ -283,6 +286,7 @@ public function testLoginInvalidPassword(): void {
283286
$this->createMock(ICacheFactory::class),
284287
$this->createMock(IEventDispatcher::class),
285288
$this->createMock(LoggerInterface::class),
289+
$this->createMock(KnownUserService::class),
286290
])
287291
->getMock();
288292
$backend = $this->createMock(\Test\Util\User\Dummy::class);
@@ -327,6 +331,7 @@ public function testPasswordlessLoginNoLastCheckUpdate(): void {
327331
$this->createMock(ICacheFactory::class),
328332
$this->createMock(IEventDispatcher::class),
329333
$this->createMock(LoggerInterface::class),
334+
$this->createMock(KnownUserService::class),
330335
])
331336
->getMock();
332337
$userSession = new Session($manager, $session, $this->timeFactory, $this->tokenProvider, $this->config, $this->random, $this->lockdownManager, $this->logger, $this->dispatcher);
@@ -371,6 +376,7 @@ public function testLoginLastCheckUpdate(): void {
371376
$this->createMock(ICacheFactory::class),
372377
$this->createMock(IEventDispatcher::class),
373378
$this->createMock(LoggerInterface::class),
379+
$this->createMock(KnownUserService::class),
374380
])
375381
->getMock();
376382
$userSession = new Session($manager, $session, $this->timeFactory, $this->tokenProvider, $this->config, $this->random, $this->lockdownManager, $this->logger, $this->dispatcher);
@@ -745,6 +751,7 @@ public function testRememberLoginValidToken(): void {
745751
$this->createMock(ICacheFactory::class),
746752
$this->createMock(IEventDispatcher::class),
747753
$this->createMock(LoggerInterface::class),
754+
$this->createMock(KnownUserService::class),
748755
])
749756
->getMock();
750757
$userSession = $this->getMockBuilder(Session::class)
@@ -835,6 +842,7 @@ public function testRememberLoginInvalidSessionToken(): void {
835842
$this->createMock(ICacheFactory::class),
836843
$this->createMock(IEventDispatcher::class),
837844
$this->createMock(LoggerInterface::class),
845+
$this->createMock(KnownUserService::class),
838846
])
839847
->getMock();
840848
$userSession = $this->getMockBuilder(Session::class)
@@ -900,6 +908,7 @@ public function testRememberLoginInvalidToken(): void {
900908
$this->createMock(ICacheFactory::class),
901909
$this->createMock(IEventDispatcher::class),
902910
$this->createMock(LoggerInterface::class),
911+
$this->createMock(KnownUserService::class),
903912
])
904913
->getMock();
905914
$userSession = $this->getMockBuilder(Session::class)
@@ -953,6 +962,7 @@ public function testRememberLoginInvalidUser(): void {
953962
$this->createMock(ICacheFactory::class),
954963
$this->createMock(IEventDispatcher::class),
955964
$this->createMock(LoggerInterface::class),
965+
$this->createMock(KnownUserService::class),
956966
])
957967
->getMock();
958968
$userSession = $this->getMockBuilder(Session::class)

0 commit comments

Comments
 (0)