Merge pull request #15596 from owncloud/issue/15589

Correctly generate the feedback URL for remote share

2 files changed, 31 insertions, 9 deletions

diff --git a/apps/files_sharing/lib/external/manager.php b/apps/files_sharing/lib/external/manager.php
index 7b8b756dd5e..0246e2d9c80 100644
--- a/apps/files_sharing/lib/external/manager.php
+++ b/apps/files_sharing/lib/external/manager.php
@@ -174,7 +174,7 @@ class Manager {
 	 */
 	private function getShare($id) {
 		$getShare = $this->connection->prepare('
-			SELECT `remote`, `share_token`, `name`
+			SELECT `remote`, `remote_id`, `share_token`, `name`
 			FROM  `*PREFIX*share_external`
 			WHERE `id` = ? AND `user` = ?');
 		$result = $getShare->execute(array($id, $this->uid));
@@ -203,7 +203,7 @@ class Manager {
 					`mountpoint_hash` = ?
 				WHERE `id` = ? AND `user` = ?');
 			$acceptShare->execute(array(1, $mountPoint, $hash, $id, $this->uid));
-			$this->sendFeedbackToRemote($share['remote'], $share['share_token'], $id, 'accept');
+			$this->sendFeedbackToRemote($share['remote'], $share['share_token'], $share['remote_id'], 'accept');
 		}
 	}
 
@@ -220,7 +220,7 @@ class Manager {
 			$removeShare = $this->connection->prepare('
 				DELETE FROM `*PREFIX*share_external` WHERE `id` = ? AND `user` = ?');
 			$removeShare->execute(array($id, $this->uid));
-			$this->sendFeedbackToRemote($share['remote'], $share['share_token'], $id, 'decline');
+			$this->sendFeedbackToRemote($share['remote'], $share['share_token'], $share['remote_id'], 'decline');
 		}
 	}
 
@@ -229,13 +229,13 @@ class Manager {
 	 *
 	 * @param string $remote
 	 * @param string $token
-	 * @param int $id
+	 * @param int $remoteId Share id on the remote host
 	 * @param string $feedback
 	 * @return boolean
 	 */
-	private function sendFeedbackToRemote($remote, $token, $id, $feedback) {
+	private function sendFeedbackToRemote($remote, $token, $remoteId, $feedback) {
 
-		$url = $remote . \OCP\Share::BASE_PATH_TO_SHARE_API . '/' . $id . '/' . $feedback . '?format=' . \OCP\Share::RESPONSE_FORMAT;
+		$url = rtrim($remote, '/') . \OCP\Share::BASE_PATH_TO_SHARE_API . '/' . $remoteId . '/' . $feedback . '?format=' . \OCP\Share::RESPONSE_FORMAT;
 		$fields = array('token' => $token);
 
 		$result = $this->httpHelper->post($url, $fields);
diff --git a/apps/files_sharing/tests/external/managertest.php b/apps/files_sharing/tests/external/managertest.php
index 3fd1f5444b2..f7b216530d5 100644
--- a/apps/files_sharing/tests/external/managertest.php
+++ b/apps/files_sharing/tests/external/managertest.php
@@ -22,16 +22,20 @@
 namespace OCA\Files_Sharing\Tests\External;
 
 use OC\Files\Storage\StorageFactory;
+use OCA\Files_Sharing\External\Manager;
 use OCA\Files_Sharing\Tests\TestCase;
 
 class ManagerTest extends TestCase {
 
-	/** @var \OCA\Files_Sharing\External\Manager **/
+	/** @var Manager **/
 	private $manager;
 
 	/** @var \OC\Files\Mount\Manager */
 	private $mountManager;
 
+	/** @var \PHPUnit_Framework_MockObject_MockObject */
+	private $httpHelper;
+
 	private $uid;
 
 	protected function setUp() {
@@ -39,16 +43,19 @@ class ManagerTest extends TestCase {
 
 		$this->uid = $this->getUniqueID('user');
 		$this->mountManager = new \OC\Files\Mount\Manager();
-		$this->manager = new \OCA\Files_Sharing\External\Manager(
+		$this->httpHelper = $httpHelper = $this->getMockBuilder('\OC\HTTPHelper')->disableOriginalConstructor()->getMock();
+		/** @var \OC\HTTPHelper $httpHelper */
+		$this->manager = new Manager(
 			\OC::$server->getDatabaseConnection(),
 			$this->mountManager,
 			new StorageFactory(),
-			$this->getMockBuilder('\OC\HTTPHelper')->disableOriginalConstructor()->getMock(),
+			$httpHelper,
 			$this->uid
 		);
 	}
 
 	public function testAddShare() {
+
 		$shareData1 = [
 			'remote' => 'http://localhost',
 			'token' => 'token1',
@@ -86,6 +93,10 @@ class ManagerTest extends TestCase {
 		$this->assertNotMount('{{TemporaryMountPointName#' . $shareData1['name'] . '}}');
 		$this->assertNotMount('{{TemporaryMountPointName#' . $shareData1['name'] . '}}-1');
 
+		$this->httpHelper->expects($this->at(0))
+			->method('post')
+			->with($this->stringStartsWith('http://localhost/ocs/v1.php/cloud/shares/' . $openShares[0]['remote_id']), $this->anything());
+
 		// Accept the first share
 		$this->manager->acceptShare($openShares[0]['id']);
 
@@ -117,6 +128,10 @@ class ManagerTest extends TestCase {
 		$this->assertNotMount('{{TemporaryMountPointName#' . $shareData1['name'] . '}}');
 		$this->assertNotMount('{{TemporaryMountPointName#' . $shareData1['name'] . '}}-1');
 
+		$this->httpHelper->expects($this->at(0))
+			->method('post')
+			->with($this->stringStartsWith('http://localhost/ocs/v1.php/cloud/shares/' . $openShares[1]['remote_id'] . '/decline'), $this->anything());
+
 		// Decline the third share
 		$this->manager->declineShare($openShares[1]['id']);
 
@@ -140,6 +155,13 @@ class ManagerTest extends TestCase {
 		$this->assertNotMount('{{TemporaryMountPointName#' . $shareData1['name'] . '}}');
 		$this->assertNotMount('{{TemporaryMountPointName#' . $shareData1['name'] . '}}-1');
 
+		$this->httpHelper->expects($this->at(0))
+			->method('post')
+			->with($this->stringStartsWith('http://localhost/ocs/v1.php/cloud/shares/' . $openShares[0]['remote_id'] . '/decline'), $this->anything());
+		$this->httpHelper->expects($this->at(1))
+			->method('post')
+			->with($this->stringStartsWith('http://localhost/ocs/v1.php/cloud/shares/' . $acceptedShares[0]['remote_id'] . '/decline'), $this->anything());
+
 		$this->manager->removeUserShares($this->uid);
 		$this->assertEmpty(\Test_Helper::invokePrivate($this->manager, 'getShares', [null]), 'Asserting all shares for the user have been deleted');