diff --git a/.drone.yml b/.drone.yml index 8ea80eb76c..54932162cd 100644 --- a/.drone.yml +++ b/.drone.yml @@ -754,6 +754,31 @@ trigger: - pull_request - push +--- +kind: pipeline +name: integration-collaboration_features + +steps: +- name: submodules + image: docker:git + commands: + - git submodule update --init +- name: integration-collaboration_features + image: nextcloudci/integration-php7.3:integration-php7.3-2 + commands: + - bash tests/drone-run-integration-tests.sh || exit 0 + - ./occ maintenance:install --admin-pass=admin --data-dir=/dev/shm/nc_int + - cd build/integration + - ./run.sh collaboration_features/ + +trigger: + branch: + - master + - stable* + event: + - pull_request + - push + --- kind: pipeline name: integration-federation_features diff --git a/apps/dav/appinfo/v1/caldav.php b/apps/dav/appinfo/v1/caldav.php index e04653ddea..236d81f66f 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 dbab1ae968..bb766bbaec 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 c7190c8131..5b95215271 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 58c6a8c63f..1d543c71bc 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 2351094a91..b74747b116 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,25 @@ class Principal implements BackendInterface { } $allowEnumeration = $this->shareManager->allowEnumeration(); - $limitEnumeration = $this->shareManager->limitEnumerationToGroups(); + $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 $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); } } @@ -296,20 +291,38 @@ 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 = $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; + } - 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 + )); }); } @@ -328,20 +341,39 @@ 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 = $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; + } - if ($limitEnumeration) { - $users = \array_filter($users, function (IUser $user) use ($currentUserGroups, $value) { - return !empty(array_intersect( - $this->groupManager->getUserGroupIds($user), - $currentUserGroups - )) || $user->getDisplayName() === $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 + )); }); } diff --git a/apps/dav/lib/RootCollection.php b/apps/dav/lib/RootCollection.php index 18874ecf74..16a209a98f 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 85efd0fd36..51ba8c1867 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 a8c7a78172..60f46ce8fa 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 117707eaf2..c9e3d44bf8 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); @@ -564,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'); @@ -586,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') @@ -599,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'); @@ -621,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/files_versions/lib/AppInfo/Application.php b/apps/files_versions/lib/AppInfo/Application.php index afbd42ffc3..e09ad7e90a 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/apps/provisioning_api/composer/composer/autoload_classmap.php b/apps/provisioning_api/composer/composer/autoload_classmap.php index 0383fddbef..e94a97c194 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 2c1682641a..b982f20321 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 863f8861d8..7ec21c3329 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/Controller/UsersController.php b/apps/provisioning_api/lib/Controller/UsersController.php index 11c3a85db9..579f9ae522 100644 --- a/apps/provisioning_api/lib/Controller/UsersController.php +++ b/apps/provisioning_api/lib/Controller/UsersController.php @@ -49,6 +49,7 @@ use libphonenumber\PhoneNumberUtil; use OC\Accounts\AccountManager; use OC\Authentication\Token\RemoteWipe; use OC\HintException; +use OC\KnownUser\KnownUserService; use OCA\Provisioning_API\FederatedShareProviderFactory; use OCA\Settings\Mailer\NewUserMailHelper; use OCP\Accounts\IAccountManager; @@ -90,6 +91,8 @@ class UsersController extends AUserData { private $secureRandom; /** @var RemoteWipe */ private $remoteWipe; + /** @var KnownUserService */ + private $knownUserService; /** @var IEventDispatcher */ private $eventDispatcher; @@ -108,6 +111,7 @@ class UsersController extends AUserData { FederatedShareProviderFactory $federatedShareProviderFactory, ISecureRandom $secureRandom, RemoteWipe $remoteWipe, + KnownUserService $knownUserService, IEventDispatcher $eventDispatcher) { parent::__construct($appName, $request, @@ -126,6 +130,7 @@ class UsersController extends AUserData { $this->federatedShareProviderFactory = $federatedShareProviderFactory; $this->secureRandom = $secureRandom; $this->remoteWipe = $remoteWipe; + $this->knownUserService = $knownUserService; $this->eventDispatcher = $eventDispatcher; } @@ -231,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) { @@ -268,6 +280,7 @@ class UsersController extends AUserData { 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; + $this->knownUserService->storeIsKnownToUser($knownTo, $userId); } return new DataResponse($matches); @@ -677,6 +690,10 @@ class UsersController extends AUserData { $userAccount[$key]['value'] = $value; try { $this->accountManager->updateUser($targetUser, $userAccount, true); + + if ($key === IAccountManager::PROPERTY_PHONE) { + $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 new file mode 100644 index 0000000000..1e021177bb --- /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\KnownUserService; +use OCP\EventDispatcher\Event; +use OCP\EventDispatcher\IEventListener; +use OCP\User\Events\UserDeletedEvent; + +class UserDeletedListener implements IEventListener { + + /** @var KnownUserService */ + private $service; + + public function __construct(KnownUserService $service) { + $this->service = $service; + } + + public function handle(Event $event): void { + if (!($event instanceof UserDeletedEvent)) { + // Unrelated + return; + } + + $user = $event->getUser(); + + // Delete all entries of this user + $this->service->deleteKnownTo($user->getUID()); + + // Delete all entries that other users know this user + $this->service->deleteByContactUserId($user->getUID()); + } +} diff --git a/apps/provisioning_api/tests/Controller/UsersControllerTest.php b/apps/provisioning_api/tests/Controller/UsersControllerTest.php index 3130fce3e9..d65e3d0791 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/apps/settings/js/admin.js b/apps/settings/js/admin.js index cffaefa382..ba6b480c79 100644 --- a/apps/settings/js/admin.js +++ b/apps/settings/js/admin.js @@ -144,6 +144,8 @@ 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/lib/Controller/UsersController.php b/apps/settings/lib/Controller/UsersController.php index bdb3236c2d..46de0b4cd9 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->deleteByContactUserId($user->getUID()); + } return new DataResponse( [ 'status' => 'success', diff --git a/apps/settings/lib/Settings/Admin/Sharing.php b/apps/settings/lib/Settings/Admin/Sharing.php index 313a182501..6285ef399a 100644 --- a/apps/settings/lib/Settings/Admin/Sharing.php +++ b/apps/settings/lib/Settings/Admin/Sharing.php @@ -73,6 +73,8 @@ 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'), + '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 9f651ce6d6..65e84d2912 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.'));?>
+

