From 7e87b122712eb43d74706770cf4760b8a548ec5d Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Daniel=20Calvi=C3=B1o=20S=C3=A1nchez?= <danxuliu@gmail.com>
Date: Tue, 26 Jan 2021 10:26:10 +0100
Subject: [PATCH] Fix valid storages removed when cleaning remote storages
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

The remote URL of a share is always stored in the database with a
trailing slash. However, when a cloud ID is generated trailing slashes
are removed.

The ID of a remote storage is generated from the cloud ID, but the
"cleanup-remote-storage" command directly used the remote URL stored in
the database. Due to this, even if the remote storage was valid, its ID
did not match the ID of the remote share generated by the command and
ended being removed.

Now the command generates the ID of remote shares using the cloud ID
instead, just like done by the remote storage, so there is no longer a
mismatch.

Signed-off-by: Daniel Calviño Sánchez <danxuliu@gmail.com>
---
 .../lib/Command/CleanupRemoteStorages.php     | 16 ++++++++---
 .../Command/CleanupRemoteStoragesTest.php     | 27 ++++++++++++++++++-
 2 files changed, 39 insertions(+), 4 deletions(-)

diff --git a/apps/files_sharing/lib/Command/CleanupRemoteStorages.php b/apps/files_sharing/lib/Command/CleanupRemoteStorages.php
index db18e7d249..cf0550aef7 100644
--- a/apps/files_sharing/lib/Command/CleanupRemoteStorages.php
+++ b/apps/files_sharing/lib/Command/CleanupRemoteStorages.php
@@ -25,6 +25,7 @@
 namespace OCA\Files_Sharing\Command;
 
 use OCP\DB\QueryBuilder\IQueryBuilder;
+use OCP\Federation\ICloudIdManager;
 use OCP\IDBConnection;
 use Symfony\Component\Console\Command\Command;
 use Symfony\Component\Console\Input\InputInterface;
@@ -42,8 +43,14 @@ class CleanupRemoteStorages extends Command {
 	 */
 	protected $connection;
 
-	public function __construct(IDBConnection $connection) {
+	/**
+	 * @var ICloudIdManager
+	 */
+	private $cloudIdManager;
+
+	public function __construct(IDBConnection $connection, ICloudIdManager $cloudIdManager) {
 		$this->connection = $connection;
+		$this->cloudIdManager = $cloudIdManager;
 		parent::__construct();
 	}
 
@@ -166,14 +173,17 @@ class CleanupRemoteStorages extends Command {
 
 	public function getRemoteShareIds() {
 		$queryBuilder = $this->connection->getQueryBuilder();
-		$queryBuilder->select(['id', 'share_token', 'remote'])
+		$queryBuilder->select(['id', 'share_token', 'owner', 'remote'])
 			->from('share_external');
 		$query = $queryBuilder->execute();
 
 		$remoteShareIds = [];
 
 		while ($row = $query->fetch()) {
-			$remoteShareIds[$row['id']] = 'shared::' . md5($row['share_token'] . '@' . $row['remote']);
+			$cloudId = $this->cloudIdManager->getCloudId($row['owner'], $row['remote']);
+			$remote = $cloudId->getRemote();
+
+			$remoteShareIds[$row['id']] = 'shared::' . md5($row['share_token'] . '@' . $remote);
 		}
 
 		return $remoteShareIds;
diff --git a/apps/files_sharing/tests/Command/CleanupRemoteStoragesTest.php b/apps/files_sharing/tests/Command/CleanupRemoteStoragesTest.php
index 3176163c44..ba4b1d0549 100644
--- a/apps/files_sharing/tests/Command/CleanupRemoteStoragesTest.php
+++ b/apps/files_sharing/tests/Command/CleanupRemoteStoragesTest.php
@@ -26,6 +26,8 @@
 namespace OCA\Files_Sharing\Tests\Command;
 
 use OCA\Files_Sharing\Command\CleanupRemoteStorages;
+use OCP\Federation\ICloudId;
+use OCP\Federation\ICloudIdManager;
 use Symfony\Component\Console\Input\InputInterface;
 use Symfony\Component\Console\Output\OutputInterface;
 use Test\TestCase;
@@ -49,6 +51,11 @@ class CleanupRemoteStoragesTest extends TestCase {
 	 */
 	private $connection;
 
+	/**
+	 * @var ICloudIdManager|\PHPUnit\Framework\MockObject\MockObject
+	 */
+	private $cloudIdManager;
+
 	private $storages = [
 		['id' => 'shared::7b4a322b22f9d0047c38d77d471ce3cf', 'share_token' => 'f2c69dad1dc0649f26976fd210fc62e1', 'remote' => 'https://hostname.tld/owncloud1', 'user' => 'user1'],
 		['id' => 'shared::efe3b456112c3780da6155d3a9b9141c', 'share_token' => 'f2c69dad1dc0649f26976fd210fc62e2', 'remote' => 'https://hostname.tld/owncloud2', 'user' => 'user2'],
@@ -109,7 +116,9 @@ class CleanupRemoteStoragesTest extends TestCase {
 			}
 		}
 
-		$this->command = new CleanupRemoteStorages($this->connection);
+		$this->cloudIdManager = $this->createMock(ICloudIdManager::class);
+
+		$this->command = new CleanupRemoteStorages($this->connection, $this->cloudIdManager);
 	}
 
 	protected function tearDown(): void {
@@ -191,6 +200,22 @@ class CleanupRemoteStoragesTest extends TestCase {
 			->method('writeln')
 			->with('5 remote share(s) exist');
 
+		$this->cloudIdManager
+			->expects($this->any())
+			->method('getCloudId')
+			->will($this->returnCallback(function (string $user, string $remote) {
+				$cloudIdMock = $this->createMock(ICloudId::class);
+
+				// The remotes are already sanitized in the original data, so
+				// they can be directly returned.
+				$cloudIdMock
+					->expects($this->any())
+					->method('getRemote')
+					->willReturn($remote);
+
+				return $cloudIdMock;
+			}));
+
 		$this->command->execute($input, $output);
 
 		$this->assertTrue($this->doesStorageExist($this->storages[0]['numeric_id']));