Merge branch 'development' into bugfix/fix-being-unable-to-clear-filters

tests/Api/AttachmentsApiTest.php

@@ -50,9 +50,7 @@ class AttachmentsApiTest extends TestCase
             ],
         ]]);
 
-        $page->restricted = true;
-        $page->save();
-        $this->entities->regenPermissions($page);
+        $this->entities->setPermissions($page, [], []);
 
         $resp = $this->getJson($this->baseEndpoint . '?count=1&sort=+id');
         $resp->assertJsonMissing(['data' => [

tests/Commands/CopyShelfPermissionsCommandTest.php

@@ -19,7 +19,7 @@ class CopyShelfPermissionsCommandTest extends TestCase
         $shelf = $this->entities->shelf();
         $child = $shelf->books()->first();
         $editorRole = $this->getEditor()->roles()->first();
-        $this->assertFalse(boolval($child->restricted), 'Child book should not be restricted by default');
+        $this->assertFalse($child->hasPermissions(), 'Child book should not be restricted by default');
         $this->assertTrue($child->permissions()->count() === 0, 'Child book should have no permissions by default');
 
         $this->entities->setPermissions($shelf, ['view', 'update'], [$editorRole]);
@@ -28,10 +28,14 @@ class CopyShelfPermissionsCommandTest extends TestCase
         ]);
         $child = $shelf->books()->first();
 
-        $this->assertTrue(boolval($child->restricted), 'Child book should now be restricted');
-        $this->assertTrue($child->permissions()->count() === 2, 'Child book should have copied permissions');
-        $this->assertDatabaseHas('entity_permissions', ['restrictable_id' => $child->id, 'action' => 'view', 'role_id' => $editorRole->id]);
-        $this->assertDatabaseHas('entity_permissions', ['restrictable_id' => $child->id, 'action' => 'update', 'role_id' => $editorRole->id]);
+        $this->assertTrue($child->hasPermissions(), 'Child book should now be restricted');
+        $this->assertEquals(2, $child->permissions()->count(), 'Child book should have copied permissions');
+        $this->assertDatabaseHas('entity_permissions', [
+            'entity_type' => 'book',
+            'entity_id' => $child->id,
+            'role_id' => $editorRole->id,
+            'view' => true, 'update' => true, 'create' => false, 'delete' => false,
+        ]);
     }
 
     public function test_copy_shelf_permissions_command_using_all()
@@ -40,7 +44,7 @@ class CopyShelfPermissionsCommandTest extends TestCase
         Bookshelf::query()->where('id', '!=', $shelf->id)->delete();
         $child = $shelf->books()->first();
         $editorRole = $this->getEditor()->roles()->first();
-        $this->assertFalse(boolval($child->restricted), 'Child book should not be restricted by default');
+        $this->assertFalse($child->hasPermissions(), 'Child book should not be restricted by default');
         $this->assertTrue($child->permissions()->count() === 0, 'Child book should have no permissions by default');
 
         $this->entities->setPermissions($shelf, ['view', 'update'], [$editorRole]);
@@ -48,9 +52,13 @@ class CopyShelfPermissionsCommandTest extends TestCase
             ->expectsQuestion('Permission settings for all shelves will be cascaded. Books assigned to multiple shelves will receive only the permissions of it\'s last processed shelf. Are you sure you want to proceed?', 'y');
         $child = $shelf->books()->first();
 
-        $this->assertTrue(boolval($child->restricted), 'Child book should now be restricted');
-        $this->assertTrue($child->permissions()->count() === 2, 'Child book should have copied permissions');
-        $this->assertDatabaseHas('entity_permissions', ['restrictable_id' => $child->id, 'action' => 'view', 'role_id' => $editorRole->id]);
-        $this->assertDatabaseHas('entity_permissions', ['restrictable_id' => $child->id, 'action' => 'update', 'role_id' => $editorRole->id]);
+        $this->assertTrue($child->hasPermissions(), 'Child book should now be restricted');
+        $this->assertEquals(2, $child->permissions()->count(), 'Child book should have copied permissions');
+        $this->assertDatabaseHas('entity_permissions', [
+            'entity_type' => 'book',
+            'entity_id' => $child->id,
+            'role_id' => $editorRole->id,
+            'view' => true, 'update' => true, 'create' => false, 'delete' => false,
+        ]);
     }
 }

