diff --git a/apps/files_sharing/tests/CapabilitiesTest.php b/apps/files_sharing/tests/CapabilitiesTest.php index eb10c9271406a..01ef13a3e6c0d 100644 --- a/apps/files_sharing/tests/CapabilitiesTest.php +++ b/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()); diff --git a/core/Controller/ProfilePageController.php b/core/Controller/ProfilePageController.php index a7ceb404fbc24..bffae69ad6f9f 100644 --- a/core/Controller/ProfilePageController.php +++ b/core/Controller/ProfilePageController.php @@ -26,14 +26,17 @@ 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\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 +51,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 +75,9 @@ public function __construct( IInitialState $initialStateService, IAccountManager $accountManager, ProfileManager $profileManager, + IShareManager $shareManager, + IGroupManager $groupManager, + KnownUserService $knownUserService, IUserManager $userManager, IUserSession $userSession, IUserStatusManager $userStatusManager @@ -71,6 +86,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,13 +101,15 @@ public function __construct( * @NoSubAdminRequired */ public function index(string $targetUserId): TemplateResponse { + $profileNotFoundTemplate = new TemplateResponse( + 'core', + '404-profile', + [], + TemplateResponse::RENDER_AS_GUEST, + ); + if (!$this->userManager->userExists($targetUserId)) { - return new TemplateResponse( - 'core', - '404-profile', - [], - TemplateResponse::RENDER_AS_GUEST, - ); + return $profileNotFoundTemplate; } $visitingUser = $this->userSession->getUser(); @@ -97,12 +117,14 @@ public function index(string $targetUserId): TemplateResponse { $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]); diff --git a/lib/private/Share20/Manager.php b/lib/private/Share20/Manager.php index ccc2d454a94de..1891e3a128337 100644 --- a/lib/private/Share20/Manager.php +++ b/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 * diff --git a/lib/public/Share/IManager.php b/lib/public/Share/IManager.php index 77a9980a8946f..2f5bc740b0082 100644 --- a/lib/public/Share/IManager.php +++ b/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 a user A can enumerate another user B + * + * @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 * diff --git a/tests/lib/Share20/ManagerTest.php b/tests/lib/Share20/ManagerTest.php index de8dc9fcc862d..de5a52bbb3ebb 100644 --- a/tests/lib/Share20/ManagerTest.php +++ b/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 @@ protected function setUp(): void { $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 @@ protected function setUp(): void { $this->urlGenerator, $this->defaults, $this->dispatcher, - $this->userSession + $this->userSession, + $this->knownUserService ); $this->defaultProvider = $this->createMock(DefaultShareProvider::class); @@ -2696,7 +2702,8 @@ public function testGetShareByToken() { $this->urlGenerator, $this->defaults, $this->dispatcher, - $this->userSession + $this->userSession, + $this->knownUserService ); $share = $this->createMock(IShare::class); @@ -2741,7 +2748,8 @@ public function testGetShareByTokenRoom() { $this->urlGenerator, $this->defaults, $this->dispatcher, - $this->userSession + $this->userSession, + $this->knownUserService ); $share = $this->createMock(IShare::class); @@ -2793,7 +2801,8 @@ public function testGetShareByTokenWithException() { $this->urlGenerator, $this->defaults, $this->dispatcher, - $this->userSession + $this->userSession, + $this->knownUserService ); $share = $this->createMock(IShare::class); @@ -4132,7 +4141,8 @@ public function testShareProviderExists($shareType, $expected) { $this->urlGenerator, $this->defaults, $this->dispatcher, - $this->userSession + $this->userSession, + $this->knownUserService ); $this->assertSame($expected, $manager->shareProviderExists($shareType) @@ -4166,7 +4176,8 @@ public function testGetSharesInFolder() { $this->urlGenerator, $this->defaults, $this->dispatcher, - $this->userSession + $this->userSession, + $this->knownUserService ); $factory->setProvider($this->defaultProvider); @@ -4231,7 +4242,8 @@ public function testGetAccessList() { $this->urlGenerator, $this->defaults, $this->dispatcher, - $this->userSession + $this->userSession, + $this->knownUserService ); $factory->setProvider($this->defaultProvider); @@ -4348,7 +4360,8 @@ public function testGetAccessListWithCurrentAccess() { $this->urlGenerator, $this->defaults, $this->dispatcher, - $this->userSession + $this->userSession, + $this->knownUserService ); $factory->setProvider($this->defaultProvider); @@ -4474,7 +4487,8 @@ public function testGetAllShares() { $this->urlGenerator, $this->defaults, $this->dispatcher, - $this->userSession + $this->userSession, + $this->knownUserService ); $factory->setProvider($this->defaultProvider);