Added testing for avatar fetching systems & config

Abstracts imageservice http interaction.
Closes #1193

diff --git a/app/Exceptions/HttpFetchException.php b/app/Exceptions/HttpFetchException.php
new file mode 100644 (file)
index 0000000..48e30e1
--- /dev/null
+++ b/app/Exceptions/HttpFetchException.php
@@ -0,0 +1,5 @@
+<?php namespace BookStack\Exceptions;
+
+use Exception;
+
+class HttpFetchException extends Exception {}

diff --git a/app/Providers/CustomFacadeProvider.php b/app/Providers/CustomFacadeProvider.php
index 98b242d219f7bac8d2f8ef6403d3f04dfd88067d..5508ee9cd3e47920a10d68e96ee496592b8217db 100644 (file)
--- a/app/Providers/CustomFacadeProvider.php
+++ b/app/Providers/CustomFacadeProvider.php
@@ -9,6 +9,7 @@ use BookStack\Actions\ViewService;
 use BookStack\Auth\Permissions\PermissionService;
 use BookStack\Settings\Setting;
 use BookStack\Settings\SettingService;
 use BookStack\Auth\Permissions\PermissionService;
 use BookStack\Settings\Setting;
 use BookStack\Settings\SettingService;

+use BookStack\Uploads\HttpFetcher;

 use BookStack\Uploads\Image;
 use BookStack\Uploads\ImageService;
 use Illuminate\Contracts\Cache\Repository;
 use BookStack\Uploads\Image;
 use BookStack\Uploads\ImageService;
 use Illuminate\Contracts\Cache\Repository;


@@ -61,7 +62,8 @@ class CustomFacadeProvider extends ServiceProvider
                 $this->app->make(Image::class),
                 $this->app->make(ImageManager::class),
                 $this->app->make(Factory::class),
                 $this->app->make(Image::class),
                 $this->app->make(ImageManager::class),
                 $this->app->make(Factory::class),

-                $this->app->make(Repository::class)
+                $this->app->make(Repository::class),
+                $this->app->make(HttpFetcher::class)

             );
         });
     }
             );
         });
     }

diff --git a/app/Uploads/HttpFetcher.php b/app/Uploads/HttpFetcher.php
new file mode 100644 (file)
index 0000000..3ebe17e
--- /dev/null
+++ b/app/Uploads/HttpFetcher.php
@@ -0,0 +1,34 @@
+<?php namespace BookStack\Uploads;
+
+use BookStack\Exceptions\HttpFetchException;
+
+class HttpFetcher
+{
+
+    /**
+     * Fetch content from an external URI.
+     * @param string $uri
+     * @return bool|string
+     * @throws HttpFetchException
+     */
+    public function fetch(string $uri)
+    {
+        $ch = curl_init();
+        curl_setopt_array($ch, [
+            CURLOPT_URL => $uri,
+            CURLOPT_RETURNTRANSFER => 1,
+            CURLOPT_CONNECTTIMEOUT => 5
+        ]);
+
+        $data = curl_exec($ch);
+        $err = curl_error($ch);
+        curl_close($ch);
+
+        if ($err) {
+            throw new HttpFetchException($err);
+        }
+
+        return $data;
+    }
+
+}
\ No newline at end of file

diff --git a/app/Uploads/ImageService.php b/app/Uploads/ImageService.php
index d5f4068ef26e64a5bf19b508f8ea1ec8d8b1a0d7..1dd8b713d66747ec57110733f2a00702828cf0f5 100644 (file)
--- a/app/Uploads/ImageService.php
+++ b/app/Uploads/ImageService.php
@@ -1,6 +1,7 @@
 <?php namespace BookStack\Uploads;
 
 use BookStack\Auth\User;
 <?php namespace BookStack\Uploads;
 
 use BookStack\Auth\User;

+use BookStack\Exceptions\HttpFetchException;

 use BookStack\Exceptions\ImageUploadException;
 use DB;
 use Exception;
 use BookStack\Exceptions\ImageUploadException;
 use DB;
 use Exception;


@@ -17,6 +18,7 @@ class ImageService extends UploadService
     protected $cache;
     protected $storageUrl;
     protected $image;
     protected $cache;
     protected $storageUrl;
     protected $image;

+    protected $http;

 
     /**
      * ImageService constructor.
 
     /**
      * ImageService constructor.


@@ -24,12 +26,14 @@ class ImageService extends UploadService
      * @param ImageManager $imageTool
      * @param FileSystem $fileSystem
      * @param Cache $cache
      * @param ImageManager $imageTool
      * @param FileSystem $fileSystem
      * @param Cache $cache

+     * @param HttpFetcher $http

      */
      */