tests/Entity/BookShelfTest.php

@@ -295,7 +295,7 @@ class BookShelfTest extends TestCase
 
         $child = $shelf->books()->first();
         $editorRole = $this->getEditor()->roles()->first();
-        $this->assertFalse(boolval($child->restricted), 'Child book should not be restricted by default');
+        $this->assertFalse($child->hasPermissions(), 'Child book should not be restricted by default');
         $this->assertTrue($child->permissions()->count() === 0, 'Child book should have no permissions by default');
 
         $this->entities->setPermissions($shelf, ['view', 'update'], [$editorRole]);
@@ -303,10 +303,14 @@ class BookShelfTest extends TestCase
         $child = $shelf->books()->first();
 
         $resp->assertRedirect($shelf->getUrl());
-        $this->assertTrue(boolval($child->restricted), 'Child book should now be restricted');
+        $this->assertTrue($child->hasPermissions(), 'Child book should now be restricted');
         $this->assertTrue($child->permissions()->count() === 2, 'Child book should have copied permissions');
-        $this->assertDatabaseHas('entity_permissions', ['restrictable_id' => $child->id, 'action' => 'view', 'role_id' => $editorRole->id]);
-        $this->assertDatabaseHas('entity_permissions', ['restrictable_id' => $child->id, 'action' => 'update', 'role_id' => $editorRole->id]);
+        $this->assertDatabaseHas('entity_permissions', [
+            'entity_type' => 'book',
+            'entity_id' => $child->id,
+            'role_id' => $editorRole->id,
+            'view' => true, 'update' => true, 'create' => false, 'delete' => false,
+        ]);
     }
 
     public function test_permission_page_has_a_warning_about_no_cascading()

tests/Entity/BookTest.php

@@ -304,9 +304,7 @@ class BookTest extends TestCase
         // Hide child content
         /** @var BookChild $page */
         foreach ($book->getDirectChildren() as $child) {
-            $child->restricted = true;
-            $child->save();
-            $this->entities->regenPermissions($child);
+            $this->entities->setPermissions($child, [], []);
         }
 
         $this->asEditor()->post($book->getUrl('/copy'), ['name' => 'My copy book']);

tests/Entity/ChapterTest.php

