Skip to content

Commit d665dfd

Browse files
redblomredblom
committed
Block sharing with selected groups.
Signed-off-by: Antoon Prins <antoon.prins@surf.nl>
1 parent 7876be3 commit d665dfd

4 files changed

Lines changed: 125 additions & 8 deletions

File tree

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

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

2627
public function __construct(
2728
private IConfig $config,
29+
private IAppConfig $appConfig,
2830
private IGroupManager $groupManager,
2931
private IUserSession $userSession,
3032
private mixed $shareWithGroupOnlyExcludeGroupsList = [],
@@ -69,6 +71,12 @@ public function search($search, $limit, $offset, ISearchResult $searchResult): b
6971
$groupIds = array_diff($groupIds, $this->shareWithGroupOnlyExcludeGroupsList);
7072
}
7173

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

‎lib/private/Share20/Manager.php‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@
2121
use OCP\Files\Node;
2222
use OCP\Files\NotFoundException;
2323
use OCP\HintException;
24+
use OCP\IAppConfig;
2425
use OCP\IConfig;
2526
use OCP\IDateTimeZone;
2627
use OCP\IGroupManager;
@@ -62,6 +63,7 @@ class Manager implements IManager {
6263
public function __construct(
6364
private LoggerInterface $logger,
6465
private IConfig $config,
66+
private IAppConfig $appConfig,
6567
private ISecureRandom $secureRandom,
6668
private IHasher $hasher,
6769
private IMountManager $mountManager,
@@ -497,6 +499,12 @@ protected function groupCreateChecks(IShare $share) {
497499
throw new \Exception($this->l->t('Group sharing is now allowed'));
498500
}
499501

502+
// Check if sharing with this group is blocked
503+
$groupsBlockList = $this->appConfig->getValueArray('files_sharing', 'groups_block_list', [], true);
504+
if(array_search($share->getSharedWith(), $groupsBlockList) !== false) {
505+
throw new \InvalidArgumentException('Sharing with group ' . $share->getSharedWith() . ' is not allowed.');
506+
}
507+
500508
// Verify if the user can share with this group
501509
if ($this->shareWithGroupMembersOnly()) {
502510
$sharedBy = $this->userManager->get($share->getSharedBy());

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

Lines changed: 33 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@
99
use OC\Collaboration\Collaborators\GroupPlugin;
1010
use OC\Collaboration\Collaborators\SearchResult;
1111
use OCP\Collaboration\Collaborators\ISearchResult;
12+
use OCP\IAppConfig;
1213
use OCP\IConfig;
1314
use OCP\IGroup;
1415
use OCP\IGroupManager;
@@ -17,10 +18,15 @@
1718
use OCP\Share\IShare;
1819
use Test\TestCase;
1920

21+
use function PHPUnit\Framework\isEmpty;
22+
2023
class GroupPluginTest extends TestCase {
2124
/** @var IConfig|\PHPUnit\Framework\MockObject\MockObject */
2225
protected $config;
2326

27+
/** @var IAppConfig|\PHPUnit\Framework\MockObject\MockObject */
28+
protected $appConfig;
29+
2430
/** @var IGroupManager|\PHPUnit\Framework\MockObject\MockObject */
2531
protected $groupManager;
2632

@@ -47,6 +53,8 @@ protected function setUp(): void {
4753

4854
$this->config = $this->createMock(IConfig::class);
4955

56+
$this->appConfig = $this->createMock(IAppConfig::class);
57+
5058
$this->groupManager = $this->createMock(IGroupManager::class);
5159

5260
$this->session = $this->createMock(IUserSession::class);
@@ -61,6 +69,7 @@ public function instantiatePlugin() {
6169
// up with configuration etc. first
6270
$this->plugin = new GroupPlugin(
6371
$this->config,
72+
$this->appConfig,
6473
$this->groupManager,
6574
$this->session
6675
);
@@ -402,16 +411,23 @@ public function dataGetGroups(): array {
402411
$this->getGroupMock('test'),
403412
],
404413
[
405-
'test', false, false, false,
414+
'test', true, true, false,
406415
[
407-
$this->getGroupMock('test', null, true),
416+
$this->getGroupMock('test0'),
408417
$this->getGroupMock('test1'),
409418
],
419+
[
420+
$this->getGroupMock('test'),
421+
$this->getGroupMock('test0'),
422+
$this->getGroupMock('test1')
423+
],
410424
[],
411-
[],
412-
[],
413-
true,
425+
[
426+
['label' => 'test1', 'value' => ['shareType' => IShare::TYPE_GROUP, 'shareWith' => 'test1']],
427+
],
428+
false,
414429
false,
430+
['test0'],
415431
],
416432
];
417433
}
@@ -429,6 +445,7 @@ public function dataGetGroups(): array {
429445
* @param array $expected
430446
* @param bool $reachedEnd
431447
* @param bool|IGroup $singleGroup
448+
* @param array $groupsBlockList
432449
*/
433450
public function testSearch(
434451
string $searchTerm,
@@ -441,6 +458,7 @@ public function testSearch(
441458
array $expected,
442459
bool $reachedEnd,
443460
$singleGroup,
461+
array $groupsBlockList = [],
444462
): void {
445463
$this->config->expects($this->any())
446464
->method('getAppValue')
@@ -462,6 +480,15 @@ function ($appName, $key, $default) use ($shareWithGroupOnly, $shareeEnumeration
462480
}
463481
);
464482

483+
if(count($groupsBlockList) > 0) {
484+
/** setup blocked groups list */
485+
$appConfig = $this->createMock(IAppConfig::class);
486+
$appConfig->method('getValueArray')
487+
->with('files_sharing', 'groups_block_list')
488+
->willReturn($groupsBlockList);
489+
$this->appConfig = $appConfig;
490+
}
491+
465492
$this->instantiatePlugin();
466493

467494
if (!$groupSharingDisabled) {
@@ -499,4 +526,4 @@ function ($appName, $key, $default) use ($shareWithGroupOnly, $shareeEnumeration
499526
}
500527
$this->assertSame($reachedEnd, $moreResults);
501528
}
502-
}
529+
}

‎tests/lib/Share20/ManagerTest.php‎

Lines changed: 74 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,7 @@
2828
use OCP\Files\NotFoundException;
2929
use OCP\Files\Storage\IStorage;
3030
use OCP\HintException;
31+
use OCP\IAppConfig;
3132
use OCP\IConfig;
3233
use OCP\IDateTimeZone;
3334
use OCP\IGroup;
@@ -72,6 +73,8 @@ class ManagerTest extends \Test\TestCase {
7273
protected $logger;
7374
/** @var IConfig|MockObject */
7475
protected $config;
76+
/** @var IAppConfig|MockObject */
77+
protected $appConfig;
7578
/** @var ISecureRandom|MockObject */
7679
protected $secureRandom;
7780
/** @var IHasher|MockObject */
@@ -113,6 +116,7 @@ class ManagerTest extends \Test\TestCase {
113116
protected function setUp(): void {
114117
$this->logger = $this->createMock(LoggerInterface::class);
115118
$this->config = $this->createMock(IConfig::class);
119+
$this->appConfig = $this->createMock(IAppConfig::class);
116120
$this->secureRandom = $this->createMock(ISecureRandom::class);
117121
$this->hasher = $this->createMock(IHasher::class);
118122
$this->mountManager = $this->createMock(IMountManager::class);
@@ -156,6 +160,7 @@ private function createManager(IProviderFactory $factory): Manager {
156160
return new Manager(
157161
$this->logger,
158162
$this->config,
163+
$this->appConfig,
159164
$this->secureRandom,
160165
$this->hasher,
161166
$this->mountManager,
@@ -183,6 +188,7 @@ private function createManagerMock() {
183188
->setConstructorArgs([
184189
$this->logger,
185190
$this->config,
191+
$this->appConfig,
186192
$this->secureRandom,
187193
$this->hasher,
188194
$this->mountManager,
@@ -2472,6 +2478,74 @@ public function testCreateShareUser(): void {
24722478
$manager->createShare($share);
24732479
}
24742480

2481+
public function testCreateShareBlockedGroups() {
2482+
/** setup blocked groups list */
2483+
$appConfig = $this->createMock(IAppConfig::class);
2484+
$appConfig->method('getValueArray')
2485+
->with('files_sharing', 'groups_block_list')
2486+
->willReturn(['blocked-group-1', 'blocked-group-2']);
2487+
$this->appConfig = $appConfig;
2488+
2489+
$shareProvider = $this->createMock(IShareProvider::class);
2490+
$shareProvider->method('getSharesByPath')->willReturn([]);
2491+
$this->factory->setProvider($shareProvider);
2492+
2493+
$manager = $this->createManagerMock()
2494+
->setMethods(['allowGroupSharing', 'canShare', 'generalCreateChecks', 'pathCreateChecks'])
2495+
->getMock();
2496+
2497+
$shareOwner = $this->createMock(IUser::class);
2498+
$shareOwner->method('getUID')->willReturn('shareOwner');
2499+
2500+
$storage = $this->createMock(IStorage::class);
2501+
$path = $this->createMock(File::class);
2502+
$path->method('getOwner')->willReturn($shareOwner);
2503+
$path->method('getName')->willReturn('target');
2504+
$path->method('getStorage')->willReturn($storage);
2505+
2506+
/** test create share with 'blocked-group-2': should throw exception */
2507+
$this->expectException(\InvalidArgumentException::class);
2508+
$share = $this->createShare(
2509+
null,
2510+
IShare::TYPE_GROUP,
2511+
$path,
2512+
'blocked-group-1',
2513+
'sharedBy',
2514+
null,
2515+
\OCP\Constants::PERMISSION_ALL);
2516+
2517+
$manager->expects($this->any())
2518+
->method('allowGroupSharing')
2519+
->willReturn(true);
2520+
$manager->expects($this->once())
2521+
->method('canShare')
2522+
->with($share)
2523+
->willReturn(true);
2524+
$manager->expects($this->once())
2525+
->method('generalCreateChecks')
2526+
->with($share);
2527+
;
2528+
$manager->expects($this->once())
2529+
->method('pathCreateChecks')
2530+
->with($path);
2531+
2532+
$this->defaultProvider
2533+
->expects($this->any())
2534+
->method('create')
2535+
->with($share)
2536+
->willReturnArgument(0);
2537+
2538+
$share->expects($this->any())
2539+
->method('setShareOwner')
2540+
->with('shareOwner');
2541+
$share->expects($this->any())
2542+
->method('setTarget')
2543+
->with('/target');
2544+
2545+
$manager->createShare($share);
2546+
2547+
}
2548+
24752549
public function testCreateShareGroup(): void {
24762550
$manager = $this->createManagerMock()
24772551
->setMethods(['canShare', 'generalCreateChecks', 'groupCreateChecks', 'pathCreateChecks'])

0 commit comments

Comments
 (0)