From b7f594e49f6468ca20c2e67040393a8117d93756 Mon Sep 17 00:00:00 2001
From: Michael Collins <15347726+michaeljcollinsuk@users.noreply.github.com>
Date: Mon, 9 Oct 2023 16:27:20 +0100
Subject: [PATCH 01/10] Update S3bucket model to add fields to capture soft
 delete details

---
 .../migrations/0031_add_soft_delete_fields.py | 31 +++++++++++++++++++
 controlpanel/api/models/s3bucket.py           |  8 +++++
 2 files changed, 39 insertions(+)
 create mode 100644 controlpanel/api/migrations/0031_add_soft_delete_fields.py

diff --git a/controlpanel/api/migrations/0031_add_soft_delete_fields.py b/controlpanel/api/migrations/0031_add_soft_delete_fields.py
new file mode 100644
index 000000000..c66261360
--- /dev/null
+++ b/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),
+        ),
+    ]
diff --git a/controlpanel/api/models/s3bucket.py b/controlpanel/api/models/s3bucket.py
index 90de336f3..0d3aecae2 100644
--- a/controlpanel/api/models/s3bucket.py
+++ b/controlpanel/api/models/s3bucket.py
@@ -56,6 +56,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()
 

From 80c579a3fbba168be14fc1b0a9002e68e92faaca Mon Sep 17 00:00:00 2001
From: Michael Collins <15347726+michaeljcollinsuk@users.noreply.github.com>
Date: Tue, 10 Oct 2023 15:40:34 +0100
Subject: [PATCH 02/10] Implement soft delete for S3Bucket, and update
 DeleteDatasource view to call it

Changes view to use form_valid rather than delete to handle deletion, as this
was a change introduced in django 4.0. See docs/release notes for full info.
---
 controlpanel/api/models/s3bucket.py       | 14 ++++++++++
 controlpanel/frontend/views/datasource.py | 17 +++++++------
 tests/api/models/test_s3bucket.py         | 13 ++++++++++
 tests/frontend/views/test_datasource.py   | 31 ++++++++++++++++++++---
 4 files changed, 63 insertions(+), 12 deletions(-)

diff --git a/controlpanel/api/models/s3bucket.py b/controlpanel/api/models/s3bucket.py
index 0d3aecae2..8d72ed467 100644
--- a/controlpanel/api/models/s3bucket.py
+++ b/controlpanel/api/models/s3bucket.py
@@ -3,10 +3,12 @@
 
 # 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
@@ -166,3 +168,15 @@ 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()
diff --git a/controlpanel/frontend/views/datasource.py b/controlpanel/frontend/views/datasource.py
index 5099089ea..6572b4bdd 100644
--- a/controlpanel/frontend/views/datasource.py
+++ b/controlpanel/frontend/views/datasource.py
@@ -6,6 +6,7 @@
 from django.contrib import messages
 from django.core.exceptions import PermissionDenied
 from django.db import transaction
+from django.http import HttpResponseRedirect
 from django.shortcuts import get_object_or_404
 from django.urls import reverse_lazy
 from django.views.generic.base import ContextMixin
@@ -198,16 +199,16 @@ class DeleteDatasource(
     permission_required = "api.destroy_s3bucket"
     success_url = reverse_lazy("list-warehouse-datasources")
 
-    def delete(self, *args, **kwargs):
-        bucket = self.get_object()
-        if not bucket.is_data_warehouse:
-            self.success_url = reverse_lazy("list-webapp-datasources")
-
-        response = super().delete(*args, **kwargs)
+    def get_success_url(self):
+        if not self.object.is_data_warehouse:
+            return reverse_lazy("list-webapp-datasources")
+        return self.success_url
 
+    def form_valid(self, *args, **kwargs):
+        self.object = self.get_object()
+        self.object.soft_delete(deleted_by=self.request.user)
         messages.success(self.request, "Successfully deleted data source")
-
-        return response
+        return HttpResponseRedirect(self.get_success_url())
 
 
 class UpdateAccessLevelMixin:
diff --git a/tests/api/models/test_s3bucket.py b/tests/api/models/test_s3bucket.py
index 9d9d89f95..deb17e0aa 100644
--- a/tests/api/models/test_s3bucket.py
+++ b/tests/api/models/test_s3bucket.py
@@ -96,3 +96,16 @@ def test_is_folder(name, expected):
 )
 def test_cluster(name, expected):
     assert isinstance(S3Bucket(name=name).cluster, expected)
