fizista / django-password-validators

Additional libraries for validating passwords in Django.
BSD 3-Clause "New" or "Revised" License
51 stars 31 forks source link

PasswordHistory Race Condition IntegrityError #28

Closed stormsurehire closed 1 year ago

stormsurehire commented 2 years ago

Hi there! This project has been great in helping us enforce higher password standards for our users, thank you.

I've noticed an issue that has come up for us every now and then, and I think it's due to a race condition when PasswordHistory is being checked and created.

The code in question is in file django_password_validators/password_history/password_validation.py, lines 102-111 in method password_changed(). It looks like this:

        try:
            PasswordHistory.objects.get(
                user_config=user_config,
                password=password_hash
            )
        except PasswordHistory.DoesNotExist:
            ols_password = PasswordHistory()
            ols_password.user_config = user_config
            ols_password.password = password_hash
            ols_password.save()

An IntegrityError comes up when there are two different threads running this code, and both try to save the old_password. The second thread that tries to save old_password will error with the IntegrityError as the unique user_config and password_hash already exist. The django docs have a good example of what is happening here when looking at the .get_or_create() method: https://docs.djangoproject.com/en/dev/ref/models/querysets/#get-or-create

I think that this should be fixed to use .get_or_create() if possible or use an atomic transaction block.

I can make a PR if needed.

fizista commented 1 year ago

Thank you. There is already a new version with suggested fixes.