wbrawner/server
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

Merge pull request #23398 from owncloud/block_group_sharing

Browse source 
Allow blocking of group sharing
This commit is contained in:
Thomas Müller 2016-03-22 21:28:13 +01:00
commit d5be21fe81
16 changed files with 218 additions and 42 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

4
apps/files_sharing/api/share20ocs.php
View file

@ -275,6 +275,10 @@ class Share20OCS {
$share->setSharedWith($shareWith);
$share->setPermissions($permissions);
} else if ($shareType === \OCP\Share::SHARE_TYPE_GROUP) {
if (!$this->shareManager->allowGroupSharing()) {
return new \OC_OCS_Result(null, 404, 'group sharing is disabled by the administrator');
}
// Valid group is required to share
if ($shareWith === null || !$this->groupManager->groupExists($shareWith)) {
return new \OC_OCS_Result(null, 404, 'please specify a valid group');

17
apps/files_sharing/api/sharees.php
View file

@ -62,6 +62,9 @@ class Sharees {
/** @var ILogger */
protected $logger;
/** @var \OCP\Share\IManager */
protected $shareManager;
/** @var bool */
protected $shareWithGroupOnly = false;
@ -97,6 +100,7 @@ class Sharees {
* @param IURLGenerator $urlGenerator
* @param IRequest $request
* @param ILogger $logger
* @param \OCP\Share\IManager $shareManager
*/
public function __construct(IGroupManager $groupManager,
IUserManager $userManager,
@ -105,7 +109,8 @@ class Sharees {
IUserSession $userSession,
IURLGenerator $urlGenerator,
IRequest $request,
ILogger $logger) {
ILogger $logger,
\OCP\Share\IManager $shareManager) {
$this->groupManager = $groupManager;
$this->userManager = $userManager;
$this->contactsManager = $contactsManager;
@ -114,6 +119,7 @@ class Sharees {
$this->urlGenerator = $urlGenerator;
$this->request = $request;
$this->logger = $logger;
$this->shareManager = $shareManager;
}
/**
@ -411,9 +417,14 @@ class Sharees {
$shareTypes = [
Share::SHARE_TYPE_USER,
Share::SHARE_TYPE_GROUP,
Share::SHARE_TYPE_REMOTE,
];
if ($this->shareManager->allowGroupSharing()) {
$shareTypes[] = Share::SHARE_TYPE_GROUP;
}
$shareTypes[] = Share::SHARE_TYPE_REMOTE;
if (isset($_GET['shareType']) && is_array($_GET['shareType'])) {
$shareTypes = array_intersect($shareTypes, $_GET['shareType']);
sort($shareTypes);

3
apps/files_sharing/appinfo/routes.php
View file

@ -126,7 +126,8 @@ $sharees = new \OCA\Files_Sharing\API\Sharees(\OC::$server->getGroupManager(),
\OC::$server->getUserSession(),
\OC::$server->getURLGenerator(),
\OC::$server->getRequest(),
\OC::$server->getLogger());
\OC::$server->getLogger(),
\OC::$server->getShareManager());
API::register('get',
'/apps/files_sharing/api/v1/sharees',

50
apps/files_sharing/tests/api/share20ocstest.php
View file

@ -763,9 +763,12 @@ class Share20OCSTest extends \Test\TestCase {
->with('valid-path')
->willReturn($path);
$group = $this->getMock('\OCP\IGroup');
$this->groupManager->method('groupExists')->with('validGroup')->willReturn(true);
$this->shareManager->expects($this->once())
->method('allowGroupSharing')
->willReturn(true);
$share->method('setPath')->with($path);
$share->method('setPermissions')->with(\OCP\Constants::PERMISSION_ALL);
$share->method('setShareType')->with(\OCP\Share::SHARE_TYPE_GROUP);
@ -779,6 +782,51 @@ class Share20OCSTest extends \Test\TestCase {
$this->assertEquals($expected->getData(), $result->getData());
}
public function testCreateShareGroupNotAllowed() {
$share = $this->getMock('\OCP\Share\IShare');
$this->shareManager->method('newShare')->willReturn($share);
$this->request
->method('getParam')
->will($this->returnValueMap([
['path', null, 'valid-path'],
['permissions', null, \OCP\Constants::PERMISSION_ALL],
['shareType', '-1', \OCP\Share::SHARE_TYPE_GROUP],
['shareWith', null, 'validGroup'],
]));
$userFolder = $this->getMock('\OCP\Files\Folder');
$this->rootFolder->expects($this->once())
->method('getUserFolder')
->with('currentUser')
->willReturn($userFolder);
$path = $this->getMock('\OCP\Files\Folder');
$storage = $this->getMock('OCP\Files\Storage');
$storage->method('instanceOfStorage')
->with('OCA\Files_Sharing\External\Storage')
->willReturn(false);
$path->method('getStorage')->willReturn($storage);
$userFolder->expects($this->once())
->method('get')
->with('valid-path')
->willReturn($path);
$this->groupManager->method('groupExists')->with('validGroup')->willReturn(true);
$this->shareManager->expects($this->once())
->method('allowGroupSharing')
->willReturn(false);
$share->method('setPath')->with($path);
$expected = new \OC_OCS_Result(null, 404, 'group sharing is disabled by the administrator');
$result = $this->ocs->createShare();
$this->assertEquals($expected->getMeta(), $result->getMeta());
$this->assertEquals($expected->getData(), $result->getData());
}
public function testCreateShareLinkNoLinksAllowed() {
$this->request
->method('getParam')

84
apps/files_sharing/tests/api/shareestest.php
View file

@ -55,6 +55,9 @@ class ShareesTest extends TestCase {
/** @var \OCP\IRequest|\PHPUnit_Framework_MockObject_MockObject */
protected $request;
/** @var \OCP\Share\IManager|\PHPUnit_Framework_MockObject_MockObject */
protected $shareManager;
protected function setUp() {
parent::setUp();
@ -78,6 +81,10 @@ class ShareesTest extends TestCase {
->disableOriginalConstructor()
->getMock();
$this->shareManager = $this->getMockBuilder('OCP\Share\IManager')
->disableOriginalConstructor()
->getMock();
$this->sharees = new Sharees(
$this->groupManager,
$this->userManager,
@ -86,7 +93,8 @@ class ShareesTest extends TestCase {
$this->session,
$this->getMockBuilder('OCP\IURLGenerator')->disableOriginalConstructor()->getMock(),
$this->request,
$this->getMockBuilder('OCP\ILogger')->disableOriginalConstructor()->getMock()
$this->getMockBuilder('OCP\ILogger')->disableOriginalConstructor()->getMock(),
$this->shareManager
);
}
@ -966,89 +974,95 @@ class ShareesTest extends TestCase {
$allTypes = [Share::SHARE_TYPE_USER, Share::SHARE_TYPE_GROUP, Share::SHARE_TYPE_REMOTE];
return [
[[], '', 'yes', true, '', null, $allTypes, 1, 200, false, true],
[[], '', 'yes', true, '', null, $allTypes, 1, 200, false, true, true],
// Test itemType
[[
'search' => '',
], '', 'yes', true, '', null, $allTypes, 1, 200, false, true],
], '', 'yes', true, '', null, $allTypes, 1, 200, false, true, true],
[[
'search' => 'foobar',
], '', 'yes', true, 'foobar', null, $allTypes, 1, 200, false, true],
], '', 'yes', true, 'foobar', null, $allTypes, 1, 200, false, true, true],
[[
'search' => 0,
], '', 'yes', true, '0', null, $allTypes, 1, 200, false, true],
], '', 'yes', true, '0', null, $allTypes, 1, 200, false, true, true],
// Test itemType
[[
'itemType' => '',
], '', 'yes', true, '', '', $allTypes, 1, 200, false, true],
], '', 'yes', true, '', '', $allTypes, 1, 200, false, true, true],
[[
'itemType' => 'folder',
], '', 'yes', true, '', 'folder', $allTypes, 1, 200, false, true],
], '', 'yes', true, '', 'folder', $allTypes, 1, 200, false, true, true],
[[
'itemType' => 0,
], '', 'yes', true, '', '0', $allTypes, 1, 200, false, true],
], '', 'yes', true, '', '0', $allTypes, 1, 200, false, true, true],
// Test shareType
[[
], '', 'yes', true, '', null, $allTypes, 1, 200, false, true],
], '', 'yes', true, '', null, $allTypes, 1, 200, false, true, true],
[[
'shareType' => 0,
], '', 'yes', true, '', null, [0], 1, 200, false, true],
], '', 'yes', true, '', null, [0], 1, 200, false, true, true],
[[
'shareType' => '0',
], '', 'yes', true, '', null, [0], 1, 200, false, true],
], '', 'yes', true, '', null, [0], 1, 200, false, true, true],
[[
'shareType' => 1,
], '', 'yes', true, '', null, [1], 1, 200, false, true],
], '', 'yes', true, '', null, [1], 1, 200, false, true, true],
[[
'shareType' => 12,
], '', 'yes', true, '', null, [], 1, 200, false, true],
], '', 'yes', true, '', null, [], 1, 200, false, true, true],
[[
'shareType' => 'foobar',
], '', 'yes', true, '', null, $allTypes, 1, 200, false, true],
], '', 'yes', true, '', null, $allTypes, 1, 200, false, true, true],
[[
'shareType' => [0, 1, 2],
], '', 'yes', true, '', null, [0, 1], 1, 200, false, true],
], '', 'yes', true, '', null, [0, 1], 1, 200, false, true, true],
[[
'shareType' => [0, 1],
], '', 'yes', true, '', null, [0, 1], 1, 200, false, true],
], '', 'yes', true, '', null, [0, 1], 1, 200, false, true, true],
[[
'shareType' => $allTypes,
], '', 'yes', true, '', null, $allTypes, 1, 200, false, true],
], '', 'yes', true, '', null, $allTypes, 1, 200, false, true, true],
[[
'shareType' => $allTypes,
], '', 'yes', false, '', null, [0, 1], 1, 200, false, true],
], '', 'yes', false, '', null, [0, 1], 1, 200, false, true, true],
[[
'shareType' => $allTypes,
], '', 'yes', true, '', null, [0, 6], 1, 200, false, true, false],
[[
'shareType' => $allTypes,
], '', 'yes', false, '', null, [0], 1, 200, false, true, false],
// Test pagination
[[
'page' => 1,
], '', 'yes', true, '', null, $allTypes, 1, 200, false, true],
], '', 'yes', true, '', null, $allTypes, 1, 200, false, true, true],
[[
'page' => 10,
], '', 'yes', true, '', null, $allTypes, 10, 200, false, true],
], '', 'yes', true, '', null, $allTypes, 10, 200, false, true, true],
// Test perPage
[[
'perPage' => 1,
], '', 'yes', true, '', null, $allTypes, 1, 1, false, true],
], '', 'yes', true, '', null, $allTypes, 1, 1, false, true, true],
[[
'perPage' => 10,
], '', 'yes', true, '', null, $allTypes, 1, 10, false, true],
], '', 'yes', true, '', null, $allTypes, 1, 10, false, true, true],
// Test $shareWithGroupOnly setting
[[], 'no', 'yes', true, '', null, $allTypes, 1, 200, false, true],
[[], 'yes', 'yes', true, '', null, $allTypes, 1, 200, true, true],
[[], 'no', 'yes', true, '', null, $allTypes, 1, 200, false, true, true],
[[], 'yes', 'yes', true, '', null, $allTypes, 1, 200, true, true, true],
// Test $shareeEnumeration setting
[[], 'no', 'yes', true, '', null, $allTypes, 1, 200, false, true],
[[], 'no', 'no', true, '', null, $allTypes, 1, 200, false, false],
[[], 'no', 'yes', true, '', null, $allTypes, 1, 200, false, true, true],
[[], 'no', 'no', true, '', null, $allTypes, 1, 200, false, false, true],
// Test keep case for search
[[
'search' => 'foo@example.com/ownCloud',
], '', 'yes', true, 'foo@example.com/ownCloud', null, $allTypes, 1, 200, false, true],
], '', 'yes', true, 'foo@example.com/ownCloud', null, $allTypes, 1, 200, false, true, true],
];
}
@ -1066,8 +1080,9 @@ class ShareesTest extends TestCase {
* @param int $perPage
* @param bool $shareWithGroupOnly
* @param bool $shareeEnumeration
* @param bool $allowGroupSharing
*/
public function testSearch($getData, $apiSetting, $enumSetting, $remoteSharingEnabled, $search, $itemType, $shareTypes, $page, $perPage, $shareWithGroupOnly, $shareeEnumeration) {
public function testSearch($getData, $apiSetting, $enumSetting, $remoteSharingEnabled, $search, $itemType, $shareTypes, $page, $perPage, $shareWithGroupOnly, $shareeEnumeration, $allowGroupSharing) {
$oldGet = $_GET;
$_GET = $getData;
@ -1082,6 +1097,10 @@ class ShareesTest extends TestCase {
['core', 'shareapi_allow_share_dialog_user_enumeration', 'yes', $enumSetting],
]);
$this->shareManager->expects($this->once())
->method('allowGroupSharing')
->willReturn($allowGroupSharing);
$sharees = $this->getMockBuilder('\OCA\Files_Sharing\API\Sharees')
->setConstructorArgs([
$this->groupManager,
@ -1091,7 +1110,8 @@ class ShareesTest extends TestCase {
$this->session,
$this->getMockBuilder('OCP\IURLGenerator')->disableOriginalConstructor()->getMock(),
$this->getMockBuilder('OCP\IRequest')->disableOriginalConstructor()->getMock(),
$this->getMockBuilder('OCP\ILogger')->disableOriginalConstructor()->getMock()
$this->getMockBuilder('OCP\ILogger')->disableOriginalConstructor()->getMock(),
$this->shareManager
])
->setMethods(array('searchSharees', 'isRemoteSharingAllowed'))
->getMock();
@ -1175,7 +1195,8 @@ class ShareesTest extends TestCase {
$this->session,
$this->getMockBuilder('OCP\IURLGenerator')->disableOriginalConstructor()->getMock(),
$this->getMockBuilder('OCP\IRequest')->disableOriginalConstructor()->getMock(),
$this->getMockBuilder('OCP\ILogger')->disableOriginalConstructor()->getMock()
$this->getMockBuilder('OCP\ILogger')->disableOriginalConstructor()->getMock(),
$this->shareManager
])
->setMethods(array('searchSharees', 'isRemoteSharingAllowed'))
->getMock();
@ -1327,7 +1348,8 @@ class ShareesTest extends TestCase {
$this->session,
$this->getMockBuilder('OCP\IURLGenerator')->disableOriginalConstructor()->getMock(),
$this->getMockBuilder('OCP\IRequest')->disableOriginalConstructor()->getMock(),
$this->getMockBuilder('OCP\ILogger')->disableOriginalConstructor()->getMock()
$this->getMockBuilder('OCP\ILogger')->disableOriginalConstructor()->getMock(),
$this->shareManager
])
->setMethods(array('getShareesForShareIds', 'getUsers', 'getGroups', 'getRemote'))
->getMock();

1
build/integration/features/bootstrap/ShareesContext.php
View file

@ -68,5 +68,6 @@ class ShareesContext implements Context, SnippetAcceptingContext {
protected function resetAppConfigs() {
$this->modifyServerConfig('core', 'shareapi_only_share_with_group_members', 'no');
$this->modifyServerConfig('core', 'shareapi_allow_share_dialog_user_enumeration', 'yes');
$this->modifyServerConfig('core', 'shareapi_allow_group_sharing', 'yes');
}
}

16
build/integration/sharees_features/sharees.feature
View file

@ -222,3 +222,19 @@ Feature: sharees
Then "groups" sharees returned is empty
Then "exact remotes" sharees returned is empty
Then "remotes" sharees returned is empty
Scenario: Group sharees not returned when group sharing is disabled
Given As an "test"
And parameter "shareapi_allow_group_sharing" of app "core" is set to "no"
When getting sharees for
| search | sharee |
| itemType | file |
Then the OCS status code should be "100"
And the HTTP status code should be "200"
And "exact users" sharees returned is empty
And "users" sharees returned are
| Sharee1 | 0 | Sharee1 |
And "exact groups" sharees returned is empty
And "groups" sharees returned is empty
And "exact remotes" sharees returned is empty
And "remotes" sharees returned is empty

3
core/js/config.php
View file

@ -162,7 +162,8 @@ $array = array(
'sharingDisabledForUser' => \OCP\Util::isSharingDisabledForUser(),
'resharingAllowed' => \OCP\Share::isResharingAllowed(),
'remoteShareAllowed' => $outgoingServer2serverShareEnabled,
'federatedCloudShareDoc' => \OC::$server->getURLGenerator()->linkToDocs('user-sharing-federated')
'federatedCloudShareDoc' => \OC::$server->getURLGenerator()->linkToDocs('user-sharing-federated'),
'allowGroupSharing' => \OC::$server->getShareManager()->allowGroupSharing()
)
)
),

