Cannot enter Django admin interface when model instances are validated on save

[Evgeny Arshinov]

Hello,

In #29549 I was advised to enable model validation on save to properly validate choice fields, like this:

@receiver(pre_save)
def pre_save_handler(sender, instance, *args, **kwargs):
    instance.full_clean()

Turned out this approach breaks Django session's middleware and, as a result, no one can enter Django admin interface (which uses the session middleware).

Scenario:

1. Use a project that includes Django admin interface.
2. Enable model validation:

from django.db.models.signals import pre_save
from django.dispatch import receiver

@receiver(pre_save)
def pre_save_handler(sender, instance, *args, **kwargs):
    instance.full_clean()

3. Visit Django admin interface start page (/admin).
4. Enter valid credentials.

The result is error 500 due to a ValidationError:

Internal Server Error: /admin/login/
Traceback (most recent call last):
  File "C:\django_sample\venv\lib\site-packages\django\core\handlers\exception.py", line 34, in inner
    response = get_response(request)
  File "C:\django_sample\venv\lib\site-packages\django\utils\deprecation.py", line 93, in __call__
    response = self.process_response(request, response)
  File "C:\django_sample\venv\lib\site-packages\django\contrib\sessions\middleware.py", line 58, in process_response
    request.session.save()
  File "C:\django_sample\venv\lib\site-packages\django\contrib\sessions\backends\db.py", line 87, in save
    obj.save(force_insert=must_create, force_update=not must_create, using=using)
  File "C:\django_sample\venv\lib\site-packages\django\db\models\base.py", line 717, in save
    force_update=force_update, update_fields=update_fields)
  File "C:\django_sample\venv\lib\site-packages\django\db\models\base.py", line 742, in save_base
    update_fields=update_fields,
  File "C:\django_sample\venv\lib\site-packages\django\dispatch\dispatcher.py", line 175, in send
    for receiver in self._live_receivers(sender)
  File "C:\django_sample\venv\lib\site-packages\django\dispatch\dispatcher.py", line 175, in <listcomp>
    for receiver in self._live_receivers(sender)
  File "C:\django_sample\django_sample\models.py", line 41, in pre_save_handler
    instance.full_clean()
  File "C:\django_sample\venv\lib\site-packages\django\db\models\base.py", line 1151, in full_clean
    raise ValidationError(errors)
django.core.exceptions.ValidationError: {'session_key': ['Session with this Session key already exists.']}

We dug this problem a bit, and found out that during the login procedure saving a django.contrib.sessions.Session instance is performed twice with the same session key:

• In the first case, the object is saved with force_insert=True, force_update=False
• In the second case, the object is saved with force_insert=False, force_update=True
• Hovewer, in both cases the object's _state.adding flag is True, which causes django.db.Model._perform_unique_checks to include the value of the unique field of the object being saved (in this case, a session key) in unique validation. Since there is already an object with the given session key in the database after the first save, validate_unique reports an error.

Traceback for the first save operation (some bottom frames omitted for brevity):

  ...
  File "C:\django_sample\venv\lib\site-packages\django\contrib\auth\views.py", line 90, in form_valid
    auth_login(self.request, form.get_user())
  File "C:\django_sample\venv\lib\site-packages\django\contrib\auth\__init__.py", line 108, in login
    request.session.cycle_key()
  File "C:\django_sample\venv\lib\site-packages\django\contrib\sessions\backends\base.py", line 298, in cycle_key
    self.create()
  File "C:\django_sample\venv\lib\site-packages\django\contrib\sessions\backends\db.py", line 55, in create
    self.save(must_create=True)
  File "C:\django_sample\venv\lib\site-packages\django\contrib\sessions\backends\db.py", line 87, in save
    obj.save(force_insert=must_create, force_update=not must_create, using=using)
  File "C:\django_sample\venv\lib\site-packages\django\db\models\base.py", line 717, in save
    force_update=force_update, update_fields=update_fields)
  File "C:\django_sample\venv\lib\site-packages\django\db\models\base.py", line 742, in save_base
    update_fields=update_fields,
  File "C:\django_sample\venv\lib\site-packages\django\dispatch\dispatcher.py", line 175, in send
    for receiver in self._live_receivers(sender)
  File "C:\django_sample\venv\lib\site-packages\django\dispatch\dispatcher.py", line 175, in <listcomp>
    for receiver in self._live_receivers(sender)
  File "C:\django_sample\django_sample\models.py", line 41, in pre_save_handler
    instance.full_clean()

Traceback for the second save operation:

  ...
  File "C:\django_sample\venv\lib\site-packages\django\contrib\sessions\middleware.py", line 58, in process_response
    request.session.save()
  File "C:\django_sample\venv\lib\site-packages\django\contrib\sessions\backends\db.py", line 87, in save
    obj.save(force_insert=must_create, force_update=not must_create, using=using)
  File "C:\django_sample\venv\lib\site-packages\django\db\models\base.py", line 717, in save
    force_update=force_update, update_fields=update_fields)
  File "C:\django_sample\venv\lib\site-packages\django\db\models\base.py", line 742, in save_base
    update_fields=update_fields,
  File "C:\django_sample\venv\lib\site-packages\django\dispatch\dispatcher.py", line 175, in send
    for receiver in self._live_receivers(sender)
  File "C:\django_sample\venv\lib\site-packages\django\dispatch\dispatcher.py", line 175, in <listcomp>
    for receiver in self._live_receivers(sender)
  File "C:\django_sample\django_sample\models.py", line 41, in pre_save_handler
    instance.full_clean()

[Evgeny Arshinov]

I have committed a demo project:

​https://github.com/earshinov/django_sample/tree/issue-29655

For reproducing:

git clone https://github.com/earshinov/django_sample.git
cd django_sample
git checkout issue-29655
python3 -m venv venv
. ./venv/bin/activate
pip install -r requirements.txt
python manage.py migrate
python manage.py createsuperuser
# enter credentials to your liking
python manage.py runserver 127.0.0.1:9726

Next, visit ​http://localhost:9726/admin/, enter the credentials and click the [Log in] button.

[Evgeny Arshinov]

As a temporary solution in our project we excluded instances of django.contrib.sessions.Session objects from validation:

@receiver(pre_save)
def pre_save_handler(sender, instance, *args, **kwargs):
    if not isinstance(instance, Session):
        instance.full_clean()
    pass

but it certainly is an ugly workaround.

[Simon Charette]

Enabling model validation on save is likely to break a few things and degrade performance significantly. I don't think that's a pattern we should encourage or support at this point.

Django and third-party applications simply don't expect ValidationError to be thrown on save(), this effectively change the signature of the function.

[Evgeny Arshinov]

Reopened #29549.