Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[sharing] fix addToGroup hook #17381

Merged
merged 2 commits into from
Jul 21, 2015
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 10 additions & 4 deletions apps/files_sharing/lib/sharedmount.php
Original file line number Diff line number Diff line change
Expand Up @@ -46,12 +46,18 @@ class SharedMount extends MountPoint implements MoveableMount {
*/
private $recipientView;

/**
* @var string
*/
private $user;

public function __construct($storage, $mountpoint, $arguments = null, $loader = null) {
// first update the mount point before creating the parent
$this->ownerPropagator = $arguments['propagator'];
$this->recipientView = new View('/' . $arguments['user'] . '/files');
$this->user = $arguments['user'];
$this->recipientView = new View('/' . $this->user . '/files');
$newMountPoint = $this->verifyMountPoint($arguments['share']);
$absMountPoint = '/' . $arguments['user'] . '/files' . $newMountPoint;
$absMountPoint = '/' . $this->user . '/files' . $newMountPoint;
$arguments['ownerView'] = new View('/' . $arguments['share']['uid_owner'] . '/files');
parent::__construct($storage, $absMountPoint, $arguments, $loader);
}
Expand Down Expand Up @@ -90,15 +96,15 @@ private function verifyMountPoint(&$share) {
* @param array $share reference to the share which should be modified
* @return bool
*/
private static function updateFileTarget($newPath, &$share) {
private function updateFileTarget($newPath, &$share) {
// if the user renames a mount point from a group share we need to create a new db entry
// for the unique name
if ($share['share_type'] === \OCP\Share::SHARE_TYPE_GROUP && empty($share['unique_name'])) {
$query = \OCP\DB::prepare('INSERT INTO `*PREFIX*share` (`item_type`, `item_source`, `item_target`,'
.' `share_type`, `share_with`, `uid_owner`, `permissions`, `stime`, `file_source`,'
.' `file_target`, `token`, `parent`) VALUES (?,?,?,?,?,?,?,?,?,?,?,?)');
$arguments = array($share['item_type'], $share['item_source'], $share['item_target'],
2, \OCP\User::getUser(), $share['uid_owner'], $share['permissions'], $share['stime'], $share['file_source'],
2, $this->user, $share['uid_owner'], $share['permissions'], $share['stime'], $share['file_source'],
$newPath, $share['token'], $share['id']);
} else {
// rename mount point
Expand Down
1 change: 1 addition & 0 deletions lib/base.php
Original file line number Diff line number Diff line change
Expand Up @@ -797,6 +797,7 @@ public static function registerShareHooks() {
if (\OC::$server->getSystemConfig()->getValue('installed')) {
OC_Hook::connect('OC_User', 'post_deleteUser', 'OC\Share\Hooks', 'post_deleteUser');
OC_Hook::connect('OC_User', 'post_addToGroup', 'OC\Share\Hooks', 'post_addToGroup');
OC_Hook::connect('OC_Group', 'pre_addToGroup', 'OC\Share\Hooks', 'pre_addToGroup');
OC_Hook::connect('OC_User', 'post_removeFromGroup', 'OC\Share\Hooks', 'post_removeFromGroup');
OC_Hook::connect('OC_User', 'post_deleteGroup', 'OC\Share\Hooks', 'post_deleteGroup');
}
Expand Down
115 changes: 87 additions & 28 deletions lib/private/share/hooks.php
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,13 @@
namespace OC\Share;

class Hooks extends \OC\Share\Constants {

/**
* remember which targets need to be updated in the post addToGroup Hook
* @var array
*/
private static $updateTargets = array();

/**
* Function that is called after a user is deleted. Cleans up the shares of that user.
* @param array $arguments
Expand All @@ -41,46 +48,98 @@ public static function post_deleteUser($arguments) {
}
}


/**
* Function that is called after a user is added to a group.
* TODO what does it do?
* Function that is called before a user is added to a group.
* check if we need to create a unique target for the user
* @param array $arguments
*/
public static function post_addToGroup($arguments) {
public static function pre_addToGroup($arguments) {
/** @var \OC\DB\Connection $db */
$db = \OC::$server->getDatabaseConnection();

$insert = $db->createQueryBuilder();

$select = $db->createQueryBuilder();
// Find the group shares and check if the user needs a unique target
$query = \OC_DB::prepare('SELECT * FROM `*PREFIX*share` WHERE `share_type` = ? AND `share_with` = ?');
$result = $query->execute(array(self::SHARE_TYPE_GROUP, $arguments['gid']));
$query = \OC_DB::prepare('INSERT INTO `*PREFIX*share` (`item_type`, `item_source`,'
.' `item_target`, `parent`, `share_type`, `share_with`, `uid_owner`, `permissions`,'
.' `stime`, `file_source`, `file_target`) VALUES (?,?,?,?,?,?,?,?,?,?,?)');
while ($item = $result->fetchRow()) {
$select->select('*')
->from('`*PREFIX*share`')
->where($select->expr()->andX(
$select->expr()->eq('`share_type`', ':shareType'),
$select->expr()->eq('`share_with`', ':shareWith')
))
->setParameter('shareType', self::SHARE_TYPE_GROUP)
->setParameter('shareWith', $arguments['gid']);

$sourceExists = \OC\Share\Share::getItemSharedWithBySource($item['item_type'], $item['item_source'], self::FORMAT_NONE, null, true, $arguments['uid']);
$result = $select->execute();

if ($sourceExists) {
$fileTarget = $sourceExists['file_target'];
$itemTarget = $sourceExists['item_target'];
while ($item = $result->fetch()) {

$itemTarget = Helper::generateTarget(
$item['item_type'],
$item['item_source'],
self::SHARE_TYPE_USER,
$arguments['uid'],
$item['uid_owner'],
null,
$item['parent']
);

if ($item['item_type'] === 'file' || $item['item_type'] === 'folder') {
$fileTarget = Helper::generateTarget(
$item['item_type'],
$item['file_target'],
self::SHARE_TYPE_USER,
$arguments['uid'],
$item['uid_owner'],
null,
$item['parent']
);
} else {
$itemTarget = Helper::generateTarget($item['item_type'], $item['item_source'], self::SHARE_TYPE_USER, $arguments['uid'],
$item['uid_owner'], null, $item['parent']);

// do we also need a file target
if ($item['item_type'] === 'file' || $item['item_type'] === 'folder') {
$fileTarget = Helper::generateTarget('file', $item['file_target'], self::SHARE_TYPE_USER, $arguments['uid'],
$item['uid_owner'], null, $item['parent']);
} else {
$fileTarget = null;
}
$fileTarget = null;
}


// Insert an extra row for the group share if the item or file target is unique for this user
if ($itemTarget != $item['item_target'] || $fileTarget != $item['file_target']) {
$query->execute(array($item['item_type'], $item['item_source'], $itemTarget, $item['id'],
self::$shareTypeGroupUserUnique, $arguments['uid'], $item['uid_owner'], $item['permissions'],
$item['stime'], $item['file_source'], $fileTarget));
\OC_DB::insertid('*PREFIX*share');
if (
($fileTarget === null && $itemTarget != $item['item_target'])
|| ($fileTarget !== null && $fileTarget !== $item['file_target'])
) {
self::$updateTargets[$arguments['gid']][] = [
'`item_type`' => $insert->expr()->literal($item['item_type']),
'`item_source`' => $insert->expr()->literal($item['item_source']),
'`item_target`' => $insert->expr()->literal($itemTarget),
'`file_target`' => $insert->expr()->literal($fileTarget),
'`parent`' => $insert->expr()->literal($item['id']),
'`share_type`' => $insert->expr()->literal(self::$shareTypeGroupUserUnique),
'`share_with`' => $insert->expr()->literal($arguments['uid']),
'`uid_owner`' => $insert->expr()->literal($item['uid_owner']),
'`permissions`' => $insert->expr()->literal($item['permissions']),
'`stime`' => $insert->expr()->literal($item['stime']),
'`file_source`' => $insert->expr()->literal($item['file_source']),
];
}
}
}

/**
* Function that is called after a user is added to a group.
* add unique target for the user if needed
* @param array $arguments
*/
public static function post_addToGroup($arguments) {
/** @var \OC\DB\Connection $db */
$db = \OC::$server->getDatabaseConnection();

$insert = $db->createQueryBuilder();
$insert->insert('`*PREFIX*share`');

if (isset(self::$updateTargets[$arguments['gid']])) {
foreach (self::$updateTargets[$arguments['gid']] as $newTarget) {
$insert->values($newTarget);
$insert->execute();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does this work correctly when you run execute multiple times? Since you keep the original $insert object.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should work because we replace the values on the object for each execute()... but I will double-check.

}
unset(self::$updateTargets[$arguments['gid']]);
}
}

Expand Down
108 changes: 108 additions & 0 deletions tests/lib/share/hooktests.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,108 @@
<?php
/**
* @author Björn Schießle <schiessle@owncloud.com>
*
* @copyright Copyright (c) 2015, ownCloud, Inc.
* @license AGPL-3.0
*
* This code is free software: you can redistribute it and/or modify
* it under the terms of the GNU Affero General Public License, version 3,
* as published by the Free Software Foundation.
*
* This program 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, version 3,
* along with this program. If not, see <http://www.gnu.org/licenses/>
*
*/


namespace OC\Tests\Share;


use Test\TestCase;

class HookTests extends TestCase {

protected function setUp() {
parent::setUp();
}

protected function tearDown() {
$query = \OC_DB::prepare('DELETE FROM `*PREFIX*share` WHERE `item_type` = ?');
$query->execute(array('test'));

parent::tearDown();
}

public function testPostAddToGroup() {

/** @var \OC\DB\Connection $connection */
$connection = \OC::$server->getDatabaseConnection();
$query = $connection->createQueryBuilder();
$expr = $query->expr();

// add some dummy values to the private $updateTargets variable
$this->invokePrivate(
new \OC\Share\Hooks(),
'updateTargets',
[
[
'group1' =>
[
[
'`item_type`' => $expr->literal('test'),
'`item_source`' => $expr->literal('42'),
'`item_target`' => $expr->literal('42'),
'`file_target`' => $expr->literal('test'),
'`share_type`' => $expr->literal('2'),
'`share_with`' => $expr->literal('group1'),
'`uid_owner`' => $expr->literal('owner'),
'`permissions`' => $expr->literal('0'),
'`stime`' => $expr->literal('676584'),
'`file_source`' => $expr->literal('42'),
],
[
'`item_type`' => $expr->literal('test'),
'`item_source`' => $expr->literal('42'),
'`item_target`' => $expr->literal('42 (2)'),
'`share_type`' => $expr->literal('2'),
'`share_with`' => $expr->literal('group1'),
'`uid_owner`' => $expr->literal('owner'),
'`permissions`' => $expr->literal('0'),
'`stime`' => $expr->literal('676584'),
]
],
'group2' =>
[
[
'`item_type`' => $expr->literal('test'),
'`item_source`' => $expr->literal('42'),
'`item_target`' => $expr->literal('42'),
'`share_type`' => $expr->literal('2'),
'`share_with`' => $expr->literal('group2'),
'`uid_owner`' => $expr->literal('owner'),
'`permissions`' => $expr->literal('0'),
'`stime`' => $expr->literal('676584'),
]
]
]
]
);

// add unique targets for group1 to database
\OC\Share\Hooks::post_addToGroup(['gid' => 'group1']);


$query->select('`share_with`')->from('`*PREFIX*share`');
$result = $query->execute()->fetchAll();
$this->assertSame(2, count($result));
foreach ($result as $r) {
$this->assertSame('group1', $r['share_with']);
}
}

}