Create permissions before perform [Draft / POC]

[sergioisidoro]

⚠️ Based on #156

Attempts to resolve #118 , so far only for DRF

[c0ntradicti0n]

Hey, is there some plan to merge this one day?

[dfunckt]

In short, no, I don't think this should be merged as it's beyond the scope of Rules and it doesn't seem to be necessary to be in rules.

I'd need to refresh my memory but I don't have the time right now and I'd rather give you a quick answer first, feel free to convince me otherwise.

[c0ntradicti0n]

Thank for your quick reply, good to have a perspective on that. I'm not really getting the point, that this is out of scope.

So it's meant to be for securing django on object-level-permissions? If you can't check your objects on creation, because they are None then, for what would you need then permissions for changing or deleting or viewing the crap you eventually got?

So I found myself then some hack, because that merge request looked a bit complicated too.

I customized the AutoPermissionViewSetMixin by the following at line 71, where the object is determined:

        elif not obj:
            obj = self.model(
                **self.serializer_class().run_validation(
                    self.request.data
                )
            )

So it creates some unsaved, virtual object of the model not in the db, some object without id. That is passed to the rule-predicates then. Surprisingly it does not fail until now. Eventually nobody has put some strange saving logic in the serializers.

So I can live now with that, let the reviewers live with that too

[dfunckt]

If you can't check your objects on creation, because they are None then, for what would you need then permissions for changing or deleting or viewing the crap you eventually got?

I hear you, but I don’t see how devising a fake object on the spot is a workable solution. On creation you don’t have an object to check permissions against by definition, and that’s also how Django behaves. I think it’s beyond the scope of Rules to solve this “problem”.

[sergioisidoro]

On creation you don’t have an object to check permissions against by definition, and that’s also how Django behaves

I think that is a limitation of Django, and while I understand the out of scope argument, that is exactly why libraries exist - to extend Django functionality.

From a OOP perspective, checking permissions on create makes a lot of sense, especially when certain models include sensitive attributes (eg. creating a user with a certain role). The object per se does not exist, but all the attributes do.

But since I also don't have time to dedicate to these PRs, please feel free to close them.

I think #156 is still relevant tho, after our discussion in the Issue #155

[c0ntradicti0n]

Hm, but if we can change this behavior with Rules, then Django is here quite customizable and not so limitated and just has a default, but will adapt to it.

On creation you don’t have an object to check permissions against by definition

Not in the database, but with the body-data django will create one and so there is some.

I discovered also another problem by thinking about the shallow object for create, if that's enough or not.

Say I have a post of a forum and it's the question if I'm allowed to change/edit it by being author it it with writing my user_id in it.
Then I have to check, if the old object is actually my post (that's what rules can check), but there is also the new object, that i have to check, I should not change my post to someone else's post.
So I have to check existing vs. new data, if they both belong to me.

The case, that the user-id is in the new object, can be secured by putting that in the serializer, but think of other only user-related checks. For instance, the object has a RelatedField-connection to an object, that marks resources of a group a user belongs to.

If I do:

        # Determine whether we've to check object permissions (for detail actions)
        old_obj = None
        new_obj = None

        extra_actions = self.get_extra_actions()
        # We have to access the unbound function via __func__
        if handler.__func__ in extra_actions:
            if handler.detail:
                old_obj = self.get_object()
        elif self.action not in ("create", "list"):
            old_obj = self.get_object()

        if self.action not in ("list",):
            new_obj = self.model(
                **self.serializer_class().run_validation(
                    self.request.data
                )
            )

        # Finally, check permission
        perm = self.get_queryset().model.get_perm(perm_type)
        if not self.request.user.has_perm(perm, (old_obj, new_obj,)):
            raise PermissionDenied

I get a tuple of the old and new object to check both or compare them.

[pavelsg]

Here's another reasoning for implementing this AND a use-case example:

As long as the reasoning goes, we have object-level permissions managed by 'django-rules', so moving access-validating logic for create operation out of rules.py seems counter-intuitive and hard to maintain with the large code-base.

For the use case, here I am, trying to check whether group member can add another group member to the membership table (mapping members and groups by id), implementing predicate is_group_member(user, membership), expecting to extract group_id from membership object (not created yet, but available within Django framework / DRF).