-    public function __construct(Image $image, ImageManager $imageTool, FileSystem $fileSystem, Cache $cache)
+    public function __construct(Image $image, ImageManager $imageTool, FileSystem $fileSystem, Cache $cache, HttpFetcher $http)

     {
         $this->image = $image;
         $this->imageTool = $imageTool;
         $this->cache = $cache;
     {
         $this->image = $image;
         $this->imageTool = $imageTool;
         $this->cache = $cache;

+        $this->http = $http;

         parent::__construct($fileSystem);
     }
 
         parent::__construct($fileSystem);
     }
 


@@ -95,8 +99,9 @@ class ImageService extends UploadService
     private function saveNewFromUrl($url, $type, $imageName = false)
     {
         $imageName = $imageName ? $imageName : basename($url);
     private function saveNewFromUrl($url, $type, $imageName = false)
     {
         $imageName = $imageName ? $imageName : basename($url);

-        $imageData = file_get_contents($url);
-        if ($imageData === false) {
+        try {
+            $imageData = $this->http->fetch($url);
+        } catch (HttpFetchException $exception) {

             throw new \Exception(trans('errors.cannot_get_image_from_url', ['url' => $url]));
         }
         return $this->saveNew($imageName, $imageData, $type);
             throw new \Exception(trans('errors.cannot_get_image_from_url', ['url' => $url]));
         }
         return $this->saveNew($imageName, $imageData, $type);


@@ -322,7 +327,13 @@ class ImageService extends UploadService
      */
     protected function getAvatarUrl()
     {
      */
     protected function getAvatarUrl()
     {

-        return trim(config('services.avatar_url'));
+        $url = trim(config('services.avatar_url'));
+
+        if (empty($url) && !config('services.disable_services')) {
+            $url = 'https://www.gravatar.com/avatar/${hash}?s=${size}&d=identicon';
+        }
+
+        return $url;

     }
 
     /**
     }
 
     /**


@@ -392,14 +403,7 @@ class ImageService extends UploadService
             }
         } else {
             try {
             }
         } else {
             try {

-                $ch = curl_init();
-                curl_setopt_array($ch, [CURLOPT_URL => $uri, CURLOPT_RETURNTRANSFER => 1, CURLOPT_CONNECTTIMEOUT => 5]);
-                $imageData = curl_exec($ch);
-                $err = curl_error($ch);
-                curl_close($ch);
-                if ($err) {
-                    throw new \Exception("Image fetch failed, Received error: " . $err);
-                }
+                $imageData = $this->http->fetch($uri);

             } catch (\Exception $e) {
             }
         }
             } catch (\Exception $e) {
             }
         }

diff --git a/composer.json b/composer.json
index 2850eb235ba616f372f69897547243a968e28b50..48b977e23ffc8b34b4b9447204568e59ea643426 100644 (file)
--- a/composer.json
+++ b/composer.json
@@ -12,6 +12,7 @@
         "ext-xml": "*",
         "ext-mbstring": "*",
         "ext-gd": "*",
         "ext-xml": "*",
         "ext-mbstring": "*",
         "ext-gd": "*",

+        "ext-curl": "*",

         "laravel/framework": "~5.5.44",
         "fideloper/proxy": "~3.3",
         "intervention/image": "^2.4",
         "laravel/framework": "~5.5.44",
         "fideloper/proxy": "~3.3",
         "intervention/image": "^2.4",

diff --git a/config/services.php b/config/services.php
index 7b9cf4e74b4da822882afb68058badf081b371cc..a07fab23f14dbe317704d6f19421562e68d72607 100644 (file)
--- a/config/services.php
+++ b/config/services.php
@@ -21,9 +21,7 @@ return [
     'drawio' => env('DRAWIO', !env('DISABLE_EXTERNAL_SERVICES', false)),
 
     // URL for fetching avatars
     'drawio' => env('DRAWIO', !env('DISABLE_EXTERNAL_SERVICES', false)),
 
     // URL for fetching avatars

-    'avatar_url' => env('AVATAR_URL',
-        env('DISABLE_EXTERNAL_SERVICES', false) ? false : 'https://www.gravatar.com/avatar/${hash}?s=${size}&d=identicon'
-    ),
+    'avatar_url' => env('AVATAR_URL', ''),

 
     // Callback URL for social authentication methods
     'callback_url' => env('APP_URL', false),
 
     // Callback URL for social authentication methods
     'callback_url' => env('APP_URL', false),

diff --git a/phpunit.xml b/phpunit.xml
index 3d18d9bbf2782ba7fdfa5b403099cf122251e0fd..804afcf5d3ed2ab681792aaffbea10e168a0e1ce 100644 (file)
--- a/phpunit.xml
+++ b/phpunit.xml
@@ -31,6 +31,7 @@
         <env name="MAIL_DRIVER" value="log"/>
         <env name="AUTH_METHOD" value="standard"/>
         <env name="DISABLE_EXTERNAL_SERVICES" value="true"/>
         <env name="MAIL_DRIVER" value="log"/>
         <env name="AUTH_METHOD" value="standard"/>
         <env name="DISABLE_EXTERNAL_SERVICES" value="true"/>

+        <env name="AVATAR_URL" value=""/>

         <env name="LDAP_VERSION" value="3"/>
         <env name="STORAGE_TYPE" value="local"/>
         <env name="GITHUB_APP_ID" value="aaaaaaaaaaaaaa"/>
         <env name="LDAP_VERSION" value="3"/>
         <env name="STORAGE_TYPE" value="local"/>
         <env name="GITHUB_APP_ID" value="aaaaaaaaaaaaaa"/>

diff --git a/tests/AttachmentTest.php b/tests/Uploads/AttachmentTest.php
similarity index 100%
rename from tests/AttachmentTest.php
rename to tests/Uploads/AttachmentTest.php

diff --git a/tests/Uploads/AvatarTest.php b/tests/Uploads/AvatarTest.php
new file mode 100644 (file)
index 0000000..ecf7037
--- /dev/null
+++ b/tests/Uploads/AvatarTest.php
@@ -0,0 +1,84 @@
+<?php namespace Tests\Uploads;
+
+use BookStack\Auth\User;
+use BookStack\Uploads\HttpFetcher;
+use Tests\TestCase;
+
+class AvatarTest extends TestCase
+{
+    use UsesImages;
+
+
+    protected function createUserRequest($user)
+    {
+        $resp = $this->asAdmin()->post('/settings/users/create', [
+            'name' => $user->name,
+            'email' => $user->email,
+            'password' => 'testing',
+            'password-confirm' => 'testing',
+        ]);
+        return User::where('email', '=', $user->email)->first();
+    }
+
+    protected function assertImageFetchFrom(string $url)
+    {
+        $http = \Mockery::mock(HttpFetcher::class);
+        $this->app->instance(HttpFetcher::class, $http);
+
+        $http->shouldReceive('fetch')
+            ->once()->with($url)
+            ->andReturn($this->getTestImageContent());
+    }
+
+    protected function deleteUserImage(User $user)
+    {
+        $this->deleteImage($user->avatar->path);
+    }
+
+    public function test_gravatar_fetched_on_user_create()
+    {
+        config()->set([
+            'services.disable_services' => false,
+        ]);
+        $user = factory(User::class)->make();
+        $this->assertImageFetchFrom('https://www.gravatar.com/avatar/'.md5(strtolower($user->email)).'?s=500&d=identicon');
+
+        $user = $this->createUserRequest($user);
+        $this->assertDatabaseHas('images', [
+            'type' => 'user',
+            'created_by' => $user->id
+        ]);
+        $this->deleteUserImage($user);
+    }
+
+
+    public function test_custom_url_used_if_set()
+    {
+        config()->set([
+            'services.avatar_url' => 'https://example.com/${email}/${hash}/${size}',
+        ]);
+
+        $user = factory(User::class)->make();
+        $url = 'https://example.com/'. urlencode(strtolower($user->email)) .'/'. md5(strtolower($user->email)).'/500';
+        $this->assertImageFetchFrom($url);
+
+        $user = $this->createUserRequest($user);
+        $this->deleteUserImage($user);
+    }
+
+    public function test_avatar_not_fetched_if_no_custom_url_and_services_disabled()
+    {
+        config()->set([
+            'services.disable_services' => true,
+        ]);
+
+        $user = factory(User::class)->make();
+
+        $http = \Mockery::mock(HttpFetcher::class);
+        $this->app->instance(HttpFetcher::class, $http);
+        $http->shouldNotReceive('fetch');
+
+        $this->createUserRequest($user);
+    }
+
+}

diff --git a/tests/ImageTest.php b/tests/Uploads/ImageTest.php
similarity index 85%
rename from tests/ImageTest.php
rename to tests/Uploads/ImageTest.php
index 38bac2ccacb07e9f6514e6167c9aceb00ea9d716..6dafa7f5a870bf553058cb9f42f9489c7d766b63 100644 (file)
--- a/tests/ImageTest.php
+++ b/tests/Uploads/ImageTest.php
@@ -1,67 +1,15 @@
-<?php namespace Tests;
+<?php namespace Tests\Uploads;

 
 use BookStack\Entities\Repos\PageRepo;
 use BookStack\Uploads\Image;
 use BookStack\Entities\Page;
 
 use BookStack\Entities\Repos\PageRepo;
 use BookStack\Uploads\Image;
 use BookStack\Entities\Page;

-use BookStack\Entities\Repos\EntityRepo;

 use BookStack\Uploads\ImageService;
 use BookStack\Uploads\ImageService;

+use Tests\TestCase;

 
 class ImageTest extends TestCase
 {
 
 class ImageTest extends TestCase
 {

-    /**
-     * Get the path to our basic test image.
-     * @return string
-     */
-    protected function getTestImageFilePath()
-    {
-        return base_path('tests/test-data/test-image.png');
-    }
-
-    /**
-     * Get a test image that can be uploaded
-     * @param $fileName
-     * @return \Illuminate\Http\UploadedFile
-     */
-    protected function getTestImage($fileName)
-    {
-        return new \Illuminate\Http\UploadedFile($this->getTestImageFilePath(), $fileName, 'image/png', 5238);
-    }
-
-    /**
-     * Get the path for a test image.
-     * @param $type
-     * @param $fileName
-     * @return string
-     */
-    protected function getTestImagePath($type, $fileName)
-    {
-        return '/uploads/images/' . $type . '/' . Date('Y-m-M') . '/' . $fileName;
-    }
-
-    /**
-     * Uploads an image with the given name.
-     * @param $name
-     * @param int $uploadedTo
-     * @return \Illuminate\Foundation\Testing\TestResponse
-     */
-    protected function uploadImage($name, $uploadedTo = 0)
-    {
-        $file = $this->getTestImage($name);
-        return $this->call('POST', '/images/gallery/upload', ['uploaded_to' => $uploadedTo], [], ['file' => $file], []);
-    }
-
-    /**
-     * Delete an uploaded image.
-     * @param $relPath
-     */
-    protected function deleteImage($relPath)
-    {
-        $path = public_path($relPath);
-        if (file_exists($path)) {
-            unlink($path);
-        }
-    }

 
 

+    use UsesImages;

 
     public function test_image_upload()
     {
 
     public function test_image_upload()
     {

diff --git a/tests/Uploads/UsesImages.php b/tests/Uploads/UsesImages.php
new file mode 100644 (file)
index 0000000..16cb7c2
--- /dev/null
+++ b/tests/Uploads/UsesImages.php
@@ -0,0 +1,69 @@
+<?php namespace Tests\Uploads;
+
+
+trait UsesImages
+{
+    /**
+     * Get the path to our basic test image.
+     * @return string
+     */
+    protected function getTestImageFilePath()
+    {
+        return base_path('tests/test-data/test-image.png');
+    }
+
+    /**
+     * Get a test image that can be uploaded
+     * @param $fileName
+     * @return \Illuminate\Http\UploadedFile
+     */
+    protected function getTestImage($fileName)
+    {
+        return new \Illuminate\Http\UploadedFile($this->getTestImageFilePath(), $fileName, 'image/png', 5238);
+    }
+
+    /**
+     * Get the raw file data for the test image.
+     * @return false|string
+     */
+    protected function getTestImageContent()
+    {
+        return file_get_contents($this->getTestImageFilePath());
+    }
+
+    /**
+     * Get the path for a test image.
+     * @param $type
+     * @param $fileName
+     * @return string
+     */
+    protected function getTestImagePath($type, $fileName)
+    {
+        return '/uploads/images/' . $type . '/' . Date('Y-m-M') . '/' . $fileName;
+    }
+
+    /**
+     * Uploads an image with the given name.
+     * @param $name
+     * @param int $uploadedTo
+     * @return \Illuminate\Foundation\Testing\TestResponse
+     */
+    protected function uploadImage($name, $uploadedTo = 0)
+    {
+        $file = $this->getTestImage($name);
+        return $this->call('POST', '/images/gallery/upload', ['uploaded_to' => $uploadedTo], [], ['file' => $file], []);
+    }
+
+    /**
+     * Delete an uploaded image.
+     * @param $relPath
+     */
+    protected function deleteImage($relPath)
+    {
+        $path = public_path($relPath);
+        if (file_exists($path)) {
+            unlink($path);
+        }
+    }
+
+}
\ No newline at end of file