flag abuse reports reviewers will handle; send to seperate queue (#21385)
* flag abuse reports reviewers will handle; send to seperate queue * add needs human review reasons to choices * flag the correct version for needshumanreview if specified * choices fix * fix webhook view * rename CinderAddonByReviewers
This commit is contained in:
Родитель
7152f505c0
Коммит
54d1c15280
|
@ -4,7 +4,8 @@ import requests
|
|||
|
||||
|
||||
class CinderEntity:
|
||||
QUEUE = 'amo-content-infringement'
|
||||
# This queue is for reports that T&S / TaskUs look at
|
||||
queue = 'amo-content-infringement'
|
||||
type = None # Needs to be defined by subclasses
|
||||
|
||||
@property
|
||||
|
@ -38,7 +39,7 @@ class CinderEntity:
|
|||
reporter.get_relationship_data(self, 'amo_reporter_of')
|
||||
]
|
||||
return {
|
||||
'queue_slug': self.QUEUE,
|
||||
'queue_slug': self.queue,
|
||||
'entity_type': self.type,
|
||||
'entity': self.get_attributes(),
|
||||
'reasoning': report_text,
|
||||
|
@ -77,7 +78,7 @@ class CinderEntity:
|
|||
'authorization': f'Bearer {settings.CINDER_API_TOKEN}',
|
||||
}
|
||||
data = {
|
||||
'queue_slug': self.QUEUE,
|
||||
'queue_slug': self.queue,
|
||||
'appealer_entity_type': appealer.type,
|
||||
'appealer_entity': appealer.get_attributes(),
|
||||
'reasoning': appeal_text,
|
||||
|
@ -150,8 +151,9 @@ class CinderUnauthenticatedReporter(CinderEntity):
|
|||
class CinderAddon(CinderEntity):
|
||||
type = 'amo_addon'
|
||||
|
||||
def __init__(self, addon):
|
||||
def __init__(self, addon, version=None):
|
||||
self.addon = addon
|
||||
self.version = version
|
||||
|
||||
@property
|
||||
def id(self):
|
||||
|
@ -202,3 +204,33 @@ class CinderRating(CinderEntity):
|
|||
cinder_user.get_relationship_data(self, 'amo_rating_author_of')
|
||||
],
|
||||
}
|
||||
|
||||
|
||||
class CinderAddonHandledByReviewers(CinderAddon):
|
||||
# This queue is not monitored on cinder - reports are resolved via AMO instead
|
||||
queue = 'amo-addon-infringement'
|
||||
|
||||
def flag_for_human_review(self, appeal=False):
|
||||
from olympia.reviewers.models import NeedsHumanReview
|
||||
|
||||
reason = (
|
||||
NeedsHumanReview.REASON_ABUSE_ADDON_VIOLATION_APPEAL
|
||||
if appeal
|
||||
else NeedsHumanReview.REASON_ABUSE_ADDON_VIOLATION
|
||||
)
|
||||
if self.version:
|
||||
NeedsHumanReview.objects.get_or_create(
|
||||
version=self.version, reason=reason, is_active=True
|
||||
)
|
||||
else:
|
||||
self.addon.set_needs_human_review_on_latest_versions(
|
||||
reason=reason, ignore_reviewed=False, unique_reason=True
|
||||
)
|
||||
|
||||
def report(self, *args, **kwargs):
|
||||
self.flag_for_human_review(appeal=False)
|
||||
return super().report(*args, **kwargs)
|
||||
|
||||
def appeal(self, *args, **kwargs):
|
||||
self.flag_for_human_review(appeal=True)
|
||||
return super().appeal(*args, **kwargs)
|
||||
|
|
|
@ -11,7 +11,13 @@ from olympia.api.utils import APIChoices, APIChoicesWithNone
|
|||
from olympia.ratings.models import Rating
|
||||
from olympia.users.models import UserProfile
|
||||
|
||||
from .cinder import CinderAddon, CinderRating, CinderUnauthenticatedReporter, CinderUser
|
||||
from .cinder import (
|
||||
CinderAddon,
|
||||
CinderAddonHandledByReviewers,
|
||||
CinderRating,
|
||||
CinderUnauthenticatedReporter,
|
||||
CinderUser,
|
||||
)
|
||||
from .utils import (
|
||||
CinderActionApprove,
|
||||
CinderActionBanUser,
|
||||
|
@ -123,6 +129,8 @@ class AbuseReport(ModelBase):
|
|||
'REPORTABLE_REASONS',
|
||||
('HATEFUL_VIOLENT_DECEPTIVE', 'ILLEGAL', 'POLICY_VIOLATION', 'OTHER'),
|
||||
)
|
||||
# Abuse in these locations are handled by reviewers
|
||||
REASONS.add_subset('REVIEWER_HANDLED', ('POLICY_VIOLATION',))
|
||||
|
||||
# https://searchfox.org
|
||||
# /mozilla-central/source/toolkit/components/telemetry/Events.yaml#122-131
|
||||
|
@ -201,6 +209,8 @@ class AbuseReport(ModelBase):
|
|||
('ADDON', 2, 'Inside Add-on'),
|
||||
('BOTH', 3, 'Both on AMO and inside Add-on'),
|
||||
)
|
||||
# Abuse in these locations are handled by reviewers
|
||||
LOCATION.add_subset('REVIEWER_HANDLED', ('ADDON', 'BOTH'))
|
||||
|
||||
# NULL if the reporter was not authenticated.
|
||||
reporter = models.ForeignKey(
|
||||
|
@ -367,6 +377,15 @@ class AbuseReport(ModelBase):
|
|||
return self.rating
|
||||
return None
|
||||
|
||||
@property
|
||||
def is_handled_by_reviewers(self):
|
||||
return (
|
||||
(target := self.target)
|
||||
and isinstance(target, Addon)
|
||||
and self.reason in AbuseReport.REASONS.REVIEWER_HANDLED
|
||||
and self.location in AbuseReport.LOCATION.REVIEWER_HANDLED
|
||||
)
|
||||
|
||||
|
||||
class CantBeAppealed(Exception):
|
||||
pass
|
||||
|
@ -409,7 +428,17 @@ class CinderReport(ModelBase):
|
|||
target = self.abuse_report.target
|
||||
if target:
|
||||
if isinstance(target, Addon):
|
||||
return CinderAddon(target)
|
||||
version = (
|
||||
self.abuse_report.addon_version
|
||||
and target.versions(manager='unfiltered_for_relations')
|
||||
.filter(version=self.abuse_report.addon_version)
|
||||
.no_transforms()
|
||||
.first()
|
||||
)
|
||||
if self.abuse_report.is_handled_by_reviewers:
|
||||
return CinderAddonHandledByReviewers(target, version)
|
||||
else:
|
||||
return CinderAddon(target, version)
|
||||
elif isinstance(target, UserProfile):
|
||||
return CinderUser(target)
|
||||
elif isinstance(target, Rating):
|
||||
|
|
|
@ -2,11 +2,14 @@ from django.conf import settings
|
|||
|
||||
import responses
|
||||
|
||||
from olympia.amo.tests import TestCase, addon_factory, user_factory
|
||||
from olympia import amo
|
||||
from olympia.amo.tests import TestCase, addon_factory, user_factory, version_factory
|
||||
from olympia.ratings.models import Rating
|
||||
from olympia.reviewers.models import NeedsHumanReview
|
||||
|
||||
from ..cinder import (
|
||||
CinderAddon,
|
||||
CinderAddonHandledByReviewers,
|
||||
CinderRating,
|
||||
CinderUnauthenticatedReporter,
|
||||
CinderUser,
|
||||
|
@ -71,9 +74,11 @@ class BaseTestCinderCase:
|
|||
def test_report(self):
|
||||
self._test_report(self.cinder_class(self._create_dummy_target()))
|
||||
|
||||
def _test_appeal(self, appealer):
|
||||
def _test_appeal(self, appealer, cinder_instance=None):
|
||||
fake_decision_id = 'decision-id-to-appeal-666'
|
||||
cinder_instance = self.cinder_class(self._create_dummy_target())
|
||||
cinder_instance = cinder_instance or self.cinder_class(
|
||||
self._create_dummy_target()
|
||||
)
|
||||
|
||||
responses.add(
|
||||
responses.POST,
|
||||
|
@ -120,7 +125,7 @@ class TestCinderAddon(BaseTestCinderCase, TestCase):
|
|||
report_text=reason, category=None, reporter=None
|
||||
)
|
||||
assert data == {
|
||||
'queue_slug': 'amo-content-infringement',
|
||||
'queue_slug': self.cinder_class.queue,
|
||||
'entity_type': 'amo_addon',
|
||||
'entity': {
|
||||
'id': str(addon.id),
|
||||
|
@ -267,6 +272,57 @@ class TestCinderAddon(BaseTestCinderCase, TestCase):
|
|||
}
|
||||
|
||||
|
||||
class TestCinderAddonHandledByReviewers(TestCinderAddon):
|
||||
cinder_class = CinderAddonHandledByReviewers
|
||||
|
||||
def setUp(self):
|
||||
user_factory(id=settings.TASK_USER_ID)
|
||||
|
||||
def test_report(self):
|
||||
addon = self._create_dummy_target()
|
||||
addon.current_version.file.update(is_signed=True)
|
||||
self._test_report(self.cinder_class(addon))
|
||||
assert (
|
||||
addon.current_version.needshumanreview_set.get().reason
|
||||
== NeedsHumanReview.REASON_ABUSE_ADDON_VIOLATION
|
||||
)
|
||||
|
||||
def test_report_with_version(self):
|
||||
addon = self._create_dummy_target()
|
||||
addon.current_version.file.update(is_signed=True)
|
||||
other_version = version_factory(
|
||||
addon=addon,
|
||||
file_kw={'is_signed': True, 'status': amo.STATUS_AWAITING_REVIEW},
|
||||
)
|
||||
self._test_report(self.cinder_class(addon, other_version))
|
||||
assert not addon.current_version.needshumanreview_set.exists()
|
||||
# that there's only one is required - _test_report calls report() multiple times
|
||||
assert (
|
||||
other_version.needshumanreview_set.get().reason
|
||||
== NeedsHumanReview.REASON_ABUSE_ADDON_VIOLATION
|
||||
)
|
||||
|
||||
def test_appeal_anonymous(self):
|
||||
addon = self._create_dummy_target()
|
||||
addon.current_version.file.update(is_signed=True)
|
||||
self._test_appeal(
|
||||
CinderUnauthenticatedReporter('itsme', 'm@r.io'), self.cinder_class(addon)
|
||||
)
|
||||
assert (
|
||||
addon.current_version.needshumanreview_set.get().reason
|
||||
== NeedsHumanReview.REASON_ABUSE_ADDON_VIOLATION_APPEAL
|
||||
)
|
||||
|
||||
def test_appeal_logged_in(self):
|
||||
addon = self._create_dummy_target()
|
||||
addon.current_version.file.update(is_signed=True)
|
||||
self._test_appeal(CinderUser(user_factory()), self.cinder_class(addon))
|
||||
assert (
|
||||
addon.current_version.needshumanreview_set.get().reason
|
||||
== NeedsHumanReview.REASON_ABUSE_ADDON_VIOLATION_APPEAL
|
||||
)
|
||||
|
||||
|
||||
class TestCinderUser(BaseTestCinderCase, TestCase):
|
||||
cinder_class = CinderUser
|
||||
|
||||
|
@ -282,7 +338,7 @@ class TestCinderUser(BaseTestCinderCase, TestCase):
|
|||
report_text=reason, category=None, reporter=None
|
||||
)
|
||||
assert data == {
|
||||
'queue_slug': 'amo-content-infringement',
|
||||
'queue_slug': self.cinder_class.queue,
|
||||
'entity_type': 'amo_user',
|
||||
'entity': {
|
||||
'id': str(user.id),
|
||||
|
|
|
@ -9,7 +9,12 @@ import responses
|
|||
from olympia.amo.tests import TestCase, addon_factory, user_factory
|
||||
from olympia.ratings.models import Rating
|
||||
|
||||
from ..cinder import CinderAddon, CinderRating, CinderUser
|
||||
from ..cinder import (
|
||||
CinderAddon,
|
||||
CinderAddonHandledByReviewers,
|
||||
CinderRating,
|
||||
CinderUser,
|
||||
)
|
||||
from ..models import AbuseReport, CinderReport
|
||||
from ..utils import (
|
||||
CinderActionApprove,
|
||||
|
@ -254,6 +259,30 @@ class TestAbuse(TestCase):
|
|||
report.update(user=None, rating=rating)
|
||||
assert report.target == rating
|
||||
|
||||
def test_is_handled_by_reviewers(self):
|
||||
addon = addon_factory()
|
||||
abuse_report = AbuseReport.objects.create(
|
||||
guid=addon.guid,
|
||||
reason=AbuseReport.REASONS.ILLEGAL,
|
||||
location=AbuseReport.LOCATION.BOTH,
|
||||
)
|
||||
# location is in REVIEWER_HANDLED (BOTH) but reason is not (ILLEGAL)
|
||||
assert not abuse_report.is_handled_by_reviewers
|
||||
|
||||
abuse_report.update(reason=AbuseReport.REASONS.POLICY_VIOLATION)
|
||||
# now reason is in REVIEWER_HANDLED it will be reported differently
|
||||
assert abuse_report.is_handled_by_reviewers
|
||||
|
||||
abuse_report.update(location=AbuseReport.LOCATION.AMO)
|
||||
# but not if the location is not in REVIEWER_HANDLED (i.e. AMO)
|
||||
assert not abuse_report.is_handled_by_reviewers
|
||||
|
||||
# test non-addons are False regardless
|
||||
abuse_report.update(location=AbuseReport.LOCATION.ADDON)
|
||||
assert abuse_report.is_handled_by_reviewers
|
||||
abuse_report.update(user=user_factory(), guid=None)
|
||||
assert not abuse_report.is_handled_by_reviewers
|
||||
|
||||
def test_constraint(self):
|
||||
report = AbuseReport()
|
||||
constraints = report.get_constraints()
|
||||
|
@ -323,14 +352,43 @@ class TestCinderReport(TestCase):
|
|||
user = user_factory()
|
||||
cinder_report = CinderReport.objects.create(
|
||||
abuse_report=AbuseReport.objects.create(
|
||||
guid=addon.guid, reason=AbuseReport.REASONS.ILLEGAL
|
||||
guid=addon.guid,
|
||||
reason=AbuseReport.REASONS.ILLEGAL,
|
||||
location=AbuseReport.LOCATION.BOTH,
|
||||
)
|
||||
)
|
||||
helper = cinder_report.get_entity_helper()
|
||||
# location is in REVIEWER_HANDLED (BOTH) but reason is not (ILLEGAL)
|
||||
assert isinstance(helper, CinderAddon)
|
||||
assert not isinstance(helper, CinderAddonHandledByReviewers)
|
||||
assert helper.addon == addon
|
||||
assert helper.version is None
|
||||
|
||||
cinder_report.abuse_report.update(guid=None, user=user)
|
||||
cinder_report.abuse_report.update(reason=AbuseReport.REASONS.POLICY_VIOLATION)
|
||||
helper = cinder_report.get_entity_helper()
|
||||
# now reason is in REVIEWER_HANDLED it will be reported differently
|
||||
assert isinstance(helper, CinderAddon)
|
||||
assert isinstance(helper, CinderAddonHandledByReviewers)
|
||||
assert helper.addon == addon
|
||||
assert helper.version is None
|
||||
|
||||
cinder_report.abuse_report.update(addon_version=addon.current_version.version)
|
||||
helper = cinder_report.get_entity_helper()
|
||||
# if we got a version too we pass it on to the helper
|
||||
assert isinstance(helper, CinderAddon)
|
||||
assert isinstance(helper, CinderAddonHandledByReviewers)
|
||||
assert helper.addon == addon
|
||||
assert helper.version == addon.current_version
|
||||
|
||||
cinder_report.abuse_report.update(location=AbuseReport.LOCATION.AMO)
|
||||
helper = cinder_report.get_entity_helper()
|
||||
# but not if the location is not in REVIEWER_HANDLED (i.e. AMO)
|
||||
assert isinstance(helper, CinderAddon)
|
||||
assert not isinstance(helper, CinderAddonHandledByReviewers)
|
||||
assert helper.addon == addon
|
||||
assert helper.version == addon.current_version
|
||||
|
||||
cinder_report.abuse_report.update(guid=None, user=user, addon_version=None)
|
||||
helper = cinder_report.get_entity_helper()
|
||||
assert isinstance(helper, CinderUser)
|
||||
assert helper.user == user
|
||||
|
|
|
@ -61,9 +61,13 @@ class CinderActionEscalateAddon(CinderAction):
|
|||
.first()
|
||||
)
|
||||
if version_obj:
|
||||
NeedsHumanReview.objects.create(version=version_obj, reason=reason)
|
||||
NeedsHumanReview.objects.create(
|
||||
version=version_obj, reason=reason, is_active=True
|
||||
)
|
||||
else:
|
||||
addon.set_needs_human_review_on_latest_versions(reason=reason)
|
||||
addon.set_needs_human_review_on_latest_versions(
|
||||
reason=reason, ignore_reviewed=False, unique_reason=True
|
||||
)
|
||||
|
||||
|
||||
class CinderActionApprove(CinderAction):
|
||||
|
|
|
@ -172,7 +172,7 @@ def cinder_webhook(request):
|
|||
):
|
||||
source = payload.get('source', {})
|
||||
job = source.get('job', {})
|
||||
if (queue_name := job.get('queue', {}).get('slug')) == CinderEntity.QUEUE:
|
||||
if (queue_name := job.get('queue', {}).get('slug')) == CinderEntity.queue:
|
||||
log.info('Valid Payload from AMO queue: %s', payload)
|
||||
job_id = job.get('id', '')
|
||||
decision_id = source.get('decision', {}).get('id')
|
||||
|
|
|
@ -684,17 +684,33 @@ class Addon(OnChangeMixin, ModelBase):
|
|||
def disable_all_files(self):
|
||||
File.objects.filter(version__addon=self).update(status=amo.STATUS_DISABLED)
|
||||
|
||||
def set_needs_human_review_on_latest_versions(self, *, reason, due_date=None):
|
||||
def set_needs_human_review_on_latest_versions(
|
||||
self, *, reason, due_date=None, ignore_reviewed=True, unique_reason=False
|
||||
):
|
||||
set_listed = self._set_needs_human_review_on_latest_signed_version(
|
||||
channel=amo.CHANNEL_LISTED, due_date=due_date, reason=reason
|
||||
channel=amo.CHANNEL_LISTED,
|
||||
due_date=due_date,
|
||||
reason=reason,
|
||||
ignore_reviewed=ignore_reviewed,
|
||||
unique_reason=unique_reason,
|
||||
)
|
||||
set_unlisted = self._set_needs_human_review_on_latest_signed_version(
|
||||
channel=amo.CHANNEL_UNLISTED, due_date=due_date, reason=reason
|
||||
channel=amo.CHANNEL_UNLISTED,
|
||||
due_date=due_date,
|
||||
reason=reason,
|
||||
ignore_reviewed=ignore_reviewed,
|
||||
unique_reason=unique_reason,
|
||||
)
|
||||
return set_listed or set_unlisted
|
||||
|
||||
def _set_needs_human_review_on_latest_signed_version(
|
||||
self, *, channel, reason, due_date=None
|
||||
self,
|
||||
*,
|
||||
channel,
|
||||
reason,
|
||||
due_date=None,
|
||||
ignore_reviewed=True,
|
||||
unique_reason=False,
|
||||
):
|
||||
from olympia.reviewers.models import NeedsHumanReview
|
||||
|
||||
|
@ -706,8 +722,10 @@ class Addon(OnChangeMixin, ModelBase):
|
|||
)
|
||||
if (
|
||||
not version
|
||||
or version.human_review_date
|
||||
or version.needshumanreview_set.filter(is_active=True).exists()
|
||||
or (ignore_reviewed and version.human_review_date)
|
||||
or version.needshumanreview_set.filter(
|
||||
is_active=True, **({'reason': reason} if unique_reason else {})
|
||||
).exists()
|
||||
):
|
||||
return False
|
||||
had_due_date_already = bool(version.due_date)
|
||||
|
|
|
@ -2063,6 +2063,40 @@ class TestAddonDueDate(TestCase):
|
|||
)
|
||||
assert version.needshumanreview_set.filter(is_active=True).count() == 0
|
||||
|
||||
assert addon.set_needs_human_review_on_latest_versions(
|
||||
reason=NeedsHumanReview.REASON_PROMOTED_GROUP, ignore_reviewed=False
|
||||
)
|
||||
assert version.needshumanreview_set.filter(is_active=True).count() == 1
|
||||
assert (
|
||||
version.needshumanreview_set.get().reason
|
||||
== NeedsHumanReview.REASON_PROMOTED_GROUP
|
||||
)
|
||||
|
||||
def test_set_needs_human_review_on_latest_versions_unique_reason(self):
|
||||
addon = Addon.objects.get(id=3615)
|
||||
version = addon.current_version
|
||||
NeedsHumanReview.objects.create(
|
||||
version=version, reason=NeedsHumanReview.REASON_SCANNER_ACTION
|
||||
)
|
||||
|
||||
assert not addon.set_needs_human_review_on_latest_versions(
|
||||
reason=NeedsHumanReview.REASON_PROMOTED_GROUP, unique_reason=False
|
||||
)
|
||||
assert version.needshumanreview_set.filter(is_active=True).count() == 1
|
||||
assert (
|
||||
version.needshumanreview_set.get().reason
|
||||
== NeedsHumanReview.REASON_SCANNER_ACTION
|
||||
)
|
||||
|
||||
assert addon.set_needs_human_review_on_latest_versions(
|
||||
reason=NeedsHumanReview.REASON_PROMOTED_GROUP, unique_reason=True
|
||||
)
|
||||
assert version.needshumanreview_set.filter(is_active=True).count() == 2
|
||||
assert list(version.needshumanreview_set.values_list('reason', flat=True)) == [
|
||||
NeedsHumanReview.REASON_SCANNER_ACTION,
|
||||
NeedsHumanReview.REASON_PROMOTED_GROUP,
|
||||
]
|
||||
|
||||
def test_set_needs_human_review_on_latest_versions_even_deleted(self):
|
||||
addon = Addon.objects.get(id=3615)
|
||||
version = addon.current_version
|
||||
|
|
|
@ -0,0 +1,18 @@
|
|||
# Generated by Django 4.2.7 on 2023-11-06 16:04
|
||||
|
||||
from django.db import migrations, models
|
||||
|
||||
|
||||
class Migration(migrations.Migration):
|
||||
|
||||
dependencies = [
|
||||
('reviewers', '0031_reviewactionreason_canned_block_reason'),
|
||||
]
|
||||
|
||||
operations = [
|
||||
migrations.AlterField(
|
||||
model_name='needshumanreview',
|
||||
name='reason',
|
||||
field=models.SmallIntegerField(choices=[(0, 'Unknown'), (1, 'Hit scanner rule'), (2, 'Belongs to a promoted group'), (3, 'Over growth threshold for usage tier'), (4, 'Previous version in channel had needs human review set'), (5, 'Sources provided while pending rejection'), (6, 'Developer replied'), (7, 'Manually set as needing human review by a reviewer'), (8, 'Auto-approved but still had an approval delay set in the past'), (9, 'Over abuse reports threshold for usage tier'), (10, 'Escalated for an abuse report, via cinder'), (11, 'Reported for abuse within the add-on'), (12, 'Appeal about a decision on abuse reported within the add-on')], default=0, editable=False),
|
||||
),
|
||||
]
|
|
@ -756,6 +756,8 @@ class NeedsHumanReview(ModelBase):
|
|||
REASON_AUTO_APPROVED_PAST_APPROVAL_DELAY = 8
|
||||
REASON_ABUSE_REPORTS_THRESHOLD = 9
|
||||
REASON_CINDER_ESCALATION = 10
|
||||
REASON_ABUSE_ADDON_VIOLATION = 11
|
||||
REASON_ABUSE_ADDON_VIOLATION_APPEAL = 12
|
||||
|
||||
reason = models.SmallIntegerField(
|
||||
default=0,
|
||||
|
@ -792,6 +794,14 @@ class NeedsHumanReview(ModelBase):
|
|||
REASON_CINDER_ESCALATION,
|
||||
'Escalated for an abuse report, via cinder',
|
||||
),
|
||||
(
|
||||
REASON_ABUSE_ADDON_VIOLATION,
|
||||
'Reported for abuse within the add-on',
|
||||
),
|
||||
(
|
||||
REASON_ABUSE_ADDON_VIOLATION_APPEAL,
|
||||
'Appeal about a decision on abuse reported within the add-on',
|
||||
),
|
||||
),
|
||||
editable=False,
|
||||
)
|
||||
|
|
Загрузка…
Ссылка в новой задаче