diff --git a/app/Auth/Permissions/PermissionApplicator.php b/app/Auth/Permissions/PermissionApplicator.php index 6ddb152a0..56d2092cb 100644 --- a/app/Auth/Permissions/PermissionApplicator.php +++ b/app/Auth/Permissions/PermissionApplicator.php @@ -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; diff --git a/app/Console/Commands/CopyShelfPermissions.php b/app/Console/Commands/CopyShelfPermissions.php index 18719ae2e..ec4c875ff 100644 --- a/app/Console/Commands/CopyShelfPermissions.php +++ b/app/Console/Commands/CopyShelfPermissions.php @@ -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.'); } diff --git a/app/Entities/Models/Book.php b/app/Entities/Models/Book.php index 4ced9248e..fc4556857 100644 --- a/app/Entities/Models/Book.php +++ b/app/Entities/Models/Book.php @@ -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. diff --git a/app/Entities/Models/Bookshelf.php b/app/Entities/Models/Bookshelf.php index 6310e616f..ad52d9d37 100644 --- a/app/Entities/Models/Bookshelf.php +++ b/app/Entities/Models/Bookshelf.php @@ -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. diff --git a/app/Entities/Models/Chapter.php b/app/Entities/Models/Chapter.php index 53eb5091f..98889ce3f 100644 --- a/app/Entities/Models/Chapter.php +++ b/app/Entities/Models/Chapter.php @@ -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. diff --git a/app/Entities/Models/Entity.php b/app/Entities/Models/Entity.php index 4c399584b..8bfe69365 100644 --- a/app/Entities/Models/Entity.php +++ b/app/Entities/Models/Entity.php @@ -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() diff --git a/app/Entities/Models/Page.php b/app/Entities/Models/Page.php index 8dd769e1c..7a60b3ada 100644 --- a/app/Entities/Models/Page.php +++ b/app/Entities/Models/Page.php @@ -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', diff --git a/app/Entities/Models/PageRevision.php b/app/Entities/Models/PageRevision.php index 6517b0080..cd22db0c8 100644 --- a/app/Entities/Models/PageRevision.php +++ b/app/Entities/Models/PageRevision.php @@ -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. diff --git a/app/Entities/Tools/Cloner.php b/app/Entities/Tools/Cloner.php index 3662b7db3..52a8f4cf0 100644 --- a/app/Entities/Tools/Cloner.php +++ b/app/Entities/Tools/Cloner.php @@ -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); diff --git a/app/Entities/Tools/HierarchyTransformer.php b/app/Entities/Tools/HierarchyTransformer.php index 50d9e2eae..43cf2390e 100644 --- a/app/Entities/Tools/HierarchyTransformer.php +++ b/app/Entities/Tools/HierarchyTransformer.php @@ -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); } } diff --git a/app/Entities/Tools/PermissionsUpdater.php b/app/Entities/Tools/PermissionsUpdater.php index 2747def18..eb4eb6b48 100644 --- a/app/Entities/Tools/PermissionsUpdater.php +++ b/app/Entities/Tools/PermissionsUpdater.php @@ -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++; } diff --git a/app/Http/Controllers/FavouriteController.php b/app/Http/Controllers/FavouriteController.php index f77b04843..e46442a64 100644 --- a/app/Http/Controllers/FavouriteController.php +++ b/app/Http/Controllers/FavouriteController.php @@ -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) { diff --git a/tests/Api/AttachmentsApiTest.php b/tests/Api/AttachmentsApiTest.php index c295f7384..4d1d3b340 100644 --- a/tests/Api/AttachmentsApiTest.php +++ b/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' => [ diff --git a/tests/Commands/CopyShelfPermissionsCommandTest.php b/tests/Commands/CopyShelfPermissionsCommandTest.php index 55b710ba9..4ff4fb78b 100644 --- a/tests/Commands/CopyShelfPermissionsCommandTest.php +++ b/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(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]); diff --git a/tests/Entity/BookShelfTest.php b/tests/Entity/BookShelfTest.php index 1e740b94e..6a0bb94d5 100644 --- a/tests/Entity/BookShelfTest.php +++ b/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(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]); diff --git a/tests/Entity/BookTest.php b/tests/Entity/BookTest.php index 2914162cf..cccff3a1f 100644 --- a/tests/Entity/BookTest.php +++ b/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']); diff --git a/tests/Entity/ChapterTest.php b/tests/Entity/ChapterTest.php index afc60c20e..b726280c9 100644 --- a/tests/Entity/ChapterTest.php +++ b/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'), [ diff --git a/tests/Entity/EntitySearchTest.php b/tests/Entity/EntitySearchTest.php index cdb500a45..21f5dfc03 100644 --- a/tests/Entity/EntitySearchTest.php +++ b/tests/Entity/EntitySearchTest.php @@ -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 diff --git a/tests/Entity/TagTest.php b/tests/Entity/TagTest.php index ab0627601..ed5c798a5 100644 --- a/tests/Entity/TagTest.php +++ b/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'); diff --git a/tests/Helpers/EntityProvider.php b/tests/Helpers/EntityProvider.php index 70678a6a5..4af6957a1 100644 --- a/tests/Helpers/EntityProvider.php +++ b/tests/Helpers/EntityProvider.php @@ -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); } diff --git a/tests/Permissions/EntityPermissionsTest.php b/tests/Permissions/EntityPermissionsTest.php index 7f91e7887..e88909dba 100644 --- a/tests/Permissions/EntityPermissionsTest.php +++ b/tests/Permissions/EntityPermissionsTest.php @@ -376,7 +376,6 @@ class EntityPermissionsTest extends TestCase ->assertSee($title); $this->put($modelInstance->getUrl('/permissions'), [ - 'restricted' => 'true', 'restrictions' => [ $roleId => [ $permission => 'true', diff --git a/tests/Uploads/AttachmentTest.php b/tests/Uploads/AttachmentTest.php index 915a9ba4d..b6fcb8f69 100644 --- a/tests/Uploads/AttachmentTest.php +++ b/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());