Client.force_login won't work for permission-only backends inheriting from BaseBackend

[Christian Hartung]

This is a follow up to #27542.

Take the MagicAdminBackend from the ​documentation. It is an Authentication Backend that inherits from BaseBackend and overrides the has_perm method. This leaves it with the default get_user method returning None, making it eligible for Client.force_login.

Some libraries like django-rules and django-guardian work around this by not inheriting from BaseBackend, and not adding a get_user method, but:

• this goes against the documentation, which states that both get_user and authenticate are required, and permission related methods are optional
• now they don't automatically gain the async functionality, making it harder for users to integrate those libraries in an async context

I don't know a good way around this. Some ideas are:

Add some introspection capabilities to BaseBackend
I was think something like:

class BaseBackend:
    supports_authentication = True
    supports_permissions = True

class ObjectPermissionBackend(BaseBackend):
    supports_authentication = False
    supports_permissions = True

This way we could change (a)authenticate and force_login to skip backends that do not support autentication, and (a)has_perm to skip backends that do not support permissions.

def _get_compatible_backends(request, **credentials):
    for backend, backend_path in _get_backends(return_tuples=True):
        if not getattr(backend, "supports_authentication", True):
            continue

        ... Check signature and so on

def _user_has_perm(user, perm, obj):
    for backend in auth.get_backends():
        if not getattr(backend, "supports_permissions", True):
            continue

        if not hasattr(backend, "has_perm"):
            continue
        
        ... Check permission

Call get_user inside force_login
Use the first backend that returns something. This was suggested on #27542, but it could be really slow and might introduce some side effects

Updating the documentation
Simply update the documentation stating that:

• get_user required only if the backend can authenticate users (authenticate returns something)
• Client.force_login selects the first backend that has a get_user.

[VIZZARD-X]

I have opened a Pull Request with the fix and regression tests:
​https://github.com/django/django/pull/20485

• The fix ensures Client.force_login and AsyncClient.aforce_login verify that the selected backend can actually retrieve a user (via get_user) before using it. This resolves the crash/incompatibility with permission-only backends like django-rules.

[Jacob Walls]

Another option is to statically declare that get_user() returns None without having to call it to find out. Not so ordinary, but we have a precedent in Django of doing this, see the ​doc'd way to declare a dependency on the natural_key method.

Then with a small-scoped change in force_login(), a rules backend could do:

get_user.noop = True

It's a lot like your option 1, but it seems like a smaller lift.

I don't think the linked PR (which calls get_user()) is acceptable, for the reason you articulate (introduces db queries, defeating the point of the test utility).

A tiny hint for the test client seems like a not disruptive change for the auth backends. It is new API, though, so we need a new features discussion to make sure we're not missing anything. It would also be nice to know if django-rules and django-guardian are willing to use the feature we build for them here.

My recommendation is to kick off a ​new-features discussion. Thanks.

[Sezer Bozkır]

Replying to Jacob Walls:

A tiny hint for the test client seems like a not disruptive change for the auth backends. It is new API, though, so we need a new features discussion to make sure we're not missing anything. It would also be nice to know if django-rules and django-guardian are willing to use the feature we build for them here.
...
My recommendation is to kick off a ​new-features discussion. Thanks.

VIZZARD-X brought this ticket to our attention, and we’ve taken it into consideration. There is indeed an issue here regarding future compatibility, and he is putting in a serious effort to address it;

​https://github.com/django-guardian/django-guardian/pull/948

I believe the second “tricky” solution you mentioned—adding a “noop” field and running tests through that field—would be an improvement that wouldn’t harm the core Django structure and would also easily ensure the tests run successfully.

Speaking as a maintainer of the Django-Guardian team, if this small patch is accepted, we would be happy to use it and incorporate this feature into the project. Because the concept of asynchronous processing is a valuable feature we also need to adapt to. I’d like to note that we’re eager to use this feature if it’s added, to ensure your tests can be written and run successfully.