diff --git a/cms/envs/common.py b/cms/envs/common.py index 6d0fd9b18d..2463a9e8a8 100644 --- a/cms/envs/common.py +++ b/cms/envs/common.py @@ -345,8 +345,7 @@ sys.path.append(PROJECT_ROOT / 'djangoapps') sys.path.append(COMMON_ROOT / 'djangoapps') # For geolocation ip database -GEOIP_PATH = REPO_ROOT / "common/static/data/geoip/GeoIP.dat" -GEOIPV6_PATH = REPO_ROOT / "common/static/data/geoip/GeoIPv6.dat" +GEOIP_PATH = REPO_ROOT / "common/static/data/geoip/GeoLite2-Country.mmdb" ############################# TEMPLATE CONFIGURATION ############################# # Mako templating diff --git a/common/static/data/geoip/GeoIP.dat b/common/static/data/geoip/GeoIP.dat deleted file mode 100644 index be8b031f7d..0000000000 Binary files a/common/static/data/geoip/GeoIP.dat and /dev/null differ diff --git a/common/static/data/geoip/GeoIPv6.dat b/common/static/data/geoip/GeoIPv6.dat deleted file mode 100644 index c2fa3b6223..0000000000 Binary files a/common/static/data/geoip/GeoIPv6.dat and /dev/null differ diff --git a/common/static/data/geoip/GeoLite2-Country.mmdb b/common/static/data/geoip/GeoLite2-Country.mmdb new file mode 100644 index 0000000000..2452231a6d Binary files /dev/null and b/common/static/data/geoip/GeoLite2-Country.mmdb differ diff --git a/conf/locale/config.yaml b/conf/locale/config.yaml index ae1c960c25..da087ae7f5 100644 --- a/conf/locale/config.yaml +++ b/conf/locale/config.yaml @@ -128,7 +128,7 @@ ignore_dirs: - src/done-xblock - src/edx-jsme - src/parse-rest - - src/pygeoip + - src/geoip2 - src/pystache-custom - src/rate-xblock - src/xblock-google-drive diff --git a/lms/envs/common.py b/lms/envs/common.py index 69e129ea56..5688bf9a0c 100644 --- a/lms/envs/common.py +++ b/lms/envs/common.py @@ -450,9 +450,7 @@ node_paths = [ NODE_PATH = ':'.join(node_paths) # For geolocation ip database -GEOIP_PATH = REPO_ROOT / "common/static/data/geoip/GeoIP.dat" -GEOIPV6_PATH = REPO_ROOT / "common/static/data/geoip/GeoIPv6.dat" - +GEOIP_PATH = REPO_ROOT / "common/static/data/geoip/GeoLite2-Country.mmdb" # Where to look for a status message STATUS_MESSAGE_PATH = ENV_ROOT / "status_message.json" diff --git a/openedx/core/djangoapps/embargo/api.py b/openedx/core/djangoapps/embargo/api.py index 0c2d39171c..9b2d790493 100644 --- a/openedx/core/djangoapps/embargo/api.py +++ b/openedx/core/djangoapps/embargo/api.py @@ -13,7 +13,7 @@ from ipware.ip import get_ip from rest_framework import status from rest_framework.response import Response -import pygeoip +import geoip2.database from student.auth import has_course_author_access from .models import CountryAccessRule, RestrictedCourse @@ -170,10 +170,16 @@ def _country_code_from_ip(ip_addr): str: A 2-letter country code. """ - if ip_addr.find(':') >= 0: - return pygeoip.GeoIP(settings.GEOIPV6_PATH).country_code_by_addr(ip_addr) - else: - return pygeoip.GeoIP(settings.GEOIP_PATH).country_code_by_addr(ip_addr) + reader = geoip2.database.Reader(settings.GEOIP_PATH) + + try: + response = reader.country(ip_addr) + # pylint: disable=no-member + country_code = response.country.iso_code + except geoip2.errors.AddressNotFoundError: + country_code = "" + reader.close() + return country_code def get_embargo_response(request, course_id, user): diff --git a/openedx/core/djangoapps/embargo/test_utils.py b/openedx/core/djangoapps/embargo/test_utils.py index 57f7039973..10e95eaab3 100644 --- a/openedx/core/djangoapps/embargo/test_utils.py +++ b/openedx/core/djangoapps/embargo/test_utils.py @@ -1,11 +1,12 @@ """Utilities for writing unit tests that involve course embargos. """ import contextlib -import mock +import maxminddb from django.core.cache import cache from django.urls import reverse -import pygeoip +import geoip2.database +from mock import MagicMock, patch from .models import Country, CountryAccessRule, RestrictedCourse @@ -45,40 +46,56 @@ def restrict_course(course_key, access_point="enrollment", disable_access_check= # with this test. cache.clear() - with mock.patch.object(pygeoip.GeoIP, 'country_code_by_addr') as mock_ip: - - # Remove all existing rules for the course - CountryAccessRule.objects.all().delete() - - # Create the country object - # Ordinarily, we'd create models for every country, - # but that would slow down the test suite. - country, __ = Country.objects.get_or_create(country='IR') - - # Create a model for the restricted course - restricted_course, __ = RestrictedCourse.objects.get_or_create(course_key=course_key) - restricted_course.enroll_msg_key = 'default' - restricted_course.access_msg_key = 'default' - restricted_course.disable_access_check = disable_access_check - restricted_course.save() - - # Ensure that there is a blacklist rule for the country - CountryAccessRule.objects.get_or_create( - restricted_course=restricted_course, - country=country, - rule_type='blacklist' - ) - + # pylint: disable=unused-argument + def mock_country(reader, country): + """ + :param reader: + :param country: + :return: + """ + magic_mock = MagicMock() + magic_mock.country = MagicMock() # Simulate that the user is coming from the blacklisted country - mock_ip.return_value = 'IR' + type(magic_mock.country).iso_code = 'IR' - # Yield the redirect url so the tests don't need to know - # the embargo messaging URL structure. - redirect_url = reverse( - 'embargo:blocked_message', - kwargs={ - 'access_point': access_point, - 'message_key': 'default' - } - ) - yield redirect_url + return magic_mock + + patcher = patch.object(maxminddb, 'open_database') + patcher.start() + country_patcher = patch.object(geoip2.database.Reader, 'country', mock_country) + country_patcher.start() + + # Remove all existing rules for the course + CountryAccessRule.objects.all().delete() + + # Create the country object + # Ordinarily, we'd create models for every country, + # but that would slow down the test suite. + country, __ = Country.objects.get_or_create(country='IR') + + # Create a model for the restricted course + restricted_course, __ = RestrictedCourse.objects.get_or_create(course_key=course_key) + restricted_course.enroll_msg_key = 'default' + restricted_course.access_msg_key = 'default' + restricted_course.disable_access_check = disable_access_check + restricted_course.save() + + # Ensure that there is a blacklist rule for the country + CountryAccessRule.objects.get_or_create( + restricted_course=restricted_course, + country=country, + rule_type='blacklist' + ) + + # Yield the redirect url so the tests don't need to know + # the embargo messaging URL structure. + redirect_url = reverse( + 'embargo:blocked_message', + kwargs={ + 'access_point': access_point, + 'message_key': 'default' + } + ) + yield redirect_url + patcher.stop() + country_patcher.stop() diff --git a/openedx/core/djangoapps/embargo/tests/test_api.py b/openedx/core/djangoapps/embargo/tests/test_api.py index 11093519a0..dc0ab9d7f6 100644 --- a/openedx/core/djangoapps/embargo/tests/test_api.py +++ b/openedx/core/djangoapps/embargo/tests/test_api.py @@ -1,12 +1,15 @@ """ Tests for EmbargoMiddleware """ - from contextlib import contextmanager -import mock -import pygeoip + +import geoip2.database +import maxminddb import ddt +import mock +from mock import patch, MagicMock + from django.conf import settings from django.test.utils import override_settings from django.core.cache import cache @@ -24,14 +27,13 @@ from student.roles import ( OrgStaffRole, OrgInstructorRole ) +from util.testing import UrlResetMixin from ..models import ( RestrictedCourse, Country, CountryAccessRule, ) -from util.testing import UrlResetMixin from .. import api as embargo_api from ..exceptions import InvalidAccessPoint -from mock import patch MODULESTORE_CONFIG = mixed_store_config(settings.COMMON_TEST_DATA_ROOT, {}) @@ -106,7 +108,8 @@ class EmbargoCheckAccessApiTests(ModuleStoreTestCase): ) # The user is set to None, because the user has not been authenticated. - result = embargo_api.check_course_access(self.course.id, ip_address='0.0.0.0') + with self._mock_geoip(""): + result = embargo_api.check_course_access(self.course.id, ip_address='0.0.0.0') self.assertTrue(result) def test_no_user_blocked(self): @@ -135,11 +138,13 @@ class EmbargoCheckAccessApiTests(ModuleStoreTestCase): def test_ip_v6(self): # Test the scenario that will go through every check # (restricted course, but pass all the checks) - result = embargo_api.check_course_access(self.course.id, user=self.user, ip_address='FE80::0202:B3FF:FE1E:8329') + with self._mock_geoip('US'): + result = embargo_api.check_course_access(self.course.id, user=self.user, + ip_address='FE80::0202:B3FF:FE1E:8329') self.assertTrue(result) def test_country_access_fallback_to_continent_code(self): - # Simulate PyGeoIP falling back to a continent code + # Simulate Geolite2 falling back to a continent code # instead of a country code. In this case, we should # allow the user access. with self._mock_geoip('EU'): @@ -160,7 +165,8 @@ class EmbargoCheckAccessApiTests(ModuleStoreTestCase): connection.cursor().execute(query, [str(self.user.profile.id)]) # Verify that we can check the user's access without error - result = embargo_api.check_course_access(self.course.id, user=self.user, ip_address='0.0.0.0') + with self._mock_geoip('US'): + result = embargo_api.check_course_access(self.course.id, user=self.user, ip_address='0.0.0.0') self.assertTrue(result) def test_caching(self): @@ -229,9 +235,27 @@ class EmbargoCheckAccessApiTests(ModuleStoreTestCase): """ Mock for the GeoIP module. """ - with mock.patch.object(pygeoip.GeoIP, 'country_code_by_addr') as mock_ip: - mock_ip.return_value = country_code - yield + + # pylint: disable=unused-argument + def mock_country(reader, country): + """ + :param reader: + :param country: + :return: + """ + magic_mock = MagicMock() + magic_mock.country = MagicMock() + type(magic_mock.country).iso_code = country_code + + return magic_mock + + patcher = patch.object(maxminddb, 'open_database') + patcher.start() + country_patcher = patch.object(geoip2.database.Reader, 'country', new=mock_country) + country_patcher.start() + self.addCleanup(patcher.stop) + self.addCleanup(country_patcher.stop) + yield @ddt.ddt diff --git a/openedx/core/djangoapps/embargo/tests/test_views.py b/openedx/core/djangoapps/embargo/tests/test_views.py index f4114065d3..ac5333f8b7 100644 --- a/openedx/core/djangoapps/embargo/tests/test_views.py +++ b/openedx/core/djangoapps/embargo/tests/test_views.py @@ -1,12 +1,12 @@ """Tests for embargo app views. """ import ddt -import mock -import pygeoip +import maxminddb +import geoip2.database from django.urls import reverse from django.conf import settings -from mock import patch +from mock import patch, MagicMock from .factories import CountryAccessRuleFactory, RestrictedCourseFactory from .. import messages @@ -124,9 +124,29 @@ class CheckCourseAccessViewTest(CourseApiFactoryMixin, ModuleStoreTestCase): self.user.is_staff = False self.user.save() # Appear to make a request from an IP in the blocked country - with mock.patch.object(pygeoip.GeoIP, 'country_code_by_addr') as mock_ip: - mock_ip.return_value = 'US' - response = self.client.get(self.url, data=self.request_data) + + # pylint: disable=unused-argument + def mock_country(reader, country): + """ + :param reader: + :param country: + :return: + """ + magic_mock = MagicMock() + magic_mock.country = MagicMock() + type(magic_mock.country).iso_code = 'US' + + return magic_mock + + patcher = patch.object(maxminddb, 'open_database') + patcher.start() + country_patcher = patch.object(geoip2.database.Reader, 'country', mock_country) + country_patcher.start() + self.addCleanup(patcher.stop) + self.addCleanup(country_patcher.stop) + + response = self.client.get(self.url, data=self.request_data) + expected_response = {'access': False} self.assertEqual(response.status_code, 200) self.assertEqual(response.data, expected_response) diff --git a/openedx/core/djangoapps/geoinfo/middleware.py b/openedx/core/djangoapps/geoinfo/middleware.py index cf89c787e8..ed74fe6275 100644 --- a/openedx/core/djangoapps/geoinfo/middleware.py +++ b/openedx/core/djangoapps/geoinfo/middleware.py @@ -15,7 +15,7 @@ import logging from django.conf import settings from ipware.ip import get_real_ip -import pygeoip +import geoip2.database log = logging.getLogger(__name__) @@ -37,10 +37,15 @@ class CountryMiddleware(object): del request.session['ip_address'] del request.session['country_code'] elif new_ip_address != old_ip_address: - if new_ip_address.find(':') >= 0: - country_code = pygeoip.GeoIP(settings.GEOIPV6_PATH).country_code_by_addr(new_ip_address) - else: - country_code = pygeoip.GeoIP(settings.GEOIP_PATH).country_code_by_addr(new_ip_address) + reader = geoip2.database.Reader(settings.GEOIP_PATH) + try: + response = reader.country(new_ip_address) + # pylint: disable=no-member + country_code = response.country.iso_code + except geoip2.errors.AddressNotFoundError: + country_code = "" + request.session['country_code'] = country_code request.session['ip_address'] = new_ip_address log.debug(u'Country code for IP: %s is set to %s', new_ip_address, country_code) + reader.close() diff --git a/openedx/core/djangoapps/geoinfo/tests/test_middleware.py b/openedx/core/djangoapps/geoinfo/tests/test_middleware.py index a7f28a9601..c36aec9dbc 100644 --- a/openedx/core/djangoapps/geoinfo/tests/test_middleware.py +++ b/openedx/core/djangoapps/geoinfo/tests/test_middleware.py @@ -1,8 +1,10 @@ """ Tests for CountryMiddleware. """ -from mock import patch -import pygeoip +import geoip2 +import maxminddb + +from mock import patch, MagicMock, PropertyMock from django.contrib.sessions.middleware import SessionMiddleware from django.test import TestCase @@ -23,13 +25,17 @@ class CountryMiddlewareTests(TestCase): self.authenticated_user = UserFactory.create() self.anonymous_user = AnonymousUserFactory.create() self.request_factory = RequestFactory() - self.patcher = patch.object(pygeoip.GeoIP, 'country_code_by_addr', self.mock_country_code_by_addr) - self.patcher.start() - self.addCleanup(self.patcher.stop) + patcher = patch.object(maxminddb, 'open_database') + patcher.start() + country_patcher = patch.object(geoip2.database.Reader, 'country', self.mock_country) + country_patcher.start() + self.addCleanup(patcher.stop) + self.addCleanup(country_patcher.stop) - def mock_country_code_by_addr(self, ip_addr): + def mock_country(self, ip_address): """ - Gives us a fake set of IPs + :param ip_address: + :return: """ ip_dict = { '117.79.83.1': 'CN', @@ -37,7 +43,12 @@ class CountryMiddlewareTests(TestCase): '4.0.0.0': 'SD', '2001:da8:20f:1502:edcf:550b:4a9c:207d': 'CN', } - return ip_dict.get(ip_addr, 'US') + + magic_mock = MagicMock() + magic_mock.country = MagicMock() + type(magic_mock.country).iso_code = PropertyMock(return_value=ip_dict.get(ip_address)) + + return magic_mock def test_country_code_added(self): request = self.request_factory.get( diff --git a/requirements/edx/base.in b/requirements/edx/base.in index 525c8c8805..7c43a84254 100644 --- a/requirements/edx/base.in +++ b/requirements/edx/base.in @@ -150,3 +150,4 @@ web-fragments # Provides the ability to render fragments o XBlock # Courseware component architecture xblock-utils # Provides utilities used by the Discussion XBlock zendesk # Python API for the Zendesk customer support system +geoip2==2.9.0 # Python API for the GeoIP web services and databases diff --git a/requirements/edx/base.txt b/requirements/edx/base.txt index ee59f4e07b..200c6182c5 100644 --- a/requirements/edx/base.txt +++ b/requirements/edx/base.txt @@ -26,7 +26,6 @@ git+https://github.com/edx/MongoDBProxy.git@25b99097615bda06bd7cdfe5669ed80dc2a7 -e . git+https://github.com/edx/edx-ora2.git@2.2.1#egg=ora2==2.2.1 -e git+https://github.com/dgrtwo/ParsePy.git@7949b9f754d1445eff8e8f20d0e967b9a6420639#egg=parse_rest --e git+https://github.com/appliedsec/pygeoip.git@95e69341cebf5a6a9fbf7c4f5439d458898bdc3b#egg=pygeoip -e git+https://github.com/dementrock/pystache_custom.git@776973740bdaad83a3b029f96e415a7d1e8bec2f#egg=pystache_custom-dev -e git+https://github.com/edx/RateXBlock.git@367e19c0f6eac8a5f002fd0f1559555f8e74bfff#egg=rate-xblock git+https://github.com/edx/RecommenderXBlock.git@1.4.0#egg=recommender-xblock==1.4.0 @@ -136,6 +135,7 @@ fs-s3fs==0.1.8 fs==2.0.18 future==0.17.1 # via pyjwkest futures==3.2.0 ; python_version == "2.7" # via python-swiftclient, s3transfer, xblock-utils +geoip2==2.9.0 glob2==0.6 gunicorn==19.0 hash-ring==1.3.1 # via django-memcached-hashring @@ -161,6 +161,7 @@ mako==1.0.2 markdown==2.6.11 markey==0.8 # via django-babel-underscore markupsafe==1.1.0 +maxminddb==1.4.1 # via geoip2 mock==1.0.1 mongoengine==0.10.0 mysql-python==1.2.5 diff --git a/requirements/edx/development.txt b/requirements/edx/development.txt index 7f82b25713..13419b5a69 100644 --- a/requirements/edx/development.txt +++ b/requirements/edx/development.txt @@ -28,7 +28,6 @@ git+https://github.com/edx/MongoDBProxy.git@25b99097615bda06bd7cdfe5669ed80dc2a7 -e . git+https://github.com/edx/edx-ora2.git@2.2.1#egg=ora2==2.2.1 -e git+https://github.com/dgrtwo/ParsePy.git@7949b9f754d1445eff8e8f20d0e967b9a6420639#egg=parse_rest --e git+https://github.com/appliedsec/pygeoip.git@95e69341cebf5a6a9fbf7c4f5439d458898bdc3b#egg=pygeoip -e git+https://github.com/dementrock/pystache_custom.git@776973740bdaad83a3b029f96e415a7d1e8bec2f#egg=pystache_custom-dev -e git+https://github.com/edx/RateXBlock.git@367e19c0f6eac8a5f002fd0f1559555f8e74bfff#egg=rate-xblock git+https://github.com/edx/RecommenderXBlock.git@1.4.0#egg=recommender-xblock==1.4.0 @@ -60,7 +59,7 @@ beautifulsoup4==4.7.1 before-after==1.0.1 billiard==3.3.0.23 bleach==2.1.4 -bok-choy==0.9.0 +bok-choy==0.9.3 boto3==1.4.8 boto==2.39.0 botocore==1.8.17 @@ -174,6 +173,7 @@ functools32==3.2.3.post2 ; python_version == "2.7" future==0.17.1 futures==3.2.0 ; python_version == "2.7" fuzzywuzzy==0.17.0 +geoip2==2.9.0 glob2==0.6 gunicorn==19.0 hash-ring==1.3.1 @@ -210,6 +210,7 @@ mando==0.6.4 markdown==2.6.11 markey==0.8 markupsafe==1.1.0 +maxminddb==1.4.1 mccabe==0.6.1 mock==1.0.1 modernize==0.6.1 @@ -236,7 +237,7 @@ pbr==5.1.2 pdfminer==20140328 piexif==1.0.2 pillow==5.4.1 -pip-tools==3.3.2 +pip-tools==3.4.0 pkgconfig==1.4.0 pluggy==0.8.1 polib==1.1.0 diff --git a/requirements/edx/github.in b/requirements/edx/github.in index 8bfe25ee45..727f31e94b 100644 --- a/requirements/edx/github.in +++ b/requirements/edx/github.in @@ -66,7 +66,6 @@ -e git+https://github.com/edx/django-openid-auth.git@0.15.1#egg=django-openid-auth==0.15.1 -e git+https://github.com/edx/MongoDBProxy.git@25b99097615bda06bd7cdfe5669ed80dc2a7fed0#egg=MongoDBProxy==0.1.0 -e git+https://github.com/dementrock/pystache_custom.git@776973740bdaad83a3b029f96e415a7d1e8bec2f#egg=pystache_custom-dev --e git+https://github.com/appliedsec/pygeoip.git@95e69341cebf5a6a9fbf7c4f5439d458898bdc3b#egg=pygeoip -e git+https://github.com/jazkarta/edx-jsme.git@690dbf75441fa91c7c4899df0b83d77f7deb5458#egg=edx-jsme -e git+https://github.com/mitodl/django-cas.git@afac57bc523f145ae826f4ed3d4fa8b2c86c5364#egg=django-cas==2.1.1 -e git+https://github.com/dgrtwo/ParsePy.git@7949b9f754d1445eff8e8f20d0e967b9a6420639#egg=parse_rest diff --git a/requirements/edx/pip-tools.txt b/requirements/edx/pip-tools.txt index a4d5950056..1178612730 100644 --- a/requirements/edx/pip-tools.txt +++ b/requirements/edx/pip-tools.txt @@ -5,5 +5,5 @@ # make upgrade # click==7.0 # via pip-tools -pip-tools==3.3.2 +pip-tools==3.4.0 six==1.11.0 diff --git a/requirements/edx/testing.txt b/requirements/edx/testing.txt index 5b074cc240..5b3e8d1072 100644 --- a/requirements/edx/testing.txt +++ b/requirements/edx/testing.txt @@ -27,7 +27,6 @@ git+https://github.com/edx/MongoDBProxy.git@25b99097615bda06bd7cdfe5669ed80dc2a7 -e . git+https://github.com/edx/edx-ora2.git@2.2.1#egg=ora2==2.2.1 -e git+https://github.com/dgrtwo/ParsePy.git@7949b9f754d1445eff8e8f20d0e967b9a6420639#egg=parse_rest --e git+https://github.com/appliedsec/pygeoip.git@95e69341cebf5a6a9fbf7c4f5439d458898bdc3b#egg=pygeoip -e git+https://github.com/dementrock/pystache_custom.git@776973740bdaad83a3b029f96e415a7d1e8bec2f#egg=pystache_custom-dev -e git+https://github.com/edx/RateXBlock.git@367e19c0f6eac8a5f002fd0f1559555f8e74bfff#egg=rate-xblock git+https://github.com/edx/RecommenderXBlock.git@1.4.0#egg=recommender-xblock==1.4.0 @@ -58,7 +57,7 @@ beautifulsoup4==4.7.1 before-after==1.0.1 billiard==3.3.0.23 bleach==2.1.4 -bok-choy==0.9.0 +bok-choy==0.9.3 boto3==1.4.8 boto==2.39.0 botocore==1.8.17 @@ -169,6 +168,7 @@ functools32==3.2.3.post2 ; python_version == "2.7" # via flake8, parsel future==0.17.1 futures==3.2.0 ; python_version == "2.7" fuzzywuzzy==0.17.0 +geoip2==2.9.0 glob2==0.6 gunicorn==19.0 hash-ring==1.3.1 @@ -204,6 +204,7 @@ mando==0.6.4 # via radon markdown==2.6.11 markey==0.8 markupsafe==1.1.0 +maxminddb==1.4.1 mccabe==0.6.1 # via flake8, pylint mock==1.0.1 mongoengine==0.10.0