From 553a25e1a1e3535d8b5ffddd96207fe63bf76e82 Mon Sep 17 00:00:00 2001 From: Joas Schilling Date: Wed, 17 Mar 2021 09:02:37 +0100 Subject: [PATCH 1/5] Improve search results when only phonebook-matches can we autocompleted Signed-off-by: Joas Schilling --- lib/composer/composer/autoload_classmap.php | 1 + lib/composer/composer/autoload_static.php | 1 + .../Collaborators/UserPlugin.php | 6 +- lib/private/User/Database.php | 44 ++++++++++++++- lib/private/User/Manager.php | 36 ++++++++++++ lib/public/IUserManager.php | 12 ++++ .../User/Backend/IGetRealUIDBackend.php | 2 +- .../User/Backend/ISearchKnownUsersBackend.php | 43 +++++++++++++++ .../Collaborators/UserPluginTest.php | 55 ++++++++++++++++--- 9 files changed, 189 insertions(+), 11 deletions(-) create mode 100644 lib/public/User/Backend/ISearchKnownUsersBackend.php diff --git a/lib/composer/composer/autoload_classmap.php b/lib/composer/composer/autoload_classmap.php index 756414a6c4..fd3dc44f26 100644 --- a/lib/composer/composer/autoload_classmap.php +++ b/lib/composer/composer/autoload_classmap.php @@ -533,6 +533,7 @@ return array( 'OCP\\User\\Backend\\IGetRealUIDBackend' => $baseDir . '/lib/public/User/Backend/IGetRealUIDBackend.php', 'OCP\\User\\Backend\\IPasswordConfirmationBackend' => $baseDir . '/lib/public/User/Backend/IPasswordConfirmationBackend.php', 'OCP\\User\\Backend\\IProvideAvatarBackend' => $baseDir . '/lib/public/User/Backend/IProvideAvatarBackend.php', + 'OCP\\User\\Backend\\ISearchKnownUsersBackend' => $baseDir . '/lib/public/User/Backend/ISearchKnownUsersBackend.php', 'OCP\\User\\Backend\\ISetDisplayNameBackend' => $baseDir . '/lib/public/User/Backend/ISetDisplayNameBackend.php', 'OCP\\User\\Backend\\ISetPasswordBackend' => $baseDir . '/lib/public/User/Backend/ISetPasswordBackend.php', 'OCP\\User\\Events\\BeforePasswordUpdatedEvent' => $baseDir . '/lib/public/User/Events/BeforePasswordUpdatedEvent.php', diff --git a/lib/composer/composer/autoload_static.php b/lib/composer/composer/autoload_static.php index 7c7243776f..797c4b325d 100644 --- a/lib/composer/composer/autoload_static.php +++ b/lib/composer/composer/autoload_static.php @@ -562,6 +562,7 @@ class ComposerStaticInit53792487c5a8370acc0b06b1a864ff4c 'OCP\\User\\Backend\\IGetRealUIDBackend' => __DIR__ . '/../../..' . '/lib/public/User/Backend/IGetRealUIDBackend.php', 'OCP\\User\\Backend\\IPasswordConfirmationBackend' => __DIR__ . '/../../..' . '/lib/public/User/Backend/IPasswordConfirmationBackend.php', 'OCP\\User\\Backend\\IProvideAvatarBackend' => __DIR__ . '/../../..' . '/lib/public/User/Backend/IProvideAvatarBackend.php', + 'OCP\\User\\Backend\\ISearchKnownUsersBackend' => __DIR__ . '/../../..' . '/lib/public/User/Backend/ISearchKnownUsersBackend.php', 'OCP\\User\\Backend\\ISetDisplayNameBackend' => __DIR__ . '/../../..' . '/lib/public/User/Backend/ISetDisplayNameBackend.php', 'OCP\\User\\Backend\\ISetPasswordBackend' => __DIR__ . '/../../..' . '/lib/public/User/Backend/ISetPasswordBackend.php', 'OCP\\User\\Events\\BeforePasswordUpdatedEvent' => __DIR__ . '/../../..' . '/lib/public/User/Events/BeforePasswordUpdatedEvent.php', diff --git a/lib/private/Collaboration/Collaborators/UserPlugin.php b/lib/private/Collaboration/Collaborators/UserPlugin.php index 06a8c6f0ef..c2132048b2 100644 --- a/lib/private/Collaboration/Collaborators/UserPlugin.php +++ b/lib/private/Collaboration/Collaborators/UserPlugin.php @@ -116,7 +116,11 @@ class UserPlugin implements ISearchPlugin { } } else { // Search in all users - $usersTmp = $this->userManager->searchDisplayName($search, $limit, $offset); + if ($this->shareeEnumerationPhone) { + $usersTmp = $this->userManager->searchKnownUsersByDisplayName($currentUserId, $search, $limit, $offset); + } else { + $usersTmp = $this->userManager->searchDisplayName($search, $limit, $offset); + } foreach ($usersTmp as $user) { if ($user->isEnabled()) { // Don't keep deactivated users $users[$user->getUID()] = $user; diff --git a/lib/private/User/Database.php b/lib/private/User/Database.php index 85e22d196e..0534d38fd4 100644 --- a/lib/private/User/Database.php +++ b/lib/private/User/Database.php @@ -70,6 +70,7 @@ use OCP\User\Backend\ICreateUserBackend; use OCP\User\Backend\IGetDisplayNameBackend; use OCP\User\Backend\IGetHomeBackend; use OCP\User\Backend\IGetRealUIDBackend; +use OCP\User\Backend\ISearchKnownUsersBackend; use OCP\User\Backend\ISetDisplayNameBackend; use OCP\User\Backend\ISetPasswordBackend; @@ -84,6 +85,7 @@ class Database extends ABackend implements ICheckPasswordBackend, IGetHomeBackend, ICountUsersBackend, + ISearchKnownUsersBackend, IGetRealUIDBackend { /** @var CappedMemoryCache */ private $cache; @@ -277,7 +279,47 @@ class Database extends ABackend implements ->orWhere($query->expr()->iLike('displayname', $query->createPositionalParameter('%' . $this->dbConn->escapeLikeParameter($search) . '%'))) ->orWhere($query->expr()->iLike('configvalue', $query->createPositionalParameter('%' . $this->dbConn->escapeLikeParameter($search) . '%'))) ->orderBy($query->func()->lower('displayname'), 'ASC') - ->orderBy('uid_lower', 'ASC') + ->addOrderBy('uid_lower', 'ASC') + ->setMaxResults($limit) + ->setFirstResult($offset); + + $result = $query->execute(); + $displayNames = []; + while ($row = $result->fetch()) { + $displayNames[(string)$row['uid']] = (string)$row['displayname']; + } + + return $displayNames; + } + + /** + * @param string $searcher + * @param string $pattern + * @param int|null $limit + * @param int|null $offset + * @return array + * @since 21.0.1 + */ + public function searchKnownUsersByDisplayName(string $searcher, string $pattern, ?int $limit = null, ?int $offset = null): array { + $limit = $this->fixLimit($limit); + + $this->fixDI(); + + $query = $this->dbConn->getQueryBuilder(); + + $query->select('u.uid', 'u.displayname') + ->from($this->table, 'u') + ->leftJoin('u', 'known_users', 'k', $query->expr()->andX( + $query->expr()->eq('k.known_user', 'u.uid'), + $query->expr()->eq('k.known_to', $query->createNamedParameter($searcher)) + )) + ->where($query->expr()->eq('k.known_to', $query->createNamedParameter($searcher))) + ->andWhere($query->expr()->orX( + $query->expr()->iLike('u.uid', $query->createNamedParameter('%' . $this->dbConn->escapeLikeParameter($pattern) . '%')), + $query->expr()->iLike('u.displayname', $query->createNamedParameter('%' . $this->dbConn->escapeLikeParameter($pattern) . '%')) + )) + ->orderBy('u.displayname', 'ASC') + ->addOrderBy('u.uid_lower', 'ASC') ->setMaxResults($limit) ->setFirstResult($offset); diff --git a/lib/private/User/Manager.php b/lib/private/User/Manager.php index 5223a80a2f..59c007b6b5 100644 --- a/lib/private/User/Manager.php +++ b/lib/private/User/Manager.php @@ -47,6 +47,7 @@ use OCP\IUserBackend; use OCP\IUserManager; use OCP\Support\Subscription\IRegistry; use OCP\User\Backend\IGetRealUIDBackend; +use OCP\User\Backend\ISearchKnownUsersBackend; use OCP\User\Events\BeforeUserCreatedEvent; use OCP\User\Events\UserCreatedEvent; use OCP\UserInterface; @@ -329,6 +330,41 @@ class Manager extends PublicEmitter implements IUserManager { return $users; } + /** + * Search known users (from phonebook sync) by displayName + * + * @param string $searcher + * @param string $pattern + * @param int|null $limit + * @param int|null $offset + * @return IUser[] + */ + public function searchKnownUsersByDisplayName(string $searcher, string $pattern, ?int $limit = null, ?int $offset = null): array { + $users = []; + foreach ($this->backends as $backend) { + if ($backend instanceof ISearchKnownUsersBackend) { + $backendUsers = $backend->searchKnownUsersByDisplayName($searcher, $pattern, $limit, $offset); + } else { + // Better than nothing, but filtering after pagination can remove lots of results. + $backendUsers = $backend->getDisplayNames($pattern, $limit, $offset); + } + if (is_array($backendUsers)) { + foreach ($backendUsers as $uid => $displayName) { + $users[] = $this->getUserObject($uid, $backend); + } + } + } + + usort($users, function ($a, $b) { + /** + * @var IUser $a + * @var IUser $b + */ + return strcasecmp($a->getDisplayName(), $b->getDisplayName()); + }); + return $users; + } + /** * @param string $uid * @param string $password diff --git a/lib/public/IUserManager.php b/lib/public/IUserManager.php index e8a7fc7827..6963bb5ddb 100644 --- a/lib/public/IUserManager.php +++ b/lib/public/IUserManager.php @@ -126,6 +126,18 @@ interface IUserManager { */ public function searchDisplayName($pattern, $limit = null, $offset = null); + /** + * Search known users (from phonebook sync) by displayName + * + * @param string $searcher + * @param string $pattern + * @param int|null $limit + * @param int|null $offset + * @return IUser[] + * @since 21.0.1 + */ + public function searchKnownUsersByDisplayName(string $searcher, string $pattern, ?int $limit = null, ?int $offset = null): array; + /** * @param string $uid * @param string $password diff --git a/lib/public/User/Backend/IGetRealUIDBackend.php b/lib/public/User/Backend/IGetRealUIDBackend.php index d724426e17..cc290eb6dc 100644 --- a/lib/public/User/Backend/IGetRealUIDBackend.php +++ b/lib/public/User/Backend/IGetRealUIDBackend.php @@ -33,7 +33,7 @@ interface IGetRealUIDBackend { /** * Some backends accept different UIDs than what is the internal UID to be used. - * For example the database backend accepts differnt cased UIDs in all the functions + * For example the database backend accepts different cased UIDs in all the functions * but the internal UID that is to be used should be correctly cased. * * This little function makes sure that the used UID will be correct hen using the user object diff --git a/lib/public/User/Backend/ISearchKnownUsersBackend.php b/lib/public/User/Backend/ISearchKnownUsersBackend.php new file mode 100644 index 0000000000..89c7a49cd3 --- /dev/null +++ b/lib/public/User/Backend/ISearchKnownUsersBackend.php @@ -0,0 +1,43 @@ + + * + * @author Joas Schilling + * + * @license GNU AGPL version 3 or any later version + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU Affero General Public License as + * published by the Free Software Foundation, either version 3 of the + * License, or (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Affero General Public License for more details. + * + * You should have received a copy of the GNU Affero General Public License + * along with this program. If not, see . + * + */ + +namespace OCP\User\Backend; + +/** + * @since 21.0.1 + */ +interface ISearchKnownUsersBackend { + + /** + * @param string $searcher + * @param string $pattern + * @param int|null $limit + * @param int|null $offset + * @return array + * @since 21.0.1 + */ + public function searchKnownUsersByDisplayName(string $searcher, string $pattern, ?int $limit = null, ?int $offset = null): array; +} diff --git a/tests/lib/Collaboration/Collaborators/UserPluginTest.php b/tests/lib/Collaboration/Collaborators/UserPluginTest.php index f2e0e7e274..43bbffc9b6 100644 --- a/tests/lib/Collaboration/Collaborators/UserPluginTest.php +++ b/tests/lib/Collaboration/Collaborators/UserPluginTest.php @@ -104,17 +104,19 @@ class UserPluginTest extends TestCase { ); } - public function mockConfig($shareWithGroupOnly, $shareeEnumeration, $shareeEnumerationLimitToGroup) { + public function mockConfig($shareWithGroupOnly, $shareeEnumeration, $shareeEnumerationLimitToGroup, $shareeEnumerationPhone = false) { $this->config->expects($this->any()) ->method('getAppValue') ->willReturnCallback( - function ($appName, $key, $default) use ($shareWithGroupOnly, $shareeEnumeration, $shareeEnumerationLimitToGroup) { + function ($appName, $key, $default) use ($shareWithGroupOnly, $shareeEnumeration, $shareeEnumerationLimitToGroup, $shareeEnumerationPhone) { if ($appName === 'core' && $key === 'shareapi_only_share_with_group_members') { return $shareWithGroupOnly ? 'yes' : 'no'; } elseif ($appName === 'core' && $key === 'shareapi_allow_share_dialog_user_enumeration') { return $shareeEnumeration ? 'yes' : 'no'; } elseif ($appName === 'core' && $key === 'shareapi_restrict_user_enumeration_to_group') { return $shareeEnumerationLimitToGroup ? 'yes' : 'no'; + } elseif ($appName === 'core' && $key === 'shareapi_restrict_user_enumeration_to_phone') { + return $shareeEnumerationPhone ? 'yes' : 'no'; } return $default; } @@ -266,6 +268,28 @@ class UserPluginTest extends TestCase { false, false, ], + [ + 'test', + false, + true, + [], + [ + $this->getUserMock('test0', 'Test'), + $this->getUserMock('test1', 'Test One'), + $this->getUserMock('test2', 'Test Two'), + ], + [ + ['label' => 'Test', 'value' => ['shareType' => IShare::TYPE_USER, 'shareWith' => 'test0'], 'icon' => 'icon-user', 'subline' => null, 'status' => [], 'shareWithDisplayNameUnique' => 'test0'], + ], + [ + ['label' => 'Test One', 'value' => ['shareType' => IShare::TYPE_USER, 'shareWith' => 'test1'], 'icon' => 'icon-user', 'subline' => null, 'status' => [], 'shareWithDisplayNameUnique' => 'test1'], + ['label' => 'Test Two', 'value' => ['shareType' => IShare::TYPE_USER, 'shareWith' => 'test2'], 'icon' => 'icon-user', 'subline' => null, 'status' => [], 'shareWithDisplayNameUnique' => 'test2'], + ], + false, + false, + [], + true, + ], [ 'test', false, @@ -443,9 +467,10 @@ class UserPluginTest extends TestCase { array $expected, $reachedEnd, $singleUser, - array $users = [] + array $users = [], + $shareeEnumerationPhone = false ) { - $this->mockConfig($shareWithGroupOnly, $shareeEnumeration, false); + $this->mockConfig($shareWithGroupOnly, $shareeEnumeration, false, $shareeEnumerationPhone); $this->instantiatePlugin(); $this->session->expects($this->any()) @@ -453,10 +478,24 @@ class UserPluginTest extends TestCase { ->willReturn($this->user); if (!$shareWithGroupOnly) { - $this->userManager->expects($this->once()) - ->method('searchDisplayName') - ->with($searchTerm, $this->limit, $this->offset) - ->willReturn($userResponse); + if ($shareeEnumerationPhone) { + $this->userManager->expects($this->once()) + ->method('searchKnownUsersByDisplayName') + ->with($this->user->getUID(), $searchTerm, $this->limit, $this->offset) + ->willReturn($userResponse); + + $this->knownUserService->method('isKnownToUser') + ->willReturnMap([ + [$this->user->getUID(), 'test0', true], + [$this->user->getUID(), 'test1', true], + [$this->user->getUID(), 'test2', true], + ]); + } else { + $this->userManager->expects($this->once()) + ->method('searchDisplayName') + ->with($searchTerm, $this->limit, $this->offset) + ->willReturn($userResponse); + } } else { $this->groupManager->method('getUserGroupIds') ->with($this->user) From f2acf492cf1ec2926f3f7a8de796c9cba168c14f Mon Sep 17 00:00:00 2001 From: Joas Schilling Date: Wed, 17 Mar 2021 09:19:42 +0100 Subject: [PATCH 2/5] Add missing index on the user column Signed-off-by: Joas Schilling --- .../Version21000Date20210309185127.php | 52 +++++++++++++++++++ lib/composer/composer/autoload_classmap.php | 1 + lib/composer/composer/autoload_static.php | 1 + 3 files changed, 54 insertions(+) create mode 100644 core/Migrations/Version21000Date20210309185127.php diff --git a/core/Migrations/Version21000Date20210309185127.php b/core/Migrations/Version21000Date20210309185127.php new file mode 100644 index 0000000000..01cbdcb864 --- /dev/null +++ b/core/Migrations/Version21000Date20210309185127.php @@ -0,0 +1,52 @@ + + * + * @author Joas Schilling + * + * @license GNU AGPL version 3 or any later version + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU Affero General Public License as + * published by the Free Software Foundation, either version 3 of the + * License, or (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Affero General Public License for more details. + * + * You should have received a copy of the GNU Affero General Public License + * along with this program. If not, see . + */ + + +namespace OC\Core\Migrations; + +use Closure; +use OCP\DB\ISchemaWrapper; +use OCP\Migration\IOutput; +use OCP\Migration\SimpleMigrationStep; + +class Version21000Date20210309185127 extends SimpleMigrationStep { + /** + * @param IOutput $output + * @param Closure $schemaClosure The `\Closure` returns a `ISchemaWrapper` + * @param array $options + * @return null|ISchemaWrapper + */ + public function changeSchema(IOutput $output, Closure $schemaClosure, array $options): ?ISchemaWrapper { + /** @var ISchemaWrapper $schema */ + $schema = $schemaClosure(); + + $table = $schema->getTable('known_users'); + if (!$table->hasIndex('ku_known_user')) { + $table->addIndex(['known_user'], 'ku_known_user'); + return $schema; + } + + return null; + } +} diff --git a/lib/composer/composer/autoload_classmap.php b/lib/composer/composer/autoload_classmap.php index fd3dc44f26..a150fca952 100644 --- a/lib/composer/composer/autoload_classmap.php +++ b/lib/composer/composer/autoload_classmap.php @@ -953,6 +953,7 @@ return array( 'OC\\Core\\Migrations\\Version21000Date20201202095923' => $baseDir . '/core/Migrations/Version21000Date20201202095923.php', 'OC\\Core\\Migrations\\Version21000Date20210119195004' => $baseDir . '/core/Migrations/Version21000Date20210119195004.php', 'OC\\Core\\Migrations\\Version21000Date20210309185126' => $baseDir . '/core/Migrations/Version21000Date20210309185126.php', + 'OC\\Core\\Migrations\\Version21000Date20210309185127' => $baseDir . '/core/Migrations/Version21000Date20210309185127.php', 'OC\\Core\\Notification\\CoreNotifier' => $baseDir . '/core/Notification/CoreNotifier.php', 'OC\\Core\\Service\\LoginFlowV2Service' => $baseDir . '/core/Service/LoginFlowV2Service.php', 'OC\\DB\\Adapter' => $baseDir . '/lib/private/DB/Adapter.php', diff --git a/lib/composer/composer/autoload_static.php b/lib/composer/composer/autoload_static.php index 797c4b325d..5cb5efbd5d 100644 --- a/lib/composer/composer/autoload_static.php +++ b/lib/composer/composer/autoload_static.php @@ -982,6 +982,7 @@ class ComposerStaticInit53792487c5a8370acc0b06b1a864ff4c 'OC\\Core\\Migrations\\Version21000Date20201202095923' => __DIR__ . '/../../..' . '/core/Migrations/Version21000Date20201202095923.php', 'OC\\Core\\Migrations\\Version21000Date20210119195004' => __DIR__ . '/../../..' . '/core/Migrations/Version21000Date20210119195004.php', 'OC\\Core\\Migrations\\Version21000Date20210309185126' => __DIR__ . '/../../..' . '/core/Migrations/Version21000Date20210309185126.php', + 'OC\\Core\\Migrations\\Version21000Date20210309185127' => __DIR__ . '/../../..' . '/core/Migrations/Version21000Date20210309185127.php', 'OC\\Core\\Notification\\CoreNotifier' => __DIR__ . '/../../..' . '/core/Notification/CoreNotifier.php', 'OC\\Core\\Service\\LoginFlowV2Service' => __DIR__ . '/../../..' . '/core/Service/LoginFlowV2Service.php', 'OC\\DB\\Adapter' => __DIR__ . '/../../..' . '/lib/private/DB/Adapter.php', From e9ea4a0f0119718796931f8dfa546293d5a95f9f Mon Sep 17 00:00:00 2001 From: Joas Schilling Date: Wed, 17 Mar 2021 09:36:06 +0100 Subject: [PATCH 3/5] Fix parameter types in docs Signed-off-by: Joas Schilling --- apps/user_ldap/lib/User_LDAP.php | 10 +++++----- apps/user_ldap/lib/User_Proxy.php | 4 ++-- lib/private/User/Backend.php | 4 ++-- lib/private/User/Database.php | 4 ++-- lib/public/UserInterface.php | 4 ++-- 5 files changed, 13 insertions(+), 13 deletions(-) diff --git a/apps/user_ldap/lib/User_LDAP.php b/apps/user_ldap/lib/User_LDAP.php index 79f230ae00..12d93ce5c8 100644 --- a/apps/user_ldap/lib/User_LDAP.php +++ b/apps/user_ldap/lib/User_LDAP.php @@ -133,7 +133,7 @@ class User_LDAP extends BackendUtility implements \OCP\IUserBackend, \OCP\UserIn return false; } } - + /** * returns the username for the given LDAP DN, if available * @@ -502,8 +502,8 @@ class User_LDAP extends BackendUtility implements \OCP\IUserBackend, \OCP\UserIn * Get a list of all display names * * @param string $search - * @param string|null $limit - * @param string|null $offset + * @param int|null $limit + * @param int|null $offset * @return array an array of all displayNames (value) and the corresponding uids (key) */ public function getDisplayNames($search = '', $limit = null, $offset = null) { @@ -574,7 +574,7 @@ class User_LDAP extends BackendUtility implements \OCP\IUserBackend, \OCP\UserIn public function getBackendName() { return 'LDAP'; } - + /** * Return access for LDAP interaction. * @param string $uid @@ -583,7 +583,7 @@ class User_LDAP extends BackendUtility implements \OCP\IUserBackend, \OCP\UserIn public function getLDAPAccess($uid) { return $this->access; } - + /** * Return LDAP connection resource from a cloned connection. * The cloned connection needs to be closed manually. diff --git a/apps/user_ldap/lib/User_Proxy.php b/apps/user_ldap/lib/User_Proxy.php index e8d0a6d694..532d8ff4cb 100644 --- a/apps/user_ldap/lib/User_Proxy.php +++ b/apps/user_ldap/lib/User_Proxy.php @@ -292,8 +292,8 @@ class User_Proxy extends Proxy implements \OCP\IUserBackend, \OCP\UserInterface, * Get a list of all display names and user ids. * * @param string $search - * @param string|null $limit - * @param string|null $offset + * @param int|null $limit + * @param int|null $offset * @return array an array of all displayNames (value) and the corresponding uids (key) */ public function getDisplayNames($search = '', $limit = null, $offset = null) { diff --git a/lib/private/User/Backend.php b/lib/private/User/Backend.php index c87dc5d2d5..d70d13673c 100644 --- a/lib/private/User/Backend.php +++ b/lib/private/User/Backend.php @@ -143,8 +143,8 @@ abstract class Backend implements UserInterface { * Get a list of all display names and user ids. * * @param string $search - * @param string|null $limit - * @param string|null $offset + * @param int|null $limit + * @param int|null $offset * @return array an array of all displayNames (value) and the corresponding uids (key) */ public function getDisplayNames($search = '', $limit = null, $offset = null) { diff --git a/lib/private/User/Database.php b/lib/private/User/Database.php index 0534d38fd4..6c04a1b900 100644 --- a/lib/private/User/Database.php +++ b/lib/private/User/Database.php @@ -256,8 +256,8 @@ class Database extends ABackend implements * Get a list of all display names and user ids. * * @param string $search - * @param string|null $limit - * @param string|null $offset + * @param int|null $limit + * @param int|null $offset * @return array an array of all displayNames (value) and the corresponding uids (key) */ public function getDisplayNames($search = '', $limit = null, $offset = null) { diff --git a/lib/public/UserInterface.php b/lib/public/UserInterface.php index 0479041e8d..42a18cca10 100644 --- a/lib/public/UserInterface.php +++ b/lib/public/UserInterface.php @@ -92,8 +92,8 @@ interface UserInterface { * Get a list of all display names and user ids. * * @param string $search - * @param string|null $limit - * @param string|null $offset + * @param int|null $limit + * @param int|null $offset * @return array an array of all displayNames (value) and the corresponding uids (key) * @since 4.5.0 */ From 3fd9d0dc506120830bc1ffc1e62c92c83a6e9e53 Mon Sep 17 00:00:00 2001 From: Joas Schilling Date: Wed, 17 Mar 2021 10:26:21 +0100 Subject: [PATCH 4/5] Also limit to user group in case enumeration is enabled for groups and phonenumbers Signed-off-by: Joas Schilling --- .../Collaborators/UserPlugin.php | 21 ++++++++++++++++++- .../Collaborators/UserPluginTest.php | 21 ++++++++++++++----- 2 files changed, 36 insertions(+), 6 deletions(-) diff --git a/lib/private/Collaboration/Collaborators/UserPlugin.php b/lib/private/Collaboration/Collaborators/UserPlugin.php index c2132048b2..12ed3e9893 100644 --- a/lib/private/Collaboration/Collaborators/UserPlugin.php +++ b/lib/private/Collaboration/Collaborators/UserPlugin.php @@ -97,7 +97,7 @@ class UserPlugin implements ISearchPlugin { $currentUserId = $this->userSession->getUser()->getUID(); $currentUserGroups = $this->groupManager->getUserGroupIds($this->userSession->getUser()); - if ($this->shareWithGroupOnly) { + if ($this->shareWithGroupOnly || $this->shareeEnumerationInGroupOnly) { // Search in all the groups this user is part of foreach ($currentUserGroups as $userGroupId) { $usersInGroup = $this->groupManager->displayNamesInGroup($userGroupId, $search, $limit, $offset); @@ -114,6 +114,25 @@ class UserPlugin implements ISearchPlugin { $hasMoreResults = true; } } + + if (!$this->shareWithGroupOnly && $this->shareeEnumerationPhone) { + $usersTmp = $this->userManager->searchKnownUsersByDisplayName($currentUserId, $search, $limit, $offset); + if (!empty($usersTmp)) { + foreach ($usersTmp as $user) { + if ($user->isEnabled()) { // Don't keep deactivated users + $users[$user->getUID()] = $user; + } + } + + uasort($users, function ($a, $b) { + /** + * @var \OC\User\User $a + * @var \OC\User\User $b + */ + return strcasecmp($a->getDisplayName(), $b->getDisplayName()); + }); + } + } } else { // Search in all users if ($this->shareeEnumerationPhone) { diff --git a/tests/lib/Collaboration/Collaborators/UserPluginTest.php b/tests/lib/Collaboration/Collaborators/UserPluginTest.php index 43bbffc9b6..acbcd42f04 100644 --- a/tests/lib/Collaboration/Collaborators/UserPluginTest.php +++ b/tests/lib/Collaboration/Collaborators/UserPluginTest.php @@ -659,9 +659,10 @@ class UserPluginTest extends TestCase { public function testSearchEnumerationLimit($search, $userGroups, $matchingUsers, $result) { $this->mockConfig(false, true, true); - $userResults = array_map(function ($user) { - return $this->getUserMock($user['uid'], $user['uid']); - }, $matchingUsers); + $userResults = []; + foreach ($matchingUsers as $user) { + $userResults[$user['uid']] = $user['uid']; + } $mappedResultExact = array_map(function ($user) { return ['label' => $user, 'value' => ['shareType' => 0, 'shareWith' => $user], 'icon' => 'icon-user', 'subline' => null, 'status' => [], 'shareWithDisplayNameUnique' => $user]; @@ -670,9 +671,19 @@ class UserPluginTest extends TestCase { return ['label' => $user, 'value' => ['shareType' => 0, 'shareWith' => $user], 'icon' => 'icon-user', 'subline' => null, 'status' => [], 'shareWithDisplayNameUnique' => $user]; }, $result['wide']); - $this->userManager->expects($this->once()) - ->method('searchDisplayName') + $this->userManager + ->method('get') + ->willReturnCallback(function ($userId) use ($userResults) { + if (isset($userResults[$userId])) { + return $this->getUserMock($userId, $userId); + } + return null; + }); + + $this->groupManager->method('displayNamesInGroup') ->willReturn($userResults); + + $this->session->expects($this->any()) ->method('getUser') ->willReturn($this->getUserMock('test', 'foo')); From e12e2ae8a3dc6905632a1719dd425d4df5c53b42 Mon Sep 17 00:00:00 2001 From: Roeland Jago Douma Date: Mon, 22 Mar 2021 20:09:25 +0100 Subject: [PATCH 5/5] Make psalm happy Signed-off-by: Roeland Jago Douma --- lib/private/DB/QueryBuilder/QueryBuilder.php | 5 +++-- lib/public/DB/QueryBuilder/IQueryBuilder.php | 2 +- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/lib/private/DB/QueryBuilder/QueryBuilder.php b/lib/private/DB/QueryBuilder/QueryBuilder.php index fb28fa2864..352829a56a 100644 --- a/lib/private/DB/QueryBuilder/QueryBuilder.php +++ b/lib/private/DB/QueryBuilder/QueryBuilder.php @@ -48,6 +48,7 @@ use OC\DB\QueryBuilder\FunctionBuilder\SqliteFunctionBuilder; use OC\DB\ResultAdapter; use OC\SystemConfig; use OCP\DB\IResult; +use OCP\DB\QueryBuilder\ICompositeExpression; use OCP\DB\QueryBuilder\ILiteral; use OCP\DB\QueryBuilder\IParameter; use OCP\DB\QueryBuilder\IQueryBuilder; @@ -282,7 +283,7 @@ class QueryBuilder implements IQueryBuilder { 'app' => 'core', ]); } - + $result = $this->queryBuilder->execute(); if (is_int($result)) { return $result; @@ -720,7 +721,7 @@ class QueryBuilder implements IQueryBuilder { * @param string $fromAlias The alias that points to a from clause. * @param string $join The table name to join. * @param string $alias The alias of the join table. - * @param string $condition The condition for the join. + * @param string|null|ICompositeExpression $condition The condition for the join. * * @return $this This QueryBuilder instance. */ diff --git a/lib/public/DB/QueryBuilder/IQueryBuilder.php b/lib/public/DB/QueryBuilder/IQueryBuilder.php index 24de7b4ce6..269203ce15 100644 --- a/lib/public/DB/QueryBuilder/IQueryBuilder.php +++ b/lib/public/DB/QueryBuilder/IQueryBuilder.php @@ -492,7 +492,7 @@ interface IQueryBuilder { * @param string $fromAlias The alias that points to a from clause. * @param string $join The table name to join. * @param string $alias The alias of the join table. - * @param string $condition The condition for the join. + * @param string|null|ICompositeExpression $condition The condition for the join. * * @return $this This QueryBuilder instance. * @since 8.2.0