Merge pull request #29427 from nextcloud/fix/user_status_enumeration_22

Backport #29260: Respect user enumeration settings in user status lists

apps/user_status/lib/AppInfo/Application.php

@@ -36,6 +36,7 @@
 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 @@ public function register(IRegistrationContext $context): void {
 		$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 {

apps/user_status/lib/Service/StatusService.php

@@ -35,6 +35,7 @@
 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 @@ public function __construct(UserStatusMapper $mapper,
 	 * @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 @@ public function findAll(?int $limit = null, ?int $offset = null): array {
 	 * @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 @@ public function setPredefinedMessage(string $userId,
 	/**
 	 * @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 @@ public function removeUserStatus(string $userId): bool {
 	 * 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();

apps/user_status/tests/Unit/Service/StatusServiceTest.php

@@ -38,6 +38,7 @@
 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 @@ protected function setUp(): void {
 		$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 @@ public function testFindAllRecentStatusChanges(): void {
 		], $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())

build/integration/collaboration_features/user_status.feature

@@ -0,0 +1,20 @@
+Feature: user_status
+  Background:
+    Given using api version "2"
+    And user "user0" exists
+    And user "user0" has status "dnd"
+
+  Scenario: listing recent user statuses with default settings
+    Then user statuses for "admin" list "user0" with status "dnd"
+
+  Scenario: empty recent user statuses with disabled/limited user enumeration
+    When parameter "shareapi_allow_share_dialog_user_enumeration" of app "core" is set to "no"
+    Then user statuses for "admin" are empty
+    When parameter "shareapi_allow_share_dialog_user_enumeration" of app "core" is set to "yes"
+    When parameter "shareapi_restrict_user_enumeration_to_group" of app "core" is set to "yes"
+    Then user statuses for "admin" are empty
+    When parameter "shareapi_restrict_user_enumeration_to_group" of app "core" is set to "no"
+    When parameter "shareapi_restrict_user_enumeration_to_phone" of app "core" is set to "yes"
+    Then user statuses for "admin" are empty
+    When parameter "shareapi_restrict_user_enumeration_to_phone" of app "core" is set to "no"
+    Then user statuses for "admin" list "user0" with status "dnd"

build/integration/features/bootstrap/CollaborationContext.php

@@ -25,6 +25,7 @@
  */
 use Behat\Behat\Context\Context;
 use Behat\Gherkin\Node\TableNode;
+use GuzzleHttp\Client;
 use PHPUnit\Framework\Assert;
 
 require __DIR__ . '/../../vendor/autoload.php';
@@ -70,4 +71,94 @@ protected function resetAppConfigs(): void {
 		$this->deleteServerConfig('core', 'shareapi_restrict_user_enumeration_full_match');
 		$this->deleteServerConfig('core', 'shareapi_only_share_with_group_members');
 	}
+
+	/**
+	 * @Given /^user "([^"]*)" has status "([^"]*)"$/
+	 * @param string $user
+	 * @param string $status
+	 */
+	public function assureUserHasStatus($user, $status) {
+		$fullUrl = $this->baseUrl . "v{$this->apiVersion}.php/apps/user_status/api/v1/user_status/status";
+		$client = new Client();
+		$options = [
+			'headers' => [
+				'OCS-APIREQUEST' => 'true',
+			],
+		];
+		if ($user === 'admin') {
+			$options['auth'] = $this->adminUser;
+		} else {
+			$options['auth'] = [$user, $this->regularUser];
+		}
+
+		$options['form_params'] = [
+			'statusType' => $status
+		];
+
+		$this->response = $client->put($fullUrl, $options);
+		$this->theHTTPStatusCodeShouldBe(200);
+
+		$fullUrl = $this->baseUrl . "v{$this->apiVersion}.php/apps/user_status/api/v1/user_status";
+		unset($options['form_params']);
+		$this->response = $client->get($fullUrl, $options);
+		$this->theHTTPStatusCodeShouldBe(200);
+
+		$returnedStatus = json_decode(json_encode(simplexml_load_string($this->response->getBody()->getContents())->data), true)['status'];
+		Assert::assertEquals($status, $returnedStatus);
+	}
+
+	/**
+	 * @param string $user
+	 * @return null|array
+	 */
+	public function getStatusList(string $user): ?array {
+		$fullUrl = $this->baseUrl . "v{$this->apiVersion}.php/apps/user_status/api/v1/statuses";
+		$client = new Client();
+		$options = [
+			'headers' => [
+				'OCS-APIREQUEST' => 'true',
+			],
+		];
+		if ($user === 'admin') {
+			$options['auth'] = $this->adminUser;
+		} else {
+			$options['auth'] = [$user, $this->regularUser];
+		}
+
+		$this->response = $client->get($fullUrl, $options);
+		$this->theHTTPStatusCodeShouldBe(200);
+
+		$contents = $this->response->getBody()->getContents();
+		return json_decode(json_encode(simplexml_load_string($contents)->data), true);
+	}
+
+	/**
+	 * @Given /^user statuses for "([^"]*)" list "([^"]*)" with status "([^"]*)"$/
+	 * @param string $user
+	 * @param string $statusUser
+	 * @param string $status
+	 */
+	public function assertStatusesList(string $user, string $statusUser, string $status): void {
+		$statusList = $this->getStatusList($user);
+		Assert::assertArrayHasKey('element', $statusList, 'Returned status list empty or broken');
+		if (array_key_exists('userId', $statusList['element'])) {
+			// If only one user has a status set, the API returns their status directly
+			Assert::assertArrayHasKey('status', $statusList['element'], 'Returned status list empty or broken');
+			$filteredStatusList = [ $statusList['element']['userId'] => $statusList['element']['status'] ];
+		} else {
+			// If more than one user have their status set, the API returns an array of their statuses
+			$filteredStatusList = array_column($statusList['element'], 'status', 'userId');
+		}
+		Assert::assertArrayHasKey($statusUser, $filteredStatusList, 'User not listed in statuses: ' . $statusUser);
+		Assert::assertEquals($status, $filteredStatusList[$statusUser]);
+	}
+
+	/**
+	 * @Given /^user statuses for "([^"]*)" are empty$/
+	 * @param string $user
+	 */
+	public function assertStatusesEmpty(string $user): void {
+		$statusList = $this->getStatusList($user);
+		Assert::assertEmpty($statusList);
+	}
 }

build/integration/features/bootstrap/Provisioning.php

@@ -13,6 +13,7 @@
  * @author Sergio Bertolín <[email protected]>
  * @author Thomas Müller <[email protected]>
  * @author Vincent Petry <[email protected]>
+ * @author Jonas Meurer <[email protected]>
  *
  * @license GNU AGPL version 3 or any later version
  *