Add checks for whether a user with access to a share can delete it
Signed-off-by: John Molakvoæ (skjnldsv) <skjnldsv@protonmail.com>
This commit is contained in:
John Molakvoæ (skjnldsv)
parent
f02cff1304
commit
b1069b29fa
2 changed files with 294 additions and 14 deletions
|
|
@ -336,21 +336,24 @@ class ShareAPIController extends OCSController {
|
|||
try {
|
||||
$this->lock($share->getNode());
|
||||
} catch (LockedException $e) {
|
||||
throw new OCSNotFoundException($this->l->t('could not delete share'));
|
||||
}
|
||||
|
||||
if (!$this->canAccessShare($share)) {
|
||||
throw new OCSNotFoundException($this->l->t('Could not delete share'));
|
||||
}
|
||||
|
||||
if ((
|
||||
$share->getShareType() === Share::SHARE_TYPE_GROUP
|
||||
|| $share->getShareType() === Share::SHARE_TYPE_ROOM
|
||||
)
|
||||
&& $share->getShareOwner() !== $this->currentUser
|
||||
&& $share->getSharedBy() !== $this->currentUser) {
|
||||
if (!$this->canAccessShare($share)) {
|
||||
throw new OCSNotFoundException($this->l->t('Wrong share ID, share doesn\'t exist'));
|
||||
}
|
||||
|
||||
// if it's a group share or a room share
|
||||
// we don't delete the share, but only the
|
||||
// mount point. Allowing it to be restored
|
||||
// from the deleted shares
|
||||
if ($this->canDeleteShareFromSelf($share)) {
|
||||
$this->shareManager->deleteFromSelf($share, $this->currentUser);
|
||||
} else {
|
||||
if (!$this->canDeleteShare($share)) {
|
||||
throw new OCSForbiddenException($this->l->t('Could not delete share'));
|
||||
}
|
||||
|
||||
$this->shareManager->deleteShare($share);
|
||||
}
|
||||
|
||||
|
|
@ -500,7 +503,6 @@ class ShareAPIController extends OCSController {
|
|||
}
|
||||
}
|
||||
|
||||
|
||||
if ($sendPasswordByTalk === 'true') {
|
||||
if (!$this->appManager->isEnabledForUser('spreed')) {
|
||||
throw new OCSForbiddenException($this->l->t('Sharing %s sending the password by Nextcloud Talk failed because Nextcloud Talk is not enabled', [$path->getPath()]));
|
||||
|
|
@ -1052,6 +1054,83 @@ class ShareAPIController extends OCSController {
|
|||
return false;
|
||||
}
|
||||
|
||||
/**
|
||||
* Does the user have delete permission on the share
|
||||
*
|
||||
* @param \OCP\Share\IShare $share the share to check
|
||||
* @return boolean
|
||||
*/
|
||||
protected function canDeleteShare(\OCP\Share\IShare $share): bool {
|
||||
// A file with permissions 0 can't be accessed by us. So Don't show it
|
||||
if ($share->getPermissions() === 0) {
|
||||
return false;
|
||||
}
|
||||
|
||||
// if the user is the recipient, i can unshare
|
||||
// the share with self
|
||||
if ($share->getShareType() === Share::SHARE_TYPE_USER &&
|
||||
$share->getSharedWith() === $this->currentUser
|
||||
) {
|
||||
return true;
|
||||
}
|
||||
|
||||
// The owner of the file and the creator of the share
|
||||
// can always delete the share
|
||||
if ($share->getShareOwner() === $this->currentUser ||
|
||||
$share->getSharedBy() === $this->currentUser
|
||||
) {
|
||||
return true;
|
||||
}
|
||||
|
||||
return false;
|
||||
}
|
||||
|
||||
/**
|
||||
* Does the user have delete permission on the share
|
||||
* This differs from the canDeleteShare function as it only
|
||||
* remove the share for the current user. It does NOT
|
||||
* completely delete the share but only the mount point.
|
||||
* It can then be restored from the deleted shares section.
|
||||
*
|
||||
* @param \OCP\Share\IShare $share the share to check
|
||||
* @return boolean
|
||||
*
|
||||
* @suppress PhanUndeclaredClassMethod
|
||||
*/
|
||||
protected function canDeleteShareFromSelf(\OCP\Share\IShare $share): bool {
|
||||
if ($share->getShareType() !== Share::SHARE_TYPE_GROUP &&
|
||||
$share->getShareType() !== Share::SHARE_TYPE_ROOM
|
||||
) {
|
||||
return false;
|
||||
}
|
||||
|
||||
if ($share->getShareOwner() === $this->currentUser ||
|
||||
$share->getSharedBy() === $this->currentUser
|
||||
) {
|
||||
// Delete the whole share, not just for self
|
||||
return false;
|
||||
}
|
||||
|
||||
// If in the recipient group, you can delete the share from self
|
||||
if ($share->getShareType() === Share::SHARE_TYPE_GROUP) {
|
||||
$sharedWith = $this->groupManager->get($share->getSharedWith());
|
||||
$user = $this->userManager->get($this->currentUser);
|
||||
if ($user !== null && $sharedWith !== null && $sharedWith->inGroup($user)) {
|
||||
return true;
|
||||
}
|
||||
}
|
||||
|
||||
if ($share->getShareType() === Share::SHARE_TYPE_ROOM) {
|
||||
try {
|
||||
return $this->getRoomShareHelper()->canAccessShare($share, $this->currentUser);
|
||||
} catch (QueryException $e) {
|
||||
return false;
|
||||
}
|
||||
}
|
||||
|
||||
return false;
|
||||
}
|
||||
|
||||
/**
|
||||
* Make sure that the passed date is valid ISO 8601
|
||||
* So YYYY-MM-DD
|
||||
|
|
@ -1228,4 +1307,5 @@ class ShareAPIController extends OCSController {
|
|||
|
||||
return false;
|
||||
}
|
||||
|
||||
}
|
||||
|
|
|
|||
|
|
@ -213,19 +213,20 @@ class ShareAPIControllerTest extends TestCase {
|
|||
|
||||
/**
|
||||
* @expectedException \OCP\AppFramework\OCS\OCSNotFoundException
|
||||
* @expectedExceptionMessage could not delete share
|
||||
* @expectedExceptionMessage Could not delete share
|
||||
*/
|
||||
public function testDeleteShareLocked() {
|
||||
$node = $this->getMockBuilder(File::class)->getMock();
|
||||
|
||||
$share = $this->newShare();
|
||||
$share->setSharedBy($this->currentUser)
|
||||
->setNode($node);
|
||||
$share->setNode($node);
|
||||
|
||||
$this->shareManager
|
||||
->expects($this->once())
|
||||
->method('getShareById')
|
||||
->with('ocinternal:42')
|
||||
->willReturn($share);
|
||||
|
||||
$this->shareManager
|
||||
->expects($this->never())
|
||||
->method('deleteShare')
|
||||
|
|
@ -236,6 +237,205 @@ class ShareAPIControllerTest extends TestCase {
|
|||
->with(\OCP\Lock\ILockingProvider::LOCK_SHARED)
|
||||
->will($this->throwException(new LockedException('mypath')));
|
||||
|
||||
$this->assertFalse($this->invokePrivate($this->ocs, 'canDeleteFromSelf', [$share]));
|
||||
$this->assertFalse($this->invokePrivate($this->ocs, 'canDeleteShare', [$share]));
|
||||
|
||||
$this->ocs->deleteShare(42);
|
||||
}
|
||||
|
||||
/**
|
||||
* You can always remove a share that was shared with you
|
||||
*/
|
||||
public function testDeleteShareWithMe() {
|
||||
$node = $this->getMockBuilder(File::class)->getMock();
|
||||
|
||||
$share = $this->newShare();
|
||||
$share->setSharedWith($this->currentUser)
|
||||
->setShareType(\OCP\Share::SHARE_TYPE_USER)
|
||||
->setNode($node);
|
||||
|
||||
$this->shareManager
|
||||
->expects($this->once())
|
||||
->method('getShareById')
|
||||
->with('ocinternal:42')
|
||||
->willReturn($share);
|
||||
|
||||
$this->shareManager
|
||||
->expects($this->once())
|
||||
->method('deleteShare')
|
||||
->with($share);
|
||||
|
||||
$node->expects($this->once())
|
||||
->method('lock')
|
||||
->with(\OCP\Lock\ILockingProvider::LOCK_SHARED);
|
||||
|
||||
$this->assertFalse($this->invokePrivate($this->ocs, 'canDeleteFromSelf', [$share]));
|
||||
$this->assertTrue($this->invokePrivate($this->ocs, 'canDeleteShare', [$share]));
|
||||
|
||||
$this->ocs->deleteShare(42);
|
||||
}
|
||||
|
||||
/**
|
||||
* You can always delete a share you own
|
||||
*/
|
||||
public function testDeleteShareOwner() {
|
||||
$node = $this->getMockBuilder(File::class)->getMock();
|
||||
|
||||
$share = $this->newShare();
|
||||
$share->setSharedBy($this->currentUser)
|
||||
->setNode($node);
|
||||
|
||||
$this->shareManager
|
||||
->expects($this->once())
|
||||
->method('getShareById')
|
||||
->with('ocinternal:42')
|
||||
->willReturn($share);
|
||||
|
||||
$this->shareManager
|
||||
->expects($this->once())
|
||||
->method('deleteShare')
|
||||
->with($share);
|
||||
|
||||
$node->expects($this->once())
|
||||
->method('lock')
|
||||
->with(\OCP\Lock\ILockingProvider::LOCK_SHARED);
|
||||
|
||||
$this->assertFalse($this->invokePrivate($this->ocs, 'canDeleteFromSelf', [$share]));
|
||||
$this->assertTrue($this->invokePrivate($this->ocs, 'canDeleteShare', [$share]));
|
||||
|
||||
$this->ocs->deleteShare(42);
|
||||
}
|
||||
|
||||
/**
|
||||
* You can always delete a share when you own
|
||||
* the file path it belong to
|
||||
*/
|
||||
public function testDeleteShareFileOwner() {
|
||||
$node = $this->getMockBuilder(File::class)->getMock();
|
||||
|
||||
$share = $this->newShare();
|
||||
$share->setShareOwner($this->currentUser)
|
||||
->setNode($node);
|
||||
|
||||
$this->shareManager
|
||||
->expects($this->once())
|
||||
->method('getShareById')
|
||||
->with('ocinternal:42')
|
||||
->willReturn($share);
|
||||
|
||||
$this->shareManager
|
||||
->expects($this->once())
|
||||
->method('deleteShare')
|
||||
->with($share);
|
||||
|
||||
$node->expects($this->once())
|
||||
->method('lock')
|
||||
->with(\OCP\Lock\ILockingProvider::LOCK_SHARED);
|
||||
|
||||
$this->assertFalse($this->invokePrivate($this->ocs, 'canDeleteFromSelf', [$share]));
|
||||
$this->assertTrue($this->invokePrivate($this->ocs, 'canDeleteShare', [$share]));
|
||||
|
||||
$this->ocs->deleteShare(42);
|
||||
}
|
||||
|
||||
/**
|
||||
* You can remove (the mountpoint, not the share)
|
||||
* a share if you're in the group the share is shared with
|
||||
*/
|
||||
public function testDeleteSharedWithMyGroup() {
|
||||
$node = $this->getMockBuilder(File::class)->getMock();
|
||||
|
||||
$share = $this->newShare();
|
||||
$share->setShareType(\OCP\Share::SHARE_TYPE_GROUP)
|
||||
->setSharedWith('group')
|
||||
->setNode($node);
|
||||
|
||||
$this->shareManager
|
||||
->expects($this->once())
|
||||
->method('getShareById')
|
||||
->with('ocinternal:42')
|
||||
->willReturn($share);
|
||||
|
||||
// canDeleteShareFromSelf
|
||||
$user = $this->createMock(IUser::class);
|
||||
$group = $this->getMockBuilder('OCP\IGroup')->getMock();
|
||||
$this->groupManager
|
||||
->method('get')
|
||||
->with('group')
|
||||
->willReturn($group);
|
||||
$this->userManager
|
||||
->method('get')
|
||||
->with($this->currentUser)
|
||||
->willReturn($user);
|
||||
$group->method('inGroup')
|
||||
->with($user)
|
||||
->willReturn(true);
|
||||
|
||||
$node->expects($this->once())
|
||||
->method('lock')
|
||||
->with(\OCP\Lock\ILockingProvider::LOCK_SHARED);
|
||||
|
||||
$this->shareManager->expects($this->once())
|
||||
->method('deleteFromSelf')
|
||||
->with($share, $this->currentUser);
|
||||
|
||||
$this->shareManager->expects($this->never())
|
||||
->method('deleteShare');
|
||||
|
||||
$this->assertTrue($this->invokePrivate($this->ocs, 'canDeleteShareFromSelf', [$share]));
|
||||
$this->assertFalse($this->invokePrivate($this->ocs, 'canDeleteShare', [$share]));
|
||||
|
||||
$this->ocs->deleteShare(42);
|
||||
}
|
||||
|
||||
/**
|
||||
* You cannot remove a share if you're not
|
||||
* in the group the share is shared with
|
||||
* @expectedException \OCP\AppFramework\OCS\OCSNotFoundException
|
||||
* @expectedExceptionMessage Wrong share ID, share doesn't exist
|
||||
*/
|
||||
public function testDeleteSharedWithGroupIDontBelongTo() {
|
||||
$node = $this->getMockBuilder(File::class)->getMock();
|
||||
|
||||
$share = $this->newShare();
|
||||
$share->setShareType(\OCP\Share::SHARE_TYPE_GROUP)
|
||||
->setSharedWith('group')
|
||||
->setNode($node);
|
||||
|
||||
$this->shareManager
|
||||
->expects($this->once())
|
||||
->method('getShareById')
|
||||
->with('ocinternal:42')
|
||||
->willReturn($share);
|
||||
|
||||
// canDeleteShareFromSelf
|
||||
$user = $this->createMock(IUser::class);
|
||||
$group = $this->getMockBuilder('OCP\IGroup')->getMock();
|
||||
$this->groupManager
|
||||
->method('get')
|
||||
->with('group')
|
||||
->willReturn($group);
|
||||
$this->userManager
|
||||
->method('get')
|
||||
->with($this->currentUser)
|
||||
->willReturn($user);
|
||||
$group->method('inGroup')
|
||||
->with($user)
|
||||
->willReturn(false);
|
||||
|
||||
$node->expects($this->once())
|
||||
->method('lock')
|
||||
->with(\OCP\Lock\ILockingProvider::LOCK_SHARED);
|
||||
|
||||
$this->shareManager->expects($this->never())
|
||||
->method('deleteFromSelf');
|
||||
|
||||
$this->shareManager->expects($this->never())
|
||||
->method('deleteShare');
|
||||
|
||||
$this->assertFalse($this->invokePrivate($this->ocs, 'canDeleteShareFromSelf', [$share]));
|
||||
$this->assertFalse($this->invokePrivate($this->ocs, 'canDeleteShare', [$share]));
|
||||
|
||||
$this->ocs->deleteShare(42);
|
||||
}
|
||||
|
||||
|
|
|
|||
Loading…
Reference in a new issue