Unverified Commit b99f8085 authored by Vincent Petry's avatar Vincent Petry
Browse files

Add repair step to fix file share permissions

parent d1c84409
...@@ -26,6 +26,7 @@ namespace OC\Repair; ...@@ -26,6 +26,7 @@ namespace OC\Repair;
use OCP\Migration\IOutput; use OCP\Migration\IOutput;
use OCP\Migration\IRepairStep; use OCP\Migration\IRepairStep;
use Doctrine\DBAL\Platforms\OraclePlatform;
/** /**
* Repairs shares with invalid data * Repairs shares with invalid data
...@@ -90,6 +91,35 @@ class RepairInvalidShares implements IRepairStep { ...@@ -90,6 +91,35 @@ class RepairInvalidShares implements IRepairStep {
} }
} }
/**
* Adjust file share permissions
*/
private function adjustFileSharePermissions(IOutput $out) {
$mask = \OCP\Constants::PERMISSION_READ | \OCP\Constants::PERMISSION_UPDATE | \OCP\Constants::PERMISSION_SHARE;
$builder = $this->connection->getQueryBuilder();
if ($this->connection->getDatabasePlatform() instanceof OraclePlatform) {
$permsFunc = $builder->createFunction(
'bitand(' . $builder->getColumnName('permissions') . ', ' . $mask . ')'
);
} else {
$permsFunc = $builder->createFunction(
'(' . $builder->getColumnName('permissions') . ' & ' . $mask . ')'
);
}
$builder
->update('share')
->set('permissions', $permsFunc)
->where($builder->expr()->eq('item_type', $builder->expr()->literal('file')))
->andWhere($builder->expr()->neq('permissions', $permsFunc));
$updatedEntries = $builder->execute();
if ($updatedEntries > 0) {
$out->info('Fixed file share permissions for ' . $updatedEntries . ' shares');
}
}
/** /**
* Remove shares where the parent share does not exist anymore * Remove shares where the parent share does not exist anymore
*/ */
...@@ -136,6 +166,9 @@ class RepairInvalidShares implements IRepairStep { ...@@ -136,6 +166,9 @@ class RepairInvalidShares implements IRepairStep {
// this situation was only possible before 9.1 // this situation was only possible before 9.1
$this->addShareLinkDeletePermission($out); $this->addShareLinkDeletePermission($out);
} }
if (version_compare($ocVersionFromBeforeUpdate, '9.2.0.2', '<')) {
$this->adjustFileSharePermissions($out);
}
$this->removeSharesNonExistingParent($out); $this->removeSharesNonExistingParent($out);
} }
......
...@@ -278,6 +278,73 @@ class RepairInvalidSharesTest extends TestCase { ...@@ -278,6 +278,73 @@ class RepairInvalidSharesTest extends TestCase {
$result->closeCursor(); $result->closeCursor();
} }
public function fileSharePermissionsProvider() {
return [
// unchanged for folder
[
'folder',
31,
31,
],
// unchanged for read-write + share
[
'file',
\OCP\Constants::PERMISSION_READ | \OCP\Constants::PERMISSION_UPDATE | \OCP\Constants::PERMISSION_SHARE,
\OCP\Constants::PERMISSION_READ | \OCP\Constants::PERMISSION_UPDATE | \OCP\Constants::PERMISSION_SHARE,
],
// fixed for all perms
[
'file',
\OCP\Constants::PERMISSION_READ | \OCP\Constants::PERMISSION_CREATE | \OCP\Constants::PERMISSION_UPDATE | \OCP\Constants::PERMISSION_DELETE | \OCP\Constants::PERMISSION_SHARE,
\OCP\Constants::PERMISSION_READ | \OCP\Constants::PERMISSION_UPDATE | \OCP\Constants::PERMISSION_SHARE,
],
];
}
/**
* Test adjusting file share permissions
*
* @dataProvider fileSharePermissionsProvider
*/
public function testFileSharePermissions($itemType, $testPerms, $expectedPerms) {
$qb = $this->connection->getQueryBuilder();
$qb->insert('share')
->values([
'share_type' => $qb->expr()->literal(Constants::SHARE_TYPE_LINK),
'uid_owner' => $qb->expr()->literal('user1'),
'item_type' => $qb->expr()->literal($itemType),
'item_source' => $qb->expr()->literal(123),
'item_target' => $qb->expr()->literal('/123'),
'file_source' => $qb->expr()->literal(123),
'file_target' => $qb->expr()->literal('/test'),
'permissions' => $qb->expr()->literal($testPerms),
'stime' => $qb->expr()->literal(time()),
])
->execute();
$shareId = $this->getLastShareId();
/** @var IOutput | \PHPUnit_Framework_MockObject_MockObject $outputMock */
$outputMock = $this->getMockBuilder('\OCP\Migration\IOutput')
->disableOriginalConstructor()
->getMock();
$this->repair->run($outputMock);
$results = $this->connection->getQueryBuilder()
->select('*')
->from('share')
->orderBy('permissions', 'ASC')
->execute()
->fetchAll();
$this->assertCount(1, $results);
$updatedShare = $results[0];
$this->assertEquals($expectedPerms, $updatedShare['permissions']);
}
/** /**
* @return int * @return int
*/ */
......
...@@ -23,7 +23,7 @@ ...@@ -23,7 +23,7 @@
// We only can count up. The 4. digit is only for the internal patchlevel to trigger DB upgrades // We only can count up. The 4. digit is only for the internal patchlevel to trigger DB upgrades
// between betas, final and RCs. This is _not_ the public version number. Reset minor/patchlevel // between betas, final and RCs. This is _not_ the public version number. Reset minor/patchlevel
// when updating major/minor version number. // when updating major/minor version number.
$OC_Version = array(9, 2, 0, 2); $OC_Version = array(9, 2, 0, 3);
// The human readable string // The human readable string
$OC_VersionString = '9.2.0 pre alpha'; $OC_VersionString = '9.2.0 pre alpha';
......
Supports Markdown
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment