From 9afc6ac985a2aeccb187544e13c4e1788953b9ea Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?John=20Molakvo=C3=A6=20=28skjnldsv=29?= Date: Mon, 26 Aug 2019 13:11:09 +0200 Subject: [PATCH] Fix EDIT permissions MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: John Molakvoæ (skjnldsv) --- .../lib/Controller/ShareAPIController.php | 107 ++++++++++++++---- 1 file changed, 82 insertions(+), 25 deletions(-) diff --git a/apps/files_sharing/lib/Controller/ShareAPIController.php b/apps/files_sharing/lib/Controller/ShareAPIController.php index 2dc852edd1bb1..1ad7f9274f26d 100644 --- a/apps/files_sharing/lib/Controller/ShareAPIController.php +++ b/apps/files_sharing/lib/Controller/ShareAPIController.php @@ -335,17 +335,23 @@ public function deleteShare(string $id): DataResponse { try { $this->lock($share->getNode()); } catch (LockedException $e) { - throw new OCSNotFoundException($this->l->t('could not delete share')); + 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 (!$this->canDeleteShare($share)) { + throw new OCSForbiddenException($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 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 (( + $share->getShareType() === Share::SHARE_TYPE_GROUP + || $share->getShareType() === Share::SHARE_TYPE_ROOM + ) + && $share->getShareOwner() !== $this->currentUser + && $share->getSharedBy() !== $this->currentUser) { $this->shareManager->deleteFromSelf($share, $this->currentUser); } else { $this->shareManager->deleteShare($share); @@ -824,12 +830,8 @@ public function updateShare( $this->lock($share->getNode()); - if (!$this->canAccessShare($share, false)) { - throw new OCSNotFoundException($this->l->t('Wrong share ID, share doesn\'t exist')); - } - - if ($share->getShareOwner() !== $this->currentUser && $share->getSharedBy() !== $this->currentUser) { - throw new OCSForbiddenException('You are not allowed to edit incoming shares'); + if (!$this->canEditShare($share)) { + throw new OCSForbiddenException($this->l->t('Wrong share ID, share doesn\'t exist')); } if ($permissions === null && @@ -989,28 +991,24 @@ public function updateShare( * Does the user have read permission on the share * * @param \OCP\Share\IShare $share the share to check - * @param boolean $checkGroups check groups as well? * @return boolean * * @suppress PhanUndeclaredClassMethod */ - protected function canAccessShare(\OCP\Share\IShare $share, bool $checkGroups = true): bool { + protected function canAccessShare(\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; } - // Owner of the file and the sharer of the file can always get share - if ($share->getShareOwner() === $this->currentUser || - $share->getSharedBy() === $this->currentUser - ) { + // If the user can edit the share, the user can read it! + if ($this->canEditShare($share)) { return true; } - - // If the share is shared with you (or a group you are a member of) - if ($share->getShareType() === Share::SHARE_TYPE_USER && - $share->getSharedWith() === $this->currentUser - ) { + + // If the share is shared with you, you can access it! + if ($share->getShareType() === Share::SHARE_TYPE_USER + && $share->getSharedWith() === $this->currentUser) { return true; } @@ -1019,7 +1017,8 @@ protected function canAccessShare(\OCP\Share\IShare $share, bool $checkGroups = return true; } - if ($checkGroups && $share->getShareType() === Share::SHARE_TYPE_GROUP) { + // If in the recipient group, you can see the share + 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)) { @@ -1043,6 +1042,64 @@ protected function canAccessShare(\OCP\Share\IShare $share, bool $checkGroups = return false; } + /** + * Does the user have edit permission on the share + * + * @param \OCP\Share\IShare $share the share to check + * @return boolean + * + * @suppress PhanUndeclaredClassMethod + */ + protected function canEditShare(\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; + } + + // The owner of the file and the creator of the share + // can always edit the share + if ($share->getShareOwner() === $this->currentUser || + $share->getSharedBy() === $this->currentUser + ) { + return true; + } + + // we do NOT support some kind of admin + // in groups. You cannot edit shares to a group + // you're in if you're not the owner! + + return false; + } + + /** + * Does the user have delete permission on the share + * + * @param \OCP\Share\IShare $share the share to check + * @return boolean + * + * @suppress PhanUndeclaredClassMethod + */ + 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; + } + + // if EDIT rights to a share, you can delete it + if ($this->canEditShare($share)) { + return true; + } + + return false; + } + /** * Make sure that the passed date is valid ISO 8601 * So YYYY-MM-DD