Myth\Auth\Authorization\PermissionModel:doesUserHavePermission() - Why Check group permissions?

[sclubricants]

I have an issue where I'm calling has_permission(int) in my application in several places. If the user does not have the permission then the database gets called every time I call has_permission(int);

In Myth\Auth\Authorization\PermissionModel:doesUserHavePermission()

        $userPerms = $this->getPermissionsForUser($userId);

        if (count($userPerms) && array_key_exists($permissionId, $userPerms))
        {
            return true;
        }

$this->getPermissionsForUser($userId) gets permissions for user - both group permissions and individual permissions. If the requested permission is found in all of the users permissions then return true.

For reference:

    public function getPermissionsForUser(int $userId): array
    {
        if (null === $found = cache("{$userId}_permissions"))
        {
            $fromUser = $this->db->table('auth_users_permissions')
                ->select('id, auth_permissions.name')
                ->join('auth_permissions', 'auth_permissions.id = permission_id', 'inner')
                ->where('user_id', $userId)
                ->get()
                ->getResultObject();
            $fromGroup = $this->db->table('auth_groups_users')
                ->select('auth_permissions.id, auth_permissions.name')
                ->join('auth_groups_permissions', 'auth_groups_permissions.group_id = auth_groups_users.group_id', 'inner')
                ->join('auth_permissions', 'auth_permissions.id = auth_groups_permissions.permission_id', 'inner')
                ->where('user_id', $userId)
                ->get()
                ->getResultObject();

            $combined = array_merge($fromUser, $fromGroup);

            $found = [];
            foreach ($combined as $row)
            {
                $found[$row->id] = strtolower($row->name);
            }

            cache()->save("{$userId}_permissions", $found, 300);
        }

        return $found;
    }

It would seem logical that if the requested permission is not found in this manner then the user does not have the permission and then return false.

However for some reason the code continues and looks up group permissions for the user once again. This seems redundant unless I'm missing something. If the user doesn't have the permission requested then it will continually lookup in the database.

        // Check group permissions
        $count = $this->db->table('auth_groups_permissions')
            ->join('auth_groups_users', 'auth_groups_users.group_id = auth_groups_permissions.group_id', 'inner')
            ->where('auth_groups_permissions.permission_id', $permissionId)
            ->where('auth_groups_users.user_id', $userId)
            ->countAllResults();

        return $count > 0;

It seems to me that the entire method doesUserHavePermission() should look like this:

    /**
     * Checks to see if a user, or one of their groups,
     * has a specific permission.
     *
     * @param int $userId
     * @param int $permissionId
     *
     * @return bool
     */
    public function doesUserHavePermission(int $userId, int $permissionId): bool
    {
        // Check user permissions and take advantage of caching
        $userPerms = $this->getPermissionsForUser($userId);

        if (count($userPerms) && array_key_exists($permissionId, $userPerms))
        {
            return true;
        }

        return false;
    }

Am I right or am I missing something?

[MGatner]

It's a little hard for me to look at all the code on mobile but if I understand what you are saying then: yes, you are correct that this is erroneously loading group permissions twice. Feel free to send over a PR with your change!