Return empty user status list if user enumeration is disabled (Fixes: #…

…27122) The functions to find statuses from other users listed other users even if with disabled enumeration (`shareapi_allow_share_dialog_user_enumeration` setting in core app settings). Now the functions respect `shareapi_allow_share_dialog_user_enumeration` and return empty lists if it is not set to `yes`. Fixes: #27122 Signed-off-by: Jonas Meurer <jonas@freesources.org>
nextcloud · Jul 8, 2021 · 260b3ed · 260b3ed
1 parent 27fb46c
commit 260b3ed
Show file tree

Hide file tree

Showing 2 changed files with 50 additions and 6 deletions.
diff --git a/apps/user_status/lib/Service/StatusService.php b/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;
 
 /**
@@ -83,22 +84,28 @@ class StatusService {
 	/** @var int */
 	public const MAXIMUM_MESSAGE_LENGTH = 80;
 
+	/** @var IConfig */
+	private $config;
+
 	/**
 	 * StatusService constructor.
 	 *
 	 * @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->config = $config;
 	}
 
 	/**
@@ -107,9 +114,10 @@ public function __construct(UserStatusMapper $mapper,
 	 * @return UserStatus[]
 	 */
 	public function findAll(?int $limit = null, ?int $offset = null): array {
+		$allowEnumeration = $this->config->getAppValue('core', 'shareapi_allow_share_dialog_user_enumeration', 'yes') === 'yes';
 		return array_map(function ($status) {
 			return $this->processStatus($status);
-		}, $this->mapper->findAll($limit, $offset));
+		}, $allowEnumeration ? $this->mapper->findAll($limit, $offset) : []);
 	}
 
 	/**
@@ -118,10 +126,11 @@ public function findAll(?int $limit = null, ?int $offset = null): array {
 	 * @return array
 	 */
 	public function findAllRecentStatusChanges(?int $limit = null, ?int $offset = null): array {
+		$allowEnumeration = $this->config->getAppValue('core', 'shareapi_allow_share_dialog_user_enumeration', 'yes') === 'yes';
 		return array_map(function ($status) {
 			return $this->processStatus($status);
-		}, $this->mapper->findAllRecent($limit, $offset));
-	}
+		}, $allowEnumeration ? $this->mapper->findAllRecent($limit, $offset) : []);
+}
 
 	/**
 	 * @param string $userId

diff --git 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\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,14 @@ 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->service = new StatusService($this->mapper,
 			$this->timeFactory,
 			$this->predefinedStatusService,
-			$this->emojiService);
+			$this->emojiService,
+			$this->config);
 	}
 
 	public function testFindAll(): void {
@@ -80,6 +88,11 @@ public function testFindAll(): void {
 			->with(20, 50)
 			->willReturn([$status1, $status2]);
 
+		$this->config->expects($this->once())
+			->method('getAppValue')
+			->with('core', 'shareapi_allow_share_dialog_user_enumeration')
+			->willReturn('yes');
+
 		$this->assertEquals([
 			$status1,
 			$status2,
@@ -95,12 +108,34 @@ public function testFindAllRecentStatusChanges(): void {
 			->with(20, 50)
 			->willReturn([$status1, $status2]);
 
+		$this->config->expects($this->once())
+			->method('getAppValue')
+			->with('core', 'shareapi_allow_share_dialog_user_enumeration')
+			->willReturn('yes');
+
 		$this->assertEquals([
 			$status1,
 			$status2,
 		], $this->service->findAllRecentStatusChanges(20, 50));
 	}
 
+	public function testFindAllRecentStatusChangesNoEnumeration(): void {
+		$status1 = $this->createMock(UserStatus::class);
+		$status2 = $this->createMock(UserStatus::class);
+
+		$this->mapper->expects($this->never())
+			->method('findAllRecent')
+			->with(20, 50)
+			->willReturn([$status1, $status2]);
+
+		$this->config->expects($this->once())
+			->method('getAppValue')
+			->with('core', 'shareapi_allow_share_dialog_user_enumeration')
+			->willReturn('no');
+
+		$this->assertEquals([], $this->service->findAllRecentStatusChanges(20, 50));
+	}
+
 	public function testFindByUserId(): void {
 		$status = $this->createMock(UserStatus::class);
 		$this->mapper->expects($this->once())