Skip to content

Commit 280d70a

Browse files
nickvergessennickvergessen
committed
fix(federation): Fix missing protocol on CloudID remote
Signed-off-by: Joas Schilling <coding@schilljs.com>
1 parent 5dcb807 commit 280d70a

File tree

6 files changed

+35
-21
lines changed

6 files changed

+35
-21
lines changed

‎apps/federatedfilesharing/tests/AddressHandlerTest.php‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -74,6 +74,11 @@ public function dataTestSplitUserRemote() {
7474
foreach ($protocols as $protocol) {
7575
$baseUrl = $user . '@' . $protocol . $remote;
7676

77+
if ($protocol === '') {
78+
// https:// protocol is expected in the final result
79+
$protocol = 'https://';
80+
}
81+
7782
$testCases[] = [$baseUrl, $user, $protocol . $remote];
7883
$testCases[] = [$baseUrl . '/', $user, $protocol . $remote];
7984
$testCases[] = [$baseUrl . '/index.php', $user, $protocol . $remote];

‎lib/private/Collaboration/Collaborators/RemotePlugin.php‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -157,7 +157,7 @@ public function search($search, $limit, $offset, ISearchResult $searchResult): b
157157
public function splitUserRemote(string $address): array {
158158
try {
159159
$cloudId = $this->cloudIdManager->resolveCloudId($address);
160-
return [$cloudId->getUser(), $cloudId->getRemote()];
160+
return [$cloudId->getUser(), $this->cloudIdManager->removeProtocolFromUrl($cloudId->getRemote(), true)];
161161
} catch (\InvalidArgumentException $e) {
162162
throw new \InvalidArgumentException('Invalid Federated Cloud ID', 0, $e);
163163
}

‎lib/private/Federation/CloudIdManager.php‎

Lines changed: 13 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -84,7 +84,7 @@ public function resolveCloudId(string $cloudId): ICloudId {
8484
}
8585

8686
// Find the first character that is not allowed in user names
87-
$id = $this->fixRemoteURL($cloudId);
87+
$id = $this->stripShareLinkFragments($cloudId);
8888
$posSlash = strpos($id, '/');
8989
$posColon = strpos($id, ':');
9090

@@ -107,6 +107,7 @@ public function resolveCloudId(string $cloudId): ICloudId {
107107
$this->userManager->validateUserId($user);
108108

109109
if (!empty($user) && !empty($remote)) {
110+
$remote = $this->ensureDefaultProtocol($remote);
110111
return new CloudId($id, $user, $remote, $this->getDisplayNameFromContact($id));
111112
}
112113
}
@@ -152,8 +153,9 @@ public function getCloudId(string $user, ?string $remote): ICloudId {
152153
// note that for remote id's we don't strip the protocol for the remote we use to construct the CloudId
153154
// this way if a user has an explicit non-https cloud id this will be preserved
154155
// we do still use the version without protocol for looking up the display name
155-
$remote = $this->fixRemoteURL($remote);
156+
$remote = $this->stripShareLinkFragments($remote);
156157
$host = $this->removeProtocolFromUrl($remote);
158+
$remote = $this->ensureDefaultProtocol($remote);
157159

158160
$key = $user . '@' . ($isLocal ? 'local' : $host);
159161
$cached = $this->cache[$key] ?? $this->memCache->get($key);
@@ -198,6 +200,14 @@ public function removeProtocolFromUrl(string $url, bool $httpsOnly = false): str
198200
return $url;
199201
}
200202

203+
protected function ensureDefaultProtocol(string $remote): string {
204+
if (!str_contains($remote, '://')) {
205+
$remote = 'https://' . $remote;
206+
}
207+
208+
return $remote;
209+
}
210+
201211
/**
202212
* Strips away a potential file names and trailing slashes:
203213
* - http://localhost
@@ -210,7 +220,7 @@ public function removeProtocolFromUrl(string $url, bool $httpsOnly = false): str
210220
* @param string $remote
211221
* @return string
212222
*/
213-
protected function fixRemoteURL(string $remote): string {
223+
protected function stripShareLinkFragments(string $remote): string {
214224
$remote = str_replace('\\', '/', $remote);
215225
if ($fileNamePosition = strpos($remote, '/index.php')) {
216226
$remote = substr($remote, 0, $fileNamePosition);

‎lib/public/Federation/ICloudIdManager.php‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -48,9 +48,11 @@ public function isValidCloudId(string $cloudId): bool;
4848
* remove scheme/protocol from an url
4949
*
5050
* @param string $url
51+
* @param bool $httpsOnly
5152
*
5253
* @return string
5354
* @since 28.0.0
55+
* @since 30.0.0 - Optional parameter $httpsOnly was added
5456
*/
55-
public function removeProtocolFromUrl(string $url): string;
57+
public function removeProtocolFromUrl(string $url, bool $httpsOnly = false): string;
5658
}

‎tests/lib/Collaboration/Collaborators/RemotePluginTest.php‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -395,6 +395,11 @@ public function dataTestSplitUserRemote() {
395395
foreach ($protocols as $protocol) {
396396
$baseUrl = $user . '@' . $protocol . $remote;
397397

398+
if ($protocol === 'https://') {
399+
// https:// protocol is not expected in the final result
400+
$protocol = '';
401+
}
402+
398403
$testCases[] = [$baseUrl, $user, $protocol . $remote];
399404
$testCases[] = [$baseUrl . '/', $user, $protocol . $remote];
400405
$testCases[] = [$baseUrl . '/index.php', $user, $protocol . $remote];

‎tests/lib/Federation/CloudIdManagerTest.php‎

Lines changed: 8 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -48,7 +48,7 @@ protected function setUp(): void {
4848
);
4949
}
5050

51-
public function cloudIdProvider() {
51+
public function cloudIdProvider(): array {
5252
return [
5353
['test@example.com', 'test', 'example.com', 'test@example.com'],
5454
['test@example.com/cloud', 'test', 'example.com/cloud', 'test@example.com/cloud'],
@@ -60,12 +60,8 @@ public function cloudIdProvider() {
6060

6161
/**
6262
* @dataProvider cloudIdProvider
63-
*
64-
* @param string $cloudId
65-
* @param string $user
66-
* @param string $remote
6763
*/
68-
public function testResolveCloudId($cloudId, $user, $remote, $cleanId) {
64+
public function testResolveCloudId(string $cloudId, string $user, string $noProtocolRemote, string $cleanId): void {
6965
$displayName = 'Ample Ex';
7066

7167
$this->contactsManager->expects($this->any())
@@ -81,12 +77,12 @@ public function testResolveCloudId($cloudId, $user, $remote, $cleanId) {
8177
$cloudId = $this->cloudIdManager->resolveCloudId($cloudId);
8278

8379
$this->assertEquals($user, $cloudId->getUser());
84-
$this->assertEquals($remote, $cloudId->getRemote());
80+
$this->assertEquals('https://' . $noProtocolRemote, $cloudId->getRemote());
8581
$this->assertEquals($cleanId, $cloudId->getId());
86-
$this->assertEquals($displayName . '@' . $remote, $cloudId->getDisplayId());
82+
$this->assertEquals($displayName . '@' . $noProtocolRemote, $cloudId->getDisplayId());
8783
}
8884

89-
public function invalidCloudIdProvider() {
85+
public function invalidCloudIdProvider(): array {
9086
return [
9187
['example.com'],
9288
['test:foo@example.com'],
@@ -100,7 +96,7 @@ public function invalidCloudIdProvider() {
10096
* @param string $cloudId
10197
*
10298
*/
103-
public function testInvalidCloudId($cloudId) {
99+
public function testInvalidCloudId(string $cloudId): void {
104100
$this->expectException(\InvalidArgumentException::class);
105101

106102
$this->contactsManager->expects($this->never())
@@ -111,10 +107,10 @@ public function testInvalidCloudId($cloudId) {
111107

112108
public function getCloudIdProvider(): array {
113109
return [
114-
['test', 'example.com', 'test@example.com'],
110+
['test', 'example.com', 'test@example.com', null, 'https://example.com', 'https://example.com'],
115111
['test', 'http://example.com', 'test@http://example.com', 'test@example.com'],
116112
['test', null, 'test@http://example.com', 'test@example.com', 'http://example.com', 'http://example.com'],
117-
['test@example.com', 'example.com', 'test@example.com@example.com'],
113+
['test@example.com', 'example.com', 'test@example.com@example.com', null, 'https://example.com', 'https://example.com'],
118114
['test@example.com', 'https://example.com', 'test@example.com@example.com'],
119115
['test@example.com', null, 'test@example.com@example.com', null, 'https://example.com', 'https://example.com'],
120116
['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 @@ public function getCloudIdProvider(): array {
123119

124120
/**
125121
* @dataProvider getCloudIdProvider
126-
*
127-
* @param string $user
128-
* @param null|string $remote
129-
* @param string $id
130122
*/
131123
public function testGetCloudId(string $user, ?string $remote, string $id, ?string $searchCloudId = null, ?string $localHost = 'https://example.com', ?string $expectedRemoteId = null): void {
132124
if ($remote !== null) {

0 commit comments

Comments
 (0)