fix(federation): Fix missing protocol on CloudID remote

Signed-off-by: Joas Schilling <coding@schilljs.com>

6 files changed, 35 insertions, 21 deletions

diff --git a/apps/federatedfilesharing/tests/AddressHandlerTest.php b/apps/federatedfilesharing/tests/AddressHandlerTest.php
index 8cfe758a364..d3737a12ea0 100644
--- a/apps/federatedfilesharing/tests/AddressHandlerTest.php
+++ b/apps/federatedfilesharing/tests/AddressHandlerTest.php
@@ -94,6 +94,11 @@ class AddressHandlerTest extends \Test\TestCase {
 				foreach ($protocols as $protocol) {
 					$baseUrl = $user . '@' . $protocol . $remote;
 
+					if ($protocol === '') {
+						// https:// protocol is expected in the final result
+						$protocol = 'https://';
+					}
+
 					$testCases[] = [$baseUrl, $user, $protocol . $remote];
 					$testCases[] = [$baseUrl . '/', $user, $protocol . $remote];
 					$testCases[] = [$baseUrl . '/index.php', $user, $protocol . $remote];
diff --git a/lib/private/Collaboration/Collaborators/RemotePlugin.php b/lib/private/Collaboration/Collaborators/RemotePlugin.php
index a0868796689..75ad5e85ec6 100644
--- a/lib/private/Collaboration/Collaborators/RemotePlugin.php
+++ b/lib/private/Collaboration/Collaborators/RemotePlugin.php
@@ -178,7 +178,7 @@ class RemotePlugin implements ISearchPlugin {
 	public function splitUserRemote(string $address): array {
 		try {
 			$cloudId = $this->cloudIdManager->resolveCloudId($address);
-			return [$cloudId->getUser(), $cloudId->getRemote()];
+			return [$cloudId->getUser(), $this->cloudIdManager->removeProtocolFromUrl($cloudId->getRemote(), true)];
 		} catch (\InvalidArgumentException $e) {
 			throw new \InvalidArgumentException('Invalid Federated Cloud ID', 0, $e);
 		}
diff --git a/lib/private/Federation/CloudIdManager.php b/lib/private/Federation/CloudIdManager.php
index 86b5ed621ba..9853aeaa1ac 100644
--- a/lib/private/Federation/CloudIdManager.php
+++ b/lib/private/Federation/CloudIdManager.php
@@ -106,7 +106,7 @@ class CloudIdManager implements ICloudIdManager {
 		}
 
 		// Find the first character that is not allowed in user names
-		$id = $this->fixRemoteURL($cloudId);
+		$id = $this->stripShareLinkFragments($cloudId);
 		$posSlash = strpos($id, '/');
 		$posColon = strpos($id, ':');
 
@@ -129,6 +129,7 @@ class CloudIdManager implements ICloudIdManager {
 			$this->userManager->validateUserId($user);
 
 			if (!empty($user) && !empty($remote)) {
+				$remote = $this->ensureDefaultProtocol($remote);
 				return new CloudId($id, $user, $remote, $this->getDisplayNameFromContact($id));
 			}
 		}
@@ -174,8 +175,9 @@ class CloudIdManager implements ICloudIdManager {
 		// note that for remote id's we don't strip the protocol for the remote we use to construct the CloudId
 		// this way if a user has an explicit non-https cloud id this will be preserved
 		// we do still use the version without protocol for looking up the display name
-		$remote = $this->fixRemoteURL($remote);
+		$remote = $this->stripShareLinkFragments($remote);
 		$host = $this->removeProtocolFromUrl($remote);
+		$remote = $this->ensureDefaultProtocol($remote);
 
 		$key = $user . '@' . ($isLocal ? 'local' : $host);
 		$cached = $this->cache[$key] ?? $this->memCache->get($key);
@@ -220,6 +222,14 @@ class CloudIdManager implements ICloudIdManager {
 		return $url;
 	}
 
+	protected function ensureDefaultProtocol(string $remote): string {
+		if (!str_contains($remote, '://')) {
+			$remote = 'https://' . $remote;
+		}
+
+		return $remote;
+	}
+
 	/**
 	 * Strips away a potential file names and trailing slashes:
 	 * - http://localhost
@@ -232,7 +242,7 @@ class CloudIdManager implements ICloudIdManager {
 	 * @param string $remote
 	 * @return string
 	 */
-	protected function fixRemoteURL(string $remote): string {
+	protected function stripShareLinkFragments(string $remote): string {
 		$remote = str_replace('\\', '/', $remote);
 		if ($fileNamePosition = strpos($remote, '/index.php')) {
 			$remote = substr($remote, 0, $fileNamePosition);
diff --git a/lib/public/Federation/ICloudIdManager.php b/lib/public/Federation/ICloudIdManager.php
index 52920751739..00fa44f9427 100644
--- a/lib/public/Federation/ICloudIdManager.php
+++ b/lib/public/Federation/ICloudIdManager.php
@@ -67,9 +67,11 @@ interface ICloudIdManager {
 	 * remove scheme/protocol from an url
 	 *
 	 * @param string $url
+	 * @param bool $httpsOnly
 	 *
 	 * @return string
 	 * @since 28.0.0
+	 * @since 30.0.0 - Optional parameter $httpsOnly was added
 	 */
-	public function removeProtocolFromUrl(string $url): string;
+	public function removeProtocolFromUrl(string $url, bool $httpsOnly = false): string;
 }
diff --git a/tests/lib/Collaboration/Collaborators/RemotePluginTest.php b/tests/lib/Collaboration/Collaborators/RemotePluginTest.php
index 4fed1f179e7..bc5e8982f2a 100644
--- a/tests/lib/Collaboration/Collaborators/RemotePluginTest.php
+++ b/tests/lib/Collaboration/Collaborators/RemotePluginTest.php
@@ -412,6 +412,11 @@ class RemotePluginTest extends TestCase {
 				foreach ($protocols as $protocol) {
 					$baseUrl = $user . '@' . $protocol . $remote;
 
+					if ($protocol === 'https://') {
+						// https:// protocol is not expected in the final result
+						$protocol = '';
+					}
+
 					$testCases[] = [$baseUrl, $user, $protocol . $remote];
 					$testCases[] = [$baseUrl . '/', $user, $protocol . $remote];
 					$testCases[] = [$baseUrl . '/index.php', $user, $protocol . $remote];
diff --git a/tests/lib/Federation/CloudIdManagerTest.php b/tests/lib/Federation/CloudIdManagerTest.php
index a144dbb7800..1b9b5bfb971 100644
--- a/tests/lib/Federation/CloudIdManagerTest.php
+++ b/tests/lib/Federation/CloudIdManagerTest.php
@@ -63,7 +63,7 @@ class CloudIdManagerTest extends TestCase {
 		);
 	}
 
-	public function cloudIdProvider() {
+	public function cloudIdProvider(): array {
 		return [
 			['test@example.com', 'test', 'example.com', 'test@example.com'],
 			['test@example.com/cloud', 'test', 'example.com/cloud', 'test@example.com/cloud'],
@@ -75,12 +75,8 @@ class CloudIdManagerTest extends TestCase {
 
 	/**
 	 * @dataProvider cloudIdProvider
-	 *
-	 * @param string $cloudId
-	 * @param string $user
-	 * @param string $remote
 	 */
-	public function testResolveCloudId($cloudId, $user, $remote, $cleanId) {
+	public function testResolveCloudId(string $cloudId, string $user, string $noProtocolRemote, string $cleanId): void {
 		$displayName = 'Ample Ex';
 
 		$this->contactsManager->expects($this->any())
@@ -96,12 +92,12 @@ class CloudIdManagerTest extends TestCase {
 		$cloudId = $this->cloudIdManager->resolveCloudId($cloudId);
 
 		$this->assertEquals($user, $cloudId->getUser());
-		$this->assertEquals($remote, $cloudId->getRemote());
+		$this->assertEquals('https://' . $noProtocolRemote, $cloudId->getRemote());
 		$this->assertEquals($cleanId, $cloudId->getId());
-		$this->assertEquals($displayName . '@' . $remote, $cloudId->getDisplayId());
+		$this->assertEquals($displayName . '@' . $noProtocolRemote, $cloudId->getDisplayId());
 	}
 
-	public function invalidCloudIdProvider() {
+	public function invalidCloudIdProvider(): array {
 		return [
 			['example.com'],
 			['test:foo@example.com'],
@@ -115,7 +111,7 @@ class CloudIdManagerTest extends TestCase {
 	 * @param string $cloudId
 	 *
 	 */
-	public function testInvalidCloudId($cloudId) {
+	public function testInvalidCloudId(string $cloudId): void {
 		$this->expectException(\InvalidArgumentException::class);
 
 		$this->contactsManager->expects($this->never())
@@ -126,10 +122,10 @@ class CloudIdManagerTest extends TestCase {
 
 	public function getCloudIdProvider(): array {
 		return [
-			['test', 'example.com', 'test@example.com'],
+			['test', 'example.com', 'test@example.com', null, 'https://example.com', 'https://example.com'],
 			['test', 'http://example.com', 'test@http://example.com', 'test@example.com'],
 			['test', null, 'test@http://example.com', 'test@example.com', 'http://example.com', 'http://example.com'],
-			['test@example.com', 'example.com', 'test@example.com@example.com'],
+			['test@example.com', 'example.com', 'test@example.com@example.com', null, 'https://example.com', 'https://example.com'],
 			['test@example.com', 'https://example.com', 'test@example.com@example.com'],
 			['test@example.com', null, 'test@example.com@example.com', null, 'https://example.com', 'https://example.com'],
 			['test@example.com', 'https://example.com/index.php/s/shareToken', 'test@example.com@example.com', null, 'https://example.com', 'https://example.com'],
@@ -138,10 +134,6 @@ class CloudIdManagerTest extends TestCase {
 
 	/**
 	 * @dataProvider getCloudIdProvider
-	 *
-	 * @param string $user
-	 * @param null|string $remote
-	 * @param string $id
 	 */
 	public function testGetCloudId(string $user, ?string $remote, string $id, ?string $searchCloudId = null, ?string $localHost = 'https://example.com', ?string $expectedRemoteId = null): void {
 		if ($remote !== null) {