]> BookStack Code Mirror - bookstack/commitdiff
Dropped use of non-view joint permissions
authorDan Brown <redacted>
Sat, 16 Jul 2022 20:50:42 +0000 (21:50 +0100)
committerDan Brown <redacted>
Sat, 16 Jul 2022 20:50:42 +0000 (21:50 +0100)
app/Auth/Permissions/JointPermissionBuilder.php
app/Auth/Permissions/PermissionApplicator.php
database/migrations/2022_07_16_170051_drop_joint_permission_type.php [new file with mode: 0644]
tests/PublicActionTest.php

index fe25e02ff1e3d3131a6b0529b0faf2e18c57f63a..9ee09a3a635110112669c32d0f57efe0a20b18dd 100644 (file)
@@ -13,6 +13,10 @@ use Illuminate\Database\Eloquent\Builder;
 use Illuminate\Database\Eloquent\Collection as EloquentCollection;
 use Illuminate\Support\Facades\DB;
 
+/**
+ * Joint permissions provide a pre-query "cached" table of view permissions for all core entity
+ * types for all roles in the system. This class generates out that table for different scenarios.
+ */
 class JointPermissionBuilder
 {
     /**
@@ -80,6 +84,7 @@ class JointPermissionBuilder
     {
         $roles = [$role];
         $role->jointPermissions()->delete();
+        $role->load('permissions');
 
         // Chunk through all books
         $this->bookFetchQuery()->chunk(20, function ($books) use ($roles) {
@@ -247,7 +252,7 @@ class JointPermissionBuilder
         // Create a mapping of explicit entity permissions
         $permissionMap = [];
         foreach ($permissions as $permission) {
-            $key = $permission->restrictable_type . ':' . $permission->restrictable_id . ':' . $permission->role_id . ':' . $permission->action;
+            $key = $permission->restrictable_type . ':' . $permission->restrictable_id . ':' . $permission->role_id;
             $isRestricted = $entityRestrictedMap[$permission->restrictable_type . ':' . $permission->restrictable_id];
             $permissionMap[$key] = $isRestricted;
         }
@@ -263,16 +268,13 @@ class JointPermissionBuilder
         // Create Joint Permission Data
         foreach ($entities as $entity) {
             foreach ($roles as $role) {
-                foreach ($this->getActions($entity) as $action) {
-                    $jointPermissions[] = $this->createJointPermissionData(
-                        $entity,
-                        $role->getRawAttribute('id'),
-                        $action,
-                        $permissionMap,
-                        $rolePermissionMap,
-                        $role->system_name === 'admin'
-                    );
-                }
+                $jointPermissions[] = $this->createJointPermissionData(
+                    $entity,
+                    $role->getRawAttribute('id'),
+                    $permissionMap,
+                    $rolePermissionMap,
+                    $role->system_name === 'admin'
+                );
             }
         }
 
@@ -312,64 +314,46 @@ class JointPermissionBuilder
     protected function getEntityPermissionsForEntities(array $entities): array
     {
         $idsByType = $this->entitiesToTypeIdMap($entities);
-        $permissionFetch = EntityPermission::query();
-
-        foreach ($idsByType as $type => $ids) {
-            $permissionFetch->orWhere(function (Builder $query) use ($type, $ids) {
-                $query->where('restrictable_type', '=', $type)->whereIn('restrictable_id', $ids);
+        $permissionFetch = EntityPermission::query()
+            ->where('action', '=', 'view')
+            ->where(function(Builder $query) use ($idsByType) {
+                foreach ($idsByType as $type => $ids) {
+                    $query->orWhere(function (Builder $query) use ($type, $ids) {
+                        $query->where('restrictable_type', '=', $type)->whereIn('restrictable_id', $ids);
+                    });
+                }
             });
-        }
 
         return $permissionFetch->get()->all();
     }
 
-    /**
-     * Get the actions related to an entity.
-     */
-    protected function getActions(SimpleEntityData $entity): array
-    {
-        $baseActions = ['view', 'update', 'delete'];
-
-        if ($entity->type === 'chapter' || $entity->type === 'book') {
-            $baseActions[] = 'page-create';
-        }
-
-        if ($entity->type === 'book') {
-            $baseActions[] = 'chapter-create';
-        }
-
-        return $baseActions;
-    }
-
     /**
      * Create entity permission data for an entity and role
      * for a particular action.
      */
-    protected function createJointPermissionData(SimpleEntityData $entity, int $roleId, string $action, array $permissionMap, array $rolePermissionMap, bool $isAdminRole): array
+    protected function createJointPermissionData(SimpleEntityData $entity, int $roleId, array $permissionMap, array $rolePermissionMap, bool $isAdminRole): array
     {
-        $permissionPrefix = (strpos($action, '-') === false ? ($entity->type . '-') : '') . $action;
+        $permissionPrefix = $entity->type . '-view';
         $roleHasPermission = isset($rolePermissionMap[$roleId . ':' . $permissionPrefix . '-all']);
         $roleHasPermissionOwn = isset($rolePermissionMap[$roleId . ':' . $permissionPrefix . '-own']);
-        $explodedAction = explode('-', $action);
-        $restrictionAction = end($explodedAction);
 
         if ($isAdminRole) {
-            return $this->createJointPermissionDataArray($entity, $roleId, $action, true, true);
+            return $this->createJointPermissionDataArray($entity, $roleId, true, true);
         }
 
         if ($entity->restricted) {
-            $hasAccess = $this->mapHasActiveRestriction($permissionMap, $entity, $roleId, $restrictionAction);
+            $hasAccess = $this->mapHasActiveRestriction($permissionMap, $entity, $roleId);
 
-            return $this->createJointPermissionDataArray($entity, $roleId, $action, $hasAccess, $hasAccess);
+            return $this->createJointPermissionDataArray($entity, $roleId, $hasAccess, $hasAccess);
         }
 
         if ($entity->type === 'book' || $entity->type === 'bookshelf') {
-            return $this->createJointPermissionDataArray($entity, $roleId, $action, $roleHasPermission, $roleHasPermissionOwn);
+            return $this->createJointPermissionDataArray($entity, $roleId, $roleHasPermission, $roleHasPermissionOwn);
         }
 
         // For chapters and pages, Check if explicit permissions are set on the Book.
         $book = $this->getBook($entity->book_id);
-        $hasExplicitAccessToParents = $this->mapHasActiveRestriction($permissionMap, $book, $roleId, $restrictionAction);
+        $hasExplicitAccessToParents = $this->mapHasActiveRestriction($permissionMap, $book, $roleId);
         $hasPermissiveAccessToParents = !$book->restricted;
 
         // For pages with a chapter, Check if explicit permissions are set on the Chapter
@@ -377,14 +361,13 @@ class JointPermissionBuilder
             $chapter = $this->getChapter($entity->chapter_id);
             $hasPermissiveAccessToParents = $hasPermissiveAccessToParents && !$chapter->restricted;
             if ($chapter->restricted) {
-                $hasExplicitAccessToParents = $this->mapHasActiveRestriction($permissionMap, $chapter, $roleId, $restrictionAction);
+                $hasExplicitAccessToParents = $this->mapHasActiveRestriction($permissionMap, $chapter, $roleId);
             }
         }
 
         return $this->createJointPermissionDataArray(
             $entity,
             $roleId,
-            $action,
             ($hasExplicitAccessToParents || ($roleHasPermission && $hasPermissiveAccessToParents)),
             ($hasExplicitAccessToParents || ($roleHasPermissionOwn && $hasPermissiveAccessToParents))
         );
@@ -393,9 +376,9 @@ class JointPermissionBuilder
     /**
      * Check for an active restriction in an entity map.
      */
-    protected function mapHasActiveRestriction(array $entityMap, SimpleEntityData $entity, int $roleId, string $action): bool
+    protected function mapHasActiveRestriction(array $entityMap, SimpleEntityData $entity, int $roleId): bool
     {
-        $key = $entity->type . ':' . $entity->id . ':' . $roleId . ':' . $action;
+        $key = $entity->type . ':' . $entity->id . ':' . $roleId;
 
         return $entityMap[$key] ?? false;
     }
@@ -404,10 +387,9 @@ class JointPermissionBuilder
      * 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, string $action, bool $permissionAll, bool $permissionOwn): array
