Added role permissions for exporting content

diff --git a/app/Http/Controllers/Api/BookExportApiController.php b/app/Http/Controllers/Api/BookExportApiController.php
index c7d121f88636cec083594f2c5f376eb2f6e273b8..028bc3a817ebf726b358ca0f7b8d47f393695bf8 100644 (file)
--- a/app/Http/Controllers/Api/BookExportApiController.php
+++ b/app/Http/Controllers/Api/BookExportApiController.php
@@ -13,6 +13,7 @@ class BookExportApiController extends ApiController
     public function __construct(ExportFormatter $exportFormatter)
     {
         $this->exportFormatter = $exportFormatter;
     public function __construct(ExportFormatter $exportFormatter)
     {
         $this->exportFormatter = $exportFormatter;

+        $this->middleware('can:content-export');

     }
 
     /**
     }
 
     /**

diff --git a/app/Http/Controllers/Api/ChapterExportApiController.php b/app/Http/Controllers/Api/ChapterExportApiController.php
index 5dfcea344def1508548f1209f1bb9eac2835d43f..5715ab2e37c6c9f7e682ad69b407f27153e3934e 100644 (file)
--- a/app/Http/Controllers/Api/ChapterExportApiController.php
+++ b/app/Http/Controllers/Api/ChapterExportApiController.php
@@ -16,6 +16,7 @@ class ChapterExportApiController extends ApiController
     public function __construct(ExportFormatter $exportFormatter)
     {
         $this->exportFormatter = $exportFormatter;
     public function __construct(ExportFormatter $exportFormatter)
     {
         $this->exportFormatter = $exportFormatter;

+        $this->middleware('can:content-export');

     }
 
     /**
     }
 
     /**

diff --git a/app/Http/Controllers/Api/PageExportApiController.php b/app/Http/Controllers/Api/PageExportApiController.php
index 7cee2fbe7cc24510be06df1cd2c1483aa68cb066..ce5700c79b9add01a9b2eb8d418f5b3a785344e8 100644 (file)
--- a/app/Http/Controllers/Api/PageExportApiController.php
+++ b/app/Http/Controllers/Api/PageExportApiController.php
@@ -13,6 +13,7 @@ class PageExportApiController extends ApiController
     public function __construct(ExportFormatter $exportFormatter)
     {
         $this->exportFormatter = $exportFormatter;
     public function __construct(ExportFormatter $exportFormatter)
     {
         $this->exportFormatter = $exportFormatter;

+        $this->middleware('can:content-export');

     }
 
     /**
     }
 
     /**

diff --git a/app/Http/Controllers/BookExportController.php b/app/Http/Controllers/BookExportController.php
index 5e6d7c6adc8f22f970550814825c43f3451fb2a3..7f6dd801752b5e3901cb6188e3dc200888f32984 100644 (file)
--- a/app/Http/Controllers/BookExportController.php
+++ b/app/Http/Controllers/BookExportController.php
@@ -18,6 +18,7 @@ class BookExportController extends Controller
     {
         $this->bookRepo = $bookRepo;
         $this->exportFormatter = $exportFormatter;
     {
         $this->bookRepo = $bookRepo;
         $this->exportFormatter = $exportFormatter;

+        $this->middleware('can:content-export');

     }
 
     /**
     }
 
     /**

diff --git a/app/Http/Controllers/ChapterExportController.php b/app/Http/Controllers/ChapterExportController.php
index 2758496d2062b307964774e4a669ff8884cd75bc..480280c99ef6dc83a169ba1762d5e4fc0edd4d36 100644 (file)
--- a/app/Http/Controllers/ChapterExportController.php
+++ b/app/Http/Controllers/ChapterExportController.php
@@ -19,6 +19,7 @@ class ChapterExportController extends Controller
     {
         $this->chapterRepo = $chapterRepo;
         $this->exportFormatter = $exportFormatter;
     {
         $this->chapterRepo = $chapterRepo;
         $this->exportFormatter = $exportFormatter;

+        $this->middleware('can:content-export');

     }
 
     /**
     }
 
     /**

diff --git a/app/Http/Controllers/PageExportController.php b/app/Http/Controllers/PageExportController.php
index 5a331521674dfa85cb9e22f4c4f7a97cb5dc01f4..0287916de28f40008eeb2d16e948d6274eb77a3a 100644 (file)
--- a/app/Http/Controllers/PageExportController.php
+++ b/app/Http/Controllers/PageExportController.php
@@ -20,6 +20,7 @@ class PageExportController extends Controller
     {
         $this->pageRepo = $pageRepo;
         $this->exportFormatter = $exportFormatter;
     {
         $this->pageRepo = $pageRepo;
         $this->exportFormatter = $exportFormatter;

+        $this->middleware('can:content-export');

     }
 
     /**
     }
 
     /**

diff --git a/app/Http/Kernel.php b/app/Http/Kernel.php
index d1f5de917123c7eba976e6533e1f10e65268b52e..2d85c870df2052b88b575eeaea4dd1fdc354fb56 100644 (file)
--- a/app/Http/Kernel.php
+++ b/app/Http/Kernel.php
@@ -48,10 +48,9 @@ class Kernel extends HttpKernel
      */
     protected $routeMiddleware = [
         'auth'       => \BookStack\Http\Middleware\Authenticate::class,
      */
     protected $routeMiddleware = [
         'auth'       => \BookStack\Http\Middleware\Authenticate::class,

-        'can'        => \Illuminate\Auth\Middleware\Authorize::class,
+        'can'       => \BookStack\Http\Middleware\CheckUserHasPermission::class,

         'guest'      => \BookStack\Http\Middleware\RedirectIfAuthenticated::class,
         'throttle'   => \Illuminate\Routing\Middleware\ThrottleRequests::class,
         'guest'      => \BookStack\Http\Middleware\RedirectIfAuthenticated::class,
         'throttle'   => \Illuminate\Routing\Middleware\ThrottleRequests::class,

-        'perm'       => \BookStack\Http\Middleware\PermissionMiddleware::class,

         'guard'      => \BookStack\Http\Middleware\CheckGuard::class,
         'mfa-setup'  => \BookStack\Http\Middleware\AuthenticatedOrPendingMfa::class,
     ];
         'guard'      => \BookStack\Http\Middleware\CheckGuard::class,
         'mfa-setup'  => \BookStack\Http\Middleware\AuthenticatedOrPendingMfa::class,
     ];

diff --git a/app/Http/Middleware/CheckUserHasPermission.php b/app/Http/Middleware/CheckUserHasPermission.php
new file mode 100644 (file)
index 0000000..4340152
--- /dev/null
+++ b/app/Http/Middleware/CheckUserHasPermission.php
@@ -0,0 +1,38 @@
+<?php
+
+namespace BookStack\Http\Middleware;
+
+use Closure;
+use Illuminate\Http\Request;
+
+class CheckUserHasPermission
+{
+    /**
+     * Handle an incoming request.
+     *
+     * @param \Illuminate\Http\Request $request
+     * @param \Closure                 $next
+     * @param                          $permission
+     *
+     * @return mixed
+     */
+    public function handle($request, Closure $next, $permission)
+    {
+        if (!user()->can($permission)) {
+            return $this->errorResponse($request);
+        }
+
+        return $next($request);
+    }
+
+
+    protected function errorResponse(Request $request)
+    {
+        if ($request->wantsJson()) {
+            return response()->json(['error' => trans('errors.permissionJson')], 403);
+        }
+
+        session()->flash('error', trans('errors.permission'));
+        return redirect('/');
+    }
+}

diff --git a/app/Http/Middleware/PermissionMiddleware.php b/app/Http/Middleware/PermissionMiddleware.php
deleted file mode 100644 (file)
index 1d7e4aa..0000000
--- a/app/Http/Middleware/PermissionMiddleware.php
+++ /dev/null
@@ -1,28 +0,0 @@
-<?php
-
-namespace BookStack\Http\Middleware;
-
-use Closure;
-
-class PermissionMiddleware
-{
-    /**
-     * Handle an incoming request.
-     *
-     * @param \Illuminate\Http\Request $request
-     * @param \Closure                 $next
-     * @param                          $permission
-     *
-     * @return mixed
-     */
-    public function handle($request, Closure $next, $permission)
-    {
-        if (!$request->user() || !$request->user()->can($permission)) {
-            session()->flash('error', trans('errors.permission'));
-
-            return redirect()->back();
-        }
-
-        return $next($request);
-    }
-}

diff --git a/database/migrations/2021_08_28_161743_add_export_role_permission.php b/database/migrations/2021_08_28_161743_add_export_role_permission.php
new file mode 100644 (file)
index 0000000..57abea0
--- /dev/null
+++ b/database/migrations/2021_08_28_161743_add_export_role_permission.php
@@ -0,0 +1,48 @@
+<?php
+
+use Carbon\Carbon;
+use Illuminate\Database\Migrations\Migration;
+
+class AddExportRolePermission extends Migration
+{
+    /**
+     * Run the migrations.
+     *
+     * @return void
+     */
+    public function up()
+    {
+        // Create new templates-manage permission and assign to admin role
+        $roles = \Illuminate\Support\Facades\DB::table('roles')->get('id');
+        $permissionId = DB::table('role_permissions')->insertGetId([
+            'name' => 'content-export',
+            'display_name' => 'Export Content',
+            'created_at' => Carbon::now()->toDateTimeString(),
+            'updated_at' => Carbon::now()->toDateTimeString(),
+        ]);
+
+        $permissionRoles = $roles->map(function ($role) use ($permissionId) {
+            return [
+                'role_id' => $role->id,
+                'permission_id' => $permissionId,
+            ];
+        })->values()->toArray();
+
+        DB::table('permission_role')->insert($permissionRoles);
+    }
+
+    /**
+     * Reverse the migrations.
+     *
+     * @return void
+     */
+    public function down()
+    {
+        // Remove content-export permission
+        $contentExportPermission = DB::table('role_permissions')
+            ->where('name', '=', 'content-export')->first();
+
+        DB::table('permission_role')->where('permission_id', '=', $contentExportPermission->id)->delete();
+        DB::table('role_permissions')->where('id', '=', 'content-export')->delete();
+    }
+}

diff --git a/resources/lang/en/settings.php b/resources/lang/en/settings.php
index 8b4215141ede6102ab9a5259ea9d182b08ff241c..4c1ae134527f068e244ed6081c2b3ad8af6f987d 100755 (executable)
--- a/resources/lang/en/settings.php
+++ b/resources/lang/en/settings.php
@@ -148,6 +148,7 @@ return [
     'role_manage_page_templates' => 'Manage page templates',
     'role_access_api' => 'Access system API',
     'role_manage_settings' => 'Manage app settings',
     'role_manage_page_templates' => 'Manage page templates',
     'role_access_api' => 'Access system API',
     'role_manage_settings' => 'Manage app settings',

+    'role_export_content' => 'Export content',

     'role_asset' => 'Asset Permissions',
     'roles_system_warning' => 'Be aware that access to any of the above three permissions can allow a user to alter their own privileges or the privileges of others in the system. Only assign roles with these permissions to trusted users.',
     'role_asset_desc' => 'These permissions control default access to the assets within the system. Permissions on Books, Chapters and Pages will override these permissions.',
     'role_asset' => 'Asset Permissions',
     'roles_system_warning' => 'Be aware that access to any of the above three permissions can allow a user to alter their own privileges or the privileges of others in the system. Only assign roles with these permissions to trusted users.',
     'role_asset_desc' => 'These permissions control default access to the assets within the system. Permissions on Books, Chapters and Pages will override these permissions.',

diff --git a/resources/views/books/show.blade.php b/resources/views/books/show.blade.php
index cc1fceb1339ad547028eef5146948c2b9bc14a91..25a6f69fad9ffec241720eac4b4b39d407903798 100644 (file)
--- a/resources/views/books/show.blade.php
+++ b/resources/views/books/show.blade.php
@@ -128,7 +128,9 @@
             @if(signedInUser())
                 @include('entities.favourite-action', ['entity' => $book])
             @endif
             @if(signedInUser())
                 @include('entities.favourite-action', ['entity' => $book])
             @endif

-            @include('entities.export-menu', ['entity' => $book])
+            @if(userCan('content-export'))
+                @include('entities.export-menu', ['entity' => $book])
+            @endif

         </div>
     </div>
 
         </div>
     </div>
 

diff --git a/resources/views/chapters/show.blade.php b/resources/views/chapters/show.blade.php
index 9e78ec9460a668d481ab11aa6a3f82ae7e55ee47..1646d4f18d0e1d33ab904eb86671dd855bbc98e9 100644 (file)
--- a/resources/views/chapters/show.blade.php
+++ b/resources/views/chapters/show.blade.php
@@ -132,7 +132,9 @@
             @if(signedInUser())
                 @include('entities.favourite-action', ['entity' => $chapter])
             @endif
             @if(signedInUser())
                 @include('entities.favourite-action', ['entity' => $chapter])
             @endif

-            @include('entities.export-menu', ['entity' => $chapter])
+            @if(userCan('content-export'))
+                @include('entities.export-menu', ['entity' => $chapter])
+            @endif

         </div>
     </div>
 @stop
         </div>
     </div>
 @stop

diff --git a/resources/views/pages/show.blade.php b/resources/views/pages/show.blade.php
index ca31ee06a6cf52dfbb8767c52469087abdc342fd..0111047c6cfec38fe0a2fea304226c7de3a6f114 100644 (file)
--- a/resources/views/pages/show.blade.php
+++ b/resources/views/pages/show.blade.php
@@ -165,7 +165,9 @@
             @if(signedInUser())
                 @include('entities.favourite-action', ['entity' => $page])
             @endif
             @if(signedInUser())
                 @include('entities.favourite-action', ['entity' => $page])
             @endif

-            @include('entities.export-menu', ['entity' => $page])
+            @if(userCan('content-export'))
+                @include('entities.export-menu', ['entity' => $page])
+            @endif

         </div>
 
     </div>
         </div>
 
     </div>

diff --git a/resources/views/settings/roles/parts/form.blade.php b/resources/views/settings/roles/parts/form.blade.php
index 2c1ed7fed80c1606d2180301cc532c0a7470ea0d..2f94398b5449846b89c57b3b7686bbe3369686ee 100644 (file)
--- a/resources/views/settings/roles/parts/form.blade.php
+++ b/resources/views/settings/roles/parts/form.blade.php
@@ -41,6 +41,7 @@
                     <div>@include('settings.roles.parts.checkbox', ['permission' => 'restrictions-manage-own', 'label' => trans('settings.role_manage_own_entity_permissions')])</div>
                     <div>@include('settings.roles.parts.checkbox', ['permission' => 'templates-manage', 'label' => trans('settings.role_manage_page_templates')])</div>
                     <div>@include('settings.roles.parts.checkbox', ['permission' => 'access-api', 'label' => trans('settings.role_access_api')])</div>
                     <div>@include('settings.roles.parts.checkbox', ['permission' => 'restrictions-manage-own', 'label' => trans('settings.role_manage_own_entity_permissions')])</div>
                     <div>@include('settings.roles.parts.checkbox', ['permission' => 'templates-manage', 'label' => trans('settings.role_manage_page_templates')])</div>
                     <div>@include('settings.roles.parts.checkbox', ['permission' => 'access-api', 'label' => trans('settings.role_access_api')])</div>

+                    <div>@include('settings.roles.parts.checkbox', ['permission' => 'content-export', 'label' => trans('settings.role_export_content')])</div>

                 </div>
                 <div>
                     <div>@include('settings.roles.parts.checkbox', ['permission' => 'settings-manage', 'label' => trans('settings.role_manage_settings')])</div>
                 </div>
                 <div>
                     <div>@include('settings.roles.parts.checkbox', ['permission' => 'settings-manage', 'label' => trans('settings.role_manage_settings')])</div>


@@ -239,7 +240,7 @@
 
 <div class="card content-wrap auto-height">
     <h2 class="list-heading">{{ trans('settings.role_users') }}</h2>
 
 <div class="card content-wrap auto-height">
     <h2 class="list-heading">{{ trans('settings.role_users') }}</h2>

-    @if(isset($role) && count($role->users) > 0)
+    @if(count($role->users ?? []) > 0)

         <div class="grid third">
             @foreach($role->users as $user)
                 <div class="user-list-item">
         <div class="grid third">
             @foreach($role->users as $user)
                 <div class="user-list-item">

diff --git a/tests/Api/BooksApiTest.php b/tests/Api/BooksApiTest.php
index 279c7ad9a73b75b0617e3045b1315b1ee3ad837f..91e2db9e52de5c4cf7bddd0df659e6cdc79c42c8 100644 (file)
--- a/tests/Api/BooksApiTest.php
+++ b/tests/Api/BooksApiTest.php
@@ -155,4 +155,17 @@ class BooksApiTest extends TestCase
         $resp->assertSee('# ' . $book->pages()->first()->name);
         $resp->assertSee('# ' . $book->chapters()->first()->name);
     }
         $resp->assertSee('# ' . $book->pages()->first()->name);
         $resp->assertSee('# ' . $book->chapters()->first()->name);
     }