+
+
+def test_soft_delete(bucket, users):
+    user = users["superuser"]
+
+    assert bucket.is_deleted is False
+    with patch("controlpanel.api.cluster.S3Bucket.mark_for_archival") as archive:
+        bucket.soft_delete(deleted_by=user)
+
+    assert bucket.is_deleted is True
+    assert bucket.deleted_by == user
+    assert bucket.deleted_at is not None
+    archive.assert_called_once()
diff --git a/tests/frontend/views/test_datasource.py b/tests/frontend/views/test_datasource.py
index 1f43f02a0..dc1f05658 100644
--- a/tests/frontend/views/test_datasource.py
+++ b/tests/frontend/views/test_datasource.py
@@ -3,7 +3,7 @@
 
 # Third-party
 import pytest
-from django.urls import reverse
+from django.urls import reverse, reverse_lazy
 from model_mommy import mommy
 from rest_framework import status
 
@@ -115,9 +115,10 @@ def create(client, *args, **kwargs):
     return client.post(reverse("create-datasource") + "?type=warehouse", data)
 
 
-def delete(client, buckets, *args):
-    return client.delete(
-        reverse("delete-datasource", kwargs={"pk": buckets["warehouse1"].id})
+def delete(client, buckets, *args, bucket=None):
+    bucket = bucket or buckets["warehouse1"]
+    return client.post(
+        reverse("delete-datasource", kwargs={"pk": bucket.id})
     )
 
 
@@ -392,3 +393,25 @@ def test_grant_access_invalid_form(client, users3buckets, users, kwargs):
 
     assert response.status_code == 200
     assert response.context_data["form"].is_valid() is False
+
+
+@pytest.mark.parametrize(
+    "bucket, success_url",
+    [
+        ("warehouse1", reverse_lazy("list-warehouse-datasources")),
+        ("app_data1", reverse_lazy("list-webapp-datasources")),
+    ]
+)
+def test_delete_calls_soft_delete(client, buckets, users, bucket, success_url):
+    admin = users["bucket_admin"]
+    bucket = buckets[bucket]
+
+    client.force_login(admin)
+    response = delete(client, buckets, bucket=bucket)
+    bucket.refresh_from_db()
+
+    assert bucket.pk is not None
+    assert bucket.is_deleted is True
+    assert bucket.deleted_by == admin
+    assert bucket.deleted_at is not None
+    assert response.url == success_url

From 30430f8ebcb1ea36d7e68d7b8dcc110e231e7f41 Mon Sep 17 00:00:00 2001
From: Michael Collins <15347726+michaeljcollinsuk@users.noreply.github.com>
Date: Tue, 10 Oct 2023 17:37:17 +0100
Subject: [PATCH 03/10] Update datasource list views and templates

- For admins, list deleted datasources
- Fix bugs in templates so that admin view displays datasource type
---
 controlpanel/frontend/forms.py                |  8 ++++---
 .../frontend/jinja2/datasource-list.html      | 13 +++++++----
 controlpanel/frontend/views/datasource.py     | 13 +++++++++--
 tests/frontend/views/test_datasource.py       | 23 ++++++++++---------
 4 files changed, 37 insertions(+), 20 deletions(-)

diff --git a/controlpanel/frontend/forms.py b/controlpanel/frontend/forms.py
index 9341dd333..4fe857c1c 100644
--- a/controlpanel/frontend/forms.py
+++ b/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):
diff --git a/controlpanel/frontend/jinja2/datasource-list.html b/controlpanel/frontend/jinja2/datasource-list.html
index 8e2027d88..7c575bc6a 100644
--- a/controlpanel/frontend/jinja2/datasource-list.html
+++ b/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 %}
diff --git a/controlpanel/frontend/views/datasource.py b/controlpanel/frontend/views/datasource.py
index 6572b4bdd..8e156e770 100644
--- a/controlpanel/frontend/views/datasource.py
+++ b/controlpanel/frontend/views/datasource.py
@@ -45,7 +45,7 @@ class DatasourceMixin(ContextMixin):
 
     def get_context_data(self, *args, **kwargs):
         context = super().get_context_data(*args, **kwargs)
-        context["all-datasources"] = self.all_datasources
+        context["all_datasources"] = self.all_datasources
         context["datasource_type"] = self.get_datasource_type()
         return context
 
@@ -76,6 +76,7 @@ def get_queryset(self):
         return S3Bucket.objects.prefetch_related("users3buckets").filter(
             is_data_warehouse=self.datasource_type == "warehouse",
             users3buckets__user=self.request.user,
+            is_deleted=False,
         )
 
     def get_context_data(self, *args, **kwargs):
@@ -85,6 +86,7 @@ def get_context_data(self, *args, **kwargs):
             "users3buckets__user"
         ).filter(
             is_data_warehouse=self.datasource_type == "warehouse",
+            is_deleted=False,
         )
         other_datasources = all_datasources.exclude(id__in=self.get_queryset())
         other_datasources_admins = {}
@@ -104,7 +106,14 @@ class AdminBucketList(BucketList):
     permission_required = "api.is_superuser"
 
     def get_queryset(self):
