Started rewriting back-end image managment

2024-10-01 05:36:00 +00:00 · 2019-04-21 15:52:29 +01:00 · 2019-04-21 15:52:29 +01:00 · aeb1fc4d49
commit aeb1fc4d49
parent 6428f32483
8 changed files with 236 additions and 68 deletions
--- a/app/Auth/UserRepo.php
+++ b/app/Auth/UserRepo.php
@ -198,7 +198,7 @@ class UserRepo
        $user->delete();
        
        // Delete user profile images
-        $profileImages = $images = Image::where('type', '=', 'user')->where('created_by', '=', $user->id)->get();
+        $profileImages = Image::where('type', '=', 'user')->where('uploaded_to', '=', $user->id)->get();
        foreach ($profileImages as $image) {
            Images::destroy($image);
        }
--- a/app/Http/Controllers/ImageController.php
+++ b/app/Http/Controllers/ImageController.php
@ -45,13 +45,21 @@ class ImageController extends Controller

    /**
     * Get all images for a specific type, Paginated
+     * @param Request $request
     * @param string $type
     * @param int $page
     * @return \Illuminate\Http\JsonResponse
     */
-    public function getAllByType($type, $page = 0)
+    public function getAllByType(Request $request, $type, $page = 0)
    {
-        $imgData = $this->imageRepo->getPaginatedByType($type, $page);
+        $uploadedToFilter = $request->get('uploaded_to', null);
+
+        // For user profile request, check access to user images
+        if ($type === 'user') {
+            $this->checkPermissionOrCurrentUser('users-manage', $uploadedToFilter ?? 0);
+        }
+
+        $imgData = $this->imageRepo->getPaginatedByType($type, $page, 24, $uploadedToFilter);
        return response()->json($imgData);
    }

@ -73,17 +81,6 @@ class ImageController extends Controller
        return response()->json($imgData);
    }

-    /**
-     * Get all images for a user.
-     * @param int $page
-     * @return \Illuminate\Http\JsonResponse
-     */
-    public function getAllForUserType($page = 0)
-    {
-        $imgData = $this->imageRepo->getPaginatedByType('user', $page, 24, $this->currentUser->id);
-        return response()->json($imgData);
-    }
-
    /**
     * Get gallery images with a specific filter such as book or page
     * @param $filter
@ -94,7 +91,7 @@ class ImageController extends Controller
    public function getGalleryFiltered(Request $request, $filter, $page = 0)
    {
        $this->validate($request, [
-            'page_id' => 'required|integer'
+            'uploaded_to' => 'required|integer'
        ]);

        $validFilters = collect(['page', 'book']);
@ -102,12 +99,57 @@ class ImageController extends Controller
            return response('Invalid filter', 500);
        }

-        $pageId = $request->get('page_id');
+        $pageId = $request->get('uploaded_to');
        $imgData = $this->imageRepo->getGalleryFiltered(strtolower($filter), $pageId, $page, 24);

        return response()->json($imgData);
    }

+    public function uploadGalleryImage(Request $request)
+    {
+        // TODO
+    }
+
+    public function uploadUserImage(Request $request)
+    {
+        // TODO
+    }
+
+    public function uploadSystemImage(Request $request)
+    {
+        // TODO
+    }
+
+    public function uploadCoverImage(Request $request)
+    {
+        // TODO
+    }
+
+    /**
+     * Upload a draw.io image into the system.
+     * @param Request $request
+     * @return \Illuminate\Contracts\Routing\ResponseFactory|\Illuminate\Http\JsonResponse|\Symfony\Component\HttpFoundation\Response
+     */
+    public function uploadDrawioImage(Request $request)
+    {
+        $this->validate($request, [
+            'image' => 'required|string',
+            'uploaded_to' => 'required|integer'
+        ]);
+        $uploadedTo = $request->get('uploaded_to', 0);
+        $page = $this->
+        $this->checkPermission('image-create-all');
+        $imageBase64Data = $request->get('image');
+
+        try {
+            $image = $this->imageRepo->saveDrawing($imageBase64Data, $uploadedTo);
+        } catch (ImageUploadException $e) {
+            return response($e->getMessage(), 500);
+        }
+
+        return response()->json($image);
+    }
+
    /**
     * Handles image uploads for use on pages.
     * @param string $type
@ -130,6 +172,12 @@ class ImageController extends Controller

        try {
            $uploadedTo = $request->get('uploaded_to', 0);
+
+            // For user profile request, check access to user images
+            if ($type === 'user') {
+                $this->checkPermissionOrCurrentUser('users-manage', $uploadedTo ?? 0);
+            }
+
            $image = $this->imageRepo->saveNew($imageUpload, $type, $uploadedTo);
        } catch (ImageUploadException $e) {
            return response($e->getMessage(), 500);
@ -137,31 +185,6 @@ class ImageController extends Controller

        return response()->json($image);
    }
-
-    /**
-     * Upload a drawing to the system.
-     * @param Request $request
-     * @return \Illuminate\Contracts\Routing\ResponseFactory|\Illuminate\Http\JsonResponse|\Symfony\Component\HttpFoundation\Response
-     */
-    public function uploadDrawing(Request $request)
-    {
-        $this->validate($request, [
-            'image' => 'required|string',
-            'uploaded_to' => 'required|integer'
-        ]);
-        $this->checkPermission('image-create-all');
-        $imageBase64Data = $request->get('image');
-
-        try {
-            $uploadedTo = $request->get('uploaded_to', 0);
-            $image = $this->imageRepo->saveDrawing($imageBase64Data, $uploadedTo);
-        } catch (ImageUploadException $e) {
-            return response($e->getMessage(), 500);
-        }
-
-        return response()->json($image);
-    }
-
    /**
     * Get the content of an image based64 encoded.
     * @param $id
@ -199,19 +222,21 @@ class ImageController extends Controller

    /**
     * Update image details
-     * @param integer $imageId
+     * @param integer $id
     * @param Request $request
     * @return \Illuminate\Http\JsonResponse
     * @throws ImageUploadException
     * @throws \Exception
     */
