Set user before creating ActivityLogs in Cinder webhook (#21510)

* Set user before creating ActivityLogs in Cinder webhook

Otherwise the activities don't get created at all since there is
no user. We need those to be recorded with the task user.

* Fix for Escalation action

src/olympia/abuse/tests/test_utils.py

@@ -2,6 +2,7 @@ from django.conf import settings
 
 from olympia import amo
 from olympia.abuse.models import AbuseReport, CinderJob
+from olympia.activity.models import ActivityLog
 from olympia.amo.tests import (
     TestCase,
     addon_factory,
@@ -9,6 +10,7 @@ from olympia.amo.tests import (
     user_factory,
     version_factory,
 )
+from olympia.core import set_user
 from olympia.ratings.models import Rating
 from olympia.reviewers.models import NeedsHumanReview
 
@@ -36,6 +38,10 @@ class TestCinderAction(TestCase):
             guid='1234',
             cinder_job=self.cinder_job,
         )
+        self.task_user = user_factory(pk=settings.TASK_USER_ID)
+        # It's the webhook's responsability to do this before calling the
+        # action. We need it for the ActivityLog creation to work.
+        set_user(self.task_user)
 
     def test_ban_user(self):
         user = user_factory()
@@ -44,6 +50,10 @@ class TestCinderAction(TestCase):
         action.process()
         user.reload()
         self.assertCloseToNow(user.banned)
+        assert ActivityLog.objects.count() == 1
+        activity = ActivityLog.objects.get(action=amo.LOG.ADMIN_USER_BANNED.id)
+        assert activity.arguments == [user]
+        assert activity.user == self.task_user
 
     def test_approve_user(self):
         user = user_factory(banned=self.days_ago(1), deleted=True)
@@ -52,35 +62,51 @@ class TestCinderAction(TestCase):
         action.process()
         user.reload()
         assert not user.banned
+        assert ActivityLog.objects.count() == 1
+        activity = ActivityLog.objects.get(action=amo.LOG.ADMIN_USER_UNBAN.id)
+        assert activity.arguments == [user]
+        assert activity.user == self.task_user
 
     def test_disable_addon(self):
         addon = addon_factory()
+        ActivityLog.objects.all().delete()
         self.cinder_job.abusereport_set.update(guid=addon.guid)
         action = CinderActionDisableAddon(self.cinder_job)
         action.process()
         assert addon.reload().status == amo.STATUS_DISABLED
+        assert ActivityLog.objects.count() == 1
+        activity = ActivityLog.objects.get(action=amo.LOG.FORCE_DISABLE.id)
+        assert activity.arguments == [addon]
+        assert activity.user == self.task_user
 
     def test_approve_appeal_addon(self):
         addon = addon_factory(status=amo.STATUS_DISABLED)
+        ActivityLog.objects.all().delete()
         self.cinder_job.abusereport_set.update(guid=addon.guid)
         action = CinderActionApproveAppealOverride(self.cinder_job)
         action.process()
         assert addon.reload().status == amo.STATUS_NULL
+        assert ActivityLog.objects.count() == 2  # Extra because of status change.
+        activity = ActivityLog.objects.get(action=amo.LOG.FORCE_ENABLE.id)
+        assert activity.arguments == [addon]
+        assert activity.user == self.task_user
 
     def test_approve_initial_addon(self):
         addon = addon_factory(status=amo.STATUS_DISABLED)
+        ActivityLog.objects.all().delete()
         self.cinder_job.abusereport_set.update(guid=addon.guid)
         action = CinderActionApproveInitialDecision(self.cinder_job)
         action.process()
         assert addon.reload().status == amo.STATUS_DISABLED
+        assert ActivityLog.objects.count() == 0
 
     def test_escalate_addon(self):
-        user_factory(id=settings.TASK_USER_ID)
         addon = addon_factory(file_kw={'is_signed': True})
         listed_version = addon.current_version
         unlisted_version = version_factory(
             addon=addon, channel=amo.CHANNEL_UNLISTED, file_kw={'is_signed': True}
         )
+        ActivityLog.objects.all().delete()
         self.cinder_job.abusereport_set.update(guid=addon.guid)
         action = CinderActionEscalateAddon(self.cinder_job)
         action.process()
@@ -93,20 +119,41 @@ class TestCinderAction(TestCase):
             unlisted_version.reload().needshumanreview_set.get().reason
             == NeedsHumanReview.REASON_CINDER_ESCALATION
         )
