Addressed fallback override cases found during testing

Had misalignment between query and usercan, The nuance between fallback
and entity-role permissions was not taken into account by the query
system. Now added with new test cases to cover.

diff --git a/app/Auth/Permissions/EntityPermissionEvaluator.php b/app/Auth/Permissions/EntityPermissionEvaluator.php
index 99e87d7694b77396ba77cf022ed72542f0e07606..f5e75be3ea754ac679d813ae471d519b431a05cc 100644 (file)
--- a/app/Auth/Permissions/EntityPermissionEvaluator.php
+++ b/app/Auth/Permissions/EntityPermissionEvaluator.php
@@ -25,22 +25,24 @@ class EntityPermissionEvaluator
         $relevantPermissions = $this->getPermissionsMapByTypeId($typeIdChain, [...$userRoleIds, 0]);
         $permitsByType = $this->collapseAndCategorisePermissions($typeIdChain, $relevantPermissions);
 
-        return $this->evaluatePermitsByType($permitsByType);
+        $status = $this->evaluatePermitsByType($permitsByType);
+
+        return is_null($status) ? null : $status === PermissionStatus::IMPLICIT_ALLOW || $status === PermissionStatus::EXPLICIT_ALLOW;
     }
 
     /**
      * @param array<string, array<string, int>> $permitsByType
      */
-    protected function evaluatePermitsByType(array $permitsByType): ?bool
+    protected function evaluatePermitsByType(array $permitsByType): ?int
     {
         // Return grant or reject from role-level if exists
         if (count($permitsByType['role']) > 0) {
-            return boolval(max($permitsByType['role']));
+            return max($permitsByType['role']) ? PermissionStatus::EXPLICIT_ALLOW : PermissionStatus::EXPLICIT_DENY;
         }
 
         // Return fallback permission if exists
         if (count($permitsByType['fallback']) > 0) {
-            return boolval($permitsByType['fallback'][0]);
+            return $permitsByType['fallback'][0] ? PermissionStatus::IMPLICIT_ALLOW : PermissionStatus::IMPLICIT_DENY;
         }
 
         return null;

diff --git a/app/Auth/Permissions/JointPermissionBuilder.php b/app/Auth/Permissions/JointPermissionBuilder.php
index bbdf4d6f8a196de6ae27703bc67b761dd4340cc9..4132a19af807e57c94814c03eb8e09d93a8311f8 100644 (file)
--- a/app/Auth/Permissions/JointPermissionBuilder.php
+++ b/app/Auth/Permissions/JointPermissionBuilder.php
@@ -262,8 +262,7 @@ class JointPermissionBuilder
         // Return evaluated entity permission status if it has an affect.
         $entityPermissionStatus = $permissionMap->evaluateEntityForRole($entity, $roleId);
         if ($entityPermissionStatus !== null) {
-            $status = $entityPermissionStatus ? PermissionStatus::EXPLICIT_ALLOW : PermissionStatus::EXPLICIT_DENY;
-            return $this->createJointPermissionDataArray($entity, $roleId, $status, $entityPermissionStatus);
+            return $this->createJointPermissionDataArray($entity, $roleId, $entityPermissionStatus, false);
         }
 
         // Otherwise default to the role-level permissions

diff --git a/app/Auth/Permissions/MassEntityPermissionEvaluator.php b/app/Auth/Permissions/MassEntityPermissionEvaluator.php
index 1bd2ec44a057f43cb3c05cf5a73ebe37ad20667c..a9deba16db342ca42e493f172d10cfe5f337928e 100644 (file)
--- a/app/Auth/Permissions/MassEntityPermissionEvaluator.php
+++ b/app/Auth/Permissions/MassEntityPermissionEvaluator.php
@@ -16,7 +16,7 @@ class MassEntityPermissionEvaluator extends EntityPermissionEvaluator
         parent::__construct($action);
     }
 
