From 0ad065cb8d7bbc3665140f0b264a8ac74167cfa8 Mon Sep 17 00:00:00 2001 From: Vincent Petry Date: Thu, 23 Jun 2016 16:37:03 +0200 Subject: [PATCH] Repair step to adjust link share delete permissions --- lib/private/Repair/RepairInvalidShares.php | 23 ++++++ tests/lib/Repair/RepairInvalidSharesTest.php | 87 ++++++++++++++++++++ version.php | 2 +- 3 files changed, 111 insertions(+), 1 deletion(-) diff --git a/lib/private/Repair/RepairInvalidShares.php b/lib/private/Repair/RepairInvalidShares.php index 30f67a1f39..728632486d 100644 --- a/lib/private/Repair/RepairInvalidShares.php +++ b/lib/private/Repair/RepairInvalidShares.php @@ -71,6 +71,25 @@ class RepairInvalidShares implements IRepairStep { } } + /** + * In the past link shares with public upload enabled were missing the delete permission. + */ + private function addShareLinkDeletePermission(IOutput $out) { + $oldPerms = \OCP\Constants::PERMISSION_READ | \OCP\Constants::PERMISSION_CREATE | \OCP\Constants::PERMISSION_UPDATE; + $newPerms = \OCP\Constants::PERMISSION_READ | \OCP\Constants::PERMISSION_CREATE | \OCP\Constants::PERMISSION_UPDATE | \OCP\Constants::PERMISSION_DELETE; + $builder = $this->connection->getQueryBuilder(); + $builder + ->update('share') + ->set('permissions', $builder->expr()->literal($newPerms)) + ->where($builder->expr()->eq('share_type', $builder->expr()->literal(\OC\Share\Constants::SHARE_TYPE_LINK))) + ->andWhere($builder->expr()->eq('permissions', $builder->expr()->literal($oldPerms))); + + $updatedEntries = $builder->execute(); + if ($updatedEntries > 0) { + $out->info('Fixed link share permissions for ' . $updatedEntries . ' shares'); + } + } + /** * Remove shares where the parent share does not exist anymore */ @@ -113,6 +132,10 @@ class RepairInvalidShares implements IRepairStep { // this situation was only possible before 8.2 $this->removeExpirationDateFromNonLinkShares($out); } + if (version_compare($ocVersionFromBeforeUpdate, '9.1.0.9', '<')) { + // this situation was only possible before 9.1 + $this->addShareLinkDeletePermission($out); + } $this->removeSharesNonExistingParent($out); } diff --git a/tests/lib/Repair/RepairInvalidSharesTest.php b/tests/lib/Repair/RepairInvalidSharesTest.php index a1e871bcc8..1ac42e53bf 100644 --- a/tests/lib/Repair/RepairInvalidSharesTest.php +++ b/tests/lib/Repair/RepairInvalidSharesTest.php @@ -123,6 +123,93 @@ class RepairInvalidSharesTest extends TestCase { $this->assertNotNull($linkShare['expiration'], 'valid link share expiration date still there'); } + /** + * Test remove expiration date for non-link shares + */ + public function testAddShareLinkDeletePermission() { + $oldPerms = \OCP\Constants::PERMISSION_READ | \OCP\Constants::PERMISSION_CREATE | \OCP\Constants::PERMISSION_UPDATE; + $newPerms = \OCP\Constants::PERMISSION_READ | \OCP\Constants::PERMISSION_CREATE | \OCP\Constants::PERMISSION_UPDATE | \OCP\Constants::PERMISSION_DELETE; + + // share with old permissions + $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('folder'), + '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($oldPerms), + 'stime' => $qb->expr()->literal(time()), + ]) + ->execute(); + + $bogusShareId = $this->getLastShareId(); + + // share with read-only permissions + $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('folder'), + '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(\OCP\Constants::PERMISSION_READ), + 'stime' => $qb->expr()->literal(time()), + ]) + ->execute(); + + $keepThisShareId = $this->getLastShareId(); + + // user share to keep + $qb = $this->connection->getQueryBuilder(); + $qb->insert('share') + ->values([ + 'share_type' => $qb->expr()->literal(Constants::SHARE_TYPE_USER), + 'share_with' => $qb->expr()->literal('recipientuser1'), + 'uid_owner' => $qb->expr()->literal('user1'), + 'item_type' => $qb->expr()->literal('folder'), + '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(3), + 'stime' => $qb->expr()->literal(time()), + ]) + ->execute(); + + $keepThisShareId2 = $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(3, $results); + + $untouchedShare = $results[0]; + $untouchedShare2 = $results[1]; + $updatedShare = $results[2]; + $this->assertEquals($keepThisShareId, $untouchedShare['id'], 'sanity check'); + $this->assertEquals($keepThisShareId2, $untouchedShare2['id'], 'sanity check'); + $this->assertEquals($bogusShareId, $updatedShare['id'], 'sanity check'); + $this->assertEquals($newPerms, $updatedShare['permissions'], 'delete permission was added'); + } + /** * Test remove shares where the parent share does not exist anymore */ diff --git a/version.php b/version.php index 3015d976e5..b439ffbbda 100644 --- a/version.php +++ b/version.php @@ -25,7 +25,7 @@ // 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 // when updating major/minor version number. -$OC_Version = array(9, 1, 0, 9); +$OC_Version = array(9, 1, 0, 10); // The human readable string $OC_VersionString = '9.1.0 beta 2';