3
core/js/shareconfigmodel.js
View file

@ -26,7 +26,8 @@
isDefaultExpireDateEnabled: oc_appconfig.core.defaultExpireDateEnabled === true,
isRemoteShareAllowed: oc_appconfig.core.remoteShareAllowed,
defaultExpireDate: oc_appconfig.core.defaultExpireDate,
isResharingAllowed: oc_appconfig.core.resharingAllowed
isResharingAllowed: oc_appconfig.core.resharingAllowed,
allowGroupSharing: oc_appconfig.core.allowGroupSharing
},
/**

20
core/js/sharedialogview.js
View file

@ -216,8 +216,12 @@
.autocomplete("option", "autoFocus", true);
response(suggestions);
} else {
var title = t('core', 'No users or groups found for {search}', {search: $('.shareWithField').val()});
if (!view.configModel.get('allowGroupSharing')) {
title = t('core', 'No users found for {search}', {search: $('.shareWithField').val()});
}
$('.shareWithField').addClass('error')
.attr('data-original-title', t('core', 'No users or groups found for {search}', {search: $('.shareWithField').val()}))
.attr('data-original-title', title)
.tooltip('hide')
.tooltip({
placement: 'bottom',
@ -386,10 +390,18 @@
},
_renderSharePlaceholderPart: function () {
var sharePlaceholder = t('core', 'Share with users or groups …');
if (this.configModel.get('isRemoteShareAllowed')) {
sharePlaceholder = t('core', 'Share with users, groups or remote users …');
var sharePlaceholder = t('core', 'Share with users…');
if (this.configModel.get('allowGroupSharing')) {
if (this.configModel.get('isRemoteShareAllowed')) {
sharePlaceholder = t('core', 'Share with users, groups or remote users…');
} else {
sharePlaceholder = t('core', 'Share with users or groups…')
}
} else if (this.configModel.get('isRemoteShareAllowed')) {
sharePlaceholder = t('core', 'Share with users or remote users…');
}
return sharePlaceholder;
},

12
lib/private/Share20/Manager.php
View file

@ -361,6 +361,11 @@ class Manager implements IManager {
* @throws \Exception
*/
protected function groupCreateChecks(\OCP\Share\IShare $share) {
// Verify group shares are allowed
if (!$this->allowGroupSharing()) {
throw new \Exception('Group sharing is now allowed');
}
// Verify if the user can share with this group
if ($this->shareWithGroupMembersOnly()) {
$sharedBy = $this->userManager->get($share->getSharedBy());
@ -1108,6 +1113,13 @@ class Manager implements IManager {
return $this->config->getAppValue('core', 'shareapi_only_share_with_group_members', 'no') === 'yes';
}
/**
* Check if users can share with groups
* @return bool
*/
public function allowGroupSharing() {
return $this->config->getAppValue('core', 'shareapi_allow_group_sharing', 'yes') === 'yes';
}
/**
* Copied from \OC_Util::isSharingDisabledForUser

7
lib/public/share/imanager.php
View file

@ -221,6 +221,13 @@ interface IManager {
*/
public function shareWithGroupMembersOnly();
/**
* Check if users can share with groups
* @return bool
* @since 9.0.1
*/
public function allowGroupSharing();
/**
* Check if sharing is disabled for the given user
*

1
settings/admin.php
View file

@ -129,6 +129,7 @@ $template->assign('allowPublicMailNotification', $appConfig->getValue('core', 's
$template->assign('allowMailNotification', $appConfig->getValue('core', 'shareapi_allow_mail_notification', 'no'));
$template->assign('allowShareDialogUserEnumeration', $appConfig->getValue('core', 'shareapi_allow_share_dialog_user_enumeration', 'yes'));
$template->assign('onlyShareWithGroupMembers', \OC\Share\Share::shareWithGroupMembersOnly());
$template->assign('allowGroupSharing', $appConfig->getValue('core', 'shareapi_allow_group_sharing', 'yes'));
$databaseOverload = (strpos(\OCP\Config::getSystemValue('dbtype'), 'sqlite') !== false);
$template->assign('databaseOverload', $databaseOverload);
$template->assign('cronErrors', $appConfig->getValue('core', 'cronErrors'));

4
settings/js/admin.js
View file

@ -161,6 +161,10 @@ $(document).ready(function(){
});
});
$('#allowGroupSharing').change(function() {
$('#allowGroupSharing').toggleClass('hidden', !this.checked);
});
$('#shareapiExcludeGroups').change(function() {
$("#selectExcludedGroups").toggleClass('hidden', !this.checked);
});

5
settings/templates/admin.php
View file

@ -239,6 +239,11 @@ if ($_['cronErrors']) {
value="1" <?php if ($_['allowResharing'] === 'yes') print_unescaped('checked="checked"'); ?> />
<label for="allowResharing"><?php p($l->t('Allow resharing'));?></label><br/>
</p>
<p class="<?php if ($_['shareAPIEnabled'] === 'no') p('hidden');?>">
<input type="checkbox" name="shareapi_allow_group_sharing" id="allowGroupSharing" class="checkbox"
value="1" <?php if ($_['allowGroupSharing'] === 'yes') print_unescaped('checked="checked"'); ?> />
<label for="allowGroupSharing"><?php p($l->t('Allow sharing with groups'));?></label><br />
</p>
<p class="<?php if ($_['shareAPIEnabled'] === 'no') p('hidden');?>">
<input type="checkbox" name="shareapi_only_share_with_group_members" id="onlyShareWithGroupMembers" class="checkbox"
value="1" <?php if ($_['onlyShareWithGroupMembers']) print_unescaped('checked="checked"'); ?> />

30
tests/lib/share20/managertest.php
View file

@ -1147,6 +1147,22 @@ class ManagerTest extends \Test\TestCase {
$this->invokePrivate($this->manager, 'userCreateChecks', [$share]);
}
/**
* @expectedException Exception
* @expectedExceptionMessage Group sharing is now allowed
*/
public function testGroupCreateChecksShareWithGroupMembersGroupSharingNotAllowed() {
$share = $this->manager->newShare();
$this->config
->method('getAppValue')
->will($this->returnValueMap([
['core', 'shareapi_allow_group_sharing', 'yes', 'no'],
]));
$this->invokePrivate($this->manager, 'groupCreateChecks', [$share]);
}
/**
* @expectedException Exception
* @expectedExceptionMessage Only sharing within your own groups is allowed
@ -1167,6 +1183,7 @@ class ManagerTest extends \Test\TestCase {
->method('getAppValue')
->will($this->returnValueMap([
['core', 'shareapi_only_share_with_group_members', 'no', 'yes'],
['core', 'shareapi_allow_group_sharing', 'yes', 'yes'],
]));
$this->invokePrivate($this->manager, 'groupCreateChecks', [$share]);
@ -1195,6 +1212,7 @@ class ManagerTest extends \Test\TestCase {
->method('getAppValue')
->will($this->returnValueMap([
['core', 'shareapi_only_share_with_group_members', 'no', 'yes'],
['core', 'shareapi_allow_group_sharing', 'yes', 'yes'],
]));
$this->invokePrivate($this->manager, 'groupCreateChecks', [$share]);
@ -1222,6 +1240,12 @@ class ManagerTest extends \Test\TestCase {
->with($path)
->willReturn([$share2]);
$this->config
->method('getAppValue')
->will($this->returnValueMap([
['core', 'shareapi_allow_group_sharing', 'yes', 'yes'],
]));
$this->invokePrivate($this->manager, 'groupCreateChecks', [$share]);
}
@ -1240,6 +1264,12 @@ class ManagerTest extends \Test\TestCase {
->with($path)
->willReturn([$share2]);
$this->config
->method('getAppValue')
->will($this->returnValueMap([
['core', 'shareapi_allow_group_sharing', 'yes', 'yes'],
]));
$this->invokePrivate($this->manager, 'groupCreateChecks', [$share]);
}