feat(core): exclude selected groups from sharing

redblom · susnux · commit 618f702b542d · 2025-04-23T21:21:05.000+02:00
Refs: #49198 Signed-off-by: Antoon Prins <antoon.prins@surf.nl>
diff --git a/apps/settings/lib/Settings/Admin/Sharing.php b/apps/settings/lib/Settings/Admin/Sharing.php
@@ -9,6 +9,7 @@
 use OCP\AppFramework\Http\TemplateResponse;
 use OCP\AppFramework\Services\IInitialState;
 use OCP\Constants;
+use OCP\IAppConfig;
 use OCP\IConfig;
 use OCP\IL10N;
 use OCP\IURLGenerator;
@@ -19,6 +20,7 @@
 class Sharing implements IDelegatedSettings {
 	public function __construct(
 		private IConfig $config,
+		private IAppConfig $appConfig,
 		private IL10N $l,
 		private IManager $shareManager,
 		private IAppManager $appManager,
@@ -41,6 +43,7 @@ public function getForm() {
 			// Built-In Sharing
 			'enabled' => $this->getHumanBooleanConfig('core', 'shareapi_enabled', true),
 			'allowGroupSharing' => $this->getHumanBooleanConfig('core', 'shareapi_allow_group_sharing', true),
+			'groupsBlockList' => $this->appConfig->getValueArray('core', 'shareapi_groups_block_list', []),
 			'allowLinks' => $this->getHumanBooleanConfig('core', 'shareapi_allow_links', true),
 			'allowLinksExcludeGroups' => json_decode($linksExcludedGroups, true) ?? [],
 			'allowPublicUpload' => $this->getHumanBooleanConfig('core', 'shareapi_allow_public_upload', true),
diff --git a/apps/settings/src/components/AdminSettingsSharingForm.vue b/apps/settings/src/components/AdminSettingsSharingForm.vue
@@ -17,6 +17,13 @@
 			<NcCheckboxRadioSwitch :checked.sync="settings.allowGroupSharing">
 				{{ t('settings', 'Allow sharing with groups') }}
 			</NcCheckboxRadioSwitch>
+			<div v-show="settings.allowGroupSharing" id="settings-sharing-api-groups-block-list" class="sharing__labeled-entry sharing__input">
+				<label for="sharing-groups-block-list">{{ t('settings', 'Groups that are excluded from sharing') }}</label>
+				<NcSettingsSelectGroup id="sharing-groups-block-list"
+					v-model="settings.groupsBlockList"
+					:label="t('settings', 'Select groups')"
+					style="width: 100%" />
+			</div>
 			<NcCheckboxRadioSwitch :checked.sync="settings.onlyShareWithGroupMembers">
 				{{ t('settings', 'Restrict users to only share with users in their groups') }}
 			</NcCheckboxRadioSwitch>
@@ -258,6 +265,7 @@ interface IShareSettings {
 	remoteExpireAfterNDays: string
 	enforceRemoteExpireDate: boolean
 	allowCustomTokens: boolean
+	groupsBlockList: string[]
 }
 
 export default defineComponent({
diff --git a/apps/settings/tests/Settings/Admin/SharingTest.php b/apps/settings/tests/Settings/Admin/SharingTest.php
@@ -10,6 +10,7 @@
 use OCP\AppFramework\Http\TemplateResponse;
 use OCP\AppFramework\Services\IInitialState;
 use OCP\Constants;
+use OCP\IAppConfig;
 use OCP\IConfig;
 use OCP\IL10N;
 use OCP\IURLGenerator;
@@ -22,6 +23,8 @@ class SharingTest extends TestCase {
 	private $admin;
 	/** @var IConfig&MockObject */
 	private $config;
+	/** @var IAppConfig */
+	private $appConfig;
 	/** @var IL10N&MockObject */
 	private $l10n;
 	/** @var IManager|MockObject */
@@ -36,6 +39,7 @@ class SharingTest extends TestCase {
 	protected function setUp(): void {
 		parent::setUp();
 		$this->config = $this->getMockBuilder(IConfig::class)->getMock();
+		$this->appConfig = $this->getMockBuilder(IAppConfig::class)->getMock();
 		$this->l10n = $this->getMockBuilder(IL10N::class)->getMock();
 
 		/** @var IManager|MockObject */
@@ -49,6 +53,7 @@ protected function setUp(): void {
 
 		$this->admin = new Sharing(
 			$this->config,
+			$this->appConfig,
 			$this->l10n,
 			$this->shareManager,
 			$this->appManager,
@@ -65,6 +70,7 @@ public function testGetFormWithoutExcludedGroups(): void {
 				['core', 'shareapi_exclude_groups_list', '', ''],
 				['core', 'shareapi_allow_links_exclude_groups', '', ''],
 				['core', 'shareapi_allow_group_sharing', 'yes', 'yes'],
+				['core', 'shareapi_groups_block_list', '[]', '[]'],
 				['core', 'shareapi_allow_links', 'yes', 'yes'],
 				['core', 'shareapi_allow_public_upload', 'yes', 'yes'],
 				['core', 'shareapi_allow_resharing', 'yes', 'yes'],
@@ -110,6 +116,7 @@ public function testGetFormWithoutExcludedGroups(): void {
 			'sharingDocumentation' => '',
 			'sharingSettings' => [
 				'allowGroupSharing' => true,
+				'groupsBlockList' => [],
 				'allowLinks' => true,
 				'allowPublicUpload' => true,
 				'allowResharing' => true,
@@ -162,6 +169,7 @@ public function testGetFormWithExcludedGroups(): void {
 				['core', 'shareapi_exclude_groups_list', '', '["NoSharers","OtherNoSharers"]'],
 				['core', 'shareapi_allow_links_exclude_groups', '', ''],
 				['core', 'shareapi_allow_group_sharing', 'yes', 'yes'],
+				['core', 'shareapi_groups_block_list', '[]', '[]'],
 				['core', 'shareapi_allow_links', 'yes', 'yes'],
 				['core', 'shareapi_allow_public_upload', 'yes', 'yes'],
 				['core', 'shareapi_allow_resharing', 'yes', 'yes'],
@@ -207,6 +215,7 @@ public function testGetFormWithExcludedGroups(): void {
 			'sharingDocumentation' => '',
 			'sharingSettings' => [
 				'allowGroupSharing' => true,
+				'groupsBlockList' => [],
 				'allowLinks' => true,
 				'allowPublicUpload' => true,
 				'allowResharing' => true,
diff --git a/lib/private/Group/Group.php b/lib/private/Group/Group.php
@@ -25,6 +25,7 @@
 use OCP\Group\Events\UserAddedEvent;
 use OCP\Group\Events\UserRemovedEvent;
 use OCP\GroupInterface;
+use OCP\IAppConfig;
 use OCP\IGroup;
 use OCP\IUser;
 use OCP\IUserManager;
@@ -51,7 +52,15 @@ class Group implements IGroup {
 	/** @var PublicEmitter */
 	private $emitter;
 
-	public function __construct(string $gid, array $backends, IEventDispatcher $dispatcher, IUserManager $userManager, ?PublicEmitter $emitter = null, ?string $displayName = null) {
+	public function __construct(
+		string $gid,
+		array $backends,
+		IEventDispatcher $dispatcher,
+		IUserManager $userManager,
+		private IAppConfig $appConfig,
+		?PublicEmitter $emitter = null,
+		?string $displayName = null,
+	) {
 		$this->gid = $gid;
 		$this->backends = $backends;
 		$this->dispatcher = $dispatcher;
@@ -376,6 +385,10 @@ public function canAddUser(): bool {
 	 * @since 16.0.0
 	 */
 	public function hideFromCollaboration(): bool {
+		if (in_array($this->gid, $this->appConfig->getValueArray('core', 'shareapi_groups_block_list', []))) {
+			return true;
+		}
+		
 		return array_reduce($this->backends, function (bool $hide, GroupInterface $backend) {
 			return $hide | ($backend instanceof IHideFromCollaborationBackend && $backend->hideGroup($this->gid));
 		}, false);
diff --git a/lib/private/Group/Manager.php b/lib/private/Group/Manager.php
@@ -16,6 +16,7 @@
 use OCP\Group\Events\BeforeGroupCreatedEvent;
 use OCP\Group\Events\GroupCreatedEvent;
 use OCP\GroupInterface;
+use OCP\IAppConfig;
 use OCP\ICacheFactory;
 use OCP\IGroup;
 use OCP\IGroupManager;
@@ -52,6 +53,9 @@ class Manager extends PublicEmitter implements IGroupManager {
 	/** @var \OC\SubAdmin */
 	private $subAdmin = null;
 
+	/** @var IAppConfig $appConfig */
+	private $appConfig;
+
 	private DisplayNameCache $displayNameCache;
 
 	private const MAX_GROUP_LENGTH = 255;
@@ -159,7 +163,7 @@ protected function getGroupObject($gid, $displayName = null) {
 			return null;
 		}
 		/** @var GroupInterface[] $backends */
-		$this->cachedGroups[$gid] = new Group($gid, $backends, $this->dispatcher, $this->userManager, $this, $displayName);
+		$this->cachedGroups[$gid] = new Group($gid, $backends, $this->dispatcher, $this->userManager, $this->getAppConfig(), $this, $displayName);
 		return $this->cachedGroups[$gid];
 	}
 
@@ -214,7 +218,7 @@ protected function getGroupsObjects(array $gids, array $displayNames = []): arra
 			if (count($backends[$gid]) === 0) {
 				continue;
 			}
-			$this->cachedGroups[$gid] = new Group($gid, $backends[$gid], $this->dispatcher, $this->userManager, $this, $displayNames[$gid]);
+			$this->cachedGroups[$gid] = new Group($gid, $backends[$gid], $this->dispatcher, $this->userManager, $this->getAppConfig(), $this, $displayNames[$gid]);
 			$groups[$gid] = $this->cachedGroups[$gid];
 		}
 		return $groups;
@@ -472,4 +476,17 @@ public function getSubAdmin() {
 
 		return $this->subAdmin;
 	}
+
+	/**
+	 * Returns the appConfig object
+	 * 
+	 * @return IAppConfig
+	 */
+	private function getAppConfig():IAppConfig {
+		if (isset($this->appConfig)) {
+			return $this->appConfig;
+		} else {
+			return \OC::$server->get(IAppConfig::class);
+		}
+	}
 }
diff --git a/lib/private/Share20/Manager.php b/lib/private/Share20/Manager.php
@@ -513,6 +513,12 @@ protected function groupCreateChecks(IShare $share) {
 			throw new \Exception($this->l->t('Group sharing is now allowed'));
 		}
 
+		// Check if sharing with this group is blocked
+		$groupsBlockList = $this->appConfig->getValueArray('core', 'shareapi_groups_block_list', []);
+		if(in_array($share->getSharedWith(), $groupsBlockList)) {
+			throw new \InvalidArgumentException('Sharing with group ' . $share->getSharedWith() . ' is not allowed.');
+		}
+
 		// Verify if the user can share with this group
 		if ($this->shareWithGroupMembersOnly()) {
 			$sharedBy = $this->userManager->get($share->getSharedBy());
diff --git a/tests/lib/Collaboration/Collaborators/GroupPluginTest.php b/tests/lib/Collaboration/Collaborators/GroupPluginTest.php
@@ -1,4 +1,5 @@
 <?php
+
 /**
  * SPDX-FileCopyrightText: 2017 Nextcloud GmbH and Nextcloud contributors
  * SPDX-License-Identifier: AGPL-3.0-or-later
@@ -9,6 +10,7 @@
 use OC\Collaboration\Collaborators\GroupPlugin;
 use OC\Collaboration\Collaborators\SearchResult;
 use OCP\Collaboration\Collaborators\ISearchResult;
+use OCP\IAppConfig;
 use OCP\IConfig;
 use OCP\IGroup;
 use OCP\IGroupManager;
@@ -21,6 +23,9 @@ class GroupPluginTest extends TestCase {
 	/** @var IConfig|\PHPUnit\Framework\MockObject\MockObject */
 	protected $config;
 
+	/** @var IAppConfig|\PHPUnit\Framework\MockObject\MockObject */
+	protected $appConfig;
+
 	/** @var IGroupManager|\PHPUnit\Framework\MockObject\MockObject */
 	protected $groupManager;
 
@@ -47,6 +52,8 @@ protected function setUp(): void {
 
 		$this->config = $this->createMock(IConfig::class);
 
+		$this->appConfig = $this->createMock(IAppConfig::class);
+
 		$this->groupManager = $this->createMock(IGroupManager::class);
 
 		$this->session = $this->createMock(IUserSession::class);
@@ -413,6 +420,24 @@ public function dataGetGroups(): array {
 				true,
 				false,
 			],
+			[
+				'test', true, true, false,
+				[
+					$this->getGroupMock('test0', 'test0', true),
+					$this->getGroupMock('test1'),
+				],
+				[
+					$this->getGroupMock('test0'),
+					$this->getGroupMock('test1')
+				],
+				[],
+				[
+					['label' => 'test1', 'value' => ['shareType' => IShare::TYPE_GROUP, 'shareWith' => 'test1']],
+				],
+				false,
+				false,
+				['test0'],
+			],
 		];
 	}
 
@@ -429,6 +454,7 @@ public function dataGetGroups(): array {
 	 * @param array $expected
 	 * @param bool $reachedEnd
 	 * @param bool|IGroup $singleGroup
+	 * @param array $groupsBlockList
 	 */
 	public function testSearch(
 		string $searchTerm,
@@ -441,6 +467,7 @@ public function testSearch(
 		array $expected,
 		bool $reachedEnd,
 		$singleGroup,
+		array $groupsBlockList = [],
 	): void {
 		$this->config->expects($this->any())
 			->method('getAppValue')
@@ -462,6 +489,15 @@ function ($appName, $key, $default) use ($shareWithGroupOnly, $shareeEnumeration
 				}
 			);
 
+		if ($groupsBlockList != []) {
+			/** setup blocked groups list */
+			$appConfig = $this->createMock(IAppConfig::class);
+			$appConfig->method('getValueArray')
+				->with('core', 'shareapi_groups_block_list')
+				->willReturn($groupsBlockList);
+			$this->appConfig = $appConfig;
+		}
+
 		$this->instantiatePlugin();
 
 		if (!$groupSharingDisabled) {
diff --git a/tests/lib/Share20/ManagerTest.php b/tests/lib/Share20/ManagerTest.php
@@ -1,4 +1,5 @@
 <?php
+
 /**
  * SPDX-FileCopyrightText: 2016-2024 Nextcloud GmbH and Nextcloud contributors
  * SPDX-FileCopyrightText: 2016 ownCloud, Inc.
@@ -2480,6 +2481,74 @@ public function testCreateShareUser(): void {
 		$manager->createShare($share);
 	}
 
+	public function testCreateShareBlockedGroups() {
+		/** setup blocked groups list */
+		$appConfig = $this->createMock(IAppConfig::class);
+		$appConfig->method('getValueArray')
+			->with('core', 'shareapi_groups_block_list')
+			->willReturn(['blocked-group-1', 'blocked-group-2']);
+		$this->appConfig = $appConfig;
+
+		$shareProvider = $this->createMock(IShareProvider::class);
+		$shareProvider->method('getSharesByPath')->willReturn([]);
+		$this->factory->setProvider($shareProvider);
+
+		$manager = $this->createManagerMock()
+			->setMethods(['allowGroupSharing', 'canShare', 'generalCreateChecks', 'pathCreateChecks'])
+			->getMock();
+
+		$shareOwner = $this->createMock(IUser::class);
+		$shareOwner->method('getUID')->willReturn('shareOwner');
+
+		$storage = $this->createMock(IStorage::class);
+		$path = $this->createMock(File::class);
+		$path->method('getOwner')->willReturn($shareOwner);
+		$path->method('getName')->willReturn('target');
+		$path->method('getStorage')->willReturn($storage);
+
+		/** test create share with 'blocked-group-2': should throw exception */
+		$this->expectException(\InvalidArgumentException::class);
+		$share = $this->createShare(
+			null,
+			IShare::TYPE_GROUP,
+			$path,
+			'blocked-group-1',
+			'sharedBy',
+			null,
+			\OCP\Constants::PERMISSION_ALL,
+		);
+
+		$manager->expects($this->any())
+			->method('allowGroupSharing')
+			->willReturn(true);
+		$manager->expects($this->once())
+			->method('canShare')
+			->with($share)
+			->willReturn(true);
+		$manager->expects($this->once())
+			->method('generalCreateChecks')
+			->with($share);
+		$manager->expects($this->once())
+			->method('pathCreateChecks')
+			->with($path);
+
+		$this->defaultProvider
+			->expects($this->any())
+			->method('create')
+			->with($share)
+			->willReturnArgument(0);
+
+		$share->expects($this->any())
+			->method('setShareOwner')
+			->with('shareOwner');
+		$share->expects($this->any())
+			->method('setTarget')
+			->with('/target');
+
+		$manager->createShare($share);
+
+	}
+
 	public function testCreateShareGroup(): void {
 		$manager = $this->createManagerMock()
 			->setMethods(['canShare', 'generalCreateChecks', 'groupCreateChecks', 'pathCreateChecks'])