@@ -101,9 +101,7 @@ class ChapterTest extends TestCase
         // Hide pages to all non-admin roles
         /** @var Page $page */
         foreach ($chapter->pages as $page) {
-            $page->restricted = true;
-            $page->save();
-            $this->entities->regenPermissions($page);
+            $this->entities->setPermissions($page, [], []);
         }
 
         $this->asEditor()->post($chapter->getUrl('/copy'), [

tests/Entity/EntitySearchTest.php

@@ -132,9 +132,8 @@ class EntitySearchTest extends TestCase
     public function test_search_filters()
     {
         $page = $this->entities->newPage(['name' => 'My new test quaffleachits', 'html' => 'this is about an orange donkey danzorbhsing']);
-        $this->asEditor();
-        $editorId = $this->getEditor()->id;
-        $editorSlug = $this->getEditor()->slug;
+        $editor = $this->getEditor();
+        $this->actingAs($editor);
 
         // Viewed filter searches
         $this->get('/search?term=' . urlencode('danzorbhsing {not_viewed_by_me}'))->assertSee($page->name);
@@ -147,22 +146,22 @@ class EntitySearchTest extends TestCase
         $this->get('/search?term=' . urlencode('danzorbhsing {created_by:me}'))->assertDontSee($page->name);
         $this->get('/search?term=' . urlencode('danzorbhsing {updated_by:me}'))->assertDontSee($page->name);
         $this->get('/search?term=' . urlencode('danzorbhsing {owned_by:me}'))->assertDontSee($page->name);
-        $this->get('/search?term=' . urlencode('danzorbhsing {updated_by:' . $editorSlug . '}'))->assertDontSee($page->name);
-        $page->created_by = $editorId;
+        $this->get('/search?term=' . urlencode('danzorbhsing {updated_by:' . $editor->slug . '}'))->assertDontSee($page->name);
+        $page->created_by = $editor->id;
         $page->save();
         $this->get('/search?term=' . urlencode('danzorbhsing {created_by:me}'))->assertSee($page->name);
-        $this->get('/search?term=' . urlencode('danzorbhsing {created_by: ' . $editorSlug . '}'))->assertSee($page->name);
+        $this->get('/search?term=' . urlencode('danzorbhsing {created_by: ' . $editor->slug . '}'))->assertSee($page->name);
         $this->get('/search?term=' . urlencode('danzorbhsing {updated_by:me}'))->assertDontSee($page->name);
         $this->get('/search?term=' . urlencode('danzorbhsing {owned_by:me}'))->assertDontSee($page->name);
-        $page->updated_by = $editorId;
+        $page->updated_by = $editor->id;
         $page->save();
         $this->get('/search?term=' . urlencode('danzorbhsing {updated_by:me}'))->assertSee($page->name);
-        $this->get('/search?term=' . urlencode('danzorbhsing {updated_by:' . $editorSlug . '}'))->assertSee($page->name);
+        $this->get('/search?term=' . urlencode('danzorbhsing {updated_by:' . $editor->slug . '}'))->assertSee($page->name);
         $this->get('/search?term=' . urlencode('danzorbhsing {owned_by:me}'))->assertDontSee($page->name);
-        $page->owned_by = $editorId;
+        $page->owned_by = $editor->id;
         $page->save();
         $this->get('/search?term=' . urlencode('danzorbhsing {owned_by:me}'))->assertSee($page->name);
-        $this->get('/search?term=' . urlencode('danzorbhsing {owned_by:' . $editorSlug . '}'))->assertSee($page->name);
+        $this->get('/search?term=' . urlencode('danzorbhsing {owned_by:' . $editor->slug . '}'))->assertSee($page->name);
 
         // Content filters
         $this->get('/search?term=' . urlencode('{in_name:danzorbhsing}'))->assertDontSee($page->name);
@@ -172,8 +171,7 @@ class EntitySearchTest extends TestCase
 
         // Restricted filter
         $this->get('/search?term=' . urlencode('danzorbhsing {is_restricted}'))->assertDontSee($page->name);
-        $page->restricted = true;
-        $page->save();
+        $this->entities->setPermissions($page, ['view'], [$editor->roles->first()]);
         $this->get('/search?term=' . urlencode('danzorbhsing {is_restricted}'))->assertSee($page->name);
 
         // Date filters

tests/Entity/TagTest.php

@@ -75,9 +75,7 @@ class TagTest extends TestCase
         $this->asEditor()->get('/ajax/tags/suggest/names?search=co')->assertSimilarJson(['color', 'country']);
 
         // Set restricted permission the page
-        $page->restricted = true;
-        $page->save();
-        $page->rebuildPermissions();
+        $this->entities->setPermissions($page, [], []);
 
         $this->asAdmin()->get('/ajax/tags/suggest/names?search=co')->assertSimilarJson(['color', 'country']);
         $this->asEditor()->get('/ajax/tags/suggest/names?search=co')->assertSimilarJson([]);
@@ -180,8 +178,7 @@ class TagTest extends TestCase
         $resp = $this->get('/tags?name=SuperCategory');
         $resp->assertSee('GreatTestContent');
 
-        $page->restricted = true;
-        $this->entities->regenPermissions($page);
+        $this->entities->setPermissions($page, [], []);
 
         $resp = $this->asEditor()->get('/tags');
         $resp->assertDontSee('SuperCategory');

tests/Helpers/EntityProvider.php

@@ -2,6 +2,7 @@
 
 namespace Tests\Helpers;
 
+use BookStack\Auth\Permissions\EntityPermission;
 use BookStack\Auth\Role;
 use BookStack\Auth\User;
 use BookStack\Entities\Models\Book;
@@ -203,21 +204,22 @@ class EntityProvider
      */
     public function setPermissions(Entity $entity, array $actions = [], array $roles = []): void
     {
-        $entity->restricted = true;
         $entity->permissions()->delete();
 
-        $permissions = [];
-        foreach ($actions as $action) {
-            foreach ($roles as $role) {
-                $permissions[] = [
-                    'role_id' => $role->id,
-                    'action'  => strtolower($action),
-                ];
+        $permissions = [
+            // Set default permissions to not allow actions so that only the provided role permissions are at play.
+            ['role_id' => 0, 'view' => false, 'create' => false, 'update' => false, 'delete' => false],
+        ];
+
+        foreach ($roles as $role) {
+            $permission = ['role_id' => $role->id];
+            foreach (EntityPermission::PERMISSIONS as $possibleAction) {
+                $permission[$possibleAction] = in_array($possibleAction, $actions);
             }
+            $permissions[] = $permission;
         }
 
         $entity->permissions()->createMany($permissions);
-        $entity->save();
         $entity->load('permissions');
         $this->regenPermissions($entity);
     }

tests/Permissions/EntityPermissionsTest.php

@@ -376,20 +376,18 @@ class EntityPermissionsTest extends TestCase
             ->assertSee($title);
 
         $this->put($modelInstance->getUrl('/permissions'), [
-            'restricted'   => 'true',
-            'restrictions' => [
+            'permissions' => [
                 $roleId => [
                     $permission => 'true',
                 ],
             ],
         ]);
 
-        $this->assertDatabaseHas($modelInstance->getTable(), ['id' => $modelInstance->id, 'restricted' => true]);
         $this->assertDatabaseHas('entity_permissions', [
-            'restrictable_id'   => $modelInstance->id,
-            'restrictable_type' => $modelInstance->getMorphClass(),
+            'entity_id'   => $modelInstance->id,
+            'entity_type' => $modelInstance->getMorphClass(),
             'role_id'           => $roleId,
-            'action'            => $permission,
+            $permission         => true,
         ]);
     }
 

tests/Permissions/RolesTest.php

@@ -163,6 +163,29 @@ class RolesTest extends TestCase
         $this->assertEquals($this->user->id, $roleA->users()->first()->id);
     }
 
+    public function test_entity_permissions_are_removed_on_delete()
+    {
+        /** @var Role $roleA */
+        $roleA = Role::query()->create(['display_name' => 'Entity Permissions Delete Test']);
+        $page = $this->entities->page();
+
+        $this->entities->setPermissions($page, ['view'], [$roleA]);
+
+        $this->assertDatabaseHas('entity_permissions', [
+            'role_id' => $roleA->id,
+            'entity_id' => $page->id,
+            'entity_type' => $page->getMorphClass(),
+        ]);
+
+        $this->asAdmin()->delete("/settings/roles/delete/$roleA->id");
+
+        $this->assertDatabaseMissing('entity_permissions', [
+            'role_id' => $roleA->id,
+            'entity_id' => $page->id,
+            'entity_type' => $page->getMorphClass(),
+        ]);
+    }
+
     public function test_image_view_notice_shown_on_role_form()
     {
         /** @var Role $role */

tests/Uploads/AttachmentTest.php

@@ -253,11 +253,7 @@ class AttachmentTest extends TestCase
         $this->uploadFile($fileName, $page->id);
         $attachment = Attachment::orderBy('id', 'desc')->take(1)->first();
 
-        $page->restricted = true;
-        $page->permissions()->delete();
-        $page->save();
-        $page->rebuildPermissions();
-        $page->load('jointPermissions');
+        $this->entities->setPermissions($page, [], []);
 
         $this->actingAs($viewer);
         $attachmentGet = $this->get($attachment->getUrl());