hasPermission function

[sourcecanada]

So still learning myth-auth, but from a designer perspective I did run into a snag out the gate.

When designing templates you might have a section of the page that should only display if the user has access to at least one of the items inside the section, otherwise there is no point in displaying it.

Based on what I could find a slight modification was needed to the function "hasPermission" to use $permission as an array which then changes the function to allow either a single permission request or an array of requests.

I've included the code below showing the modified hasPermissions function in the FlatAuthorization.php file. Is there a better way to accomplish this goal? I'd like to know if i'm missing something.

public function hasPermission($permission, int $userId)
{
	if (!is_array($permission))
	{
		## Just one permission was set, put it in an array
		$permission = array($permission);
	}
        
        foreach ($permission AS $key => $permission_name)
        {
            // @phpstan-ignore-next-line
            if (empty($permission_name) || (! is_string($permission_name) && ! is_numeric($permission_name)))
            {
                return null;
            }

            if (empty($userId) || ! is_numeric($userId))
            {
                return null;
            }
        }
        
        foreach($permission AS $key => $permission_name)
        {
            // Get the Permission ID
            $permissionId = $this->getPermissionID($permission_name);

            if (! is_numeric($permissionId))
            {
                return false;
            }

            // First check the permission model. If that exists, then we're golden.
            if ($this->permissionModel->doesUserHavePermission($userId, (int)$permissionId))
            {           
                return true;
            }        

            // Still here? Then we have one last check to make - any user private permissions.
            if ($this->doesUserHavePermission($userId, (int)$permissionId))
            {
                return true;
            }
        }
        ## Nothing worked sorry, access denied
        return false;
    }

[mjamilasfihani]

this is a good idea 👍
have you tried to use return instead of checking first at the line $this->doesUserHavePermission($userId, ...), so we can remove return false in the last

[sourcecanada]

The final return false is there so that if all iterations of the array loop fail it needs to return false. I kept the flow of the original function intact when I coded this and that was how they had it.

So even though it was checked previously it still can't return "false".

A return function can only be triggered once during the running of the function moving the false inside the loop would potentially kill the loop before checking all of the permissions in the array.

[mjamilasfihani]

Maybe you can turn this discussion into issue? So this idea will always scope in.

[MGatner]

Even better how about submitting a Pull Request @sourcecanada? My one recommendation would be that instead of changing hasPermission() to accept different input types (I haven't looked to see if this will break anything, it may not) what about creating a new method hasPermissions() that explicitly accepts an array? The former method can then be adjusted to call the latter with a single element array.

[MGatner]

Just looked a little more, and the interface does specify a parameter type as int|string:

    /**
     * Checks a user's groups to see if they have the specified permission.
     *
     * @param int|string $permission
     * @param int        $userId
     *
     * @return mixed
     */
    public function hasPermission($permission, int $userId);

It is only in the docblock so it wouldn't break actual code but it would still technically be a breaking change for static analysis, etc.

That said, adding a new method to an existing interface is definitely a breaking change so options are:

1. Beg @lonnieezell to allow the docblock change
2. Add hasPermissions() like I suggested and just don't include it on the interface (you will need to be explicit about using FlatAuthorization to know the method exists)
3. Add this to the interface for the next major version, not sure when that will be

[sourcecanada]

I did think of that after the fact, writing a seperate function that handles the array and then just calls the hasPermission function in a loop. That said, it seems rational to expect that functionality to be native when you look at how useful that functionality is when building a web application.