+
+    public function test_cant_export_when_not_have_permission()
+    {
+        $types = ['html', 'plaintext', 'pdf', 'markdown'];
+        $this->actingAsApiEditor();
+        $this->removePermissionFromUser($this->getEditor(), 'content-export');
+
+        $book = Book::visible()->first();
+        foreach ($types as $type) {
+            $resp = $this->get($this->baseEndpoint . "/{$book->id}/export/{$type}");
+            $this->assertPermissionError($resp);
+        }
+    }

 }
 }

diff --git a/tests/Api/ChaptersApiTest.php b/tests/Api/ChaptersApiTest.php
index b3dd0ae6b408ecb20badca3dcb552b193f8da3eb..c9ed1a2892e19a715dd344e4a4ac8e6b5302b41a 100644 (file)
--- a/tests/Api/ChaptersApiTest.php
+++ b/tests/Api/ChaptersApiTest.php
@@ -200,4 +200,17 @@ class ChaptersApiTest extends TestCase
         $resp->assertSee('# ' . $chapter->name);
         $resp->assertSee('# ' . $chapter->pages()->first()->name);
     }
         $resp->assertSee('# ' . $chapter->name);
         $resp->assertSee('# ' . $chapter->pages()->first()->name);
     }

+
+    public function test_cant_export_when_not_have_permission()
+    {
+        $types = ['html', 'plaintext', 'pdf', 'markdown'];
+        $this->actingAsApiEditor();
+        $this->removePermissionFromUser($this->getEditor(), 'content-export');
+
+        $chapter = Chapter::visible()->has('pages')->first();
+        foreach ($types as $type) {
+            $resp = $this->get($this->baseEndpoint . "/{$chapter->id}/export/{$type}");
+            $this->assertPermissionError($resp);
+        }
+    }

 }
 }

