Merge pull request #55302 from nextcloud/backport/55134/stable31

apps/settings/src/components/PersonalInfo/AvatarSection.vue

@@ -182,8 +182,7 @@ export default {
 				if (data.status === 'success') {
 					this.handleAvatarUpdate(false)
 				} else if (data.data === 'notsquare') {
-					const tempAvatar = generateUrl('/avatar/tmp') + '?requesttoken=' + encodeURIComponent(OC.requestToken) + '#' + Math.floor(Math.random() * 1000)
-					this.$refs.cropper.replace(tempAvatar)
+					this.$refs.cropper.replace(data.image)
 					this.showCropper = true
 				} else {
 					showError(data.data.message)

build/integration/features/avatar.feature

@@ -21,32 +21,9 @@ Feature: avatar
     And last avatar is a square of size 512
     And last avatar is not a single color
 
-
-
-  Scenario: get temporary non-square user avatar before cropping it
-    Given Logging in using web as "user0"
-    And logged in user posts temporary avatar from file "data/coloured-pattern-non-square.png"
-    When logged in user gets temporary avatar
-    Then The following headers should be set
-      | Content-Type | image/png |
-    # "last avatar" also includes the last temporary avatar
-    And last avatar is not a square
-    And last avatar is not a single color
-
-  Scenario: get non-square user avatar before cropping it
-    Given Logging in using web as "user0"
-    And logged in user posts temporary avatar from file "data/coloured-pattern-non-square.png"
-    # Avatar needs to be cropped to finish setting it
-    When user "user0" gets avatar for user "user0"
-    Then The following headers should be set
-      | Content-Type | image/png |
-      | X-NC-IsCustomAvatar | 0 |
-    And last avatar is a square of size 512
-    And last avatar is not a single color
-
   Scenario: set square user avatar from file
     Given Logging in using web as "user0"
-    When logged in user posts temporary avatar from file "data/green-square-256.png"
+    When logged in user posts avatar from file "data/green-square-256.png"
     And user "user0" gets avatar for user "user0"
     And The following headers should be set
       | Content-Type | image/png |
@@ -64,7 +41,7 @@ Feature: avatar
   Scenario: set square user avatar from internal path
     Given user "user0" uploads file "data/green-square-256.png" to "/internal-green-square-256.png"
     And Logging in using web as "user0"
-    When logged in user posts temporary avatar from internal path "internal-green-square-256.png"
+    When logged in user posts avatar from internal path "internal-green-square-256.png"
     And user "user0" gets avatar for user "user0" with size "64"
     And The following headers should be set
       | Content-Type | image/png |
@@ -78,82 +55,21 @@ Feature: avatar
     And last avatar is a square of size 64
     And last avatar is a single "#00FF00" color
 
-  Scenario: set non-square user avatar from file
-    Given Logging in using web as "user0"
-    When logged in user posts temporary avatar from file "data/coloured-pattern-non-square.png"
-    And logged in user crops temporary avatar
-      | x | 384 |
-      | y | 256 |
-      | w | 128 |
-      | h | 128 |
-    Then logged in user gets temporary avatar with 404
-    And user "user0" gets avatar for user "user0"
-    And The following headers should be set
-      | Content-Type | image/png |
-      | X-NC-IsCustomAvatar | 1 |
-    And last avatar is a square of size 512
-    And last avatar is a single "#FF0000" color
-    And user "anonymous" gets avatar for user "user0"
-    And The following headers should be set
-      | Content-Type | image/png |
-      | X-NC-IsCustomAvatar | 1 |
-    And last avatar is a square of size 512
-    And last avatar is a single "#FF0000" color
-
-  Scenario: set non-square user avatar from internal path
-    Given user "user0" uploads file "data/coloured-pattern-non-square.png" to "/internal-coloured-pattern-non-square.png"
-    And Logging in using web as "user0"
-    When logged in user posts temporary avatar from internal path "internal-coloured-pattern-non-square.png"
-    And logged in user crops temporary avatar
-      | x | 704 |
-      | y | 320 |
-      | w | 64 |
-      | h | 64 |
-    Then logged in user gets temporary avatar with 404
-    And user "user0" gets avatar for user "user0" with size "64"
-    And The following headers should be set
-      | Content-Type | image/png |
-      | X-NC-IsCustomAvatar | 1 |
-    And last avatar is a square of size 64
-    And last avatar is a single "#00FF00" color
-    And user "anonymous" gets avatar for user "user0" with size "64"
-    And The following headers should be set
-      | Content-Type | image/png |
-      | X-NC-IsCustomAvatar | 1 |
-    And last avatar is a square of size 64
-    And last avatar is a single "#00FF00" color
-
-  Scenario: cropped user avatar needs to be squared
-    Given Logging in using web as "user0"
-    And logged in user posts temporary avatar from file "data/coloured-pattern-non-square.png"
-    When logged in user crops temporary avatar with 400
-      | x | 384 |
-      | y | 256 |
-      | w | 192 |
-      | h | 128 |
-
-
-
   Scenario: delete user avatar
     Given Logging in using web as "user0"
-    And logged in user posts temporary avatar from file "data/coloured-pattern-non-square.png"
-    And logged in user crops temporary avatar
-      | x | 384 |
-      | y | 256 |
-      | w | 128 |
-      | h | 128 |
+    And logged in user posts avatar from file "data/green-square-256.png"
     And user "user0" gets avatar for user "user0"
     And The following headers should be set
       | Content-Type | image/png |
       | X-NC-IsCustomAvatar | 1 |
     And last avatar is a square of size 512
-    And last avatar is a single "#FF0000" color
+    And last avatar is a single "#00FF00" color
     And user "anonymous" gets avatar for user "user0"
     And The following headers should be set
       | Content-Type | image/png |
       | X-NC-IsCustomAvatar | 1 |
     And last avatar is a square of size 512
-    And last avatar is a single "#FF0000" color
+    And last avatar is a single "#00FF00" color
     When logged in user deletes the user avatar
     Then user "user0" gets avatar for user "user0"
     And The following headers should be set
@@ -168,40 +84,6 @@ Feature: avatar
     And last avatar is a square of size 512
     And last avatar is not a single color
 
-
-
-  Scenario: get user avatar with a larger size than the original one
-    Given Logging in using web as "user0"
-    And logged in user posts temporary avatar from file "data/coloured-pattern-non-square.png"
-    And logged in user crops temporary avatar
-      | x | 384 |
-      | y | 256 |
-      | w | 128 |
-      | h | 128 |
-    When user "user0" gets avatar for user "user0" with size "192"
-    Then The following headers should be set
-      | Content-Type | image/png |
-      | X-NC-IsCustomAvatar | 1 |
-    And last avatar is a square of size 512
-    And last avatar is a single "#FF0000" color
-
-  Scenario: get user avatar with a smaller size than the original one
-    Given Logging in using web as "user0"
-    And logged in user posts temporary avatar from file "data/coloured-pattern-non-square.png"
-    And logged in user crops temporary avatar
-      | x | 384 |
-      | y | 256 |
-      | w | 128 |
-      | h | 128 |
-    When user "user0" gets avatar for user "user0" with size "96"
-    Then The following headers should be set
-      | Content-Type | image/png |
-      | X-NC-IsCustomAvatar | 1 |
-    And last avatar is a square of size 512
-    And last avatar is a single "#FF0000" color
-
-
-
   Scenario: get default guest avatar
     When user "user0" gets avatar for guest "guest0"
     Then The following headers should be set

build/integration/features/bootstrap/Avatar.php

@@ -3,7 +3,6 @@
  * SPDX-FileCopyrightText: 2020 Nextcloud GmbH and Nextcloud contributors
  * SPDX-License-Identifier: AGPL-3.0-or-later
  */
-use Behat\Gherkin\Node\TableNode;
 use PHPUnit\Framework\Assert;
 
 require __DIR__ . '/../../vendor/autoload.php';
@@ -67,30 +66,11 @@ trait Avatar {
 	}
 
 	/**
-	 * @When logged in user gets temporary avatar
-	 */
-	public function loggedInUserGetsTemporaryAvatar() {
-		$this->loggedInUserGetsTemporaryAvatarWith('200');
-	}
-
-	/**
-	 * @When logged in user gets temporary avatar with :statusCode
-	 *
-	 * @param string $statusCode
-	 */
-	public function loggedInUserGetsTemporaryAvatarWith(string $statusCode) {
-		$this->sendingAToWithRequesttoken('GET', '/index.php/avatar/tmp');
-		$this->theHTTPStatusCodeShouldBe($statusCode);
-
-		$this->getLastAvatar();
-	}
-
-	/**
-	 * @When logged in user posts temporary avatar from file :source
+	 * @When logged in user posts avatar from file :source
 	 *
 	 * @param string $source
 	 */
-	public function loggedInUserPostsTemporaryAvatarFromFile(string $source) {
+	public function loggedInUserPostsAvatarFromFile(string $source) {
 		$file = \GuzzleHttp\Psr7\Utils::streamFor(fopen($source, 'r'));
 
 		$this->sendingAToWithRequesttoken('POST', '/index.php/avatar',
@@ -106,40 +86,15 @@ trait Avatar {
 	}
 
 	/**
-	 * @When logged in user posts temporary avatar from internal path :path
+	 * @When logged in user posts avatar from internal path :path
 	 *
 	 * @param string $path
 	 */
-	public function loggedInUserPostsTemporaryAvatarFromInternalPath(string $path) {
+	public function loggedInUserPostsAvatarFromInternalPath(string $path) {
 		$this->sendingAToWithRequesttoken('POST', '/index.php/avatar?path=' . $path);
 		$this->theHTTPStatusCodeShouldBe('200');
 	}
 
-	/**
-	 * @When logged in user crops temporary avatar
-	 *
-	 * @param TableNode $crop
-	 */
-	public function loggedInUserCropsTemporaryAvatar(TableNode $crop) {
-		$this->loggedInUserCropsTemporaryAvatarWith('200', $crop);
-	}
-
-	/**
-	 * @When logged in user crops temporary avatar with :statusCode
-	 *
-	 * @param string $statusCode
-	 * @param TableNode $crop
-	 */
-	public function loggedInUserCropsTemporaryAvatarWith(string $statusCode, TableNode $crop) {
-		$parameters = [];
-		foreach ($crop->getRowsHash() as $key => $value) {
-			$parameters[] = 'crop[' . $key . ']=' . $value;
-		}
-
-		$this->sendingAToWithRequesttoken('POST', '/index.php/avatar/cropped?' . implode('&', $parameters));
-		$this->theHTTPStatusCodeShouldBe($statusCode);
-	}
-
 	/**
 	 * @When logged in user deletes the user avatar
 	 */

core/Controller/AvatarController.php

@@ -15,14 +15,12 @@ use OCP\AppFramework\Http\Attribute\NoAdminRequired;
 use OCP\AppFramework\Http\Attribute\NoCSRFRequired;
 use OCP\AppFramework\Http\Attribute\OpenAPI;
 use OCP\AppFramework\Http\Attribute\PublicPage;
-use OCP\AppFramework\Http\DataDisplayResponse;
 use OCP\AppFramework\Http\FileDisplayResponse;
 use OCP\AppFramework\Http\JSONResponse;
 use OCP\AppFramework\Http\Response;
 use OCP\Files\File;
 use OCP\Files\IRootFolder;
 use OCP\IAvatarManager;
-use OCP\ICache;
 use OCP\IL10N;
 use OCP\IRequest;
 use OCP\IUserManager;
@@ -38,7 +36,6 @@ class AvatarController extends Controller {
 		string $appName,
 		IRequest $request,
 		protected IAvatarManager $avatarManager,
-		protected ICache $cache,
 		protected IL10N $l10n,
 		protected IUserManager $userManager,
 		protected IRootFolder $rootFolder,
@@ -199,8 +196,7 @@ class AvatarController extends Controller {
 						Http::STATUS_BAD_REQUEST
 					);
 				}
-				$this->cache->set('avatar_upload', file_get_contents($files['tmp_name'][0]), 7200);
-				$content = $this->cache->get('avatar_upload');
+				$content = file_get_contents($files['tmp_name'][0]);
 				unlink($files['tmp_name'][0]);
 			} else {
 				$phpFileUploadErrors = [
@@ -247,8 +243,6 @@ class AvatarController extends Controller {
 					try {
 						$avatar = $this->avatarManager->getAvatar($this->userId);
 						$avatar->set($image);
-						// Clean up
-						$this->cache->remove('tmpAvatar');
 						return new JSONResponse(['status' => 'success']);
 					} catch (\Throwable $e) {
 						$this->logger->error($e->getMessage(), ['exception' => $e, 'app' => 'core']);
@@ -256,9 +250,8 @@ class AvatarController extends Controller {
 					}
 				}
 
-				$this->cache->set('tmpAvatar', $image->data(), 7200);
 				return new JSONResponse(
-					['data' => 'notsquare'],
+					['data' => 'notsquare', 'image' => 'data:' . $mimeType . ';base64,' . base64_encode($image->data())],
 					Http::STATUS_OK
 				);
 			} else {
@@ -285,71 +278,4 @@ class AvatarController extends Controller {
 			return new JSONResponse(['data' => ['message' => $this->l10n->t('An error occurred. Please contact your admin.')]], Http::STATUS_BAD_REQUEST);
 		}
 	}
-
-	/**
-	 * @return JSONResponse|DataDisplayResponse
-	 */
-	#[NoAdminRequired]
-	#[FrontpageRoute(verb: 'GET', url: '/avatar/tmp')]
-	public function getTmpAvatar() {
-		$tmpAvatar = $this->cache->get('tmpAvatar');
-		if (is_null($tmpAvatar)) {
-			return new JSONResponse(['data' => [
-				'message' => $this->l10n->t('No temporary profile picture available, try again')
-			]],
-				Http::STATUS_NOT_FOUND);
-		}
-
-		$image = new \OCP\Image();
-		$image->loadFromData($tmpAvatar);
-
-		$resp = new DataDisplayResponse(
-			$image->data() ?? '',
-			Http::STATUS_OK,
-			['Content-Type' => $image->mimeType()]);
-
-		$resp->setETag((string)crc32($image->data() ?? ''));
-		$resp->cacheFor(0);
-		$resp->setLastModified(new \DateTime('now', new \DateTimeZone('GMT')));
-		return $resp;
-	}
-
-	#[NoAdminRequired]
-	#[FrontpageRoute(verb: 'POST', url: '/avatar/cropped')]
-	public function postCroppedAvatar(?array $crop = null): JSONResponse {
-		if (is_null($crop)) {
-			return new JSONResponse(['data' => ['message' => $this->l10n->t('No crop data provided')]],
-				Http::STATUS_BAD_REQUEST);
-		}
-
-		if (!isset($crop['x'], $crop['y'], $crop['w'], $crop['h'])) {
-			return new JSONResponse(['data' => ['message' => $this->l10n->t('No valid crop data provided')]],
-				Http::STATUS_BAD_REQUEST);
-		}
-
-		$tmpAvatar = $this->cache->get('tmpAvatar');
-		if (is_null($tmpAvatar)) {
-			return new JSONResponse(['data' => [
-				'message' => $this->l10n->t('No temporary profile picture available, try again')
-			]],
-				Http::STATUS_BAD_REQUEST);
-		}
-
-		$image = new \OCP\Image();
-		$image->loadFromData($tmpAvatar);
-		$image->crop($crop['x'], $crop['y'], (int)round($crop['w']), (int)round($crop['h']));
-		try {
-			$avatar = $this->avatarManager->getAvatar($this->userId);
-			$avatar->set($image);
-			// Clean up
-			$this->cache->remove('tmpAvatar');
-			return new JSONResponse(['status' => 'success']);
-		} catch (\OC\NotSquareException $e) {
-			return new JSONResponse(['data' => ['message' => $this->l10n->t('Crop is not square')]],
-				Http::STATUS_BAD_REQUEST);
-		} catch (\Exception $e) {
-			$this->logger->error($e->getMessage(), ['exception' => $e, 'app' => 'core']);
-			return new JSONResponse(['data' => ['message' => $this->l10n->t('An error occurred. Please contact your admin.')]], Http::STATUS_BAD_REQUEST);
-		}
-	}
 }

tests/Core/Controller/AvatarControllerTest.php

@@ -28,7 +28,6 @@ use OCP\Files\NotPermittedException;
 use OCP\Files\SimpleFS\ISimpleFile;
 use OCP\IAvatar;
 use OCP\IAvatarManager;
-use OCP\ICache;
 use OCP\IL10N;
 use OCP\IRequest;
 use OCP\IUser;
@@ -54,8 +53,6 @@ class AvatarControllerTest extends \Test\TestCase {
 	private $avatarFile;
 	/** @var IAvatarManager|\PHPUnit\Framework\MockObject\MockObject */
 	private $avatarManager;
-	/** @var ICache|\PHPUnit\Framework\MockObject\MockObject */
-	private $cache;
 	/** @var IL10N|\PHPUnit\Framework\MockObject\MockObject */
 	private $l;
 	/** @var IUserManager|\PHPUnit\Framework\MockObject\MockObject */
@@ -73,8 +70,6 @@ class AvatarControllerTest extends \Test\TestCase {
 		parent::setUp();
 
 		$this->avatarManager = $this->getMockBuilder('OCP\IAvatarManager')->getMock();
-		$this->cache = $this->getMockBuilder('OCP\ICache')
-			->disableOriginalConstructor()->getMock();
 		$this->l = $this->getMockBuilder(IL10N::class)->getMock();
 		$this->l->method('t')->willReturnArgument(0);
 		$this->userManager = $this->getMockBuilder(IUserManager::class)->getMock();
@@ -97,7 +92,6 @@ class AvatarControllerTest extends \Test\TestCase {
 			'core',
 			$this->request,
 			$this->avatarManager,
-			$this->cache,
 			$this->l,
 			$this->userManager,
 			$this->rootFolder,
@@ -297,25 +291,6 @@ class AvatarControllerTest extends \Test\TestCase {
 		$this->assertEquals($expectedResponse, $this->avatarController->deleteAvatar());
 	}
 
-	/**
-	 * Trying to get a tmp avatar when it is not available. 404
-	 */
-	public function testTmpAvatarNoTmp(): void {
-		$response = $this->avatarController->getTmpAvatar();
-		$this->assertEquals(Http::STATUS_NOT_FOUND, $response->getStatus());
-	}
-
-	/**
-	 * Fetch tmp avatar
-	 */
-	public function testTmpAvatarValid(): void {
-		$this->cache->method('get')->willReturn(file_get_contents(\OC::$SERVERROOT . '/tests/data/testimage.jpg'));
-
-		$response = $this->avatarController->getTmpAvatar();
-		$this->assertEquals(Http::STATUS_OK, $response->getStatus());
-	}
-
-
 	/**
 	 * When trying to post a new avatar a path or image should be posted.
 	 */
@@ -334,9 +309,6 @@ class AvatarControllerTest extends \Test\TestCase {
 		$copyRes = copy(\OC::$SERVERROOT . '/tests/data/testimage.jpg', $fileName);
 		$this->assertTrue($copyRes);
 
-		//Create file in cache
-		$this->cache->method('get')->willReturn(file_get_contents(\OC::$SERVERROOT . '/tests/data/testimage.jpg'));
-
 		//Create request return
 		$reqRet = ['error' => [0], 'tmp_name' => [$fileName], 'size' => [filesize(\OC::$SERVERROOT . '/tests/data/testimage.jpg')]];
 		$this->request->method('getUploadedFile')->willReturn($reqRet);
@@ -372,9 +344,6 @@ class AvatarControllerTest extends \Test\TestCase {
 		$copyRes = copy(\OC::$SERVERROOT . '/tests/data/testimage.gif', $fileName);
 		$this->assertTrue($copyRes);
 
-		//Create file in cache
-		$this->cache->method('get')->willReturn(file_get_contents(\OC::$SERVERROOT . '/tests/data/testimage.gif'));
-
 		//Create request return
 		$reqRet = ['error' => [0], 'tmp_name' => [$fileName], 'size' => [filesize(\OC::$SERVERROOT . '/tests/data/testimage.gif')]];
 		$this->request->method('getUploadedFile')->willReturn($reqRet);
@@ -463,93 +432,6 @@ class AvatarControllerTest extends \Test\TestCase {
 		$this->assertEquals($expectedResponse, $this->avatarController->postAvatar('avatar.jpg'));
 	}
 
-	/**
-	 * Test what happens if the upload of the avatar fails
-	 */
-	public function testPostAvatarException(): void {
-		$this->cache->expects($this->once())
-			->method('set')
-			->will($this->throwException(new \Exception('foo')));
-		$file = $this->getMockBuilder('OCP\Files\File')
-			->disableOriginalConstructor()->getMock();
-		$file->expects($this->once())
-			->method('getContent')
-			->willReturn(file_get_contents(\OC::$SERVERROOT . '/tests/data/testimage.jpg'));
-		$file->expects($this->once())
-			->method('getMimeType')
-			->willReturn('image/jpeg');
-		$userFolder = $this->getMockBuilder('OCP\Files\Folder')->getMock();
-		$this->rootFolder->method('getUserFolder')->with('userid')->willReturn($userFolder);
-		$userFolder->method('get')->willReturn($file);
-
-		$this->logger->expects($this->once())
-			->method('error')
-			->with('foo', ['exception' => new \Exception('foo'), 'app' => 'core']);
-		$expectedResponse = new Http\JSONResponse(['data' => ['message' => 'An error occurred. Please contact your admin.']], Http::STATUS_OK);
-		$this->assertEquals($expectedResponse, $this->avatarController->postAvatar('avatar.jpg'));
-	}
-
-
-	/**
-	 * Test invalid crop argument
-	 */
-	public function testPostCroppedAvatarInvalidCrop(): void {
-		$response = $this->avatarController->postCroppedAvatar([]);
-
-		$this->assertEquals(Http::STATUS_BAD_REQUEST, $response->getStatus());
-	}
-
-	/**
-	 * Test no tmp avatar to crop
-	 */
-	public function testPostCroppedAvatarNoTmpAvatar(): void {
-		$response = $this->avatarController->postCroppedAvatar(['x' => 0, 'y' => 0, 'w' => 10, 'h' => 10]);
-
-		$this->assertEquals(Http::STATUS_BAD_REQUEST, $response->getStatus());
-	}
-
-	/**
-	 * Test with non square crop
-	 */
-	public function testPostCroppedAvatarNoSquareCrop(): void {
-		$this->cache->method('get')->willReturn(file_get_contents(\OC::$SERVERROOT . '/tests/data/testimage.jpg'));
-
-		$this->avatarMock->method('set')->will($this->throwException(new \OC\NotSquareException));
-		$this->avatarManager->method('getAvatar')->willReturn($this->avatarMock);
-		$response = $this->avatarController->postCroppedAvatar(['x' => 0, 'y' => 0, 'w' => 10, 'h' => 11]);
-
-		$this->assertEquals(Http::STATUS_BAD_REQUEST, $response->getStatus());
-	}
-
-	/**
-	 * Check for proper reply on proper crop argument
-	 */
-	public function testPostCroppedAvatarValidCrop(): void {
-		$this->cache->method('get')->willReturn(file_get_contents(\OC::$SERVERROOT . '/tests/data/testimage.jpg'));
-		$this->avatarManager->method('getAvatar')->willReturn($this->avatarMock);
-		$response = $this->avatarController->postCroppedAvatar(['x' => 0, 'y' => 0, 'w' => 10, 'h' => 10]);
-
-		$this->assertEquals(Http::STATUS_OK, $response->getStatus());
-		$this->assertEquals('success', $response->getData()['status']);
-	}
-
-	/**
-	 * Test what happens if the cropping of the avatar fails
-	 */
-	public function testPostCroppedAvatarException(): void {
-		$this->cache->method('get')->willReturn(file_get_contents(\OC::$SERVERROOT . '/tests/data/testimage.jpg'));
-
-		$this->avatarMock->method('set')->will($this->throwException(new \Exception('foo')));
-		$this->avatarManager->method('getAvatar')->willReturn($this->avatarMock);
-
-		$this->logger->expects($this->once())
-			->method('error')
-			->with('foo', ['exception' => new \Exception('foo'), 'app' => 'core']);
-		$expectedResponse = new Http\JSONResponse(['data' => ['message' => 'An error occurred. Please contact your admin.']], Http::STATUS_BAD_REQUEST);
-		$this->assertEquals($expectedResponse, $this->avatarController->postCroppedAvatar(['x' => 0, 'y' => 0, 'w' => 10, 'h' => 11]));
-	}
-
-
 	/**
 	 * Check for proper reply on proper crop argument
 	 */