Skip to content

Commit

Permalink
Merge pull request #25449 from nextcloud/backport/25393/stable19
Browse files Browse the repository at this point in the history
[stable19] add repair job for unencoded group share uris
  • Loading branch information
rullzer authored Feb 17, 2021
2 parents d9a6723 + 8315de9 commit 23e3730
Show file tree
Hide file tree
Showing 6 changed files with 340 additions and 1 deletion.
1 change: 1 addition & 0 deletions apps/dav/lib/DAV/Sharing/Backend.php
Original file line number Diff line number Diff line change
Expand Up @@ -194,6 +194,7 @@ public function getShares($resourceId) {
->from('dav_shares')
->where($query->expr()->eq('resourceid', $query->createNamedParameter($resourceId)))
->andWhere($query->expr()->eq('type', $query->createNamedParameter($this->resourceType)))
->groupBy(['principaluri', 'access'])
->execute();

$shares = [];
Expand Down
1 change: 1 addition & 0 deletions lib/composer/composer/autoload_classmap.php
Original file line number Diff line number Diff line change
Expand Up @@ -1190,6 +1190,7 @@
'OC\\Repair\\Owncloud\\DropAccountTermsTable' => $baseDir . '/lib/private/Repair/Owncloud/DropAccountTermsTable.php',
'OC\\Repair\\Owncloud\\SaveAccountsTableData' => $baseDir . '/lib/private/Repair/Owncloud/SaveAccountsTableData.php',
'OC\\Repair\\RemoveLinkShares' => $baseDir . '/lib/private/Repair/RemoveLinkShares.php',
'OC\\Repair\\RepairDavShares' => $baseDir . '/lib/private/Repair/RepairDavShares.php',
'OC\\Repair\\RepairInvalidShares' => $baseDir . '/lib/private/Repair/RepairInvalidShares.php',
'OC\\Repair\\RepairMimeTypes' => $baseDir . '/lib/private/Repair/RepairMimeTypes.php',
'OC\\Repair\\SqliteAutoincrement' => $baseDir . '/lib/private/Repair/SqliteAutoincrement.php',
Expand Down
1 change: 1 addition & 0 deletions lib/composer/composer/autoload_static.php
Original file line number Diff line number Diff line change
Expand Up @@ -1219,6 +1219,7 @@ class ComposerStaticInit53792487c5a8370acc0b06b1a864ff4c
'OC\\Repair\\Owncloud\\DropAccountTermsTable' => __DIR__ . '/../../..' . '/lib/private/Repair/Owncloud/DropAccountTermsTable.php',
'OC\\Repair\\Owncloud\\SaveAccountsTableData' => __DIR__ . '/../../..' . '/lib/private/Repair/Owncloud/SaveAccountsTableData.php',
'OC\\Repair\\RemoveLinkShares' => __DIR__ . '/../../..' . '/lib/private/Repair/RemoveLinkShares.php',
'OC\\Repair\\RepairDavShares' => __DIR__ . '/../../..' . '/lib/private/Repair/RepairDavShares.php',
'OC\\Repair\\RepairInvalidShares' => __DIR__ . '/../../..' . '/lib/private/Repair/RepairInvalidShares.php',
'OC\\Repair\\RepairMimeTypes' => __DIR__ . '/../../..' . '/lib/private/Repair/RepairMimeTypes.php',
'OC\\Repair\\SqliteAutoincrement' => __DIR__ . '/../../..' . '/lib/private/Repair/SqliteAutoincrement.php',
Expand Down
4 changes: 3 additions & 1 deletion lib/private/Repair.php
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,7 @@
use OC\Repair\Owncloud\DropAccountTermsTable;
use OC\Repair\Owncloud\SaveAccountsTableData;
use OC\Repair\RemoveLinkShares;
use OC\Repair\RepairDavShares;
use OC\Repair\RepairInvalidShares;
use OC\Repair\RepairMimeTypes;
use OC\Repair\SqliteAutoincrement;
Expand Down Expand Up @@ -156,6 +157,7 @@ public static function getRepairSteps() {
new RemoveLinkShares(\OC::$server->getDatabaseConnection(), \OC::$server->getConfig(), \OC::$server->getGroupManager(), \OC::$server->getNotificationManager(), \OC::$server->query(ITimeFactory::class)),
new ClearCollectionsAccessCache(\OC::$server->getConfig(), \OC::$server->query(IManager::class)),
\OC::$server->query(ResetGeneratedAvatarFlag::class),
\OC::$server->query(RepairDavShares::class)
];
}

Expand Down Expand Up @@ -184,7 +186,7 @@ public static function getBeforeUpgradeRepairSteps() {
new Collation(\OC::$server->getConfig(), \OC::$server->getLogger(), $connection, true),
new SqliteAutoincrement($connection),
new SaveAccountsTableData($connection, $config),
new DropAccountTermsTable($connection)
new DropAccountTermsTable($connection),
];

