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.

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

[master] Respect sharing.federation.allowHttpFallback in FederatedSha… #31197

Merged
merged 2 commits into from
Apr 20, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
3 changes: 2 additions & 1 deletion apps/federatedfilesharing/lib/AppInfo/Application.php
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,8 @@ protected function initFederatedShareProvider() {
$addressHandler,
\OC::$server->getHTTPClientService(),
$discoveryManager,
\OC::$server->getJobList()
\OC::$server->getJobList(),
\OC::$server->getConfig()
);
$tokenHandler = new \OCA\FederatedFileSharing\TokenHandler(
\OC::$server->getSecureRandom()
Expand Down
3 changes: 2 additions & 1 deletion apps/federatedfilesharing/lib/BackgroundJob/RetryJob.php
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,8 @@ public function __construct(Notifications $notifications = null) {
$addressHandler,
\OC::$server->getHTTPClientService(),
$discoveryManager,
\OC::$server->getJobList()
\OC::$server->getJobList(),
\OC::$server->getConfig()
);
}

Expand Down
17 changes: 14 additions & 3 deletions apps/federatedfilesharing/lib/Notifications.php
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@
use OCP\AppFramework\Http;
use OCP\BackgroundJob\IJobList;
use OCP\Http\Client\IClientService;
use OCP\IConfig;

