diff --git a/lib/Controller/PageController.php b/lib/Controller/PageController.php index 6680d0000..efef1d7c1 100644 --- a/lib/Controller/PageController.php +++ b/lib/Controller/PageController.php @@ -25,6 +25,7 @@ namespace OCA\Contacts\Controller; use OC\App\CompareVersion; +use OCA\Contacts\Service\GroupSharingService; use OCP\App\IAppManager; use OCP\AppFramework\Controller; use OCP\AppFramework\Http\TemplateResponse; @@ -60,6 +61,8 @@ class PageController extends Controller { /** @var CompareVersion */ private $compareVersion; + private GroupSharingService $groupSharingService; + public function __construct(IRequest $request, IConfig $config, IInitialStateService $initialStateService, @@ -67,7 +70,8 @@ public function __construct(IRequest $request, IUserSession $userSession, SocialApiService $socialApiService, IAppManager $appManager, - CompareVersion $compareVersion) { + CompareVersion $compareVersion, + GroupSharingService $groupSharingService) { parent::__construct(Application::APP_ID, $request); $this->config = $config; @@ -77,6 +81,7 @@ public function __construct(IRequest $request, $this->socialApiService = $socialApiService; $this->appManager = $appManager; $this->compareVersion = $compareVersion; + $this->groupSharingService = $groupSharingService; } /** @@ -87,10 +92,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'); @@ -107,7 +109,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; diff --git a/lib/Service/GroupSharingService.php b/lib/Service/GroupSharingService.php new file mode 100644 index 000000000..9322f43a4 --- /dev/null +++ b/lib/Service/GroupSharingService.php @@ -0,0 +1,46 @@ +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, + }; + } +} diff --git a/src/components/AppNavigation/Settings/SettingsAddressbookShare.vue b/src/components/AppNavigation/Settings/SettingsAddressbookShare.vue index 05b248936..e3e9bd1ae 100644 --- a/src/components/AppNavigation/Settings/SettingsAddressbookShare.vue +++ b/src/components/AppNavigation/Settings/SettingsAddressbookShare.vue @@ -118,20 +118,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 { diff --git a/tests/unit/Controller/PageControllerTest.php b/tests/unit/Controller/PageControllerTest.php index a60bfcc08..7cc75280b 100644 --- a/tests/unit/Controller/PageControllerTest.php +++ b/tests/unit/Controller/PageControllerTest.php @@ -26,6 +26,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; @@ -64,6 +65,8 @@ class PageControllerTest extends TestCase { /** @var CompareVersion|MockObject*/ private $compareVersion; + private GroupSharingService|MockObject $groupSharingService; + protected function setUp(): void { parent::setUp(); @@ -75,6 +78,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, @@ -84,7 +88,8 @@ protected function setUp(): void { $this->userSession, $this->socialApi, $this->appManager, - $this->compareVersion + $this->compareVersion, + $this->groupSharingService, ); } diff --git a/tests/unit/Service/GroupSharingServiceTest.php b/tests/unit/Service/GroupSharingServiceTest.php new file mode 100644 index 000000000..42b89fda7 --- /dev/null +++ b/tests/unit/Service/GroupSharingServiceTest.php @@ -0,0 +1,112 @@ +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); + } +}