Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Respect sharing.federation.allowHttpFallback config option #37153

Merged
merged 1 commit into from
Mar 24, 2020
Merged

Respect sharing.federation.allowHttpFallback config option #37153

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 9 additions & 8 deletions apps/federatedfilesharing/lib/Notifications.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -330,15 +330,15 @@ protected function tryHttpPostToShareEndpoint($remoteDomain, $urlSuffix, array $
$try = 0;

while ($result['success'] === false && $try < 2) {
if ($useOcm) {
$endpoint = $this->discoveryManager->getOcmShareEndpoint($protocol . $remoteDomain);
$endpoint .= $urlSuffix;
} else {
$relativePath = $this->discoveryManager->getShareEndpoint($protocol . $remoteDomain);
$endpoint = $protocol . $remoteDomain . $relativePath . $urlSuffix . '?format=' . self::RESPONSE_FORMAT;
}

try {
if ($useOcm) {
$endpoint = $this->discoveryManager->getOcmShareEndpoint($protocol . $remoteDomain);
$endpoint .= $urlSuffix;
} else {
$relativePath = $this->discoveryManager->getShareEndpoint($protocol . $remoteDomain);
$endpoint = $protocol . $remoteDomain . $relativePath . $urlSuffix . '?format=' . self::RESPONSE_FORMAT;
}

$options = [
'timeout' => 10,
'connect_timeout' => 10,
Expand All @@ -364,6 +364,7 @@ protected function tryHttpPostToShareEndpoint($remoteDomain, $urlSuffix, array $
if ($e->getCode() === Http::STATUS_INTERNAL_SERVER_ERROR) {
throw $e;
}

$allowHttpFallback = $this->config->getSystemValue('sharing.federation.allowHttpFallback', false) === true;
if (!$allowHttpFallback) {
break;
Expand Down
122 changes: 101 additions & 21 deletions apps/federatedfilesharing/tests/NotificationsTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,9 @@
/**
* @author Björn Schießle <[email protected]>
* @author Thomas Müller <[email protected]>
* @author Piotr Mrowczynski <[email protected]>
*
* @copyright Copyright (c) 2018, ownCloud GmbH
* @copyright Copyright (c) 2020, ownCloud GmbH
* @license AGPL-3.0
*
* This code is free software: you can redistribute it and/or modify
Expand All @@ -23,6 +24,7 @@
namespace OCA\FederatedFileSharing\Tests;

use GuzzleHttp\Exception\ClientException;
use GuzzleHttp\Exception\RequestException;
use OC\AppFramework\Http;
use OCA\FederatedFileSharing\AddressHandler;
use OCA\FederatedFileSharing\DiscoveryManager;
Expand Down Expand Up @@ -207,50 +209,128 @@ function ($options) {
* @dataProvider dataTryHttpPostToShareEndpointInException
*
* @param $exception
* @param $isDiscoveryException
* @param $allowHttpFallback
* @param $expectedTries
* @param array $expected
*/
public function testTryHttpPostToShareEndpointInException($exception, $expected) {
public function testTryHttpPostToShareEndpointInException($exception, $isDiscoveryException, $allowHttpFallback, $expectedTries, $expected) {
$notifications = $this->getInstance();
$clientMock = $this->createMock(IClient::class);
$clientMock->method('post')
->willThrowException($exception);
$this->httpClientService->method('newClient')->willReturn($clientMock);
$result = $this->invokePrivate(
$notifications,
'tryHttpPostToShareEndpoint',
[
'domain',
'/notifications',
[],
]
);

$this->assertEquals($expected, $result);
if ($isDiscoveryException) {
$this->discoveryManager->expects($this->exactly($expectedTries))->method('getShareEndpoint')
->willThrowException($exception);
$this->createMock(IClient::class)
->expects($this->exactly(0))->method('post');
} else {
$this->discoveryManager->expects($this->exactly($expectedTries))->method('getShareEndpoint');
$clientMock = $this->createMock(IClient::class);
$clientMock->expects($this->exactly($expectedTries))->method('post')
->willThrowException($exception);
$this->httpClientService->method('newClient')->willReturn($clientMock);
}

$this->config->expects($this->any())
->method('getSystemValue')
->with('sharing.federation.allowHttpFallback', false)
->willReturn($allowHttpFallback);

try {
$result = $this->invokePrivate(
$notifications,
'tryHttpPostToShareEndpoint',
[
'domain',
'/notifications',
[]
]
);

$this->assertEquals($expected, $result);
} catch (\Exception $e) {
$this->assertNull($expected);
}
}

public function dataTryHttpPostToShareEndpointInException() {
$responseMock = $this->createMock(IResponse::class);
$responseMock->method('getBody')
->willReturn('User does not exist');
$clientExceptionMock = $this->createMock(ClientException::class);
$clientExceptionMock->method('getResponse')->willReturn($responseMock);

$exceptionMock = $this->createMock(\Exception::class);
$discExceptionMock = $this->createMock(ClientException::class);
$discExceptionMock->method('getResponse')->willReturn($responseMock);

$postExceptionMock = $this->createMock(ClientException::class);
$postExceptionMock->method('getResponse')->willReturn($responseMock);

$exceptionMock = new \Exception('Internal Server Error', 500);

$httpExceptionMock = new \Exception('Invalid SSL Certificate', 526);

return [
// expect client exception in post with 2 tries
[
$clientExceptionMock,
$postExceptionMock,
false,
false,
2,
[
'success' => false,
'result' => 'User does not exist',
]
],
// expect client exception in discovery with 2 tries
[
$exceptionMock,
$discExceptionMock,
true,
false,
2,
[
'success' => false,
'result' => 'User does not exist',
]
],
// expect http exception in post with 1 try as http fallback not allowed
[
$httpExceptionMock,
false,
false,
1,
[
'success' => false,
'result' => '',
]
],
// expect http exception in discovery with 1 try as http fallback not allowed
[
$httpExceptionMock,
true,
false,
1,
[
'success' => false,
'result' => '',
]
],
// expect http exception in discovery with 2 tries as http fallback allowed
[
$httpExceptionMock,
true,
true,
2,
[
'success' => false,
'result' => '',
]
],
// expect internal server error with error thrown in test
[
$exceptionMock,
false,
false,
1,
null
],
];
}

Expand Down
6 changes: 6 additions & 0 deletions changelog/unreleased/37153
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
Bugfix: Respect sharing.federation.allowHttpFallback config option

Federated share can be created for server without SSL, by setting config
option sharing.federation.allowHttpFallback=true.

https://github.com/owncloud/core/pull/37153