fix(api-keys): make name required and unique (#8891)

This commit is contained in:
Víctor Fernández Poyatos
2025-10-10 12:35:27 +02:00
committed by GitHub
parent 335db928dc
commit 8794515318
5 changed files with 124 additions and 14 deletions
+13 -2
View File
@@ -2,6 +2,7 @@
import uuid
import django.core.validators
import django.db.models.deletion
import drf_simple_apikey.models
from django.conf import settings
@@ -20,7 +21,13 @@ class Migration(migrations.Migration):
migrations.CreateModel(
name="TenantAPIKey",
fields=[
("name", models.CharField(blank=True, max_length=255, null=True)),
(
"name",
models.CharField(
max_length=255,
validators=[django.core.validators.MinLengthValidator(3)],
),
),
(
"expiry_date",
models.DateTimeField(
@@ -110,7 +117,11 @@ class Migration(migrations.Migration):
"constraints": [
models.UniqueConstraint(
fields=("tenant_id", "prefix"), name="unique_api_key_prefixes"
)
),
models.UniqueConstraint(
fields=("tenant_id", "name"),
name="unique_api_key_name_per_tenant",
),
],
},
),
+5
View File
@@ -220,6 +220,7 @@ class Membership(models.Model):
class TenantAPIKey(AbstractAPIKey, RowLevelSecurityProtectedModel):
id = models.UUIDField(primary_key=True, default=uuid4, editable=False)
name = models.CharField(max_length=100, validators=[MinLengthValidator(3)])
prefix = models.CharField(
max_length=11,
unique=True,
@@ -255,6 +256,10 @@ class TenantAPIKey(AbstractAPIKey, RowLevelSecurityProtectedModel):
fields=("tenant_id", "prefix"),
name="unique_api_key_prefixes",
),
models.UniqueConstraint(
fields=("tenant_id", "name"),
name="unique_api_key_name_per_tenant",
),
]
indexes = [
+20 -10
View File
@@ -12460,8 +12460,8 @@ components:
properties:
name:
type: string
nullable: true
maxLength: 255
minLength: 3
maxLength: 100
prefix:
type: string
readOnly: true
@@ -12481,6 +12481,8 @@ components:
readOnly: true
nullable: true
description: Last time this API key was used for authentication
required:
- name
relationships:
type: object
properties:
@@ -15767,8 +15769,8 @@ components:
properties:
name:
type: string
nullable: true
maxLength: 255
maxLength: 100
minLength: 3
prefix:
type: string
readOnly: true
@@ -15790,6 +15792,8 @@ components:
format: date-time
nullable: true
description: Last time this API key was used for authentication
required:
- name
relationships:
type: object
properties:
@@ -15836,8 +15840,8 @@ components:
properties:
name:
type: string
nullable: true
maxLength: 255
maxLength: 100
minLength: 3
prefix:
type: string
readOnly: true
@@ -15863,6 +15867,8 @@ components:
api_key:
type: string
readOnly: true
required:
- name
relationships:
type: object
properties:
@@ -15913,8 +15919,8 @@ components:
properties:
name:
type: string
nullable: true
maxLength: 255
minLength: 3
maxLength: 100
prefix:
type: string
readOnly: true
@@ -15941,6 +15947,8 @@ components:
api_key:
type: string
readOnly: true
required:
- name
relationships:
type: object
properties:
@@ -16008,8 +16016,8 @@ components:
properties:
name:
type: string
nullable: true
maxLength: 255
maxLength: 100
minLength: 3
prefix:
type: string
readOnly: true
@@ -16028,6 +16036,8 @@ components:
readOnly: true
nullable: true
description: Last time this API key was used for authentication
required:
- name
relationships:
type: object
properties:
+68 -2
View File
@@ -7739,8 +7739,6 @@ class TestTenantApiKeyViewSet:
(
[
{"name": "New API Key"},
{"name": ""},
{},
]
),
)
@@ -7781,6 +7779,18 @@ class TestTenantApiKeyViewSet:
{"name": "Invalid Expiry", "expires_at": "not-a-date"},
"expires_at",
),
(
{"name": ""},
"name",
),
(
{},
"name",
),
(
{"name": "AB"}, # Too short (min length is 3)
"name",
),
]
),
)
@@ -7809,6 +7819,58 @@ class TestTenantApiKeyViewSet:
== f"/data/attributes/{error_pointer}"
)
def test_api_keys_create_duplicate_name(
self, authenticated_client, api_keys_fixture
):
"""Test creating an API key with a duplicate name fails."""
# Use the name of an existing API key
existing_name = api_keys_fixture[0].name
data = {
"data": {
"type": "api-keys",
"attributes": {
"name": existing_name,
},
}
}
response = authenticated_client.post(
reverse("api-key-list"),
data=json.dumps(data),
content_type="application/vnd.api+json",
)
assert response.status_code == status.HTTP_400_BAD_REQUEST
assert "errors" in response.json()
error_detail = response.json()["errors"][0]["detail"]
assert "already exists" in error_detail.lower()
def test_api_keys_update_duplicate_name(
self, authenticated_client, api_keys_fixture
):
"""Test updating an API key with a duplicate name fails."""
# Get two different API keys
first_api_key = api_keys_fixture[0]
second_api_key = api_keys_fixture[1]
# Try to update the second API key to have the same name as the first one
data = {
"data": {
"type": "api-keys",
"id": str(second_api_key.id),
"attributes": {
"name": first_api_key.name,
},
}
}
response = authenticated_client.patch(
reverse("api-key-detail", kwargs={"pk": second_api_key.id}),
data=json.dumps(data),
content_type="application/vnd.api+json",
)
assert response.status_code == status.HTTP_400_BAD_REQUEST
assert "errors" in response.json()
error_detail = response.json()["errors"][0]["detail"]
assert "already exists" in error_detail.lower()
def test_api_keys_create_multiple_unique_prefixes(
self, authenticated_client, api_keys_fixture
):
@@ -8260,6 +8322,10 @@ class TestTenantApiKeyViewSet:
assert included_user["type"] == "users"
assert included_user["id"] == str(api_key.entity.id)
# Refresh entity from database to get current state
# (in case other tests modified the shared session-scoped user fixture)
api_key.entity.refresh_from_db()
# Verify UserIncludeSerializer fields are present
user_attrs = included_user["attributes"]
assert "name" in user_attrs
+18
View File
@@ -2872,6 +2872,13 @@ class TenantApiKeyCreateSerializer(RLSSerializer, BaseWriteSerializer):
"api_key": {"read_only": True},
}
def validate_name(self, value):
"""Validate that the name is unique within the tenant."""
tenant_id = self.context.get("tenant_id")
if TenantAPIKey.objects.filter(tenant_id=tenant_id, name=value).exists():
raise ValidationError("An API key with this name already exists.")
return value
def get_api_key(self, obj):
"""Return the raw API key if it was stored during creation."""
return getattr(obj, "_raw_api_key", None)
@@ -2913,3 +2920,14 @@ class TenantApiKeyUpdateSerializer(RLSSerializer, BaseWriteSerializer):
"inserted_at": {"read_only": True},
"last_used_at": {"read_only": True},
}
def validate_name(self, value):
"""Validate that the name is unique within the tenant, excluding current instance."""
tenant_id = self.context.get("tenant_id")
if (
TenantAPIKey.objects.filter(tenant_id=tenant_id, name=value)
.exclude(id=self.instance.id)
.exists()
):
raise ValidationError("An API key with this name already exists.")
return value