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] dont allow user to override admin sharing settings when its disabled #34842

Merged
merged 1 commit into from
Mar 20, 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
6 changes: 5 additions & 1 deletion apps/federatedfilesharing/lib/FederatedShareProvider.php
Original file line number Diff line number Diff line change
Expand Up @@ -1065,11 +1065,15 @@ protected function getAccepted($remote, $shareWith) {
'auto_accept_trusted',
'no'
);
if ($globalAutoAcceptValue !== 'yes') {
return false;
}
$autoAccept = $this->config->getUserValue(
$shareWith,
'federatedfilesharing',
'auto_accept_share_trusted',
$globalAutoAcceptValue);
$globalAutoAcceptValue
);
if ($autoAccept !== 'yes') {
return false;
}
Expand Down
2 changes: 1 addition & 1 deletion apps/federatedfilesharing/lib/Panels/AdminPanel.php
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ public function getPanel() {
$tmpl->assign('incomingServer2serverShareEnabled', $this->shareProvider->isIncomingServer2serverShareEnabled());
$tmpl->assign(
'autoAcceptTrusted',
$this->config->getAppValue('federatedfilesharing', 'auto_accept_trusted', 'no')
$this->config->getAppValue('federatedfilesharing', 'auto_accept_trusted', 'no') === 'yes'
);
return $tmpl;
}
Expand Down
24 changes: 18 additions & 6 deletions apps/federatedfilesharing/lib/Panels/SharingPersonalPanel.php
Original file line number Diff line number Diff line change
Expand Up @@ -46,17 +46,29 @@ public function __construct(IConfig $config, IUserSession $userSession) {
* @return TemplateResponse | Template
*/
public function getPanel() {
$tmpl = new Template('federatedfilesharing', 'settings-personal-sharing');
$showEmptyTemplate = true;
$globalAutoAcceptShareEnabled = $this->config->getAppValue(
'federatedfilesharing',
'auto_accept_trusted',
'no'
);
$autoAcceptShareEnabled = $this->config->getUserValue(
$this->userSession->getUser()->getUID(),
'federatedfilesharing',
'auto_accept_share_trusted',
'yes'
);
$tmpl = new Template('federatedfilesharing', 'settings-personal-sharing');
$tmpl->assign(
'userAutoAcceptShareTrustedEnabled',
$autoAcceptShareEnabled
$globalAutoAcceptShareEnabled
);
if ($globalAutoAcceptShareEnabled === 'yes') {
$showEmptyTemplate = false;
$tmpl->assign(
'userAutoAcceptShareTrustedEnabled',
$autoAcceptShareEnabled
);
}
if ($showEmptyTemplate) {
return new Template('federatedfilesharing', 'settings-personal-sharing-empty');
}
return $tmpl;
}

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
<?php
/**
* @author Semih Serhat Karakaya <[email protected]>
*
* @copyright Copyright (c) 2019, ownCloud GmbH
* @license AGPL-3.0
*
* This code is free software: you can redistribute it and/or modify
* it under the terms of the GNU Affero General Public License, version 3,
* as published by the Free Software Foundation.
*
* This program is distributed in the hope that it will be useful,
* but WITHOUT ANY WARRANTY; without even the implied warranty of
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
* GNU Affero General Public License for more details.
*
* You should have received a copy of the GNU Affero General Public License, version 3,
* along with this program. If not, see <http://www.gnu.org/licenses/>
*
*/

/** @var \OCP\IL10N $l */
?>

<div class="section" id="files_sharing_settings">
<h2 class="app-name"><?php p($l->t('Federated Cloud Sharing'));?></h2>
<p><?php p($l->t('Nothing to configure.')); ?></p>
</div>
18 changes: 11 additions & 7 deletions apps/federatedfilesharing/templates/settings-personal-sharing.php
Original file line number Diff line number Diff line change
Expand Up @@ -21,16 +21,20 @@

/** @var array $_ */
/** @var \OCP\IL10N $l */

script('federatedfilesharing', 'settings-personal-sharing');
?>

<form class="section" id="federatedfilesharing_settings">
<h2 class="app-name"><?php p($l->t('Federated Cloud Sharing'));?></h2>
<input type="checkbox" name="auto_accept_share_trusted" id="userAutoAcceptShareTrustedInput" class="checkbox"
value="1" <?php if ($_['userAutoAcceptShareTrustedEnabled'] === 'yes') {
print_unescaped('checked="checked"');
} ?> />
<label for="userAutoAcceptShareTrustedInput">
<?php p($l->t('Automatically accept remote shares from trusted servers'));?>
</label><br/>
<?php if (isset($_['userAutoAcceptShareTrustedEnabled'])): ?>
<?php if ($_['userAutoAcceptShareTrustedEnabled'] === 'yes'): ?>
<input type="checkbox" name="auto_accept_share_trusted" id="userAutoAcceptShareTrustedInput" class="checkbox" value="1" checked="checked" />
<?php else: ?>
<input type="checkbox" name="auto_accept_share_trusted" id="userAutoAcceptShareTrustedInput" class="checkbox" value="1" />
<?php endif; ?>
<label for="userAutoAcceptShareTrustedInput">
<?php p($l->t('Automatically accept remote shares from trusted servers')); ?>
</label><br/>
<?php endif; ?>
</form>
Original file line number Diff line number Diff line change
Expand Up @@ -939,9 +939,10 @@ public function dataTestGetAccepted() {
[false, 'yes', 'yes', false, false],
// autoaccept
[false, 'yes', 'yes', true, true],
// userAutoAccept overrides globalAutoAccept
// users can override globalAutoAccept when globalAutoAccept enabled
[false, 'yes', 'no', true, false],
[false, 'no', 'yes', true, true],
[false, 'no', 'yes', true, false],
[false, 'yes', 'yes', true, true],
];
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,20 @@ public function testGetPriority() {
$this->assertEquals(40, $this->sharingPersonalPanel->getPriority());
}

public function testGetPanel() {
public function globalFederatedSharingConfigProvider() {
return [
[['auto_accept_trusted' => 'yes'], '<form class="section" id="federatedfilesharing_settings">'],
[['auto_accept_trusted' => 'no'], '<p>Nothing to configure.</p>'],
];
}

/**
* @dataProvider globalFederatedSharingConfigProvider
*
* @param array $globalConfigs
* @param string $expectedString
*/
public function testGetPanel($globalConfigs, $expectedString) {
$mockUser = $this->getMockBuilder(IUser::class)
->disableOriginalConstructor()
->getMock();
Expand All @@ -67,8 +80,12 @@ public function testGetPanel() {
$this->userSession->expects($this->any())
->method('getUser')
->willReturn($mockUser);

$this->config->expects($this->once())
->method('getAppValue')
->with('federatedfilesharing', 'auto_accept_trusted', 'no')
->willReturn($globalConfigs['auto_accept_trusted']);

$templateHtml = $this->sharingPersonalPanel->getPanel()->fetchPage();
$this->assertContains('<form class="section" id="federatedfilesharing_settings">', $templateHtml);
$this->assertContains($expectedString, $templateHtml);
}
}
5 changes: 4 additions & 1 deletion apps/files_sharing/lib/Controller/Share20OcsController.php
Original file line number Diff line number Diff line change
Expand Up @@ -378,8 +378,11 @@ public function createShare() {

$shareWith = $this->request->getParam('shareWith', null);

$autoAccept = false;
$globalAutoAcceptValue = $this->config->getAppValue('core', 'shareapi_auto_accept_share', 'yes');
$autoAccept = $this->config->getUserValue($shareWith, 'files_sharing', 'auto_accept_share', $globalAutoAcceptValue) === 'yes';
if ($globalAutoAcceptValue === 'yes') {
$autoAccept = $this->config->getUserValue($shareWith, 'files_sharing', 'auto_accept_share', $globalAutoAcceptValue) === 'yes';
}
if ($shareType === Share::SHARE_TYPE_USER) {
// Valid user is required to share
if ($shareWith === null || !$this->userManager->userExists($shareWith)) {
Expand Down
24 changes: 18 additions & 6 deletions apps/files_sharing/lib/Panels/Personal/PersonalPanel.php
Original file line number Diff line number Diff line change
Expand Up @@ -46,17 +46,29 @@ public function __construct(IConfig $config, IUserSession $userSession) {
* @return TemplateResponse | Template
*/
public function getPanel() {
$tmpl = new Template('files_sharing', 'settings-personal');
$showEmptyTemplate = true;
$globalAutoAcceptShareEnabled = $this->config->getAppValue(
'core',
'shareapi_auto_accept_share',
'yes'
);
$autoAcceptShareEnabled = $this->config->getUserValue(
$this->userSession->getUser()->getUID(),
'files_sharing',
'auto_accept_share',
'yes'
);
$tmpl = new Template('files_sharing', 'settings-personal');
$tmpl->assign(
'userAutoAcceptShareEnabled',
$autoAcceptShareEnabled
$globalAutoAcceptShareEnabled
);
if ($globalAutoAcceptShareEnabled === 'yes') {
$showEmptyTemplate = false;
$tmpl->assign(
'userAutoAcceptShareEnabled',
$autoAcceptShareEnabled
);
}
if ($showEmptyTemplate) {
return new Template('files_sharing', 'settings-personal-empty');
}
return $tmpl;
}

Expand Down
28 changes: 28 additions & 0 deletions apps/files_sharing/templates/settings-personal-empty.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
<?php
/**
* @author Semih Serhat Karakaya <[email protected]>
*
* @copyright Copyright (c) 2019, ownCloud GmbH
* @license AGPL-3.0
*
* This code is free software: you can redistribute it and/or modify
* it under the terms of the GNU Affero General Public License, version 3,
* as published by the Free Software Foundation.
*
* This program is distributed in the hope that it will be useful,
* but WITHOUT ANY WARRANTY; without even the implied warranty of
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
* GNU Affero General Public License for more details.
*
* You should have received a copy of the GNU Affero General Public License, version 3,
* along with this program. If not, see <http://www.gnu.org/licenses/>
*
*/

/** @var \OCP\IL10N $l */
?>

<div class="section" id="files_sharing_settings">
<h2 class="app-name"><?php p($l->t('Sharing'));?></h2>
<p><?php p($l->t('Nothing to configure.')); ?></p>
</div>
18 changes: 11 additions & 7 deletions apps/files_sharing/templates/settings-personal.php
Original file line number Diff line number Diff line change
Expand Up @@ -21,16 +21,20 @@

/** @var array $_ */
/** @var \OCP\IL10N $l */

script('files_sharing', 'settings-personal');
?>

<form class="section" id="files_sharing_settings">
<h2 class="app-name"><?php p($l->t('Sharing'));?></h2>
<input type="checkbox" name="auto_accept_share" id="userAutoAcceptShareInput" class="checkbox"
value="1" <?php if ($_['userAutoAcceptShareEnabled'] === 'yes') {
print_unescaped('checked="checked"');
} ?> />
<label for="userAutoAcceptShareInput">
<?php p($l->t('Automatically accept new incoming local user shares'));?>
</label><br/>
<?php if (isset($_['userAutoAcceptShareEnabled'])): ?>
<?php if ($_['userAutoAcceptShareEnabled'] === 'yes'): ?>
<input type="checkbox" name="auto_accept_share" id="userAutoAcceptShareInput" class="checkbox" value="1" checked="checked" />
<?php else: ?>
<input type="checkbox" name="auto_accept_share" id="userAutoAcceptShareInput" class="checkbox" value="1" />
<?php endif; ?>
<label for="userAutoAcceptShareInput">
<?php p($l->t('Automatically accept new incoming local user shares')); ?>
</label><br/>
<?php endif; ?>
</form>
21 changes: 19 additions & 2 deletions apps/files_sharing/tests/Panels/Personal/PersonalPanelTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,20 @@ public function testGetPriority() {
$this->assertEquals(100, $this->personalPanel->getPriority());
}

public function testGetPanel() {
public function globalSharingConfigProvider() {
return [
[['shareapi_auto_accept_share' => 'yes'], '<form class="section" id="files_sharing_settings">'],
[['shareapi_auto_accept_share' => 'no'], '<p>Nothing to configure.</p>'],
];
}

/**
* @dataProvider globalSharingConfigProvider
*
* @param array $globalConfigs
* @param string $expectedString
*/
public function testGetPanel($globalConfigs, $expectedString) {
$mockUser = $this->getMockBuilder(IUser::class)
->disableOriginalConstructor()
->getMock();
Expand All @@ -67,8 +80,12 @@ public function testGetPanel() {
$this->userSession->expects($this->any())
->method('getUser')
->willReturn($mockUser);
$this->config->expects($this->once())
->method('getAppValue')
->with('core', 'shareapi_auto_accept_share', 'yes')
->willReturn($globalConfigs['shareapi_auto_accept_share']);

$templateHtml = $this->personalPanel->getPanel()->fetchPage();
$this->assertContains('<form class="section" id="files_sharing_settings">', $templateHtml);
$this->assertContains($expectedString, $templateHtml);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -80,4 +80,26 @@ public function switchAutoAcceptingFederatedShares($action) {
$this->getSession(), $action
);
}

/**
* @Then User-based auto accepting checkbox should not be displayed on the personal sharing settings page in the webUI
*
* @return void
*/
public function autoAcceptingCheckboxShouldNotBeDisplayedOnThePersonalSharingSettingsPageInTheWebui() {
PHPUnit_Framework_Assert::assertFalse(
$this->personalSharingSettingsPage->isAutoAcceptLocalSharesCheckboxDisplayed()
);
}

/**
* @Then User-based auto accepting from trusted servers checkbox should not be displayed on the personal sharing settings page in the webUI
*
* @return void
*/
public function autoAcceptingFederatedCheckboxShouldNotBeDisplayedOnThePersonalSharingSettingsPageInTheWebui() {
PHPUnit_Framework_Assert::assertFalse(
$this->personalSharingSettingsPage->isAutoAcceptFederatedSharesCheckboxDisplayed()
);
}
}
32 changes: 30 additions & 2 deletions tests/acceptance/features/lib/PersonalSharingSettingsPage.php
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,8 @@ class PersonalSharingSettingsPage extends SharingSettingsPage {
* @var string $path
*/
protected $path = '/index.php/settings/personal?sectionid=sharing';
protected $personalSharingPanelDivXpath
= '//div[@id="OCA\Files_Sharing\Panels\Personal\PersonalPanel"]';
protected $autoAcceptLocalSharesCheckboxXpath
= '//label[@for="userAutoAcceptShareInput"]';
protected $autoAcceptLocalSharesCheckboxXpathCheckboxId
Expand Down Expand Up @@ -76,7 +78,7 @@ public function toggleAutoAcceptingFederatedShares(Session $session, $action) {

/**
* there is no reliable loading indicator on the personal sharing settings page,
* so just wait for the auto accept checkbox to be there and all Ajax calls to finish
* so just wait for files_sharing personal panel div to be there and all Ajax calls to finish
*
* @param Session $session
* @param int $timeout_msec
Expand All @@ -89,7 +91,33 @@ public function waitTillPageIsLoaded(
) {
$this->waitForOutstandingAjaxCalls($session);
$this->waitTillXpathIsVisible(
$this->autoAcceptLocalSharesCheckboxXpath, $timeout_msec
$this->personalSharingPanelDivXpath, $timeout_msec
);
}

/**
* Check if the auto accept local shares checkbox is shown in the webui
*
* @return boolean
*/
public function isAutoAcceptLocalSharesCheckboxDisplayed() {
$localAutoAcceptCheckbox = $this->find('xpath', $this->autoAcceptLocalSharesCheckboxXpath);
if ($localAutoAcceptCheckbox === null) {
return false;
}
return true;
}

/**
* Check if the auto accept local shares checkbox is shown in the webui
*
* @return boolean
*/
public function isAutoAcceptFederatedSharesCheckboxDisplayed() {
$localAutoAcceptCheckbox = $this->find('xpath', $this->autoAcceptFederatedSharesCheckboxXpath);
if ($localAutoAcceptCheckbox === null) {
return false;
}
return true;
}
}
Loading