diff --git a/core/Controller/AvatarController.php b/core/Controller/AvatarController.php index 45c0daece0..28416264eb 100644 --- a/core/Controller/AvatarController.php +++ b/core/Controller/AvatarController.php @@ -80,18 +80,6 @@ class AvatarController extends Controller { /** @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, @@ -137,7 +125,7 @@ class AvatarController extends Controller { $user = $this->userManager->get($userId); if ($user === null) { - return $this->return404(); + return new JSONResponse([], Http::STATUS_NOT_FOUND); } $account = $this->accountManager->getAccount($user); @@ -145,7 +133,7 @@ class AvatarController extends Controller { if ($scope !== IAccountManager::VISIBILITY_PUBLIC && $this->userId === null) { // Public avatar access is not allowed - return $this->return404(); + return new JSONResponse([], Http::STATUS_NOT_FOUND); } try { @@ -157,7 +145,7 @@ class AvatarController extends Controller { ['Content-Type' => $avatarFile->getMimeType()] ); } catch (\Exception $e) { - return $this->return404(); + return new JSONResponse([], Http::STATUS_NOT_FOUND); } // Cache for 30 minutes @@ -165,12 +153,6 @@ class AvatarController extends Controller { return $resp; } - private function return404(): Http\Response { - $resp = new Http\Response(); - $resp->setStatus(Http::STATUS_NOT_FOUND); - return $resp; - } - /** * @NoAdminRequired * diff --git a/tests/Core/Controller/AvatarControllerTest.php b/tests/Core/Controller/AvatarControllerTest.php index 5fce8fc635..9441bfc124 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\ICache; use OCP\Files\File; @@ -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() { 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 */