Checking of object-permissions for TreeItem

[go-run-jump]

I really like the functionality of this library and would like to use it in a projekt together with django-guardian for object-permissions. This enables me to give a specific user or group the permission to view a TreeItem.

At the moment the check for permissions in line 893 of sitetreeapp.py can't do that as it is not model-specific: user_perms = set(context['user'].get_all_permissions())

If I add the line below (new syntax for brevity) it works as I want it to. user_perms.add(f"{item._meta.app_label}.{context['user'].get_all_permissions(item)}")

Is there another way to achieve that properly?

Would you be willing to accept a pull-request with such an enhancement?

[idlesign]

Let's think it over.

user_perms.add(f"{item._meta.app_label}.{context['user'].get_all_permissions(item)}")

I wonder how that works, since get_all_permissions(item) returns a set and your resulting permission name should boil into something like myapp.{'two', 'one'}. Does that work with guardian, shouldn't there be two separate permissions, like myapp.two and myapp.one?

[go-run-jump]

You're absolutly right. It should be more like

obj_perms = context['user'].get_all_permissions(item)
labeled_obj_perms = {f"{item._meta.app_label}.{perm}" for perm in obj_perms}
user_perms.update(labeled_obj_perms)

[idlesign]

We could prepend app label as an option (through settings), but before that I'd like to make sure it solves the problem.

So I'd like to ask you to make a fork of this repo and add guardian into demo, so that I could see how your solution works. If it works fine, we could merge app code changes (excluding demo).

[idlesign]

I've checked the demo, thank you.

In #265 I introduce a new better support for customization. It should cover a wide variety of use cases including yours. Please give it a try, documentation is updated accordingly. Will wait for you feedback.

[go-run-jump]

get_permissions will only be called once in check_access because of if user_perms is _UNSET. This means checking per item will not work. I guess this is for unneccessary double checks if the permissions are model-based. Of course now it is possible to overwrite check_access, which is great, but at least this method doesn't do what it promises.

What are your thoughts about this?

[idlesign]

It seems that we need a caching dict in _current_user_permissions for per-item access checking, but that should be weighted against caching removal.