-    public function evaluateEntityForRole(SimpleEntityData $entity, int $roleId): ?bool
+    public function evaluateEntityForRole(SimpleEntityData $entity, int $roleId): ?int
     {
         $typeIdChain = $this->gatherEntityChainTypeIds($entity);
         $relevantPermissions = $this->getPermissionMapByTypeIdForChainAndRole($typeIdChain, $roleId);

diff --git a/dev/docs/permission-scenario-testing.md b/dev/docs/permission-scenario-testing.md
index e738fe972596b08ace63707b874b56772546addf..54b1bcfe107311aa3bd263e64475a5bfe017c546 100644 (file)
--- a/dev/docs/permission-scenario-testing.md
+++ b/dev/docs/permission-scenario-testing.md
@@ -229,7 +229,7 @@ User denied page permission.
 
 User denied page permission.
 
-#### test_80_multi_role_inherited_deny_via_parent
+#### test_75_multi_role_inherited_deny_via_parent
 
 - Page permissions have inherit enabled.
 - Chapter permissions have inherit enabled.
@@ -238,3 +238,83 @@ User denied page permission.
 - User has Role A & B.
 
 User denied page permission.
+
+#### test_80_fallback_override_allow
+
+- Page permissions have inherit disabled.
+- Page fallback has entity deny permission.
+- Role A has entity allow page permission.
+- User has Role A.
+
+User granted page permission.
+
+#### test_81_fallback_override_deny
+
+- Page permissions have inherit disabled.
+- Page fallback has entity allow permission.
+- Role A has entity deny page permission.
+- User has Role A.
+
+User denied page permission.
+
+#### test_84_fallback_override_allow_multi_role
+
+- Page permissions have inherit disabled.
+- Page fallback has entity deny permission.
+- Role A has entity allow page permission.
+- Role B has no entity page permissions.
+- User has Role A & B.
+
+User granted page permission.
+
+#### test_85_fallback_override_deny_multi_role
+
+- Page permissions have inherit disabled.
+- Page fallback has entity allow permission.
+- Role A has entity deny page permission.
+- Role B has no entity page permissions.
+- User has Role A & B.
+
+User denied page permission.
+
+#### test_86_fallback_override_allow_inherit
+
+- Chapter permissions have inherit disabled.
+- Page permissions have inherit enabled.
+- Chapter fallback has entity deny permission.
+- Role A has entity allow chapter permission.
+- User has Role A.
+
+User granted page permission.
+
+#### test_87_fallback_override_deny_inherit
+
+- Chapter permissions have inherit disabled.
+- Page permissions have inherit enabled.
+- Chapter fallback has entity allow permission.
+- Role A has entity deny chapter permission.
+- User has Role A.
+
+User denied page permission.
+
+#### test_88_fallback_override_allow_multi_role_inherit
+
+- Chapter permissions have inherit disabled.
+- Page permissions have inherit enabled.
+- Chapter fallback has entity deny permission.
+- Role A has entity allow chapter permission.
+- Role B has no entity chapter permissions.
+- User has Role A & B.
+
+User granted page permission.
+
+#### test_89_fallback_override_deny_multi_role_inherit
+
+- Chapter permissions have inherit disabled.
+- Page permissions have inherit enabled.
+- Chapter fallback has entity allow permission.
+- Role A has entity deny chapter permission.
+- Role B has no entity chapter permissions.
+- User has Role A & B.
+
+User denied page permission.
\ No newline at end of file

diff --git a/tests/Helpers/PermissionsProvider.php b/tests/Helpers/PermissionsProvider.php
index 2cbfb1af583388afae693198006844a1bf83248b..b93c45e25e00c9d44a0df7d1a7279b6ba7837ac9 100644 (file)
--- a/tests/Helpers/PermissionsProvider.php
+++ b/tests/Helpers/PermissionsProvider.php
@@ -101,6 +101,13 @@ class PermissionsProvider
         $this->addEntityPermissionEntries($entity, [$permissionData]);
     }
 
