From dcdbea54e60d13d2508b71ebdcb7992f2ae5ef34 Mon Sep 17 00:00:00 2001 From: Joas Schilling Date: Wed, 21 Aug 2019 16:15:23 +0200 Subject: [PATCH 01/13] Respect the accepted flag for group and user shares Signed-off-by: Joas Schilling --- apps/files_sharing/lib/External/Manager.php | 3 +- apps/files_sharing/lib/MountProvider.php | 12 ++++++- lib/private/Share20/DefaultShareProvider.php | 3 ++ lib/private/Share20/Share.php | 17 ++++++++++ lib/public/Share/IShare.php | 34 ++++++++++++++++++++ 5 files changed, 67 insertions(+), 2 deletions(-) diff --git a/apps/files_sharing/lib/External/Manager.php b/apps/files_sharing/lib/External/Manager.php index 0cf7d89923..aeb95a8f2f 100644 --- a/apps/files_sharing/lib/External/Manager.php +++ b/apps/files_sharing/lib/External/Manager.php @@ -44,6 +44,7 @@ use OCP\IUserManager; use OCP\Notification\IManager; use OCP\OCS\IDiscoveryService; use OCP\Share; +use OCP\Share\IShare; class Manager { const STORAGE = '\OCA\Files_Sharing\External\Storage'; @@ -151,7 +152,7 @@ class Manager { 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; + $accepted = $accepted ? IShare::STATUS_ACCEPTED : IShare::STATUS_PENDING; $name = Filesystem::normalizePath('/' . $name); if (!$accepted) { diff --git a/apps/files_sharing/lib/MountProvider.php b/apps/files_sharing/lib/MountProvider.php index 47a787350d..1f73396996 100644 --- a/apps/files_sharing/lib/MountProvider.php +++ b/apps/files_sharing/lib/MountProvider.php @@ -35,6 +35,7 @@ use OCP\IConfig; use OCP\ILogger; use OCP\IUser; use OCP\Share\IManager; +use OCP\Share\IShare; class MountProvider implements IMountProvider { /** @@ -94,6 +95,11 @@ class MountProvider implements IMountProvider { try { /** @var \OCP\Share\IShare $parentShare */ $parentShare = $share[0]; + + if ($parentShare->getStatus() !== IShare::STATUS_ACCEPTED) { + continue; + } + $owner = $parentShare->getShareOwner(); if (!isset($ownerViews[$owner])) { $ownerViews[$owner] = new View('/' . $parentShare->getShareOwner() . '/files'); @@ -188,8 +194,11 @@ class MountProvider implements IMountProvider { // use most permissive permissions $permissions = 0; + $status = IShare::STATUS_PENDING; foreach ($shares as $share) { $permissions |= $share->getPermissions(); + $status = max($status, $share->getStatus()); + if ($share->getTarget() !== $superShare->getTarget()) { // adjust target, for database consistency $share->setTarget($superShare->getTarget()); @@ -216,7 +225,8 @@ class MountProvider implements IMountProvider { } } - $superShare->setPermissions($permissions); + $superShare->setPermissions($permissions) + ->setStatus($status); $result[] = [$superShare, $shares]; } diff --git a/lib/private/Share20/DefaultShareProvider.php b/lib/private/Share20/DefaultShareProvider.php index 05b3094e6a..61c6236415 100644 --- a/lib/private/Share20/DefaultShareProvider.php +++ b/lib/private/Share20/DefaultShareProvider.php @@ -142,6 +142,7 @@ class DefaultShareProvider implements IShareProvider { if ($share->getShareType() === \OCP\Share::SHARE_TYPE_USER) { //Set the UID of the user we share with $qb->setValue('share_with', $qb->createNamedParameter($share->getSharedWith())); + $qb->setValue('accepted', $qb->createNamedParameter(IShare::STATUS_PENDING)); } else if ($share->getShareType() === \OCP\Share::SHARE_TYPE_GROUP) { //Set the GID of the group we share with $qb->setValue('share_with', $qb->createNamedParameter($share->getSharedWith())); @@ -932,6 +933,7 @@ class DefaultShareProvider implements IShareProvider { ->setTarget($data['file_target']) ->setNote($data['note']) ->setMailSend((bool)$data['mail_send']) + ->setStatus((int)$data['accepted']) ->setLabel($data['label']); $shareTime = new \DateTime(); @@ -1020,6 +1022,7 @@ class DefaultShareProvider implements IShareProvider { while($data = $stmt->fetch()) { $shareMap[$data['parent']]->setPermissions((int)$data['permissions']); + $shareMap[$data['parent']]->setStatus((int)$data['accepted']); $shareMap[$data['parent']]->setTarget($data['file_target']); $shareMap[$data['parent']]->setParent($data['parent']); } diff --git a/lib/private/Share20/Share.php b/lib/private/Share20/Share.php index 9ce88b5af2..57b5304b10 100644 --- a/lib/private/Share20/Share.php +++ b/lib/private/Share20/Share.php @@ -58,6 +58,8 @@ class Share implements \OCP\Share\IShare { private $shareOwner; /** @var int */ private $permissions; + /** @var int */ + private $status; /** @var string */ private $note = ''; /** @var \DateTime */ @@ -318,6 +320,21 @@ class Share implements \OCP\Share\IShare { return $this->permissions; } + /** + * @inheritdoc + */ + public function setStatus(int $status): IShare { + $this->status = $status; + return $this; + } + + /** + * @inheritdoc + */ + public function getStatus(): int { + return $this->status; + } + /** * @inheritdoc */ diff --git a/lib/public/Share/IShare.php b/lib/public/Share/IShare.php index 3b16fcaec0..a4d120da5f 100644 --- a/lib/public/Share/IShare.php +++ b/lib/public/Share/IShare.php @@ -96,6 +96,21 @@ interface IShare { */ // const TYPE_USERROOM = 11; + /** + * @since 18.0.0 + */ + public const STATUS_PENDING = 0; + + /** + * @since 18.0.0 + */ + public const STATUS_ACCEPTED = 1; + + /** + * @since 18.0.0 + */ + public const STATUS_REJECTED = 2; + /** * Set the internal id of the share * It is only allowed to set the internal id of a share once. @@ -279,6 +294,25 @@ interface IShare { */ public function getPermissions(); + /** + * Set the accepted status + * See self::STATUS_* + * + * @param int $status + * @return IShare The modified object + * @since 18.0.0 + */ + public function setStatus(int $status): IShare; + + /** + * Get the accepted status + * See self::STATUS_* + * + * @return int + * @since 18.0.0 + */ + public function getStatus(): int; + /** * Attach a note to a share * From e96c9e0e4a402277a9f18470c77503dd914c6de4 Mon Sep 17 00:00:00 2001 From: Joas Schilling Date: Thu, 22 Aug 2019 03:17:17 +0200 Subject: [PATCH 02/13] Add the notifier and the API endpoint for user shares Signed-off-by: Joas Schilling --- apps/federatedfilesharing/lib/Notifier.php | 2 +- apps/files_sharing/appinfo/app.php | 1 + apps/files_sharing/appinfo/routes.php | 5 + .../composer/composer/autoload_classmap.php | 1 + .../composer/composer/autoload_static.php | 1 + .../files_sharing/lib/AppInfo/Application.php | 11 ++ .../lib/Controller/ShareAPIController.php | 39 +++++++ .../lib/Notification/Listener.php | 72 ++++++++++++ .../lib/Notification/Notifier.php | 105 +++++++++++++++++- lib/private/Share20/DefaultShareProvider.php | 1 + 10 files changed, 232 insertions(+), 6 deletions(-) create mode 100644 apps/files_sharing/lib/Notification/Listener.php diff --git a/apps/federatedfilesharing/lib/Notifier.php b/apps/federatedfilesharing/lib/Notifier.php index 02a46d65bb..1916390a1a 100644 --- a/apps/federatedfilesharing/lib/Notifier.php +++ b/apps/federatedfilesharing/lib/Notifier.php @@ -86,7 +86,7 @@ class Notifier implements INotifier { * @throws \InvalidArgumentException */ public function prepare(INotification $notification, string $languageCode): INotification { - if ($notification->getApp() !== 'files_sharing') { + if ($notification->getApp() !== 'files_sharing' || $notification->getObjectType() !== 'remote_share') { // Not my app => throw throw new \InvalidArgumentException(); } diff --git a/apps/files_sharing/appinfo/app.php b/apps/files_sharing/appinfo/app.php index c4f44095dc..8d4b511c26 100644 --- a/apps/files_sharing/appinfo/app.php +++ b/apps/files_sharing/appinfo/app.php @@ -38,6 +38,7 @@ use OCA\Files_Sharing\AppInfo\Application; $application = \OC::$server->query(Application::class); $application->registerMountProviders(); +$application->register(); $eventDispatcher = \OC::$server->getEventDispatcher(); $eventDispatcher->addListener( diff --git a/apps/files_sharing/appinfo/routes.php b/apps/files_sharing/appinfo/routes.php index ce7ba40919..ab5e829f86 100644 --- a/apps/files_sharing/appinfo/routes.php +++ b/apps/files_sharing/appinfo/routes.php @@ -73,6 +73,11 @@ return [ 'url' => '/api/v1/shares/{id}', 'verb' => 'DELETE', ], + [ + 'name' => 'ShareAPI#acceptShare', + 'url' => '/api/v1/shares/pending/{id}', + 'verb' => 'POST', + ], /* * Deleted Shares */ diff --git a/apps/files_sharing/composer/composer/autoload_classmap.php b/apps/files_sharing/composer/composer/autoload_classmap.php index 63c31f0a3c..d5609a9a60 100644 --- a/apps/files_sharing/composer/composer/autoload_classmap.php +++ b/apps/files_sharing/composer/composer/autoload_classmap.php @@ -51,6 +51,7 @@ return array( 'OCA\\Files_Sharing\\Migration\\OwncloudGuestShareType' => $baseDir . '/../lib/Migration/OwncloudGuestShareType.php', 'OCA\\Files_Sharing\\Migration\\SetPasswordColumn' => $baseDir . '/../lib/Migration/SetPasswordColumn.php', 'OCA\\Files_Sharing\\MountProvider' => $baseDir . '/../lib/MountProvider.php', + 'OCA\\Files_Sharing\\Notification\\Listener' => $baseDir . '/../lib/Notification/Listener.php', 'OCA\\Files_Sharing\\Notification\\Notifier' => $baseDir . '/../lib/Notification/Notifier.php', 'OCA\\Files_Sharing\\Scanner' => $baseDir . '/../lib/Scanner.php', 'OCA\\Files_Sharing\\ShareBackend\\File' => $baseDir . '/../lib/ShareBackend/File.php', diff --git a/apps/files_sharing/composer/composer/autoload_static.php b/apps/files_sharing/composer/composer/autoload_static.php index 659903300c..0975feafec 100644 --- a/apps/files_sharing/composer/composer/autoload_static.php +++ b/apps/files_sharing/composer/composer/autoload_static.php @@ -66,6 +66,7 @@ class ComposerStaticInitFiles_Sharing 'OCA\\Files_Sharing\\Migration\\OwncloudGuestShareType' => __DIR__ . '/..' . '/../lib/Migration/OwncloudGuestShareType.php', 'OCA\\Files_Sharing\\Migration\\SetPasswordColumn' => __DIR__ . '/..' . '/../lib/Migration/SetPasswordColumn.php', 'OCA\\Files_Sharing\\MountProvider' => __DIR__ . '/..' . '/../lib/MountProvider.php', + 'OCA\\Files_Sharing\\Notification\\Listener' => __DIR__ . '/..' . '/../lib/Notification/Listener.php', 'OCA\\Files_Sharing\\Notification\\Notifier' => __DIR__ . '/..' . '/../lib/Notification/Notifier.php', 'OCA\\Files_Sharing\\Scanner' => __DIR__ . '/..' . '/../lib/Scanner.php', 'OCA\\Files_Sharing\\ShareBackend\\File' => __DIR__ . '/..' . '/../lib/ShareBackend/File.php', diff --git a/apps/files_sharing/lib/AppInfo/Application.php b/apps/files_sharing/lib/AppInfo/Application.php index bba87ba991..92b3a92d43 100644 --- a/apps/files_sharing/lib/AppInfo/Application.php +++ b/apps/files_sharing/lib/AppInfo/Application.php @@ -32,6 +32,7 @@ namespace OCA\Files_Sharing\AppInfo; use OCA\Files_Sharing\Middleware\OCSShareAPIMiddleware; use OCA\Files_Sharing\Middleware\ShareInfoMiddleware; use OCA\Files_Sharing\MountProvider; +use OCA\Files_Sharing\Notification\Listener; use OCA\Files_Sharing\Notification\Notifier; use OCP\AppFramework\App; use OC\AppFramework\Utility\SimpleContainer; @@ -45,6 +46,7 @@ use \OCP\IContainer; use OCP\IServerContainer; use OCA\Files_Sharing\Capabilities; use OCA\Files_Sharing\External\Manager; +use Symfony\Component\EventDispatcher\GenericEvent; class Application extends App { public function __construct(array $urlParams = array()) { @@ -178,4 +180,13 @@ class Application extends App { $mountProviderCollection->registerProvider($this->getContainer()->query('MountProvider')); $mountProviderCollection->registerProvider($this->getContainer()->query('ExternalMountProvider')); } + + public function register(): void { + $dispatcher = $this->getContainer()->getServer()->getEventDispatcher(); + $dispatcher->addListener('OCP\Share::postShare', function(GenericEvent $event) { + /** @var Listener $listener */ + $listener = $this->getContainer()->query(Listener::class); + $listener->shareNotification($event); + }); + } } diff --git a/apps/files_sharing/lib/Controller/ShareAPIController.php b/apps/files_sharing/lib/Controller/ShareAPIController.php index 66b2383ea7..acb95a0a3d 100644 --- a/apps/files_sharing/lib/Controller/ShareAPIController.php +++ b/apps/files_sharing/lib/Controller/ShareAPIController.php @@ -946,6 +946,45 @@ class ShareAPIController extends OCSController { return new DataResponse($this->formatShare($share)); } + /** + * @NoAdminRequired + * + * @param string $id + * @return DataResponse + * @throws OCSNotFoundException + * @throws OCSException + * @throws OCSBadRequestException + */ + public function acceptShare(string $id): DataResponse { + try { + $share = $this->getShareById($id); + } catch (ShareNotFound $e) { + throw new OCSNotFoundException($this->l->t('Wrong share ID, share doesn\'t exist')); + } + + if (!$this->canAccessShare($share, false)) { + throw new OCSNotFoundException($this->l->t('Wrong share ID, share doesn\'t exist')); + } + + if ($share->getShareType() !== Share::SHARE_TYPE_USER || + $share->getSharedWith() !== $this->currentUser) { + throw new OCSNotFoundException($this->l->t('Wrong share ID, share doesn\'t exist')); + } + + $share->setStatus(IShare::STATUS_ACCEPTED); + + try { + $this->shareManager->updateShare($share); + } catch (GenericShareException $e) { + $code = $e->getCode() === 0 ? 403 : $e->getCode(); + throw new OCSException($e->getHint(), $code); + } catch (\Exception $e) { + throw new OCSBadRequestException($e->getMessage(), $e); + } + + return new DataResponse(); + } + /** * Does the user have read permission on the share * diff --git a/apps/files_sharing/lib/Notification/Listener.php b/apps/files_sharing/lib/Notification/Listener.php new file mode 100644 index 0000000000..98c40f3167 --- /dev/null +++ b/apps/files_sharing/lib/Notification/Listener.php @@ -0,0 +1,72 @@ + + * + * @author Joas Schilling + * + * @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 OCA\Files_Sharing\Notification; + +use OC\Share\Share; +use OCP\Notification\IManager; +use OCP\Notification\INotification; +use OCP\Share\IShare; +use Symfony\Component\EventDispatcher\GenericEvent; + +class Listener { + + /** @var IManager */ + protected $notificationManager; + + public function __construct( + IManager $notificationManager + ) { + $this->notificationManager = $notificationManager; + } + + /** + * @param GenericEvent $event + */ + public function shareNotification(GenericEvent $event): void { + /** @var IShare $share */ + $share = $event->getSubject(); + $notification = $this->instantiateNotification($share); + + if ($share->getShareType() === Share::SHARE_TYPE_USER) { + $notification->setSubject('incoming_user_share') + ->setUser($share->getSharedWith()); + $this->notificationManager->notify($notification); + } + } + + /** + * @param IShare $share + * @return INotification + */ + protected function instantiateNotification(IShare $share): INotification { + $notification = $this->notificationManager->createNotification(); + $notification + ->setApp('files_sharing') + ->setObject('share', $share->getFullId()) + ->setDateTime($share->getShareTime()); + + return $notification; + } +} diff --git a/apps/files_sharing/lib/Notification/Notifier.php b/apps/files_sharing/lib/Notification/Notifier.php index a9028ec9cf..6ae009895d 100644 --- a/apps/files_sharing/lib/Notification/Notifier.php +++ b/apps/files_sharing/lib/Notification/Notifier.php @@ -2,8 +2,10 @@ declare(strict_types=1); /** * @copyright Copyright (c) 2019, Roeland Jago Douma + * @copyright Copyright (c) 2019, Joas Schilling * * @author Roeland Jago Douma + * @author Joas Schilling * * @license GNU AGPL version 3 or any later version * @@ -25,43 +27,70 @@ declare(strict_types=1); namespace OCA\Files_Sharing\Notification; use OCP\Files\IRootFolder; +use OCP\IL10N; +use OCP\IURLGenerator; use OCP\L10N\IFactory; use OCP\Notification\AlreadyProcessedException; use OCP\Notification\INotification; use OCP\Notification\INotifier; use OCP\Share\Exceptions\ShareNotFound; use OCP\Share\IManager; +use OCP\Share\IShare; class Notifier implements INotifier { /** @var IFactory */ protected $l10nFactory; - /** @var IManager */ private $shareManager; - /** @var IRootFolder */ private $rootFolder; + /** @var IURLGenerator */ + protected $url; + public function __construct(IFactory $l10nFactory, IManager $shareManager, - IRootFolder $rootFolder) { + IRootFolder $rootFolder, + IURLGenerator $url) { $this->l10nFactory = $l10nFactory; $this->shareManager = $shareManager; $this->rootFolder = $rootFolder; + $this->url = $url; } + /** + * Identifier of the notifier, only use [a-z0-9_] + * + * @return string + * @since 17.0.0 + */ public function getID(): string { return 'files_sharing'; } + /** + * Human readable name describing the notifier + * + * @return string + * @since 17.0.0 + */ public function getName(): string { - return $this->l10nFactory->get('files_sharing')->t('Files sharing'); + return $this->l10nFactory->get('files_sharing')->t('File sharing'); } + /** + * @param INotification $notification + * @param string $languageCode The code of the language that should be used to prepare the notification + * @return INotification + * @throws \InvalidArgumentException When the notification was not prepared by a notifier + * @throws AlreadyProcessedException When the notification is not needed anymore and should be deleted + * @since 9.0.0 + */ public function prepare(INotification $notification, string $languageCode): INotification { if ($notification->getApp() !== 'files_sharing' || - $notification->getSubject() !== 'expiresTomorrow') { + ($notification->getSubject() !== 'expiresTomorrow' && + $notification->getObjectType() !== 'share')) { throw new \InvalidArgumentException('Unhandled app or subject'); } @@ -74,6 +103,15 @@ class Notifier implements INotifier { throw new AlreadyProcessedException(); } + if ($notification->getSubject() === 'expiresTomorrow') { + $notification = $this->parseShareExpiration($share, $notification, $l); + } else { + $notification = $this->parseShareInvitation($share, $notification, $l); + } + return $notification; + } + + protected function parseShareExpiration(IShare $share, INotification $notification, IL10N $l): INotification { $node = $share->getNode(); $userFolder = $this->rootFolder->getUserFolder($notification->getUser()); $path = $userFolder->getRelativePath($node->getPath()); @@ -95,4 +133,61 @@ class Notifier implements INotifier { return $notification; } + + protected function parseShareInvitation(IShare $share, INotification $notification, IL10N $l): INotification { + if ($share->getShareType() === IShare::TYPE_USER) { + if ($share->getSharedWith() !== $notification->getUser()) { + throw new AlreadyProcessedException(); + } + + if ($share->getStatus() !== IShare::STATUS_PENDING) { + throw new AlreadyProcessedException(); + } + } + + switch ($notification->getSubject()) { + case 'incoming_user_share': + $subject = $l->t('You received {share} as a share from {user}'); + $subjectParameters = [ + 'share' => [ + 'type' => 'highlight', + 'id' => $notification->getObjectId(), + 'name' => $share->getNode()->getName(), + ], + 'user' => [ + 'type' => 'user', + 'id' => $share->getShareOwner(), + 'name' => $share->getShareOwner(), + ], + ]; + + $placeholders = $replacements = []; + foreach ($subjectParameters as $placeholder => $parameter) { + $placeholders[] = '{' . $placeholder . '}'; + $replacements[] = $parameter['name']; + } + + $notification->setParsedSubject(str_replace($placeholders, $replacements, $subject)) + ->setRichSubject($subject, $subjectParameters) + ->setIcon($this->url->getAbsoluteURL($this->url->imagePath('core', 'actions/share.svg'))); + + $acceptAction = $notification->createAction(); + $acceptAction->setParsedLabel($l->t('Accept')) + ->setLink($this->url->linkToOCSRouteAbsolute('files_sharing.ShareAPI.acceptShare', ['id' => $share->getId()]), 'POST') + ->setPrimary(true); + $notification->addParsedAction($acceptAction); + + $rejectAction = $notification->createAction(); + $rejectAction->setParsedLabel($l->t('Reject')) + ->setLink($this->url->linkToOCSRouteAbsolute('files_sharing.ShareAPI.deleteShare', ['id' => $share->getId()]), 'DELETE') + ->setPrimary(false); + $notification->addParsedAction($rejectAction); + + return $notification; + break; + + default: + throw new \InvalidArgumentException('Invalid subject'); + } + } } diff --git a/lib/private/Share20/DefaultShareProvider.php b/lib/private/Share20/DefaultShareProvider.php index 61c6236415..66d28869db 100644 --- a/lib/private/Share20/DefaultShareProvider.php +++ b/lib/private/Share20/DefaultShareProvider.php @@ -253,6 +253,7 @@ class DefaultShareProvider implements IShareProvider { ->set('file_source', $qb->createNamedParameter($share->getNode()->getId())) ->set('expiration', $qb->createNamedParameter($share->getExpirationDate(), IQueryBuilder::PARAM_DATE)) ->set('note', $qb->createNamedParameter($share->getNote())) + ->set('accepted', $qb->createNamedParameter($share->getStatus())) ->execute(); } else if ($share->getShareType() === \OCP\Share::SHARE_TYPE_GROUP) { $qb = $this->dbConn->getQueryBuilder(); From c79a56481bc4bd9fb94b0dfbf483537400c76569 Mon Sep 17 00:00:00 2001 From: Joas Schilling Date: Mon, 26 Aug 2019 14:40:02 +0200 Subject: [PATCH 03/13] Notifications for group shares Signed-off-by: Joas Schilling --- apps/files_sharing/lib/Notification/Listener.php | 15 ++++++++++++++- 1 file changed, 14 insertions(+), 1 deletion(-) diff --git a/apps/files_sharing/lib/Notification/Listener.php b/apps/files_sharing/lib/Notification/Listener.php index 98c40f3167..b2e00613e7 100644 --- a/apps/files_sharing/lib/Notification/Listener.php +++ b/apps/files_sharing/lib/Notification/Listener.php @@ -25,6 +25,7 @@ declare(strict_types=1); namespace OCA\Files_Sharing\Notification; use OC\Share\Share; +use OCP\IGroupManager; use OCP\Notification\IManager; use OCP\Notification\INotification; use OCP\Share\IShare; @@ -34,11 +35,15 @@ class Listener { /** @var IManager */ protected $notificationManager; + /** @var IGroupManager */ + protected $groupManager; public function __construct( - IManager $notificationManager + IManager $notificationManager, + IGroupManager $groupManager ) { $this->notificationManager = $notificationManager; + $this->groupManager = $groupManager; } /** @@ -53,6 +58,14 @@ class Listener { $notification->setSubject('incoming_user_share') ->setUser($share->getSharedWith()); $this->notificationManager->notify($notification); + } else if ($share->getShareType() === Share::SHARE_TYPE_GROUP) { + $notification->setSubject('incoming_group_share'); + $group = $this->groupManager->get($share->getSharedWith()); + + foreach ($group->getUsers() as $user) { + $notification->setUser($user->getUID()); + $this->notificationManager->notify($notification); + } } } From 520042bbd0512e19717d18705c3b045b2d8400a7 Mon Sep 17 00:00:00 2001 From: Joas Schilling Date: Wed, 4 Sep 2019 16:50:52 +0200 Subject: [PATCH 04/13] Allow to accept group shares Signed-off-by: Joas Schilling --- .../lib/Controller/ShareAPIController.php | 16 +-- .../lib/Notification/Listener.php | 5 + .../lib/Notification/Notifier.php | 94 ++++++++++--- lib/private/Share20/DefaultShareProvider.php | 125 ++++++++++++++---- lib/private/Share20/Manager.php | 24 ++++ lib/public/Share/IManager.php | 12 ++ lib/public/Share/IShareProvider.php | 10 ++ 7 files changed, 230 insertions(+), 56 deletions(-) diff --git a/apps/files_sharing/lib/Controller/ShareAPIController.php b/apps/files_sharing/lib/Controller/ShareAPIController.php index acb95a0a3d..9c5f6abee6 100644 --- a/apps/files_sharing/lib/Controller/ShareAPIController.php +++ b/apps/files_sharing/lib/Controller/ShareAPIController.php @@ -962,19 +962,17 @@ class ShareAPIController extends OCSController { throw new OCSNotFoundException($this->l->t('Wrong share ID, share doesn\'t exist')); } - if (!$this->canAccessShare($share, false)) { + if (!$this->canAccessShare($share)) { throw new OCSNotFoundException($this->l->t('Wrong share ID, share doesn\'t exist')); } - if ($share->getShareType() !== Share::SHARE_TYPE_USER || - $share->getSharedWith() !== $this->currentUser) { - throw new OCSNotFoundException($this->l->t('Wrong share ID, share doesn\'t exist')); + if ($share->getShareType() !== IShare::TYPE_USER && + $share->getShareType() !== IShare::TYPE_GROUP) { + throw new OCSNotFoundException($this->l->t('Share type does not support accepting')); } - $share->setStatus(IShare::STATUS_ACCEPTED); - try { - $this->shareManager->updateShare($share); + $this->shareManager->acceptShare($share, $this->currentUser); } catch (GenericShareException $e) { $code = $e->getCode() === 0 ? 403 : $e->getCode(); throw new OCSException($e->getHint(), $code); @@ -1117,8 +1115,8 @@ class ShareAPIController extends OCSController { * @suppress PhanUndeclaredClassMethod */ protected function canDeleteShareFromSelf(\OCP\Share\IShare $share): bool { - if ($share->getShareType() !== Share::SHARE_TYPE_GROUP && - $share->getShareType() !== Share::SHARE_TYPE_ROOM + if ($share->getShareType() !== IShare::TYPE_GROUP && + $share->getShareType() !== IShare::TYPE_ROOM ) { return false; } diff --git a/apps/files_sharing/lib/Notification/Listener.php b/apps/files_sharing/lib/Notification/Listener.php index b2e00613e7..fd4daca28e 100644 --- a/apps/files_sharing/lib/Notification/Listener.php +++ b/apps/files_sharing/lib/Notification/Listener.php @@ -63,6 +63,11 @@ class Listener { $group = $this->groupManager->get($share->getSharedWith()); foreach ($group->getUsers() as $user) { + if ($user->getUID() === $share->getShareOwner() || + $user->getUID() === $share->getSharedBy()) { + continue; + } + $notification->setUser($user->getUID()); $this->notificationManager->notify($notification); } diff --git a/apps/files_sharing/lib/Notification/Notifier.php b/apps/files_sharing/lib/Notification/Notifier.php index 6ae009895d..03d7038e6f 100644 --- a/apps/files_sharing/lib/Notification/Notifier.php +++ b/apps/files_sharing/lib/Notification/Notifier.php @@ -28,7 +28,10 @@ namespace OCA\Files_Sharing\Notification; use OCP\Files\IRootFolder; use OCP\IL10N; +use OCP\IGroupManager; use OCP\IURLGenerator; +use OCP\IUser; +use OCP\IUserManager; use OCP\L10N\IFactory; use OCP\Notification\AlreadyProcessedException; use OCP\Notification\INotification; @@ -45,6 +48,10 @@ class Notifier implements INotifier { private $shareManager; /** @var IRootFolder */ private $rootFolder; + /** @var IGroupManager */ + protected $groupManager; + /** @var IUserManager */ + protected $userManager; /** @var IURLGenerator */ protected $url; @@ -52,10 +59,14 @@ class Notifier implements INotifier { public function __construct(IFactory $l10nFactory, IManager $shareManager, IRootFolder $rootFolder, + IGroupManager $groupManager, + IUserManager $userManager, IURLGenerator $url) { $this->l10nFactory = $l10nFactory; $this->shareManager = $shareManager; $this->rootFolder = $rootFolder; + $this->groupManager = $groupManager; + $this->userManager = $userManager; $this->url = $url; } @@ -135,11 +146,12 @@ class Notifier implements INotifier { } protected function parseShareInvitation(IShare $share, INotification $notification, IL10N $l): INotification { + if ($share->getShareType() === IShare::TYPE_USER) { - if ($share->getSharedWith() !== $notification->getUser()) { + if ($share->getStatus() !== IShare::STATUS_PENDING) { throw new AlreadyProcessedException(); } - + } else if ($share->getShareType() === IShare::TYPE_GROUP) { if ($share->getStatus() !== IShare::STATUS_PENDING) { throw new AlreadyProcessedException(); } @@ -147,6 +159,10 @@ class Notifier implements INotifier { switch ($notification->getSubject()) { case 'incoming_user_share': + if ($share->getSharedWith() !== $notification->getUser()) { + throw new AlreadyProcessedException(); + } + $subject = $l->t('You received {share} as a share from {user}'); $subjectParameters = [ 'share' => [ @@ -160,34 +176,70 @@ class Notifier implements INotifier { 'name' => $share->getShareOwner(), ], ]; + break; - $placeholders = $replacements = []; - foreach ($subjectParameters as $placeholder => $parameter) { - $placeholders[] = '{' . $placeholder . '}'; - $replacements[] = $parameter['name']; + case 'incoming_group_share': + $user = $this->userManager->get($notification->getUser()); + if (!$user instanceof IUser) { + throw new AlreadyProcessedException(); } - $notification->setParsedSubject(str_replace($placeholders, $replacements, $subject)) - ->setRichSubject($subject, $subjectParameters) - ->setIcon($this->url->getAbsoluteURL($this->url->imagePath('core', 'actions/share.svg'))); + $group = $this->groupManager->get($share->getSharedWith()); + if (!$group->inGroup($user)) { + throw new AlreadyProcessedException(); + } - $acceptAction = $notification->createAction(); - $acceptAction->setParsedLabel($l->t('Accept')) - ->setLink($this->url->linkToOCSRouteAbsolute('files_sharing.ShareAPI.acceptShare', ['id' => $share->getId()]), 'POST') - ->setPrimary(true); - $notification->addParsedAction($acceptAction); + if ($share->getPermissions() === 0) { + // Already rejected + throw new AlreadyProcessedException(); + } - $rejectAction = $notification->createAction(); - $rejectAction->setParsedLabel($l->t('Reject')) - ->setLink($this->url->linkToOCSRouteAbsolute('files_sharing.ShareAPI.deleteShare', ['id' => $share->getId()]), 'DELETE') - ->setPrimary(false); - $notification->addParsedAction($rejectAction); - - return $notification; + $subject = $l->t('You received {share} to group {group} as a share from {user}'); + $subjectParameters = [ + 'share' => [ + 'type' => 'highlight', + 'id' => $notification->getObjectId(), + 'name' => $share->getNode()->getName(), + ], + 'group' => [ + 'type' => 'user-group', + 'id' => $group->getGID(), + 'name' => $group->getDisplayName(), + ], + 'user' => [ + 'type' => 'user', + 'id' => $share->getShareOwner(), + 'name' => $share->getShareOwner(), + ], + ]; break; default: throw new \InvalidArgumentException('Invalid subject'); } + + $placeholders = $replacements = []; + foreach ($subjectParameters as $placeholder => $parameter) { + $placeholders[] = '{' . $placeholder . '}'; + $replacements[] = $parameter['name']; + } + + $notification->setParsedSubject(str_replace($placeholders, $replacements, $subject)) + ->setRichSubject($subject, $subjectParameters) + ->setIcon($this->url->getAbsoluteURL($this->url->imagePath('core', 'actions/share.svg'))); + + $acceptAction = $notification->createAction(); + $acceptAction->setParsedLabel($l->t('Accept')) + ->setLink($this->url->linkToOCSRouteAbsolute('files_sharing.ShareAPI.acceptShare', ['id' => $share->getId()]), 'POST') + ->setPrimary(true); + $notification->addParsedAction($acceptAction); + + $rejectAction = $notification->createAction(); + $rejectAction->setParsedLabel($l->t('Reject')) + ->setLink($this->url->linkToOCSRouteAbsolute('files_sharing.ShareAPI.deleteShare', ['id' => $share->getId()]), 'DELETE') + ->setPrimary(false); + $notification->addParsedAction($rejectAction); + + return $notification; } } diff --git a/lib/private/Share20/DefaultShareProvider.php b/lib/private/Share20/DefaultShareProvider.php index 66d28869db..ffcaf4f106 100644 --- a/lib/private/Share20/DefaultShareProvider.php +++ b/lib/private/Share20/DefaultShareProvider.php @@ -320,6 +320,71 @@ class DefaultShareProvider implements IShareProvider { return $share; } + /** + * Accept a share. + * + * @param IShare $share + * @param string $recipient + * @return IShare The share object + * @since 9.0.0 + */ + public function acceptShare(IShare $share, string $recipient): IShare { + if ($share->getShareType() === IShare::TYPE_GROUP) { + $group = $this->groupManager->get($share->getSharedWith()); + $user = $this->userManager->get($recipient); + + if (is_null($group)) { + throw new ProviderException('Group "' . $share->getSharedWith() . '" does not exist'); + } + + if (!$group->inGroup($user)) { + throw new ProviderException('Recipient not in receiving group'); + } + + // Try to fetch user specific share + $qb = $this->dbConn->getQueryBuilder(); + $stmt = $qb->select('*') + ->from('share') + ->where($qb->expr()->eq('share_type', $qb->createNamedParameter(self::SHARE_TYPE_USERGROUP))) + ->andWhere($qb->expr()->eq('share_with', $qb->createNamedParameter($recipient))) + ->andWhere($qb->expr()->eq('parent', $qb->createNamedParameter($share->getId()))) + ->andWhere($qb->expr()->orX( + $qb->expr()->eq('item_type', $qb->createNamedParameter('file')), + $qb->expr()->eq('item_type', $qb->createNamedParameter('folder')) + )) + ->execute(); + + $data = $stmt->fetch(); + + /* + * Check if there already is a user specific group share. + * If there is update it (if required). + */ + if ($data === false) { + $id = $this->createUserSpecificGroupShare($share, $recipient); + } else { + $id = $data['id']; + } + + } else if ($share->getShareType() === IShare::TYPE_USER) { + if ($share->getSharedWith() !== $recipient) { + throw new ProviderException('Recipient does not match'); + } + + $id = $share->getId(); + } else { + throw new ProviderException('Invalid shareType'); + } + + $qb = $this->dbConn->getQueryBuilder(); + $qb->update('share') + ->set('accepted', $qb->createNamedParameter(IShare::STATUS_ACCEPTED)) + ->where($qb->expr()->eq('id', $qb->createNamedParameter($id))) + ->execute(); + + return $share; + } + /** * Get all children of this share * FIXME: remove once https://github.com/owncloud/core/pull/21660 is in @@ -384,13 +449,13 @@ class DefaultShareProvider implements IShareProvider { * Unshare a share from the recipient. If this is a group share * this means we need a special entry in the share db. * - * @param \OCP\Share\IShare $share + * @param IShare $share * @param string $recipient UserId of recipient * @throws BackendError * @throws ProviderException */ - public function deleteFromSelf(\OCP\Share\IShare $share, $recipient) { - if ($share->getShareType() === \OCP\Share::SHARE_TYPE_GROUP) { + public function deleteFromSelf(IShare $share, $recipient) { + if ($share->getShareType() === IShare::TYPE_GROUP) { $group = $this->groupManager->get($share->getSharedWith()); $user = $this->userManager->get($recipient); @@ -423,37 +488,23 @@ class DefaultShareProvider implements IShareProvider { * If there is update it (if required). */ if ($data === false) { - $qb = $this->dbConn->getQueryBuilder(); - - $type = $share->getNodeType(); - - //Insert new share - $qb->insert('share') - ->values([ - 'share_type' => $qb->createNamedParameter(self::SHARE_TYPE_USERGROUP), - 'share_with' => $qb->createNamedParameter($recipient), - 'uid_owner' => $qb->createNamedParameter($share->getShareOwner()), - 'uid_initiator' => $qb->createNamedParameter($share->getSharedBy()), - 'parent' => $qb->createNamedParameter($share->getId()), - 'item_type' => $qb->createNamedParameter($type), - 'item_source' => $qb->createNamedParameter($share->getNodeId()), - 'file_source' => $qb->createNamedParameter($share->getNodeId()), - 'file_target' => $qb->createNamedParameter($share->getTarget()), - 'permissions' => $qb->createNamedParameter(0), - 'stime' => $qb->createNamedParameter($share->getShareTime()->getTimestamp()), - ])->execute(); - - } else if ($data['permissions'] !== 0) { + $id = $this->createUserSpecificGroupShare($share, $recipient); + $permissions = $share->getPermissions(); + } else { + $permissions = $data['permissions']; + $id = $data['id']; + } + if ($permissions !== 0) { // Update existing usergroup share $qb = $this->dbConn->getQueryBuilder(); $qb->update('share') ->set('permissions', $qb->createNamedParameter(0)) - ->where($qb->expr()->eq('id', $qb->createNamedParameter($data['id']))) + ->where($qb->expr()->eq('id', $qb->createNamedParameter($id))) ->execute(); } - } else if ($share->getShareType() === \OCP\Share::SHARE_TYPE_USER) { + } else if ($share->getShareType() === IShare::TYPE_USER) { if ($share->getSharedWith() !== $recipient) { throw new ProviderException('Recipient does not match'); @@ -466,6 +517,28 @@ class DefaultShareProvider implements IShareProvider { } } + protected function createUserSpecificGroupShare(IShare $share, string $recipient): int { + $type = $share->getNodeType(); + + $qb = $this->dbConn->getQueryBuilder(); + $qb->insert('share') + ->values([ + 'share_type' => $qb->createNamedParameter(self::SHARE_TYPE_USERGROUP), + 'share_with' => $qb->createNamedParameter($recipient), + 'uid_owner' => $qb->createNamedParameter($share->getShareOwner()), + 'uid_initiator' => $qb->createNamedParameter($share->getSharedBy()), + 'parent' => $qb->createNamedParameter($share->getId()), + 'item_type' => $qb->createNamedParameter($type), + 'item_source' => $qb->createNamedParameter($share->getNodeId()), + 'file_source' => $qb->createNamedParameter($share->getNodeId()), + 'file_target' => $qb->createNamedParameter($share->getTarget()), + 'permissions' => $qb->createNamedParameter($share->getPermissions()), + 'stime' => $qb->createNamedParameter($share->getShareTime()->getTimestamp()), + ])->execute(); + + return $qb->getLastInsertId(); + } + /** * @inheritdoc * diff --git a/lib/private/Share20/Manager.php b/lib/private/Share20/Manager.php index 728dd60e75..ba370e7724 100644 --- a/lib/private/Share20/Manager.php +++ b/lib/private/Share20/Manager.php @@ -928,6 +928,30 @@ class Manager implements IManager { return $share; } + /** + * Accept a share. + * + * @param IShare $share + * @param string $recipientId + * @return IShare The share object + * @throws \InvalidArgumentException + * @since 9.0.0 + */ + public function acceptShare(IShare $share, string $recipientId): IShare { + [$providerId, ] = $this->splitFullId($share->getFullId()); + $provider = $this->factory->getProvider($providerId); + + if (!method_exists($provider, 'acceptShare')) { + // TODO FIX ME + throw new \InvalidArgumentException('not supported'); + } + $provider->acceptShare($share, $recipientId); + $event = new GenericEvent($share); + $this->eventDispatcher->dispatch('OCP\Share::postAcceptShare', $event); + + return $share; + } + /** * Updates the password of the given share if it is not the same as the * password of the original share. diff --git a/lib/public/Share/IManager.php b/lib/public/Share/IManager.php index 8bb7291d6b..3127c74be5 100644 --- a/lib/public/Share/IManager.php +++ b/lib/public/Share/IManager.php @@ -54,6 +54,7 @@ interface IManager { * Update a share. * The target of the share can't be changed this way: use moveShare * The share can't be removed this way (permission 0): use deleteShare + * The state can't be changed this way: use acceptShare * * @param IShare $share * @return IShare The share object @@ -62,6 +63,17 @@ interface IManager { */ public function updateShare(IShare $share); + /** + * Accept a share. + * + * @param IShare $share + * @param string $recipientId + * @return IShare The share object + * @throws \InvalidArgumentException + * @since 18.0.0 + */ + public function acceptShare(IShare $share, string $recipientId): IShare; + /** * Delete a share * diff --git a/lib/public/Share/IShareProvider.php b/lib/public/Share/IShareProvider.php index c881592826..49ab4bef3e 100644 --- a/lib/public/Share/IShareProvider.php +++ b/lib/public/Share/IShareProvider.php @@ -63,6 +63,16 @@ interface IShareProvider { */ public function update(\OCP\Share\IShare $share); + /** + * Accept a share. + * + * @param IShare $share + * @param string $recipient + * @return IShare The share object + * @since 17.0.0 + */ +// public function acceptShare(IShare $share, string $recipient): IShare; + /** * Delete a share * From 8986910a7d6a0bd7ee08bca68f1f5008612e38dc Mon Sep 17 00:00:00 2001 From: Joas Schilling Date: Fri, 6 Sep 2019 13:06:21 +0200 Subject: [PATCH 05/13] Add notifications for users that are added to the group Signed-off-by: Joas Schilling --- .../files_sharing/lib/AppInfo/Application.php | 6 +++ .../lib/Notification/Listener.php | 48 +++++++++++++++++-- 2 files changed, 51 insertions(+), 3 deletions(-) diff --git a/apps/files_sharing/lib/AppInfo/Application.php b/apps/files_sharing/lib/AppInfo/Application.php index 92b3a92d43..5f48088d71 100644 --- a/apps/files_sharing/lib/AppInfo/Application.php +++ b/apps/files_sharing/lib/AppInfo/Application.php @@ -43,6 +43,7 @@ use OCP\AppFramework\Utility\IControllerMethodReflector; use OCP\Defaults; use OCP\Federation\ICloudIdManager; use \OCP\IContainer; +use OCP\IGroup; use OCP\IServerContainer; use OCA\Files_Sharing\Capabilities; use OCA\Files_Sharing\External\Manager; @@ -188,5 +189,10 @@ class Application extends App { $listener = $this->getContainer()->query(Listener::class); $listener->shareNotification($event); }); + $dispatcher->addListener(IGroup::class . '::postAddUser', function(GenericEvent $event) { + /** @var Listener $listener */ + $listener = $this->getContainer()->query(Listener::class); + $listener->userAddedToGroup($event); + }); } } diff --git a/apps/files_sharing/lib/Notification/Listener.php b/apps/files_sharing/lib/Notification/Listener.php index fd4daca28e..1f8358dd3b 100644 --- a/apps/files_sharing/lib/Notification/Listener.php +++ b/apps/files_sharing/lib/Notification/Listener.php @@ -25,24 +25,31 @@ declare(strict_types=1); namespace OCA\Files_Sharing\Notification; use OC\Share\Share; +use OCP\IGroup; use OCP\IGroupManager; -use OCP\Notification\IManager; +use OCP\IUser; +use OCP\Notification\IManager as INotificationManager; use OCP\Notification\INotification; +use OCP\Share\IManager as IShareManager; use OCP\Share\IShare; use Symfony\Component\EventDispatcher\GenericEvent; class Listener { - /** @var IManager */ + /** @var INotificationManager */ protected $notificationManager; + /** @var IShareManager */ + protected $shareManager; /** @var IGroupManager */ protected $groupManager; public function __construct( - IManager $notificationManager, + INotificationManager $notificationManager, + IShareManager $shareManager, IGroupManager $groupManager ) { $this->notificationManager = $notificationManager; + $this->shareManager = $shareManager; $this->groupManager = $groupManager; } @@ -74,6 +81,41 @@ class Listener { } } + /** + * @param GenericEvent $event + */ + public function userAddedToGroup(GenericEvent $event): void { + /** @var IGroup $group */ + $group = $event->getSubject(); + /** @var IUser $user */ + $user = $event->getArgument('user'); + + $offset = 0; + while (true) { + $shares = $this->shareManager->getSharedWith($user->getUID(), IShare::TYPE_GROUP, null, 50, $offset); + if (empty($shares)) { + break; + } + + foreach ($shares as $share) { + if ($share->getSharedWith() !== $group->getGID()) { + continue; + } + + if ($user->getUID() === $share->getShareOwner() || + $user->getUID() === $share->getSharedBy()) { + continue; + } + + $notification = $this->instantiateNotification($share); + $notification->setSubject('incoming_group_share') + ->setUser($user->getUID()); + $this->notificationManager->notify($notification); + } + $offset += 50; + } + } + /** * @param IShare $share * @return INotification From 60da25681eb7f7f4b611c0c899b3485a8c66ef65 Mon Sep 17 00:00:00 2001 From: Joas Schilling Date: Fri, 6 Sep 2019 13:25:08 +0200 Subject: [PATCH 06/13] Only restrict loading of group and user shares for now Signed-off-by: Joas Schilling --- apps/files_sharing/lib/MountProvider.php | 4 +++- lib/public/Share/IShare.php | 6 ++++++ 2 files changed, 9 insertions(+), 1 deletion(-) diff --git a/apps/files_sharing/lib/MountProvider.php b/apps/files_sharing/lib/MountProvider.php index 1f73396996..81a7e96186 100644 --- a/apps/files_sharing/lib/MountProvider.php +++ b/apps/files_sharing/lib/MountProvider.php @@ -96,7 +96,9 @@ class MountProvider implements IMountProvider { /** @var \OCP\Share\IShare $parentShare */ $parentShare = $share[0]; - if ($parentShare->getStatus() !== IShare::STATUS_ACCEPTED) { + if (($parentShare->getShareType() === IShare::TYPE_GROUP || + $parentShare->getShareType() === IShare::TYPE_USERGROUP || + $parentShare->getShareType() === IShare::TYPE_USER) && $parentShare->getStatus() !== IShare::STATUS_ACCEPTED) { continue; } diff --git a/lib/public/Share/IShare.php b/lib/public/Share/IShare.php index a4d120da5f..1064f59e6d 100644 --- a/lib/public/Share/IShare.php +++ b/lib/public/Share/IShare.php @@ -49,6 +49,12 @@ interface IShare { */ public const TYPE_GROUP = 1; + /** + * @internal + * @since 18.0.0 + */ + public const TYPE_USERGROUP = 2; + /** * @since 17.0.0 */ From 43517cfac6b32a2470a0bdf2b85007b2e5b3233c Mon Sep 17 00:00:00 2001 From: Joas Schilling Date: Fri, 6 Sep 2019 13:26:37 +0200 Subject: [PATCH 07/13] Accept existing shares on update Signed-off-by: Joas Schilling --- apps/files_sharing/appinfo/info.xml | 3 +- .../composer/composer/autoload_classmap.php | 1 + .../composer/composer/autoload_static.php | 1 + .../lib/Migration/SetAcceptedStatus.php | 79 +++++++++++++++++++ 4 files changed, 83 insertions(+), 1 deletion(-) create mode 100644 apps/files_sharing/lib/Migration/SetAcceptedStatus.php diff --git a/apps/files_sharing/appinfo/info.xml b/apps/files_sharing/appinfo/info.xml index a90303828f..111ed9c45c 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.10.0 + 1.10.1 agpl Michael Gapczynski Bjoern Schiessle @@ -36,6 +36,7 @@ Turning the feature off removes shared files and folders on the server for all s OCA\Files_Sharing\Migration\OwncloudGuestShareType OCA\Files_Sharing\Migration\SetPasswordColumn + OCA\Files_Sharing\Migration\SetAcceptedStatus diff --git a/apps/files_sharing/composer/composer/autoload_classmap.php b/apps/files_sharing/composer/composer/autoload_classmap.php index d5609a9a60..d73838c07d 100644 --- a/apps/files_sharing/composer/composer/autoload_classmap.php +++ b/apps/files_sharing/composer/composer/autoload_classmap.php @@ -49,6 +49,7 @@ return array( 'OCA\\Files_Sharing\\Middleware\\ShareInfoMiddleware' => $baseDir . '/../lib/Middleware/ShareInfoMiddleware.php', 'OCA\\Files_Sharing\\Middleware\\SharingCheckMiddleware' => $baseDir . '/../lib/Middleware/SharingCheckMiddleware.php', 'OCA\\Files_Sharing\\Migration\\OwncloudGuestShareType' => $baseDir . '/../lib/Migration/OwncloudGuestShareType.php', + 'OCA\\Files_Sharing\\Migration\\SetAcceptedStatus' => $baseDir . '/../lib/Migration/SetAcceptedStatus.php', 'OCA\\Files_Sharing\\Migration\\SetPasswordColumn' => $baseDir . '/../lib/Migration/SetPasswordColumn.php', 'OCA\\Files_Sharing\\MountProvider' => $baseDir . '/../lib/MountProvider.php', 'OCA\\Files_Sharing\\Notification\\Listener' => $baseDir . '/../lib/Notification/Listener.php', diff --git a/apps/files_sharing/composer/composer/autoload_static.php b/apps/files_sharing/composer/composer/autoload_static.php index 0975feafec..4cdf874b9e 100644 --- a/apps/files_sharing/composer/composer/autoload_static.php +++ b/apps/files_sharing/composer/composer/autoload_static.php @@ -64,6 +64,7 @@ class ComposerStaticInitFiles_Sharing 'OCA\\Files_Sharing\\Middleware\\ShareInfoMiddleware' => __DIR__ . '/..' . '/../lib/Middleware/ShareInfoMiddleware.php', 'OCA\\Files_Sharing\\Middleware\\SharingCheckMiddleware' => __DIR__ . '/..' . '/../lib/Middleware/SharingCheckMiddleware.php', 'OCA\\Files_Sharing\\Migration\\OwncloudGuestShareType' => __DIR__ . '/..' . '/../lib/Migration/OwncloudGuestShareType.php', + 'OCA\\Files_Sharing\\Migration\\SetAcceptedStatus' => __DIR__ . '/..' . '/../lib/Migration/SetAcceptedStatus.php', 'OCA\\Files_Sharing\\Migration\\SetPasswordColumn' => __DIR__ . '/..' . '/../lib/Migration/SetPasswordColumn.php', 'OCA\\Files_Sharing\\MountProvider' => __DIR__ . '/..' . '/../lib/MountProvider.php', 'OCA\\Files_Sharing\\Notification\\Listener' => __DIR__ . '/..' . '/../lib/Notification/Listener.php', diff --git a/apps/files_sharing/lib/Migration/SetAcceptedStatus.php b/apps/files_sharing/lib/Migration/SetAcceptedStatus.php new file mode 100644 index 0000000000..e8701488ef --- /dev/null +++ b/apps/files_sharing/lib/Migration/SetAcceptedStatus.php @@ -0,0 +1,79 @@ + + * + * @author Joas Schilling + * + * @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 OCA\Files_Sharing\Migration; + +use OCP\DB\QueryBuilder\IQueryBuilder; +use OCP\IConfig; +use OCP\IDBConnection; +use OCP\Migration\IOutput; +use OCP\Migration\IRepairStep; +use OCP\Share\IShare; + +class SetAcceptedStatus implements IRepairStep { + + /** @var IDBConnection */ + private $connection; + + /** @var IConfig */ + private $config; + + + public function __construct(IDBConnection $connection, IConfig $config) { + $this->connection = $connection; + $this->config = $config; + } + + /** + * Returns the step's name + * + * @return string + * @since 9.1.0 + */ + public function getName(): string { + return 'Set existing shares as accepted'; + } + + /** + * @param IOutput $output + */ + public function run(IOutput $output): void { + if (!$this->shouldRun()) { + return; + } + + $query = $this->connection->getQueryBuilder(); + $query + ->update('share') + ->set('accepted', $query->createParameter(IShare::STATUS_ACCEPTED)) + ->where($query->expr()->in('share_type', $query->createNamedParameter([IShare::TYPE_USER, IShare::TYPE_GROUP, IShare::TYPE_USERGROUP], IQueryBuilder::PARAM_INT_ARRAY))); + $query->execute(); + } + + protected function shouldRun() { + $appVersion = $this->config->getAppValue('files_sharing', 'installed_version', '0.0.0'); + return version_compare($appVersion, '1.10.1', '<'); + } + +} From 19e1892d507c52b36ec4f7e42cc1e0989158ef79 Mon Sep 17 00:00:00 2001 From: Joas Schilling Date: Mon, 9 Sep 2019 14:31:14 +0200 Subject: [PATCH 08/13] Use constants for the magic strings Signed-off-by: Joas Schilling --- apps/files_sharing/lib/Notification/Listener.php | 11 +++++------ apps/files_sharing/lib/Notification/Notifier.php | 6 ++++-- 2 files changed, 9 insertions(+), 8 deletions(-) diff --git a/apps/files_sharing/lib/Notification/Listener.php b/apps/files_sharing/lib/Notification/Listener.php index 1f8358dd3b..9d23898e57 100644 --- a/apps/files_sharing/lib/Notification/Listener.php +++ b/apps/files_sharing/lib/Notification/Listener.php @@ -24,7 +24,6 @@ declare(strict_types=1); namespace OCA\Files_Sharing\Notification; -use OC\Share\Share; use OCP\IGroup; use OCP\IGroupManager; use OCP\IUser; @@ -61,12 +60,12 @@ class Listener { $share = $event->getSubject(); $notification = $this->instantiateNotification($share); - if ($share->getShareType() === Share::SHARE_TYPE_USER) { - $notification->setSubject('incoming_user_share') + if ($share->getShareType() === IShare::TYPE_USER) { + $notification->setSubject(Notifier::INCOMING_USER_SHARE) ->setUser($share->getSharedWith()); $this->notificationManager->notify($notification); - } else if ($share->getShareType() === Share::SHARE_TYPE_GROUP) { - $notification->setSubject('incoming_group_share'); + } else if ($share->getShareType() === IShare::TYPE_GROUP) { + $notification->setSubject(Notifier::INCOMING_GROUP_SHARE); $group = $this->groupManager->get($share->getSharedWith()); foreach ($group->getUsers() as $user) { @@ -108,7 +107,7 @@ class Listener { } $notification = $this->instantiateNotification($share); - $notification->setSubject('incoming_group_share') + $notification->setSubject(Notifier::INCOMING_GROUP_SHARE) ->setUser($user->getUID()); $this->notificationManager->notify($notification); } diff --git a/apps/files_sharing/lib/Notification/Notifier.php b/apps/files_sharing/lib/Notification/Notifier.php index 03d7038e6f..57b96be57b 100644 --- a/apps/files_sharing/lib/Notification/Notifier.php +++ b/apps/files_sharing/lib/Notification/Notifier.php @@ -41,6 +41,8 @@ use OCP\Share\IManager; use OCP\Share\IShare; class Notifier implements INotifier { + public const INCOMING_USER_SHARE = 'incoming_user_share'; + public const INCOMING_GROUP_SHARE = 'incoming_group_share'; /** @var IFactory */ protected $l10nFactory; @@ -158,7 +160,7 @@ class Notifier implements INotifier { } switch ($notification->getSubject()) { - case 'incoming_user_share': + case self::INCOMING_USER_SHARE: if ($share->getSharedWith() !== $notification->getUser()) { throw new AlreadyProcessedException(); } @@ -178,7 +180,7 @@ class Notifier implements INotifier { ]; break; - case 'incoming_group_share': + case self::INCOMING_GROUP_SHARE: $user = $this->userManager->get($notification->getUser()); if (!$user instanceof IUser) { throw new AlreadyProcessedException(); From e79ab7d0c565425c9d103930787f37c1ed3a9ed6 Mon Sep 17 00:00:00 2001 From: Joas Schilling Date: Mon, 9 Sep 2019 14:32:11 +0200 Subject: [PATCH 09/13] The share manager knows which provider can accept shares Signed-off-by: Joas Schilling --- apps/files_sharing/lib/Controller/ShareAPIController.php | 5 ----- lib/private/Share20/Manager.php | 2 +- 2 files changed, 1 insertion(+), 6 deletions(-) diff --git a/apps/files_sharing/lib/Controller/ShareAPIController.php b/apps/files_sharing/lib/Controller/ShareAPIController.php index 9c5f6abee6..90a76e2223 100644 --- a/apps/files_sharing/lib/Controller/ShareAPIController.php +++ b/apps/files_sharing/lib/Controller/ShareAPIController.php @@ -966,11 +966,6 @@ class ShareAPIController extends OCSController { throw new OCSNotFoundException($this->l->t('Wrong share ID, share doesn\'t exist')); } - if ($share->getShareType() !== IShare::TYPE_USER && - $share->getShareType() !== IShare::TYPE_GROUP) { - throw new OCSNotFoundException($this->l->t('Share type does not support accepting')); - } - try { $this->shareManager->acceptShare($share, $this->currentUser); } catch (GenericShareException $e) { diff --git a/lib/private/Share20/Manager.php b/lib/private/Share20/Manager.php index ba370e7724..298aa6f0f5 100644 --- a/lib/private/Share20/Manager.php +++ b/lib/private/Share20/Manager.php @@ -943,7 +943,7 @@ class Manager implements IManager { if (!method_exists($provider, 'acceptShare')) { // TODO FIX ME - throw new \InvalidArgumentException('not supported'); + throw new \InvalidArgumentException('Share provider does not support accepting'); } $provider->acceptShare($share, $recipientId); $event = new GenericEvent($share); From 0fe4b717dcc5ad50853e0a887bfa60416657988d Mon Sep 17 00:00:00 2001 From: Joas Schilling Date: Mon, 9 Sep 2019 14:40:14 +0200 Subject: [PATCH 10/13] Correctly use the displayname of the sharer in the notification Signed-off-by: Joas Schilling --- .../lib/Notification/Notifier.php | 22 ++++++++++++++----- 1 file changed, 16 insertions(+), 6 deletions(-) diff --git a/apps/files_sharing/lib/Notification/Notifier.php b/apps/files_sharing/lib/Notification/Notifier.php index 57b96be57b..52bb0eb823 100644 --- a/apps/files_sharing/lib/Notification/Notifier.php +++ b/apps/files_sharing/lib/Notification/Notifier.php @@ -165,7 +165,12 @@ class Notifier implements INotifier { throw new AlreadyProcessedException(); } - $subject = $l->t('You received {share} as a share from {user}'); + $sharer = $this->userManager->get($share->getSharedBy()); + if (!$sharer instanceof IUser) { + throw new \InvalidArgumentException('Temporary failure'); + } + + $subject = $l->t('You received {share} as a share by {user}'); $subjectParameters = [ 'share' => [ 'type' => 'highlight', @@ -174,8 +179,8 @@ class Notifier implements INotifier { ], 'user' => [ 'type' => 'user', - 'id' => $share->getShareOwner(), - 'name' => $share->getShareOwner(), + 'id' => $sharer->getUID(), + 'name' => $sharer->getDisplayName(), ], ]; break; @@ -196,7 +201,12 @@ class Notifier implements INotifier { throw new AlreadyProcessedException(); } - $subject = $l->t('You received {share} to group {group} as a share from {user}'); + $sharer = $this->userManager->get($share->getSharedBy()); + if (!$sharer instanceof IUser) { + throw new \InvalidArgumentException('Temporary failure'); + } + + $subject = $l->t('You received {share} to group {group} as a share by {user}'); $subjectParameters = [ 'share' => [ 'type' => 'highlight', @@ -210,8 +220,8 @@ class Notifier implements INotifier { ], 'user' => [ 'type' => 'user', - 'id' => $share->getShareOwner(), - 'name' => $share->getShareOwner(), + 'id' => $sharer->getUID(), + 'name' => $sharer->getDisplayName(), ], ]; break; From 6d9afca14f3b94d6c62307a2b248c7d9c56e9736 Mon Sep 17 00:00:00 2001 From: Joas Schilling Date: Thu, 31 Oct 2019 11:23:35 +0100 Subject: [PATCH 11/13] Fix "Invalid parameter number: mixed named and positional parameters" Signed-off-by: Joas Schilling --- apps/files_sharing/lib/Migration/SetAcceptedStatus.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/apps/files_sharing/lib/Migration/SetAcceptedStatus.php b/apps/files_sharing/lib/Migration/SetAcceptedStatus.php index e8701488ef..935372ca3f 100644 --- a/apps/files_sharing/lib/Migration/SetAcceptedStatus.php +++ b/apps/files_sharing/lib/Migration/SetAcceptedStatus.php @@ -66,7 +66,7 @@ class SetAcceptedStatus implements IRepairStep { $query = $this->connection->getQueryBuilder(); $query ->update('share') - ->set('accepted', $query->createParameter(IShare::STATUS_ACCEPTED)) + ->set('accepted', $query->createNamedParameter(IShare::STATUS_ACCEPTED)) ->where($query->expr()->in('share_type', $query->createNamedParameter([IShare::TYPE_USER, IShare::TYPE_GROUP, IShare::TYPE_USERGROUP], IQueryBuilder::PARAM_INT_ARRAY))); $query->execute(); } From d3b62893afc282c4e49c1bcd349b624370e61c3f Mon Sep 17 00:00:00 2001 From: Joas Schilling Date: Thu, 31 Oct 2019 12:02:09 +0100 Subject: [PATCH 12/13] Make sure the super share also has a valid type Signed-off-by: Joas Schilling --- apps/files_sharing/lib/MountProvider.php | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/apps/files_sharing/lib/MountProvider.php b/apps/files_sharing/lib/MountProvider.php index 81a7e96186..6dd1dc2f31 100644 --- a/apps/files_sharing/lib/MountProvider.php +++ b/apps/files_sharing/lib/MountProvider.php @@ -96,9 +96,10 @@ class MountProvider implements IMountProvider { /** @var \OCP\Share\IShare $parentShare */ $parentShare = $share[0]; - if (($parentShare->getShareType() === IShare::TYPE_GROUP || - $parentShare->getShareType() === IShare::TYPE_USERGROUP || - $parentShare->getShareType() === IShare::TYPE_USER) && $parentShare->getStatus() !== IShare::STATUS_ACCEPTED) { + if ($parentShare->getStatus() !== IShare::STATUS_ACCEPTED && + ($parentShare->getShareType() === IShare::TYPE_GROUP || + $parentShare->getShareType() === IShare::TYPE_USERGROUP || + $parentShare->getShareType() === IShare::TYPE_USER)) { continue; } @@ -192,6 +193,7 @@ class MountProvider implements IMountProvider { $superShare->setId($shares[0]->getId()) ->setShareOwner($shares[0]->getShareOwner()) ->setNodeId($shares[0]->getNodeId()) + ->setShareType($shares[0]->getShareType()) ->setTarget($shares[0]->getTarget()); // use most permissive permissions From e16321368f27f3b9534b3fb835fbe6c6fc85077e Mon Sep 17 00:00:00 2001 From: Roeland Jago Douma Date: Mon, 18 Nov 2019 16:11:03 +0100 Subject: [PATCH 13/13] Fixes Signed-off-by: Roeland Jago Douma --- apps/files_sharing/lib/External/Manager.php | 2 +- lib/private/Share20/DefaultShareProvider.php | 1 + 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/apps/files_sharing/lib/External/Manager.php b/apps/files_sharing/lib/External/Manager.php index aeb95a8f2f..55f366fd65 100644 --- a/apps/files_sharing/lib/External/Manager.php +++ b/apps/files_sharing/lib/External/Manager.php @@ -155,7 +155,7 @@ class Manager { $accepted = $accepted ? IShare::STATUS_ACCEPTED : IShare::STATUS_PENDING; $name = Filesystem::normalizePath('/' . $name); - if (!$accepted) { + if ($accepted !== IShare::STATUS_ACCEPTED) { // To avoid conflicts with the mount point generation later, // we only use a temporary mount point name here. The real // mount point name will be generated when accepting the share, diff --git a/lib/private/Share20/DefaultShareProvider.php b/lib/private/Share20/DefaultShareProvider.php index ffcaf4f106..7a146bf789 100644 --- a/lib/private/Share20/DefaultShareProvider.php +++ b/lib/private/Share20/DefaultShareProvider.php @@ -355,6 +355,7 @@ class DefaultShareProvider implements IShareProvider { ->execute(); $data = $stmt->fetch(); + $stmt->closeCursor(); /* * Check if there already is a user specific group share.