Policy::view must also pass Policy::viewAny

[AdrianwithaW]

Seems like when the Policy::view method, Nova also applies the Policy::viewAny method. This seems counter-intuitive to me for what I would think would be an obvious scenario when implementing roles:

1) An Admin User can view the index, view a record, as well as create, update and delete User records - so the viewAny, view, create, update and delete polices would all return true, in my case based on a can(…) check of their role and/or permissions. 2) A Standard User cannot view the index, create or delete User records, thus the viewAny, create and delete policies would all return false, based on a can(…) check of their role and/or permissions. However the User would likely be able to view and update their own account, so the desired functionality would be that the view and update polices could return true in the case where the model being passed through has the same id as the current user.

This extends beyond a user model to any model where a super user has access to the full CRUD of a model, where a standard user may only have access to their own authorised instance of the model if it belongs to them.

Ultimately, view should not also rely on also being able to pass viewAny because you should be able to grant view access to something without being forced to grant them "view all" access.

Hope that's helpful and make sense. Am submitting as a question to first clarify whether there's a reason I'm missing that it has to be like this, but I'd imagine if you wanted to enforce the viewAny method with the view method, you could simply call it where desired and pass through the $user model.

[niekdemelker]

I guess that if the viewAny() returns false you are not permitted to view any of the resource. If you return null instead of false the next check will run.

so if you change the function to something like the snippet below it should work

public function viewAny($user, $ability)
{
    if ($user->can('viewAny')) {
        return true;
    }
}

[AdrianwithaW]

Not sure I follow…? I would still need viewAny to return true for the user model which wouldn't be the case. I don't want them to be able to view any user model, but do want them to be able to view their user model.

[niekdemelker]

In the snippet I provided the function returns true or null, when you return true, the policy succeeds and the resource is shown. If the function returns false, the policy fails and you cannot see the resource. If the function returns null, the policy will ask for the next check. For example view().

[AdrianwithaW]

Sorry, I understand the true / false nature, but I would still have to grant the user viewAny access for all users, just to let them view their own user model which is the bit that doesn't make sense to me.

It's the cascading part I don't see the point in - viewAny and view should be unrelated as they're capable of dealing with the related records individually. That way view can have logic that allows access to certain individual records without having to grant access to any/all records.

The mapping of the policies looks like it follows the index, view, create, update, delete pattern, but in reality viewAny doesn't fit with index because it overlaps with view.

[niekdemelker]

After reading again I guess I misunderstood. If you only want a user to only see himself in the overview you shouldn't use viewAny() for that. The viewAny() is something like a before() function (i guess). To only show the current user in the index for example you should configure the view() method, something like this:

    public function view(User $loggedInUser, User $user)
    {
        return $loggedInUser->id == $user->id;
    }

please note that the above snippet is very much untested, I know it works for the update function like the PostPolicy from the docs

class PostPolicy
{
    /**
     * Determine if the given post can be updated by the user.
     *
     * @param  \App\User  $user
     * @param  \App\Post  $post
     * @return bool
     */
    public function update(User $user, Post $post)
    {
        return $user->id === $post->user_id;
    }
}

hope this is of some help

[AdrianwithaW]

Yeah, that's exactly how I have configured it. But before view(…) is run, it passes through viewAny(…) which the user doesn't have access to. So it never gets a chance to evaluate view(…) as being true.

[niekdemelker]

this is in the nature of the viewAny(). This function runs also before delete() and update(). so when you return false in your viewAny() it is like returning false in the before(). The viewAny() is there to hide the entire resourse.

https://nova.laravel.com/docs/2.0/resources/authorization.html#hiding-entire-resources

EDIT: i am not sure if i am correct on the viewAny() running before delete() and update(), starting to doubt here haha.

[AdrianwithaW]

From my testing viewAny() only occurs before view() which is what doesn't quite make sense. It seems like it is intentional, it just makes it overly complicated to allow access to a particular record.

[niekdemelker]

what if you don't use the viewAny() at all. And only the view() i mentioned earlier? is that giving you only the logged in user?

