Fix EDIT permissions

Signed-off-by: John Molakvoæ (skjnldsv) <skjnldsv@protonmail.com>
nextcloud · Aug 27, 2019 · 9afc6ac · 9afc6ac
1 parent 2194a02
commit 9afc6ac
Showing 1 changed file with 82 additions and 25 deletions.
diff --git 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