+        assert ActivityLog.objects.count() == 2
+        activity = ActivityLog.objects.filter(
+            action=amo.LOG.NEEDS_HUMAN_REVIEW_AUTOMATIC.id
+        ).order_by('pk')[0]
+        assert activity.arguments == [listed_version]
+        assert activity.user == self.task_user
+        activity = ActivityLog.objects.filter(
+            action=amo.LOG.NEEDS_HUMAN_REVIEW_AUTOMATIC.id
+        ).order_by('pk')[1]
+        assert activity.arguments == [unlisted_version]
+        assert activity.user == self.task_user
 
         # but if we have a version specified, we flag that version
         NeedsHumanReview.objects.all().delete()
         other_version = version_factory(
             addon=addon, file_kw={'status': amo.STATUS_DISABLED, 'is_signed': True}
         )
+        assert not other_version.due_date
+        ActivityLog.objects.all().delete()
         self.cinder_job.abusereport_set.update(addon_version=other_version.version)
         action.process()
         assert not listed_version.reload().needshumanreview_set.exists()
         assert not unlisted_version.reload().needshumanreview_set.exists()
+        other_version.reload()
+        assert other_version.due_date
         assert (
-            other_version.reload().needshumanreview_set.get().reason
+            other_version.needshumanreview_set.get().reason
             == NeedsHumanReview.REASON_CINDER_ESCALATION
         )
+        assert ActivityLog.objects.count() == 1
+        activity = ActivityLog.objects.get(
+            action=amo.LOG.NEEDS_HUMAN_REVIEW_AUTOMATIC.id
+        )
+        assert activity.arguments == [other_version]
+        assert activity.user == self.task_user
 
     def test_delete_collection(self):
         collection = collection_factory(author=user_factory())
@@ -116,6 +163,10 @@ class TestCinderAction(TestCase):
         assert collection.reload()
         assert collection.deleted
         assert collection.slug
+        assert ActivityLog.objects.count() == 1
+        activity = ActivityLog.objects.get(action=amo.LOG.COLLECTION_DELETED.id)
+        assert activity.arguments == [collection]
+        assert activity.user == self.task_user
 
     def test_approve_initial_collection(self):
         collection = collection_factory(author=user_factory(), deleted=True)
@@ -124,6 +175,7 @@ class TestCinderAction(TestCase):
         action.process()
         assert collection.reload()
         assert collection.deleted
+        assert ActivityLog.objects.count() == 0
 
     def test_approve_appeal_collection(self):
         collection = collection_factory(author=user_factory(), deleted=True)
@@ -132,28 +184,44 @@ class TestCinderAction(TestCase):
         action.process()
         assert collection.reload()
         assert not collection.deleted
+        assert ActivityLog.objects.count() == 1
+        activity = ActivityLog.objects.get(action=amo.LOG.COLLECTION_UNDELETED.id)
+        assert activity.arguments == [collection]
+        assert activity.user == self.task_user
 
     def test_delete_rating(self):
         rating = Rating.objects.create(addon=addon_factory(), user=user_factory())
         self.cinder_job.abusereport_set.update(rating=rating, guid=None)
+        ActivityLog.objects.all().delete()
         action = CinderActionDeleteRating(self.cinder_job)
         action.process()
         assert rating.reload().deleted
+        assert ActivityLog.objects.count() == 1
+        activity = ActivityLog.objects.get(action=amo.LOG.DELETE_RATING.id)
+        assert activity.arguments == [rating.addon, rating]
+        assert activity.user == self.task_user
 
     def test_approve_initial_rating(self):
         rating = Rating.objects.create(
             addon=addon_factory(), user=user_factory(), deleted=True
         )
         self.cinder_job.abusereport_set.update(rating=rating, guid=None)
+        ActivityLog.objects.all().delete()
         action = CinderActionApproveInitialDecision(self.cinder_job)
         action.process()
         assert rating.reload().deleted
+        assert ActivityLog.objects.count() == 0
 
     def test_approve_appeal_rating(self):
         rating = Rating.objects.create(
             addon=addon_factory(), user=user_factory(), deleted=True
         )