diff --git a/tests/Api/PagesApiTest.php b/tests/Api/PagesApiTest.php
index eca606234bf4791af06101af8cac011ec63867c9..4eb109d9dec3acf35653740aa51f5af714d122c4 100644 (file)
--- a/tests/Api/PagesApiTest.php
+++ b/tests/Api/PagesApiTest.php
@@ -292,4 +292,17 @@ class PagesApiTest extends TestCase
         $resp->assertSee('# ' . $page->name);
         $resp->assertHeader('Content-Disposition', 'attachment; filename="' . $page->slug . '.md"');
     }
         $resp->assertSee('# ' . $page->name);
         $resp->assertHeader('Content-Disposition', 'attachment; filename="' . $page->slug . '.md"');
     }

+
+    public function test_cant_export_when_not_have_permission()
+    {
+        $types = ['html', 'plaintext', 'pdf', 'markdown'];
+        $this->actingAsApiEditor();
+        $this->removePermissionFromUser($this->getEditor(), 'content-export');
+
+        $page = Page::visible()->first();
+        foreach ($types as $type) {
+            $resp = $this->get($this->baseEndpoint . "/{$page->id}/export/{$type}");
+            $this->assertPermissionError($resp);
+        }
+    }

 }
 }

diff --git a/tests/Entity/ExportTest.php b/tests/Entity/ExportTest.php
index 4c6fb1a74f43da2c09011c42f9dabb1267bca328..32077aebc5740d01faaa5fb9871e58726502ac40 100644 (file)
--- a/tests/Entity/ExportTest.php
+++ b/tests/Entity/ExportTest.php
@@ -2,6 +2,7 @@
 
 namespace Tests\Entity;
 
 
 namespace Tests\Entity;
 

