Merge pull request #1212 from ministryofjustice/DPAT-1749

Dpat 1749 soft-delete datasources

controlpanel/api/admin.py

@@ -24,8 +24,8 @@ class AppAdmin(admin.ModelAdmin):
 
 
 class S3Admin(admin.ModelAdmin):
-    list_display = ("name", "created_by", "created", "is_data_warehouse")
-    list_filter = ("created_by", "is_data_warehouse")
+    list_display = ("name", "created_by", "created", "is_data_warehouse", "is_deleted")
+    list_filter = ("created_by", "is_data_warehouse", "is_deleted")
     search_fields = ("name",)
 
 

controlpanel/api/migrations/0031_add_soft_delete_fields.py

@@ -0,0 +1,31 @@
+# Generated by Django 4.2.1 on 2023-10-09 15:33
+
+# Third-party
+import django.db.models.deletion
+from django.conf import settings
+from django.db import migrations, models
+
+
+class Migration(migrations.Migration):
+
+    dependencies = [
+        ('api', '0030_task'),
+    ]
+
+    operations = [
+        migrations.AddField(
+            model_name='s3bucket',
+            name='deleted_at',
+            field=models.DateTimeField(null=True),
+        ),
+        migrations.AddField(
+            model_name='s3bucket',
+            name='deleted_by',
+            field=models.ForeignKey(null=True, on_delete=django.db.models.deletion.SET_NULL, related_name='deleted_s3buckets', to=settings.AUTH_USER_MODEL),
+        ),
+        migrations.AddField(
+            model_name='s3bucket',
+            name='is_deleted',
+            field=models.BooleanField(default=False),
+        ),
+    ]

controlpanel/api/models/policys3bucket.py

@@ -39,6 +39,7 @@ def grant_bucket_access(self):
         )
 
     def revoke_bucket_access(self):
+        # TODO update to use a Task to revoke access, to match user/app access
         if self.s3bucket.is_folder:
             return cluster.RoleGroup(self.policy).revoke_folder_access(
                 root_folder_path=self.s3bucket.name

controlpanel/api/models/s3bucket.py

@@ -3,16 +3,19 @@
 
 # Third-party
 from django.conf import settings
+from django.contrib.auth.models import User
 from django.core.validators import MinLengthValidator
 from django.db import models
 from django.db.models import Q
 from django.db.transaction import atomic
+from django.utils import timezone
 from django_extensions.db.models import TimeStampedModel
 
 # First-party/Local
 from controlpanel.api import cluster, tasks, validators
 from controlpanel.api.models.apps3bucket import AppS3Bucket
 from controlpanel.api.models.users3bucket import UserS3Bucket
+from controlpanel.api.tasks.s3bucket import S3BucketRevokeAllAccess
 
 
 def s3bucket_console_url(name):
@@ -56,6 +59,14 @@ class S3Bucket(TimeStampedModel):
     is_data_warehouse = models.BooleanField(default=False)
     # TODO remove this field - it's unused
     location_url = models.CharField(max_length=128, null=True)
+    is_deleted = models.BooleanField(default=False)
+    deleted_by = models.ForeignKey(
+        "User",
+        on_delete=models.SET_NULL,
+        null=True,
+        related_name="deleted_s3buckets"
+    )
+    deleted_at = models.DateTimeField(null=True)
 
     objects = S3BucketQuerySet.as_manager()
 
@@ -158,3 +169,17 @@ def delete(self, *args, **kwargs):
         if not self.is_folder:
             self.cluster.mark_for_archival()
         super().delete(*args, **kwargs)
+
+    def soft_delete(self, deleted_by: User):
+        """
+        Mark the object as deleted, but do not remove it from the database
+        """
+        self.is_deleted = True
+        self.deleted_by = deleted_by
+        self.deleted_at = timezone.now()
+        self.save()
+        # TODO update to handle deleting folders
+        if not self.is_folder:
+            self.cluster.mark_for_archival()
+
+        S3BucketRevokeAllAccess(self, self.deleted_by).create_task()

controlpanel/api/serializers.py

@@ -209,12 +209,18 @@ class Meta:
             "created_by",
             "is_data_warehouse",
             "location_url",
+            "is_deleted",
+            "deleted_by",
+            "deleted_at",
         )
         read_only_fields = (
             "apps3buckets",
             "users3buckets",
             "created_by",
             "url",
+            "is_deleted",
+            "deleted_by",
+            "deleted_at",
         )
 
 

controlpanel/api/tasks/handlers/__init__.py

@@ -5,6 +5,7 @@
     CreateS3Bucket,
     GrantAppS3BucketAccess,
     GrantUserS3BucketAccess,