[AdrianwithaW]

Yeah that's my current solution - I have to just return true in every viewAny() then individually secure each index in the model accordingly. It's unnecessarily complicated given that if viewAny was just mapped to the index, I could use $loggedInUser->id == $user->id or $loggedInUser->id == $model->user_id; on any applicable view, update, delete policies.

[niekdemelker]

I found something else you may find interesting, If you only want to display the current user in your index table you could use the indexQuery() in your \App\Nova\User class.

    /**
     * Build an "index" query for the given resource.
     *
     * @param  NovaRequest  $request
     * @param  Builder  $query
     *
     * @return Builder
     */
    public static function indexQuery(NovaRequest $request, $query)
    {
        return $query->where('id', Auth::id());
    }

you can add some extra logic, like permission checks, because an admin probably wants to see all users.

this viewAny() seems a bis disappointing but there are some good usecases for it. you could block everyone who is not a editor, and in the update() you could say something like $user->id === $post->user_id. this way every editor can see all posts but only the owner could update it.

[AdrianwithaW]

No in this case, I have a bunch of models where the logged in user should only be able to see models that they have access to. I've had to utilise indexQuery, createQuery etc where possible but in some cases I don't want them to be able to access the index view, only the individual models they control.

Easiest example is users - they can't see any other users other than themselves so no need to access the indexQuery. But it means I have a bunch of logic to check in the indexQuery when it could simply be done by returning false on the viewAny.

[niekdemelker]

hmm, i see. I don't think Nova has a solution out of the box for this, But there is a package out there who probably can help you.

https://novapackages.com/packages/Jubeki/nova-card-linkable

this gives you the option to directlink to a resource. beside this i'm out of options. Maybe someone else has an idea to accomplish your goal.

[Kovah]

I have the exact same problem. Users should be allowed to view and edit their own profile, but should not be able to view the user index nor view any other profile.

Currently, the system seems to call viewAny() before checking the access via view(). But other than stated in the documentation, returning null from the viewAny() method does not lead to the view() method to be evaluated:

public function viewAny(User $user)
{
    if ($user->can('user::view:all')) {
        return true;
    }

    return null;
}

public function view(User $user, User $model)
{
    return $user->id === $model->id;
}

The resource index for users is now hidden in Nova, but opening the user detail view (/nova/resources/users/1) leads to a 403 too. Not sure if this is actually a bug or I misunderstand the documentation.

[niekdemelker]

Now taylor published some of his upgrade migration, we can read what the viewAny is supposed to do within the framework: https://laravel.com/docs/master/upgrade#authorized-resources

Seems like the viewAny() should only check on the index page and should not apply on the regular view(). The programmer could get the current behaviour with some simple code while the other way around seems impossible.

public function viewAny(User $user)
{
    if ($user->can('user::view:all')) {
        return true;
    }
}

public function view(User $user, User $model)
{
    $viewAny = $this->viewAny($user);

    if (! is_null($viewAny)) {
        return $viewAny;
    }

    return $user->id === $model->id;
}

[AdrianwithaW]

To me, this easily fixed by not having any other policy methods first check the output of viewAny. What would the downside to this be? I might add it as an issue now that it’s been fleshed out a bit.

[niekdemelker]

@AdrianwithaW totally agree, I think the viewAny() needs to check for the index route and the view() needs to check for the show route, no mixing and strange behavior thank you.

The url states the viewAny() is for the index page, no word about the view() so i guess the way it works in nova is not how it is gonna work in Laravel 6, so there needs some correction somewhere.

[Kovah]

Thanks for the fast feedback. In that case I would wait for a fix to be released and then try again. Hope this can be solved. 👍

[Kovah]

Is there a way to notify the Nova team (whoever maintains it?) so they are aware of this bug?

[jbrooksuk]

Is there a way to notify the Nova team (whoever maintains it?) so they are aware of this bug?

We read every issue and we will get to it, we've been working on some low-hanging fruit in terms of issues and features first.

