From 7b6c88f17c595f3b0d88fe383827794b83dba3e7 Mon Sep 17 00:00:00 2001 From: Dan Brown Date: Fri, 1 Jul 2016 20:11:49 +0100 Subject: [PATCH] Fixed error on image deletion Also Added tests to cover image upload and deletion. Fixes #136. --- app/Http/Controllers/ImageController.php | 6 +- app/Providers/AppServiceProvider.php | 7 +- app/Services/PermissionService.php | 9 ++- app/helpers.php | 10 ++- tests/ImageTest.php | 95 +++++++++++++++++++++++ tests/TestCase.php | 10 ++- tests/test-image.jpg | Bin 0 -> 5238 bytes 7 files changed, 124 insertions(+), 13 deletions(-) create mode 100644 tests/ImageTest.php create mode 100644 tests/test-image.jpg diff --git a/app/Http/Controllers/ImageController.php b/app/Http/Controllers/ImageController.php index 2e5d5f303..621c23e85 100644 --- a/app/Http/Controllers/ImageController.php +++ b/app/Http/Controllers/ImageController.php @@ -51,9 +51,9 @@ class ImageController extends Controller $this->validate($request, [ 'term' => 'required|string' ]); - + $searchTerm = $request->get('term'); - $imgData = $this->imageRepo->searchPaginatedByType($type, $page,24, $searchTerm); + $imgData = $this->imageRepo->searchPaginatedByType($type, $page, 24, $searchTerm); return response()->json($imgData); } @@ -99,7 +99,7 @@ class ImageController extends Controller { $this->checkPermission('image-create-all'); $this->validate($request, [ - 'file' => 'image|mimes:jpeg,gif,png' + 'file' => 'is_image' ]); $imageUpload = $request->file('file'); diff --git a/app/Providers/AppServiceProvider.php b/app/Providers/AppServiceProvider.php index 8bcbcbdad..f214c9141 100644 --- a/app/Providers/AppServiceProvider.php +++ b/app/Providers/AppServiceProvider.php @@ -15,7 +15,12 @@ class AppServiceProvider extends ServiceProvider */ public function boot() { - // + // Custom validation methods + \Validator::extend('is_image', function($attribute, $value, $parameters, $validator) { + $imageMimes = ['image/png', 'image/bmp', 'image/gif', 'image/jpeg', 'image/jpg', 'image/tiff', 'image/webp']; + return in_array($value->getMimeType(), $imageMimes); + }); + } /** diff --git a/app/Services/PermissionService.php b/app/Services/PermissionService.php index 218cb30a5..0fffe60f2 100644 --- a/app/Services/PermissionService.php +++ b/app/Services/PermissionService.php @@ -4,6 +4,7 @@ use BookStack\Book; use BookStack\Chapter; use BookStack\Entity; use BookStack\JointPermission; +use BookStack\Ownable; use BookStack\Page; use BookStack\Role; use BookStack\User; @@ -307,16 +308,16 @@ class PermissionService /** * Checks if an entity has a restriction set upon it. - * @param Entity $entity + * @param Ownable $ownable * @param $permission * @return bool */ - public function checkEntityUserAccess(Entity $entity, $permission) + public function checkOwnableUserAccess(Ownable $ownable, $permission) { if ($this->isAdmin) return true; $explodedPermission = explode('-', $permission); - $baseQuery = $entity->where('id', '=', $entity->id); + $baseQuery = $ownable->where('id', '=', $ownable->id); $action = end($explodedPermission); $this->currentAction = $action; @@ -327,7 +328,7 @@ class PermissionService $allPermission = $this->currentUser && $this->currentUser->can($permission . '-all'); $ownPermission = $this->currentUser && $this->currentUser->can($permission . '-own'); $this->currentAction = 'view'; - $isOwner = $this->currentUser && $this->currentUser->id === $entity->created_by; + $isOwner = $this->currentUser && $this->currentUser->id === $ownable->created_by; return ($allPermission || ($isOwner && $ownPermission)); } diff --git a/app/helpers.php b/app/helpers.php index b8f61d94e..42e4c1894 100644 --- a/app/helpers.php +++ b/app/helpers.php @@ -1,5 +1,7 @@ user() && auth()->user()->can($permission); } // Check permission on ownable item - $permissionService = app('BookStack\Services\PermissionService'); - return $permissionService->checkEntityUserAccess($ownable, $permission); + $permissionService = app(\BookStack\Services\PermissionService::class); + return $permissionService->checkOwnableUserAccess($ownable, $permission); } /** diff --git a/tests/ImageTest.php b/tests/ImageTest.php new file mode 100644 index 000000000..806a36acc --- /dev/null +++ b/tests/ImageTest.php @@ -0,0 +1,95 @@ +getTestImage($name); + $this->call('POST', '/images/gallery/upload', ['uploaded_to' => $uploadedTo], [], ['file' => $file], []); + return $this->getTestImagePath('gallery', $name); + } + + /** + * Delete an uploaded image. + * @param $relPath + */ + protected function deleteImage($relPath) + { + unlink(public_path($relPath)); + } + + + public function test_image_upload() + { + $page = \BookStack\Page::first(); + $this->asAdmin(); + $admin = $this->getAdmin(); + $imageName = 'first-image.jpg'; + + $relPath = $this->uploadImage($imageName, $page->id); + $this->assertResponseOk(); + + $this->assertTrue(file_exists(public_path($relPath)), 'Uploaded image exists'); + + $this->seeInDatabase('images', [ + 'url' => $relPath, + 'type' => 'gallery', + 'uploaded_to' => $page->id, + 'path' => $relPath, + 'created_by' => $admin->id, + 'updated_by' => $admin->id, + 'name' => $imageName + ]); + + $this->deleteImage($relPath); + } + + public function test_image_delete() + { + $page = \BookStack\Page::first(); + $this->asAdmin(); + $imageName = 'first-image.jpg'; + + $relPath = $this->uploadImage($imageName, $page->id); + $image = \BookStack\Image::first(); + + $this->call('DELETE', '/images/' . $image->id); + $this->assertResponseOk(); + + $this->dontSeeInDatabase('images', [ + 'url' => $relPath, + 'type' => 'gallery' + ]); + + $this->assertFalse(file_exists(public_path($relPath)), 'Uploaded image has been deleted'); + } + +} \ No newline at end of file diff --git a/tests/TestCase.php b/tests/TestCase.php index 4c2893f4e..6a8c2d732 100644 --- a/tests/TestCase.php +++ b/tests/TestCase.php @@ -39,11 +39,19 @@ class TestCase extends Illuminate\Foundation\Testing\TestCase */ public function asAdmin() { + return $this->actingAs($this->getAdmin()); + } + + /** + * Get the current admin user. + * @return mixed + */ + public function getAdmin() { if($this->admin === null) { $adminRole = \BookStack\Role::getRole('admin'); $this->admin = $adminRole->users->first(); } - return $this->actingAs($this->admin); + return $this->admin; } /** diff --git a/tests/test-image.jpg b/tests/test-image.jpg new file mode 100644 index 0000000000000000000000000000000000000000..fb8da91011c2d691eb038afe579d60a8451d28cf GIT binary patch literal 5238 zcmbW42UJtdx`t=(bP^yTkzPXYRe{iZ?QEdnZ>rWC8$^qiaM=EKCM~6O+jGcCe=V`LChl`T-0G01I>h2xcb4 zxvjKc1(4=sXG2dwf_VJ9%{~A~CIIegI(pLSzsLViL}A8p69Irck?7?tb^;UOY=omz z661K|(+JZ-qIe9WdCWx$L|Bx^VLw^pN6b&Q1zs&ri=BL>}(of6GXOw{xYW<~{bS#+CNPF!$I8UR1+Ts6j*&j9V@Tad4pdMKSv_LID9-sl#yFP%X5&KH z1wmm@43q?IgtkKYP$5(a9fE41^U!6e73zTop%Lf_^a}b2&BG{|1k+$CSP9mK8L$=X z0Lfu7;qu!$C&{#ATO-E~@P0)^LA2bskkIq2vL|39upzG1?=)34A z=xOvKh94t|QO6i#oG^ZvFiZ+22UChUhPj04z}&}7U}mvctT0v?YlwBk`eP%p8?ZaE z`>|)S*Rg}xXV@7W4kw0F$C=?gaKX4lTn?@R_ZO}OcLz6t`-&&xW$=1kVYsV93eCj1_%>`Z+sL!Wj-^$ReX_rS$ySuXZSk! z#`r!Fi9|V~5z&hnLEKC%C!QmA6Q2<0NJ1nvk_{<-IsS2qZQg5Y&r7fi+rHiGT zrC-TVWz1zFWQt{4WM0#k(XHsw^m2MTeOgvh)=@S|_K^C`OIUl(#a%bfp$@9rG zR`ONauGFA3sVu7Oq@1pNN_k9$ zU&TTtUZqCmo+?(Ap&F&SU-h;ctfsFPu2!Wspbo1Ws7I*pS0B{CXc%e4Xw+y7X_7Rp zG?O(?X+F^s)^gR_qSc@^rLCa7MtiSzm-eELzD~5x5uH(8Azf$PExJv*ANADqn0i%u z_m=Z7w_l#Qym9%573wS4D-Ns})~D*b>2KF>)BkQ@Xuvf%WAMsQ!7#{hpWzUL#_(k9 zWON&0jI50^jjkHa85Ha%kc%uLQK*zAzmxH;WC$h_Ko z+(OnO*rLYbsinLn%kr4zODk2YNUOiBrmS_Wxz?Agzt|YtY_hp-i?VgF&9m*d6SDKM ztF(J;FJ~WSf69K^LEmA6!*xfDql@ES#~~*vC#F-a)3mdp^CstZ7m~|Lmr9qXu4=Aa z*CsdE&DpKUZOmQKJ;uGk19&)j6nQ-IRQ8PbyyAuR^7N|mdf~0>z0tdCC3R)s%9AT+ zS6QzrST*XS;*;dlwpws?!0Ou7UwmzS3w_7^wEZ^u_4@uM#jICsGj<{SWvE$bVd!L-Sy)lnt8k0(((tztHW8H(A0izi4@Q28@`ySfwG{0e zeU^jg1aU6M2*pIjw8u)vrp6A&smJBSjmI0um&8wVUAV^*poDb^jfphmWvVYpC23pI zlVtPcs^qUJt5YtdQc`15`_k0Xa?@U>+ovDhfZo8|(6&)_W7fteo2)h+%78LLGTJib zGqW=%Hal!Sk;Ru4m33>2&X&S0pSSvMZOWF)-kkkxo5QwKIRZJ{oT2R|+YjzQ?}*sZ zpSwJ_B6l&5m3K2=C%-g*VJCCv&0V^?%6Bd84&B{fU{G*i4|Y$?o(Fp^_MRxD6s8wG zD{?EUFQymg6@M-XEa@p-QF^e9uPmu-yxg_Cu|lC@PsKuIc;)>n>#DQ+B=_a+`?8<4 z|IPu+17{9O9n3#CUma0Be8}NYeT`C0>0#{Q-K5e(~V~|&m1}{d^Z0aIG1$p_4%Ol59(a%+Ac6I zoVloYao;8CrM!BmKCOPbA-rL{(Z6xe{kv1n+8n>o9|fMX}xQExAUIUz25uo_Xi*NJQyAd7#bgD z4Ns0lk9>HT{P5f8<}u7z-Xrp((#MjIYyMXK`|P;E__ZgtPx_v&d^-A!`RvW}gy#zr z*)RBCl)jXCSv#pWdG(e3tJ|;FzMgm!_vYK%oOhIW`=(T;F21*X-#6_yJ@JA2Vd>+} zPhy{re_rvqZN_uv@hoTd+n2nrVqZ_pG3L7Fedk|(OIg4zlz&(K-n8hlIJU%DT3Q+f zHh@GR5D9n^k&h_APZFfc(5Ms&RY^ihL`G9ZM_WThLtT&IXr`xcYoM-S?qgx=?CR;| zsblK5-q(G-qlc#(FA2miAV4Kk6=^g@x8)kk-TvQgsSQx@ARct1APRsf5Q+jVwF7b9 zpCDf2y{Y_Y5P(r=EC!hq`R57VECi#L9s&{y2_mD&$nWpF5^PIkDRdaXFd(8-iv~3m zY6%30jAX=Q75DUCOFLJ1M8Vb;4J`4VgrZ$(fu*;SkI_&t0#Ssuo71!6DC2Ez!c73Q7ohv+c9nyTa;Rn(c` z+GLg-S9a}yQ>s|trOTF<5M3|m<_AWFDR&ON`z6^(3w&Fy|no$YousrGqYSuP;IV8ZbiK3X8acO!>RbeiL%`5eNApdgjA?T?@%FLjZVm@ z+W4%$Ikl%#?+Qv;PnHmGW#uoYB5dK}*IHxbvNf#wL#0mm%`6N>?1W{t-bQrCis+&z8)Hk=%bc!UM z^iH<8Y~HKU-Q9g}4pV9suYJ=a>+|{t{(JY#kmha)`rK9VC?Ci*NfVi1zu)sBG}LIf zz1`H`b`=6CSIS=)RfzfJ_Et@oNf|cVUNS1~(wm)^oO|CR=p$y}azpmYhvtS__GQ{S zfz|8(_G%%%b1Xm~%rBEJo6KlHZihzsZKIzzwE0KGPWfjh zNY;|>86@-%EwC2wo)`fnL6WEbA; zTFhzBFsU@qShpx15MU*vd(Xh+N#-_HXYVq?nnB`Bz5JqV?r>G6=3X=J;-jk0UgI?a zYgA)f2A8`{zS$5Wa?{ki;QlSuxZs>4NpAQ zSbO1;JZlw$!|6+)+%P{+-@Zd-h%a_8^KpUHlH7kcHQBC&FQi;WWJj;e-iiAz_99d|1M&5k@en>MJ#)Ib{e(>S+TN1 z%sa3Xz305PaD_~dMt!t+P}7;#cZ*}C`*!;*vn4yZwJ0WSGFBMT8tvIo#j2oJyEuER zX3knEn1ymLm$;m>I&xeS^DXq13Q1E_Jz_-7Wvjp|{pZ2A#N~4hM`NG6z1l%1v@C&? z>H@?lQizve#09@k1_eNL9ZRyVXV5QGk-t;gx?u9-E?APpHM=IbGjUJ%xa@$>u2>_- zkhuFE1@7tX^DRg=3Ca&o{b4GIVhPBItA6C-xsN=rY;vM=Rv~ph?sCtrz7HRtD7)O# z`{6A*@-#y#`Nx;%YzR5@7A0dNbB1 z$+#k(rInA(8k19qG&>hHQ7t0$JY8{U7)c+>X|Zo1NdH|&jKi6T2)d(2uapT#`zsIg z@AP97l-j+lU1yV@xw&s4u$sERymiVWYman;wM)TkpO+<53$k2+nvVi5$^p@G>-O)z zI6so}aiQg2(Qs`1VtaB4cYWHn(R#-A1kK06Nla-~{QYLDspn0ZDcV`39a~~H%9u~7AoH6oo*hh4hgw(^k}_9RiRi$xTEL{ZZ7yVqjA^8 zBF5;DY1`~U<%&)>FF~1)F3V>>Z(Z?lW_e}r80+Ix(WK%r<5EBQk}kY0*G7V|(~#dEWpXWSL%%oaw{_C{aW{ivH8AJow&HW7B8ZR%0` z-E4k?ykxBF^njM_rG3