From e2a72d16aa496cef250986d9f7cc02dd9564e647 Mon Sep 17 00:00:00 2001
From: Dan Brown <ssddanbrown@googlemail.com>
Date: Sat, 21 Jan 2023 13:03:47 +0000
Subject: [PATCH] Made adjustments to fit copied work into dev branch
Ported non-compatible elements, Now all tests passing apart from some
specific permission scenario tests which are probably correctly failing.
Updates some tests to better avoid messing environment state.
---
dev/docs/permission-scenario-testing.md | 183 +--------------
.../RegeneratePermissionsCommandTest.php | 20 +-
tests/Entity/BookShelfTest.php | 1 +
tests/Helpers/PermissionsProvider.php | 12 +-
tests/Permissions/EntityPermissionsTest.php | 14 +-
.../Scenarios/EntityUserPermissionsTest.php | 209 ------------------
tests/TestCase.php | 3 +
tests/Unit/FrameworkAssumptionTest.php | 2 +-
8 files changed, 31 insertions(+), 413 deletions(-)
delete mode 100644 tests/Permissions/Scenarios/EntityUserPermissionsTest.php
diff --git a/dev/docs/permission-scenario-testing.md b/dev/docs/permission-scenario-testing.md
index 6d0935f09..e738fe972 100644
--- a/dev/docs/permission-scenario-testing.md
+++ b/dev/docs/permission-scenario-testing.md
@@ -6,19 +6,16 @@ Test cases are written ability abstract, since all abilities should act the same
Tests are categorised by the most specific element involved in the scenario, where the below list is most specific to least:
-- User entity permissions.
- Role entity permissions.
- Fallback entity permissions.
- Role permissions.
-- TODO - Test fallback in the context of the above.
-
## General Permission Logical Rules
The below are some general rules we follow to standardise the behaviour of permissions in the platform:
- Most specific permission application (as above) take priority and can deny less specific permissions.
-- Parent user/role entity permissions that may be inherited, are considered to essentially be applied on the item they are inherited to unless a lower level has its own permission rule for an already specific role/user.
+- Parent role entity permissions that may be inherited, are considered to essentially be applied on the item they are inherited to unless a lower level has its own permission rule for an already specific role.
- Where both grant and deny exist at the same specificity, we side towards grant.
## Cases
@@ -241,181 +238,3 @@ User denied page permission.
- User has Role A & B.
User denied page permission.
-
----
-
-### Entity User Permissions
-
-These are tests related to entity-level user-specific permission overrides.
-
-#### test_01_explicit_allow
-
-- Page permissions have inherit disabled.
-- User has entity allow page permission.
-
-User granted page permission.
-
-#### test_02_explicit_deny
-
-- Page permissions have inherit disabled.
-- User has entity deny page permission.
-
-User denied page permission.
-
-#### test_10_allow_inherit
-
-- Page permissions have inherit enabled.
-- Chapter permissions have inherit disabled.
-- User has entity allow chapter permission.
-
-User granted page permission.
-
-#### test_11_deny_inherit
-
-- Page permissions have inherit enabled.
-- Chapter permissions have inherit disabled.
-- User has entity deny chapter permission.
-
-User denied page permission.
-
-#### test_12_allow_inherit_override
-
-- Page permissions have inherit enabled.
-- Chapter permissions have inherit disabled.
-- User has entity deny chapter permission.
-- User has entity allow page permission.
-
-User granted page permission.
-
-#### test_13_deny_inherit_override
-
-- Page permissions have inherit enabled.
-- Chapter permissions have inherit disabled.
-- User has entity allow chapter permission.
-- User has entity deny page permission.
-
-User denied page permission.
-
-#### test_40_entity_role_override_allow
-
-- Page permissions have inherit disabled.
-- User has entity allow page permission.
-- Role A has entity deny page permission.
-- User has role A.
-
-User granted page permission.
-
-#### test_41_entity_role_override_deny
-
-- Page permissions have inherit disabled.
-- User has entity deny page permission.
-- Role A has entity allow page permission.
-- User has role A.
-
-User denied page permission.
-
-#### test_42_entity_role_override_allow_via_inherit
-
-- Page permissions have inherit enabled.
-- Chapter permissions have inherit disabled.
-- User has entity allow chapter permission.
-- Role A has entity deny page permission.
-- User has role A.
-
-User granted page permission.
-
-#### test_43_entity_role_override_deny_via_inherit
-
-- Page permissions have inherit enabled.
-- Chapter permissions have inherit disabled.
-- User has entity deny chapter permission.
-- Role A has entity allow page permission.
-- User has role A.
-
-User denied page permission.
-
-#### test_50_role_override_allow
-
-- Page permissions have inherit enabled.
-- Role A has no page role permission.
-- User has entity allow page permission.
-- User has Role A.
-
-User granted page permission.
-
-#### test_51_role_override_deny
-
-- Page permissions have inherit enabled.
-- Role A has all-page role permission.
-- User has entity deny page permission.
-- User has Role A.
-
-User denied page permission.
-
-#### test_60_inherited_role_override_allow
-
-- Page permissions have inherit enabled.
-- Role A has no page role permission.
-- User has entity allow chapter permission.
-- User has Role A.
-
-User granted page permission.
-
-#### test_61_inherited_role_override_deny
-
-- Page permissions have inherit enabled.
-- Role A has view-all page role permission.
-- User has entity deny chapter permission.
-- User has Role A.
-
-User denied page permission.
-
-#### test_61_inherited_role_override_deny_on_own
-
-- Page permissions have inherit enabled.
-- Role A has view-own page role permission.
-- User has entity deny chapter permission.
-- User has Role A.
-- User owns Page.
-
-User denied page permission.
-
-#### test_70_all_override_allow
-
-- Page permissions have inherit enabled.
-- Role A has no page role permission.
-- Role A has entity deny page permission.
-- User has entity allow page permission.
-- User has Role A.
-
-User granted page permission.
-
-#### test_71_all_override_deny
-
-- Page permissions have inherit enabled.
-- Role A has page-all role permission.
-- Role A has entity allow page permission.
-- User has entity deny page permission.
-- User has Role A.
-
-User denied page permission.
-
-#### test_80_inherited_all_override_allow
-
-- Page permissions have inherit enabled.
-- Role A has no page role permission.
-- Role A has entity deny chapter permission.
-- User has entity allow chapter permission.
-- User has Role A.
-
-User granted page permission.
-
-#### test_81_inherited_all_override_deny
-
-- Page permissions have inherit enabled.
-- Role A has view-all page role permission.
-- Role A has entity allow chapter permission.
-- User has entity deny chapter permission.
-- User has Role A.
-
-User denied page permission.
\ No newline at end of file
diff --git a/tests/Commands/RegeneratePermissionsCommandTest.php b/tests/Commands/RegeneratePermissionsCommandTest.php
index cc53b460d..b916a8060 100644
--- a/tests/Commands/RegeneratePermissionsCommandTest.php
+++ b/tests/Commands/RegeneratePermissionsCommandTest.php
@@ -3,6 +3,8 @@
namespace Tests\Commands;
use BookStack\Auth\Permissions\CollapsedPermission;
+use BookStack\Auth\Permissions\EntityPermission;
+use BookStack\Auth\Permissions\JointPermission;
use Illuminate\Support\Facades\Artisan;
use Illuminate\Support\Facades\DB;
use Tests\TestCase;
@@ -14,21 +16,25 @@ class RegeneratePermissionsCommandTest extends TestCase
DB::rollBack();
$page = $this->entities->page();
$editor = $this->users->editor();
- $this->permissions->addEntityPermission($page, ['view'], null, $editor);
- CollapsedPermission::query()->truncate();
+ $role = $editor->roles()->first();
+ $this->permissions->addEntityPermission($page, ['view'], $role);
+ JointPermission::query()->truncate();
- $this->assertDatabaseMissing('entity_permissions_collapsed', ['entity_id' => $page->id]);
+ $this->assertDatabaseMissing('joint_permissions', ['entity_id' => $page->id]);
$exitCode = Artisan::call('bookstack:regenerate-permissions');
$this->assertTrue($exitCode === 0, 'Command executed successfully');
- $this->assertDatabaseHas('entity_permissions_collapsed', [
+ $this->assertDatabaseHas('joint_permissions', [
'entity_id' => $page->id,
- 'user_id' => $editor->id,
- 'view' => 1,
+ 'entity_type' => 'page',
+ 'role_id' => $role->id,
+ 'has_permission' => 1,
]);
- CollapsedPermission::query()->truncate();
+ $page->permissions()->delete();
+ $page->rebuildPermissions();
+
DB::beginTransaction();
}
}
diff --git a/tests/Entity/BookShelfTest.php b/tests/Entity/BookShelfTest.php
index 5c6489281..d953f3692 100644
--- a/tests/Entity/BookShelfTest.php
+++ b/tests/Entity/BookShelfTest.php
@@ -21,6 +21,7 @@ class BookShelfTest extends TestCase
$this->withHtml($resp)->assertElementContains('header', 'Shelves');
$viewer->roles()->delete();
+ $this->permissions->grantUserRolePermissions($viewer, []);
$resp = $this->actingAs($viewer)->get('/');
$this->withHtml($resp)->assertElementNotContains('header', 'Shelves');
diff --git a/tests/Helpers/PermissionsProvider.php b/tests/Helpers/PermissionsProvider.php
index ac9a2a68a..2cbfb1af5 100644
--- a/tests/Helpers/PermissionsProvider.php
+++ b/tests/Helpers/PermissionsProvider.php
@@ -85,7 +85,7 @@ class PermissionsProvider
if (!$inherit) {
// Set default permissions to not allow actions so that only the provided role permissions are at play.
- $permissions[] = ['role_id' => null, 'user_id' => null, 'view' => false, 'create' => false, 'update' => false, 'delete' => false];
+ $permissions[] = ['role_id' => 0, 'view' => false, 'create' => false, 'update' => false, 'delete' => false];
}
foreach ($roles as $role) {
@@ -95,9 +95,9 @@ class PermissionsProvider
$this->addEntityPermissionEntries($entity, $permissions);
}
- public function addEntityPermission(Entity $entity, array $actionList, ?Role $role = null, ?User $user = null)
+ public function addEntityPermission(Entity $entity, array $actionList, Role $role)
{
- $permissionData = $this->actionListToEntityPermissionData($actionList, $role->id ?? null, $user->id ?? null);
+ $permissionData = $this->actionListToEntityPermissionData($actionList, $role->id);
$this->addEntityPermissionEntries($entity, [$permissionData]);
}
@@ -107,7 +107,7 @@ class PermissionsProvider
*/
public function disableEntityInheritedPermissions(Entity $entity): void
{
- $entity->permissions()->whereNull(['user_id', 'role_id'])->delete();
+ $entity->permissions()->where('role_id', '=', 0)->delete();
$fallback = $this->actionListToEntityPermissionData([]);
$this->addEntityPermissionEntries($entity, [$fallback]);
}
@@ -124,9 +124,9 @@ class PermissionsProvider
* the format to entity permission data, where permission is granted if the action is in the
* given actionList array.
*/
- protected function actionListToEntityPermissionData(array $actionList, int $roleId = null, int $userId = null): array
+ protected function actionListToEntityPermissionData(array $actionList, int $roleId = 0): array
{
- $permissionData = ['role_id' => $roleId, 'user_id' => $userId];
+ $permissionData = ['role_id' => $roleId];
foreach (EntityPermission::PERMISSIONS as $possibleAction) {
$permissionData[$possibleAction] = in_array($possibleAction, $actionList);
}
diff --git a/tests/Permissions/EntityPermissionsTest.php b/tests/Permissions/EntityPermissionsTest.php
index 68a4ed244..ab8b1242d 100644
--- a/tests/Permissions/EntityPermissionsTest.php
+++ b/tests/Permissions/EntityPermissionsTest.php
@@ -379,19 +379,17 @@ class EntityPermissionsTest extends TestCase
$this->put($modelInstance->getUrl('/permissions'), [
'permissions' => [
- 'role' => [
- $roleId => [
- $permission => 'true',
- ],
+ $roleId => [
+ $permission => 'true',
],
],
]);
$this->assertDatabaseHas('entity_permissions', [
- 'entity_id' => $modelInstance->id,
- 'entity_type' => $modelInstance->getMorphClass(),
- 'role_id' => $roleId,
- $permission => true,
+ 'entity_id' => $modelInstance->id,
+ 'entity_type' => $modelInstance->getMorphClass(),
+ 'role_id' => $roleId,
+ $permission => true,
]);
}
diff --git a/tests/Permissions/Scenarios/EntityUserPermissionsTest.php b/tests/Permissions/Scenarios/EntityUserPermissionsTest.php
deleted file mode 100644
index 4fa805805..000000000
--- a/tests/Permissions/Scenarios/EntityUserPermissionsTest.php
+++ /dev/null
@@ -1,209 +0,0 @@
-<?php
-
-namespace Tests\Permissions\Scenarios;
-
-class EntityUserPermissionsTest extends PermissionScenarioTestCase
-{
- public function test_01_explicit_allow()
- {
- $user = $this->users->newUser();
- $page = $this->entities->page();
- $this->permissions->disableEntityInheritedPermissions($page);
- $this->permissions->addEntityPermission($page, ['view'], null, $user);
-
- $this->assertVisibleToUser($page, $user);
- }
-
- public function test_02_explicit_deny()
- {
- $user = $this->users->newUser();
- $page = $this->entities->page();
- $this->permissions->disableEntityInheritedPermissions($page);
- $this->permissions->addEntityPermission($page, [], null, $user);
-
- $this->assertNotVisibleToUser($page, $user);
- }
-
- public function test_10_allow_inherit()
- {
- $user = $this->users->newUser();
- $page = $this->entities->pageWithinChapter();
- $chapter = $page->chapter;
- $this->permissions->disableEntityInheritedPermissions($chapter);
- $this->permissions->addEntityPermission($chapter, ['view'], null, $user);
-
- $this->assertVisibleToUser($page, $user);
- }
-
- public function test_11_deny_inherit()
- {
- $user = $this->users->newUser();
- $page = $this->entities->pageWithinChapter();
- $chapter = $page->chapter;
- $this->permissions->disableEntityInheritedPermissions($chapter);
- $this->permissions->addEntityPermission($chapter, [], null, $user);
-
- $this->assertNotVisibleToUser($page, $user);
- }
-
- public function test_12_allow_inherit_override()
- {
- $user = $this->users->newUser();
- $page = $this->entities->pageWithinChapter();
- $chapter = $page->chapter;
- $this->permissions->disableEntityInheritedPermissions($chapter);
- $this->permissions->addEntityPermission($chapter, [], null, $user);
- $this->permissions->addEntityPermission($page, ['view'], null, $user);
-
- $this->assertVisibleToUser($page, $user);
- }
-
- public function test_13_deny_inherit_override()
- {
- $user = $this->users->newUser();
- $page = $this->entities->pageWithinChapter();
- $chapter = $page->chapter;
- $this->permissions->disableEntityInheritedPermissions($chapter);
- $this->permissions->addEntityPermission($chapter, ['view'], null, $user);
- $this->permissions->addEntityPermission($page, ['deny'], null, $user);
-
- $this->assertNotVisibleToUser($page, $user);
- }
-
- public function test_40_entity_role_override_allow()
- {
- [$user, $role] = $this->users->newUserWithRole();
- $page = $this->entities->page();
- $this->permissions->disableEntityInheritedPermissions($page);
- $this->permissions->addEntityPermission($page, ['view'], null, $user);
- $this->permissions->addEntityPermission($page, [], $role);
-
- $this->assertVisibleToUser($page, $user);
- }
-
- public function test_41_entity_role_override_deny()
- {
- [$user, $role] = $this->users->newUserWithRole();
- $page = $this->entities->page();
- $this->permissions->disableEntityInheritedPermissions($page);
- $this->permissions->addEntityPermission($page, [], null, $user);
- $this->permissions->addEntityPermission($page, ['view'], $role);
-
- $this->assertNotVisibleToUser($page, $user);
- }
-
- public function test_42_entity_role_override_allow_via_inherit()
- {
- [$user, $role] = $this->users->newUserWithRole();
- $page = $this->entities->pageWithinChapter();
- $chapter = $page->chapter;
- $this->permissions->disableEntityInheritedPermissions($chapter);
- $this->permissions->addEntityPermission($chapter, ['view'], null, $user);
- $this->permissions->addEntityPermission($page, [], $role);
-
- $this->assertVisibleToUser($page, $user);
- }
-
- public function test_43_entity_role_override_deny_via_inherit()
- {
- [$user, $role] = $this->users->newUserWithRole();
- $page = $this->entities->pageWithinChapter();
- $chapter = $page->chapter;
- $this->permissions->disableEntityInheritedPermissions($chapter);
- $this->permissions->addEntityPermission($chapter, [], null, $user);
- $this->permissions->addEntityPermission($page, ['view'], $role);
-
- $this->assertNotVisibleToUser($page, $user);
- }
-
- public function test_50_role_override_allow()
- {
- [$user, $roleA] = $this->users->newUserWithRole();
- $page = $this->entities->page();
- $this->permissions->addEntityPermission($page, ['view'], null, $user);
-
- $this->assertVisibleToUser($page, $user);
- }
-
- public function test_51_role_override_deny()
- {
- [$user, $roleA] = $this->users->newUserWithRole([], ['page-view-all']);
- $page = $this->entities->page();
- $this->permissions->addEntityPermission($page, [], null, $user);
-
- $this->assertNotVisibleToUser($page, $user);
- }
-
- public function test_60_inherited_role_override_allow()
- {
- [$user, $roleA] = $this->users->newUserWithRole([], []);
- $page = $this->entities->pageWithinChapter();
- $chapter = $page->chapter;
- $this->permissions->addEntityPermission($chapter, ['view'], null, $user);
-
- $this->assertVisibleToUser($page, $user);
- }
-
- public function test_61_inherited_role_override_deny()
- {
- [$user, $roleA] = $this->users->newUserWithRole([], ['page-view-all']);
- $page = $this->entities->pageWithinChapter();
- $chapter = $page->chapter;
- $this->permissions->addEntityPermission($chapter, [], null, $user);
-
- $this->assertNotVisibleToUser($page, $user);
- }
-
- public function test_61_inherited_role_override_deny_on_own()
- {
- [$user, $roleA] = $this->users->newUserWithRole([], ['page-view-own']);
- $page = $this->entities->pageWithinChapter();
- $chapter = $page->chapter;
- $this->permissions->addEntityPermission($chapter, [], null, $user);
- $this->permissions->changeEntityOwner($page, $user);
-
- $this->assertNotVisibleToUser($page, $user);
- }
-
- public function test_70_all_override_allow()
- {
- [$user, $roleA] = $this->users->newUserWithRole([], []);
- $page = $this->entities->page();
- $this->permissions->addEntityPermission($page, [], $roleA, null);
- $this->permissions->addEntityPermission($page, ['view'], null, $user);
-
- $this->assertVisibleToUser($page, $user);
- }
-
- public function test_71_all_override_deny()
- {
- [$user, $roleA] = $this->users->newUserWithRole([], ['page-view-all']);
- $page = $this->entities->page();
- $this->permissions->addEntityPermission($page, ['view'], $roleA, null);
- $this->permissions->addEntityPermission($page, [], null, $user);
-
- $this->assertNotVisibleToUser($page, $user);
- }
-
- public function test_80_inherited_all_override_allow()
- {
- [$user, $roleA] = $this->users->newUserWithRole([], []);
- $page = $this->entities->pageWithinChapter();
- $chapter = $page->chapter;
- $this->permissions->addEntityPermission($chapter, [], $roleA, null);
- $this->permissions->addEntityPermission($chapter, ['view'], null, $user);
-
- $this->assertVisibleToUser($page, $user);
- }
-
- public function test_81_inherited_all_override_deny()
- {
- [$user, $roleA] = $this->users->newUserWithRole([], ['page-view-all']);
- $page = $this->entities->pageWithinChapter();
- $chapter = $page->chapter;
- $this->permissions->addEntityPermission($chapter, ['view'], $roleA, null);
- $this->permissions->addEntityPermission($chapter, [], null, $user);
-
- $this->assertNotVisibleToUser($page, $user);
- }
-}
diff --git a/tests/TestCase.php b/tests/TestCase.php
index 70fd0da1d..a5d75655c 100644
--- a/tests/TestCase.php
+++ b/tests/TestCase.php
@@ -153,9 +153,12 @@ abstract class TestCase extends BaseTestCase
DB::purge();
config()->set('database.connections.mysql_testing.database', $database);
+ DB::beginTransaction();
$callback();
+ DB::rollBack();
+
if (is_null($originalVal)) {
unset($_SERVER[$name]);
} else {
diff --git a/tests/Unit/FrameworkAssumptionTest.php b/tests/Unit/FrameworkAssumptionTest.php
index d4feff60c..54d315de9 100644
--- a/tests/Unit/FrameworkAssumptionTest.php
+++ b/tests/Unit/FrameworkAssumptionTest.php
@@ -25,7 +25,7 @@ class FrameworkAssumptionTest extends TestCase
// Page has SoftDeletes trait by default, so we apply our custom scope and ensure
// it stacks on the global scope to filter out deleted items.
$query = Page::query()->scopes('visible')->toSql();
- $this->assertStringContainsString('entity_permissions_collapsed', $query);
+ $this->assertStringContainsString('joint_permissions', $query);
$this->assertStringContainsString('`deleted_at` is null', $query);
}
}