+        ActivityLog.objects.all().delete()
         self.cinder_job.abusereport_set.update(rating=rating, guid=None)
         action = CinderActionApproveAppealOverride(self.cinder_job)
         action.process()
         assert not rating.reload().deleted
+        assert ActivityLog.objects.count() == 1
+        activity = ActivityLog.objects.get(action=amo.LOG.UNDELETE_RATING.id)
+        assert activity.arguments == [rating, rating.addon]
+        assert activity.user == self.task_user

src/olympia/abuse/tests/test_views.py

@@ -5,6 +5,7 @@ import os
 from datetime import datetime
 from unittest import mock
 
+from django.conf import settings
 from django.test.utils import override_settings
 from django.urls import reverse
 from django.utils.encoding import force_bytes
@@ -24,6 +25,7 @@ from olympia.amo.tests import (
     reverse_ns,
     user_factory,
 )
+from olympia.core import get_user, set_user
 from olympia.ratings.models import Rating
 
 from ..models import AbuseReport, CinderJob
@@ -806,6 +808,9 @@ class TestUserAbuseViewSetLoggedIn(UserAbuseViewSetTestBase, TestCase):
 @override_settings(CINDER_WEBHOOK_TOKEN='webhook-token')
 @override_settings(CINDER_QUEUE_PREFIX='amo-')
 class TestCinderWebhook(TestCase):
+    def setUp(self):
+        self.task_user = user_factory(pk=settings.TASK_USER_ID)
+
     def get_data(self):
         webhook_file = os.path.join(TESTS_DIR, 'assets', 'cinder_webhook.json')
         with open(webhook_file) as file_object:
@@ -1010,6 +1015,13 @@ class TestCinderWebhook(TestCase):
             }
         }
 
+    def test_set_user(self):
+        set_user(user_factory())
+        req = self.get_request()
+        response = cinder_webhook(req)
+        assert response.status_code == 200
+        assert get_user() == self.task_user
+
 
 class RatingAbuseViewSetTestBase:
     client_class = APITestClientSessionID

src/olympia/abuse/utils.py

@@ -34,7 +34,6 @@ class CinderActionBanUser(CinderAction):
 
     def process(self):
         if isinstance(self.target, UserProfile) and not self.target.banned:
-            log_create(amo.LOG.ADMIN_USER_BANNED, self.target)
             UserProfile.objects.filter(
                 pk=self.target.pk
             ).ban_and_disable_related_content()
@@ -69,11 +68,11 @@ class CinderActionEscalateAddon(CinderAction):
                 .filter(version__in=reported_versions)
                 .no_transforms()
             )
-            NeedsHumanReview.objects.bulk_create(
+            # We need custom save() and post_save to be triggered, so we can't
-                (
+            # optimize this via bulk_create().
-                    NeedsHumanReview(version=version_obj, reason=reason, is_active=True)
+            for version in version_objs:
-                    for version_obj in version_objs
+                NeedsHumanReview.objects.create(
-                )
+                    version=version, reason=reason, is_active=True
                 )
             # If we have more versions specified than versions we flagged, flag latest
             # to be safe. (Either because there was an unknown version, or a None)

src/olympia/abuse/views.py

@@ -27,7 +27,9 @@ from olympia.accounts.views import AccountViewSet
 from olympia.addons.views import AddonViewSet
 from olympia.api.throttling import GranularIPRateThrottle, GranularUserRateThrottle
 from olympia.bandwagon.views import CollectionViewSet
+from olympia.core import set_user
 from olympia.ratings.views import RatingViewSet
+from olympia.users.models import UserProfile
 
 from .cinder import CinderEntity
 from .forms import AbuseAppealEmailForm, AbuseAppealForm
@@ -196,6 +198,10 @@ def filter_enforcement_actions(enforcement_actions, cinder_report):
 @authentication_classes(())
 @permission_classes((CinderInboundPermission,))
 def cinder_webhook(request):
+    # Normally setting the user would be done in the authentication class but
+    # we don't have one here.
+    set_user(UserProfile.objects.get(pk=settings.TASK_USER_ID))
+
     try:
         if request.data.get('event') != 'decision.created':
             log.info('Non-decision payload received: %s', str(request.data)[:255])