mirror of
https://github.com/nextcloud/server.git
synced 2026-04-29 10:03:32 -04:00
Merge pull request #47326 from nextcloud/revert-47227-backport/47192/stable28
Revert "[stable28] Apply group limit on remove from group"
This commit is contained in:
Arthur Schiwon
GitHub
commit
edd8a03a50
4 changed files with 8 additions and 168 deletions
|
|
@ -1,107 +0,0 @@
|
|||
/**
|
||||
* SPDX-FileCopyrightText: 2024 Nextcloud GmbH and Nextcloud contributors
|
||||
* SPDX-License-Identifier: AGPL-3.0-or-later
|
||||
*/
|
||||
|
||||
import { User } from "@nextcloud/cypress"
|
||||
import { createShare } from "./FilesSharingUtils.ts"
|
||||
|
||||
describe('Limit to sharing to people in the same group', () => {
|
||||
let alice: User
|
||||
let bob: User
|
||||
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')
|
||||
|
||||
cy.createRandomUser()
|
||||
.then(user => {
|
||||
alice = user
|
||||
cy.createRandomUser()
|
||||
})
|
||||
.then(user => {
|
||||
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}`)
|
||||
|
||||
cy.login(alice)
|
||||
cy.visit('/apps/files')
|
||||
createShare(randomFileName1, bob.userId)
|
||||
cy.login(bob)
|
||||
cy.visit('/apps/files')
|
||||
createShare(randomFileName2, alice.userId)
|
||||
})
|
||||
})
|
||||
|
||||
after(() => {
|
||||
cy.runOccCommand('config:app:set core shareapi_only_share_with_group_members --value no')
|
||||
})
|
||||
|
||||
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 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')
|
||||
cy.get(`[data-cy-files-list] [data-cy-files-list-row-name="${randomFileName2}"]`).should('not.exist')
|
||||
})
|
||||
|
||||
it('Bob cannot 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('not.exist')
|
||||
})
|
||||
})
|
||||
})
|
||||
|
|
@ -53,7 +53,6 @@ use OCP\L10N\IFactory;
|
|||
use OCP\Mail\IMailer;
|
||||
use OCP\Share\Exceptions\ShareNotFound;
|
||||
use OCP\Share\IAttributes;
|
||||
use OCP\Share\IManager;
|
||||
use OCP\Share\IShare;
|
||||
use OCP\Share\IShareProvider;
|
||||
use function str_starts_with;
|
||||
|
|
@ -103,7 +102,6 @@ class DefaultShareProvider implements IShareProvider {
|
|||
IFactory $l10nFactory,
|
||||
IURLGenerator $urlGenerator,
|
||||
ITimeFactory $timeFactory,
|
||||
private IManager $shareManager,
|
||||
) {
|
||||
$this->dbConn = $connection;
|
||||
$this->userManager = $userManager;
|
||||
|
|
@ -1306,7 +1304,6 @@ class DefaultShareProvider implements IShareProvider {
|
|||
*
|
||||
* @param string $uid
|
||||
* @param string $gid
|
||||
* @return void
|
||||
*/
|
||||
public function userDeletedFromGroup($uid, $gid) {
|
||||
/*
|
||||
|
|
@ -1318,7 +1315,7 @@ class DefaultShareProvider implements IShareProvider {
|
|||
->where($qb->expr()->eq('share_type', $qb->createNamedParameter(IShare::TYPE_GROUP)))
|
||||
->andWhere($qb->expr()->eq('share_with', $qb->createNamedParameter($gid)));
|
||||
|
||||
$cursor = $qb->executeQuery();
|
||||
$cursor = $qb->execute();
|
||||
$ids = [];
|
||||
while ($row = $cursor->fetch()) {
|
||||
$ids[] = (int)$row['id'];
|
||||
|
|
@ -1335,45 +1332,7 @@ class DefaultShareProvider implements IShareProvider {
|
|||
->where($qb->expr()->eq('share_type', $qb->createNamedParameter(IShare::TYPE_USERGROUP)))
|
||||
->andWhere($qb->expr()->eq('share_with', $qb->createNamedParameter($uid)))
|
||||
->andWhere($qb->expr()->in('parent', $qb->createNamedParameter($chunk, IQueryBuilder::PARAM_INT_ARRAY)));
|
||||
$qb->executeStatement();
|
||||
}
|
||||
}
|
||||
|
||||
if ($this->shareManager->shareWithGroupMembersOnly()) {
|
||||
$user = $this->userManager->get($uid);
|
||||
if ($user === null) {
|
||||
return;
|
||||
}
|
||||
$userGroups = $this->groupManager->getUserGroupIds($user);
|
||||
|
||||
// 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);
|
||||
|
||||
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);
|
||||
}
|
||||
$qb->execute();
|
||||
}
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -106,7 +106,6 @@ class ProviderFactory implements IProviderFactory {
|
|||
$this->serverContainer->getL10NFactory(),
|
||||
$this->serverContainer->getURLGenerator(),
|
||||
$this->serverContainer->query(ITimeFactory::class),
|
||||
$this->serverContainer->get(IManager::class),
|
||||
);
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -39,7 +39,6 @@ use OCP\IUser;
|
|||
use OCP\IUserManager;
|
||||
use OCP\L10N\IFactory;
|
||||
use OCP\Mail\IMailer;
|
||||
use OCP\Share\IManager as IShareManager;
|
||||
use OCP\Share\IShare;
|
||||
use PHPUnit\Framework\MockObject\MockObject;
|
||||
|
||||
|
|
@ -83,9 +82,6 @@ class DefaultShareProviderTest extends \Test\TestCase {
|
|||
/** @var ITimeFactory|MockObject */
|
||||
protected $timeFactory;
|
||||
|
||||
/** @var IShareManager&MockObject */
|
||||
protected $shareManager;
|
||||
|
||||
protected function setUp(): void {
|
||||
$this->dbConn = \OC::$server->getDatabaseConnection();
|
||||
$this->userManager = $this->createMock(IUserManager::class);
|
||||
|
|
@ -97,7 +93,6 @@ class DefaultShareProviderTest extends \Test\TestCase {
|
|||
$this->defaults = $this->getMockBuilder(Defaults::class)->disableOriginalConstructor()->getMock();
|
||||
$this->urlGenerator = $this->createMock(IURLGenerator::class);
|
||||
$this->timeFactory = $this->createMock(ITimeFactory::class);
|
||||
$this->shareManager = $this->createMock(IShareManager::class);
|
||||
|
||||
$this->userManager->expects($this->any())->method('userExists')->willReturn(true);
|
||||
$this->timeFactory->expects($this->any())->method('now')->willReturn(new \DateTimeImmutable("2023-05-04 00:00 Europe/Berlin"));
|
||||
|
|
@ -114,8 +109,7 @@ class DefaultShareProviderTest extends \Test\TestCase {
|
|||
$this->defaults,
|
||||
$this->l10nFactory,
|
||||
$this->urlGenerator,
|
||||
$this->timeFactory,
|
||||
$this->shareManager,
|
||||
$this->timeFactory
|
||||
);
|
||||
}
|
||||
|
||||
|
|
@ -476,8 +470,7 @@ class DefaultShareProviderTest extends \Test\TestCase {
|
|||
$this->defaults,
|
||||
$this->l10nFactory,
|
||||
$this->urlGenerator,
|
||||
$this->timeFactory,
|
||||
$this->shareManager,
|
||||
$this->timeFactory
|
||||
])
|
||||
->setMethods(['getShareById'])
|
||||
->getMock();
|
||||
|
|
@ -572,8 +565,7 @@ class DefaultShareProviderTest extends \Test\TestCase {
|
|||
$this->defaults,
|
||||
$this->l10nFactory,
|
||||
$this->urlGenerator,
|
||||
$this->timeFactory,
|
||||
$this->shareManager,
|
||||
$this->timeFactory
|
||||
])
|
||||
->setMethods(['getShareById'])
|
||||
->getMock();
|
||||
|
|
@ -2533,8 +2525,7 @@ class DefaultShareProviderTest extends \Test\TestCase {
|
|||
$this->defaults,
|
||||
$this->l10nFactory,
|
||||
$this->urlGenerator,
|
||||
$this->timeFactory,
|
||||
$this->shareManager,
|
||||
$this->timeFactory
|
||||
);
|
||||
|
||||
$password = md5(time());
|
||||
|
|
@ -2632,8 +2623,7 @@ class DefaultShareProviderTest extends \Test\TestCase {
|
|||
$this->defaults,
|
||||
$this->l10nFactory,
|
||||
$this->urlGenerator,
|
||||
$this->timeFactory,
|
||||
$this->shareManager,
|
||||
$this->timeFactory
|
||||
);
|
||||
|
||||
$u1 = $userManager->createUser('testShare1', 'test');
|
||||
|
|
@ -2729,8 +2719,7 @@ class DefaultShareProviderTest extends \Test\TestCase {
|
|||
$this->defaults,
|
||||
$this->l10nFactory,
|
||||
$this->urlGenerator,
|
||||
$this->timeFactory,
|
||||
$this->shareManager,
|
||||
$this->timeFactory
|
||||
);
|
||||
|
||||
$u1 = $userManager->createUser('testShare1', 'test');
|
||||
|
|
|
|||
Loading…
Reference in a new issue