Implemented alternate approach to current joint_permissions

Is a tweak upon the existing approach, mainly to store and query role
permission access in a way that allows muli-level states that may
override eachother. These states are represented in the new PermissionStatus
class.

This also simplifies how own permissions are stored and queried, to be
part of a single column.

diff --git a/app/Actions/Activity.php b/app/Actions/Activity.php
index 3b1408cb94ded300edfe39c978b1b3389b2474cc..0789fe123115f0044166fa9ecc7e10b9f0d1d2cd 100644 (file)
--- a/app/Actions/Activity.php
+++ b/app/Actions/Activity.php
@@ -2,10 +2,12 @@
 
 namespace BookStack\Actions;
 
 
 namespace BookStack\Actions;
 

+use BookStack\Auth\Permissions\JointPermission;

 use BookStack\Auth\User;
 use BookStack\Entities\Models\Entity;
 use BookStack\Model;
 use Illuminate\Database\Eloquent\Relations\BelongsTo;
 use BookStack\Auth\User;
 use BookStack\Entities\Models\Entity;
 use BookStack\Model;
 use Illuminate\Database\Eloquent\Relations\BelongsTo;

+use Illuminate\Database\Eloquent\Relations\HasMany;

 use Illuminate\Database\Eloquent\Relations\MorphTo;
 use Illuminate\Support\Str;
 
 use Illuminate\Database\Eloquent\Relations\MorphTo;
 use Illuminate\Support\Str;
 


@@ -40,6 +42,12 @@ class Activity extends Model
         return $this->belongsTo(User::class);
     }
 
         return $this->belongsTo(User::class);
     }
 

+    public function jointPermissions(): HasMany
+    {
+        return $this->hasMany(JointPermission::class, 'entity_id', 'entity_id')
+            ->whereColumn('activities.entity_type', '=', 'joint_permissions.entity_type');
+    }
+

     /**
      * Returns text from the language files, Looks up by using the activity key.
      */
     /**
      * Returns text from the language files, Looks up by using the activity key.
      */

diff --git a/app/Actions/Favourite.php b/app/Actions/Favourite.php
index f4589418226d5875b908a82a33e3aff1d9870aa6..c5d12a1514ace68a5b407a9fae2e945f5c228533 100644 (file)
--- a/app/Actions/Favourite.php
+++ b/app/Actions/Favourite.php
@@ -2,7 +2,9 @@
 
 namespace BookStack\Actions;
 
 
 namespace BookStack\Actions;
 

+use BookStack\Auth\Permissions\JointPermission;

 use BookStack\Model;
 use BookStack\Model;

+use Illuminate\Database\Eloquent\Relations\HasMany;

 use Illuminate\Database\Eloquent\Relations\MorphTo;
 
 class Favourite extends Model
 use Illuminate\Database\Eloquent\Relations\MorphTo;
 
 class Favourite extends Model


@@ -16,4 +18,10 @@ class Favourite extends Model
     {
         return $this->morphTo();
     }
     {
         return $this->morphTo();
     }

+
+    public function jointPermissions(): HasMany
+    {
+        return $this->hasMany(JointPermission::class, 'entity_id', 'favouritable_id')
+            ->whereColumn('favourites.favouritable_type', '=', 'joint_permissions.entity_type');
+    }

 }
 }

diff --git a/app/Actions/Tag.php b/app/Actions/Tag.php
index 609c299ad887f2758c973ec197f1b572f1a11eb2..e173faea0c8250db944d0e365d23262667b46be2 100644 (file)
--- a/app/Actions/Tag.php
+++ b/app/Actions/Tag.php
@@ -2,8 +2,10 @@
 
 namespace BookStack\Actions;
 
 
 namespace BookStack\Actions;
 

+use BookStack\Auth\Permissions\JointPermission;

 use BookStack\Model;
 use Illuminate\Database\Eloquent\Factories\HasFactory;
 use BookStack\Model;
 use Illuminate\Database\Eloquent\Factories\HasFactory;

+use Illuminate\Database\Eloquent\Relations\HasMany;

 use Illuminate\Database\Eloquent\Relations\MorphTo;
 
 /**
 use Illuminate\Database\Eloquent\Relations\MorphTo;
 
 /**


@@ -27,6 +29,12 @@ class Tag extends Model
         return $this->morphTo('entity');
     }
 
         return $this->morphTo('entity');
     }
 

+    public function jointPermissions(): HasMany
+    {
+        return $this->hasMany(JointPermission::class, 'entity_id', 'entity_id')
+            ->whereColumn('tags.entity_type', '=', 'joint_permissions.entity_type');
+    }
+

     /**
      * Get a full URL to start a tag name search for this tag name.
      */
     /**
      * Get a full URL to start a tag name search for this tag name.
      */

