From f62d9b50fda96fe2acd6ebb2eb11e1f6bcf54116 Mon Sep 17 00:00:00 2001 From: Joas Schilling Date: Tue, 9 Mar 2021 20:32:08 +0100 Subject: Store when a user knows another user based on phone number Signed-off-by: Joas Schilling --- .../lib/Controller/UsersController.php | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+) (limited to 'apps') diff --git a/apps/provisioning_api/lib/Controller/UsersController.php b/apps/provisioning_api/lib/Controller/UsersController.php index 11c3a85db9b..9cc8372daa4 100644 --- a/apps/provisioning_api/lib/Controller/UsersController.php +++ b/apps/provisioning_api/lib/Controller/UsersController.php @@ -49,6 +49,8 @@ use libphonenumber\PhoneNumberUtil; use OC\Accounts\AccountManager; use OC\Authentication\Token\RemoteWipe; use OC\HintException; +use OC\KnownUser\KnownUser; +use OC\KnownUser\KnownUserMapper; use OCA\Provisioning_API\FederatedShareProviderFactory; use OCA\Settings\Mailer\NewUserMailHelper; use OCP\Accounts\IAccountManager; @@ -90,6 +92,8 @@ class UsersController extends AUserData { private $secureRandom; /** @var RemoteWipe */ private $remoteWipe; + /** @var KnownUserMapper */ + private $knownUserMapper; /** @var IEventDispatcher */ private $eventDispatcher; @@ -108,6 +112,7 @@ class UsersController extends AUserData { FederatedShareProviderFactory $federatedShareProviderFactory, ISecureRandom $secureRandom, RemoteWipe $remoteWipe, + KnownUserMapper $knownUserMapper, IEventDispatcher $eventDispatcher) { parent::__construct($appName, $request, @@ -126,6 +131,7 @@ class UsersController extends AUserData { $this->federatedShareProviderFactory = $federatedShareProviderFactory; $this->secureRandom = $secureRandom; $this->remoteWipe = $remoteWipe; + $this->knownUserMapper = $knownUserMapper; $this->eventDispatcher = $eventDispatcher; } @@ -265,9 +271,25 @@ class UsersController extends AUserData { } $matches = []; + $knownUsers = []; foreach ($userMatches as $phone => $userId) { // Not using the ICloudIdManager as that would run a search for each contact to find the display name in the address book $matches[$normalizedNumberToKey[$phone]] = $userId . '@' . $cloudUrl; + $knownUsers[] = $userId; + } + + /** @var IUser $user */ + $user = $this->userSession->getUser(); + $knownTo = $user->getUID(); + + // Cleanup all previous entries and only allow new matches + $this->knownUserMapper->deleteKnownTo($knownTo); + + foreach ($knownUsers as $knownUser) { + $entity = new KnownUser(); + $entity->setKnownTo($knownTo); + $entity->setKnownUser($knownUser); + $this->knownUserMapper->insert($entity); } return new DataResponse($matches); -- cgit v1.2.3 From 55a5d26c562c5221c93f191532372cc8fde76adb Mon Sep 17 00:00:00 2001 From: Joas Schilling Date: Tue, 9 Mar 2021 20:32:50 +0100 Subject: Delete matches when a user changes their phone number Signed-off-by: Joas Schilling --- apps/provisioning_api/lib/Controller/UsersController.php | 4 ++++ 1 file changed, 4 insertions(+) (limited to 'apps') diff --git a/apps/provisioning_api/lib/Controller/UsersController.php b/apps/provisioning_api/lib/Controller/UsersController.php index 9cc8372daa4..648faf8ad20 100644 --- a/apps/provisioning_api/lib/Controller/UsersController.php +++ b/apps/provisioning_api/lib/Controller/UsersController.php @@ -699,6 +699,10 @@ class UsersController extends AUserData { $userAccount[$key]['value'] = $value; try { $this->accountManager->updateUser($targetUser, $userAccount, true); + + if ($key === IAccountManager::PROPERTY_PHONE) { + $this->knownUserMapper->deleteKnownUser($targetUser->getUID()); + } } catch (\InvalidArgumentException $e) { throw new OCSException('Invalid ' . $e->getMessage(), 102); } -- cgit v1.2.3 From 7baefcfc74d12da8a3850b6747da7c055c56e522 Mon Sep 17 00:00:00 2001 From: Joas Schilling Date: Tue, 9 Mar 2021 20:36:02 +0100 Subject: Delete matches when the user is being deleted Signed-off-by: Joas Schilling --- .../composer/composer/autoload_classmap.php | 1 + .../composer/composer/autoload_static.php | 1 + apps/provisioning_api/lib/AppInfo/Application.php | 4 ++ .../lib/Listener/UserDeletedListener.php | 54 ++++++++++++++++++++++ 4 files changed, 60 insertions(+) create mode 100644 apps/provisioning_api/lib/Listener/UserDeletedListener.php (limited to 'apps') diff --git a/apps/provisioning_api/composer/composer/autoload_classmap.php b/apps/provisioning_api/composer/composer/autoload_classmap.php index 0383fddbefd..e94a97c1949 100644 --- a/apps/provisioning_api/composer/composer/autoload_classmap.php +++ b/apps/provisioning_api/composer/composer/autoload_classmap.php @@ -14,6 +14,7 @@ return array( 'OCA\\Provisioning_API\\Controller\\GroupsController' => $baseDir . '/../lib/Controller/GroupsController.php', 'OCA\\Provisioning_API\\Controller\\UsersController' => $baseDir . '/../lib/Controller/UsersController.php', 'OCA\\Provisioning_API\\FederatedShareProviderFactory' => $baseDir . '/../lib/FederatedShareProviderFactory.php', + 'OCA\\Provisioning_API\\Listener\\UserDeletedListener' => $baseDir . '/../lib/Listener/UserDeletedListener.php', 'OCA\\Provisioning_API\\Middleware\\Exceptions\\NotSubAdminException' => $baseDir . '/../lib/Middleware/Exceptions/NotSubAdminException.php', 'OCA\\Provisioning_API\\Middleware\\ProvisioningApiMiddleware' => $baseDir . '/../lib/Middleware/ProvisioningApiMiddleware.php', ); diff --git a/apps/provisioning_api/composer/composer/autoload_static.php b/apps/provisioning_api/composer/composer/autoload_static.php index 2c1682641a1..b982f203211 100644 --- a/apps/provisioning_api/composer/composer/autoload_static.php +++ b/apps/provisioning_api/composer/composer/autoload_static.php @@ -29,6 +29,7 @@ class ComposerStaticInitProvisioning_API 'OCA\\Provisioning_API\\Controller\\GroupsController' => __DIR__ . '/..' . '/../lib/Controller/GroupsController.php', 'OCA\\Provisioning_API\\Controller\\UsersController' => __DIR__ . '/..' . '/../lib/Controller/UsersController.php', 'OCA\\Provisioning_API\\FederatedShareProviderFactory' => __DIR__ . '/..' . '/../lib/FederatedShareProviderFactory.php', + 'OCA\\Provisioning_API\\Listener\\UserDeletedListener' => __DIR__ . '/..' . '/../lib/Listener/UserDeletedListener.php', 'OCA\\Provisioning_API\\Middleware\\Exceptions\\NotSubAdminException' => __DIR__ . '/..' . '/../lib/Middleware/Exceptions/NotSubAdminException.php', 'OCA\\Provisioning_API\\Middleware\\ProvisioningApiMiddleware' => __DIR__ . '/..' . '/../lib/Middleware/ProvisioningApiMiddleware.php', ); diff --git a/apps/provisioning_api/lib/AppInfo/Application.php b/apps/provisioning_api/lib/AppInfo/Application.php index 863f8861d8b..7ec21c3329e 100644 --- a/apps/provisioning_api/lib/AppInfo/Application.php +++ b/apps/provisioning_api/lib/AppInfo/Application.php @@ -29,6 +29,7 @@ namespace OCA\Provisioning_API\AppInfo; use OC\Group\Manager as GroupManager; +use OCA\Provisioning_API\Listener\UserDeletedListener; use OCA\Provisioning_API\Middleware\ProvisioningApiMiddleware; use OCA\Settings\Mailer\NewUserMailHelper; use OCP\AppFramework\App; @@ -47,6 +48,7 @@ use OCP\L10N\IFactory; use OCP\Mail\IMailer; use OCP\Security\ICrypto; use OCP\Security\ISecureRandom; +use OCP\User\Events\UserDeletedEvent; use OCP\Util; use Psr\Container\ContainerInterface; @@ -56,6 +58,8 @@ class Application extends App implements IBootstrap { } public function register(IRegistrationContext $context): void { + $context->registerEventListener(UserDeletedEvent::class, UserDeletedListener::class); + $context->registerService(NewUserMailHelper::class, function (ContainerInterface $c) { return new NewUserMailHelper( $c->get(Defaults::class), diff --git a/apps/provisioning_api/lib/Listener/UserDeletedListener.php b/apps/provisioning_api/lib/Listener/UserDeletedListener.php new file mode 100644 index 00000000000..bcbf8cc85b6 --- /dev/null +++ b/apps/provisioning_api/lib/Listener/UserDeletedListener.php @@ -0,0 +1,54 @@ + + * + * @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 OCA\Provisioning_API\Listener; + +use OC\KnownUser\KnownUserMapper; +use OCP\EventDispatcher\Event; +use OCP\EventDispatcher\IEventListener; +use OCP\User\Events\UserDeletedEvent; + +class UserDeletedListener implements IEventListener { + + /** @var KnownUserMapper */ + private $knownUserMapper; + + public function __construct(KnownUserMapper $knownUserMapper) { + $this->knownUserMapper = $knownUserMapper; + } + + public function handle(Event $event): void { + if (!($event instanceof UserDeletedEvent)) { + // Unrelated + return; + } + + $user = $event->getUser(); + + // Delete all entries of this user + $this->knownUserMapper->deleteKnownTo($user->getUID()); + + // Delete all entries that other users know this user + $this->knownUserMapper->deleteKnownUser($user->getUID()); + } +} -- cgit v1.2.3 From c7be18c0d6cf6a5af2251abdfa18cdccd11da33b Mon Sep 17 00:00:00 2001 From: Joas Schilling Date: Tue, 9 Mar 2021 21:22:59 +0100 Subject: Add a service to find out if a user knows another user Signed-off-by: Joas Schilling --- .../lib/Controller/UsersController.php | 37 +++++-------- .../lib/Listener/UserDeletedListener.php | 14 ++--- .../tests/Controller/UsersControllerTest.php | 23 ++++++++ lib/composer/composer/autoload_classmap.php | 1 + lib/composer/composer/autoload_static.php | 1 + lib/private/KnownUser/KnownUserMapper.php | 13 +++++ lib/private/KnownUser/KnownUserService.php | 62 ++++++++++++++++++++++ 7 files changed, 121 insertions(+), 30 deletions(-) create mode 100644 lib/private/KnownUser/KnownUserService.php (limited to 'apps') diff --git a/apps/provisioning_api/lib/Controller/UsersController.php b/apps/provisioning_api/lib/Controller/UsersController.php index 648faf8ad20..d1dba3ea6ee 100644 --- a/apps/provisioning_api/lib/Controller/UsersController.php +++ b/apps/provisioning_api/lib/Controller/UsersController.php @@ -49,8 +49,7 @@ use libphonenumber\PhoneNumberUtil; use OC\Accounts\AccountManager; use OC\Authentication\Token\RemoteWipe; use OC\HintException; -use OC\KnownUser\KnownUser; -use OC\KnownUser\KnownUserMapper; +use OC\KnownUser\KnownUserService; use OCA\Provisioning_API\FederatedShareProviderFactory; use OCA\Settings\Mailer\NewUserMailHelper; use OCP\Accounts\IAccountManager; @@ -92,8 +91,8 @@ class UsersController extends AUserData { private $secureRandom; /** @var RemoteWipe */ private $remoteWipe; - /** @var KnownUserMapper */ - private $knownUserMapper; + /** @var KnownUserService */ + private $knownUserService; /** @var IEventDispatcher */ private $eventDispatcher; @@ -112,7 +111,7 @@ class UsersController extends AUserData { FederatedShareProviderFactory $federatedShareProviderFactory, ISecureRandom $secureRandom, RemoteWipe $remoteWipe, - KnownUserMapper $knownUserMapper, + KnownUserService $knownUserService, IEventDispatcher $eventDispatcher) { parent::__construct($appName, $request, @@ -131,7 +130,7 @@ class UsersController extends AUserData { $this->federatedShareProviderFactory = $federatedShareProviderFactory; $this->secureRandom = $secureRandom; $this->remoteWipe = $remoteWipe; - $this->knownUserMapper = $knownUserMapper; + $this->knownUserService = $knownUserService; $this->eventDispatcher = $eventDispatcher; } @@ -237,6 +236,13 @@ class UsersController extends AUserData { return new DataResponse([], Http::STATUS_BAD_REQUEST); } + /** @var IUser $user */ + $user = $this->userSession->getUser(); + $knownTo = $user->getUID(); + + // Cleanup all previous entries and only allow new matches + $this->knownUserService->deleteKnownTo($knownTo); + $normalizedNumberToKey = []; foreach ($search as $key => $phoneNumbers) { foreach ($phoneNumbers as $phone) { @@ -271,25 +277,10 @@ class UsersController extends AUserData { } $matches = []; - $knownUsers = []; foreach ($userMatches as $phone => $userId) { // Not using the ICloudIdManager as that would run a search for each contact to find the display name in the address book $matches[$normalizedNumberToKey[$phone]] = $userId . '@' . $cloudUrl; - $knownUsers[] = $userId; - } - - /** @var IUser $user */ - $user = $this->userSession->getUser(); - $knownTo = $user->getUID(); - - // Cleanup all previous entries and only allow new matches - $this->knownUserMapper->deleteKnownTo($knownTo); - - foreach ($knownUsers as $knownUser) { - $entity = new KnownUser(); - $entity->setKnownTo($knownTo); - $entity->setKnownUser($knownUser); - $this->knownUserMapper->insert($entity); + $this->knownUserService->storeIsKnownToUser($knownTo, $userId); } return new DataResponse($matches); @@ -701,7 +692,7 @@ class UsersController extends AUserData { $this->accountManager->updateUser($targetUser, $userAccount, true); if ($key === IAccountManager::PROPERTY_PHONE) { - $this->knownUserMapper->deleteKnownUser($targetUser->getUID()); + $this->knownUserService->deleteKnownUser($targetUser->getUID()); } } catch (\InvalidArgumentException $e) { throw new OCSException('Invalid ' . $e->getMessage(), 102); diff --git a/apps/provisioning_api/lib/Listener/UserDeletedListener.php b/apps/provisioning_api/lib/Listener/UserDeletedListener.php index bcbf8cc85b6..f4fdb973080 100644 --- a/apps/provisioning_api/lib/Listener/UserDeletedListener.php +++ b/apps/provisioning_api/lib/Listener/UserDeletedListener.php @@ -23,18 +23,18 @@ declare(strict_types=1); namespace OCA\Provisioning_API\Listener; -use OC\KnownUser\KnownUserMapper; +use OC\KnownUser\KnownUserService; use OCP\EventDispatcher\Event; use OCP\EventDispatcher\IEventListener; use OCP\User\Events\UserDeletedEvent; class UserDeletedListener implements IEventListener { - /** @var KnownUserMapper */ - private $knownUserMapper; + /** @var KnownUserService */ + private $service; - public function __construct(KnownUserMapper $knownUserMapper) { - $this->knownUserMapper = $knownUserMapper; + public function __construct(KnownUserService $service) { + $this->service = $service; } public function handle(Event $event): void { @@ -46,9 +46,9 @@ class UserDeletedListener implements IEventListener { $user = $event->getUser(); // Delete all entries of this user - $this->knownUserMapper->deleteKnownTo($user->getUID()); + $this->service->deleteKnownTo($user->getUID()); // Delete all entries that other users know this user - $this->knownUserMapper->deleteKnownUser($user->getUID()); + $this->service->deleteKnownUser($user->getUID()); } } diff --git a/apps/provisioning_api/tests/Controller/UsersControllerTest.php b/apps/provisioning_api/tests/Controller/UsersControllerTest.php index 3130fce3e9c..d65e3d07913 100644 --- a/apps/provisioning_api/tests/Controller/UsersControllerTest.php +++ b/apps/provisioning_api/tests/Controller/UsersControllerTest.php @@ -44,6 +44,7 @@ use Exception; use OC\Accounts\AccountManager; use OC\Authentication\Token\RemoteWipe; use OC\Group\Manager; +use OC\KnownUser\KnownUserService; use OC\SubAdmin; use OCA\FederatedFileSharing\FederatedShareProvider; use OCA\Provisioning_API\Controller\UsersController; @@ -102,6 +103,8 @@ class UsersControllerTest extends TestCase { private $secureRandom; /** @var RemoteWipe|MockObject */ private $remoteWipe; + /** @var KnownUserService|MockObject */ + private $knownUserService; /** @var IEventDispatcher */ private $eventDispatcher; @@ -122,6 +125,7 @@ class UsersControllerTest extends TestCase { $this->federatedShareProviderFactory = $this->createMock(FederatedShareProviderFactory::class); $this->secureRandom = $this->createMock(ISecureRandom::class); $this->remoteWipe = $this->createMock(RemoteWipe::class); + $this->knownUserService = $this->createMock(KnownUserService::class); $this->eventDispatcher = $this->createMock(IEventDispatcher::class); $this->api = $this->getMockBuilder(UsersController::class) @@ -141,6 +145,7 @@ class UsersControllerTest extends TestCase { $this->federatedShareProviderFactory, $this->secureRandom, $this->remoteWipe, + $this->knownUserService, $this->eventDispatcher, ]) ->setMethods(['fillStorageInfo']) @@ -405,6 +410,7 @@ class UsersControllerTest extends TestCase { $this->federatedShareProviderFactory, $this->secureRandom, $this->remoteWipe, + $this->knownUserService, $this->eventDispatcher, ]) ->setMethods(['editUser']) @@ -1400,6 +1406,13 @@ class UsersControllerTest extends TestCase { * @param array $expected */ public function testSearchByPhoneNumbers(string $location, array $search, int $status, ?array $searchUsers, ?array $userMatches, array $expected) { + $knownTo = 'knownTo'; + $user = $this->createMock(IUser::class); + $user->method('getUID') + ->willReturn($knownTo); + $this->userSession->method('getUser') + ->willReturn($user); + if ($searchUsers === null) { $this->accountManager->expects($this->never()) ->method('searchUsers'); @@ -1408,6 +1421,14 @@ class UsersControllerTest extends TestCase { ->method('searchUsers') ->with(IAccountManager::PROPERTY_PHONE, $searchUsers) ->willReturn($userMatches); + + $this->knownUserService->expects($this->once()) + ->method('deleteKnownTo') + ->with($knownTo); + + $this->knownUserService->expects($this->exactly(count($expected))) + ->method('storeIsKnownToUser') + ->with($knownTo, $this->anything()); } $this->urlGenerator->method('getAbsoluteURL') @@ -3229,6 +3250,7 @@ class UsersControllerTest extends TestCase { $this->federatedShareProviderFactory, $this->secureRandom, $this->remoteWipe, + $this->knownUserService, $this->eventDispatcher, ]) ->setMethods(['getUserData']) @@ -3295,6 +3317,7 @@ class UsersControllerTest extends TestCase { $this->federatedShareProviderFactory, $this->secureRandom, $this->remoteWipe, + $this->knownUserService, $this->eventDispatcher, ]) ->setMethods(['getUserData']) diff --git a/lib/composer/composer/autoload_classmap.php b/lib/composer/composer/autoload_classmap.php index 65c33cf2569..65f050ed265 100644 --- a/lib/composer/composer/autoload_classmap.php +++ b/lib/composer/composer/autoload_classmap.php @@ -1168,6 +1168,7 @@ return array( 'OC\\IntegrityCheck\\Iterator\\ExcludeFoldersByPathFilterIterator' => $baseDir . '/lib/private/IntegrityCheck/Iterator/ExcludeFoldersByPathFilterIterator.php', 'OC\\KnownUser\\KnownUser' => $baseDir . '/lib/private/KnownUser/KnownUser.php', 'OC\\KnownUser\\KnownUserMapper' => $baseDir . '/lib/private/KnownUser/KnownUserMapper.php', + 'OC\\KnownUser\\KnownUserService' => $baseDir . '/lib/private/KnownUser/KnownUserService.php', 'OC\\L10N\\Factory' => $baseDir . '/lib/private/L10N/Factory.php', 'OC\\L10N\\L10N' => $baseDir . '/lib/private/L10N/L10N.php', 'OC\\L10N\\L10NString' => $baseDir . '/lib/private/L10N/L10NString.php', diff --git a/lib/composer/composer/autoload_static.php b/lib/composer/composer/autoload_static.php index bd49ecd7428..de07f1831bb 100644 --- a/lib/composer/composer/autoload_static.php +++ b/lib/composer/composer/autoload_static.php @@ -1197,6 +1197,7 @@ class ComposerStaticInit53792487c5a8370acc0b06b1a864ff4c 'OC\\IntegrityCheck\\Iterator\\ExcludeFoldersByPathFilterIterator' => __DIR__ . '/../../..' . '/lib/private/IntegrityCheck/Iterator/ExcludeFoldersByPathFilterIterator.php', 'OC\\KnownUser\\KnownUser' => __DIR__ . '/../../..' . '/lib/private/KnownUser/KnownUser.php', 'OC\\KnownUser\\KnownUserMapper' => __DIR__ . '/../../..' . '/lib/private/KnownUser/KnownUserMapper.php', + 'OC\\KnownUser\\KnownUserService' => __DIR__ . '/../../..' . '/lib/private/KnownUser/KnownUserService.php', 'OC\\L10N\\Factory' => __DIR__ . '/../../..' . '/lib/private/L10N/Factory.php', 'OC\\L10N\\L10N' => __DIR__ . '/../../..' . '/lib/private/L10N/L10N.php', 'OC\\L10N\\L10NString' => __DIR__ . '/../../..' . '/lib/private/L10N/L10NString.php', diff --git a/lib/private/KnownUser/KnownUserMapper.php b/lib/private/KnownUser/KnownUserMapper.php index 86e13a2e2f0..1144e2fd212 100644 --- a/lib/private/KnownUser/KnownUserMapper.php +++ b/lib/private/KnownUser/KnownUserMapper.php @@ -62,6 +62,19 @@ class KnownUserMapper extends QBMapper { return (int) $query->execute(); } + /** + * @param string $knownTo + * @return KnownUser[] + */ + public function getKnownTo(string $knownTo): array { + $query = $this->db->getQueryBuilder(); + $query->select('*') + ->from($this->getTableName()) + ->where($query->expr()->eq('known_to', $query->createNamedParameter($knownTo))); + + return $this->findEntities($query); + } + public function createKnownUserFromRow(array $row): KnownUser { return $this->mapRowToEntity([ 'id' => $row['s_id'], diff --git a/lib/private/KnownUser/KnownUserService.php b/lib/private/KnownUser/KnownUserService.php new file mode 100644 index 00000000000..3a97b967c3a --- /dev/null +++ b/lib/private/KnownUser/KnownUserService.php @@ -0,0 +1,62 @@ + + * + * @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\KnownUser; + +class KnownUserService { + /** @var KnownUserMapper */ + protected $mapper; + /** @var array */ + protected $knownUsers = []; + + public function __construct(KnownUserMapper $mapper) { + $this->mapper = $mapper; + } + + public function deleteKnownTo(string $knownTo): int { + return $this->mapper->deleteKnownTo($knownTo); + } + + public function deleteKnownUser(string $knownUser): int { + return $this->mapper->deleteKnownUser($knownUser); + } + + public function storeIsKnownToUser(string $knownTo, string $knownUser): void { + $entity = new KnownUser(); + $entity->setKnownTo($knownTo); + $entity->setKnownUser($knownUser); + $this->mapper->insert($entity); + } + + public function isKnownToUser(string $knownTo, string $user): bool { + if (!isset($this->knownUsers[$knownTo])) { + $entities = $this->mapper->getKnownTo($knownTo); + $this->knownUsers[$knownTo] = []; + foreach ($entities as $entity) { + $this->knownUsers[$knownTo][$entity->getKnownUser()] = true; + } + } + + return isset($this->knownUsers[$knownTo][$user]); + } +} -- cgit v1.2.3 From b71268e38b96e69057824e0eeb8f937ad015a927 Mon Sep 17 00:00:00 2001 From: Joas Schilling Date: Tue, 9 Mar 2021 21:46:42 +0100 Subject: Add a config setting to restrict autocompletion to phonebook matches Signed-off-by: Joas Schilling --- apps/settings/js/admin.js | 1 + apps/settings/lib/Settings/Admin/Sharing.php | 1 + apps/settings/templates/settings/admin/sharing.php | 12 +- apps/settings/tests/Settings/Admin/SharingTest.php | 221 ++++----------------- 4 files changed, 57 insertions(+), 178 deletions(-) (limited to 'apps') diff --git a/apps/settings/js/admin.js b/apps/settings/js/admin.js index cffaefa3821..72b167d7e0d 100644 --- a/apps/settings/js/admin.js +++ b/apps/settings/js/admin.js @@ -144,6 +144,7 @@ window.addEventListener('DOMContentLoaded', function(){ $('#shareapi_allow_share_dialog_user_enumeration').on('change', function() { $('#shareapi_restrict_user_enumeration_to_group_setting').toggleClass('hidden', !this.checked); + $('#shareapi_restrict_user_enumeration_to_phone_setting').toggleClass('hidden', !this.checked); }) $('#allowLinks').change(function() { diff --git a/apps/settings/lib/Settings/Admin/Sharing.php b/apps/settings/lib/Settings/Admin/Sharing.php index 313a182501d..19eed576cd7 100644 --- a/apps/settings/lib/Settings/Admin/Sharing.php +++ b/apps/settings/lib/Settings/Admin/Sharing.php @@ -73,6 +73,7 @@ class Sharing implements ISettings { 'allowResharing' => $this->config->getAppValue('core', 'shareapi_allow_resharing', 'yes'), 'allowShareDialogUserEnumeration' => $this->config->getAppValue('core', 'shareapi_allow_share_dialog_user_enumeration', 'yes'), 'restrictUserEnumerationToGroup' => $this->config->getAppValue('core', 'shareapi_restrict_user_enumeration_to_group', 'no'), + 'restrictUserEnumerationToPhone' => $this->config->getAppValue('core', 'shareapi_restrict_user_enumeration_to_phone', 'no'), 'enforceLinkPassword' => Util::isPublicLinkPasswordRequired(), 'onlyShareWithGroupMembers' => $this->shareManager->shareWithGroupMembersOnly(), 'shareAPIEnabled' => $this->config->getAppValue('core', 'shareapi_enabled', 'yes'), diff --git a/apps/settings/templates/settings/admin/sharing.php b/apps/settings/templates/settings/admin/sharing.php index 9f651ce6d6c..b02a7d2764c 100644 --- a/apps/settings/templates/settings/admin/sharing.php +++ b/apps/settings/templates/settings/admin/sharing.php @@ -173,7 +173,17 @@ /> -
+
+

+ +

+ /> +

diff --git a/apps/settings/tests/Settings/Admin/SharingTest.php b/apps/settings/tests/Settings/Admin/SharingTest.php index 52e83f8ba7f..5d0794170a0 100644 --- a/apps/settings/tests/Settings/Admin/SharingTest.php +++ b/apps/settings/tests/Settings/Admin/SharingTest.php @@ -64,95 +64,28 @@ class SharingTest extends TestCase { public function testGetFormWithoutExcludedGroups() { $this->config - ->expects($this->at(0)) ->method('getAppValue') - ->with('core', 'shareapi_exclude_groups_list', '') - ->willReturn(''); - $this->config - ->expects($this->at(1)) - ->method('getAppValue') - ->with('core', 'shareapi_allow_group_sharing', 'yes') - ->willReturn('yes'); - $this->config - ->expects($this->at(2)) - ->method('getAppValue') - ->with('core', 'shareapi_allow_links', 'yes') - ->willReturn('yes'); - $this->config - ->expects($this->at(3)) - ->method('getAppValue') - ->with('core', 'shareapi_allow_public_upload', 'yes') - ->willReturn('yes'); - $this->config - ->expects($this->at(4)) - ->method('getAppValue') - ->with('core', 'shareapi_allow_resharing', 'yes') - ->willReturn('yes'); - $this->config - ->expects($this->at(5)) - ->method('getAppValue') - ->with('core', 'shareapi_allow_share_dialog_user_enumeration', 'yes') - ->willReturn('yes'); - $this->config - ->expects($this->at(6)) - ->method('getAppValue') - ->with('core', 'shareapi_restrict_user_enumeration_to_group', 'no') - ->willReturn('no'); - $this->config - ->expects($this->at(7)) - ->method('getAppValue') - ->with('core', 'shareapi_enabled', 'yes') - ->willReturn('yes'); - $this->config - ->expects($this->at(8)) - ->method('getAppValue') - ->with('core', 'shareapi_default_expire_date', 'no') - ->willReturn('no'); - $this->config - ->expects($this->at(9)) - ->method('getAppValue') - ->with('core', 'shareapi_expire_after_n_days', '7') - ->willReturn('7'); - $this->config - ->expects($this->at(10)) - ->method('getAppValue') - ->with('core', 'shareapi_enforce_expire_date', 'no') - ->willReturn('no'); - $this->config - ->expects($this->at(11)) - ->method('getAppValue') - ->with('core', 'shareapi_exclude_groups', 'no') - ->willReturn('no'); - $this->config - ->expects($this->at(12)) - ->method('getAppValue') - ->with('core', 'shareapi_public_link_disclaimertext', null) - ->willReturn('Lorem ipsum'); - $this->config - ->expects($this->at(13)) - ->method('getAppValue') - ->with('core', 'shareapi_enable_link_password_by_default', 'no') - ->willReturn('yes'); - $this->config - ->expects($this->at(14)) - ->method('getAppValue') - ->with('core', 'shareapi_default_permissions', Constants::PERMISSION_ALL) - ->willReturn(Constants::PERMISSION_ALL); - $this->config - ->expects($this->at(15)) - ->method('getAppValue') - ->with('core', 'shareapi_default_internal_expire_date', 'no') - ->willReturn('no'); - $this->config - ->expects($this->at(16)) - ->method('getAppValue') - ->with('core', 'shareapi_internal_expire_after_n_days', '7') - ->willReturn('7'); - $this->config - ->expects($this->at(17)) - ->method('getAppValue') - ->with('core', 'shareapi_enforce_internal_expire_date', 'no') - ->willReturn('no'); + ->willReturnMap([ + ['core', 'shareapi_exclude_groups_list', '', ''], + ['core', 'shareapi_allow_group_sharing', 'yes', 'yes'], + ['core', 'shareapi_allow_links', 'yes', 'yes'], + ['core', 'shareapi_allow_public_upload', 'yes', 'yes'], + ['core', 'shareapi_allow_resharing', 'yes', 'yes'], + ['core', 'shareapi_allow_share_dialog_user_enumeration', 'yes', 'yes'], + ['core', 'shareapi_restrict_user_enumeration_to_group', 'no', 'no'], + ['core', 'shareapi_restrict_user_enumeration_to_phone', 'no', 'no'], + ['core', 'shareapi_enabled', 'yes', 'yes'], + ['core', 'shareapi_default_expire_date', 'no', 'no'], + ['core', 'shareapi_expire_after_n_days', '7', '7'], + ['core', 'shareapi_enforce_expire_date', 'no', 'no'], + ['core', 'shareapi_exclude_groups', 'no', 'no'], + ['core', 'shareapi_public_link_disclaimertext', null, 'Lorem ipsum'], + ['core', 'shareapi_enable_link_password_by_default', 'no', 'yes'], + ['core', 'shareapi_default_permissions', Constants::PERMISSION_ALL, Constants::PERMISSION_ALL], + ['core', 'shareapi_default_internal_expire_date', 'no', 'no'], + ['core', 'shareapi_internal_expire_after_n_days', '7', '7'], + ['core', 'shareapi_enforce_internal_expire_date', 'no', 'no'], + ]); $expected = new TemplateResponse( 'settings', @@ -164,6 +97,7 @@ class SharingTest extends TestCase { 'allowResharing' => 'yes', 'allowShareDialogUserEnumeration' => 'yes', 'restrictUserEnumerationToGroup' => 'no', + 'restrictUserEnumerationToPhone' => 'no', 'enforceLinkPassword' => false, 'onlyShareWithGroupMembers' => false, 'shareAPIEnabled' => 'yes', @@ -188,96 +122,28 @@ class SharingTest extends TestCase { public function testGetFormWithExcludedGroups() { $this->config - ->expects($this->at(0)) ->method('getAppValue') - ->with('core', 'shareapi_exclude_groups_list', '') - ->willReturn('["NoSharers","OtherNoSharers"]'); - $this->config - ->expects($this->at(1)) - ->method('getAppValue') - ->with('core', 'shareapi_allow_group_sharing', 'yes') - ->willReturn('yes'); - $this->config - ->expects($this->at(2)) - ->method('getAppValue') - ->with('core', 'shareapi_allow_links', 'yes') - ->willReturn('yes'); - $this->config - ->expects($this->at(3)) - ->method('getAppValue') - ->with('core', 'shareapi_allow_public_upload', 'yes') - ->willReturn('yes'); - $this->config - ->expects($this->at(4)) - ->method('getAppValue') - ->with('core', 'shareapi_allow_resharing', 'yes') - ->willReturn('yes'); - $this->config - ->expects($this->at(5)) - ->method('getAppValue') - ->with('core', 'shareapi_allow_share_dialog_user_enumeration', 'yes') - ->willReturn('yes'); - $this->config - ->expects($this->at(6)) - ->method('getAppValue') - ->with('core', 'shareapi_restrict_user_enumeration_to_group', 'no') - ->willReturn('no'); - $this->config - ->expects($this->at(7)) - ->method('getAppValue') - ->with('core', 'shareapi_enabled', 'yes') - ->willReturn('yes'); - $this->config - ->expects($this->at(8)) - ->method('getAppValue') - ->with('core', 'shareapi_default_expire_date', 'no') - ->willReturn('no'); - $this->config - ->expects($this->at(9)) - ->method('getAppValue') - ->with('core', 'shareapi_expire_after_n_days', '7') - ->willReturn('7'); - $this->config - ->expects($this->at(10)) - ->method('getAppValue') - ->with('core', 'shareapi_enforce_expire_date', 'no') - ->willReturn('no'); - $this->config - ->expects($this->at(11)) - ->method('getAppValue') - ->with('core', 'shareapi_exclude_groups', 'no') - ->willReturn('yes'); - $this->config - ->expects($this->at(12)) - ->method('getAppValue') - ->with('core', 'shareapi_public_link_disclaimertext', null) - ->willReturn('Lorem ipsum'); - $this->config - ->expects($this->at(13)) - ->method('getAppValue') - ->with('core', 'shareapi_enable_link_password_by_default', 'no') - ->willReturn('yes'); - $this->config - ->expects($this->at(14)) - ->method('getAppValue') - ->with('core', 'shareapi_default_permissions', Constants::PERMISSION_ALL) - ->willReturn(Constants::PERMISSION_ALL); - $this->config - ->expects($this->at(15)) - ->method('getAppValue') - ->with('core', 'shareapi_default_internal_expire_date', 'no') - ->willReturn('no'); - $this->config - ->expects($this->at(16)) - ->method('getAppValue') - ->with('core', 'shareapi_internal_expire_after_n_days', '7') - ->willReturn('7'); - $this->config - ->expects($this->at(17)) - ->method('getAppValue') - ->with('core', 'shareapi_enforce_internal_expire_date', 'no') - ->willReturn('no'); - + ->willReturnMap([ + ['core', 'shareapi_exclude_groups_list', '', '["NoSharers","OtherNoSharers"]'], + ['core', 'shareapi_allow_group_sharing', 'yes', 'yes'], + ['core', 'shareapi_allow_links', 'yes', 'yes'], + ['core', 'shareapi_allow_public_upload', 'yes', 'yes'], + ['core', 'shareapi_allow_resharing', 'yes', 'yes'], + ['core', 'shareapi_allow_share_dialog_user_enumeration', 'yes', 'yes'], + ['core', 'shareapi_restrict_user_enumeration_to_group', 'no', 'no'], + ['core', 'shareapi_restrict_user_enumeration_to_phone', 'no', 'no'], + ['core', 'shareapi_enabled', 'yes', 'yes'], + ['core', 'shareapi_default_expire_date', 'no', 'no'], + ['core', 'shareapi_expire_after_n_days', '7', '7'], + ['core', 'shareapi_enforce_expire_date', 'no', 'no'], + ['core', 'shareapi_exclude_groups', 'no', 'yes'], + ['core', 'shareapi_public_link_disclaimertext', null, 'Lorem ipsum'], + ['core', 'shareapi_enable_link_password_by_default', 'no', 'yes'], + ['core', 'shareapi_default_permissions', Constants::PERMISSION_ALL, Constants::PERMISSION_ALL], + ['core', 'shareapi_default_internal_expire_date', 'no', 'no'], + ['core', 'shareapi_internal_expire_after_n_days', '7', '7'], + ['core', 'shareapi_enforce_internal_expire_date', 'no', 'no'], + ]); $expected = new TemplateResponse( 'settings', @@ -289,6 +155,7 @@ class SharingTest extends TestCase { 'allowResharing' => 'yes', 'allowShareDialogUserEnumeration' => 'yes', 'restrictUserEnumerationToGroup' => 'no', + 'restrictUserEnumerationToPhone' => 'no', 'enforceLinkPassword' => false, 'onlyShareWithGroupMembers' => false, 'shareAPIEnabled' => 'yes', -- cgit v1.2.3 From 236aa194e2704454aa0b21228773071e3223a719 Mon Sep 17 00:00:00 2001 From: Joas Schilling Date: Tue, 9 Mar 2021 21:48:48 +0100 Subject: Restrict autocompletion also based on the phonebook known users Signed-off-by: Joas Schilling --- apps/dav/appinfo/v1/caldav.php | 2 + apps/dav/appinfo/v1/carddav.php | 2 + apps/dav/lib/CardDAV/SystemAddressbook.php | 5 +- apps/dav/lib/Command/CreateCalendar.php | 2 + apps/dav/lib/Connector/Sabre/Principal.php | 88 +++-- apps/dav/lib/RootCollection.php | 2 + .../tests/unit/CalDAV/AbstractCalDavBackend.php | 2 + apps/dav/tests/unit/CardDAV/CardDavBackendTest.php | 2 + .../tests/unit/Connector/Sabre/PrincipalTest.php | 8 +- apps/files_versions/lib/AppInfo/Application.php | 2 + .../Collaboration/Collaborators/MailPlugin.php | 33 +- .../Collaboration/Collaborators/UserPlugin.php | 29 +- .../Contacts/ContactsMenu/ContactsStore.php | 106 +++--- lib/private/Share20/Manager.php | 5 + lib/public/Share/IManager.php | 8 + .../Collaboration/Collaborators/MailPluginTest.php | 14 +- .../Collaboration/Collaborators/UserPluginTest.php | 7 + .../Contacts/ContactsMenu/ContactsStoreTest.php | 379 ++++++++++++++++++--- 18 files changed, 556 insertions(+), 140 deletions(-) (limited to 'apps') diff --git a/apps/dav/appinfo/v1/caldav.php b/apps/dav/appinfo/v1/caldav.php index e04653ddea1..236d81f66f8 100644 --- a/apps/dav/appinfo/v1/caldav.php +++ b/apps/dav/appinfo/v1/caldav.php @@ -27,6 +27,7 @@ */ // Backends +use OC\KnownUser\KnownUserService; use OCA\DAV\CalDAV\CalDavBackend; use OCA\DAV\Connector\LegacyDAVACL; use OCA\DAV\CalDAV\CalendarRoot; @@ -50,6 +51,7 @@ $principalBackend = new Principal( \OC::$server->getUserSession(), \OC::$server->getAppManager(), \OC::$server->query(\OCA\DAV\CalDAV\Proxy\ProxyMapper::class), + \OC::$server->get(KnownUserService::class), \OC::$server->getConfig(), 'principals/' ); diff --git a/apps/dav/appinfo/v1/carddav.php b/apps/dav/appinfo/v1/carddav.php index dbab1ae9681..bb766bbaeca 100644 --- a/apps/dav/appinfo/v1/carddav.php +++ b/apps/dav/appinfo/v1/carddav.php @@ -27,6 +27,7 @@ */ // Backends +use OC\KnownUser\KnownUserService; use OCA\DAV\AppInfo\PluginManager; use OCA\DAV\CardDAV\AddressBookRoot; use OCA\DAV\CardDAV\CardDavBackend; @@ -53,6 +54,7 @@ $principalBackend = new Principal( \OC::$server->getUserSession(), \OC::$server->getAppManager(), \OC::$server->query(\OCA\DAV\CalDAV\Proxy\ProxyMapper::class), + \OC::$server->get(KnownUserService::class), \OC::$server->getConfig(), 'principals/' ); diff --git a/apps/dav/lib/CardDAV/SystemAddressbook.php b/apps/dav/lib/CardDAV/SystemAddressbook.php index c7190c81319..5b952152711 100644 --- a/apps/dav/lib/CardDAV/SystemAddressbook.php +++ b/apps/dav/lib/CardDAV/SystemAddressbook.php @@ -43,8 +43,9 @@ class SystemAddressbook extends AddressBook { public function getChildren() { $shareEnumeration = $this->config->getAppValue('core', 'shareapi_allow_share_dialog_user_enumeration', 'yes') === 'yes'; - $restrictShareEnumeration = $this->config->getAppValue('core', 'shareapi_restrict_user_enumeration_to_group', 'no') === 'yes'; - if (!$shareEnumeration || ($shareEnumeration && $restrictShareEnumeration)) { + $shareEnumerationGroup = $this->config->getAppValue('core', 'shareapi_restrict_user_enumeration_to_group', 'no') === 'yes'; + $shareEnumerationPhone = $this->config->getAppValue('core', 'shareapi_restrict_user_enumeration_to_phone', 'no') === 'yes'; + if (!$shareEnumeration || $shareEnumerationGroup || $shareEnumerationPhone) { return []; } diff --git a/apps/dav/lib/Command/CreateCalendar.php b/apps/dav/lib/Command/CreateCalendar.php index 58c6a8c63fb..1d543c71bc2 100644 --- a/apps/dav/lib/Command/CreateCalendar.php +++ b/apps/dav/lib/Command/CreateCalendar.php @@ -27,6 +27,7 @@ namespace OCA\DAV\Command; +use OC\KnownUser\KnownUserService; use OCA\DAV\CalDAV\CalDavBackend; use OCA\DAV\CalDAV\Proxy\ProxyMapper; use OCA\DAV\Connector\Sabre\Principal; @@ -86,6 +87,7 @@ class CreateCalendar extends Command { \OC::$server->getUserSession(), \OC::$server->getAppManager(), \OC::$server->query(ProxyMapper::class), + \OC::$server->get(KnownUserService::class), \OC::$server->getConfig() ); $random = \OC::$server->getSecureRandom(); diff --git a/apps/dav/lib/Connector/Sabre/Principal.php b/apps/dav/lib/Connector/Sabre/Principal.php index 2351094a917..bb456c954e9 100644 --- a/apps/dav/lib/Connector/Sabre/Principal.php +++ b/apps/dav/lib/Connector/Sabre/Principal.php @@ -36,6 +36,7 @@ namespace OCA\DAV\Connector\Sabre; +use OC\KnownUser\KnownUserService; use OCA\Circles\Exceptions\CircleDoesNotExistException; use OCA\DAV\CalDAV\Proxy\ProxyMapper; use OCA\DAV\Traits\PrincipalProxyTrait; @@ -82,27 +83,19 @@ class Principal implements BackendInterface { /** @var ProxyMapper */ private $proxyMapper; + /** @var KnownUserService */ + private $knownUserService; + /** @var IConfig */ private $config; - /** - * Principal constructor. - * - * @param IUserManager $userManager - * @param IGroupManager $groupManager - * @param IShareManager $shareManager - * @param IUserSession $userSession - * @param IAppManager $appManager - * @param ProxyMapper $proxyMapper - * @param IConfig $config - * @param string $principalPrefix - */ public function __construct(IUserManager $userManager, IGroupManager $groupManager, IShareManager $shareManager, IUserSession $userSession, IAppManager $appManager, ProxyMapper $proxyMapper, + KnownUserService $knownUserService, IConfig $config, string $principalPrefix = 'principals/users/') { $this->userManager = $userManager; @@ -113,6 +106,7 @@ class Principal implements BackendInterface { $this->principalPrefix = trim($principalPrefix, '/'); $this->hasGroups = $this->hasCircles = ($principalPrefix === 'principals/users/'); $this->proxyMapper = $proxyMapper; + $this->knownUserService = $knownUserService; $this->config = $config; } @@ -267,24 +261,24 @@ class Principal implements BackendInterface { } $allowEnumeration = $this->shareManager->allowEnumeration(); - $limitEnumeration = $this->shareManager->limitEnumerationToGroups(); + $limitEnumerationGroup = $this->shareManager->limitEnumerationToGroups(); + $limitEnumerationPhone = $this->shareManager->limitEnumerationToPhone(); // If sharing is restricted to group members only, // return only members that have groups in common $restrictGroups = false; + $currentUser = $this->userSession->getUser(); if ($this->shareManager->shareWithGroupMembersOnly()) { - $user = $this->userSession->getUser(); - if (!$user) { + if (!$currentUser instanceof IUser) { return []; } - $restrictGroups = $this->groupManager->getUserGroupIds($user); + $restrictGroups = $this->groupManager->getUserGroupIds($currentUser); } $currentUserGroups = []; - if ($limitEnumeration) { - $currentUser = $this->userSession->getUser(); - if ($currentUser) { + if ($limitEnumerationGroup) { + if ($currentUser instanceof IUser) { $currentUserGroups = $this->groupManager->getUserGroupIds($currentUser); } } @@ -302,14 +296,28 @@ class Principal implements BackendInterface { $users = \array_filter($users, static function (IUser $user) use ($value) { return $user->getEMailAddress() === $value; }); - } + } else { + $users = \array_filter($users, function (IUser $user) use ($currentUser, $value, $limitEnumerationPhone, $limitEnumerationGroup, $currentUserGroups) { + if ($user->getEMailAddress() === $value) { + return true; + } - if ($limitEnumeration) { - $users = \array_filter($users, function (IUser $user) use ($currentUserGroups, $value) { - return !empty(array_intersect( - $this->groupManager->getUserGroupIds($user), - $currentUserGroups - )) || $user->getEMailAddress() === $value; + if ($limitEnumerationPhone + && $currentUser instanceof IUser + && $this->knownUserService->isKnownToUser($currentUser->getUID(), $user->getUID())) { + // Synced phonebook match + return true; + } + + if (!$limitEnumerationGroup) { + // No limitation on enumeration, all allowed + return true; + } + + return !empty($currentUserGroups) && !empty(array_intersect( + $this->groupManager->getUserGroupIds($user), + $currentUserGroups + )); }); } @@ -334,14 +342,28 @@ class Principal implements BackendInterface { $users = \array_filter($users, static function (IUser $user) use ($value) { return $user->getDisplayName() === $value; }); - } + } else { + $users = \array_filter($users, function (IUser $user) use ($currentUser, $value, $limitEnumerationPhone, $limitEnumerationGroup, $currentUserGroups) { + if ($user->getDisplayName() === $value) { + return true; + } + + if ($limitEnumerationPhone + && $currentUser instanceof IUser + && $this->knownUserService->isKnownToUser($currentUser->getUID(), $user->getUID())) { + // Synced phonebook match + return true; + } + + if (!$limitEnumerationGroup) { + // No limitation on enumeration, all allowed + return true; + } - if ($limitEnumeration) { - $users = \array_filter($users, function (IUser $user) use ($currentUserGroups, $value) { - return !empty(array_intersect( - $this->groupManager->getUserGroupIds($user), - $currentUserGroups - )) || $user->getDisplayName() === $value; + return !empty($currentUserGroups) && !empty(array_intersect( + $this->groupManager->getUserGroupIds($user), + $currentUserGroups + )); }); } diff --git a/apps/dav/lib/RootCollection.php b/apps/dav/lib/RootCollection.php index 18874ecf748..16a209a98f0 100644 --- a/apps/dav/lib/RootCollection.php +++ b/apps/dav/lib/RootCollection.php @@ -28,6 +28,7 @@ namespace OCA\DAV; +use OC\KnownUser\KnownUserService; use OCA\DAV\AppInfo\PluginManager; use OCA\DAV\CalDAV\CalDavBackend; use OCA\DAV\CalDAV\CalendarRoot; @@ -70,6 +71,7 @@ class RootCollection extends SimpleCollection { \OC::$server->getUserSession(), \OC::$server->getAppManager(), $proxyMapper, + \OC::$server->get(KnownUserService::class), \OC::$server->getConfig() ); $groupPrincipalBackend = new GroupPrincipalBackend($groupManager, $userSession, $shareManager, $config); diff --git a/apps/dav/tests/unit/CalDAV/AbstractCalDavBackend.php b/apps/dav/tests/unit/CalDAV/AbstractCalDavBackend.php index 85efd0fd369..51ba8c1867a 100644 --- a/apps/dav/tests/unit/CalDAV/AbstractCalDavBackend.php +++ b/apps/dav/tests/unit/CalDAV/AbstractCalDavBackend.php @@ -27,6 +27,7 @@ namespace OCA\DAV\Tests\unit\CalDAV; +use OC\KnownUser\KnownUserService; use OCA\DAV\CalDAV\CalDavBackend; use OCA\DAV\CalDAV\Proxy\ProxyMapper; use OCA\DAV\Connector\Sabre\Principal; @@ -92,6 +93,7 @@ abstract class AbstractCalDavBackend extends TestCase { $this->createMock(IUserSession::class), $this->createMock(IAppManager::class), $this->createMock(ProxyMapper::class), + $this->createMock(KnownUserService::class), $this->createMock(IConfig::class), ]) ->setMethods(['getPrincipalByPath', 'getGroupMembership']) diff --git a/apps/dav/tests/unit/CardDAV/CardDavBackendTest.php b/apps/dav/tests/unit/CardDAV/CardDavBackendTest.php index a8c7a781724..60f46ce8fac 100644 --- a/apps/dav/tests/unit/CardDAV/CardDavBackendTest.php +++ b/apps/dav/tests/unit/CardDAV/CardDavBackendTest.php @@ -33,6 +33,7 @@ namespace OCA\DAV\Tests\unit\CardDAV; +use OC\KnownUser\KnownUserService; use OCA\DAV\CalDAV\Proxy\ProxyMapper; use OCA\DAV\CardDAV\AddressBook; use OCA\DAV\CardDAV\CardDavBackend; @@ -139,6 +140,7 @@ class CardDavBackendTest extends TestCase { $this->createMock(IUserSession::class), $this->createMock(IAppManager::class), $this->createMock(ProxyMapper::class), + $this->createMock(KnownUserService::class), $this->createMock(IConfig::class), ]) ->setMethods(['getPrincipalByPath', 'getGroupMembership']) diff --git a/apps/dav/tests/unit/Connector/Sabre/PrincipalTest.php b/apps/dav/tests/unit/Connector/Sabre/PrincipalTest.php index 117707eaf2a..33c1ec1b587 100644 --- a/apps/dav/tests/unit/Connector/Sabre/PrincipalTest.php +++ b/apps/dav/tests/unit/Connector/Sabre/PrincipalTest.php @@ -30,6 +30,7 @@ namespace OCA\DAV\Tests\unit\Connector\Sabre; +use OC\KnownUser\KnownUserService; use OC\User\User; use OCA\DAV\CalDAV\Proxy\Proxy; use OCA\DAV\CalDAV\Proxy\ProxyMapper; @@ -41,6 +42,7 @@ use OCP\IUser; use OCP\IUserManager; use OCP\IUserSession; use OCP\Share\IManager; +use PHPUnit\Framework\MockObject\MockObject; use Sabre\DAV\PropPatch; use Test\TestCase; @@ -67,6 +69,8 @@ class PrincipalTest extends TestCase { /** @var ProxyMapper | \PHPUnit\Framework\MockObject\MockObject */ private $proxyMapper; + /** @var KnownUserService|MockObject */ + private $knownUserService; /** @var IConfig | \PHPUnit\Framework\MockObject\MockObject */ private $config; @@ -77,6 +81,7 @@ class PrincipalTest extends TestCase { $this->userSession = $this->createMock(IUserSession::class); $this->appManager = $this->createMock(IAppManager::class); $this->proxyMapper = $this->createMock(ProxyMapper::class); + $this->knownUserService = $this->createMock(KnownUserService::class); $this->config = $this->createMock(IConfig::class); $this->connector = new \OCA\DAV\Connector\Sabre\Principal( @@ -86,6 +91,7 @@ class PrincipalTest extends TestCase { $this->userSession, $this->appManager, $this->proxyMapper, + $this->knownUserService, $this->config ); parent::setUp(); @@ -442,7 +448,7 @@ class PrincipalTest extends TestCase { if ($groupsOnly) { $user = $this->createMock(IUser::class); - $this->userSession->expects($this->once()) + $this->userSession->expects($this->atLeastOnce()) ->method('getUser') ->willReturn($user); diff --git a/apps/files_versions/lib/AppInfo/Application.php b/apps/files_versions/lib/AppInfo/Application.php index afbd42ffc3f..e09ad7e90ae 100644 --- a/apps/files_versions/lib/AppInfo/Application.php +++ b/apps/files_versions/lib/AppInfo/Application.php @@ -27,6 +27,7 @@ namespace OCA\Files_Versions\AppInfo; +use OC\KnownUser\KnownUserService; use OCA\DAV\CalDAV\Proxy\ProxyMapper; use OCA\DAV\Connector\Sabre\Principal; use OCA\Files\Event\LoadAdditionalScriptsEvent; @@ -72,6 +73,7 @@ class Application extends App implements IBootstrap { $server->getUserSession(), $server->getAppManager(), $server->get(ProxyMapper::class), + $server->get(KnownUserService::class), $server->getConfig() ); }); diff --git a/lib/private/Collaboration/Collaborators/MailPlugin.php b/lib/private/Collaboration/Collaborators/MailPlugin.php index 7bdd29afc4e..7da8cede6aa 100644 --- a/lib/private/Collaboration/Collaborators/MailPlugin.php +++ b/lib/private/Collaboration/Collaborators/MailPlugin.php @@ -27,6 +27,7 @@ namespace OC\Collaboration\Collaborators; +use OC\KnownUser\KnownUserService; use OCP\Collaboration\Collaborators\ISearchPlugin; use OCP\Collaboration\Collaborators\ISearchResult; use OCP\Collaboration\Collaborators\SearchResultType; @@ -40,8 +41,14 @@ use OCP\IUserSession; use OCP\Share\IShare; class MailPlugin implements ISearchPlugin { - protected $shareeEnumeration; + /* @var bool */ protected $shareWithGroupOnly; + /* @var bool */ + protected $shareeEnumeration; + /* @var bool */ + protected $shareeEnumerationInGroupOnly; + /* @var bool */ + protected $shareeEnumerationPhone; /** @var IManager */ private $contactsManager; @@ -52,20 +59,28 @@ class MailPlugin implements ISearchPlugin { /** @var IGroupManager */ private $groupManager; - + /** @var KnownUserService */ + private $knownUserService; /** @var IUserSession */ private $userSession; - public function __construct(IManager $contactsManager, ICloudIdManager $cloudIdManager, IConfig $config, IGroupManager $groupManager, IUserSession $userSession) { + public function __construct(IManager $contactsManager, + ICloudIdManager $cloudIdManager, + IConfig $config, + IGroupManager $groupManager, + KnownUserService $knownUserService, + IUserSession $userSession) { $this->contactsManager = $contactsManager; $this->cloudIdManager = $cloudIdManager; $this->config = $config; $this->groupManager = $groupManager; + $this->knownUserService = $knownUserService; $this->userSession = $userSession; $this->shareeEnumeration = $this->config->getAppValue('core', 'shareapi_allow_share_dialog_user_enumeration', 'yes') === 'yes'; $this->shareWithGroupOnly = $this->config->getAppValue('core', 'shareapi_only_share_with_group_members', 'no') === 'yes'; $this->shareeEnumerationInGroupOnly = $this->shareeEnumeration && $this->config->getAppValue('core', 'shareapi_restrict_user_enumeration_to_group', 'no') === 'yes'; + $this->shareeEnumerationPhone = $this->shareeEnumeration && $this->config->getAppValue('core', 'shareapi_restrict_user_enumeration_to_phone', 'no') === 'yes'; } /** @@ -77,6 +92,8 @@ class MailPlugin implements ISearchPlugin { * @since 13.0.0 */ public function search($search, $limit, $offset, ISearchResult $searchResult) { + $currentUserId = $this->userSession->getUser()->getUID(); + $result = $userResults = ['wide' => [], 'exact' => []]; $userType = new SearchResultType('users'); $emailType = new SearchResultType('emails'); @@ -152,8 +169,12 @@ class MailPlugin implements ISearchPlugin { continue; } - $addToWide = !$this->shareeEnumerationInGroupOnly; - if ($this->shareeEnumerationInGroupOnly) { + $addToWide = !($this->shareeEnumerationInGroupOnly || $this->shareeEnumerationPhone); + if (!$addToWide && $this->shareeEnumerationPhone && $this->knownUserService->isKnownToUser($currentUserId, $contact['UID'])) { + $addToWide = true; + } + + if (!$addToWide && $this->shareeEnumerationInGroupOnly) { $addToWide = false; $userGroups = $this->groupManager->getUserGroupIds($this->userSession->getUser()); foreach ($userGroups as $userGroup) { @@ -181,7 +202,7 @@ class MailPlugin implements ISearchPlugin { } if ($exactEmailMatch - || isset($contact['FN']) && strtolower($contact['FN']) === $lowerSearch) { + || (isset($contact['FN']) && strtolower($contact['FN']) === $lowerSearch)) { if ($exactEmailMatch) { $searchResult->markExactIdMatch($emailType); } diff --git a/lib/private/Collaboration/Collaborators/UserPlugin.php b/lib/private/Collaboration/Collaborators/UserPlugin.php index d832a42000c..5114ccd8eb5 100644 --- a/lib/private/Collaboration/Collaborators/UserPlugin.php +++ b/lib/private/Collaboration/Collaborators/UserPlugin.php @@ -32,6 +32,7 @@ namespace OC\Collaboration\Collaborators; +use OC\KnownUser\KnownUserService; use OCP\Collaboration\Collaborators\ISearchPlugin; use OCP\Collaboration\Collaborators\ISearchResult; use OCP\Collaboration\Collaborators\SearchResultType; @@ -46,8 +47,12 @@ use OCP\UserStatus\IManager as IUserStatusManager; class UserPlugin implements ISearchPlugin { /* @var bool */ protected $shareWithGroupOnly; + /* @var bool */ protected $shareeEnumeration; + /* @var bool */ protected $shareeEnumerationInGroupOnly; + /* @var bool */ + protected $shareeEnumerationPhone; /** @var IConfig */ private $config; @@ -57,33 +62,29 @@ class UserPlugin implements ISearchPlugin { private $userSession; /** @var IUserManager */ private $userManager; + /** @var KnownUserService */ + private $knownUserService; /** @var IUserStatusManager */ private $userStatusManager; - /** - * UserPlugin constructor. - * - * @param IConfig $config - * @param IUserManager $userManager - * @param IGroupManager $groupManager - * @param IUserSession $userSession - * @param IUserStatusManager $userStatusManager - */ public function __construct(IConfig $config, IUserManager $userManager, IGroupManager $groupManager, IUserSession $userSession, + KnownUserService $knownUserService, IUserStatusManager $userStatusManager) { $this->config = $config; $this->groupManager = $groupManager; $this->userSession = $userSession; $this->userManager = $userManager; + $this->knownUserService = $knownUserService; $this->userStatusManager = $userStatusManager; $this->shareWithGroupOnly = $this->config->getAppValue('core', 'shareapi_only_share_with_group_members', 'no') === 'yes'; $this->shareeEnumeration = $this->config->getAppValue('core', 'shareapi_allow_share_dialog_user_enumeration', 'yes') === 'yes'; $this->shareeEnumerationInGroupOnly = $this->shareeEnumeration && $this->config->getAppValue('core', 'shareapi_restrict_user_enumeration_to_group', 'no') === 'yes'; + $this->shareeEnumerationPhone = $this->shareeEnumeration && $this->config->getAppValue('core', 'shareapi_restrict_user_enumeration_to_phone', 'no') === 'yes'; } public function search($search, $limit, $offset, ISearchResult $searchResult) { @@ -91,6 +92,7 @@ class UserPlugin implements ISearchPlugin { $users = []; $hasMoreResults = false; + $currentUserId = $this->userSession->getUser()->getUID(); $currentUserGroups = $this->groupManager->getUserGroupIds($this->userSession->getUser()); if ($this->shareWithGroupOnly) { // Search in all the groups this user is part of @@ -168,11 +170,16 @@ class UserPlugin implements ISearchPlugin { ]; } else { $addToWideResults = false; - if ($this->shareeEnumeration && !$this->shareeEnumerationInGroupOnly) { + if ($this->shareeEnumeration && + !($this->shareeEnumerationInGroupOnly || $this->shareeEnumerationPhone)) { + $addToWideResults = true; + } + + if ($this->shareeEnumerationPhone && $this->knownUserService->isKnownToUser($currentUserId, $user->getUID())) { $addToWideResults = true; } - if ($this->shareeEnumerationInGroupOnly) { + if (!$addToWideResults && $this->shareeEnumerationInGroupOnly) { $commonGroups = array_intersect($currentUserGroups, $this->groupManager->getUserGroupIds($user)); if (!empty($commonGroups)) { $addToWideResults = true; diff --git a/lib/private/Contacts/ContactsMenu/ContactsStore.php b/lib/private/Contacts/ContactsMenu/ContactsStore.php index e2bd7edc63d..852765506c0 100644 --- a/lib/private/Contacts/ContactsMenu/ContactsStore.php +++ b/lib/private/Contacts/ContactsMenu/ContactsStore.php @@ -31,6 +31,7 @@ namespace OC\Contacts\ContactsMenu; +use OC\KnownUser\KnownUserService; use OCP\Contacts\ContactsMenu\IContactsStore; use OCP\Contacts\ContactsMenu\IEntry; use OCP\Contacts\IManager; @@ -53,20 +54,19 @@ class ContactsStore implements IContactsStore { /** @var IGroupManager */ private $groupManager; - /** - * @param IManager $contactsManager - * @param IConfig $config - * @param IUserManager $userManager - * @param IGroupManager $groupManager - */ + /** @var KnownUserService */ + private $knownUserService; + public function __construct(IManager $contactsManager, IConfig $config, IUserManager $userManager, - IGroupManager $groupManager) { + IGroupManager $groupManager, + KnownUserService $knownUserService) { $this->contactsManager = $contactsManager; $this->config = $config; $this->userManager = $userManager; $this->groupManager = $groupManager; + $this->knownUserService = $knownUserService; } /** @@ -103,7 +103,7 @@ class ContactsStore implements IContactsStore { } /** - * Filters the contacts. Applies 3 filters: + * Filters the contacts. Applied filters: * 1. filter the current user * 2. if the `shareapi_allow_share_dialog_user_enumeration` config option is * enabled it will filter all local users @@ -122,20 +122,21 @@ class ContactsStore implements IContactsStore { array $entries, $filter) { $disallowEnumeration = $this->config->getAppValue('core', 'shareapi_allow_share_dialog_user_enumeration', 'yes') !== 'yes'; - $restrictEnumeration = $this->config->getAppValue('core', 'shareapi_restrict_user_enumeration_to_group', 'no') === 'yes'; + $restrictEnumerationGroup = $this->config->getAppValue('core', 'shareapi_restrict_user_enumeration_to_group', 'no') === 'yes'; + $restrictEnumerationPhone = $this->config->getAppValue('core', 'shareapi_restrict_user_enumeration_to_phone', 'no') === 'yes'; $excludedGroups = $this->config->getAppValue('core', 'shareapi_exclude_groups', 'no') === 'yes'; // whether to filter out local users $skipLocal = false; - // whether to filter out all users which doesn't have the same group as the current user - $ownGroupsOnly = $this->config->getAppValue('core', 'shareapi_only_share_with_group_members', 'no') === 'yes' || $restrictEnumeration; + // whether to filter out all users which don't have a common group as the current user + $ownGroupsOnly = $this->config->getAppValue('core', 'shareapi_only_share_with_group_members', 'no') === 'yes'; $selfGroups = $this->groupManager->getUserGroupIds($self); if ($excludedGroups) { $excludedGroups = $this->config->getAppValue('core', 'shareapi_exclude_groups_list', ''); $decodedExcludeGroups = json_decode($excludedGroups, true); - $excludeGroupsList = ($decodedExcludeGroups !== null) ? $decodedExcludeGroups : []; + $excludeGroupsList = $decodedExcludeGroups ?? []; if (count(array_intersect($excludeGroupsList, $selfGroups)) !== 0) { // a group of the current user is excluded -> filter all local users @@ -145,47 +146,76 @@ class ContactsStore implements IContactsStore { $selfUID = $self->getUID(); - return array_values(array_filter($entries, function (IEntry $entry) use ($self, $skipLocal, $ownGroupsOnly, $selfGroups, $selfUID, $disallowEnumeration, $filter) { - if ($skipLocal && $entry->getProperty('isLocalSystemBook') === true) { + return array_values(array_filter($entries, function (IEntry $entry) use ($skipLocal, $ownGroupsOnly, $selfGroups, $selfUID, $disallowEnumeration, $restrictEnumerationGroup, $restrictEnumerationPhone, $filter) { + if ($entry->getProperty('UID') === $selfUID) { return false; } - // Prevent enumerating local users - if ($disallowEnumeration && $entry->getProperty('isLocalSystemBook')) { - $filterUser = true; + if ($entry->getProperty('isLocalSystemBook')) { + if ($skipLocal) { + return false; + } + + $checkedCommonGroupAlready = false; + + // Prevent enumerating local users + if ($disallowEnumeration) { + $filterUser = true; - $mailAddresses = $entry->getEMailAddresses(); - foreach ($mailAddresses as $mailAddress) { - if ($mailAddress === $filter) { + $mailAddresses = $entry->getEMailAddresses(); + foreach ($mailAddresses as $mailAddress) { + if ($mailAddress === $filter) { + $filterUser = false; + break; + } + } + + if ($entry->getProperty('UID') && $entry->getProperty('UID') === $filter) { $filterUser = false; - break; } - } - if ($entry->getProperty('UID') && $entry->getProperty('UID') === $filter) { - $filterUser = false; - } + if ($filterUser) { + return false; + } + } elseif ($restrictEnumerationPhone || $restrictEnumerationGroup) { + $canEnumerate = false; + if ($restrictEnumerationPhone) { + $canEnumerate = $this->knownUserService->isKnownToUser($selfUID, $entry->getProperty('UID')); + } - if ($filterUser) { - return false; - } - } + if (!$canEnumerate && $restrictEnumerationGroup) { + $user = $this->userManager->get($entry->getProperty('UID')); - if ($ownGroupsOnly && $entry->getProperty('isLocalSystemBook') === true) { - $uid = $this->userManager->get($entry->getProperty('UID')); + if ($user === null) { + return false; + } - if ($uid === null) { - return false; + $contactGroups = $this->groupManager->getUserGroupIds($user); + $canEnumerate = !empty(array_intersect($contactGroups, $selfGroups)); + $checkedCommonGroupAlready = true; + } + + if (!$canEnumerate) { + return false; + } } - $contactGroups = $this->groupManager->getUserGroupIds($uid); - if (count(array_intersect($contactGroups, $selfGroups)) === 0) { - // no groups in common, so shouldn't see the contact - return false; + if ($ownGroupsOnly && !$checkedCommonGroupAlready) { + $user = $this->userManager->get($entry->getProperty('UID')); + + if ($user === null) { + return false; + } + + $contactGroups = $this->groupManager->getUserGroupIds($user); + if (empty(array_intersect($contactGroups, $selfGroups))) { + // no groups in common, so shouldn't see the contact + return false; + } } } - return $entry->getProperty('UID') !== $selfUID; + return true; })); } diff --git a/lib/private/Share20/Manager.php b/lib/private/Share20/Manager.php index 5c8dba5915a..6e072740884 100644 --- a/lib/private/Share20/Manager.php +++ b/lib/private/Share20/Manager.php @@ -1829,6 +1829,11 @@ class Manager implements IManager { $this->config->getAppValue('core', 'shareapi_restrict_user_enumeration_to_group', 'no') === 'yes'; } + public function limitEnumerationToPhone(): bool { + return $this->allowEnumeration() && + $this->config->getAppValue('core', 'shareapi_restrict_user_enumeration_to_phone', 'no') === 'yes'; + } + /** * Copied from \OC_Util::isSharingDisabledForUser * diff --git a/lib/public/Share/IManager.php b/lib/public/Share/IManager.php index 635ccc1483d..0c8732b4b15 100644 --- a/lib/public/Share/IManager.php +++ b/lib/public/Share/IManager.php @@ -384,6 +384,14 @@ interface IManager { */ public function limitEnumerationToGroups(): bool; + /** + * Check if user enumeration is limited to the phonebook matches + * + * @return bool + * @since 21.0.1 + */ + public function limitEnumerationToPhone(): bool; + /** * Check if sharing is disabled for the given user * diff --git a/tests/lib/Collaboration/Collaborators/MailPluginTest.php b/tests/lib/Collaboration/Collaborators/MailPluginTest.php index 141d4b680b7..3128231a108 100644 --- a/tests/lib/Collaboration/Collaborators/MailPluginTest.php +++ b/tests/lib/Collaboration/Collaborators/MailPluginTest.php @@ -26,6 +26,7 @@ namespace Test\Collaboration\Collaborators; use OC\Collaboration\Collaborators\MailPlugin; use OC\Collaboration\Collaborators\SearchResult; use OC\Federation\CloudIdManager; +use OC\KnownUser\KnownUserService; use OCP\Collaboration\Collaborators\SearchResultType; use OCP\Contacts\IManager; use OCP\Federation\ICloudIdManager; @@ -55,6 +56,9 @@ class MailPluginTest extends TestCase { /** @var IGroupManager|\PHPUnit\Framework\MockObject\MockObject */ protected $groupManager; + /** @var KnownUserService|\PHPUnit\Framework\MockObject\MockObject */ + protected $knownUserService; + /** @var IUserSession|\PHPUnit\Framework\MockObject\MockObject */ protected $userSession; @@ -64,6 +68,7 @@ class MailPluginTest extends TestCase { $this->config = $this->createMock(IConfig::class); $this->contactsManager = $this->createMock(IManager::class); $this->groupManager = $this->createMock(IGroupManager::class); + $this->knownUserService = $this->createMock(KnownUserService::class); $this->userSession = $this->createMock(IUserSession::class); $this->cloudIdManager = new CloudIdManager($this->contactsManager); @@ -71,7 +76,14 @@ class MailPluginTest extends TestCase { } public function instantiatePlugin() { - $this->plugin = new MailPlugin($this->contactsManager, $this->cloudIdManager, $this->config, $this->groupManager, $this->userSession); + $this->plugin = new MailPlugin( + $this->contactsManager, + $this->cloudIdManager, + $this->config, + $this->groupManager, + $this->knownUserService, + $this->userSession + ); } /** diff --git a/tests/lib/Collaboration/Collaborators/UserPluginTest.php b/tests/lib/Collaboration/Collaborators/UserPluginTest.php index 2806540d00e..f2e0e7e274b 100644 --- a/tests/lib/Collaboration/Collaborators/UserPluginTest.php +++ b/tests/lib/Collaboration/Collaborators/UserPluginTest.php @@ -25,6 +25,7 @@ namespace Test\Collaboration\Collaborators; use OC\Collaboration\Collaborators\SearchResult; use OC\Collaboration\Collaborators\UserPlugin; +use OC\KnownUser\KnownUserService; use OCP\Collaboration\Collaborators\ISearchResult; use OCP\IConfig; use OCP\IGroup; @@ -49,6 +50,9 @@ class UserPluginTest extends TestCase { /** @var IUserSession|\PHPUnit\Framework\MockObject\MockObject */ protected $session; + /** @var KnownUserService|\PHPUnit\Framework\MockObject\MockObject */ + protected $knownUserService; + /** @var IUserStatusManager|\PHPUnit\Framework\MockObject\MockObject */ protected $userStatusManager; @@ -78,6 +82,8 @@ class UserPluginTest extends TestCase { $this->session = $this->createMock(IUserSession::class); + $this->knownUserService = $this->createMock(KnownUserService::class); + $this->userStatusManager = $this->createMock(IUserStatusManager::class); $this->searchResult = new SearchResult(); @@ -93,6 +99,7 @@ class UserPluginTest extends TestCase { $this->userManager, $this->groupManager, $this->session, + $this->knownUserService, $this->userStatusManager ); } diff --git a/tests/lib/Contacts/ContactsMenu/ContactsStoreTest.php b/tests/lib/Contacts/ContactsMenu/ContactsStoreTest.php index acfe83ac558..ad83178096e 100644 --- a/tests/lib/Contacts/ContactsMenu/ContactsStoreTest.php +++ b/tests/lib/Contacts/ContactsMenu/ContactsStoreTest.php @@ -26,11 +26,13 @@ namespace Tests\Contacts\ContactsMenu; use OC\Contacts\ContactsMenu\ContactsStore; +use OC\KnownUser\KnownUserService; use OCP\Contacts\IManager; use OCP\IConfig; use OCP\IGroupManager; use OCP\IUser; use OCP\IUserManager; +use PHPUnit\Framework\MockObject\MockObject; use Test\TestCase; class ContactsStoreTest extends TestCase { @@ -44,6 +46,8 @@ class ContactsStoreTest extends TestCase { private $groupManager; /** @var IConfig|\PHPUnit\Framework\MockObject\MockObject */ private $config; + /** @var KnownUserService|MockObject */ + private $knownUserService; protected function setUp(): void { parent::setUp(); @@ -52,7 +56,14 @@ class ContactsStoreTest extends TestCase { $this->userManager = $this->createMock(IUserManager::class); $this->groupManager = $this->createMock(IGroupManager::class); $this->config = $this->createMock(IConfig::class); - $this->contactsStore = new ContactsStore($this->contactsManager, $this->config, $this->userManager, $this->groupManager); + $this->knownUserService = $this->createMock(KnownUserService::class); + $this->contactsStore = new ContactsStore( + $this->contactsManager, + $this->config, + $this->userManager, + $this->groupManager, + $this->knownUserService + ); } public function testGetContactsWithoutFilter() { @@ -171,29 +182,16 @@ class ContactsStoreTest extends TestCase { } public function testGetContactsWhenUserIsInExcludeGroups() { - $this->config->expects($this->at(0))->method('getAppValue') - ->with($this->equalTo('core'), $this->equalTo('shareapi_allow_share_dialog_user_enumeration'), $this->equalTo('yes')) - ->willReturn('yes'); - - $this->config->expects($this->at(1)) + $this->config ->method('getAppValue') - ->with($this->equalTo('core'), $this->equalTo('shareapi_restrict_user_enumeration_to_group'), $this->equalTo('no')) - ->willReturn('no'); - - $this->config->expects($this->at(2)) - ->method('getAppValue') - ->with($this->equalTo('core'), $this->equalTo('shareapi_exclude_groups'), $this->equalTo('no')) - ->willReturn('yes'); - - $this->config->expects($this->at(3)) - ->method('getAppValue') - ->with($this->equalTo('core'), $this->equalTo('shareapi_only_share_with_group_members'), $this->equalTo('no')) - ->willReturn('yes'); - - $this->config->expects($this->at(4)) - ->method('getAppValue') - ->with($this->equalTo('core'), $this->equalTo('shareapi_exclude_groups_list'), $this->equalTo('')) - ->willReturn('["group1", "group5", "group6"]'); + ->willReturnMap([ + ['core', 'shareapi_allow_share_dialog_user_enumeration', 'yes', 'yes'], + ['core', 'shareapi_restrict_user_enumeration_to_group', 'no', 'no'], + ['core', 'shareapi_restrict_user_enumeration_to_phone', 'no', 'no'], + ['core', 'shareapi_exclude_groups', 'no', 'yes'], + ['core', 'shareapi_only_share_with_group_members', 'no', 'yes'], + ['core', 'shareapi_exclude_groups_list', '', '["group1", "group5", "group6"]'], + ]); /** @var IUser|\PHPUnit\Framework\MockObject\MockObject $currentUser */ $currentUser = $this->createMock(IUser::class); @@ -228,22 +226,94 @@ class ContactsStoreTest extends TestCase { } public function testGetContactsOnlyShareIfInTheSameGroup() { - $this->config->expects($this->at(0))->method('getAppValue') - ->with($this->equalTo('core'), $this->equalTo('shareapi_allow_share_dialog_user_enumeration'), $this->equalTo('yes')) - ->willReturn('yes'); + $this->config + ->method('getAppValue') + ->willReturnMap([ + ['core', 'shareapi_allow_share_dialog_user_enumeration', 'yes', 'yes'], + ['core', 'shareapi_restrict_user_enumeration_to_group', 'no', 'no'], + ['core', 'shareapi_restrict_user_enumeration_to_phone', 'no', 'no'], + ['core', 'shareapi_exclude_groups', 'no', 'no'], + ['core', 'shareapi_only_share_with_group_members', 'no', 'yes'], + ]); - $this->config->expects($this->at(1)) ->method('getAppValue') - ->with($this->equalTo('core'), $this->equalTo('shareapi_restrict_user_enumeration_to_group'), $this->equalTo('no')) - ->willReturn('no'); + /** @var IUser|\PHPUnit\Framework\MockObject\MockObject $currentUser */ + $currentUser = $this->createMock(IUser::class); + $currentUser->expects($this->once()) + ->method('getUID') + ->willReturn('user001'); - $this->config->expects($this->at(2)) ->method('getAppValue') - ->with($this->equalTo('core'), $this->equalTo('shareapi_exclude_groups'), $this->equalTo('no')) - ->willReturn('no'); + $this->groupManager->expects($this->at(0)) + ->method('getUserGroupIds') + ->with($this->equalTo($currentUser)) + ->willReturn(['group1', 'group2', 'group3']); + + $user1 = $this->createMock(IUser::class); + $this->userManager->expects($this->at(0)) + ->method('get') + ->with('user1') + ->willReturn($user1); + $this->groupManager->expects($this->at(1)) + ->method('getUserGroupIds') + ->with($this->equalTo($user1)) + ->willReturn(['group1']); + $user2 = $this->createMock(IUser::class); + $this->userManager->expects($this->at(1)) + ->method('get') + ->with('user2') + ->willReturn($user2); + $this->groupManager->expects($this->at(2)) + ->method('getUserGroupIds') + ->with($this->equalTo($user2)) + ->willReturn(['group2', 'group3']); + $user3 = $this->createMock(IUser::class); + $this->userManager->expects($this->at(2)) + ->method('get') + ->with('user3') + ->willReturn($user3); + $this->groupManager->expects($this->at(3)) + ->method('getUserGroupIds') + ->with($this->equalTo($user3)) + ->willReturn(['group8', 'group9']); + + $this->contactsManager->expects($this->once()) + ->method('search') + ->with($this->equalTo(''), $this->equalTo(['FN', 'EMAIL'])) + ->willReturn([ + [ + 'UID' => 'user1', + 'isLocalSystemBook' => true + ], + [ + 'UID' => 'user2', + 'isLocalSystemBook' => true + ], + [ + 'UID' => 'user3', + 'isLocalSystemBook' => true + ], + [ + 'UID' => 'contact', + ], + ]); + + $entries = $this->contactsStore->getContacts($currentUser, ''); + + $this->assertCount(3, $entries); + $this->assertEquals('user1', $entries[0]->getProperty('UID')); + $this->assertEquals('user2', $entries[1]->getProperty('UID')); + $this->assertEquals('contact', $entries[2]->getProperty('UID')); + } - $this->config->expects($this->at(3)) + public function testGetContactsOnlyEnumerateIfInTheSameGroup() { + $this->config ->method('getAppValue') - ->with($this->equalTo('core'), $this->equalTo('shareapi_only_share_with_group_members'), $this->equalTo('no')) - ->willReturn('yes'); + ->willReturnMap([ + ['core', 'shareapi_allow_share_dialog_user_enumeration', 'yes', 'yes'], + ['core', 'shareapi_restrict_user_enumeration_to_group', 'no', 'yes'], + ['core', 'shareapi_restrict_user_enumeration_to_phone', 'no', 'no'], + ['core', 'shareapi_exclude_groups', 'no', 'no'], + ['core', 'shareapi_only_share_with_group_members', 'no', 'yes'], + ]); /** @var IUser|\PHPUnit\Framework\MockObject\MockObject $currentUser */ $currentUser = $this->createMock(IUser::class); @@ -313,23 +383,229 @@ class ContactsStoreTest extends TestCase { $this->assertEquals('contact', $entries[2]->getProperty('UID')); } - public function testGetContactsOnlyEnumerateIfInTheSameGroup() { - $this->config->expects($this->at(0))->method('getAppValue') - ->with($this->equalTo('core'), $this->equalTo('shareapi_allow_share_dialog_user_enumeration'), $this->equalTo('yes')) - ->willReturn('yes'); + public function testGetContactsOnlyEnumerateIfPhoneBookMatch() { + $this->config + ->method('getAppValue') + ->willReturnMap([ + ['core', 'shareapi_allow_share_dialog_user_enumeration', 'yes', 'yes'], + ['core', 'shareapi_restrict_user_enumeration_to_group', 'no', 'no'], + ['core', 'shareapi_restrict_user_enumeration_to_phone', 'no', 'yes'], + ['core', 'shareapi_exclude_groups', 'no', 'no'], + ['core', 'shareapi_only_share_with_group_members', 'no', 'no'], + ]); - $this->config->expects($this->at(1)) ->method('getAppValue') - ->with($this->equalTo('core'), $this->equalTo('shareapi_restrict_user_enumeration_to_group'), $this->equalTo('no')) - ->willReturn('yes'); + /** @var IUser|\PHPUnit\Framework\MockObject\MockObject $currentUser */ + $currentUser = $this->createMock(IUser::class); + $currentUser->expects($this->once()) + ->method('getUID') + ->willReturn('user001'); - $this->config->expects($this->at(2)) ->method('getAppValue') - ->with($this->equalTo('core'), $this->equalTo('shareapi_exclude_groups'), $this->equalTo('no')) - ->willReturn('no'); + $this->groupManager->expects($this->at(0)) + ->method('getUserGroupIds') + ->with($this->equalTo($currentUser)) + ->willReturn(['group1', 'group2', 'group3']); + + $this->knownUserService->method('isKnownToUser') + ->willReturnMap([ + ['user001', 'user1', true], + ['user001', 'user2', true], + ['user001', 'user3', false], + ]); + + $this->contactsManager->expects($this->once()) + ->method('search') + ->with($this->equalTo(''), $this->equalTo(['FN', 'EMAIL'])) + ->willReturn([ + [ + 'UID' => 'user1', + 'isLocalSystemBook' => true + ], + [ + 'UID' => 'user2', + 'isLocalSystemBook' => true + ], + [ + 'UID' => 'user3', + 'isLocalSystemBook' => true + ], + [ + 'UID' => 'contact', + ], + ]); + + $entries = $this->contactsStore->getContacts($currentUser, ''); + + $this->assertCount(3, $entries); + $this->assertEquals('user1', $entries[0]->getProperty('UID')); + $this->assertEquals('user2', $entries[1]->getProperty('UID')); + $this->assertEquals('contact', $entries[2]->getProperty('UID')); + } - $this->config->expects($this->at(3)) + public function testGetContactsOnlyEnumerateIfPhoneBookMatchWithOwnGroupsOnly() { + $this->config ->method('getAppValue') - ->with($this->equalTo('core'), $this->equalTo('shareapi_only_share_with_group_members'), $this->equalTo('no')) - ->willReturn('no'); + ->willReturnMap([ + ['core', 'shareapi_allow_share_dialog_user_enumeration', 'yes', 'yes'], + ['core', 'shareapi_restrict_user_enumeration_to_group', 'no', 'no'], + ['core', 'shareapi_restrict_user_enumeration_to_phone', 'no', 'yes'], + ['core', 'shareapi_exclude_groups', 'no', 'no'], + ['core', 'shareapi_only_share_with_group_members', 'no', 'yes'], + ]); + + /** @var IUser|\PHPUnit\Framework\MockObject\MockObject $currentUser */ + $currentUser = $this->createMock(IUser::class); + $currentUser->expects($this->once()) + ->method('getUID') + ->willReturn('user001'); + + $this->groupManager->expects($this->at(0)) + ->method('getUserGroupIds') + ->with($this->equalTo($currentUser)) + ->willReturn(['group1', 'group2', 'group3']); + + $user1 = $this->createMock(IUser::class); + $this->userManager->expects($this->at(0)) + ->method('get') + ->with('user1') + ->willReturn($user1); + $this->groupManager->expects($this->at(1)) + ->method('getUserGroupIds') + ->with($this->equalTo($user1)) + ->willReturn(['group1']); + $user2 = $this->createMock(IUser::class); + $this->userManager->expects($this->at(1)) + ->method('get') + ->with('user2') + ->willReturn($user2); + $this->groupManager->expects($this->at(2)) + ->method('getUserGroupIds') + ->with($this->equalTo($user2)) + ->willReturn(['group2', 'group3']); + $user3 = $this->createMock(IUser::class); + $this->userManager->expects($this->at(2)) + ->method('get') + ->with('user3') + ->willReturn($user3); + $this->groupManager->expects($this->at(3)) + ->method('getUserGroupIds') + ->with($this->equalTo($user3)) + ->willReturn(['group8', 'group9']); + + $this->knownUserService->method('isKnownToUser') + ->willReturnMap([ + ['user001', 'user1', true], + ['user001', 'user2', true], + ['user001', 'user3', true], + ]); + + $this->contactsManager->expects($this->once()) + ->method('search') + ->with($this->equalTo(''), $this->equalTo(['FN', 'EMAIL'])) + ->willReturn([ + [ + 'UID' => 'user1', + 'isLocalSystemBook' => true + ], + [ + 'UID' => 'user2', + 'isLocalSystemBook' => true + ], + [ + 'UID' => 'user3', + 'isLocalSystemBook' => true + ], + [ + 'UID' => 'contact', + ], + ]); + + $entries = $this->contactsStore->getContacts($currentUser, ''); + + $this->assertCount(3, $entries); + $this->assertEquals('user1', $entries[0]->getProperty('UID')); + $this->assertEquals('user2', $entries[1]->getProperty('UID')); + $this->assertEquals('contact', $entries[2]->getProperty('UID')); + } + + public function testGetContactsOnlyEnumerateIfPhoneBookOrSameGroup() { + $this->config + ->method('getAppValue') + ->willReturnMap([ + ['core', 'shareapi_allow_share_dialog_user_enumeration', 'yes', 'yes'], + ['core', 'shareapi_restrict_user_enumeration_to_group', 'no', 'yes'], + ['core', 'shareapi_restrict_user_enumeration_to_phone', 'no', 'yes'], + ['core', 'shareapi_exclude_groups', 'no', 'no'], + ['core', 'shareapi_only_share_with_group_members', 'no', 'no'], + ]); + + /** @var IUser|\PHPUnit\Framework\MockObject\MockObject $currentUser */ + $currentUser = $this->createMock(IUser::class); + $currentUser->expects($this->once()) + ->method('getUID') + ->willReturn('user001'); + + $this->groupManager->expects($this->at(0)) + ->method('getUserGroupIds') + ->with($this->equalTo($currentUser)) + ->willReturn(['group1', 'group2', 'group3']); + + $user1 = $this->createMock(IUser::class); + $this->userManager->expects($this->at(0)) + ->method('get') + ->with('user1') + ->willReturn($user1); + $this->groupManager->expects($this->at(1)) + ->method('getUserGroupIds') + ->with($this->equalTo($user1)) + ->willReturn(['group1']); + + $this->knownUserService->method('isKnownToUser') + ->willReturnMap([ + ['user001', 'user1', false], + ['user001', 'user2', true], + ['user001', 'user3', true], + ]); + + $this->contactsManager->expects($this->once()) + ->method('search') + ->with($this->equalTo(''), $this->equalTo(['FN', 'EMAIL'])) + ->willReturn([ + [ + 'UID' => 'user1', + 'isLocalSystemBook' => true + ], + [ + 'UID' => 'user2', + 'isLocalSystemBook' => true + ], + [ + 'UID' => 'user3', + 'isLocalSystemBook' => true + ], + [ + 'UID' => 'contact', + ], + ]); + + $entries = $this->contactsStore->getContacts($currentUser, ''); + + $this->assertCount(4, $entries); + $this->assertEquals('user1', $entries[0]->getProperty('UID')); + $this->assertEquals('user2', $entries[1]->getProperty('UID')); + $this->assertEquals('user3', $entries[2]->getProperty('UID')); + $this->assertEquals('contact', $entries[3]->getProperty('UID')); + } + + public function testGetContactsOnlyEnumerateIfPhoneBookOrSameGroupInOwnGroupsOnly() { + $this->config + ->method('getAppValue') + ->willReturnMap([ + ['core', 'shareapi_allow_share_dialog_user_enumeration', 'yes', 'yes'], + ['core', 'shareapi_restrict_user_enumeration_to_group', 'no', 'yes'], + ['core', 'shareapi_restrict_user_enumeration_to_phone', 'no', 'yes'], + ['core', 'shareapi_exclude_groups', 'no', 'no'], + ['core', 'shareapi_only_share_with_group_members', 'no', 'yes'], + ]); /** @var IUser|\PHPUnit\Framework\MockObject\MockObject $currentUser */ $currentUser = $this->createMock(IUser::class); @@ -370,6 +646,13 @@ class ContactsStoreTest extends TestCase { ->with($this->equalTo($user3)) ->willReturn(['group8', 'group9']); + $this->knownUserService->method('isKnownToUser') + ->willReturnMap([ + ['user001', 'user1', false], + ['user001', 'user2', true], + ['user001', 'user3', true], + ]); + $this->contactsManager->expects($this->once()) ->method('search') ->with($this->equalTo(''), $this->equalTo(['FN', 'EMAIL'])) -- cgit v1.2.3 From 177ae33ba1023dcc2a9c1bfce0e2b551ed7b746d Mon Sep 17 00:00:00 2001 From: Joas Schilling Date: Wed, 10 Mar 2021 15:17:36 +0100 Subject: Add a hint that the settings are OR based Signed-off-by: Joas Schilling --- apps/settings/js/admin.js | 1 + apps/settings/templates/settings/admin/sharing.php | 5 +++++ 2 files changed, 6 insertions(+) (limited to 'apps') diff --git a/apps/settings/js/admin.js b/apps/settings/js/admin.js index 72b167d7e0d..ba6b480c79d 100644 --- a/apps/settings/js/admin.js +++ b/apps/settings/js/admin.js @@ -145,6 +145,7 @@ window.addEventListener('DOMContentLoaded', function(){ $('#shareapi_allow_share_dialog_user_enumeration').on('change', function() { $('#shareapi_restrict_user_enumeration_to_group_setting').toggleClass('hidden', !this.checked); $('#shareapi_restrict_user_enumeration_to_phone_setting').toggleClass('hidden', !this.checked); + $('#shareapi_restrict_user_enumeration_combinewarning_setting').toggleClass('hidden', !this.checked); }) $('#allowLinks').change(function() { diff --git a/apps/settings/templates/settings/admin/sharing.php b/apps/settings/templates/settings/admin/sharing.php index b02a7d2764c..a72bf0bd590 100644 --- a/apps/settings/templates/settings/admin/sharing.php +++ b/apps/settings/templates/settings/admin/sharing.php @@ -185,6 +185,11 @@ } ?> />

+

+ t('If autocompletion "same group" and "phonebook matches" are enabled a match in either is enough to show the user.'));?>
+

Date: Wed, 10 Mar 2021 17:18:44 +0100 Subject: Add a setting to restrict returning a full match unless in phonebook or same group Signed-off-by: Joas Schilling --- apps/dav/lib/Connector/Sabre/Principal.php | 36 ++++++--- .../tests/unit/Connector/Sabre/PrincipalTest.php | 51 ++++++++++++ apps/settings/lib/Settings/Admin/Sharing.php | 1 + apps/settings/templates/settings/admin/sharing.php | 11 ++- apps/settings/tests/Settings/Admin/SharingTest.php | 4 + .../collaboration_features/autocomplete.feature | 36 +++++++++ .../features/bootstrap/CollaborationContext.php | 1 + .../Collaboration/Collaborators/MailPlugin.php | 5 +- .../Collaboration/Collaborators/UserPlugin.php | 6 +- .../Contacts/ContactsMenu/ContactsStore.php | 7 +- lib/private/Share20/Manager.php | 4 + lib/public/Share/IManager.php | 8 ++ .../Contacts/ContactsMenu/ContactsStoreTest.php | 93 +++++++++++++++++++++- 13 files changed, 243 insertions(+), 20 deletions(-) (limited to 'apps') diff --git a/apps/dav/lib/Connector/Sabre/Principal.php b/apps/dav/lib/Connector/Sabre/Principal.php index bb456c954e9..b74747b1163 100644 --- a/apps/dav/lib/Connector/Sabre/Principal.php +++ b/apps/dav/lib/Connector/Sabre/Principal.php @@ -263,6 +263,7 @@ class Principal implements BackendInterface { $allowEnumeration = $this->shareManager->allowEnumeration(); $limitEnumerationGroup = $this->shareManager->limitEnumerationToGroups(); $limitEnumerationPhone = $this->shareManager->limitEnumerationToPhone(); + $allowEnumerationFullMatch = $this->shareManager->allowEnumerationFullMatch(); // If sharing is restricted to group members only, // return only members that have groups in common @@ -290,15 +291,19 @@ class Principal implements BackendInterface { foreach ($searchProperties as $prop => $value) { switch ($prop) { case '{http://sabredav.org/ns}email-address': - $users = $this->userManager->getByEmail($value); - if (!$allowEnumeration) { - $users = \array_filter($users, static function (IUser $user) use ($value) { - return $user->getEMailAddress() === $value; - }); + if ($allowEnumerationFullMatch) { + $users = $this->userManager->getByEmail($value); + $users = \array_filter($users, static function (IUser $user) use ($value) { + return $user->getEMailAddress() === $value; + }); + } else { + $users = []; + } } else { - $users = \array_filter($users, function (IUser $user) use ($currentUser, $value, $limitEnumerationPhone, $limitEnumerationGroup, $currentUserGroups) { - if ($user->getEMailAddress() === $value) { + $users = $this->userManager->getByEmail($value); + $users = \array_filter($users, function (IUser $user) use ($currentUser, $value, $limitEnumerationPhone, $limitEnumerationGroup, $allowEnumerationFullMatch, $currentUserGroups) { + if ($allowEnumerationFullMatch && $user->getEMailAddress() === $value) { return true; } @@ -336,15 +341,20 @@ class Principal implements BackendInterface { break; case '{DAV:}displayname': - $users = $this->userManager->searchDisplayName($value, $searchLimit); if (!$allowEnumeration) { - $users = \array_filter($users, static function (IUser $user) use ($value) { - return $user->getDisplayName() === $value; - }); + if ($allowEnumerationFullMatch) { + $users = $this->userManager->searchDisplayName($value, $searchLimit); + $users = \array_filter($users, static function (IUser $user) use ($value) { + return $user->getDisplayName() === $value; + }); + } else { + $users = []; + } } else { - $users = \array_filter($users, function (IUser $user) use ($currentUser, $value, $limitEnumerationPhone, $limitEnumerationGroup, $currentUserGroups) { - if ($user->getDisplayName() === $value) { + $users = $this->userManager->searchDisplayName($value, $searchLimit); + $users = \array_filter($users, function (IUser $user) use ($currentUser, $value, $limitEnumerationPhone, $limitEnumerationGroup, $allowEnumerationFullMatch, $currentUserGroups) { + if ($allowEnumerationFullMatch && $user->getDisplayName() === $value) { return true; } diff --git a/apps/dav/tests/unit/Connector/Sabre/PrincipalTest.php b/apps/dav/tests/unit/Connector/Sabre/PrincipalTest.php index 33c1ec1b587..c9e3d44bf88 100644 --- a/apps/dav/tests/unit/Connector/Sabre/PrincipalTest.php +++ b/apps/dav/tests/unit/Connector/Sabre/PrincipalTest.php @@ -570,6 +570,10 @@ class PrincipalTest extends TestCase { ->method('shareWithGroupMembersOnly') ->willReturn(false); + $this->shareManager->expects($this->once()) + ->method('allowEnumerationFullMatch') + ->willReturn(true); + $user2 = $this->createMock(IUser::class); $user2->method('getUID')->willReturn('user2'); $user2->method('getDisplayName')->willReturn('User 2'); @@ -592,6 +596,27 @@ class PrincipalTest extends TestCase { ['{DAV:}displayname' => 'User 2'])); } + public function testSearchPrincipalWithEnumerationDisabledDisplaynameOnFullMatch() { + $this->shareManager->expects($this->once()) + ->method('shareAPIEnabled') + ->willReturn(true); + + $this->shareManager->expects($this->once()) + ->method('allowEnumeration') + ->willReturn(false); + + $this->shareManager->expects($this->once()) + ->method('shareWithGroupMembersOnly') + ->willReturn(false); + + $this->shareManager->expects($this->once()) + ->method('allowEnumerationFullMatch') + ->willReturn(false); + + $this->assertEquals([], $this->connector->searchPrincipals('principals/users', + ['{DAV:}displayname' => 'User 2'])); + } + public function testSearchPrincipalWithEnumerationDisabledEmail() { $this->shareManager->expects($this->once()) ->method('shareAPIEnabled') @@ -605,6 +630,10 @@ class PrincipalTest extends TestCase { ->method('shareWithGroupMembersOnly') ->willReturn(false); + $this->shareManager->expects($this->once()) + ->method('allowEnumerationFullMatch') + ->willReturn(true); + $user2 = $this->createMock(IUser::class); $user2->method('getUID')->willReturn('user2'); $user2->method('getDisplayName')->willReturn('User 2'); @@ -627,6 +656,28 @@ class PrincipalTest extends TestCase { ['{http://sabredav.org/ns}email-address' => 'user2@foo.bar'])); } + public function testSearchPrincipalWithEnumerationDisabledEmailOnFullMatch() { + $this->shareManager->expects($this->once()) + ->method('shareAPIEnabled') + ->willReturn(true); + + $this->shareManager->expects($this->once()) + ->method('allowEnumeration') + ->willReturn(false); + + $this->shareManager->expects($this->once()) + ->method('shareWithGroupMembersOnly') + ->willReturn(false); + + $this->shareManager->expects($this->once()) + ->method('allowEnumerationFullMatch') + ->willReturn(false); + + + $this->assertEquals([], $this->connector->searchPrincipals('principals/users', + ['{http://sabredav.org/ns}email-address' => 'user2@foo.bar'])); + } + public function testSearchPrincipalWithEnumerationLimitedDisplayname() { $this->shareManager->expects($this->at(0)) ->method('shareAPIEnabled') diff --git a/apps/settings/lib/Settings/Admin/Sharing.php b/apps/settings/lib/Settings/Admin/Sharing.php index 19eed576cd7..6285ef399a8 100644 --- a/apps/settings/lib/Settings/Admin/Sharing.php +++ b/apps/settings/lib/Settings/Admin/Sharing.php @@ -74,6 +74,7 @@ class Sharing implements ISettings { 'allowShareDialogUserEnumeration' => $this->config->getAppValue('core', 'shareapi_allow_share_dialog_user_enumeration', 'yes'), 'restrictUserEnumerationToGroup' => $this->config->getAppValue('core', 'shareapi_restrict_user_enumeration_to_group', 'no'), 'restrictUserEnumerationToPhone' => $this->config->getAppValue('core', 'shareapi_restrict_user_enumeration_to_phone', 'no'), + 'restrictUserEnumerationFullMatch' => $this->config->getAppValue('core', 'shareapi_restrict_user_enumeration_full_match', 'yes'), 'enforceLinkPassword' => Util::isPublicLinkPasswordRequired(), 'onlyShareWithGroupMembers' => $this->shareManager->shareWithGroupMembersOnly(), 'shareAPIEnabled' => $this->config->getAppValue('core', 'shareapi_enabled', 'yes'), diff --git a/apps/settings/templates/settings/admin/sharing.php b/apps/settings/templates/settings/admin/sharing.php index a72bf0bd590..d7c24943b24 100644 --- a/apps/settings/templates/settings/admin/sharing.php +++ b/apps/settings/templates/settings/admin/sharing.php @@ -163,7 +163,7 @@ /> -
+

t('If autocompletion "same group" and "phonebook matches" are enabled a match in either is enough to show the user.'));?>

+

+ /> +
+

'yes', 'restrictUserEnumerationToGroup' => 'no', 'restrictUserEnumerationToPhone' => 'no', + 'restrictUserEnumerationFullMatch' => 'yes', 'enforceLinkPassword' => false, 'onlyShareWithGroupMembers' => false, 'shareAPIEnabled' => 'yes', @@ -132,6 +134,7 @@ class SharingTest extends TestCase { ['core', 'shareapi_allow_share_dialog_user_enumeration', 'yes', 'yes'], ['core', 'shareapi_restrict_user_enumeration_to_group', 'no', 'no'], ['core', 'shareapi_restrict_user_enumeration_to_phone', 'no', 'no'], + ['core', 'shareapi_restrict_user_enumeration_full_match', 'yes', 'yes'], ['core', 'shareapi_enabled', 'yes', 'yes'], ['core', 'shareapi_default_expire_date', 'no', 'no'], ['core', 'shareapi_expire_after_n_days', '7', '7'], @@ -156,6 +159,7 @@ class SharingTest extends TestCase { 'allowShareDialogUserEnumeration' => 'yes', 'restrictUserEnumerationToGroup' => 'no', 'restrictUserEnumerationToPhone' => 'no', + 'restrictUserEnumerationFullMatch' => 'yes', 'enforceLinkPassword' => false, 'onlyShareWithGroupMembers' => false, 'shareAPIEnabled' => 'yes', diff --git a/build/integration/collaboration_features/autocomplete.feature b/build/integration/collaboration_features/autocomplete.feature index 0ca8ebbc100..5e294709d7f 100644 --- a/build/integration/collaboration_features/autocomplete.feature +++ b/build/integration/collaboration_features/autocomplete.feature @@ -3,6 +3,7 @@ Feature: autocomplete Given using api version "2" And group "commongroup" exists And user "admin" belongs to group "commongroup" + And user "auto" exists And user "autocomplete" exists And user "autocomplete2" exists And user "autocomplete2" belongs to group "commongroup" @@ -20,9 +21,15 @@ Feature: autocomplete When parameter "shareapi_allow_share_dialog_user_enumeration" of app "core" is set to "no" Then get autocomplete for "auto" | id | source | + | auto | users | Then get autocomplete for "autocomplete" | id | source | | autocomplete | users | + When parameter "shareapi_restrict_user_enumeration_full_match" of app "core" is set to "no" + Then get autocomplete for "auto" + | id | source | + Then get autocomplete for "autocomplete" + | id | source | Scenario: getting autocomplete with limited enumeration by group @@ -30,6 +37,7 @@ Feature: autocomplete When parameter "shareapi_restrict_user_enumeration_to_group" of app "core" is set to "yes" Then get autocomplete for "auto" | id | source | + | auto | users | | autocomplete2 | users | Then get autocomplete for "autocomplete" | id | source | @@ -38,6 +46,13 @@ Feature: autocomplete Then get autocomplete for "autocomplete2" | id | source | | autocomplete2 | users | + When parameter "shareapi_restrict_user_enumeration_full_match" of app "core" is set to "no" + Then get autocomplete for "autocomplete" + | id | source | + | autocomplete2 | users | + Then get autocomplete for "autocomplete2" + | id | source | + | autocomplete2 | users | Scenario: getting autocomplete with limited enumeration by phone @@ -45,6 +60,7 @@ Feature: autocomplete When parameter "shareapi_restrict_user_enumeration_to_phone" of app "core" is set to "yes" Then get autocomplete for "auto" | id | source | + | auto | users | # autocomplete stores their phone number Given As an "autocomplete" @@ -57,10 +73,17 @@ Feature: autocomplete Given As an "admin" Then get autocomplete for "auto" | id | source | + | auto | users | # admin populates they have the phone number When search users by phone for region "DE" with | random-string1 | 0711 / 252 428-90 | + Then get autocomplete for "auto" + | id | source | + | auto | users | + | autocomplete | users | + + When parameter "shareapi_restrict_user_enumeration_full_match" of app "core" is set to "no" Then get autocomplete for "auto" | id | source | | autocomplete | users | @@ -83,6 +106,13 @@ Feature: autocomplete When search users by phone for region "DE" with | random-string1 | 0711 / 252 428-90 | + Then get autocomplete for "auto" + | id | source | + | auto | users | + | autocomplete | users | + | autocomplete2 | users | + + When parameter "shareapi_restrict_user_enumeration_full_match" of app "core" is set to "no" Then get autocomplete for "auto" | id | source | | autocomplete | users | @@ -108,6 +138,7 @@ Feature: autocomplete Then get autocomplete for "auto" | id | source | + | auto | users | | autocomplete | users | | autocomplete2 | users | When parameter "shareapi_only_share_with_group_members" of app "core" is set to "yes" @@ -121,6 +152,7 @@ Feature: autocomplete When parameter "shareapi_restrict_user_enumeration_to_phone" of app "core" is set to "yes" Then get autocomplete for "auto" | id | source | + | auto | users | # autocomplete stores their phone number Given As an "autocomplete" @@ -133,12 +165,14 @@ Feature: autocomplete Given As an "admin" Then get autocomplete for "auto" | id | source | + | auto | users | # admin populates they have the phone number When search users by phone for region "DE" with | random-string1 | 0711 / 252 428-90 | Then get autocomplete for "auto" | id | source | + | auto | users | | autocomplete | users | # autocomplete changes their phone number @@ -152,12 +186,14 @@ Feature: autocomplete Given As an "admin" Then get autocomplete for "auto" | id | source | + | auto | users | # admin populates they have the new phone number When search users by phone for region "DE" with | random-string1 | 0711 / 252 428-91 | Then get autocomplete for "auto" | id | source | + | auto | users | | autocomplete | users | diff --git a/build/integration/features/bootstrap/CollaborationContext.php b/build/integration/features/bootstrap/CollaborationContext.php index 8207267bf4d..cdba167e677 100644 --- a/build/integration/features/bootstrap/CollaborationContext.php +++ b/build/integration/features/bootstrap/CollaborationContext.php @@ -66,6 +66,7 @@ class CollaborationContext implements Context { $this->deleteServerConfig('core', 'shareapi_allow_share_dialog_user_enumeration'); $this->deleteServerConfig('core', 'shareapi_restrict_user_enumeration_to_group'); $this->deleteServerConfig('core', 'shareapi_restrict_user_enumeration_to_phone'); + $this->deleteServerConfig('core', 'shareapi_restrict_user_enumeration_full_match'); $this->deleteServerConfig('core', 'shareapi_only_share_with_group_members'); } } diff --git a/lib/private/Collaboration/Collaborators/MailPlugin.php b/lib/private/Collaboration/Collaborators/MailPlugin.php index 7da8cede6aa..240e16374d5 100644 --- a/lib/private/Collaboration/Collaborators/MailPlugin.php +++ b/lib/private/Collaboration/Collaborators/MailPlugin.php @@ -49,6 +49,8 @@ class MailPlugin implements ISearchPlugin { protected $shareeEnumerationInGroupOnly; /* @var bool */ protected $shareeEnumerationPhone; + /* @var bool */ + protected $shareeEnumerationFullMatch; /** @var IManager */ private $contactsManager; @@ -81,6 +83,7 @@ class MailPlugin implements ISearchPlugin { $this->shareWithGroupOnly = $this->config->getAppValue('core', 'shareapi_only_share_with_group_members', 'no') === 'yes'; $this->shareeEnumerationInGroupOnly = $this->shareeEnumeration && $this->config->getAppValue('core', 'shareapi_restrict_user_enumeration_to_group', 'no') === 'yes'; $this->shareeEnumerationPhone = $this->shareeEnumeration && $this->config->getAppValue('core', 'shareapi_restrict_user_enumeration_to_phone', 'no') === 'yes'; + $this->shareeEnumerationFullMatch = $this->config->getAppValue('core', 'shareapi_restrict_user_enumeration_full_match', 'yes') === 'yes'; } /** @@ -137,7 +140,7 @@ class MailPlugin implements ISearchPlugin { continue; } } - if ($exactEmailMatch) { + if ($exactEmailMatch && $this->shareeEnumerationFullMatch) { try { $cloud = $this->cloudIdManager->resolveCloudId($contact['CLOUD'][0]); } catch (\InvalidArgumentException $e) { diff --git a/lib/private/Collaboration/Collaborators/UserPlugin.php b/lib/private/Collaboration/Collaborators/UserPlugin.php index 5114ccd8eb5..06a8c6f0efd 100644 --- a/lib/private/Collaboration/Collaborators/UserPlugin.php +++ b/lib/private/Collaboration/Collaborators/UserPlugin.php @@ -53,6 +53,8 @@ class UserPlugin implements ISearchPlugin { protected $shareeEnumerationInGroupOnly; /* @var bool */ protected $shareeEnumerationPhone; + /* @var bool */ + protected $shareeEnumerationFullMatch; /** @var IConfig */ private $config; @@ -85,6 +87,7 @@ class UserPlugin implements ISearchPlugin { $this->shareeEnumeration = $this->config->getAppValue('core', 'shareapi_allow_share_dialog_user_enumeration', 'yes') === 'yes'; $this->shareeEnumerationInGroupOnly = $this->shareeEnumeration && $this->config->getAppValue('core', 'shareapi_restrict_user_enumeration_to_group', 'no') === 'yes'; $this->shareeEnumerationPhone = $this->shareeEnumeration && $this->config->getAppValue('core', 'shareapi_restrict_user_enumeration_to_phone', 'no') === 'yes'; + $this->shareeEnumerationFullMatch = $this->config->getAppValue('core', 'shareapi_restrict_user_enumeration_full_match', 'yes') === 'yes'; } public function search($search, $limit, $offset, ISearchResult $searchResult) { @@ -150,6 +153,7 @@ class UserPlugin implements ISearchPlugin { if ( + $this->shareeEnumerationFullMatch && $lowerSearch !== '' && (strtolower($uid) === $lowerSearch || strtolower($userDisplayName) === $lowerSearch || strtolower($userEmail) === $lowerSearch) @@ -202,7 +206,7 @@ class UserPlugin implements ISearchPlugin { } } - if ($offset === 0 && !$foundUserById) { + if ($this->shareeEnumerationFullMatch && $offset === 0 && !$foundUserById) { // On page one we try if the search result has a direct hit on the // user id and if so, we add that to the exact match list $user = $this->userManager->get($search); diff --git a/lib/private/Contacts/ContactsMenu/ContactsStore.php b/lib/private/Contacts/ContactsMenu/ContactsStore.php index 852765506c0..e0e0bf832b3 100644 --- a/lib/private/Contacts/ContactsMenu/ContactsStore.php +++ b/lib/private/Contacts/ContactsMenu/ContactsStore.php @@ -124,6 +124,7 @@ class ContactsStore implements IContactsStore { $disallowEnumeration = $this->config->getAppValue('core', 'shareapi_allow_share_dialog_user_enumeration', 'yes') !== 'yes'; $restrictEnumerationGroup = $this->config->getAppValue('core', 'shareapi_restrict_user_enumeration_to_group', 'no') === 'yes'; $restrictEnumerationPhone = $this->config->getAppValue('core', 'shareapi_restrict_user_enumeration_to_phone', 'no') === 'yes'; + $allowEnumerationFullMatch = $this->config->getAppValue('core', 'shareapi_restrict_user_enumeration_full_match', 'yes') === 'yes'; $excludedGroups = $this->config->getAppValue('core', 'shareapi_exclude_groups', 'no') === 'yes'; // whether to filter out local users @@ -146,7 +147,7 @@ class ContactsStore implements IContactsStore { $selfUID = $self->getUID(); - return array_values(array_filter($entries, function (IEntry $entry) use ($skipLocal, $ownGroupsOnly, $selfGroups, $selfUID, $disallowEnumeration, $restrictEnumerationGroup, $restrictEnumerationPhone, $filter) { + return array_values(array_filter($entries, function (IEntry $entry) use ($skipLocal, $ownGroupsOnly, $selfGroups, $selfUID, $disallowEnumeration, $restrictEnumerationGroup, $restrictEnumerationPhone, $allowEnumerationFullMatch, $filter) { if ($entry->getProperty('UID') === $selfUID) { return false; } @@ -160,6 +161,10 @@ class ContactsStore implements IContactsStore { // Prevent enumerating local users if ($disallowEnumeration) { + if (!$allowEnumerationFullMatch) { + return false; + } + $filterUser = true; $mailAddresses = $entry->getEMailAddresses(); diff --git a/lib/private/Share20/Manager.php b/lib/private/Share20/Manager.php index 6e072740884..ce1ec1d60f6 100644 --- a/lib/private/Share20/Manager.php +++ b/lib/private/Share20/Manager.php @@ -1834,6 +1834,10 @@ class Manager implements IManager { $this->config->getAppValue('core', 'shareapi_restrict_user_enumeration_to_phone', 'no') === 'yes'; } + public function allowEnumerationFullMatch(): bool { + return $this->config->getAppValue('core', 'shareapi_restrict_user_enumeration_full_match', 'yes') === 'yes'; + } + /** * Copied from \OC_Util::isSharingDisabledForUser * diff --git a/lib/public/Share/IManager.php b/lib/public/Share/IManager.php index 0c8732b4b15..606e6429918 100644 --- a/lib/public/Share/IManager.php +++ b/lib/public/Share/IManager.php @@ -392,6 +392,14 @@ interface IManager { */ public function limitEnumerationToPhone(): bool; + /** + * Check if user enumeration is allowed to return on full match + * + * @return bool + * @since 21.0.1 + */ + public function allowEnumerationFullMatch(): bool; + /** * Check if sharing is disabled for the given user * diff --git a/tests/lib/Contacts/ContactsMenu/ContactsStoreTest.php b/tests/lib/Contacts/ContactsMenu/ContactsStoreTest.php index ad83178096e..ad201d86a2a 100644 --- a/tests/lib/Contacts/ContactsMenu/ContactsStoreTest.php +++ b/tests/lib/Contacts/ContactsMenu/ContactsStoreTest.php @@ -683,9 +683,12 @@ class ContactsStoreTest extends TestCase { } public function testGetContactsWithFilter() { - $this->config->expects($this->at(0))->method('getAppValue') - ->with($this->equalTo('core'), $this->equalTo('shareapi_allow_share_dialog_user_enumeration'), $this->equalTo('yes')) - ->willReturn('no'); + $this->config + ->method('getAppValue') + ->willReturnMap([ + ['core', 'shareapi_allow_share_dialog_user_enumeration', 'yes', 'no'], + ['core', 'shareapi_restrict_user_enumeration_full_match', 'yes', 'yes'], + ]); /** @var IUser|\PHPUnit\Framework\MockObject\MockObject $user */ $user = $this->createMock(IUser::class); @@ -766,6 +769,90 @@ class ContactsStoreTest extends TestCase { ], $entry[0]->getEMailAddresses()); } + public function testGetContactsWithFilterWithoutFullMatch() { + $this->config + ->method('getAppValue') + ->willReturnMap([ + ['core', 'shareapi_allow_share_dialog_user_enumeration', 'yes', 'no'], + ['core', 'shareapi_restrict_user_enumeration_full_match', 'yes', 'no'], + ]); + + /** @var IUser|\PHPUnit\Framework\MockObject\MockObject $user */ + $user = $this->createMock(IUser::class); + $this->contactsManager->expects($this->any()) + ->method('search') + ->willReturn([ + [ + 'UID' => 'a567', + 'FN' => 'Darren Roner', + 'EMAIL' => [ + 'darren@roner.au', + ], + 'isLocalSystemBook' => true, + ], + [ + 'UID' => 'john', + 'FN' => 'John Doe', + 'EMAIL' => [ + 'john@example.com', + ], + 'isLocalSystemBook' => true, + ], + [ + 'FN' => 'Anne D', + 'EMAIL' => [ + 'anne@example.com', + ], + 'isLocalSystemBook' => false, + ], + ]); + $user->expects($this->any()) + ->method('getUID') + ->willReturn('user123'); + + // Complete match on UID should not match + $entry = $this->contactsStore->getContacts($user, 'a567'); + $this->assertSame(1, count($entry)); + $this->assertEquals([ + 'anne@example.com' + ], $entry[0]->getEMailAddresses()); + + // Partial match on UID should not match + $entry = $this->contactsStore->getContacts($user, 'a56'); + $this->assertSame(1, count($entry)); + $this->assertEquals([ + 'anne@example.com' + ], $entry[0]->getEMailAddresses()); + + // Complete match on email should not match + $entry = $this->contactsStore->getContacts($user, 'john@example.com'); + $this->assertSame(1, count($entry)); + $this->assertEquals([ + 'anne@example.com' + ], $entry[0]->getEMailAddresses()); + + // Partial match on email should not match + $entry = $this->contactsStore->getContacts($user, 'john@example.co'); + $this->assertSame(1, count($entry)); + $this->assertEquals([ + 'anne@example.com' + ], $entry[0]->getEMailAddresses()); + + // Match on FN should not match + $entry = $this->contactsStore->getContacts($user, 'Darren Roner'); + $this->assertSame(1, count($entry)); + $this->assertEquals([ + 'anne@example.com' + ], $entry[0]->getEMailAddresses()); + + // Don't filter users in local addressbook + $entry = $this->contactsStore->getContacts($user, 'Anne D'); + $this->assertSame(1, count($entry)); + $this->assertEquals([ + 'anne@example.com' + ], $entry[0]->getEMailAddresses()); + } + public function testFindOneUser() { $this->config->expects($this->at(0))->method('getAppValue') ->with($this->equalTo('core'), $this->equalTo('shareapi_allow_share_dialog_user_enumeration'), $this->equalTo('yes')) -- cgit v1.2.3 From 78efabb0730d250da387debf12418a3b5808018c Mon Sep 17 00:00:00 2001 From: Joas Schilling Date: Wed, 10 Mar 2021 17:51:28 +0100 Subject: Fix CS hopefully Signed-off-by: Joas Schilling --- apps/settings/templates/settings/admin/sharing.php | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) (limited to 'apps') diff --git a/apps/settings/templates/settings/admin/sharing.php b/apps/settings/templates/settings/admin/sharing.php index d7c24943b24..65e84d29120 100644 --- a/apps/settings/templates/settings/admin/sharing.php +++ b/apps/settings/templates/settings/admin/sharing.php @@ -191,12 +191,12 @@ t('If autocompletion "same group" and "phonebook matches" are enabled a match in either is enough to show the user.'));?>

+ p('hidden'); +}?>"> /> + print_unescaped('checked="checked"'); +} ?> />

-- cgit v1.2.3 From 07095fd122b295d8ac1cbf8109f8ecbe6ac60801 Mon Sep 17 00:00:00 2001 From: Joas Schilling Date: Wed, 10 Mar 2021 19:37:10 +0100 Subject: Also clear the knownUser when changing via the settings endpoint Signed-off-by: Joas Schilling --- apps/settings/lib/Controller/UsersController.php | 24 +++++++++++++++++++++- .../tests/Controller/UsersControllerTest.php | 6 ++++++ 2 files changed, 29 insertions(+), 1 deletion(-) (limited to 'apps') diff --git a/apps/settings/lib/Controller/UsersController.php b/apps/settings/lib/Controller/UsersController.php index bdb3236c2df..c4893b778fa 100644 --- a/apps/settings/lib/Controller/UsersController.php +++ b/apps/settings/lib/Controller/UsersController.php @@ -42,6 +42,7 @@ use OC\AppFramework\Http; use OC\Encryption\Exceptions\ModuleDoesNotExistsException; use OC\ForbiddenException; use OC\Group\Manager as GroupManager; +use OC\KnownUser\KnownUserService; use OC\L10N\Factory; use OC\Security\IdentityProof\Manager; use OC\User\Manager as UserManager; @@ -96,6 +97,8 @@ class UsersController extends Controller { private $jobList; /** @var IManager */ private $encryptionManager; + /** @var KnownUserService */ + private $knownUserService; /** @var IEventDispatcher */ private $dispatcher; @@ -116,6 +119,7 @@ class UsersController extends Controller { Manager $keyManager, IJobList $jobList, IManager $encryptionManager, + KnownUserService $knownUserService, IEventDispatcher $dispatcher ) { parent::__construct($appName, $request); @@ -132,6 +136,7 @@ class UsersController extends Controller { $this->keyManager = $keyManager; $this->jobList = $jobList; $this->encryptionManager = $encryptionManager; + $this->knownUserService = $knownUserService; $this->dispatcher = $dispatcher; } @@ -363,6 +368,19 @@ class UsersController extends Controller { ?string $twitter = null, ?string $twitterScope = null ) { + $user = $this->userSession->getUser(); + if (!$user instanceof IUser) { + return new DataResponse( + [ + 'status' => 'error', + 'data' => [ + 'message' => $this->l10n->t('Invalid user') + ] + ], + Http::STATUS_UNAUTHORIZED + ); + } + $email = strtolower($email); if (!empty($email) && !$this->mailer->validateMailAddress($email)) { return new DataResponse( @@ -375,8 +393,9 @@ class UsersController extends Controller { Http::STATUS_UNPROCESSABLE_ENTITY ); } - $user = $this->userSession->getUser(); + $data = $this->accountManager->getUser($user); + $beforeData = $data; $data[IAccountManager::PROPERTY_AVATAR] = ['scope' => $avatarScope]; if ($this->config->getSystemValue('allow_user_to_change_display_name', true) !== false) { $data[IAccountManager::PROPERTY_DISPLAYNAME] = ['value' => $displayname, 'scope' => $displaynameScope]; @@ -393,6 +412,9 @@ class UsersController extends Controller { } try { $data = $this->saveUserSettings($user, $data); + if ($beforeData[IAccountManager::PROPERTY_PHONE]['value'] !== $data[IAccountManager::PROPERTY_PHONE]['value']) { + $this->knownUserService->deleteKnownUser($user->getUID()); + } return new DataResponse( [ 'status' => 'success', diff --git a/apps/settings/tests/Controller/UsersControllerTest.php b/apps/settings/tests/Controller/UsersControllerTest.php index 1a9af2ea8c9..b14e8d00d60 100644 --- a/apps/settings/tests/Controller/UsersControllerTest.php +++ b/apps/settings/tests/Controller/UsersControllerTest.php @@ -32,6 +32,7 @@ namespace OCA\Settings\Tests\Controller; use OC\Accounts\AccountManager; use OC\Encryption\Exceptions\ModuleDoesNotExistsException; use OC\Group\Manager; +use OC\KnownUser\KnownUserService; use OCA\Settings\Controller\UsersController; use OCP\Accounts\IAccountManager; use OCP\App\IAppManager; @@ -91,6 +92,8 @@ class UsersControllerTest extends \Test\TestCase { private $securityManager; /** @var IManager | \PHPUnit\Framework\MockObject\MockObject */ private $encryptionManager; + /** @var KnownUserService|\PHPUnit\Framework\MockObject\MockObject */ + private $knownUserService; /** @var IEncryptionModule | \PHPUnit\Framework\MockObject\MockObject */ private $encryptionModule; /** @var IEventDispatcher|\PHPUnit\Framework\MockObject\MockObject */ @@ -111,6 +114,7 @@ class UsersControllerTest extends \Test\TestCase { $this->securityManager = $this->getMockBuilder(\OC\Security\IdentityProof\Manager::class)->disableOriginalConstructor()->getMock(); $this->jobList = $this->createMock(IJobList::class); $this->encryptionManager = $this->createMock(IManager::class); + $this->knownUserService = $this->createMock(KnownUserService::class); $this->dispatcher = $this->createMock(IEventDispatcher::class); $this->l->method('t') @@ -147,6 +151,7 @@ class UsersControllerTest extends \Test\TestCase { $this->securityManager, $this->jobList, $this->encryptionManager, + $this->knownUserService, $this->dispatcher ); } else { @@ -168,6 +173,7 @@ class UsersControllerTest extends \Test\TestCase { $this->securityManager, $this->jobList, $this->encryptionManager, + $this->knownUserService, $this->dispatcher ] )->setMethods($mockedMethods)->getMock(); -- cgit v1.2.3 From 61ed57b757201541b2038ccab10b092a77c4fb64 Mon Sep 17 00:00:00 2001 From: Joas Schilling Date: Wed, 10 Mar 2021 20:30:29 +0100 Subject: Rename some parameters and methods to make the API more clear Signed-off-by: Joas Schilling --- .../lib/Controller/UsersController.php | 2 +- .../lib/Listener/UserDeletedListener.php | 2 +- apps/settings/lib/Controller/UsersController.php | 2 +- lib/private/KnownUser/KnownUserMapper.php | 4 ++- lib/private/KnownUser/KnownUserService.php | 39 ++++++++++++++++++---- 5 files changed, 38 insertions(+), 11 deletions(-) (limited to 'apps') diff --git a/apps/provisioning_api/lib/Controller/UsersController.php b/apps/provisioning_api/lib/Controller/UsersController.php index d1dba3ea6ee..579f9ae522a 100644 --- a/apps/provisioning_api/lib/Controller/UsersController.php +++ b/apps/provisioning_api/lib/Controller/UsersController.php @@ -692,7 +692,7 @@ class UsersController extends AUserData { $this->accountManager->updateUser($targetUser, $userAccount, true); if ($key === IAccountManager::PROPERTY_PHONE) { - $this->knownUserService->deleteKnownUser($targetUser->getUID()); + $this->knownUserService->deleteByContactUserId($targetUser->getUID()); } } catch (\InvalidArgumentException $e) { throw new OCSException('Invalid ' . $e->getMessage(), 102); diff --git a/apps/provisioning_api/lib/Listener/UserDeletedListener.php b/apps/provisioning_api/lib/Listener/UserDeletedListener.php index f4fdb973080..1e021177bb4 100644 --- a/apps/provisioning_api/lib/Listener/UserDeletedListener.php +++ b/apps/provisioning_api/lib/Listener/UserDeletedListener.php @@ -49,6 +49,6 @@ class UserDeletedListener implements IEventListener { $this->service->deleteKnownTo($user->getUID()); // Delete all entries that other users know this user - $this->service->deleteKnownUser($user->getUID()); + $this->service->deleteByContactUserId($user->getUID()); } } diff --git a/apps/settings/lib/Controller/UsersController.php b/apps/settings/lib/Controller/UsersController.php index c4893b778fa..46de0b4cd96 100644 --- a/apps/settings/lib/Controller/UsersController.php +++ b/apps/settings/lib/Controller/UsersController.php @@ -413,7 +413,7 @@ class UsersController extends Controller { try { $data = $this->saveUserSettings($user, $data); if ($beforeData[IAccountManager::PROPERTY_PHONE]['value'] !== $data[IAccountManager::PROPERTY_PHONE]['value']) { - $this->knownUserService->deleteKnownUser($user->getUID()); + $this->knownUserService->deleteByContactUserId($user->getUID()); } return new DataResponse( [ diff --git a/lib/private/KnownUser/KnownUserMapper.php b/lib/private/KnownUser/KnownUserMapper.php index 1144e2fd212..e77e4752702 100644 --- a/lib/private/KnownUser/KnownUserMapper.php +++ b/lib/private/KnownUser/KnownUserMapper.php @@ -63,10 +63,12 @@ class KnownUserMapper extends QBMapper { } /** + * Returns all "known users" for the given "known to" user + * * @param string $knownTo * @return KnownUser[] */ - public function getKnownTo(string $knownTo): array { + public function getKnownUsers(string $knownTo): array { $query = $this->db->getQueryBuilder(); $query->select('*') ->from($this->getTableName()) diff --git a/lib/private/KnownUser/KnownUserService.php b/lib/private/KnownUser/KnownUserService.php index 3a97b967c3a..96af21c836f 100644 --- a/lib/private/KnownUser/KnownUserService.php +++ b/lib/private/KnownUser/KnownUserService.php @@ -33,30 +33,55 @@ class KnownUserService { $this->mapper = $mapper; } + /** + * Delete all matches where the given user is the owner of the phonebook + * + * @param string $knownTo + * @return int Number of deleted matches + */ public function deleteKnownTo(string $knownTo): int { return $this->mapper->deleteKnownTo($knownTo); } - public function deleteKnownUser(string $knownUser): int { - return $this->mapper->deleteKnownUser($knownUser); + /** + * Delete all matches where the given user is the one in the phonebook + * + * @param string $contactUserId + * @return int Number of deleted matches + */ + public function deleteByContactUserId(string $contactUserId): int { + return $this->mapper->deleteKnownUser($contactUserId); } - public function storeIsKnownToUser(string $knownTo, string $knownUser): void { + /** + * Store a match because $knownTo has $contactUserId in his phonebook + * + * @param string $knownTo User id of the owner of the phonebook + * @param string $contactUserId User id of the contact in the phonebook + */ + public function storeIsKnownToUser(string $knownTo, string $contactUserId): void { $entity = new KnownUser(); $entity->setKnownTo($knownTo); - $entity->setKnownUser($knownUser); + $entity->setKnownUser($contactUserId); $this->mapper->insert($entity); } - public function isKnownToUser(string $knownTo, string $user): bool { + /** + * Check if $contactUserId is in the phonebook of $knownTo + * + * @param string $knownTo User id of the owner of the phonebook + * @param string $contactUserId User id of the contact in the phonebook + * @return bool + */ + public function isKnownToUser(string $knownTo, string $contactUserId): bool { if (!isset($this->knownUsers[$knownTo])) { - $entities = $this->mapper->getKnownTo($knownTo); + $entities = $this->mapper->getKnownUsers($knownTo); $this->knownUsers[$knownTo] = []; foreach ($entities as $entity) { $this->knownUsers[$knownTo][$entity->getKnownUser()] = true; } } - return isset($this->knownUsers[$knownTo][$user]); + return isset($this->knownUsers[$knownTo][$contactUserId]); } } -- cgit v1.2.3