Repair step to adjust link share delete permissions

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);
 	}

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
 	 */

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';