diff --git a/basket/news/admin.py b/basket/news/admin.py index dc6c18180..084b8e8fb 100644 --- a/basket/news/admin.py +++ b/basket/news/admin.py @@ -55,7 +55,8 @@ class NewsletterGroupAdmin(admin.ModelAdmin): @admin.register(APIUser) class APIUserAdmin(admin.ModelAdmin): - list_display = ("name", "enabled") + list_display = ("name", "enabled", "created", "last_accessed") + readonly_fields = ("api_key", "created", "last_accessed") @admin.register(Newsletter) diff --git a/basket/news/migrations/0002_apiuser_created_apiuser_last_accessed.py b/basket/news/migrations/0002_apiuser_created_apiuser_last_accessed.py new file mode 100644 index 000000000..006f90f45 --- /dev/null +++ b/basket/news/migrations/0002_apiuser_created_apiuser_last_accessed.py @@ -0,0 +1,25 @@ +# Generated by Django 4.2.17 on 2025-01-14 23:41 + +import datetime + +from django.db import migrations, models + + +class Migration(migrations.Migration): + dependencies = [ + ("news", "0001_squashed_0001_to_0032"), + ] + + operations = [ + migrations.AddField( + model_name="apiuser", + name="created", + field=models.DateTimeField(auto_now_add=True, default=datetime.datetime(2025, 1, 1, 0, 0)), + preserve_default=False, + ), + migrations.AddField( + model_name="apiuser", + name="last_accessed", + field=models.DateTimeField(blank=True, null=True), + ), + ] diff --git a/basket/news/models.py b/basket/news/models.py index d2368789e..b36790184 100644 --- a/basket/news/models.py +++ b/basket/news/models.py @@ -6,6 +6,7 @@ import sentry_sdk +from basket import metrics from basket.base.rq import get_enqueue_kwargs, get_queue from basket.news.fields import LocaleField @@ -140,6 +141,8 @@ class APIUser(models.Model): name = models.CharField(max_length=256, help_text="Descriptive name of this user") api_key = models.CharField(max_length=40, default=get_uuid, db_index=True) enabled = models.BooleanField(default=True) + created = models.DateTimeField(auto_now_add=True) + last_accessed = models.DateTimeField(null=True, blank=True) class Meta: verbose_name = "API User" @@ -148,8 +151,28 @@ def __str__(self): # pragma: no cover return f"{self.name} ({self.api_key})" @classmethod - def is_valid(cls, api_key): - return cls.objects.filter(api_key=api_key, enabled=True).exists() + def is_valid(cls, api_key: str) -> bool: + """ + Checks if the API key is valid and enabled. + + Updates the `last_accessed` field if the key is valid. + + Returns: + bool: True if the API key is valid and enabled, False otherwise. + + """ + try: + obj = cls.objects.get(api_key=api_key) + if obj.enabled: + obj.last_accessed = now() + obj.save(update_fields=["last_accessed"]) + metrics.incr("api.key.is_valid", tags=["value:true"]) + return True + except APIUser.DoesNotExist: + pass + + metrics.incr("api.key.is_valid", tags=["value:false"]) + return False def _is_query_dict(arg): diff --git a/basket/news/tests/test_models.py b/basket/news/tests/test_models.py index 7243ed8fe..4f8b010bd 100644 --- a/basket/news/tests/test_models.py +++ b/basket/news/tests/test_models.py @@ -3,6 +3,9 @@ from django.test import TestCase from django.test.utils import override_settings +import pytest + +from basket.base.utils import is_valid_uuid from basket.news import models @@ -80,7 +83,8 @@ def test_retry(self, mock_enqueue, mock_random): assert mock_enqueue.call_args.kwargs["retry"].intervals == [60, 90] -class NewsletterTest: +@pytest.mark.django_db +class TestNewsletter: def test_newsletter_strips_languages(self): n = models.Newsletter.objects.create( slug="slug", @@ -89,3 +93,45 @@ def test_newsletter_strips_languages(self): ) obj = models.Newsletter.objects.get(id=n.id) assert obj.languages == "en,fr,de" + + +@pytest.mark.django_db +class TestAPIUser: + def _add_api_user(self, name=None): + name = name or "The Dude" + return models.APIUser.objects.create(name=name) + + def test_api_user(self): + user = self._add_api_user() + assert str(user) == f"{user.name} ({user.api_key})" + assert user.api_key is not None + assert is_valid_uuid(user.api_key) + assert user.enabled is True + assert user.created is not None + assert user.last_accessed is None + + def test_api_user_generate_key_unique(self): + user1 = self._add_api_user() + user2 = self._add_api_user() + assert user1.api_key != user2.api_key + + def test_api_is_valid(self, metricsmock): + user = self._add_api_user() + assert user.last_accessed is None + assert models.APIUser.is_valid(user.api_key) is True + metricsmock.assert_incr_once("api.key.is_valid", tags=["value:true"]) + # Test `is_valid` also updates `last_accessed`. + user.refresh_from_db() + assert user.last_accessed is not None + + def test_api_is_valid_disabled(self, metricsmock): + user = self._add_api_user() + user.enabled = False + user.save() + assert models.APIUser.is_valid(user.api_key) is False + metricsmock.assert_incr_once("api.key.is_valid", tags=["value:false"]) + assert user.last_accessed is None + + def test_api_is_valid_invalid(self, metricsmock): + assert models.APIUser.is_valid("invalid") is False + metricsmock.assert_incr_once("api.key.is_valid", tags=["value:false"])