+use BookStack\Auth\Role;

 use BookStack\Entities\Models\Book;
 use BookStack\Entities\Models\Chapter;
 use BookStack\Entities\Models\Page;
 use BookStack\Entities\Models\Book;
 use BookStack\Entities\Models\Chapter;
 use BookStack\Entities\Models\Page;


@@ -340,4 +341,29 @@ class ExportTest extends TestCase
         $resp->assertSee('# ' . $chapter->name);
         $resp->assertSee('# ' . $page->name);
     }
         $resp->assertSee('# ' . $chapter->name);
         $resp->assertSee('# ' . $page->name);
     }

+
+    public function test_export_option_only_visible_and_accessible_with_permission()
+    {
+        $book = Book::query()->whereHas('pages')->whereHas('chapters')->first();
+        $chapter = $book->chapters()->first();
+        $page = $chapter->pages()->first();
+        $entities = [$book, $chapter, $page];
+        $user = $this->getViewer();
+        $this->actingAs($user);
+
+        foreach ($entities as $entity) {
+            $resp = $this->get($entity->getUrl());
+            $resp->assertSee("/export/pdf");
+        }
+
+        /** @var Role $role */
+        $this->removePermissionFromUser($user, 'content-export');
+
+        foreach ($entities as $entity) {
+            $resp = $this->get($entity->getUrl());
+            $resp->assertDontSee("/export/pdf");
+            $resp = $this->get($entity->getUrl("/export/pdf"));
+            $this->assertPermissionError($resp);
+        }
+    }

 }
 }

