Remove code that updates the PK (#251)

* Remove code that updates the PK

And note the internal inconsistency.

* After removing the PK upgrade, testing the upgrade is easier

Author: davidfischer

src/rest_framework_api_key/models.py

@@ -134,11 +134,11 @@ def is_valid(self, key: str) -> bool:
         # Transparently update the key to use the preferred hasher
         # if it is using an outdated hasher.
         if valid and not key_generator.using_preferred_hasher(self.hashed_key):
-            new_hashed_key = key_generator.hash(key)
-            type(self).objects.filter(prefix=self.prefix).update(
-                id=concatenate(self.prefix, new_hashed_key),
-                hashed_key=new_hashed_key,
-            )
+            # Note that since the PK includes the hashed key,
+            # they will be internally inconsistent following this upgrade.
+            # See: https://github.com/florimondmanca/djangorestframework-api-key/issues/128
+            self.hashed_key = key_generator.hash(key)
+            self.save()
 
         return valid
 

tests/test_model.py

@@ -2,6 +2,7 @@
 import string
 
 import pytest
+from django.contrib.auth.hashers import make_password
 from django.core.exceptions import ValidationError
 from django.db.utils import IntegrityError
 from test_project.heroes.models import Hero, HeroAPIKey
@@ -65,6 +66,29 @@ def test_custom_api_key_model() -> None:
     assert hero.api_keys.first() == hero_api_key
 
 
+@pytest.mark.django_db
+def test_api_key_hash_upgrade() -> None:
+    """Tests the hashing algo upgrade from Django's PW hashers to sha512."""
+    key_generator = APIKey.objects.key_generator
+
+    api_key, generated_key = APIKey.objects.create_key(name="test")
+    assert api_key.is_valid(generated_key)
+    assert key_generator.using_preferred_hasher(api_key.hashed_key)
+
+    # Use Django's built-in hashers, the old way of storing a key
+    api_key.hashed_key = make_password(generated_key)
+    api_key.save()
+
+    # Simple sanity check to ensure the hash is still being checked
+    # and that we aren't using the preferred hasher (using Django's slower hashers)
+    assert not api_key.is_valid(key_generator.hash("invalid-key"))
+    assert not key_generator.using_preferred_hasher(api_key.hashed_key)
+
+    # After calling `is_valid`, the key has been upgraded to use the preferred hasher
+    assert api_key.is_valid(generated_key)
+    assert key_generator.using_preferred_hasher(api_key.hashed_key)
+
+
 @pytest.mark.django_db
 def test_api_key_manager_get_from_key() -> None:
     api_key, generated_key = APIKey.objects.create_key(name="test")