Repository: florimondmanca/djangorestframework-api-key

Issue:
Optional expiry date
**Is your feature request related to a problem? Please describe.**
At the moment, an API key is forever valid. It may be useful to create temporary and/or renewable API keys, with a limited lifespan and hence an **expiry date**.

**Describe the solution you'd like**
- Allow to define an API key's expiry date upon creation, or set one after creation.
- If no expiry date is set, the API key never expires (current behavior).
- If one is set (e.g. 2019-05-11), the API key should expire at midnight UTC (i.e. 2019-05-12 00:00 UTC).

**Describe alternatives you've considered**
- Require an expiry date: this would both break current behavior and be too much for simplest use cases.

**Implementation ideas**
- Add a nullable and blank `.expiry_date` field to `APIKey`, and create a migration.
- Update `HasAPIKey` to check the expiry date on the API key, if applicable.

**Additional context**
This was originally mentioned by @ssrebelious in https://github.com/florimondmanca/djangorestframework-api-key/issues/16#issuecomment-479383613.


Discussion:

Patch:
diff --git a/CHANGELOG.md b/CHANGELOG.md
index cf4a4e4..589e0f1 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -7,6 +7,19 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ## [Unreleased]
 
+**NOTE**: this release contains migrations. In your Django project, run them using:
+
+```python
+python manage.py migrate rest_framework_api_key
+```
+
+### Added
+
+- API keys can now have an optional `expiry_date`.
+- `HasAPIKey` denies access if the API key has expired, i.e. if `expiry_date`, if set, is in the past.
+- It is now possible to search by `prefix` in the API key admin panel.
+- The `prefix` is now displayed in the edit view of the API key admin panel.
+
 ## [1.1.0]
 
 Released: 2019-05-14
diff --git a/rest_framework_api_key/_decorators.py b/rest_framework_api_key/_decorators.py
new file mode 100644
index 0000000..c5e4b29
--- /dev/null
+++ b/rest_framework_api_key/_decorators.py
@@ -0,0 +1,18 @@
+class DjangoProperty(property):
+    def __init__(self, fget):
+        super().__init__(fget=fget)
+        self.name = None
+
+    def __set_name__(self, owner, name):
+        setattr(owner, name + "_func", self.fget)
+
+
+def djangoproperty(**kwargs):
+    """A Django admin-friendly variant of @property."""
+
+    def decorate(func):
+        for key, value in kwargs.items():
+            setattr(func, key, value)
+        return DjangoProperty(func)
+
+    return decorate
diff --git a/rest_framework_api_key/_helpers.py b/rest_framework_api_key/_helpers.py
index 20036f3..23b2d5d 100644
--- a/rest_framework_api_key/_helpers.py
+++ b/rest_framework_api_key/_helpers.py
@@ -23,4 +23,3 @@ def generate_key() -> Tuple[str, str]:
 
 def check_key(key: str, hashed_key: str) -> bool:
     return check_password(key, hashed_key)
-
diff --git a/rest_framework_api_key/admin.py b/rest_framework_api_key/admin.py
index cc8fe10..46c683f 100644
--- a/rest_framework_api_key/admin.py
+++ b/rest_framework_api_key/admin.py
@@ -8,20 +8,27 @@ from .models import APIKey
 
 @admin.register(APIKey)
 class APIKeyAdmin(admin.ModelAdmin):
-    list_display = ("name", "prefix", "created", "revoked")
-    list_filter = ("created", "revoked")
-
-    readonly_fields = ("get_api_key",)
-    search_fields = ("name",)
-
-    fieldsets = ((None, {"fields": ("name", 'has_expired', 'expiry_date', "revoked", "get_api_key")}),)
+    list_display = (
+        "name",
+        "prefix",
+        "created",
+        "expiry_date",
+        "has_expired_func",
+        "revoked",
+    )
+    list_filter = ("created",)
+    search_fields = ("name", "prefix")
+    fieldsets = (
+        (None, {"fields": ("name", "prefix", "expiry_date", "revoked")}),
+    )
 
     def get_readonly_fields(
         self, request, obj: APIKey = None
     ) -> typing.Tuple[str]:
+        fields = ("prefix",)
         if obj is not None and obj.revoked:
-            return self.readonly_fields + ("name", "revoked")
-        return self.readonly_fields
+            fields = fields + ("name", "revoked", "expiry_date")
+        return fields
 
     def get_api_key(self, obj: APIKey) -> str:
         if obj.pk:
diff --git a/rest_framework_api_key/migrations/0002_apikey_expiry_date.py b/rest_framework_api_key/migrations/0002_apikey_expiry_date.py
deleted file mode 100644
index f6fb979..0000000
--- a/rest_framework_api_key/migrations/0002_apikey_expiry_date.py
+++ /dev/null
@@ -1,18 +0,0 @@
-# Generated by Django 2.2.1 on 2019-05-15 12:59
-
-from django.db import migrations, models
-
-
-class Migration(migrations.Migration):
-
-    dependencies = [
-        ('rest_framework_api_key', '0001_initial'),
-    ]
-
-    operations = [
-        migrations.AddField(
-            model_name='apikey',
-            name='expiry_date',
-            field=models.DateTimeField(blank=True, null=True),
-        ),
-    ]
diff --git a/rest_framework_api_key/migrations/0002_auto_20190529_2243.py b/rest_framework_api_key/migrations/0002_auto_20190529_2243.py
new file mode 100644
index 0000000..ad2bf69
--- /dev/null
+++ b/rest_framework_api_key/migrations/0002_auto_20190529_2243.py
@@ -0,0 +1,28 @@
+# Generated by Django 2.2.1 on 2019-05-29 22:43
+
+from django.db import migrations, models
+
+
+class Migration(migrations.Migration):
+
+    dependencies = [
+        ('rest_framework_api_key', '0001_initial'),
+    ]
+
+    operations = [
+        migrations.AddField(
+            model_name='apikey',
+            name='expiry_date',
+            field=models.DateTimeField(blank=True, help_text='Once API key expires, clients cannot use it anymore.', null=True, verbose_name='Expires'),
+        ),
+        migrations.AlterField(
+            model_name='apikey',
+            name='name',
+            field=models.CharField(default=None, help_text='A free-form name for the API key. Need not be unique. 50 characters max.', max_length=50),
+        ),
+        migrations.AlterField(
+            model_name='apikey',
+            name='revoked',
+            field=models.BooleanField(blank=True, default=False, help_text='If the API key is revoked, clients cannot use it anymore. (This cannot be undone.)'),
+        ),
+    ]
diff --git a/rest_framework_api_key/migrations/0003_apikey_has_expired.py b/rest_framework_api_key/migrations/0003_apikey_has_expired.py
deleted file mode 100644
index a0eb6ee..0000000
--- a/rest_framework_api_key/migrations/0003_apikey_has_expired.py
+++ /dev/null
@@ -1,18 +0,0 @@
-# Generated by Django 2.2.1 on 2019-05-17 15:33
-
-from django.db import migrations, models
-
-
-class Migration(migrations.Migration):
-
-    dependencies = [
-        ('rest_framework_api_key', '0002_apikey_expiry_date'),
-    ]
-
-    operations = [
-        migrations.AddField(
-            model_name='apikey',
-            name='has_expired',
-            field=models.BooleanField(default=False),
-        ),
-    ]
diff --git a/rest_framework_api_key/models.py b/rest_framework_api_key/models.py
index 13aec08..c9aaeef 100644
--- a/rest_framework_api_key/models.py
+++ b/rest_framework_api_key/models.py
@@ -1,8 +1,9 @@
 from typing import Tuple
-from datetime import datetime
 from django.core.exceptions import ValidationError
 from django.db import models
+from django.utils import timezone
 from ._helpers import check_key, generate_key
+from ._decorators import djangoproperty
 
 
 class APIKeyManager(models.Manager):
@@ -20,13 +21,19 @@ class APIKeyManager(models.Manager):
 
     def is_valid(self, key: str) -> bool:
         prefix, _, _ = key.partition(".")
+
         try:
-            obj = self.get(id__startswith=prefix, revoked=False)
+            api_key = self.get(id__startswith=prefix, revoked=False)
         except self.model.DoesNotExist:
             return False
-        if obj.has_expired:
-            return obj.expiry_date > datetime.now(obj.expiry_date.tzinfo)
-        return obj.is_valid(key)
+
+        if not api_key.is_valid(key):
+            return False
+
+        if api_key.has_expired:
+            return False
+
+        return True
 
 
 class APIKey(models.Model):
@@ -34,14 +41,31 @@ class APIKey(models.Model):
 
     id = models.CharField(max_length=100, unique=True, primary_key=True)
     created = models.DateTimeField(auto_now_add=True, db_index=True)