-    public function update($imageId, Request $request)
+    public function update($id, Request $request)
    {
        $this->validate($request, [
            'name' => 'required|min:2|string'
        ]);
-        $image = $this->imageRepo->getById($imageId);
+
+        $image = $this->imageRepo->getById($id);
        $this->checkOwnablePermission('image-update', $image);
+
        $image = $this->imageRepo->updateImageDetails($image, $request->all());
        return response()->json($image);
    }
--- a/app/Uploads/ImageRepo.php
+++ b/app/Uploads/ImageRepo.php
@ -2,6 +2,7 @@

 use BookStack\Auth\Permissions\PermissionService;
 use BookStack\Entities\Page;
+use BookStack\Http\Requests\Request;
 use Symfony\Component\HttpFoundation\File\UploadedFile;

 class ImageRepo
@ -44,12 +45,12 @@ class ImageRepo
     * @param $query
     * @param int $page
     * @param int $pageSize
+     * @param bool $filterOnPage
     * @return array
     */
    private function returnPaginated($query, $page = 0, $pageSize = 24)
    {
-        $images = $this->restrictionService->filterRelatedPages($query, 'images', 'uploaded_to');
-        $images = $images->orderBy('created_at', 'desc')->skip($pageSize * $page)->take($pageSize + 1)->get();
+        $images = $query->orderBy('created_at', 'desc')->skip($pageSize * $page)->take($pageSize + 1)->get();
        $hasMore = count($images) > $pageSize;

        $returnImages = $images->take(24);
@ -68,15 +69,20 @@ class ImageRepo
     * @param string $type
     * @param int $page
     * @param int $pageSize
-     * @param bool|int $userFilter
+     * @param int $uploadedTo
     * @return array
     */
-    public function getPaginatedByType($type, $page = 0, $pageSize = 24, $userFilter = false)
+    public function getPaginatedByType(string $type, int $page = 0, int $pageSize = 24, int $uploadedTo = null)
    {
-        $images = $this->image->where('type', '=', strtolower($type));
+        $images = $this->image->newQuery()->where('type', '=', strtolower($type));

-        if ($userFilter !== false) {
-            $images = $images->where('created_by', '=', $userFilter);
+        if ($uploadedTo !== null) {
+            $images = $images->where('uploaded_to', '=', $uploadedTo);
+        }
+
+        // Filter by page access if gallery
+        if ($type === 'gallery') {
+            $images = $this->restrictionService->filterRelatedPages($images, 'images', 'uploaded_to');
        }

        return $this->returnPaginated($images, $page, $pageSize);
@ -90,9 +96,17 @@ class ImageRepo
     * @param string $searchTerm
     * @return array
     */
-    public function searchPaginatedByType($type, $searchTerm, $page = 0, $pageSize = 24)
+    public function searchPaginatedByType(Request $request, $type, $searchTerm, $page = 0, $pageSize = 24)
    {
-        $images = $this->image->where('type', '=', strtolower($type))->where('name', 'LIKE', '%' . $searchTerm . '%');
+        // TODO - Filter by uploaded_to
+        $images = $this->image->newQuery()
+            ->where('type', '=', strtolower($type))
+            ->where('name', 'LIKE', '%' . $searchTerm . '%');
+
+        if ($type === 'gallery') {
+            $images = $this->restrictionService->filterRelatedPages($images, 'images', 'uploaded_to');
+        }
+
        return $this->returnPaginated($images, $page, $pageSize);
    }

@ -118,6 +132,7 @@ class ImageRepo
            $images = $images->whereIn('uploaded_to', $validPageIds);
        }

+        $images = $this->restrictionService->filterRelatedPages($images, 'images', 'uploaded_to');
        return $this->returnPaginated($images, $pageNum, $pageSize);
    }

--- a/database/migrations/2019_04_21_131855_set_user_profile_images_uploaded_to.php
+++ b/database/migrations/2019_04_21_131855_set_user_profile_images_uploaded_to.php
@ -0,0 +1,36 @@
+<?php
+
+use Illuminate\Support\Facades\Schema;
+use Illuminate\Database\Schema\Blueprint;
+use Illuminate\Database\Migrations\Migration;
+
+class SetUserProfileImagesUploadedTo extends Migration
+{
+    /**
+     * Run the migrations.
+     *
+     * @return void
+     */
+    public function up()
+    {
+        DB::table('images')
+            ->where('type', '=', 'user')
+            ->update([
+                'uploaded_to' => DB::raw('`created_by`')
+            ]);
+    }
+
+    /**
+     * Reverse the migrations.
+     *
+     * @return void
+     */
+    public function down()
+    {
+        DB::table('images')
+            ->where('type', '=', 'user')
+            ->update([
+                'uploaded_to' => 0
+            ]);
+    }
+}
--- a/resources/assets/js/vues/image-manager.js
+++ b/resources/assets/js/vues/image-manager.js
@ -59,7 +59,7 @@ const methods = {
    fetchData() {
        let url = baseUrl + page;
        let query = {};
-        if (this.uploadedTo !== false) query.page_id = this.uploadedTo;
+        if (this.uploadedTo !== false) query.uploaded_to = this.uploadedTo;
        if (this.searching) query.term = this.searchTerm;

        this.$http.get(url, {params: query}).then(response => {
@ -133,7 +133,7 @@ const methods = {
    },

    saveImageDetails() {
-        let url = window.baseUrl(`/images/update/${this.selectedImage.id}`);
+        let url = window.baseUrl(`/images/${this.selectedImage.id}`);
        this.$http.put(url, this.selectedImage).then(response => {
            this.$events.emit('success', trans('components.image_update_success'));
        }).catch(error => {
--- a/resources/views/users/edit.blade.php
+++ b/resources/views/users/edit.blade.php
@ -87,5 +87,5 @@
        @endif
    </div>

-    @include('components.image-manager', ['imageType' => 'user'])
+    @include('components.image-manager', ['imageType' => 'user', 'uploaded_to' => $user->id])
@stop
--- a/routes/web.php
+++ b/routes/web.php
@ -105,19 +105,28 @@ Route::group(['middleware' => 'auth'], function () {
    // Image routes
    Route::group(['prefix' => 'images'], function() {
        // Get for user images
-        Route::get('/user/all', 'ImageController@getAllForUserType');
-        Route::get('/user/all/{page}', 'ImageController@getAllForUserType');
+//        Route::get('/user/all', 'ImageController@getAllForUserType');
+//        Route::get('/user/all/{page}', 'ImageController@getAllForUserType');
        // Standard get, update and deletion for all types
        Route::get('/thumb/{id}/{width}/{height}/{crop}', 'ImageController@getThumbnail');
        Route::get('/base64/{id}', 'ImageController@getBase64Image');
-        Route::put('/update/{imageId}', 'ImageController@update');
-        Route::post('/drawing/upload', 'ImageController@uploadDrawing');
        Route::get('/usage/{id}', 'ImageController@usage');
-        Route::post('/{type}/upload', 'ImageController@uploadByType');
        Route::get('/{type}/all', 'ImageController@getAllByType');
        Route::get('/{type}/all/{page}', 'ImageController@getAllByType');
        Route::get('/{type}/search/{page}', 'ImageController@searchByType');
        Route::get('/gallery/{filter}/{page}', 'ImageController@getGalleryFiltered');
+
+        // TODO - Remove use of abstract "Type" variable (Above)
+        // TODO - Clearly define each endpoint so logic for each is clear
+        // TODO - Move into per-type controllers
+        // TODO - Test and fully think about permissions and each stage
+        Route::post('/drawio', 'ImageController@uploadDrawioImage');
+        Route::post('/gallery', 'ImageController@uploadGalleryImage');
+        Route::post('/user', 'ImageController@uploadUserImage');
+        Route::post('/system', 'ImageController@uploadSystemImage');
+        Route::post('/cover', 'ImageController@uploadCoverImage');
+
+        Route::put('/{id}', 'ImageController@update');
        Route::delete('/{id}', 'ImageController@destroy');
    });

--- a/tests/Uploads/ImageTest.php
+++ b/tests/Uploads/ImageTest.php
@ -217,18 +217,97 @@ class ImageTest extends TestCase
        $this->assertTrue($testImageData === $uploadedImageData, "Uploaded image file data does not match our test image as expected");
    }

+    protected function getTestProfileImage()
+    {
+        $imageName = 'profile.png';
+        $relPath = $this->getTestImagePath('user', $imageName);
+        $this->deleteImage($relPath);
+
+        return $this->getTestImage($imageName);
+    }
+
+    public function test_user_image_upload()
+    {
+        $editor = $this->getEditor();
+        $admin = $this->getAdmin();
+        $this->actingAs($admin);
+
+        $file = $this->getTestProfileImage();
+        $this->call('POST', '/images/user/upload', ['uploaded_to' => $editor->id], [], ['file' => $file], []);
+
+        $this->assertDatabaseHas('images', [
+            'type' => 'user',
+            'uploaded_to' => $editor->id,
+            'created_by' => $admin->id,
+        ]);
+    }
+
+    public function test_standard_user_with_manage_users_permission_can_view_other_profile_images()
+    {
+        $editor = $this->getEditor();
+        $this->giveUserPermissions($editor, ['users-manage']);
+
+        $admin = $this->getAdmin();
+
+        $this->actingAs($admin);
+        $file = $this->getTestProfileImage();
+        $this->call('POST', '/images/user/upload', ['uploaded_to' => $admin->id], [], ['file' => $file], []);
+
+        $expectedJson = [
+            'name' => 'profile.png',
+            'uploaded_to' => $admin->id,
+            'type' => 'user'
+        ];
+
+        $this->actingAs($editor);
+        $adminImagesGet = $this->get("/images/user/all/0?uploaded_to=" . $admin->id);
+        $adminImagesGet->assertStatus(200)->assertJsonFragment($expectedJson);
+
+        $allImagesGet = $this->get("/images/user/all/0");
+        $allImagesGet->assertStatus(200)->assertJsonFragment($expectedJson);
+    }
+
+    public function test_standard_user_cant_view_other_profile_images()
+    {
+        $editor = $this->getEditor();
+        $admin = $this->getAdmin();
+
+        $this->actingAs($admin);
+        $file = $this->getTestProfileImage();
+        $this->call('POST', '/images/user/upload', ['uploaded_to' => $admin->id], [], ['file' => $file], []);
+
+        $this->actingAs($editor);
+        $adminImagesGet = $this->get("/images/user/all/0?uploaded_to=" . $admin->id);
+        $adminImagesGet->assertStatus(302);
+
+        $allImagesGet = $this->get("/images/user/all/0");
+        $allImagesGet->assertStatus(302);
+    }
+
+    public function test_standard_user_cant_upload_other_profile_images()
+    {
+        $editor = $this->getEditor();
+        $admin = $this->getAdmin();
+
+        $this->actingAs($editor);
+        $file = $this->getTestProfileImage();
+        $upload = $this->call('POST', '/images/user/upload', ['uploaded_to' => $admin->id], [], ['file' => $file], []);
+        $upload->assertStatus(302);
+
+        $this->assertDatabaseMissing('images', [
+            'type' => 'user',
+            'uploaded_to' => $admin->id,
+        ]);
+    }
+
    public function test_user_images_deleted_on_user_deletion()
    {
        $editor = $this->getEditor();
        $this->actingAs($editor);

-        $imageName = 'profile.png';
-        $relPath = $this->getTestImagePath('gallery', $imageName);
-        $this->deleteImage($relPath);
-
-        $file = $this->getTestImage($imageName);
-        $this->call('POST', '/images/user/upload', [], [], ['file' => $file], []);
-        $this->call('POST', '/images/user/upload', [], [], ['file' => $file], []);
+        $file = $this->getTestProfileImage();
+        $this->call('POST', '/images/user/upload', ['uploaded_to' => $editor->id], [], ['file' => $file], []);
+        $this->call('POST', '/images/user/upload', ['uploaded_to' => $editor->id], [], ['file' => $file], []);

        $profileImages = Image::where('type', '=', 'user')->where('created_by', '=', $editor->id)->get();
        $this->assertTrue($profileImages->count() === 2, "Found profile images does not match upload count");
@ -239,6 +318,10 @@ class ImageTest extends TestCase
            'type' => 'user',
            'created_by' => $editor->id
        ]);
+        $this->assertDatabaseMissing('images', [
+            'type' => 'user',
+            'uploaded_to' => $editor->id
+        ]);
    }

    public function test_deleted_unused_images()