Skip to content

Commit

Permalink
Remove code that updates the PK (#251)
Browse files Browse the repository at this point in the history
* Remove code that updates the PK

And note the internal inconsistency.

* After removing the PK upgrade, testing the upgrade is easier
  • Loading branch information
davidfischer authored Sep 17, 2023
1 parent 3a1609f commit f93a1c4
Show file tree
Hide file tree
Showing 2 changed files with 29 additions and 5 deletions.
10 changes: 5 additions & 5 deletions src/rest_framework_api_key/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
24 changes: 24 additions & 0 deletions tests/test_model.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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")
Expand Down

0 comments on commit f93a1c4

Please sign in to comment.