Merge pull request #27751 from nextcloud/enh/23769/accept-decline-fed-sharing-ui

apps/federatedfilesharing/lib/OCM/CloudFederationProviderFiles.php

@@ -59,6 +59,7 @@ use OCP\Share\Exceptions\ShareNotFound;
 use OCP\Share\IManager;
 use OCP\Share\IShare;
 use OCP\Util;
+use Psr\Log\LoggerInterface;
 
 class CloudFederationProviderFiles implements ICloudFederationProvider {
 
@@ -250,7 +251,8 @@ class CloudFederationProviderFiles implements ICloudFederationProvider {
 				\OC::$server->getGroupManager(),
 				\OC::$server->getUserManager(),
 				$shareWith,
-				\OC::$server->query(IEventDispatcher::class)
+				\OC::$server->query(IEventDispatcher::class),
+				\OC::$server->get(LoggerInterface::class)
 			);
 
 			try {

apps/files/js/fileactions.js

@@ -155,6 +155,9 @@
 			if (_.isFunction(action.render)) {
 				actionSpec.render = action.render;
 			}
+			if (_.isFunction(action.shouldRender)) {
+				actionSpec.shouldRender = action.shouldRender;
+			}
 			if (!this.actions[mime]) {
 				this.actions[mime] = {};
 			}
@@ -397,6 +400,11 @@
 		 * @param {OCA.Files.FileActionContext} context rendering context
 		 */
 		_renderInlineAction: function(actionSpec, isDefault, context) {
+			if (actionSpec.shouldRender) {
+				if (!actionSpec.shouldRender(context)) {
+					return;
+				}
+			}
 			var renderFunc = actionSpec.render || _.bind(this._defaultRenderAction, this);
 			var $actionEl = renderFunc(actionSpec, isDefault, context);
 			if (!$actionEl || !$actionEl.length) {

apps/files/js/gotoplugin.js

@@ -29,6 +29,11 @@
 			if (this.disallowedLists.indexOf(fileList.id) !== -1) {
 				return;
 			}
+			// lists where the "Open" default action is disabled should
+			// also have the goto action disabled
+			if (fileList._defaultFileActionsDisabled) {
+				return
+			}
 			var fileActions = fileList.fileActions;
 
 			fileActions.registerAction({

apps/files_sharing/js/app.js

@@ -141,6 +141,8 @@ OCA.Sharing.App = {
 			{
 				id: 'shares.pending',
 				showPending: true,
+				detailsViewEnabled: false,
+				defaultFileActionsDisabled: true,
 				sharedWithUser: true,
 				fileActions: this._acceptShareAction(),
 				config: OCA.Files.App.getFilesConfig(),
@@ -296,7 +298,11 @@ OCA.Sharing.App = {
 			type: OCA.Files.FileActions.TYPE_INLINE,
 			actionHandler(fileName, context) {
 				const shareId = context.$file.data('shareId')
-				$.post(OC.linkToOCS('apps/files_sharing/api/v1/shares/pending/{shareId}', { shareId }))
+				let shareBase = 'shares/pending'
+				if (context.$file.attr('data-remote-id')) {
+					shareBase = 'remote_shares/pending'
+				}
+				$.post(OC.linkToOCS('apps/files_sharing/api/v1/' + shareBase + '/{shareId}', { shareId }))
 					.success(function(result) {
 						context.fileList.remove(context.fileInfoModel.attributes.name)
 					}).fail(function() {
@@ -311,10 +317,23 @@ OCA.Sharing.App = {
 			permissions: OC.PERMISSION_ALL,
 			iconClass: 'icon-close',
 			type: OCA.Files.FileActions.TYPE_INLINE,
+			shouldRender(context) {
+				// disable rejecting group shares from the pending list because they anyway
+				// land back into that same list
+				if (context.$file.attr('data-remote-id') && parseInt(context.$file.attr('data-share-type'), 10) === OC.Share.SHARE_TYPE_REMOTE_GROUP) {
+					return false
+				}
+				return true
+			},
 			actionHandler(fileName, context) {
 				const shareId = context.$file.data('shareId')
+				let shareBase = 'shares'
+				if (context.$file.attr('data-remote-id')) {
+					shareBase = 'remote_shares/pending'
+				}
+
 				$.ajax({
-					url: OC.linkToOCS('apps/files_sharing/api/v1/shares/{shareId}', { shareId }),
+					url: OC.linkToOCS('apps/files_sharing/api/v1/' + shareBase + '/{shareId}', { shareId }),
 					type: 'DELETE',
 				}).success(function(result) {
 					context.fileList.remove(context.fileInfoModel.attributes.name)

apps/files_sharing/js/sharedfilelist.js

@@ -96,6 +96,14 @@
 					$tr.attr('data-share-permissions', permission)
 				}
 
+				if (fileData.remoteId) {
+					$tr.attr('data-remote-id', fileData.remoteId)
+				}
+
+				if (fileData.shareType) {
+					$tr.attr('data-share-type', fileData.shareType)
+				}
+
 				// add row with expiration date for link only shares - influenced by _createRow of filelist
 				if (this._linksOnly) {
 					var expirationTimestamp = 0
@@ -212,6 +220,18 @@
 					}
 				}
 
+				var pendingRemoteShares = {
+					url: OC.linkToOCS('apps/files_sharing/api/v1/remote_shares/pending'),
+					/* jshint camelcase: false */
+					data: {
+						format: 'json'
+					},
+					type: 'GET',
+					beforeSend: function(xhr) {
+						xhr.setRequestHeader('OCS-APIREQUEST', 'true')
+					}
+				}
+
 				var shares = {
 					url: OC.linkToOCS('apps/files_sharing/api/v1/shares'),
 					/* jshint camelcase: false */
@@ -245,6 +265,7 @@
 					promises.push($.ajax(deletedShares))
 				} else if (this._showPending) {
 					promises.push($.ajax(pendingShares))
+					promises.push($.ajax(pendingRemoteShares))
 				} else {
 					promises.push($.ajax(shares))
 
@@ -292,7 +313,12 @@
 				}
 
 				if (additionalShares && additionalShares.ocs && additionalShares.ocs.data) {
-					files = files.concat(this._makeFilesFromShares(additionalShares.ocs.data, !this._sharedWithUser))
+					if (this._showPending) {
+						// in this case the second callback is about pending remote shares
+						files = files.concat(this._makeFilesFromRemoteShares(additionalShares.ocs.data))
+					} else {
+						files = files.concat(this._makeFilesFromShares(additionalShares.ocs.data, !this._sharedWithUser))
+					}
 				}
 
 				this.setFiles(files)
@@ -311,12 +337,29 @@
 							mtime: share.mtime * 1000,
 							mimetype: share.mimetype,
 							type: share.type,
+							// remote share types are different and need to be mapped
+							shareType: (parseInt(share.share_type, 10) === 1) ? OC.Share.SHARE_TYPE_REMOTE_GROUP : OC.Share.SHARE_TYPE_REMOTE,
 							id: share.file_id,
 							path: OC.dirname(share.mountpoint),
 							permissions: share.permissions,
 							tags: share.tags || []
 						}
 
+						if (share.remote_id) {
+							// remote share
+							if (share.accepted !== '1') {
+								file.name = OC.basename(share.name)
+								file.path = '/'
+							}
+							file.remoteId = share.remote_id
+							file.shareOwnerId = share.owner
+						}
+
+						if (!file.mimetype) {
+							// pending shares usually have no type, so default to showing a directory icon
+							file.mimetype = 'dir-shared'
+						}
+
 						file.shares = [{
 							id: share.id,
 							type: OC.Share.SHARE_TYPE_REMOTE

apps/files_sharing/lib/AppInfo/Application.php

@@ -60,6 +60,7 @@ use OCP\Share\Events\ShareCreatedEvent;
 use OCP\Share\IManager;
 use OCP\Util;
 use Psr\Container\ContainerInterface;
+use Psr\Log\LoggerInterface;
 use Symfony\Component\EventDispatcher\EventDispatcherInterface;
 use Symfony\Component\EventDispatcher\GenericEvent;
 
@@ -98,7 +99,8 @@ class Application extends App {
 				$server->getGroupManager(),
 				$server->getUserManager(),
 				$uid,
-				$server->query(IEventDispatcher::class)
+				$server->query(IEventDispatcher::class),
+				$server->get(LoggerInterface::class)
 			);
 		});
 

apps/files_sharing/lib/External/Manager.php

@@ -14,6 +14,7 @@
  * @author Robin Appelman <robin@icewind.nl>
  * @author Roeland Jago Douma <roeland@famdouma.nl>
  * @author Stefan Weil <sw@weilnetz.de>
+ * @author Vincent Petry <vincent@nextcloud.com>
  *
  * @license AGPL-3.0
  *
@@ -36,6 +37,7 @@ use Doctrine\DBAL\Driver\Exception;
 use OC\Files\Filesystem;
 use OCA\FederatedFileSharing\Events\FederatedShareAddedEvent;
 use OCA\Files_Sharing\Helper;
+use OCP\DB\QueryBuilder\IQueryBuilder;
 use OCP\EventDispatcher\IEventDispatcher;
 use OCP\Federation\ICloudFederationFactory;
 use OCP\Federation\ICloudFederationProviderManager;
@@ -49,6 +51,7 @@ use OCP\Notification\IManager;
 use OCP\OCS\IDiscoveryService;
 use OCP\Share;
 use OCP\Share\IShare;
+use Psr\Log\LoggerInterface;
 
 class Manager {
 	public const STORAGE = '\OCA\Files_Sharing\External\Storage';
@@ -89,18 +92,24 @@ class Manager {
 	/** @var IEventDispatcher */
 	private $eventDispatcher;
 
-	public function __construct(IDBConnection $connection,
-								\OC\Files\Mount\Manager $mountManager,
-								IStorageFactory $storageLoader,
-								IClientService $clientService,
-								IManager $notificationManager,
-								IDiscoveryService $discoveryService,
-								ICloudFederationProviderManager $cloudFederationProviderManager,
-								ICloudFederationFactory $cloudFederationFactory,
-								IGroupManager $groupManager,
-								IUserManager $userManager,
-								?string $uid,
-								IEventDispatcher $eventDispatcher) {
+	/** @var LoggerInterface */
+	private $logger;
+
+	public function __construct(
+		IDBConnection $connection,
+		\OC\Files\Mount\Manager $mountManager,
+		IStorageFactory $storageLoader,
+		IClientService $clientService,
+		IManager $notificationManager,
+		IDiscoveryService $discoveryService,
+		ICloudFederationProviderManager $cloudFederationProviderManager,
+		ICloudFederationFactory $cloudFederationFactory,
+		IGroupManager $groupManager,
+		IUserManager $userManager,
+		?string $uid,
+		IEventDispatcher $eventDispatcher,
+		LoggerInterface $logger
+	) {
 		$this->connection = $connection;
 		$this->mountManager = $mountManager;
 		$this->storageLoader = $storageLoader;
@@ -113,6 +122,7 @@ class Manager {
 		$this->groupManager = $groupManager;
 		$this->userManager = $userManager;
 		$this->eventDispatcher = $eventDispatcher;
+		$this->logger = $logger;
 	}
 
 	/**
@@ -218,7 +228,7 @@ class Manager {
 	 * @param int $id share id
 	 * @return mixed share of false
 	 */
-	public function getShare($id) {
+	private function fetchShare($id) {
 		$getShare = $this->connection->prepare('
 			SELECT `id`, `remote`, `remote_id`, `share_token`, `name`, `owner`, `user`, `mountpoint`, `accepted`, `parent`, `share_type`, `password`, `mountpoint_hash`
 			FROM  `*PREFIX*share_external`
@@ -226,14 +236,46 @@ class Manager {
 		$result = $getShare->execute([$id]);
 		$share = $result->fetch();
 		$result->closeCursor();
+		return $share;
+	}
+
+	private function fetchUserShare($parentId, $uid) {
+		$getShare = $this->connection->prepare('
+			SELECT `id`, `remote`, `remote_id`, `share_token`, `name`, `owner`, `user`, `mountpoint`, `accepted`, `parent`, `share_type`, `password`, `mountpoint_hash`
+			FROM  `*PREFIX*share_external`
+			WHERE `parent` = ? AND `user` = ?');
+		$result = $getShare->execute([$parentId, $uid]);
+		$share = $result->fetch();
+		$result->closeCursor();
+		if ($share !== false) {
+			return $share;
+		}
+		return null;
+	}
+
+	/**
+	 * get share
+	 *
+	 * @param int $id share id
+	 * @return mixed share of false
+	 */
+	public function getShare($id) {
+		$share = $this->fetchShare($id);
 		$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'] === IShare::TYPE_USER && $share['user'] === $this->uid) {
 			return $share;
 		} elseif ($validShare && (int)$share['share_type'] === IShare::TYPE_GROUP) {
+			$parentId = (int)$share['parent'];
+			if ($parentId !== -1) {
+				// we just retrieved a sub-share, switch to the parent entry for verification
+				$groupShare = $this->fetchShare($parentId);
+			} else {
+				$groupShare = $share;
+			}
 			$user = $this->userManager->get($this->uid);
-			if ($this->groupManager->get($share['user'])->inGroup($user)) {
+			if ($this->groupManager->get($groupShare['user'])->inGroup($user)) {
 				return $share;
 			}
 		}
@@ -241,6 +283,20 @@ class Manager {
 		return false;
 	}
 
+	/**
+	 * Updates accepted flag in the database
+	 *
+	 * @param int $id
+	 */
+	private function updateAccepted(int $shareId, bool $accepted) : void {
+		$query = $this->connection->prepare('
+			UPDATE `*PREFIX*share_external`
+			SET `accepted` = ?
+			WHERE `id` = ?');
+		$updateResult = $query->execute([$accepted ? 1 : 0, $shareId]);
+		$updateResult->closeCursor();
+	}
+
 	/**
 	 * accept server-to-server share
 	 *
@@ -268,21 +324,46 @@ class Manager {
 				WHERE `id` = ? AND `user` = ?');
 				$userShareAccepted = $acceptShare->execute([1, $mountPoint, $hash, $id, $this->uid]);
 			} else {
-				try {
-					$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']);
-					$result = true;
-				} catch (Exception $e) {
-					$result = false;
+				$parentId = (int)$share['parent'];
+				if ($parentId !== -1) {
+					// this is the sub-share
+					$subshare = $share;
+				} else {
+					$subshare = $this->fetchUserShare($id, $this->uid);
+				}
+
+				if ($subshare !== null) {
+					try {
+						$acceptShare = $this->connection->prepare('
+						UPDATE `*PREFIX*share_external`
+						SET `accepted` = ?,
+							`mountpoint` = ?,
+							`mountpoint_hash` = ?
+						WHERE `id` = ? AND `user` = ?');
+						$acceptShare->execute([1, $mountPoint, $hash, $subshare['id'], $this->uid]);
+						$result = true;
+					} catch (Exception $e) {
+						$this->logger->emergency('Could not update share', ['exception' => $e]);
+						$result = false;
+					}
+				} else {
+					try {
+						$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']);
+						$result = true;
+					} catch (Exception $e) {
+						$this->logger->emergency('Could not create share', ['exception' => $e]);
+						$result = false;
+					}
 				}
 			}
 			if ($userShareAccepted !== false) {
@@ -318,23 +399,42 @@ class Manager {
 			$this->processNotification($id);
 			$result = true;
 		} elseif ($share && (int)$share['share_type'] === IShare::TYPE_GROUP) {
-			try {
-				$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']);
-				$result = true;
-			} catch (Exception $e) {
-				$result = false;
+			$parentId = (int)$share['parent'];
+			if ($parentId !== -1) {
+				// this is the sub-share
+				$subshare = $share;
+			} else {
+				$subshare = $this->fetchUserShare($id, $this->uid);
+			}
+
+			if ($subshare !== null) {
+				try {
+					$this->updateAccepted((int)$subshare['id'], false);
+					$result = true;
+				} catch (Exception $e) {
+					$this->logger->emergency('Could not update share', ['exception' => $e]);
+					$result = false;
+				}
+			} else {
+				try {
+					$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']);
+					$result = true;
+				} catch (Exception $e) {
+					$this->logger->emergency('Could not create share', ['exception' => $e]);
+					$result = false;
+				}
 			}
 			$this->processNotification($id);
 		}
@@ -497,6 +597,10 @@ class Manager {
 
 	public function removeShare($mountPoint): bool {
 		$mountPointObj = $this->mountManager->find($mountPoint);
+		if ($mountPointObj === null) {
+			$this->logger->error('Mount point to remove share not found', ['mountPoint' => $mountPoint]);
+			return false;
+		}
 		$id = $mountPointObj->getStorage()->getCache()->getId('');
 
 		$mountPoint = $this->stripPath($mountPoint);
@@ -519,22 +623,18 @@ class Manager {
 				}
 
 				$query = $this->connection->prepare('
-				DELETE FROM `*PREFIX*share_external`
-				WHERE `id` = ?
+					DELETE FROM `*PREFIX*share_external`
+					WHERE `id` = ?
 				');
 				$deleteResult = $query->execute([(int)$share['id']]);
 				$deleteResult->closeCursor();
 			} elseif ($share !== false && (int)$share['share_type'] === IShare::TYPE_GROUP) {
-				$query = $this->connection->prepare('
-					UPDATE `*PREFIX*share_external`
-					SET `accepted` = ?
-					WHERE `id` = ?');
-				$updateResult = $query->execute([0, (int)$share['id']]);
-				$updateResult->closeCursor();
+				$this->updateAccepted((int)$share['id'], false);
 			}
 
 			$this->removeReShares($id);
 		} catch (\Doctrine\DBAL\Exception $ex) {
+			$this->logger->emergency('Could not update share', ['exception' => $ex]);
 			return false;
 		}
 
@@ -571,24 +671,73 @@ class Manager {
 	 */
 	public function removeUserShares($uid): bool {
 		try {
+			// TODO: use query builder
 			$getShare = $this->connection->prepare('
-				SELECT `remote`, `share_token`, `remote_id`
+				SELECT `id`, `remote`, `share_type`, `share_token`, `remote_id`
 				FROM  `*PREFIX*share_external`
-				WHERE `user` = ?');
-			$result = $getShare->execute([$uid]);
+				WHERE `user` = ?
+				AND `share_type` = ?');
+			$result = $getShare->execute([$uid, IShare::TYPE_USER]);
 			$shares = $result->fetchAll();
 			$result->closeCursor();
+
 			foreach ($shares as $share) {
 				$this->sendFeedbackToRemote($share['remote'], $share['share_token'], $share['remote_id'], 'decline');
 			}
 
-			$query = $this->connection->prepare('
-				DELETE FROM `*PREFIX*share_external`
-				WHERE `user` = ?
-			');
-			$deleteResult = $query->execute([$uid]);
-			$deleteResult->closeCursor();
+			$qb = $this->connection->getQueryBuilder();
+			$qb->delete('share_external')
+				// user field can specify a user or a group
+				->where($qb->expr()->eq('user', $qb->createNamedParameter($uid)))
+				->andWhere(
+					$qb->expr()->orX(
+						// delete direct shares
+						$qb->expr()->eq('share_type', $qb->expr()->literal(IShare::TYPE_USER)),
+						// delete sub-shares of group shares for that user
+						$qb->expr()->andX(
+							$qb->expr()->eq('share_type', $qb->expr()->literal(IShare::TYPE_GROUP)),
+							$qb->expr()->neq('parent', $qb->expr()->literal(-1)),
+						)
+					)
+				);
+			$qb->execute();
 		} catch (\Doctrine\DBAL\Exception $ex) {
+			$this->logger->emergency('Could not delete user shares', ['exception' => $ex]);
+			return false;
+		}
+
+		return true;
+	}
+
+	public function removeGroupShares($gid): bool {
+		try {
+			$getShare = $this->connection->prepare('
+				SELECT `id`, `remote`, `share_type`, `share_token`, `remote_id`
+				FROM  `*PREFIX*share_external`
+				WHERE `user` = ?
+				AND `share_type` = ?');
+			$result = $getShare->execute([$gid, IShare::TYPE_GROUP]);
+			$shares = $result->fetchAll();
+			$result->closeCursor();
+
+			$deletedGroupShares = [];
+			$qb = $this->connection->getQueryBuilder();
+			// delete group share entry and matching sub-entries
+			$qb->delete('share_external')
+			   ->where(
+				   $qb->expr()->orX(
+					   $qb->expr()->eq('id', $qb->createParameter('share_id')),
+					   $qb->expr()->eq('parent', $qb->createParameter('share_parent_id'))
+				   )
+			   );
+
+			foreach ($shares as $share) {
+				$qb->setParameter('share_id', $share['id']);
+				$qb->setParameter('share_parent_id', $share['id']);
+				$qb->execute();
+			}
+		} catch (\Doctrine\DBAL\Exception $ex) {
+			$this->logger->emergency('Could not delete user shares', ['exception' => $ex]);
 			return false;
 		}
 
@@ -629,23 +778,44 @@ class Manager {
 			$userGroups[] = $group->getGID();
 		}
 
-		$query = 'SELECT `id`, `remote`, `remote_id`, `share_token`, `name`, `owner`, `user`, `mountpoint`, `accepted`
-		          FROM `*PREFIX*share_external`
-				  WHERE (`user` = ? OR `user` IN (?))';
-		$parameters = [$this->uid, implode(',',$userGroups)];
-		if (!is_null($accepted)) {
-			$query .= ' AND `accepted` = ?';
-			$parameters[] = (int) $accepted;
-		}
-		$query .= ' ORDER BY `id` ASC';
+		$qb = $this->connection->getQueryBuilder();
+		$qb->select('id', 'share_type', 'parent', 'remote', 'remote_id', 'share_token', 'name', 'owner', 'user', 'mountpoint', 'accepted')
+			->from('share_external')
+			->where(
+				$qb->expr()->orX(
+					$qb->expr()->eq('user', $qb->createNamedParameter($this->uid)),
+					$qb->expr()->in(
+						'user',
+						$qb->createNamedParameter($userGroups, IQueryBuilder::PARAM_STR_ARRAY)
+					)
+				)
+			)
+			->orderBy('id', 'ASC');
 
-		$sharesQuery = $this->connection->prepare($query);
 		try {
-			$result = $sharesQuery->execute($parameters);
+			$result = $qb->execute();
 			$shares = $result->fetchAll();
 			$result->closeCursor();
-			return $shares;
+
+			// remove parent group share entry if we have a specific user share entry for the user
+			$toRemove = [];
+			foreach ($shares as $share) {
+				if ((int)$share['share_type'] === IShare::TYPE_GROUP && (int)$share['parent'] > 0) {
+					$toRemove[] = $share['parent'];
+				}
+			}
+			$shares = array_filter($shares, function ($share) use ($toRemove) {
+				return !in_array($share['id'], $toRemove, true);
+			});
+
+			if (!is_null($accepted)) {
+				$shares = array_filter($shares, function ($share) use ($accepted) {
+					return (bool)$share['accepted'] === $accepted;
+				});
+			}
+			return array_values($shares);
 		} catch (\Doctrine\DBAL\Exception $e) {
+			$this->logger->emergency('Error when retrieving shares', ['exception' => $e]);
 			return [];
 		}
 	}

apps/files_sharing/lib/Hooks.php

@@ -28,6 +28,7 @@ namespace OCA\Files_Sharing;
 
 use OC\Files\Filesystem;
 use OCP\EventDispatcher\IEventDispatcher;
+use Psr\Log\LoggerInterface;
 
 class Hooks {
 	public static function deleteUser($params) {
@@ -43,7 +44,8 @@ class Hooks {
 			\OC::$server->getGroupManager(),
 			\OC::$server->getUserManager(),
 			$params['uid'],
-			\OC::$server->query(IEventDispatcher::class)
+			\OC::$server->query(IEventDispatcher::class),
+			\OC::$server->get(LoggerInterface::class)
 		);
 
 		$manager->removeUserShares($params['uid']);

apps/files_sharing/src/share.js

@@ -181,6 +181,8 @@ import escapeHTML from 'escape-html'
 								hasShares = true
 							} else if (shareType === OC.Share.SHARE_TYPE_REMOTE) {
 								hasShares = true
+							} else if (shareType === OC.Share.SHARE_TYPE_REMOTE_GROUP) {
+								hasShares = true
 							} else if (shareType === OC.Share.SHARE_TYPE_CIRCLE) {
 								hasShares = true
 							} else if (shareType === OC.Share.SHARE_TYPE_ROOM) {
@@ -230,6 +232,10 @@ import escapeHTML from 'escape-html'
 				},
 				type: OCA.Files.FileActions.TYPE_INLINE,
 				actionHandler: function(fileName, context) {
+					// details view disabled in some share lists
+					if (!fileList._detailsView) {
+						return
+					}
 					// do not open sidebar if permission is set and equal to 0
 					var permissions = parseInt(context.$file.data('share-permissions'), 10)
 					if (isNaN(permissions) || permissions > 0) {

apps/files_sharing/tests/External/ManagerTest.php

@@ -41,10 +41,12 @@ use OCP\Federation\ICloudFederationFactory;
 use OCP\Federation\ICloudFederationProviderManager;
 use OCP\Http\Client\IClientService;
 use OCP\Http\Client\IResponse;
+use OCP\IGroup;
 use OCP\IGroupManager;
 use OCP\IURLGenerator;
 use OCP\IUserManager;
 use OCP\Share\IShare;
+use Psr\Log\LoggerInterface;
 use Test\Traits\UserTrait;
 
 /**
@@ -81,6 +83,9 @@ class ManagerTest extends TestCase {
 	/** @var \PHPUnit\Framework\MockObject\MockObject|IUserManager */
 	private $userManager;
 
+	/** @var LoggerInterface */
+	private $logger;
+
 	private $uid;
 
 	/**
@@ -112,7 +117,43 @@ class ManagerTest extends TestCase {
 			->method('search')
 			->willReturn([]);
 
-		$this->manager = $this->getMockBuilder(Manager::class)
+		$this->logger = $this->createMock(LoggerInterface::class);
+		$this->logger->expects($this->never())->method('emergency');
+
+		$this->manager = $this->createManagerForUser($this->uid);
+
+		$this->testMountProvider = new MountProvider(\OC::$server->getDatabaseConnection(), function () {
+			return $this->manager;
+		}, new CloudIdManager($this->contactsManager, $this->createMock(IURLGenerator::class), $this->userManager));
+
+		$group1 = $this->createMock(IGroup::class);
+		$group1->expects($this->any())->method('getGID')->willReturn('group1');
+		$group1->expects($this->any())->method('inGroup')->with($this->user)->willReturn(true);
+
+		$group2 = $this->createMock(IGroup::class);
+		$group2->expects($this->any())->method('getGID')->willReturn('group2');
+		$group2->expects($this->any())->method('inGroup')->with($this->user)->willReturn(true);
+
+		$this->userManager->expects($this->any())->method('get')->willReturn($this->user);
+		$this->groupManager->expects($this->any())->method(('getUserGroups'))->willReturn([$group1, $group2]);
+		$this->groupManager->expects($this->any())->method(('get'))
+			->will($this->returnValueMap([
+				['group1', $group1],
+				['group2', $group2],
+			]));
+	}
+
+	protected function tearDown(): void {
+		// clear the share external table to avoid side effects
+		$query = \OC::$server->getDatabaseConnection()->prepare('DELETE FROM `*PREFIX*share_external`');
+		$result = $query->execute();
+		$result->closeCursor();
+
+		parent::tearDown();
+	}
+
+	private function createManagerForUser($userId) {
+		return $this->getMockBuilder(Manager::class)
 			->setConstructorArgs(
 				[
 					\OC::$server->getDatabaseConnection(),
@@ -125,25 +166,23 @@ class ManagerTest extends TestCase {
 					$this->cloudFederationFactory,
 					$this->groupManager,
 					$this->userManager,
-					$this->uid,
+					$userId,
 					$this->eventDispatcher,
+					$this->logger,
 				]
 			)->setMethods(['tryOCMEndPoint'])->getMock();
-
-		$this->testMountProvider = new MountProvider(\OC::$server->getDatabaseConnection(), function () {
-			return $this->manager;
-		}, new CloudIdManager($this->contactsManager, $this->createMock(IURLGenerator::class), $this->userManager));
 	}
 
 	private function setupMounts() {
+		$this->mountManager->clear();
 		$mounts = $this->testMountProvider->getMountsForUser($this->user, new StorageFactory());
 		foreach ($mounts as $mount) {
 			$this->mountManager->addMount($mount);
 		}
 	}
 
-	public function testAddShare() {
-		$shareData1 = [
+	public function testAddUserShare() {
+		$this->doTestAddShare([
 			'remote' => 'http://localhost',
 			'token' => 'token1',
 			'password' => '',
@@ -153,23 +192,41 @@ class ManagerTest extends TestCase {
 			'accepted' => false,
 			'user' => $this->uid,
 			'remoteId' => '2342'
-		];
+		], false);
+	}
+
+	public function testAddGroupShare() {
+		$this->doTestAddShare([
+			'remote' => 'http://localhost',
+			'token' => 'token1',
+			'password' => '',
+			'name' => '/SharedFolder',
+			'owner' => 'foobar',
+			'shareType' => IShare::TYPE_GROUP,
+			'accepted' => false,
+			'user' => 'group1',
+			'remoteId' => '2342'
+		], true);
+	}
+
+	public function doTestAddShare($shareData1, $isGroup = false) {
 		$shareData2 = $shareData1;
 		$shareData2['token'] = 'token2';
 		$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', '2342', 'accept')->willReturn(false);
-		$this->manager->expects($this->at(1))->method('tryOCMEndPoint')->with('http://localhost', 'token3', '2342', 'decline')->willReturn(false);
+		if ($isGroup) {
+			$this->manager->expects($this->never())->method('tryOCMEndPoint');
+		} else {
+			$this->manager->expects($this->at(0))->method('tryOCMEndPoint')->with('http://localhost', 'token1', '2342', 'accept')->willReturn(false);
+			$this->manager->expects($this->at(1))->method('tryOCMEndPoint')->with('http://localhost', 'token3', '2342', 'decline')->willReturn(false);
+		}
 
 		// Add a share for "user"
 		$this->assertSame(null, call_user_func_array([$this->manager, 'addShare'], $shareData1));
 		$openShares = $this->manager->getOpenShares();
 		$this->assertCount(1, $openShares);
-		$this->assertExternalShareEntry($shareData1, $openShares[0], 1, '{{TemporaryMountPointName#' . $shareData1['name'] . '}}');
+		$this->assertExternalShareEntry($shareData1, $openShares[0], 1, '{{TemporaryMountPointName#' . $shareData1['name'] . '}}', $shareData1['user']);
 
 		$this->setupMounts();
 		$this->assertNotMount('SharedFolder');
@@ -179,46 +236,48 @@ class ManagerTest extends TestCase {
 		$this->assertSame(null, call_user_func_array([$this->manager, 'addShare'], $shareData2));
 		$openShares = $this->manager->getOpenShares();
 		$this->assertCount(2, $openShares);
-		$this->assertExternalShareEntry($shareData1, $openShares[0], 1, '{{TemporaryMountPointName#' . $shareData1['name'] . '}}');
+		$this->assertExternalShareEntry($shareData1, $openShares[0], 1, '{{TemporaryMountPointName#' . $shareData1['name'] . '}}', $shareData1['user']);
 		// New share falls back to "-1" appendix, because the name is already taken
-		$this->assertExternalShareEntry($shareData2, $openShares[1], 2, '{{TemporaryMountPointName#' . $shareData2['name'] . '}}-1');
+		$this->assertExternalShareEntry($shareData2, $openShares[1], 2, '{{TemporaryMountPointName#' . $shareData2['name'] . '}}-1', $shareData2['user']);
 
 		$this->setupMounts();
 		$this->assertNotMount('SharedFolder');
 		$this->assertNotMount('{{TemporaryMountPointName#' . $shareData1['name'] . '}}');
 		$this->assertNotMount('{{TemporaryMountPointName#' . $shareData1['name'] . '}}-1');
 
-		$client = $this->getMockBuilder('OCP\Http\Client\IClient')
-			->disableOriginalConstructor()->getMock();
-		$this->clientService->expects($this->at(0))
-			->method('newClient')
-			->willReturn($client);
-		$response = $this->createMock(IResponse::class);
-		$response->method('getBody')
-			->willReturn(json_encode([
-				'ocs' => [
-					'meta' => [
-						'statuscode' => 200,
+		if (!$isGroup) {
+			$client = $this->getMockBuilder('OCP\Http\Client\IClient')
+				->disableOriginalConstructor()->getMock();
+			$this->clientService->expects($this->at(0))
+				->method('newClient')
+				->willReturn($client);
+			$response = $this->createMock(IResponse::class);
+			$response->method('getBody')
+				->willReturn(json_encode([
+					'ocs' => [
+						'meta' => [
+							'statuscode' => 200,
+						]
 					]
-				]
-			]));
-		$client->expects($this->once())
-			->method('post')
-			->with($this->stringStartsWith('http://localhost/ocs/v2.php/cloud/shares/' . $openShares[0]['remote_id']), $this->anything())
-			->willReturn($response);
+				]));
+			$client->expects($this->once())
+				->method('post')
+				->with($this->stringStartsWith('http://localhost/ocs/v2.php/cloud/shares/' . $openShares[0]['remote_id']), $this->anything())
+				->willReturn($response);
+		}
 
 		// Accept the first share
-		$this->manager->acceptShare($openShares[0]['id']);
+		$this->assertTrue($this->manager->acceptShare($openShares[0]['id']));
 
 		// Check remaining shares - Accepted
 		$acceptedShares = self::invokePrivate($this->manager, 'getShares', [true]);
 		$this->assertCount(1, $acceptedShares);
 		$shareData1['accepted'] = true;
-		$this->assertExternalShareEntry($shareData1, $acceptedShares[0], 1, $shareData1['name']);
+		$this->assertExternalShareEntry($shareData1, $acceptedShares[0], 1, $shareData1['name'], $this->uid);
 		// Check remaining shares - Open
 		$openShares = $this->manager->getOpenShares();
 		$this->assertCount(1, $openShares);
-		$this->assertExternalShareEntry($shareData2, $openShares[0], 2, '{{TemporaryMountPointName#' . $shareData2['name'] . '}}-1');
+		$this->assertExternalShareEntry($shareData2, $openShares[0], 2, '{{TemporaryMountPointName#' . $shareData2['name'] . '}}-1', $shareData2['user']);
 
 		$this->setupMounts();
 		$this->assertMount($shareData1['name']);
@@ -229,36 +288,42 @@ class ManagerTest extends TestCase {
 		$this->assertSame(null, call_user_func_array([$this->manager, 'addShare'], $shareData3));
 		$openShares = $this->manager->getOpenShares();
 		$this->assertCount(2, $openShares);
-		$this->assertExternalShareEntry($shareData2, $openShares[0], 2, '{{TemporaryMountPointName#' . $shareData2['name'] . '}}-1');
-		// New share falls back to the original name (no "-\d", because the name is not taken)
-		$this->assertExternalShareEntry($shareData3, $openShares[1], 3, '{{TemporaryMountPointName#' . $shareData3['name'] . '}}');
+		$this->assertExternalShareEntry($shareData2, $openShares[0], 2, '{{TemporaryMountPointName#' . $shareData2['name'] . '}}-1', $shareData2['user']);
+		if (!$isGroup) {
+			// New share falls back to the original name (no "-\d", because the name is not taken)
+			$this->assertExternalShareEntry($shareData3, $openShares[1], 3, '{{TemporaryMountPointName#' . $shareData3['name'] . '}}', $shareData3['user']);
+		} else {
+			$this->assertExternalShareEntry($shareData3, $openShares[1], 3, '{{TemporaryMountPointName#' . $shareData3['name'] . '}}-2', $shareData3['user']);
+		}
 
 		$this->setupMounts();
 		$this->assertMount($shareData1['name']);
 		$this->assertNotMount('{{TemporaryMountPointName#' . $shareData1['name'] . '}}');
 		$this->assertNotMount('{{TemporaryMountPointName#' . $shareData1['name'] . '}}-1');
 
-		$client = $this->getMockBuilder('OCP\Http\Client\IClient')
-			->disableOriginalConstructor()->getMock();
-		$this->clientService->expects($this->at(0))
-			->method('newClient')
-			->willReturn($client);
-		$response = $this->createMock(IResponse::class);
-		$response->method('getBody')
-			->willReturn(json_encode([
-				'ocs' => [
-					'meta' => [
-						'statuscode' => 200,
+		if (!$isGroup) {
+			$client = $this->getMockBuilder('OCP\Http\Client\IClient')
+				->disableOriginalConstructor()->getMock();
+			$this->clientService->expects($this->at(0))
+				->method('newClient')
+				->willReturn($client);
+			$response = $this->createMock(IResponse::class);
+			$response->method('getBody')
+				->willReturn(json_encode([
+					'ocs' => [
+						'meta' => [
+							'statuscode' => 200,
+						]
 					]
-				]
-			]));
-		$client->expects($this->once())
-			->method('post')
-			->with($this->stringStartsWith('http://localhost/ocs/v2.php/cloud/shares/' . $openShares[1]['remote_id'] . '/decline'), $this->anything())
-			->willReturn($response);
+				]));
+			$client->expects($this->once())
+				->method('post')
+				->with($this->stringStartsWith('http://localhost/ocs/v2.php/cloud/shares/' . $openShares[1]['remote_id'] . '/decline'), $this->anything())
+				->willReturn($response);
+		}
 
 		// Decline the third share
-		$this->manager->declineShare($openShares[1]['id']);
+		$this->assertTrue($this->manager->declineShare($openShares[1]['id']));
 
 		$this->setupMounts();
 		$this->assertMount($shareData1['name']);
@@ -269,46 +334,62 @@ class ManagerTest extends TestCase {
 		$acceptedShares = self::invokePrivate($this->manager, 'getShares', [true]);
 		$this->assertCount(1, $acceptedShares);
 		$shareData1['accepted'] = true;
-		$this->assertExternalShareEntry($shareData1, $acceptedShares[0], 1, $shareData1['name']);
+		$this->assertExternalShareEntry($shareData1, $acceptedShares[0], 1, $shareData1['name'], $this->uid);
 		// Check remaining shares - Open
 		$openShares = $this->manager->getOpenShares();
-		$this->assertCount(1, $openShares);
-		$this->assertExternalShareEntry($shareData2, $openShares[0], 2, '{{TemporaryMountPointName#' . $shareData2['name'] . '}}-1');
+		if ($isGroup) {
+			// declining a group share adds it back to pending instead of deleting it
+			$this->assertCount(2, $openShares);
+			// this is a group share that is still open
+			$this->assertExternalShareEntry($shareData2, $openShares[0], 2, '{{TemporaryMountPointName#' . $shareData2['name'] . '}}-1', $shareData2['user']);
+			// this is the user share sub-entry matching the group share which got declined
+			$this->assertExternalShareEntry($shareData3, $openShares[1], 2, '{{TemporaryMountPointName#' . $shareData3['name'] . '}}-2', $this->uid);
+		} else {
+			$this->assertCount(1, $openShares);
+			$this->assertExternalShareEntry($shareData2, $openShares[0], 2, '{{TemporaryMountPointName#' . $shareData2['name'] . '}}-1', $this->uid);
+		}
 
 		$this->setupMounts();
 		$this->assertMount($shareData1['name']);
 		$this->assertNotMount('{{TemporaryMountPointName#' . $shareData1['name'] . '}}');
 		$this->assertNotMount('{{TemporaryMountPointName#' . $shareData1['name'] . '}}-1');
 
-		$client1 = $this->getMockBuilder('OCP\Http\Client\IClient')
-			->disableOriginalConstructor()->getMock();
-		$client2 = $this->getMockBuilder('OCP\Http\Client\IClient')
-			->disableOriginalConstructor()->getMock();
-		$this->clientService->expects($this->at(0))
-			->method('newClient')
-			->willReturn($client1);
-		$this->clientService->expects($this->at(1))
-			->method('newClient')
-			->willReturn($client2);
-		$response = $this->createMock(IResponse::class);
-		$response->method('getBody')
-			->willReturn(json_encode([
-				'ocs' => [
-					'meta' => [
-						'statuscode' => 200,
+		if ($isGroup) {
+			// no http requests here
+			$this->manager->removeGroupShares('group1');
+		} else {
+			$client1 = $this->getMockBuilder('OCP\Http\Client\IClient')
+				->disableOriginalConstructor()->getMock();
+			$client2 = $this->getMockBuilder('OCP\Http\Client\IClient')
+				->disableOriginalConstructor()->getMock();
+			$this->clientService->expects($this->at(0))
+				->method('newClient')
+				->willReturn($client1);
+			$this->clientService->expects($this->at(1))
+				->method('newClient')
+				->willReturn($client2);
+			$response = $this->createMock(IResponse::class);
+			$response->method('getBody')
+				->willReturn(json_encode([
+					'ocs' => [
+						'meta' => [
+							'statuscode' => 200,
+						]
 					]
-				]
-			]));
-		$client1->expects($this->once())
-			->method('post')
-			->with($this->stringStartsWith('http://localhost/ocs/v2.php/cloud/shares/' . $openShares[0]['remote_id'] . '/decline'), $this->anything())
-			->willReturn($response);
-		$client2->expects($this->once())
-			->method('post')
-			->with($this->stringStartsWith('http://localhost/ocs/v2.php/cloud/shares/' . $acceptedShares[0]['remote_id'] . '/decline'), $this->anything())
-			->willReturn($response);
+				]));
+
+			$client1->expects($this->once())
+				->method('post')
+				->with($this->stringStartsWith('http://localhost/ocs/v2.php/cloud/shares/' . $openShares[0]['remote_id'] . '/decline'), $this->anything())
+				->willReturn($response);
+			$client2->expects($this->once())
+				->method('post')
+				->with($this->stringStartsWith('http://localhost/ocs/v2.php/cloud/shares/' . $acceptedShares[0]['remote_id'] . '/decline'), $this->anything())
+				->willReturn($response);
+
+			$this->manager->removeUserShares($this->uid);
+		}
 
-		$this->manager->removeUserShares($this->uid);
 		$this->assertEmpty(self::invokePrivate($this->manager, 'getShares', [null]), 'Asserting all shares for the user have been deleted');
 
 		$this->mountManager->clear();
@@ -318,19 +399,323 @@ class ManagerTest extends TestCase {
 		$this->assertNotMount('{{TemporaryMountPointName#' . $shareData1['name'] . '}}-1');
 	}
 
+	private function verifyAcceptedGroupShare($shareData) {
+		$openShares = $this->manager->getOpenShares();
+		$this->assertCount(0, $openShares);
+		$acceptedShares = self::invokePrivate($this->manager, 'getShares', [true]);
+		$this->assertCount(1, $acceptedShares);
+		$shareData['accepted'] = true;
+		$this->assertExternalShareEntry($shareData, $acceptedShares[0], 0, $shareData['name'], $this->uid);
+		$this->setupMounts();
+		$this->assertMount($shareData['name']);
+	}
+
+	private function verifyDeclinedGroupShare($shareData, $tempMount = null) {
+		if ($tempMount === null) {
+			$tempMount = '{{TemporaryMountPointName#/SharedFolder}}';
+		}
+		$openShares = $this->manager->getOpenShares();
+		$this->assertCount(1, $openShares);
+		$acceptedShares = self::invokePrivate($this->manager, 'getShares', [true]);
+		$this->assertCount(0, $acceptedShares);
+		$this->assertExternalShareEntry($shareData, $openShares[0], 0, $tempMount, $this->uid);
+		$this->setupMounts();
+		$this->assertNotMount($shareData['name']);
+		$this->assertNotMount($tempMount);
+	}
+
+	private function createTestUserShare($userId = 'user1') {
+		$shareData = [
+			'remote' => 'http://localhost',
+			'token' => 'token1',
+			'password' => '',
+			'name' => '/SharedFolder',
+			'owner' => 'foobar',
+			'shareType' => IShare::TYPE_USER,
+			'accepted' => false,
+			'user' => $userId,
+			'remoteId' => '2342'
+		];
+
+		$this->assertSame(null, call_user_func_array([$this->manager, 'addShare'], $shareData));
+
+		return $shareData;
+	}
+	private function createTestGroupShare($groupId = 'group1') {
+		$shareData = [
+			'remote' => 'http://localhost',
+			'token' => 'token1',
+			'password' => '',
+			'name' => '/SharedFolder',
+			'owner' => 'foobar',
+			'shareType' => IShare::TYPE_GROUP,
+			'accepted' => false,
+			'user' => $groupId,
+			'remoteId' => '2342'
+		];
+
+		$this->assertSame(null, call_user_func_array([$this->manager, 'addShare'], $shareData));
+
+		$allShares = self::invokePrivate($this->manager, 'getShares', [null]);
+		foreach ($allShares as $share) {
+			if ($share['user'] === $groupId) {
+				// this will hold the main group entry
+				$groupShare = $share;
+				break;
+			}
+		}
+
+		return [$shareData, $groupShare];
+	}
+
+	public function testAcceptOriginalGroupShare() {
+		[$shareData, $groupShare] = $this->createTestGroupShare();
+		$this->assertTrue($this->manager->acceptShare($groupShare['id']));
+		$this->verifyAcceptedGroupShare($shareData);
+
+		// a second time
+		$this->assertTrue($this->manager->acceptShare($groupShare['id']));
+		$this->verifyAcceptedGroupShare($shareData);
+	}
+
+	public function testAcceptGroupShareAgainThroughGroupShare() {
+		[$shareData, $groupShare] = $this->createTestGroupShare();
+		$this->assertTrue($this->manager->acceptShare($groupShare['id']));
+		$this->verifyAcceptedGroupShare($shareData);
+
+		// decline again, this keeps the sub-share
+		$this->assertTrue($this->manager->declineShare($groupShare['id']));
+		$this->verifyDeclinedGroupShare($shareData, '/SharedFolder');
+
+		// this will return sub-entries
+		$openShares = $this->manager->getOpenShares();
+		$this->assertCount(1, $openShares);
+
+		// accept through group share
+		$this->assertTrue($this->manager->acceptShare($groupShare['id']));
+		$this->verifyAcceptedGroupShare($shareData, '/SharedFolder');
+
+		// accept a second time
+		$this->assertTrue($this->manager->acceptShare($groupShare['id']));
+		$this->verifyAcceptedGroupShare($shareData, '/SharedFolder');
+	}
+
+	public function testAcceptGroupShareAgainThroughSubShare() {
+		[$shareData, $groupShare] = $this->createTestGroupShare();
+		$this->assertTrue($this->manager->acceptShare($groupShare['id']));
+		$this->verifyAcceptedGroupShare($shareData);
+
+		// decline again, this keeps the sub-share
+		$this->assertTrue($this->manager->declineShare($groupShare['id']));
+		$this->verifyDeclinedGroupShare($shareData, '/SharedFolder');
+
+		// this will return sub-entries
+		$openShares = $this->manager->getOpenShares();
+		$this->assertCount(1, $openShares);
+
+		// accept through sub-share
+		$this->assertTrue($this->manager->acceptShare($openShares[0]['id']));
+		$this->verifyAcceptedGroupShare($shareData);
+
+		// accept a second time
+		$this->assertTrue($this->manager->acceptShare($openShares[0]['id']));
+		$this->verifyAcceptedGroupShare($shareData);
+	}
+
+	public function testDeclineOriginalGroupShare() {
+		[$shareData, $groupShare] = $this->createTestGroupShare();
+		$this->assertTrue($this->manager->declineShare($groupShare['id']));
+		$this->verifyDeclinedGroupShare($shareData);
+
+		// a second time
+		$this->assertTrue($this->manager->declineShare($groupShare['id']));
+		$this->verifyDeclinedGroupShare($shareData);
+	}
+
+	public function testDeclineGroupShareAgainThroughGroupShare() {
+		[$shareData, $groupShare] = $this->createTestGroupShare();
+		$this->assertTrue($this->manager->acceptShare($groupShare['id']));
+		$this->verifyAcceptedGroupShare($shareData);
+
+		// decline again, this keeps the sub-share
+		$this->assertTrue($this->manager->declineShare($groupShare['id']));
+		$this->verifyDeclinedGroupShare($shareData, '/SharedFolder');
+
+		// a second time
+		$this->assertTrue($this->manager->declineShare($groupShare['id']));
+		$this->verifyDeclinedGroupShare($shareData, '/SharedFolder');
+	}
+
+	public function testDeclineGroupShareAgainThroughSubshare() {
+		[$shareData, $groupShare] = $this->createTestGroupShare();
+		$this->assertTrue($this->manager->acceptShare($groupShare['id']));
+		$this->verifyAcceptedGroupShare($shareData);
+
+		// this will return sub-entries
+		$allShares = self::invokePrivate($this->manager, 'getShares', [null]);
+		$this->assertCount(1, $allShares);
+
+		// decline again through sub-share
+		$this->assertTrue($this->manager->declineShare($allShares[0]['id']));
+		$this->verifyDeclinedGroupShare($shareData, '/SharedFolder');
+
+		// a second time
+		$this->assertTrue($this->manager->declineShare($allShares[0]['id']));
+		$this->verifyDeclinedGroupShare($shareData, '/SharedFolder');
+	}
+
+	public function testDeclineGroupShareAgainThroughMountPoint() {
+		[$shareData, $groupShare] = $this->createTestGroupShare();
+		$this->assertTrue($this->manager->acceptShare($groupShare['id']));
+		$this->verifyAcceptedGroupShare($shareData);
+
+		// decline through mount point name
+		$this->assertTrue($this->manager->removeShare($this->uid . '/files/' . $shareData['name']));
+		$this->verifyDeclinedGroupShare($shareData, '/SharedFolder');
+
+		// second time must fail as the mount point is gone
+		$this->assertFalse($this->manager->removeShare($this->uid . '/files/' . $shareData['name']));
+	}
+
+	public function testDeclineThenAcceptGroupShareAgainThroughGroupShare() {
+		[$shareData, $groupShare] = $this->createTestGroupShare();
+		// decline, this creates a declined sub-share
+		$this->assertTrue($this->manager->declineShare($groupShare['id']));
+		$this->verifyDeclinedGroupShare($shareData);
+
+		// this will return sub-entries
+		$openShares = $this->manager->getOpenShares();
+
+		// accept through sub-share
+		$this->assertTrue($this->manager->acceptShare($groupShare['id']));
+		$this->verifyAcceptedGroupShare($shareData, '/SharedFolder');
+
+		// accept a second time
+		$this->assertTrue($this->manager->acceptShare($groupShare['id']));
+		$this->verifyAcceptedGroupShare($shareData, '/SharedFolder');
+	}
+
+	public function testDeclineThenAcceptGroupShareAgainThroughSubShare() {
+		[$shareData, $groupShare] = $this->createTestGroupShare();
+		// decline, this creates a declined sub-share
+		$this->assertTrue($this->manager->declineShare($groupShare['id']));
+		$this->verifyDeclinedGroupShare($shareData);
+
+		// this will return sub-entries
+		$openShares = $this->manager->getOpenShares();
+
+		// accept through sub-share
+		$this->assertTrue($this->manager->acceptShare($openShares[0]['id']));
+		$this->verifyAcceptedGroupShare($shareData);
+
+		// accept a second time
+		$this->assertTrue($this->manager->acceptShare($openShares[0]['id']));
+		$this->verifyAcceptedGroupShare($shareData);
+	}
+
+	public function testDeleteUserShares() {
+		// user 1 shares
+
+		$shareData = $this->createTestUserShare($this->uid);
+
+		[$shareData, $groupShare] = $this->createTestGroupShare();
+
+		$shares = $this->manager->getOpenShares();
+		$this->assertCount(2, $shares);
+
+		$this->assertTrue($this->manager->acceptShare($groupShare['id']));
+
+		// user 2 shares
+		$manager2 = $this->createManagerForUser('user2');
+		$shareData2 = [
+			'remote' => 'http://localhost',
+			'token' => 'token1',
+			'password' => '',
+			'name' => '/SharedFolder',
+			'owner' => 'foobar',
+			'shareType' => IShare::TYPE_USER,
+			'accepted' => false,
+			'user' => 'user2',
+			'remoteId' => '2342'
+		];
+		$this->assertSame(null, call_user_func_array([$manager2, 'addShare'], $shareData2));
+
+		$user2Shares = $manager2->getOpenShares();
+		$this->assertCount(2, $user2Shares);
+
+		$this->manager->expects($this->at(0))->method('tryOCMEndPoint')->with('http://localhost', 'token1', '2342', 'decline')->willReturn([]);
+		$this->manager->removeUserShares($this->uid);
+
+		$user1Shares = $this->manager->getOpenShares();
+		// user share is gone, group is still there
+		$this->assertCount(1, $user1Shares);
+		$this->assertEquals($user1Shares[0]['share_type'], IShare::TYPE_GROUP);
+
+		// user 2 shares untouched
+		$user2Shares = $manager2->getOpenShares();
+		$this->assertCount(2, $user2Shares);
+		$this->assertEquals($user2Shares[0]['share_type'], IShare::TYPE_GROUP);
+		$this->assertEquals($user2Shares[0]['user'], 'group1');
+		$this->assertEquals($user2Shares[1]['share_type'], IShare::TYPE_USER);
+		$this->assertEquals($user2Shares[1]['user'], 'user2');
+	}
+
+	public function testDeleteGroupShares() {
+		$shareData = $this->createTestUserShare($this->uid);
+
+		[$shareData, $groupShare] = $this->createTestGroupShare();
+
+		$shares = $this->manager->getOpenShares();
+		$this->assertCount(2, $shares);
+
+		$this->assertTrue($this->manager->acceptShare($groupShare['id']));
+
+		// user 2 shares
+		$manager2 = $this->createManagerForUser('user2');
+		$shareData2 = [
+			'remote' => 'http://localhost',
+			'token' => 'token1',
+			'password' => '',
+			'name' => '/SharedFolder',
+			'owner' => 'foobar',
+			'shareType' => IShare::TYPE_USER,
+			'accepted' => false,
+			'user' => 'user2',
+			'remoteId' => '2342'
+		];
+		$this->assertSame(null, call_user_func_array([$manager2, 'addShare'], $shareData2));
+
+		$user2Shares = $manager2->getOpenShares();
+		$this->assertCount(2, $user2Shares);
+
+		$this->manager->expects($this->never())->method('tryOCMEndPoint');
+		$this->manager->removeGroupShares('group1');
+
+		$user1Shares = $this->manager->getOpenShares();
+		// user share is gone, group is still there
+		$this->assertCount(1, $user1Shares);
+		$this->assertEquals($user1Shares[0]['share_type'], IShare::TYPE_USER);
+
+		// user 2 shares untouched
+		$user2Shares = $manager2->getOpenShares();
+		$this->assertCount(1, $user2Shares);
+		$this->assertEquals($user2Shares[0]['share_type'], IShare::TYPE_USER);
+		$this->assertEquals($user2Shares[0]['user'], 'user2');
+	}
+
 	/**
 	 * @param array $expected
 	 * @param array $actual
 	 * @param int $share
 	 * @param string $mountPoint
 	 */
-	protected function assertExternalShareEntry($expected, $actual, $share, $mountPoint) {
+	protected function assertExternalShareEntry($expected, $actual, $share, $mountPoint, $targetEntity) {
 		$this->assertEquals($expected['remote'], $actual['remote'], 'Asserting remote of a share #' . $share);
 		$this->assertEquals($expected['token'], $actual['share_token'], 'Asserting token of a share #' . $share);
 		$this->assertEquals($expected['name'], $actual['name'], 'Asserting name of a share #' . $share);
 		$this->assertEquals($expected['owner'], $actual['owner'], 'Asserting owner of a share #' . $share);
 		$this->assertEquals($expected['accepted'], (int) $actual['accepted'], 'Asserting accept of a share #' . $share);
-		$this->assertEquals($expected['user'], $actual['user'], 'Asserting user of a share #' . $share);
+		$this->assertEquals($targetEntity, $actual['user'], 'Asserting user of a share #' . $share);
 		$this->assertEquals($mountPoint, $actual['mountpoint'], 'Asserting mountpoint of a share #' . $share);
 	}