Add repair step to fix file share permissions

owncloud · Nov 10, 2016 · b99f808 · b99f808
1 parent d1c8440
commit b99f808
Show file tree

Hide file tree

Showing 3 changed files with 101 additions and 1 deletion.
diff --git a/lib/private/Repair/RepairInvalidShares.php b/lib/private/Repair/RepairInvalidShares.php
@@ -26,6 +26,7 @@
 
 use OCP\Migration\IOutput;
 use OCP\Migration\IRepairStep;
+use Doctrine\DBAL\Platforms\OraclePlatform;
 
 /**
  * Repairs shares with invalid data
@@ -90,6 +91,35 @@ private function addShareLinkDeletePermission(IOutput $out) {
 		}
 	}
 
+	/**
+	 * 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
 	 */
@@ -136,6 +166,9 @@ public function run(IOutput $out) {
 			// this situation was only possible before 9.1
 			$this->addShareLinkDeletePermission($out);
 		}
+		if (version_compare($ocVersionFromBeforeUpdate, '9.2.0.2', '<')) {
+			$this->adjustFileSharePermissions($out);
+		}
 
 		$this->removeSharesNonExistingParent($out);
 	}

diff --git a/tests/lib/Repair/RepairInvalidSharesTest.php b/tests/lib/Repair/RepairInvalidSharesTest.php
@@ -278,6 +278,73 @@ public function testSharesNonExistingParent() {
 		$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
 	 */

diff --git a/version.php b/version.php
@@ -23,7 +23,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, 2, 0, 2);
+$OC_Version = array(9, 2, 0, 3);
 
 // The human readable string
 $OC_VersionString = '9.2.0 pre alpha';