Move common logic to share manager

Signed-off-by: Joas Schilling <coding@schilljs.com>

apps/files_sharing/tests/CapabilitiesTest.php

@@ -28,6 +28,7 @@
  */
 namespace OCA\Files_Sharing\Tests;
 
+use OC\KnownUser\KnownUserService;
 use OC\Share20\Manager;
 use OCA\Files_Sharing\Capabilities;
 use OCP\EventDispatcher\IEventDispatcher;
@@ -94,7 +95,8 @@ class CapabilitiesTest extends \Test\TestCase {
 			$this->createMock(IURLGenerator::class),
 			$this->createMock(\OC_Defaults::class),
 			$this->createMock(IEventDispatcher::class),
-			$this->createMock(IUserSession::class)
+			$this->createMock(IUserSession::class),
+			$this->createMock(KnownUserService::class)
 		);
 		$cap = new Capabilities($config, $shareManager);
 		$result = $this->getFilesSharingPart($cap->getCapabilities());

core/Controller/ProfilePageController.php

@@ -34,6 +34,7 @@ use OCP\AppFramework\Http\TemplateResponse;
 use OCP\AppFramework\Services\IInitialState;
 use OCP\IGroupManager;
 use OCP\IRequest;
+use OCP\IUser;
 use OCP\IUserManager;
 use OCP\IUserSession;
 use OCP\Share\IManager as IShareManager;
@@ -108,12 +109,11 @@ class ProfilePageController extends Controller {
 			TemplateResponse::RENDER_AS_GUEST,
 		);
 
