Skip to content

Commit

Permalink
feat(tags): prevent system tag modifications (#3605)
Browse files Browse the repository at this point in the history
  • Loading branch information
matthewelwell authored Mar 13, 2024
1 parent 9d737ad commit 974dfc5
Show file tree
Hide file tree
Showing 4 changed files with 88 additions and 8 deletions.
25 changes: 18 additions & 7 deletions api/conftest.py
Original file line number Diff line number Diff line change
Expand Up @@ -165,13 +165,20 @@ def chargebee_subscription(organisation: Organisation) -> Subscription:


@pytest.fixture()
def project(organisation):
return Project.objects.create(name="Test Project", organisation=organisation)
def tag(project):
return Tag.objects.create(label="tag", project=project, color="#000000")


@pytest.fixture()
def tag(project):
return Tag.objects.create(label="tag", project=project, color="#000000")
def system_tag(project: Project) -> Tag:
return Tag.objects.create(
label="system-tag", project=project, color="#FFFFFF", is_system_tag=True
)


@pytest.fixture()
def project(organisation):
return Project.objects.create(name="Test Project", organisation=organisation)


@pytest.fixture()
Expand Down Expand Up @@ -229,13 +236,17 @@ def with_project_permissions(
"""

def _with_project_permissions(
permission_keys: list[str], project_id: typing.Optional[int] = None
permission_keys: list[str] = None,
project_id: typing.Optional[int] = None,
admin: bool = False,
) -> UserProjectPermission:
project_id = project_id or project.id
upp, __ = UserProjectPermission.objects.get_or_create(
project_id=project_id, user=staff_user
project_id=project_id, user=staff_user, admin=admin
)
upp.permissions.add(*permission_keys)

if permission_keys:
upp.permissions.add(*permission_keys)

return upp

Expand Down
7 changes: 7 additions & 0 deletions api/projects/tags/serializers.py
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
from typing import Any

from rest_framework import serializers

from projects.tags.models import Tag
Expand All @@ -18,3 +20,8 @@ class Meta:
"type",
)
read_only_fields = ("project", "uuid", "is_system_tag", "type")

def validate(self, attrs: dict[str, Any]) -> dict[str, Any]:
if self.instance and self.instance.is_system_tag:
raise serializers.ValidationError("Cannot update a system tag.")
return super().validate(attrs)
14 changes: 13 additions & 1 deletion api/projects/tags/views.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
from rest_framework import viewsets
from rest_framework import status, viewsets
from rest_framework.decorators import action
from rest_framework.generics import get_object_or_404
from rest_framework.permissions import IsAuthenticated
Expand Down Expand Up @@ -36,6 +36,18 @@ def perform_update(self, serializer):
project_id = int(self.kwargs["project_pk"])
serializer.save(project_id=project_id)

def destroy(self, request: Request, *args, **kwargs):
instance = self.get_object()

if instance.is_system_tag:
return Response(
status=status.HTTP_400_BAD_REQUEST,
data={"message": "Cannot delete a system tag."},
)

self.perform_destroy(instance)
return Response(status=status.HTTP_204_NO_CONTENT)

@action(
detail=False,
url_path=r"get-by-uuid/(?P<uuid>[0-9a-f-]+)",
Expand Down
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
import json

import pytest
from django.urls import reverse
from pytest_lazyfixture import lazy_fixture
Expand Down Expand Up @@ -68,3 +70,51 @@ def test_get_tag_by__uuid_returns_200_for_user_with_view_project_permission(
# Then
assert response.status_code == status.HTTP_200_OK
assert response.json()["uuid"] == str(tag.uuid)


def test_cannot_delete_a_system_tag(
staff_client: APIClient,
with_project_permissions: WithProjectPermissionsCallable,
project: Project,
system_tag: Tag,
) -> None:
# Given
url = reverse("api-v1:projects:tags-detail", args=(project.id, system_tag.id))

with_project_permissions(admin=True)

# When
response = staff_client.delete(url)

# Then
assert response.status_code == status.HTTP_400_BAD_REQUEST
assert response.json() == {"message": "Cannot delete a system tag."}


def test_cannot_update_a_system_tag(
staff_client: APIClient,
with_project_permissions: WithProjectPermissionsCallable,
project: Project,
system_tag: Tag,
):
# Given
url = reverse("api-v1:projects:tags-detail", args=(project.id, system_tag.id))

with_project_permissions(admin=True)

updated_label = f"{system_tag.label} updated"

data = {
"id": system_tag.id,
"color": system_tag.color,
"label": updated_label,
"project": system_tag.project_id,
"description": "",
}

# When
response = staff_client.put(url, json.dumps(data), content_type="application/json")

# Then
assert response.status_code == status.HTTP_400_BAD_REQUEST
assert response.json() == {"non_field_errors": ["Cannot update a system tag."]}

0 comments on commit 974dfc5

Please sign in to comment.