Skip to content

Commit

Permalink
Merge pull request #29429 from nextcloud/fix/user_status_enumeration_20
Browse files Browse the repository at this point in the history
Backport #29260: Respect user enumeration settings in user status lists
  • Loading branch information
LukasReschke authored Oct 25, 2021
2 parents 2a4e92d + f8719bc commit 0f9e090
Show file tree
Hide file tree
Showing 6 changed files with 286 additions and 7 deletions.
12 changes: 10 additions & 2 deletions apps/user_status/lib/AppInfo/Application.php
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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 {
Expand Down
37 changes: 33 additions & 4 deletions apps/user_status/lib/Service/StatusService.php
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@
use OCA\UserStatus\Exception\StatusMessageTooLongException;
use OCP\AppFramework\Db\DoesNotExistException;
use OCP\AppFramework\Utility\ITimeFactory;
use OCP\IConfig;
use OCP\UserStatus\IUserStatus;

/**
Expand All @@ -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
*/
Expand Down Expand Up @@ -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';
}

/**
Expand All @@ -106,6 +121,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));
Expand All @@ -117,6 +139,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));
Expand Down Expand Up @@ -224,7 +253,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
Expand Down Expand Up @@ -332,7 +361,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();
Expand Down
59 changes: 58 additions & 1 deletion apps/user_status/tests/Unit/Service/StatusServiceTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,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;

Expand All @@ -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;

Expand All @@ -64,10 +68,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 {
Expand Down Expand Up @@ -100,6 +114,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())
Expand Down
20 changes: 20 additions & 0 deletions build/integration/collaboration_features/user_status.feature
Original file line number Diff line number Diff line change
@@ -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"
164 changes: 164 additions & 0 deletions build/integration/features/bootstrap/CollaborationContext.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,164 @@
<?php

declare(strict_types=1);

/**
* @copyright Copyright (c) 2021, Joas Schilling <[email protected]>
*
* @author Joas Schilling <[email protected]>
*
* @license GNU AGPL version 3 or any later version
*
* This program is free software: you can redistribute it and/or modify
* it under the terms of the GNU Affero General Public License as
* published by the Free Software Foundation, either version 3 of the
* License, or (at your option) any later version.
*
* This program is distributed in the hope that it will be useful,
* but WITHOUT ANY WARRANTY; without even the implied warranty of
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
* GNU Affero General Public License for more details.
*
* You should have received a copy of the GNU Affero General Public License
* along with this program. If not, see <http://www.gnu.org/licenses/>.
*
*/
use Behat\Behat\Context\Context;
use Behat\Gherkin\Node\TableNode;
use GuzzleHttp\Client;
use PHPUnit\Framework\Assert;

require __DIR__ . '/../../vendor/autoload.php';

class CollaborationContext implements Context {
use Provisioning;
use AppConfiguration;

/**
* @Then /^get autocomplete for "([^"]*)"$/
* @param TableNode|null $formData
*/
public function getAutocomplete(string $search, TableNode $formData): void {
$query = $search === 'null' ? null : $search;

$this->sendRequestForJSON('GET', '/core/autocomplete/get?itemType=files&itemId=123&search=' . $query, [
'itemType' => 'files',
'itemId' => '123',
'search' => $query,
]);
$this->theHTTPStatusCodeShouldBe(200);

$data = json_decode($this->response->getBody()->getContents(), true);
$suggestions = $data['ocs']['data'];

Assert::assertCount(count($formData->getHash()), $suggestions, 'Suggestion count does not match');
Assert::assertEquals($formData->getHash(), array_map(static function ($suggestion, $expected) {
$data = [];
if (isset($expected['id'])) {
$data['id'] = $suggestion['id'];
}
if (isset($expected['source'])) {
$data['source'] = $suggestion['source'];
}
return $data;
}, $suggestions, $formData->getHash()));
}

protected function resetAppConfigs(): void {
$this->deleteServerConfig('core', 'shareapi_allow_share_dialog_user_enumeration');
$this->deleteServerConfig('core', 'shareapi_restrict_user_enumeration_to_group');
$this->deleteServerConfig('core', 'shareapi_restrict_user_enumeration_to_phone');
$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);
}
}
Loading

0 comments on commit 0f9e090

Please sign in to comment.