Merge pull request #23973 from owncloud/share_move_post_delete_from_g…

…roup_hook

Move post_removeFromGroup to shareManager

apps/federatedfilesharing/lib/federatedshareprovider.php

@@ -590,4 +590,15 @@ public function groupDeleted($gid) {
 		// We don't handle groups here
 		return;
 	}
+
+	/**
+	 * This provider does not handle groups
+	 *
+	 * @param string $uid
+	 * @param string $gid
+	 */
+	public function userDeletedFromGroup($uid, $gid) {
+		// We don't handle groups here
+		return;
+	}
 }

lib/base.php

@@ -776,7 +776,7 @@ public static function registerPreviewHooks() {
 	public static function registerShareHooks() {
 		if (\OC::$server->getSystemConfig()->getValue('installed')) {
 			OC_Hook::connect('OC_User', 'post_deleteUser', 'OC\Share20\Hooks', 'post_deleteUser');
-			OC_Hook::connect('OC_User', 'post_removeFromGroup', 'OC\Share\Hooks', 'post_removeFromGroup');
+			OC_Hook::connect('OC_User', 'post_removeFromGroup', 'OC\Share20\Hooks', 'post_removeFromGroup');
 			OC_Hook::connect('OC_User', 'post_deleteGroup', 'OC\Share20\Hooks', 'post_deleteGroup');
 		}
 	}

lib/private/Share20/DefaultShareProvider.php

@@ -910,4 +910,42 @@ public function groupDeleted($gid) {
 			->andWhere($qb->expr()->eq('share_with', $qb->createNamedParameter($gid)));
 		$qb->execute();
 	}
+
+	/**
+	 * Delete custom group shares to this group for this user
+	 *
+	 * @param string $uid
+	 * @param string $gid
+	 */
+	public function userDeletedFromGroup($uid, $gid) {
+		/*
+		 * Get all group shares
+		 */
+		$qb = $this->dbConn->getQueryBuilder();
+		$qb->select('id')
+			->from('share')
+			->where($qb->expr()->eq('share_type', $qb->createNamedParameter(\OCP\Share::SHARE_TYPE_GROUP)))
+			->andWhere($qb->expr()->eq('share_with', $qb->createNamedParameter($gid)));
+
+		$cursor = $qb->execute();
+		$ids = [];
+		while($row = $cursor->fetch()) {
+			$ids[] = (int)$row['id'];
+		}
+		$cursor->closeCursor();
+
+		if (!empty($ids)) {
+			$chunks = array_chunk($ids, 100);
+			foreach ($chunks as $chunk) {
+				/*
+				 * Delete all special shares wit this users for the found group shares
+				 */
+				$qb->delete('share')
+					->where($qb->expr()->eq('share_type', $qb->createNamedParameter(self::SHARE_TYPE_USERGROUP)))
+					->andWhere($qb->expr()->eq('share_with', $qb->createNamedParameter($uid)))
+					->andWhere($qb->expr()->in('parent', $qb->createNamedParameter($chunk, IQueryBuilder::PARAM_INT_ARRAY)));
+				$qb->execute();
+			}
+		}
+	}
 }

lib/private/Share20/Hooks.php

@@ -28,4 +28,8 @@ public static function post_deleteUser($arguments) {
 	public static function post_deleteGroup($arguments) {
 		\OC::$server->getShareManager()->groupDeleted($arguments['gid']);
 	}
+
+	public static function post_removeFromGroup($arguments) {
+		\OC::$server->getShareManager()->userDeletedFromGroup($arguments['uid'], $arguments['gid']);
+	}
 }

lib/private/Share20/Manager.php

@@ -1056,6 +1056,14 @@ public function groupDeleted($gid) {
 		$provider->groupDeleted($gid);
 	}
 
+	/**
+	 * @inheritdoc
+	 */
+	public function userDeletedFromGroup($uid, $gid) {
+		$provider = $this->factory->getProviderForType(\OCP\Share::SHARE_TYPE_GROUP);
+		$provider->userDeletedFromGroup($uid, $gid);
+	}
+
 	/**
 	 * Get access list to a path. This means
 	 * all the users and groups that can access a given path.

lib/public/Share/IManager.php

@@ -164,11 +164,21 @@ public function userDeleted($uid);
 	 * The group with $gid is deleted
 	 * We need to clear up all shares to this group
 	 *
-	 * @param $gid
+	 * @param string $gid
 	 * @since 9.1.0
 	 */
 	public function groupDeleted($gid);
 