+    protected function createJointPermissionDataArray(SimpleEntityData $entity, int $roleId, bool $permissionAll, bool $permissionOwn): array
     {
         return [
-            'action'             => $action,
             'entity_id'          => $entity->id,
             'entity_type'        => $entity->type,
             'has_permission'     => $permissionAll,
index 4b648532a5bb43f9a81a4469d315d8ed9db7a49a..b89857c5c3c773560ad5d82e6fe3d0802daf32e4 100644 (file)
@@ -113,8 +113,6 @@ class PermissionApplicator
         return $query->where(function (Builder $parentQuery) {
             $parentQuery->whereHas('jointPermissions', function (Builder $permissionQuery) {
                 $permissionQuery->whereIn('role_id', $this->getCurrentUserRoleIds())
-                    // TODO - Delete line once only views
-                    ->where('action', '=', 'view')
                     ->where(function (Builder $query) {
                         $this->addJointHasPermissionCheck($query, $this->currentUser()->id);
                     });
@@ -154,7 +152,6 @@ class PermissionApplicator
             $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'])
-                ->where('joint_permissions.action', '=', 'view')
                 ->whereIn('joint_permissions.role_id', $this->getCurrentUserRoleIds())
                 ->where(function (QueryBuilder $query) {
                     $this->addJointHasPermissionCheck($query, $this->currentUser()->id);
@@ -189,7 +186,6 @@ class PermissionApplicator
             $permissionQuery->select('joint_permissions.role_id')->from('joint_permissions')
                 ->whereColumn('joint_permissions.entity_id', '=', $fullPageIdColumn)
                 ->where('joint_permissions.entity_type', '=', $morphClass)
-                ->where('joint_permissions.action', '=', 'view')
                 ->whereIn('joint_permissions.role_id', $this->getCurrentUserRoleIds())
                 ->where(function (QueryBuilder $query) {
                     $this->addJointHasPermissionCheck($query, $this->currentUser()->id);
diff --git a/database/migrations/2022_07_16_170051_drop_joint_permission_type.php b/database/migrations/2022_07_16_170051_drop_joint_permission_type.php
new file mode 100644 (file)
index 0000000..f34f736
--- /dev/null
@@ -0,0 +1,41 @@
+<?php
+
+use Illuminate\Database\Migrations\Migration;
+use Illuminate\Database\Schema\Blueprint;
+use Illuminate\Support\Facades\DB;
+use Illuminate\Support\Facades\Schema;
+
+class DropJointPermissionType extends Migration
+{
+    /**
+     * Run the migrations.
+     *
+     * @return void
+     */
+    public function up()
+    {
+        DB::table('joint_permissions')
+            ->where('action', '!=', 'view')
+            ->delete();
+
+        Schema::table('joint_permissions', function (Blueprint $table) {
+            $table->dropPrimary(['role_id', 'entity_type', 'entity_id', 'action']);
+            $table->dropColumn('action');
+            $table->primary(['role_id', 'entity_type', 'entity_id'], 'joint_primary');
+        });
+    }
+
+    /**
+     * Reverse the migrations.
+     *
+     * @return void
+     */
+    public function down()
+    {
+        Schema::table('joint_permissions', function (Blueprint $table) {
+            $table->string('action');
+            $table->dropPrimary(['role_id', 'entity_type', 'entity_id']);
+            $table->primary(['role_id', 'entity_type', 'entity_id', 'action']);
+        });
+    }
+}
index 2841f4ef44d581592c259e6d71957ac494603d80..ced40d0a8e96e133617370ec00a4c00df04d015e 100644 (file)
@@ -90,6 +90,7 @@ class PublicActionTest extends TestCase
             $publicRole->attachPermission($perm);
         }
         $this->app->make(JointPermissionBuilder::class)->rebuildForRole($publicRole);
+        user()->clearPermissionCache();
 
         /** @var Chapter $chapter */
         $chapter = Chapter::query()->first();
Morty Proxy This is a proxified and sanitized view of the page, visit original site.