-    name = models.CharField(max_length=50, blank=False, default=None)
+    name = models.CharField(
+        max_length=50,
+        blank=False,
+        default=None,
+        help_text=(
+            "A free-form name for the API key. "
+            "Need not be unique. "
+            "50 characters max."
+        ),
+    )
     revoked = models.BooleanField(
         blank=True,
         default=False,
-        help_text="If the API key is revoked, clients cannot use it anymore.",
+        help_text=(
+            "If the API key is revoked, clients cannot use it anymore. "
+            "(This cannot be undone.)"
+        ),
+    )
+    expiry_date = models.DateTimeField(
+        blank=True,
+        null=True,
+        verbose_name="Expires",
+        help_text="Once API key expires, clients cannot use it anymore.",
     )
-    expiry_date = models.DateTimeField(blank=True, null=True)
-    has_expired = models.BooleanField(default=False)
+
     class Meta:  # noqa
         ordering = ("-created",)
         verbose_name = "API key"
@@ -52,12 +76,15 @@ class APIKey(models.Model):
         # Store the initial value of `revoked` to detect changes.
         self._initial_revoked = self.revoked
 
+    @djangoproperty(short_description="Prefix")
     def prefix(self) -> str:
         return self.pk.partition(".")[0]
 
-    # Ensure compatibility with Django admin.
-    prefix.short_description = "Prefix"
-    prefix = property(prefix)
+    @djangoproperty(short_description="Has expired", boolean=True)
+    def has_expired(self) -> bool:
+        if self.expiry_date is None:
+            return False
+        return self.expiry_date < timezone.now()
 
     def is_valid(self, key: str) -> bool:
         _, _, hashed_key = self.pk.partition(".")


Test patch:
diff --git a/tests/dateutils.py b/tests/dateutils.py
new file mode 100644
index 0000000..b0f46bd
--- /dev/null
+++ b/tests/dateutils.py
@@ -0,0 +1,5 @@
+from datetime import datetime, timedelta
+
+NOW = datetime.now()
+TOMORROW = NOW + timedelta(days=1)
+YESTERDAY = NOW - timedelta(days=1)
diff --git a/tests/test_api_key_model.py b/tests/test_api_key_model.py
index e22de0d..e70f943 100644
--- a/tests/test_api_key_model.py
+++ b/tests/test_api_key_model.py
@@ -5,6 +5,8 @@ from django.core.exceptions import ValidationError
 from django.db.utils import IntegrityError
 from rest_framework_api_key.models import APIKey
 
+from .dateutils import NOW, TOMORROW, YESTERDAY
+
 pytestmark = pytest.mark.django_db
 
 
@@ -40,3 +42,12 @@ def test_cannot_unrevoke():
 
     with pytest.raises(ValidationError):
         api_key.clean()
+
+
+@pytest.mark.parametrize(
+    "expiry_date, has_expired",
+    [(None, False), (NOW, True), (TOMORROW, False), (YESTERDAY, True)],
+)
+def test_has_expired(expiry_date, has_expired):
+    api_key, _ = APIKey.objects.create_key(name="test", expiry_date=expiry_date)
+    assert api_key.has_expired is has_expired
diff --git a/tests/test_has_api_key.py b/tests/test_has_api_key.py
index 33d5af8..3d127c1 100644
--- a/tests/test_has_api_key.py
+++ b/tests/test_has_api_key.py
@@ -1,3 +1,5 @@
+from datetime import datetime, timedelta
+
 import pytest
 from django.conf.global_settings import PASSWORD_HASHERS
 from django.test import override_settings
@@ -67,6 +69,21 @@ def test_if_revoked_then_permission_denied(create_request, view):
     assert response.status_code == 403
 
 
+NOW = datetime.now()
+TOMORROW = NOW + timedelta(days=1)
+TWO_DAYS_AGO = NOW - timedelta(days=2)
+
+
+@pytest.mark.parametrize(
+    "expiry_date, ok", [(TOMORROW, True), (TWO_DAYS_AGO, False)]
+)
+def test_expiry_date(create_request, view, expiry_date, ok):
+    request = create_request(expiry_date=expiry_date)
+    response = view(request)
+    status_code = 200 if ok else 403
+    assert response.status_code == status_code
+
+
 def test_object_permission(create_request):
     class DenyObject(permissions.BasePermission):
         def has_object_permission(self, request, view, obj):