+

+ /> +

diff --git a/apps/settings/tests/Controller/UsersControllerTest.php b/apps/settings/tests/Controller/UsersControllerTest.php index 1a9af2ea8c..b14e8d00d6 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(); diff --git a/apps/settings/tests/Settings/Admin/SharingTest.php b/apps/settings/tests/Settings/Admin/SharingTest.php index 52e83f8ba7..1f24ef13d4 100644 --- a/apps/settings/tests/Settings/Admin/SharingTest.php +++ b/apps/settings/tests/Settings/Admin/SharingTest.php @@ -64,95 +64,29 @@ 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_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'], + ['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 +98,8 @@ class SharingTest extends TestCase { 'allowResharing' => 'yes', 'allowShareDialogUserEnumeration' => 'yes', 'restrictUserEnumerationToGroup' => 'no', + 'restrictUserEnumerationToPhone' => 'no', + 'restrictUserEnumerationFullMatch' => 'yes', 'enforceLinkPassword' => false, 'onlyShareWithGroupMembers' => false, 'shareAPIEnabled' => 'yes', @@ -188,96 +124,29 @@ 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_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'], + ['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 +158,8 @@ class SharingTest extends TestCase { 'allowResharing' => 'yes', '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 new file mode 100644 index 0000000000..e20993e420 --- /dev/null +++ b/build/integration/collaboration_features/autocomplete.feature @@ -0,0 +1,218 @@ +Feature: autocomplete + Background: + 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" + + Scenario: getting autocomplete + Given As an "admin" + When 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 | + | auto | users | + | autocomplete | users | + | autocomplete2 | users | + + + Scenario: getting autocomplete without enumeration + Given As an "admin" + 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 + Given As an "admin" + 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 | + | autocomplete | users | + | autocomplete2 | users | + 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 + Given As an "admin" + 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" + And sending "PUT" to "/cloud/users/autocomplete" with + | key | phone | + | value | +49 711 / 25 24 28-90 | + And the HTTP status code should be "200" + And the OCS status code should be "200" + + 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 | + + + Scenario: getting autocomplete with limited enumeration by group or phone + Given As an "admin" + When parameter "shareapi_restrict_user_enumeration_to_group" of app "core" is set to "yes" + And parameter "shareapi_restrict_user_enumeration_to_phone" of app "core" is set to "yes" + + # autocomplete stores their phone number + Given As an "autocomplete" + And sending "PUT" to "/cloud/users/autocomplete" with + | key | phone | + | value | +49 711 / 25 24 28-90 | + And the HTTP status code should be "200" + And the OCS status code should be "200" + # admin populates they have the phone number + Given As an "admin" + 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 | + | autocomplete2 | users | + + + Scenario: getting autocomplete with limited enumeration but sharing is group restricted + Given As an "admin" + When parameter "shareapi_restrict_user_enumeration_to_group" of app "core" is set to "yes" + And parameter "shareapi_restrict_user_enumeration_to_phone" of app "core" is set to "yes" + + # autocomplete stores their phone number + Given As an "autocomplete" + And sending "PUT" to "/cloud/users/autocomplete" with + | key | phone | + | value | +49 711 / 25 24 28-90 | + And the HTTP status code should be "200" + And the OCS status code should be "200" + # admin populates they have the phone number + Given As an "admin" + 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_only_share_with_group_members" of app "core" is set to "yes" + Then get autocomplete for "auto" + | id | source | + | autocomplete2 | users | + + + Scenario: getting autocomplete with limited enumeration by phone but user changes it + Given As an "admin" + 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" + And sending "PUT" to "/cloud/users/autocomplete" with + | key | phone | + | value | +49 711 / 25 24 28-90 | + And the HTTP status code should be "200" + And the OCS status code should be "200" + + 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 + Given As an "autocomplete" + And sending "PUT" to "/cloud/users/autocomplete" with + | key | phone | + | value | +49 711 / 25 24 28-91 | + And the HTTP status code should be "200" + And the OCS status code should be "200" + + 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 | + + + Scenario: getting autocomplete without enumeration and sharing is group restricted + Given As an "admin" + When parameter "shareapi_allow_share_dialog_user_enumeration" of app "core" is set to "no" + And parameter "shareapi_only_share_with_group_members" of app "core" is set to "yes" + + Then get autocomplete for "auto" + | id | source | + Then get autocomplete for "autocomplete" + | id | source | + Then get autocomplete for "autocomplete2" + | id | source | + | autocomplete2 | users | diff --git a/build/integration/config/behat.yml b/build/integration/config/behat.yml index 79ffe58f6b..0e577f5925 100644 --- a/build/integration/config/behat.yml +++ b/build/integration/config/behat.yml @@ -45,6 +45,16 @@ default: - admin - admin regular_user_password: 123456 + collaboration: + paths: + - "%paths.base%/../collaboration_features" + contexts: + - CollaborationContext: + baseUrl: http://localhost:8080/ocs/ + admin: + - admin + - admin + regular_user_password: 123456 sharees: paths: - "%paths.base%/../sharees_features" diff --git a/build/integration/features/bootstrap/BasicStructure.php b/build/integration/features/bootstrap/BasicStructure.php index 5b01e80707..cc5ac2e14b 100644 --- a/build/integration/features/bootstrap/BasicStructure.php +++ b/build/integration/features/bootstrap/BasicStructure.php @@ -202,6 +202,40 @@ trait BasicStructure { } } + /** + * @param string $verb + * @param string $url + * @param TableNode|array|null $body + * @param array $headers + */ + protected function sendRequestForJSON(string $verb, string $url, $body = null, array $headers = []): void { + $fullUrl = $this->baseUrl . "v{$this->apiVersion}.php" . $url; + $client = new Client(); + $options = []; + if ($this->currentUser === 'admin') { + $options['auth'] = ['admin', 'admin']; + } elseif (strpos($this->currentUser, 'guest') !== 0) { + $options['auth'] = [$this->currentUser, self::TEST_PASSWORD]; + } + if ($body instanceof TableNode) { + $fd = $body->getRowsHash(); + $options['form_params'] = $fd; + } elseif (is_array($body)) { + $options['form_params'] = $body; + } + + $options['headers'] = array_merge($headers, [ + 'OCS-ApiRequest' => 'true', + 'Accept' => 'application/json', + ]); + + try { + $this->response = $client->{$verb}($fullUrl, $options); + } catch (ClientException $ex) { + $this->response = $ex->getResponse(); + } + } + /** * @When /^sending "([^"]*)" with exact url to "([^"]*)"$/ * @param string $verb diff --git a/build/integration/features/bootstrap/CollaborationContext.php b/build/integration/features/bootstrap/CollaborationContext.php new file mode 100644 index 0000000000..cdba167e67 --- /dev/null +++ b/build/integration/features/bootstrap/CollaborationContext.php @@ -0,0 +1,72 @@ + + * + * @author Joas Schilling + * + * @license AGPL-3.0 + * + * This code is free software: you can redistribute it and/or modify + * it under the terms of the GNU Affero General Public License, version 3, + * as published by the Free Software Foundation. + * + * 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, version 3, + * along with this program. If not, see + * + */ + +use Behat\Behat\Context\Context; +use Behat\Gherkin\Node\TableNode; +use PHPUnit\Framework\Assert; + +require __DIR__ . '/../../vendor/autoload.php'; + +class CollaborationContext implements Context { + use Provisioning; + use AppConfiguration; + + /** + * @Then /^get autocomplete for "([^"]*)"$/ + * @param TableNode|null $formData + */ + public function getAutocomplete(string $search, TableNode $formData): void { + $query = $search === 'null' ? null : $search; + + $this->sendRequestForJSON('GET', '/core/autocomplete/get?itemType=files&itemId=123&search=' . $query, [ + 'itemType' => 'files', + 'itemId' => '123', + 'search' => $query, + ]); + $this->theHTTPStatusCodeShouldBe(200); + + $data = json_decode($this->response->getBody()->getContents(), true); + $suggestions = $data['ocs']['data']; + + Assert::assertCount(count($formData->getHash()), $suggestions, 'Suggestion count does not match'); + Assert::assertEquals($formData->getHash(), array_map(static function ($suggestion, $expected) { + $data = []; + if (isset($expected['id'])) { + $data['id'] = $suggestion['id']; + } + if (isset($expected['source'])) { + $data['source'] = $suggestion['source']; + } + return $data; + }, $suggestions, $formData->getHash())); + } + + protected function resetAppConfigs(): void { + $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/core/Migrations/Version21000Date20210309185126.php b/core/Migrations/Version21000Date20210309185126.php new file mode 100644 index 0000000000..675cda9e03 --- /dev/null +++ b/core/Migrations/Version21000Date20210309185126.php @@ -0,0 +1,69 @@ + + * + * @author Joas Schilling + * + * @license GNU AGPL version 3 or any later version + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU Affero General Public License as + * published by the Free Software Foundation, either version 3 of the + * License, or (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Affero General Public License for more details. + * + * You should have received a copy of the GNU Affero General Public License + * along with this program. If not, see . + */ + +namespace OC\Core\Migrations; + +use Closure; +use Doctrine\DBAL\Types\Types; +use OCP\DB\ISchemaWrapper; +use OCP\Migration\IOutput; +use OCP\Migration\SimpleMigrationStep; + +class Version21000Date20210309185126 extends SimpleMigrationStep { + /** + * @param IOutput $output + * @param Closure $schemaClosure The `\Closure` returns a `ISchemaWrapper` + * @param array $options + * @return null|ISchemaWrapper + */ + public function changeSchema(IOutput $output, Closure $schemaClosure, array $options): ?ISchemaWrapper { + /** @var ISchemaWrapper $schema */ + $schema = $schemaClosure(); + + if (!$schema->hasTable('known_users')) { + $table = $schema->createTable('known_users'); + + // Auto increment id + $table->addColumn('id', Types::BIGINT, [ + 'autoincrement' => true, + 'notnull' => true, + ]); + + $table->addColumn('known_to', Types::STRING, [ + 'notnull' => true, + 'length' => 255, + ]); + $table->addColumn('known_user', Types::STRING, [ + 'notnull' => true, + 'length' => 255, + ]); + + $table->setPrimaryKey(['id']); + $table->addIndex(['known_to'], 'ku_known_to'); + return $schema; + } + + return null; + } +} diff --git a/lib/composer/composer/autoload_classmap.php b/lib/composer/composer/autoload_classmap.php index 2e6b68f35b..65f050ed26 100644 --- a/lib/composer/composer/autoload_classmap.php +++ b/lib/composer/composer/autoload_classmap.php @@ -948,6 +948,7 @@ return array( 'OC\\Core\\Migrations\\Version21000Date20201120141228' => $baseDir . '/core/Migrations/Version21000Date20201120141228.php', 'OC\\Core\\Migrations\\Version21000Date20201202095923' => $baseDir . '/core/Migrations/Version21000Date20201202095923.php', 'OC\\Core\\Migrations\\Version21000Date20210119195004' => $baseDir . '/core/Migrations/Version21000Date20210119195004.php', + 'OC\\Core\\Migrations\\Version21000Date20210309185126' => $baseDir . '/core/Migrations/Version21000Date20210309185126.php', 'OC\\Core\\Migrations\\Version22000Date20210216080825' => $baseDir . '/core/Migrations/Version22000Date20210216080825.php', 'OC\\Core\\Notification\\CoreNotifier' => $baseDir . '/core/Notification/CoreNotifier.php', 'OC\\Core\\Service\\LoginFlowV2Service' => $baseDir . '/core/Service/LoginFlowV2Service.php', @@ -1165,6 +1166,9 @@ return array( 'OC\\IntegrityCheck\\Helpers\\FileAccessHelper' => $baseDir . '/lib/private/IntegrityCheck/Helpers/FileAccessHelper.php', 'OC\\IntegrityCheck\\Iterator\\ExcludeFileByNameFilterIterator' => $baseDir . '/lib/private/IntegrityCheck/Iterator/ExcludeFileByNameFilterIterator.php', '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 0a7ebd3ea3..de07f1831b 100644 --- a/lib/composer/composer/autoload_static.php +++ b/lib/composer/composer/autoload_static.php @@ -977,6 +977,7 @@ class ComposerStaticInit53792487c5a8370acc0b06b1a864ff4c 'OC\\Core\\Migrations\\Version21000Date20201120141228' => __DIR__ . '/../../..' . '/core/Migrations/Version21000Date20201120141228.php', 'OC\\Core\\Migrations\\Version21000Date20201202095923' => __DIR__ . '/../../..' . '/core/Migrations/Version21000Date20201202095923.php', 'OC\\Core\\Migrations\\Version21000Date20210119195004' => __DIR__ . '/../../..' . '/core/Migrations/Version21000Date20210119195004.php', + 'OC\\Core\\Migrations\\Version21000Date20210309185126' => __DIR__ . '/../../..' . '/core/Migrations/Version21000Date20210309185126.php', 'OC\\Core\\Migrations\\Version22000Date20210216080825' => __DIR__ . '/../../..' . '/core/Migrations/Version22000Date20210216080825.php', 'OC\\Core\\Notification\\CoreNotifier' => __DIR__ . '/../../..' . '/core/Notification/CoreNotifier.php', 'OC\\Core\\Service\\LoginFlowV2Service' => __DIR__ . '/../../..' . '/core/Service/LoginFlowV2Service.php', @@ -1194,6 +1195,9 @@ class ComposerStaticInit53792487c5a8370acc0b06b1a864ff4c 'OC\\IntegrityCheck\\Helpers\\FileAccessHelper' => __DIR__ . '/../../..' . '/lib/private/IntegrityCheck/Helpers/FileAccessHelper.php', 'OC\\IntegrityCheck\\Iterator\\ExcludeFileByNameFilterIterator' => __DIR__ . '/../../..' . '/lib/private/IntegrityCheck/Iterator/ExcludeFileByNameFilterIterator.php', '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/Collaboration/Collaborators/MailPlugin.php b/lib/private/Collaboration/Collaborators/MailPlugin.php index 7bdd29afc4..240e16374d 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,16 @@ 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 bool */ + protected $shareeEnumerationFullMatch; /** @var IManager */ private $contactsManager; @@ -52,20 +61,29 @@ 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'; + $this->shareeEnumerationFullMatch = $this->config->getAppValue('core', 'shareapi_restrict_user_enumeration_full_match', 'yes') === 'yes'; } /** @@ -77,6 +95,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'); @@ -120,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) { @@ -152,8 +172,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 +205,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 d832a42000..06a8c6f0ef 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,14 @@ 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 bool */ + protected $shareeEnumerationFullMatch; /** @var IConfig */ private $config; @@ -57,33 +64,30 @@ 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'; + $this->shareeEnumerationFullMatch = $this->config->getAppValue('core', 'shareapi_restrict_user_enumeration_full_match', 'yes') === 'yes'; } public function search($search, $limit, $offset, ISearchResult $searchResult) { @@ -91,6 +95,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 @@ -148,6 +153,7 @@ class UserPlugin implements ISearchPlugin { if ( + $this->shareeEnumerationFullMatch && $lowerSearch !== '' && (strtolower($uid) === $lowerSearch || strtolower($userDisplayName) === $lowerSearch || strtolower($userEmail) === $lowerSearch) @@ -168,11 +174,16 @@ class UserPlugin implements ISearchPlugin { ]; } else { $addToWideResults = false; - if ($this->shareeEnumeration && !$this->shareeEnumerationInGroupOnly) { + if ($this->shareeEnumeration && + !($this->shareeEnumerationInGroupOnly || $this->shareeEnumerationPhone)) { $addToWideResults = true; } - if ($this->shareeEnumerationInGroupOnly) { + if ($this->shareeEnumerationPhone && $this->knownUserService->isKnownToUser($currentUserId, $user->getUID())) { + $addToWideResults = true; + } + + if (!$addToWideResults && $this->shareeEnumerationInGroupOnly) { $commonGroups = array_intersect($currentUserGroups, $this->groupManager->getUserGroupIds($user)); if (!empty($commonGroups)) { $addToWideResults = true; @@ -195,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 e2bd7edc63..69f26c7969 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,22 @@ 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'; + $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 $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 +147,80 @@ 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, $allowEnumerationFullMatch, $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; + } - $mailAddresses = $entry->getEMailAddresses(); - foreach ($mailAddresses as $mailAddress) { - if ($mailAddress === $filter) { - $filterUser = false; - break; + $checkedCommonGroupAlready = false; + + // Prevent enumerating local users + if ($disallowEnumeration) { + if (!$allowEnumerationFullMatch) { + return false; + } + + $filterOutUser = true; + + $mailAddresses = $entry->getEMailAddresses(); + foreach ($mailAddresses as $mailAddress) { + if ($mailAddress === $filter) { + $filterOutUser = false; + break; + } + } + + if ($entry->getProperty('UID') && $entry->getProperty('UID') === $filter) { + $filterOutUser = false; + } + + if ($filterOutUser) { + return false; + } + } elseif ($restrictEnumerationPhone || $restrictEnumerationGroup) { + $canEnumerate = false; + if ($restrictEnumerationPhone) { + $canEnumerate = $this->knownUserService->isKnownToUser($selfUID, $entry->getProperty('UID')); + } + + if (!$canEnumerate && $restrictEnumerationGroup) { + $user = $this->userManager->get($entry->getProperty('UID')); + + if ($user === null) { + return false; + } + + $contactGroups = $this->groupManager->getUserGroupIds($user); + $canEnumerate = !empty(array_intersect($contactGroups, $selfGroups)); + $checkedCommonGroupAlready = true; + } + + if (!$canEnumerate) { + return false; } } - if ($entry->getProperty('UID') && $entry->getProperty('UID') === $filter) { - $filterUser = false; - } + if ($ownGroupsOnly && !$checkedCommonGroupAlready) { + $user = $this->userManager->get($entry->getProperty('UID')); - if ($filterUser) { - return false; + if (!$user instanceof IUser) { + 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; + } } } - if ($ownGroupsOnly && $entry->getProperty('isLocalSystemBook') === true) { - $uid = $this->userManager->get($entry->getProperty('UID')); - - if ($uid === null) { - 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; - } - } - - return $entry->getProperty('UID') !== $selfUID; + return true; })); } diff --git a/lib/private/KnownUser/KnownUser.php b/lib/private/KnownUser/KnownUser.php new file mode 100644 index 0000000000..939c9199c7 --- /dev/null +++ b/lib/private/KnownUser/KnownUser.php @@ -0,0 +1,46 @@ + + * + * @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; + +use OCP\AppFramework\Db\Entity; + +/** + * @method void setKnownTo(string $knownTo) + * @method string getKnownTo() + * @method void setKnownUser(string $knownUser) + * @method string getKnownUser() + */ +class KnownUser extends Entity { + + /** @var string */ + protected $knownTo; + + /** @var string */ + protected $knownUser; + + public function __construct() { + $this->addType('knownTo', 'string'); + $this->addType('knownUser', 'string'); + } +} diff --git a/lib/private/KnownUser/KnownUserMapper.php b/lib/private/KnownUser/KnownUserMapper.php new file mode 100644 index 0000000000..e77e475270 --- /dev/null +++ b/lib/private/KnownUser/KnownUserMapper.php @@ -0,0 +1,87 @@ + + * + * @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; + +use OCP\AppFramework\Db\QBMapper; +use OCP\IDBConnection; + +/** + * @method KnownUser mapRowToEntity(array $row) + */ +class KnownUserMapper extends QBMapper { + + /** + * @param IDBConnection $db + */ + public function __construct(IDBConnection $db) { + parent::__construct($db, 'known_users', KnownUser::class); + } + + /** + * @param string $knownTo + * @return int Number of deleted entities + */ + public function deleteKnownTo(string $knownTo): int { + $query = $this->db->getQueryBuilder(); + $query->delete($this->getTableName()) + ->where($query->expr()->eq('known_to', $query->createNamedParameter($knownTo))); + + return (int) $query->execute(); + } + + /** + * @param string $knownUser + * @return int Number of deleted entities + */ + public function deleteKnownUser(string $knownUser): int { + $query = $this->db->getQueryBuilder(); + $query->delete($this->getTableName()) + ->where($query->expr()->eq('known_user', $query->createNamedParameter($knownUser))); + + return (int) $query->execute(); + } + + /** + * Returns all "known users" for the given "known to" user + * + * @param string $knownTo + * @return KnownUser[] + */ + public function getKnownUsers(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'], + 'known_to' => $row['known_to'], + 'known_user' => $row['known_user'], + ]); + } +} diff --git a/lib/private/KnownUser/KnownUserService.php b/lib/private/KnownUser/KnownUserService.php new file mode 100644 index 0000000000..96af21c836 --- /dev/null +++ b/lib/private/KnownUser/KnownUserService.php @@ -0,0 +1,87 @@ + + * + * @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; + } + + /** + * 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); + } + + /** + * 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); + } + + /** + * 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($contactUserId); + $this->mapper->insert($entity); + } + + /** + * 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->getKnownUsers($knownTo); + $this->knownUsers[$knownTo] = []; + foreach ($entities as $entity) { + $this->knownUsers[$knownTo][$entity->getKnownUser()] = true; + } + } + + return isset($this->knownUsers[$knownTo][$contactUserId]); + } +} diff --git a/lib/private/Share20/Manager.php b/lib/private/Share20/Manager.php index 5c8dba5915..ce1ec1d60f 100644 --- a/lib/private/Share20/Manager.php +++ b/lib/private/Share20/Manager.php @@ -1829,6 +1829,15 @@ 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'; + } + + 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 635ccc1483..606e642991 100644 --- a/lib/public/Share/IManager.php +++ b/lib/public/Share/IManager.php @@ -384,6 +384,22 @@ 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 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/acceptance/features/bootstrap/SettingsContext.php b/tests/acceptance/features/bootstrap/SettingsContext.php index 2a3aeff2f2..6b60b2c317 100644 --- a/tests/acceptance/features/bootstrap/SettingsContext.php +++ b/tests/acceptance/features/bootstrap/SettingsContext.php @@ -71,16 +71,16 @@ class SettingsContext implements Context, ActorAwareInterface { // forThe()->checkbox("Restrict username...") can not be used here; that // would return the checkbox itself, but the element that the user // interacts with is the label. - return Locator::forThe()->xpath("//label[normalize-space() = 'Restrict username autocompletion to users within the same groups']")-> - describedAs("Restrict username autocompletion to groups checkbox in Sharing section in Administration Sharing Settings"); + return Locator::forThe()->xpath("//label[normalize-space() = 'Allow username autocompletion to users within the same groups']")-> + describedAs("Allow username autocompletion to users within the same groups checkbox in Sharing section in Administration Sharing Settings"); } /** * @return Locator */ public static function restrictUsernameAutocompletionToGroupsCheckboxInput() { - return Locator::forThe()->checkbox("Restrict username autocompletion to users within the same groups")-> - describedAs("Restrict username autocompletion to groups checkbox input in Sharing section in Administration Sharing Settings"); + return Locator::forThe()->checkbox("Allow username autocompletion to users within the same groups")-> + describedAs("Allow username autocompletion to users within the same groups checkbox input in Sharing section in Administration Sharing Settings"); } /** diff --git a/tests/lib/Collaboration/Collaborators/MailPluginTest.php b/tests/lib/Collaboration/Collaborators/MailPluginTest.php index 141d4b680b..3128231a10 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 2806540d00..f2e0e7e274 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 acfe83ac55..ad201d86a2 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,15 @@ 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->expects($this->at(1)) ->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('no'); - - $this->config->expects($this->at(3)) + $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', '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'], + ]); /** @var IUser|\PHPUnit\Framework\MockObject\MockObject $currentUser */ $currentUser = $this->createMock(IUser::class); @@ -314,22 +305,15 @@ class ContactsStoreTest extends TestCase { } 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'); - - $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'); - - $this->config->expects($this->at(2)) ->method('getAppValue') - ->with($this->equalTo('core'), $this->equalTo('shareapi_exclude_groups'), $this->equalTo('no')) - ->willReturn('no'); - - $this->config->expects($this->at(3)) + $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', '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); @@ -399,10 +383,312 @@ class ContactsStoreTest extends TestCase { $this->assertEquals('contact', $entries[2]->getProperty('UID')); } + 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'], + ]); + + /** @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']); + + $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')); + } + + public function testGetContactsOnlyEnumerateIfPhoneBookMatchWithOwnGroupsOnly() { + $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', '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); + $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', 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(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 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); @@ -483,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')) diff --git a/version.php b/version.php index 90b23f3427..695717e652 100644 --- a/version.php +++ b/version.php @@ -30,7 +30,7 @@ // between betas, final and RCs. This is _not_ the public version number. Reset minor/patchlevel // when updating major/minor version number. -$OC_Version = [22, 0, 0, 1]; +$OC_Version = [22, 0, 0, 2]; // The human readable string $OC_VersionString = '22.0.0 alpha';