+    public function setFallbackPermissions(Entity $entity, array $actionList)
+    {
+        $entity->permissions()->where('role_id', '=', 0)->delete();
+        $permissionData = $this->actionListToEntityPermissionData($actionList, 0);
+        $this->addEntityPermissionEntries($entity, [$permissionData]);
+    }
+
     /**
      * Disable inherited permissions on the given entity.
      * Effectively sets the "Other Users" UI permission option to not inherit, with no permissions.

diff --git a/tests/Permissions/Scenarios/EntityRolePermissionsTest.php b/tests/Permissions/Scenarios/EntityRolePermissionsTest.php
index b92ce620b78c595c159c4bf1b021c14a3bd5cc51..c8f1401e7745ea2a49c8619b2a83fbb9dfcd2422 100644 (file)
--- a/tests/Permissions/Scenarios/EntityRolePermissionsTest.php
+++ b/tests/Permissions/Scenarios/EntityRolePermissionsTest.php
@@ -187,7 +187,7 @@ class EntityRolePermissionsTest extends PermissionScenarioTestCase
         $this->assertNotVisibleToUser($page, $user);
     }
 
-    public function test_80_multi_role_inherited_deny_via_parent()
+    public function test_75_multi_role_inherited_deny_via_parent()
     {
         [$user, $roleA] = $this->users->newUserWithRole([], ['page-view-all']);
         $roleB = $this->users->attachNewRole($user);
@@ -198,4 +198,99 @@ class EntityRolePermissionsTest extends PermissionScenarioTestCase
 
         $this->assertNotVisibleToUser($page, $user);
     }
+
+    public function test_80_fallback_override_allow()
+    {
+        [$user, $roleA] = $this->users->newUserWithRole();
+        $page = $this->entities->page();
+
+        $this->permissions->setFallbackPermissions($page, []);
+        $this->permissions->addEntityPermission($page, ['view'], $roleA);
+
+        $this->assertVisibleToUser($page, $user);
+    }
+    public function test_81_fallback_override_deny()
+    {
+        [$user, $roleA] = $this->users->newUserWithRole();
+        $page = $this->entities->page();
+
+        $this->permissions->setFallbackPermissions($page, ['view']);
+        $this->permissions->addEntityPermission($page, [], $roleA);
+
+        $this->assertNotVisibleToUser($page, $user);
+    }
+
+    public function test_84_fallback_override_allow_multi_role()
+    {
+        [$user, $roleA] = $this->users->newUserWithRole();
+        $roleB = $this->users->attachNewRole($user);
+        $page = $this->entities->page();
+
+        $this->permissions->setFallbackPermissions($page, []);
+        $this->permissions->addEntityPermission($page, ['view'], $roleA);
+
+        $this->assertVisibleToUser($page, $user);
+    }
+
+    public function test_85_fallback_override_deny_multi_role()
+    {
+        [$user, $roleA] = $this->users->newUserWithRole();
+        $roleB = $this->users->attachNewRole($user);
+        $page = $this->entities->page();
+
+        $this->permissions->setFallbackPermissions($page, ['view']);
+        $this->permissions->addEntityPermission($page, [], $roleA);
+
+        $this->assertNotVisibleToUser($page, $user);
+    }
+
+    public function test_86_fallback_override_allow_inherit()
+    {
+        [$user, $roleA] = $this->users->newUserWithRole();
+        $page = $this->entities->page();
+        $chapter = $page->chapter;
+
+        $this->permissions->setFallbackPermissions($chapter, []);
+        $this->permissions->addEntityPermission($chapter, ['view'], $roleA);
+
+        $this->assertVisibleToUser($page, $user);
+    }
+
+    public function test_87_fallback_override_deny_inherit()
+    {
+        [$user, $roleA] = $this->users->newUserWithRole();
+        $page = $this->entities->page();
+        $chapter = $page->chapter;
+
+        $this->permissions->setFallbackPermissions($chapter, ['view']);
+        $this->permissions->addEntityPermission($chapter, [], $roleA);
+
+        $this->assertNotVisibleToUser($page, $user);
+    }
+
+    public function test_88_fallback_override_allow_multi_role_inherit()
+    {
+        [$user, $roleA] = $this->users->newUserWithRole();
+        $roleB = $this->users->attachNewRole($user);
+        $page = $this->entities->page();
+        $chapter = $page->chapter;
+
+        $this->permissions->setFallbackPermissions($chapter, []);
+        $this->permissions->addEntityPermission($chapter, ['view'], $roleA);
+
+        $this->assertVisibleToUser($page, $user);
+    }
+
+    public function test_89_fallback_override_deny_multi_role_inherit()
+    {
+        [$user, $roleA] = $this->users->newUserWithRole();
+        $roleB = $this->users->attachNewRole($user);
+        $page = $this->entities->page();
+        $chapter = $page->chapter;
+
+        $this->permissions->setFallbackPermissions($chapter, ['view']);
+        $this->permissions->addEntityPermission($chapter, [], $roleA);
+
+        $this->assertNotVisibleToUser($page, $user);
+    }
 }