fix: Remove shares only if there are no more common groups between users

Signed-off-by: Côme Chilliet <come.chilliet@nextcloud.com>
nextcloud · Aug 12, 2024 · f217c6e · f217c6e
1 parent dcad99b
commit f217c6e
Show file tree

Hide file tree

Showing 2 changed files with 64 additions and 39 deletions.
diff --git a/cypress/e2e/files_sharing/limit_to_same_group.cy.ts b/cypress/e2e/files_sharing/limit_to_same_group.cy.ts
@@ -29,11 +29,15 @@ describe('Limit to sharing to people in the same group', () => {
 	let randomFileName1 = ''
 	let randomFileName2 = ''
 	let randomGroupName = ''
+	let randomGroupName2 = ''
+	let randomGroupName3 = ''
 
 	before(() => {
 		randomFileName1 = Math.random().toString(36).replace(/[^a-z]+/g, '').substring(0, 10) + '.txt'
 		randomFileName2 = Math.random().toString(36).replace(/[^a-z]+/g, '').substring(0, 10) + '.txt'
 		randomGroupName = Math.random().toString(36).replace(/[^a-z]+/g, '').substring(0, 10)
+		randomGroupName2 = Math.random().toString(36).replace(/[^a-z]+/g, '').substring(0, 10)
+		randomGroupName3 = Math.random().toString(36).replace(/[^a-z]+/g, '').substring(0, 10)
 
 		cy.runOccCommand('config:app:set core shareapi_only_share_with_group_members --value yes')
 
@@ -46,8 +50,13 @@ describe('Limit to sharing to people in the same group', () => {
 				bob = user
 
 				cy.runOccCommand(`group:add ${randomGroupName}`)
+				cy.runOccCommand(`group:add ${randomGroupName2}`)
+				cy.runOccCommand(`group:add ${randomGroupName3}`)
 				cy.runOccCommand(`group:adduser ${randomGroupName} ${alice.userId}`)
 				cy.runOccCommand(`group:adduser ${randomGroupName} ${bob.userId}`)
+				cy.runOccCommand(`group:adduser ${randomGroupName2} ${alice.userId}`)
+				cy.runOccCommand(`group:adduser ${randomGroupName2} ${bob.userId}`)
+				cy.runOccCommand(`group:adduser ${randomGroupName3} ${bob.userId}`)
 
 				cy.uploadContent(alice, new Blob(['share to bob'], { type: 'text/plain' }), 'text/plain', `/${randomFileName1}`)
 				cy.uploadContent(bob, new Blob(['share by bob'], { type: 'text/plain' }), 'text/plain', `/${randomFileName2}`)
@@ -77,11 +86,29 @@ describe('Limit to sharing to people in the same group', () => {
 		cy.get(`[data-cy-files-list] [data-cy-files-list-row-name="${randomFileName1}"]`).should('exist')
 	})
 
-	context('Bob is removed from the group', () => {
+	context('Bob is removed from the first group', () => {
 		before(() => {
 			cy.runOccCommand(`group:removeuser ${randomGroupName} ${bob.userId}`)
 		})
 
+		it('Alice can see the shared file', () => {
+			cy.login(alice)
+			cy.visit('/apps/files')
+			cy.get(`[data-cy-files-list] [data-cy-files-list-row-name="${randomFileName2}"]`).should('exist')
+		})
+
+		it('Bob can see the shared file', () => {
+			cy.login(alice)
+			cy.visit('/apps/files')
+			cy.get(`[data-cy-files-list] [data-cy-files-list-row-name="${randomFileName1}"]`).should('exist')
+		})
+	})
+
+	context('Bob is removed from the second group', () => {
+		before(() => {
+			cy.runOccCommand(`group:removeuser ${randomGroupName2} ${bob.userId}`)
+		})
+
 		it('Alice cannot see the shared file', () => {
 			cy.login(alice)
 			cy.visit('/apps/files')

diff --git a/lib/private/Share20/DefaultShareProvider.php b/lib/private/Share20/DefaultShareProvider.php
@@ -1225,6 +1225,7 @@ public function groupDeleted($gid) {
 	 *
 	 * @param string $uid
 	 * @param string $gid
+	 * @return void
 	 */
 	public function userDeletedFromGroup($uid, $gid) {
 		/*
@@ -1258,45 +1259,42 @@ public function userDeletedFromGroup($uid, $gid) {
 		}
 
 		if ($this->shareManager->shareWithGroupMembersOnly()) {
-			$deleteQuery = $this->dbConn->getQueryBuilder();
-			$deleteQuery->delete('share')
-				->where($deleteQuery->expr()->in('id', $deleteQuery->createParameter('id')));
+			$user = $this->userManager->get($uid);
+			if ($user === null) {
+				return;
+			}
+			$userGroups = $this->groupManager->getUserGroupIds($user);
+			$userGroups = array_diff($userGroups, $this->shareManager->shareWithGroupMembersOnlyExcludeGroupsList());
+
+			// Delete user shares received by the user from users in the group.
+			$userReceivedShares = $this->shareManager->getSharedWith($uid, IShare::TYPE_USER, null, -1);
+			foreach ($userReceivedShares as $share) {
+				$owner = $this->userManager->get($share->getSharedBy());
+				if ($owner === null) {
+					continue;
+				}
+				$ownerGroups = $this->groupManager->getUserGroupIds($owner);
+				$mutualGroups = array_intersect($userGroups, $ownerGroups);
 
-			// Delete direct shares received by the user from users in the group.
-			$selectInboundShares = $this->dbConn->getQueryBuilder();
-			$selectInboundShares->select('id')
-				->from('share', 's')
-				->join('s', 'group_user', 'g', 's.uid_initiator = g.uid')
-				->where($selectInboundShares->expr()->eq('share_type', $selectInboundShares->createNamedParameter(IShare::TYPE_USER)))
-				->andWhere($selectInboundShares->expr()->eq('share_with', $selectInboundShares->createNamedParameter($uid)))
-				->andWhere($selectInboundShares->expr()->eq('gid', $selectInboundShares->createNamedParameter($gid)))
-				->setMaxResults(1000)
-				->executeQuery();
-
-			do {
-				$rows = $selectInboundShares->executeQuery();
-				$ids = $rows->fetchAll();
-				$deleteQuery->setParameter('id', array_column($ids, 'id'), IQueryBuilder::PARAM_INT_ARRAY);
-				$deleteQuery->executeStatement();
-			} while (count($ids) > 0);
-
-			// Delete direct shares from the user to users in the group.
-			$selectOutboundShares = $this->dbConn->getQueryBuilder();
-			$selectOutboundShares->select('id')
-				->from('share', 's')
-				->join('s', 'group_user', 'g', 's.share_with = g.uid')
-				->where($selectOutboundShares->expr()->eq('share_type', $selectOutboundShares->createNamedParameter(IShare::TYPE_USER)))
-				->andWhere($selectOutboundShares->expr()->eq('uid_initiator', $selectOutboundShares->createNamedParameter($uid)))
-				->andWhere($selectOutboundShares->expr()->eq('gid', $selectOutboundShares->createNamedParameter($gid)))
-				->setMaxResults(1000)
-				->executeQuery();
-
-			do {
-				$rows = $selectOutboundShares->executeQuery();
-				$ids = $rows->fetchAll();
-				$deleteQuery->setParameter('id', array_column($ids, 'id'), IQueryBuilder::PARAM_INT_ARRAY);
-				$deleteQuery->executeStatement();
-			} while (count($ids) > 0);
+				if (count($mutualGroups) === 0) {
+					$this->shareManager->deleteShare($share);
+				}
+			}
+
+			// Delete user shares from the user to users in the group.
+			$userEmittedShares = $this->shareManager->getSharesBy($uid, IShare::TYPE_USER, null, true, -1);
+			foreach ($userEmittedShares as $share) {
+				$recipient = $this->userManager->get($share->getSharedWith());
+				if ($recipient === null) {
+					continue;
+				}
+				$recipientGroups = $this->groupManager->getUserGroupIds($recipient);
+				$mutualGroups = array_intersect($userGroups, $recipientGroups);
+
+				if (count($mutualGroups) === 0) {
+					$this->shareManager->deleteShare($share);
+				}
+			}
 		}
 	}