+    S3BucketRevokeAllAccess,
     S3BucketRevokeAppAccess,
     S3BucketRevokeUserAccess,
 )
@@ -16,3 +17,4 @@
 create_app_auth_settings = celery_app.register_task(CreateAppAuthSettings())
 revoke_user_s3bucket_access = celery_app.register_task(S3BucketRevokeUserAccess())
 revoke_app_s3bucket_access = celery_app.register_task(S3BucketRevokeAppAccess())
+revoke_all_access_s3bucket = celery_app.register_task(S3BucketRevokeAllAccess())

controlpanel/api/tasks/handlers/s3.py

@@ -1,9 +1,10 @@
 # Third-party
-from celery import Task as CeleryTask
+from django.db.models.deletion import Collector
 
 # First-party/Local
 from controlpanel.api import cluster
 from controlpanel.api.models import App, AppS3Bucket, S3Bucket, User, UserS3Bucket
+from controlpanel.api.models.access_to_s3bucket import AccessToS3Bucket
 from controlpanel.api.tasks.handlers.base import BaseModelTaskHandler, BaseTaskHandler
 
 
@@ -73,3 +74,24 @@ def handle(self, bucket_arn, app_pk):
             self.complete()
         cluster.App(app).revoke_bucket_access(bucket_arn)
         self.complete()
+
+
+class S3BucketRevokeAllAccess(BaseModelTaskHandler):
+    model = S3Bucket
+    name = "s3bucket_revoke_all_access"
+
+    def handle(self, *args, **kwargs):
+        """
+        When an S3Bucket is soft-deleted, the related objects that handle access will
+        remain in place. In order to keep IAM roles updated, this task collects objects
+        that would have been deleted by a cascade, and revokes access to deleted bucket
+        """
+        task_user = User.objects.filter(pk=self.task_user_pk).first()
+        collector = Collector(using="default")
+        collector.collect([self.object])
+        for model, instance in collector.instances_with_model():
+            if not issubclass(model, AccessToS3Bucket):
+                continue
+
+            instance.current_user = task_user
+            instance.revoke_bucket_access()

controlpanel/api/tasks/s3bucket.py

@@ -25,6 +25,19 @@ def _get_args_list(self):
         ]
 
 
+class S3BucketRevokeAllAccess(TaskBase):
+    ENTITY_CLASS = "S3Bucket"
+    QUEUE_NAME = settings.S3_QUEUE_NAME
+
+    @property
+    def task_name(self):
+        return "s3bucket_revoke_all_access"
+
+    @property
+    def task_description(self):
+        return "Revokes all access to an S3 bucket"
+
+
 class S3AccessMixin:
     ACTION = None
     ROLE = None

controlpanel/frontend/forms.py

@@ -146,7 +146,7 @@ class CreateAppForm(AppAuth0Form):
         required=False,
     )
     existing_datasource_id = DatasourceChoiceField(
-        queryset=S3Bucket.objects.filter(is_data_warehouse=False),
+        queryset=S3Bucket.objects.filter(is_data_warehouse=False, is_deleted=False),
         empty_label="Select",
         required=False,
     )
@@ -353,9 +353,11 @@ def __init__(self, *args, **kwargs):
         if self.exclude_connected:
             self.fields["datasource"].queryset = S3Bucket.objects.exclude(
                 id__in=[a.s3bucket_id for a in self.app.apps3buckets.all()],
-            )
+            ).filter(is_deleted=False)
         else:
-            self.fields["datasource"].queryset = S3Bucket.objects.all()
+            self.fields["datasource"].queryset = S3Bucket.objects.filter(
+                is_deleted=False,
+            )
 
 
 class CreateIAMManagedPolicyForm(forms.Form):

controlpanel/frontend/jinja2/datasource-detail.html

@@ -20,13 +20,25 @@
 <h1 class="govuk-heading-xl">{{ page_title }}</h1>
 
 <p class="govuk-body">
-  <a href="{{ bucket.aws_url }}" class="govuk-button govuk-button--secondary" target="_blank" rel="noopener">
-    Open on AWS
-  </a>
+  {% if bucket.is_deleted %}
+    <div class="govuk-warning-text">
+      <span class="govuk-warning-text__icon" aria-hidden="true">!</span>
+      <strong class="govuk-warning-text__text">
+          <span class="govuk-warning-text__assistive">Warning</span>
+          This bucket was deleted by <a href="{{ url('manage-user', kwargs={ "pk": bucket.deleted_by.auth0_id }) }}">
+          {{ user_name(bucket.deleted_by) }}</a> on {{ bucket.deleted_at.strftime("%Y/%m/%d %H:%M:%S") }}.<br>
+          All access listed below has been revoked in IAM.
+      </strong>
+    </div>
+  {% else %}
+    <a href="{{ bucket.aws_url }}" class="govuk-button govuk-button--secondary" target="_blank" rel="noopener">
+      Open on AWS
+    </a>
+  {% endif %}
 </p>
 
 <section class="cpanel-section track_task">
