From c8631d607ef827f0eb29312faf7b2b808e1a8e7d Mon Sep 17 00:00:00 2001 From: Bjoern Schiessle Date: Mon, 25 Jun 2018 11:40:16 +0200 Subject: [PATCH 01/14] add setting to enable/disable federated group sharing Signed-off-by: Bjoern Schiessle --- .../lib/FederatedShareProvider.php | 45 ++++++++++++++++++- .../lib/Settings/Admin.php | 3 ++ .../templates/settings-admin.php | 17 ++++++- 3 files changed, 63 insertions(+), 2 deletions(-) diff --git a/apps/federatedfilesharing/lib/FederatedShareProvider.php b/apps/federatedfilesharing/lib/FederatedShareProvider.php index f81b826f12..d32560c4ff 100644 --- a/apps/federatedfilesharing/lib/FederatedShareProvider.php +++ b/apps/federatedfilesharing/lib/FederatedShareProvider.php @@ -30,6 +30,7 @@ namespace OCA\FederatedFileSharing; use OC\Share20\Share; +use OCP\Federation\ICloudFederationProviderManager; use OCP\Federation\ICloudIdManager; use OCP\DB\QueryBuilder\IQueryBuilder; use OCP\Files\Folder; @@ -92,6 +93,9 @@ class FederatedShareProvider implements IShareProvider { /** @var \OCP\GlobalScale\IConfig */ private $gsConfig; + /** @var ICloudFederationProviderManager */ + private $cloudFederationProviderManager; + /** * DefaultShareProvider constructor. * @@ -106,6 +110,7 @@ class FederatedShareProvider implements IShareProvider { * @param IUserManager $userManager * @param ICloudIdManager $cloudIdManager * @param \OCP\GlobalScale\IConfig $globalScaleConfig + * @param ICloudFederationProviderManager $cloudFederationProviderManager */ public function __construct( IDBConnection $connection, @@ -118,7 +123,8 @@ class FederatedShareProvider implements IShareProvider { IConfig $config, IUserManager $userManager, ICloudIdManager $cloudIdManager, - \OCP\GlobalScale\IConfig $globalScaleConfig + \OCP\GlobalScale\IConfig $globalScaleConfig, + ICloudFederationProviderManager $cloudFederationProviderManager ) { $this->dbConnection = $connection; $this->addressHandler = $addressHandler; @@ -131,6 +137,7 @@ class FederatedShareProvider implements IShareProvider { $this->userManager = $userManager; $this->cloudIdManager = $cloudIdManager; $this->gsConfig = $globalScaleConfig; + $this->cloudFederationProviderManager = $cloudFederationProviderManager; } /** @@ -967,6 +974,42 @@ class FederatedShareProvider implements IShareProvider { return ($result === 'yes'); } + + /** + * check if users from other Nextcloud instances are allowed to send federated group shares + * + * @return bool + */ + public function isOutgoingServer2serverGroupShareEnabled() { + if ($this->gsConfig->onlyInternalFederation()) { + return false; + } + $result = $this->config->getAppValue('files_sharing', 'outgoing_server2server_group_share_enabled', 'no'); + return ($result === 'yes'); + } + + /** + * check if users are allowed to receive federated group shares + * + * @return bool + */ + public function isIncomingServer2serverGroupShareEnabled() { + if ($this->gsConfig->onlyInternalFederation()) { + return false; + } + $result = $this->config->getAppValue('files_sharing', 'incoming_server2server_group_share_enabled', 'no'); + return ($result === 'yes'); + } + + /** + * check if federated group sharing is supported, therefore the OCM API need to be enabled + * + * @return bool + */ + public function isFederatedGroupSharingSupported() { + return $this->cloudFederationProviderManager->isReady(); + } + /** * Check if querying sharees on the lookup server is enabled * diff --git a/apps/federatedfilesharing/lib/Settings/Admin.php b/apps/federatedfilesharing/lib/Settings/Admin.php index cbeaa167ff..aea7957a49 100644 --- a/apps/federatedfilesharing/lib/Settings/Admin.php +++ b/apps/federatedfilesharing/lib/Settings/Admin.php @@ -58,6 +58,9 @@ class Admin implements ISettings { 'internalOnly' => $this->gsConfig->onlyInternalFederation(), 'outgoingServer2serverShareEnabled' => $this->fedShareProvider->isOutgoingServer2serverShareEnabled(), 'incomingServer2serverShareEnabled' => $this->fedShareProvider->isIncomingServer2serverShareEnabled(), + 'federatedGroupSharingSupported' => $this->fedShareProvider->isFederatedGroupSharingSupported(), + 'outgoingServer2serverGroupShareEnabled' => $this->fedShareProvider->isOutgoingServer2serverGroupShareEnabled(), + 'incomingServer2serverGroupShareEnabled' => $this->fedShareProvider->isIncomingServer2serverGroupShareEnabled(), 'lookupServerEnabled' => $this->fedShareProvider->isLookupServerQueriesEnabled(), 'lookupServerUploadEnabled' => $this->fedShareProvider->isLookupServerUploadEnabled(), ]; diff --git a/apps/federatedfilesharing/templates/settings-admin.php b/apps/federatedfilesharing/templates/settings-admin.php index 187d75f21a..1b9b88dc03 100644 --- a/apps/federatedfilesharing/templates/settings-admin.php +++ b/apps/federatedfilesharing/templates/settings-admin.php @@ -23,7 +23,6 @@ script('federatedfilesharing', 'settings-admin'); t('Allow users on this server to send shares to other servers'));?>

-

/> @@ -31,6 +30,22 @@ script('federatedfilesharing', 'settings-admin'); t('Allow users on this server to receive shares from other servers'));?>

+ +

+ /> + +

+

+ /> +
+

+

/> From b23032e4c569f7ba54197171cbb02ed8119b6811 Mon Sep 17 00:00:00 2001 From: Bjoern Schiessle Date: Wed, 13 Jun 2018 14:19:59 +0200 Subject: [PATCH 02/14] implement federated group shares Signed-off-by: Bjoern Schiessle --- apps/cloud_federation_api/lib/Config.php | 9 +- .../Controller/RequestHandlerController.php | 45 +++++-- .../lib/AppInfo/Application.php | 7 +- .../lib/FederatedShareProvider.php | 44 +++++-- .../lib/Notifications.php | 7 +- .../lib/ocm/CloudFederationProviderFiles.php | 113 +++++++++++++----- apps/files_sharing/appinfo/database.xml | 11 ++ apps/files_sharing/appinfo/info.xml | 2 +- .../lib/Controller/ShareAPIController.php | 93 +++++++------- apps/files_sharing/lib/External/Manager.php | 31 +++-- lib/private/Share/Constants.php | 1 + lib/private/Share20/Manager.php | 11 ++ lib/private/Share20/ProviderFactory.php | 5 +- lib/public/Share/IManager.php | 8 ++ 14 files changed, 274 insertions(+), 113 deletions(-) diff --git a/apps/cloud_federation_api/lib/Config.php b/apps/cloud_federation_api/lib/Config.php index 7d42960dea..2fb842697d 100644 --- a/apps/cloud_federation_api/lib/Config.php +++ b/apps/cloud_federation_api/lib/Config.php @@ -47,11 +47,14 @@ class Config { * * @param string $resourceType * @return array - * @throws \OCP\Federation\Exceptions\ProviderDoesNotExistsException */ public function getSupportedShareTypes($resourceType) { - $provider = $this->cloudFederationProviderManager->getCloudFederationProvider($resourceType); - return $provider->getSupportedShareTypes(); + try { + $provider = $this->cloudFederationProviderManager->getCloudFederationProvider($resourceType); + return $provider->getSupportedShareTypes(); + } catch (\Exception $e) { + return []; + } } } diff --git a/apps/cloud_federation_api/lib/Controller/RequestHandlerController.php b/apps/cloud_federation_api/lib/Controller/RequestHandlerController.php index 27dd89f2ab..e7c6c415d3 100644 --- a/apps/cloud_federation_api/lib/Controller/RequestHandlerController.php +++ b/apps/cloud_federation_api/lib/Controller/RequestHandlerController.php @@ -35,6 +35,7 @@ use OCP\Federation\ICloudFederationFactory; use OCP\Federation\ICloudFederationProviderManager; use OCP\Federation\Exceptions\ProviderDoesNotExistsException; use OCP\Federation\ICloudIdManager; +use OCP\IGroupManager; use OCP\ILogger; use OCP\IRequest; use OCP\IURLGenerator; @@ -57,6 +58,9 @@ class RequestHandlerController extends Controller { /** @var IUserManager */ private $userManager; + /** @var IGroupManager */ + private $groupManager; + /** @var IURLGenerator */ private $urlGenerator; @@ -76,6 +80,7 @@ class RequestHandlerController extends Controller { IRequest $request, ILogger $logger, IUserManager $userManager, + IGroupManager $groupManager, IURLGenerator $urlGenerator, ICloudFederationProviderManager $cloudFederationProviderManager, Config $config, @@ -86,6 +91,7 @@ class RequestHandlerController extends Controller { $this->logger = $logger; $this->userManager = $userManager; + $this->groupManager = $groupManager; $this->urlGenerator = $urlGenerator; $this->cloudFederationProviderManager = $cloudFederationProviderManager; $this->config = $config; @@ -136,17 +142,37 @@ class RequestHandlerController extends Controller { ); } - $cloudId = $this->cloudIdManager->resolveCloudId($shareWith); - $shareWithLocalId = $cloudId->getUser(); - $shareWith = $this->mapUid($shareWithLocalId); - - if (!$this->userManager->userExists($shareWith)) { + $supportedShareTypes = $this->config->getSupportedShareTypes($resourceType); + if (!in_array($shareType, $supportedShareTypes)) { return new JSONResponse( - ['message' => 'User "' . $shareWith . '" does not exists at ' . $this->urlGenerator->getBaseUrl()], - Http::STATUS_BAD_REQUEST + ['message' => 'Share type "' . $shareType . '" not implemented'], + Http::STATUS_NOT_IMPLEMENTED ); } + $cloudId = $this->cloudIdManager->resolveCloudId($shareWith); + $shareWith = $cloudId->getUser(); + + if ($shareType === 'user') { + $shareWith = $this->mapUid($shareWith); + + if (!$this->userManager->userExists($shareWith)) { + return new JSONResponse( + ['message' => 'User "' . $shareWith . '" does not exists at ' . $this->urlGenerator->getBaseUrl()], + Http::STATUS_BAD_REQUEST + ); + } + } + + if ($shareType === 'group') { + if(!$this->groupManager->groupExists($shareWith)) { + return new JSONResponse( + ['message' => 'Group "' . $shareWith . '" does not exists at ' . $this->urlGenerator->getBaseUrl()], + Http::STATUS_BAD_REQUEST + ); + } + } + // if no explicit display name is given, we use the uid as display name $ownerDisplayName = $ownerDisplayName === null ? $owner : $ownerDisplayName; $sharedByDisplayName = $sharedByDisplayName === null ? $sharedBy : $sharedByDisplayName; @@ -161,7 +187,7 @@ class RequestHandlerController extends Controller { $provider = $this->cloudFederationProviderManager->getCloudFederationProvider($resourceType); $share = $this->factory->getCloudFederationShare($shareWith, $name, $description, $providerId, $owner, $ownerDisplayName, $sharedBy, $sharedByDisplayName, '', $shareType, $resourceType); $share->setProtocol($protocol); - $id = $provider->shareReceived($share); + $provider->shareReceived($share); } catch (ProviderDoesNotExistsException $e) { return new JSONResponse( ['message' => $e->getMessage()], @@ -179,7 +205,7 @@ class RequestHandlerController extends Controller { ); } - $user = $this->userManager->get($shareWithLocalId); + $user = $this->userManager->get($shareWith); $recipientDisplayName = ''; if($user) { $recipientDisplayName = $user->getDisplayName(); @@ -259,7 +285,6 @@ class RequestHandlerController extends Controller { * @return string mixed */ private function mapUid($uid) { - \OC::$server->getURLGenerator()->linkToDocs('key'); // FIXME this should be a method in the user management instead $this->logger->debug('shareWith before, ' . $uid, ['app' => $this->appName]); \OCP\Util::emitHook( diff --git a/apps/federatedfilesharing/lib/AppInfo/Application.php b/apps/federatedfilesharing/lib/AppInfo/Application.php index 7034783121..73ac062be0 100644 --- a/apps/federatedfilesharing/lib/AppInfo/Application.php +++ b/apps/federatedfilesharing/lib/AppInfo/Application.php @@ -65,7 +65,8 @@ class Application extends App { $server->getURLGenerator(), $server->getCloudFederationFactory(), $server->getCloudFederationProviderManager(), - $server->getDatabaseConnection() + $server->getDatabaseConnection(), + $server->getGroupManager() ); }); @@ -145,7 +146,9 @@ class Application extends App { \OC::$server->getConfig(), \OC::$server->getUserManager(), \OC::$server->getCloudIdManager(), - $c->query(IConfig::class) + $c->query(IConfig::class), + \OC::$server->getCloudFederationProviderManager() + ); } diff --git a/apps/federatedfilesharing/lib/FederatedShareProvider.php b/apps/federatedfilesharing/lib/FederatedShareProvider.php index d32560c4ff..d1560d90ef 100644 --- a/apps/federatedfilesharing/lib/FederatedShareProvider.php +++ b/apps/federatedfilesharing/lib/FederatedShareProvider.php @@ -96,6 +96,9 @@ class FederatedShareProvider implements IShareProvider { /** @var ICloudFederationProviderManager */ private $cloudFederationProviderManager; + /** @var array list of supported share types */ + private $supportedShareType = [\OCP\Share::SHARE_TYPE_REMOTE_GROUP, \OCP\Share::SHARE_TYPE_REMOTE]; + /** * DefaultShareProvider constructor. * @@ -164,12 +167,23 @@ class FederatedShareProvider implements IShareProvider { $itemType = $share->getNodeType(); $permissions = $share->getPermissions(); $sharedBy = $share->getSharedBy(); + $shareType = $share->getShareType(); + + if ($shareType === \OCP\Share::SHARE_TYPE_REMOTE_GROUP && + !$this->isOutgoingServer2serverGroupShareEnabled() + ) { + $message = 'It is not allowed to send federated group shares from this server.'; + $message_t = $this->l->t('It is not allowed to send federated group shares from this server.'); + $this->logger->debug($message, ['app' => 'Federated File Sharing']); + throw new \Exception($message_t); + } /* * Check if file is not already shared with the remote user */ - $alreadyShared = $this->getSharedWith($shareWith, self::SHARE_TYPE_REMOTE, $share->getNode(), 1, 0); - if (!empty($alreadyShared)) { + $alreadyShared = $this->getSharedWith($shareWith, \OCP\Share::SHARE_TYPE_REMOTE, $share->getNode(), 1, 0); + $alreadySharedGroup = $this->getSharedWith($shareWith, \OCP\Share::SHARE_TYPE_REMOTE_GROUP, $share->getNode(), 1, 0); + if (!empty($alreadyShared) || !empty($alreadySharedGroup)) { $message = 'Sharing %s failed, because this item is already shared with %s'; $message_t = $this->l->t('Sharing %s failed, because this item is already shared with %s', array($share->getNode()->getName(), $shareWith)); $this->logger->debug(sprintf($message, $share->getNode()->getName(), $shareWith), ['app' => 'Federated File Sharing']); @@ -200,7 +214,7 @@ class FederatedShareProvider implements IShareProvider { if ($remoteShare) { try { $ownerCloudId = $this->cloudIdManager->getCloudId($remoteShare['owner'], $remoteShare['remote']); - $shareId = $this->addShareToDB($itemSource, $itemType, $shareWith, $sharedBy, $ownerCloudId->getId(), $permissions, 'tmp_token_' . time()); + $shareId = $this->addShareToDB($itemSource, $itemType, $shareWith, $sharedBy, $ownerCloudId->getId(), $permissions, 'tmp_token_' . time(), $shareType); $share->setId($shareId); list($token, $remoteId) = $this->askOwnerToReShare($shareWith, $share, $shareId); // remote share was create successfully if we get a valid token as return @@ -245,7 +259,8 @@ class FederatedShareProvider implements IShareProvider { $share->getSharedBy(), $share->getShareOwner(), $share->getPermissions(), - $token + $token, + $share->getShareType() ); $failure = false; @@ -265,7 +280,8 @@ class FederatedShareProvider implements IShareProvider { $share->getShareOwner(), $ownerCloudId->getId(), $share->getSharedBy(), - $sharedByFederatedId + $sharedByFederatedId, + $share->getShareType() ); if ($send === false) { @@ -349,12 +365,13 @@ class FederatedShareProvider implements IShareProvider { * @param string $uidOwner * @param int $permissions * @param string $token + * @param int $shareType * @return int */ - private function addShareToDB($itemSource, $itemType, $shareWith, $sharedBy, $uidOwner, $permissions, $token) { + private function addShareToDB($itemSource, $itemType, $shareWith, $sharedBy, $uidOwner, $permissions, $token, $shareType) { $qb = $this->dbConnection->getQueryBuilder(); $qb->insert('share') - ->setValue('share_type', $qb->createNamedParameter(self::SHARE_TYPE_REMOTE)) + ->setValue('share_type', $qb->createNamedParameter($shareType)) ->setValue('item_type', $qb->createNamedParameter($itemType)) ->setValue('item_source', $qb->createNamedParameter($itemSource)) ->setValue('file_source', $qb->createNamedParameter($itemSource)) @@ -498,7 +515,7 @@ class FederatedShareProvider implements IShareProvider { $qb->select('*') ->from('share') ->where($qb->expr()->eq('parent', $qb->createNamedParameter($parent->getId()))) - ->andWhere($qb->expr()->eq('share_type', $qb->createNamedParameter(self::SHARE_TYPE_REMOTE))) + ->andWhere($qb->expr()->in('share_type', $qb->createNamedParameter($this->supportedShareType, IQueryBuilder::PARAM_INT_ARRAY))) ->orderBy('id'); $cursor = $qb->execute(); @@ -647,7 +664,7 @@ class FederatedShareProvider implements IShareProvider { $qb->select('*') ->from('share'); - $qb->andWhere($qb->expr()->eq('share_type', $qb->createNamedParameter(self::SHARE_TYPE_REMOTE))); + $qb->andWhere($qb->expr()->eq('share_type', $qb->createNamedParameter($shareType))); /** * Reshares for this user are shares where they are the owner. @@ -704,7 +721,7 @@ class FederatedShareProvider implements IShareProvider { $qb->select('*') ->from('share') ->where($qb->expr()->eq('id', $qb->createNamedParameter($id))) - ->andWhere($qb->expr()->eq('share_type', $qb->createNamedParameter(self::SHARE_TYPE_REMOTE))); + ->andWhere($qb->expr()->in('share_type', $qb->createNamedParameter($this->supportedShareType, IQueryBuilder::PARAM_INT_ARRAY))); $cursor = $qb->execute(); $data = $cursor->fetch(); @@ -732,10 +749,11 @@ class FederatedShareProvider implements IShareProvider { public function getSharesByPath(Node $path) { $qb = $this->dbConnection->getQueryBuilder(); + // get federated user shares $cursor = $qb->select('*') ->from('share') ->andWhere($qb->expr()->eq('file_source', $qb->createNamedParameter($path->getId()))) - ->andWhere($qb->expr()->eq('share_type', $qb->createNamedParameter(self::SHARE_TYPE_REMOTE))) + ->andWhere($qb->expr()->in('share_type', $qb->createNamedParameter($this->supportedShareType, IQueryBuilder::PARAM_INT_ARRAY))) ->execute(); $shares = []; @@ -768,7 +786,7 @@ class FederatedShareProvider implements IShareProvider { } $qb->setFirstResult($offset); - $qb->where($qb->expr()->eq('share_type', $qb->createNamedParameter(self::SHARE_TYPE_REMOTE))); + $qb->where($qb->expr()->in('share_type', $qb->createNamedParameter($this->supportedShareType, IQueryBuilder::PARAM_INT_ARRAY))); $qb->andWhere($qb->expr()->eq('share_with', $qb->createNamedParameter($userId))); // Filter by node if provided @@ -799,7 +817,7 @@ class FederatedShareProvider implements IShareProvider { $cursor = $qb->select('*') ->from('share') - ->where($qb->expr()->eq('share_type', $qb->createNamedParameter(self::SHARE_TYPE_REMOTE))) + ->where($qb->expr()->in('share_type', $qb->createNamedParameter($this->supportedShareType, IQueryBuilder::PARAM_INT_ARRAY))) ->andWhere($qb->expr()->eq('token', $qb->createNamedParameter($token))) ->execute(); diff --git a/apps/federatedfilesharing/lib/Notifications.php b/apps/federatedfilesharing/lib/Notifications.php index fdba3e113d..70733bc9e2 100644 --- a/apps/federatedfilesharing/lib/Notifications.php +++ b/apps/federatedfilesharing/lib/Notifications.php @@ -88,11 +88,12 @@ class Notifications { * @param string $ownerFederatedId * @param string $sharedBy * @param string $sharedByFederatedId + * @param int $shareType (can be a remote user or group share) * @return bool * @throws \OC\HintException * @throws \OC\ServerNotAvailableException */ - public function sendRemoteShare($token, $shareWith, $name, $remote_id, $owner, $ownerFederatedId, $sharedBy, $sharedByFederatedId) { + public function sendRemoteShare($token, $shareWith, $name, $remote_id, $owner, $ownerFederatedId, $sharedBy, $sharedByFederatedId, $shareType) { list($user, $remote) = $this->addressHandler->splitUserRemote($shareWith); @@ -109,6 +110,7 @@ class Notifications { 'sharedBy' => $sharedBy, 'sharedByFederatedId' => $sharedByFederatedId, 'remote' => $local, + 'shareType' => $shareType ); $result = $this->tryHttpPostToShareEndpoint($remote, '', $fields); @@ -392,7 +394,7 @@ class Notifications { $fields['sharedByFederatedId'], $fields['sharedBy'], $fields['token'], - 'user', + $fields['shareType'], 'file' ); return $this->federationProviderManager->sendShare($share); @@ -406,6 +408,7 @@ class Notifications { 'sharedSecret' => $fields['token'], 'shareWith' => $fields['shareWith'], 'senderId' => $fields['localId'], + 'shareType' => $fields['shareType'], 'message' => 'Ask owner to reshare the file' ] ); diff --git a/apps/federatedfilesharing/lib/ocm/CloudFederationProviderFiles.php b/apps/federatedfilesharing/lib/ocm/CloudFederationProviderFiles.php index 00750f924e..cb7b6478e7 100644 --- a/apps/federatedfilesharing/lib/ocm/CloudFederationProviderFiles.php +++ b/apps/federatedfilesharing/lib/ocm/CloudFederationProviderFiles.php @@ -40,10 +40,12 @@ use OCP\Federation\ICloudFederationShare; use OCP\Federation\ICloudIdManager; use OCP\Files\NotFoundException; use OCP\IDBConnection; +use OCP\IGroupManager; use OCP\ILogger; use OCP\IURLGenerator; use OCP\IUserManager; use OCP\Notification\IManager as INotificationManager; +use OCP\Share; use OCP\Share\Exceptions\ShareNotFound; use OCP\Share\IShare; use OCP\Util; @@ -86,6 +88,9 @@ class CloudFederationProviderFiles implements ICloudFederationProvider { /** @var IDBConnection */ private $connection; + /** @var IGroupManager */ + private $groupManager; + /** * CloudFederationProvider constructor. * @@ -101,6 +106,7 @@ class CloudFederationProviderFiles implements ICloudFederationProvider { * @param ICloudFederationFactory $cloudFederationFactory * @param ICloudFederationProviderManager $cloudFederationProviderManager * @param IDBConnection $connection + * @param IGroupManager $groupManager */ public function __construct(IAppManager $appManager, FederatedShareProvider $federatedShareProvider, @@ -113,7 +119,8 @@ class CloudFederationProviderFiles implements ICloudFederationProvider { IURLGenerator $urlGenerator, ICloudFederationFactory $cloudFederationFactory, ICloudFederationProviderManager $cloudFederationProviderManager, - IDBConnection $connection + IDBConnection $connection, + IGroupManager $groupManager ) { $this->appManager = $appManager; $this->federatedShareProvider = $federatedShareProvider; @@ -127,6 +134,7 @@ class CloudFederationProviderFiles implements ICloudFederationProvider { $this->cloudFederationFactory = $cloudFederationFactory; $this->cloudFederationProviderManager = $cloudFederationProviderManager; $this->connection = $connection; + $this->groupManager = $groupManager; } @@ -175,6 +183,7 @@ class CloudFederationProviderFiles implements ICloudFederationProvider { $remoteId = $share->getProviderId(); $sharedByFederatedId = $share->getSharedBy(); $ownerFederatedId = $share->getOwner(); + $shareType = $this->mapShareTypeToNextcloud($share->getShareType()); // if no explicit information about the person who created the share was send // we assume that the share comes from the owner @@ -190,19 +199,25 @@ class CloudFederationProviderFiles implements ICloudFederationProvider { } // FIXME this should be a method in the user management instead - $this->logger->debug('shareWith before, ' . $shareWith, ['app' => 'files_sharing']); - Util::emitHook( - '\OCA\Files_Sharing\API\Server2Server', - 'preLoginNameUsedAsUserName', - array('uid' => &$shareWith) - ); - $this->logger->debug('shareWith after, ' . $shareWith, ['app' => 'files_sharing']); + if ($shareType === Share::SHARE_TYPE_USER) { + $this->logger->debug('shareWith before, ' . $shareWith, ['app' => 'files_sharing']); + Util::emitHook( + '\OCA\Files_Sharing\API\Server2Server', + 'preLoginNameUsedAsUserName', + array('uid' => &$shareWith) + ); + $this->logger->debug('shareWith after, ' . $shareWith, ['app' => 'files_sharing']); - if (!$this->userManager->userExists($shareWith)) { - throw new ProviderCouldNotAddShareException('User does not exists', '',Http::STATUS_BAD_REQUEST); + if (!$this->userManager->userExists($shareWith)) { + throw new ProviderCouldNotAddShareException('User does not exists', '',Http::STATUS_BAD_REQUEST); + } + + \OC_Util::setupFS($shareWith); } - \OC_Util::setupFS($shareWith); + if ($shareType === Share::SHARE_TYPE_GROUP && !$this->groupManager->groupExists($shareWith)) { + throw new ProviderCouldNotAddShareException('Group does not exists', '',Http::STATUS_BAD_REQUEST); + } $externalManager = new \OCA\Files_Sharing\External\Manager( \OC::$server->getDatabaseConnection(), @@ -217,7 +232,7 @@ class CloudFederationProviderFiles implements ICloudFederationProvider { ); try { - $externalManager->addShare($remote, $token, '', $name, $owner, false, $shareWith, $remoteId); + $externalManager->addShare($remote, $token, '', $name, $owner, $shareType,false, $shareWith, $remoteId); $shareId = \OC::$server->getDatabaseConnection()->lastInsertId('*PREFIX*share_external'); $event = $this->activityManager->generateEvent(); @@ -228,25 +243,14 @@ class CloudFederationProviderFiles implements ICloudFederationProvider { ->setObject('remote_share', (int)$shareId, $name); \OC::$server->getActivityManager()->publish($event); - $notification = $this->notificationManager->createNotification(); - $notification->setApp('files_sharing') - ->setUser($shareWith) - ->setDateTime(new \DateTime()) - ->setObject('remote_share', $shareId) - ->setSubject('remote_share', [$ownerFederatedId, $sharedByFederatedId, trim($name, '/')]); - - $declineAction = $notification->createAction(); - $declineAction->setLabel('decline') - ->setLink($this->urlGenerator->getAbsoluteURL($this->urlGenerator->linkTo('', 'ocs/v2.php/apps/files_sharing/api/v1/remote_shares/pending/' . $shareId)), 'DELETE'); - $notification->addAction($declineAction); - - $acceptAction = $notification->createAction(); - $acceptAction->setLabel('accept') - ->setLink($this->urlGenerator->getAbsoluteURL($this->urlGenerator->linkTo('', 'ocs/v2.php/apps/files_sharing/api/v1/remote_shares/pending/' . $shareId)), 'POST'); - $notification->addAction($acceptAction); - - $this->notificationManager->notify($notification); - + if ($shareType === Share::SHARE_TYPE_USER) { + $this->notifyAboutNewShare($shareWith, $shareId, $ownerFederatedId, $sharedByFederatedId, $name); + } else { + $groupMembers = $this->groupManager->get($shareWith)->getUsers(); + foreach ($groupMembers as $user) { + $this->notifyAboutNewShare($user, $shareId, $ownerFederatedId, $sharedByFederatedId, $name); + } + } return $shareId; } catch (\Exception $e) { $this->logger->logException($e, [ @@ -297,6 +301,51 @@ class CloudFederationProviderFiles implements ICloudFederationProvider { throw new BadRequestException([$notificationType]); } + /** + * map OCM share type (strings) to Nextcloud internal share types (integer) + * + * @param string $shareType + * @return int + */ + private function mapShareTypeToNextcloud($shareType) { + $result = Share::SHARE_TYPE_USER; + if ($shareType === 'group') { + $result = Share::SHARE_TYPE_GROUP; + } + + return $result; + } + + /** + * notify user about new federated share + * + * @param $shareWith + * @param $shareId + * @param $ownerFederatedId + * @param $sharedByFederatedId + * @param $name + */ + private function notifyAboutNewShare($shareWith, $shareId, $ownerFederatedId, $sharedByFederatedId, $name) { + $notification = $this->notificationManager->createNotification(); + $notification->setApp('files_sharing') + ->setUser($shareWith) + ->setDateTime(new \DateTime()) + ->setObject('remote_share', $shareId) + ->setSubject('remote_share', [$ownerFederatedId, $sharedByFederatedId, trim($name, '/')]); + + $declineAction = $notification->createAction(); + $declineAction->setLabel('decline') + ->setLink($this->urlGenerator->getAbsoluteURL($this->urlGenerator->linkTo('', 'ocs/v2.php/apps/files_sharing/api/v1/remote_shares/pending/' . $shareId)), 'DELETE'); + $notification->addAction($declineAction); + + $acceptAction = $notification->createAction(); + $acceptAction->setLabel('accept') + ->setLink($this->urlGenerator->getAbsoluteURL($this->urlGenerator->linkTo('', 'ocs/v2.php/apps/files_sharing/api/v1/remote_shares/pending/' . $shareId)), 'POST'); + $notification->addAction($acceptAction); + + $this->notificationManager->notify($notification); + } + /** * process notification that the recipient accepted a share * @@ -771,6 +820,6 @@ class CloudFederationProviderFiles implements ICloudFederationProvider { * @since 14.0.0 */ public function getSupportedShareTypes() { - return ['user']; + return ['user', 'group']; } } diff --git a/apps/files_sharing/appinfo/database.xml b/apps/files_sharing/appinfo/database.xml index a70be408da..c3cfb9e1c8 100644 --- a/apps/files_sharing/appinfo/database.xml +++ b/apps/files_sharing/appinfo/database.xml @@ -15,6 +15,17 @@ 1 4 + + parent + integer + -1 + 4 + + + share_type + integer + 4 + remote text diff --git a/apps/files_sharing/appinfo/info.xml b/apps/files_sharing/appinfo/info.xml index 94a31bea63..f2725880df 100644 --- a/apps/files_sharing/appinfo/info.xml +++ b/apps/files_sharing/appinfo/info.xml @@ -9,7 +9,7 @@ Turning the feature off removes shared files and folders on the server for all share recipients, and also on the sync clients and mobile apps. More information is available in the Nextcloud Documentation. - 1.6.1 + 1.6.2 agpl Michael Gapczynski Bjoern Schiessle diff --git a/apps/files_sharing/lib/Controller/ShareAPIController.php b/apps/files_sharing/lib/Controller/ShareAPIController.php index 67ff9eae6d..59b763ecf8 100644 --- a/apps/files_sharing/lib/Controller/ShareAPIController.php +++ b/apps/files_sharing/lib/Controller/ShareAPIController.php @@ -48,6 +48,7 @@ use OCP\IRequest; use OCP\IURLGenerator; use OCP\Files\IRootFolder; use OCP\Lock\LockedException; +use OCP\Share; use OCP\Share\IManager; use OCP\Share\Exceptions\ShareNotFound; use OCP\Share\Exceptions\GenericShareException; @@ -181,15 +182,15 @@ class ShareAPIController extends OCSController { $result['expiration'] = $expiration->format('Y-m-d 00:00:00'); } - if ($share->getShareType() === \OCP\Share::SHARE_TYPE_USER) { + if ($share->getShareType() === Share::SHARE_TYPE_USER) { $sharedWith = $this->userManager->get($share->getSharedWith()); $result['share_with'] = $share->getSharedWith(); $result['share_with_displayname'] = $sharedWith !== null ? $sharedWith->getDisplayName() : $share->getSharedWith(); - } else if ($share->getShareType() === \OCP\Share::SHARE_TYPE_GROUP) { + } else if ($share->getShareType() === Share::SHARE_TYPE_GROUP) { $group = $this->groupManager->get($share->getSharedWith()); $result['share_with'] = $share->getSharedWith(); $result['share_with_displayname'] = $group !== null ? $group->getDisplayName() : $share->getSharedWith(); - } else if ($share->getShareType() === \OCP\Share::SHARE_TYPE_LINK) { + } else if ($share->getShareType() === Share::SHARE_TYPE_LINK) { $result['share_with'] = $share->getPassword(); $result['share_with_displayname'] = $share->getPassword(); @@ -197,16 +198,16 @@ class ShareAPIController extends OCSController { $result['token'] = $share->getToken(); $result['url'] = $this->urlGenerator->linkToRouteAbsolute('files_sharing.sharecontroller.showShare', ['token' => $share->getToken()]); - } else if ($share->getShareType() === \OCP\Share::SHARE_TYPE_REMOTE) { + } else if ($share->getShareType() === Share::SHARE_TYPE_REMOTE || $share->getShareType() || Share::SHARE_TYPE_REMOTE_GROUP) { $result['share_with'] = $share->getSharedWith(); $result['share_with_displayname'] = $this->getDisplayNameFromAddressBook($share->getSharedWith(), 'CLOUD'); $result['token'] = $share->getToken(); - } else if ($share->getShareType() === \OCP\Share::SHARE_TYPE_EMAIL) { + } else if ($share->getShareType() === Share::SHARE_TYPE_EMAIL) { $result['share_with'] = $share->getSharedWith(); $result['password'] = $share->getPassword(); $result['share_with_displayname'] = $this->getDisplayNameFromAddressBook($share->getSharedWith(), 'EMAIL'); $result['token'] = $share->getToken(); - } else if ($share->getShareType() === \OCP\Share::SHARE_TYPE_CIRCLE) { + } else if ($share->getShareType() === Share::SHARE_TYPE_CIRCLE) { // getSharedWith() returns either "name (type, owner)" or // "name (type, owner) [id]", depending on the Circles app version. $hasCircleId = (substr($share->getSharedWith(), -1) === ']'); @@ -301,7 +302,7 @@ class ShareAPIController extends OCSController { throw new OCSNotFoundException($this->l->t('Could not delete share')); } - if ($share->getShareType() === \OCP\Share::SHARE_TYPE_GROUP && + if ($share->getShareType() === Share::SHARE_TYPE_GROUP && $share->getShareOwner() !== $this->currentUser && $share->getSharedBy() !== $this->currentUser) { $this->shareManager->deleteFromSelf($share, $this->currentUser); @@ -388,14 +389,14 @@ class ShareAPIController extends OCSController { $permissions &= ~($permissions & ~$path->getPermissions()); } - if ($shareType === \OCP\Share::SHARE_TYPE_USER) { + if ($shareType === Share::SHARE_TYPE_USER) { // Valid user is required to share if ($shareWith === null || !$this->userManager->userExists($shareWith)) { throw new OCSNotFoundException($this->l->t('Please specify a valid user')); } $share->setSharedWith($shareWith); $share->setPermissions($permissions); - } else if ($shareType === \OCP\Share::SHARE_TYPE_GROUP) { + } else if ($shareType === Share::SHARE_TYPE_GROUP) { if (!$this->shareManager->allowGroupSharing()) { throw new OCSNotFoundException($this->l->t('Group sharing is disabled by the administrator')); } @@ -406,7 +407,7 @@ class ShareAPIController extends OCSController { } $share->setSharedWith($shareWith); $share->setPermissions($permissions); - } else if ($shareType === \OCP\Share::SHARE_TYPE_LINK) { + } else if ($shareType === Share::SHARE_TYPE_LINK) { //Can we even share links? if (!$this->shareManager->shareApiAllowLinks()) { throw new OCSNotFoundException($this->l->t('Public link sharing is disabled by the administrator')); @@ -416,7 +417,7 @@ class ShareAPIController extends OCSController { * For now we only allow 1 link share. * Return the existing link share if this is a duplicate */ - $existingShares = $this->shareManager->getSharesBy($this->currentUser, \OCP\Share::SHARE_TYPE_LINK, $path, false, 1, 0); + $existingShares = $this->shareManager->getSharesBy($this->currentUser, Share::SHARE_TYPE_LINK, $path, false, 1, 0); if (!empty($existingShares)) { return new DataResponse($this->formatShare($existingShares[0])); } @@ -457,21 +458,28 @@ class ShareAPIController extends OCSController { } } - } else if ($shareType === \OCP\Share::SHARE_TYPE_REMOTE) { + } else if ($shareType === Share::SHARE_TYPE_REMOTE) { if (!$this->shareManager->outgoingServer2ServerSharesAllowed()) { throw new OCSForbiddenException($this->l->t('Sharing %s failed because the back end does not allow shares from type %s', [$path->getPath(), $shareType])); } $share->setSharedWith($shareWith); $share->setPermissions($permissions); - } else if ($shareType === \OCP\Share::SHARE_TYPE_EMAIL) { + } else if ($shareType === Share::SHARE_TYPE_REMOTE_GROUP) { + if (!$this->shareManager->outgoingServer2ServerGroupSharesAllowed()) { + throw new OCSForbiddenException($this->l->t('Sharing %s failed because the back end does not allow shares from type %s', [$path->getPath(), $shareType])); + } + + $share->setSharedWith($shareWith); + $share->setPermissions($permissions); + } else if ($shareType === Share::SHARE_TYPE_EMAIL) { if ($share->getNodeType() === 'file') { $share->setPermissions(Constants::PERMISSION_READ); } else { $share->setPermissions($permissions); } $share->setSharedWith($shareWith); - } else if ($shareType === \OCP\Share::SHARE_TYPE_CIRCLE) { + } else if ($shareType === Share::SHARE_TYPE_CIRCLE) { if (!\OC::$server->getAppManager()->isEnabledForUser('circles') || !class_exists('\OCA\Circles\ShareByCircleProvider')) { throw new OCSNotFoundException($this->l->t('You cannot share to a Circle if the app is not enabled')); } @@ -512,9 +520,9 @@ class ShareAPIController extends OCSController { */ private function getSharedWithMe($node = null, bool $includeTags): DataResponse { - $userShares = $this->shareManager->getSharedWith($this->currentUser, \OCP\Share::SHARE_TYPE_USER, $node, -1, 0); - $groupShares = $this->shareManager->getSharedWith($this->currentUser, \OCP\Share::SHARE_TYPE_GROUP, $node, -1, 0); - $circleShares = $this->shareManager->getSharedWith($this->currentUser, \OCP\Share::SHARE_TYPE_CIRCLE, $node, -1, 0); + $userShares = $this->shareManager->getSharedWith($this->currentUser, Share::SHARE_TYPE_USER, $node, -1, 0); + $groupShares = $this->shareManager->getSharedWith($this->currentUser, Share::SHARE_TYPE_GROUP, $node, -1, 0); + $circleShares = $this->shareManager->getSharedWith($this->currentUser, Share::SHARE_TYPE_CIRCLE, $node, -1, 0); $shares = array_merge($userShares, $groupShares, $circleShares); @@ -554,14 +562,14 @@ class ShareAPIController extends OCSController { /** @var \OCP\Share\IShare[] $shares */ $shares = []; foreach ($nodes as $node) { - $shares = array_merge($shares, $this->shareManager->getSharesBy($this->currentUser, \OCP\Share::SHARE_TYPE_USER, $node, false, -1, 0)); - $shares = array_merge($shares, $this->shareManager->getSharesBy($this->currentUser, \OCP\Share::SHARE_TYPE_GROUP, $node, false, -1, 0)); - $shares = array_merge($shares, $this->shareManager->getSharesBy($this->currentUser, \OCP\Share::SHARE_TYPE_LINK, $node, false, -1, 0)); - if($this->shareManager->shareProviderExists(\OCP\Share::SHARE_TYPE_EMAIL)) { - $shares = array_merge($shares, $this->shareManager->getSharesBy($this->currentUser, \OCP\Share::SHARE_TYPE_EMAIL, $node, false, -1, 0)); + $shares = array_merge($shares, $this->shareManager->getSharesBy($this->currentUser, Share::SHARE_TYPE_USER, $node, false, -1, 0)); + $shares = array_merge($shares, $this->shareManager->getSharesBy($this->currentUser, Share::SHARE_TYPE_GROUP, $node, false, -1, 0)); + $shares = array_merge($shares, $this->shareManager->getSharesBy($this->currentUser, Share::SHARE_TYPE_LINK, $node, false, -1, 0)); + if($this->shareManager->shareProviderExists(Share::SHARE_TYPE_EMAIL)) { + $shares = array_merge($shares, $this->shareManager->getSharesBy($this->currentUser, Share::SHARE_TYPE_EMAIL, $node, false, -1, 0)); } if ($this->shareManager->outgoingServer2ServerSharesAllowed()) { - $shares = array_merge($shares, $this->shareManager->getSharesBy($this->currentUser, \OCP\Share::SHARE_TYPE_REMOTE, $node, false, -1, 0)); + $shares = array_merge($shares, $this->shareManager->getSharesBy($this->currentUser, Share::SHARE_TYPE_REMOTE, $node, false, -1, 0)); } } @@ -635,16 +643,16 @@ class ShareAPIController extends OCSController { } // Get all shares - $userShares = $this->shareManager->getSharesBy($this->currentUser, \OCP\Share::SHARE_TYPE_USER, $path, $reshares, -1, 0); - $groupShares = $this->shareManager->getSharesBy($this->currentUser, \OCP\Share::SHARE_TYPE_GROUP, $path, $reshares, -1, 0); - $linkShares = $this->shareManager->getSharesBy($this->currentUser, \OCP\Share::SHARE_TYPE_LINK, $path, $reshares, -1, 0); - if ($this->shareManager->shareProviderExists(\OCP\Share::SHARE_TYPE_EMAIL)) { - $mailShares = $this->shareManager->getSharesBy($this->currentUser, \OCP\Share::SHARE_TYPE_EMAIL, $path, $reshares, -1, 0); + $userShares = $this->shareManager->getSharesBy($this->currentUser, Share::SHARE_TYPE_USER, $path, $reshares, -1, 0); + $groupShares = $this->shareManager->getSharesBy($this->currentUser, Share::SHARE_TYPE_GROUP, $path, $reshares, -1, 0); + $linkShares = $this->shareManager->getSharesBy($this->currentUser, Share::SHARE_TYPE_LINK, $path, $reshares, -1, 0); + if ($this->shareManager->shareProviderExists(Share::SHARE_TYPE_EMAIL)) { + $mailShares = $this->shareManager->getSharesBy($this->currentUser, Share::SHARE_TYPE_EMAIL, $path, $reshares, -1, 0); } else { $mailShares = []; } - if ($this->shareManager->shareProviderExists(\OCP\Share::SHARE_TYPE_CIRCLE)) { - $circleShares = $this->shareManager->getSharesBy($this->currentUser, \OCP\Share::SHARE_TYPE_CIRCLE, $path, $reshares, -1, 0); + if ($this->shareManager->shareProviderExists(Share::SHARE_TYPE_CIRCLE)) { + $circleShares = $this->shareManager->getSharesBy($this->currentUser, Share::SHARE_TYPE_CIRCLE, $path, $reshares, -1, 0); } else { $circleShares = []; } @@ -652,7 +660,12 @@ class ShareAPIController extends OCSController { $shares = array_merge($userShares, $groupShares, $linkShares, $mailShares, $circleShares); if ($this->shareManager->outgoingServer2ServerSharesAllowed()) { - $federatedShares = $this->shareManager->getSharesBy($this->currentUser, \OCP\Share::SHARE_TYPE_REMOTE, $path, $reshares, -1, 0); + $federatedShares = $this->shareManager->getSharesBy($this->currentUser, Share::SHARE_TYPE_REMOTE, $path, $reshares, -1, 0); + $shares = array_merge($shares, $federatedShares); + } + + if ($this->shareManager->outgoingServer2ServerGroupSharesAllowed()) { + $federatedShares = $this->shareManager->getSharesBy($this->currentUser, Share::SHARE_TYPE_REMOTE_GROUP, $path, $reshares, -1, 0); $shares = array_merge($shares, $federatedShares); } @@ -711,7 +724,7 @@ class ShareAPIController extends OCSController { /* * expirationdate, password and publicUpload only make sense for link shares */ - if ($share->getShareType() === \OCP\Share::SHARE_TYPE_LINK) { + if ($share->getShareType() === Share::SHARE_TYPE_LINK) { $newPermissions = null; if ($publicUpload === 'true') { @@ -783,7 +796,7 @@ class ShareAPIController extends OCSController { $share->setPermissions($permissions); } - if ($share->getShareType() === \OCP\Share::SHARE_TYPE_EMAIL) { + if ($share->getShareType() === Share::SHARE_TYPE_EMAIL) { if ($password === '') { $share->setPassword(null); } else if ($password !== null) { @@ -806,8 +819,8 @@ class ShareAPIController extends OCSController { if ($permissions !== null && $share->getShareOwner() !== $this->currentUser) { /* Check if this is an incomming share */ - $incomingShares = $this->shareManager->getSharedWith($this->currentUser, \OCP\Share::SHARE_TYPE_USER, $share->getNode(), -1, 0); - $incomingShares = array_merge($incomingShares, $this->shareManager->getSharedWith($this->currentUser, \OCP\Share::SHARE_TYPE_GROUP, $share->getNode(), -1, 0)); + $incomingShares = $this->shareManager->getSharedWith($this->currentUser, Share::SHARE_TYPE_USER, $share->getNode(), -1, 0); + $incomingShares = array_merge($incomingShares, $this->shareManager->getSharedWith($this->currentUser, Share::SHARE_TYPE_GROUP, $share->getNode(), -1, 0)); /** @var \OCP\Share\IShare[] $incomingShares */ if (!empty($incomingShares)) { @@ -846,13 +859,13 @@ class ShareAPIController extends OCSController { } // If the share is shared with you (or a group you are a member of) - if ($share->getShareType() === \OCP\Share::SHARE_TYPE_USER && + if ($share->getShareType() === Share::SHARE_TYPE_USER && $share->getSharedWith() === $this->currentUser ) { return true; } - if ($checkGroups && $share->getShareType() === \OCP\Share::SHARE_TYPE_GROUP) { + if ($checkGroups && $share->getShareType() === Share::SHARE_TYPE_GROUP) { $sharedWith = $this->groupManager->get($share->getSharedWith()); $user = $this->userManager->get($this->currentUser); if ($user !== null && $sharedWith !== null && $sharedWith->inGroup($user)) { @@ -860,7 +873,7 @@ class ShareAPIController extends OCSController { } } - if ($share->getShareType() === \OCP\Share::SHARE_TYPE_CIRCLE) { + if ($share->getShareType() === Share::SHARE_TYPE_CIRCLE) { // TODO: have a sanity check like above? return true; } @@ -915,7 +928,7 @@ class ShareAPIController extends OCSController { try { - if ($this->shareManager->shareProviderExists(\OCP\Share::SHARE_TYPE_CIRCLE)) { + if ($this->shareManager->shareProviderExists(Share::SHARE_TYPE_CIRCLE)) { $share = $this->shareManager->getShareById('ocCircleShare:' . $id, $this->currentUser); return $share; } @@ -924,7 +937,7 @@ class ShareAPIController extends OCSController { } try { - if ($this->shareManager->shareProviderExists(\OCP\Share::SHARE_TYPE_EMAIL)) { + if ($this->shareManager->shareProviderExists(Share::SHARE_TYPE_EMAIL)) { $share = $this->shareManager->getShareById('ocMailShare:' . $id, $this->currentUser); return $share; } diff --git a/apps/files_sharing/lib/External/Manager.php b/apps/files_sharing/lib/External/Manager.php index 02783560af..4875e7e26c 100644 --- a/apps/files_sharing/lib/External/Manager.php +++ b/apps/files_sharing/lib/External/Manager.php @@ -126,12 +126,15 @@ class Manager { * @param string $password * @param string $name * @param string $owner + * @param int $shareType * @param boolean $accepted * @param string $user * @param int $remoteId + * @param int $parent * @return Mount|null + * @throws \Doctrine\DBAL\DBALException */ - public function addShare($remote, $token, $password, $name, $owner, $accepted=false, $user = null, $remoteId = -1) { + public function addShare($remote, $token, $password, $name, $owner, $shareType, $accepted=false, $user = null, $remoteId = -1, $parent = -1) { $user = $user ? $user : $this->uid; $accepted = $accepted ? 1 : 0; @@ -156,6 +159,7 @@ class Manager { 'mountpoint_hash' => $hash, 'accepted' => $accepted, 'remote_id' => $remoteId, + 'share_type' => $shareType, ]; $i = 1; @@ -174,10 +178,10 @@ class Manager { $query = $this->connection->prepare(' INSERT INTO `*PREFIX*share_external` - (`remote`, `share_token`, `password`, `name`, `owner`, `user`, `mountpoint`, `mountpoint_hash`, `accepted`, `remote_id`) - VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?) + (`remote`, `share_token`, `password`, `name`, `owner`, `user`, `mountpoint`, `mountpoint_hash`, `accepted`, `remote_id`, `parent`, `share_type`) + VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?) '); - $query->execute(array($remote, $token, $password, $name, $owner, $user, $mountPoint, $hash, $accepted, $remoteId)); + $query->execute(array($remote, $token, $password, $name, $owner, $user, $mountPoint, $hash, $accepted, $remoteId, $parent, $shareType)); $options = array( 'remote' => $remote, @@ -223,13 +227,17 @@ class Manager { $mountPoint = Filesystem::normalizePath($mountPoint); $hash = md5($mountPoint); - $acceptShare = $this->connection->prepare(' + if($share['share_type'] === \OCP\Share::SHARE_TYPE_USER) { + $acceptShare = $this->connection->prepare(' UPDATE `*PREFIX*share_external` SET `accepted` = ?, `mountpoint` = ?, `mountpoint_hash` = ? WHERE `id` = ? AND `user` = ?'); - $updated = $acceptShare->execute(array(1, $mountPoint, $hash, $id, $this->uid)); + $updated = $acceptShare->execute(array(1, $mountPoint, $hash, $id, $this->uid)); + } else { + // TODO group share, add additional row for the user who accepted it + } if ($updated === true) { $this->sendFeedbackToRemote($share['remote'], $share['share_token'], $share['remote_id'], 'accept'); \OC_Hook::emit(Share::class, 'federated_share_added', ['server' => $share['remote']]); @@ -537,10 +545,17 @@ class Manager { * @return array list of open server-to-server shares */ private function getShares($accepted) { + $user = $this->userManager->get($this->uid); + $groups = $this->groupManager->getUserGroups($user); + $userGroups = []; + foreach ($groups as $group) { + $userGroups[] = $group->getGID(); + } + $query = 'SELECT `id`, `remote`, `remote_id`, `share_token`, `name`, `owner`, `user`, `mountpoint`, `accepted` FROM `*PREFIX*share_external` - WHERE `user` = ?'; - $parameters = [$this->uid]; + WHERE `user` = ? OR `user` IN (?)'; + $parameters = [$this->uid, implode(',',$userGroups)]; if (!is_null($accepted)) { $query .= ' AND `accepted` = ?'; $parameters[] = (int) $accepted; diff --git a/lib/private/Share/Constants.php b/lib/private/Share/Constants.php index f351f8d7fd..4eb79734c0 100644 --- a/lib/private/Share/Constants.php +++ b/lib/private/Share/Constants.php @@ -37,6 +37,7 @@ class Constants { const SHARE_TYPE_REMOTE = 6; const SHARE_TYPE_CIRCLE = 7; const SHARE_TYPE_GUEST = 8; + const SHARE_TYPE_REMOTE_GROUP = 9; const FORMAT_NONE = -1; const FORMAT_STATUSES = -2; diff --git a/lib/private/Share20/Manager.php b/lib/private/Share20/Manager.php index 5116351a6b..76b523afd1 100644 --- a/lib/private/Share20/Manager.php +++ b/lib/private/Share20/Manager.php @@ -226,6 +226,10 @@ class Manager implements IManager { if ($share->getSharedWith() === null) { throw new \InvalidArgumentException('SharedWith should not be empty'); } + } else if ($share->getShareType() === \OCP\Share::SHARE_TYPE_REMOTE_GROUP) { + if ($share->getSharedWith() === null) { + throw new \InvalidArgumentException('SharedWith should not be empty'); + } } else if ($share->getShareType() === \OCP\Share::SHARE_TYPE_EMAIL) { if ($share->getSharedWith() === null) { throw new \InvalidArgumentException('SharedWith should not be empty'); @@ -1579,6 +1583,13 @@ class Manager implements IManager { return $this->config->getAppValue('files_sharing', 'outgoing_server2server_share_enabled', 'yes') === 'yes'; } + /** + * @inheritdoc + */ + public function outgoingServer2ServerGroupSharesAllowed() { + return $this->config->getAppValue('files_sharing', 'outgoing_server2server_group_share_enabled', 'no') === 'yes'; + } + /** * @inheritdoc */ diff --git a/lib/private/Share20/ProviderFactory.php b/lib/private/Share20/ProviderFactory.php index 7d866db24f..80ef9412ff 100644 --- a/lib/private/Share20/ProviderFactory.php +++ b/lib/private/Share20/ProviderFactory.php @@ -135,7 +135,8 @@ class ProviderFactory implements IProviderFactory { $this->serverContainer->getConfig(), $this->serverContainer->getUserManager(), $this->serverContainer->getCloudIdManager(), - $this->serverContainer->getGlobalScaleConfig() + $this->serverContainer->getGlobalScaleConfig(), + $this->serverContainer->getCloudFederationProviderManager() ); } @@ -250,7 +251,7 @@ class ProviderFactory implements IProviderFactory { $shareType === \OCP\Share::SHARE_TYPE_LINK ) { $provider = $this->defaultShareProvider(); - } else if ($shareType === \OCP\Share::SHARE_TYPE_REMOTE) { + } else if ($shareType === \OCP\Share::SHARE_TYPE_REMOTE || \OCP\Share::SHARE_TYPE_REMOTE_GROUP) { $provider = $this->federatedShareProvider(); } else if ($shareType === \OCP\Share::SHARE_TYPE_EMAIL) { $provider = $this->getShareByMailProvider(); diff --git a/lib/public/Share/IManager.php b/lib/public/Share/IManager.php index d4fc3e1474..302be52332 100644 --- a/lib/public/Share/IManager.php +++ b/lib/public/Share/IManager.php @@ -369,6 +369,14 @@ interface IManager { */ public function outgoingServer2ServerSharesAllowed(); + /** + * Check if outgoing server2server shares are allowed + * @return bool + * @since 14.0.0 + */ + public function outgoingServer2ServerGroupSharesAllowed(); + + /** * Check if a given share provider exists * @param int $shareType From 8bc0de3469422b7e31db0da7bb5716b165f0b562 Mon Sep 17 00:00:00 2001 From: Bjoern Schiessle Date: Fri, 29 Jun 2018 11:07:34 +0200 Subject: [PATCH 03/14] log error messgage Signed-off-by: Bjoern Schiessle --- lib/private/Federation/CloudFederationProviderManager.php | 1 + 1 file changed, 1 insertion(+) diff --git a/lib/private/Federation/CloudFederationProviderManager.php b/lib/private/Federation/CloudFederationProviderManager.php index 254ceb03bf..94b2f05274 100644 --- a/lib/private/Federation/CloudFederationProviderManager.php +++ b/lib/private/Federation/CloudFederationProviderManager.php @@ -157,6 +157,7 @@ class CloudFederationProviderManager implements ICloudFederationProviderManager // we re-throw the exception and fall back to the old behaviour. // (flat re-shares has been introduced in Nextcloud 9.1) if ($e->getCode() === Http::STATUS_INTERNAL_SERVER_ERROR) { + $this->logger->debug($e->getMessage()); throw $e; } } From 4c00c80a838a29205780055d9451b3cd9749b14c Mon Sep 17 00:00:00 2001 From: Bjoern Schiessle Date: Fri, 29 Jun 2018 11:07:57 +0200 Subject: [PATCH 04/14] translate nextcloud share types to OCM share types Signed-off-by: Bjoern Schiessle --- lib/private/Federation/CloudFederationShare.php | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/lib/private/Federation/CloudFederationShare.php b/lib/private/Federation/CloudFederationShare.php index 0c2795188f..e0360d78cb 100644 --- a/lib/private/Federation/CloudFederationShare.php +++ b/lib/private/Federation/CloudFederationShare.php @@ -204,7 +204,11 @@ class CloudFederationShare implements ICloudFederationShare { * @since 14.0.0 */ public function setShareType($shareType) { - $this->share['shareType'] = $shareType; + if ($shareType === 'group' || $shareType === \OCP\Share::SHARE_TYPE_REMOTE_GROUP) { + $this->share['shareType'] = 'group'; + } else { + $this->share['shareType'] = 'user'; + } } /** From 7a48a85dca1a64a9f95851a2020ae0db18ad5e9c Mon Sep 17 00:00:00 2001 From: Bjoern Schiessle Date: Fri, 29 Jun 2018 11:30:48 +0200 Subject: [PATCH 05/14] fix notification when a group share is received Signed-off-by: Bjoern Schiessle --- .../lib/ocm/CloudFederationProviderFiles.php | 24 ++++++++++++------- 1 file changed, 15 insertions(+), 9 deletions(-) diff --git a/apps/federatedfilesharing/lib/ocm/CloudFederationProviderFiles.php b/apps/federatedfilesharing/lib/ocm/CloudFederationProviderFiles.php index cb7b6478e7..67866fa169 100644 --- a/apps/federatedfilesharing/lib/ocm/CloudFederationProviderFiles.php +++ b/apps/federatedfilesharing/lib/ocm/CloudFederationProviderFiles.php @@ -235,20 +235,26 @@ class CloudFederationProviderFiles implements ICloudFederationProvider { $externalManager->addShare($remote, $token, '', $name, $owner, $shareType,false, $shareWith, $remoteId); $shareId = \OC::$server->getDatabaseConnection()->lastInsertId('*PREFIX*share_external'); - $event = $this->activityManager->generateEvent(); - $event->setApp('files_sharing') - ->setType('remote_share') - ->setSubject(RemoteShares::SUBJECT_REMOTE_SHARE_RECEIVED, [$ownerFederatedId, trim($name, '/')]) - ->setAffectedUser($shareWith) - ->setObject('remote_share', (int)$shareId, $name); - \OC::$server->getActivityManager()->publish($event); - if ($shareType === Share::SHARE_TYPE_USER) { + $event = $this->activityManager->generateEvent(); + $event->setApp('files_sharing') + ->setType('remote_share') + ->setSubject(RemoteShares::SUBJECT_REMOTE_SHARE_RECEIVED, [$ownerFederatedId, trim($name, '/')]) + ->setAffectedUser($shareWith) + ->setObject('remote_share', (int)$shareId, $name); + \OC::$server->getActivityManager()->publish($event); $this->notifyAboutNewShare($shareWith, $shareId, $ownerFederatedId, $sharedByFederatedId, $name); } else { $groupMembers = $this->groupManager->get($shareWith)->getUsers(); foreach ($groupMembers as $user) { - $this->notifyAboutNewShare($user, $shareId, $ownerFederatedId, $sharedByFederatedId, $name); + $event = $this->activityManager->generateEvent(); + $event->setApp('files_sharing') + ->setType('remote_share') + ->setSubject(RemoteShares::SUBJECT_REMOTE_SHARE_RECEIVED, [$ownerFederatedId, trim($name, '/')]) + ->setAffectedUser($user->getUID()) + ->setObject('remote_share', (int)$shareId, $name); + \OC::$server->getActivityManager()->publish($event); + $this->notifyAboutNewShare($user->getUID(), $shareId, $ownerFederatedId, $sharedByFederatedId, $name); } } return $shareId; From f5a816262c79d84c373d635fced8ca20cdd0d0ea Mon Sep 17 00:00:00 2001 From: Bjoern Schiessle Date: Fri, 29 Jun 2018 16:35:04 +0200 Subject: [PATCH 06/14] accept/decline group shares Signed-off-by: Bjoern Schiessle --- .../lib/ocm/CloudFederationProviderFiles.php | 2 + .../files_sharing/lib/AppInfo/Application.php | 2 + apps/files_sharing/lib/External/Manager.php | 107 +++++++++++++++--- apps/files_sharing/lib/Hooks.php | 2 + 4 files changed, 98 insertions(+), 15 deletions(-) diff --git a/apps/federatedfilesharing/lib/ocm/CloudFederationProviderFiles.php b/apps/federatedfilesharing/lib/ocm/CloudFederationProviderFiles.php index 67866fa169..1f46c6b677 100644 --- a/apps/federatedfilesharing/lib/ocm/CloudFederationProviderFiles.php +++ b/apps/federatedfilesharing/lib/ocm/CloudFederationProviderFiles.php @@ -228,6 +228,8 @@ class CloudFederationProviderFiles implements ICloudFederationProvider { \OC::$server->query(\OCP\OCS\IDiscoveryService::class), \OC::$server->getCloudFederationProviderManager(), \OC::$server->getCloudFederationFactory(), + \OC::$server->getGroupManager(), + \OC::$server->getUserManager(), $shareWith ); diff --git a/apps/files_sharing/lib/AppInfo/Application.php b/apps/files_sharing/lib/AppInfo/Application.php index e6ab4eb2cf..a7d4755fbf 100644 --- a/apps/files_sharing/lib/AppInfo/Application.php +++ b/apps/files_sharing/lib/AppInfo/Application.php @@ -105,6 +105,8 @@ class Application extends App { $server->query(\OCP\OCS\IDiscoveryService::class), $server->getCloudFederationProviderManager(), $server->getCloudFederationFactory(), + $server->getGroupManager(), + $server->getUserManager(), $uid ); }); diff --git a/apps/files_sharing/lib/External/Manager.php b/apps/files_sharing/lib/External/Manager.php index 4875e7e26c..7b3e77771c 100644 --- a/apps/files_sharing/lib/External/Manager.php +++ b/apps/files_sharing/lib/External/Manager.php @@ -39,6 +39,8 @@ use OCP\Files; use OCP\Files\Storage\IStorageFactory; use OCP\Http\Client\IClientService; use OCP\IDBConnection; +use OCP\IGroupManager; +use OCP\IUserManager; use OCP\Notification\IManager; use OCP\OCS\IDiscoveryService; use OCP\Share; @@ -87,6 +89,12 @@ class Manager { /** @var ICloudFederationFactory */ private $cloudFederationFactory; + /** @var IGroupManager */ + private $groupManager; + + /** @var IUserManager */ + private $userManager; + /** * @param IDBConnection $connection * @param \OC\Files\Mount\Manager $mountManager @@ -96,6 +104,8 @@ class Manager { * @param IDiscoveryService $discoveryService * @param ICloudFederationProviderManager $cloudFederationProviderManager * @param ICloudFederationFactory $cloudFederationFactory + * @param IGroupManager $groupManager + * @param IUserManager $userManager * @param string $uid */ public function __construct(IDBConnection $connection, @@ -106,6 +116,8 @@ class Manager { IDiscoveryService $discoveryService, ICloudFederationProviderManager $cloudFederationProviderManager, ICloudFederationFactory $cloudFederationFactory, + IGroupManager $groupManager, + IUserManager $userManager, $uid) { $this->connection = $connection; $this->mountManager = $mountManager; @@ -116,6 +128,8 @@ class Manager { $this->discoveryService = $discoveryService; $this->cloudFederationProviderManager = $cloudFederationProviderManager; $this->cloudFederationFactory = $cloudFederationFactory; + $this->groupManager = $groupManager; + $this->userManager = $userManager; } /** @@ -176,12 +190,7 @@ class Manager { $mountPoint = Filesystem::normalizePath('/' . $mountPoint); $hash = md5($mountPoint); - $query = $this->connection->prepare(' - INSERT INTO `*PREFIX*share_external` - (`remote`, `share_token`, `password`, `name`, `owner`, `user`, `mountpoint`, `mountpoint_hash`, `accepted`, `remote_id`, `parent`, `share_type`) - VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?) - '); - $query->execute(array($remote, $token, $password, $name, $owner, $user, $mountPoint, $hash, $accepted, $remoteId, $parent, $shareType)); + $this->writeShareToDb($remote, $token, $password, $name, $owner, $user, $mountPoint, $hash, $accepted, $remoteId, $parent, $shareType); $options = array( 'remote' => $remote, @@ -193,6 +202,32 @@ class Manager { return $this->mountShare($options); } + /** + * write remote share to the database + * + * @param $remote + * @param $token + * @param $password + * @param $name + * @param $owner + * @param $user + * @param $mountPoint + * @param $hash + * @param $accepted + * @param $remoteId + * @param $parent + * @param $shareType + * @return bool + */ + private function writeShareToDb($remote, $token, $password, $name, $owner, $user, $mountPoint, $hash, $accepted, $remoteId, $parent, $shareType) { + $query = $this->connection->prepare(' + INSERT INTO `*PREFIX*share_external` + (`remote`, `share_token`, `password`, `name`, `owner`, `user`, `mountpoint`, `mountpoint_hash`, `accepted`, `remote_id`, `parent`, `share_type`) + VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?) + '); + return $query->execute(array($remote, $token, $password, $name, $owner, $user, $mountPoint, $hash, $accepted, $remoteId, $parent, $shareType)); + } + /** * get share * @@ -201,12 +236,27 @@ class Manager { */ public function getShare($id) { $getShare = $this->connection->prepare(' - SELECT `id`, `remote`, `remote_id`, `share_token`, `name`, `owner`, `user`, `mountpoint`, `accepted` + SELECT `id`, `remote`, `remote_id`, `share_token`, `name`, `owner`, `user`, `mountpoint`, `accepted`, `parent`, `share_type`, `password`, `mountpoint_hash` FROM `*PREFIX*share_external` - WHERE `id` = ? AND `user` = ?'); - $result = $getShare->execute(array($id, $this->uid)); + WHERE `id` = ?'); + $result = $getShare->execute(array($id)); + + $share = $result ? $getShare->fetch() : []; + + $validShare = is_array($share) && isset($share['share_type']) && isset($share['user']); + + // check if the user is allowed to access it + if ($validShare && (int)$share['share_type'] === Share::SHARE_TYPE_USER && $share['user'] === $this->uid) { + return $share; + } else if ($validShare && (int)$share['share_type'] === Share::SHARE_TYPE_GROUP) { + $user = $this->userManager->get($this->uid); + if ($this->groupManager->get($share['user'])->inGroup($user)) { + return $share; + } + } + + return false; - return $result ? $getShare->fetch() : false; } /** @@ -227,7 +277,7 @@ class Manager { $mountPoint = Filesystem::normalizePath($mountPoint); $hash = md5($mountPoint); - if($share['share_type'] === \OCP\Share::SHARE_TYPE_USER) { + if($share['share_type'] === Share::SHARE_TYPE_USER) { $acceptShare = $this->connection->prepare(' UPDATE `*PREFIX*share_external` SET `accepted` = ?, @@ -236,6 +286,17 @@ class Manager { WHERE `id` = ? AND `user` = ?'); $updated = $acceptShare->execute(array(1, $mountPoint, $hash, $id, $this->uid)); } else { + $result = $this->writeShareToDb( + $share['remote'], + $share['share_token'], + $share['password'], + $share['name'], + $share['owner'], + $this->uid, + $mountPoint, $hash, 1, + $share['remote_id'], + $id, + $share['share_type']); // TODO group share, add additional row for the user who accepted it } if ($updated === true) { @@ -260,18 +321,34 @@ class Manager { public function declineShare($id) { $share = $this->getShare($id); + $result = false; - if ($share) { + if ($share && (int)$share['share_type'] === Share::SHARE_TYPE_USER) { $removeShare = $this->connection->prepare(' DELETE FROM `*PREFIX*share_external` WHERE `id` = ? AND `user` = ?'); $removeShare->execute(array($id, $this->uid)); $this->sendFeedbackToRemote($share['remote'], $share['share_token'], $share['remote_id'], 'decline'); $this->processNotification($id); - return true; + $result = true; + } else if ($share && (int)$share['share_type'] === Share::SHARE_TYPE_GROUP) { + $result = $this->writeShareToDb( + $share['remote'], + $share['share_token'], + $share['password'], + $share['name'], + $share['owner'], + $this->uid, + $share['mountpoint'], + $share['mountpoint_hash'], + 0, + $share['remote_id'], + $id, + $share['share_type']); + $this->processNotification($id); } - return false; + return $result; } /** @@ -305,7 +382,7 @@ class Manager { $federationEndpoints = $this->discoveryService->discover($remote, 'FEDERATED_SHARING'); $endpoint = isset($federationEndpoints['share']) ? $federationEndpoints['share'] : '/ocs/v2.php/cloud/shares'; - $url = rtrim($remote, '/') . $endpoint . '/' . $remoteId . '/' . $feedback . '?format=' . \OCP\Share::RESPONSE_FORMAT; + $url = rtrim($remote, '/') . $endpoint . '/' . $remoteId . '/' . $feedback . '?format=' . Share::RESPONSE_FORMAT; $fields = array('token' => $token); $client = $this->clientService->newClient(); diff --git a/apps/files_sharing/lib/Hooks.php b/apps/files_sharing/lib/Hooks.php index cd66fd7702..99e876eaaf 100644 --- a/apps/files_sharing/lib/Hooks.php +++ b/apps/files_sharing/lib/Hooks.php @@ -42,6 +42,8 @@ class Hooks { \OC::$server->query(\OCP\OCS\IDiscoveryService::class), \OC::$server->getCloudFederationProviderManager(), \OC::$server->getCloudFederationFactory(), + \OC::$server->getGroupManager(), + \OC::$server->getUserManager(), $params['uid']); $manager->removeUserShares($params['uid']); From 7d2add622e1948f85e0c2abe4fc5a71123b7bcde Mon Sep 17 00:00:00 2001 From: Bjoern Schiessle Date: Fri, 29 Jun 2018 17:47:36 +0200 Subject: [PATCH 07/14] unshare from self Signed-off-by: Bjoern Schiessle --- apps/files_sharing/lib/External/Manager.php | 25 ++++++++++++--------- 1 file changed, 15 insertions(+), 10 deletions(-) diff --git a/apps/files_sharing/lib/External/Manager.php b/apps/files_sharing/lib/External/Manager.php index 7b3e77771c..9f8acbce4c 100644 --- a/apps/files_sharing/lib/External/Manager.php +++ b/apps/files_sharing/lib/External/Manager.php @@ -515,28 +515,33 @@ class Manager { $hash = md5($mountPoint); $getShare = $this->connection->prepare(' - SELECT `remote`, `share_token`, `remote_id` + SELECT `remote`, `share_token`, `remote_id`, `share_type`, `id` FROM `*PREFIX*share_external` WHERE `mountpoint_hash` = ? AND `user` = ?'); $result = $getShare->execute(array($hash, $this->uid)); - if ($result) { + $share = $getShare->fetch(); + $getShare->closeCursor(); + if ($result && (int)$share['share_type'] === Share::SHARE_TYPE_USER) { try { - $share = $getShare->fetch(); $this->sendFeedbackToRemote($share['remote'], $share['share_token'], $share['remote_id'], 'decline'); } catch (\Exception $e) { // if we fail to notify the remote (probably cause the remote is down) // we still want the share to be gone to prevent undeletable remotes } - } - $getShare->closeCursor(); - $query = $this->connection->prepare(' + $query = $this->connection->prepare(' DELETE FROM `*PREFIX*share_external` - WHERE `mountpoint_hash` = ? - AND `user` = ? - '); - $result = (bool)$query->execute(array($hash, $this->uid)); + WHERE `id` = ? + '); + $result = (bool)$query->execute(array((int)$share['id'])); + } else if ($result && (int)$share['share_type'] === Share::SHARE_TYPE_GROUP) { + $query = $this->connection->prepare(' + UPDATE `*PREFIX*share_external` + SET `accepted` = ? + WHERE `id` = ?'); + $result = (bool)$query->execute(array(0, (int)$share['id'])); + } if($result) { $this->removeReShares($id); From 1d5a9c3f979b23bac98243ba2245a21bdff722c8 Mon Sep 17 00:00:00 2001 From: Bjoern Schiessle Date: Fri, 29 Jun 2018 18:02:12 +0200 Subject: [PATCH 08/14] handle unshare request from owner Signed-off-by: Bjoern Schiessle --- .../lib/ocm/CloudFederationProviderFiles.php | 43 +++++++++++-------- 1 file changed, 25 insertions(+), 18 deletions(-) diff --git a/apps/federatedfilesharing/lib/ocm/CloudFederationProviderFiles.php b/apps/federatedfilesharing/lib/ocm/CloudFederationProviderFiles.php index 1f46c6b677..1d922ce60b 100644 --- a/apps/federatedfilesharing/lib/ocm/CloudFederationProviderFiles.php +++ b/apps/federatedfilesharing/lib/ocm/CloudFederationProviderFiles.php @@ -578,25 +578,32 @@ class CloudFederationProviderFiles implements ICloudFederationProvider { $qb->execute(); - if ($share['accepted']) { - $path = trim($mountpoint, '/'); - } else { - $path = trim($share['name'], '/'); + // delete all child in case of a group share + $qb = $this->connection->getQueryBuilder(); + $qb->delete('share_external') + ->where($qb->expr()->eq('parent', $qb->createNamedParameter((int)$share['id']))); + $qb->execute(); + + if ((int)$share['share_type'] === Share::SHARE_TYPE_USER) { + if ($share['accepted']) { + $path = trim($mountpoint, '/'); + } else { + $path = trim($share['name'], '/'); + } + $notification = $this->notificationManager->createNotification(); + $notification->setApp('files_sharing') + ->setUser($share['user']) + ->setObject('remote_share', (int)$share['id']); + $this->notificationManager->markProcessed($notification); + + $event = $this->activityManager->generateEvent(); + $event->setApp('files_sharing') + ->setType('remote_share') + ->setSubject(RemoteShares::SUBJECT_REMOTE_SHARE_UNSHARED, [$owner->getId(), $path]) + ->setAffectedUser($user) + ->setObject('remote_share', (int)$share['id'], $path); + \OC::$server->getActivityManager()->publish($event); } - - $notification = $this->notificationManager->createNotification(); - $notification->setApp('files_sharing') - ->setUser($share['user']) - ->setObject('remote_share', (int)$share['id']); - $this->notificationManager->markProcessed($notification); - - $event = $this->activityManager->generateEvent(); - $event->setApp('files_sharing') - ->setType('remote_share') - ->setSubject(RemoteShares::SUBJECT_REMOTE_SHARE_UNSHARED, [$owner->getId(), $path]) - ->setAffectedUser($user) - ->setObject('remote_share', (int)$share['id'], $path); - \OC::$server->getActivityManager()->publish($event); } return []; From 2abc70563221457843346e452f916bf2b49a00ad Mon Sep 17 00:00:00 2001 From: Bjoern Schiessle Date: Mon, 2 Jul 2018 11:29:03 +0200 Subject: [PATCH 09/14] start to get fed group shares into the share dialog Signed-off-by: Bjoern Schiessle --- .../lib/Controller/ShareesAPIController.php | 10 +++ core/js/share.js | 1 + core/js/sharedialogshareelistview.js | 8 +- core/js/sharedialogview.js | 26 ++++++- .../Collaborators/RemoteGroupPlugin.php | 78 +++++++++++++++++++ lib/private/Server.php | 2 + 6 files changed, 120 insertions(+), 5 deletions(-) create mode 100644 lib/private/Collaboration/Collaborators/RemoteGroupPlugin.php diff --git a/apps/files_sharing/lib/Controller/ShareesAPIController.php b/apps/files_sharing/lib/Controller/ShareesAPIController.php index d25f24f6f7..c60e873b82 100644 --- a/apps/files_sharing/lib/Controller/ShareesAPIController.php +++ b/apps/files_sharing/lib/Controller/ShareesAPIController.php @@ -67,12 +67,14 @@ class ShareesAPIController extends OCSController { 'users' => [], 'groups' => [], 'remotes' => [], + 'remote_groups' => [], 'emails' => [], 'circles' => [], ], 'users' => [], 'groups' => [], 'remotes' => [], + 'remote_groups' => [], 'emails' => [], 'lookup' => [], 'circles' => [], @@ -153,6 +155,10 @@ class ShareesAPIController extends OCSController { $shareTypes[] = Share::SHARE_TYPE_REMOTE; } + if ($this->isRemoteGroupSharingAllowed($itemType)) { + $shareTypes[] = Share::SHARE_TYPE_REMOTE_GROUP; + } + if ($this->shareManager->shareProviderExists(Share::SHARE_TYPE_EMAIL)) { $shareTypes[] = Share::SHARE_TYPE_EMAIL; } @@ -216,6 +222,10 @@ class ShareesAPIController extends OCSController { } } + protected function isRemoteGroupSharingAllowed(string $itemType): bool { + return true; + } + /** * Generates a bunch of pagination links for the current page diff --git a/core/js/share.js b/core/js/share.js index f301de2541..e4d9364b2d 100644 --- a/core/js/share.js +++ b/core/js/share.js @@ -11,6 +11,7 @@ OC.Share = _.extend(OC.Share || {}, { SHARE_TYPE_REMOTE:6, SHARE_TYPE_CIRCLE:7, SHARE_TYPE_GUEST:8, + SHARE_TYPE_REMOTE_GROUP:9, /** * Regular expression for splitting parts of remote share owners: diff --git a/core/js/sharedialogshareelistview.js b/core/js/sharedialogshareelistview.js index 1e873a7208..0ff4c36b71 100644 --- a/core/js/sharedialogshareelistview.js +++ b/core/js/sharedialogshareelistview.js @@ -198,6 +198,8 @@ shareWithDisplayName = shareWithDisplayName + " (" + t('core', 'group') + ')'; } else if (shareType === OC.Share.SHARE_TYPE_REMOTE) { shareWithDisplayName = shareWithDisplayName + " (" + t('core', 'remote') + ')'; + } else if (shareType === OC.Share.SHARE_TYPE_REMOTE_GROUP) { + shareWithDisplayName = shareWithDisplayName + " (" + t('core', 'remote group') + ')'; } else if (shareType === OC.Share.SHARE_TYPE_EMAIL) { shareWithDisplayName = shareWithDisplayName + " (" + t('core', 'email') + ')'; } else if (shareType === OC.Share.SHARE_TYPE_CIRCLE) { @@ -207,7 +209,10 @@ shareWithTitle = shareWith + " (" + t('core', 'group') + ')'; } else if (shareType === OC.Share.SHARE_TYPE_REMOTE) { shareWithTitle = shareWith + " (" + t('core', 'remote') + ')'; - } else if (shareType === OC.Share.SHARE_TYPE_EMAIL) { + } else if (shareType === OC.Share.SHARE_TYPE_REMOTE_GROUP) { + shareWithTitle = shareWith + " (" + t('core', 'remote group') + ')'; + } + else if (shareType === OC.Share.SHARE_TYPE_EMAIL) { shareWithTitle = shareWith + " (" + t('core', 'email') + ')'; } else if (shareType === OC.Share.SHARE_TYPE_CIRCLE) { shareWithTitle = shareWith; @@ -243,6 +248,7 @@ shareId: this.model.get('shares')[shareIndex].id, modSeed: shareType !== OC.Share.SHARE_TYPE_USER && shareType !== OC.Share.SHARE_TYPE_CIRCLE, isRemoteShare: shareType === OC.Share.SHARE_TYPE_REMOTE, + isRemoteGroupShare: shareType === OC.Share.SHARE_TYPE_REMOTE_GROUP, isMailShare: shareType === OC.Share.SHARE_TYPE_EMAIL, isCircleShare: shareType === OC.Share.SHARE_TYPE_CIRCLE, isFileSharedByMail: shareType === OC.Share.SHARE_TYPE_EMAIL && !this.model.isFolder(), diff --git a/core/js/sharedialogview.js b/core/js/sharedialogview.js index dede768fad..d886e45856 100644 --- a/core/js/sharedialogview.js +++ b/core/js/sharedialogview.js @@ -93,6 +93,9 @@ this.configModel.on('change:isRemoteShareAllowed', function() { view.render(); }); + this.configModel.on('change:isRemoteGroupShareAllowed', function() { + view.render(); + }); this.model.on('change:permissions', function() { view.render(); }); @@ -161,7 +164,7 @@ }, function (result) { if (result.ocs.meta.statuscode === 100) { - var filter = function(users, groups, remotes, emails, circles) { + var filter = function(users, groups, remotes, remote_groups, emails, circles) { if (typeof(emails) === 'undefined') { emails = []; } @@ -172,6 +175,7 @@ var usersLength; var groupsLength; var remotesLength; + var remoteGroupsLength; var emailsLength; var circlesLength; @@ -228,6 +232,14 @@ break; } } + } else if (share.share_type === OC.Share.SHARE_TYPE_REMOTE_GROUP) { + remoteGroupsLength = remote_groups.length; + for (j = 0; j < remoteGroupsLength; j++) { + if (remote_groups[j].value.shareWith === share.share_with) { + remote_groups.splice(j, 1); + break; + } + } } else if (share.share_type === OC.Share.SHARE_TYPE_EMAIL) { emailsLength = emails.length; for (j = 0; j < emailsLength; j++) { @@ -252,6 +264,7 @@ result.ocs.data.exact.users, result.ocs.data.exact.groups, result.ocs.data.exact.remotes, + result.ocs.data.exact.remote_groups, result.ocs.data.exact.emails, result.ocs.data.exact.circles ); @@ -259,6 +272,7 @@ var exactUsers = result.ocs.data.exact.users; var exactGroups = result.ocs.data.exact.groups; var exactRemotes = result.ocs.data.exact.remotes; + var exactRemoteGroups = result.ocs.data.exact.remote_groups; var exactEmails = []; if (typeof(result.ocs.data.emails) !== 'undefined') { exactEmails = result.ocs.data.exact.emails; @@ -268,12 +282,13 @@ exactCircles = result.ocs.data.exact.circles; } - var exactMatches = exactUsers.concat(exactGroups).concat(exactRemotes).concat(exactEmails).concat(exactCircles); + var exactMatches = exactUsers.concat(exactGroups).concat(exactRemotes).concat(exactRemoteGroups).concat(exactEmails).concat(exactCircles); filter( result.ocs.data.users, result.ocs.data.groups, result.ocs.data.remotes, + result.ocs.data.remote_groups, result.ocs.data.emails, result.ocs.data.circles ); @@ -281,6 +296,7 @@ var users = result.ocs.data.users; var groups = result.ocs.data.groups; var remotes = result.ocs.data.remotes; + var remoteGroups = result.ocs.data.remote_groups; var lookup = result.ocs.data.lookup; var emails = []; if (typeof(result.ocs.data.emails) !== 'undefined') { @@ -291,7 +307,7 @@ circles = result.ocs.data.circles; } - var suggestions = exactMatches.concat(users).concat(groups).concat(remotes).concat(emails).concat(circles).concat(lookup); + var suggestions = exactMatches.concat(users).concat(groups).concat(remotes).concat(remoteGroups).concat(emails).concat(circles).concat(lookup); deferred.resolve(suggestions, exactMatches); } else { @@ -414,7 +430,9 @@ if (item.value.shareType === OC.Share.SHARE_TYPE_GROUP) { text = t('core', '{sharee} (group)', { sharee: text }, undefined, { escape: false }); } else if (item.value.shareType === OC.Share.SHARE_TYPE_REMOTE) { - text = t('core', '{sharee} (remote)', { sharee: text }, undefined, { escape: false }); + text = t('core', '{sharee} (remote)', {sharee: text}, undefined, {escape: false}); + } else if (item.value.shareType === OC.Share.SHARE_TYPE_REMOTE_GROUP) { + text = t('core', '{sharee} (remote group)', { sharee: text }, undefined, { escape: false }); } else if (item.value.shareType === OC.Share.SHARE_TYPE_EMAIL) { text = t('core', '{sharee} (email)', { sharee: text }, undefined, { escape: false }); } else if (item.value.shareType === OC.Share.SHARE_TYPE_CIRCLE) { diff --git a/lib/private/Collaboration/Collaborators/RemoteGroupPlugin.php b/lib/private/Collaboration/Collaborators/RemoteGroupPlugin.php new file mode 100644 index 0000000000..c48a1fbae2 --- /dev/null +++ b/lib/private/Collaboration/Collaborators/RemoteGroupPlugin.php @@ -0,0 +1,78 @@ + + * + * @author Arthur Schiwon + * + * @license GNU AGPL version 3 or any later version + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU Affero General Public License as + * published by the Free Software Foundation, either version 3 of the + * License, or (at your option) any later version. + * + * 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 + * along with this program. If not, see . + * + */ + +namespace OC\Collaboration\Collaborators; + + +use OCP\Collaboration\Collaborators\ISearchPlugin; +use OCP\Collaboration\Collaborators\ISearchResult; +use OCP\Collaboration\Collaborators\SearchResultType; +use OCP\Contacts\IManager; +use OCP\Federation\ICloudFederationProviderManager; +use OCP\Federation\ICloudIdManager; +use OCP\IConfig; +use OCP\Share; + +class RemoteGroupPlugin implements ISearchPlugin { + protected $shareeEnumeration; + + /** @var ICloudIdManager */ + private $cloudIdManager; + /** @var IConfig */ + private $config; + /** @var bool */ + private $enabled = false; + + public function __construct(ICloudFederationProviderManager $cloudFederationProviderManager, ICloudIdManager $cloudIdManager) { + try { + $fileSharingProvider = $cloudFederationProviderManager->getCloudFederationProvider('file'); + $supportedShareTypes = $fileSharingProvider->getSupportedShareTypes(); + if (in_array('group', $supportedShareTypes)) { + $this->enabled = true; + } + } catch (\Exception $e) { + // do nothing, just don't enable federated group shares + } + $this->cloudIdManager = $cloudIdManager; + } + + public function search($search, $limit, $offset, ISearchResult $searchResult) { + $result = ['wide' => [], 'exact' => []]; + $resultType = new SearchResultType('remote_groups'); + + if ($this->enabled && $this->cloudIdManager->isValidCloudId($search) && $offset === 0) { + $result['exact'][] = [ + 'label' => $search, + 'value' => [ + 'shareType' => Share::SHARE_TYPE_REMOTE_GROUP, + 'shareWith' => $search, + ], + ]; + } + + $searchResult->addResultSet($resultType, $result['wide'], $result['exact']); + + return true; + } + +} diff --git a/lib/private/Server.php b/lib/private/Server.php index 90e9e81c10..c9f8001631 100644 --- a/lib/private/Server.php +++ b/lib/private/Server.php @@ -60,6 +60,7 @@ use OC\Authentication\LoginCredentials\Store; use OC\Authentication\Token\IProvider; use OC\Collaboration\Collaborators\GroupPlugin; use OC\Collaboration\Collaborators\MailPlugin; +use OC\Collaboration\Collaborators\RemoteGroupPlugin; use OC\Collaboration\Collaborators\RemotePlugin; use OC\Collaboration\Collaborators\UserPlugin; use OC\Command\CronBus; @@ -1066,6 +1067,7 @@ class Server extends ServerContainer implements IServerContainer { $instance->registerPlugin(['shareType' => 'SHARE_TYPE_GROUP', 'class' => GroupPlugin::class]); $instance->registerPlugin(['shareType' => 'SHARE_TYPE_EMAIL', 'class' => MailPlugin::class]); $instance->registerPlugin(['shareType' => 'SHARE_TYPE_REMOTE', 'class' => RemotePlugin::class]); + $instance->registerPlugin(['shareType' => 'SHARE_TYPE_REMOTE_GROUP', 'class' => RemoteGroupPlugin::class]); return $instance; }); From 5b06a7d773d303a9b01c98eb714fd1829129646f Mon Sep 17 00:00:00 2001 From: Bjoern Schiessle Date: Wed, 4 Jul 2018 10:33:13 +0200 Subject: [PATCH 10/14] update autoloader Signed-off-by: Bjoern Schiessle --- lib/composer/composer/autoload_classmap.php | 1 + lib/composer/composer/autoload_static.php | 1 + 2 files changed, 2 insertions(+) diff --git a/lib/composer/composer/autoload_classmap.php b/lib/composer/composer/autoload_classmap.php index faf17a109b..7113f60bc6 100644 --- a/lib/composer/composer/autoload_classmap.php +++ b/lib/composer/composer/autoload_classmap.php @@ -471,6 +471,7 @@ return array( 'OC\\Collaboration\\Collaborators\\GroupPlugin' => $baseDir . '/lib/private/Collaboration/Collaborators/GroupPlugin.php', 'OC\\Collaboration\\Collaborators\\LookupPlugin' => $baseDir . '/lib/private/Collaboration/Collaborators/LookupPlugin.php', 'OC\\Collaboration\\Collaborators\\MailPlugin' => $baseDir . '/lib/private/Collaboration/Collaborators/MailPlugin.php', + 'OC\\Collaboration\\Collaborators\\RemoteGroupPlugin' => $baseDir . '/lib/private/Collaboration/Collaborators/RemoteGroupPlugin.php', 'OC\\Collaboration\\Collaborators\\RemotePlugin' => $baseDir . '/lib/private/Collaboration/Collaborators/RemotePlugin.php', 'OC\\Collaboration\\Collaborators\\Search' => $baseDir . '/lib/private/Collaboration/Collaborators/Search.php', 'OC\\Collaboration\\Collaborators\\SearchResult' => $baseDir . '/lib/private/Collaboration/Collaborators/SearchResult.php', diff --git a/lib/composer/composer/autoload_static.php b/lib/composer/composer/autoload_static.php index a8d748137e..ffb50df5b0 100644 --- a/lib/composer/composer/autoload_static.php +++ b/lib/composer/composer/autoload_static.php @@ -501,6 +501,7 @@ class ComposerStaticInit53792487c5a8370acc0b06b1a864ff4c 'OC\\Collaboration\\Collaborators\\GroupPlugin' => __DIR__ . '/../../..' . '/lib/private/Collaboration/Collaborators/GroupPlugin.php', 'OC\\Collaboration\\Collaborators\\LookupPlugin' => __DIR__ . '/../../..' . '/lib/private/Collaboration/Collaborators/LookupPlugin.php', 'OC\\Collaboration\\Collaborators\\MailPlugin' => __DIR__ . '/../../..' . '/lib/private/Collaboration/Collaborators/MailPlugin.php', + 'OC\\Collaboration\\Collaborators\\RemoteGroupPlugin' => __DIR__ . '/../../..' . '/lib/private/Collaboration/Collaborators/RemoteGroupPlugin.php', 'OC\\Collaboration\\Collaborators\\RemotePlugin' => __DIR__ . '/../../..' . '/lib/private/Collaboration/Collaborators/RemotePlugin.php', 'OC\\Collaboration\\Collaborators\\Search' => __DIR__ . '/../../..' . '/lib/private/Collaboration/Collaborators/Search.php', 'OC\\Collaboration\\Collaborators\\SearchResult' => __DIR__ . '/../../..' . '/lib/private/Collaboration/Collaborators/SearchResult.php', From 3942d731d21e6369c395358699a9561519992069 Mon Sep 17 00:00:00 2001 From: Bjoern Schiessle Date: Wed, 4 Jul 2018 10:39:13 +0200 Subject: [PATCH 11/14] update unit tests Signed-off-by: Bjoern Schiessle --- .../tests/FederatedShareProviderTest.php | 28 ++++++- .../tests/Settings/AdminTest.php | 21 ++++- .../lib/Controller/ShareesAPIController.php | 8 +- apps/files_sharing/lib/External/Manager.php | 8 +- apps/files_sharing/lib/ShareBackend/File.php | 4 + .../Controller/ShareesAPIControllerTest.php | 78 +++++++++++-------- .../tests/External/ManagerTest.php | 13 ++++ 7 files changed, 120 insertions(+), 40 deletions(-) diff --git a/apps/federatedfilesharing/tests/FederatedShareProviderTest.php b/apps/federatedfilesharing/tests/FederatedShareProviderTest.php index 52266383b3..98ad8832fa 100644 --- a/apps/federatedfilesharing/tests/FederatedShareProviderTest.php +++ b/apps/federatedfilesharing/tests/FederatedShareProviderTest.php @@ -33,6 +33,7 @@ use OCA\FederatedFileSharing\AddressHandler; use OCA\FederatedFileSharing\FederatedShareProvider; use OCA\FederatedFileSharing\Notifications; use OCA\FederatedFileSharing\TokenHandler; +use OCP\Federation\ICloudFederationProviderManager; use OCP\Federation\ICloudIdManager; use OCP\Files\File; use OCP\Files\IRootFolder; @@ -80,6 +81,8 @@ class FederatedShareProviderTest extends \Test\TestCase { /** @var ICloudIdManager */ private $cloudIdManager; + /** @var \PHPUnit_Framework_MockObject_MockObject|ICloudFederationProviderManager */ + private $cloudFederationProviderManager; public function setUp() { parent::setUp(); @@ -107,6 +110,8 @@ class FederatedShareProviderTest extends \Test\TestCase { $this->userManager->expects($this->any())->method('userExists')->willReturn(true); + $this->cloudFederationProviderManager = $this->createMock(ICloudFederationProviderManager::class); + $this->provider = new FederatedShareProvider( $this->connection, $this->addressHandler, @@ -118,7 +123,8 @@ class FederatedShareProviderTest extends \Test\TestCase { $this->config, $this->userManager, $this->cloudIdManager, - $this->gsConfig + $this->gsConfig, + $this->cloudFederationProviderManager ); $this->shareManager = \OC::$server->getShareManager(); @@ -141,6 +147,7 @@ class FederatedShareProviderTest extends \Test\TestCase { ->setSharedBy('sharedBy') ->setShareOwner('shareOwner') ->setPermissions(19) + ->setShareType(\OCP\Share::SHARE_TYPE_REMOTE) ->setNode($node); $this->tokenHandler->method('generateToken')->willReturn('token'); @@ -212,6 +219,7 @@ class FederatedShareProviderTest extends \Test\TestCase { ->setSharedBy('sharedBy') ->setShareOwner('shareOwner') ->setPermissions(19) + ->setShareType(\OCP\Share::SHARE_TYPE_REMOTE) ->setNode($node); $this->tokenHandler->method('generateToken')->willReturn('token'); @@ -268,6 +276,7 @@ class FederatedShareProviderTest extends \Test\TestCase { ->setSharedBy('sharedBy') ->setShareOwner('shareOwner') ->setPermissions(19) + ->setShareType(\OCP\Share::SHARE_TYPE_REMOTE) ->setNode($node); $this->tokenHandler->method('generateToken')->willReturn('token'); @@ -367,6 +376,7 @@ class FederatedShareProviderTest extends \Test\TestCase { ->setSharedBy('sharedBy') ->setShareOwner('shareOwner') ->setPermissions(19) + ->setShareType(\OCP\Share::SHARE_TYPE_REMOTE) ->setNode($node); $this->tokenHandler->method('generateToken')->willReturn('token'); @@ -417,7 +427,8 @@ class FederatedShareProviderTest extends \Test\TestCase { $this->config, $this->userManager, $this->cloudIdManager, - $this->gsConfig + $this->gsConfig, + $this->cloudFederationProviderManager ] )->setMethods(['sendPermissionUpdate'])->getMock(); @@ -435,6 +446,7 @@ class FederatedShareProviderTest extends \Test\TestCase { ->setSharedBy($sharedBy) ->setShareOwner($owner) ->setPermissions(19) + ->setShareType(\OCP\Share::SHARE_TYPE_REMOTE) ->setNode($node); $this->tokenHandler->method('generateToken')->willReturn('token'); @@ -505,6 +517,7 @@ class FederatedShareProviderTest extends \Test\TestCase { ->setSharedBy('sharedBy') ->setShareOwner('shareOwner') ->setPermissions(19) + ->setShareType(\OCP\Share::SHARE_TYPE_REMOTE) ->setNode($node); $this->provider->create($share); @@ -513,6 +526,7 @@ class FederatedShareProviderTest extends \Test\TestCase { ->setSharedBy('sharedBy2') ->setShareOwner('shareOwner') ->setPermissions(19) + ->setShareType(\OCP\Share::SHARE_TYPE_REMOTE) ->setNode($node); $this->provider->create($share2); @@ -543,6 +557,7 @@ class FederatedShareProviderTest extends \Test\TestCase { ->setSharedBy('sharedBy') ->setShareOwner('shareOwner') ->setPermissions(19) + ->setShareType(\OCP\Share::SHARE_TYPE_REMOTE) ->setNode($node); $this->provider->create($share); @@ -555,6 +570,7 @@ class FederatedShareProviderTest extends \Test\TestCase { ->setSharedBy('sharedBy') ->setShareOwner('shareOwner') ->setPermissions(19) + ->setShareType(\OCP\Share::SHARE_TYPE_REMOTE) ->setNode($node2); $this->provider->create($share2); @@ -584,6 +600,7 @@ class FederatedShareProviderTest extends \Test\TestCase { ->setSharedBy('shareOwner') ->setShareOwner('shareOwner') ->setPermissions(19) + ->setShareType(\OCP\Share::SHARE_TYPE_REMOTE) ->setNode($node); $this->provider->create($share); @@ -592,6 +609,7 @@ class FederatedShareProviderTest extends \Test\TestCase { ->setSharedBy('sharedBy') ->setShareOwner('shareOwner') ->setPermissions(19) + ->setShareType(\OCP\Share::SHARE_TYPE_REMOTE) ->setNode($node); $this->provider->create($share2); @@ -628,6 +646,7 @@ class FederatedShareProviderTest extends \Test\TestCase { ->setSharedBy('sharedBy') ->setShareOwner('shareOwner') ->setPermissions(19) + ->setShareType(\OCP\Share::SHARE_TYPE_REMOTE) ->setNode($node); $this->provider->create($share); @@ -636,6 +655,7 @@ class FederatedShareProviderTest extends \Test\TestCase { ->setSharedBy('sharedBy') ->setShareOwner('shareOwner') ->setPermissions(19) + ->setShareType(\OCP\Share::SHARE_TYPE_REMOTE) ->setNode($node); $this->provider->create($share2); @@ -826,6 +846,7 @@ class FederatedShareProviderTest extends \Test\TestCase { ->setSharedBy($u1->getUID()) ->setShareOwner($u1->getUID()) ->setPermissions(\OCP\Constants::PERMISSION_READ) + ->setShareType(\OCP\Share::SHARE_TYPE_REMOTE) ->setNode($file1); $this->provider->create($share1); @@ -834,6 +855,7 @@ class FederatedShareProviderTest extends \Test\TestCase { ->setSharedBy($u2->getUID()) ->setShareOwner($u1->getUID()) ->setPermissions(\OCP\Constants::PERMISSION_READ) + ->setShareType(\OCP\Share::SHARE_TYPE_REMOTE) ->setNode($file2); $this->provider->create($share2); @@ -880,6 +902,7 @@ class FederatedShareProviderTest extends \Test\TestCase { ->setSharedBy($u1->getUID()) ->setShareOwner($u1->getUID()) ->setPermissions(\OCP\Constants::PERMISSION_READ) + ->setShareType(\OCP\Share::SHARE_TYPE_REMOTE) ->setNode($file1); $this->provider->create($share1); @@ -888,6 +911,7 @@ class FederatedShareProviderTest extends \Test\TestCase { ->setSharedBy($u1->getUID()) ->setShareOwner($u1->getUID()) ->setPermissions(\OCP\Constants::PERMISSION_READ) + ->setShareType(\OCP\Share::SHARE_TYPE_REMOTE) ->setNode($file1); $this->provider->create($share2); diff --git a/apps/federatedfilesharing/tests/Settings/AdminTest.php b/apps/federatedfilesharing/tests/Settings/AdminTest.php index debd2bec63..f0cf3b77d3 100644 --- a/apps/federatedfilesharing/tests/Settings/AdminTest.php +++ b/apps/federatedfilesharing/tests/Settings/AdminTest.php @@ -72,6 +72,10 @@ class AdminTest extends TestCase { ->expects($this->once()) ->method('isIncomingServer2serverShareEnabled') ->willReturn($state); + $this->federatedShareProvider + ->expects($this->once()) + ->method('isIncomingServer2serverShareEnabled') + ->willReturn($state); $this->federatedShareProvider ->expects($this->once()) ->method('isLookupServerQueriesEnabled') @@ -80,6 +84,18 @@ class AdminTest extends TestCase { ->expects($this->once()) ->method('isLookupServerUploadEnabled') ->willReturn($state); + $this->federatedShareProvider + ->expects($this->once()) + ->method('isFederatedGroupSharingSupported') + ->willReturn($state); + $this->federatedShareProvider + ->expects($this->once()) + ->method('isOutgoingServer2serverGroupShareEnabled') + ->willReturn($state); + $this->federatedShareProvider + ->expects($this->once()) + ->method('isIncomingServer2serverGroupShareEnabled') + ->willReturn($state); $this->gsConfig->expects($this->once())->method('onlyInternalFederation') ->willReturn($state); @@ -88,7 +104,10 @@ class AdminTest extends TestCase { 'outgoingServer2serverShareEnabled' => $state, 'incomingServer2serverShareEnabled' => $state, 'lookupServerEnabled' => $state, - 'lookupServerUploadEnabled' => $state + 'lookupServerUploadEnabled' => $state, + 'federatedGroupSharingSupported' => $state, + 'outgoingServer2serverGroupShareEnabled' => $state, + 'incomingServer2serverGroupShareEnabled' => $state, ]; $expected = new TemplateResponse('federatedfilesharing', 'settings-admin', $params, ''); $this->assertEquals($expected, $this->admin->getForm()); diff --git a/apps/files_sharing/lib/Controller/ShareesAPIController.php b/apps/files_sharing/lib/Controller/ShareesAPIController.php index c60e873b82..ee95661d4c 100644 --- a/apps/files_sharing/lib/Controller/ShareesAPIController.php +++ b/apps/files_sharing/lib/Controller/ShareesAPIController.php @@ -223,7 +223,13 @@ class ShareesAPIController extends OCSController { } protected function isRemoteGroupSharingAllowed(string $itemType): bool { - return true; + try { + // FIXME: static foo makes unit testing unnecessarily difficult + $backend = \OC\Share\Share::getBackend($itemType); + return $backend->isShareTypeAllowed(Share::SHARE_TYPE_REMOTE_GROUP); + } catch (\Exception $e) { + return false; + } } diff --git a/apps/files_sharing/lib/External/Manager.php b/apps/files_sharing/lib/External/Manager.php index 9f8acbce4c..a3b037e498 100644 --- a/apps/files_sharing/lib/External/Manager.php +++ b/apps/files_sharing/lib/External/Manager.php @@ -276,15 +276,16 @@ class Manager { $mountPoint = Files::buildNotExistingFileName($shareFolder, $share['name']); $mountPoint = Filesystem::normalizePath($mountPoint); $hash = md5($mountPoint); + $userShareAccepted = false; - if($share['share_type'] === Share::SHARE_TYPE_USER) { + if((int)$share['share_type'] === Share::SHARE_TYPE_USER) { $acceptShare = $this->connection->prepare(' UPDATE `*PREFIX*share_external` SET `accepted` = ?, `mountpoint` = ?, `mountpoint_hash` = ? WHERE `id` = ? AND `user` = ?'); - $updated = $acceptShare->execute(array(1, $mountPoint, $hash, $id, $this->uid)); + $userShareAccepted = $acceptShare->execute(array(1, $mountPoint, $hash, $id, $this->uid)); } else { $result = $this->writeShareToDb( $share['remote'], @@ -297,9 +298,8 @@ class Manager { $share['remote_id'], $id, $share['share_type']); - // TODO group share, add additional row for the user who accepted it } - if ($updated === true) { + if ($userShareAccepted === true) { $this->sendFeedbackToRemote($share['remote'], $share['share_token'], $share['remote_id'], 'accept'); \OC_Hook::emit(Share::class, 'federated_share_added', ['server' => $share['remote']]); $result = true; diff --git a/apps/files_sharing/lib/ShareBackend/File.php b/apps/files_sharing/lib/ShareBackend/File.php index bb28c1a33a..bd32741e67 100644 --- a/apps/files_sharing/lib/ShareBackend/File.php +++ b/apps/files_sharing/lib/ShareBackend/File.php @@ -195,6 +195,10 @@ class File implements \OCP\Share_Backend_File_Dependent { return $this->federatedShareProvider->isOutgoingServer2serverShareEnabled(); } + if ($shareType === \OCP\Share::SHARE_TYPE_REMOTE_GROUP) { + return $this->federatedShareProvider->isOutgoingServer2serverGroupShareEnabled(); + } + return true; } diff --git a/apps/files_sharing/tests/Controller/ShareesAPIControllerTest.php b/apps/files_sharing/tests/Controller/ShareesAPIControllerTest.php index 98ae953a31..d8c706da03 100644 --- a/apps/files_sharing/tests/Controller/ShareesAPIControllerTest.php +++ b/apps/files_sharing/tests/Controller/ShareesAPIControllerTest.php @@ -85,120 +85,125 @@ class ShareesAPIControllerTest extends TestCase { public function dataSearch() { $noRemote = [Share::SHARE_TYPE_USER, Share::SHARE_TYPE_GROUP, Share::SHARE_TYPE_EMAIL]; - $allTypes = [Share::SHARE_TYPE_USER, Share::SHARE_TYPE_GROUP, Share::SHARE_TYPE_REMOTE, Share::SHARE_TYPE_EMAIL]; + $allTypes = [Share::SHARE_TYPE_USER, Share::SHARE_TYPE_GROUP, Share::SHARE_TYPE_REMOTE, Share::SHARE_TYPE_REMOTE_GROUP, Share::SHARE_TYPE_EMAIL]; return [ - [[], '', 'yes', true, true, $noRemote, false, true, true], + [[], '', 'yes', true, true, true, $noRemote, false, true, true], // Test itemType [[ 'search' => '', - ], '', 'yes', true, true, $noRemote, false, true, true], + ], '', 'yes', true, true, true, $noRemote, false, true, true], [[ 'search' => 'foobar', - ], '', 'yes', true, true, $noRemote, false, true, true], + ], '', 'yes', true, true, true, $noRemote, false, true, true], [[ 'search' => 0, - ], '', 'yes', true, true, $noRemote, false, true, true], + ], '', 'yes', true, true, true, $noRemote, false, true, true], // Test itemType [[ 'itemType' => '', - ], '', 'yes', true, true, $noRemote, false, true, true], + ], '', 'yes', true, true, true, $noRemote, false, true, true], [[ 'itemType' => 'folder', - ], '', 'yes', true, true, $allTypes, false, true, true], + ], '', 'yes', true, true, true, $allTypes, false, true, true], [[ 'itemType' => 0, - ], '', 'yes', true, true, $noRemote, false, true, true], - + ], '', 'yes', true, true , true, $noRemote, false, true, true], // Test shareType [[ 'itemType' => 'call', - ], '', 'yes', true, true, $noRemote, false, true, true], + ], '', 'yes', true, true, true, $noRemote, false, true, true], [[ 'itemType' => 'folder', - ], '', 'yes', true, true, $allTypes, false, true, true], + ], '', 'yes', true, true, true, $allTypes, false, true, true], [[ 'itemType' => 'folder', 'shareType' => 0, - ], '', 'yes', true, false, [0], false, true, true], + ], '', 'yes', true, true, false, [0], false, true, true], [[ 'itemType' => 'folder', 'shareType' => '0', - ], '', 'yes', true, false, [0], false, true, true], + ], '', 'yes', true, true, false, [0], false, true, true], [[ 'itemType' => 'folder', 'shareType' => 1, - ], '', 'yes', true, false, [1], false, true, true], + ], '', 'yes', true, true, false, [1], false, true, true], [[ 'itemType' => 'folder', 'shareType' => 12, - ], '', 'yes', true, false, [], false, true, true], + ], '', 'yes', true, true, false, [], false, true, true], [[ 'itemType' => 'folder', 'shareType' => 'foobar', - ], '', 'yes', true, true, $allTypes, false, true, true], + ], '', 'yes', true, true, true, $allTypes, false, true, true], + [[ 'itemType' => 'folder', 'shareType' => [0, 1, 2], - ], '', 'yes', false, false, [0, 1], false, true, true], + ], '', 'yes', false, false, false, [0, 1], false, true, true], [[ 'itemType' => 'folder', 'shareType' => [0, 1], - ], '', 'yes', false, false, [0, 1], false, true, true], + ], '', 'yes', false, false, false, [0, 1], false, true, true], [[ 'itemType' => 'folder', 'shareType' => $allTypes, - ], '', 'yes', true, true, $allTypes, false, true, true], + ], '', 'yes', true, true, true, $allTypes, false, true, true], [[ 'itemType' => 'folder', 'shareType' => $allTypes, - ], '', 'yes', false, false, [0, 1], false, true, true], + ], '', 'yes', false, false, false, [0, 1], false, true, true], [[ 'itemType' => 'folder', 'shareType' => $allTypes, - ], '', 'yes', true, false, [0, 6], false, true, false], + ], '', 'yes', true, false, false, [0, 6], false, true, false], [[ 'itemType' => 'folder', 'shareType' => $allTypes, - ], '', 'yes', false, true, [0, 4], false, true, false], + ], '', 'yes', false, false, true, [0, 4], false, true, false], + [[ + 'itemType' => 'folder', + 'shareType' => $allTypes, + ], '', 'yes', true, true, false, [0, 6, 9], false, true, false], // Test pagination [[ 'itemType' => 'folder', 'page' => 1, - ], '', 'yes', true, true, $allTypes, false, true, true], + ], '', 'yes', true, true, true, $allTypes, false, true, true], [[ 'itemType' => 'folder', 'page' => 10, - ], '', 'yes', true, true, $allTypes, false, true, true], + ], '', 'yes', true, true, true, $allTypes, false, true, true], // Test perPage [[ 'itemType' => 'folder', 'perPage' => 1, - ], '', 'yes', true, true, $allTypes, false, true, true], + ], '', 'yes', true, true, true, $allTypes, false, true, true], [[ 'itemType' => 'folder', 'perPage' => 10, - ], '', 'yes', true, true, $allTypes, false, true, true], + ], '', 'yes', true, true, true, $allTypes, false, true, true], // Test $shareWithGroupOnly setting [[ 'itemType' => 'folder', - ], 'no', 'yes', true, true, $allTypes, false, true, true], + ], 'no', 'yes', true, true, true, $allTypes, false, true, true], [[ 'itemType' => 'folder', - ], 'yes', 'yes', true, true, $allTypes, true, true, true], + ], 'yes', 'yes', true, true, true, $allTypes, true, true, true], // Test $shareeEnumeration setting [[ 'itemType' => 'folder', - ], 'no', 'yes', true, true, $allTypes, false, true, true], + ], 'no', 'yes', true, true, true, $allTypes, false, true, true], [[ 'itemType' => 'folder', - ], 'no', 'no', true, true, $allTypes, false, false, true], + ], 'no', 'no', true, true, true, $allTypes, false, false, true], + ]; } @@ -209,13 +214,15 @@ class ShareesAPIControllerTest extends TestCase { * @param string $apiSetting * @param string $enumSetting * @param bool $remoteSharingEnabled + * @param bool $isRemoteGroupSharingEnabled * @param bool $emailSharingEnabled * @param array $shareTypes * @param bool $shareWithGroupOnly * @param bool $shareeEnumeration * @param bool $allowGroupSharing + * @throws OCSBadRequestException */ - public function testSearch($getData, $apiSetting, $enumSetting, $remoteSharingEnabled, $emailSharingEnabled, $shareTypes, $shareWithGroupOnly, $shareeEnumeration, $allowGroupSharing) { + public function testSearch($getData, $apiSetting, $enumSetting, $remoteSharingEnabled, $isRemoteGroupSharingEnabled, $emailSharingEnabled, $shareTypes, $shareWithGroupOnly, $shareeEnumeration, $allowGroupSharing) { $search = isset($getData['search']) ? $getData['search'] : ''; $itemType = isset($getData['itemType']) ? $getData['itemType'] : 'irrelevant'; $page = isset($getData['page']) ? $getData['page'] : 1; @@ -251,7 +258,7 @@ class ShareesAPIControllerTest extends TestCase { $this->shareManager, $this->collaboratorSearch ]) - ->setMethods(['isRemoteSharingAllowed', 'shareProviderExists']) + ->setMethods(['isRemoteSharingAllowed', 'shareProviderExists', 'isRemoteGroupSharingAllowed']) ->getMock(); $this->collaboratorSearch->expects($this->once()) @@ -264,6 +271,13 @@ class ShareesAPIControllerTest extends TestCase { ->with($itemType) ->willReturn($remoteSharingEnabled); + + $sharees->expects($this->any()) + ->method('isRemoteGroupSharingAllowed') + ->with($itemType) + ->willReturn($isRemoteGroupSharingEnabled); + + $this->shareManager->expects($this->any()) ->method('shareProviderExists') ->with(\OCP\Share::SHARE_TYPE_EMAIL) diff --git a/apps/files_sharing/tests/External/ManagerTest.php b/apps/files_sharing/tests/External/ManagerTest.php index 2cc88160a8..753836dd58 100644 --- a/apps/files_sharing/tests/External/ManagerTest.php +++ b/apps/files_sharing/tests/External/ManagerTest.php @@ -35,6 +35,8 @@ use OCA\Files_Sharing\Tests\TestCase; use OCP\Federation\ICloudFederationFactory; use OCP\Federation\ICloudFederationProviderManager; use OCP\Http\Client\IClientService; +use OCP\IGroupManager; +use OCP\IUserManager; use Test\Traits\UserTrait; /** @@ -62,6 +64,12 @@ class ManagerTest extends TestCase { /** @var ICloudFederationFactory|\PHPUnit_Framework_MockObject_MockObject */ private $cloudFederationFactory; + /** @var \PHPUnit_Framework_MockObject_MockObject|IGroupManager */ + private $groupManager; + + /** @var \PHPUnit_Framework_MockObject_MockObject|IUserManager */ + private $userManager; + private $uid; /** @@ -81,6 +89,8 @@ class ManagerTest extends TestCase { ->disableOriginalConstructor()->getMock(); $this->cloudFederationProviderManager = $this->createMock(ICloudFederationProviderManager::class); $this->cloudFederationFactory = $this->createMock(ICloudFederationFactory::class); + $this->groupManager = $this->createMock(IGroupManager::class); + $this->userManager = $this->createMock(IUserManager::class); $this->manager = $this->getMockBuilder(Manager::class) ->setConstructorArgs( @@ -93,6 +103,8 @@ class ManagerTest extends TestCase { \OC::$server->query(\OCP\OCS\IDiscoveryService::class), $this->cloudFederationProviderManager, $this->cloudFederationFactory, + $this->groupManager, + $this->userManager, $this->uid ] )->setMethods(['tryOCMEndPoint'])->getMock(); @@ -117,6 +129,7 @@ class ManagerTest extends TestCase { 'password' => '', 'name' => '/SharedFolder', 'owner' => 'foobar', + 'shareType' => \OCP\Share::SHARE_TYPE_USER, 'accepted' => false, 'user' => $this->uid, ]; From f0aaf62b24a0ecdb964665c94cfac330b86afed2 Mon Sep 17 00:00:00 2001 From: Bjoern Schiessle Date: Wed, 4 Jul 2018 13:13:36 +0200 Subject: [PATCH 12/14] fix js tests Signed-off-by: Bjoern Schiessle --- core/js/tests/specs/sharedialogviewSpec.js | 106 ++++++++++++++++----- 1 file changed, 82 insertions(+), 24 deletions(-) diff --git a/core/js/tests/specs/sharedialogviewSpec.js b/core/js/tests/specs/sharedialogviewSpec.js index d363915984..83c8905320 100644 --- a/core/js/tests/specs/sharedialogviewSpec.js +++ b/core/js/tests/specs/sharedialogviewSpec.js @@ -489,11 +489,13 @@ describe('OC.Share.ShareDialogView', function() { 'exact': { 'users': [], 'groups': [], - 'remotes': [] + 'remotes': [], + 'remote_groups': [], }, 'users': [], 'groups': [], 'remotes': [], + 'remote_groups': [], 'lookup': [] } } @@ -529,7 +531,8 @@ describe('OC.Share.ShareDialogView', function() { 'exact': { 'users': [], 'groups': [], - 'remotes': [] + 'remotes': [], + 'remote_groups': [], }, 'users': [ { @@ -542,6 +545,7 @@ describe('OC.Share.ShareDialogView', function() { ], 'groups': [], 'remotes': [], + 'remote_groups': [], 'lookup': [] } } @@ -589,11 +593,13 @@ describe('OC.Share.ShareDialogView', function() { } ], 'groups': [], - 'remotes': [] + 'remotes': [], + 'remote_groups': [], }, 'users': [], 'groups': [], 'remotes': [], + 'remote_groups': [], 'lookup': [] } } @@ -651,7 +657,8 @@ describe('OC.Share.ShareDialogView', function() { } } ], - 'remotes': [] + 'remotes': [], + 'remote_groups': [], }, 'users': [ { @@ -679,6 +686,7 @@ describe('OC.Share.ShareDialogView', function() { } ], 'remotes': [], + 'remote_groups': [], 'lookup': [] } } @@ -744,11 +752,13 @@ describe('OC.Share.ShareDialogView', function() { } ], 'groups': [], - 'remotes': [] + 'remotes': [], + 'remote_groups': [], }, 'users': [], 'groups': [], 'remotes': [], + 'remote_groups': [], 'lookup': [] } } @@ -817,11 +827,13 @@ describe('OC.Share.ShareDialogView', function() { } ], 'groups': [], - 'remotes': [] + 'remotes': [], + 'remote_groups': [], }, 'users': [], 'groups': [], 'remotes': [], + 'remote_groups': [], 'lookup': [] } } @@ -928,7 +940,8 @@ describe('OC.Share.ShareDialogView', function() { } ], 'groups': [], - 'remotes': [] + 'remotes': [], + 'remote_groups': [], }, 'users': [ { @@ -941,6 +954,7 @@ describe('OC.Share.ShareDialogView', function() { ], 'groups': [], 'remotes': [], + 'remote_groups': [], 'lookup': [] } } @@ -983,7 +997,8 @@ describe('OC.Share.ShareDialogView', function() { } } ], - 'remotes': [] + 'remotes': [], + 'remote_groups': [], }, 'users': [], 'groups': [ @@ -996,6 +1011,7 @@ describe('OC.Share.ShareDialogView', function() { } ], 'remotes': [], + 'remote_groups': [], 'lookup': [] } } @@ -1038,7 +1054,8 @@ describe('OC.Share.ShareDialogView', function() { 'shareWith': 'foo@bar.com/baz' } } - ] + ], + 'remote_groups': [], }, 'users': [], 'groups': [], @@ -1051,6 +1068,7 @@ describe('OC.Share.ShareDialogView', function() { } } ], + 'remote_groups': [], 'lookup': [] } } @@ -1086,6 +1104,7 @@ describe('OC.Share.ShareDialogView', function() { 'users': [], 'groups': [], 'remotes': [], + 'remote_groups': [], 'emails': [ { 'label': 'foo@bar.com', @@ -1099,6 +1118,7 @@ describe('OC.Share.ShareDialogView', function() { 'users': [], 'groups': [], 'remotes': [], + 'remote_groups': [], 'lookup': [], 'emails': [ { @@ -1143,6 +1163,7 @@ describe('OC.Share.ShareDialogView', function() { 'users': [], 'groups': [], 'remotes': [], + 'remote_groups': [], 'circles': [ { 'label': 'CircleName (type, owner)', @@ -1163,6 +1184,7 @@ describe('OC.Share.ShareDialogView', function() { 'users': [], 'groups': [], 'remotes': [], + 'remote_groups': [], 'lookup': [], 'circles': [ { @@ -1211,7 +1233,8 @@ describe('OC.Share.ShareDialogView', function() { 'exact': { 'users': [], 'groups': [], - 'remotes': [] + 'remotes': [], + 'remote_groups': [], }, 'users': [ { @@ -1231,6 +1254,7 @@ describe('OC.Share.ShareDialogView', function() { ], 'groups': [], 'remotes': [], + 'remote_groups': [], 'lookup': [] } } @@ -1270,7 +1294,8 @@ describe('OC.Share.ShareDialogView', function() { 'exact': { 'users': [], 'groups': [], - 'remotes': [] + 'remotes': [], + 'remote_groups': [], }, 'users': [ { @@ -1290,6 +1315,7 @@ describe('OC.Share.ShareDialogView', function() { ], 'groups': [], 'remotes': [], + 'remote_groups': [], 'lookup': [] } } @@ -1364,7 +1390,8 @@ describe('OC.Share.ShareDialogView', function() { 'exact': { 'users': [], 'groups': [], - 'remotes': [] + 'remotes': [], + 'remote_groups': [], }, 'users': [ { @@ -1384,6 +1411,7 @@ describe('OC.Share.ShareDialogView', function() { ], 'groups': [], 'remotes': [], + 'remote_groups': [], 'lookup': [] } } @@ -1423,7 +1451,8 @@ describe('OC.Share.ShareDialogView', function() { } ], 'groups': [], - 'remotes': [] + 'remotes': [], + 'remote_groups': [], }, 'users': [ { @@ -1436,6 +1465,7 @@ describe('OC.Share.ShareDialogView', function() { ], 'groups': [], 'remotes': [], + 'remote_groups': [], 'lookup': [] } } @@ -1467,7 +1497,8 @@ describe('OC.Share.ShareDialogView', function() { 'exact': { 'users': [], 'groups': [], - 'remotes': [] + 'remotes': [], + 'remote_groups': [], }, 'users': [], 'groups': [ @@ -1487,6 +1518,7 @@ describe('OC.Share.ShareDialogView', function() { } ], 'remotes': [], + 'remote_groups': [], 'lookup': [] } } @@ -1526,7 +1558,8 @@ describe('OC.Share.ShareDialogView', function() { } } ], - 'remotes': [] + 'remotes': [], + 'remote_groups': [], }, 'users': [], 'groups': [ @@ -1539,6 +1572,7 @@ describe('OC.Share.ShareDialogView', function() { } ], 'remotes': [], + 'remote_groups': [], 'lookup': [] } } @@ -1570,7 +1604,8 @@ describe('OC.Share.ShareDialogView', function() { 'exact': { 'users': [], 'groups': [], - 'remotes': [] + 'remotes': [], + 'remote_groups': [], }, 'users': [], 'groups': [], @@ -1590,6 +1625,7 @@ describe('OC.Share.ShareDialogView', function() { } } ], + 'remote_groups': [], 'lookup': [] } } @@ -1629,7 +1665,8 @@ describe('OC.Share.ShareDialogView', function() { 'shareWith': 'foo@bar.com/baz' } } - ] + ], + 'remote_groups': [], }, 'users': [], 'groups': [], @@ -1642,6 +1679,7 @@ describe('OC.Share.ShareDialogView', function() { } } ], + 'remote_groups': [], 'lookup': [] } } @@ -1674,12 +1712,14 @@ describe('OC.Share.ShareDialogView', function() { 'users': [], 'groups': [], 'remotes': [], + 'remote_groups': [], 'emails': [] }, 'users': [], 'groups': [], 'remotes': [], 'lookup': [], + 'remote_groups': [], 'emails': [ { 'label': 'foo@bar.com', @@ -1727,6 +1767,7 @@ describe('OC.Share.ShareDialogView', function() { 'users': [], 'groups': [], 'remotes': [], + 'remote_groups': [], 'emails': [ { 'label': 'foo@bar.com', @@ -1740,6 +1781,7 @@ describe('OC.Share.ShareDialogView', function() { 'users': [], 'groups': [], 'remotes': [], + 'remote_groups': [], 'lookup': [], 'emails': [ { @@ -1781,11 +1823,13 @@ describe('OC.Share.ShareDialogView', function() { 'users': [], 'groups': [], 'remotes': [], + 'remote_groups': [], 'circles': [] }, 'users': [], 'groups': [], 'remotes': [], + 'remote_groups': [], 'lookup': [], 'circles': [ { @@ -1834,6 +1878,7 @@ describe('OC.Share.ShareDialogView', function() { 'users': [], 'groups': [], 'remotes': [], + 'remote_groups': [], 'circles': [ { 'label': 'CircleName (type, owner)', @@ -1854,6 +1899,7 @@ describe('OC.Share.ShareDialogView', function() { 'users': [], 'groups': [], 'remotes': [], + 'remote_groups': [], 'lookup': [], 'circles': [ { @@ -2031,11 +2077,13 @@ describe('OC.Share.ShareDialogView', function() { 'exact': { 'users': [], 'groups': [], - 'remotes': [] + 'remotes': [], + 'remote_groups': [], }, 'users': [], 'groups': [], 'remotes': [], + 'remote_groups': [], 'lookup': [] } } @@ -2126,11 +2174,13 @@ describe('OC.Share.ShareDialogView', function() { } ], 'groups': [], - 'remotes': [] + 'remotes': [], + 'remote_groups': [], }, 'users': [], 'groups': [], 'remotes': [], + 'remote_groups': [], 'lookup': [] } } @@ -2191,11 +2241,13 @@ describe('OC.Share.ShareDialogView', function() { } ], 'groups': [], - 'remotes': [] + 'remotes': [], + 'remote_groups': [], }, 'users': [], 'groups': [], 'remotes': [], + 'remote_groups': [], 'lookup': [] } } @@ -2248,12 +2300,14 @@ describe('OC.Share.ShareDialogView', function() { 'exact': { 'users': [], 'groups': [], - 'remotes': [] + 'remotes': [], + 'remote_groups': [], }, 'users': [], 'groups': [], 'remotes': [], - 'lookup': [] + 'lookup': [], + 'remote_groups': [], } } }); @@ -2292,7 +2346,8 @@ describe('OC.Share.ShareDialogView', function() { 'exact': { 'users': [], 'groups': [], - 'remotes': [] + 'remotes': [], + 'remote_groups': [], }, 'users': [ { @@ -2305,6 +2360,7 @@ describe('OC.Share.ShareDialogView', function() { ], 'groups': [], 'remotes': [], + 'remote_groups': [], 'lookup': [] } } @@ -2356,11 +2412,13 @@ describe('OC.Share.ShareDialogView', function() { } } ], - 'remotes': [] + 'remotes': [], + 'remote_groups': [], }, 'users': [], 'groups': [], 'remotes': [], + 'remote_groups': [], 'lookup': [] } } From 0c09f566c24e45b3d4dc91dc2bddc341e4f76fbd Mon Sep 17 00:00:00 2001 From: Bjoern Schiessle Date: Thu, 5 Jul 2018 13:51:34 +0200 Subject: [PATCH 13/14] add some integration tests Signed-off-by: Bjoern Schiessle --- .../features/bootstrap/FederationContext.php | 27 ++++++ .../federation_features/federated.feature | 94 +++++++++++++++++++ 2 files changed, 121 insertions(+) diff --git a/build/integration/features/bootstrap/FederationContext.php b/build/integration/features/bootstrap/FederationContext.php index f3eb004fb8..12bf9c0fe6 100644 --- a/build/integration/features/bootstrap/FederationContext.php +++ b/build/integration/features/bootstrap/FederationContext.php @@ -35,6 +35,7 @@ require __DIR__ . '/../../vendor/autoload.php'; class FederationContext implements Context, SnippetAcceptingContext { use WebDav; + use AppConfiguration; /** * @Given /^User "([^"]*)" from server "(LOCAL|REMOTE)" shares "([^"]*)" with user "([^"]*)" from server "(LOCAL|REMOTE)"$/ @@ -56,6 +57,27 @@ class FederationContext implements Context, SnippetAcceptingContext { $this->usingServer($previous); } + + /** + * @Given /^User "([^"]*)" from server "(LOCAL|REMOTE)" shares "([^"]*)" with group "([^"]*)" from server "(LOCAL|REMOTE)"$/ + * + * @param string $sharerUser + * @param string $sharerServer "LOCAL" or "REMOTE" + * @param string $sharerPath + * @param string $shareeUser + * @param string $shareeServer "LOCAL" or "REMOTE" + */ + public function federateGroupSharing($sharerUser, $sharerServer, $sharerPath, $shareeGroup, $shareeServer){ + if ($shareeServer == "REMOTE"){ + $shareWith = "$shareeGroup@" . substr($this->remoteBaseUrl, 0, -4); + } else { + $shareWith = "$shareeGroup@" . substr($this->localBaseUrl, 0, -4); + } + $previous = $this->usingServer($sharerServer); + $this->createShare($sharerUser, $sharerPath, 9, $shareWith, null, null, null); + $this->usingServer($previous); + } + /** * @When /^User "([^"]*)" from server "(LOCAL|REMOTE)" accepts last pending share$/ * @param string $user @@ -73,4 +95,9 @@ class FederationContext implements Context, SnippetAcceptingContext { $this->theOCSStatusCodeShouldBe('100'); $this->usingServer($previous); } + + protected function resetAppConfigs() { + $this->modifyServerConfig('files_sharing', 'incoming_server2server_group_share_enabled', 'no'); + $this->modifyServerConfig('files_sharing', 'outgoing_server2server_group_share_enabled', 'no'); + } } diff --git a/build/integration/federation_features/federated.feature b/build/integration/federation_features/federated.feature index c7b20cf86a..87515e2cca 100644 --- a/build/integration/federation_features/federated.feature +++ b/build/integration/federation_features/federated.feature @@ -28,6 +28,40 @@ Feature: federated | share_with | user1@REMOTE | | share_with_displayname | user1@REMOTE | + Scenario: Federated group share a file with another server + Given Using server "REMOTE" + And parameter "incoming_server2server_group_share_enabled" of app "files_sharing" is set to "yes" + And user "gs-user1" exists + And user "gs-user2" exists + And group "group1" exists + And As an "admin" + And Add user "gs-user1" to the group "group1" + And Add user "gs-user2" to the group "group1" + And Using server "LOCAL" + And parameter "outgoing_server2server_group_share_enabled" of app "files_sharing" is set to "yes" + And user "gs-user0" exists + When User "gs-user0" from server "LOCAL" shares "/textfile0.txt" with group "group1" from server "REMOTE" + Then the OCS status code should be "100" + And the HTTP status code should be "200" + And Share fields of last share match with + | id | A_NUMBER | + | item_type | file | + | item_source | A_NUMBER | + | share_type | 9 | + | file_source | A_NUMBER | + | path | /textfile0.txt | + | permissions | 19 | + | stime | A_NUMBER | + | storage | A_NUMBER | + | mail_send | 0 | + | uid_owner | gs-user0 | + | storage_id | home::gs-user0 | + | file_parent | A_NUMBER | + | displayname_owner | gs-user0 | + | share_with | group1@REMOTE | + | share_with_displayname | group1@REMOTE | + + Scenario: Federate share a file with local server Given Using server "LOCAL" And user "user0" exists @@ -76,6 +110,49 @@ Feature: federated | mountpoint | {{TemporaryMountPointName#/textfile0.txt}} | | accepted | 0 | + Scenario: Remote sharee can see the pending group share + Given Using server "REMOTE" + And parameter "incoming_server2server_group_share_enabled" of app "files_sharing" is set to "yes" + And user "gs-user1" exists + And user "gs-user2" exists + And group "group1" exists + And As an "admin" + And Add user "gs-user1" to the group "group1" + And Add user "gs-user2" to the group "group1" + And Using server "LOCAL" + And parameter "outgoing_server2server_group_share_enabled" of app "files_sharing" is set to "yes" + And user "gs-user0" exists + When User "gs-user0" from server "LOCAL" shares "/textfile0.txt" with group "group1" from server "REMOTE" + And Using server "REMOTE" + And As an "gs-user1" + When sending "GET" to "/apps/files_sharing/api/v1/remote_shares/pending" + Then the OCS status code should be "100" + And the HTTP status code should be "200" + And Share fields of last share match with + | id | A_NUMBER | + | remote | LOCAL | + | remote_id | A_NUMBER | + | share_token | A_TOKEN | + | name | /textfile0.txt | + | owner | gs-user0 | + | user | group1 | + | mountpoint | {{TemporaryMountPointName#/textfile0.txt}} | + | accepted | 0 | + And As an "gs-user2" + When sending "GET" to "/apps/files_sharing/api/v1/remote_shares/pending" + Then the OCS status code should be "100" + And the HTTP status code should be "200" + And Share fields of last share match with + | id | A_NUMBER | + | remote | LOCAL | + | remote_id | A_NUMBER | + | share_token | A_TOKEN | + | name | /textfile0.txt | + | owner | gs-user0 | + | user | group1 | + | mountpoint | {{TemporaryMountPointName#/textfile0.txt}} | + | accepted | 0 | + Scenario: accept a pending remote share Given Using server "REMOTE" And user "user1" exists @@ -86,6 +163,23 @@ Feature: federated Then the OCS status code should be "100" And the HTTP status code should be "200" + Scenario: accept a pending remote group share + Given Using server "REMOTE" + And parameter "incoming_server2server_group_share_enabled" of app "files_sharing" is set to "yes" + And user "gs-user1" exists + And user "gs-user2" exists + And group "group1" exists + And As an "admin" + And Add user "gs-user1" to the group "group1" + And Add user "gs-user2" to the group "group1" + And Using server "LOCAL" + And parameter "outgoing_server2server_group_share_enabled" of app "files_sharing" is set to "yes" + And user "gs-user0" exists + When User "gs-user0" from server "LOCAL" shares "/textfile0.txt" with group "group1" from server "REMOTE" + When User "gs-user1" from server "REMOTE" accepts last pending share + Then the OCS status code should be "100" + And the HTTP status code should be "200" + Scenario: Reshare a federated shared file Given Using server "REMOTE" And user "user1" exists From 412f6a6eccd7b4b955d63922150dbe2104cd269d Mon Sep 17 00:00:00 2001 From: Bjoern Schiessle Date: Wed, 11 Jul 2018 10:21:00 +0200 Subject: [PATCH 14/14] fix unit tests Signed-off-by: Bjoern Schiessle --- apps/files_sharing/lib/External/Manager.php | 2 +- apps/files_sharing/tests/External/ManagerTest.php | 3 +++ 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/apps/files_sharing/lib/External/Manager.php b/apps/files_sharing/lib/External/Manager.php index a3b037e498..6a036b663d 100644 --- a/apps/files_sharing/lib/External/Manager.php +++ b/apps/files_sharing/lib/External/Manager.php @@ -636,7 +636,7 @@ class Manager { $query = 'SELECT `id`, `remote`, `remote_id`, `share_token`, `name`, `owner`, `user`, `mountpoint`, `accepted` FROM `*PREFIX*share_external` - WHERE `user` = ? OR `user` IN (?)'; + WHERE (`user` = ? OR `user` IN (?))'; $parameters = [$this->uid, implode(',',$userGroups)]; if (!is_null($accepted)) { $query .= ' AND `accepted` = ?'; diff --git a/apps/files_sharing/tests/External/ManagerTest.php b/apps/files_sharing/tests/External/ManagerTest.php index 753836dd58..93c17ca10c 100644 --- a/apps/files_sharing/tests/External/ManagerTest.php +++ b/apps/files_sharing/tests/External/ManagerTest.php @@ -138,6 +138,9 @@ class ManagerTest extends TestCase { $shareData3 = $shareData1; $shareData3['token'] = 'token3'; + $this->userManager->expects($this->any())->method('get')->willReturn($this->user); + $this->groupManager->expects($this->any())->method(('getUserGroups'))->willReturn([]); + $this->manager->expects($this->at(0))->method('tryOCMEndPoint')->with('http://localhost', 'token1', -1, 'accept')->willReturn(false); $this->manager->expects($this->at(1))->method('tryOCMEndPoint')->with('http://localhost', 'token3', -1, 'decline')->willReturn(false);