Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

fix(tenant): fix delete tenants behavior #6014

Merged
merged 1 commit into from
Dec 13, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1,956 changes: 1,025 additions & 931 deletions api/poetry.lock

Large diffs are not rendered by default.

2 changes: 1 addition & 1 deletion api/pyproject.toml
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ drf-nested-routers = "^0.94.1"
drf-spectacular = "0.27.2"
drf-spectacular-jsonapi = "0.5.1"
gunicorn = "23.0.0"
prowler = {git = "https://github.com/prowler-cloud/prowler.git", branch = "master"}
prowler = {git = "https://github.com/prowler-cloud/prowler.git", tag = "5.0.0"}
psycopg2-binary = "2.9.9"
pytest-celery = {extras = ["redis"], version = "^1.0.1"}
# Needed for prowler compatibility
Expand Down
28 changes: 25 additions & 3 deletions api/src/backend/api/base_views.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import uuid

from django.db import transaction, connection
from django.db import connection, transaction
from rest_framework import permissions
from rest_framework.exceptions import NotAuthenticated
from rest_framework.filters import SearchFilter
Expand Down Expand Up @@ -69,10 +69,32 @@ def dispatch(self, request, *args, **kwargs):
return super().dispatch(request, *args, **kwargs)

def initial(self, request, *args, **kwargs):
user_id = str(request.user.id)
if (
request.resolver_match.url_name != "tenant-detail"
and request.method != "DELETE"
):
user_id = str(request.user.id)

with connection.cursor() as cursor:
cursor.execute(f"SELECT set_config('api.user_id', '{user_id}', TRUE);")
return super().initial(request, *args, **kwargs)

# TODO: DRY this when we have time
if request.auth is None:
raise NotAuthenticated

tenant_id = request.auth.get("tenant_id")
if tenant_id is None:
raise NotAuthenticated("Tenant ID is not present in token")

try:
uuid.UUID(tenant_id)
except ValueError:
raise ValidationError("Tenant ID must be a valid UUID")

with connection.cursor() as cursor:
cursor.execute(f"SELECT set_config('api.user_id', '{user_id}', TRUE);")
cursor.execute(f"SELECT set_config('api.tenant_id', '{tenant_id}', TRUE);")
self.request.tenant_id = tenant_id
return super().initial(request, *args, **kwargs)


Expand Down
13 changes: 12 additions & 1 deletion api/src/backend/api/tests/test_views.py
Original file line number Diff line number Diff line change
Expand Up @@ -418,13 +418,24 @@ def test_tenants_partial_update_invalid_content(
)
assert response.status_code == status.HTTP_400_BAD_REQUEST

def test_tenants_delete(self, authenticated_client, tenants_fixture):
@patch("api.db_router.MainRouter.admin_db", new="default")
@patch("api.v1.views.delete_tenant_task.apply_async")
def test_tenants_delete(
self, delete_tenant_mock, authenticated_client, tenants_fixture
):
def _delete_tenant(kwargs):
Tenant.objects.filter(pk=kwargs.get("tenant_id")).delete()

delete_tenant_mock.side_effect = _delete_tenant
tenant1, *_ = tenants_fixture
response = authenticated_client.delete(
reverse("tenant-detail", kwargs={"pk": tenant1.id})
)
assert response.status_code == status.HTTP_204_NO_CONTENT
assert Tenant.objects.count() == len(tenants_fixture) - 1
assert Membership.objects.filter(tenant_id=tenant1.id).count() == 0
# User is not deleted because it has another membership
assert User.objects.count() == 1

def test_tenants_delete_invalid(self, authenticated_client):
response = authenticated_client.delete(
Expand Down
22 changes: 21 additions & 1 deletion api/src/backend/api/v1/views.py
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@
from tasks.tasks import (
check_provider_connection_task,
delete_provider_task,
delete_tenant_task,
perform_scan_summary_task,
perform_scan_task,
)
Expand Down Expand Up @@ -171,7 +172,7 @@ class SchemaView(SpectacularAPIView):

def get(self, request, *args, **kwargs):
spectacular_settings.TITLE = "Prowler API"
spectacular_settings.VERSION = "1.0.0"
spectacular_settings.VERSION = "1.0.1"
spectacular_settings.DESCRIPTION = (
"Prowler API specification.\n\nThis file is auto-generated."
)
Expand Down Expand Up @@ -401,6 +402,25 @@ def create(self, request, *args, **kwargs):
)
return Response(data=serializer.data, status=status.HTTP_201_CREATED)

def destroy(self, request, *args, **kwargs):
# This will perform validation and raise a 404 if the tenant does not exist
tenant_id = kwargs.get("pk")
get_object_or_404(Tenant, id=tenant_id)

with transaction.atomic():
# Delete memberships
Membership.objects.using(MainRouter.admin_db).filter(
tenant_id=tenant_id
).delete()

