Stop storing ip_address in abuse reports - lookup country via geoip instead (#10745)

Stop storing ip_address in abuse reports - lookup country via geoip instead
This commit is contained in:
Mathieu Pillard 2019-02-22 12:32:37 +01:00 коммит произвёл GitHub
Родитель 859b448ae6
Коммит 34f21aa8f8
Не найден ключ, соответствующий данной подписи
Идентификатор ключа GPG: 4AEE18F83AFDEB23
18 изменённых файлов: 131 добавлений и 19 удалений

Просмотреть файл

@ -46,6 +46,9 @@ RUN apt-get update && apt-get install -y \
pngcrush \
# our makefile and ui-tests require uuid to be installed
uuid \
# Use libmaxmind for speedy geoip lookups
libmaxminddb0 \
libmaxminddb-dev \
&& rm -rf /var/lib/apt/lists/*
RUN apt-get update && apt-get -t stretch-backports install -y \
@ -53,6 +56,12 @@ RUN apt-get update && apt-get -t stretch-backports install -y \
libgit2-dev \
&& rm -rf /var/lib/apt/lists/*
ADD http://geolite.maxmind.com/download/geoip/database/GeoLite2-Country.mmdb.gz /tmp
RUN mkdir -p /usr/local/share/GeoIP \
&& gunzip -c /tmp/GeoLite2-Country.mmdb.gz > /usr/local/share/GeoIP/GeoLite2-Country.mmdb \
&& rm -f /tmp/GeoLite2-Country.mmdb.gz
# Compile required locale
RUN localedef -i en_US -f UTF-8 en_US.UTF-8

Просмотреть файл

@ -48,6 +48,9 @@ RUN apt-get update && apt-get install -y \
librsvg2-bin \
# Use pngcrush to optimize the PNGs uploaded by developers
pngcrush \
# Use libmaxmind for speedy geoip lookups
libmaxminddb0 \
libmaxminddb-dev \
&& rm -rf /var/lib/apt/lists/*
RUN apt-get update && apt-get -t stretch-backports install -y \

Просмотреть файл

@ -44,6 +44,9 @@ RUN apt-get update && apt-get install -y \
pngcrush \
# our makefile and ui-tests require uuid to be installed
uuid \
# Use libmaxmind for speedy geoip lookups
libmaxminddb0 \
libmaxminddb-dev \
&& rm -rf /var/lib/apt/lists/*
RUN apt-get update && apt-get -t stretch-backports install -y \
@ -51,6 +54,12 @@ RUN apt-get update && apt-get -t stretch-backports install -y \
libgit2-dev \
&& rm -rf /var/lib/apt/lists/*
ADD http://geolite.maxmind.com/download/geoip/database/GeoLite2-Country.mmdb.gz /tmp
RUN mkdir -p /usr/local/share/GeoIP \
&& gunzip -c /tmp/GeoLite2-Country.mmdb.gz > /usr/local/share/GeoIP/GeoLite2-Country.mmdb \
&& rm -f /tmp/GeoLite2-Country.mmdb.gz
# Compile required locale
RUN localedef -i en_US -f UTF-8 en_US.UTF-8

Просмотреть файл

@ -48,6 +48,9 @@ RUN apt-get update && apt-get install -y \
librsvg2-bin \
# Use pngcrush to optimize the PNGs uploaded by developers
pngcrush \
# Use libmaxmind for speedy geoip lookups
libmaxminddb0 \
libmaxminddb-dev \
&& rm -rf /var/lib/apt/lists/*
RUN apt-get update && apt-get -t stretch-backports install -y \

Просмотреть файл

@ -39,6 +39,9 @@ MarkupSafe==1.1.0 \
--hash=sha256:1c25694ca680b6919de53a4bb3bdd0602beafc63ff001fea2f2fc16ec3a11834 \
--hash=sha256:7d263e5770efddf465a9e31b78362d84d015cc894ca2c131901a4445eaa61ee1 \
--hash=sha256:4e97332c9ce444b0c2c38dd22ddc61c743eb208d916e4265a2a3b575bdccb1d3
# maxminddb is required by geoip2
maxminddb==1.4.1 \
--hash=sha256:df1451bcd848199905ac0de4631b3d02d6a655ad28ba5e5a4ca29a23358db712
mysqlclient==1.3.14 \
--hash=sha256:3981ae9ce545901a36a8b7aed76ed02960a429f75dc53b7ad77fb2f9ab7cd56b # pyup: <1.4
scandir==1.9.0 \
@ -236,6 +239,9 @@ feedparser==5.2.1 \
funcsigs==1.0.2 \
--hash=sha256:330cc27ccbf7f1e992e69fef78261dc7c6569012cf397db8d3de0234e6c937ca \
--hash=sha256:a7bb0f2cf3a3fd1ab2732cb49eba4252c2af4240442415b4abce3b87022a8f50
geoip2==2.9.0 \
--hash=sha256:a37ddac2d200ffb97c736da8b8ba9d5d8dc47da6ec0f162a461b681ecac53a14 \
--hash=sha256:f7ffe9d258e71a42cf622ce6350d976de1d0312b9f2fbce3975c7d838b57ecf0
# html5lib is required by rdflib
html5lib==1.0.1 \
--hash=sha256:20b159aa3badc9d5ee8f5c647e5efd02ed2a66ab8d354930bd9ff139fc1dc0a3 \

Просмотреть файл

@ -41,10 +41,10 @@ class AbuseReportTypeFilter(admin.SimpleListFilter):
class AbuseReportAdmin(admin.ModelAdmin):
raw_id_fields = ('addon', 'user', 'reporter')
readonly_fields = ('ip_address', 'message', 'created', 'addon', 'user',
'guid', 'reporter')
list_display = ('reporter', 'ip_address', 'type', 'target', 'message',
'created')
readonly_fields = ('country_code', 'message', 'created',
'addon', 'user', 'guid', 'reporter')
list_display = ('reporter', 'country_code', 'type', 'target',
'message', 'created')
list_filter = (AbuseReportTypeFilter, )
actions = ('delete_selected',)

Просмотреть файл

Просмотреть файл

Просмотреть файл

@ -0,0 +1,25 @@
from django.core.management.base import BaseCommand, CommandError
import olympia.core.logger
from olympia.abuse.models import AbuseReport
log = olympia.core.logger.getLogger('z.abuse')
class Command(BaseCommand):
def setup_check(self):
value = AbuseReport.lookup_country_code_from_ip('1.1.1.1')
if not value:
raise CommandError('GeoIP lookups does not appear to be working.')
def handle(self, *args, **kwargs):
self.setup_check()
qs = AbuseReport.objects.only('ip_address', 'country_code').filter(
ip_address__isnull=False, country_code__isnull=True)
for report in qs:
log.info('Looking up country_code for abuse report %d', report.pk)
value = AbuseReport.lookup_country_code_from_ip(report.ip_address)
report.update(country_code=value)

Просмотреть файл

@ -1,10 +1,15 @@
from django import forms
from django.conf import settings
from django.contrib.gis.geoip2 import GeoIP2, GeoIP2Exception
from django.core.validators import validate_ipv46_address
from django.db import models
from django.utils import translation
from django.utils.encoding import python_2_unicode_compatible
import six
from geoip2.errors import GeoIP2Error
from olympia import amo
from olympia.addons.models import Addon
from olympia.amo.models import ModelBase
@ -53,7 +58,10 @@ class AbuseReport(ModelBase):
reporter = models.ForeignKey(
UserProfile, null=True, blank=True, related_name='abuse_reported',
on_delete=models.SET_NULL)
ip_address = models.CharField(max_length=255, default='0.0.0.0')
# ip_address should be removed in a future release once we've migrated
# existing reports in the database to 'country'.
ip_address = models.CharField(max_length=255, default=None, null=True)
country_code = models.CharField(max_length=2, default=None, null=True)
# An abuse report can be for an addon or a user.
# If user is non-null then both addon and guid should be null.
# If user is null then addon should be non-null if guid was in our DB,
@ -128,6 +136,21 @@ class AbuseReport(ModelBase):
if creation:
self.send()
@classmethod
def lookup_country_code_from_ip(cls, ip):
try:
# Early check to avoid initializing GeoIP2 on invalid addresses
if not ip:
raise forms.ValidationError('No IP')
validate_ipv46_address(ip)
geoip = GeoIP2()
value = geoip.country_code(ip)
# Annoyingly, we have to catch both django's GeoIP2Exception (setup
# issue) and geoip2's GeoIP2Error (lookup issue)
except (forms.ValidationError, GeoIP2Exception, GeoIP2Error):
value = ''
return value
@property
def target(self):
return self.addon or self.user
@ -146,8 +169,9 @@ class AbuseReport(ModelBase):
def send_abuse_report(request, obj, message):
# Only used by legacy frontend
report = AbuseReport(ip_address=request.META.get('REMOTE_ADDR'),
message=message)
country_code = AbuseReport.lookup_country_code_from_ip(
request.META.get('REMOTE_ADDR'))
report = AbuseReport(ip_address=country_code, message=message)
if request.user.is_authenticated:
report.reporter = request.user
if isinstance(obj, Addon):

Просмотреть файл

@ -23,7 +23,8 @@ class BaseAbuseReportSerializer(serializers.ModelSerializer):
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')
output['country_code'] = AbuseReport.lookup_country_code_from_ip(
request.META.get('REMOTE_ADDR'))
if request.user.is_authenticated:
output['reporter'] = request.user
return output

Просмотреть файл

@ -19,8 +19,7 @@ class TestAbuse(TestCase):
# Create a few abuse reports
AbuseReport.objects.create(addon=addon, message='Foo')
AbuseReport.objects.create(
addon=addon, ip_address='1.2.3.4', reporter=user_factory(),
message='Bar')
addon=addon, reporter=user_factory(), message='Bar')
# This is a report for an addon not in the database
AbuseReport.objects.create(guid='@guid', message='Foo')
AbuseReport.objects.create(user=user_factory(), message='Eheheheh')

Просмотреть файл

@ -2,9 +2,10 @@
from django.conf import settings
from django.core import mail
import mock
import six
from olympia.abuse.models import AbuseReport
from olympia.abuse.models import AbuseReport, GeoIP2Error, GeoIP2Exception
from olympia.amo.tests import TestCase
@ -56,3 +57,20 @@ class TestAbuse(TestCase):
mail.outbox[0].subject ==
u'[Addon] Abuse Report for foo@bar.org')
assert 'GUID not in database' in mail.outbox[0].body
@mock.patch('olympia.abuse.models.GeoIP2')
def test_lookup_country_code_from_ip(self, GeoIP2_mock):
GeoIP2_mock.return_value.country_code.return_value = 'ZZ'
assert AbuseReport.lookup_country_code_from_ip('') == ''
assert AbuseReport.lookup_country_code_from_ip('notanip') == ''
assert GeoIP2_mock.return_value.country_code.call_count == 0
GeoIP2_mock.return_value.country_code.return_value = 'ZZ'
assert AbuseReport.lookup_country_code_from_ip('127.0.0.1') == 'ZZ'
assert AbuseReport.lookup_country_code_from_ip('::1') == 'ZZ'
GeoIP2_mock.return_value.country_code.side_effect = GeoIP2Exception
assert AbuseReport.lookup_country_code_from_ip('127.0.0.1') == ''
GeoIP2_mock.return_value.country_code.side_effect = GeoIP2Error
assert AbuseReport.lookup_country_code_from_ip('127.0.0.1') == ''

Просмотреть файл

@ -4,6 +4,7 @@ from datetime import datetime
from django.core import mail
import mock
import six
from olympia import amo
@ -17,13 +18,18 @@ class AddonAbuseViewSetTestBase(object):
def setUp(self):
self.url = reverse_ns('abusereportaddon-list')
geoip_patcher = mock.patch('olympia.abuse.models.GeoIP2')
self.GeoIP2_mock = geoip_patcher.start()
self.GeoIP2_mock.return_value.country_code.return_value = 'ZZ'
self.addCleanup(geoip_patcher.stop)
def check_reporter(self, report):
raise NotImplementedError
def check_report(self, report, text):
assert six.text_type(report) == text
assert report.ip_address == '123.45.67.89'
assert report.ip_address is None
assert report.country_code == 'ZZ'
assert mail.outbox[0].subject == text
self.check_reporter(report)
@ -257,13 +263,17 @@ class UserAbuseViewSetTestBase(object):
def setUp(self):
self.url = reverse_ns('abusereportuser-list')
geoip_patcher = mock.patch('olympia.abuse.models.GeoIP2')
self.GeoIP2_mock = geoip_patcher.start()
self.GeoIP2_mock.return_value.country_code.return_value = 'ZZ'
self.addCleanup(geoip_patcher.stop)
def check_reporter(self, report):
raise NotImplementedError
def check_report(self, report, text):
assert six.text_type(report) == text
assert report.ip_address == '123.45.67.89'
assert report.country_code == 'ZZ'
assert mail.outbox[0].subject == text
self.check_reporter(report)

Просмотреть файл

@ -1831,3 +1831,5 @@ AKISMET_API_URL = 'https://{api_key}.rest.akismet.com/1.1/{action}'
AKISMET_API_KEY = env('AKISMET_API_KEY', default=None)
AKISMET_API_TIMEOUT = 5
AKISMET_REAL_SUBMIT = False
GEOIP_PATH = '/usr/local/share/GeoIP/GeoLite2-Country.mmdb'

Просмотреть файл

@ -0,0 +1,3 @@
ALTER TABLE `abuse_reports`
ADD COLUMN `country_code` varchar(2),
MODIFY `ip_address` varchar(255);

Просмотреть файл

@ -11,8 +11,8 @@
{% else %}
{% set object_name = report.user|user_link %}
{% endif %}
{% trans user=user_link, date=report.created|date, ip_address=report.ip_address, object=object_name %}
{{ user }} [{{ ip_address }}] reported {{ object }} on {{ date }}
{% trans user=user_link, date=report.created|date, country_code=report.country_code, object=object_name %}
{{ user }} [{{ country_code }}] reported {{ object }} on {{ date }}
{% endtrans %}
<blockquote>{{ report.message }}</blockquote>
</li>

Просмотреть файл

@ -4225,7 +4225,7 @@ class TestReview(ReviewBase):
def test_abuse_reports(self):
report = AbuseReport.objects.create(
addon=self.addon, message=u'Et mël mazim ludus.',
ip_address='10.1.2.3')
country_code='FR')
created_at = defaultfilters.date(report.created)
response = self.client.get(self.url)
assert response.status_code == 200
@ -4246,13 +4246,13 @@ class TestReview(ReviewBase):
assert doc('.abuse_reports')
assert (
doc('.abuse_reports').text() ==
u'anonymous [10.1.2.3] reported Public on %s\nEt mël mazim ludus.'
u'anonymous [FR] reported Public on %s\nEt mël mazim ludus.'
% created_at)
def test_abuse_reports_developers(self):
report = AbuseReport.objects.create(
user=self.addon.listed_authors[0], message=u'Foo, Bâr!',
ip_address='10.4.5.6')
country_code='DE')
created_at = defaultfilters.date(report.created)
AutoApprovalSummary.objects.create(
verdict=amo.AUTO_APPROVED, version=self.version)
@ -4263,7 +4263,7 @@ class TestReview(ReviewBase):
assert doc('.abuse_reports')
assert (
doc('.abuse_reports').text() ==
u'anonymous [10.4.5.6] reported regularuser التطب on %s\nFoo, Bâr!'
u'anonymous [DE] reported regularuser التطب on %s\nFoo, Bâr!'
% created_at)
def test_user_ratings(self):