Merge pull request #15596 from owncloud/issue/15589
Correctly generate the feedback URL for remote share
This commit is contained in:
commit
8c7db2536d
|
@ -174,7 +174,7 @@ class Manager {
|
||||||
*/
|
*/
|
||||||
private function getShare($id) {
|
private function getShare($id) {
|
||||||
$getShare = $this->connection->prepare('
|
$getShare = $this->connection->prepare('
|
||||||
SELECT `remote`, `share_token`, `name`
|
SELECT `remote`, `remote_id`, `share_token`, `name`
|
||||||
FROM `*PREFIX*share_external`
|
FROM `*PREFIX*share_external`
|
||||||
WHERE `id` = ? AND `user` = ?');
|
WHERE `id` = ? AND `user` = ?');
|
||||||
$result = $getShare->execute(array($id, $this->uid));
|
$result = $getShare->execute(array($id, $this->uid));
|
||||||
|
@ -203,7 +203,7 @@ class Manager {
|
||||||
`mountpoint_hash` = ?
|
`mountpoint_hash` = ?
|
||||||
WHERE `id` = ? AND `user` = ?');
|
WHERE `id` = ? AND `user` = ?');
|
||||||
$acceptShare->execute(array(1, $mountPoint, $hash, $id, $this->uid));
|
$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('
|
$removeShare = $this->connection->prepare('
|
||||||
DELETE FROM `*PREFIX*share_external` WHERE `id` = ? AND `user` = ?');
|
DELETE FROM `*PREFIX*share_external` WHERE `id` = ? AND `user` = ?');
|
||||||
$removeShare->execute(array($id, $this->uid));
|
$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 $remote
|
||||||
* @param string $token
|
* @param string $token
|
||||||
* @param int $id
|
* @param int $remoteId Share id on the remote host
|
||||||
* @param string $feedback
|
* @param string $feedback
|
||||||
* @return boolean
|
* @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);
|
$fields = array('token' => $token);
|
||||||
|
|
||||||
$result = $this->httpHelper->post($url, $fields);
|
$result = $this->httpHelper->post($url, $fields);
|
||||||
|
|
|
@ -22,16 +22,20 @@
|
||||||
namespace OCA\Files_Sharing\Tests\External;
|
namespace OCA\Files_Sharing\Tests\External;
|
||||||
|
|
||||||
use OC\Files\Storage\StorageFactory;
|
use OC\Files\Storage\StorageFactory;
|
||||||
|
use OCA\Files_Sharing\External\Manager;
|
||||||
use OCA\Files_Sharing\Tests\TestCase;
|
use OCA\Files_Sharing\Tests\TestCase;
|
||||||
|
|
||||||
class ManagerTest extends TestCase {
|
class ManagerTest extends TestCase {
|
||||||
|
|
||||||
/** @var \OCA\Files_Sharing\External\Manager **/
|
/** @var Manager **/
|
||||||
private $manager;
|
private $manager;
|
||||||
|
|
||||||
/** @var \OC\Files\Mount\Manager */
|
/** @var \OC\Files\Mount\Manager */
|
||||||
private $mountManager;
|
private $mountManager;
|
||||||
|
|
||||||
|
/** @var \PHPUnit_Framework_MockObject_MockObject */
|
||||||
|
private $httpHelper;
|
||||||
|
|
||||||
private $uid;
|
private $uid;
|
||||||
|
|
||||||
protected function setUp() {
|
protected function setUp() {
|
||||||
|
@ -39,16 +43,19 @@ class ManagerTest extends TestCase {
|
||||||
|
|
||||||
$this->uid = $this->getUniqueID('user');
|
$this->uid = $this->getUniqueID('user');
|
||||||
$this->mountManager = new \OC\Files\Mount\Manager();
|
$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(),
|
\OC::$server->getDatabaseConnection(),
|
||||||
$this->mountManager,
|
$this->mountManager,
|
||||||
new StorageFactory(),
|
new StorageFactory(),
|
||||||
$this->getMockBuilder('\OC\HTTPHelper')->disableOriginalConstructor()->getMock(),
|
$httpHelper,
|
||||||
$this->uid
|
$this->uid
|
||||||
);
|
);
|
||||||
}
|
}
|
||||||
|
|
||||||
public function testAddShare() {
|
public function testAddShare() {
|
||||||
|
|
||||||
$shareData1 = [
|
$shareData1 = [
|
||||||
'remote' => 'http://localhost',
|
'remote' => 'http://localhost',
|
||||||
'token' => 'token1',
|
'token' => 'token1',
|
||||||
|
@ -86,6 +93,10 @@ class ManagerTest extends TestCase {
|
||||||
$this->assertNotMount('{{TemporaryMountPointName#' . $shareData1['name'] . '}}');
|
$this->assertNotMount('{{TemporaryMountPointName#' . $shareData1['name'] . '}}');
|
||||||
$this->assertNotMount('{{TemporaryMountPointName#' . $shareData1['name'] . '}}-1');
|
$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
|
// Accept the first share
|
||||||
$this->manager->acceptShare($openShares[0]['id']);
|
$this->manager->acceptShare($openShares[0]['id']);
|
||||||
|
|
||||||
|
@ -117,6 +128,10 @@ class ManagerTest extends TestCase {
|
||||||
$this->assertNotMount('{{TemporaryMountPointName#' . $shareData1['name'] . '}}');
|
$this->assertNotMount('{{TemporaryMountPointName#' . $shareData1['name'] . '}}');
|
||||||
$this->assertNotMount('{{TemporaryMountPointName#' . $shareData1['name'] . '}}-1');
|
$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
|
// Decline the third share
|
||||||
$this->manager->declineShare($openShares[1]['id']);
|
$this->manager->declineShare($openShares[1]['id']);
|
||||||
|
|
||||||
|
@ -140,6 +155,13 @@ class ManagerTest extends TestCase {
|
||||||
$this->assertNotMount('{{TemporaryMountPointName#' . $shareData1['name'] . '}}');
|
$this->assertNotMount('{{TemporaryMountPointName#' . $shareData1['name'] . '}}');
|
||||||
$this->assertNotMount('{{TemporaryMountPointName#' . $shareData1['name'] . '}}-1');
|
$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->manager->removeUserShares($this->uid);
|
||||||
$this->assertEmpty(\Test_Helper::invokePrivate($this->manager, 'getShares', [null]), 'Asserting all shares for the user have been deleted');
|
$this->assertEmpty(\Test_Helper::invokePrivate($this->manager, 'getShares', [null]), 'Asserting all shares for the user have been deleted');
|
||||||
|
|
||||||
|
|
|
@ -2395,15 +2395,17 @@ class Share extends Constants {
|
||||||
*
|
*
|
||||||
* @param string $url
|
* @param string $url
|
||||||
* @param array $fields post parameters
|
* @param array $fields post parameters
|
||||||
* @return bool
|
* @return array
|
||||||
*/
|
*/
|
||||||
private static function tryHttpPost($url, $fields) {
|
private static function tryHttpPost($url, $fields) {
|
||||||
$protocol = 'https://';
|
$protocol = 'https://';
|
||||||
$success = false;
|
$result = [
|
||||||
|
'success' => false,
|
||||||
|
'result' => '',
|
||||||
|
];
|
||||||
$try = 0;
|
$try = 0;
|
||||||
while ($success === false && $try < 2) {
|
while ($result['success'] === false && $try < 2) {
|
||||||
$result = \OC::$server->getHTTPHelper()->post($protocol . $url, $fields);
|
$result = \OC::$server->getHTTPHelper()->post($protocol . $url, $fields);
|
||||||
$success = $result['success'];
|
|
||||||
$try++;
|
$try++;
|
||||||
$protocol = 'http://';
|
$protocol = 'http://';
|
||||||
}
|
}
|
||||||
|
@ -2426,7 +2428,7 @@ class Share extends Constants {
|
||||||
list($user, $remote) = explode('@', $shareWith, 2);
|
list($user, $remote) = explode('@', $shareWith, 2);
|
||||||
|
|
||||||
if ($user && $remote) {
|
if ($user && $remote) {
|
||||||
$url = $remote . self::BASE_PATH_TO_SHARE_API . '?format=' . self::RESPONSE_FORMAT;
|
$url = rtrim($remote, '/') . self::BASE_PATH_TO_SHARE_API . '?format=' . self::RESPONSE_FORMAT;
|
||||||
|
|
||||||
$local = \OC::$server->getURLGenerator()->getAbsoluteURL('/');
|
$local = \OC::$server->getURLGenerator()->getAbsoluteURL('/');
|
||||||
|
|
||||||
|
@ -2459,8 +2461,9 @@ class Share extends Constants {
|
||||||
* @return bool
|
* @return bool
|
||||||
*/
|
*/
|
||||||
private static function sendRemoteUnshare($remote, $id, $token) {
|
private static function sendRemoteUnshare($remote, $id, $token) {
|
||||||
$url = $remote . self::BASE_PATH_TO_SHARE_API . '/' . $id . '/unshare?format=' . self::RESPONSE_FORMAT;
|
$url = rtrim($remote, '/') . self::BASE_PATH_TO_SHARE_API . '/' . $id . '/unshare?format=' . self::RESPONSE_FORMAT;
|
||||||
$fields = array('token' => $token, 'format' => 'json');
|
$fields = array('token' => $token, 'format' => 'json');
|
||||||
|
$url = self::removeProtocolFromUrl($url);
|
||||||
$result = self::tryHttpPost($url, $fields);
|
$result = self::tryHttpPost($url, $fields);
|
||||||
$status = json_decode($result['result'], true);
|
$status = json_decode($result['result'], true);
|
||||||
|
|
||||||
|
|
|
@ -105,6 +105,12 @@ class Test_Share extends \Test\TestCase {
|
||||||
parent::tearDown();
|
parent::tearDown();
|
||||||
}
|
}
|
||||||
|
|
||||||
|
protected function setHttpHelper($httpHelper) {
|
||||||
|
\OC::$server->registerService('HTTPHelper', function () use ($httpHelper) {
|
||||||
|
return $httpHelper;
|
||||||
|
});
|
||||||
|
}
|
||||||
|
|
||||||
public function testShareInvalidShareType() {
|
public function testShareInvalidShareType() {
|
||||||
$message = 'Share type foobar is not valid for test.txt';
|
$message = 'Share type foobar is not valid for test.txt';
|
||||||
try {
|
try {
|
||||||
|
@ -1024,10 +1030,58 @@ class Test_Share extends \Test\TestCase {
|
||||||
);
|
);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
public function dataRemoteShareUrlCalls() {
|
||||||
|
return [
|
||||||
|
['admin@localhost', 'localhost'],
|
||||||
|
['admin@https://localhost', 'localhost'],
|
||||||
|
['admin@http://localhost', 'localhost'],
|
||||||
|
['admin@localhost/subFolder', 'localhost/subFolder'],
|
||||||
|
];
|
||||||
|
}
|
||||||
|
|
||||||
|
/**
|
||||||
|
* @dataProvider dataRemoteShareUrlCalls
|
||||||
|
*
|
||||||
|
* @param string $shareWith
|
||||||
|
* @param string $urlHost
|
||||||
|
*/
|
||||||
|
public function testRemoteShareUrlCalls($shareWith, $urlHost) {
|
||||||
|
$oldHttpHelper = \OC::$server->query('HTTPHelper');
|
||||||
|
$httpHelperMock = $this->getMockBuilder('OC\HttpHelper')
|
||||||
|
->disableOriginalConstructor()
|
||||||
|
->getMock();
|
||||||
|
$this->setHttpHelper($httpHelperMock);
|
||||||
|
|
||||||
|
$httpHelperMock->expects($this->at(0))
|
||||||
|
->method('post')
|
||||||
|
->with($this->stringStartsWith('https://' . $urlHost . '/ocs/v1.php/cloud/shares'), $this->anything())
|
||||||
|
->willReturn(['success' => false, 'result' => 'Exception']);
|
||||||
|
$httpHelperMock->expects($this->at(1))
|
||||||
|
->method('post')
|
||||||
|
->with($this->stringStartsWith('http://' . $urlHost . '/ocs/v1.php/cloud/shares'), $this->anything())
|
||||||
|
->willReturn(['success' => true, 'result' => json_encode(['ocs' => ['meta' => ['statuscode' => 100]]])]);
|
||||||
|
|
||||||
|
\OCP\Share::shareItem('test', 'test.txt', \OCP\Share::SHARE_TYPE_REMOTE, $shareWith, \OCP\Constants::PERMISSION_READ);
|
||||||
|
$shares = \OCP\Share::getItemShared('test', 'test.txt');
|
||||||
|
$share = array_shift($shares);
|
||||||
|
|
||||||
|
$httpHelperMock->expects($this->at(0))
|
||||||
|
->method('post')
|
||||||
|
->with($this->stringStartsWith('https://' . $urlHost . '/ocs/v1.php/cloud/shares/' . $share['id'] . '/unshare'), $this->anything())
|
||||||
|
->willReturn(['success' => false, 'result' => 'Exception']);
|
||||||
|
$httpHelperMock->expects($this->at(1))
|
||||||
|
->method('post')
|
||||||
|
->with($this->stringStartsWith('http://' . $urlHost . '/ocs/v1.php/cloud/shares/' . $share['id'] . '/unshare'), $this->anything())
|
||||||
|
->willReturn(['success' => true, 'result' => json_encode(['ocs' => ['meta' => ['statuscode' => 100]]])]);
|
||||||
|
|
||||||
|
\OCP\Share::unshare('test', 'test.txt', \OCP\Share::SHARE_TYPE_REMOTE, $shareWith);
|
||||||
|
$this->setHttpHelper($oldHttpHelper);
|
||||||
|
}
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* @dataProvider dataProviderTestGroupItems
|
* @dataProvider dataProviderTestGroupItems
|
||||||
* @param type $ungrouped
|
* @param array $ungrouped
|
||||||
* @param type $grouped
|
* @param array $grouped
|
||||||
*/
|
*/
|
||||||
function testGroupItems($ungrouped, $grouped) {
|
function testGroupItems($ungrouped, $grouped) {
|
||||||
|
|
||||||
|
|
Loading…
Reference in New Issue