pytest-django
pytest-django copied to clipboard
admin_client / admin_user fixtures are dependent on AUTHENTICATION_BACKEND order
I just upgraded to pytest-django 4.1.0 from 3.10.0 and a lot of my tests broke which rely on admin_client / admin_user.
It turns out that the code in https://github.com/pytest-dev/pytest-django/commit/79b7754669660543b593bdad471e73a9dabc04ed uses the first backend that has get_user
, which may return None
.
In my case I have base settings in a file settings_global.py
and recommend to import that from the settings.py
in production via:
from .settings_global import * # noqa
and modify settings after that line.
settings_global.py
contains:
AUTHENTICATION_BACKENDS = (
'django.contrib.auth.backends.RemoteUserBackend',
'guardian.backends.ObjectPermissionBackend',
)
Using RemoteUserBackend is recommended for production (using kerberos, AD, x509 client certs or whatever the apache authentication mechanism is deemed appropriate for the production site).
So for settings_test.py
I have:
AUTHENTICATION_BACKENDS += ( # noqa
'django.contrib.auth.backends.ModelBackend', # this is default'
)
(Of course it's explicitly discouraged to use ModelBackend for production.
This results in RemoteUserBackend
being used first for tests, which are supposed to use ModelBackend.
The test failures are caused by user being None, which the views automatically convert to the Guardian AnonymousUser instance, which of course has no permissions.
Of course the simple fix is:
AUTHENTICATION_BACKENDS = ( # noqa
'django.contrib.auth.backends.ModelBackend', # this is default'
) + AUTHENTICATION_BACKENDS # noqa
So ModelBackend is used first for tests.
I'm not sure if that's intended, as the tests worked with 3.10.0.
At least I'd suggest that this should be documented.
I forgot to mention that the apiclient
from django-rest-framework also has a method force_authenticate
which doesn't behave like that, so it might be worth looking at their implementation.
So IIUC, login
calls the regular authenticate
function which iterates over all AUTHENTICATION_BACKENDS
until one returns a user or PermissionDenied
, and in your case it would reach the ModelBackend
at the end and succeed. Also login
fails silently (returns False
which we don't check).
But force_login
(without an explicit backend
argument) just uses the first AUTHENTICATION_BACKEND
for some reason. But I don't quite follow how it actually fails. From what I can see the request.user
would still be the admin user. I don't quite follow the Guardian AnonymousUser user bit, can you explain further?
Of course the simple fix is:
Seems to me like it's the right thing to do, regardless of the pytest-django change - saves the tests the two unsuccessful backend iterations.
I forgot to mention that the apiclient from django-rest-framework also has a method force_authenticate which doesn't behave like that, so it might be worth looking at their implementation.
They seemed to have achieved it through a (IMO) ugly hack - they set a private field on the request and the actual non-test code special cases it -- eek.
Sorry for the late reply.
So IIUC,
login
calls the regularauthenticate
function which iterates over allAUTHENTICATION_BACKENDS
until one returns a user orPermissionDenied
, and in your case it would reach theModelBackend
at the end and succeed. Alsologin
fails silently (returnsFalse
which we don't check).
I didn't really look into that.
But
force_login
(without an explicitbackend
argument) just uses the firstAUTHENTICATION_BACKEND
for some reason. But I don't quite follow how it actually fails.
Well, this is the code from django 2.2.17:
def force_login(self, user, backend=None):
def get_backend():
from django.contrib.auth import load_backend
for backend_path in settings.AUTHENTICATION_BACKENDS:
backend = load_backend(backend_path)
if hasattr(backend, 'get_user'):
return backend_path
if backend is None:
backend = get_backend()
user.backend = backend
self._login(user, backend)
The inner function get_backend
will return the first backend that has get_user
and use that. In the case of my broken configuration that would be RemoteUserBackend
. That is the only backend being tried by login in that case.
The result is None
.
I don't know how admin_client was logged in before.
From what I can see the
request.user
would still be the admin user. I don't quite follow the Guardian AnonymousUser user bit, can you explain further?
See the two blue boxes in https://django-guardian.readthedocs.io/en/stable/configuration.html#configuration but I really don't think this is relevant, since the result from django's force_login is None
unexpectedly.
Of course the simple fix is:
Seems to me like it's the right thing to do, regardless of the pytest-django change - saves the tests the two unsuccessful backend iterations.
Probably yes. Note that there are no backend iterations. The first backend is tried only.
I forgot to mention that the apiclient from django-rest-framework also has a method force_authenticate which doesn't behave like that, so it might be worth looking at their implementation.
They seemed to have achieved it through a (IMO) ugly hack - they set a private field on the request and the actual non-test code special cases it -- eek.
I'd agree that special casing the non test code is indeed calling for trouble. On the other hand I don't mind slapping attributes on objects, but probably I've learned that bad habit from DRF (they do it in quite a lot of places).
So I've run into the same issue. It seems that my test project settings like so leads to many 302 login redirects when trying to use the admin_client to access admin URLs:
AUTHENTICATION_BACKENDS = [
"ariadne_jwt.backends.JSONWebTokenBackend",
"django.contrib.auth.backends.ModelBackend",
]
Changing the order to be like the following allowed the tests to start passing again:
AUTHENTICATION_BACKENDS = [
"django.contrib.auth.backends.ModelBackend",
"ariadne_jwt.backends.JSONWebTokenBackend",
]
The ariadne-jwt backend is here: https://github.com/Usama0121/ariadne-jwt/blob/master/ariadne_jwt/backends.py
I think the solution would be to modify the admin_client
fixture so that users could specify which backend to use, but this would unfortunately be a breaking change as it would change the way the fixture has to work. Below is an example of how the fixture would have to change and how the usage would change:
# fixtures.py
@pytest.fixture()
def admin_client(
db: None,
admin_user,
) -> "django.test.client.Client":
"""A Django test client logged in as an admin user."""
def _inner(backend=None):
from django.test.client import Client
client = Client()
client.force_login(admin_user, backend=backend)
return client
return _inner
# my_tests.py
def test_foo(admin_client):
client = admin_client(backend=my_cool_backend)
response = admin_client.get('/admin/')
assert response.status_code == 200
There may be some other solution that is not a breaking change, however.