diff --git a/docs/topics/api/abuse.rst b/docs/topics/api/abuse.rst index 6b56c7d07e..c1b8156989 100644 --- a/docs/topics/api/abuse.rst +++ b/docs/topics/api/abuse.rst @@ -19,7 +19,7 @@ Submitting an add-on abuse report The following API endpoint allows an abuse report to be submitted for an Add-on, either listed on https://addons.mozilla.org or not. Authentication is not required, but is recommended so reports can be responded -to if nessecary. +to if necessary. .. http:post:: /api/v4/abuse/report/addon/ @@ -27,6 +27,20 @@ to if nessecary. :json object|null reporter: The user who submitted the report, if authenticated. :>json int reporter.id: The id of the user who submitted the report. :>json string reporter.name: The name of the user who submitted the report. @@ -37,6 +51,20 @@ to if nessecary. :>json int|null addon.id: The add-on id on AMO. If the guid submitted didn't match a known add-on on AMO, then null. :>json string|null addon.slug: The add-on slug. If the guid submitted didn't match a known add-on on AMO, then null. :>json string message: The body/content of the abuse report. + :>json string|null addon_install_entry_point: The add-on install entry point. + :>json string|null addon_install_method: The add-on install method. + :>json string|null addon_install_origin: The add-on install origin. + :>json string|null addon_name: The add-on name in the locale used by the client. + :>json string|null addon_signature: The add-on signature state. + :>json string|null addon_summary: The add-on summary in the locale used by the client. + :>json string|null addon_version: The add-on version string. + :>json string|null application: The application used by the client. + :>json string|null application_locale: The locale used by the client for the application. + :>json string|null client_id: The client's hashed telemetry ID. + :>json string|null install_date: The add-on install date. + :>json string|null operating_system: The client's operating system. + :>json string|null operating_system_version: The client's operating system version. + :>json string|null reason: The reason for the report. ------------------------------ @@ -47,7 +75,7 @@ Submitting a user abuse report The following API endpoint allows an abuse report to be submitted for a user account on https://addons.mozilla.org. Authentication is not required, but is recommended -so reports can be responded to if nessecary. +so reports can be responded to if necessary. .. http:post:: /api/v4/abuse/report/user/ diff --git a/src/olympia/abuse/models.py b/src/olympia/abuse/models.py index ca1ef577d2..1140755a2e 100644 --- a/src/olympia/abuse/models.py +++ b/src/olympia/abuse/models.py @@ -14,6 +14,41 @@ from olympia.users.models import UserProfile @python_2_unicode_compatible class AbuseReport(ModelBase): + # Note: those choices don't need to be translated for now, the + # human-readable values are only exposed in the admin. The values will be + # updated once they are finalized in the PRD. + ADDON_SIGNATURE_CHOICES = ( + (None, 'None'), + ) + REASON_CHOICES = ( + (None, 'None'), + (1, 'Malware'), + (2, 'Spam / Advertising'), + (3, 'Search takeover'), + (4, 'New tab takeover'), + (5, 'Breaks websites'), + (6, 'Offensive'), + (7, 'Doesn\'t match description'), + (8, 'Doesn\'t work'), + ) + REASON_CHOICES_API = { + None: None, + 1: 'malware', + 2: 'spam_or_advertising', + 3: 'search_takeover', + 4: 'new_tab_takeover', + 5: 'breaks_websites', + 6: 'offensive', + 7: 'does_not_match_description', + 8: 'does_not_work', + } + ADDON_INSTALL_METHOD_CHOICES = ( + (None, 'None'), + ) + ADDON_INSTALL_ENTRY_POINTS_CHOICES = ( + (None, 'None'), + ) + # NULL if the reporter is anonymous. reporter = models.ForeignKey( UserProfile, null=True, blank=True, related_name='abuse_reported', @@ -33,6 +68,43 @@ class AbuseReport(ModelBase): on_delete=models.SET_NULL) message = models.TextField() + # Extra optional fields for more information, giving some context that is + # meant to be extracted automatically by the client (i.e. Firefox) and + # submitted via the API. + client_id = models.CharField( + default=None, max_length=64, blank=True, null=True) + addon_name = models.CharField( + default=None, max_length=255, blank=True, null=True) + addon_summary = models.CharField( + default=None, max_length=255, blank=True, null=True) + addon_version = models.CharField( + default=None, max_length=255, blank=True, null=True) + addon_signature = models.PositiveSmallIntegerField( + default=None, choices=ADDON_SIGNATURE_CHOICES, blank=True, null=True) + application = models.PositiveSmallIntegerField( + default=amo.FIREFOX.id, choices=amo.APPS_CHOICES, blank=True, + null=True) + application_version = models.CharField( + default=None, max_length=255, blank=True, null=True) + application_locale = models.CharField( + default=None, max_length=255, blank=True, null=True) + operating_system = models.CharField( + default=None, max_length=255, blank=True, null=True) + operating_system_version = models.CharField( + default=None, max_length=255, blank=True, null=True) + install_date = models.DateTimeField( + default=None, blank=True, null=True) + reason = models.PositiveSmallIntegerField( + default=None, choices=REASON_CHOICES, blank=True, null=True) + addon_install_origin = models.CharField( + default=None, max_length=255, blank=True, null=True) + addon_install_method = models.PositiveSmallIntegerField( + default=None, choices=ADDON_INSTALL_METHOD_CHOICES, blank=True, + null=True) + addon_install_entry_point = models.PositiveSmallIntegerField( + default=None, choices=ADDON_INSTALL_ENTRY_POINTS_CHOICES, blank=True, + null=True) + class Meta: db_table = 'abuse_reports' @@ -40,7 +112,7 @@ class AbuseReport(ModelBase): if self.reporter: user_name = '%s (%s)' % (self.reporter.name, self.reporter.email) else: - user_name = 'An anonymous coward' + user_name = 'An anonymous user' target_url = ('%s%s' % (settings.SITE_URL, self.target.get_url_path()) if self.target else 'GUID not in database') @@ -50,6 +122,12 @@ class AbuseReport(ModelBase): send_mail( six.text_type(self), msg, recipient_list=(settings.ABUSE_EMAIL,)) + def save(self, *args, **kwargs): + creation = not self.pk + super(AbuseReport, self).save(*args, **kwargs) + if creation: + self.send() + @property def target(self): return self.addon or self.user @@ -77,4 +155,3 @@ def send_abuse_report(request, obj, message): elif isinstance(obj, UserProfile): report.user = obj report.save() - report.send() diff --git a/src/olympia/abuse/serializers.py b/src/olympia/abuse/serializers.py index 87370e9ab7..2a3cae0eec 100644 --- a/src/olympia/abuse/serializers.py +++ b/src/olympia/abuse/serializers.py @@ -1,16 +1,79 @@ from rest_framework import serializers +from olympia import amo from olympia.abuse.models import AbuseReport from olympia.accounts.serializers import BaseUserSerializer +from olympia.api.fields import ReverseChoiceField -class AddonAbuseReportSerializer(serializers.ModelSerializer): - addon = serializers.SerializerMethodField() +class BaseAbuseReportSerializer(serializers.ModelSerializer): reporter = BaseUserSerializer(read_only=True) class Meta: model = AbuseReport - fields = ('reporter', 'addon', 'message') + fields = ('message', 'reporter') + + def validate_target(self, data, target_name): + if target_name not in data: + msg = serializers.Field.default_error_messages['required'] + raise serializers.ValidationError({ + target_name: [msg] + }) + + def to_internal_value(self, data): + output = super(BaseAbuseReportSerializer, self).to_internal_value(data) + request = self.context['request'] + output['ip_address'] = request.META.get('REMOTE_ADDR') + if request.user.is_authenticated: + output['reporter'] = request.user + return output + + +class AddonAbuseReportSerializer(BaseAbuseReportSerializer): + addon = serializers.SerializerMethodField() + reason = ReverseChoiceField( + choices=list(AbuseReport.REASON_CHOICES_API.items()), required=False) + application = ReverseChoiceField( + choices=list((v.id, k) for k, v in amo.APPS.items()), required=False) + + class Meta: + model = AbuseReport + fields = BaseAbuseReportSerializer.Meta.fields + ( + 'addon', + 'addon_install_entry_point', + 'addon_install_method', + 'addon_install_origin', + 'addon_name', + 'addon_signature', + 'addon_summary', + 'addon_version', + 'application', + 'application_locale', + 'application_version', + 'client_id', + 'install_date', + 'operating_system', + 'operating_system_version', + 'reason', + ) + + def to_internal_value(self, data): + self.validate_target(data, 'addon') + view = self.context.get('view') + output = { + # get_guid() needs to be called first because get_addon_object() + # would otherwise 404 on add-ons that don't match an existing + # add-on in our database. + 'guid': view.get_guid(), + 'addon': view.get_addon_object(), + } + # Pop 'addon' before passing it to super(), we already have the + # output value. + data.pop('addon') + output.update( + super(AddonAbuseReportSerializer, self).to_internal_value(data) + ) + return output def get_addon(self, obj): addon = obj.addon @@ -23,10 +86,25 @@ class AddonAbuseReportSerializer(serializers.ModelSerializer): } -class UserAbuseReportSerializer(serializers.ModelSerializer): - reporter = BaseUserSerializer(read_only=True) - user = BaseUserSerializer() +class UserAbuseReportSerializer(BaseAbuseReportSerializer): + user = BaseUserSerializer(required=False) # We validate it ourselves. class Meta: model = AbuseReport - fields = ('reporter', 'user', 'message') + fields = BaseAbuseReportSerializer.Meta.fields + ( + 'user', + ) + + def to_internal_value(self, data): + view = self.context.get('view') + self.validate_target(data, 'user') + output = { + 'user': view.get_user_object() + } + # Pop 'user' before passing it to super(), we already have the + # output value and did the validation above. + data.pop('user') + output.update( + super(UserAbuseReportSerializer, self).to_internal_value(data) + ) + return output diff --git a/src/olympia/abuse/tests/test_serializers.py b/src/olympia/abuse/tests/test_serializers.py index b7496890d4..2bf79a037f 100644 --- a/src/olympia/abuse/tests/test_serializers.py +++ b/src/olympia/abuse/tests/test_serializers.py @@ -18,7 +18,22 @@ class TestAddonAbuseReportSerializer(TestCase): 'addon': {'guid': addon.guid, 'id': addon.id, 'slug': addon.slug}, - 'message': 'bad stuff'} + 'message': 'bad stuff', + 'addon_install_entry_point': None, + 'addon_install_method': None, + 'addon_install_origin': None, + 'addon_name': None, + 'addon_signature': None, + 'addon_summary': None, + 'addon_version': None, + 'application': 'firefox', + 'application_locale': None, + 'application_version': None, + 'client_id': None, + 'install_date': None, + 'operating_system': None, + 'operating_system_version': None, + 'reason': None} def test_guid_report(self): report = AbuseReport(guid='@guid', message='bad stuff') @@ -27,7 +42,22 @@ class TestAddonAbuseReportSerializer(TestCase): 'addon': {'guid': '@guid', 'id': None, 'slug': None}, - 'message': 'bad stuff'} + 'message': 'bad stuff', + 'addon_install_entry_point': None, + 'addon_install_method': None, + 'addon_install_origin': None, + 'addon_name': None, + 'addon_signature': None, + 'addon_summary': None, + 'addon_version': None, + 'application': 'firefox', + 'application_locale': None, + 'application_version': None, + 'client_id': None, + 'install_date': None, + 'operating_system': None, + 'operating_system_version': None, + 'reason': None} class TestUserAbuseReportSerializer(TestCase): diff --git a/src/olympia/abuse/tests/test_views.py b/src/olympia/abuse/tests/test_views.py index 51c3a6b092..01888dd271 100644 --- a/src/olympia/abuse/tests/test_views.py +++ b/src/olympia/abuse/tests/test_views.py @@ -1,4 +1,6 @@ +# -*- coding: utf-8 -*- import json +from datetime import datetime from django.core import mail @@ -102,7 +104,7 @@ class AddonAbuseViewSetTestBase(object): data={'message': 'abuse!'}) assert response.status_code == 400 assert json.loads(response.content) == { - 'detail': 'Need an addon parameter'} + 'addon': ['This field is required.']} def test_message_required_empty(self): addon = addon_factory() @@ -112,7 +114,7 @@ class AddonAbuseViewSetTestBase(object): 'message': ''}) assert response.status_code == 400 assert json.loads(response.content) == { - 'detail': 'Abuse reports need a message'} + 'message': ['This field may not be blank.']} def test_message_required_missing(self): addon = addon_factory() @@ -121,7 +123,7 @@ class AddonAbuseViewSetTestBase(object): data={'addon': six.text_type(addon.id)}) assert response.status_code == 400 assert json.loads(response.content) == { - 'detail': 'Abuse reports need a message'} + 'message': ['This field is required.']} def test_throttle(self): addon = addon_factory() @@ -138,6 +140,102 @@ class AddonAbuseViewSetTestBase(object): REMOTE_ADDR='123.45.67.89') assert response.status_code == 429 + def test_optional_fields(self): + # FIXME: when choices for addon_signature, addon_install_method and + # addon_install_entry_point are finalized, update this test. In the + # meantime we pass None. + data = { + 'addon': '@mysteryaddon', + 'message': u'This is abusé!', + 'client_id': 'i' * 64, + 'addon_name': u'Addon Næme', + 'addon_summary': u'Addon sûmmary', + 'addon_version': '0.01.01', + 'addon_signature': None, + 'application': 'firefox', + 'application_version': '42.0.1', + 'application_locale': u'Lô-käl', + 'operating_system': u'Sømething OS', + 'install_date': '2004-08-15T16:23:42', + 'reason': 'spam_or_advertising', + 'addon_install_origin': 'http://example.com/', + 'addon_install_method': None, + 'addon_install_entry_point': None, + } + response = self.client.post( + self.url, + data=data, + REMOTE_ADDR='123.45.67.89') + assert response.status_code == 201, response.content + + assert AbuseReport.objects.filter(guid=data['addon']).exists() + report = AbuseReport.objects.get(guid=data['addon']) + self.check_report( + report, u'[Addon] Abuse Report for %s' % data['addon']) + assert not report.addon # Not an add-on in database, that's ok. + # Straightforward comparisons: + for field in ('message', 'client_id', 'addon_name', 'addon_summary', + 'addon_version', 'application_version', + 'application_locale', 'operating_system', + 'addon_install_origin'): + assert getattr(report, field) == data[field], field + # More complex comparisons: + assert report.addon_signature is None + assert report.application == amo.FIREFOX.id + assert report.install_date == datetime(2004, 8, 15, 16, 23, 42) + assert report.reason == 2 # Spam / Advertising + assert report.addon_install_method is None + assert report.addon_install_entry_point is None + + def test_optional_fields_errors(self): + data = { + 'addon': '@mysteryaddon', + 'message': u'Message cân be quite big if needed' * 256, + 'client_id': 'i' * 65, + 'addon_name': 'a' * 256, + 'addon_summary': 's' * 256, + 'addon_version': 'v' * 256, + 'addon_signature': 'Something not in signature choices', + 'application': 'FIRE! EXCLAMATION MARK', + 'application_version': '1' * 256, + 'application_locale': 'l' * 256, + 'operating_system': 'o' * 256, + 'install_date': 'not_a_date', + 'reason': 'Something not in reason choices', + 'addon_install_origin': 'u' * 256, + 'addon_install_method': 'Something not in install method choices', + 'addon_install_entry_point': 'Something not in entrypoint choices', + } + response = self.client.post( + self.url, + data=data, + REMOTE_ADDR='123.45.67.89') + assert response.status_code == 400 + expected_max_length_message = ( + 'Ensure this field has no more than %d characters.') + expected_choices_message = '"%s" is not a valid choice.' + assert response.json() == { + 'client_id': [expected_max_length_message % 64], + 'addon_name': [expected_max_length_message % 255], + 'addon_summary': [expected_max_length_message % 255], + 'addon_version': [expected_max_length_message % 255], + 'addon_signature': [ + expected_choices_message % data['addon_signature']], + 'application': [expected_choices_message % data['application']], + 'application_version': [expected_max_length_message % 255], + 'application_locale': [expected_max_length_message % 255], + 'operating_system': [expected_max_length_message % 255], + 'install_date': [ + 'Datetime has wrong format. Use one of these formats ' + 'instead: YYYY-MM-DDThh:mm[:ss[.uuuuuu]][+HH:MM|-HH:MM|Z].'], + 'reason': [expected_choices_message % data['reason']], + 'addon_install_origin': [expected_max_length_message % 255], + 'addon_install_method': [ + expected_choices_message % data['addon_install_method']], + 'addon_install_entry_point': [ + expected_choices_message % data['addon_install_entry_point']], + } + class TestAddonAbuseViewSetLoggedOut(AddonAbuseViewSetTestBase, TestCase): def check_reporter(self, report): @@ -201,7 +299,7 @@ class UserAbuseViewSetTestBase(object): data={'message': 'abuse!'}) assert response.status_code == 400 assert json.loads(response.content) == { - 'detail': 'Need a user parameter'} + 'user': ['This field is required.']} def test_message_required_empty(self): user = user_factory() @@ -210,7 +308,7 @@ class UserAbuseViewSetTestBase(object): data={'user': six.text_type(user.username), 'message': ''}) assert response.status_code == 400 assert json.loads(response.content) == { - 'detail': 'Abuse reports need a message'} + 'message': ['This field may not be blank.']} def test_message_required_missing(self): user = user_factory() @@ -219,7 +317,7 @@ class UserAbuseViewSetTestBase(object): data={'user': six.text_type(user.username)}) assert response.status_code == 400 assert json.loads(response.content) == { - 'detail': 'Abuse reports need a message'} + 'message': ['This field is required.']} def test_throttle(self): user = user_factory() diff --git a/src/olympia/abuse/views.py b/src/olympia/abuse/views.py index 198b783461..6bf9ebbe36 100644 --- a/src/olympia/abuse/views.py +++ b/src/olympia/abuse/views.py @@ -1,12 +1,8 @@ from django.http import Http404 -from rest_framework import status -from rest_framework.exceptions import ParseError from rest_framework.mixins import CreateModelMixin -from rest_framework.response import Response from rest_framework.viewsets import GenericViewSet -from olympia.abuse.models import AbuseReport from olympia.abuse.serializers import ( AddonAbuseReportSerializer, UserAbuseReportSerializer) from olympia.accounts.views import AccountViewSet @@ -59,37 +55,13 @@ class AddonAbuseViewSet(CreateModelMixin, GenericViewSet): return guid return None - def create(self, request, *args, **kwargs): - addon_id = self.request.data.get('addon') - if not addon_id: - raise ParseError('Need an addon parameter') - - message = self.request.data.get('message') - if not message: - raise ParseError('Abuse reports need a message') - - abuse_kwargs = { - 'ip_address': request.META.get('REMOTE_ADDR'), - 'message': message, - # get_guid() must be called first or addons not in our DB will 404. - 'guid': self.get_guid(), - 'addon': self.get_addon_object()} - if request.user.is_authenticated: - abuse_kwargs['reporter'] = request.user - - report = AbuseReport.objects.create(**abuse_kwargs) - report.send() - - serializer = self.get_serializer(report) - return Response(serializer.data, status=status.HTTP_201_CREATED) - class UserAbuseViewSet(CreateModelMixin, GenericViewSet): permission_classes = [] serializer_class = UserAbuseReportSerializer throttle_classes = (AbuseThrottle,) - def get_user_object(self, user_id): + def get_user_object(self): if hasattr(self, 'user_object'): return self.user_object @@ -101,25 +73,3 @@ class UserAbuseViewSet(CreateModelMixin, GenericViewSet): return AccountViewSet( request=self.request, permission_classes=[], kwargs={'pk': self.kwargs['user_pk']}).get_object() - - def create(self, request, *args, **kwargs): - user_id = self.request.data.get('user') - if not user_id: - raise ParseError('Need a user parameter') - - message = self.request.data.get('message') - if not message: - raise ParseError('Abuse reports need a message') - - abuse_kwargs = { - 'ip_address': request.META.get('REMOTE_ADDR'), - 'message': message, - 'user': self.get_user_object(user_id)} - if request.user.is_authenticated: - abuse_kwargs['reporter'] = request.user - - report = AbuseReport.objects.create(**abuse_kwargs) - report.send() - - serializer = self.get_serializer(report) - return Response(serializer.data, status=status.HTTP_201_CREATED) diff --git a/src/olympia/migrations/1073-add-abuse-report-fields.sql b/src/olympia/migrations/1073-add-abuse-report-fields.sql new file mode 100644 index 0000000000..30aaeb3f62 --- /dev/null +++ b/src/olympia/migrations/1073-add-abuse-report-fields.sql @@ -0,0 +1,16 @@ +ALTER TABLE `abuse_reports` + ADD COLUMN `client_id` varchar(64), + ADD COLUMN `addon_name` varchar(255), + ADD COLUMN `addon_summary` varchar(255), + ADD COLUMN `addon_version` varchar(255), + ADD COLUMN `addon_signature` smallint UNSIGNED, + ADD COLUMN `application` smallint UNSIGNED, + ADD COLUMN `application_version` varchar(255), + ADD COLUMN `application_locale` varchar(255), + ADD COLUMN `operating_system` varchar(255), + ADD COLUMN `operating_system_version` varchar(255), + ADD COLUMN `install_date` datetime(6), + ADD COLUMN `reason` smallint UNSIGNED, + ADD COLUMN `addon_install_origin` varchar(255), + ADD COLUMN `addon_install_method` smallint UNSIGNED, + ADD COLUMN `addon_install_entry_point` smallint UNSIGNED; diff --git a/src/olympia/reviewers/tests/test_views.py b/src/olympia/reviewers/tests/test_views.py index c5bbd61564..dd8f0adbbd 100644 --- a/src/olympia/reviewers/tests/test_views.py +++ b/src/olympia/reviewers/tests/test_views.py @@ -4721,7 +4721,7 @@ class TestAbuseReports(TestCase): AbuseReport.objects.create(user=someone, message=u'hey nöw') # Make a user abuse report for one of the add-on developers: it should # show up. - AbuseReport.objects.create(user=addon_developer, message='bü!') + AbuseReport.objects.create(user=addon_developer, message=u'bü!') def test_abuse_reports_list(self): assert self.client.login(email='admin@mozilla.com')