Test new case for Manager::userHasPermission()

CHANGELOG.md

@@ -59,7 +59,9 @@
 - Enh #252: Return `$this` instead of throwing "already assigned" exception in `Manager::assign()` (@arogachev)
 - Enh #248: Add `SimpleRuleFactory` (@arogachev)
 - Enh #251: Allow checking for user's roles in `ManagerInterface::userHasPermission()` (@arogachev)
-- Chg #259: Rename `$ruleContext` argument to `$context` in `RuleInterface::execute()` (@arogachev) 
+- Chg #259: Rename `$ruleContext` argument to `$context` in `RuleInterface::execute()` (@arogachev)
+- Bug #260: Fix `Manager::userHasPermission()` to return `true` for the case when a user have access via at least one 
+  hierarchy branch (@arogachev) 
 
 ## 1.0.2 April 20, 2023
 

src/Manager.php

@@ -70,37 +70,44 @@ public function userHasPermission(
         }
 
         $hierarchy = $this->itemsStorage->getHierarchy($item->getName());
-        if ($guestRole !== null && !array_key_exists($guestRole->getName(), $hierarchy)) {
-            $hierarchy[$guestRole->getName()] = ['item' => $guestRole, 'children' => []];
-        }
-
         $itemNames = array_map(static fn (array $treeItem): string => $treeItem['item']->getName(), $hierarchy);
         $userItemNames = $guestRole !== null
             ? [$guestRole->getName()]
             : $this->assignmentsStorage->filterUserItemNames((string) $userId, $itemNames);
+        $userItemNamesMap = [];
+        foreach ($userItemNames as $userItemName) {
+            $userItemNamesMap[$userItemName] = null;
+        }
 
-        $userItems = [];
-        foreach ($userItemNames as $itemName) {
-            $userItems[$itemName] = $hierarchy[$itemName]['item'];
-            foreach ($hierarchy[$itemName]['children'] ?? [] as $item) {
-                $userItems[$item->getName()] = $item;
+        foreach ($hierarchy as $data) {
+            if (
+                !array_key_exists($data['item']->getName(), $userItemNamesMap) ||
+                !$this->executeRule($userId === null ? $userId : (string) $userId, $data['item'], $parameters)
+            ) {
+                continue;
             }
-        }
 
-        if (
-            empty($userItems) ||
-            ($guestRole !== null && count($userItems) === 1 && array_key_exists('guest', $userItems))
-        ) {
-            return false;
-        }
+            $hasPermission = true;
+            foreach ($data['children'] as $childItem) {
+                if (!$this->executeRule($userId === null ? $userId : (string) $userId, $childItem, $parameters)) {
+                    $hasPermission = false;
+
+                    /**
+                     * @infection-ignore-all Break_
+                     * Replacing with `continue` works as well, but there is no point in further checks, because at
+                     * least one failed rule execution means access is not granted via current iterated hierarchy
+                     * branch.
+                     */
+                    break;
+                }
+            }
 
-        foreach ($userItems as $item) {
-            if (!$this->executeRule($userId === null ? $userId : (string) $userId, $item, $parameters)) {
-                return false;
+            if ($hasPermission) {
+                return true;
             }
         }
 
-        return true;
+        return false;
     }
 
     public function canAddChild(string $parentName, string $childName): bool

tests/Common/ManagerLogicTestTrait.php

@@ -22,6 +22,7 @@
 use Yiisoft\Rbac\Tests\Support\BanRule;
 use Yiisoft\Rbac\Tests\Support\FakeAssignmentsStorage;
 use Yiisoft\Rbac\Tests\Support\FakeItemsStorage;
+use Yiisoft\Rbac\Tests\Support\FalseRule;
 use Yiisoft\Rbac\Tests\Support\GuestRule;
 use Yiisoft\Rbac\Tests\Support\SubscriptionRule;
 use Yiisoft\Rbac\Tests\Support\TrueRule;
@@ -297,6 +298,21 @@ public function testUserHasPermissionWithRolesAllowed(
         );
     }
 
+    public function testUserHasPermissionWithOneHierarchyBranch(): void
+    {
+        $manager = $this
+            ->createFilledManager()
+            ->addPermission(new Permission('Permission'))
+            ->addRole(new Role('Role 1'))
+            ->addRole((new Role('Role 2'))->withRuleName(FalseRule::class))
+            ->addChild('Role 1', 'Permission')
+            ->addChild('Role 2', 'Permission')
+            ->assign(itemName: 'Role 1', userId: 'User')
+            ->assign(itemName: 'Role 2', userId: 'User');
+
+        $this->assertTrue($manager->userHasPermission('User', 'Permission'));
+    }
+
     public function testCanAddExistingChild(): void
     {
         $manager = $this->createFilledManager();