return $steps;
Expand Down
138 changes: 138 additions & 0 deletions lib/private/Repair/RepairDavShares.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,138 @@
<?php

declare(strict_types=1);
/**
* @copyright Copyright (c) 2020 Arthur Schiwon <[email protected]>
*
* @author Arthur Schiwon <[email protected]>
*
* @license GNU AGPL version 3 or any later version
*
* This program is free software: you can redistribute it and/or modify
* it under the terms of the GNU Affero General Public License as
* published by the Free Software Foundation, either version 3 of the
* License, or (at your option) any later version.
*
* This program is distributed in the hope that it will be useful,
* but WITHOUT ANY WARRANTY; without even the implied warranty of
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
* GNU Affero General Public License for more details.
*
* You should have received a copy of the GNU Affero General Public License
* along with this program. If not, see <http://www.gnu.org/licenses/>.
*
*/

namespace OC\Repair;

use Doctrine\DBAL\DBALException;
use OCP\IConfig;
use OCP\IDBConnection;
use OCP\IGroupManager;
use OCP\Migration\IOutput;
use OCP\Migration\IRepairStep;
use Psr\Log\LoggerInterface;
use function strlen;
use function substr;
use function urldecode;
use function urlencode;

class RepairDavShares implements IRepairStep {
protected const GROUP_PRINCIPAL_PREFIX = 'principals/groups/';

/** @var IConfig */
private $config;
/** @var IDBConnection */
private $dbc;
/** @var IGroupManager */
private $groupManager;
/** @var LoggerInterface */
private $logger;
/** @var bool */
private $hintInvalidShares = false;

public function __construct(
IConfig $config,
IDBConnection $dbc,
IGroupManager $groupManager,
LoggerInterface $logger
) {
$this->config = $config;
$this->dbc = $dbc;
$this->groupManager = $groupManager;
$this->logger = $logger;
}

/**
* @inheritDoc
*/
public function getName() {
return 'Repair DAV shares';
}

protected function repairUnencodedGroupShares() {
$qb = $this->dbc->getQueryBuilder();
$qb->select(['id', 'principaluri'])
->from('dav_shares')
->where($qb->expr()->like('principaluri', $qb->createNamedParameter(self::GROUP_PRINCIPAL_PREFIX . '%')));

$updateQuery = $this->dbc->getQueryBuilder();
$updateQuery->update('dav_shares')
->set('principaluri', $updateQuery->createParameter('updatedPrincipalUri'))
->where($updateQuery->expr()->eq('id', $updateQuery->createParameter('shareId')));

$statement = $qb->execute();
while ($share = $statement->fetch()) {
$gid = substr($share['principaluri'], strlen(self::GROUP_PRINCIPAL_PREFIX));
$decodedGid = urldecode($gid);
$encodedGid = urlencode($gid);
if ($gid === $encodedGid
|| !$this->groupManager->groupExists($gid)
|| ($gid !== $decodedGid && $this->groupManager->groupExists($decodedGid))
) {
$this->hintInvalidShares = $this->hintInvalidShares || $gid !== $encodedGid;
continue;
}

// Repair when
// + the group name needs encoding
// + AND it is not encoded yet
// + AND there are no ambivalent groups

try {
$fixedPrincipal = self::GROUP_PRINCIPAL_PREFIX . $encodedGid;
$logParameters = [
'app' => 'core',
'id' => $share['id'],
'old' => $share['principaluri'],
'new' => $fixedPrincipal,
];
$updateQuery
->setParameter('updatedPrincipalUri', $fixedPrincipal)
->setParameter('shareId', $share['id'])
->execute();
$this->logger->info('Repaired principal for dav share {id} from {old} to {new}', $logParameters);
} catch (DBALException $e) {
$logParameters['message'] = $e->getMessage();
$logParameters['exception'] = $e;
$this->logger->info('Could not repair principal for dav share {id} from {old} to {new}: {message}', $logParameters);
}
}
return true;
}

/**
* @inheritDoc
*/
public function run(IOutput $output) {
$versionFromBeforeUpdate = $this->config->getSystemValue('version', '0.0.0');
if (version_compare($versionFromBeforeUpdate, '19.0.9', '<')
&& $this->repairUnencodedGroupShares()
) {
$output->info('Repaired DAV group shares');
if ($this->hintInvalidShares) {
$output->info('Invalid shares might be left in the database, running "occ dav:remove-invalid-shares" can remove them.');
}
}
}
}
196 changes: 196 additions & 0 deletions tests/lib/Repair/RepairDavSharesTest.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,196 @@
<?php

declare(strict_types=1);
/**
* @copyright Copyright (c) 2021 Arthur Schiwon <[email protected]>
*
* @author Arthur Schiwon <[email protected]>
*
* @license GNU AGPL version 3 or any later version
*
* This program is free software: you can redistribute it and/or modify
* it under the terms of the GNU Affero General Public License as
* published by the Free Software Foundation, either version 3 of the
* License, or (at your option) any later version.
*
* This program is distributed in the hope that it will be useful,
* but WITHOUT ANY WARRANTY; without even the implied warranty of
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
* GNU Affero General Public License for more details.
*
* You should have received a copy of the GNU Affero General Public License
* along with this program. If not, see <http://www.gnu.org/licenses/>.
*
*/

