]> BookStack Code Mirror - bookstack/commitdiff
Removed most usages of restricted entitiy property
authorDan Brown <redacted>
Mon, 10 Oct 2022 15:58:26 +0000 (16:58 +0100)
committerDan Brown <redacted>
Mon, 10 Oct 2022 15:58:26 +0000 (16:58 +0100)
22 files changed:
app/Auth/Permissions/PermissionApplicator.php
app/Console/Commands/CopyShelfPermissions.php
app/Entities/Models/Book.php
app/Entities/Models/Bookshelf.php
app/Entities/Models/Chapter.php
app/Entities/Models/Entity.php
app/Entities/Models/Page.php
app/Entities/Models/PageRevision.php
app/Entities/Tools/Cloner.php
app/Entities/Tools/HierarchyTransformer.php
app/Entities/Tools/PermissionsUpdater.php
app/Http/Controllers/FavouriteController.php
tests/Api/AttachmentsApiTest.php
tests/Commands/CopyShelfPermissionsCommandTest.php
tests/Entity/BookShelfTest.php
tests/Entity/BookTest.php
tests/Entity/ChapterTest.php
tests/Entity/EntitySearchTest.php
tests/Entity/TagTest.php
tests/Helpers/EntityProvider.php
tests/Permissions/EntityPermissionsTest.php
tests/Uploads/AttachmentTest.php

index 6ddb152a00ecc57084ec9768e699cf88f3441d0f..56d2092cb74a20846ea17c94aa870eeccc1bd46a 100644 (file)
@@ -66,6 +66,8 @@ class PermissionApplicator
             return true;
         }
 
