User permissions: define as callables/closures

[steirico]

Plugins allow to define custom user blueprints for adding user roles and specific permissions. Currently, permissions are limit to simple boolean values:

'blueprints' => [
    'users/my-role' => [
        'name'    => 'my-role',
        'permissions' => [
            'site' => [
                'changeTitle' => true, 
                'update' => false
            ]
        ]
    ]
]

In order to implement more versatile, user and role-based permissions, it would be very convenient to be able to use callables / closures beside the boolean values:

'blueprints' => [
    'users/my-role' => [
        'name'    => 'my-role',
        'permissions' => [
            'site' => [
                'changeTitle' => function($page, $user){
                     var $granted = false;
                     // implement me
                     return $granted;
                 }, 
                'update' => MyClass::canUpdate,
            ]
        ]
    ]
]

As indicated, the current page and user instance shall be passed to the callable / closure.

[lukasbestle]

We actually had that in Kirby 2 and it caused a lot of problems when the behavior of the callback wasn't consistent/used rules that depended on changing data. That's why we reduced the complexity of permissions in Kirby 3.

I'm still leaving this idea issue open, but I can't promise that we will be able to implement this.

[steirico]

@lukasbestle No problem, I'm willing to contribute 😉: https://github.com/steirico/kirby/commits/feature/user-permissions-callable

[lukasbestle]

Implementing the feature is not the issue – as I said, we already had it in Kirby 2. The issue is that the feature will be unreliable.

[lukasbestle]

Idea by @texnixe: Support for queries in permissions:

options:
  read: "{{ page.authorIsCurrentUser }}"

It will also have some reliability issues, but isn't as complex as a fully custom callback.

[steirico]

Using query language instead of callables / closures is absolutely fine. I would vote for query language since among others it allows to use model methods, isn't it?

But keep in mind: @texnixe's idea is target towards defining permission in page, file or site blueprints. In contrast, my approach is to define permissions on role's user blueprint.

Since Kirby allows to define permissions in these different blueprint types, the new possibilities should apply to all them.

[lukasbestle]

since among others it allows to use model methods, isn't it?

Yes, it would!

@texnixe's idea is target towards defining permission in page, file or site blueprints. In contrast, my approach is to define permissions on role's user blueprint.

The same syntax could be implemented for role blueprints as well. I don't see why this is limited to page/file/site blueprints. Access to the relevant variables would be provided to the queries no matter where they are defined.

Now that I think about it though, the syntax would need to look like this:

options:
  read: "page.authorIsCurrentUser"

as the return value of the query result needs to be boolean and not a template string.

[texnixe]

I actually think the fact that you would be able to use model methods in query language makes this approach much more versatile. So let's hope this gets implemented asap.

[steirico]

I agree!

As I said, I'm absolutely willing to contribute, since I'm faced with a concrete requirement.

Could you provide any hints on which class implements the query string processing?

[distantnative]

So let's hope this gets implemented asap.

Implementing the query on the backend side is not that complicated. The bigger issue is that the Panel hasn't really been set out for changing permissions, as far as I am aware. It currently assumes stable permissions and doesn't re-check them as often as needed once introducing queries.

[texnixe]

For the life of me, I can't find where this would have to be implemented.

But a side effect of this quest was that I found we can override the isReadable() method in a page model with all sorts of conditions, so that it seems effectively possible to lock pages a user is not supposed to see, based on content in a field. I don't know how hacky this approach is and what could be possible side effects, but seems to work.

[steirico]

The bigger issue is that the Panel hasn't really been set out for changing permissions, as far as I am aware. It currently assumes stable permissions and doesn't re-check them as often as needed once introducing queries.

Get me right if I am wrong. But as far as I'm familiar with Kirby's code base and from experience while implementing the idea presented in this issue and implementing a POC for an ACL plugin I don't see any problem here:

• While the panel requests data for rendering the front end of a given page, the backend checks all related permissions. Accordingly, the backend provides information if e.g. the "page add" button shall be enabled or disabled. The panel then interprets these information accordingly.
• When the panel performs POST requests e.g. for adding a new page, the backend checks all related permissions as well.

IMHO, all critical operations respect/check permissions.

Sure, in the example above the panel would not update the "page add" button if permission change unless the current view is reloaded. But honestly, if using hooks for permission checks, one gets absolutely no indication in the panel, if an operation is available or not.