Skip to content

Commit 8be3645

Browse files
ssddanbrownssddanbrown
committed
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.
1 parent d1bd6d0 commit 8be3645

File tree

6 files changed

+192
-9
lines changed

6 files changed

+192
-9
lines changed

‎app/Auth/Permissions/EntityPermissionEvaluator.php‎

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -25,22 +25,24 @@ public function evaluateEntityForUser(Entity $entity, array $userRoleIds): ?bool
2525
$relevantPermissions = $this->getPermissionsMapByTypeId($typeIdChain, [...$userRoleIds, 0]);
2626
$permitsByType = $this->collapseAndCategorisePermissions($typeIdChain, $relevantPermissions);
2727

28-
return $this->evaluatePermitsByType($permitsByType);
28+
$status = $this->evaluatePermitsByType($permitsByType);
29+
30+
return is_null($status) ? null : $status === PermissionStatus::IMPLICIT_ALLOW || $status === PermissionStatus::EXPLICIT_ALLOW;
2931
}
3032

3133
/**
3234
* @param array<string, array<string, int>> $permitsByType
3335
*/
34-
protected function evaluatePermitsByType(array $permitsByType): ?bool
36+
protected function evaluatePermitsByType(array $permitsByType): ?int
3537
{
3638
// Return grant or reject from role-level if exists
3739
if (count($permitsByType['role']) > 0) {
38-
return boolval(max($permitsByType['role']));
40+
return max($permitsByType['role']) ? PermissionStatus::EXPLICIT_ALLOW : PermissionStatus::EXPLICIT_DENY;
3941
}
4042

4143
// Return fallback permission if exists
4244
if (count($permitsByType['fallback']) > 0) {
43-
return boolval($permitsByType['fallback'][0]);
45+
return $permitsByType['fallback'][0] ? PermissionStatus::IMPLICIT_ALLOW : PermissionStatus::IMPLICIT_DENY;
4446
}
4547

4648
return null;

‎app/Auth/Permissions/JointPermissionBuilder.php‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -262,8 +262,7 @@ protected function createJointPermissionData(SimpleEntityData $entity, int $role
262262
// Return evaluated entity permission status if it has an affect.
263263
$entityPermissionStatus = $permissionMap->evaluateEntityForRole($entity, $roleId);
264264
if ($entityPermissionStatus !== null) {
265-
$status = $entityPermissionStatus ? PermissionStatus::EXPLICIT_ALLOW : PermissionStatus::EXPLICIT_DENY;
266-
return $this->createJointPermissionDataArray($entity, $roleId, $status, $entityPermissionStatus);
265+
return $this->createJointPermissionDataArray($entity, $roleId, $entityPermissionStatus, false);
267266
}
268267

269268
// Otherwise default to the role-level permissions

‎app/Auth/Permissions/MassEntityPermissionEvaluator.php‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@ public function __construct(array $entitiesInvolved, string $action)
1616
parent::__construct($action);
1717
}
1818

19-
public function evaluateEntityForRole(SimpleEntityData $entity, int $roleId): ?bool
19+
public function evaluateEntityForRole(SimpleEntityData $entity, int $roleId): ?int
2020
{
2121
$typeIdChain = $this->gatherEntityChainTypeIds($entity);
2222
$relevantPermissions = $this->getPermissionMapByTypeIdForChainAndRole($typeIdChain, $roleId);

‎dev/docs/permission-scenario-testing.md‎

Lines changed: 81 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -229,7 +229,7 @@ User denied page permission.
229229

230230
User denied page permission.
231231

232-
#### test_80_multi_role_inherited_deny_via_parent
232+
#### test_75_multi_role_inherited_deny_via_parent
233233

234234
- Page permissions have inherit enabled.
235235
- Chapter permissions have inherit enabled.
@@ -238,3 +238,83 @@ User denied page permission.
238238
- User has Role A & B.
239239

240240
User denied page permission.
241+
242+
#### test_80_fallback_override_allow
243+
244+
- Page permissions have inherit disabled.
245+
- Page fallback has entity deny permission.
246+
- Role A has entity allow page permission.
247+
- User has Role A.
248+
249+
User granted page permission.
250+
251+
#### test_81_fallback_override_deny
252+
253+
- Page permissions have inherit disabled.
254+
- Page fallback has entity allow permission.
255+
- Role A has entity deny page permission.
256+
- User has Role A.
257+
258+
User denied page permission.
259+
260+
#### test_84_fallback_override_allow_multi_role
261+
262+
- Page permissions have inherit disabled.
263+
- Page fallback has entity deny permission.
264+
- Role A has entity allow page permission.
265+
- Role B has no entity page permissions.
266+
- User has Role A & B.
267+
268+
User granted page permission.
269+
270+
#### test_85_fallback_override_deny_multi_role
271+
272+
- Page permissions have inherit disabled.
273+
- Page fallback has entity allow permission.
274+
- Role A has entity deny page permission.
275+
- Role B has no entity page permissions.
276+
- User has Role A & B.
277+
278+
User denied page permission.
279+
280+
#### test_86_fallback_override_allow_inherit
281+
282+
- Chapter permissions have inherit disabled.
283+
- Page permissions have inherit enabled.
284+
- Chapter fallback has entity deny permission.
285+
- Role A has entity allow chapter permission.
286+
- User has Role A.
287+
288+
User granted page permission.
289+
290+
#### test_87_fallback_override_deny_inherit
291+
292+
- Chapter permissions have inherit disabled.
293+
- Page permissions have inherit enabled.
294+
- Chapter fallback has entity allow permission.
295+
- Role A has entity deny chapter permission.
296+
- User has Role A.
297+
298+
User denied page permission.
299+
300+
#### test_88_fallback_override_allow_multi_role_inherit
301+
302+
- Chapter permissions have inherit disabled.
303+
- Page permissions have inherit enabled.
304+
- Chapter fallback has entity deny permission.
305+
- Role A has entity allow chapter permission.
306+
- Role B has no entity chapter permissions.
307+
- User has Role A & B.
308+
309+
User granted page permission.
310+
311+
#### test_89_fallback_override_deny_multi_role_inherit
312+
313+
- Chapter permissions have inherit disabled.
314+
- Page permissions have inherit enabled.
315+
- Chapter fallback has entity allow permission.
316+
- Role A has entity deny chapter permission.
317+
- Role B has no entity chapter permissions.
318+
- User has Role A & B.
319+
320+
User denied page permission.

‎tests/Helpers/PermissionsProvider.php‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -101,6 +101,13 @@ public function addEntityPermission(Entity $entity, array $actionList, Role $rol
101101
$this->addEntityPermissionEntries($entity, [$permissionData]);
102102
}
103103

104+
public function setFallbackPermissions(Entity $entity, array $actionList)
105+
{
106+
$entity->permissions()->where('role_id', '=', 0)->delete();
107+
$permissionData = $this->actionListToEntityPermissionData($actionList, 0);
108+
$this->addEntityPermissionEntries($entity, [$permissionData]);
109+
}
110+
104111
/**
105112
* Disable inherited permissions on the given entity.
106113
* Effectively sets the "Other Users" UI permission option to not inherit, with no permissions.

‎tests/Permissions/Scenarios/EntityRolePermissionsTest.php‎

Lines changed: 96 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -187,7 +187,7 @@ public function test_70_multi_role_inheriting_deny()
187187
$this->assertNotVisibleToUser($page, $user);
188188
}
189189

190-
public function test_80_multi_role_inherited_deny_via_parent()
190+
public function test_75_multi_role_inherited_deny_via_parent()
191191
{
192192
[$user, $roleA] = $this->users->newUserWithRole([], ['page-view-all']);
193193
$roleB = $this->users->attachNewRole($user);
@@ -198,4 +198,99 @@ public function test_80_multi_role_inherited_deny_via_parent()
198198

199199
$this->assertNotVisibleToUser($page, $user);
200200
}
201+
202+
public function test_80_fallback_override_allow()
203+
{
204+
[$user, $roleA] = $this->users->newUserWithRole();
205+
$page = $this->entities->page();
206+
207+
$this->permissions->setFallbackPermissions($page, []);
208+
$this->permissions->addEntityPermission($page, ['view'], $roleA);
209+
210+
$this->assertVisibleToUser($page, $user);
211+
}
212+
public function test_81_fallback_override_deny()
213+
{
214+
[$user, $roleA] = $this->users->newUserWithRole();
215+
$page = $this->entities->page();
216+
217+
$this->permissions->setFallbackPermissions($page, ['view']);
218+
$this->permissions->addEntityPermission($page, [], $roleA);
219+
220+
$this->assertNotVisibleToUser($page, $user);
221+
}
222+
223+
public function test_84_fallback_override_allow_multi_role()
224+
{
225+
[$user, $roleA] = $this->users->newUserWithRole();
226+
$roleB = $this->users->attachNewRole($user);
227+
$page = $this->entities->page();
228+
229+
$this->permissions->setFallbackPermissions($page, []);
230+
$this->permissions->addEntityPermission($page, ['view'], $roleA);
231+
232+
$this->assertVisibleToUser($page, $user);
233+
}
234+
235+
public function test_85_fallback_override_deny_multi_role()
236+
{
237+
[$user, $roleA] = $this->users->newUserWithRole();
238+
$roleB = $this->users->attachNewRole($user);
239+
$page = $this->entities->page();
240+
241+
$this->permissions->setFallbackPermissions($page, ['view']);
242+
$this->permissions->addEntityPermission($page, [], $roleA);
243+
244+
$this->assertNotVisibleToUser($page, $user);
245+
}
246+
247+
public function test_86_fallback_override_allow_inherit()
248+
{
249+
[$user, $roleA] = $this->users->newUserWithRole();
250+
$page = $this->entities->page();
251+
$chapter = $page->chapter;
252+
253+
$this->permissions->setFallbackPermissions($chapter, []);
254+
$this->permissions->addEntityPermission($chapter, ['view'], $roleA);
255+
256+
$this->assertVisibleToUser($page, $user);
257+
}
258+
259+
public function test_87_fallback_override_deny_inherit()
260+
{
261+
[$user, $roleA] = $this->users->newUserWithRole();
262+
$page = $this->entities->page();
263+
$chapter = $page->chapter;
264+
265+
$this->permissions->setFallbackPermissions($chapter, ['view']);
266+
$this->permissions->addEntityPermission($chapter, [], $roleA);
267+
268+
$this->assertNotVisibleToUser($page, $user);
269+
}
270+
271+
public function test_88_fallback_override_allow_multi_role_inherit()
272+
{
273+
[$user, $roleA] = $this->users->newUserWithRole();
274+
$roleB = $this->users->attachNewRole($user);
275+
$page = $this->entities->page();
276+
$chapter = $page->chapter;
277+
278+
$this->permissions->setFallbackPermissions($chapter, []);
279+
$this->permissions->addEntityPermission($chapter, ['view'], $roleA);
280+
281+
$this->assertVisibleToUser($page, $user);
282+
}
283+
284+
public function test_89_fallback_override_deny_multi_role_inherit()
285+
{
286+
[$user, $roleA] = $this->users->newUserWithRole();
287+
$roleB = $this->users->attachNewRole($user);
288+
$page = $this->entities->page();
289+
$chapter = $page->chapter;
290+
291+
$this->permissions->setFallbackPermissions($chapter, ['view']);
292+
$this->permissions->addEntityPermission($chapter, [], $roleA);
293+
294+
$this->assertNotVisibleToUser($page, $user);
295+
}
201296
}

0 commit comments

Comments
 (0)