Merge pull request #29559 from nextcloud/feat/28139/profile-respect-user-enumeration

Respect user enumeration settings on profile

Author: nickvergessen

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 @@ private function getResults(array $map) {
 			$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

@@ -26,14 +26,18 @@
 
 namespace OC\Core\Controller;
 
+use OC\KnownUser\KnownUserService;
+use OC\Profile\ProfileManager;
 use OCP\Accounts\IAccountManager;
 use OCP\AppFramework\Controller;
 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 OC\Profile\ProfileManager;
+use OCP\Share\IManager as IShareManager;
 use OCP\UserStatus\IManager as IUserStatusManager;
 
 class ProfilePageController extends Controller {
@@ -48,6 +52,15 @@ class ProfilePageController extends Controller {
 	/** @var ProfileManager */
 	private $profileManager;
 
+	/** @var IShareManager */
+	private $shareManager;
+
+	/** @var IGroupManager */
+	private $groupManager;
+
+	/** @var KnownUserService */
+	private $knownUserService;
+
 	/** @var IUserManager */
 	private $userManager;
 
@@ -63,6 +76,9 @@ public function __construct(
 		IInitialState $initialStateService,
 		IAccountManager $accountManager,
 		ProfileManager $profileManager,
+		IShareManager $shareManager,
+		IGroupManager $groupManager,
+		KnownUserService $knownUserService,
 		IUserManager $userManager,
 		IUserSession $userSession,
 		IUserStatusManager $userStatusManager
@@ -71,6 +87,9 @@ public function __construct(
 		$this->initialStateService = $initialStateService;
 		$this->accountManager = $accountManager;
 		$this->profileManager = $profileManager;
+		$this->shareManager = $shareManager;
+		$this->groupManager = $groupManager;
+		$this->knownUserService = $knownUserService;
 		$this->userManager = $userManager;
 		$this->userSession = $userSession;
 		$this->userStatusManager = $userStatusManager;
@@ -83,31 +102,34 @@ public function __construct(
 	 * @NoSubAdminRequired
 	 */
 	public function index(string $targetUserId): TemplateResponse {
-		if (!$this->userManager->userExists($targetUserId)) {
-			return new TemplateResponse(
-				'core',
-				'404-profile',
-				[],
-				TemplateResponse::RENDER_AS_GUEST,
-			);
-		}
+		$profileNotFoundTemplate = new TemplateResponse(
+			'core',
+			'404-profile',
+			[],
+			TemplateResponse::RENDER_AS_GUEST,
+		);
 
-		$visitingUser = $this->userSession->getUser();
 		$targetUser = $this->userManager->get($targetUserId);
+		if (!$targetUser instanceof IUser) {
+			return $profileNotFoundTemplate;
+		}
+		$visitingUser = $this->userSession->getUser();
 		$targetAccount = $this->accountManager->getAccount($targetUser);
 
 		if (!$this->isProfileEnabled($targetAccount)) {
-			return new TemplateResponse(
-				'core',
-				'404-profile',
-				[],
-				TemplateResponse::RENDER_AS_GUEST,
-			);
+			return $profileNotFoundTemplate;
+		}
+
+		// Run user enumeration checks only if viewing another user's profile
+		if ($targetUser !== $visitingUser) {
+			if (!$this->shareManager->currentUserCanEnumerateTargetUser($visitingUser, $targetUser)) {
+				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 @@ public function __construct($webRoot, \OC\Config $config) {
 				$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 @@
 
 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 @@ public function __construct(
 		IURLGenerator $urlGenerator,
 		\OC_Defaults $defaults,
 		IEventDispatcher $dispatcher,
-		IUserSession $userSession
+		IUserSession $userSession,
+		KnownUserService $knownUserService
 	) {
 		$this->logger = $logger;
 		$this->config = $config;
@@ -160,6 +165,7 @@ public function __construct(
 		$this->defaults = $defaults;
 		$this->dispatcher = $dispatcher;
 		$this->userSession = $userSession;
+		$this->knownUserService = $knownUserService;
 	}
 
 	/**
@@ -1909,6 +1915,42 @@ public function allowEnumerationFullMatch(): bool {
 		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 @@
 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 @@ public function limitEnumerationToPhone(): bool;
 	 */
 	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
 	 *