Tag based permissions are shady

[luceos]

On the current beta 7 (must have been in there for ages though). Tag based permissions are possible by prefixing your permission with discussion.. During code execution to check whether a user has a specific permission one uses the can method, like this

$event->actor->can('discussion.startPrivateDiscussionWithUsers'); // Global permission
$event->actor->can('discussion.startPrivateDiscussionWithUsers', $event->model); // Per discussion permission check, not working
$event->actor->can('startPrivateDiscussionWithUsers', $event->model); // Per discussion permission check

---

1. Having to prefix a permission with discussion. is not very easy to understand, it's also not documented.
2. The second one doesn't work because The Tags extension DiscussionPolicy prefixes the permission again with discussion.. I've seen it mentioned in the core DiscussionPolicy and am quite confused why I need to prefix the global check and not the discussion specific check.

This issue is a question to clarify and a request to improve the logic for stable. Perhaps we can remove the need of declaring the permission in the admin.js completely by simply hitting a Permissions helper somewhere?

[tobyzerner]

Can you please explain in more detail what your desired behaviour is? I assume startPrivateDiscussionWithUsers a global permission, which, if granted, allows a user to start private discussions with other users. Where do tags come in? Why do you need a discussion-specific permission? What is $event->modal?

In general, permissions with a discussion. prefix apply to specific discussion instances, and thus they can become tag-specific as well. For example, discussion.reply allows you to reply to a discussion. Whenever you call can with a discussion model as the second argument, Flarum will check for the given ability prefixed with discussion..

The tags extension automatically adds on logic whenever you're checking a permission for a specific discussion. So if someone asks "can user reply to this discussion", tags will check for the appropriate tagX.discussion.reply permission(s).

Permissions without a prefix apply globally, rather than to a specific discussion instance. You need the startDiscussion permission in order for the "Start Discussion" button to be clickable. In this case, there is no specific discussion instance - you can either start discussions, or you can't.

So I suspect you should just be doing this:

$event->actor->can('startPrivateDiscussionWithUsers'); // global permission

Tags won't interfere with that.

This will all be documented for stable.

[tobyzerner]

Perhaps we can remove the need of declaring the permission in the admin.js completely by simply hitting a Permissions helper somewhere?

Yes we will endeavour to make this easier.

[luceos]

Okay so I had a typo, modal should have been model. The model at hand is a discussion object.

---

If you want to define a permission that can be configured in the Permissions tab for different Tags, you have to prefix it with discussion. as we both mentioned above. Not prefixing a permission like that only makes it global. That's fine for permissions that are not tag based. I'd like the admin of the forum to specify who can create private discussions in specific tags.

Checking whether someone has the global permission requires you to drop the second argument, something that confused me. If you don't it won't work. Perhaps that makes sense as the unsaved Discussion object has no relationship tags with any entries in it..

I think we concluded the same thing 😁

[tobyzerner]

Ahhh gotcha. You're right, that's an oversight. Will have a think...

[luceos]

And one other thing I mentioned. What's the idea behind prefixing discussion. in both DiscussionPolicy classes of Core and Tags? I assume you either use the full permission string or it should something like namespaces (discussion:)? That's another thing that confused me. The check that requires the second argument requires a non-prefixed permission.

[tobyzerner]

It's been ages since we made the above comments, and this is a complex issue so I apologize that I probably haven't yet fully re-grasped the details of it...

What's the idea behind prefixing discussion. in both DiscussionPolicy classes of Core and Tags?
That's another thing that confused me. The check that requires the second argument requires a non-prefixed permission.

This discussion might be a useful refresher: https://discuss.flarum.org/d/4817-difference-between-user-can-and-user-haspermission/3

So whenever you're calling hasPermission (like in the DiscussionPolicies) you're simply asking if the user has that permission (ie. are they a part of a group which has the permission in the permissions table)

Whenever you're calling can you're essentially delegating to event listeners/policies - which basically detect the type of the second argument (the model) and then call hasPermission with a prefix (or namespace, whatever you want to call it) added to the start of the "ability".

Did you get per-tag permissions working with byobu?

In the interest of making this better, can we try and summarize (with code examples if possible):

• What thing(s) are not possible with the current implementation?
• What thing(s) are possible but unintuitive in the current implementation (and how could they be better)?
• What thing(s) (if anything) are confusing at first but make sense after some time - and thus should be a focus of the documentation

[luceos]

Everything is possible with the current implementation, if you know how.

The problem I had, was how prefixing works when comparing can vs hasPermission and when adding a tag-based permission in the frontend, because it needs to be prefixed with discussion.. The fact that you have to prefix the permission based on the object you assign the permission for isn't obvious. Especially for a permission that is tag-related, I'd expect a tag. prefix or something. Only after thorough research did I find out how to add a tag-based permission in the permission grid.

I must confess I never used the policies and gates functionality of Laravel, so that's perhaps lack of knowledge that made this hard to grasp for me.

[tobyzerner]

Cool so it's mainly an issue of documentation then (and wouldn't be surprised if in the process of writing documentation I work out ways to make the whole thing more intuitive)

[luceos]

How permissions are registered most definitely needs a more intuitive approach 👍

[stale]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. We do this to keep the amount of open issues to a manageable minimum.
In any case, thanks for taking an interest in this software and contributing by opening the issue in the first place!

[BartVB]

As far as I know the above is still an issue.

Registering permissions can be simplified and documentation should be improved.

[clarkwinkelmann]

I'm just reading through this discussion now.

Personally I think the discussion. prefix makes sense. It means it's a permission that we expect to query through the gate by passing a $discussion object as the subject.

Tags is then able to use that as a hint that this permission will be called with a discussion and should be tag-scoped.

Maybe we should split the issue. One about documenting the prefix. Another one for possible ways to register permissions.

Personally I find the current solution quite good. You do need the ability to customize groups of discussions, titles, icons and such in the frontend, so that would be hard to move to the backend completely. In the backend there are some useful methods that allows a policy to fallback to the permission automatically if you don't define more logic, or you can make your own policies and define which permissions you want to check.

To me it looks like it's all documentation related. We should make the differences between policy names and permission names clear, and explain how they interact together.

Everything documentation-related is closely linked to flarum/framework#1832 . Depending how we solve that other issue this will introduce changes in how permissions work.

Do we want to keep this issue opened at all ?

[stale]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. We do this to keep the amount of open issues to a manageable minimum.
In any case, thanks for taking an interest in this software and contributing by opening the issue in the first place!

[askvortsov1]

flarum/framework#2092 should yield a solution to this

[stale]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. We do this to keep the amount of open issues to a manageable minimum.
In any case, thanks for taking an interest in this software and contributing by opening the issue in the first place!

[BartVB]

Still relevant.

[stale]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. We do this to keep the amount of open issues to a manageable minimum.
In any case, thanks for taking an interest in this software and contributing by opening the issue in the first place!