Respect user enumeration settings in user status lists

So far, the functions to find user statuses listed didn't respect user
enumeration settings (`shareapi_allow_share_dialog_user_enumeration`
and `shareapi_restrict_user_enumeration_to_group` core app settings).

Fix this privacy issue by returning an empty list in case
`shareapi_allow_share_dialog_user_enumeration` is unset or
`shareapi_restrict_user_enumeration_to_group` is set.

In the long run, we might want to return users from common groups if
`shareapi_restrict_user_enumeration_to_group` is set. It's complicated
to implement this in a way that scales, though. See the discussion at
https://github.com/nextcloud/server/pull/27879#pullrequestreview-753655308
for details.

Also, don't register the user_status dashboard widget at all if
`shareapi_allow_share_dialog_user_enumeration` is unset or
`shareapi_restrict_user_enumeration_to_group` is set.

Fixes: #27122
Signed-off-by: Jonas Meurer <jonas@freesources.org>

diff --git a/apps/user_status/lib/AppInfo/Application.php b/apps/user_status/lib/AppInfo/Application.php
index 8ccf0f497c33f4ee2ae6f4457741c01db1543d38..7a47fc45c95ce78f3fd7b4223be020bf4b84fdcf 100644 (file)
--- 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 f121089ad8e0068f3ede4ea0780035ca954ae6fa..2efdba0eb565239cf316d627e65b34290cb208d2 100644 (file)
--- a/apps/user_status/lib/Service/StatusService.php
+++ b/apps/user_status/lib/Service/StatusService.php
@@ -35,6 +35,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;
 
 /**
@@ -56,6 +57,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
         */
@@ -88,17 +98,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';
        }
 
        /**
@@ -107,6 +122,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));
@@ -118,6 +140,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));
@@ -225,7 +254,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
@@ -333,7 +362,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 b4215778a990cfef3774448c5f0a7df1670d30f8..7141c809b31984fe90a5a4ae37678a349ce5f75b 100644 (file)
--- a/apps/user_status/tests/Unit/Service/StatusServiceTest.php
+++ b/apps/user_status/tests/Unit/Service/StatusServiceTest.php
@@ -38,6 +38,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;
 
@@ -55,6 +56,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;
 
@@ -65,10 +69,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 {
@@ -101,6 +115,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())