diff --git a/app/Actions/View.php b/app/Actions/View.php
index 16961bd914bef8b29fc659d6f689fd7f56b99376..706467133fa5ecc36ec6aaeaecd9e48f88595f48 100644 (file)
--- a/app/Actions/View.php
+++ b/app/Actions/View.php
@@ -2,8 +2,10 @@
 
 namespace BookStack\Actions;
 
 
 namespace BookStack\Actions;
 

+use BookStack\Auth\Permissions\JointPermission;

 use BookStack\Interfaces\Viewable;
 use BookStack\Model;
 use BookStack\Interfaces\Viewable;
 use BookStack\Model;

+use Illuminate\Database\Eloquent\Relations\HasMany;

 use Illuminate\Database\Eloquent\Relations\MorphTo;
 
 /**
 use Illuminate\Database\Eloquent\Relations\MorphTo;
 
 /**


@@ -28,6 +30,12 @@ class View extends Model
         return $this->morphTo();
     }
 
         return $this->morphTo();
     }
 

+    public function jointPermissions(): HasMany
+    {
+        return $this->hasMany(JointPermission::class, 'entity_id', 'viewable_id')
+            ->whereColumn('views.viewable_type', '=', 'joint_permissions.entity_type');
+    }
+

     /**
      * Increment the current user's view count for the given viewable model.
      */
     /**
      * Increment the current user's view count for the given viewable model.
      */

diff --git a/app/Auth/Permissions/JointPermissionBuilder.php b/app/Auth/Permissions/JointPermissionBuilder.php
index 91207e3ab2be268cc9952bc7a03e223fd3e4b67c..bbdf4d6f8a196de6ae27703bc67b761dd4340cc9 100644 (file)
--- a/app/Auth/Permissions/JointPermissionBuilder.php
+++ b/app/Auth/Permissions/JointPermissionBuilder.php
@@ -256,35 +256,38 @@ class JointPermissionBuilder
     {
         // Ensure system admin role retains permissions
         if ($isAdminRole) {
     {
         // Ensure system admin role retains permissions
         if ($isAdminRole) {

-            return $this->createJointPermissionDataArray($entity, $roleId, true, true);
+            return $this->createJointPermissionDataArray($entity, $roleId, PermissionStatus::EXPLICIT_ALLOW, true);

         }
 
         // Return evaluated entity permission status if it has an affect.
         $entityPermissionStatus = $permissionMap->evaluateEntityForRole($entity, $roleId);
         if ($entityPermissionStatus !== null) {
         }
 
         // Return evaluated entity permission status if it has an affect.
         $entityPermissionStatus = $permissionMap->evaluateEntityForRole($entity, $roleId);
         if ($entityPermissionStatus !== null) {

-            return $this->createJointPermissionDataArray($entity, $roleId, $entityPermissionStatus, $entityPermissionStatus);
+            $status = $entityPermissionStatus ? PermissionStatus::EXPLICIT_ALLOW : PermissionStatus::EXPLICIT_DENY;
+            return $this->createJointPermissionDataArray($entity, $roleId, $status, $entityPermissionStatus);

         }
 
         // Otherwise default to the role-level permissions
         $permissionPrefix = $entity->type . '-view';
         $roleHasPermission = isset($rolePermissionMap[$roleId . ':' . $permissionPrefix . '-all']);
         $roleHasPermissionOwn = isset($rolePermissionMap[$roleId . ':' . $permissionPrefix . '-own']);
         }
 
         // Otherwise default to the role-level permissions
         $permissionPrefix = $entity->type . '-view';
         $roleHasPermission = isset($rolePermissionMap[$roleId . ':' . $permissionPrefix . '-all']);
         $roleHasPermissionOwn = isset($rolePermissionMap[$roleId . ':' . $permissionPrefix . '-own']);

-        return $this->createJointPermissionDataArray($entity, $roleId, $roleHasPermission, $roleHasPermissionOwn);
+        $status = $roleHasPermission ? PermissionStatus::IMPLICIT_ALLOW : PermissionStatus::IMPLICIT_DENY;
+        return $this->createJointPermissionDataArray($entity, $roleId, $status, $roleHasPermissionOwn);

     }
 
     /**
      * Create an array of data with the information of an entity jointPermissions.
      * Used to build data for bulk insertion.
      */
     }
 
     /**
      * Create an array of data with the information of an entity jointPermissions.
      * Used to build data for bulk insertion.
      */

