Migrated remaining relation permission usages

Now all tests are passing.
Some level of manual checks to do.

diff --git a/app/Auth/Permissions/PermissionApplicator.php b/app/Auth/Permissions/PermissionApplicator.php
index 4f95465af159bd80a62ef4a7fd13817fb2f2c79a..437ddb0fba33ff3274aec763bbcf00ae3fc36d4b 100644 (file)
--- a/app/Auth/Permissions/PermissionApplicator.php
+++ b/app/Auth/Permissions/PermissionApplicator.php
@@ -150,51 +150,16 @@ class PermissionApplicator
      */
     public function restrictPageRelationQuery(Builder $query, string $tableName, string $pageIdColumn): Builder
     {
-        // TODO - Refactor
         $fullPageIdColumn = $tableName . '.' . $pageIdColumn;
-        $morphClass = (new Page())->getMorphClass();
-
-        $existsQuery = function ($permissionQuery) use ($fullPageIdColumn, $morphClass) {
-            /** @var Builder $permissionQuery */
-            $permissionQuery->select('joint_permissions.role_id')->from('joint_permissions')
-                ->whereColumn('joint_permissions.entity_id', '=', $fullPageIdColumn)
-                ->where('joint_permissions.entity_type', '=', $morphClass)
-                ->whereIn('joint_permissions.role_id', $this->getCurrentUserRoleIds())
-                ->where(function (QueryBuilder $query) {
-                    $this->addJointHasPermissionCheck($query, $this->currentUser()->id);
-                });
-        };
-
-        $q = $query->where(function ($query) use ($existsQuery, $fullPageIdColumn) {
-            $query->whereExists($existsQuery)
-                ->orWhere($fullPageIdColumn, '=', 0);
-        });
-
-        // Prevent visibility of non-owned draft pages
-        $q->whereExists(function (QueryBuilder $query) use ($fullPageIdColumn) {
-            $query->select('id')->from('pages')
-                ->whereColumn('pages.id', '=', $fullPageIdColumn)
-                ->where(function (QueryBuilder $query) {
-                    $query->where('pages.draft', '=', false)
-                        ->orWhere('pages.owned_by', '=', $this->currentUser()->id);
+        return $this->restrictEntityQuery($query)
+            ->where(function ($query) use ($fullPageIdColumn) {
+                /** @var Builder $query */
+                $query->whereExists(function (QueryBuilder $query) use ($fullPageIdColumn) {
+                    $query->select('id')->from('pages')
+                        ->whereColumn('pages.id', '=', $fullPageIdColumn)
+                        ->where('pages.draft', '=', false);
                 });
-        });
-
-        return $q;
-    }
-
-    /**
-     * Add the query for checking the given user id has permission
-     * within the join_permissions table.
-     *
-     * @param QueryBuilder|Builder $query
-     */
-    protected function addJointHasPermissionCheck($query, int $userIdToCheck)
-    {
-        $query->where('joint_permissions.has_permission', '=', true)->orWhere(function ($query) use ($userIdToCheck) {
-            $query->where('joint_permissions.has_permission_own', '=', true)
-                ->where('joint_permissions.owned_by', '=', $userIdToCheck);
-        });
+            });
     }
 
     /**

diff --git a/app/Auth/User.php b/app/Auth/User.php
index 6e66bc808296c4c531568ac516c93412000a07c9..cf9f20e52cc015f9a3073faba6d5d0892d31cd86 100644 (file)
--- a/app/Auth/User.php
+++ b/app/Auth/User.php
@@ -200,6 +200,7 @@ class User extends Model implements AuthenticatableContract, CanResetPasswordCon
     public function attachRole(Role $role)
     {
         $this->roles()->attach($role->id);
+        $this->unsetRelation('roles');
     }
 
     /**

diff --git a/app/Uploads/Attachment.php b/app/Uploads/Attachment.php
index 6c7066ff9701be00137b4d65c3a90f61d5ee038b..fc86d36ea8235b58265b6b03f9acb711c2e5e2dc 100644 (file)
--- a/app/Uploads/Attachment.php
+++ b/app/Uploads/Attachment.php
@@ -2,6 +2,7 @@
 
 namespace BookStack\Uploads;
 
+use BookStack\Auth\Permissions\JointPermission;
 use BookStack\Auth\Permissions\PermissionApplicator;
 use BookStack\Auth\User;
 use BookStack\Entities\Models\Entity;
@@ -10,6 +11,7 @@ use BookStack\Model;
 use BookStack\Traits\HasCreatorAndUpdater;
 use Illuminate\Database\Eloquent\Builder;
 use Illuminate\Database\Eloquent\Relations\BelongsTo;
+use Illuminate\Database\Eloquent\Relations\HasMany;
 
 /**
  * @property int    $id
@@ -56,6 +58,12 @@ class Attachment extends Model
         return $this->belongsTo(Page::class, 'uploaded_to');
     }
 
+    public function jointPermissions(): HasMany
+    {
+        return $this->hasMany(JointPermission::class, 'entity_id', 'uploaded_to')
+            ->where('joint_permissions.entity_type', '=', 'page');
+    }
+
     /**
      * Get the url of this file.
      */

diff --git a/app/Uploads/Image.php b/app/Uploads/Image.php
index bdf10f080fe99cd991f52bc0174e75cc4c5ac00f..c21a3b03fe5a27aee99faca7417b0f4a661b8b0c 100644 (file)
--- a/app/Uploads/Image.php
+++ b/app/Uploads/Image.php
@@ -2,10 +2,12 @@
 
 namespace BookStack\Uploads;
 
+use BookStack\Auth\Permissions\JointPermission;
 use BookStack\Entities\Models\Page;
 use BookStack\Model;
 use BookStack\Traits\HasCreatorAndUpdater;
 use Illuminate\Database\Eloquent\Factories\HasFactory;
+use Illuminate\Database\Eloquent\Relations\HasMany;
 
 /**
  * @property int    $id
@@ -25,6 +27,12 @@ class Image extends Model
     protected $fillable = ['name'];
     protected $hidden = [];
 
+    public function jointPermissions(): HasMany
+    {
+        return $this->hasMany(JointPermission::class, 'entity_id', 'uploaded_to')
+            ->where('joint_permissions.entity_type', '=', 'page');
+    }
+
     /**
      * Get a thumbnail for this image.
      *

diff --git a/tests/Commands/RegeneratePermissionsCommandTest.php b/tests/Commands/RegeneratePermissionsCommandTest.php
index b916a8060f84bdb4afa3082f22c773e0e1a5d02a..9cf7dec93d6ae54bd4a5c6ae281fa7abb268cc62 100644 (file)
--- a/tests/Commands/RegeneratePermissionsCommandTest.php
+++ b/tests/Commands/RegeneratePermissionsCommandTest.php
@@ -29,7 +29,7 @@ class RegeneratePermissionsCommandTest extends TestCase
             'entity_id' => $page->id,
             'entity_type' => 'page',
             'role_id' => $role->id,
-            'has_permission' => 1,
+            'status' => 3, // Explicit allow
         ]);
 
         $page->permissions()->delete();

diff --git a/tests/Permissions/EntityPermissionsTest.php b/tests/Permissions/EntityPermissionsTest.php
index ab8b1242df2db3c4871f773cb63289e1b5c28544..99a8bd88c1dd26fa75bf281ac22d303146f6deda 100644 (file)
--- a/tests/Permissions/EntityPermissionsTest.php
+++ b/tests/Permissions/EntityPermissionsTest.php
@@ -663,7 +663,7 @@ class EntityPermissionsTest extends TestCase
         $chapter = $this->entities->chapter();
         $book = $chapter->book;
 
-        $this->permissions->setEntityPermissions($book, ['edit'], [$viewerRole], false);
+        $this->permissions->setEntityPermissions($book, ['update'], [$viewerRole], false);
         $this->permissions->setEntityPermissions($chapter, [], [$viewerRole], true);
 
         $this->assertFalse(userCan('chapter-update', $chapter));
@@ -678,9 +678,10 @@ class EntityPermissionsTest extends TestCase
         $chapter = $this->entities->chapter();
         $book = $chapter->book;
 
-        $this->permissions->setEntityPermissions($book, ['edit'], [$editorRole], false);
+        $this->permissions->setEntityPermissions($book, ['update'], [$editorRole], false);
         $this->permissions->setEntityPermissions($chapter, [], [$viewerRole], true);
 
+        $this->actingAs($user);
         $this->assertTrue(userCan('chapter-update', $chapter));
     }