+	/**
+	 * The user $uid is deleted from the group $gid
+	 * All user specific group shares have to be removed
+	 *
+	 * @param string $uid
+	 * @param string $gid
+	 * @since 9.1.0
+	 */
+	public function userDeletedFromGroup($uid, $gid);
+
 	/**
 	 * Instantiates a new share object. This is to be passed to
 	 * createShare.

lib/public/Share/IShareProvider.php

@@ -166,4 +166,15 @@ public function userDeleted($uid, $shareType);
 	 * @since 9.1.0
 	 */
 	public function groupDeleted($gid);
+
+	/**
+	 * A user is deleted from a group
+	 * We have to clean up all the related user specific group shares
+	 * Providers not handling group shares should just return
+	 *
+	 * @param string $uid
+	 * @param string $gid
+	 * @since 9.1.0
+	 */
+	public function userDeletedFromGroup($uid, $gid);
 }

tests/lib/share/share.php

@@ -611,162 +611,6 @@ protected function shareUserOneTestFileWithGroupOne() {
 		);
 	}
 
-	public function testShareWithGroup() {
-		// Invalid shares
-		$message = 'Sharing test.txt failed, because the group foobar does not exist';
-		try {
-			OCP\Share::shareItem('test', 'test.txt', OCP\Share::SHARE_TYPE_GROUP, 'foobar', \OCP\Constants::PERMISSION_READ);
-			$this->fail('Exception was expected: '.$message);
-		} catch (Exception $exception) {
-			$this->assertEquals($message, $exception->getMessage());
-		}
-		$policy = \OC::$server->getAppConfig()->getValue('core', 'shareapi_only_share_with_group_members', 'no');
-		\OC::$server->getAppConfig()->setValue('core', 'shareapi_only_share_with_group_members', 'yes');
-		$message = 'Sharing test.txt failed, because '.$this->user1.' is not a member of the group '.$this->group2;
-		try {
-			OCP\Share::shareItem('test', 'test.txt', OCP\Share::SHARE_TYPE_GROUP, $this->group2, \OCP\Constants::PERMISSION_READ);
-			$this->fail('Exception was expected: '.$message);
-		} catch (Exception $exception) {
-			$this->assertEquals($message, $exception->getMessage());
-		}
-		\OC::$server->getAppConfig()->setValue('core', 'shareapi_only_share_with_group_members', $policy);
-
-		// Valid share
-		$this->shareUserOneTestFileWithGroupOne();
-
-		// check if only the group share was created and not a single db-entry for each user
-		$statement = \OCP\DB::prepare('select `id` from `*PREFIX*share`');
-		$query = $statement->execute();
-		$result = $query->fetchAll();
-		$this->assertSame(1, count($result));
-
-
-		// Attempt to share again
-		OC_User::setUserId($this->user1);
-		$message = 'Sharing test.txt failed, because this item is already shared with '.$this->group1;
-		try {
-			OCP\Share::shareItem('test', 'test.txt', OCP\Share::SHARE_TYPE_GROUP, $this->group1, \OCP\Constants::PERMISSION_READ);
-			$this->fail('Exception was expected: '.$message);
-		} catch (Exception $exception) {
-			$this->assertEquals($message, $exception->getMessage());
-		}
-
-		// Attempt to share back to owner of group share
-		OC_User::setUserId($this->user2);
-		$message = 'Sharing failed, because the user '.$this->user1.' is the original sharer';
-		try {
-			OCP\Share::shareItem('test', 'test.txt', OCP\Share::SHARE_TYPE_USER, $this->user1, \OCP\Constants::PERMISSION_READ);
-			$this->fail('Exception was expected: '.$message);
-		} catch (Exception $exception) {
-			$this->assertEquals($message, $exception->getMessage());
-		}
-
-		// Attempt to share back to group
-		$message = 'Sharing test.txt failed, because this item is already shared with '.$this->group1;
-		try {
-			OCP\Share::shareItem('test', 'test.txt', OCP\Share::SHARE_TYPE_GROUP, $this->group1, \OCP\Constants::PERMISSION_READ);
-			$this->fail('Exception was expected: '.$message);
-		} catch (Exception $exception) {
-			$this->assertEquals($message, $exception->getMessage());
-		}
-
-		// Attempt to share back to member of group
-		$message ='Sharing test.txt failed, because this item is already shared with '.$this->user3;
-		try {
-			OCP\Share::shareItem('test', 'test.txt', OCP\Share::SHARE_TYPE_USER, $this->user3, \OCP\Constants::PERMISSION_READ);
-			$this->fail('Exception was expected: '.$message);
-		} catch (Exception $exception) {
-			$this->assertEquals($message, $exception->getMessage());
-		}
-
-		// Unshare
-		OC_User::setUserId($this->user1);
-		$this->assertTrue(OCP\Share::unshare('test', 'test.txt', OCP\Share::SHARE_TYPE_GROUP, $this->group1));
-
-		// Valid share with same person - user then group
-		$this->assertTrue(OCP\Share::shareItem('test', 'test.txt', OCP\Share::SHARE_TYPE_USER, $this->user2, \OCP\Constants::PERMISSION_READ | \OCP\Constants::PERMISSION_DELETE | \OCP\Constants::PERMISSION_SHARE));
-		$this->assertTrue(OCP\Share::shareItem('test', 'test.txt', OCP\Share::SHARE_TYPE_GROUP, $this->group1, \OCP\Constants::PERMISSION_READ | \OCP\Constants::PERMISSION_UPDATE));
-		OC_User::setUserId($this->user2);
-		$this->assertEquals(array('test.txt'), OCP\Share::getItemsSharedWith('test', Test_Share_Backend::FORMAT_TARGET));
-		$this->assertEquals(array(\OCP\Constants::PERMISSION_READ | \OCP\Constants::PERMISSION_UPDATE | \OCP\Constants::PERMISSION_DELETE | \OCP\Constants::PERMISSION_SHARE), OCP\Share::getItemSharedWith('test', 'test.txt', Test_Share_Backend::FORMAT_PERMISSIONS));
-		OC_User::setUserId($this->user3);
-		$this->assertEquals(array('test.txt'), OCP\Share::getItemsSharedWith('test', Test_Share_Backend::FORMAT_TARGET));
-		$this->assertEquals(array(\OCP\Constants::PERMISSION_READ | \OCP\Constants::PERMISSION_UPDATE), OCP\Share::getItemSharedWith('test', 'test.txt', Test_Share_Backend::FORMAT_PERMISSIONS));
-
-		// Valid reshare
-		OC_User::setUserId($this->user2);
-		$this->assertTrue(OCP\Share::shareItem('test', 'test.txt', OCP\Share::SHARE_TYPE_USER, $this->user4, \OCP\Constants::PERMISSION_READ));
-		OC_User::setUserId($this->user4);
-		$this->assertEquals(array('test.txt'), OCP\Share::getItemsSharedWith('test', Test_Share_Backend::FORMAT_TARGET));
-
-		// Unshare from user only
-		OC_User::setUserId($this->user1);
-		$this->assertTrue(OCP\Share::unshare('test', 'test.txt', OCP\Share::SHARE_TYPE_USER, $this->user2));
-		OC_User::setUserId($this->user2);
-		$this->assertEquals(array(\OCP\Constants::PERMISSION_READ | \OCP\Constants::PERMISSION_UPDATE), OCP\Share::getItemSharedWith('test', 'test.txt', Test_Share_Backend::FORMAT_PERMISSIONS));
-		OC_User::setUserId($this->user4);
-		$this->assertEquals(array('test.txt'), OCP\Share::getItemsSharedWith('test', Test_Share_Backend::FORMAT_TARGET));
-
-		// Valid share with same person - group then user
-		OC_User::setUserId($this->user1);
-		$this->assertTrue(OCP\Share::shareItem('test', 'test.txt', OCP\Share::SHARE_TYPE_USER, $this->user2, \OCP\Constants::PERMISSION_READ | \OCP\Constants::PERMISSION_DELETE));
-		OC_User::setUserId($this->user2);
-		$this->assertEquals(array('test.txt'), OCP\Share::getItemsSharedWith('test', Test_Share_Backend::FORMAT_TARGET));
-		$this->assertEquals(array(\OCP\Constants::PERMISSION_READ | \OCP\Constants::PERMISSION_UPDATE | \OCP\Constants::PERMISSION_DELETE), OCP\Share::getItemSharedWith('test', 'test.txt', Test_Share_Backend::FORMAT_PERMISSIONS));
-
-		// Unshare from group only
-		OC_User::setUserId($this->user1);
-		$this->assertTrue(OCP\Share::unshare('test', 'test.txt', OCP\Share::SHARE_TYPE_GROUP, $this->group1));
-		OC_User::setUserId($this->user2);
-		$this->assertEquals(array(\OCP\Constants::PERMISSION_READ | \OCP\Constants::PERMISSION_DELETE), OCP\Share::getItemSharedWith('test', 'test.txt', Test_Share_Backend::FORMAT_PERMISSIONS));
-
-		// Attempt user specific target conflict
-		OC_User::setUserId($this->user3);
-		\OCP\Util::connectHook('OCP\\Share', 'post_shared', 'DummyHookListener', 'listen');
-
-		$this->assertTrue(OCP\Share::shareItem('test', 'share.txt', OCP\Share::SHARE_TYPE_GROUP, $this->group1, \OCP\Constants::PERMISSION_READ | \OCP\Constants::PERMISSION_SHARE));
-		$this->assertEquals(OCP\Share::SHARE_TYPE_GROUP, DummyHookListener::$shareType);
-		OC_User::setUserId($this->user2);
-		$to_test = OCP\Share::getItemsSharedWith('test', Test_Share_Backend::FORMAT_TARGET);
-		$this->assertEquals(2, count($to_test));
-		$this->assertTrue(in_array('test.txt', $to_test));
-		$this->assertTrue(in_array('test1.txt', $to_test));
-
-		// Valid reshare
-		$this->assertTrue(OCP\Share::shareItem('test', 'share.txt', OCP\Share::SHARE_TYPE_USER, $this->user4, \OCP\Constants::PERMISSION_READ | \OCP\Constants::PERMISSION_SHARE));
-		OC_User::setUserId($this->user4);
-		$this->assertEquals(array('test1.txt'), OCP\Share::getItemsSharedWith('test', Test_Share_Backend::FORMAT_TARGET));
-
-		// Remove user from group
-		OC_Group::removeFromGroup($this->user2, $this->group1);
-		OC_User::setUserId($this->user2);
-		$this->assertEquals(array('test.txt'), OCP\Share::getItemsSharedWith('test', Test_Share_Backend::FORMAT_TARGET));
-		OC_User::setUserId($this->user4);
-		$this->assertEquals(array(), OCP\Share::getItemsSharedWith('test', Test_Share_Backend::FORMAT_TARGET));
-
-		// Add user to group
-		OC_Group::addToGroup($this->user4, $this->group1);
-		$this->assertEquals(array('test.txt'), OCP\Share::getItemsSharedWith('test', Test_Share_Backend::FORMAT_TARGET));
-
-		// Unshare from self
-		$this->assertTrue(OCP\Share::unshareFromSelf('test', 'test.txt'));
-		$this->assertEquals(array(), OCP\Share::getItemsSharedWith('test', Test_Share_Backend::FORMAT_TARGET));
-		OC_User::setUserId($this->user2);
-		$this->assertEquals(array('test.txt'), OCP\Share::getItemsSharedWith('test', Test_Share_Backend::FORMAT_TARGET));
-
-		// Unshare from self via source
-		OC_User::setUserId($this->user1);
-		$this->assertTrue(OCP\Share::unshareFromSelf('test', 'share.txt', true));
-		$this->assertEquals(array(), OCP\Share::getItemsSharedWith('test', Test_Share_Backend::FORMAT_TARGET));
-
-		// Remove group
-		OC_Group::deleteGroup($this->group1);
-		OC_User::setUserId($this->user4);
-		$this->assertEquals(array(), OCP\Share::getItemsSharedWith('test', Test_Share_Backend::FORMAT_TARGET));
-		OC_User::setUserId($this->user3);
-		$this->assertEquals(array(), OCP\Share::getItemsShared('test'));
-	}
-
 	/**
 	 * Test that unsharing from group will also delete all
 	 * child entries