Merge pull request #19511 from owncloud/etag-share-permissions-update

Mark shares as dirty when changing permissions

5 files changed, 37 insertions, 5 deletions

diff --git a/apps/files_sharing/lib/propagation/changewatcher.php b/apps/files_sharing/lib/propagation/changewatcher.php
index 9da3ffe51a2..e61c161da19 100644
--- a/apps/files_sharing/lib/propagation/changewatcher.php
+++ b/apps/files_sharing/lib/propagation/changewatcher.php
@@ -99,4 +99,12 @@ class ChangeWatcher {
 			$propagator->propagateChanges();
 		}
 	}
+
+	public function permissionsHook($params) {
+		$share = $params['share'];
+
+		if ($share['item_type'] === 'file' || $share['item_type'] === 'folder') {
+			$this->recipientPropagator->markDirty($share, microtime(true));
+		}
+	}
 }
diff --git a/apps/files_sharing/lib/propagation/propagationmanager.php b/apps/files_sharing/lib/propagation/propagationmanager.php
index 35048f89cfb..6ed70e93f84 100644
--- a/apps/files_sharing/lib/propagation/propagationmanager.php
+++ b/apps/files_sharing/lib/propagation/propagationmanager.php
@@ -25,6 +25,7 @@ use OC\Files\Filesystem;
 use OC\Files\View;
 use OCP\IConfig;
 use OCP\IUserSession;
+use OCP\Util;
 
 
 /**
@@ -134,8 +135,9 @@ class PropagationManager {
 
 		// for marking shares owned by the active user as dirty when a file inside them changes
 		$this->listenToOwnerChanges($user->getUID(), $user->getUID());
-		\OC_Hook::connect('OC_Filesystem', 'post_write', $watcher, 'writeHook');
-		\OC_Hook::connect('OC_Filesystem', 'post_delete', $watcher, 'writeHook');
-		\OC_Hook::connect('OC_Filesystem', 'post_rename', $watcher, 'renameHook');
+		Util::connectHook('OC_Filesystem', 'post_write', $watcher, 'writeHook');
+		Util::connectHook('OC_Filesystem', 'post_delete', $watcher, 'writeHook');
+		Util::connectHook('OC_Filesystem', 'post_rename', $watcher, 'renameHook');
+		Util::connectHook('OCP\Share', 'post_update_permissions', $watcher, 'permissionsHook');
 	}
 }
diff --git a/apps/files_sharing/lib/propagation/recipientpropagator.php b/apps/files_sharing/lib/propagation/recipientpropagator.php
index 420cacb3d2f..8e064e2ee54 100644
--- a/apps/files_sharing/lib/propagation/recipientpropagator.php
+++ b/apps/files_sharing/lib/propagation/recipientpropagator.php
@@ -105,7 +105,7 @@ class RecipientPropagator {
 
 	/**
 	 * @param array $share
-	 * @param int $time
+	 * @param float $time
 	 */
 	public function markDirty($share, $time = null) {
 		if ($time === null) {
diff --git a/apps/files_sharing/tests/etagpropagation.php b/apps/files_sharing/tests/etagpropagation.php
index 6d23959d66d..f7f8ec35288 100644
--- a/apps/files_sharing/tests/etagpropagation.php
+++ b/apps/files_sharing/tests/etagpropagation.php
@@ -47,6 +47,7 @@ class EtagPropagation extends TestCase {
 		\OC_Hook::clear('OC_Filesystem', 'post_write');
 		\OC_Hook::clear('OC_Filesystem', 'post_delete');
 		\OC_Hook::clear('OC_Filesystem', 'post_rename');
+		\OC_Hook::clear('OCP\Share', 'post_update_permissions');
 		parent::tearDown();
 	}
 
@@ -406,4 +407,17 @@ class EtagPropagation extends TestCase {
 
 		$this->assertAllUnchaged();
 	}
+
+	public function testEtagChangeOnPermissionsChange() {
+		$this->loginAsUser(self::TEST_FILES_SHARING_API_USER1);
+
+		$view = new View('/' . self::TEST_FILES_SHARING_API_USER1 . '/files');
+		$folderInfo = $view->getFileInfo('/sub1/sub2/folder');
+
+		\OCP\Share::setPermissions('folder', $folderInfo->getId(), \OCP\Share::SHARE_TYPE_USER, self::TEST_FILES_SHARING_API_USER2, 17);
+
+		$this->assertEtagsForFoldersChanged([self::TEST_FILES_SHARING_API_USER2, self::TEST_FILES_SHARING_API_USER4]);
+
+		$this->assertAllUnchaged();
+	}
 }
diff --git a/lib/private/share/share.php b/lib/private/share/share.php
index 49eeb3829c4..932586b5c27 100644
--- a/lib/private/share/share.php
+++ b/lib/private/share/share.php
@@ -1099,6 +1099,7 @@ class Share extends Constants {
 					'uidOwner' => \OC_User::getUser(),
 					'permissions' => $permissions,
 					'path' => $item['path'],
+					'share' => $item
 				));
 			}
 			// Check if permissions were removed
@@ -1109,16 +1110,18 @@ class Share extends Constants {
 					Helper::delete($item['id'], true, null, null, true);
 				} else {
 					$ids = array();
+					$items = [];
 					$parents = array($item['id']);
 					while (!empty($parents)) {
 						$parents = "'".implode("','", $parents)."'";
-						$query = \OC_DB::prepare('SELECT `id`, `permissions` FROM `*PREFIX*share`'
+						$query = \OC_DB::prepare('SELECT `id`, `permissions`, `item_type` FROM `*PREFIX*share`'
 							.' WHERE `parent` IN ('.$parents.')');
 						$result = $query->execute();
 						// Reset parents array, only go through loop again if
 						// items are found that need permissions removed
 						$parents = array();
 						while ($item = $result->fetchRow()) {
+							$items[] = $item;
 							// Check if permissions need to be removed
 							if ($item['permissions'] & ~$permissions) {
 								// Add to list of items that need permissions removed
@@ -1140,8 +1143,13 @@ class Share extends Constants {
 							.' WHERE `id` IN ('.$ids.')');
 						$query->execute(array($permissions));
 					}
+
+					foreach ($items as $item) {
+						\OC_Hook::emit('OCP\Share', 'post_update_permissions', ['share' => $item]);
+					}
 				}
 			}
+
 			return true;
 		}
 		$message = 'Setting permissions for %s failed, because the item was not found';