diff --git a/tests/SharedTestHelpers.php b/tests/SharedTestHelpers.php
index 0bc80924e499024a7435aeacc353b48891f115dc..df6c613df4eb2162b2346cb87ebd225bf7b968db 100644 (file)
--- a/tests/SharedTestHelpers.php
+++ b/tests/SharedTestHelpers.php
@@ -4,6 +4,7 @@ namespace Tests;
 
 use BookStack\Auth\Permissions\PermissionService;
 use BookStack\Auth\Permissions\PermissionsRepo;
 
 use BookStack\Auth\Permissions\PermissionService;
 use BookStack\Auth\Permissions\PermissionsRepo;

+use BookStack\Auth\Permissions\RolePermission;

 use BookStack\Auth\Role;
 use BookStack\Auth\User;
 use BookStack\Entities\Models\Book;
 use BookStack\Auth\Role;
 use BookStack\Auth\User;
 use BookStack\Entities\Models\Book;


@@ -18,6 +19,7 @@ use BookStack\Entities\Repos\PageRepo;
 use BookStack\Settings\SettingService;
 use BookStack\Uploads\HttpFetcher;
 use Illuminate\Foundation\Testing\Assert as PHPUnit;
 use BookStack\Settings\SettingService;
 use BookStack\Uploads\HttpFetcher;
 use Illuminate\Foundation\Testing\Assert as PHPUnit;