+        // The chain order here is very important due to the fact we walk up the chain
+        // in the loop below. Earlier items in the chain have higher priority.
         $chain = [$entity];
         if ($entity instanceof Page && $entity->chapter_id) {
             $chain[] = $entity->chapter;
@@ -76,16 +78,26 @@ class PermissionApplicator
         }
 
         foreach ($chain as $currentEntity) {
-            if (is_null($currentEntity->restricted)) {
-                throw new InvalidArgumentException('Entity restricted field used but has not been loaded');
+            $allowedByRoleId = $currentEntity->permissions()
+                ->whereIn('role_id', [0, ...$userRoleIds])
+                ->pluck($action, 'role_id');
+
+            // Continue up the chain if no applicable entity permission overrides.
+            if (empty($allowedByRoleId)) {
+                continue;
             }
 
-            if ($currentEntity->restricted) {
-                return $currentEntity->permissions()
-                    ->whereIn('role_id', $userRoleIds)
-                    ->where($action, '=', true)
-                    ->count() > 0;
+            // If we have user-role-specific permissions set, allow if any of those
+            // role permissions allow access.
+            $hasDefault = $allowedByRoleId->has(0);
+            if (!$hasDefault || $allowedByRoleId->count() > 1) {
+                return $allowedByRoleId->search(function (bool $allowed, int $roleId) {
+                        return $roleId !== 0 && $allowed;
+                }) !== false;
             }
+
+            // Otherwise, return the default "Other roles" fallback value.
+            return $allowedByRoleId->get(0);
         }
 
         return null;
index 18719ae2efeb482f6ae6c245626f8863a332a1cd..ec4c875ffa97d3935d0101383559d7ff63508c54 100644 (file)
@@ -66,11 +66,11 @@ class CopyShelfPermissions extends Command
                 return;
             }
 
-            $shelves = Bookshelf::query()->get(['id', 'restricted']);
+            $shelves = Bookshelf::query()->get(['id']);
         }
 
         if ($shelfSlug) {
-            $shelves = Bookshelf::query()->where('slug', '=', $shelfSlug)->get(['id', 'restricted']);
+            $shelves = Bookshelf::query()->where('slug', '=', $shelfSlug)->get(['id']);
             if ($shelves->count() === 0) {
                 $this->info('No shelves found with the given slug.');
             }
index 4ced9248e64e8b813a5f9a03692ad22d8371a4ae..fc4556857c72a5d00ed9ec1bb68734489e973e36 100644 (file)
@@ -28,7 +28,7 @@ class Book extends Entity implements HasCoverImage
     public $searchFactor = 1.2;
 
     protected $fillable = ['name', 'description'];
-    protected $hidden = ['restricted', 'pivot', 'image_id', 'deleted_at'];
+    protected $hidden = ['pivot', 'image_id', 'deleted_at'];
 
     /**
      * Get the url for this book.
index 6310e616f1dc9a42f4729e72a35eb2e07014893b..ad52d9d37d4f0ce7df280e7d0fd0052c1b79491b 100644 (file)
@@ -17,7 +17,7 @@ class Bookshelf extends Entity implements HasCoverImage
 
     protected $fillable = ['name', 'description', 'image_id'];
 
-    protected $hidden = ['restricted', 'image_id', 'deleted_at'];
+    protected $hidden = ['image_id', 'deleted_at'];
 
     /**
      * Get the books in this shelf.
index 53eb5091f736cc958a8b09c82e1e9845cbaf2da8..98889ce3f38a430c15d281d64b20b05491fc9bb9 100644 (file)
@@ -19,7 +19,7 @@ class Chapter extends BookChild
     public $searchFactor = 1.2;
 
     protected $fillable = ['name', 'description', 'priority'];
-    protected $hidden = ['restricted', 'pivot', 'deleted_at'];
+    protected $hidden = ['pivot', 'deleted_at'];
 
     /**
      * Get the pages that this chapter contains.
index 4c399584b3b32366a217836992645f170e086d05..8bfe69365ef47e529de6178a03f9e3fe72d80781 100644 (file)
@@ -42,7 +42,6 @@ use Illuminate\Database\Eloquent\SoftDeletes;
  * @property Carbon     $deleted_at
  * @property int        $created_by
  * @property int        $updated_by
- * @property bool       $restricted
  * @property Collection $tags
  *
  * @method static Entity|Builder visible()
index 8dd769e1cdb25d2c8dc7db3905e9500b6ca50823..7a60b3ada05a2c78f5e4c95f7886b1d31ed42619 100644 (file)
@@ -39,7 +39,7 @@ class Page extends BookChild
 
     public $textField = 'text';
 
-    protected $hidden = ['html', 'markdown', 'text', 'restricted', 'pivot', 'deleted_at'];
+    protected $hidden = ['html', 'markdown', 'text', 'pivot', 'deleted_at'];
 
     protected $casts = [
         'draft'    => 'boolean',
index 6517b0080bace9a69fd8892972e1542ac83a34bc..cd22db0c83c19e62441af50bd9488281aac9c503 100644 (file)
@@ -31,7 +31,7 @@ use Illuminate\Database\Eloquent\Relations\BelongsTo;
 class PageRevision extends Model implements Loggable
 {
     protected $fillable = ['name', 'text', 'summary'];
-    protected $hidden = ['html', 'markdown', 'restricted', 'text'];
+    protected $hidden = ['html', 'markdown', 'text'];
 
     /**
      * Get the user that created the page revision.
index 3662b7db326c2ed4a4de92d4fa227ebfb8159660..52a8f4cf0f2760f45d546803a0e8215d61e6de0f 100644 (file)
@@ -122,7 +122,6 @@ class Cloner
      */
     public function copyEntityPermissions(Entity $sourceEntity, Entity $targetEntity): void
     {
-        $targetEntity->restricted = $sourceEntity->restricted;
         $permissions = $sourceEntity->permissions()->get(['role_id', 'view', 'create', 'update', 'delete'])->toArray();
         $targetEntity->permissions()->delete();
         $targetEntity->permissions()->createMany($permissions);
index 50d9e2eaeef3db682deba89112dea6dca167e36e..43cf2390e7a5de683399674bc040323866eafa66 100644 (file)
@@ -65,7 +65,7 @@ class HierarchyTransformer
         foreach ($book->chapters as $index => $chapter) {
             $newBook = $this->transformChapterToBook($chapter);
             $shelfBookSyncData[$newBook->id] = ['order' => $index];
-            if (!$newBook->restricted) {
+            if (!$newBook->hasPermissions()) {
                 $this->cloner->copyEntityPermissions($shelf, $newBook);
             }
         }
index 2747def18df1daf723d8777ec3f89506e74a5f1f..eb4eb6b48581ae037fd95f911b46beea836bee83 100644 (file)
@@ -75,9 +75,8 @@ class PermissionsUpdater
      */
     public function updateBookPermissionsFromShelf(Bookshelf $shelf, $checkUserPermissions = true): int
     {
-        // TODO - Fix for new format
         $shelfPermissions = $shelf->permissions()->get(['role_id', 'view', 'create', 'update', 'delete'])->toArray();
-        $shelfBooks = $shelf->books()->get(['id', 'restricted', 'owned_by']);
+        $shelfBooks = $shelf->books()->get(['id', 'owned_by']);
         $updatedBookCount = 0;
 
         /** @var Book $book */
@@ -86,9 +85,7 @@ class PermissionsUpdater
                 continue;
             }
             $book->permissions()->delete();
-            $book->restricted = $shelf->restricted;
             $book->permissions()->createMany($shelfPermissions);
-            $book->save();
             $book->rebuildPermissions();
             $updatedBookCount++;
         }
index f77b04843ec049f1a0cdf0ccd32413755cb08c08..e46442a643449348ed0f212fee1fb7a00c091b65 100644 (file)
@@ -87,7 +87,7 @@ class FavouriteController extends Controller
 
         $modelInstance = $model->newQuery()
             ->where('id', '=', $modelInfo['id'])
-            ->first(['id', 'name', 'restricted', 'owned_by']);
+            ->first(['id', 'name', 'owned_by']);
 
         $inaccessibleEntity = ($modelInstance instanceof Entity && !userCan('view', $modelInstance));
         if (is_null($modelInstance) || $inaccessibleEntity) {
index c295f738439c2cd1a5cc53d8d0a6b378207e077c..4d1d3b340b153c3c588ebdcb7103453d5f98d4be 100644 (file)
@@ -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' => [
index 55b710ba9afeb28d8854426d37ed7a940ed0a04d..4ff4fb78b1a10a39fb23f32725c237d69235fef7 100644 (file)
@@ -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(boolval($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,7 +28,7 @@ class CopyShelfPermissionsCommandTest extends TestCase
         ]);
         $child = $shelf->books()->first();
 
-        $this->assertTrue(boolval($child->restricted), 'Child book should now be restricted');
+        $this->assertTrue(boolval($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]);
@@ -40,7 +40,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(boolval($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,7 +48,7 @@ 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(boolval($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]);
index 1e740b94eb89820086492aefcfa0b2c00a5dd427..6a0bb94d5cb21c3711d55cd774305d0b808e7eba 100644 (file)
@@ -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(boolval($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,7 +303,7 @@ 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(boolval($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]);
index 2914162cf6dfb7f3b9a10f5a4f7ea330e38467c2..cccff3a1f58da6edb792267e85ca4c0297ba0856 100644 (file)
@@ -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']);
index afc60c20eda782819bb23c5f05fbfad94f5441b4..b726280c9184afe8ded72ca442d8b53c58d67bb9 100644 (file)
@@ -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'), [
index cdb500a45ccdeb71b70382881b4303c05e3c542d..21f5dfc03d119de64641a51a24c11fbc572baa39 100644 (file)
@@ -172,8 +172,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, [], []);
         $this->get('/search?term=' . urlencode('danzorbhsing {is_restricted}'))->assertSee($page->name);
 
         // Date filters
index ab06276013f041a4c99a686f6f379a4e0e42969a..ed5c798a5f614be01dbffc714f23ec2badf71dac 100644 (file)
@@ -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');
index 70678a6a513f92c8c49cb344622acc2bb6661b37..4af6957a182a7d369a5078ccfe01f55296d01497 100644 (file)
@@ -204,7 +204,6 @@ class EntityProvider
      */
     public function setPermissions(Entity $entity, array $actions = [], array $roles = []): void
     {
-        $entity->restricted = true;
         $entity->permissions()->delete();
 
         $permissions = [];
@@ -217,7 +216,6 @@ class EntityProvider
         }
 
         $entity->permissions()->createMany($permissions);
-        $entity->save();
         $entity->load('permissions');
         $this->regenPermissions($entity);
     }
index 7f91e7887b96887a610cfcff59c354b16f1c8f4c..e88909dba16ef25f8455398b270e0657d64cd4aa 100644 (file)
@@ -376,7 +376,6 @@ class EntityPermissionsTest extends TestCase
             ->assertSee($title);
 
         $this->put($modelInstance->getUrl('/permissions'), [
-            'restricted'   => 'true',
             'restrictions' => [
                 $roleId => [
                     $permission => 'true',
index 915a9ba4d26a5fec7bcd4454919cecd19e2ecd8e..b6fcb8f69995d3767d5dc1a8f3f140b5f469d66a 100644 (file)
@@ -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());