Auth check in indexQuery only checks on last condition

HI all,

using Nova I built a very simple auhtorization concept. Every user has one or multiple roles whereas every role has one or multiple access rights assigned to it.

When showing a list of resources to the user sending the request I want them to only see those entries that are tied to the user itself (belongsToMany in this case). But if the user has a certain role he/she can see all the entries regardless of whether they are assigned to it or not.

I am using the following code for it:

 public static function indexQuery(NovaRequest $request, $query)
    {
        $user = $request->user();
        $roles = $user->roles;

        foreach($roles as $role) {
            $rights = $role->rights;
            foreach($rights as $right) {
                if($right->name == 'manage_entries') {
                    return $query;
                }
                elseif($right->name == 'view_any_entries') {
                    return $query->where('user_id', $user->id);
                }
            }
        }
    }

This code works - if only one condition applies to the request sending user. But if the roles of a user contain both rights, the query only goes for the elseif and show those that the user is assgend to. I though the logic would be: if you have "manage_entries" being part of your roles then you're good, here: see all, no more checks. But instead I get to see only those entries that the user is assigned to.

It's very likely I miss out some logic here big time but I have no clue what it is. Would appreciate any remark!

Thank you.

jstn

4 years ago

Best Answer

Level 1

In your logic, the return will depend on the order of which the rights are returned. Eg. manage_entries is the first in the rights array, it will return $query. If view_any_entries is the first in the rights array, it will return $query->where('user_id', $user->id);

What you, I assume, want to do is check all rights for manage_entries and return unconstrained query, otherwise constrain on individual rights.

public static function indexQuery(NovaRequest $request, $query)
{
    $user = $request->user();
    $roles = $user->roles;
    $canManageEntries = false;
    $canViewEntries = false;

    foreach ($roles as $role) {
        $rights = $role->rights;
        foreach ($rights as $right) {
            if ($right->name == 'manage_entries') {
                $canManageEntries = true;
            }
            if ($right->name == 'view_any_entries') {
                $canViewEntries = true;
            }
        }
    }
    
    if ($canManageEntries) { // Return unconstrained query when `manage_entries` found
        return $query;
    } elseif ($canViewEntries) { // Return constrained query when `view_any_entries` found
        return $query->where('user_id', $user->id);
    }

    // Return null if neither rights found.
    return null;
}

1 like

acrm

4 years ago

Level 1

@jstn Thank you for taking the time and your quick feedback. Your solution makes sense, just tried it out and it works as expected.

As I am new to programming, I wondered why my code didn't loop through all rights and checked for the first if condition but with your "stimulus" it makes sense as the application goes through each right for the entrie foreach loop and checks if one of the listed conditions strikes - as soon as one is found it goes with that - a bit like russian roulette here ;-).

Thank you, have a nice day!

Please or to participate in this conversation.