mbk-lab
/
nextcloud
2
0
Fork 0
Code Issues Pull Requests Projects Releases Wiki Activity

Merge pull request #14580 from owncloud/issue/13765-duplicate-remote-share 

"Integrity constraint violation" when sharing the same item twice with the same user
This commit is contained in:
Morris Jobke 2015-03-24 15:05:58 +01:00
parent cc2092a511 8ebb198ef3
commit 965d97a8f5
4 changed files with 209 additions and 26 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
apps/files_sharing/ajax/external.php
View File

@ -38,8 +38,6 @@ $externalManager = new \OCA\Files_Sharing\External\Manager(
\OC::$server->getUserSession()->getUser()->getUID()
);
$name = OCP\Files::buildNotExistingFileName('/', $name);
// check for ssl cert
if (substr($remote, 0, 5) === 'https' and !OC_Util::getUrlContent($remote)) {
\OCP\JSON::error(array('data' => array('message' => $l->t('Invalid or untrusted SSL certificate'))));

2
apps/files_sharing/api/server2server.php
View File

@ -64,8 +64,6 @@ class Server2Server {
$shareWith
);
$name = \OCP\Files::buildNotExistingFileName('/', $name);
try {
$externalManager->addShare($remote, $token, '', $name, $owner, false, $shareWith, $remoteId);

85
apps/files_sharing/lib/external/manager.php vendored
View File

@ -9,6 +9,7 @@
namespace OCA\Files_Sharing\External;
use OC\Files\Filesystem;
use OCP\Files;
class Manager {
const STORAGE = '\OCA\Files_Sharing\External\Storage';
@ -29,7 +30,7 @@ class Manager {
private $mountManager;
/**
* @var \OC\Files\Storage\StorageFactory
* @var \OCP\Files\Storage\IStorageFactory
*/
private $storageLoader;
@ -41,12 +42,12 @@ class Manager {
/**
* @param \OCP\IDBConnection $connection
* @param \OC\Files\Mount\Manager $mountManager
* @param \OC\Files\Storage\StorageFactory $storageLoader
* @param \OCP\Files\Storage\IStorageFactory $storageLoader
* @param \OC\HTTPHelper $httpHelper
* @param string $uid
*/
public function __construct(\OCP\IDBConnection $connection, \OC\Files\Mount\Manager $mountManager,
\OC\Files\Storage\StorageFactory $storageLoader, \OC\HTTPHelper $httpHelper, $uid) {
\OCP\Files\Storage\IStorageFactory $storageLoader, \OC\HTTPHelper $httpHelper, $uid) {
$this->connection = $connection;
$this->mountManager = $mountManager;
$this->storageLoader = $storageLoader;
@ -65,24 +66,56 @@ class Manager {
* @param boolean $accepted
* @param string $user
* @param int $remoteId
* @return mixed
* @return Mount|null
*/
public function addShare($remote, $token, $password, $name, $owner, $accepted=false, $user = null, $remoteId = -1) {
$user = $user ? $user : $this->uid;
$accepted = $accepted ? 1 : 0;
$name = Filesystem::normalizePath('/' . $name);
$mountPoint = Filesystem::normalizePath('/' . $name);
if (!$accepted) {
// To avoid conflicts with the mount point generation later,
// we only use a temporary mount point name here. The real
// mount point name will be generated when accepting the share,
// using the original share item name.
$tmpMountPointName = '{{TemporaryMountPointName#' . $name . '}}';
$mountPoint = $tmpMountPointName;
$hash = md5($tmpMountPointName);
$data = [
'remote' => $remote,
'share_token' => $token,
'password' => $password,
'name' => $name,
'owner' => $owner,
'user' => $user,
'mountpoint' => $mountPoint,
'mountpoint_hash' => $hash,
'accepted' => $accepted,
'remote_id' => $remoteId,
];
$i = 1;
while (!$this->connection->insertIfNotExist('*PREFIX*share_external', $data, ['user', 'mountpoint_hash'])) {
// The external share already exists for the user
$data['mountpoint'] = $tmpMountPointName . '-' . $i;
$data['mountpoint_hash'] = md5($data['mountpoint']);
$i++;
}
return null;
}
$mountPoint = Files::buildNotExistingFileName('/', $name);
$mountPoint = Filesystem::normalizePath('/' . $mountPoint);
$hash = md5($mountPoint);
$query = $this->connection->prepare('
INSERT INTO `*PREFIX*share_external`
(`remote`, `share_token`, `password`, `name`, `owner`, `user`, `mountpoint`, `mountpoint_hash`, `accepted`, `remote_id`)
VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?)
');
$hash = md5($mountPoint);
$query->execute(array($remote, $token, $password, $name, $owner, $user, $mountPoint, $hash, $accepted, $remoteId));
if ($accepted) {
$options = array(
'remote' => $remote,
'token' => $token,
@ -92,7 +125,6 @@ class Manager {
);
return $this->mountShare($options);
}
}
private function setupMounts() {
// don't setup server-to-server shares if the admin disabled it
@ -124,7 +156,7 @@ class Manager {
*/
private function getShare($id) {
$getShare = $this->connection->prepare('
SELECT `remote`, `share_token`
SELECT `remote`, `share_token`, `name`
FROM `*PREFIX*share_external`
WHERE `id` = ? AND `user` = ?');
$result = $getShare->execute(array($id, $this->uid));
@ -142,11 +174,17 @@ class Manager {
$share = $this->getShare($id);
if ($share) {
$mountPoint = Files::buildNotExistingFileName('/', $share['name']);
$mountPoint = Filesystem::normalizePath('/' . $mountPoint);
$hash = md5($mountPoint);
$acceptShare = $this->connection->prepare('
UPDATE `*PREFIX*share_external`
SET `accepted` = ?
SET `accepted` = ?,
`mountpoint` = ?,
`mountpoint_hash` = ?
WHERE `id` = ? AND `user` = ?');
$acceptShare->execute(array(1, $id, $this->uid));
$acceptShare->execute(array(1, $mountPoint, $hash, $id, $this->uid));
$this->sendFeedbackToRemote($share['remote'], $share['share_token'], $id, 'accept');
}
}
@ -315,10 +353,29 @@ class Manager {
* @return array list of open server-to-server shares
*/
public function getOpenShares() {
$openShares = $this->connection->prepare('SELECT * FROM `*PREFIX*share_external` WHERE `accepted` = ? AND `user` = ?');
$result = $openShares->execute(array(0, $this->uid));
return $this->getShares(false);
}
return $result ? $openShares->fetchAll() : array();
/**
* return a list of shares for the user
*
* @param bool|null $accepted True for accepted only,
* false for not accepted,
* null for all shares of the user
* @return array list of open server-to-server shares
*/
private function getShares($accepted) {
$query = 'SELECT * FROM `*PREFIX*share_external` WHERE `user` = ?';
$parameters = [$this->uid];
if (!is_null($accepted)) {
$query .= 'AND `accepted` = ?';
$parameters[] = (int) $accepted;
}
$query .= ' ORDER BY `id` ASC';
$shares = $this->connection->prepare($query);
$result = $shares->execute($parameters);
return $result ? $shares->fetchAll() : [];
}
}

130
apps/files_sharing/tests/external/managertest.php vendored Normal file
View File

@ -0,0 +1,130 @@
<?php
/**
* ownCloud
*
* @author Joas Schilling
* @copyright 2015 Joas Schilling <nickvergessen@owncloud.com>
*
* This library is free software; you can redistribute it and/or
* modify it under the terms of the GNU AFFERO GENERAL PUBLIC LICENSE
* License as published by the Free Software Foundation; either
* version 3 of the License, or any later version.
*
* This library is distributed in the hope that it will be useful,
* but WITHOUT ANY WARRANTY; without even the implied warranty of
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
* GNU AFFERO GENERAL PUBLIC LICENSE for more details.
*
* You should have received a copy of the GNU Affero General Public
* License along with this library. If not, see <http://www.gnu.org/licenses/>.
*
*/
namespace OCA\Files_Sharing\Tests\External;
use OCA\Files_Sharing\Tests\TestCase;
class ManagerTest extends TestCase {
/** @var \OCA\Files_Sharing\External\Manager **/
private $manager;
private $uid;
protected function setUp() {
parent::setUp();
$this->uid = $this->getUniqueID('user');
$this->manager = new \OCA\Files_Sharing\External\Manager(
\OC::$server->getDatabaseConnection(),
$this->getMockBuilder('\OC\Files\Mount\Manager')->disableOriginalConstructor()->getMock(),
$this->getMockBuilder('\OCP\Files\Storage\IStorageFactory')->disableOriginalConstructor()->getMock(),
$this->getMockBuilder('\OC\HTTPHelper')->disableOriginalConstructor()->getMock(),
$this->uid
);
}
public function testAddShare() {
$shareData1 = [
'remote' => 'localhost',
'token' => 'token1',
'password' => '',
'name' => '/SharedFolder',
'owner' => 'foobar',
'accepted' => false,
'user' => $this->uid,
];
$shareData2 = $shareData1;
$shareData2['token'] = 'token2';
$shareData3 = $shareData1;
$shareData3['token'] = 'token3';
// Add a share for "user"
$this->assertSame(null, call_user_func_array([$this->manager, 'addShare'], $shareData1));
$openShares = $this->manager->getOpenShares();
$this->assertCount(1, $openShares);
$this->assertExternalShareEntry($shareData1, $openShares[0], 1, '{{TemporaryMountPointName#' . $shareData1['name'] . '}}');
// Add a second share for "user" with the same name
$this->assertSame(null, call_user_func_array([$this->manager, 'addShare'], $shareData2));
$openShares = $this->manager->getOpenShares();
$this->assertCount(2, $openShares);
$this->assertExternalShareEntry($shareData1, $openShares[0], 1, '{{TemporaryMountPointName#' . $shareData1['name'] . '}}');
// New share falls back to "-1" appendix, because the name is already taken
$this->assertExternalShareEntry($shareData2, $openShares[1], 2, '{{TemporaryMountPointName#' . $shareData2['name'] . '}}-1');
// Accept the first share
$this->manager->acceptShare($openShares[0]['id']);
// Check remaining shares - Accepted
$acceptedShares = \Test_Helper::invokePrivate($this->manager, 'getShares', [true]);
$this->assertCount(1, $acceptedShares);
$shareData1['accepted'] = true;
$this->assertExternalShareEntry($shareData1, $acceptedShares[0], 1, $shareData1['name']);
// Check remaining shares - Open
$openShares = $this->manager->getOpenShares();
$this->assertCount(1, $openShares);
$this->assertExternalShareEntry($shareData2, $openShares[0], 2, '{{TemporaryMountPointName#' . $shareData2['name'] . '}}-1');
// Add another share for "user" with the same name
$this->assertSame(null, call_user_func_array([$this->manager, 'addShare'], $shareData3));
$openShares = $this->manager->getOpenShares();
$this->assertCount(2, $openShares);
$this->assertExternalShareEntry($shareData2, $openShares[0], 2, '{{TemporaryMountPointName#' . $shareData2['name'] . '}}-1');
// New share falls back to the original name (no "-\d", because the name is not taken)
$this->assertExternalShareEntry($shareData3, $openShares[1], 3, '{{TemporaryMountPointName#' . $shareData3['name'] . '}}');
// Decline the third share
$this->manager->declineShare($openShares[1]['id']);
// Check remaining shares - Accepted
$acceptedShares = \Test_Helper::invokePrivate($this->manager, 'getShares', [true]);
$this->assertCount(1, $acceptedShares);
$shareData1['accepted'] = true;
$this->assertExternalShareEntry($shareData1, $acceptedShares[0], 1, $shareData1['name']);
// Check remaining shares - Open
$openShares = $this->manager->getOpenShares();
$this->assertCount(1, $openShares);
$this->assertExternalShareEntry($shareData2, $openShares[0], 2, '{{TemporaryMountPointName#' . $shareData2['name'] . '}}-1');
$this->manager->removeUserShares($this->uid);
$this->assertEmpty(\Test_Helper::invokePrivate($this->manager, 'getShares', [null]), 'Asserting all shares for the user have been deleted');
}
/**
* @param array $expected
* @param array $actual
* @param int $share
* @param string $mountPoint
*/
protected function assertExternalShareEntry($expected, $actual, $share, $mountPoint) {
$this->assertEquals($expected['remote'], $actual['remote'], 'Asserting remote of a share #' . $share);
$this->assertEquals($expected['token'], $actual['share_token'], 'Asserting token of a share #' . $share);
$this->assertEquals($expected['name'], $actual['name'], 'Asserting name of a share #' . $share);
$this->assertEquals($expected['owner'], $actual['owner'], 'Asserting owner of a share #' . $share);
$this->assertEquals($expected['accepted'], (int) $actual['accepted'], 'Asserting accept of a share #' . $share);
$this->assertEquals($expected['user'], $actual['user'], 'Asserting user of a share #' . $share);
$this->assertEquals($mountPoint, $actual['mountpoint'], 'Asserting mountpoint of a share #' . $share);
}
}