[bgeree]

A possible workaround is to override the availableForNavigation() method in your resource, this way you can hide the resource from the navigation. It's still accessible via direct link! So you have to override the indexQuery() (just to make sure) and you might want to alter the behavior of the redirectAfterUpdate() too based on the access level.

[ReubenPorter]

Hi @AdrianwithaW , I seem to be trying to achieve something you were, did you manage to find a good work around?

[AdrianwithaW]

Hi @AdrianwithaW , I seem to be trying to achieve something you were, did you manage to find a good work around?

Yeah I modified the view policy to not require passing viewAny also. Haven’t found a downside to it to be honest, I can then simply have a check in my view policy that only returns true if the user owns the element in whatever way necessary. For the user case I originally posted above it works great.

[davi5e]

I found this issue since I'm having the exact same problem everyone else mentioned.

I don't know if this is specific to Nova or if this is a Laravel unintended behavior (in this particular case), but seems that the view() method should have a higher priority than viewAny(). Like so, a user would be able to see his profile only, hiding the resource in the sidebar and the whole Model...

I don't know if this applies to any other model besides users, though... Maybe this assumption facilitates a future patch that solves this conundrum.

[Kovah]

@jbrooksuk This small bug ticket is open over half a year now. It would be cool if this could get fixed any time soon.

[steveooo1]

Hi @AdrianwithaW , I seem to be trying to achieve something you were, did you manage to find a good work around?

Yeah I modified the view policy to not require passing viewAny also. Haven’t found a downside to it to be honest, I can then simply have a check in my view policy that only returns true if the user owns the element in whatever way necessary. For the user case I originally posted above it works great.

@AdrianwithaW Can you share the code for this view policy modification? In which file?

[steveooo1]