-    protected function createJointPermissionDataArray(SimpleEntityData $entity, int $roleId, bool $permissionAll, bool $permissionOwn): array
+    protected function createJointPermissionDataArray(SimpleEntityData $entity, int $roleId, int $permissionStatus, bool $hasPermissionOwn): array

     {
     {

+        $ownPermissionActive = ($hasPermissionOwn && $permissionStatus !== PermissionStatus::EXPLICIT_DENY && $entity->owned_by);
+

         return [
         return [

-            'entity_id'          => $entity->id,
-            'entity_type'        => $entity->type,
-            'has_permission'     => $permissionAll,
-            'has_permission_own' => $permissionOwn,
-            'owned_by'           => $entity->owned_by,
-            'role_id'            => $roleId,
+            'entity_id'   => $entity->id,
+            'entity_type' => $entity->type,
+            'role_id'     => $roleId,
+            'status'      => $permissionStatus,
+            'owner_id'    => $ownPermissionActive ? $entity->owned_by : null,

         ];
     }
 }
         ];
     }
 }

diff --git a/app/Auth/Permissions/PermissionApplicator.php b/app/Auth/Permissions/PermissionApplicator.php
index e4564ddf590966b9c58fe24badcaaeecf0747161..4f95465af159bd80a62ef4a7fd13817fb2f2c79a 100644 (file)
--- a/app/Auth/Permissions/PermissionApplicator.php
+++ b/app/Auth/Permissions/PermissionApplicator.php
@@ -95,13 +95,11 @@ class PermissionApplicator
         return $query->where(function (Builder $parentQuery) {
             $parentQuery->whereHas('jointPermissions', function (Builder $permissionQuery) {
                 $permissionQuery->select(['entity_id', 'entity_type'])
         return $query->where(function (Builder $parentQuery) {
             $parentQuery->whereHas('jointPermissions', function (Builder $permissionQuery) {
                 $permissionQuery->select(['entity_id', 'entity_type'])

-                    ->selectRaw('max(owned_by) as owned_by')
-                    ->selectRaw('max(has_permission) as has_permission')
-                    ->selectRaw('max(has_permission_own) as has_permission_own')
+                    ->selectRaw('max(owner_id) as owner_id')
+                    ->selectRaw('max(status) as status')

                     ->whereIn('role_id', $this->getCurrentUserRoleIds())
                     ->groupBy(['entity_type', 'entity_id'])
                     ->whereIn('role_id', $this->getCurrentUserRoleIds())
                     ->groupBy(['entity_type', 'entity_id'])

-                    ->havingRaw('has_permission > 0')
-                    ->orHavingRaw('(has_permission_own > 0 and owned_by = ?)', [$this->currentUser()->id]);
+                    ->havingRaw('(status IN (1, 3) or owner_id = ?)', [$this->currentUser()->id]);

             });
         });
     }
             });
         });
     }


@@ -125,35 +123,23 @@ class PermissionApplicator
      * Filter items that have entities set as a polymorphic relation.
      * For simplicity, this will not return results attached to draft pages.
      * Draft pages should never really have related items though.
      * Filter items that have entities set as a polymorphic relation.
      * For simplicity, this will not return results attached to draft pages.
      * Draft pages should never really have related items though.

-     *
-     * @param Builder|QueryBuilder $query

      */
      */

