Skip to content

Commit 9fc4bd5

Browse files
LukasReschkeLukasReschke
authored
Merge pull request #29428 from nextcloud/fix/user_status_enumeration_21
Backport #29260: Respect user enumeration settings in user status lists
2 parents 082d429 + e838e63 commit 9fc4bd5

6 files changed

Lines changed: 213 additions & 7 deletions

File tree

‎apps/user_status/lib/AppInfo/Application.php‎

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,7 @@
3636
use OCP\AppFramework\Bootstrap\IBootstrap;
3737
use OCP\AppFramework\Bootstrap\IRegistrationContext;
3838
use OCP\AppFramework\Http\Events\BeforeTemplateRenderedEvent;
39+
use OCP\IConfig;
3940
use OCP\User\Events\UserDeletedEvent;
4041
use OCP\User\Events\UserLiveStatusEvent;
4142
use OCP\UserStatus\IManager;
@@ -71,8 +72,15 @@ public function register(IRegistrationContext $context): void {
7172
$context->registerEventListener(UserLiveStatusEvent::class, UserLiveStatusListener::class);
7273
$context->registerEventListener(BeforeTemplateRenderedEvent::class, BeforeTemplateRenderedListener::class);
7374

74-
// Register the Dashboard panel
75-
$context->registerDashboardWidget(UserStatusWidget::class);
75+
$config = $this->getContainer()->query(IConfig::class);
76+
$shareeEnumeration = $config->getAppValue('core', 'shareapi_allow_share_dialog_user_enumeration', 'yes') === 'yes';
77+
$shareeEnumerationInGroupOnly = $shareeEnumeration && $config->getAppValue('core', 'shareapi_restrict_user_enumeration_to_group', 'no') === 'yes';
78+
$shareeEnumerationPhone = $shareeEnumeration && $config->getAppValue('core', 'shareapi_restrict_user_enumeration_to_phone', 'no') === 'yes';
79+
80+
// Register the Dashboard panel if user enumeration is enabled and not limited
81+
if ($shareeEnumeration && !$shareeEnumerationInGroupOnly && !$shareeEnumerationPhone) {
82+
$context->registerDashboardWidget(UserStatusWidget::class);
83+
}
7684
}
7785

