aaronn / django-rest-framework-passwordless

Passwordless Auth for Django REST Framework
MIT License
708 stars 153 forks source link

Unique constraint violation on CallbackToken #77

Open poco-loco-athul opened 3 years ago

poco-loco-athul commented 3 years ago

Trace back:

[ERROR   ] Internal Server Error: /auth/mobile/
Traceback (most recent call last):
  File "/home/ubuntu/.virtualenvs/project/lib/python3.6/site-packages/django/db/backends/utils.py", line 86, in _execute
    return self.cursor.execute(sql, params)
  File "/home/ubuntu/.virtualenvs/project/lib/python3.6/site-packages/newrelic/hooks/database_psycopg2.py", line 65, in execute
    **kwargs)
  File "/home/ubuntu/.virtualenvs/project/lib/python3.6/site-packages/newrelic/hooks/database_dbapi2.py", line 39, in execute
    *args, **kwargs)
psycopg2.errors.UniqueViolation: duplicate key value violates unique constraint "drfpasswordless_callbacktoken_is_active_key_type_78906b19_uniq"
DETAIL:  Key (is_active, key, type)=(f, 585258, AUTH) already exists.

There is about 14,000 Callback token in the system. Using drfpasswordless==1.5.6

Here is my understanding of the problem: In this scenario, there is two callback token with key '585258'. For one active is true, for others it is false. When passwordless try to create another token for the user with active true, drfpasswordless.signals.invalidate_previous_tokens runs and creates this error.

@receiver(signals.post_save, sender=CallbackToken)
def invalidate_previous_tokens(sender, instance, created, **kwargs):
    """
    Invalidates all previously issued tokens of that type when a new one is created, used, or anything like that.
    """

    if instance.user.pk in api_settings.PASSWORDLESS_DEMO_USERS.keys():
        return

    if isinstance(instance, CallbackToken):
        CallbackToken.objects.active().filter(user=instance.user, type=instance.type).exclude(id=instance.id).update(is_active=False)
aaronn commented 3 years ago

Hm– we could try generating a few more times on an IntegrityError.

taime commented 2 years ago

Why do we need unique keys here? I mean, ok we will generate 2 times the code 123456 (in theory this can happen). What's wrong with that? Only the last one key should be active and we don't care about all previous keys, right?