Modify permission operands to use custom messages

[last-partizan]

Refs #6427

This is continuation of #6499 #6502

This PR fixes error where permissions were missing .message property after applying & or |.

I'll mark this as a draft, and add the same handling for .code property. And maybe see if we can somehow refactor/optimize/make_better.

And I will add documentation. Probably, the only point worth mentioning is the new message_inverted property, because other change is just a bugfix, and everyone already expects that .message and .code will not be lost when using combined permission.

[last-partizan]

I started working on getting this into proper shape, and I'm having trouble with this part.

And I need some help

Imagine a case:

class IsWhitelistedPath(BasePermission):
    message = "Path not whitelisted"
    ...

...
permission_classess = [IsAuthenticated | IsWhitelistedPath]

Here, in case of error, message will be "Path not whitelisted", completely ignoring the first one. This is wrong. And we do not have message available at the init time, because "AND" (and some of my permission classes) are setting self.message dynamically in has_permission call.

I come up with something like this:

class OR(OperatorBase):

    def has_permission(self, request, view):
        collector = ResultCollector()
        for perm in self._permissions:
            if perm.has_permission(request, view):
                return True
            else:
                collector.add_message_and_code(perm)
        collector.finalize(self)
        return False
   ...

class ResultCollector:
    def __init__(self):
        self.messages = ()
        self.codes = ()

    def add_message_and_code(self, perm):
        message = getattr(perm, 'message', None)
        code = getattr(perm, 'code', None)
        self.messages += (message,)
        self.codes += (code,)

    def finalize(self, perm):
        perm.message = self.messages
        perm.code = self.codes

Each failed permission adds it's message to a list, and we can handle this later, to produce a list of errors.

In this example, that will be:

[
    ErrorDetail('You do not have permission to perform this action.', 'permission_denied'),
    ErrorDetail('Path not whitelisted.'),
]

Maybe we should wrap it into additional container, something like {"detail": "Any of the listed perms required", "errors": [{"message": ..., "code": ...}]`... to make it clearer...

What do you think? @auvipy @tomchristie @samitnuk

[auvipy]

I would suggest make the CI green first

[last-partizan]

Done.

And, now it includes my proposed changes.

[tomchristie]

What do you think?

I would suggest undocumenting operands for permissions, and informally deprecating them with comments in the codebase.

Ie. No enforced behavioral change, but improved doc guidance.

[last-partizan]

What do you think?

I would suggest undocumenting operands for permissions, and informally deprecating them with comments in the codebase.

Ie. No enforced behavioral change, but improved doc guidance.

You mean, removing a section from docs mentioning bitwise operators?

https://www.django-rest-framework.org/api-guide/permissions/#permissions

Provided they inherit from rest_framework.permissions.BasePermission, permissions can be composed using standard Python bitwise operators. For example, IsAuthenticatedOrReadOnly could be written:
...
permission_classes = [IsAuthenticated|ReadOnly]

@tomchristie Maybe I misunderstood something, but this does not look like "improved doc guidance".

Operands are working fine, and the only minor problem that resulting errors are missing message and code, and it can be fixed.

[anehx]

Is there a reason why this was closed? I'd really like to have a solution for this use case as it is not that uncommon.

[last-partizan]

As it stands REST framework will not be accepting further contributions.

#9560 (comment)