Skip null groups in group manager (#26871) (#26956)

* Skip null groups in group manager (#26871)

* Skip null groups in group manager

* Also skip null groups in group manager's search function

* Add more group null checks in sharing code

* Add unit tests for null group safety in group manager

* Add unit tests for sharing code null group checks

* Added tests for null groups handling in sharing code

* Ignore moveShare optional repair in mount provider

In some cases, data is inconsistent in the oc_share table due to legacy
data. The mount provider might attempt to make it consistent but if the
target group does not exist any more it cannot work. In such case we
simply ignore the exception as it is not critical. Keeping the
exception would break user accounts as they would be unable to use
their filesystem.

* Adjust null group handing + tests

* Fix new group manager tests

Signed-off-by: Morris Jobke <hey@morrisjobke.de>

3 files changed, 25 insertions, 6 deletions

diff --git a/lib/private/Group/Manager.php b/lib/private/Group/Manager.php
index e7f0acc95a1..30fde1ad193 100644
--- a/lib/private/Group/Manager.php
+++ b/lib/private/Group/Manager.php
@@ -213,7 +213,12 @@ class Manager extends PublicEmitter implements IGroupManager {
 		foreach ($this->backends as $backend) {
 			$groupIds = $backend->getGroups($search, $limit, $offset);
 			foreach ($groupIds as $groupId) {
-				$groups[$groupId] = $this->get($groupId);
+				$aGroup = $this->get($groupId);
+				if (!is_null($aGroup)) {
+					$groups[$groupId] = $aGroup;
+				} else {
+					\OC::$server->getLogger()->debug('Group "' . $groupId . '" was returned by search but not found through direct access', array('app' => 'core'));
+				}
 			}
 			if (!is_null($limit) and $limit <= 0) {
 				return array_values($groups);
@@ -246,7 +251,12 @@ class Manager extends PublicEmitter implements IGroupManager {
 			$groupIds = $backend->getUserGroups($uid);
 			if (is_array($groupIds)) {
 				foreach ($groupIds as $groupId) {
-					$groups[$groupId] = $this->get($groupId);
+					$aGroup = $this->get($groupId);
+					if (!is_null($aGroup)) {
+						$groups[$groupId] = $aGroup;
+					} else {
+						\OC::$server->getLogger()->debug('User "' . $uid . '" belongs to deleted group: "' . $groupId . '"', array('app' => 'core'));
+					}
 				}
 			}
 		}
diff --git a/lib/private/Share20/DefaultShareProvider.php b/lib/private/Share20/DefaultShareProvider.php
index f7ef3875e29..b1863f6b08d 100644
--- a/lib/private/Share20/DefaultShareProvider.php
+++ b/lib/private/Share20/DefaultShareProvider.php
@@ -329,6 +329,10 @@ class DefaultShareProvider implements IShareProvider {
 			$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');
 			}
diff --git a/lib/private/Share20/Manager.php b/lib/private/Share20/Manager.php
index cd1d52c3bbf..d6d5db324bb 100644
--- a/lib/private/Share20/Manager.php
+++ b/lib/private/Share20/Manager.php
@@ -393,10 +393,12 @@ class Manager implements IManager {
 			// The share is already shared with this user via a group share
 			if ($existingShare->getShareType() === \OCP\Share::SHARE_TYPE_GROUP) {
 				$group = $this->groupManager->get($existingShare->getSharedWith());
-				$user = $this->userManager->get($share->getSharedWith());
+				if (!is_null($group)) {
+					$user = $this->userManager->get($share->getSharedWith());
 
-				if ($group->inGroup($user) && $existingShare->getShareOwner() !== $share->getShareOwner()) {
-					throw new \Exception('Path already shared with this user');
+					if ($group->inGroup($user) && $existingShare->getShareOwner() !== $share->getShareOwner()) {
+						throw new \Exception('Path already shared with this user');
+					}
 				}
 			}
 		}
@@ -418,7 +420,7 @@ class Manager implements IManager {
 		if ($this->shareWithGroupMembersOnly()) {
 			$sharedBy = $this->userManager->get($share->getSharedBy());
 			$sharedWith = $this->groupManager->get($share->getSharedWith());
-			if (!$sharedWith->inGroup($sharedBy)) {
+			if (is_null($sharedWith) || !$sharedWith->inGroup($sharedBy)) {
 				throw new \Exception('Only sharing within your own groups is allowed');
 			}
 		}
@@ -887,6 +889,9 @@ class Manager implements IManager {
 
 		if ($share->getShareType() === \OCP\Share::SHARE_TYPE_GROUP) {
 			$sharedWith = $this->groupManager->get($share->getSharedWith());
+			if (is_null($sharedWith)) {
+				throw new \InvalidArgumentException('Group "' . $share->getSharedWith() . '" does not exist');
+			}
 			$recipient = $this->userManager->get($recipientId);
 			if (!$sharedWith->inGroup($recipient)) {
 				throw new \InvalidArgumentException('Invalid recipient');