[Django] #36837: Client.force_login won't work for permission-only backends inheriting from BaseBackend

[Django]

#36837: Client.force_login won't work for permission-only backends inheriting from
BaseBackend
-------------------------------------+-------------------------------------
Reporter: Christian Hartung | Type:
| Cleanup/optimization
Status: new | Component:
| contrib.auth
Version: 6.0 | Severity: Normal
Keywords: | Triage Stage:
| Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
This is a follow up to #27542.

Take the `MagicAdminBackend` from the
[https://docs.djangoproject.com/en/6.0/topics/auth/customizing/#handling-
authorization-in-custom-backends 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`.
--
Ticket URL: <https://code.djangoproject.com/ticket/36837>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.

[Django]

#36837: Client.force_login won't work for permission-only backends inheriting from
BaseBackend
-------------------------------------+-------------------------------------

Reporter: Christian Hartung | Owner: (none)
Type: | Status: new
Cleanup/optimization |
Component: contrib.auth | Version: 6.0
Severity: Normal | Resolution:
Keywords: | Triage Stage:
| Unreviewed
Has patch: 1 | Needs documentation: 0

Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Changes (by VIZZARD-X):

* has_patch: 0 => 1

--
Ticket URL: <https://code.djangoproject.com/ticket/36837#comment:1>

[Django]

#36837: Client.force_login won't work for permission-only backends inheriting from
BaseBackend
-------------------------------------+-------------------------------------
Reporter: Christian Hartung | Owner: (none)
Type: | Status: new
Cleanup/optimization |
Component: contrib.auth | Version: 6.0
Severity: Normal | Resolution:
Keywords: | Triage Stage:
| Unreviewed
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Comment (by 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`.
--
Ticket URL: <https://code.djangoproject.com/ticket/36837#comment:2>

[Django]

#36837: Client.force_login won't work for permission-only backends inheriting from
BaseBackend
-------------------------------------+-------------------------------------
Reporter: Christian Hartung | Owner: (none)

Type: New feature | Status: closed

Component: contrib.auth | Version: 6.0
Severity: Normal | Resolution:

| needsnewfeatureprocess
Keywords: | Triage Stage:
| Unreviewed
Has patch: 0 | Needs documentation: 0

Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Changes (by Jacob Walls):

* has_patch: 1 => 0
* resolution: => needsnewfeatureprocess
* status: new => closed
* type: Cleanup/optimization => New feature

Comment:

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
[https://docs.djangoproject.com/en/6.0/topics/serialization/#dependencies-
during-serialization 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:

{{{#!py
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 [https://github.com/django/new-features
new-features] discussion. Thanks.
--
Ticket URL: <https://code.djangoproject.com/ticket/36837#comment:3>

[Django]

#36837: Client.force_login won't work for permission-only backends inheriting from
BaseBackend
-------------------------------------+-------------------------------------
Reporter: Christian Hartung | Owner: (none)
Type: New feature | Status: closed
Component: contrib.auth | Version: 6.0
Severity: Normal | Resolution:
| needsnewfeatureprocess
Keywords: | Triage Stage:
| Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Comment (by Natgho):

Replying to [comment:3 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 [https://github.com/django/new-
features 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.
--
Ticket URL: <https://code.djangoproject.com/ticket/36837#comment:4>

[Django]

#36837: Client.force_login won't work for permission-only backends inheriting from
BaseBackend
-------------------------------------+-------------------------------------
Reporter: Christian Hartung | Owner: (none)
Type: New feature | Status: closed
Component: contrib.auth | Version: 6.0
Severity: Normal | Resolution:
| needsnewfeatureprocess

Keywords: | Triage Stage: Accepted

Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Changes (by Jacob Walls):

* stage: Unreviewed => Accepted

Comment:

Accepting based off new-features discussion and confirmation django-
guardian would use this. A PR is welcome.
--
Ticket URL: <https://code.djangoproject.com/ticket/36837#comment:5>

[Django]

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

-----------------------------------+------------------------------------

Reporter: Christian Hartung | Owner: (none)

Type: New feature | Status: new

Component: contrib.auth | Version: 6.0
Severity: Normal | Resolution:

Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0

-----------------------------------+------------------------------------
Changes (by Jacob Walls):

* resolution: needsnewfeatureprocess =>
* status: closed => new

--
Ticket URL: <https://code.djangoproject.com/ticket/36837#comment:6>

[Django]

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

-----------------------------------+---------------------------------------
Reporter: Christian Hartung | Owner: SnippyCodes
Type: New feature | Status: assigned

Component: contrib.auth | Version: 6.0
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted

Has patch: 1 | Needs documentation: 0

Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0

-----------------------------------+---------------------------------------
Changes (by SnippyCodes):

* has_patch: 0 => 1

* owner: (none) => SnippyCodes
* status: new => assigned

--
Ticket URL: <https://code.djangoproject.com/ticket/36837#comment:7>

[Django]

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

-----------------------------------+------------------------------------
Reporter: Christian Hartung | Owner: (none)
Type: New feature | Status: new

Component: contrib.auth | Version: 6.0
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-----------------------------------+------------------------------------

Changes (by SnippyCodes):

* owner: SnippyCodes => (none)
* status: assigned => new

--
Ticket URL: <https://code.djangoproject.com/ticket/36837#comment:8>

[Django]

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

-------------------------------------+-------------------------------------
Reporter: Christian Hartung | Owner: Sezer
| Bozkır

Type: New feature | Status: assigned

Component: contrib.auth | Version: 6.0
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0

-------------------------------------+-------------------------------------
Changes (by Jacob Walls):

* owner: (none) => Sezer Bozkır

* status: new => assigned

Comment:

[https://github.com/django/django/pull/21065 PR]
--
Ticket URL: <https://code.djangoproject.com/ticket/36837#comment:9>