Skip to content

Commit 3b03177

Browse files
redblomredblom
committed
Move check for group block list to Group method hideFromCollaboration()
Some refactoring and cs fixing Signed-off-by: Antoon Prins <antoon.prins@surf.nl>
1 parent 274a6ce commit 3b03177

File tree

7 files changed

+58
-26
lines changed

7 files changed

+58
-26
lines changed

‎apps/settings/src/components/AdminSettingsSharingForm.vue‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@
1818
{{ t('settings', 'Allow sharing with groups') }}
1919
</NcCheckboxRadioSwitch>
2020
<div v-show="settings.allowGroupSharing" id="settings-sharing-api-groups-block-list" class="sharing__labeled-entry sharing__input">
21-
<label for="sharing-groups-block-list">{{ t('settings', 'Groups that should be excluded from sharing') }}</label>
21+
<label for="sharing-groups-block-list">{{ t('settings', 'Groups that are excluded from sharing') }}</label>
2222
<NcSettingsSelectGroup id="sharing-groups-block-list"
2323
v-model="settings.groupsBlockList"
2424
:label="t('settings', 'Select groups')"

‎lib/private/Collaboration/Collaborators/GroupPlugin.php‎

Lines changed: 2 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,6 @@
88
use OCP\Collaboration\Collaborators\ISearchPlugin;
99
use OCP\Collaboration\Collaborators\ISearchResult;
1010
use OCP\Collaboration\Collaborators\SearchResultType;
11-
use OCP\IAppConfig;
1211
use OCP\IConfig;
1312
use OCP\IGroup;
1413
use OCP\IGroupManager;
@@ -26,7 +25,6 @@ class GroupPlugin implements ISearchPlugin {
2625

2726
public function __construct(
2827
private IConfig $config,
29-
private IAppConfig $appConfig,
3028
private IGroupManager $groupManager,
3129
private IUserSession $userSession,
3230
private mixed $shareWithGroupOnlyExcludeGroupsList = [],
@@ -71,12 +69,6 @@ public function search($search, $limit, $offset, ISearchResult $searchResult): b
7169
$groupIds = array_diff($groupIds, $this->shareWithGroupOnlyExcludeGroupsList);
7270
}
7371

74-
// Check for blocked groups
75-
$groupsBlockList = $this->appConfig->getValueArray('core', 'shareapi_groups_block_list', []);
76-
if (!empty($groupsBlockList)) {
77-
$groupIds = array_diff($groupIds, $groupsBlockList);
78-
}
79-
8072
$lowerSearch = strtolower($search);
8173
foreach ($groups as $group) {
8274
if ($group->hideFromCollaboration()) {
@@ -114,8 +106,8 @@ public function search($search, $limit, $offset, ISearchResult $searchResult): b
114106
// On page one we try if the search result has a direct hit on the
115107
// user id and if so, we add that to the exact match list
116108
$group = $this->groupManager->get($search);
117-
if ($group instanceof IGroup && !$group->hideFromCollaboration() && array_search($group->getGID(), $groupsBlockList) === false && (!$this->shareWithGroupOnly || in_array($group->getGID(), $userGroups))) {
118-
$result['exact'][] = [
109+
if ($group instanceof IGroup && !$group->hideFromCollaboration() && (!$this->shareWithGroupOnly || in_array($group->getGID(), $userGroups))) {
110+
$result['exact'][] = [
119111
'label' => $group->getDisplayName(),
120112
'value' => [
121113
'shareType' => IShare::TYPE_GROUP,

‎lib/private/Group/Group.php‎

Lines changed: 14 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@
2525
use OCP\Group\Events\UserAddedEvent;
2626
use OCP\Group\Events\UserRemovedEvent;
2727
use OCP\GroupInterface;
28+
use OCP\IAppConfig;
2829
use OCP\IGroup;
2930
use OCP\IUser;
3031
use OCP\IUserManager;
@@ -51,7 +52,15 @@ class Group implements IGroup {
5152
/** @var PublicEmitter */
5253
private $emitter;
5354

54-
public function __construct(string $gid, array $backends, IEventDispatcher $dispatcher, IUserManager $userManager, ?PublicEmitter $emitter = null, ?string $displayName = null) {
55+
public function __construct(
56+
string $gid,
57+
array $backends,
58+
IEventDispatcher $dispatcher,
59+
IUserManager $userManager,
60+
private IAppConfig $appConfig,
61+
?PublicEmitter $emitter = null,
62+
?string $displayName = null,
63+
) {
5564
$this->gid = $gid;
5665
$this->backends = $backends;
5766
$this->dispatcher = $dispatcher;
@@ -376,6 +385,10 @@ public function canAddUser(): bool {
376385
* @since 16.0.0
377386
*/
378387
public function hideFromCollaboration(): bool {
388+
if (in_array($this->gid, $this->appConfig->getValueArray('core', 'shareapi_groups_block_list', []))) {
389+
return true;
390+
}
391+
379392
return array_reduce($this->backends, function (bool $hide, GroupInterface $backend) {
380393
return $hide | ($backend instanceof IHideFromCollaborationBackend && $backend->hideGroup($this->gid));
381394
}, false);

‎lib/private/Group/Manager.php‎

Lines changed: 19 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@
1616
use OCP\Group\Events\BeforeGroupCreatedEvent;
1717
use OCP\Group\Events\GroupCreatedEvent;
1818
use OCP\GroupInterface;
19+
use OCP\IAppConfig;
1920
use OCP\ICacheFactory;
2021
use OCP\IGroup;
2122
use OCP\IGroupManager;
@@ -52,6 +53,9 @@ class Manager extends PublicEmitter implements IGroupManager {
5253
/** @var \OC\SubAdmin */
5354
private $subAdmin = null;
5455

56+
/** @var IAppConfig $appConfig */
57+
private $appConfig;
58+
5559
private DisplayNameCache $displayNameCache;
5660

5761
private const MAX_GROUP_LENGTH = 255;
@@ -159,7 +163,7 @@ protected function getGroupObject($gid, $displayName = null) {
159163
return null;
160164
}
161165
/** @var GroupInterface[] $backends */
162-
$this->cachedGroups[$gid] = new Group($gid, $backends, $this->dispatcher, $this->userManager, $this, $displayName);
166+
$this->cachedGroups[$gid] = new Group($gid, $backends, $this->dispatcher, $this->userManager, $this->getAppConfig(), $this, $displayName);
163167
return $this->cachedGroups[$gid];
164168
}
165169

@@ -214,7 +218,7 @@ protected function getGroupsObjects(array $gids, array $displayNames = []): arra
214218
if (count($backends[$gid]) === 0) {
215219
continue;
216220
}
217-
$this->cachedGroups[$gid] = new Group($gid, $backends[$gid], $this->dispatcher, $this->userManager, $this, $displayNames[$gid]);
221+
$this->cachedGroups[$gid] = new Group($gid, $backends[$gid], $this->dispatcher, $this->userManager, $this->getAppConfig(), $this, $displayNames[$gid]);
218222
$groups[$gid] = $this->cachedGroups[$gid];
219223
}
220224
return $groups;
@@ -472,4 +476,17 @@ public function getSubAdmin() {
472476

473477
return $this->subAdmin;
474478
}
479+
480+
/**
481+
* Returns the appConfig object
482+
*
483+
* @return IAppConfig
484+
*/
485+
private function getAppConfig():IAppConfig {
486+
if (isset($this->appConfig)) {
487+
return $this->appConfig;
488+
} else {
489+
return \OC::$server->get(IAppConfig::class);
490+
}
491+
}
475492
}

‎lib/private/Share20/Manager.php‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -515,7 +515,7 @@ protected function groupCreateChecks(IShare $share) {
515515

516516
// Check if sharing with this group is blocked
517517
$groupsBlockList = $this->appConfig->getValueArray('core', 'shareapi_groups_block_list', []);
518-
if(array_search($share->getSharedWith(), $groupsBlockList) !== false) {
518+
if(in_array($share->getSharedWith(), $groupsBlockList)) {
519519
throw new \InvalidArgumentException('Sharing with group ' . $share->getSharedWith() . ' is not allowed.');
520520
}
521521

‎tests/lib/Collaboration/Collaborators/GroupPluginTest.php‎

Lines changed: 17 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
<?php
2+
23
/**
34
* SPDX-FileCopyrightText: 2017 Nextcloud GmbH and Nextcloud contributors
45
* SPDX-License-Identifier: AGPL-3.0-or-later
@@ -18,8 +19,6 @@
1819
use OCP\Share\IShare;
1920
use Test\TestCase;
2021

21-
use function PHPUnit\Framework\isEmpty;
22-
2322
class GroupPluginTest extends TestCase {
2423
/** @var IConfig|\PHPUnit\Framework\MockObject\MockObject */
2524
protected $config;
@@ -69,7 +68,6 @@ public function instantiatePlugin() {
6968
// up with configuration etc. first
7069
$this->plugin = new GroupPlugin(
7170
$this->config,
72-
$this->appConfig,
7371
$this->groupManager,
7472
$this->session
7573
);
@@ -410,15 +408,26 @@ public function dataGetGroups(): array {
410408
true,
411409
$this->getGroupMock('test'),
412410
],
411+
[
412+
'test', false, false, false,
413+
[
414+
$this->getGroupMock('test', null, true),
415+
$this->getGroupMock('test1'),
416+
],
417+
[],
418+
[],
419+
[],
420+
true,
421+
false,
422+
],
413423
[
414424
'test', true, true, false,
415425
[
416-
$this->getGroupMock('test0'),
426+
$this->getGroupMock('test0', 'test0', true),
417427
$this->getGroupMock('test1'),
418428
],
419429
[
420-
$this->getGroupMock('test'),
421-
$this->getGroupMock('test0'),
430+
$this->getGroupMock('test0'),
422431
$this->getGroupMock('test1')
423432
],
424433
[],
@@ -480,7 +489,7 @@ function ($appName, $key, $default) use ($shareWithGroupOnly, $shareeEnumeration
480489
}
481490
);
482491

483-
if(count($groupsBlockList) > 0) {
492+
if ($groupsBlockList != []) {
484493
/** setup blocked groups list */
485494
$appConfig = $this->createMock(IAppConfig::class);
486495
$appConfig->method('getValueArray')
@@ -526,4 +535,4 @@ function ($appName, $key, $default) use ($shareWithGroupOnly, $shareeEnumeration
526535
}
527536
$this->assertSame($reachedEnd, $moreResults);
528537
}
529-
}
538+
}

‎tests/lib/Share20/ManagerTest.php‎

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
<?php
2+
23
/**
34
* SPDX-FileCopyrightText: 2016-2024 Nextcloud GmbH and Nextcloud contributors
45
* SPDX-FileCopyrightText: 2016 ownCloud, Inc.
@@ -2514,9 +2515,10 @@ public function testCreateShareBlockedGroups() {
25142515
'blocked-group-1',
25152516
'sharedBy',
25162517
null,
2517-
\OCP\Constants::PERMISSION_ALL);
2518+
\OCP\Constants::PERMISSION_ALL,
2519+
);
25182520

2519-
$manager->expects($this->any())
2521+
$manager->expects($this->any())
25202522
->method('allowGroupSharing')
25212523
->willReturn(true);
25222524
$manager->expects($this->once())
@@ -2526,7 +2528,6 @@ public function testCreateShareBlockedGroups() {
25262528
$manager->expects($this->once())
25272529
->method('generalCreateChecks')
25282530
->with($share);
2529-
;
25302531
$manager->expects($this->once())
25312532
->method('pathCreateChecks')
25322533
->with($path);

0 commit comments

Comments
 (0)