User.has_perm should forward **kwargs to allow more flexibility in authentication backends

[Matteo Parrucci]

The situation
I would like to check user permission based on the django.contrib.sites I'm in. To get the active site, django.contrib.sites needs the request so he can match the hostname. I'm using django-rules at the moment but It's quite generic.

The Problem
Not having the request in has_perm/has_perms I cannot check permissions based on the currently active site
The "call stack" detail is here: ​https://github.com/dfunckt/django-rules/issues/130

A possible solution
Adding **kwargs to has_perm and has_perms signatures would allow greater flexibility for the custom authentication backends leaving intact what we have now. In my case, for example, I could check permission in my django-rules predicates having the the request passed in and check if I have rights for the current site, but I think my case is quite limited and all new authentication backends would benefit from this change.

[Matteo Parrucci]

I've implemented it in a demo project and it now looks like this:

Into django.contrib.auth.models.AbstractUser

    def has_perm(self, perm, obj=None, **kwargs):
        return _user_has_perm(self, perm, obj=obj, **kwargs)

    def has_perms(self, perm_list, obj=None, **kwargs):
        return all(self.has_perm(perm, obj, **kwargs) for perm in perm_list)

In the same module the checking permission function:

def _user_has_perm(user, perm, obj, **kwargs):
    """
    A backend can raise `PermissionDenied` to short-circuit permission checking.
    """
    for backend in get_backends():
        if not hasattr(backend, 'has_perm'):
            continue
        try:
            if backend.has_perm(user, perm, obj, **kwargs):
                return True
        except PermissionDenied:
            return False
    return False

Into django.contrib.auth.backends.* change the has_perm signature to the following leaving the implementation as is.

def has_perm(self, user_obj, perm, obj=None, **kwargs):

And it works for me now. It is a small change and I think it could be useful for others too. Basically I only added kwargs to the signatures and forwarded it to the auth backends.

[Carlton Gibson]

Hey Matteo — could you put the proposed change into a PR so we can see the diff? Essentially, if it's not a breaking change then OK, otherwise it's difficult — much easier to see that in a PR. Thanks.

Seems reasonable from the description. I'll provisionally Accept on that basis. (Can't immediately see how adding **kwargs would be breaking change so …)

[Matteo Parrucci]

Here it is the pull request:

​https://github.com/django/django/pull/13785

It is somehow a breaking change in the meaning that all auth backends should change and also accept a **kwargs from now on.
I'm open to suggestions to avoid this.

[Florian Apolloner]

Adding **kwargs to has_perm and has_perms signatures would allow greater flexibility for the custom authentication backends leaving intact what we have now.

But how would a 3rd party application that uses has_perm know which **kwargs to pass in? This is all nice and well if you control the code, but as soon as 3rd party apps (like the admin, even though there you can probably manually override the check) come into play you no longer get anything passed. What would you fall back to then?

What we really need would be to pass in some context (or make available) that can be populate from elsewhere (maybe already from a middleware). But to pass that down to the auth backends we would most likely need to use thread locals :/

[Matteo Parrucci]

Replying to Florian Apolloner:

But how would a 3rd party application that uses has_perm know which **kwargs to pass in? This is all nice and well if you control the code, but as soon as 3rd party apps (like the admin, even though there you can probably manually override the check) come into play you no longer get anything passed. What would you fall back to then?

Ok, you are right on this, I was thinking it with a project-wide scope but thinking it in the reusable apps scope it makes no sense.

What we really need would be to pass in some context (or make available) that can be populate from elsewhere (maybe already from a middleware). But to pass that down to the auth backends we would most likely need to use thread locals :/

And what if there are no more kwargs and has_perm gets and forwards to the backends simply the request as context?

• It would be more than enough to check the active site
• Could be populated through middlewares in case we need something else.
• In the admin and in the views that uses permissionmixin we always have the request to pass to has_perm

[Florian Apolloner]

And what if there are no more kwargs and has_perm gets and forwards to the backends simply the request as context?

That would work, but would add strong coupling to the request cycle, which is imo rather ugly. And yes, we already have it for authenticate but I still don't like it -- although it might be the better option in that regard :/ And even if we were to do that, we'd need to adjust ​https://github.com/django/django/blob/master/django/contrib/auth/decorators.py otherwise you'd probably get the request passed in 10% of the cases.

[Carlton Gibson]

Given the difficulties here (and on the PR) let's move this to wontfix — pending an acceptable solution, it doesn't look feasible. Thanks all.