can('viewDiscussions') returns wrong result?

matteocontrini

Hello,

I'm using the DiscussionStarted event in an extension to check if a new discussion can be seen by a guest, and then perform an action based on that.

However, it appears that if I restrict the ability to start a discussion with a tag to admins, while retaining the viewDiscussions permission for guests, the following code returns false when posting a discussion with that tag:

(new Guest)->can('viewDiscussions', $event->discussion)

With other discussions it instead returns true correctly.

Is this a bug or am I missing something?

Sami (SychO)

Is this on Beta 15 ?

matteocontrini

Sami (SychO) yes, sorry, forgot to mention 🙂

clarkwinkelmann

matteocontrini viewDiscussions is a global permission which isn't part of or isn't proxied by the discussion policy, so it just returns the value of the global viewDiscussions permission without any regard to which discussion you passed as second parameter.

Flarum doesn't have a way to directly tell if a discussion instance is visible to an arbitrary user without making any database request, because the whole visibility layer is implemented as database query scopes.

What you need to do is create a new database query that will take that arbitrary user and determine if the user can see the discussion.

Something like this (untested):

Discussion::whereVisibleTo(new Guest)->where('id', $discussionId)->exists()

matteocontrini

clarkwinkelmann viewDiscussions is a global permission which isn't part of or isn't proxied by the discussion policy, so it just returns the value of the global viewDiscussions permission without any regard to which discussion you passed as second parameter.

I see. So why does it return different values depending on the value that is passed as the second parameter? 🤔

Also, is there a way to know whether a permission is global, other than diving for three hours into Flarum's source code?

I thought that permissions that in the admin panel can be scoped to a specific tag accepted a model instance as the second parameter.

clarkwinkelmann Something like this (untested):

I'll give it a try, thank you 🙂

clarkwinkelmann

matteocontrini we are working towards some more logical solutions for policies.

There's no single truth, but it's mostly this: there are two kind of policies that accept models: those which are explicitly defined in a policy, Like ->can('rename', $discussion)

The other kind are the "magic" ones handled by the general ->can() method on the policy. Basically if there's no method, for example for ->can('reply', $discussion), then that magic method automatically checks for the discussion.reply permission, which won't behave differently per discussion by default.

There is an extra layer of magic introduced by the Tags extension. If an ability has no dedicated method, the magic ->can() is replaced with one that takes into account the tags of the discussion.

In your exact case, I assume calling ->can('viewDiscussions') might return true when a discussion with no tags is passed since it passes through the policy and just goes to the default (I think there's another default at the gate level if absolutely nothing matches, it just checks it without prefixes), but as soon as the discussion has one tag, a prefixed check will be made which will probably always return false since no such prefixed permission exists in the database.

matteocontrini

clarkwinkelmann it's a little confusing indeed...

clarkwinkelmann In your exact case, I assume calling ->can('viewDiscussions') might return true when a discussion with no tags

It doesn't seem so, it returns true also when there are tags. It may be a "coincidence" but it also returns false when a discussion cannot be actually seen by a guest, according to my tests. That's why I assumed that was the correct way to do it, it seemed to work... 😅

@askvortsov this is one of the things that I think could be improved in the docs. The current docs are a good introduction, but I find that as soon as you try to do something in practice you get lost in Flarum's source code trying to understand which permissions are there, how that can works (and if it works), etc. It's tiring, honestly 😃

tankerkiller125

matteocontrini Admittedly the way the ->can() works is so confusing that even when I was working on splitting the user edit permission I got confused and used it the wrong way. And I think it's incredibly hard to document/explain how it works through written word.

Sami (SychO)

Might this help ? it may not be 100% accurate because of some of the magic that is still present, but it does give the general idea. If a model is not specified with the can method, the system'll check with the global policies, which work the same way other policies do.

flarum_authorization

matteocontrini

Sami (SychO) that's helpful, thanks, some doubts:

1) when you say "Database permission value", you mean the permission that was set through the admin panel which depends only on the actor, I suppose?

2) I think I don't get why global permissions exist. For example, why doesn't the viewDiscussions permission allow to specify a model?

Also, sorry for being repetitive 😃 but I think that core permissions should be at some point listed in the docs. I wouldn't have used the viewDiscussions permission in that way if I knew it didn't accept the second parameter, but I still have no idea where I should have seen that... The docs are currently quite misleading about this:

// Check whether a user can perform an action.
$canDoSomething = $actor->can('viewDiscussions');

// Check whether a user can perform an action on a subject.
$canDoSomething = $actor->can('reply', $discussion);

clarkwinkelmann

In addition / as part of to explaining how it works we'll also need to clarify the naming.

Permissions are technically always global. A permission is defined by a string and is given to one or more groups. Permissions can be directly checked using User::hasPermission($name)

Abilities are what you can check against the gate using User::can($ability, $arguments). When an argument is given on the gate, we will attempt to call the ability on a model policy, which is a way of grouping gate definitions by eloquent model. Abilities/policies are usually based on a permission, together with additional logic.

Visibility scopes are what defines what is visible to a user. Those are also based on Permissions and additional logic. They can be based on abilities, but that's more rare, because an ability requires a model to have been already retrieved.

Abilities/policies/gates are a concept borrowed from Laravel, but it's actually our own implementation. Visibility scopes is something that isn't found in Laravel.

askvortsov

Sami (SychO) I think we should put this drawing in the docs (with a few tweaks), it's quite nice!

matteocontrini this is one of the things that I think could be improved in the docs.

Absolutely agreed! And more charts should help for complicated processes like this. I think I've rewritten the Authorization docs 2 times already, with more iterations, it should keep getting better.

matteocontrini I think I don't get why global permissions exist. For example, why doesn't the viewDiscussions permission allow to specify a model?

The secret here is that "global permissions" don't really exist. Permissions are just strings in the database that can be added and removed to groups.

The "global" bit here is the ->can() check:

If ->can() is called with just an ability name (note the distinction between "ability" and "permission" is quite important (another thing to emphasize in the docs)), the authorization system will look for "global policies"; that being, those that don't depend on a subject model.
If ->can() is called with a subject, any policies registered to that subject's model will be applied as @Sami (SychO) mentioned (first looking for method names that match the ability name, then falling back to the can policy method). Note that some core models (Discussion, User, Post (I think that's all?)) will also implicitly expand the method name. So calling $actor->can('duplicate', $discussion) will check if the user has a 'discussion.duplicate' permission as part of the can check.

In either case, if policies return nothing, it'll return true iff the user is in a group that has a permission matching the ability name or the user is an admin.

Now, getting back to viewDiscussions. Note that it's plural: it's not designed to tell if a user can view a particular discussion, but rather whether users can generally view discussions in some part of the forum. So it can't be scoped to a particular discussion (well, it can, but that won't do anything productive), but it CAN be scoped to a tag: $actor->can('viewDiscussions', $tag), and because there is a TagPolicy implementation for this in the tags extension, that will tell whether a user can view discussions (plural) in the tag in question.