We have the same issue with the viewAny and view policies.. We want a user to use and see a resource called system via a BelongsTo relatable field on a different resource, but not be able to have the System resource appear in the navigation and not accessable via /resources/system/*.

[AdrianwithaW]

Hi @AdrianwithaW , I seem to be trying to achieve something you were, did you manage to find a good work around?

Yeah I modified the view policy to not require passing viewAny also. Haven’t found a downside to it to be honest, I can then simply have a check in my view policy that only returns true if the user owns the element in whatever way necessary. For the user case I originally posted above it works great.

@AdrianwithaW Can you share the code for this view policy modification? In which file?

I don't have access to the exact codebase I made this change in but I think the change was as follows in nova/src/Authorizable.php …

public function authorizedToView(Request $request)
    {
        return $this->authorizedTo($request, 'view') && $this->authorizedToViewAny($request);
    }

… was modified to …

public function authorizedToView(Request $request)
    {
        return $this->authorizedTo($request, 'view');
    }

If that's not it, you basically just need to find the exact point where viewAny and view are both required to pass.

[steveooo1]

Okay, thanks. I currently solved it by a mixture of some smart logic around availableForNavigaiton() and indexQuery() inside the app/Nova/Resource.php. This makes things okay.

[davi5e]

@steveooo1 could you post a diff or even an example to achieve this? :)

[ThisIsMyFavouriteJar]

I ran into the same problem as the OP. (Basically I want my admin to see all users, and users should ONLY be able to edit themselves, and NOT able to see the entire index.)

I solved this as follows:

I set the UserPolicy so that only the admin can edit all users, and the user can only see and edit himself. ViewAny was still set to true, because we will hide the resource from the sidemenu later.

UserPolicy:

• ViewAny : true
• View : admin or user (return $user->id == $model->id || $user->is_admin;)
• Create : admin
• Update : admin or user
• Delete : admin

In my Nova User Resource I also added the following to only show the resource in the sidebar for the admin / hide it for the user: public static function availableForNavigation(Request $request) { return $request->user()->is_admin; }

And I also added the following IndexQuery to make sure the user REALLY cannot see any other users (even when typing in the index URL directly):

public static function indexQuery(NovaRequest $request, $query) { if($request->user()->is_admin) { return $query; } return $query->where('id', $request->user()->id); }

This way my admin can see all users, and perform all actions on them. The user can only edit or view himself. Technically the user can still visit the index by typing in the right URL, but even if that happens he can only see himself, so there is really no harm in that.

Hope this helps, otherwise I misunderstood the problem ;)

[comes]

nova/src/Http/Request:48 (Nova version 2.12.0)

    public function resource()
    {
        return tap(Nova::resourceForKey($this->route('resource')), function ($resource) {
            abort_if(is_null($resource), 404);
            abort_if(! $resource::authorizedToViewAny($this), 403);
        });
    }

I do understand what this code does, but I do not understand why it is important hiere to check, if a user can view any of the needed resources. The use case is: please let me see and edit my own user. I ran into the same problem and i'd like to understand the 'why?', before I'd like to provide a PR for this issue.

cheers, comes

[comes]

Hej,

in my personal case, I'd like to give an user the posibiliy to edit his own userprofile in nova. I was able to modify my viewAny Gate like this:

public function viewAny(User $user)
    {
        if ( request()->route('resource') === 'users' && (request()->get('resourceId') == $user->id || request()->route('resourceId') == $user->id ) ) {
            return true;
        }

        return $user->can("user.index");
    }

It is not possible to view any resources, but you are allowed to view your own user model.

cheers, comes

[stale[bot]]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

[davidhemphill]

Yeah, the behavior is set in stone, at least for this series of Nova, where we'll revisit. If you cannot viewAny, you probably can't view and that is what we'll stick with. If you need different behavior, feel free to override the gate methods manually to accommodate your use-case. Thanks!

[tractorcow]

I've been looking at this issue, and I've come to realise that the logic behind the decision to keep this as-is makes some sense. Having viewAny return true if you can only view a subset of records is correct, because you can view any (even if not all).

The way I understand it is, viewAny means "any could be viewable", which is different from "all are viewable".

In my app I resolved this ambiguity be declaring a viewAll gate in my policy, and where a user passes viewAny, but fails viewAll, I just make sure to disambiguate this in my view gate on a case by case basis.

Then in my nova resource, I can use these checks to control what's visible to users:

1. Restrict the global index view to only those with viewAll permission (note: This doesn't actually secure access to anything, just hides it from the menu). I could also add a viewAny check here if I wanted it to show for those users too.

    public static function availableForNavigation(Request $request): bool
    {
        return Gate::allows('viewAll', \App\Models\User::class);
    }

2. Filter viewable records via indexQuery. Note the below is just an example pseudocode from one use case; YMMV depending on what you are trying to achieve.

    public static function indexQuery(NovaRequest $request, $query): Builder
    {
        $filtered = parent::indexQuery($request, $query);

        // viewAll requires no filtering
        if (Gate::allows('viewAll', \App\Models\AssessmentSubmission::class)) {
            return $filtered;
        }

        // viewAny means some filtering required; Example
        if (Gate::allows('viewAny', \App\Models\AssessmentSubmission::class)) {
            return $filtered->where('assessment_submissions.marker_id', '=', static::getUser()->id);
        }

        throw new AuthorizationException();
    }

It's also important to make sure that you implement your view gate correctly, otherwise you are relying on users not simply guessing the resource link to other items to view them.

[AdrianwithaW]

This is a good solution @tractorcow but I don't think that quite fits my original scenario though. The issue is more with the view policy which implies it's a viewOne policy that provides a gate against the resource being accessed unless they have permission. Because it's about viewing that one resource, I personally don't quite see why it should pass through the viewAny policy first. I think a better implementation would be that if the user doesn't have explicit access (in this case because it's not their resource to view) only then would you do a secondary check for whether they have a viewAny or viewAll permission. Purely my opinion, I'm perhaps misinterpreting the nomenclature of the policies.

[tractorcow]

Yeah it's a bit redundant to pass viewAny if view can be called, I'll give you that. :)

Just trying to come up with the most charitable interpretation of the current behaviour I could. :)