diff options
author | Lukas Reschke <lukas@statuscode.ch> | 2021-10-25 15:53:55 +0300 |
---|---|---|
committer | GitHub <noreply@github.com> | 2021-10-25 15:53:55 +0300 |
commit | 0f9e090f0c914ec8e132b1474ef4fa4a8630606e (patch) | |
tree | 4b99b1bf73c9b4126476fdaebf243e5f34cb1310 /apps | |
parent | 2a4e92dcbdb4c68ec8476d8c77adbee2680d5707 (diff) | |
parent | f8719bce2f57d8dd596897fbcb7f3eb8994589f1 (diff) |
Merge pull request #29429 from nextcloud/fix/user_status_enumeration_20
Backport #29260: Respect user enumeration settings in user status lists
Diffstat (limited to 'apps')
-rw-r--r-- | apps/user_status/lib/AppInfo/Application.php | 12 | ||||
-rw-r--r-- | apps/user_status/lib/Service/StatusService.php | 37 | ||||
-rw-r--r-- | apps/user_status/tests/Unit/Service/StatusServiceTest.php | 59 |
3 files changed, 101 insertions, 7 deletions
diff --git a/apps/user_status/lib/AppInfo/Application.php b/apps/user_status/lib/AppInfo/Application.php index 2e511500ea6..3a2d742b26e 100644 --- a/apps/user_status/lib/AppInfo/Application.php +++ b/apps/user_status/lib/AppInfo/Application.php @@ -36,6 +36,7 @@ use OCP\AppFramework\Bootstrap\IBootContext; use OCP\AppFramework\Bootstrap\IBootstrap; use OCP\AppFramework\Bootstrap\IRegistrationContext; use OCP\AppFramework\Http\Events\BeforeTemplateRenderedEvent; +use OCP\IConfig; use OCP\User\Events\UserDeletedEvent; use OCP\User\Events\UserLiveStatusEvent; use OCP\UserStatus\IManager; @@ -71,8 +72,15 @@ class Application extends App implements IBootstrap { $context->registerEventListener(UserLiveStatusEvent::class, UserLiveStatusListener::class); $context->registerEventListener(BeforeTemplateRenderedEvent::class, BeforeTemplateRenderedListener::class); - // Register the Dashboard panel - $context->registerDashboardWidget(UserStatusWidget::class); + $config = $this->getContainer()->query(IConfig::class); + $shareeEnumeration = $config->getAppValue('core', 'shareapi_allow_share_dialog_user_enumeration', 'yes') === 'yes'; + $shareeEnumerationInGroupOnly = $shareeEnumeration && $config->getAppValue('core', 'shareapi_restrict_user_enumeration_to_group', 'no') === 'yes'; + $shareeEnumerationPhone = $shareeEnumeration && $config->getAppValue('core', 'shareapi_restrict_user_enumeration_to_phone', 'no') === 'yes'; + + // Register the Dashboard panel if user enumeration is enabled and not limited + if ($shareeEnumeration && !$shareeEnumerationInGroupOnly && !$shareeEnumerationPhone) { + $context->registerDashboardWidget(UserStatusWidget::class); + } } public function boot(IBootContext $context): void { diff --git a/apps/user_status/lib/Service/StatusService.php b/apps/user_status/lib/Service/StatusService.php index 1545e3da4f0..1c32b0e8b71 100644 --- a/apps/user_status/lib/Service/StatusService.php +++ b/apps/user_status/lib/Service/StatusService.php @@ -34,6 +34,7 @@ use OCA\UserStatus\Exception\InvalidStatusTypeException; use OCA\UserStatus\Exception\StatusMessageTooLongException; use OCP\AppFramework\Db\DoesNotExistException; use OCP\AppFramework\Utility\ITimeFactory; +use OCP\IConfig; use OCP\UserStatus\IUserStatus; /** @@ -55,6 +56,15 @@ class StatusService { /** @var EmojiService */ private $emojiService; + /** @var bool */ + private $shareeEnumeration; + + /** @var bool */ + private $shareeEnumerationInGroupOnly; + + /** @var bool */ + private $shareeEnumerationPhone; + /** * List of priorities ordered by their priority */ @@ -87,17 +97,22 @@ class StatusService { * * @param UserStatusMapper $mapper * @param ITimeFactory $timeFactory - * @param PredefinedStatusService $defaultStatusService, + * @param PredefinedStatusService $defaultStatusService * @param EmojiService $emojiService + * @param IConfig $config */ public function __construct(UserStatusMapper $mapper, ITimeFactory $timeFactory, PredefinedStatusService $defaultStatusService, - EmojiService $emojiService) { + EmojiService $emojiService, + IConfig $config) { $this->mapper = $mapper; $this->timeFactory = $timeFactory; $this->predefinedStatusService = $defaultStatusService; $this->emojiService = $emojiService; + $this->shareeEnumeration = $config->getAppValue('core', 'shareapi_allow_share_dialog_user_enumeration', 'yes') === 'yes'; + $this->shareeEnumerationInGroupOnly = $this->shareeEnumeration && $config->getAppValue('core', 'shareapi_restrict_user_enumeration_to_group', 'no') === 'yes'; + $this->shareeEnumerationPhone = $this->shareeEnumeration && $config->getAppValue('core', 'shareapi_restrict_user_enumeration_to_phone', 'no') === 'yes'; } /** @@ -106,6 +121,13 @@ class StatusService { * @return UserStatus[] */ public function findAll(?int $limit = null, ?int $offset = null): array { + // Return empty array if user enumeration is disabled or limited to groups + // TODO: find a solution that scales to get only users from common groups if user enumeration is limited to + // groups. See discussion at https://github.com/nextcloud/server/pull/27879#discussion_r729715936 + if (!$this->shareeEnumeration || $this->shareeEnumerationInGroupOnly || $this->shareeEnumerationPhone) { + return []; + } + return array_map(function ($status) { return $this->processStatus($status); }, $this->mapper->findAll($limit, $offset)); @@ -117,6 +139,13 @@ class StatusService { * @return array */ public function findAllRecentStatusChanges(?int $limit = null, ?int $offset = null): array { + // Return empty array if user enumeration is disabled or limited to groups + // TODO: find a solution that scales to get only users from common groups if user enumeration is limited to + // groups. See discussion at https://github.com/nextcloud/server/pull/27879#discussion_r729715936 + if (!$this->shareeEnumeration || $this->shareeEnumerationInGroupOnly || $this->shareeEnumerationPhone) { + return []; + } + return array_map(function ($status) { return $this->processStatus($status); }, $this->mapper->findAllRecent($limit, $offset)); @@ -224,7 +253,7 @@ class StatusService { /** * @param string $userId * @param string|null $statusIcon - * @param string|null $message + * @param string $message * @param int|null $clearAt * @return UserStatus * @throws InvalidClearAtException @@ -332,7 +361,7 @@ class StatusService { * up to date and provides translated default status if needed * * @param UserStatus $status - * @returns UserStatus + * @return UserStatus */ private function processStatus(UserStatus $status): UserStatus { $clearAt = $status->getClearAt(); diff --git a/apps/user_status/tests/Unit/Service/StatusServiceTest.php b/apps/user_status/tests/Unit/Service/StatusServiceTest.php index 77209b70f48..167646aac82 100644 --- a/apps/user_status/tests/Unit/Service/StatusServiceTest.php +++ b/apps/user_status/tests/Unit/Service/StatusServiceTest.php @@ -37,6 +37,7 @@ use OCA\UserStatus\Service\PredefinedStatusService; use OCA\UserStatus\Service\StatusService; use OCP\AppFramework\Db\DoesNotExistException; use OCP\AppFramework\Utility\ITimeFactory; +use OCP\IConfig; use OCP\UserStatus\IUserStatus; use Test\TestCase; @@ -54,6 +55,9 @@ class StatusServiceTest extends TestCase { /** @var EmojiService|\PHPUnit\Framework\MockObject\MockObject */ private $emojiService; + /** @var IConfig|\PHPUnit\Framework\MockObject\MockObject */ + private $config; + /** @var StatusService */ private $service; @@ -64,10 +68,20 @@ class StatusServiceTest extends TestCase { $this->timeFactory = $this->createMock(ITimeFactory::class); $this->predefinedStatusService = $this->createMock(PredefinedStatusService::class); $this->emojiService = $this->createMock(EmojiService::class); + + $this->config = $this->createMock(IConfig::class); + + $this->config->method('getAppValue') + ->willReturnMap([ + ['core', 'shareapi_allow_share_dialog_user_enumeration', 'yes', 'yes'], + ['core', 'shareapi_restrict_user_enumeration_to_group', 'no', 'no'] + ]); + $this->service = new StatusService($this->mapper, $this->timeFactory, $this->predefinedStatusService, - $this->emojiService); + $this->emojiService, + $this->config); } public function testFindAll(): void { @@ -100,6 +114,49 @@ class StatusServiceTest extends TestCase { ], $this->service->findAllRecentStatusChanges(20, 50)); } + public function testFindAllRecentStatusChangesNoEnumeration(): void { + $status1 = $this->createMock(UserStatus::class); + $status2 = $this->createMock(UserStatus::class); + + $this->mapper->method('findAllRecent') + ->with(20, 50) + ->willReturn([$status1, $status2]); + + // Rebuild $this->service with user enumeration turned off + $this->config = $this->createMock(IConfig::class); + + $this->config->method('getAppValue') + ->willReturnMap([ + ['core', 'shareapi_allow_share_dialog_user_enumeration', 'yes', 'no'], + ['core', 'shareapi_restrict_user_enumeration_to_group', 'no', 'no'] + ]); + + $this->service = new StatusService($this->mapper, + $this->timeFactory, + $this->predefinedStatusService, + $this->emojiService, + $this->config); + + $this->assertEquals([], $this->service->findAllRecentStatusChanges(20, 50)); + + // Rebuild $this->service with user enumeration limited to common groups + $this->config = $this->createMock(IConfig::class); + + $this->config->method('getAppValue') + ->willReturnMap([ + ['core', 'shareapi_allow_share_dialog_user_enumeration', 'yes', 'yes'], + ['core', 'shareapi_restrict_user_enumeration_to_group', 'no', 'yes'] + ]); + + $this->service = new StatusService($this->mapper, + $this->timeFactory, + $this->predefinedStatusService, + $this->emojiService, + $this->config); + + $this->assertEquals([], $this->service->findAllRecentStatusChanges(20, 50)); + } + public function testFindByUserId(): void { $status = $this->createMock(UserStatus::class); $this->mapper->expects($this->once()) |