# Delete users without memberships
User.objects.using(MainRouter.admin_db).filter(
membership__isnull=True
).delete()
# Delete tenant in batches
delete_tenant_task.apply_async(kwargs={"tenant_id": tenant_id})
return Response(status=status.HTTP_204_NO_CONTENT)


@extend_schema_view(
list=extend_schema(
Expand Down
28 changes: 26 additions & 2 deletions api/src/backend/tasks/jobs/deletion.py
Original file line number Diff line number Diff line change
@@ -1,8 +1,9 @@
from celery.utils.log import get_task_logger
from django.db import transaction

from api.db_utils import batch_delete
from api.models import Finding, Provider, Resource, Scan, ScanSummary
from api.db_router import MainRouter
from api.db_utils import batch_delete, tenant_transaction
from api.models import Finding, Provider, Resource, Scan, ScanSummary, Tenant

logger = get_task_logger(__name__)

Expand Down Expand Up @@ -49,3 +50,26 @@ def delete_provider(pk: str):
deletion_summary.update(provider_summary)

return deletion_summary


def delete_tenant(pk: str):
"""
Gracefully deletes an instance of a tenant along with its related data.

Args:
pk (str): The primary key of the Tenant instance to delete.

Returns:
dict: A dictionary with the count of deleted objects per model,
including related models.
"""
deletion_summary = {}

for provider in Provider.objects.using(MainRouter.admin_db).filter(tenant_id=pk):
with tenant_transaction(pk):
summary = delete_provider(provider.id)
deletion_summary.update(summary)

Tenant.objects.using(MainRouter.admin_db).filter(id=pk).delete()

return deletion_summary
7 changes: 6 additions & 1 deletion api/src/backend/tasks/tasks.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
from config.celery import RLSTask
from django_celery_beat.models import PeriodicTask
from tasks.jobs.connection import check_provider_connection
from tasks.jobs.deletion import delete_provider
from tasks.jobs.deletion import delete_provider, delete_tenant
from tasks.jobs.scan import aggregate_findings, perform_prowler_scan

from api.db_utils import tenant_transaction
Expand Down Expand Up @@ -134,3 +134,8 @@ def perform_scheduled_scan_task(self, tenant_id: str, provider_id: str):
@shared_task(name="scan-summary")
def perform_scan_summary_task(tenant_id: str, scan_id: str):
return aggregate_findings(tenant_id=tenant_id, scan_id=scan_id)


@shared_task(name="tenant-deletion")
def delete_tenant_task(tenant_id: str):
return delete_tenant(pk=tenant_id)
51 changes: 46 additions & 5 deletions api/src/backend/tasks/tests/test_deletion.py
Original file line number Diff line number Diff line change
@@ -1,22 +1,63 @@
from unittest.mock import patch

import pytest
from django.core.exceptions import ObjectDoesNotExist
from tasks.jobs.deletion import delete_provider
from tasks.jobs.deletion import delete_provider, delete_tenant

from api.models import Provider
from api.models import Provider, Tenant


@pytest.mark.django_db
class TestDeleteInstance:
def test_delete_instance_success(self, providers_fixture):
class TestDeleteProvider:
def test_delete_provider_success(self, providers_fixture):
instance = providers_fixture[0]
result = delete_provider(instance.id)

assert result
with pytest.raises(ObjectDoesNotExist):
Provider.objects.get(pk=instance.id)

def test_delete_instance_does_not_exist(self):
def test_delete_provider_does_not_exist(self):
non_existent_pk = "babf6796-cfcc-4fd3-9dcf-88d012247645"

with pytest.raises(ObjectDoesNotExist):
delete_provider(non_existent_pk)


@patch("api.db_router.MainRouter.admin_db", new="default")
@pytest.mark.django_db
class TestDeleteTenant:
def test_delete_tenant_success(self, tenants_fixture, providers_fixture):
"""
Test successful deletion of a tenant and its related data.
"""
tenant = tenants_fixture[0]
providers = Provider.objects.filter(tenant_id=tenant.id)

# Ensure the tenant and related providers exist before deletion
assert Tenant.objects.filter(id=tenant.id).exists()
assert providers.exists()

# Call the function and validate the result
deletion_summary = delete_tenant(tenant.id)

assert deletion_summary is not None
assert not Tenant.objects.filter(id=tenant.id).exists()
assert not Provider.objects.filter(tenant_id=tenant.id).exists()

def test_delete_tenant_with_no_providers(self, tenants_fixture):
"""
Test deletion of a tenant with no related providers.
"""
tenant = tenants_fixture[1] # Assume this tenant has no providers
providers = Provider.objects.filter(tenant_id=tenant.id)

# Ensure the tenant exists but has no related providers
assert Tenant.objects.filter(id=tenant.id).exists()
assert not providers.exists()

# Call the function and validate the result
deletion_summary = delete_tenant(tenant.id)

assert deletion_summary == {} # No providers, so empty summary
assert not Tenant.objects.filter(id=tenant.id).exists()
Loading