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

[stable10] Auto accept incoming federated shares from trusted servers #34206

Merged
merged 1 commit into from
Feb 7, 2019
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/js/settings-admin.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,8 @@ $(document).ready(function() {
if (this.checked) {
value = 'yes';
}
OC.AppConfig.setValue('files_sharing', $(this).attr('name'), value);
var app = (this.id !== 'autoAcceptTrusted') ? 'files_sharing' : 'federatedfilesharing';
OC.AppConfig.setValue(app, $(this).attr('name'), value);
});

$('.section .icon-info').tipsy({gravity: 'w'});
Expand Down
11 changes: 10 additions & 1 deletion apps/federatedfilesharing/lib/AdminPanel.php
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
*/
namespace OCA\FederatedFileSharing;

use OCP\IConfig;
use OCP\Settings\ISettings;
use OCP\Template;

Expand All @@ -28,13 +29,17 @@ class AdminPanel implements ISettings {
/** @var FederatedShareProvider */
protected $shareProvider;

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

/**
* AdminPanel constructor.
*
* @param FederatedShareProvider $shareProvider
*/
public function __construct(FederatedShareProvider $shareProvider) {
public function __construct(FederatedShareProvider $shareProvider, IConfig $config) {
$this->shareProvider = $shareProvider;
$this->config = $config;
}

public function getPriority() {
Expand All @@ -49,6 +54,10 @@ public function getPanel() {
$tmpl = new Template('federatedfilesharing', 'settings-admin');
$tmpl->assign('outgoingServer2serverShareEnabled', $this->shareProvider->isOutgoingServer2serverShareEnabled());
$tmpl->assign('incomingServer2serverShareEnabled', $this->shareProvider->isIncomingServer2serverShareEnabled());
$tmpl->assign(
'autoAcceptTrusted',
$this->config->getAppValue('federatedfilesharing', 'auto_accept_trusted', 'no')
);
return $tmpl;
}
}
1 change: 1 addition & 0 deletions apps/federatedfilesharing/lib/AppInfo/Application.php
Original file line number Diff line number Diff line change
Expand Up @@ -200,6 +200,7 @@ protected function initFederatedShareProvider() {

$this->federatedShareProvider = new FederatedShareProvider(
\OC::$server->getDatabaseConnection(),
\OC::$server->getEventDispatcher(),
$addressHandler,
$notifications,
$tokenHandler,
Expand Down
42 changes: 41 additions & 1 deletion apps/federatedfilesharing/lib/FederatedShareProvider.php
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,8 @@
use OCP\Share\Exceptions\ShareNotFound;
use OCP\Share\IShare;
use OCP\Share\IShareProvider;
use Symfony\Component\EventDispatcher\EventDispatcherInterface;
use Symfony\Component\EventDispatcher\GenericEvent;

/**
* Class FederatedShareProvider
Expand All @@ -50,6 +52,9 @@ class FederatedShareProvider implements IShareProvider {
/** @var IDBConnection */
private $dbConnection;

/** @var EventDispatcherInterface */
private $eventDispatcher;

/** @var AddressHandler */
private $addressHandler;

Expand Down Expand Up @@ -84,6 +89,7 @@ class FederatedShareProvider implements IShareProvider {
* DefaultShareProvider constructor.
*
* @param IDBConnection $connection
* @param EventDispatcherInterface $eventDispatcher
* @param AddressHandler $addressHandler
* @param Notifications $notifications
* @param TokenHandler $tokenHandler
Expand All @@ -95,6 +101,7 @@ class FederatedShareProvider implements IShareProvider {
*/
public function __construct(
IDBConnection $connection,
EventDispatcherInterface $eventDispatcher,
AddressHandler $addressHandler,
Notifications $notifications,
TokenHandler $tokenHandler,
Expand All @@ -105,6 +112,7 @@ public function __construct(
IUserManager $userManager
) {
$this->dbConnection = $connection;
$this->eventDispatcher = $eventDispatcher;
$this->addressHandler = $addressHandler;
$this->notifications = $notifications;
$this->tokenHandler = $tokenHandler;
Expand Down Expand Up @@ -1026,8 +1034,40 @@ public function addShare($remote, $token, $name, $owner, $shareWith, $remoteId)
$shareWith
);
$externalManager->addShare(
$remote, $token, '', $name, $owner, false, $shareWith, $remoteId
$remote,
$token,
'',
$name,
$owner,
$this->getAccepted($remote),
$shareWith,
$remoteId
);
return $this->dbConnection->lastInsertId("*PREFIX*{$this->externalShareTable}");
}

/**
* @param string $remote
*
* @return bool
*/
protected function getAccepted($remote) {
$event = $this->eventDispatcher->dispatch(
'remoteshare.received',
new GenericEvent('', ['remote' => $remote])
);
if ($event->getArgument('autoAddServers')) {
return false;
}
$autoAccept = $this->config->getAppValue(
'federatedfilesharing',
'auto_accept_trusted',
'no'
);
if ($autoAccept !== 'yes') {
return false;
}

return $event->getArgument('isRemoteTrusted') === true;
}
}
10 changes: 10 additions & 0 deletions apps/federatedfilesharing/templates/settings-admin.php
Original file line number Diff line number Diff line change
Expand Up @@ -29,4 +29,14 @@
<?php p($l->t('Allow users on this server to receive shares from other servers'));?>
</label><br/>
</p>

<p>
<input type="checkbox" name="auto_accept_trusted" id="autoAcceptTrusted" class="checkbox"
value="1" <?php if ($_['autoAcceptTrusted']) {
print_unescaped('checked="checked"');
} ?> />
<label for="autoAcceptTrusted">
<?php p($l->t('Automatically accept remote shares from trusted servers'));?>
</label><br/>
</p>
</div>
6 changes: 5 additions & 1 deletion apps/federatedfilesharing/tests/AdminPanelTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@

use OCA\FederatedFileSharing\AdminPanel;
use OCA\FederatedFileSharing\FederatedShareProvider;
use OCP\IConfig;

/**
* @package OCA\FederatedFileSharing\Tests
Expand All @@ -33,13 +34,16 @@ class AdminPanelTest extends \Test\TestCase {
private $panel;
/** @var FederatedShareProvider */
private $shareProvider;
/** @var IConfig */
private $config;

public function setUp() {
parent::setUp();
$this->shareProvider = $this->getMockBuilder(FederatedShareProvider::class)
->disableOriginalConstructor()
->getMock();
$this->panel = new AdminPanel($this->shareProvider);
$this->config = $this->createMock(IConfig::class);
$this->panel = new AdminPanel($this->shareProvider, $this->config);
}

public function testGetSection() {
Expand Down
58 changes: 56 additions & 2 deletions apps/federatedfilesharing/tests/FederatedShareProviderTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,8 @@
use OCP\Share\IShare;
use OCP\Files\Folder;
use OCP\IUser;
use Symfony\Component\EventDispatcher\EventDispatcherInterface;
use Symfony\Component\EventDispatcher\GenericEvent;

/**
* Class FederatedShareProviderTest
Expand All @@ -50,6 +52,8 @@ class FederatedShareProviderTest extends \Test\TestCase {

/** @var IDBConnection */
protected $connection;
/** @var EventDispatcherInterface */
protected $eventDispatcher;
/** @var AddressHandler | \PHPUnit_Framework_MockObject_MockObject */
protected $addressHandler;
/** @var Notifications | \PHPUnit_Framework_MockObject_MockObject */
Expand All @@ -76,6 +80,9 @@ public function setUp() {
parent::setUp();

$this->connection = \OC::$server->getDatabaseConnection();
$this->eventDispatcher = $this->getMockBuilder(EventDispatcherInterface::class)
->disableOriginalConstructor()
->getMock();
$this->notifications = $this->getMockBuilder('OCA\FederatedFileSharing\Notifications')
->disableOriginalConstructor()
->getMock();
Expand All @@ -91,13 +98,13 @@ public function setUp() {
$this->rootFolder = $this->createMock('OCP\Files\IRootFolder');
$this->config = $this->createMock('OCP\IConfig');
$this->userManager = $this->createMock('OCP\IUserManager');
//$this->addressHandler = new AddressHandler(\OC::$server->getURLGenerator(), $this->l);
$this->addressHandler = $this->getMockBuilder('OCA\FederatedFileSharing\AddressHandler')->disableOriginalConstructor()->getMock();

$this->userManager->expects($this->any())->method('userExists')->willReturn(true);

$this->provider = new FederatedShareProvider(
$this->connection,
$this->eventDispatcher,
$this->addressHandler,
$this->notifications,
$this->tokenHandler,
Expand Down Expand Up @@ -464,10 +471,11 @@ public function testCreateAlreadyShared() {
*
*/
public function testUpdate($owner, $sharedBy) {
$this->provider = $this->getMockBuilder('OCA\FederatedFileSharing\FederatedShareProvider')
$this->provider = $this->getMockBuilder(FederatedShareProvider::class)
->setConstructorArgs(
[
$this->connection,
\OC::$server->getEventDispatcher(),
$this->addressHandler,
$this->notifications,
$this->tokenHandler,
Expand Down Expand Up @@ -884,4 +892,50 @@ public function testUpdateForRecipientReturnsShare() {

$this->assertEquals($share, $returnedShare);
}

/**
* @dataProvider dataTestGetAccepted
*
*/
public function testGetAccepted($autoAddServers, $autoAccept, $isRemoteTrusted, $expected) {
$this->config->method('getAppValue')
->with('federatedfilesharing', 'auto_accept_trusted', 'no')
->willReturn($autoAccept);

$event = new GenericEvent(
'',
[
'autoAddServers' => $autoAddServers,
'isRemoteTrusted' => $isRemoteTrusted
]
);
$this->eventDispatcher->method('dispatch')
->with('remoteshare.received', $this->anything())
->willReturn($event);

$shouldAutoAccept = $this->invokePrivate(
$this->provider,
'getAccepted',
['remote']
);

$this->assertEquals($expected, $shouldAutoAccept);
}

public function dataTestGetAccepted() {
return [
// never autoaccept when auto add to trusted is on
[true, 'yes', true, false],
[true, 'yes', false, false],
[true, 'no', true, false],
[true, 'no', false, false],
// never autoaccept when auto autoaccept is off
[false, 'no', false, false],
[false, 'no', true, false],
// never autoaccept when remote is not trusted
[false, 'yes', false, false],
// autoaccept
[false, 'yes', true, true],
];
}
}
10 changes: 10 additions & 0 deletions apps/federation/lib/AppInfo/Application.php
Original file line number Diff line number Diff line change
Expand Up @@ -120,6 +120,16 @@ public function registerHooks() {
}
}
});

$dispatcher->addListener(
'remoteshare.received',
function ($event) use ($container) {
$remote = $event->getArgument('remote');
$trustedServers = $container->query('TrustedServers');
$event->setArgument('autoAddServers', $trustedServers->getAutoAddServers());
$event->setArgument('isRemoteTrusted', $trustedServers->isTrustedServer($remote));
}
);
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ Feature: Federation Sharing - sharing with users on other cloud storages
And using server "LOCAL"
And user "user1" has been created with default attributes
And user "user1" has logged in using the webUI
And parameter "auto_accept_trusted" of app "federatedfilesharing" has been set to "no"

Scenario: test the single steps of sharing a folder to a remote server
When the user shares folder "simple-folder" with remote user "user1@%remote_server_without_scheme%" using the webUI
Expand Down Expand Up @@ -53,6 +54,17 @@ Feature: Federation Sharing - sharing with users on other cloud storages
Then file "lorem (2).txt" should not be listed on the webUI
And file "lorem (2).txt" should not be listed in the shared-with-you page on the webUI

Scenario: automatically accept a federation share when it is allowed by the config
Given parameter "autoAddServers" of app "federation" has been set to "1"
And user "user1" from server "REMOTE" has shared "simple-folder" with user "user1" from server "LOCAL"
And user "user1" from server "LOCAL" has accepted the last pending share
And the user has reloaded the current page of the webUI
And parameter "auto_accept_trusted" of app "federatedfilesharing" has been set to "yes"
And parameter "autoAddServers" of app "federation" has been set to "0"
When user "user1" from server "REMOTE" shares "/lorem.txt" with user "user1" from server "LOCAL" using the sharing API
And the user has reloaded the current page of the webUI
Then file "lorem (2).txt" should be listed on the webUI

@skipOnMICROSOFTEDGE
Scenario: share a folder with an remote user and prohibit deleting - local server shares - remote server receives
When the user shares folder "simple-folder" with remote user "user1@%remote_server_without_scheme%" using the webUI
Expand Down