Store the user that revokes all access to a bucket

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()

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

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()

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)

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"],
+    )