fix(federation): Fix missing protocol on CloudID remote

Signed-off-by: Joas Schilling <[email protected]>

Author: nickvergessen

lib/private/Federation/CloudIdManager.php

@@ -84,7 +84,7 @@ public function resolveCloudId(string $cloudId): ICloudId {
 		}
 
 		// 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, ':');
 
@@ -103,6 +103,7 @@ public function resolveCloudId(string $cloudId): ICloudId {
 		if ($lastValidAtPos !== false) {
 			$user = substr($id, 0, $lastValidAtPos);
 			$remote = substr($id, $lastValidAtPos + 1);
+			$remote = $this->ensureDefaultProtocol($remote);
 
 			$this->userManager->validateUserId($user);
 
@@ -152,8 +153,9 @@ public function getCloudId(string $user, ?string $remote): ICloudId {
 		// 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);
@@ -198,6 +200,14 @@ public function removeProtocolFromUrl(string $url, bool $httpsOnly = false): str
 		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
@@ -210,7 +220,7 @@ public function removeProtocolFromUrl(string $url, bool $httpsOnly = false): str
 	 * @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);

tests/lib/Federation/CloudIdManagerTest.php

@@ -48,7 +48,7 @@ protected function setUp(): void {
 		);
 	}
 
-	public function cloudIdProvider() {
+	public function cloudIdProvider(): array {
 		return [
 			['[email protected]', 'test', 'example.com', '[email protected]'],
 			['[email protected]/cloud', 'test', 'example.com/cloud', '[email protected]/cloud'],
@@ -60,12 +60,8 @@ public function cloudIdProvider() {
 
 	/**
 	 * @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())
@@ -81,12 +77,12 @@ public function testResolveCloudId($cloudId, $user, $remote, $cleanId) {
 		$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:[email protected]'],
@@ -100,7 +96,7 @@ public function invalidCloudIdProvider() {
 	 * @param string $cloudId
 	 *
 	 */
-	public function testInvalidCloudId($cloudId) {
+	public function testInvalidCloudId(string $cloudId): void {
 		$this->expectException(\InvalidArgumentException::class);
 
 		$this->contactsManager->expects($this->never())
@@ -111,10 +107,10 @@ public function testInvalidCloudId($cloudId) {
 
 	public function getCloudIdProvider(): array {
 		return [
-			['test', 'example.com', '[email protected]'],
+			['test', 'example.com', '[email protected]', null, 'https://example.com', 'https://example.com'],
 			['test', 'http://example.com', 'test@http://example.com', '[email protected]'],
 			['test', null, 'test@http://example.com', '[email protected]', 'http://example.com', 'http://example.com'],
-			['[email protected]', 'example.com', '[email protected]@example.com'],
+			['[email protected]', 'example.com', '[email protected]@example.com', null, 'https://example.com', 'https://example.com'],
 			['[email protected]', 'https://example.com', '[email protected]@example.com'],
 			['[email protected]', null, '[email protected]@example.com', null, 'https://example.com', 'https://example.com'],
 			['[email protected]', 'https://example.com/index.php/s/shareToken', '[email protected]@example.com', null, 'https://example.com', 'https://example.com'],
@@ -123,10 +119,6 @@ public function getCloudIdProvider(): array {
 
 	/**
 	 * @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) {