-    public function restrictEntityRelationQuery($query, string $tableName, string $entityIdColumn, string $entityTypeColumn)
+    public function restrictEntityRelationQuery(Builder $query, string $tableName, string $entityIdColumn, string $entityTypeColumn): Builder

     {
         $tableDetails = ['tableName' => $tableName, 'entityIdColumn' => $entityIdColumn, 'entityTypeColumn' => $entityTypeColumn];
         $pageMorphClass = (new Page())->getMorphClass();
 
     {
         $tableDetails = ['tableName' => $tableName, 'entityIdColumn' => $entityIdColumn, 'entityTypeColumn' => $entityTypeColumn];
         $pageMorphClass = (new Page())->getMorphClass();
 

-        $q = $query->whereExists(function ($permissionQuery) use (&$tableDetails) {
-            /** @var Builder $permissionQuery */
-            $permissionQuery->select(['role_id'])->from('joint_permissions')
-                ->whereColumn('joint_permissions.entity_id', '=', $tableDetails['tableName'] . '.' . $tableDetails['entityIdColumn'])
-                ->whereColumn('joint_permissions.entity_type', '=', $tableDetails['tableName'] . '.' . $tableDetails['entityTypeColumn'])
-                ->whereIn('joint_permissions.role_id', $this->getCurrentUserRoleIds())
-                ->where(function (QueryBuilder $query) {
-                    $this->addJointHasPermissionCheck($query, $this->currentUser()->id);
-                });
-        })->where(function ($query) use ($tableDetails, $pageMorphClass) {
-            /** @var Builder $query */
-            $query->where($tableDetails['entityTypeColumn'], '!=', $pageMorphClass)
+        return $this->restrictEntityQuery($query)
+            ->where(function ($query) use ($tableDetails, $pageMorphClass) {
+                /** @var Builder $query */
+                $query->where($tableDetails['entityTypeColumn'], '!=', $pageMorphClass)

                 ->orWhereExists(function (QueryBuilder $query) use ($tableDetails, $pageMorphClass) {
                     $query->select('id')->from('pages')
                         ->whereColumn('pages.id', '=', $tableDetails['tableName'] . '.' . $tableDetails['entityIdColumn'])
                         ->where($tableDetails['tableName'] . '.' . $tableDetails['entityTypeColumn'], '=', $pageMorphClass)
                         ->where('pages.draft', '=', false);
                 });
                 ->orWhereExists(function (QueryBuilder $query) use ($tableDetails, $pageMorphClass) {
                     $query->select('id')->from('pages')
                         ->whereColumn('pages.id', '=', $tableDetails['tableName'] . '.' . $tableDetails['entityIdColumn'])
                         ->where($tableDetails['tableName'] . '.' . $tableDetails['entityTypeColumn'], '=', $pageMorphClass)
                         ->where('pages.draft', '=', false);
                 });

-        });
-
-        return $q;
+            });

     }
 
     /**
     }
 
     /**


@@ -164,6 +150,7 @@ class PermissionApplicator
      */
     public function restrictPageRelationQuery(Builder $query, string $tableName, string $pageIdColumn): Builder
     {
      */
     public function restrictPageRelationQuery(Builder $query, string $tableName, string $pageIdColumn): Builder
     {

+        // TODO - Refactor

         $fullPageIdColumn = $tableName . '.' . $pageIdColumn;
         $morphClass = (new Page())->getMorphClass();
 
         $fullPageIdColumn = $tableName . '.' . $pageIdColumn;
         $morphClass = (new Page())->getMorphClass();
 

diff --git a/app/Auth/Permissions/PermissionStatus.php b/app/Auth/Permissions/PermissionStatus.php
new file mode 100644 (file)
index 0000000..f8e55c2
--- /dev/null
+++ b/app/Auth/Permissions/PermissionStatus.php
@@ -0,0 +1,11 @@
+<?php
+
+namespace BookStack\Auth\Permissions;
+
+class PermissionStatus
+{
+    const IMPLICIT_DENY = 0;
+    const IMPLICIT_ALLOW = 1;
+    const EXPLICIT_DENY = 2;
+    const EXPLICIT_ALLOW = 3;
+}

diff --git a/app/References/Reference.php b/app/References/Reference.php
index 1a1c56af365b8e202b2c88e404cd308bd3e670c0..0e5b17d0b7a72eb3ea6c5ba978c48befd01f4900 100644 (file)
--- a/app/References/Reference.php
+++ b/app/References/Reference.php
@@ -2,7 +2,9 @@
 
 namespace BookStack\References;
 
 
 namespace BookStack\References;
 

+use BookStack\Auth\Permissions\JointPermission;

 use Illuminate\Database\Eloquent\Model;
 use Illuminate\Database\Eloquent\Model;

+use Illuminate\Database\Eloquent\Relations\HasMany;

 use Illuminate\Database\Eloquent\Relations\MorphTo;
 
 /**
 use Illuminate\Database\Eloquent\Relations\MorphTo;
 
 /**


@@ -24,4 +26,10 @@ class Reference extends Model
     {
         return $this->morphTo('to');
     }
     {
         return $this->morphTo('to');
     }

+
+    public function jointPermissions(): HasMany
+    {
+        return $this->hasMany(JointPermission::class, 'entity_id', 'from_id')
+            ->whereColumn('references.from_type', '=', 'joint_permissions.entity_type');
+    }

 }
 }

diff --git a/app/References/ReferenceFetcher.php b/app/References/ReferenceFetcher.php
index a73463a950a44d4edcd28c8e0b44e06858523326..41579185798ed541d7d8970c5f7cb2023c1fdcb7 100644 (file)
--- a/app/References/ReferenceFetcher.php
+++ b/app/References/ReferenceFetcher.php
@@ -5,6 +5,7 @@ namespace BookStack\References;
 use BookStack\Auth\Permissions\PermissionApplicator;
 use BookStack\Entities\Models\Entity;
 use BookStack\Entities\Models\Page;
 use BookStack\Auth\Permissions\PermissionApplicator;
 use BookStack\Entities\Models\Entity;
 use BookStack\Entities\Models\Page;

+use Illuminate\Database\Eloquent\Builder;

 use Illuminate\Database\Eloquent\Collection;
 use Illuminate\Database\Eloquent\Relations\Relation;
 
 use Illuminate\Database\Eloquent\Collection;
 use Illuminate\Database\Eloquent\Relations\Relation;
 


@@ -23,8 +24,7 @@ class ReferenceFetcher
      */
     public function getPageReferencesToEntity(Entity $entity): Collection
     {
      */
     public function getPageReferencesToEntity(Entity $entity): Collection
     {

-        $baseQuery = $entity->referencesTo()
-            ->where('from_type', '=', (new Page())->getMorphClass())
+        $baseQuery = $this->queryPageReferencesToEntity($entity)

             ->with([
                 'from'         => fn (Relation $query) => $query->select(Page::$listAttributes),
                 'from.book'    => fn (Relation $query) => $query->scopes('visible'),
             ->with([
                 'from'         => fn (Relation $query) => $query->select(Page::$listAttributes),
                 'from.book'    => fn (Relation $query) => $query->scopes('visible'),


@@ -47,11 +47,8 @@ class ReferenceFetcher
      */
     public function getPageReferenceCountToEntity(Entity $entity): int
     {
      */
     public function getPageReferenceCountToEntity(Entity $entity): int
     {

-        $baseQuery = $entity->referencesTo()
-            ->where('from_type', '=', (new Page())->getMorphClass());
-

         $count = $this->permissions->restrictEntityRelationQuery(
         $count = $this->permissions->restrictEntityRelationQuery(

-            $baseQuery,
+            $this->queryPageReferencesToEntity($entity),

             'references',
             'from_id',
             'from_type'
             'references',
             'from_id',
             'from_type'


@@ -59,4 +56,12 @@ class ReferenceFetcher
 
         return $count;
     }
 
         return $count;
     }

+
+    protected function queryPageReferencesToEntity(Entity $entity): Builder
+    {
+        return Reference::query()
+            ->where('to_type', '=', $entity->getMorphClass())
+            ->where('to_id', '=', $entity->id)
+            ->where('from_type', '=', (new Page())->getMorphClass());
+    }

 }
 }

diff --git a/database/migrations/2023_01_24_104625_refactor_joint_permissions_storage.php b/database/migrations/2023_01_24_104625_refactor_joint_permissions_storage.php
new file mode 100644 (file)
index 0000000..49ebf5c
--- /dev/null
+++ b/database/migrations/2023_01_24_104625_refactor_joint_permissions_storage.php
@@ -0,0 +1,52 @@
+<?php
+
+use BookStack\Auth\Permissions\JointPermissionBuilder;
+use Illuminate\Database\Migrations\Migration;
+use Illuminate\Database\Schema\Blueprint;
+use Illuminate\Support\Facades\DB;
+use Illuminate\Support\Facades\Schema;
+
+class RefactorJointPermissionsStorage extends Migration
+{
+    /**
+     * Run the migrations.
+     *
+     * @return void
+     */
+    public function up()
+    {
+        // Truncate before schema changes to avoid performance issues
+        // since we'll need to rebuild anyway.
+        DB::table('joint_permissions')->truncate();
+
+        if (Schema::hasColumn('joint_permissions', 'owned_by')) {
+            Schema::table('joint_permissions', function (Blueprint $table) {
+                $table->dropColumn(['has_permission', 'has_permission_own', 'owned_by']);
+
+                $table->unsignedTinyInteger('status')->index();
+                $table->unsignedInteger('owner_id')->nullable()->index();
+            });
+        }
+
+        // Rebuild permissions
+        app(JointPermissionBuilder::class)->rebuildForAll();
+    }
+
+    /**
+     * Reverse the migrations.
+     *
+     * @return void
+     */
+    public function down()
+    {
+        DB::table('joint_permissions')->truncate();
+
+        Schema::table('joint_permissions', function (Blueprint $table) {
+            $table->dropColumn(['status', 'owner_id']);
+
+            $table->boolean('has_permission')->index();
+            $table->boolean('has_permission_own')->index();
+            $table->unsignedInteger('created_by')->index();
+        });
+    }
+}