Merge pull request #2817 from nextcloud/fix-invalid-share-perms

Fix invalid share perms

lib/private/DB/QueryBuilder/ExpressionBuilder/ExpressionBuilder.php

@@ -30,6 +30,8 @@ use OC\DB\QueryBuilder\Literal;
 use OC\DB\QueryBuilder\QueryFunction;
 use OC\DB\QueryBuilder\QuoteHelper;
 use OCP\DB\QueryBuilder\IExpressionBuilder;
+use OCP\DB\QueryBuilder\ILiteral;
+use OCP\DB\QueryBuilder\IQueryFunction;
 use OCP\IDBConnection;
 
 class ExpressionBuilder implements IExpressionBuilder {
@@ -39,12 +41,16 @@ class ExpressionBuilder implements IExpressionBuilder {
 	/** @var QuoteHelper */
 	protected $helper;
 
+	/** @var IDBConnection */
+	protected $connection;
+
 	/**
 	 * Initializes a new <tt>ExpressionBuilder</tt>.
 	 *
 	 * @param \OCP\IDBConnection $connection
 	 */
 	public function __construct(IDBConnection $connection) {
+		$this->connection = $connection;
 		$this->helper = new QuoteHelper();
 		$this->expressionBuilder = new DoctrineExpressionBuilder($connection);
 	}
@@ -344,13 +350,43 @@ class ExpressionBuilder implements IExpressionBuilder {
 		return $this->expressionBuilder->notIn($x, $y);
 	}
 
+	/**
+	 * Binary AND Operator copies a bit to the result if it exists in both operands.
+	 *
+	 * @param string|ILiteral $x The field or value to check
+	 * @param int $y Bitmap that must be set
+	 * @return IQueryFunction
+	 * @since 12.0.0
+	 */
+	public function bitwiseAnd($x, $y) {
+		return new QueryFunction($this->connection->getDatabasePlatform()->getBitAndComparisonExpression(
+			$this->helper->quoteColumnName($x),
+			$y
+		));
+	}
+
+	/**
+	 * Binary OR Operator copies a bit if it exists in either operand.
+	 *
+	 * @param string|ILiteral $x The field or value to check
+	 * @param int $y Bitmap that must be set
+	 * @return IQueryFunction
+	 * @since 12.0.0
+	 */
+	public function bitwiseOr($x, $y) {
+		return new QueryFunction($this->connection->getDatabasePlatform()->getBitOrComparisonExpression(
+			$this->helper->quoteColumnName($x),
+			$y
+		));
+	}
+
 	/**
 	 * Quotes a given input parameter.
 	 *
 	 * @param mixed $input The parameter to be quoted.
 	 * @param mixed|null $type One of the IQueryBuilder::PARAM_* constants
 	 *
-	 * @return Literal
+	 * @return ILiteral
 	 */
 	public function literal($input, $type = null) {
 		return new Literal($this->expressionBuilder->literal($input, $type));

lib/private/Repair/RepairInvalidShares.php

@@ -27,6 +27,7 @@ namespace OC\Repair;
 
 use OCP\Migration\IOutput;
 use OCP\Migration\IRepairStep;
+use Doctrine\DBAL\Platforms\OraclePlatform;
 
 /**
  * Repairs shares with invalid data
@@ -91,6 +92,26 @@ 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();
+
+		$permsFunc = $builder->expr()->bitwiseAnd('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
 	 */
@@ -137,6 +158,9 @@ class RepairInvalidShares implements IRepairStep {
 			// this situation was only possible before 9.1
 			$this->addShareLinkDeletePermission($out);
 		}
+		if (version_compare($ocVersionFromBeforeUpdate, '12.0.0.11', '<')) {
+			$this->adjustFileSharePermissions($out);
+		}
 
 		$this->removeSharesNonExistingParent($out);
 	}

lib/public/DB/QueryBuilder/IExpressionBuilder.php

@@ -305,6 +305,27 @@ interface IExpressionBuilder {
 	 */
 	public function notIn($x, $y, $type = null);
 
+
+	/**
+	 * Creates a bitwise AND comparison
+	 *
+	 * @param string|ILiteral $x The field or value to check
+	 * @param int $y Bitmap that must be set
+	 * @return IQueryFunction
+	 * @since 12.0.0
+	 */
+	public function bitwiseAnd($x, $y);
+
+	/**
+	 * Creates a bitwise OR comparison
+	 *
+	 * @param string|ILiteral $x The field or value to check
+	 * @param int $y Bitmap that must be set
+	 * @return IQueryFunction
+	 * @since 12.0.0
+	 */
+	public function bitwiseOr($x, $y);
+
 	/**
 	 * Quotes a given input parameter.
 	 *

tests/lib/Repair/RepairInvalidSharesTest.php

@@ -278,6 +278,73 @@ class RepairInvalidSharesTest extends TestCase {
 		$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
 	 */

version.php

@@ -26,7 +26,7 @@
 // between betas, final and RCs. This is _not_ the public version number. Reset minor/patchlevel
 // when updating major/minor version number.
 
-$OC_Version = array(12, 0, 0, 11);
+$OC_Version = array(12, 0, 0, 12);
 
 // The human readable string
 $OC_VersionString = '12.0 alpha';