Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Respect user enumeration settings in user status lists #29260

Merged
merged 2 commits into from
Oct 20, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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) {
nickvergessen marked this conversation as resolved.
Show resolved Hide resolved
$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 @@ -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;

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

/**
Expand All @@ -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));
Expand All @@ -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));
Expand Down Expand Up @@ -226,7 +255,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 @@ -334,7 +363,7 @@ public function removeUserStatus(string $userId, bool $isBackup = false): 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 @@ -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;

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

Expand All @@ -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 {
Expand Down Expand Up @@ -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())
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"
mejo- marked this conversation as resolved.
Show resolved Hide resolved

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"
91 changes: 91 additions & 0 deletions build/integration/features/bootstrap/CollaborationContext.php
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -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);
}
}
1 change: 1 addition & 0 deletions build/integration/features/bootstrap/Provisioning.php
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
* @author Sergio Bertolín <sbertolin@solidgear.es>
* @author Thomas Müller <thomas.mueller@tmit.eu>
* @author Vincent Petry <vincent@nextcloud.com>
* @author Jonas Meurer <jonas@freesources.org>
*
* @license GNU AGPL version 3 or any later version
*
Expand Down