7886
public function boot(IBootContext $context): void {

‎apps/user_status/lib/Service/StatusService.php‎

Lines changed: 33 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,7 @@
3434
use OCA\UserStatus\Exception\StatusMessageTooLongException;
3535
use OCP\AppFramework\Db\DoesNotExistException;
3636
use OCP\AppFramework\Utility\ITimeFactory;
37+
use OCP\IConfig;
3738
use OCP\UserStatus\IUserStatus;
3839

3940
/**
@@ -55,6 +56,15 @@ class StatusService {
5556
/** @var EmojiService */
5657
private $emojiService;
5758

59+
/** @var bool */
60+
private $shareeEnumeration;
61+
62+
/** @var bool */
63+
private $shareeEnumerationInGroupOnly;
64+
65+
/** @var bool */
66+
private $shareeEnumerationPhone;
67+
5868
/**
5969
* List of priorities ordered by their priority
6070
*/
@@ -87,17 +97,22 @@ class StatusService {
8797
*
8898
* @param UserStatusMapper $mapper
8999
* @param ITimeFactory $timeFactory
90-
* @param PredefinedStatusService $defaultStatusService,
100+
* @param PredefinedStatusService $defaultStatusService
91101
* @param EmojiService $emojiService
102+
* @param IConfig $config
92103
*/
93104
public function __construct(UserStatusMapper $mapper,
94105
ITimeFactory $timeFactory,
95106
PredefinedStatusService $defaultStatusService,
96-
EmojiService $emojiService) {
107+
EmojiService $emojiService,
108+
IConfig $config) {
97109
$this->mapper = $mapper;
98110
$this->timeFactory = $timeFactory;
99111
$this->predefinedStatusService = $defaultStatusService;
100112
$this->emojiService = $emojiService;
113+
$this->shareeEnumeration = $config->getAppValue('core', 'shareapi_allow_share_dialog_user_enumeration', 'yes') === 'yes';
114+
$this->shareeEnumerationInGroupOnly = $this->shareeEnumeration && $config->getAppValue('core', 'shareapi_restrict_user_enumeration_to_group', 'no') === 'yes';
115+
$this->shareeEnumerationPhone = $this->shareeEnumeration && $config->getAppValue('core', 'shareapi_restrict_user_enumeration_to_phone', 'no') === 'yes';
101116
}
102117

103118
/**
@@ -106,6 +121,13 @@ public function __construct(UserStatusMapper $mapper,
106121
* @return UserStatus[]
107122
*/
108123
public function findAll(?int $limit = null, ?int $offset = null): array {
124+
// Return empty array if user enumeration is disabled or limited to groups
125+
// TODO: find a solution that scales to get only users from common groups if user enumeration is limited to
126+
// groups. See discussion at https://github.com/nextcloud/server/pull/27879#discussion_r729715936
127+
if (!$this->shareeEnumeration || $this->shareeEnumerationInGroupOnly || $this->shareeEnumerationPhone) {
128+
return [];
129+
}
130+
109131
return array_map(function ($status) {
110132
return $this->processStatus($status);
111133
}, $this->mapper->findAll($limit, $offset));
@@ -117,6 +139,13 @@ public function findAll(?int $limit = null, ?int $offset = null): array {
117139
* @return array
118140
*/
119141
public function findAllRecentStatusChanges(?int $limit = null, ?int $offset = null): array {
142+
// Return empty array if user enumeration is disabled or limited to groups
143+
// TODO: find a solution that scales to get only users from common groups if user enumeration is limited to
144+
// groups. See discussion at https://github.com/nextcloud/server/pull/27879#discussion_r729715936
145+
if (!$this->shareeEnumeration || $this->shareeEnumerationInGroupOnly || $this->shareeEnumerationPhone) {
146+
return [];
147+
}
148+
120149
return array_map(function ($status) {
121150
return $this->processStatus($status);
122151
}, $this->mapper->findAllRecent($limit, $offset));
@@ -224,7 +253,7 @@ public function setPredefinedMessage(string $userId,
224253
/**
225254
* @param string $userId
226255
* @param string|null $statusIcon
227-
* @param string|null $message
256+
* @param string $message
228257
* @param int|null $clearAt
229258
* @return UserStatus
230259
* @throws InvalidClearAtException
@@ -332,7 +361,7 @@ public function removeUserStatus(string $userId): bool {
332361
* up to date and provides translated default status if needed
333362
*
334363
* @param UserStatus $status
335-
* @returns UserStatus
364+
* @return UserStatus
336365
*/
337366
private function processStatus(UserStatus $status): UserStatus {
338367
$clearAt = $status->getClearAt();

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

Lines changed: 58 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,7 @@
3737
use OCA\UserStatus\Service\StatusService;
3838
use OCP\AppFramework\Db\DoesNotExistException;
3939
use OCP\AppFramework\Utility\ITimeFactory;
40+
use OCP\IConfig;
4041
use OCP\UserStatus\IUserStatus;
4142
use Test\TestCase;
4243

@@ -54,6 +55,9 @@ class StatusServiceTest extends TestCase {
5455
/** @var EmojiService|\PHPUnit\Framework\MockObject\MockObject */
5556
private $emojiService;
5657

58+
/** @var IConfig|\PHPUnit\Framework\MockObject\MockObject */
59+
private $config;
60+
5761
/** @var StatusService */
5862
private $service;
5963

@@ -64,10 +68,20 @@ protected function setUp(): void {
6468
$this->timeFactory = $this->createMock(ITimeFactory::class);
6569
$this->predefinedStatusService = $this->createMock(PredefinedStatusService::class);
6670
$this->emojiService = $this->createMock(EmojiService::class);
71+
72+
$this->config = $this->createMock(IConfig::class);
73+
74+
$this->config->method('getAppValue')
75+
->willReturnMap([
76+
['core', 'shareapi_allow_share_dialog_user_enumeration', 'yes', 'yes'],
77+
['core', 'shareapi_restrict_user_enumeration_to_group', 'no', 'no']
78+
]);
79+
6780
$this->service = new StatusService($this->mapper,
6881
$this->timeFactory,
6982
$this->predefinedStatusService,
70-
$this->emojiService);
83+
$this->emojiService,
84+
$this->config);
7185
}
7286

7387
public function testFindAll(): void {
@@ -100,6 +114,49 @@ public function testFindAllRecentStatusChanges(): void {
100114
], $this->service->findAllRecentStatusChanges(20, 50));
101115
}
102116

117+
public function testFindAllRecentStatusChangesNoEnumeration(): void {
118+
$status1 = $this->createMock(UserStatus::class);
119+
$status2 = $this->createMock(UserStatus::class);
120+
121+
$this->mapper->method('findAllRecent')
122+
->with(20, 50)
123+
->willReturn([$status1, $status2]);
124+
125+
// Rebuild $this->service with user enumeration turned off
126+
$this->config = $this->createMock(IConfig::class);
127+
128+
$this->config->method('getAppValue')
129+
->willReturnMap([
130+
['core', 'shareapi_allow_share_dialog_user_enumeration', 'yes', 'no'],
131+
['core', 'shareapi_restrict_user_enumeration_to_group', 'no', 'no']
132+
]);
133+
134+
$this->service = new StatusService($this->mapper,
135+
$this->timeFactory,
136+
$this->predefinedStatusService,
137+
$this->emojiService,
138+
$this->config);
139+
140+
$this->assertEquals([], $this->service->findAllRecentStatusChanges(20, 50));
141+
142+
// Rebuild $this->service with user enumeration limited to common groups
143+
$this->config = $this->createMock(IConfig::class);
144+
145+
$this->config->method('getAppValue')
146+
->willReturnMap([
147+
['core', 'shareapi_allow_share_dialog_user_enumeration', 'yes', 'yes'],
148+
['core', 'shareapi_restrict_user_enumeration_to_group', 'no', 'yes']
149+
]);
150+
151+
$this->service = new StatusService($this->mapper,
152+
$this->timeFactory,
153+
$this->predefinedStatusService,
154+
$this->emojiService,
155+
$this->config);
156+
157+
$this->assertEquals([], $this->service->findAllRecentStatusChanges(20, 50));
158+
}
159+
103160
public function testFindByUserId(): void {
104161
$status = $this->createMock(UserStatus::class);
105162
$this->mapper->expects($this->once())

‎build/integration/collaboration_features/user_status.feature‎

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,20 @@
1+
Feature: user_status
2+
Background:
3+
Given using api version "2"
4+
And user "user0" exists
5+
And user "user0" has status "dnd"
6+
7+
Scenario: listing recent user statuses with default settings
8+
Then user statuses for "admin" list "user0" with status "dnd"
9+
10+
Scenario: empty recent user statuses with disabled/limited user enumeration
11+
When parameter "shareapi_allow_share_dialog_user_enumeration" of app "core" is set to "no"
12+
Then user statuses for "admin" are empty
13+
When parameter "shareapi_allow_share_dialog_user_enumeration" of app "core" is set to "yes"
14+
When parameter "shareapi_restrict_user_enumeration_to_group" of app "core" is set to "yes"
15+
Then user statuses for "admin" are empty
16+
When parameter "shareapi_restrict_user_enumeration_to_group" of app "core" is set to "no"
17+
When parameter "shareapi_restrict_user_enumeration_to_phone" of app "core" is set to "yes"
18+
Then user statuses for "admin" are empty
19+
When parameter "shareapi_restrict_user_enumeration_to_phone" of app "core" is set to "no"
20+
Then user statuses for "admin" list "user0" with status "dnd"

‎build/integration/features/bootstrap/CollaborationContext.php‎

Lines changed: 91 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@
2424

2525
use Behat\Behat\Context\Context;
2626
use Behat\Gherkin\Node\TableNode;
27+
use GuzzleHttp\Client;
2728
use PHPUnit\Framework\Assert;
2829

2930
require __DIR__ . '/../../vendor/autoload.php';
@@ -69,4 +70,94 @@ protected function resetAppConfigs(): void {
6970
$this->deleteServerConfig('core', 'shareapi_restrict_user_enumeration_full_match');
7071
$this->deleteServerConfig('core', 'shareapi_only_share_with_group_members');
7172
}
73+
74+
/**
75+
* @Given /^user "([^"]*)" has status "([^"]*)"$/
76+
* @param string $user
77+
* @param string $status
78+
*/
79+
public function assureUserHasStatus($user, $status) {
80+
$fullUrl = $this->baseUrl . "v{$this->apiVersion}.php/apps/user_status/api/v1/user_status/status";
81+
$client = new Client();
82+
$options = [
83+
'headers' => [
84+
'OCS-APIREQUEST' => 'true',
85+
],
86+
];
87+
if ($user === 'admin') {
88+
$options['auth'] = $this->adminUser;
89+
} else {
90+
$options['auth'] = [$user, $this->regularUser];
91+
}
92+
93+
$options['form_params'] = [
94+
'statusType' => $status
95+
];
96+
97+
$this->response = $client->put($fullUrl, $options);
98+
$this->theHTTPStatusCodeShouldBe(200);
99+
100+
$fullUrl = $this->baseUrl . "v{$this->apiVersion}.php/apps/user_status/api/v1/user_status";
101+
unset($options['form_params']);
102+
$this->response = $client->get($fullUrl, $options);
103+
$this->theHTTPStatusCodeShouldBe(200);
104+
105+
$returnedStatus = json_decode(json_encode(simplexml_load_string($this->response->getBody()->getContents())->data), true)['status'];
106+
Assert::assertEquals($status, $returnedStatus);
107+
}
108+
109+
/**
110+
* @param string $user
111+
* @return null|array
112+
*/
113+
public function getStatusList(string $user): ?array {
114+
$fullUrl = $this->baseUrl . "v{$this->apiVersion}.php/apps/user_status/api/v1/statuses";
115+
$client = new Client();
116+
$options = [
117+
'headers' => [
118+
'OCS-APIREQUEST' => 'true',
119+
],
120+
];
121+
if ($user === 'admin') {
122+
$options['auth'] = $this->adminUser;
123+
} else {
124+
$options['auth'] = [$user, $this->regularUser];
125+
}
126+
127+
$this->response = $client->get($fullUrl, $options);
128+
$this->theHTTPStatusCodeShouldBe(200);
129+
130+
$contents = $this->response->getBody()->getContents();
131+
return json_decode(json_encode(simplexml_load_string($contents)->data), true);
132+
}
133+
134+
/**
135+
* @Given /^user statuses for "([^"]*)" list "([^"]*)" with status "([^"]*)"$/
136+
* @param string $user
137+
* @param string $statusUser
138+
* @param string $status
139+
*/
140+
public function assertStatusesList(string $user, string $statusUser, string $status): void {
141+
$statusList = $this->getStatusList($user);
142+
Assert::assertArrayHasKey('element', $statusList, 'Returned status list empty or broken');
143+
if (array_key_exists('userId', $statusList['element'])) {
144+
// If only one user has a status set, the API returns their status directly
145+
Assert::assertArrayHasKey('status', $statusList['element'], 'Returned status list empty or broken');
146+
$filteredStatusList = [ $statusList['element']['userId'] => $statusList['element']['status'] ];
147+
} else {
148+
// If more than one user have their status set, the API returns an array of their statuses
149+
$filteredStatusList = array_column($statusList['element'], 'status', 'userId');
150+
}
151+
Assert::assertArrayHasKey($statusUser, $filteredStatusList, 'User not listed in statuses: ' . $statusUser);
152+
Assert::assertEquals($status, $filteredStatusList[$statusUser]);
153+
}
154+
155+
/**
156+
* @Given /^user statuses for "([^"]*)" are empty$/
157+
* @param string $user
158+
*/
159+
public function assertStatusesEmpty(string $user): void {
160+
$statusList = $this->getStatusList($user);
161+
Assert::assertEmpty($statusList);
162+
}
72163
}

‎build/integration/features/bootstrap/Provisioning.php‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@
1313
* @author Sergio Bertolín <sbertolin@solidgear.es>
1414
* @author Thomas Müller <thomas.mueller@tmit.eu>
1515
* @author Vincent Petry <vincent@nextcloud.com>
16+
* @author Jonas Meurer <jonas@freesources.org>
1617
*
1718
* @license GNU AGPL version 3 or any later version
1819
*

0 commit comments

Comments
 (0)