-  <h2 class="govuk-heading-m">Users and groups with access</h2>
+  <h2 class="govuk-heading-m">Users and groups with{% if bucket.is_deleted %} revoked{% endif %} access</h2>
   <table class="govuk-table">
     <thead class="govuk-table__head">
       <tr class="govuk-table__row">
@@ -63,7 +75,7 @@ <h2 class="govuk-heading-m">Users and groups with access</h2>
         {%- endif %}
         </td>
         <td class="govuk-table__cell">
-          {% if request.user.has_perm('api.update_users3bucket', member) %}
+          {% if request.user.has_perm('api.update_users3bucket', member) and not bucket.is_deleted %}
           <a href="{% if member.user -%}{{ url("update-access-level", kwargs={"pk": member.id}) }}{%- else -%}{{ url("update-policy-access-level", kwargs={"pk": member.id}) }}{%- endif %}"
              class="govuk-button govuk-button--secondary right">
             Edit access level
@@ -73,25 +85,27 @@ <h2 class="govuk-heading-m">Users and groups with access</h2>
       </tr>
     {% endfor %}
     </tbody>
+
+    {% set plural = access_list|length > 1 %}
     <tfoot class="govuk-table__foot">
       <tr class="govuk-table__row">
         <td class="govuk-table__cell" colspan="3">
           {{ access_list|length }}
-          user{%- if access_list|length != 1 -%}s{% endif %} or group{%- if access_list|length != 1 -%}s have{% else %} has{% endif %}
+          user{%- if plural -%}s{% endif %} or group{%- if plural -%}s{% endif %}{% if bucket.is_deleted %} had{% elif plural %} have{% else %} has{% endif %}
           access to this {{ datasource_type }} data source
       </tr>
     </tfoot>
   </table>
 
-  {% if request.user.has_perm('api.create_users3bucket', bucket) and users_options|length %}
+  {% if request.user.has_perm('api.create_users3bucket', bucket) and users_options|length and not bucket.is_deleted %}
     <a href="{{ url('grant-datasource-access', kwargs={'pk': bucket.pk}) }}"
        class="govuk-button govuk-button--secondary">
       Grant user access
     </a>
   {% endif %}
 
   {% if request.user.has_perm('api.manage_groups') %}
-    {% if request.user.has_perm('api.create_policys3bucket', bucket) and policies_options|length %}
+    {% if request.user.has_perm('api.create_policys3bucket', bucket) and policies_options|length and not bucket.is_deleted %}
       <a href="{{ url('grant-datasource-policy-access', kwargs={'pk': bucket.pk}) }}"
          class="govuk-button govuk-button--secondary">
         Grant group access
@@ -212,7 +226,7 @@ <h2 class="govuk-heading-m">Data access log</h2>
 </section>
 {% endif %}
 
-{% if request.user.has_perm('api.destroy_s3bucket', bucket) %}
+{% if not bucket.is_deleted and request.user.has_perm('api.destroy_s3bucket', bucket) %}
 <section class="cpanel-section">
   <form action="{{ url('delete-datasource', kwargs={ "pk": bucket.id }) }}" method="post">
     {{ csrf_input }}

controlpanel/frontend/jinja2/datasource-list.html

@@ -4,12 +4,12 @@
 
 {% extends "base.html" %}
 
-{% if datasource_type %}
-  {% set page_name = datasource_type + "-datasource-list" %}
-  {% set page_title = "Your " + datasource_type + " data sources" %}
-{% else %}
+{% if all_datasources %}
   {% set page_name = "all-datasources" %}
   {% set page_title = "All data sources" %}
+{% else %}
+  {% set page_name = datasource_type + "-datasource-list" %}
+  {% set page_title = "Your " + datasource_type + " data sources" %}
 {% endif %}
 
 {% set access_levels_html %}
@@ -71,4 +71,9 @@ <h3 class="govuk-heading-m">Other {{ datasource_type }} data sources</h3>
   </table>
 {% endif %}
 
+{% if request.user.is_superuser and deleted_datasources %}
+  <h3 class="govuk-heading-m">Deleted data sources</h3>
+  {{ datasource_list(deleted_datasources, datasource_type|default(""), request.user) }}
+{% endif %}
+
 {% endblock %}