-        return S3Bucket.objects.prefetch_related("users3buckets").all()
+        return S3Bucket.objects.prefetch_related("users3buckets").filter(
+            is_deleted=False
+        )
+
+    def get_context_data(self, *args, **kwargs):
+        context = super().get_context_data(*args, **kwargs)
+        context["deleted_datasources"] = S3Bucket.objects.filter(is_deleted=True)
+        return context
 
 
 class WebappBucketList(BucketList):
diff --git a/tests/frontend/views/test_datasource.py b/tests/frontend/views/test_datasource.py
index dc1f05658..53459a57d 100644
--- a/tests/frontend/views/test_datasource.py
+++ b/tests/frontend/views/test_datasource.py
@@ -214,23 +214,24 @@ def test_access_permissions(client, users3buckets, users, view, user, expected_s
 
 
 @pytest.mark.parametrize(
-    "view,user,expected_count",
+    "view,user,expected_count,show_deleted",
     [
-        (list_warehouse, "superuser", 0),
-        (list_warehouse, "normal_user", 0),
-        (list_warehouse, "bucket_viewer", 1),
-        (list_warehouse, "bucket_admin", 2),
-        (list_app_data, "superuser", 0),
-        (list_app_data, "normal_user", 0),
-        (list_app_data, "bucket_viewer", 1),
-        (list_app_data, "bucket_admin", 2),
-        (list_all, "superuser", 5),
+        (list_warehouse, "superuser", 0, False),
+        (list_warehouse, "normal_user", 0, False),
+        (list_warehouse, "bucket_viewer", 1, False),
+        (list_warehouse, "bucket_admin", 2, False),
+        (list_app_data, "superuser", 0, False),
+        (list_app_data, "normal_user", 0, False),
+        (list_app_data, "bucket_viewer", 1, False),
+        (list_app_data, "bucket_admin", 2, False),
+        (list_all, "superuser", 5, True),
     ],
 )
-def test_list(client, buckets, users, view, user, expected_count):
+def test_list(client, buckets, users, view, user, expected_count, show_deleted):
     client.force_login(users[user])
     response = view(client, buckets, users)
     assert len(response.context_data["object_list"]) == expected_count
+    assert ("deleted_datasources" in response.context_data) is show_deleted
 
 
 @pytest.mark.parametrize(

From 9c6fc906ac4b1440b826e3220c45e377545dc241 Mon Sep 17 00:00:00 2001
From: Michael Collins <15347726+michaeljcollinsuk@users.noreply.github.com>
Date: Wed, 11 Oct 2023 15:54:07 +0100
Subject: [PATCH 04/10] Add soft delete fields to the S3BucketSerializer

---
 controlpanel/api/serializers.py  | 6 ++++++
 tests/api/views/test_s3bucket.py | 3 +++
 2 files changed, 9 insertions(+)

diff --git a/controlpanel/api/serializers.py b/controlpanel/api/serializers.py
index ac17f4114..7761379c4 100644
--- a/controlpanel/api/serializers.py
+++ b/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",
         )
 
 
diff --git a/tests/api/views/test_s3bucket.py b/tests/api/views/test_s3bucket.py
index 29da0ca30..00e3baa85 100644
--- a/tests/api/views/test_s3bucket.py
+++ b/tests/api/views/test_s3bucket.py
@@ -54,6 +54,9 @@ def test_detail(client, bucket):
         "created_by",
         "is_data_warehouse",
         "location_url",
+        "is_deleted",
+        "deleted_by",
+        "deleted_at",
     }
     assert set(response.data) == expected_s3bucket_fields
 

From 7c01998c10318d690aaf7f9d0aa1caf6e6581cf8 Mon Sep 17 00:00:00 2001
From: Michael Collins <15347726+michaeljcollinsuk@users.noreply.github.com>
Date: Wed, 11 Oct 2023 17:16:27 +0100
Subject: [PATCH 05/10] Update datasource detail page only accessible to
 superuser when deleted

---
 .../frontend/jinja2/datasource-detail.html    | 13 +++++--
 controlpanel/frontend/views/datasource.py     |  6 +++
 tests/frontend/views/test_datasource.py       | 37 ++++++++++++++++++-
 3 files changed, 51 insertions(+), 5 deletions(-)

diff --git a/controlpanel/frontend/jinja2/datasource-detail.html b/controlpanel/frontend/jinja2/datasource-detail.html
index 0c133010a..d7ac893f2 100644
--- a/controlpanel/frontend/jinja2/datasource-detail.html
+++ b/controlpanel/frontend/jinja2/datasource-detail.html
@@ -20,9 +20,15 @@
 <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 %}