+use Illuminate\Http\JsonResponse;

 use Illuminate\Support\Env;
 use Illuminate\Support\Facades\Log;
 use Mockery;
 use Illuminate\Support\Env;
 use Illuminate\Support\Facades\Log;
 use Mockery;


@@ -184,6 +186,19 @@ trait SharedTestHelpers
         $user->clearPermissionCache();
     }
 
         $user->clearPermissionCache();
     }
 

+    /**
+     * Completely remove the given permission name from the given user.
+     */
+    protected function removePermissionFromUser(User $user, string $permission)
+    {
+        $permission = RolePermission::query()->where('name', '=', $permission)->first();
+        /** @var Role $role */
+        foreach ($user->roles as $role) {
+            $role->detachPermission($permission);
+        }
+        $user->clearPermissionCache();
+    }
+

     /**
      * Create a new basic role for testing purposes.
      */
     /**
      * Create a new basic role for testing purposes.
      */


@@ -274,8 +289,17 @@ trait SharedTestHelpers
     private function isPermissionError($response): bool
     {
         return $response->status() === 302
     private function isPermissionError($response): bool
     {
         return $response->status() === 302

-            && $response->headers->get('Location') === url('/')
-            && strpos(session()->pull('error', ''), 'You do not have permission to access') === 0;
+            && (
+                (
+                    $response->headers->get('Location') === url('/')
+                    && strpos(session()->pull('error', ''), 'You do not have permission to access') === 0
+                )
+                ||
+                (
+                    $response instanceof JsonResponse &&
+                    $response->json(['error' => 'You do not have permission to perform the requested action.'])
+                )
+            );

     }
 
     /**
     }
 
     /**