class Notifications {
const RESPONSE_FORMAT = 'json'; // default response format for ocs calls
Expand All @@ -45,22 +46,28 @@ class Notifications {
/** @var IJobList */
private $jobList;

/** @var IConfig */
private $config;

/**
* @param AddressHandler $addressHandler
* @param IClientService $httpClientService
* @param DiscoveryManager $discoveryManager
* @param IJobList $jobList
* @param IConfig $config
*/
public function __construct(
AddressHandler $addressHandler,
IClientService $httpClientService,
DiscoveryManager $discoveryManager,
IJobList $jobList
IJobList $jobList,
IConfig $config
) {
$this->addressHandler = $addressHandler;
$this->httpClientService = $httpClientService;
$this->discoveryManager = $discoveryManager;
$this->jobList = $jobList;
$this->config = $config;
}

/**
Expand Down Expand Up @@ -122,8 +129,7 @@ public function sendRemoteShare($token, $shareWith, $name, $remote_id, $owner, $
* @param string $shareWith
* @param int $permission
* @return bool
* @throws \OC\HintException
* @throws \OC\ServerNotAvailableException
* @throws \Exception
*/
public function requestReShare($token, $id, $shareId, $remote, $shareWith, $permission) {

Expand Down Expand Up @@ -222,6 +228,7 @@ public function sendDeclineShare($remote, $remoteId, $token) {
* @param array $data
* @param int $try
* @return boolean
* @throws \Exception
*/
public function sendUpdateToRemote($remote, $remoteId, $token, $action, $data = [], $try = 0) {

Expand Down Expand Up @@ -303,6 +310,10 @@ 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;
}
$try++;
$protocol = 'http://';
}
Expand Down
24 changes: 16 additions & 8 deletions apps/federatedfilesharing/tests/NotificationsTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,8 @@
use OCA\FederatedFileSharing\Notifications;
use OCP\BackgroundJob\IJobList;
use OCP\Http\Client\IClientService;
use OCP\IConfig;
use OCA\FederatedFileSharing\BackgroundJob\RetryJob;

class NotificationsTest extends \Test\TestCase {

Expand All @@ -44,14 +46,18 @@ class NotificationsTest extends \Test\TestCase {
/** @var IJobList | \PHPUnit_Framework_MockObject_MockObject */
private $jobList;

/** @var IConfig | \PHPUnit_Framework_MockObject_MockObject */
private $config;

public function setUp() {
parent::setUp();

$this->jobList = $this->createMock('OCP\BackgroundJob\IJobList');
$this->discoveryManager = $this->getMockBuilder('OCA\FederatedFileSharing\DiscoveryManager')
$this->jobList = $this->createMock(IJobList::class);
$this->config = $this->createMock(IConfig::class);
$this->discoveryManager = $this->getMockBuilder(DiscoveryManager::class)
->disableOriginalConstructor()->getMock();
$this->httpClientService = $this->createMock('OCP\Http\Client\IClientService');
$this->addressHandler = $this->getMockBuilder('OCA\FederatedFileSharing\AddressHandler')
$this->httpClientService = $this->createMock(IClientService::class);
$this->addressHandler = $this->getMockBuilder(AddressHandler::class)
->disableOriginalConstructor()->getMock();

}
Expand All @@ -68,16 +74,18 @@ private function getInstance(array $mockedMethods = []) {
$this->addressHandler,
$this->httpClientService,
$this->discoveryManager,
$this->jobList
$this->jobList,
$this->config
);
} else {
$instance = $this->getMockBuilder('OCA\FederatedFileSharing\Notifications')
$instance = $this->getMockBuilder(Notifications::class)
->setConstructorArgs(
[
$this->addressHandler,
$this->httpClientService,
$this->discoveryManager,
$this->jobList
$this->jobList,
$this->config
]
)->setMethods($mockedMethods)->getMock();
}
Expand Down Expand Up @@ -113,7 +121,7 @@ public function testSendUpdateToRemote($try, $httpRequestResult, $expected) {
if ($try === 0 && $expected === false) {
$this->jobList->expects($this->once())->method('add')
->with(
'OCA\FederatedFileSharing\BackgroundJob\RetryJob',
RetryJob::class,
[
'remote' => $remote,
'remoteId' => $id,
Expand Down
3 changes: 2 additions & 1 deletion lib/private/Share20/ProviderFactory.php
Original file line number Diff line number Diff line change
Expand Up @@ -102,7 +102,8 @@ protected function federatedShareProvider() {
$addressHandler,
$this->serverContainer->getHTTPClientService(),
$discoveryManager,
$this->serverContainer->getJobList()
$this->serverContainer->getJobList(),
$this->serverContainer->getConfig()
);
$tokenHandler = new TokenHandler(
$this->serverContainer->getSecureRandom()
Expand Down
3 changes: 2 additions & 1 deletion ocs/routes.php
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,8 @@
$addressHandler,
\OC::$server->getHTTPClientService(),
new \OCA\FederatedFileSharing\DiscoveryManager(\OC::$server->getMemCacheFactory(), \OC::$server->getHTTPClientService()),
\OC::$server->getJobList()
\OC::$server->getJobList(),
\OC::$server->getConfig()
);
$s2s = new OCA\FederatedFileSharing\RequestHandler(
$federatedSharingApp->getFederatedShareProvider(),
Expand Down
43 changes: 43 additions & 0 deletions tests/acceptance/features/bootstrap/WebUISharingContext.php
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,8 @@ class WebUISharingContext extends RawMinkContext implements Context {
private $createdPublicLinks = [];

private $oldMinCharactersForAutocomplete = null;
private $oldFedSharingFallbackSetting = null;

/**
* WebUISharingContext constructor.
*
Expand Down Expand Up @@ -346,6 +348,30 @@ public function setMinCharactersForAutocomplete($minCharacters) {
);
}

/**
* @Given the administrator has allowed http fallback for federation sharing
*
* @return void
*/
public function allowHttpFallbackForFedSharing() {
if (is_null($this->oldFedSharingFallbackSetting)) {
$oldFedSharingFallbackSetting = SetupHelper::runOcc(
['config:system:get', 'sharing.federation.allowHttpFallback']
)['stdOut'];
$this->oldFedSharingFallbackSetting = trim(
$oldFedSharingFallbackSetting
);
}
SetupHelper::runOcc(
[
'config:system:set',
'sharing.federation.allowHttpFallback',
'--type boolean',
'--value true',
]
);
}

/**
* @When the user declines the offered remote shares using the webUI
* @Given the user has declined the offered remote shares
Expand Down Expand Up @@ -609,6 +635,7 @@ public function before(BeforeScenarioScope $scope) {
* @return void
*/
public function tearDownScenario() {
//TODO make a function that can be used for different settings
if ($this->oldMinCharactersForAutocomplete === "") {
SetupHelper::runOcc(['config:system:delete', 'user.search_min_length']);
} elseif (!is_null($this->oldMinCharactersForAutocomplete)) {
Expand All @@ -621,6 +648,22 @@ public function tearDownScenario() {
]
);
}

if ($this->oldFedSharingFallbackSetting === "") {
SetupHelper::runOcc(
['config:system:delete', 'sharing.federation.allowHttpFallback']
);
} elseif (!is_null($this->oldFedSharingFallbackSetting)) {
SetupHelper::runOcc(
[
'config:system:set',
'sharing.federation.allowHttpFallback',
'--type boolean',
'--value',
$this->oldFedSharingFallbackSetting
]
);
}
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,8 @@ I want to share files with any users on other cloud storages
So that other users have access to these files

Background:
Given these users have been created:
Given the administrator has allowed http fallback for federation sharing
And these users have been created:
|username|password|displayname|email |
|user1 |1234 |User One |u1@oc.com.np|
|user2 |1234 |User Two |u2@oc.com.np|
Expand Down
10 changes: 10 additions & 0 deletions tests/acceptance/run.sh
Original file line number Diff line number Diff line change
Expand Up @@ -90,6 +90,9 @@ fi
PREVIOUS_SKELETON_DIR=$($OCC --no-warnings config:system:get skeletondirectory)
$OCC config:system:set skeletondirectory --value="$(pwd)/skeleton"

PREVIOUS_HTTP_FALLBACK_SETTING=$($OCC --no-warnings config:system:get sharing.federation.allowHttpFallback)
$OCC config:system:set sharing.federation.allowHttpFallback --type boolean --value true

#Enable external storage app
$OCC config:app:set core enable_external_storage --value=yes
$OCC config:system:set files_external_allow_create_new_local --value=true
Expand Down Expand Up @@ -177,6 +180,13 @@ else
$OCC config:system:set skeletondirectory --value="$PREVIOUS_SKELETON_DIR"
fi

# Put back HTTP fallback setting
if test "A$PREVIOUS_HTTP_FALLBACK_SETTING" = "A"; then
$OCC config:system:delete sharing.federation.allowHttpFallback
else
$OCC config:system:set sharing.federation.allowHttpFallback --type boolean --value="$PREVIOUS_HTTP_FALLBACK_SETTING"
fi

# Clear storage folder
rm -Rf work/local_storage/*

Expand Down