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

Signed-off-by: Côme Chilliet <come.chilliet@nextcloud.com>

diff --git a/cypress/e2e/files_sharing/limit_to_same_group.cy.ts b/cypress/e2e/files_sharing/limit_to_same_group.cy.ts
index 6d9a4170cbacbe3814633af799a5c120f25f6eed..0c771c931f7ca79f08ad0463df209dd76d49730c 100644 (file)
--- 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
index 2d9ec7d3d68bf6b1a8f6a39f25589a85f0aefdc4..65c4a167311c8ac555d397bef6a161b4ad18809b 100644 (file)
--- a/lib/private/Share20/DefaultShareProvider.php
+++ b/lib/private/Share20/DefaultShareProvider.php
@@ -1306,6 +1306,7 @@ class DefaultShareProvider implements IShareProvider {
         *
         * @param string $uid
         * @param string $gid
+        * @return void
         */
        public function userDeletedFromGroup($uid, $gid) {
                /*
@@ -1339,45 +1340,42 @@ class DefaultShareProvider implements IShareProvider {
                }
 
                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);
+                               }
+                       }
                }
        }