From b553b43b685f03c8a849d272fbcdd0e8189e6edb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Calvi=C3=B1o=20S=C3=A1nchez?= Date: Thu, 3 Dec 2020 13:07:54 +0100 Subject: [PATCH 01/53] Make possible to send requests as anonymous users in integration tests MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Until now requests always had "auth" headers either for an admin or a regular user, depending on the value of "currentUser". Now, if "currentUser" starts by "anonymous" no "auth" header is sent, which makes possible to also test requests with users not logged in. Signed-off-by: Daniel Calviño Sánchez --- build/integration/features/bootstrap/BasicStructure.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/build/integration/features/bootstrap/BasicStructure.php b/build/integration/features/bootstrap/BasicStructure.php index eed0f173ced46..2c08d6ff03304 100644 --- a/build/integration/features/bootstrap/BasicStructure.php +++ b/build/integration/features/bootstrap/BasicStructure.php @@ -178,7 +178,7 @@ public function sendingToWith($verb, $url, $body) { $options = []; if ($this->currentUser === 'admin') { $options['auth'] = $this->adminUser; - } else { + } elseif (strpos($this->currentUser, 'anonymous') !== 0) { $options['auth'] = [$this->currentUser, $this->regularUser]; } $options['headers'] = [ @@ -218,7 +218,7 @@ public function sendingToWithDirectUrl($verb, $url, $body) { $options = []; if ($this->currentUser === 'admin') { $options['auth'] = $this->adminUser; - } else { + } elseif (strpos($this->currentUser, 'anonymous') !== 0) { $options['auth'] = [$this->currentUser, $this->regularUser]; } if ($body instanceof TableNode) { From 184742e6ff1fe27ebc663dd1adff9ae87408d87d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Calvi=C3=B1o=20S=C3=A1nchez?= Date: Thu, 3 Dec 2020 13:11:19 +0100 Subject: [PATCH 02/53] Make possible to set body in requesttoken requests in integration tests MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit "sendingAToWithRequesttoken" needs to be used to test some non OCS endpoints which require the request token to be sent in the request. Now it is possible to specify the body (or, rather, additional contents beside the cookies and the request token) for those requests, as it will be needed for example to upload an avatar. Signed-off-by: Daniel Calviño Sánchez --- .../features/bootstrap/BasicStructure.php | 24 +++++++++++++------ 1 file changed, 17 insertions(+), 7 deletions(-) diff --git a/build/integration/features/bootstrap/BasicStructure.php b/build/integration/features/bootstrap/BasicStructure.php index 2c08d6ff03304..4775a23b90265 100644 --- a/build/integration/features/bootstrap/BasicStructure.php +++ b/build/integration/features/bootstrap/BasicStructure.php @@ -307,21 +307,31 @@ public function loggingInUsingWebAs($user) { * @When Sending a :method to :url with requesttoken * @param string $method * @param string $url + * @param TableNode|array|null $body */ - public function sendingAToWithRequesttoken($method, $url) { + public function sendingAToWithRequesttoken($method, $url, $body = null) { $baseUrl = substr($this->baseUrl, 0, -5); + $options = [ + 'cookies' => $this->cookieJar, + 'headers' => [ + 'requesttoken' => $this->requestToken + ], + ]; + + if ($body instanceof TableNode) { + $fd = $body->getRowsHash(); + $options['form_params'] = $fd; + } elseif ($body) { + $options = array_merge($options, $body); + } + $client = new Client(); try { $this->response = $client->request( $method, $baseUrl . $url, - [ - 'cookies' => $this->cookieJar, - 'headers' => [ - 'requesttoken' => $this->requestToken - ] - ] + $options ); } catch (ClientException $e) { $this->response = $e->getResponse(); From 2cc22a06b4d56e1d46d1de45993b3149455c3bb4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Calvi=C3=B1o=20S=C3=A1nchez?= Date: Thu, 3 Dec 2020 14:19:43 +0100 Subject: [PATCH 03/53] Add integration tests for user avatars MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Daniel Calviño Sánchez --- .drone.yml | 25 +++ build/integration/data/coloured-pattern.png | Bin 0 -> 2447 bytes build/integration/data/green-square-256.png | Bin 0 -> 645 bytes build/integration/features/avatar.feature | 135 ++++++++++++ .../integration/features/bootstrap/Avatar.php | 208 ++++++++++++++++++ .../features/bootstrap/BasicStructure.php | 1 + 6 files changed, 369 insertions(+) create mode 100644 build/integration/data/coloured-pattern.png create mode 100644 build/integration/data/green-square-256.png create mode 100644 build/integration/features/avatar.feature create mode 100644 build/integration/features/bootstrap/Avatar.php diff --git a/.drone.yml b/.drone.yml index aa718998203d5..e9cacb376fc9d 100644 --- a/.drone.yml +++ b/.drone.yml @@ -857,6 +857,31 @@ trigger: - pull_request - push +--- +kind: pipeline +name: integration-avatar + +steps: +- name: submodules + image: docker:git + commands: + - git submodule update --init +- name: integration-auth + image: nextcloudci/integration-php7.3:integration-php7.3-2 + commands: + - bash tests/drone-run-integration-tests.sh || exit 0 + - ./occ maintenance:install --admin-pass=admin --data-dir=/dev/shm/nc_int + - cd build/integration + - ./run.sh features/avatar.feature + +trigger: + branch: + - master + - stable* + event: + - pull_request + - push + --- kind: pipeline name: integration-maintenance-mode diff --git a/build/integration/data/coloured-pattern.png b/build/integration/data/coloured-pattern.png new file mode 100644 index 0000000000000000000000000000000000000000..cf43787f3fda7b081bb113e6843fa332c4a5108a GIT binary patch literal 2447 zcmeAS@N?(olHy`uVBq!ia0y~yU;#3j7&w@K)a3P@?|>9%fk$L90|U1(2s1Lwnj--e zWH0gbb!C6T!^9&ab9IW|380W{W=KSdbAE1aYF-JD%fR4Vl$uzQnxasiS(2gP?&%wl zqL<1JG>_HO#WAGf*4vwmyayBnSPs@TuS22ImLZ0r zdEy)!i-(lIb7~DNI&@DtZ;mON*vHE ziVP$|GwGp1nwv-nJ9vByHaC$FpAEn$V_-}G>Y%G9cs?`#<`S-nXuG%zVg9HN) w1GTxdF%`+ZNKBgegxUdu;a*yVI>eF~?g#~@|DmDUoS^jK>FVdQ&MBb@061uAZ~y=R literal 0 HcmV?d00001 diff --git a/build/integration/features/avatar.feature b/build/integration/features/avatar.feature new file mode 100644 index 0000000000000..8580471ef5512 --- /dev/null +++ b/build/integration/features/avatar.feature @@ -0,0 +1,135 @@ +Feature: avatar + + Background: + Given user "user0" exists + + Scenario: get default user avatar + 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 128 + And last avatar is not a single color + + Scenario: get default user avatar as an anonymous user + When user "anonymous" 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 128 + And last avatar is not a single color + + + + Scenario: get temporary user avatar before cropping it + Given Logging in using web as "user0" + And logged in user posts temporary avatar from file "data/green-square-256.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 a square of size 256 + And last avatar is a single "#00FF00" color + + Scenario: get user avatar before cropping it + Given Logging in using web as "user0" + And logged in user posts temporary avatar from file "data/green-square-256.png" + # Avatar needs to be cropped to finish setting it even if it is squared + 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 128 + And last avatar is not a single color + + + + Scenario: set user avatar from file + Given Logging in using web as "user0" + When logged in user posts temporary avatar from file "data/coloured-pattern.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 128 + 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 128 + And last avatar is a single "#FF0000" color + + Scenario: set user avatar from internal path + Given user "user0" uploads file "data/coloured-pattern.png" to "/internal-coloured-pattern.png" + And Logging in using web as "user0" + When logged in user posts temporary avatar from internal path "internal-coloured-pattern.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.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.png" + And logged in user crops temporary avatar + | x | 384 | + | y | 256 | + | w | 128 | + | h | 128 | + 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 128 + 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 128 + And last avatar is a single "#FF0000" color + When logged in user deletes the user avatar + Then user "user0" gets avatar for user "user0" + And The following headers should be set + | Content-Type | image/png | + | X-NC-IsCustomAvatar | 0 | + And last avatar is a square of size 128 + And last avatar is not a single color + And user "anonymous" gets avatar for user "user0" + And The following headers should be set + | Content-Type | image/png | + | X-NC-IsCustomAvatar | 0 | + And last avatar is a square of size 128 + And last avatar is not a single color diff --git a/build/integration/features/bootstrap/Avatar.php b/build/integration/features/bootstrap/Avatar.php new file mode 100644 index 0000000000000..215a3386ab8bc --- /dev/null +++ b/build/integration/features/bootstrap/Avatar.php @@ -0,0 +1,208 @@ + + * + * @license GNU AGPL version 3 or any later version + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU Affero General Public License as + * published by the Free Software Foundation, either version 3 of the + * License, or (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Affero General Public License for more details. + * + * You should have received a copy of the GNU Affero General Public License + * along with this program. If not, see . + * + */ + +use Behat\Gherkin\Node\TableNode; +use PHPUnit\Framework\Assert; + +require __DIR__ . '/../../vendor/autoload.php'; + +trait Avatar { + + /** @var string **/ + private $lastAvatar; + + /** @AfterScenario **/ + public function cleanupLastAvatar() { + $this->lastAvatar = null; + } + + private function getLastAvatar() { + $this->lastAvatar = ''; + + $body = $this->response->getBody(); + while (!$body->eof()) { + $this->lastAvatar .= $body->read(8192); + } + $body->close(); + } + + /** + * @When user :user gets avatar for user :userAvatar + * + * @param string $user + * @param string $userAvatar + */ + public function userGetsAvatarForUser(string $user, string $userAvatar) { + $this->userGetsAvatarForUserWithSize($user, $userAvatar, '128'); + } + + /** + * @When user :user gets avatar for user :userAvatar with size :size + * + * @param string $user + * @param string $userAvatar + * @param string $size + */ + public function userGetsAvatarForUserWithSize(string $user, string $userAvatar, string $size) { + $this->asAn($user); + $this->sendingToDirectUrl('GET', '/index.php/avatar/' . $userAvatar . '/' . $size); + $this->theHTTPStatusCodeShouldBe('200'); + + $this->getLastAvatar(); + } + + /** + * @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 + * + * @param string $source + */ + public function loggedInUserPostsTemporaryAvatarFromFile(string $source) { + $file = \GuzzleHttp\Psr7\stream_for(fopen($source, 'r')); + + $this->sendingAToWithRequesttoken('POST', '/index.php/avatar', + [ + 'multipart' => [ + [ + 'name' => 'files[]', + 'contents' => $file + ] + ] + ]); + $this->theHTTPStatusCodeShouldBe('200'); + } + + /** + * @When logged in user posts temporary avatar from internal path :path + * + * @param string $path + */ + public function loggedInUserPostsTemporaryAvatarFromInternalPath(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 + */ + public function loggedInUserDeletesTheUserAvatar() { + $this->sendingAToWithRequesttoken('DELETE', '/index.php/avatar'); + $this->theHTTPStatusCodeShouldBe('200'); + } + + /** + * @Then last avatar is a square of size :size + * + * @param string size + */ + public function lastAvatarIsASquareOfSize(string $size) { + list($width, $height) = getimagesizefromstring($this->lastAvatar); + + Assert::assertEquals($width, $height, 'Avatar is not a square'); + Assert::assertEquals($size, $width); + } + + /** + * @Then last avatar is not a single color + */ + public function lastAvatarIsNotASingleColor() { + Assert::assertEquals(null, $this->getColorFromLastAvatar()); + } + + /** + * @Then last avatar is a single :color color + * + * @param string $color + * @param string $size + */ + public function lastAvatarIsASingleColor(string $color) { + Assert::assertEquals($color, $this->getColorFromLastAvatar()); + } + + private function getColorFromLastAvatar() { + $image = imagecreatefromstring($this->lastAvatar); + + $firstPixelColor = imagecolorat($image, 0, 0); + + for ($i = 0; $i < imagesx($image); $i++) { + for ($j = 0; $j < imagesx($image); $j++) { + $currentPixelColor = imagecolorat($image, $i, $j); + + if ($firstPixelColor !== $currentPixelColor) { + imagedestroy($image); + + return null; + } + } + } + + imagedestroy($image); + + // Assume that the image is a truecolor image and thus the index is the + // RGB value of the pixel as an integer. + return '#' . str_pad(strtoupper(dechex($firstPixelColor)), 6, '0', STR_PAD_LEFT); + } +} diff --git a/build/integration/features/bootstrap/BasicStructure.php b/build/integration/features/bootstrap/BasicStructure.php index 4775a23b90265..ac5530be5a5ea 100644 --- a/build/integration/features/bootstrap/BasicStructure.php +++ b/build/integration/features/bootstrap/BasicStructure.php @@ -44,6 +44,7 @@ trait BasicStructure { use Auth; + use Avatar; use Download; use Mail; use Trashbin; From 1552add4caaa5239e67d416dd75c4d7ca9085419 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Calvi=C3=B1o=20S=C3=A1nchez?= Date: Fri, 4 Dec 2020 04:28:51 +0100 Subject: [PATCH 04/53] Add integration tests for resized user avatars MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Even on solid color images the resizing can cause some small artifacts that slightly modify the color of certain pixels. Due to this now the color comparison is no longer strict but fuzzy. Signed-off-by: Daniel Calviño Sánchez --- build/integration/features/avatar.feature | 32 ++++++++++ .../integration/features/bootstrap/Avatar.php | 63 ++++++++++++++++--- 2 files changed, 88 insertions(+), 7 deletions(-) diff --git a/build/integration/features/avatar.feature b/build/integration/features/avatar.feature index 8580471ef5512..579417844efdf 100644 --- a/build/integration/features/avatar.feature +++ b/build/integration/features/avatar.feature @@ -133,3 +133,35 @@ Feature: avatar | X-NC-IsCustomAvatar | 0 | And last avatar is a square of size 128 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.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 192 + 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.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 96 + And last avatar is a single "#FF0000" color diff --git a/build/integration/features/bootstrap/Avatar.php b/build/integration/features/bootstrap/Avatar.php index 215a3386ab8bc..90cc36067bf7f 100644 --- a/build/integration/features/bootstrap/Avatar.php +++ b/build/integration/features/bootstrap/Avatar.php @@ -179,19 +179,50 @@ public function lastAvatarIsNotASingleColor() { * @param string $size */ public function lastAvatarIsASingleColor(string $color) { - Assert::assertEquals($color, $this->getColorFromLastAvatar()); + $expectedColor = $this->hexStringToRgbColor($color); + $colorFromLastAvatar = $this->getColorFromLastAvatar(); + + Assert::assertTrue($this->isSameColor($expectedColor, $colorFromLastAvatar), + $this->rgbColorToHexString($colorFromLastAvatar) . ' does not match expected ' . $color); + } + + private function hexStringToRgbColor($hexString) { + // Strip initial "#" + $hexString = substr($hexString, 1); + + $rgbColorInt = hexdec($hexString); + + // RGBA hex strings are not supported; the given string is assumed to be + // an RGB hex string. + return [ + 'red' => ($rgbColorInt >> 16) & 0xFF, + 'green' => ($rgbColorInt >> 8) & 0xFF, + 'blue' => $rgbColorInt & 0xFF, + 'alpha' => 0 + ]; + } + + private function rgbColorToHexString($rgbColor) { + $rgbColorInt = ($rgbColor['red'] << 16) + ($rgbColor['green'] << 8) + ($rgbColor['blue']); + + return '#' . str_pad(strtoupper(dechex($rgbColorInt)), 6, '0', STR_PAD_LEFT); } private function getColorFromLastAvatar() { $image = imagecreatefromstring($this->lastAvatar); - $firstPixelColor = imagecolorat($image, 0, 0); + $firstPixelColorIndex = imagecolorat($image, 0, 0); + $firstPixelColor = imagecolorsforindex($image, $firstPixelColorIndex); for ($i = 0; $i < imagesx($image); $i++) { for ($j = 0; $j < imagesx($image); $j++) { - $currentPixelColor = imagecolorat($image, $i, $j); + $currentPixelColorIndex = imagecolorat($image, $i, $j); + $currentPixelColor = imagecolorsforindex($image, $currentPixelColorIndex); - if ($firstPixelColor !== $currentPixelColor) { + // The colors are compared with a small allowed delta, as even + // on solid color images the resizing can cause some small + // artifacts that slightly modify the color of certain pixels. + if (!$this->isSameColor($firstPixelColor, $currentPixelColor)) { imagedestroy($image); return null; @@ -201,8 +232,26 @@ private function getColorFromLastAvatar() { imagedestroy($image); - // Assume that the image is a truecolor image and thus the index is the - // RGB value of the pixel as an integer. - return '#' . str_pad(strtoupper(dechex($firstPixelColor)), 6, '0', STR_PAD_LEFT); + return $firstPixelColor; + } + + private function isSameColor(array $firstColor, array $secondColor, int $allowedDelta = 1) { + if ($this->isSameColorComponent($firstColor['red'], $secondColor['red'], $allowedDelta) && + $this->isSameColorComponent($firstColor['green'], $secondColor['green'], $allowedDelta) && + $this->isSameColorComponent($firstColor['blue'], $secondColor['blue'], $allowedDelta) && + $this->isSameColorComponent($firstColor['alpha'], $secondColor['alpha'], $allowedDelta)) { + return true; + } + + return false; + } + + private function isSameColorComponent(int $firstColorComponent, int $secondColorComponent, int $allowedDelta) { + if ($firstColorComponent >= ($secondColorComponent - $allowedDelta) && + $firstColorComponent <= ($secondColorComponent + $allowedDelta)) { + return true; + } + + return false; } } From b4b3276a5be575d6edefca1214c38c2bd0f0b4e5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Calvi=C3=B1o=20S=C3=A1nchez?= Date: Mon, 7 Dec 2020 01:39:36 +0100 Subject: [PATCH 05/53] Add integration tests for getting guest avatars MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Daniel Calviño Sánchez --- build/integration/features/avatar.feature | 16 ++++++++++++++++ build/integration/features/bootstrap/Avatar.php | 14 ++++++++++++++ 2 files changed, 30 insertions(+) diff --git a/build/integration/features/avatar.feature b/build/integration/features/avatar.feature index 579417844efdf..f7926615c011f 100644 --- a/build/integration/features/avatar.feature +++ b/build/integration/features/avatar.feature @@ -165,3 +165,19 @@ Feature: avatar | X-NC-IsCustomAvatar | 1 | And last avatar is a square of size 96 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 + | Content-Type | image/png | + And last avatar is a square of size 128 + And last avatar is not a single color + + Scenario: get default guest avatar as an anonymous user + When user "anonymous" gets avatar for guest "guest0" + Then The following headers should be set + | Content-Type | image/png | + And last avatar is a square of size 128 + And last avatar is not a single color diff --git a/build/integration/features/bootstrap/Avatar.php b/build/integration/features/bootstrap/Avatar.php index 90cc36067bf7f..388715340c6e5 100644 --- a/build/integration/features/bootstrap/Avatar.php +++ b/build/integration/features/bootstrap/Avatar.php @@ -71,6 +71,20 @@ public function userGetsAvatarForUserWithSize(string $user, string $userAvatar, $this->getLastAvatar(); } + /** + * @When user :user gets avatar for guest :guestAvatar + * + * @param string $user + * @param string $guestAvatar + */ + public function userGetsAvatarForGuest(string $user, string $guestAvatar) { + $this->asAn($user); + $this->sendingToDirectUrl('GET', '/index.php/avatar/guest/' . $guestAvatar . '/128'); + $this->theHTTPStatusCodeShouldBe('201'); + + $this->getLastAvatar(); + } + /** * @When logged in user gets temporary avatar */ From 1de8dc38b0933c8791267557e44058396c1f2540 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Calvi=C3=B1o=20S=C3=A1nchez?= Date: Mon, 7 Dec 2020 04:37:53 +0100 Subject: [PATCH 06/53] Add getter for generic avatars to IAvatarManager MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Daniel Calviño Sánchez --- lib/private/Avatar/AvatarManager.php | 12 ++++++++++++ lib/public/IAvatarManager.php | 13 +++++++++++++ 2 files changed, 25 insertions(+) diff --git a/lib/private/Avatar/AvatarManager.php b/lib/private/Avatar/AvatarManager.php index 5102396224d87..2f10a22b026a9 100644 --- a/lib/private/Avatar/AvatarManager.php +++ b/lib/private/Avatar/AvatarManager.php @@ -152,4 +152,16 @@ public function deleteUserAvatar(string $userId): void { public function getGuestAvatar(string $name): IAvatar { return new GuestAvatar($name, $this->logger); } + + public function getGenericAvatar(string $type, string $id): IAvatar { + if ($type === 'user') { + return $this->getAvatar($id); + } + + if ($type === 'guest') { + return $this->getGuestAvatar($id); + } + + throw new \InvalidArgumentException('Unknown avatar type: ' . $type); + } } diff --git a/lib/public/IAvatarManager.php b/lib/public/IAvatarManager.php index 75ea886c16a7c..479d4dd1ee52e 100644 --- a/lib/public/IAvatarManager.php +++ b/lib/public/IAvatarManager.php @@ -56,4 +56,17 @@ public function getAvatar(string $user) : IAvatar; * @since 16.0.0 */ public function getGuestAvatar(string $name): IAvatar; + + /** + * Returns an avatar instance of the given type. + * + * @param string $type the type of the avatar + * @param string $id the identifier for the avatar of the given type + * @return IAvatar + * @throws \InvalidArgumentException if the type is not known or the id is + * not valid + * @throws \Exception if getting the avatar failed + * @since 21.0.0 + */ + public function getGenericAvatar(string $type, string $id): IAvatar; } From bcce5a65c000dd0cf51b903ee20feb148dc210af Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Calvi=C3=B1o=20S=C3=A1nchez?= Date: Mon, 7 Dec 2020 04:39:13 +0100 Subject: [PATCH 07/53] Add OCS endpoint for avatars MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Daniel Calviño Sánchez --- core/Controller/GenericAvatarController.php | 173 ++++++++++++++++++++ core/routes.php | 3 + 2 files changed, 176 insertions(+) create mode 100644 core/Controller/GenericAvatarController.php diff --git a/core/Controller/GenericAvatarController.php b/core/Controller/GenericAvatarController.php new file mode 100644 index 0000000000000..dedf6e3f7568f --- /dev/null +++ b/core/Controller/GenericAvatarController.php @@ -0,0 +1,173 @@ + + * + * @license GNU AGPL version 3 or any later version + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU Affero General Public License as + * published by the Free Software Foundation, either version 3 of the + * License, or (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Affero General Public License for more details. + * + * You should have received a copy of the GNU Affero General Public License + * along with this program. If not, see . + * + */ + +namespace OC\Core\Controller; + +use OCP\AppFramework\OCSController; +use OCP\AppFramework\Http; +use OCP\AppFramework\Http\FileDisplayResponse; +use OCP\AppFramework\Http\JSONResponse; +use OCP\IAvatarManager; +use OCP\IL10N; +use OCP\ILogger; +use OCP\IRequest; + +class GenericAvatarController extends OCSController { + + /** @var IAvatarManager */ + protected $avatarManager; + + /** @var IL10N */ + protected $l; + + /** @var ILogger */ + protected $logger; + + public function __construct($appName, + IRequest $request, + IAvatarManager $avatarManager, + IL10N $l10n, + ILogger $logger) { + parent::__construct($appName, $request); + + $this->avatarManager = $avatarManager; + $this->l = $l10n; + $this->logger = $logger; + } + + /** + * @PublicPage + * + * @param string $avatarType + * @param string $avatarId + * @param int $size + * @return JSONResponse|FileDisplayResponse + */ + public function getAvatar(string $avatarType, string $avatarId, int $size) { + // min/max size + if ($size > 2048) { + $size = 2048; + } elseif ($size <= 0) { + $size = 64; + } + + try { + $avatar = $this->avatarManager->getGenericAvatar($avatarType, $avatarId); + $avatarFile = $avatar->getFile($size); + $response = new FileDisplayResponse( + $avatarFile, + Http::STATUS_OK, + [ + 'Content-Type' => $avatarFile->getMimeType(), + 'X-NC-IsCustomAvatar' => (int)$avatar->isCustomAvatar() + ] + ); + } catch (\Exception $e) { + return new JSONResponse([], Http::STATUS_NOT_FOUND); + } + + return $response; + } + + /** + * @PublicPage + * + * @param string $path + * @return JSONResponse + */ + public function setAvatar(string $avatarType, string $avatarId) { + $files = $this->request->getUploadedFile('files'); + + if (is_null($files)) { + return new JSONResponse( + ['data' => ['message' => $this->l->t('No file provided')]], + Http::STATUS_BAD_REQUEST + ); + } + + if ( + $files['error'][0] !== 0 || + !is_uploaded_file($files['tmp_name'][0]) || + \OC\Files\Filesystem::isFileBlacklisted($files['tmp_name'][0]) + ) { + return new JSONResponse( + ['data' => ['message' => $this->l->t('Invalid file provided')]], + Http::STATUS_BAD_REQUEST + ); + } + + if ($files['size'][0] > 20 * 1024 * 1024) { + return new JSONResponse( + ['data' => ['message' => $this->l->t('File is too big')]], + Http::STATUS_BAD_REQUEST + ); + } + + $content = file_get_contents($files['tmp_name'][0]); + unlink($files['tmp_name'][0]); + + $image = new \OC_Image(); + $image->loadFromData($content); + + try { + $avatar = $this->avatarManager->getGenericAvatar($avatarType, $avatarId); + $avatar->set($image); + return new JSONResponse( + ['status' => 'success'] + ); + } catch (\OC\NotSquareException $e) { + return new JSONResponse( + ['data' => ['message' => $this->l->t('Crop is not square')]], + Http::STATUS_BAD_REQUEST + ); + } catch (\Exception $e) { + $this->logger->logException($e, ['app' => 'core']); + return new JSONResponse( + ['data' => ['message' => $this->l->t('An error occurred. Please contact your admin.')]], + Http::STATUS_BAD_REQUEST + ); + } + } + + /** + * @PublicPage + * + * @return JSONResponse + */ + public function deleteAvatar(string $avatarType, string $avatarId) { + try { + $avatar = $this->avatarManager->getGenericAvatar($avatarType, $avatarId); + $avatar->remove(); + return new JSONResponse(); + } catch (\Exception $e) { + $this->logger->logException($e, ['app' => 'core']); + return new JSONResponse( + ['data' => ['message' => $this->l->t('An error occurred. Please contact your admin.')]], + Http::STATUS_BAD_REQUEST + ); + } + } +} diff --git a/core/routes.php b/core/routes.php index 9fa378dc1d8ed..07fa69c732067 100644 --- a/core/routes.php +++ b/core/routes.php @@ -103,6 +103,9 @@ ['root' => '/core', 'name' => 'AppPassword#getAppPassword', 'url' => '/getapppassword', 'verb' => 'GET'], ['root' => '/core', 'name' => 'AppPassword#rotateAppPassword', 'url' => '/apppassword/rotate', 'verb' => 'POST'], ['root' => '/core', 'name' => 'AppPassword#deleteAppPassword', 'url' => '/apppassword', 'verb' => 'DELETE'], + ['root' => '/core', 'name' => 'GenericAvatar#getAvatar', 'url' => '/avatar/{avatarType}/{avatarId}/{size}', 'verb' => 'GET'], + ['root' => '/core', 'name' => 'GenericAvatar#setAvatar', 'url' => '/avatar/{avatarType}/{avatarId}', 'verb' => 'POST'], + ['root' => '/core', 'name' => 'GenericAvatar#deleteAvatar', 'url' => '/avatar/{avatarType}/{avatarId}', 'verb' => 'DELETE'], ['root' => '/collaboration', 'name' => 'CollaborationResources#searchCollections', 'url' => '/resources/collections/search/{filter}', 'verb' => 'GET'], ['root' => '/collaboration', 'name' => 'CollaborationResources#listCollection', 'url' => '/resources/collections/{collectionId}', 'verb' => 'GET'], From 2522439afeed8ae68cde389c2a5cfb80389d6621 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Calvi=C3=B1o=20S=C3=A1nchez?= Date: Mon, 7 Dec 2020 04:39:48 +0100 Subject: [PATCH 08/53] Make possible to set body in OCS requests in integration tests MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit "sendingToWith" is used to test OCS endpoints. Now it is possible to specify the body (or, rather, additional contents beside the cookies and the request token) for those requests, as it will be needed for example to upload an avatar. Signed-off-by: Daniel Calviño Sánchez --- build/integration/features/bootstrap/BasicStructure.php | 2 ++ 1 file changed, 2 insertions(+) diff --git a/build/integration/features/bootstrap/BasicStructure.php b/build/integration/features/bootstrap/BasicStructure.php index ac5530be5a5ea..b47840b75cadb 100644 --- a/build/integration/features/bootstrap/BasicStructure.php +++ b/build/integration/features/bootstrap/BasicStructure.php @@ -188,6 +188,8 @@ public function sendingToWith($verb, $url, $body) { if ($body instanceof TableNode) { $fd = $body->getRowsHash(); $options['form_params'] = $fd; + } elseif ($body) { + $options = array_merge($options, $body); } // TODO: Fix this hack! From b190431db54cea70022638721cfb9444cac45820 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Calvi=C3=B1o=20S=C3=A1nchez?= Date: Mon, 7 Dec 2020 04:42:50 +0100 Subject: [PATCH 09/53] Add integration tests for getting and setting generic avatars MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Daniel Calviño Sánchez --- build/integration/features/avatar.feature | 219 ++++++++++++++++++ .../integration/features/bootstrap/Avatar.php | 105 +++++++++ 2 files changed, 324 insertions(+) diff --git a/build/integration/features/avatar.feature b/build/integration/features/avatar.feature index f7926615c011f..3d2d5b9c1af14 100644 --- a/build/integration/features/avatar.feature +++ b/build/integration/features/avatar.feature @@ -2,6 +2,225 @@ Feature: avatar Background: Given user "user0" exists + Given user "user1" exists + + Scenario: get default generic user avatar + When user "user0" gets avatar for type "user" and id "user0" + Then The following headers should be set + | Content-Type | image/png | + | X-NC-IsCustomAvatar | 0 | + And last avatar is a square of size 128 + And last avatar is not a single color + + Scenario: get default generic user avatar as an anonymous user + When user "anonymous" gets avatar for type "user" and id "user0" + Then The following headers should be set + | Content-Type | image/png | + | X-NC-IsCustomAvatar | 0 | + And last avatar is a square of size 128 + And last avatar is not a single color + + Scenario: get default generic guest avatar + When user "user0" gets avatar for type "guest" and id "guest0" + Then The following headers should be set + | Content-Type | image/png | + | X-NC-IsCustomAvatar | 0 | + And last avatar is a square of size 128 + And last avatar is not a single color + + Scenario: get default generic guest avatar as an anonymous user + When user "anonymous" gets avatar for type "guest" and id "guest0" + Then The following headers should be set + | Content-Type | image/png | + | X-NC-IsCustomAvatar | 0 | + And last avatar is a square of size 128 + And last avatar is not a single color + + Scenario: get generic unknown avatar + When user "user0" gets avatar for type "unknown" and id "user0" with size "128" with 404 + + + + Scenario: set generic user avatar + When user "user0" sets avatar for type "user" and id "user0" from file "data/green-square-256.png" + Then user "user0" gets avatar for type "user" and id "user0" with size "256" + And The following headers should be set + | Content-Type | image/png | + | X-NC-IsCustomAvatar | 1 | + And last avatar is a square of size 256 + And last avatar is a single "#00FF00" color + And user "anonymous" gets avatar for type "user" and id "user0" with size "256" + And The following headers should be set + | Content-Type | image/png | + | X-NC-IsCustomAvatar | 1 | + And last avatar is a square of size 256 + And last avatar is a single "#00FF00" color + + Scenario: set generic user avatar as another user + When user "user1" sets avatar for type "user" and id "user0" from file "data/green-square-256.png" with "404" + Then user "user0" gets avatar for type "user" and id "user0" + And The following headers should be set + | Content-Type | image/png | + | X-NC-IsCustomAvatar | 0 | + And last avatar is a square of size 128 + And last avatar is not a single color + + Scenario: set generic user avatar as an anonymous user + When user "anonymous" sets avatar for type "user" and id "user0" from file "data/green-square-256.png" with "404" + Then user "user0" gets avatar for type "user" and id "user0" + And The following headers should be set + | Content-Type | image/png | + | X-NC-IsCustomAvatar | 0 | + And last avatar is a square of size 128 + And last avatar is not a single color + + Scenario: set non squared image as generic user avatar + When user "user0" sets avatar for type "user" and id "user0" from file "data/coloured-pattern.png" with "400" + Then user "user0" gets avatar for type "user" and id "user0" + And The following headers should be set + | Content-Type | image/png | + | X-NC-IsCustomAvatar | 0 | + And last avatar is a square of size 128 + And last avatar is not a single color + + Scenario: set not an image as generic user avatar + When user "user0" sets avatar for type "user" and id "user0" from file "data/textfile.txt" with "400" + Then user "user0" gets avatar for type "user" and id "user0" + And The following headers should be set + | Content-Type | image/png | + | X-NC-IsCustomAvatar | 0 | + And last avatar is a square of size 128 + And last avatar is not a single color + + Scenario: set generic guest avatar + # Silently ignored + When user "user0" sets avatar for type "guest" and id "guest0" from file "data/green-square-256.png" + Then user "user0" gets avatar for type "guest" and id "guest0" + And The following headers should be set + | Content-Type | image/png | + | X-NC-IsCustomAvatar | 0 | + And last avatar is a square of size 128 + And last avatar is not a single color + + + + Scenario: delete generic user avatar + Given user "user0" sets avatar for type "user" and id "user0" from file "data/green-square-256.png" + And user "user0" gets avatar for type "user" and id "user0" with size "256" + And The following headers should be set + | Content-Type | image/png | + | X-NC-IsCustomAvatar | 1 | + And last avatar is a square of size 256 + And last avatar is a single "#00FF00" color + And user "anonymous" gets avatar for type "user" and id "user0" with size "256" + And The following headers should be set + | Content-Type | image/png | + | X-NC-IsCustomAvatar | 1 | + And last avatar is a square of size 256 + And last avatar is a single "#00FF00" color + When user "user0" deletes avatar for type "user" and id "user0" + Then user "user0" gets avatar for type "user" and id "user0" + And The following headers should be set + | Content-Type | image/png | + | X-NC-IsCustomAvatar | 0 | + And last avatar is a square of size 128 + And last avatar is not a single color + And user "anonymous" gets avatar for type "user" and id "user0" + And The following headers should be set + | Content-Type | image/png | + | X-NC-IsCustomAvatar | 0 | + And last avatar is a square of size 128 + And last avatar is not a single color + + Scenario: delete generic user avatar as another user + Given user "user0" sets avatar for type "user" and id "user0" from file "data/green-square-256.png" + And user "user0" gets avatar for type "user" and id "user0" with size "256" + And The following headers should be set + | Content-Type | image/png | + | X-NC-IsCustomAvatar | 1 | + And last avatar is a square of size 256 + And last avatar is a single "#00FF00" color + When user "user1" deletes avatar for type "user" and id "user0" with "404" + Then user "user0" gets avatar for type "user" and id "user0" with size "256" + And The following headers should be set + | Content-Type | image/png | + | X-NC-IsCustomAvatar | 1 | + And last avatar is a square of size 128 + And last avatar is a single "#00FF00" color + + Scenario: delete generic user avatar as an anonymous user + Given user "user0" sets avatar for type "user" and id "user0" from file "data/green-square-256.png" + And user "user0" gets avatar for type "user" and id "user0" with size "256" + And The following headers should be set + | Content-Type | image/png | + | X-NC-IsCustomAvatar | 1 | + And last avatar is a square of size 256 + And last avatar is a single "#00FF00" color + When user "anonymous" deletes avatar for type "user" and id "user0" with "404" + Then user "user0" gets avatar for type "user" and id "user0" with size "256" + And The following headers should be set + | Content-Type | image/png | + | X-NC-IsCustomAvatar | 1 | + And last avatar is a square of size 128 + And last avatar is a single "#00FF00" color + + Scenario: delete generic guest avatar + # Silently ignored + When user "user0" deletes avatar for type "guest" and id "guest0" + Then user "user0" gets avatar for type "guest" and id "guest0" + And The following headers should be set + | Content-Type | image/png | + | X-NC-IsCustomAvatar | 0 | + And last avatar is a square of size 128 + And last avatar is not a single color + + + + Scenario: get generic user avatar with a larger size than the original one + Given user "user0" sets avatar for type "user" and id "user0" from file "data/green-square-256.png" + When user "user0" gets avatar for type "user" and id "user0" with size "512" + 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 "#00FF00" color + + Scenario: get generic user avatar with a smaller size than the original one + Given user "user0" sets avatar for type "user" and id "user0" from file "data/green-square-256.png" + When user "user0" gets avatar for type "user" and id "user0" with size "128" + Then The following headers should be set + | Content-Type | image/png | + | X-NC-IsCustomAvatar | 1 | + And last avatar is a square of size 128 + And last avatar is a single "#00FF00" color + + + + Scenario: get user avatar after setting generic user avatar + Given user "user0" sets avatar for type "user" and id "user0" from file "data/green-square-256.png" + When user "user0" gets avatar for user "user0" with size "256" + And The following headers should be set + | Content-Type | image/png | + | X-NC-IsCustomAvatar | 1 | + And last avatar is a square of size 256 + And last avatar is a single "#00FF00" color + + Scenario: get generic user avatar after setting user avatar + Given Logging in using web as "user0" + And logged in user posts temporary avatar from file "data/coloured-pattern.png" + And logged in user crops temporary avatar + | x | 384 | + | y | 256 | + | w | 128 | + | h | 128 | + When user "user0" gets avatar for type "user" and id "user0" + Then The following headers should be set + | Content-Type | image/png | + | X-NC-IsCustomAvatar | 1 | + And last avatar is a square of size 128 + And last avatar is a single "#FF0000" color + + Scenario: get default user avatar When user "user0" gets avatar for user "user0" diff --git a/build/integration/features/bootstrap/Avatar.php b/build/integration/features/bootstrap/Avatar.php index 388715340c6e5..eda591a1e5408 100644 --- a/build/integration/features/bootstrap/Avatar.php +++ b/build/integration/features/bootstrap/Avatar.php @@ -45,6 +45,111 @@ private function getLastAvatar() { } $body->close(); } + /** + * @When user :user gets avatar for type :type and id :id + * + * @param string $user + * @param string $type + * @param string $id + */ + public function userGetsAvatarForTypeAndId(string $user, string $type, string $id) { + $this->userGetsAvatarForTypeAndIdWithSize($user, $type, $id, '128'); + } + + /** + * @When user :user gets avatar for type :type and id :id with size :size + * + * @param string $user + * @param string $type + * @param string $id + * @param string $size + */ + public function userGetsAvatarForTypeAndIdWithSize(string $user, string $type, string $id, string $size) { + $this->userGetsAvatarForTypeAndIdWithSizeWith($user, $type, $id, $size, '200'); + } + + /** + * @When user :user gets avatar for type :type and id :id with size :size with :statusCode + * + * @param string $user + * @param string $type + * @param string $id + * @param string $size + * @param string $statusCode + */ + public function userGetsAvatarForTypeAndIdWithSizeWith(string $user, string $type, string $id, string $size, string $statusCode) { + $this->asAn($user); + $this->sendingToWith('GET', '/core/avatar/' . $type . '/' . $id . '/' . $size, null); + $this->theHTTPStatusCodeShouldBe($statusCode); + + if ($statusCode !== '200') { + return; + } + + $this->getLastAvatar(); + } + + /** + * @When user :user sets avatar for type :type and id :id from file :source + * + * @param string $user + * @param string $type + * @param string $id + * @param string $source + */ + public function userSetsAvatarForTypeAndIdFromFile(string $user, string $type, string $id, string $source) { + $this->userSetsAvatarForTypeAndIdFromFileWith($user, $type, $id, $source, '200'); + } + + /** + * @When user :user sets avatar for type :type and id :id from file :source with :statusCode + * + * @param string $user + * @param string $type + * @param string $id + * @param string $source + * @param string $statusCode + */ + public function userSetsAvatarForTypeAndIdFromFileWith(string $user, string $type, string $id, string $source, string $statusCode) { + $file = \GuzzleHttp\Psr7\stream_for(fopen($source, 'r')); + + $this->asAn($user); + $this->sendingToWith('POST', '/core/avatar/' . $type . '/' . $id, + [ + 'multipart' => [ + [ + 'name' => 'files[]', + 'contents' => $file + ] + ] + ]); + $this->theHTTPStatusCodeShouldBe($statusCode); + } + + /** + * @When user :user deletes avatar for type :type and id :id + * + * @param string $user + * @param string $type + * @param string $id + */ + public function userDeletesAvatarForTypeAndId(string $user, string $type, string $id) { + $this->userDeletesAvatarForTypeAndIdWith($user, $type, $id, '200'); + } + + /** + * @When user :user deletes avatar for type :type and id :id with :statusCode + * + * @param string $user + * @param string $type + * @param string $id + * @param string $statusCode + */ + public function userDeletesAvatarForTypeAndIdWith(string $user, string $type, string $id, string $statusCode) { + $this->asAn($user); + $this->sendingToWith('DELETE', '/core/avatar/' . $type . '/' . $id, null); + $this->theHTTPStatusCodeShouldBe($statusCode); + } /** * @When user :user gets avatar for user :userAvatar From 63cbd7bb0fa2d65aabe4b1d151edc9deb0a421e5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Calvi=C3=B1o=20S=C3=A1nchez?= Date: Wed, 9 Dec 2020 11:37:42 +0100 Subject: [PATCH 10/53] fixup! Add OCS endpoint for avatars MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Daniel Calviño Sánchez --- core/Controller/GenericAvatarController.php | 33 +++++++++++---------- 1 file changed, 17 insertions(+), 16 deletions(-) diff --git a/core/Controller/GenericAvatarController.php b/core/Controller/GenericAvatarController.php index dedf6e3f7568f..f71b28e0edf2e 100644 --- a/core/Controller/GenericAvatarController.php +++ b/core/Controller/GenericAvatarController.php @@ -28,8 +28,9 @@ use OCP\AppFramework\OCSController; use OCP\AppFramework\Http; +use OCP\AppFramework\Http\DataResponse; use OCP\AppFramework\Http\FileDisplayResponse; -use OCP\AppFramework\Http\JSONResponse; +use OCP\AppFramework\Http\Response; use OCP\IAvatarManager; use OCP\IL10N; use OCP\ILogger; @@ -64,9 +65,9 @@ public function __construct($appName, * @param string $avatarType * @param string $avatarId * @param int $size - * @return JSONResponse|FileDisplayResponse + * @return DataResponse|FileDisplayResponse */ - public function getAvatar(string $avatarType, string $avatarId, int $size) { + public function getAvatar(string $avatarType, string $avatarId, int $size): Response { // min/max size if ($size > 2048) { $size = 2048; @@ -86,7 +87,7 @@ public function getAvatar(string $avatarType, string $avatarId, int $size) { ] ); } catch (\Exception $e) { - return new JSONResponse([], Http::STATUS_NOT_FOUND); + return new DataResponse([], Http::STATUS_NOT_FOUND); } return $response; @@ -96,13 +97,13 @@ public function getAvatar(string $avatarType, string $avatarId, int $size) { * @PublicPage * * @param string $path - * @return JSONResponse + * @return DataResponse */ - public function setAvatar(string $avatarType, string $avatarId) { + public function setAvatar(string $avatarType, string $avatarId): DataResponse { $files = $this->request->getUploadedFile('files'); if (is_null($files)) { - return new JSONResponse( + return new DataResponse( ['data' => ['message' => $this->l->t('No file provided')]], Http::STATUS_BAD_REQUEST ); @@ -113,14 +114,14 @@ public function setAvatar(string $avatarType, string $avatarId) { !is_uploaded_file($files['tmp_name'][0]) || \OC\Files\Filesystem::isFileBlacklisted($files['tmp_name'][0]) ) { - return new JSONResponse( + return new DataResponse( ['data' => ['message' => $this->l->t('Invalid file provided')]], Http::STATUS_BAD_REQUEST ); } if ($files['size'][0] > 20 * 1024 * 1024) { - return new JSONResponse( + return new DataResponse( ['data' => ['message' => $this->l->t('File is too big')]], Http::STATUS_BAD_REQUEST ); @@ -135,17 +136,17 @@ public function setAvatar(string $avatarType, string $avatarId) { try { $avatar = $this->avatarManager->getGenericAvatar($avatarType, $avatarId); $avatar->set($image); - return new JSONResponse( + return new DataResponse( ['status' => 'success'] ); } catch (\OC\NotSquareException $e) { - return new JSONResponse( + return new DataResponse( ['data' => ['message' => $this->l->t('Crop is not square')]], Http::STATUS_BAD_REQUEST ); } catch (\Exception $e) { $this->logger->logException($e, ['app' => 'core']); - return new JSONResponse( + return new DataResponse( ['data' => ['message' => $this->l->t('An error occurred. Please contact your admin.')]], Http::STATUS_BAD_REQUEST ); @@ -155,16 +156,16 @@ public function setAvatar(string $avatarType, string $avatarId) { /** * @PublicPage * - * @return JSONResponse + * @return DataResponse */ - public function deleteAvatar(string $avatarType, string $avatarId) { + public function deleteAvatar(string $avatarType, string $avatarId): DataResponse { try { $avatar = $this->avatarManager->getGenericAvatar($avatarType, $avatarId); $avatar->remove(); - return new JSONResponse(); + return new DataResponse(); } catch (\Exception $e) { $this->logger->logException($e, ['app' => 'core']); - return new JSONResponse( + return new DataResponse( ['data' => ['message' => $this->l->t('An error occurred. Please contact your admin.')]], Http::STATUS_BAD_REQUEST ); From a31a7fd9c5e85e2cb823bc44d44dcfc681d3f83c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Calvi=C3=B1o=20S=C3=A1nchez?= Date: Thu, 10 Dec 2020 02:26:13 +0100 Subject: [PATCH 11/53] fixup! Add OCS endpoint for avatars MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Daniel Calviño Sánchez --- lib/composer/composer/autoload_classmap.php | 1 + lib/composer/composer/autoload_static.php | 1 + 2 files changed, 2 insertions(+) diff --git a/lib/composer/composer/autoload_classmap.php b/lib/composer/composer/autoload_classmap.php index 0400e68109056..db2153b26703b 100644 --- a/lib/composer/composer/autoload_classmap.php +++ b/lib/composer/composer/autoload_classmap.php @@ -875,6 +875,7 @@ 'OC\\Core\\Controller\\CollaborationResourcesController' => $baseDir . '/core/Controller/CollaborationResourcesController.php', 'OC\\Core\\Controller\\ContactsMenuController' => $baseDir . '/core/Controller/ContactsMenuController.php', 'OC\\Core\\Controller\\CssController' => $baseDir . '/core/Controller/CssController.php', + 'OC\\Core\\Controller\\GenericAvatarController' => $baseDir . '/core/Controller/GenericAvatarController.php', 'OC\\Core\\Controller\\GuestAvatarController' => $baseDir . '/core/Controller/GuestAvatarController.php', 'OC\\Core\\Controller\\JsController' => $baseDir . '/core/Controller/JsController.php', 'OC\\Core\\Controller\\LoginController' => $baseDir . '/core/Controller/LoginController.php', diff --git a/lib/composer/composer/autoload_static.php b/lib/composer/composer/autoload_static.php index b9b4f2f307b8c..c1c44693ae7c6 100644 --- a/lib/composer/composer/autoload_static.php +++ b/lib/composer/composer/autoload_static.php @@ -904,6 +904,7 @@ class ComposerStaticInit53792487c5a8370acc0b06b1a864ff4c 'OC\\Core\\Controller\\CollaborationResourcesController' => __DIR__ . '/../../..' . '/core/Controller/CollaborationResourcesController.php', 'OC\\Core\\Controller\\ContactsMenuController' => __DIR__ . '/../../..' . '/core/Controller/ContactsMenuController.php', 'OC\\Core\\Controller\\CssController' => __DIR__ . '/../../..' . '/core/Controller/CssController.php', + 'OC\\Core\\Controller\\GenericAvatarController' => __DIR__ . '/../../..' . '/core/Controller/GenericAvatarController.php', 'OC\\Core\\Controller\\GuestAvatarController' => __DIR__ . '/../../..' . '/core/Controller/GuestAvatarController.php', 'OC\\Core\\Controller\\JsController' => __DIR__ . '/../../..' . '/core/Controller/JsController.php', 'OC\\Core\\Controller\\LoginController' => __DIR__ . '/../../..' . '/core/Controller/LoginController.php', From 92c9fc0a6acf61b35ec1c41991b3f695cdf82f65 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Calvi=C3=B1o=20S=C3=A1nchez?= Date: Thu, 10 Dec 2020 03:13:07 +0100 Subject: [PATCH 12/53] Make possible to define custom avatar types MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Daniel Calviño Sánchez --- lib/composer/composer/autoload_classmap.php | 1 + lib/composer/composer/autoload_static.php | 1 + lib/private/Avatar/AvatarManager.php | 38 ++++++++++++++++- lib/private/Server.php | 3 +- lib/public/IAvatarManager.php | 9 ++++ lib/public/IAvatarProvider.php | 46 +++++++++++++++++++++ 6 files changed, 95 insertions(+), 3 deletions(-) create mode 100644 lib/public/IAvatarProvider.php diff --git a/lib/composer/composer/autoload_classmap.php b/lib/composer/composer/autoload_classmap.php index db2153b26703b..1d03137148f97 100644 --- a/lib/composer/composer/autoload_classmap.php +++ b/lib/composer/composer/autoload_classmap.php @@ -363,6 +363,7 @@ 'OCP\\IAppConfig' => $baseDir . '/lib/public/IAppConfig.php', 'OCP\\IAvatar' => $baseDir . '/lib/public/IAvatar.php', 'OCP\\IAvatarManager' => $baseDir . '/lib/public/IAvatarManager.php', + 'OCP\\IAvatarProvider' => $baseDir . '/lib/public/IAvatarProvider.php', 'OCP\\ICache' => $baseDir . '/lib/public/ICache.php', 'OCP\\ICacheFactory' => $baseDir . '/lib/public/ICacheFactory.php', 'OCP\\ICertificate' => $baseDir . '/lib/public/ICertificate.php', diff --git a/lib/composer/composer/autoload_static.php b/lib/composer/composer/autoload_static.php index c1c44693ae7c6..cc581a600837b 100644 --- a/lib/composer/composer/autoload_static.php +++ b/lib/composer/composer/autoload_static.php @@ -392,6 +392,7 @@ class ComposerStaticInit53792487c5a8370acc0b06b1a864ff4c 'OCP\\IAppConfig' => __DIR__ . '/../../..' . '/lib/public/IAppConfig.php', 'OCP\\IAvatar' => __DIR__ . '/../../..' . '/lib/public/IAvatar.php', 'OCP\\IAvatarManager' => __DIR__ . '/../../..' . '/lib/public/IAvatarManager.php', + 'OCP\\IAvatarProvider' => __DIR__ . '/../../..' . '/lib/public/IAvatarProvider.php', 'OCP\\ICache' => __DIR__ . '/../../..' . '/lib/public/ICache.php', 'OCP\\ICacheFactory' => __DIR__ . '/../../..' . '/lib/public/ICacheFactory.php', 'OCP\\ICertificate' => __DIR__ . '/../../..' . '/lib/public/ICertificate.php', diff --git a/lib/private/Avatar/AvatarManager.php b/lib/private/Avatar/AvatarManager.php index 2f10a22b026a9..92b0243814d6c 100644 --- a/lib/private/Avatar/AvatarManager.php +++ b/lib/private/Avatar/AvatarManager.php @@ -41,9 +41,11 @@ use OCP\Files\NotPermittedException; use OCP\IAvatar; use OCP\IAvatarManager; +use OCP\IAvatarProvider; use OCP\IConfig; use OCP\IL10N; use OCP\ILogger; +use OCP\IServerContainer; /** * This class implements methods to access Avatar functionality @@ -65,6 +67,15 @@ class AvatarManager implements IAvatarManager { /** @var IConfig */ private $config; + /** @var IServerContainer */ + private $serverContainer; + + /** @var string[] */ + private $providerClasses; + + /** @var IAvatarProvider[] */ + private $providers; + /** * AvatarManager constructor. * @@ -73,18 +84,33 @@ class AvatarManager implements IAvatarManager { * @param IL10N $l * @param ILogger $logger * @param IConfig $config + * @param IServerContainer $serverContainer */ public function __construct( Manager $userManager, IAppData $appData, IL10N $l, ILogger $logger, - IConfig $config) { + IConfig $config, + IServerContainer $serverContainer) { $this->userManager = $userManager; $this->appData = $appData; $this->l = $l; $this->logger = $logger; $this->config = $config; + $this->serverContainer = $serverContainer; + $this->providerClasses = []; + $this->providers = []; + } + + /** + * Registers a provider for the given avatar type. + * + * @param string $type avatar type to associate with the provider + * @param string $class class name, must implement OCP\IAvatarProvider + */ + public function registerAvatarProvider(string $type, string $class): void { + $this->providerClasses[$type] = $class; } /** @@ -162,6 +188,14 @@ public function getGenericAvatar(string $type, string $id): IAvatar { return $this->getGuestAvatar($id); } - throw new \InvalidArgumentException('Unknown avatar type: ' . $type); + if (!array_key_exists($type, $this->providerClasses)) { + throw new \InvalidArgumentException('Unknown avatar type: ' . $type); + } + + if (!array_key_exists($type, $this->providers)) { + $this->providers[$type] = $this->serverContainer->get($this->providerClasses[$type]); + } + + return $this->providers[$type]->getAvatar($id); } } diff --git a/lib/private/Server.php b/lib/private/Server.php index b426c9c454dc6..ed43312da246a 100644 --- a/lib/private/Server.php +++ b/lib/private/Server.php @@ -720,7 +720,8 @@ public function __construct($webRoot, \OC\Config $config) { $c->getAppDataDir('avatar'), $c->getL10N('lib'), $c->get(ILogger::class), - $c->get(\OCP\IConfig::class) + $c->get(\OCP\IConfig::class), + $c->get(IServerContainer::class) ); }); $this->registerAlias(IAvatarManager::class, AvatarManager::class); diff --git a/lib/public/IAvatarManager.php b/lib/public/IAvatarManager.php index 479d4dd1ee52e..1babfe656e261 100644 --- a/lib/public/IAvatarManager.php +++ b/lib/public/IAvatarManager.php @@ -37,6 +37,15 @@ interface IAvatarManager { + /** + * Registers a provider for the given avatar type. + * + * @param string $type avatar type to associate with the provider + * @param string $class class name, must implement OCP\IAvatarProvider + * @since 21.0.0 + */ + public function registerAvatarProvider(string $type, string $class): void; + /** * return a user specific instance of \OCP\IAvatar * @see IAvatar diff --git a/lib/public/IAvatarProvider.php b/lib/public/IAvatarProvider.php new file mode 100644 index 0000000000000..3bdee79502fdf --- /dev/null +++ b/lib/public/IAvatarProvider.php @@ -0,0 +1,46 @@ + + * + * @license GNU AGPL version 3 or any later version + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU Affero General Public License as + * published by the Free Software Foundation, either version 3 of the + * License, or (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Affero General Public License for more details. + * + * You should have received a copy of the GNU Affero General Public License + * along with this program. If not, see . + * + */ + +namespace OCP; + +/** + * This class acts as a factory for avatar instances. + * + * @since 21.0.0 + */ +interface IAvatarProvider { + + /** + * Returns an IAvatar instance for the given id. + * + * @param string $id the identifier of the avatar. + * @return IAvatar the avatar instance. + * @throws \Exception if an error occurred while getting the avatar. + * @since 21.0.0 + */ + public function getAvatar(string $id): IAvatar; + +} From a83660837ca8f8e3385cfeeb129fe90f4774d0ff Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Calvi=C3=B1o=20S=C3=A1nchez?= Date: Thu, 10 Dec 2020 04:28:40 +0100 Subject: [PATCH 13/53] fixup! Add OCS endpoint for avatars MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Daniel Calviño Sánchez --- core/Controller/GenericAvatarController.php | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/core/Controller/GenericAvatarController.php b/core/Controller/GenericAvatarController.php index f71b28e0edf2e..b2ae5c2933848 100644 --- a/core/Controller/GenericAvatarController.php +++ b/core/Controller/GenericAvatarController.php @@ -33,8 +33,8 @@ use OCP\AppFramework\Http\Response; use OCP\IAvatarManager; use OCP\IL10N; -use OCP\ILogger; use OCP\IRequest; +use Psr\Log\LoggerInterface; class GenericAvatarController extends OCSController { @@ -44,14 +44,14 @@ class GenericAvatarController extends OCSController { /** @var IL10N */ protected $l; - /** @var ILogger */ + /** @var LoggerInterface */ protected $logger; public function __construct($appName, IRequest $request, IAvatarManager $avatarManager, IL10N $l10n, - ILogger $logger) { + LoggerInterface $logger) { parent::__construct($appName, $request); $this->avatarManager = $avatarManager; @@ -145,7 +145,7 @@ public function setAvatar(string $avatarType, string $avatarId): DataResponse { Http::STATUS_BAD_REQUEST ); } catch (\Exception $e) { - $this->logger->logException($e, ['app' => 'core']); + $this->logger->error('Error when setting avatar', ['app' => 'core', 'exception' => $e]); return new DataResponse( ['data' => ['message' => $this->l->t('An error occurred. Please contact your admin.')]], Http::STATUS_BAD_REQUEST @@ -164,7 +164,7 @@ public function deleteAvatar(string $avatarType, string $avatarId): DataResponse $avatar->remove(); return new DataResponse(); } catch (\Exception $e) { - $this->logger->logException($e, ['app' => 'core']); + $this->logger->error('Error when deleting avatar', ['app' => 'core', 'exception' => $e]); return new DataResponse( ['data' => ['message' => $this->l->t('An error occurred. Please contact your admin.')]], Http::STATUS_BAD_REQUEST From 95e01779e468bcfecfd90bd67402f0481351b58a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Calvi=C3=B1o=20S=C3=A1nchez?= Date: Thu, 10 Dec 2020 05:06:10 +0100 Subject: [PATCH 14/53] Move registration of IAvatarProviders to IRegistrationContext MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Daniel Calviño Sánchez --- .../Bootstrap/RegistrationContext.php | 22 ++++++++++++ lib/private/Avatar/AvatarManager.php | 34 ++++++++++--------- lib/private/Server.php | 4 ++- .../Bootstrap/IRegistrationContext.php | 14 ++++++++ lib/public/IAvatarManager.php | 9 ----- 5 files changed, 57 insertions(+), 26 deletions(-) diff --git a/lib/private/AppFramework/Bootstrap/RegistrationContext.php b/lib/private/AppFramework/Bootstrap/RegistrationContext.php index 12fca23c51fd8..272a62390c315 100644 --- a/lib/private/AppFramework/Bootstrap/RegistrationContext.php +++ b/lib/private/AppFramework/Bootstrap/RegistrationContext.php @@ -72,6 +72,9 @@ class RegistrationContext { /** @var array[] */ private $initialStates = []; + /** @var array[] */ + private $avatarProviders = []; + /** @var ILogger */ private $logger; @@ -174,6 +177,14 @@ public function registerInitialStateProvider(string $class): void { $class ); } + + public function registerAvatarProvider(string $type, string $class): void { + $this->context->registerAvatarProvider( + $this->appId, + $type, + $class + ); + } }; } @@ -260,6 +271,10 @@ public function registerInitialState(string $appId, string $class): void { ]; } + public function registerAvatarProvider(string $appId, string $type, string $class): void { + $this->avatarProviders[$type] = $class; + } + /** * @param App[] $apps */ @@ -437,4 +452,11 @@ public function getAlternativeLogins(): array { public function getInitialStates(): array { return $this->initialStates; } + + /** + * @erturn array[] + */ + public function getAvatarProviders(): array { + return $this->avatarProviders; + } } diff --git a/lib/private/Avatar/AvatarManager.php b/lib/private/Avatar/AvatarManager.php index 92b0243814d6c..3b3d709f53dff 100644 --- a/lib/private/Avatar/AvatarManager.php +++ b/lib/private/Avatar/AvatarManager.php @@ -34,6 +34,7 @@ namespace OC\Avatar; +use OC\AppFramework\Bootstrap\Coordinator; use OC\User\Manager; use OC\User\NoUserException; use OCP\Files\IAppData; @@ -70,8 +71,8 @@ class AvatarManager implements IAvatarManager { /** @var IServerContainer */ private $serverContainer; - /** @var string[] */ - private $providerClasses; + /** @var Coordinator */ + private $bootstrapCoordinator; /** @var IAvatarProvider[] */ private $providers; @@ -85,6 +86,7 @@ class AvatarManager implements IAvatarManager { * @param ILogger $logger * @param IConfig $config * @param IServerContainer $serverContainer + * @param Coordinator $bootstrapCoordinator */ public function __construct( Manager $userManager, @@ -92,27 +94,18 @@ public function __construct( IL10N $l, ILogger $logger, IConfig $config, - IServerContainer $serverContainer) { + IServerContainer $serverContainer, + Coordinator $bootstrapCoordinator) { $this->userManager = $userManager; $this->appData = $appData; $this->l = $l; $this->logger = $logger; $this->config = $config; $this->serverContainer = $serverContainer; - $this->providerClasses = []; + $this->bootstrapCoordinator = $bootstrapCoordinator; $this->providers = []; } - /** - * Registers a provider for the given avatar type. - * - * @param string $type avatar type to associate with the provider - * @param string $class class name, must implement OCP\IAvatarProvider - */ - public function registerAvatarProvider(string $type, string $class): void { - $this->providerClasses[$type] = $class; - } - /** * return a user specific instance of \OCP\IAvatar * @see \OCP\IAvatar @@ -188,12 +181,21 @@ public function getGenericAvatar(string $type, string $id): IAvatar { return $this->getGuestAvatar($id); } - if (!array_key_exists($type, $this->providerClasses)) { + $context = $this->bootstrapCoordinator->getRegistrationContext(); + + if ($context === null) { + // Too early, nothing to do yet + throw new \InvalidArgumentException('Unknown avatar type: ' . $type); + } + + $providerClasses = $context->getAvatarProviders(); + + if (!array_key_exists($type, $providerClasses)) { throw new \InvalidArgumentException('Unknown avatar type: ' . $type); } if (!array_key_exists($type, $this->providers)) { - $this->providers[$type] = $this->serverContainer->get($this->providerClasses[$type]); + $this->providers[$type] = $this->serverContainer->get($providerClasses[$type]); } return $this->providers[$type]->getAvatar($id); diff --git a/lib/private/Server.php b/lib/private/Server.php index ed43312da246a..a23e19a5f3f56 100644 --- a/lib/private/Server.php +++ b/lib/private/Server.php @@ -60,6 +60,7 @@ use OC\App\AppStore\Bundles\BundleFetcher; use OC\App\AppStore\Fetcher\AppFetcher; use OC\App\AppStore\Fetcher\CategoryFetcher; +use OC\AppFramework\Bootstrap\Coordinator; use OC\AppFramework\Http\Request; use OC\AppFramework\Utility\TimeFactory; use OC\Authentication\Events\LoginFailed; @@ -721,7 +722,8 @@ public function __construct($webRoot, \OC\Config $config) { $c->getL10N('lib'), $c->get(ILogger::class), $c->get(\OCP\IConfig::class), - $c->get(IServerContainer::class) + $c->get(IServerContainer::class), + $c->get(Coordinator::class) ); }); $this->registerAlias(IAvatarManager::class, AvatarManager::class); diff --git a/lib/public/AppFramework/Bootstrap/IRegistrationContext.php b/lib/public/AppFramework/Bootstrap/IRegistrationContext.php index aaf5ef00bfaf7..328b5d248166c 100644 --- a/lib/public/AppFramework/Bootstrap/IRegistrationContext.php +++ b/lib/public/AppFramework/Bootstrap/IRegistrationContext.php @@ -180,4 +180,18 @@ public function registerAlternativeLogin(string $class): void; * @since 21.0.0 */ public function registerInitialStateProvider(string $class): void; + + /** + * Register an avatar provider + * + * It is allowed to register more than one provider per app. + * + * @param string $class + * @psalm-param class-string<\OCP\IAvatarProvider> $class + * + * @return void + * + * @since 21.0.0 + */ + public function registerAvatarProvider(string $type, string $class): void; } diff --git a/lib/public/IAvatarManager.php b/lib/public/IAvatarManager.php index 1babfe656e261..479d4dd1ee52e 100644 --- a/lib/public/IAvatarManager.php +++ b/lib/public/IAvatarManager.php @@ -37,15 +37,6 @@ interface IAvatarManager { - /** - * Registers a provider for the given avatar type. - * - * @param string $type avatar type to associate with the provider - * @param string $class class name, must implement OCP\IAvatarProvider - * @since 21.0.0 - */ - public function registerAvatarProvider(string $type, string $class): void; - /** * return a user specific instance of \OCP\IAvatar * @see IAvatar From f04a16a263cdd9f618debdac19e2d7739167f8d1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Calvi=C3=B1o=20S=C3=A1nchez?= Date: Mon, 14 Dec 2020 16:31:09 +0100 Subject: [PATCH 15/53] fixup! Make possible to define custom avatar types MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Daniel Calviño Sánchez --- lib/private/Avatar/AvatarManager.php | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/lib/private/Avatar/AvatarManager.php b/lib/private/Avatar/AvatarManager.php index 3b3d709f53dff..7db82af133d28 100644 --- a/lib/private/Avatar/AvatarManager.php +++ b/lib/private/Avatar/AvatarManager.php @@ -75,7 +75,7 @@ class AvatarManager implements IAvatarManager { private $bootstrapCoordinator; /** @var IAvatarProvider[] */ - private $providers; + private $providers = []; /** * AvatarManager constructor. @@ -103,7 +103,6 @@ public function __construct( $this->config = $config; $this->serverContainer = $serverContainer; $this->bootstrapCoordinator = $bootstrapCoordinator; - $this->providers = []; } /** From ebf242a1efe9377c9a78d372113ebc744ceba60a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Calvi=C3=B1o=20S=C3=A1nchez?= Date: Mon, 14 Dec 2020 16:31:59 +0100 Subject: [PATCH 16/53] fixup! Add OCS endpoint for avatars MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Daniel Calviño Sánchez --- core/Controller/GenericAvatarController.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/core/Controller/GenericAvatarController.php b/core/Controller/GenericAvatarController.php index b2ae5c2933848..4761729507be2 100644 --- a/core/Controller/GenericAvatarController.php +++ b/core/Controller/GenericAvatarController.php @@ -83,7 +83,7 @@ public function getAvatar(string $avatarType, string $avatarId, int $size): Resp Http::STATUS_OK, [ 'Content-Type' => $avatarFile->getMimeType(), - 'X-NC-IsCustomAvatar' => (int)$avatar->isCustomAvatar() + 'X-NC-IsCustomAvatar' => $avatar->isCustomAvatar() ? '1' : '0', ] ); } catch (\Exception $e) { From bf9169e0a366b726e6373f6720d4c7fbc2c34fe8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Calvi=C3=B1o=20S=C3=A1nchez?= Date: Mon, 14 Dec 2020 16:33:49 +0100 Subject: [PATCH 17/53] fixup! Add getter for generic avatars to IAvatarManager MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Daniel Calviño Sánchez --- lib/public/IAvatarManager.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/public/IAvatarManager.php b/lib/public/IAvatarManager.php index 479d4dd1ee52e..7468a25b419cf 100644 --- a/lib/public/IAvatarManager.php +++ b/lib/public/IAvatarManager.php @@ -58,7 +58,7 @@ public function getAvatar(string $user) : IAvatar; public function getGuestAvatar(string $name): IAvatar; /** - * Returns an avatar instance of the given type. + * Returns an avatar instance of the given type * * @param string $type the type of the avatar * @param string $id the identifier for the avatar of the given type From 1e133099e5dd080b59ccf4f4237e7de20865b01e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Calvi=C3=B1o=20S=C3=A1nchez?= Date: Mon, 14 Dec 2020 16:35:14 +0100 Subject: [PATCH 18/53] fixup! Make possible to define custom avatar types MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Daniel Calviño Sánchez --- lib/public/IAvatarProvider.php | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/lib/public/IAvatarProvider.php b/lib/public/IAvatarProvider.php index 3bdee79502fdf..ab722f99e677e 100644 --- a/lib/public/IAvatarProvider.php +++ b/lib/public/IAvatarProvider.php @@ -27,18 +27,18 @@ namespace OCP; /** - * This class acts as a factory for avatar instances. + * This class acts as a factory for avatar instances * * @since 21.0.0 */ interface IAvatarProvider { /** - * Returns an IAvatar instance for the given id. + * Returns an IAvatar instance for the given id * - * @param string $id the identifier of the avatar. - * @return IAvatar the avatar instance. - * @throws \Exception if an error occurred while getting the avatar. + * @param string $id the identifier of the avatar + * @return IAvatar the avatar instance + * @throws \Exception if an error occurred while getting the avatar * @since 21.0.0 */ public function getAvatar(string $id): IAvatar; From 6e43ce76bd807da9cf91782b795e459b6d901d34 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Calvi=C3=B1o=20S=C3=A1nchez?= Date: Mon, 14 Dec 2020 17:09:08 +0100 Subject: [PATCH 19/53] fixup! Add OCS endpoint for avatars MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Daniel Calviño Sánchez --- core/Controller/GenericAvatarController.php | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/core/Controller/GenericAvatarController.php b/core/Controller/GenericAvatarController.php index 4761729507be2..816b66a3b93b3 100644 --- a/core/Controller/GenericAvatarController.php +++ b/core/Controller/GenericAvatarController.php @@ -31,6 +31,7 @@ use OCP\AppFramework\Http\DataResponse; use OCP\AppFramework\Http\FileDisplayResponse; use OCP\AppFramework\Http\Response; +use OCP\Files\NotFoundException; use OCP\IAvatarManager; use OCP\IL10N; use OCP\IRequest; @@ -86,7 +87,12 @@ public function getAvatar(string $avatarType, string $avatarId, int $size): Resp 'X-NC-IsCustomAvatar' => $avatar->isCustomAvatar() ? '1' : '0', ] ); + } catch (\InvalidArgumentException $e) { + return new DataResponse([], Http::STATUS_NOT_FOUND); + } catch (NotFoundException $e) { + return new DataResponse([], Http::STATUS_NOT_FOUND); } catch (\Exception $e) { + $this->logger->error('Error when getting avatar', ['app' => 'core', 'exception' => $e]); return new DataResponse([], Http::STATUS_NOT_FOUND); } From 90ac35dae1abccc7916b9d64c5971202274b24b1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Calvi=C3=B1o=20S=C3=A1nchez?= Date: Mon, 14 Dec 2020 20:14:51 +0100 Subject: [PATCH 20/53] fixup! Make possible to define custom avatar types MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Daniel Calviño Sánchez --- tests/lib/Avatar/AvatarManagerTest.php | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/tests/lib/Avatar/AvatarManagerTest.php b/tests/lib/Avatar/AvatarManagerTest.php index 5a061cd10e9e4..4796df0fe4f55 100644 --- a/tests/lib/Avatar/AvatarManagerTest.php +++ b/tests/lib/Avatar/AvatarManagerTest.php @@ -32,6 +32,7 @@ use OCP\IConfig; use OCP\IL10N; use OCP\ILogger; +use OCP\IServerContainer; use OCP\IUser; /** @@ -65,7 +66,8 @@ protected function setUp(): void { $this->appData, $this->l10n, $this->logger, - $this->config + $this->config, + $this->createMock(IServerContainer::class) ); } From 5d0102ecc5441e086539fbf2651fd0bd9810c8dd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Calvi=C3=B1o=20S=C3=A1nchez?= Date: Mon, 14 Dec 2020 20:16:27 +0100 Subject: [PATCH 21/53] fixup! Move registration of IAvatarProviders to IRegistrationContext MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Daniel Calviño Sánchez --- tests/lib/Avatar/AvatarManagerTest.php | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/tests/lib/Avatar/AvatarManagerTest.php b/tests/lib/Avatar/AvatarManagerTest.php index 4796df0fe4f55..8d29b5040f15e 100644 --- a/tests/lib/Avatar/AvatarManagerTest.php +++ b/tests/lib/Avatar/AvatarManagerTest.php @@ -24,6 +24,7 @@ namespace Test\Avatar; +use OC\AppFramework\Bootstrap\Coordinator; use OC\Avatar\AvatarManager; use OC\Avatar\UserAvatar; use OC\User\Manager; @@ -67,7 +68,8 @@ protected function setUp(): void { $this->l10n, $this->logger, $this->config, - $this->createMock(IServerContainer::class) + $this->createMock(IServerContainer::class), + $this->createMock(Coordinator::class) ); } From ab910ecffebc5b50e8cdda9cd8c7647654f57196 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Calvi=C3=B1o=20S=C3=A1nchez?= Date: Mon, 14 Dec 2020 19:58:26 +0100 Subject: [PATCH 22/53] fixup! Move registration of IAvatarProviders to IRegistrationContext MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Daniel Calviño Sánchez --- lib/private/AppFramework/Bootstrap/RegistrationContext.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/private/AppFramework/Bootstrap/RegistrationContext.php b/lib/private/AppFramework/Bootstrap/RegistrationContext.php index 272a62390c315..d4c32dc7caa60 100644 --- a/lib/private/AppFramework/Bootstrap/RegistrationContext.php +++ b/lib/private/AppFramework/Bootstrap/RegistrationContext.php @@ -72,7 +72,7 @@ class RegistrationContext { /** @var array[] */ private $initialStates = []; - /** @var array[] */ + /** @var string[] */ private $avatarProviders = []; /** @var ILogger */ From a2c63ff4890943b6b999abe0ee19fbe391c806eb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Calvi=C3=B1o=20S=C3=A1nchez?= Date: Mon, 14 Dec 2020 20:11:01 +0100 Subject: [PATCH 23/53] fixup! Make possible to define custom avatar types MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Daniel Calviño Sánchez --- lib/public/IAvatarProvider.php | 1 - 1 file changed, 1 deletion(-) diff --git a/lib/public/IAvatarProvider.php b/lib/public/IAvatarProvider.php index ab722f99e677e..16ba85ab27352 100644 --- a/lib/public/IAvatarProvider.php +++ b/lib/public/IAvatarProvider.php @@ -42,5 +42,4 @@ interface IAvatarProvider { * @since 21.0.0 */ public function getAvatar(string $id): IAvatar; - } From 76e4bd23c04109375bc7619dc7e6663ebb186573 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Calvi=C3=B1o=20S=C3=A1nchez?= Date: Mon, 14 Dec 2020 18:49:09 +0100 Subject: [PATCH 24/53] Move deprecated ILogger to LoggerInterface in avatar private classes MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Daniel Calviño Sánchez --- lib/private/Avatar/Avatar.php | 8 ++++---- lib/private/Avatar/AvatarManager.php | 8 ++++---- lib/private/Avatar/GuestAvatar.php | 6 +++--- lib/private/Avatar/UserAvatar.php | 6 +++--- lib/private/Server.php | 2 +- tests/lib/Avatar/AvatarManagerTest.php | 6 +++--- tests/lib/Avatar/GuestAvatarTest.php | 6 +++--- tests/lib/Avatar/UserAvatarTest.php | 4 ++-- 8 files changed, 23 insertions(+), 23 deletions(-) diff --git a/lib/private/Avatar/Avatar.php b/lib/private/Avatar/Avatar.php index 02fc04eae3663..0877a97613eb5 100644 --- a/lib/private/Avatar/Avatar.php +++ b/lib/private/Avatar/Avatar.php @@ -43,14 +43,14 @@ use OC_Image; use OCP\Files\NotFoundException; use OCP\IAvatar; -use OCP\ILogger; +use Psr\Log\LoggerInterface; /** * This class gets and sets users avatars. */ abstract class Avatar implements IAvatar { - /** @var ILogger */ + /** @var LoggerInterface */ protected $logger; /** @@ -72,9 +72,9 @@ abstract class Avatar implements IAvatar { /** * The base avatar constructor. * - * @param ILogger $logger The logger + * @param LoggerInterface $logger The logger */ - public function __construct(ILogger $logger) { + public function __construct(LoggerInterface $logger) { $this->logger = $logger; } diff --git a/lib/private/Avatar/AvatarManager.php b/lib/private/Avatar/AvatarManager.php index 7db82af133d28..6af11a4ee4a1c 100644 --- a/lib/private/Avatar/AvatarManager.php +++ b/lib/private/Avatar/AvatarManager.php @@ -45,8 +45,8 @@ use OCP\IAvatarProvider; use OCP\IConfig; use OCP\IL10N; -use OCP\ILogger; use OCP\IServerContainer; +use Psr\Log\LoggerInterface; /** * This class implements methods to access Avatar functionality @@ -62,7 +62,7 @@ class AvatarManager implements IAvatarManager { /** @var IL10N */ private $l; - /** @var ILogger */ + /** @var LoggerInterface */ private $logger; /** @var IConfig */ @@ -83,7 +83,7 @@ class AvatarManager implements IAvatarManager { * @param Manager $userManager * @param IAppData $appData * @param IL10N $l - * @param ILogger $logger + * @param LoggerInterface $logger * @param IConfig $config * @param IServerContainer $serverContainer * @param Coordinator $bootstrapCoordinator @@ -92,7 +92,7 @@ public function __construct( Manager $userManager, IAppData $appData, IL10N $l, - ILogger $logger, + LoggerInterface $logger, IConfig $config, IServerContainer $serverContainer, Coordinator $bootstrapCoordinator) { diff --git a/lib/private/Avatar/GuestAvatar.php b/lib/private/Avatar/GuestAvatar.php index cc7e21b9fe647..c208353d72644 100644 --- a/lib/private/Avatar/GuestAvatar.php +++ b/lib/private/Avatar/GuestAvatar.php @@ -27,7 +27,7 @@ namespace OC\Avatar; use OCP\Files\SimpleFS\InMemoryFile; -use OCP\ILogger; +use Psr\Log\LoggerInterface; /** * This class represents a guest user's avatar. @@ -44,9 +44,9 @@ class GuestAvatar extends Avatar { * GuestAvatar constructor. * * @param string $userDisplayName The guest user display name - * @param ILogger $logger The logger + * @param LoggerInterface $logger The logger */ - public function __construct(string $userDisplayName, ILogger $logger) { + public function __construct(string $userDisplayName, LoggerInterface $logger) { parent::__construct($logger); $this->userDisplayName = $userDisplayName; } diff --git a/lib/private/Avatar/UserAvatar.php b/lib/private/Avatar/UserAvatar.php index f7ace429f7d98..421b2eb5cfd37 100644 --- a/lib/private/Avatar/UserAvatar.php +++ b/lib/private/Avatar/UserAvatar.php @@ -39,7 +39,7 @@ use OCP\IConfig; use OCP\IImage; use OCP\IL10N; -use OCP\ILogger; +use Psr\Log\LoggerInterface; /** * This class represents a registered user's avatar. @@ -64,13 +64,13 @@ class UserAvatar extends Avatar { * @param ISimpleFolder $folder The avatar files folder * @param IL10N $l The localization helper * @param User $user The user this class manages the avatar for - * @param ILogger $logger The logger + * @param LoggerInterface $logger The logger */ public function __construct( ISimpleFolder $folder, IL10N $l, $user, - ILogger $logger, + LoggerInterface $logger, IConfig $config) { parent::__construct($logger); $this->folder = $folder; diff --git a/lib/private/Server.php b/lib/private/Server.php index a23e19a5f3f56..61e42d4516120 100644 --- a/lib/private/Server.php +++ b/lib/private/Server.php @@ -720,7 +720,7 @@ public function __construct($webRoot, \OC\Config $config) { $c->get(\OC\User\Manager::class), $c->getAppDataDir('avatar'), $c->getL10N('lib'), - $c->get(ILogger::class), + $c->get(LoggerInterface::class), $c->get(\OCP\IConfig::class), $c->get(IServerContainer::class), $c->get(Coordinator::class) diff --git a/tests/lib/Avatar/AvatarManagerTest.php b/tests/lib/Avatar/AvatarManagerTest.php index 8d29b5040f15e..f8d4b33cc5a01 100644 --- a/tests/lib/Avatar/AvatarManagerTest.php +++ b/tests/lib/Avatar/AvatarManagerTest.php @@ -32,9 +32,9 @@ use OCP\Files\SimpleFS\ISimpleFolder; use OCP\IConfig; use OCP\IL10N; -use OCP\ILogger; use OCP\IServerContainer; use OCP\IUser; +use Psr\Log\LoggerInterface; /** * Class AvatarManagerTest @@ -46,7 +46,7 @@ class AvatarManagerTest extends \Test\TestCase { private $appData; /** @var IL10N|\PHPUnit\Framework\MockObject\MockObject */ private $l10n; - /** @var ILogger|\PHPUnit\Framework\MockObject\MockObject */ + /** @var LoggerInterface|\PHPUnit\Framework\MockObject\MockObject */ private $logger; /** @var IConfig|\PHPUnit\Framework\MockObject\MockObject */ private $config; @@ -59,7 +59,7 @@ protected function setUp(): void { $this->userManager = $this->createMock(Manager::class); $this->appData = $this->createMock(IAppData::class); $this->l10n = $this->createMock(IL10N::class); - $this->logger = $this->createMock(ILogger::class); + $this->logger = $this->createMock(LoggerInterface::class); $this->config = $this->createMock(IConfig::class); $this->avatarManager = new AvatarManager( diff --git a/tests/lib/Avatar/GuestAvatarTest.php b/tests/lib/Avatar/GuestAvatarTest.php index 1c424234f100c..b8e6d8ae2e8a6 100644 --- a/tests/lib/Avatar/GuestAvatarTest.php +++ b/tests/lib/Avatar/GuestAvatarTest.php @@ -26,8 +26,8 @@ use OC\Avatar\GuestAvatar; use OCP\Files\SimpleFS\InMemoryFile; -use OCP\ILogger; use PHPUnit\Framework\MockObject\MockObject; +use Psr\Log\LoggerInterface; use Test\TestCase; /** @@ -48,8 +48,8 @@ class GuestAvatarTest extends TestCase { * @return void */ public function setupGuestAvatar() { - /* @var MockObject|ILogger $logger */ - $logger = $this->getMockBuilder(ILogger::class)->getMock(); + /* @var MockObject|LoggerInterface $logger */ + $logger = $this->getMockBuilder(LoggerInterface::class)->getMock(); $this->guestAvatar = new GuestAvatar('einstein', $logger); } diff --git a/tests/lib/Avatar/UserAvatarTest.php b/tests/lib/Avatar/UserAvatarTest.php index cf0edad950277..d0de36fd09a59 100644 --- a/tests/lib/Avatar/UserAvatarTest.php +++ b/tests/lib/Avatar/UserAvatarTest.php @@ -16,7 +16,7 @@ use OCP\Files\SimpleFS\ISimpleFile; use OCP\IConfig; use OCP\IL10N; -use OCP\ILogger; +use Psr\Log\LoggerInterface; class UserAvatarTest extends \Test\TestCase { /** @var Folder | \PHPUnit\Framework\MockObject\MockObject */ @@ -286,7 +286,7 @@ private function getUserAvatar($user) { $this->folder, $l, $user, - $this->createMock(ILogger::class), + $this->createMock(LoggerInterface::class), $this->config ); } From 612dbe6faa5cdd46ad958f4ca1126edb72363e5d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Calvi=C3=B1o=20S=C3=A1nchez?= Date: Mon, 14 Dec 2020 20:07:30 +0100 Subject: [PATCH 25/53] Add explicit providers for user and guest avatars MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit User and guest avatars were treated as built-in types and handled directly by the AvatarManager. Now a provider is introduced for each type, so they are handled like any generic avatar type. The AvatarManagerTest only tested getting user avatars, so the unit test was renamed and adjusted to test the UserAvatarProvider instead. Signed-off-by: Daniel Calviño Sánchez --- .../AppFramework/Bootstrap/Coordinator.php | 15 ++- lib/private/Avatar/AvatarManager.php | 26 +----- lib/private/Avatar/GuestAvatarProvider.php | 52 +++++++++++ lib/private/Avatar/UserAvatarProvider.php | 93 +++++++++++++++++++ ...gerTest.php => UserAvatarProviderTest.php} | 30 +++--- 5 files changed, 172 insertions(+), 44 deletions(-) create mode 100644 lib/private/Avatar/GuestAvatarProvider.php create mode 100644 lib/private/Avatar/UserAvatarProvider.php rename tests/lib/Avatar/{AvatarManagerTest.php => UserAvatarProviderTest.php} (80%) diff --git a/lib/private/AppFramework/Bootstrap/Coordinator.php b/lib/private/AppFramework/Bootstrap/Coordinator.php index 06a17e5242b57..7d6957fafae19 100644 --- a/lib/private/AppFramework/Bootstrap/Coordinator.php +++ b/lib/private/AppFramework/Bootstrap/Coordinator.php @@ -29,6 +29,8 @@ namespace OC\AppFramework\Bootstrap; +use OC\Avatar\GuestAvatarProvider; +use OC\Avatar\UserAvatarProvider; use OC\Support\CrashReport\Registry; use OC_App; use OCP\AppFramework\App; @@ -79,6 +81,11 @@ public function __construct(IServerContainer $container, } public function runInitialRegistration(): void { + if ($this->registrationContext === null) { + $this->registrationContext = new RegistrationContext($this->logger); + } + + $this->registerCore(); $this->registerApps(OC_App::getEnabledApps()); } @@ -86,13 +93,15 @@ public function runLazyRegistration(string $appId): void { $this->registerApps([$appId]); } + private function registerCore(): void { + $this->registrationContext->registerAvatarProvider('', 'user', UserAvatarProvider::class); + $this->registrationContext->registerAvatarProvider('', 'guest', GuestAvatarProvider::class); + } + /** * @param string[] $appIds */ private function registerApps(array $appIds): void { - if ($this->registrationContext === null) { - $this->registrationContext = new RegistrationContext($this->logger); - } $apps = []; foreach ($appIds as $appId) { /* diff --git a/lib/private/Avatar/AvatarManager.php b/lib/private/Avatar/AvatarManager.php index 6af11a4ee4a1c..f6f0b86038a18 100644 --- a/lib/private/Avatar/AvatarManager.php +++ b/lib/private/Avatar/AvatarManager.php @@ -114,21 +114,7 @@ public function __construct( * @throws NotFoundException In case there is no user folder yet */ public function getAvatar(string $userId) : IAvatar { - $user = $this->userManager->get($userId); - if ($user === null) { - throw new \Exception('user does not exist'); - } - - // sanitize userID - fixes casing issue (needed for the filesystem stuff that is done below) - $userId = $user->getUID(); - - try { - $folder = $this->appData->getFolder($userId); - } catch (NotFoundException $e) { - $folder = $this->appData->newFolder($userId); - } - - return new UserAvatar($folder, $this->l, $user, $this->logger, $this->config); + return $this->getGenericAvatar('user', $userId); } /** @@ -168,18 +154,10 @@ public function deleteUserAvatar(string $userId): void { * @return IAvatar */ public function getGuestAvatar(string $name): IAvatar { - return new GuestAvatar($name, $this->logger); + return $this->getGenericAvatar('guest', $name); } public function getGenericAvatar(string $type, string $id): IAvatar { - if ($type === 'user') { - return $this->getAvatar($id); - } - - if ($type === 'guest') { - return $this->getGuestAvatar($id); - } - $context = $this->bootstrapCoordinator->getRegistrationContext(); if ($context === null) { diff --git a/lib/private/Avatar/GuestAvatarProvider.php b/lib/private/Avatar/GuestAvatarProvider.php new file mode 100644 index 0000000000000..2bd5b162c73cb --- /dev/null +++ b/lib/private/Avatar/GuestAvatarProvider.php @@ -0,0 +1,52 @@ + + * + * @license GNU AGPL version 3 or any later version + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU Affero General Public License as + * published by the Free Software Foundation, either version 3 of the + * License, or (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Affero General Public License for more details. + * + * You should have received a copy of the GNU Affero General Public License + * along with this program. If not, see . + * + */ + +namespace OC\Avatar; + +use OCP\IAvatar; +use OCP\IAvatarProvider; +use Psr\Log\LoggerInterface; + +class GuestAvatarProvider implements IAvatarProvider { + + /** @var LoggerInterface */ + private $logger; + + public function __construct( + LoggerInterface $logger) { + $this->logger = $logger; + } + + /** + * Returns a GuestAvatar instance for the given guest name + * + * @param string $id the guest name, e.g. "Albert" + * @returns IAvatar + */ + public function getAvatar(string $id): IAvatar { + return new GuestAvatar($id, $this->logger); + } +} diff --git a/lib/private/Avatar/UserAvatarProvider.php b/lib/private/Avatar/UserAvatarProvider.php new file mode 100644 index 0000000000000..fedc273346452 --- /dev/null +++ b/lib/private/Avatar/UserAvatarProvider.php @@ -0,0 +1,93 @@ + + * + * @license GNU AGPL version 3 or any later version + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU Affero General Public License as + * published by the Free Software Foundation, either version 3 of the + * License, or (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Affero General Public License for more details. + * + * You should have received a copy of the GNU Affero General Public License + * along with this program. If not, see . + * + */ + +namespace OC\Avatar; + +use OCP\Files\IAppData; +use OCP\Files\NotFoundException; +use OCP\IAvatar; +use OCP\IAvatarProvider; +use OCP\IConfig; +use OCP\IL10N; +use OCP\IUserManager; +use Psr\Log\LoggerInterface; + +class UserAvatarProvider implements IAvatarProvider { + + /** @var IUserManager */ + private $userManager; + + /** @var IAppData */ + private $appData; + + /** @var IL10N */ + private $l; + + /** @var LoggerInterface */ + private $logger; + + /** @var IConfig */ + private $config; + + public function __construct( + IUserManager $userManager, + IAppData $appData, + IL10N $l, + LoggerInterface $logger, + IConfig $config) { + $this->userManager = $userManager; + $this->appData = $appData; + $this->l = $l; + $this->logger = $logger; + $this->config = $config; + } + + /** + * Returns a UserAvatar instance for the given user id + * + * @param string $id the user id + * @returns IAvatar + * @throws \Exception In case the username is potentially dangerous + * @throws NotFoundException In case there is no user folder yet + */ + public function getAvatar(string $id): IAvatar { + $user = $this->userManager->get($id); + if ($user === null) { + throw new \Exception('user does not exist'); + } + + // sanitize userID - fixes casing issue (needed for the filesystem stuff that is done below) + $userId = $user->getUID(); + + try { + $folder = $this->appData->getFolder($userId); + } catch (NotFoundException $e) { + $folder = $this->appData->newFolder($userId); + } + + return new UserAvatar($folder, $this->l, $user, $this->logger, $this->config); + } +} diff --git a/tests/lib/Avatar/AvatarManagerTest.php b/tests/lib/Avatar/UserAvatarProviderTest.php similarity index 80% rename from tests/lib/Avatar/AvatarManagerTest.php rename to tests/lib/Avatar/UserAvatarProviderTest.php index f8d4b33cc5a01..e0cf25434d775 100644 --- a/tests/lib/Avatar/AvatarManagerTest.php +++ b/tests/lib/Avatar/UserAvatarProviderTest.php @@ -24,23 +24,21 @@ namespace Test\Avatar; -use OC\AppFramework\Bootstrap\Coordinator; -use OC\Avatar\AvatarManager; use OC\Avatar\UserAvatar; -use OC\User\Manager; +use OC\Avatar\UserAvatarProvider; use OCP\Files\IAppData; use OCP\Files\SimpleFS\ISimpleFolder; use OCP\IConfig; use OCP\IL10N; -use OCP\IServerContainer; use OCP\IUser; +use OCP\IUserManager; use Psr\Log\LoggerInterface; /** - * Class AvatarManagerTest + * Class UserAvatarProviderTest */ -class AvatarManagerTest extends \Test\TestCase { - /** @var Manager|\PHPUnit\Framework\MockObject\MockObject */ +class UserAvatarProviderTest extends \Test\TestCase { + /** @var IUserManager|\PHPUnit\Framework\MockObject\MockObject */ private $userManager; /** @var IAppData|\PHPUnit\Framework\MockObject\MockObject */ private $appData; @@ -50,26 +48,24 @@ class AvatarManagerTest extends \Test\TestCase { private $logger; /** @var IConfig|\PHPUnit\Framework\MockObject\MockObject */ private $config; - /** @var AvatarManager | \PHPUnit\Framework\MockObject\MockObject */ - private $avatarManager; + /** @var UserAvatarProvider | \PHPUnit\Framework\MockObject\MockObject */ + private $userAvatarProvider; protected function setUp(): void { parent::setUp(); - $this->userManager = $this->createMock(Manager::class); + $this->userManager = $this->createMock(IUserManager::class); $this->appData = $this->createMock(IAppData::class); $this->l10n = $this->createMock(IL10N::class); $this->logger = $this->createMock(LoggerInterface::class); $this->config = $this->createMock(IConfig::class); - $this->avatarManager = new AvatarManager( + $this->userAvatarProvider = new UserAvatarProvider( $this->userManager, $this->appData, $this->l10n, $this->logger, - $this->config, - $this->createMock(IServerContainer::class), - $this->createMock(Coordinator::class) + $this->config ); } @@ -84,7 +80,7 @@ public function testGetAvatarInvalidUser() { ->with('invalidUser') ->willReturn(null); - $this->avatarManager->getAvatar('invalidUser'); + $this->userAvatarProvider->getAvatar('invalidUser'); } public function testGetAvatarValidUser() { @@ -106,7 +102,7 @@ public function testGetAvatarValidUser() { ->willReturn($folder); $expected = new UserAvatar($folder, $this->l10n, $user, $this->logger, $this->config); - $this->assertEquals($expected, $this->avatarManager->getAvatar('valid-user')); + $this->assertEquals($expected, $this->userAvatarProvider->getAvatar('valid-user')); } public function testGetAvatarValidUserDifferentCasing() { @@ -128,6 +124,6 @@ public function testGetAvatarValidUserDifferentCasing() { ->willReturn($folder); $expected = new UserAvatar($folder, $this->l10n, $user, $this->logger, $this->config); - $this->assertEquals($expected, $this->avatarManager->getAvatar('vaLid-USER')); + $this->assertEquals($expected, $this->userAvatarProvider->getAvatar('vaLid-USER')); } } From 0e953eb48cf3fa567daa6192b1b07b77d38e99df Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Calvi=C3=B1o=20S=C3=A1nchez?= Date: Mon, 14 Dec 2020 20:10:34 +0100 Subject: [PATCH 26/53] Remove no longer needed attributes from AvatarManager MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Daniel Calviño Sánchez --- lib/private/Avatar/AvatarManager.php | 14 -------------- lib/private/Server.php | 2 -- 2 files changed, 16 deletions(-) diff --git a/lib/private/Avatar/AvatarManager.php b/lib/private/Avatar/AvatarManager.php index f6f0b86038a18..9ad86a454d16e 100644 --- a/lib/private/Avatar/AvatarManager.php +++ b/lib/private/Avatar/AvatarManager.php @@ -35,7 +35,6 @@ namespace OC\Avatar; use OC\AppFramework\Bootstrap\Coordinator; -use OC\User\Manager; use OC\User\NoUserException; use OCP\Files\IAppData; use OCP\Files\NotFoundException; @@ -44,7 +43,6 @@ use OCP\IAvatarManager; use OCP\IAvatarProvider; use OCP\IConfig; -use OCP\IL10N; use OCP\IServerContainer; use Psr\Log\LoggerInterface; @@ -53,15 +51,9 @@ */ class AvatarManager implements IAvatarManager { - /** @var Manager */ - private $userManager; - /** @var IAppData */ private $appData; - /** @var IL10N */ - private $l; - /** @var LoggerInterface */ private $logger; @@ -80,25 +72,19 @@ class AvatarManager implements IAvatarManager { /** * AvatarManager constructor. * - * @param Manager $userManager * @param IAppData $appData - * @param IL10N $l * @param LoggerInterface $logger * @param IConfig $config * @param IServerContainer $serverContainer * @param Coordinator $bootstrapCoordinator */ public function __construct( - Manager $userManager, IAppData $appData, - IL10N $l, LoggerInterface $logger, IConfig $config, IServerContainer $serverContainer, Coordinator $bootstrapCoordinator) { - $this->userManager = $userManager; $this->appData = $appData; - $this->l = $l; $this->logger = $logger; $this->config = $config; $this->serverContainer = $serverContainer; diff --git a/lib/private/Server.php b/lib/private/Server.php index 61e42d4516120..31b85b5dc1629 100644 --- a/lib/private/Server.php +++ b/lib/private/Server.php @@ -717,9 +717,7 @@ public function __construct($webRoot, \OC\Config $config) { $this->registerService(AvatarManager::class, function (Server $c) { return new AvatarManager( - $c->get(\OC\User\Manager::class), $c->getAppDataDir('avatar'), - $c->getL10N('lib'), $c->get(LoggerInterface::class), $c->get(\OCP\IConfig::class), $c->get(IServerContainer::class), From 0bb311ff0d2c91bc6471070ddad24d32fd3cc406 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Calvi=C3=B1o=20S=C3=A1nchez?= Date: Mon, 14 Dec 2020 20:37:32 +0100 Subject: [PATCH 27/53] Get the avatar provider rather than the avatar itself from the manager MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This will make possible to use other future IAvatarProvider methods. Signed-off-by: Daniel Calviño Sánchez --- core/Controller/GenericAvatarController.php | 6 +++--- lib/private/Avatar/AvatarManager.php | 8 ++++---- lib/public/IAvatarManager.php | 11 ++++------- 3 files changed, 11 insertions(+), 14 deletions(-) diff --git a/core/Controller/GenericAvatarController.php b/core/Controller/GenericAvatarController.php index 816b66a3b93b3..af5c8f791b1fb 100644 --- a/core/Controller/GenericAvatarController.php +++ b/core/Controller/GenericAvatarController.php @@ -77,7 +77,7 @@ public function getAvatar(string $avatarType, string $avatarId, int $size): Resp } try { - $avatar = $this->avatarManager->getGenericAvatar($avatarType, $avatarId); + $avatar = $this->avatarManager->getAvatarProvider($avatarType)->getAvatar($avatarId); $avatarFile = $avatar->getFile($size); $response = new FileDisplayResponse( $avatarFile, @@ -140,7 +140,7 @@ public function setAvatar(string $avatarType, string $avatarId): DataResponse { $image->loadFromData($content); try { - $avatar = $this->avatarManager->getGenericAvatar($avatarType, $avatarId); + $avatar = $this->avatarManager->getAvatarProvider($avatarType)->getAvatar($avatarId); $avatar->set($image); return new DataResponse( ['status' => 'success'] @@ -166,7 +166,7 @@ public function setAvatar(string $avatarType, string $avatarId): DataResponse { */ public function deleteAvatar(string $avatarType, string $avatarId): DataResponse { try { - $avatar = $this->avatarManager->getGenericAvatar($avatarType, $avatarId); + $avatar = $this->avatarManager->getAvatarProvider($avatarType)->getAvatar($avatarId); $avatar->remove(); return new DataResponse(); } catch (\Exception $e) { diff --git a/lib/private/Avatar/AvatarManager.php b/lib/private/Avatar/AvatarManager.php index 9ad86a454d16e..7032b7c9c0ad6 100644 --- a/lib/private/Avatar/AvatarManager.php +++ b/lib/private/Avatar/AvatarManager.php @@ -100,7 +100,7 @@ public function __construct( * @throws NotFoundException In case there is no user folder yet */ public function getAvatar(string $userId) : IAvatar { - return $this->getGenericAvatar('user', $userId); + return $this->getAvatarProvider('user')->getAvatar($userId); } /** @@ -140,10 +140,10 @@ public function deleteUserAvatar(string $userId): void { * @return IAvatar */ public function getGuestAvatar(string $name): IAvatar { - return $this->getGenericAvatar('guest', $name); + return $this->getAvatarProvider('guest')->getAvatar($name); } - public function getGenericAvatar(string $type, string $id): IAvatar { + public function getAvatarProvider(string $type): IAvatarProvider { $context = $this->bootstrapCoordinator->getRegistrationContext(); if ($context === null) { @@ -161,6 +161,6 @@ public function getGenericAvatar(string $type, string $id): IAvatar { $this->providers[$type] = $this->serverContainer->get($providerClasses[$type]); } - return $this->providers[$type]->getAvatar($id); + return $this->providers[$type]; } } diff --git a/lib/public/IAvatarManager.php b/lib/public/IAvatarManager.php index 7468a25b419cf..611c8a4e8415b 100644 --- a/lib/public/IAvatarManager.php +++ b/lib/public/IAvatarManager.php @@ -58,15 +58,12 @@ public function getAvatar(string $user) : IAvatar; public function getGuestAvatar(string $name): IAvatar; /** - * Returns an avatar instance of the given type + * Returns an avatar provider instance of the given type * * @param string $type the type of the avatar - * @param string $id the identifier for the avatar of the given type - * @return IAvatar - * @throws \InvalidArgumentException if the type is not known or the id is - * not valid - * @throws \Exception if getting the avatar failed + * @return IAvatarProvider + * @throws \InvalidArgumentException if the type is not known * @since 21.0.0 */ - public function getGenericAvatar(string $type, string $id): IAvatar; + public function getAvatarProvider(string $type): IAvatarProvider; } From 611881b06cb46cc3fd2f32bcb7fcf9ff4cf47a3f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Calvi=C3=B1o=20S=C3=A1nchez?= Date: Mon, 14 Dec 2020 20:43:15 +0100 Subject: [PATCH 28/53] Add method to get the cache duration of an avatar to IAvatarProvider MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Daniel Calviño Sánchez --- core/Controller/GenericAvatarController.php | 8 +++++++- lib/private/Avatar/GuestAvatarProvider.php | 11 +++++++++++ lib/private/Avatar/UserAvatarProvider.php | 11 +++++++++++ lib/public/IAvatarProvider.php | 10 ++++++++++ 4 files changed, 39 insertions(+), 1 deletion(-) diff --git a/core/Controller/GenericAvatarController.php b/core/Controller/GenericAvatarController.php index af5c8f791b1fb..c6a4fbdf508ca 100644 --- a/core/Controller/GenericAvatarController.php +++ b/core/Controller/GenericAvatarController.php @@ -77,7 +77,8 @@ public function getAvatar(string $avatarType, string $avatarId, int $size): Resp } try { - $avatar = $this->avatarManager->getAvatarProvider($avatarType)->getAvatar($avatarId); + $avatarProvider = $this->avatarManager->getAvatarProvider($avatarType); + $avatar = $avatarProvider->getAvatar($avatarId); $avatarFile = $avatar->getFile($size); $response = new FileDisplayResponse( $avatarFile, @@ -96,6 +97,11 @@ public function getAvatar(string $avatarType, string $avatarId, int $size): Resp return new DataResponse([], Http::STATUS_NOT_FOUND); } + $cache = $avatarProvider->getCacheTimeToLive($avatar); + if ($cache !== null) { + $response->cacheFor($cache); + } + return $response; } diff --git a/lib/private/Avatar/GuestAvatarProvider.php b/lib/private/Avatar/GuestAvatarProvider.php index 2bd5b162c73cb..af235a5686d68 100644 --- a/lib/private/Avatar/GuestAvatarProvider.php +++ b/lib/private/Avatar/GuestAvatarProvider.php @@ -49,4 +49,15 @@ public function __construct( public function getAvatar(string $id): IAvatar { return new GuestAvatar($id, $this->logger); } + + /** + * Returns the cache duration for guest avatars in seconds + * + * @param IAvatar $avatar ignored, same duration for all guest avatars + * @return int the cache duration + */ + public function getCacheTimeToLive(IAvatar $avatar): int { + // Cache for 30 minutes + return 60 * 30; + } } diff --git a/lib/private/Avatar/UserAvatarProvider.php b/lib/private/Avatar/UserAvatarProvider.php index fedc273346452..4dcaf383211cf 100644 --- a/lib/private/Avatar/UserAvatarProvider.php +++ b/lib/private/Avatar/UserAvatarProvider.php @@ -90,4 +90,15 @@ public function getAvatar(string $id): IAvatar { return new UserAvatar($folder, $this->l, $user, $this->logger, $this->config); } + + /** + * Returns the cache duration for user avatars in seconds + * + * @param IAvatar $avatar ignored, same duration for all user avatars + * @return int the cache duration + */ + public function getCacheTimeToLive(IAvatar $avatar): int { + // Cache for 1 day + return 60 * 60 * 24; + } } diff --git a/lib/public/IAvatarProvider.php b/lib/public/IAvatarProvider.php index 16ba85ab27352..3796cea24130a 100644 --- a/lib/public/IAvatarProvider.php +++ b/lib/public/IAvatarProvider.php @@ -42,4 +42,14 @@ interface IAvatarProvider { * @since 21.0.0 */ public function getAvatar(string $id): IAvatar; + + /** + * Returns the cache duration in seconds + * + * @param IAvatar $avatar the specific avatar, returned by this provider, to + * get the cache for + * @return int the cache duration + * @since 21.0.0 + */ + public function getCacheTimeToLive(IAvatar $avatar): int; } From 6c99a9a07716f93770ca91e3a0665f95ff643026 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Calvi=C3=B1o=20S=C3=A1nchez?= Date: Mon, 14 Dec 2020 21:27:51 +0100 Subject: [PATCH 29/53] fixup! Add method to get the cache duration of an avatar to IAvatarProvider MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Daniel Calviño Sánchez --- lib/private/Avatar/GuestAvatarProvider.php | 4 ++-- lib/private/Avatar/UserAvatarProvider.php | 4 ++-- lib/public/IAvatarProvider.php | 4 ++-- 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/lib/private/Avatar/GuestAvatarProvider.php b/lib/private/Avatar/GuestAvatarProvider.php index af235a5686d68..c7f74f21a6e8a 100644 --- a/lib/private/Avatar/GuestAvatarProvider.php +++ b/lib/private/Avatar/GuestAvatarProvider.php @@ -54,9 +54,9 @@ public function getAvatar(string $id): IAvatar { * Returns the cache duration for guest avatars in seconds * * @param IAvatar $avatar ignored, same duration for all guest avatars - * @return int the cache duration + * @return int|null the cache duration */ - public function getCacheTimeToLive(IAvatar $avatar): int { + public function getCacheTimeToLive(IAvatar $avatar): ?int { // Cache for 30 minutes return 60 * 30; } diff --git a/lib/private/Avatar/UserAvatarProvider.php b/lib/private/Avatar/UserAvatarProvider.php index 4dcaf383211cf..961ba1cf1f07e 100644 --- a/lib/private/Avatar/UserAvatarProvider.php +++ b/lib/private/Avatar/UserAvatarProvider.php @@ -95,9 +95,9 @@ public function getAvatar(string $id): IAvatar { * Returns the cache duration for user avatars in seconds * * @param IAvatar $avatar ignored, same duration for all user avatars - * @return int the cache duration + * @return int|null the cache duration */ - public function getCacheTimeToLive(IAvatar $avatar): int { + public function getCacheTimeToLive(IAvatar $avatar): ?int { // Cache for 1 day return 60 * 60 * 24; } diff --git a/lib/public/IAvatarProvider.php b/lib/public/IAvatarProvider.php index 3796cea24130a..af651f463fcf9 100644 --- a/lib/public/IAvatarProvider.php +++ b/lib/public/IAvatarProvider.php @@ -48,8 +48,8 @@ public function getAvatar(string $id): IAvatar; * * @param IAvatar $avatar the specific avatar, returned by this provider, to * get the cache for - * @return int the cache duration + * @return int|null the cache duration, or null for no cache * @since 21.0.0 */ - public function getCacheTimeToLive(IAvatar $avatar): int; + public function getCacheTimeToLive(IAvatar $avatar): ?int; } From 60797936fdee447aa6df207227246f195ac733c3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Calvi=C3=B1o=20S=C3=A1nchez?= Date: Tue, 15 Dec 2020 11:55:48 +0100 Subject: [PATCH 30/53] fixup! Add explicit providers for user and guest avatars MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Daniel Calviño Sánchez --- lib/private/Avatar/UserAvatarProvider.php | 11 +++++----- tests/lib/Avatar/UserAvatarProviderTest.php | 23 ++++++++++++++++++--- 2 files changed, 26 insertions(+), 8 deletions(-) diff --git a/lib/private/Avatar/UserAvatarProvider.php b/lib/private/Avatar/UserAvatarProvider.php index 961ba1cf1f07e..97588682cb453 100644 --- a/lib/private/Avatar/UserAvatarProvider.php +++ b/lib/private/Avatar/UserAvatarProvider.php @@ -26,13 +26,14 @@ namespace OC\Avatar; -use OCP\Files\IAppData; +use OC\Files\AppData\Factory as AppDataFactory; use OCP\Files\NotFoundException; use OCP\IAvatar; use OCP\IAvatarProvider; use OCP\IConfig; use OCP\IL10N; use OCP\IUserManager; +use OCP\L10N\IFactory as L10NFactory; use Psr\Log\LoggerInterface; class UserAvatarProvider implements IAvatarProvider { @@ -54,13 +55,13 @@ class UserAvatarProvider implements IAvatarProvider { public function __construct( IUserManager $userManager, - IAppData $appData, - IL10N $l, + AppDataFactory $appDataFactory, + L10NFactory $lFactory, LoggerInterface $logger, IConfig $config) { $this->userManager = $userManager; - $this->appData = $appData; - $this->l = $l; + $this->appData = $appDataFactory->get('avatar'); + $this->l = $lFactory->get('lib'); $this->logger = $logger; $this->config = $config; } diff --git a/tests/lib/Avatar/UserAvatarProviderTest.php b/tests/lib/Avatar/UserAvatarProviderTest.php index e0cf25434d775..2432fd1401d1e 100644 --- a/tests/lib/Avatar/UserAvatarProviderTest.php +++ b/tests/lib/Avatar/UserAvatarProviderTest.php @@ -26,12 +26,15 @@ use OC\Avatar\UserAvatar; use OC\Avatar\UserAvatarProvider; +use OC\Files\AppData\AppData; +use OC\Files\AppData\Factory as AppDataFactory; use OCP\Files\IAppData; use OCP\Files\SimpleFS\ISimpleFolder; use OCP\IConfig; use OCP\IL10N; use OCP\IUser; use OCP\IUserManager; +use OCP\L10N\IFactory as L10NFactory; use Psr\Log\LoggerInterface; /** @@ -55,15 +58,29 @@ protected function setUp(): void { parent::setUp(); $this->userManager = $this->createMock(IUserManager::class); - $this->appData = $this->createMock(IAppData::class); + // The specific subclass rather than the interface needs to be mocked so + // PHPUnit does not complain about the returned type from the mocked + // "AppDataFactory::get". + $this->appData = $this->createMock(AppData::class); $this->l10n = $this->createMock(IL10N::class); $this->logger = $this->createMock(LoggerInterface::class); $this->config = $this->createMock(IConfig::class); + $appDataFactory = $this->createMock(AppDataFactory::class); + $appDataFactory + ->method('get') + ->with('avatar') + ->willReturn($this->appData); + $l10nFactory = $this->createMock(L10NFactory::class); + $l10nFactory + ->method('get') + ->with('lib') + ->willReturn($this->l10n); + $this->userAvatarProvider = new UserAvatarProvider( $this->userManager, - $this->appData, - $this->l10n, + $appDataFactory, + $l10nFactory, $this->logger, $this->config ); From bcd207406cc8a07c4286a951682efdf3c2a85a04 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Calvi=C3=B1o=20S=C3=A1nchez?= Date: Tue, 15 Dec 2020 12:34:40 +0100 Subject: [PATCH 31/53] fixup! Move registration of IAvatarProviders to IRegistrationContext MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Daniel Calviño Sánchez --- lib/private/Avatar/AvatarManager.php | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/lib/private/Avatar/AvatarManager.php b/lib/private/Avatar/AvatarManager.php index 7032b7c9c0ad6..32887eab3a71f 100644 --- a/lib/private/Avatar/AvatarManager.php +++ b/lib/private/Avatar/AvatarManager.php @@ -147,8 +147,7 @@ public function getAvatarProvider(string $type): IAvatarProvider { $context = $this->bootstrapCoordinator->getRegistrationContext(); if ($context === null) { - // Too early, nothing to do yet - throw new \InvalidArgumentException('Unknown avatar type: ' . $type); + throw new RuntimeException("Avatar requested before the apps had been fully registered"); } $providerClasses = $context->getAvatarProviders(); From 23f4c1de6055f0c1816a5c1e839bd89dbb8f9bad Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Calvi=C3=B1o=20S=C3=A1nchez?= Date: Tue, 15 Dec 2020 12:36:51 +0100 Subject: [PATCH 32/53] fixup! Get the avatar provider rather than the avatar itself from the manager MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Daniel Calviño Sánchez --- lib/private/Avatar/AvatarManager.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/private/Avatar/AvatarManager.php b/lib/private/Avatar/AvatarManager.php index 32887eab3a71f..33fd5f9946ab4 100644 --- a/lib/private/Avatar/AvatarManager.php +++ b/lib/private/Avatar/AvatarManager.php @@ -147,7 +147,7 @@ public function getAvatarProvider(string $type): IAvatarProvider { $context = $this->bootstrapCoordinator->getRegistrationContext(); if ($context === null) { - throw new RuntimeException("Avatar requested before the apps had been fully registered"); + throw new RuntimeException("Avatar provider requested before the apps had been fully registered"); } $providerClasses = $context->getAvatarProviders(); From e88d3b6a4ca54cbd40abf53fc5e4b0f814732d2b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Calvi=C3=B1o=20S=C3=A1nchez?= Date: Tue, 15 Dec 2020 14:01:40 +0100 Subject: [PATCH 33/53] fixup! Make possible to define custom avatar types MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Daniel Calviño Sánchez --- core/Controller/GenericAvatarController.php | 3 ++ lib/composer/composer/autoload_classmap.php | 1 + lib/composer/composer/autoload_static.php | 1 + lib/public/AvatarProviderException.php | 39 +++++++++++++++++++++ lib/public/IAvatarProvider.php | 3 +- 5 files changed, 46 insertions(+), 1 deletion(-) create mode 100644 lib/public/AvatarProviderException.php diff --git a/core/Controller/GenericAvatarController.php b/core/Controller/GenericAvatarController.php index c6a4fbdf508ca..b6d4ace8ea82c 100644 --- a/core/Controller/GenericAvatarController.php +++ b/core/Controller/GenericAvatarController.php @@ -31,6 +31,7 @@ use OCP\AppFramework\Http\DataResponse; use OCP\AppFramework\Http\FileDisplayResponse; use OCP\AppFramework\Http\Response; +use OCP\AvatarProviderException; use OCP\Files\NotFoundException; use OCP\IAvatarManager; use OCP\IL10N; @@ -90,6 +91,8 @@ public function getAvatar(string $avatarType, string $avatarId, int $size): Resp ); } catch (\InvalidArgumentException $e) { return new DataResponse([], Http::STATUS_NOT_FOUND); + } catch (AvatarProviderException $e) { + return new DataResponse([], Http::STATUS_NOT_FOUND); } catch (NotFoundException $e) { return new DataResponse([], Http::STATUS_NOT_FOUND); } catch (\Exception $e) { diff --git a/lib/composer/composer/autoload_classmap.php b/lib/composer/composer/autoload_classmap.php index 1d03137148f97..29f7d80025090 100644 --- a/lib/composer/composer/autoload_classmap.php +++ b/lib/composer/composer/autoload_classmap.php @@ -105,6 +105,7 @@ 'OCP\\Authentication\\TwoFactorAuth\\TwoFactorException' => $baseDir . '/lib/public/Authentication/TwoFactorAuth/TwoFactorException.php', 'OCP\\Authentication\\TwoFactorAuth\\TwoFactorProviderDisabled' => $baseDir . '/lib/public/Authentication/TwoFactorAuth/TwoFactorProviderDisabled.php', 'OCP\\AutoloadNotAllowedException' => $baseDir . '/lib/public/AutoloadNotAllowedException.php', + 'OCP\\AvatarProviderException' => $baseDir . '/lib/public/AvatarProviderException.php', 'OCP\\BackgroundJob' => $baseDir . '/lib/public/BackgroundJob.php', 'OCP\\BackgroundJob\\IJob' => $baseDir . '/lib/public/BackgroundJob/IJob.php', 'OCP\\BackgroundJob\\IJobList' => $baseDir . '/lib/public/BackgroundJob/IJobList.php', diff --git a/lib/composer/composer/autoload_static.php b/lib/composer/composer/autoload_static.php index cc581a600837b..77363e5e179e6 100644 --- a/lib/composer/composer/autoload_static.php +++ b/lib/composer/composer/autoload_static.php @@ -134,6 +134,7 @@ class ComposerStaticInit53792487c5a8370acc0b06b1a864ff4c 'OCP\\Authentication\\TwoFactorAuth\\TwoFactorException' => __DIR__ . '/../../..' . '/lib/public/Authentication/TwoFactorAuth/TwoFactorException.php', 'OCP\\Authentication\\TwoFactorAuth\\TwoFactorProviderDisabled' => __DIR__ . '/../../..' . '/lib/public/Authentication/TwoFactorAuth/TwoFactorProviderDisabled.php', 'OCP\\AutoloadNotAllowedException' => __DIR__ . '/../../..' . '/lib/public/AutoloadNotAllowedException.php', + 'OCP\\AvatarProviderException' => __DIR__ . '/../../..' . '/lib/public/AvatarProviderException.php', 'OCP\\BackgroundJob' => __DIR__ . '/../../..' . '/lib/public/BackgroundJob.php', 'OCP\\BackgroundJob\\IJob' => __DIR__ . '/../../..' . '/lib/public/BackgroundJob/IJob.php', 'OCP\\BackgroundJob\\IJobList' => __DIR__ . '/../../..' . '/lib/public/BackgroundJob/IJobList.php', diff --git a/lib/public/AvatarProviderException.php b/lib/public/AvatarProviderException.php new file mode 100644 index 0000000000000..57bebda95ccdb --- /dev/null +++ b/lib/public/AvatarProviderException.php @@ -0,0 +1,39 @@ + + * + * @license GNU AGPL version 3 or any later version + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU Affero General Public License as + * published by the Free Software Foundation, either version 3 of the + * License, or (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Affero General Public License for more details. + * + * You should have received a copy of the GNU Affero General Public License + * along with this program. If not, see . + * + */ + +namespace OCP; + +/** + * Generic exception thrown when an AvatarProvider can not perform an action + * + * @since 21.0.0 + */ +class AvatarProviderException extends \RuntimeException { + + public function __construct(string $message = "", int $code = 0, \Exception $previous = null) { + parent::__construct($message, $code, $previous); + } +} diff --git a/lib/public/IAvatarProvider.php b/lib/public/IAvatarProvider.php index af651f463fcf9..8dd32e01a6f5d 100644 --- a/lib/public/IAvatarProvider.php +++ b/lib/public/IAvatarProvider.php @@ -38,7 +38,8 @@ interface IAvatarProvider { * * @param string $id the identifier of the avatar * @return IAvatar the avatar instance - * @throws \Exception if an error occurred while getting the avatar + * @throws AvatarProviderException if an error occurred while getting the + * avatar * @since 21.0.0 */ public function getAvatar(string $id): IAvatar; From 4a2694301f1b25ee673adcb3c551c05ecacad77a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Calvi=C3=B1o=20S=C3=A1nchez?= Date: Tue, 15 Dec 2020 14:05:33 +0100 Subject: [PATCH 34/53] fixup! Add explicit providers for user and guest avatars MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Daniel Calviño Sánchez --- lib/composer/composer/autoload_classmap.php | 2 ++ lib/composer/composer/autoload_static.php | 2 ++ 2 files changed, 4 insertions(+) diff --git a/lib/composer/composer/autoload_classmap.php b/lib/composer/composer/autoload_classmap.php index 29f7d80025090..1da2055002701 100644 --- a/lib/composer/composer/autoload_classmap.php +++ b/lib/composer/composer/autoload_classmap.php @@ -723,7 +723,9 @@ 'OC\\Avatar\\Avatar' => $baseDir . '/lib/private/Avatar/Avatar.php', 'OC\\Avatar\\AvatarManager' => $baseDir . '/lib/private/Avatar/AvatarManager.php', 'OC\\Avatar\\GuestAvatar' => $baseDir . '/lib/private/Avatar/GuestAvatar.php', + 'OC\\Avatar\\GuestAvatarProvider' => $baseDir . '/lib/private/Avatar/GuestAvatarProvider.php', 'OC\\Avatar\\UserAvatar' => $baseDir . '/lib/private/Avatar/UserAvatar.php', + 'OC\\Avatar\\UserAvatarProvider' => $baseDir . '/lib/private/Avatar/UserAvatarProvider.php', 'OC\\BackgroundJob\\Job' => $baseDir . '/lib/private/BackgroundJob/Job.php', 'OC\\BackgroundJob\\JobList' => $baseDir . '/lib/private/BackgroundJob/JobList.php', 'OC\\BackgroundJob\\Legacy\\QueuedJob' => $baseDir . '/lib/private/BackgroundJob/Legacy/QueuedJob.php', diff --git a/lib/composer/composer/autoload_static.php b/lib/composer/composer/autoload_static.php index 77363e5e179e6..11532cdc3ae4e 100644 --- a/lib/composer/composer/autoload_static.php +++ b/lib/composer/composer/autoload_static.php @@ -752,7 +752,9 @@ class ComposerStaticInit53792487c5a8370acc0b06b1a864ff4c 'OC\\Avatar\\Avatar' => __DIR__ . '/../../..' . '/lib/private/Avatar/Avatar.php', 'OC\\Avatar\\AvatarManager' => __DIR__ . '/../../..' . '/lib/private/Avatar/AvatarManager.php', 'OC\\Avatar\\GuestAvatar' => __DIR__ . '/../../..' . '/lib/private/Avatar/GuestAvatar.php', + 'OC\\Avatar\\GuestAvatarProvider' => __DIR__ . '/../../..' . '/lib/private/Avatar/GuestAvatarProvider.php', 'OC\\Avatar\\UserAvatar' => __DIR__ . '/../../..' . '/lib/private/Avatar/UserAvatar.php', + 'OC\\Avatar\\UserAvatarProvider' => __DIR__ . '/../../..' . '/lib/private/Avatar/UserAvatarProvider.php', 'OC\\BackgroundJob\\Job' => __DIR__ . '/../../..' . '/lib/private/BackgroundJob/Job.php', 'OC\\BackgroundJob\\JobList' => __DIR__ . '/../../..' . '/lib/private/BackgroundJob/JobList.php', 'OC\\BackgroundJob\\Legacy\\QueuedJob' => __DIR__ . '/../../..' . '/lib/private/BackgroundJob/Legacy/QueuedJob.php', From fa0342afc7d583436abd51ec6e03ecdbca3af51a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Calvi=C3=B1o=20S=C3=A1nchez?= Date: Tue, 15 Dec 2020 14:10:46 +0100 Subject: [PATCH 35/53] fixup! Add explicit providers for user and guest avatars MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Daniel Calviño Sánchez --- lib/private/Avatar/UserAvatarProvider.php | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/lib/private/Avatar/UserAvatarProvider.php b/lib/private/Avatar/UserAvatarProvider.php index 97588682cb453..a8b90f188d775 100644 --- a/lib/private/Avatar/UserAvatarProvider.php +++ b/lib/private/Avatar/UserAvatarProvider.php @@ -27,6 +27,7 @@ namespace OC\Avatar; use OC\Files\AppData\Factory as AppDataFactory; +use OCP\AvatarProviderException; use OCP\Files\NotFoundException; use OCP\IAvatar; use OCP\IAvatarProvider; @@ -71,13 +72,13 @@ public function __construct( * * @param string $id the user id * @returns IAvatar - * @throws \Exception In case the username is potentially dangerous - * @throws NotFoundException In case there is no user folder yet + * @throws AvatarProviderException if the user name does not exist + * @throws NotFoundException if there is no user folder yet */ public function getAvatar(string $id): IAvatar { $user = $this->userManager->get($id); if ($user === null) { - throw new \Exception('user does not exist'); + throw new AvatarProviderException('user ' . $id . ' does not exist'); } // sanitize userID - fixes casing issue (needed for the filesystem stuff that is done below) From ebc3f14ee20fcb49c57bde3e2b6b56c3b68cba85 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Calvi=C3=B1o=20S=C3=A1nchez?= Date: Tue, 15 Dec 2020 14:15:26 +0100 Subject: [PATCH 36/53] fixup! Get the avatar provider rather than the avatar itself from the manager MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Daniel Calviño Sánchez --- core/Controller/GenericAvatarController.php | 3 --- 1 file changed, 3 deletions(-) diff --git a/core/Controller/GenericAvatarController.php b/core/Controller/GenericAvatarController.php index b6d4ace8ea82c..8bc72ba873afe 100644 --- a/core/Controller/GenericAvatarController.php +++ b/core/Controller/GenericAvatarController.php @@ -95,9 +95,6 @@ public function getAvatar(string $avatarType, string $avatarId, int $size): Resp return new DataResponse([], Http::STATUS_NOT_FOUND); } catch (NotFoundException $e) { return new DataResponse([], Http::STATUS_NOT_FOUND); - } catch (\Exception $e) { - $this->logger->error('Error when getting avatar', ['app' => 'core', 'exception' => $e]); - return new DataResponse([], Http::STATUS_NOT_FOUND); } $cache = $avatarProvider->getCacheTimeToLive($avatar); From 03eed3e65ec529da3349cbf9ac03f27a6d4e4955 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Calvi=C3=B1o=20S=C3=A1nchez?= Date: Tue, 15 Dec 2020 20:46:18 +0100 Subject: [PATCH 37/53] fixup! Add integration tests for getting and setting generic avatars MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Daniel Calviño Sánchez --- build/integration/features/avatar.feature | 1 + 1 file changed, 1 insertion(+) diff --git a/build/integration/features/avatar.feature b/build/integration/features/avatar.feature index 3d2d5b9c1af14..b7c365248e0e8 100644 --- a/build/integration/features/avatar.feature +++ b/build/integration/features/avatar.feature @@ -1,6 +1,7 @@ Feature: avatar Background: + Given using api version "2" Given user "user0" exists Given user "user1" exists From 1348cabd3d98541f5effe0e55890db2933237426 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Calvi=C3=B1o=20S=C3=A1nchez?= Date: Tue, 15 Dec 2020 20:46:29 +0100 Subject: [PATCH 38/53] fixup! Add integration tests for getting and setting generic avatars MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Daniel Calviño Sánchez --- build/integration/features/avatar.feature | 50 ----------------------- 1 file changed, 50 deletions(-) diff --git a/build/integration/features/avatar.feature b/build/integration/features/avatar.feature index b7c365248e0e8..2c52a424d8bd0 100644 --- a/build/integration/features/avatar.feature +++ b/build/integration/features/avatar.feature @@ -57,24 +57,6 @@ Feature: avatar And last avatar is a square of size 256 And last avatar is a single "#00FF00" color - Scenario: set generic user avatar as another user - When user "user1" sets avatar for type "user" and id "user0" from file "data/green-square-256.png" with "404" - Then user "user0" gets avatar for type "user" and id "user0" - And The following headers should be set - | Content-Type | image/png | - | X-NC-IsCustomAvatar | 0 | - And last avatar is a square of size 128 - And last avatar is not a single color - - Scenario: set generic user avatar as an anonymous user - When user "anonymous" sets avatar for type "user" and id "user0" from file "data/green-square-256.png" with "404" - Then user "user0" gets avatar for type "user" and id "user0" - And The following headers should be set - | Content-Type | image/png | - | X-NC-IsCustomAvatar | 0 | - And last avatar is a square of size 128 - And last avatar is not a single color - Scenario: set non squared image as generic user avatar When user "user0" sets avatar for type "user" and id "user0" from file "data/coloured-pattern.png" with "400" Then user "user0" gets avatar for type "user" and id "user0" @@ -133,38 +115,6 @@ Feature: avatar And last avatar is a square of size 128 And last avatar is not a single color - Scenario: delete generic user avatar as another user - Given user "user0" sets avatar for type "user" and id "user0" from file "data/green-square-256.png" - And user "user0" gets avatar for type "user" and id "user0" with size "256" - And The following headers should be set - | Content-Type | image/png | - | X-NC-IsCustomAvatar | 1 | - And last avatar is a square of size 256 - And last avatar is a single "#00FF00" color - When user "user1" deletes avatar for type "user" and id "user0" with "404" - Then user "user0" gets avatar for type "user" and id "user0" with size "256" - And The following headers should be set - | Content-Type | image/png | - | X-NC-IsCustomAvatar | 1 | - And last avatar is a square of size 128 - And last avatar is a single "#00FF00" color - - Scenario: delete generic user avatar as an anonymous user - Given user "user0" sets avatar for type "user" and id "user0" from file "data/green-square-256.png" - And user "user0" gets avatar for type "user" and id "user0" with size "256" - And The following headers should be set - | Content-Type | image/png | - | X-NC-IsCustomAvatar | 1 | - And last avatar is a square of size 256 - And last avatar is a single "#00FF00" color - When user "anonymous" deletes avatar for type "user" and id "user0" with "404" - Then user "user0" gets avatar for type "user" and id "user0" with size "256" - And The following headers should be set - | Content-Type | image/png | - | X-NC-IsCustomAvatar | 1 | - And last avatar is a square of size 128 - And last avatar is a single "#00FF00" color - Scenario: delete generic guest avatar # Silently ignored When user "user0" deletes avatar for type "guest" and id "guest0" From 8d69a4e354690d691358fae692431936347fac89 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Calvi=C3=B1o=20S=C3=A1nchez?= Date: Wed, 16 Dec 2020 00:01:20 +0100 Subject: [PATCH 39/53] fixup! Add OCS endpoint for avatars MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Daniel Calviño Sánchez --- core/Controller/GenericAvatarController.php | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/core/Controller/GenericAvatarController.php b/core/Controller/GenericAvatarController.php index 8bc72ba873afe..e5a8646a208fd 100644 --- a/core/Controller/GenericAvatarController.php +++ b/core/Controller/GenericAvatarController.php @@ -151,6 +151,8 @@ public function setAvatar(string $avatarType, string $avatarId): DataResponse { return new DataResponse( ['status' => 'success'] ); + } catch (\InvalidArgumentException $e) { + return new DataResponse([], Http::STATUS_NOT_FOUND); } catch (\OC\NotSquareException $e) { return new DataResponse( ['data' => ['message' => $this->l->t('Crop is not square')]], @@ -175,6 +177,8 @@ public function deleteAvatar(string $avatarType, string $avatarId): DataResponse $avatar = $this->avatarManager->getAvatarProvider($avatarType)->getAvatar($avatarId); $avatar->remove(); return new DataResponse(); + } catch (\InvalidArgumentException $e) { + return new DataResponse([], Http::STATUS_NOT_FOUND); } catch (\Exception $e) { $this->logger->error('Error when deleting avatar', ['app' => 'core', 'exception' => $e]); return new DataResponse( From 4368ec0d2331b72ae455bc06a6e82388960637e7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Calvi=C3=B1o=20S=C3=A1nchez?= Date: Wed, 16 Dec 2020 00:03:10 +0100 Subject: [PATCH 40/53] fixup! Make possible to define custom avatar types MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Daniel Calviño Sánchez --- core/Controller/GenericAvatarController.php | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/core/Controller/GenericAvatarController.php b/core/Controller/GenericAvatarController.php index e5a8646a208fd..482d7c58e8bcf 100644 --- a/core/Controller/GenericAvatarController.php +++ b/core/Controller/GenericAvatarController.php @@ -153,6 +153,8 @@ public function setAvatar(string $avatarType, string $avatarId): DataResponse { ); } catch (\InvalidArgumentException $e) { return new DataResponse([], Http::STATUS_NOT_FOUND); + } catch (AvatarProviderException $e) { + return new DataResponse([], Http::STATUS_NOT_FOUND); } catch (\OC\NotSquareException $e) { return new DataResponse( ['data' => ['message' => $this->l->t('Crop is not square')]], @@ -179,6 +181,8 @@ public function deleteAvatar(string $avatarType, string $avatarId): DataResponse return new DataResponse(); } catch (\InvalidArgumentException $e) { return new DataResponse([], Http::STATUS_NOT_FOUND); + } catch (AvatarProviderException $e) { + return new DataResponse([], Http::STATUS_NOT_FOUND); } catch (\Exception $e) { $this->logger->error('Error when deleting avatar', ['app' => 'core', 'exception' => $e]); return new DataResponse( From 6b8f2905d658ea085a29cd0171f0a0651105a027 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Calvi=C3=B1o=20S=C3=A1nchez?= Date: Wed, 16 Dec 2020 00:03:37 +0100 Subject: [PATCH 41/53] fixup! Add integration tests for getting and setting generic avatars MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Daniel Calviño Sánchez --- build/integration/features/avatar.feature | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/build/integration/features/avatar.feature b/build/integration/features/avatar.feature index 2c52a424d8bd0..47aadc3fa6e8b 100644 --- a/build/integration/features/avatar.feature +++ b/build/integration/features/avatar.feature @@ -85,6 +85,9 @@ Feature: avatar And last avatar is a square of size 128 And last avatar is not a single color + Scenario: set generic unknown avatar + When user "user0" sets avatar for type "unknown" and id "user0" from file "data/green-square-256.png" with "404" + Scenario: delete generic user avatar @@ -125,6 +128,9 @@ Feature: avatar And last avatar is a square of size 128 And last avatar is not a single color + Scenario: delete generic unknown avatar + When user "user0" deletes avatar for type "unknown" and id "user0" with "404" + Scenario: get generic user avatar with a larger size than the original one From 68b298e673a3279901c43a4723647803643163ec Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Calvi=C3=B1o=20S=C3=A1nchez?= Date: Wed, 16 Dec 2020 01:05:13 +0100 Subject: [PATCH 42/53] fixup! Add method to get the cache duration of an avatar to IAvatarProvider MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Daniel Calviño Sánchez --- lib/public/IAvatarProvider.php | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/lib/public/IAvatarProvider.php b/lib/public/IAvatarProvider.php index 8dd32e01a6f5d..76fd47aba7c6c 100644 --- a/lib/public/IAvatarProvider.php +++ b/lib/public/IAvatarProvider.php @@ -47,9 +47,14 @@ public function getAvatar(string $id): IAvatar; /** * Returns the cache duration in seconds * + * Implementers of IAvatarProvider may not throw \InvalidArgumentException + * if the behaviour does not depend on specific avatar instances. + * * @param IAvatar $avatar the specific avatar, returned by this provider, to * get the cache for * @return int|null the cache duration, or null for no cache + * @throws \InvalidArgumentException if the given avatar is not supported by + * this provider * @since 21.0.0 */ public function getCacheTimeToLive(IAvatar $avatar): ?int; From cc7cd189715a06c5ecf6de923e67a9aae29eaa97 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Calvi=C3=B1o=20S=C3=A1nchez?= Date: Wed, 16 Dec 2020 01:08:24 +0100 Subject: [PATCH 43/53] Split try/catch blocks to catch only the relevant exceptions MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Daniel Calviño Sánchez --- core/Controller/GenericAvatarController.php | 35 ++++++++++++++------- 1 file changed, 23 insertions(+), 12 deletions(-) diff --git a/core/Controller/GenericAvatarController.php b/core/Controller/GenericAvatarController.php index 482d7c58e8bcf..d5ea4e9e9c1f1 100644 --- a/core/Controller/GenericAvatarController.php +++ b/core/Controller/GenericAvatarController.php @@ -80,6 +80,13 @@ public function getAvatar(string $avatarType, string $avatarId, int $size): Resp try { $avatarProvider = $this->avatarManager->getAvatarProvider($avatarType); $avatar = $avatarProvider->getAvatar($avatarId); + } catch (\InvalidArgumentException $e) { + return new DataResponse([], Http::STATUS_NOT_FOUND); + } catch (AvatarProviderException $e) { + return new DataResponse([], Http::STATUS_NOT_FOUND); + } + + try { $avatarFile = $avatar->getFile($size); $response = new FileDisplayResponse( $avatarFile, @@ -89,10 +96,6 @@ public function getAvatar(string $avatarType, string $avatarId, int $size): Resp 'X-NC-IsCustomAvatar' => $avatar->isCustomAvatar() ? '1' : '0', ] ); - } catch (\InvalidArgumentException $e) { - return new DataResponse([], Http::STATUS_NOT_FOUND); - } catch (AvatarProviderException $e) { - return new DataResponse([], Http::STATUS_NOT_FOUND); } catch (NotFoundException $e) { return new DataResponse([], Http::STATUS_NOT_FOUND); } @@ -146,15 +149,19 @@ public function setAvatar(string $avatarType, string $avatarId): DataResponse { $image->loadFromData($content); try { - $avatar = $this->avatarManager->getAvatarProvider($avatarType)->getAvatar($avatarId); - $avatar->set($image); - return new DataResponse( - ['status' => 'success'] - ); + $avatarProvider = $this->avatarManager->getAvatarProvider($avatarType); + $avatar = $avatarProvider->getAvatar($avatarId); } catch (\InvalidArgumentException $e) { return new DataResponse([], Http::STATUS_NOT_FOUND); } catch (AvatarProviderException $e) { return new DataResponse([], Http::STATUS_NOT_FOUND); + } + + try { + $avatar->set($image); + return new DataResponse( + ['status' => 'success'] + ); } catch (\OC\NotSquareException $e) { return new DataResponse( ['data' => ['message' => $this->l->t('Crop is not square')]], @@ -176,13 +183,17 @@ public function setAvatar(string $avatarType, string $avatarId): DataResponse { */ public function deleteAvatar(string $avatarType, string $avatarId): DataResponse { try { - $avatar = $this->avatarManager->getAvatarProvider($avatarType)->getAvatar($avatarId); - $avatar->remove(); - return new DataResponse(); + $avatarProvider = $this->avatarManager->getAvatarProvider($avatarType); + $avatar = $avatarProvider->getAvatar($avatarId); } catch (\InvalidArgumentException $e) { return new DataResponse([], Http::STATUS_NOT_FOUND); } catch (AvatarProviderException $e) { return new DataResponse([], Http::STATUS_NOT_FOUND); + } + + try { + $avatar->remove(); + return new DataResponse(); } catch (\Exception $e) { $this->logger->error('Error when deleting avatar', ['app' => 'core', 'exception' => $e]); return new DataResponse( From 6ed5fde99b8b06427537512ae3c3c6491829c21b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Calvi=C3=B1o=20S=C3=A1nchez?= Date: Wed, 16 Dec 2020 01:23:31 +0100 Subject: [PATCH 44/53] Add method to check if an avatar can be accessed by the current user MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit IAvatarProvider implementers now need to define whether the current user can access or not a given IAvatar (created by the provider itself). With this the GenericAvatarController can allow or deny access to avatars in a generic way. The controller returns STATUS_NOT_FOUND instead of STATUS_FORBIDDEN to prevent revealing to unauthorized users the existence of an element with the given id. Adding the method to IAvatarProvider prevents touching IAvatar, which might require changes in apps, and also keeps the access control logic separate from the avatar logic, which is more focused on handling of files and images. Signed-off-by: Daniel Calviño Sánchez --- core/Controller/GenericAvatarController.php | 4 ++++ lib/private/Avatar/GuestAvatarProvider.php | 10 ++++++++++ lib/private/Avatar/UserAvatarProvider.php | 10 ++++++++++ lib/public/IAvatarProvider.php | 18 ++++++++++++++++++ 4 files changed, 42 insertions(+) diff --git a/core/Controller/GenericAvatarController.php b/core/Controller/GenericAvatarController.php index d5ea4e9e9c1f1..2deeb3282f711 100644 --- a/core/Controller/GenericAvatarController.php +++ b/core/Controller/GenericAvatarController.php @@ -86,6 +86,10 @@ public function getAvatar(string $avatarType, string $avatarId, int $size): Resp return new DataResponse([], Http::STATUS_NOT_FOUND); } + if (!$avatarProvider->canBeAccessedByCurrentUser($avatar)) { + return new DataResponse([], Http::STATUS_NOT_FOUND); + } + try { $avatarFile = $avatar->getFile($size); $response = new FileDisplayResponse( diff --git a/lib/private/Avatar/GuestAvatarProvider.php b/lib/private/Avatar/GuestAvatarProvider.php index c7f74f21a6e8a..928c88f03f64d 100644 --- a/lib/private/Avatar/GuestAvatarProvider.php +++ b/lib/private/Avatar/GuestAvatarProvider.php @@ -50,6 +50,16 @@ public function getAvatar(string $id): IAvatar { return new GuestAvatar($id, $this->logger); } + /** + * Returns whether the current user can access the given avatar or not + * + * @param IAvatar $avatar ignored + * @return bool true, as all users can access guest avatars + */ + public function canBeAccessedByCurrentUser(IAvatar $avatar): bool { + return true; + } + /** * Returns the cache duration for guest avatars in seconds * diff --git a/lib/private/Avatar/UserAvatarProvider.php b/lib/private/Avatar/UserAvatarProvider.php index a8b90f188d775..fe8f0973ef93e 100644 --- a/lib/private/Avatar/UserAvatarProvider.php +++ b/lib/private/Avatar/UserAvatarProvider.php @@ -93,6 +93,16 @@ public function getAvatar(string $id): IAvatar { return new UserAvatar($folder, $this->l, $user, $this->logger, $this->config); } + /** + * Returns whether the current user can access the given avatar or not + * + * @param IAvatar $avatar ignored + * @return bool true, as all users can access user avatars + */ + public function canBeAccessedByCurrentUser(IAvatar $avatar): bool { + return true; + } + /** * Returns the cache duration for user avatars in seconds * diff --git a/lib/public/IAvatarProvider.php b/lib/public/IAvatarProvider.php index 76fd47aba7c6c..09a50f1ba4466 100644 --- a/lib/public/IAvatarProvider.php +++ b/lib/public/IAvatarProvider.php @@ -44,6 +44,24 @@ interface IAvatarProvider { */ public function getAvatar(string $id): IAvatar; + /** + * Returns whether the current user can access the given avatar or not + * + * Clients of IAvatarProvider should not try to access the avatar if not + * allowed, but they can ignore it if it makes sense. + * + * Implementers of IAvatarProvider may not throw \InvalidArgumentException + * if the behaviour does not depend on specific avatar instances. + * + * @param IAvatar $avatar the avatar to check + * @return bool true if the current user can access the avatar, false + * otherwise + * @throws \InvalidArgumentException if the given avatar is not supported by + * this provider + * @since 21.0.0 + */ + public function canBeAccessedByCurrentUser(IAvatar $avatar): bool; + /** * Returns the cache duration in seconds * From 65c8c222957895ccdc9ad42925dfc4dc53bceaad Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Calvi=C3=B1o=20S=C3=A1nchez?= Date: Wed, 16 Dec 2020 01:33:44 +0100 Subject: [PATCH 45/53] Add method to check if an avatar can be modified by the current user MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit IAvatarProvider implementers now need to define whether the current user can modify or not a given IAvatar (created by the provider itself). With this the GenericAvatarController can allow or deny modification (including deletion) of avatars in a generic way. The controller returns STATUS_NOT_FOUND instead of STATUS_FORBIDDEN to prevent revealing to unauthorized users the existence of an element with the given id. Adding the method to IAvatarProvider prevents touching IAvatar, which might require changes in apps, and also keeps the access control logic separate from the avatar logic, which is more focused on handling of files and images. Signed-off-by: Daniel Calviño Sánchez --- build/integration/features/avatar.feature | 6 ++--- core/Controller/GenericAvatarController.php | 8 ++++++ lib/private/Avatar/GuestAvatarProvider.php | 11 ++++++++ lib/private/Avatar/UserAvatar.php | 4 +++ lib/private/Avatar/UserAvatarProvider.php | 29 ++++++++++++++++++++- lib/public/IAvatarProvider.php | 18 +++++++++++++ 6 files changed, 71 insertions(+), 5 deletions(-) diff --git a/build/integration/features/avatar.feature b/build/integration/features/avatar.feature index 47aadc3fa6e8b..7193913572016 100644 --- a/build/integration/features/avatar.feature +++ b/build/integration/features/avatar.feature @@ -76,8 +76,7 @@ Feature: avatar And last avatar is not a single color Scenario: set generic guest avatar - # Silently ignored - When user "user0" sets avatar for type "guest" and id "guest0" from file "data/green-square-256.png" + When user "user0" sets avatar for type "guest" and id "guest0" from file "data/green-square-256.png" with "404" Then user "user0" gets avatar for type "guest" and id "guest0" And The following headers should be set | Content-Type | image/png | @@ -119,8 +118,7 @@ Feature: avatar And last avatar is not a single color Scenario: delete generic guest avatar - # Silently ignored - When user "user0" deletes avatar for type "guest" and id "guest0" + When user "user0" deletes avatar for type "guest" and id "guest0" with "404" Then user "user0" gets avatar for type "guest" and id "guest0" And The following headers should be set | Content-Type | image/png | diff --git a/core/Controller/GenericAvatarController.php b/core/Controller/GenericAvatarController.php index 2deeb3282f711..1b6c0ddd7a275 100644 --- a/core/Controller/GenericAvatarController.php +++ b/core/Controller/GenericAvatarController.php @@ -161,6 +161,10 @@ public function setAvatar(string $avatarType, string $avatarId): DataResponse { return new DataResponse([], Http::STATUS_NOT_FOUND); } + if (!$avatarProvider->canBeModifiedByCurrentUser($avatar)) { + return new DataResponse([], Http::STATUS_NOT_FOUND); + } + try { $avatar->set($image); return new DataResponse( @@ -195,6 +199,10 @@ public function deleteAvatar(string $avatarType, string $avatarId): DataResponse return new DataResponse([], Http::STATUS_NOT_FOUND); } + if (!$avatarProvider->canBeModifiedByCurrentUser($avatar)) { + return new DataResponse([], Http::STATUS_NOT_FOUND); + } + try { $avatar->remove(); return new DataResponse(); diff --git a/lib/private/Avatar/GuestAvatarProvider.php b/lib/private/Avatar/GuestAvatarProvider.php index 928c88f03f64d..c395b8af76090 100644 --- a/lib/private/Avatar/GuestAvatarProvider.php +++ b/lib/private/Avatar/GuestAvatarProvider.php @@ -60,6 +60,17 @@ public function canBeAccessedByCurrentUser(IAvatar $avatar): bool { return true; } + /** + * Returns whether the current user can modify the given avatar or not + * + * @param IAvatar $avatar ignored + * @return bool false, as guest avatars can not be modified even by the + * guest of the avatar + */ + public function canBeModifiedByCurrentUser(IAvatar $avatar): bool { + return false; + } + /** * Returns the cache duration for guest avatars in seconds * diff --git a/lib/private/Avatar/UserAvatar.php b/lib/private/Avatar/UserAvatar.php index 421b2eb5cfd37..f6469b3f14964 100644 --- a/lib/private/Avatar/UserAvatar.php +++ b/lib/private/Avatar/UserAvatar.php @@ -79,6 +79,10 @@ public function __construct( $this->config = $config; } + public function getUser(): User { + return $this->user; + } + /** * Check if an avatar exists for the user * diff --git a/lib/private/Avatar/UserAvatarProvider.php b/lib/private/Avatar/UserAvatarProvider.php index fe8f0973ef93e..401358cf56a59 100644 --- a/lib/private/Avatar/UserAvatarProvider.php +++ b/lib/private/Avatar/UserAvatarProvider.php @@ -33,7 +33,9 @@ use OCP\IAvatarProvider; use OCP\IConfig; use OCP\IL10N; +use OCP\IUser; use OCP\IUserManager; +use OCP\IUserSession; use OCP\L10N\IFactory as L10NFactory; use Psr\Log\LoggerInterface; @@ -54,17 +56,22 @@ class UserAvatarProvider implements IAvatarProvider { /** @var IConfig */ private $config; + /** @var IUser|null */ + protected $currentUser; + public function __construct( IUserManager $userManager, AppDataFactory $appDataFactory, L10NFactory $lFactory, LoggerInterface $logger, - IConfig $config) { + IConfig $config, + IUserSession $userSession) { $this->userManager = $userManager; $this->appData = $appDataFactory->get('avatar'); $this->l = $lFactory->get('lib'); $this->logger = $logger; $this->config = $config; + $this->currentUser = $userSession->getUser(); } /** @@ -103,6 +110,26 @@ public function canBeAccessedByCurrentUser(IAvatar $avatar): bool { return true; } + /** + * Returns whether the current user can modify the given avatar or not + * + * @param IAvatar $avatar the avatar to check + * @return bool true if the current user is the user of the avatar, false + * otherwise + * @throws \InvalidArgumentException if the given avatar is not a UserAvatar + */ + public function canBeModifiedByCurrentUser(IAvatar $avatar): bool { + if (!($avatar instanceof UserAvatar)) { + throw new \InvalidArgumentException(); + } + + if (!$this->currentUser) { + return false; + } + + return $avatar->getUser()->getUID() === $this->currentUser->getUID(); + } + /** * Returns the cache duration for user avatars in seconds * diff --git a/lib/public/IAvatarProvider.php b/lib/public/IAvatarProvider.php index 09a50f1ba4466..bbd1cb2b6a01c 100644 --- a/lib/public/IAvatarProvider.php +++ b/lib/public/IAvatarProvider.php @@ -62,6 +62,24 @@ public function getAvatar(string $id): IAvatar; */ public function canBeAccessedByCurrentUser(IAvatar $avatar): bool; + /** + * Returns whether the current user can modify the given avatar or not + * + * Clients of IAvatarProvider should not try to modify the avatar (including + * deletion) if not allowed, but they can ignore it if it makes sense. + * + * Implementers of IAvatarProvider may not throw \InvalidArgumentException + * if the behaviour does not depend on specific avatar instances. + * + * @param IAvatar $avatar the avatar to check + * @return bool true if the current user can modify the avatar, false + * otherwise + * @throws \InvalidArgumentException if the given avatar is not supported by + * this provider + * @since 21.0.0 + */ + public function canBeModifiedByCurrentUser(IAvatar $avatar): bool; + /** * Returns the cache duration in seconds * From 135d14b2b58d9ef0fe6834d2a7b698b4b9c7df60 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Calvi=C3=B1o=20S=C3=A1nchez?= Date: Wed, 16 Dec 2020 01:36:53 +0100 Subject: [PATCH 46/53] Add integration tests for unauthorized modification of avatars MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Daniel Calviño Sánchez --- build/integration/features/avatar.feature | 50 +++++++++++++++++++++++ 1 file changed, 50 insertions(+) diff --git a/build/integration/features/avatar.feature b/build/integration/features/avatar.feature index 7193913572016..5ff33c23e1676 100644 --- a/build/integration/features/avatar.feature +++ b/build/integration/features/avatar.feature @@ -57,6 +57,24 @@ Feature: avatar And last avatar is a square of size 256 And last avatar is a single "#00FF00" color + Scenario: set generic user avatar as another user + When user "user1" sets avatar for type "user" and id "user0" from file "data/green-square-256.png" with "404" + Then user "user0" gets avatar for type "user" and id "user0" + And The following headers should be set + | Content-Type | image/png | + | X-NC-IsCustomAvatar | 0 | + And last avatar is a square of size 128 + And last avatar is not a single color + + Scenario: set generic user avatar as an anonymous user + When user "anonymous" sets avatar for type "user" and id "user0" from file "data/green-square-256.png" with "404" + Then user "user0" gets avatar for type "user" and id "user0" + And The following headers should be set + | Content-Type | image/png | + | X-NC-IsCustomAvatar | 0 | + And last avatar is a square of size 128 + And last avatar is not a single color + Scenario: set non squared image as generic user avatar When user "user0" sets avatar for type "user" and id "user0" from file "data/coloured-pattern.png" with "400" Then user "user0" gets avatar for type "user" and id "user0" @@ -117,6 +135,38 @@ Feature: avatar And last avatar is a square of size 128 And last avatar is not a single color + Scenario: delete generic user avatar as another user + Given user "user0" sets avatar for type "user" and id "user0" from file "data/green-square-256.png" + And user "user0" gets avatar for type "user" and id "user0" with size "256" + And The following headers should be set + | Content-Type | image/png | + | X-NC-IsCustomAvatar | 1 | + And last avatar is a square of size 256 + And last avatar is a single "#00FF00" color + When user "user1" deletes avatar for type "user" and id "user0" with "404" + Then user "user0" gets avatar for type "user" and id "user0" with size "256" + And The following headers should be set + | Content-Type | image/png | + | X-NC-IsCustomAvatar | 1 | + And last avatar is a square of size 256 + And last avatar is a single "#00FF00" color + + Scenario: delete generic user avatar as an anonymous user + Given user "user0" sets avatar for type "user" and id "user0" from file "data/green-square-256.png" + And user "user0" gets avatar for type "user" and id "user0" with size "256" + And The following headers should be set + | Content-Type | image/png | + | X-NC-IsCustomAvatar | 1 | + And last avatar is a square of size 256 + And last avatar is a single "#00FF00" color + When user "anonymous" deletes avatar for type "user" and id "user0" with "404" + Then user "user0" gets avatar for type "user" and id "user0" with size "256" + And The following headers should be set + | Content-Type | image/png | + | X-NC-IsCustomAvatar | 1 | + And last avatar is a square of size 256 + And last avatar is a single "#00FF00" color + Scenario: delete generic guest avatar When user "user0" deletes avatar for type "guest" and id "guest0" with "404" Then user "user0" gets avatar for type "guest" and id "guest0" From 330fac872282eec66ff114cb315b23df9f51908b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Calvi=C3=B1o=20S=C3=A1nchez?= Date: Wed, 16 Dec 2020 03:21:39 +0100 Subject: [PATCH 47/53] Add method to get the version of an avatar MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Daniel Calviño Sánchez --- lib/private/Avatar/GuestAvatarProvider.php | 10 ++++++++++ lib/private/Avatar/UserAvatarProvider.php | 15 +++++++++++++++ lib/public/IAvatarProvider.php | 16 ++++++++++++++++ 3 files changed, 41 insertions(+) diff --git a/lib/private/Avatar/GuestAvatarProvider.php b/lib/private/Avatar/GuestAvatarProvider.php index c395b8af76090..0768fece2980e 100644 --- a/lib/private/Avatar/GuestAvatarProvider.php +++ b/lib/private/Avatar/GuestAvatarProvider.php @@ -71,6 +71,16 @@ public function canBeModifiedByCurrentUser(IAvatar $avatar): bool { return false; } + /** + * Returns the latest value of the avatar version + * + * @param IAvatar $avatar ignored + * @return int 0, as versions are not supported by guest avatars + */ + public function getVersion(IAvatar $avatar): int { + return 0; + } + /** * Returns the cache duration for guest avatars in seconds * diff --git a/lib/private/Avatar/UserAvatarProvider.php b/lib/private/Avatar/UserAvatarProvider.php index 401358cf56a59..ecdeccb032efa 100644 --- a/lib/private/Avatar/UserAvatarProvider.php +++ b/lib/private/Avatar/UserAvatarProvider.php @@ -130,6 +130,21 @@ public function canBeModifiedByCurrentUser(IAvatar $avatar): bool { return $avatar->getUser()->getUID() === $this->currentUser->getUID(); } + /** + * Returns the latest value of the avatar version + * + * @param IAvatar $avatar the avatar to check + * @return int the latest value of the avatar version + * @throws \InvalidArgumentException if the given avatar is not a UserAvatar + */ + public function getVersion(IAvatar $avatar): int { + if (!($avatar instanceof UserAvatar)) { + throw new \InvalidArgumentException(); + } + + return (int) $this->config->getUserValue($avatar->getUser()->getUID(), 'avatar', 'version', 0); + } + /** * Returns the cache duration for user avatars in seconds * diff --git a/lib/public/IAvatarProvider.php b/lib/public/IAvatarProvider.php index bbd1cb2b6a01c..a20c0c2b06b8f 100644 --- a/lib/public/IAvatarProvider.php +++ b/lib/public/IAvatarProvider.php @@ -80,6 +80,22 @@ public function canBeAccessedByCurrentUser(IAvatar $avatar): bool; */ public function canBeModifiedByCurrentUser(IAvatar $avatar): bool; + /** + * Returns the latest value of the avatar version + * + * Implementers of IAvatarProvider may not throw \InvalidArgumentException + * if the behaviour does not depend on specific avatar instances (for + * example, if versions are not supported and the same version is always + * returned). + * + * @param IAvatar $avatar the avatar to check + * @return int the latest value of the avatar version + * @throws \InvalidArgumentException if the given avatar is not supported by + * this provider + * @since 21.0.0 + */ + public function getVersion(IAvatar $avatar): int; + /** * Returns the cache duration in seconds * From 68fa0457ec58d4c1c2737e51376c9ad9e2e71127 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Calvi=C3=B1o=20S=C3=A1nchez?= Date: Wed, 16 Dec 2020 04:22:36 +0100 Subject: [PATCH 48/53] Limit the returned sizes of the avatars to a predefined set MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Daniel Calviño Sánchez --- core/Controller/GenericAvatarController.php | 37 +++++- .../GenericAvatarControllerTest.php | 109 ++++++++++++++++++ 2 files changed, 140 insertions(+), 6 deletions(-) create mode 100644 tests/Core/Controller/GenericAvatarControllerTest.php diff --git a/core/Controller/GenericAvatarController.php b/core/Controller/GenericAvatarController.php index 1b6c0ddd7a275..d98912633881f 100644 --- a/core/Controller/GenericAvatarController.php +++ b/core/Controller/GenericAvatarController.php @@ -70,12 +70,7 @@ public function __construct($appName, * @return DataResponse|FileDisplayResponse */ public function getAvatar(string $avatarType, string $avatarId, int $size): Response { - // min/max size - if ($size > 2048) { - $size = 2048; - } elseif ($size <= 0) { - $size = 64; - } + $size = $this->sanitizeSize($size); try { $avatarProvider = $this->avatarManager->getAvatarProvider($avatarType); @@ -112,6 +107,36 @@ public function getAvatar(string $avatarType, string $avatarId, int $size): Resp return $response; } + /** + * Returns the closest value to the predefined set of sizes + * + * @param int $size the size to sanitize + * @return int the sanitized size + */ + private function sanitizeSize(int $size): int { + $validSizes = [64, 128, 256, 512]; + + if ($size < $validSizes[0]) { + return $validSizes[0]; + } + + if ($size > $validSizes[count($validSizes) - 1]) { + return $validSizes[count($validSizes) - 1]; + } + + for ($i = 0; $i < count($validSizes) - 1; $i++) { + if ($size >= $validSizes[$i] && $size <= $validSizes[$i + 1]) { + $middlePoint = ($validSizes[$i] + $validSizes[$i + 1]) / 2; + if ($size < $middlePoint) { + return $validSizes[$i]; + } + return $validSizes[$i + 1]; + } + } + + return $size; + } + /** * @PublicPage * diff --git a/tests/Core/Controller/GenericAvatarControllerTest.php b/tests/Core/Controller/GenericAvatarControllerTest.php new file mode 100644 index 0000000000000..005aa2c39aa01 --- /dev/null +++ b/tests/Core/Controller/GenericAvatarControllerTest.php @@ -0,0 +1,109 @@ + + * + * @license GNU AGPL version 3 or any later version + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU Affero General Public License as + * published by the Free Software Foundation, either version 3 of the + * License, or (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Affero General Public License for more details. + * + * You should have received a copy of the GNU Affero General Public License + * along with this program. If not, see . + * + */ + +namespace Tests\Core\Controller; + +use OC\Core\Controller\GenericAvatarController; +use OCP\IAvatarManager; +use OCP\IL10N; +use OCP\IRequest; +use Psr\Log\LoggerInterface; + +class GenericAvatarControllerTest extends \Test\TestCase { + + /** @var GenericAvatarController */ + private $genericAvatarController; + + /** @var IRequest|\PHPUnit\Framework\MockObject\MockObject */ + private $request; + /** @var IAvatarManager|\PHPUnit\Framework\MockObject\MockObject */ + private $avatarManager; + /** @var IL10N|\PHPUnit\Framework\MockObject\MockObject */ + private $l; + /** @var LoggerInterface|\PHPUnit\Framework\MockObject\MockObject */ + private $logger; + + protected function setUp(): void { + parent::setUp(); + + $this->request = $this->getMockBuilder(IRequest::class)->getMock(); + $this->avatarManager = $this->getMockBuilder('OCP\IAvatarManager')->getMock(); + $this->l = $this->getMockBuilder(IL10N::class)->getMock(); + $this->l->method('t')->willReturnArgument(0); + $this->logger = $this->getMockBuilder(LoggerInterface::class)->getMock(); + + $this->genericAvatarController = new GenericAvatarController( + 'core', + $this->request, + $this->avatarManager, + $this->l, + $this->logger + ); + } + + public function dataSanitizeSize(): array { + return [ + [-1, 64], + [32, 64], + + [64, 64], + [65, 64], + + [95, 64], + [96, 128], + + [127, 128], + [128, 128], + [129, 128], + + [191, 128], + [192, 256], + + [255, 256], + [256, 256], + [257, 256], + + [383, 256], + [384, 512], + + [511, 512], + [512, 512], + + [8192, 512], + + ]; + } + + /** + * @dataProvider dataSanitizeSize + * + * @param int $inputSize + * @param int $expectedSize + */ + public function testSanitizeSize(int $inputSize, int $expectedSize) { + $this->assertEquals($expectedSize, $this->invokePrivate($this->genericAvatarController, 'sanitizeSize', [$inputSize])); + } +} From 601d74178a56462f66596a0ad0cf1d3e9f5e24e3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Calvi=C3=B1o=20S=C3=A1nchez?= Date: Wed, 16 Dec 2020 04:31:49 +0100 Subject: [PATCH 49/53] fixup! Get the avatar provider rather than the avatar itself from the manager MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Daniel Calviño Sánchez --- lib/private/Avatar/AvatarManager.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/private/Avatar/AvatarManager.php b/lib/private/Avatar/AvatarManager.php index 33fd5f9946ab4..899e6f5619b1b 100644 --- a/lib/private/Avatar/AvatarManager.php +++ b/lib/private/Avatar/AvatarManager.php @@ -147,7 +147,7 @@ public function getAvatarProvider(string $type): IAvatarProvider { $context = $this->bootstrapCoordinator->getRegistrationContext(); if ($context === null) { - throw new RuntimeException("Avatar provider requested before the apps had been fully registered"); + throw new \RuntimeException("Avatar provider requested before the apps had been fully registered"); } $providerClasses = $context->getAvatarProviders(); From 6391a2b742351cf062f4085d0c6a3dcb7c8eb7bc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Calvi=C3=B1o=20S=C3=A1nchez?= Date: Wed, 16 Dec 2020 04:33:31 +0100 Subject: [PATCH 50/53] fixup! Add explicit providers for user and guest avatars MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Daniel Calviño Sánchez --- lib/private/Avatar/UserAvatarProvider.php | 1 + 1 file changed, 1 insertion(+) diff --git a/lib/private/Avatar/UserAvatarProvider.php b/lib/private/Avatar/UserAvatarProvider.php index ecdeccb032efa..65d71a5d2096b 100644 --- a/lib/private/Avatar/UserAvatarProvider.php +++ b/lib/private/Avatar/UserAvatarProvider.php @@ -28,6 +28,7 @@ use OC\Files\AppData\Factory as AppDataFactory; use OCP\AvatarProviderException; +use OCP\Files\IAppData; use OCP\Files\NotFoundException; use OCP\IAvatar; use OCP\IAvatarProvider; From 029d22cb9fc1347cec836079d6fb071828efac54 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Calvi=C3=B1o=20S=C3=A1nchez?= Date: Wed, 16 Dec 2020 04:44:39 +0100 Subject: [PATCH 51/53] fixup! Add method to check if an avatar can be modified by the current user MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Daniel Calviño Sánchez --- tests/lib/Avatar/UserAvatarProviderTest.php | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/tests/lib/Avatar/UserAvatarProviderTest.php b/tests/lib/Avatar/UserAvatarProviderTest.php index 2432fd1401d1e..03d98df2d3312 100644 --- a/tests/lib/Avatar/UserAvatarProviderTest.php +++ b/tests/lib/Avatar/UserAvatarProviderTest.php @@ -34,6 +34,7 @@ use OCP\IL10N; use OCP\IUser; use OCP\IUserManager; +use OCP\IUserSession; use OCP\L10N\IFactory as L10NFactory; use Psr\Log\LoggerInterface; @@ -51,6 +52,8 @@ class UserAvatarProviderTest extends \Test\TestCase { private $logger; /** @var IConfig|\PHPUnit\Framework\MockObject\MockObject */ private $config; + /** @var IUser|\PHPUnit\Framework\MockObject\MockObject */ + private $currentUser; /** @var UserAvatarProvider | \PHPUnit\Framework\MockObject\MockObject */ private $userAvatarProvider; @@ -65,6 +68,7 @@ protected function setUp(): void { $this->l10n = $this->createMock(IL10N::class); $this->logger = $this->createMock(LoggerInterface::class); $this->config = $this->createMock(IConfig::class); + $this->currentUser = $this->createMock(IUser::class); $appDataFactory = $this->createMock(AppDataFactory::class); $appDataFactory @@ -76,13 +80,18 @@ protected function setUp(): void { ->method('get') ->with('lib') ->willReturn($this->l10n); + $userSession = $this->createMock(IUserSession::class); + $userSession + ->method('getUser') + ->willReturn($this->currentUser); $this->userAvatarProvider = new UserAvatarProvider( $this->userManager, $appDataFactory, $l10nFactory, $this->logger, - $this->config + $this->config, + $userSession ); } From 3a4b92673c078abf3bd4e50b762fae7a5521d458 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Calvi=C3=B1o=20S=C3=A1nchez?= Date: Wed, 16 Dec 2020 04:48:16 +0100 Subject: [PATCH 52/53] fixup! Add explicit providers for user and guest avatars MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Daniel Calviño Sánchez --- tests/lib/Avatar/UserAvatarProviderTest.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/lib/Avatar/UserAvatarProviderTest.php b/tests/lib/Avatar/UserAvatarProviderTest.php index 03d98df2d3312..40c15366946fa 100644 --- a/tests/lib/Avatar/UserAvatarProviderTest.php +++ b/tests/lib/Avatar/UserAvatarProviderTest.php @@ -98,7 +98,7 @@ protected function setUp(): void { public function testGetAvatarInvalidUser() { $this->expectException(\Exception::class); - $this->expectExceptionMessage('user does not exist'); + $this->expectExceptionMessage('user invalidUser does not exist'); $this->userManager ->expects($this->once()) From 6895199a7aacdd442af15f250251c38708c6b0eb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Calvi=C3=B1o=20S=C3=A1nchez?= Date: Wed, 16 Dec 2020 04:50:14 +0100 Subject: [PATCH 53/53] fixup! Make possible to define custom avatar types MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Daniel Calviño Sánchez --- lib/public/AvatarProviderException.php | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/lib/public/AvatarProviderException.php b/lib/public/AvatarProviderException.php index 57bebda95ccdb..87380d15fa2ff 100644 --- a/lib/public/AvatarProviderException.php +++ b/lib/public/AvatarProviderException.php @@ -33,6 +33,11 @@ */ class AvatarProviderException extends \RuntimeException { + /** + * @param string $message + * @param int $code + * @param \Exception $previous + */ public function __construct(string $message = "", int $code = 0, \Exception $previous = null) { parent::__construct($message, $code, $previous); }