Skip to content

Commit

Permalink
Merge pull request #4108 from nextcloud/fix/respect-group-sharing-in-…
Browse files Browse the repository at this point in the history
…frontend

fix: respect advanced group sharing settings in frontend
  • Loading branch information
st3iny authored Sep 23, 2024
2 parents 87d360c + 5450606 commit a900c69
Show file tree
Hide file tree
Showing 5 changed files with 186 additions and 15 deletions.
14 changes: 8 additions & 6 deletions lib/Controller/PageController.php
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@

use OC\App\CompareVersion;
use OCA\Contacts\AppInfo\Application;
use OCA\Contacts\Service\GroupSharingService;
use OCA\Contacts\Service\SocialApiService;
use OCP\App\IAppManager;

Expand Down Expand Up @@ -42,14 +43,17 @@ class PageController extends Controller {
/** @var CompareVersion */
private $compareVersion;

private GroupSharingService $groupSharingService;

public function __construct(IRequest $request,
IConfig $config,
IInitialStateService $initialStateService,
IFactory $languageFactory,
IUserSession $userSession,
SocialApiService $socialApiService,
IAppManager $appManager,
CompareVersion $compareVersion) {
CompareVersion $compareVersion,
GroupSharingService $groupSharingService) {
parent::__construct(Application::APP_ID, $request);

$this->config = $config;
Expand All @@ -59,6 +63,7 @@ public function __construct(IRequest $request,
$this->socialApiService = $socialApiService;
$this->appManager = $appManager;
$this->compareVersion = $compareVersion;
$this->groupSharingService = $groupSharingService;
}

/**
Expand All @@ -69,10 +74,7 @@ public function __construct(IRequest $request,
*/
public function index(): TemplateResponse {
$user = $this->userSession->getUser();
$userId = '';
if (!is_null($user)) {
$userId = $user->getUid();
}
$userId = $user->getUid();

$locales = $this->languageFactory->findAvailableLocales();
$defaultProfile = $this->config->getAppValue(Application::APP_ID, 'defaultProfile', 'HOME');
Expand All @@ -89,7 +91,7 @@ public function index(): TemplateResponse {
// if circles is not installed, we use 0.0.0
$isCircleVersionCompatible = $this->compareVersion->isCompatible($circleVersion ? $circleVersion : '0.0.0', 22);
// Check whether group sharing is enabled or not
$isGroupSharingEnabled = $this->config->getAppValue('core', 'shareapi_allow_group_sharing', 'yes') === 'yes';
$isGroupSharingEnabled = $this->groupSharingService->isGroupSharingAllowed($user);
$talkVersion = $this->appManager->getAppVersion('spreed');
$isTalkEnabled = $this->appManager->isEnabledForUser('spreed') === true;

Expand Down
47 changes: 47 additions & 0 deletions lib/Service/GroupSharingService.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
<?php

declare(strict_types=1);

/**
* SPDX-FileCopyrightText: 2024 Nextcloud GmbH and Nextcloud contributors
* SPDX-License-Identifier: AGPL-3.0-or-later
*/

namespace OCA\Contacts\Service;

use OCP\IConfig;
use OCP\IGroup;
use OCP\IGroupManager;
use OCP\IUser;
use OCP\Share\IManager as IShareManager;

class GroupSharingService {

public function __construct(
private IConfig $config,
private IGroupManager $groupManager,
private IShareManager $shareManager,
) {
}

public function isGroupSharingAllowed(IUser $user): bool {
if (!$this->shareManager->allowGroupSharing()) {
return false;
}

$userGroups = $this->groupManager->getUserGroups($user);
$userGroupNames = array_map(static fn (IGroup $group) => $group->getGID(), $userGroups);

$excludeGroupList = json_decode($this->config->getAppValue('core', 'shareapi_exclude_groups_list', '[]'));
$excludeGroups = $this->config->getAppValue('core', 'shareapi_exclude_groups');

// "no" => Allow sharing for everyone
// "yes" => Exclude listed groups from sharing
// "allow" => Limit sharing to listed groups
return match ($excludeGroups) {
'yes' => count(array_intersect($userGroupNames, $excludeGroupList)) === 0,
'allow' => count(array_intersect($userGroupNames, $excludeGroupList)) > 0,
default => true,
};
}
}
21 changes: 13 additions & 8 deletions src/components/AppNavigation/Settings/SettingsAddressbookShare.vue
Original file line number Diff line number Diff line change
Expand Up @@ -101,20 +101,25 @@ export default {
this.usersOrGroups = []
if (query.length > 0) {
const results = await client.principalPropertySearchByDisplayname(query)
this.usersOrGroups = results.reduce((list, result) => {
if (['GROUP', 'INDIVIDUAL'].indexOf(result.calendarUserType) > -1
&& !this.addressbook.shares.some((share) => share.uri === result.principalScheme)) {
this.usersOrGroups = results
.filter((result) => {
const allowedCalendarUserTypes = ['INDIVIDUAL']
if (this.isGroupSharingEnabled) {
allowedCalendarUserTypes.push('GROUP')
}
return allowedCalendarUserTypes.includes(result.calendarUserType)
&& !this.addressbook.shares.some((share) => share.uri === result.principalScheme)
})
.map((result) => {
const isGroup = result.calendarUserType === 'GROUP'
list.push({
return {
user: urldecode(result[isGroup ? 'groupId' : 'userId']),
displayName: result.displayname,
icon: isGroup ? 'icon-group' : 'icon-user',
uri: urldecode(result.principalScheme),
isGroup,
})
}
return list
}, [])
}
})
this.isLoading = false
this.inputGiven = true
} else {
Expand Down
7 changes: 6 additions & 1 deletion tests/unit/Controller/PageControllerTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@

use ChristophWurst\Nextcloud\Testing\TestCase;
use OC\App\CompareVersion;
use OCA\Contacts\Service\GroupSharingService;
use OCA\Contacts\Service\SocialApiService;
use OCP\App\IAppManager;
use OCP\AppFramework\Http\TemplateResponse;
Expand Down Expand Up @@ -46,6 +47,8 @@ class PageControllerTest extends TestCase {
/** @var CompareVersion|MockObject */
private $compareVersion;

private GroupSharingService|MockObject $groupSharingService;

protected function setUp(): void {
parent::setUp();

Expand All @@ -57,6 +60,7 @@ protected function setUp(): void {
$this->socialApi = $this->createMock(SocialApiService::class);
$this->appManager = $this->createMock(IAppManager::class);
$this->compareVersion = $this->createMock(CompareVersion::class);
$this->groupSharingService = $this->createMock(GroupSharingService::class);

$this->controller = new PageController(
$this->request,
Expand All @@ -66,7 +70,8 @@ protected function setUp(): void {
$this->userSession,
$this->socialApi,
$this->appManager,
$this->compareVersion
$this->compareVersion,
$this->groupSharingService,
);
}

Expand Down
112 changes: 112 additions & 0 deletions tests/unit/Service/GroupSharingServiceTest.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,112 @@
<?php

declare(strict_types=1);

/**
* SPDX-FileCopyrightText: 2024 Nextcloud GmbH and Nextcloud contributors
* SPDX-License-Identifier: AGPL-3.0-or-later
*/

namespace unit\Service;

use ChristophWurst\Nextcloud\Testing\TestCase;
use OCA\Contacts\Service\GroupSharingService;
use OCP\IConfig;
use OCP\IGroup;
use OCP\IGroupManager;
use OCP\IUser;
use OCP\Share\IManager as IShareManager;
use PHPUnit\Framework\MockObject\MockObject;

class GroupSharingServiceTest extends TestCase {
private GroupSharingService $service;
private IConfig|MockObject $config;
private IGroupManager|MockObject $groupManager;
private IShareManager|MockObject $shareManager;

protected function setUp(): void {
parent::setUp();

$this->config = $this->createMock(IConfig::class);
$this->groupManager = $this->createMock(IGroupManager::class);
$this->shareManager = $this->createMock(IShareManager::class);

$this->service = new GroupSharingService(
$this->config,
$this->groupManager,
$this->shareManager,
);
}

public function provideTestIsGroupSharingAllowed(): array {
return [
// Basic group sharing is forbidden (-> short circuit)
[false, false, '["group1"]', 'no'],
[false, false, '["group1"]', 'yes'],
[false, false, '["group1"]', 'allow'],

// Basic sharing is allowed and allow sharing with every group
[true, true, '["group1"]', 'no'],
[true, true, '[]', 'no'],
[true, true, '["group99"]', 'no'],

// Basic sharing is allowed and user's group is excluded
[false, true, '["group1"]', 'yes'],
[false, true, '["group2"]', 'yes'],
[false, true, '["group2", "group3"]', 'yes'],

// Basic sharing is allowed and user's group is not excluded
[true, true, '["group3"]', 'yes'],
[true, true, '[]', 'yes'],

// Basic sharing is allowed and user's group is included
[true, true, '["group1"]', 'allow'],
[true, true, '["group2"]', 'allow'],
[true, true, '["group2", "group3"]', 'allow'],

// Basic sharing is allowed and user's group is not included
[false, true, '["group3"]', 'allow'],
[false, true, '[]', 'allow'],
];
}

/** @dataProvider provideTestIsGroupSharingAllowed */
public function testIsGroupSharingAllowed(
bool $expected,
bool $allowGroupSharing,
string $excludeGroupList,
string $excludeGroups,
): void {
$user = $this->createMock(IUser::class);
$group1 = $this->createMock(IGroup::class);
$group1->method('getGID')
->willReturn('group1');
$group2 = $this->createMock(IGroup::class);
$group2->method('getGID')
->willReturn('group2');

$this->shareManager->expects(self::once())
->method('allowGroupSharing')
->willReturn($allowGroupSharing);
if ($allowGroupSharing) {
$this->groupManager->expects(self::once())
->method('getUserGroups')
->with($user)
->willReturn([$group1, $group2]);
$this->config->expects(self::exactly(2))
->method('getAppValue')
->willReturnMap([
['core', 'shareapi_exclude_groups_list', '[]', $excludeGroupList],
['core', 'shareapi_exclude_groups', '', $excludeGroups],
]);
} else {
$this->groupManager->expects(self::never())
->method('getUserGroups');
$this->config->expects(self::never())
->method('getAppValue');
}

$actual = $this->service->isGroupSharingAllowed($user);
$this->assertEquals($expected, $actual);
}
}

0 comments on commit a900c69

Please sign in to comment.