namespace Test\Repair;

use Doctrine\DBAL\Driver\Statement;
use OC\Repair\RepairDavShares;
use OCP\DB\QueryBuilder\IExpressionBuilder;
use OCP\DB\QueryBuilder\IQueryBuilder;
use OCP\IConfig;
use OCP\IDBConnection;
use OCP\IGroupManager;
use Psr\Log\LoggerInterface;
use Test\TestCase;
use OCP\Migration\IOutput;
use function in_array;

class RepairDavSharesTest extends TestCase {

/** @var IOutput|\PHPUnit\Framework\MockObject\MockObject */
protected $output;
/** @var IConfig|\PHPUnit\Framework\MockObject\MockObject */
protected $config;
/** @var IDBConnection|\PHPUnit\Framework\MockObject\MockObject */
protected $dbc;
/** @var IGroupManager|\PHPUnit\Framework\MockObject\MockObject */
protected $groupManager;
/** @var \PHPUnit\Framework\MockObject\MockObject|LoggerInterface */
protected $logger;
/** @var RepairDavSharesTest */
protected $repair;

public function setUp(): void {
parent::setUp();

$this->output = $this->createMock(IOutput::class);

$this->config = $this->createMock(IConfig::class);
$this->dbc = $this->createMock(IDBConnection::class);
$this->groupManager = $this->createMock(IGroupManager::class);
$this->logger = $this->createMock(LoggerInterface::class);

$this->repair = new RepairDavShares(
$this->config,
$this->dbc,
$this->groupManager,
$this->logger
);
}

public function testRun() {
$this->config->expects($this->any())
->method('getSystemValue')
->with('version', '0.0.0')
->willReturn('19.0.2');

$this->output->expects($this->atLeastOnce())
->method('info');

$existingGroups = [
'Innocent',
'Wants Repair',
'Well förmed',
'family+friends',
'family friends',
];

$shareResultData = [
[
// No update, nothing to escape
'id' => 0,
'principaluri' => 'principals/groups/Innocent',
],
[
// Update
'id' => 1,
'principaluri' => 'principals/groups/Wants Repair',
],
[
// No update, already proper
'id' => 2,
'principaluri' => 'principals/groups/Well+f%C3%B6rmed',
],
[
// No update, unknown group
'id' => 3,
'principaluri' => 'principals/groups/Not known',
],
[
// No update, unknown group
'id' => 4,
'principaluri' => 'principals/groups/Also%2F%2FNot%23Known',
],
[
// No update, group exists in both forms
'id' => 5,
'principaluri' => 'principals/groups/family+friends',
],
[
// No update, already proper
'id' => 6,
'principaluri' => 'principals/groups/family%2Bfriends',
],
[
// Update
'id' => 7,
'principaluri' => 'principals/groups/family friends',
],
];

$shareResults = $this->createMock(Statement::class);
$shareResults->expects($this->any())
->method('fetch')
->willReturnCallback(function () use (&$shareResultData) {
return array_pop($shareResultData);
});

$expressionBuilder = $this->createMock(IExpressionBuilder::class);

$selectMock = $this->createMock(IQueryBuilder::class);
$selectMock->expects($this->any())
->method('expr')
->willReturn($expressionBuilder);
$selectMock->expects($this->once())
->method('select')
->willReturnSelf();
$selectMock->expects($this->once())
->method('from')
->willReturnSelf();
$selectMock->expects($this->once())
->method('where')
->willReturnSelf();
$selectMock->expects($this->once())
->method('execute')
->willReturn($shareResults);

$updateMock = $this->createMock(IQueryBuilder::class);
$updateMock->expects($this->any())
->method('expr')
->willReturn($expressionBuilder);
$updateMock->expects($this->once())
->method('update')
->willReturnSelf();
$updateMock->expects($this->any())
->method('set')
->willReturnSelf();
$updateMock->expects($this->once())
->method('where')
->willReturnSelf();
$updateMock->expects($this->exactly(4))
->method('setParameter')
->withConsecutive(
['updatedPrincipalUri', 'principals/groups/' . urlencode('family friends')],
['shareId', 7],
['updatedPrincipalUri', 'principals/groups/' . urlencode('Wants Repair')],
['shareId', 1]
)
->willReturnSelf();
$updateMock->expects($this->exactly(2))
->method('execute');

$this->dbc->expects($this->atLeast(2))
->method('getQueryBuilder')
->willReturnOnConsecutiveCalls($selectMock, $updateMock);

$this->groupManager->expects($this->any())
->method('groupExists')
->willReturnCallback(function (string $gid) use ($existingGroups) {
return in_array($gid, $existingGroups);
});

$this->repair->run($this->output);
}
}

0 comments on commit 23e3730

Please sign in to comment.