diff options
6 files changed, 35 insertions, 21 deletions
diff --git a/apps/federatedfilesharing/tests/AddressHandlerTest.php b/apps/federatedfilesharing/tests/AddressHandlerTest.php index 75dc69dfcee..dc89316746e 100644 --- a/apps/federatedfilesharing/tests/AddressHandlerTest.php +++ b/apps/federatedfilesharing/tests/AddressHandlerTest.php @@ -74,6 +74,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 e46b71eb710..788ece70cb9 100644 --- a/lib/private/Collaboration/Collaborators/RemotePlugin.php +++ b/lib/private/Collaboration/Collaborators/RemotePlugin.php @@ -157,7 +157,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 cd5e4d511c3..3528d06a167 100644 --- a/lib/private/Federation/CloudIdManager.php +++ b/lib/private/Federation/CloudIdManager.php @@ -84,7 +84,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, ':'); @@ -107,6 +107,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)); } } @@ -152,8 +153,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); @@ -198,6 +200,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 @@ -210,7 +220,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 09ef0aae0fa..03b6ced18f5 100644 --- a/lib/public/Federation/ICloudIdManager.php +++ b/lib/public/Federation/ICloudIdManager.php @@ -48,9 +48,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 39fdd3a2d22..73116ecad68 100644 --- a/tests/lib/Collaboration/Collaborators/RemotePluginTest.php +++ b/tests/lib/Collaboration/Collaborators/RemotePluginTest.php @@ -395,6 +395,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 40477944019..13e6b111864 100644 --- a/tests/lib/Federation/CloudIdManagerTest.php +++ b/tests/lib/Federation/CloudIdManagerTest.php @@ -48,7 +48,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'], @@ -60,12 +60,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()) @@ -81,12 +77,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'], @@ -100,7 +96,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()) @@ -111,10 +107,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'], @@ -123,10 +119,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) { |