+    <p>
+      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") }}.
+    </p>
+  {% 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">
@@ -212,6 +218,7 @@ <h2 class="govuk-heading-m">Data access log</h2>
 </section>
 {% endif %}
 
+<!-- TODO replace with a restore button when bucket has been soft-deleted -->
 {% if request.user.has_perm('api.destroy_s3bucket', bucket) %}
 <section class="cpanel-section">
   <form action="{{ url('delete-datasource', kwargs={ "pk": bucket.id }) }}" method="post">
diff --git a/controlpanel/frontend/views/datasource.py b/controlpanel/frontend/views/datasource.py
index 8e156e770..94c79b12a 100644
--- a/controlpanel/frontend/views/datasource.py
+++ b/controlpanel/frontend/views/datasource.py
@@ -131,6 +131,12 @@ class BucketDetail(
     permission_required = "api.retrieve_s3bucket"
     template_name = "datasource-detail.html"
 
+    def get_queryset(self):
+        queryset = super().get_queryset()
+        if not self.request.user.is_superuser:
+            queryset = queryset.filter(is_deleted=False)
+        return queryset
+
     def get_context_data(self, *args, **kwargs):
         context = super().get_context_data(*args, **kwargs)
         bucket = kwargs["object"]
diff --git a/tests/frontend/views/test_datasource.py b/tests/frontend/views/test_datasource.py
index 53459a57d..464f3ece9 100644
--- a/tests/frontend/views/test_datasource.py
+++ b/tests/frontend/views/test_datasource.py
@@ -101,9 +101,10 @@ def list_all(client, *args):
     return client.get(reverse("list-all-datasources"))
 
 
-def detail(client, buckets, *args):
+def detail(client, buckets, *args, bucket=None):
+    bucket = bucket or buckets["warehouse1"]
     return client.get(
-        reverse("manage-datasource", kwargs={"pk": buckets["warehouse1"].id})
+        reverse("manage-datasource", kwargs={"pk": bucket.id})
     )
 
 
@@ -416,3 +417,35 @@ def test_delete_calls_soft_delete(client, buckets, users, bucket, success_url):
     assert bucket.deleted_by == admin
     assert bucket.deleted_at is not None
     assert response.url == success_url
+
+
+@pytest.mark.parametrize(
+    "user, bucket, expected_status",
+    [
+        ("superuser", "app_data1", status.HTTP_200_OK),
+        ("superuser", "app_data2", status.HTTP_200_OK),
+        ("superuser", "warehouse1", status.HTTP_200_OK),
+        ("superuser", "warehouse2", status.HTTP_200_OK),
+        ("superuser", "other", status.HTTP_200_OK),
+        ("bucket_viewer", "app_data1", status.HTTP_404_NOT_FOUND),
+        ("bucket_viewer", "app_data2", status.HTTP_404_NOT_FOUND),
+        ("bucket_viewer", "warehouse1", status.HTTP_404_NOT_FOUND),
+        ("bucket_viewer", "warehouse2", status.HTTP_404_NOT_FOUND),
+        ("bucket_viewer", "other", status.HTTP_404_NOT_FOUND),
+        ("bucket_admin", "app_data1", status.HTTP_404_NOT_FOUND),
+        ("bucket_admin", "app_data2", status.HTTP_404_NOT_FOUND),
+        ("bucket_admin", "warehouse1", status.HTTP_404_NOT_FOUND),
+        ("bucket_admin", "warehouse2", status.HTTP_404_NOT_FOUND),
+        ("bucket_admin", "other", status.HTTP_404_NOT_FOUND),
+
+    ]
+)
+def test_detail_for_deleted_datasouce(client, buckets, users, user, bucket, expected_status):
+    user = users[user]
+    bucket = buckets[bucket]
+    bucket.soft_delete(deleted_by=user)
+
+    client.force_login(user)
+    response = detail(client, user, bucket=bucket)
+
+    assert response.status_code == expected_status

From 4e6de07ebb4d040ff93eef91a2e1f0ffcf6796d7 Mon Sep 17 00:00:00 2001
From: Michael Collins <15347726+michaeljcollinsuk@users.noreply.github.com>
Date: Fri, 13 Oct 2023 14:43:42 +0100
Subject: [PATCH 06/10] Add task to revoke all access to a S3bucket when it is
 soft deleted

---
 controlpanel/api/models/s3bucket.py           |  3 +++
 controlpanel/api/tasks/handlers/__init__.py   |  2 ++
 controlpanel/api/tasks/handlers/s3.py         | 21 +++++++++++++++++-
 controlpanel/api/tasks/s3bucket.py            | 13 +++++++++++
 .../frontend/jinja2/datasource-detail.html    | 18 ++++++++++-----
 tests/api/models/test_s3bucket.py             |  7 +++++-
 tests/api/tasks/test_s3.py                    | 22 ++++++++++++++++++-
 tests/frontend/views/test_datasource.py       | 14 ++++++++++--
 8 files changed, 89 insertions(+), 11 deletions(-)

diff --git a/controlpanel/api/models/s3bucket.py b/controlpanel/api/models/s3bucket.py
index 8d72ed467..9d508b905 100644
--- a/controlpanel/api/models/s3bucket.py
+++ b/controlpanel/api/models/s3bucket.py
@@ -15,6 +15,7 @@
 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):
@@ -180,3 +181,5 @@ def soft_delete(self, deleted_by: User):
         # TODO update to handle deleting folders
         if not self.is_folder:
             self.cluster.mark_for_archival()
+
+        S3BucketRevokeAllAccess(self, self.deleted_by).create_task()
diff --git a/controlpanel/api/tasks/handlers/__init__.py b/controlpanel/api/tasks/handlers/__init__.py
index 454fd9094..f51323c79 100644
--- a/controlpanel/api/tasks/handlers/__init__.py
+++ b/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())
diff --git a/controlpanel/api/tasks/handlers/s3.py b/controlpanel/api/tasks/handlers/s3.py
index fd8fe922e..f37e90b19 100644
--- a/controlpanel/api/tasks/handlers/s3.py
+++ b/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,21 @@ 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
+        """
+        collector = Collector(using="default")
+        collector.collect([self.object])
+        for model, instance in collector.instances_with_model():
+            if not issubclass(model, AccessToS3Bucket):
+                continue
+            instance.revoke_bucket_access()
diff --git a/controlpanel/api/tasks/s3bucket.py b/controlpanel/api/tasks/s3bucket.py
index c990ca7a1..4e3e8944d 100644
--- a/controlpanel/api/tasks/s3bucket.py
+++ b/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
diff --git a/controlpanel/frontend/jinja2/datasource-detail.html b/controlpanel/frontend/jinja2/datasource-detail.html
index d7ac893f2..24c34a158 100644
--- a/controlpanel/frontend/jinja2/datasource-detail.html
+++ b/controlpanel/frontend/jinja2/datasource-detail.html
@@ -21,9 +21,15 @@ <h1 class="govuk-heading-xl">{{ page_title }}</h1>
 
 <p class="govuk-body">
   {% if bucket.is_deleted %}
-    <p>
-      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") }}.
-    </p>
+    <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
@@ -69,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
@@ -89,7 +95,7 @@ <h2 class="govuk-heading-m">Users and groups with access</h2>
     </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
@@ -97,7 +103,7 @@ <h2 class="govuk-heading-m">Users and groups with access</h2>
   {% 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
diff --git a/tests/api/models/test_s3bucket.py b/tests/api/models/test_s3bucket.py
index deb17e0aa..67dcd6db3 100644
--- a/tests/api/models/test_s3bucket.py
+++ b/tests/api/models/test_s3bucket.py
@@ -98,7 +98,7 @@ def test_cluster(name, expected):
     assert isinstance(S3Bucket(name=name).cluster, expected)
 
 
-def test_soft_delete(bucket, users):
+def test_soft_delete(bucket, users, sqs, helpers):
     user = users["superuser"]
 
     assert bucket.is_deleted is False
@@ -109,3 +109,8 @@ def test_soft_delete(bucket, users):
     assert bucket.deleted_by == user
     assert bucket.deleted_at is not None
     archive.assert_called_once()
+
+    messages = helpers.retrieve_messages(sqs, queue_name=settings.S3_QUEUE_NAME)
+    helpers.validate_task_with_sqs_messages(
+        messages, S3Bucket.__name__, bucket.id, queue_name=settings.S3_QUEUE_NAME,
+    )
diff --git a/tests/api/tasks/test_s3.py b/tests/api/tasks/test_s3.py
index dc275accf..88b329dd4 100644
--- a/tests/api/tasks/test_s3.py
+++ b/tests/api/tasks/test_s3.py
@@ -1,6 +1,8 @@
+# Standard library
+from unittest.mock import MagicMock, patch
+
 # Third-party
 import pytest
-from mock import patch
 from model_mommy import mommy
 
 # First-party/Local
@@ -9,6 +11,7 @@
     create_s3bucket,
     grant_app_s3bucket_access,
     grant_user_s3bucket_access,
+    revoke_all_access_s3bucket,
     revoke_app_s3bucket_access,
     revoke_user_s3bucket_access,
 )
@@ -112,3 +115,20 @@ def test_revoke_app_access(cluster, complete):
         app_bucket_access.s3bucket.arn
     )
     complete.assert_called_once()
+
+
+@pytest.mark.django_db
+@patch("controlpanel.api.models.UserS3Bucket.revoke_bucket_access", new=MagicMock())
+@patch("controlpanel.api.models.AppS3Bucket.revoke_bucket_access", new=MagicMock())
+@patch("controlpanel.api.models.PolicyS3Bucket.revoke_bucket_access", new=MagicMock())
+def test_revoke_all_access():
+    bucket = mommy.make("api.S3Bucket")
+    user_access = mommy.make("api.UserS3Bucket", s3bucket=bucket)
+    app_access = mommy.make("api.AppS3Bucket", s3bucket=bucket)
+    policy_access = mommy.make("api.PolicyS3Bucket", s3bucket=bucket)
+
+    revoke_all_access_s3bucket(bucket.pk, bucket.created_by)
+
+    user_access.revoke_bucket_access.assert_called_once()
+    app_access.revoke_bucket_access.assert_called_once()
+    policy_access.revoke_bucket_access.assert_called_once()
diff --git a/tests/frontend/views/test_datasource.py b/tests/frontend/views/test_datasource.py
index 464f3ece9..2f4a892ac 100644
--- a/tests/frontend/views/test_datasource.py
+++ b/tests/frontend/views/test_datasource.py
@@ -3,6 +3,7 @@
 
 # Third-party
 import pytest
+from django.conf import settings
 from django.urls import reverse, reverse_lazy
 from model_mommy import mommy
 from rest_framework import status
@@ -404,7 +405,9 @@ def test_grant_access_invalid_form(client, users3buckets, users, kwargs):
         ("app_data1", reverse_lazy("list-webapp-datasources")),
     ]
 )
-def test_delete_calls_soft_delete(client, buckets, users, bucket, success_url):
+def test_delete_calls_soft_delete(
+    client, buckets, users, bucket, success_url, sqs, helpers,
+):
     admin = users["bucket_admin"]
     bucket = buckets[bucket]
 
@@ -418,6 +421,11 @@ def test_delete_calls_soft_delete(client, buckets, users, bucket, success_url):
     assert bucket.deleted_at is not None
     assert response.url == success_url
 
+    messages = helpers.retrieve_messages(sqs, queue_name=settings.S3_QUEUE_NAME)
+    helpers.validate_task_with_sqs_messages(
+        messages, S3Bucket.__name__, bucket.id, queue_name=settings.S3_QUEUE_NAME,
+    )
+
 
 @pytest.mark.parametrize(
     "user, bucket, expected_status",
@@ -440,7 +448,9 @@ def test_delete_calls_soft_delete(client, buckets, users, bucket, success_url):
 
     ]
 )
-def test_detail_for_deleted_datasouce(client, buckets, users, user, bucket, expected_status):
+def test_detail_for_deleted_datasource(
+    client, buckets, users, user, bucket, expected_status
+):
     user = users[user]
     bucket = buckets[bucket]
     bucket.soft_delete(deleted_by=user)

From fd2dc39a1bbd6e5e28ef456b567d9edef11b67e8 Mon Sep 17 00:00:00 2001
From: Michael Collins <15347726+michaeljcollinsuk@users.noreply.github.com>
Date: Tue, 17 Oct 2023 16:19:55 +0100
Subject: [PATCH 07/10] Store the user that revokes all access to a bucket

---
 controlpanel/api/models/apps3bucket.py    |  5 +++--
 controlpanel/api/models/policys3bucket.py |  3 ++-
 controlpanel/api/models/users3bucket.py   |  5 +++--
 controlpanel/api/tasks/handlers/s3.py     |  4 +++-
 tests/api/tasks/test_s3.py                | 17 ++++++++++++-----
 5 files changed, 23 insertions(+), 11 deletions(-)

diff --git a/controlpanel/api/models/apps3bucket.py b/controlpanel/api/models/apps3bucket.py
index 9cd5d44d4..33078e797 100644
--- a/controlpanel/api/models/apps3bucket.py
+++ b/controlpanel/api/models/apps3bucket.py
@@ -43,5 +43,6 @@ def __repr__(self):
     def grant_bucket_access(self):
         tasks.S3BucketGrantToApp(self, self.current_user).create_task()
 
-    def revoke_bucket_access(self):
-        tasks.S3BucketRevokeAppAccess(self, self.current_user).create_task()
+    def revoke_bucket_access(self, revoked_by=None):
+        revoked_by = revoked_by or None
+        tasks.S3BucketRevokeAppAccess(self, revoked_by).create_task()
diff --git a/controlpanel/api/models/policys3bucket.py b/controlpanel/api/models/policys3bucket.py
index 6781f279e..a0c6b29ec 100644
--- a/controlpanel/api/models/policys3bucket.py
+++ b/controlpanel/api/models/policys3bucket.py
@@ -38,7 +38,8 @@ def grant_bucket_access(self):
             self.resources,
         )
 
-    def revoke_bucket_access(self):
+    def revoke_bucket_access(self, revoked_by=None):
+        # 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
diff --git a/controlpanel/api/models/users3bucket.py b/controlpanel/api/models/users3bucket.py
index 7e5cfbec3..2f9d00d4e 100644
--- a/controlpanel/api/models/users3bucket.py
+++ b/controlpanel/api/models/users3bucket.py
@@ -47,8 +47,9 @@ def __repr__(self):
     def grant_bucket_access(self):
         tasks.S3BucketGrantToUser(self, self.current_user).create_task()
 
-    def revoke_bucket_access(self):
+    def revoke_bucket_access(self, revoked_by=None):
         # TODO when soft delete is added, this should be updated to use the user that
         # has deleted the parent S3bucket to ensure we store the user that has sent the
         # task in the case of cascading deletes
-        tasks.S3BucketRevokeUserAccess(self, self.current_user).create_task()
+        revoked_by = revoked_by or self.current_user
+        tasks.S3BucketRevokeUserAccess(self, revoked_by).create_task()
diff --git a/controlpanel/api/tasks/handlers/s3.py b/controlpanel/api/tasks/handlers/s3.py
index f37e90b19..b748031a1 100644
--- a/controlpanel/api/tasks/handlers/s3.py
+++ b/controlpanel/api/tasks/handlers/s3.py
@@ -86,9 +86,11 @@ def handle(self, *args, **kwargs):
         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.revoke_bucket_access()
+
+            instance.revoke_bucket_access(revoked_by=task_user)
diff --git a/tests/api/tasks/test_s3.py b/tests/api/tasks/test_s3.py
index 88b329dd4..20331f4f6 100644
--- a/tests/api/tasks/test_s3.py
+++ b/tests/api/tasks/test_s3.py
@@ -121,14 +121,21 @@ def test_revoke_app_access(cluster, complete):
 @patch("controlpanel.api.models.UserS3Bucket.revoke_bucket_access", new=MagicMock())
 @patch("controlpanel.api.models.AppS3Bucket.revoke_bucket_access", new=MagicMock())
 @patch("controlpanel.api.models.PolicyS3Bucket.revoke_bucket_access", new=MagicMock())
-def test_revoke_all_access():
+def test_revoke_all_access(users):
     bucket = mommy.make("api.S3Bucket")
     user_access = mommy.make("api.UserS3Bucket", s3bucket=bucket)
     app_access = mommy.make("api.AppS3Bucket", s3bucket=bucket)
     policy_access = mommy.make("api.PolicyS3Bucket", s3bucket=bucket)
+    task = mommy.make("api.Task", user_id=users["superuser"].pk)
 
-    revoke_all_access_s3bucket(bucket.pk, bucket.created_by)
+    revoke_all_access_s3bucket(bucket.pk, task.user_id)
 
-    user_access.revoke_bucket_access.assert_called_once()
-    app_access.revoke_bucket_access.assert_called_once()
-    policy_access.revoke_bucket_access.assert_called_once()
+    user_access.revoke_bucket_access.assert_called_once_with(
+        revoked_by=users["superuser"],
+    )
+    app_access.revoke_bucket_access.assert_called_once_with(
+        revoked_by=users["superuser"],
+    )
+    policy_access.revoke_bucket_access.assert_called_once_with(
+        revoked_by=users["superuser"],
+    )

From 4393815cef1ea1d4c869a4afd7dbf8104cdf9774 Mon Sep 17 00:00:00 2001
From: Michael Collins <15347726+michaeljcollinsuk@users.noreply.github.com>
Date: Tue, 17 Oct 2023 17:24:28 +0100
Subject: [PATCH 08/10] Update S3Bucket admin to use is_deleted

---
 controlpanel/api/admin.py | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/controlpanel/api/admin.py b/controlpanel/api/admin.py
index c64022b71..22d6bcf05 100644
--- a/controlpanel/api/admin.py
+++ b/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",)
 
 

From 69b7fbaa07131a5895419eccc999119205b24c20 Mon Sep 17 00:00:00 2001
From: Michael Collins <15347726+michaeljcollinsuk@users.noreply.github.com>
Date: Wed, 18 Oct 2023 09:49:31 +0100
Subject: [PATCH 09/10] Update datasource detail template to improve display
 when bucket deleted

---
 controlpanel/frontend/jinja2/datasource-detail.html | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/controlpanel/frontend/jinja2/datasource-detail.html b/controlpanel/frontend/jinja2/datasource-detail.html
index 24c34a158..82106174a 100644
--- a/controlpanel/frontend/jinja2/datasource-detail.html
+++ b/controlpanel/frontend/jinja2/datasource-detail.html
@@ -38,7 +38,7 @@ <h1 class="govuk-heading-xl">{{ page_title }}</h1>
 </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">
@@ -85,11 +85,13 @@ <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>
@@ -224,8 +226,7 @@ <h2 class="govuk-heading-m">Data access log</h2>
 </section>
 {% endif %}
 
-<!-- TODO replace with a restore button when bucket has been soft-deleted -->
-{% 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 }}

From e075f926cc031802291183ce9a94754f5c196391 Mon Sep 17 00:00:00 2001
From: Michael Collins <15347726+michaeljcollinsuk@users.noreply.github.com>
Date: Wed, 18 Oct 2023 17:06:33 +0100
Subject: [PATCH 10/10] Remove revoked_by arg

---
 controlpanel/api/models/apps3bucket.py    |  5 ++---
 controlpanel/api/models/policys3bucket.py |  4 ++--
 controlpanel/api/models/users3bucket.py   |  5 ++---
 controlpanel/api/tasks/handlers/s3.py     |  3 ++-
 tests/api/tasks/test_s3.py                | 12 +++---------
 5 files changed, 11 insertions(+), 18 deletions(-)

diff --git a/controlpanel/api/models/apps3bucket.py b/controlpanel/api/models/apps3bucket.py
index 33078e797..9cd5d44d4 100644
--- a/controlpanel/api/models/apps3bucket.py
+++ b/controlpanel/api/models/apps3bucket.py
@@ -43,6 +43,5 @@ def __repr__(self):
     def grant_bucket_access(self):
         tasks.S3BucketGrantToApp(self, self.current_user).create_task()
 
-    def revoke_bucket_access(self, revoked_by=None):
-        revoked_by = revoked_by or None
-        tasks.S3BucketRevokeAppAccess(self, revoked_by).create_task()
+    def revoke_bucket_access(self):
+        tasks.S3BucketRevokeAppAccess(self, self.current_user).create_task()
diff --git a/controlpanel/api/models/policys3bucket.py b/controlpanel/api/models/policys3bucket.py
index a0c6b29ec..feb7e4299 100644
--- a/controlpanel/api/models/policys3bucket.py
+++ b/controlpanel/api/models/policys3bucket.py
@@ -38,8 +38,8 @@ def grant_bucket_access(self):
             self.resources,
         )
 
-    def revoke_bucket_access(self, revoked_by=None):
-        # TODO update to use a Task to revoke access, to match user/app access{{
+    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
diff --git a/controlpanel/api/models/users3bucket.py b/controlpanel/api/models/users3bucket.py
index 2f9d00d4e..7e5cfbec3 100644
--- a/controlpanel/api/models/users3bucket.py
+++ b/controlpanel/api/models/users3bucket.py
@@ -47,9 +47,8 @@ def __repr__(self):
     def grant_bucket_access(self):
         tasks.S3BucketGrantToUser(self, self.current_user).create_task()
 
-    def revoke_bucket_access(self, revoked_by=None):
+    def revoke_bucket_access(self):
         # TODO when soft delete is added, this should be updated to use the user that
         # has deleted the parent S3bucket to ensure we store the user that has sent the
         # task in the case of cascading deletes
-        revoked_by = revoked_by or self.current_user
-        tasks.S3BucketRevokeUserAccess(self, revoked_by).create_task()
+        tasks.S3BucketRevokeUserAccess(self, self.current_user).create_task()
diff --git a/controlpanel/api/tasks/handlers/s3.py b/controlpanel/api/tasks/handlers/s3.py
index b748031a1..69b7ab0d7 100644
--- a/controlpanel/api/tasks/handlers/s3.py
+++ b/controlpanel/api/tasks/handlers/s3.py
@@ -93,4 +93,5 @@ def handle(self, *args, **kwargs):
             if not issubclass(model, AccessToS3Bucket):
                 continue
 
-            instance.revoke_bucket_access(revoked_by=task_user)
+            instance.current_user = task_user
+            instance.revoke_bucket_access()
diff --git a/tests/api/tasks/test_s3.py b/tests/api/tasks/test_s3.py
index 20331f4f6..368283731 100644
--- a/tests/api/tasks/test_s3.py
+++ b/tests/api/tasks/test_s3.py
@@ -130,12 +130,6 @@ def test_revoke_all_access(users):
 
     revoke_all_access_s3bucket(bucket.pk, task.user_id)
 
-    user_access.revoke_bucket_access.assert_called_once_with(
-        revoked_by=users["superuser"],
-    )
-    app_access.revoke_bucket_access.assert_called_once_with(
-        revoked_by=users["superuser"],
-    )
-    policy_access.revoke_bucket_access.assert_called_once_with(
-        revoked_by=users["superuser"],
-    )
+    user_access.revoke_bucket_access.assert_called_once()
+    app_access.revoke_bucket_access.assert_called_once()
+    policy_access.revoke_bucket_access.assert_called_once()