diff --git a/app/Auth/UserRepo.php b/app/Auth/UserRepo.php index 1a73d0072..94ebb27ab 100644 --- 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); } diff --git a/app/Http/Controllers/ImageController.php b/app/Http/Controllers/ImageController.php index 4d6f759b3..ae2d74305 100644 --- 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); } diff --git a/app/Uploads/ImageRepo.php b/app/Uploads/ImageRepo.php index 0ef8cad48..c13d995bd 100644 --- 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); } diff --git 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 new file mode 100644 index 000000000..f1d00b3e5 --- /dev/null +++ b/database/migrations/2019_04_21_131855_set_user_profile_images_uploaded_to.php @@ -0,0 +1,36 @@ +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 + ]); + } +} diff --git a/resources/assets/js/vues/image-manager.js b/resources/assets/js/vues/image-manager.js index 6bfc2662d..843aae378 100644 --- 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 => { diff --git a/resources/views/users/edit.blade.php b/resources/views/users/edit.blade.php index b9ad052c7..7c8175d9a 100644 --- a/resources/views/users/edit.blade.php +++ b/resources/views/users/edit.blade.php @@ -87,5 +87,5 @@ @endif - @include('components.image-manager', ['imageType' => 'user']) + @include('components.image-manager', ['imageType' => 'user', 'uploaded_to' => $user->id]) @stop diff --git a/routes/web.php b/routes/web.php index 695f61654..933179aa7 100644 --- 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'); }); diff --git a/tests/Uploads/ImageTest.php b/tests/Uploads/ImageTest.php index 8373a809c..c2e21f95c 100644 --- 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()