From 0817024e6f817a04c1258dba971f412550f6a058 Mon Sep 17 00:00:00 2001 From: Robin Appelman Date: Thu, 1 Oct 2015 15:37:26 +0200 Subject: [PATCH 1/4] Mark shares as dirty when changing permissions --- .../files_sharing/lib/propagation/changewatcher.php | 8 ++++++++ .../lib/propagation/propagationmanager.php | 8 +++++--- apps/files_sharing/tests/etagpropagation.php | 13 +++++++++++++ lib/private/share/share.php | 10 +++++++++- 4 files changed, 35 insertions(+), 4 deletions(-) diff --git a/apps/files_sharing/lib/propagation/changewatcher.php b/apps/files_sharing/lib/propagation/changewatcher.php index 9da3ffe51a2..f9b28c7f967 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, time()); + } + } } diff --git a/apps/files_sharing/lib/propagation/propagationmanager.php b/apps/files_sharing/lib/propagation/propagationmanager.php index d220551f8d6..7fcf467de9a 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; /** @@ -119,8 +120,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/tests/etagpropagation.php b/apps/files_sharing/tests/etagpropagation.php index 6d23959d66d..427e371cd06 100644 --- a/apps/files_sharing/tests/etagpropagation.php +++ b/apps/files_sharing/tests/etagpropagation.php @@ -406,4 +406,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 97c36a03c86..7f9b899bf63 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'; From 41f9946272e4c41e96bd0918ff1b2adfd113b92a Mon Sep 17 00:00:00 2001 From: Robin Appelman Date: Thu, 1 Oct 2015 16:20:48 +0200 Subject: [PATCH 2/4] clear hooks for tests --- apps/files_sharing/tests/etagpropagation.php | 1 + 1 file changed, 1 insertion(+) diff --git a/apps/files_sharing/tests/etagpropagation.php b/apps/files_sharing/tests/etagpropagation.php index 427e371cd06..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(); } From 47c71d8e8771cab6d5442e790f3edb94213b586e Mon Sep 17 00:00:00 2001 From: Robin Appelman Date: Tue, 6 Oct 2015 13:00:17 +0200 Subject: [PATCH 3/4] use microtime --- apps/files_sharing/lib/propagation/changewatcher.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/apps/files_sharing/lib/propagation/changewatcher.php b/apps/files_sharing/lib/propagation/changewatcher.php index f9b28c7f967..e61c161da19 100644 --- a/apps/files_sharing/lib/propagation/changewatcher.php +++ b/apps/files_sharing/lib/propagation/changewatcher.php @@ -104,7 +104,7 @@ class ChangeWatcher { $share = $params['share']; if ($share['item_type'] === 'file' || $share['item_type'] === 'folder') { - $this->recipientPropagator->markDirty($share, time()); + $this->recipientPropagator->markDirty($share, microtime(true)); } } } From 17756ebf5bb7c2bdf9e949f3fc517c39ed284600 Mon Sep 17 00:00:00 2001 From: Robin Appelman Date: Tue, 6 Oct 2015 16:37:43 +0200 Subject: [PATCH 4/4] fix phpdoc --- apps/files_sharing/lib/propagation/recipientpropagator.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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) {