diff --git a/core/Controller/AvatarController.php b/core/Controller/AvatarController.php index cef68ec834..31e9b84d71 100644 --- a/core/Controller/AvatarController.php +++ b/core/Controller/AvatarController.php @@ -29,6 +29,7 @@ namespace OC\Core\Controller; use OC\AppFramework\Utility\TimeFactory; +use OCP\Accounts\IAccountManager; use OCP\AppFramework\Controller; use OCP\AppFramework\Http; use OCP\AppFramework\Http\DataDisplayResponse; @@ -77,19 +78,9 @@ class AvatarController extends Controller { /** @var TimeFactory */ protected $timeFactory; + /** @var IAccountManager */ + private $accountManager; - /** - * @param string $appName - * @param IRequest $request - * @param IAvatarManager $avatarManager - * @param ICache $cache - * @param IL10N $l10n - * @param IUserManager $userManager - * @param IRootFolder $rootFolder - * @param ILogger $logger - * @param string $userId - * @param TimeFactory $timeFactory - */ public function __construct($appName, IRequest $request, IAvatarManager $avatarManager, @@ -99,7 +90,8 @@ class AvatarController extends Controller { IRootFolder $rootFolder, ILogger $logger, $userId, - TimeFactory $timeFactory) { + TimeFactory $timeFactory, + IAccountManager $accountManager) { parent::__construct($appName, $request); $this->avatarManager = $avatarManager; @@ -110,6 +102,7 @@ class AvatarController extends Controller { $this->logger = $logger; $this->userId = $userId; $this->timeFactory = $timeFactory; + $this->accountManager = $accountManager; } @@ -131,6 +124,19 @@ class AvatarController extends Controller { $size = 64; } + $user = $this->userManager->get($userId); + if ($user === null) { + return new JSONResponse([], Http::STATUS_NOT_FOUND); + } + + $account = $this->accountManager->getAccount($user); + $scope = $account->getProperty(IAccountManager::PROPERTY_AVATAR)->getScope(); + + if ($scope !== IAccountManager::VISIBILITY_PUBLIC && $this->userId === null) { + // Public avatar access is not allowed + return new JSONResponse([], Http::STATUS_NOT_FOUND); + } + try { $avatar = $this->avatarManager->getAvatar($userId); $avatarFile = $avatar->getFile($size); @@ -140,9 +146,7 @@ class AvatarController extends Controller { ['Content-Type' => $avatarFile->getMimeType()] ); } catch (\Exception $e) { - $resp = new Http\Response(); - $resp->setStatus(Http::STATUS_NOT_FOUND); - return $resp; + return new JSONResponse([], Http::STATUS_NOT_FOUND); } // Cache for 30 minutes diff --git a/tests/Core/Controller/AvatarControllerTest.php b/tests/Core/Controller/AvatarControllerTest.php index 9135a6bc92..284c82310a 100644 --- a/tests/Core/Controller/AvatarControllerTest.php +++ b/tests/Core/Controller/AvatarControllerTest.php @@ -33,6 +33,9 @@ namespace Tests\Core\Controller; use OC\AppFramework\Utility\TimeFactory; use OC\Core\Controller\AvatarController; +use OCP\Accounts\IAccount; +use OCP\Accounts\IAccountManager; +use OCP\Accounts\IAccountProperty; use OCP\AppFramework\Http; use OCP\Files\File; use OCP\Files\IRootFolder; @@ -46,6 +49,7 @@ use OCP\ILogger; use OCP\IRequest; use OCP\IUser; use OCP\IUserManager; +use PHPUnit\Framework\MockObject\MockObject; /** * Class AvatarControllerTest @@ -78,6 +82,8 @@ class AvatarControllerTest extends \Test\TestCase { private $request; /** @var TimeFactory|\PHPUnit_Framework_MockObject_MockObject */ private $timeFactory; + /** @var IAccountManager|MockObject */ + private $accountManager; protected function setUp(): void { parent::setUp(); @@ -92,6 +98,7 @@ class AvatarControllerTest extends \Test\TestCase { $this->rootFolder = $this->getMockBuilder('OCP\Files\IRootFolder')->getMock(); $this->logger = $this->getMockBuilder(ILogger::class)->getMock(); $this->timeFactory = $this->getMockBuilder('OC\AppFramework\Utility\TimeFactory')->getMock(); + $this->accountManager = $this->createMock(IAccountManager::class); $this->avatarMock = $this->getMockBuilder('OCP\IAvatar')->getMock(); $this->userMock = $this->getMockBuilder(IUser::class)->getMock(); @@ -106,7 +113,8 @@ class AvatarControllerTest extends \Test\TestCase { $this->rootFolder, $this->logger, 'userid', - $this->timeFactory + $this->timeFactory, + $this->accountManager ); // Configure userMock @@ -137,6 +145,39 @@ class AvatarControllerTest extends \Test\TestCase { $this->assertEquals(Http::STATUS_NOT_FOUND, $response->getStatus()); } + public function testAvatarNotPublic() { + $account = $this->createMock(IAccount::class); + $this->accountManager->method('getAccount') + ->with($this->userMock) + ->willReturn($account); + + $property = $this->createMock(IAccountProperty::class); + $account->method('getProperty') + ->with(IAccountManager::PROPERTY_AVATAR) + ->willReturn($property); + + $property->method('getScope') + ->willReturn(IAccountManager::VISIBILITY_PRIVATE); + + $controller = new AvatarController( + 'core', + $this->request, + $this->avatarManager, + $this->cache, + $this->l, + $this->userManager, + $this->rootFolder, + $this->logger, + null, + $this->timeFactory, + $this->accountManager + ); + + $result = $controller->getAvatar('userId', 128); + + $this->assertEquals(Http::STATUS_NOT_FOUND, $result->getStatus()); + } + /** * Fetch the user's avatar */