-		if (!$this->userManager->userExists($targetUserId)) {
+		$targetUser = $this->userManager->get($targetUserId);
+		if (!$targetUser instanceof IUser) {
 			return $profileNotFoundTemplate;
 		}
-
 		$visitingUser = $this->userSession->getUser();
-		$targetUser = $this->userManager->get($targetUserId);
 		$targetAccount = $this->accountManager->getAccount($targetUser);
 
 		if (!$this->isProfileEnabled($targetAccount)) {
@@ -122,37 +122,14 @@ class ProfilePageController extends Controller {
 
 		// Run user enumeration checks only if viewing another user's profile
 		if ($targetUser !== $visitingUser) {
-			if ($this->shareManager->allowEnumerationFullMatch()) {
-				// Full id match is allowed
-			} elseif (!$this->shareManager->allowEnumeration()) {
+			if (!$this->shareManager->currentUserCanEnumerateTargetUser($visitingUser, $targetUser)) {
 				return $profileNotFoundTemplate;
-			} else {
-				if ($this->shareManager->limitEnumerationToGroups() || $this->shareManager->limitEnumerationToPhone()) {
-					$targerUserGroupIds = $this->groupManager->getUserGroupIds($targetUser);
-					$visitingUserGroupIds = $this->groupManager->getUserGroupIds($visitingUser);
-					if ($this->shareManager->limitEnumerationToGroups() && $this->shareManager->limitEnumerationToPhone()) {
-						if (
-							empty(array_intersect($targerUserGroupIds, $visitingUserGroupIds))
-							&& !$this->knownUserService->isKnownToUser($targetUser->getUID(), $visitingUser->getUID())
-						) {
-							return $profileNotFoundTemplate;
-						}
-					} elseif ($this->shareManager->limitEnumerationToGroups()) {
-						if (empty(array_intersect($targerUserGroupIds, $visitingUserGroupIds))) {
-							return $profileNotFoundTemplate;
-						}
-					} elseif ($this->shareManager->limitEnumerationToPhone()) {
-						if (!$this->knownUserService->isKnownToUser($targetUser->getUID(), $visitingUser->getUID())) {
-							return $profileNotFoundTemplate;
-						}
-					}
-				}
 			}
 		}
 
 		$userStatuses = $this->userStatusManager->getUserStatuses([$targetUserId]);
-		$status = array_shift($userStatuses);
-		if (!empty($status)) {
+		$status = $userStatuses[$targetUserId] ?? null;
+		if ($status !== null) {
 			$this->initialStateService->provideInitialState('status', [
 				'icon' => $status->getIcon(),
 				'message' => $status->getMessage(),

lib/private/Server.php

@@ -1253,7 +1253,8 @@ class Server extends ServerContainer implements IServerContainer {
 				$c->get(IURLGenerator::class),
 				$c->get('ThemingDefaults'),
 				$c->get(IEventDispatcher::class),
-				$c->get(IUserSession::class)
+				$c->get(IUserSession::class),
+				$c->get(KnownUserService::class)
 			);
 
 			return $manager;

lib/private/Share20/Manager.php

@@ -43,6 +43,7 @@ namespace OC\Share20;
 
 use OC\Cache\CappedMemoryCache;
 use OC\Files\Mount\MoveableMount;
+use OC\KnownUser\KnownUserService;
 use OC\Share20\Exception\ProviderException;
 use OCA\Files_Sharing\AppInfo\Application;
 use OCA\Files_Sharing\ISharedStorage;
@@ -118,7 +119,10 @@ class Manager implements IManager {
 	private $defaults;
 	/** @var IEventDispatcher */
 	private $dispatcher;
+	/** @var IUserSession */
 	private $userSession;
+	/** @var KnownUserService */
+	private $knownUserService;
 
 	public function __construct(
 		ILogger $logger,
@@ -137,7 +141,8 @@ class Manager implements IManager {
 		IURLGenerator $urlGenerator,
 		\OC_Defaults $defaults,
 		IEventDispatcher $dispatcher,
-		IUserSession $userSession
+		IUserSession $userSession,
+		KnownUserService $knownUserService
 	) {
 		$this->logger = $logger;
 		$this->config = $config;
@@ -160,6 +165,7 @@ class Manager implements IManager {
 		$this->defaults = $defaults;
 		$this->dispatcher = $dispatcher;
 		$this->userSession = $userSession;
+		$this->knownUserService = $knownUserService;
 	}
 
 	/**
@@ -1909,6 +1915,42 @@ class Manager implements IManager {
 		return $this->config->getAppValue('core', 'shareapi_restrict_user_enumeration_full_match', 'yes') === 'yes';
 	}
 
+	public function currentUserCanEnumerateTargetUser(?IUser $currentUser, IUser $targetUser): bool {
+		if ($this->allowEnumerationFullMatch()) {
+			return true;
+		}
+
+		if (!$this->allowEnumeration()) {
+			return false;
+		}
+
+		if (!$this->limitEnumerationToPhone() && !$this->limitEnumerationToGroups()) {
+			// Enumeration is enabled and not restricted: OK
+			return true;
+		}
+
+		if (!$currentUser instanceof IUser) {
+			// Enumeration restrictions require an account
+			return false;
+		}
+
+		// Enumeration is limited to phone match
+		if ($this->limitEnumerationToPhone() && $this->knownUserService->isKnownToUser($currentUser->getUID(), $targetUser->getUID())) {
+			return true;
+		}
+
+		// Enumeration is limited to groups
+		if ($this->limitEnumerationToGroups()) {
+			$currentUserGroupIds = $this->groupManager->getUserGroupIds($currentUser);
+			$targetUserGroupIds = $this->groupManager->getUserGroupIds($targetUser);
+			if (!empty(array_intersect($currentUserGroupIds, $targetUserGroupIds))) {
+				return true;
+			}
+		}
+
+		return false;
+	}
+
 	/**
 	 * Copied from \OC_Util::isSharingDisabledForUser
 	 *

lib/public/Share/IManager.php

@@ -32,6 +32,7 @@ namespace OCP\Share;
 use OCP\Files\Folder;
 use OCP\Files\Node;
 
+use OCP\IUser;
 use OCP\Share\Exceptions\GenericShareException;
 use OCP\Share\Exceptions\ShareNotFound;
 
@@ -447,6 +448,16 @@ interface IManager {
 	 */
 	public function allowEnumerationFullMatch(): bool;
 
+	/**
+	 * Check if the current user can enumerate the target user
+	 *
+	 * @param IUser|null $currentUser
+	 * @param IUser $targetUser
+	 * @return bool
+	 * @since 23.0.0
+	 */
+	public function currentUserCanEnumerateTargetUser(?IUser $currentUser, IUser $targetUser): bool;
+
 	/**
 	 * Check if sharing is disabled for the given user
 	 *

tests/lib/Share20/ManagerTest.php

@@ -22,6 +22,7 @@
 namespace Test\Share20;
 
 use OC\Files\Mount\MoveableMount;
+use OC\KnownUser\KnownUserService;
 use OC\Share20\DefaultShareProvider;
 use OC\Share20\Exception;
 use OC\Share20\Manager;
@@ -105,7 +106,10 @@ class ManagerTest extends \Test\TestCase {
 	protected $urlGenerator;
 	/** @var  \OC_Defaults|MockObject */
 	protected $defaults;
+	/** @var IUserSession|MockObject  */
 	protected $userSession;
+	/** @var KnownUserService|MockObject  */
+	protected $knownUserService;
 
 	protected function setUp(): void {
 		$this->logger = $this->createMock(ILogger::class);
@@ -122,6 +126,7 @@ class ManagerTest extends \Test\TestCase {
 		$this->defaults = $this->createMock(\OC_Defaults::class);
 		$this->dispatcher = $this->createMock(IEventDispatcher::class);
 		$this->userSession = $this->createMock(IUserSession::class);
+		$this->knownUserService = $this->createMock(KnownUserService::class);
 
 		$this->l10nFactory = $this->createMock(IFactory::class);
 		$this->l = $this->createMock(IL10N::class);
@@ -153,7 +158,8 @@ class ManagerTest extends \Test\TestCase {
 			$this->urlGenerator,
 			$this->defaults,
 			$this->dispatcher,
-			$this->userSession
+			$this->userSession,
+			$this->knownUserService
 		);
 
 		$this->defaultProvider = $this->createMock(DefaultShareProvider::class);
@@ -183,7 +189,8 @@ class ManagerTest extends \Test\TestCase {
 				$this->urlGenerator,
 				$this->defaults,
 				$this->dispatcher,
-				$this->userSession
+				$this->userSession,
+				$this->knownUserService
 			]);
 	}
 
@@ -2696,7 +2703,8 @@ class ManagerTest extends \Test\TestCase {
 			$this->urlGenerator,
 			$this->defaults,
 			$this->dispatcher,
-			$this->userSession
+			$this->userSession,
+			$this->knownUserService
 		);
 
 		$share = $this->createMock(IShare::class);
@@ -2741,7 +2749,8 @@ class ManagerTest extends \Test\TestCase {
 			$this->urlGenerator,
 			$this->defaults,
 			$this->dispatcher,
-			$this->userSession
+			$this->userSession,
+			$this->knownUserService
 		);
 
 		$share = $this->createMock(IShare::class);
@@ -2793,7 +2802,8 @@ class ManagerTest extends \Test\TestCase {
 			$this->urlGenerator,
 			$this->defaults,
 			$this->dispatcher,
-			$this->userSession
+			$this->userSession,
+			$this->knownUserService
 		);
 
 		$share = $this->createMock(IShare::class);
@@ -4132,7 +4142,8 @@ class ManagerTest extends \Test\TestCase {
 			$this->urlGenerator,
 			$this->defaults,
 			$this->dispatcher,
-			$this->userSession
+			$this->userSession,
+			$this->knownUserService
 		);
 		$this->assertSame($expected,
 			$manager->shareProviderExists($shareType)
@@ -4166,7 +4177,8 @@ class ManagerTest extends \Test\TestCase {
 			$this->urlGenerator,
 			$this->defaults,
 			$this->dispatcher,
-			$this->userSession
+			$this->userSession,
+			$this->knownUserService
 		);
 
 		$factory->setProvider($this->defaultProvider);
@@ -4231,7 +4243,8 @@ class ManagerTest extends \Test\TestCase {
 			$this->urlGenerator,
 			$this->defaults,
 			$this->dispatcher,
-			$this->userSession
+			$this->userSession,
+			$this->knownUserService
 		);
 
 		$factory->setProvider($this->defaultProvider);
@@ -4348,7 +4361,8 @@ class ManagerTest extends \Test\TestCase {
 			$this->urlGenerator,
 			$this->defaults,
 			$this->dispatcher,
-			$this->userSession
+			$this->userSession,
+			$this->knownUserService
 		);
 
 		$factory->setProvider($this->defaultProvider);
@@ -4474,7 +4488,8 @@ class ManagerTest extends \Test\TestCase {
 			$this->urlGenerator,
 			$this->defaults,
 			$this->dispatcher,
-			$this->userSession
+			$this->userSession,
+			$this->knownUserService
 		);
 
 		$factory->setProvider($this->defaultProvider);