From ba6e61b907a28e136948d5775071a2dcd4e1a1a6 Mon Sep 17 00:00:00 2001 From: Alan Boudreault Date: Sat, 1 Mar 2014 18:44:58 -0500 Subject: [PATCH 1/4] Extended Embargo feature to support site access restriction and ip network --- cms/envs/common.py | 3 + common/djangoapps/embargo/forms.py | 23 ++---- common/djangoapps/embargo/middleware.py | 53 ++++++++++--- common/djangoapps/embargo/models.py | 31 +++++++- common/djangoapps/embargo/tests/test_forms.py | 26 +++++-- .../embargo/tests/test_middleware.py | 78 +++++++++++++++++++ .../djangoapps/embargo/tests/test_models.py | 16 ++++ lms/envs/common.py | 3 + requirements/edx/base.txt | 3 + 9 files changed, 202 insertions(+), 34 deletions(-) diff --git a/cms/envs/common.py b/cms/envs/common.py index f7195d2c54..01e167098a 100644 --- a/cms/envs/common.py +++ b/cms/envs/common.py @@ -88,6 +88,9 @@ FEATURES = { # Toggles embargo functionality 'EMBARGO': False, + # Toggle embargo site functionality + 'EMBARGO_SITE': False, + # Turn on/off Microsites feature 'USE_MICROSITES': False, diff --git a/common/djangoapps/embargo/forms.py b/common/djangoapps/embargo/forms.py index 09ea11445b..36684f342a 100644 --- a/common/djangoapps/embargo/forms.py +++ b/common/djangoapps/embargo/forms.py @@ -7,7 +7,7 @@ from django import forms from embargo.models import EmbargoedCourse, EmbargoedState, IPFilter from embargo.fixtures.country_codes import COUNTRY_CODES -import socket +import ipaddr from xmodule.modulestore.django import modulestore @@ -76,21 +76,12 @@ class IPFilterForm(forms.ModelForm): # pylint: disable=incomplete-protocol class Meta: # pylint: disable=missing-docstring model = IPFilter - def _is_valid_ipv4(self, address): - """Whether or not address is a valid ipv4 address""" + def _is_valid_ip(self, address): + """Whether or not address is a valid ipv4 address or ipv6 address""" try: - # Is this an ipv4 address? - socket.inet_pton(socket.AF_INET, address) - except socket.error: - return False - return True - - def _is_valid_ipv6(self, address): - """Whether or not address is a valid ipv6 address""" - try: - # Is this an ipv6 address? - socket.inet_pton(socket.AF_INET6, address) - except socket.error: + # Is this an valid ip address? + ipaddr.IPNetwork(address) + except ValueError: return False return True @@ -105,7 +96,7 @@ class IPFilterForm(forms.ModelForm): # pylint: disable=incomplete-protocol error_addresses = [] for addr in addresses.split(','): address = addr.strip() - if not (self._is_valid_ipv4(address) or self._is_valid_ipv6(address)): + if not self._is_valid_ip(address): error_addresses.append(address) if error_addresses: msg = 'Invalid IP Address(es): {0}'.format(error_addresses) diff --git a/common/djangoapps/embargo/middleware.py b/common/djangoapps/embargo/middleware.py index 2330065a94..f8ea70d565 100644 --- a/common/djangoapps/embargo/middleware.py +++ b/common/djangoapps/embargo/middleware.py @@ -1,11 +1,34 @@ -""" -Middleware for embargoing courses. +"""Middleware for embargoing site and courses. IMPORTANT NOTE: This code WILL NOT WORK if you have a misconfigured proxy server. If you are configuring embargo functionality, or if you are experiencing mysterious problems with embargoing, please check that your reverse proxy is setting any of the well known client IP address headers (ex., HTTP_X_FORWARDED_FOR). + +This middleware allows you to: + +* Embargoing courses (access restriction by courses) +* Embargoing site (access restriction of the main site) + +Embargo can restrict by states and whitelist/blacklist (IP Addresses +(ie. 10.0.0.0) or Networks (ie. 10.0.0.0/24)). + +Usage: + +# Enable the middleware in your settings + +# To enable Embargoing by courses, add: +FEATURES['EMBARGO'] = True # blocked ip will be redirected to /embargo + +# To enable Embargoing site: +FEATURES['EMBARGO_SITE'] = True + +# With EMBARGO_SITE, you can define an external to redirect with: +EMBARGO_SITE_REDIRECT_URL = 'https://www.edx.org/' + +# if EMBARGO_SITE_REDIRECT_URL is missing, a HttpResponseForbidden is returned. + """ import logging import pygeoip @@ -13,6 +36,7 @@ import pygeoip from django.core.exceptions import MiddlewareNotUsed from django.conf import settings from django.shortcuts import redirect +from django.http import HttpResponseRedirect, HttpResponseForbidden from ipware.ip import get_ip from util.request import course_id_from_url @@ -23,14 +47,16 @@ log = logging.getLogger(__name__) class EmbargoMiddleware(object): """ - Middleware for embargoing courses + Middleware for embargoing site and courses This is configured by creating ``EmbargoedCourse``, ``EmbargoedState``, and optionally ``IPFilter`` rows in the database, using the django admin site. """ def __init__(self): + self.site_enabled = settings.FEATURES.get('EMBARGO_SITE', False) # If embargoing is turned off, make this middleware do nothing - if not settings.FEATURES.get('EMBARGO', False): + if not settings.FEATURES.get('EMBARGO', False) and \ + not self.site_enabled: raise MiddlewareNotUsed() def process_request(self, request): @@ -41,21 +67,28 @@ class EmbargoMiddleware(object): course_id = course_id_from_url(url) # If they're trying to access a course that cares about embargoes - if EmbargoedCourse.is_embargoed(course_id): + if self.site_enabled or EmbargoedCourse.is_embargoed(course_id): + response = redirect('embargo') + # Set the proper response if site is enabled + if self.site_enabled: + redirect_url = getattr(settings, 'EMBARGO_SITE_REDIRECT_URL', None) + response = HttpResponseRedirect(redirect_url) if redirect_url \ + else HttpResponseForbidden('Access Denied') + # If we're having performance issues, add caching here ip_addr = get_ip(request) # if blacklisted, immediately fail if ip_addr in IPFilter.current().blacklist_ips: - log.info("Embargo: Restricting IP address %s to course %s because IP is blacklisted.", ip_addr, course_id) - return redirect('embargo') + log.info("Embargo: Restricting IP address %s because IP is blacklisted.", ip_addr) + return response country_code_from_ip = pygeoip.GeoIP(settings.GEOIP_PATH).country_code_by_addr(ip_addr) is_embargoed = country_code_from_ip in EmbargoedState.current().embargoed_countries_list # Fail if country is embargoed and the ip address isn't explicitly whitelisted if is_embargoed and ip_addr not in IPFilter.current().whitelist_ips: log.info( - "Embargo: Restricting IP address %s to course %s because IP is from country %s.", - ip_addr, course_id, country_code_from_ip + "Embargo: Restricting IP address %s because IP is from country %s.", + ip_addr, country_code_from_ip ) - return redirect('embargo') + return response diff --git a/common/djangoapps/embargo/models.py b/common/djangoapps/embargo/models.py index 4ee6613859..a48e39f1f2 100644 --- a/common/djangoapps/embargo/models.py +++ b/common/djangoapps/embargo/models.py @@ -10,6 +10,9 @@ file and check it in at the same time as your model changes. To do that, 2. ./manage.py lms schemamigration embargo --auto description_of_your_change 3. Add the migration file created in edx-platform/common/djangoapps/embargo/migrations/ """ + +import ipaddr + from django.db import models from config_models.models import ConfigurationModel @@ -79,6 +82,30 @@ class IPFilter(ConfigurationModel): help_text="A comma-separated list of IP addresses that should fall under embargo restrictions." ) + class IPFilterList(object): + """ + Represent a list of IP addresses with support of networks. + """ + + def __init__(self, ips): + self.networks = [ipaddr.IPNetwork(ip) for ip in ips] + + def __iter__(self): + for network in self.networks: + yield network + + def __contains__(self, ip): + try: + ip = ipaddr.IPAddress(ip) + except ValueError: + return False + + for network in self.networks: + if network.Contains(ip): + return True + + return False + @property def whitelist_ips(self): """ @@ -86,7 +113,7 @@ class IPFilter(ConfigurationModel): """ if self.whitelist == '': return [] - return [addr.strip() for addr in self.whitelist.split(',')] # pylint: disable=no-member + return self.IPFilterList([addr.strip() for addr in self.whitelist.split(',')]) # pylint: disable=no-member @property def blacklist_ips(self): @@ -95,4 +122,4 @@ class IPFilter(ConfigurationModel): """ if self.blacklist == '': return [] - return [addr.strip() for addr in self.blacklist.split(',')] # pylint: disable=no-member + return self.IPFilterList([addr.strip() for addr in self.blacklist.split(',')]) # pylint: disable=no-member diff --git a/common/djangoapps/embargo/tests/test_forms.py b/common/djangoapps/embargo/tests/test_forms.py index cea030c23d..dc14515474 100644 --- a/common/djangoapps/embargo/tests/test_forms.py +++ b/common/djangoapps/embargo/tests/test_forms.py @@ -156,8 +156,8 @@ class IPFilterFormTest(TestCase): # should be able to do both ipv4 and ipv6 # spacing should not matter form_data = { - 'whitelist': '127.0.0.1, 2003:dead:beef:4dad:23:46:bb:101', - 'blacklist': ' 18.244.1.5 , 2002:c0a8:101::42, 18.36.22.1' + 'whitelist': '127.0.0.1, 2003:dead:beef:4dad:23:46:bb:101, 1.1.0.1/32, 1.0.0.0/24', + 'blacklist': ' 18.244.1.5 , 2002:c0a8:101::42, 18.36.22.1, 1.0.0.0/16' } form = IPFilterForm(data=form_data) self.assertTrue(form.is_valid()) @@ -169,6 +169,20 @@ class IPFilterFormTest(TestCase): for addr in '18.244.1.5, 2002:c0a8:101::42, 18.36.22.1'.split(','): self.assertIn(addr.strip(), blacklist) + # Network tests + # ips not in whitelist network + for addr in '1.1.0.2, 1.0.1.0'.split(','): + self.assertNotIn(addr.strip(), whitelist) + # ips in whitelist network + for addr in '1.1.0.1, 1.0.0.100'.split(','): + self.assertIn(addr.strip(), whitelist) + # ips not in blacklist network + for addr in '2.0.0.0, 1.1.0.0'.split(','): + self.assertNotIn(addr.strip(), blacklist) + # ips in blacklist network + for addr in '1.0.100.0, 1.0.0.10'.split(','): + self.assertIn(addr.strip(), blacklist) + # Test clearing by adding an empty list is OK too form_data = { 'whitelist': '', @@ -183,15 +197,15 @@ class IPFilterFormTest(TestCase): def test_add_invalid_ips(self): # test adding invalid ip addresses form_data = { - 'whitelist': '.0.0.1, :dead:beef:::', - 'blacklist': ' 18.244.* , 999999:c0a8:101::42' + 'whitelist': '.0.0.1, :dead:beef:::, 1.0.0.0/55', + 'blacklist': ' 18.244.* , 999999:c0a8:101::42, 1.0.0.0/' } form = IPFilterForm(data=form_data) self.assertFalse(form.is_valid()) - wmsg = "Invalid IP Address(es): [u'.0.0.1', u':dead:beef:::'] Please fix the error(s) and try again." + wmsg = "Invalid IP Address(es): [u'.0.0.1', u':dead:beef:::', u'1.0.0.0/55'] Please fix the error(s) and try again." self.assertEquals(wmsg, form._errors['whitelist'][0]) # pylint: disable=protected-access - bmsg = "Invalid IP Address(es): [u'18.244.*', u'999999:c0a8:101::42'] Please fix the error(s) and try again." + bmsg = "Invalid IP Address(es): [u'18.244.*', u'999999:c0a8:101::42', u'1.0.0.0/'] Please fix the error(s) and try again." self.assertEquals(bmsg, form._errors['blacklist'][0]) # pylint: disable=protected-access with self.assertRaisesRegexp(ValueError, "The IPFilter could not be created because the data didn't validate."): diff --git a/common/djangoapps/embargo/tests/test_middleware.py b/common/djangoapps/embargo/tests/test_middleware.py index c3f418c905..db33ae02da 100644 --- a/common/djangoapps/embargo/tests/test_middleware.py +++ b/common/djangoapps/embargo/tests/test_middleware.py @@ -129,6 +129,62 @@ class EmbargoMiddlewareTests(TestCase): response = self.client.get(self.regular_page, HTTP_X_FORWARDED_FOR='5.0.0.0', REMOTE_ADDR='5.0.0.0') self.assertEqual(response.status_code, 200) + @unittest.skipUnless(settings.ROOT_URLCONF == 'lms.urls', 'Test only valid in lms') + def test_ip_network_exceptions(self): + # Explicitly whitelist/blacklist some IP networks + IPFilter( + whitelist='1.0.0.1/24', + blacklist='5.0.0.0/16,1.1.0.0/24', + changed_by=self.user, + enabled=True + ).save() + + # Accessing an embargoed page from a blocked IP that's been whitelisted with a network + # should succeed + response = self.client.get(self.embargoed_page, HTTP_X_FORWARDED_FOR='1.0.0.0', REMOTE_ADDR='1.0.0.0') + self.assertEqual(response.status_code, 200) + + # Accessing a regular course from a blocked IP that's been whitelisted with a network + # should succeed + response = self.client.get(self.regular_page, HTTP_X_FORWARDED_FOR='1.0.0.0', REMOTE_ADDR='1.0.0.0') + self.assertEqual(response.status_code, 200) + + # Accessing an embargoed course from non-embargoed IP that's been blacklisted with a network + # should cause a redirect + response = self.client.get(self.embargoed_page, HTTP_X_FORWARDED_FOR='5.0.0.100', REMOTE_ADDR='5.0.0.100') + self.assertEqual(response.status_code, 302) + # Following the redirect should give us the embargo page + response = self.client.get( + self.embargoed_page, + HTTP_X_FORWARDED_FOR='5.0.0.0', + REMOTE_ADDR='1.0.0.0', + follow=True + ) + self.assertIn(self.embargo_text, response.content) + + # Accessing an embargoed course from non-embargoed IP that's been blaclisted with a network + # should cause a redirect + response = self.client.get(self.embargoed_page, HTTP_X_FORWARDED_FOR='1.1.0.1', REMOTE_ADDR='1.1.0.1') + self.assertEqual(response.status_code, 302) + # Following the redirect should give us the embargo page + response = self.client.get( + self.embargoed_page, + HTTP_X_FORWARDED_FOR='1.1.0.0', + REMOTE_ADDR='1.1.0.0', + follow=True + ) + self.assertIn(self.embargo_text, response.content) + + # Accessing an embargoed from a blocked IP that's not blacklisted by the network rule. + # should succeed + response = self.client.get(self.embargoed_page, HTTP_X_FORWARDED_FOR='1.1.1.0', REMOTE_ADDR='1.1.1.0') + self.assertEqual(response.status_code, 200) + + # Accessing a regular course from a non-embargoed IP that's been blacklisted + # should succeed + response = self.client.get(self.regular_page, HTTP_X_FORWARDED_FOR='5.0.0.0', REMOTE_ADDR='5.0.0.0') + self.assertEqual(response.status_code, 200) + @unittest.skipUnless(settings.ROOT_URLCONF == 'lms.urls', 'Test only valid in lms') @mock.patch.dict(settings.FEATURES, {'EMBARGO': False}) def test_countries_embargo_off(self): @@ -157,3 +213,25 @@ class EmbargoMiddlewareTests(TestCase): # Accessing a regular course from a non-embargoed IP that's been blacklisted should succeed response = self.client.get(self.regular_page, HTTP_X_FORWARDED_FOR='5.0.0.0', REMOTE_ADDR='5.0.0.0') self.assertEqual(response.status_code, 200) + + @unittest.skipUnless(settings.ROOT_URLCONF == 'lms.urls', 'Test only valid in lms') + @mock.patch.dict(settings.FEATURES, {'EMBARGO': False, 'EMBARGO_SITE': True}) + def test_embargo_off_embargo_site_on(self): + # When the middleware is turned on with SITE, main site access should be restricted + # Accessing a regular page from a blocked IP is denied. + response = self.client.get(self.regular_page, HTTP_X_FORWARDED_FOR='1.0.0.0', REMOTE_ADDR='1.0.0.0') + self.assertEqual(response.status_code, 403) + + # Accessing a regular page from a non blocked IP should succeed + response = self.client.get(self.regular_page, HTTP_X_FORWARDED_FOR='5.0.0.0', REMOTE_ADDR='5.0.0.0') + self.assertEqual(response.status_code, 200) + + @unittest.skipUnless(settings.ROOT_URLCONF == 'lms.urls', 'Test only valid in lms') + @mock.patch.dict(settings.FEATURES, {'EMBARGO': False, 'EMBARGO_SITE': True}) + @override_settings(EMBARGO_SITE_REDIRECT_URL='https://www.edx.org/') + def test_embargo_off_embargo_site_on_with_redirect_url(self): + # When the middleware is turned on with EMBARGO_SITE, main site access + # should be restricted. Accessing a regular page from a blocked IP is + # denied, and redirected to EMBARGO_SITE_REDIRECT_URL rather than returning a 403. + response = self.client.get(self.regular_page, HTTP_X_FORWARDED_FOR='1.0.0.0', REMOTE_ADDR='1.0.0.0') + self.assertEqual(response.status_code, 302) diff --git a/common/djangoapps/embargo/tests/test_models.py b/common/djangoapps/embargo/tests/test_models.py index 12c66295b8..fb2e4c89c2 100644 --- a/common/djangoapps/embargo/tests/test_models.py +++ b/common/djangoapps/embargo/tests/test_models.py @@ -78,3 +78,19 @@ class EmbargoModelsTest(TestCase): self.assertTrue(whitelist in cwhitelist) cblacklist = IPFilter.current().blacklist_ips self.assertTrue(blacklist in cblacklist) + + # network tests + whitelist = '1.0.0.0/24' + blacklist = '1.1.0.0/16' + + IPFilter(whitelist=whitelist, blacklist=blacklist).save() + + cwhitelist = IPFilter.current().whitelist_ips + self.assertTrue('1.0.0.100' in cwhitelist) + self.assertTrue('1.0.0.10' in cwhitelist) + self.assertFalse('1.0.1.0' in cwhitelist) + cblacklist = IPFilter.current().blacklist_ips + self.assertTrue('1.1.0.0' in cblacklist) + self.assertTrue('1.1.0.1' in cblacklist) + self.assertTrue('1.1.1.0' in cblacklist) + self.assertFalse('1.2.0.0' in cblacklist) diff --git a/lms/envs/common.py b/lms/envs/common.py index da59ca3d7c..371c5ebf17 100644 --- a/lms/envs/common.py +++ b/lms/envs/common.py @@ -228,6 +228,9 @@ FEATURES = { # Toggle embargo functionality 'EMBARGO': False, + # Toggle embargo site functionality + 'EMBARGO_SITE': False, + # Whether the Wiki subsystem should be accessible via the direct /wiki/ paths. Setting this to True means # that people can submit content and modify the Wiki in any arbitrary manner. We're leaving this as True in the # defaults, so that we maintain current behavior diff --git a/requirements/edx/base.txt b/requirements/edx/base.txt index 1b1d1ad282..3976a825ea 100644 --- a/requirements/edx/base.txt +++ b/requirements/edx/base.txt @@ -91,6 +91,9 @@ sphinx_rtd_theme==0.1.5 Babel==1.3 transifex-client==0.10 +# Ip network support for Embargo feature +ipaddr==2.1.11 + # We've tried several times to update the debug toolbar to version 1.0.1, # and had problems each time, resulting in us rolling back to 0.9.4. Before # submitting another pull request to do this update, check the following: From 25948e369419730efe91a131b7976dfa35ca1267 Mon Sep 17 00:00:00 2001 From: Alan Boudreault Date: Sat, 1 Mar 2014 21:45:45 -0500 Subject: [PATCH 2/4] Fix minor typo in a test --- common/djangoapps/embargo/tests/test_middleware.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/common/djangoapps/embargo/tests/test_middleware.py b/common/djangoapps/embargo/tests/test_middleware.py index db33ae02da..528ea18f3b 100644 --- a/common/djangoapps/embargo/tests/test_middleware.py +++ b/common/djangoapps/embargo/tests/test_middleware.py @@ -156,8 +156,8 @@ class EmbargoMiddlewareTests(TestCase): # Following the redirect should give us the embargo page response = self.client.get( self.embargoed_page, - HTTP_X_FORWARDED_FOR='5.0.0.0', - REMOTE_ADDR='1.0.0.0', + HTTP_X_FORWARDED_FOR='5.0.0.100', + REMOTE_ADDR='5.0.0.100', follow=True ) self.assertIn(self.embargo_text, response.content) From db161543d8a793010f327a517afe41c23e4e2337 Mon Sep 17 00:00:00 2001 From: Alan Boudreault Date: Mon, 19 May 2014 15:53:45 -0400 Subject: [PATCH 3/4] Modified EMBARGO_SITE to SITE_EMBARGOED, improved logs --- cms/envs/common.py | 2 +- common/djangoapps/embargo/middleware.py | 29 +++++++++++++------ .../embargo/tests/test_middleware.py | 6 ++-- lms/envs/common.py | 2 +- 4 files changed, 25 insertions(+), 14 deletions(-) diff --git a/cms/envs/common.py b/cms/envs/common.py index 01e167098a..58909c024a 100644 --- a/cms/envs/common.py +++ b/cms/envs/common.py @@ -89,7 +89,7 @@ FEATURES = { 'EMBARGO': False, # Toggle embargo site functionality - 'EMBARGO_SITE': False, + 'SITE_EMBARGOED': False, # Turn on/off Microsites feature 'USE_MICROSITES': False, diff --git a/common/djangoapps/embargo/middleware.py b/common/djangoapps/embargo/middleware.py index f8ea70d565..6eb9af373a 100644 --- a/common/djangoapps/embargo/middleware.py +++ b/common/djangoapps/embargo/middleware.py @@ -22,9 +22,9 @@ Usage: FEATURES['EMBARGO'] = True # blocked ip will be redirected to /embargo # To enable Embargoing site: -FEATURES['EMBARGO_SITE'] = True +FEATURES['SITE_EMBARGOED'] = True -# With EMBARGO_SITE, you can define an external to redirect with: +# With SITE_EMBARGOED, you can define an external to redirect with: EMBARGO_SITE_REDIRECT_URL = 'https://www.edx.org/' # if EMBARGO_SITE_REDIRECT_URL is missing, a HttpResponseForbidden is returned. @@ -53,7 +53,7 @@ class EmbargoMiddleware(object): optionally ``IPFilter`` rows in the database, using the django admin site. """ def __init__(self): - self.site_enabled = settings.FEATURES.get('EMBARGO_SITE', False) + self.site_enabled = settings.FEATURES.get('SITE_EMBARGOED', False) # If embargoing is turned off, make this middleware do nothing if not settings.FEATURES.get('EMBARGO', False) and \ not self.site_enabled: @@ -65,9 +65,10 @@ class EmbargoMiddleware(object): """ url = request.path course_id = course_id_from_url(url) + course_is_embargoed = EmbargoedCourse.is_embargoed(course_id) # If they're trying to access a course that cares about embargoes - if self.site_enabled or EmbargoedCourse.is_embargoed(course_id): + if self.site_enabled or course_is_embargoed: response = redirect('embargo') # Set the proper response if site is enabled if self.site_enabled: @@ -80,15 +81,25 @@ class EmbargoMiddleware(object): # if blacklisted, immediately fail if ip_addr in IPFilter.current().blacklist_ips: - log.info("Embargo: Restricting IP address %s because IP is blacklisted.", ip_addr) + if course_is_embargoed: + msg = "Embargo: Restricting IP address %s to course %s because IP is blacklisted." % \ + (ip_addr, course_id) + else: + msg = "Embargo: Restricting IP address %s because IP is blacklisted." % ip_addr + + log.info(msg) return response country_code_from_ip = pygeoip.GeoIP(settings.GEOIP_PATH).country_code_by_addr(ip_addr) is_embargoed = country_code_from_ip in EmbargoedState.current().embargoed_countries_list # Fail if country is embargoed and the ip address isn't explicitly whitelisted if is_embargoed and ip_addr not in IPFilter.current().whitelist_ips: - log.info( - "Embargo: Restricting IP address %s because IP is from country %s.", - ip_addr, country_code_from_ip - ) + if course_is_embargoed: + msg = "Embargo: Restricting IP address %s to course %s because IP is from country %s." % \ + (ip_addr, course_id, country_code_from_ip) + else: + msg = "Embargo: Restricting IP address %s because IP is from country %s." % \ + (ip_addr, country_code_from_ip) + + log.info(msg) return response diff --git a/common/djangoapps/embargo/tests/test_middleware.py b/common/djangoapps/embargo/tests/test_middleware.py index 528ea18f3b..8cec3b2279 100644 --- a/common/djangoapps/embargo/tests/test_middleware.py +++ b/common/djangoapps/embargo/tests/test_middleware.py @@ -215,7 +215,7 @@ class EmbargoMiddlewareTests(TestCase): self.assertEqual(response.status_code, 200) @unittest.skipUnless(settings.ROOT_URLCONF == 'lms.urls', 'Test only valid in lms') - @mock.patch.dict(settings.FEATURES, {'EMBARGO': False, 'EMBARGO_SITE': True}) + @mock.patch.dict(settings.FEATURES, {'EMBARGO': False, 'SITE_EMBARGOED': True}) def test_embargo_off_embargo_site_on(self): # When the middleware is turned on with SITE, main site access should be restricted # Accessing a regular page from a blocked IP is denied. @@ -227,10 +227,10 @@ class EmbargoMiddlewareTests(TestCase): self.assertEqual(response.status_code, 200) @unittest.skipUnless(settings.ROOT_URLCONF == 'lms.urls', 'Test only valid in lms') - @mock.patch.dict(settings.FEATURES, {'EMBARGO': False, 'EMBARGO_SITE': True}) + @mock.patch.dict(settings.FEATURES, {'EMBARGO': False, 'SITE_EMBARGOED': True}) @override_settings(EMBARGO_SITE_REDIRECT_URL='https://www.edx.org/') def test_embargo_off_embargo_site_on_with_redirect_url(self): - # When the middleware is turned on with EMBARGO_SITE, main site access + # When the middleware is turned on with SITE_EMBARGOED, main site access # should be restricted. Accessing a regular page from a blocked IP is # denied, and redirected to EMBARGO_SITE_REDIRECT_URL rather than returning a 403. response = self.client.get(self.regular_page, HTTP_X_FORWARDED_FOR='1.0.0.0', REMOTE_ADDR='1.0.0.0') diff --git a/lms/envs/common.py b/lms/envs/common.py index 371c5ebf17..24eafabc66 100644 --- a/lms/envs/common.py +++ b/lms/envs/common.py @@ -229,7 +229,7 @@ FEATURES = { 'EMBARGO': False, # Toggle embargo site functionality - 'EMBARGO_SITE': False, + 'SITE_EMBARGOED': False, # Whether the Wiki subsystem should be accessible via the direct /wiki/ paths. Setting this to True means # that people can submit content and modify the Wiki in any arbitrary manner. We're leaving this as True in the From 877ec3f963f3ca38319e9a04db1a047df4bf3477 Mon Sep 17 00:00:00 2001 From: Alan Boudreault Date: Tue, 20 May 2014 09:50:41 -0400 Subject: [PATCH 4/4] some minor improvements to embargo --- cms/envs/common.py | 7 +++++-- common/djangoapps/embargo/middleware.py | 6 +++--- common/djangoapps/embargo/models.py | 2 +- common/djangoapps/embargo/tests/test_forms.py | 8 ++++---- common/djangoapps/embargo/tests/test_models.py | 2 +- lms/envs/common.py | 7 +++++-- 6 files changed, 19 insertions(+), 13 deletions(-) diff --git a/cms/envs/common.py b/cms/envs/common.py index 58909c024a..858de4e03c 100644 --- a/cms/envs/common.py +++ b/cms/envs/common.py @@ -85,10 +85,10 @@ FEATURES = { # Hide any Personally Identifiable Information from application logs 'SQUELCH_PII_IN_LOGS': False, - # Toggles embargo functionality + # Toggles the embargo functionality, which enable embargoing for particular courses 'EMBARGO': False, - # Toggle embargo site functionality + # Toggles the embargo site functionality, which enable embargoing for the whole site 'SITE_EMBARGOED': False, # Turn on/off Microsites feature @@ -305,6 +305,9 @@ LOCALE_PATHS = (REPO_ROOT + '/conf/locale',) # edx-platform/conf/locale/ # Messages MESSAGE_STORAGE = 'django.contrib.messages.storage.session.SessionStorage' +##### EMBARGO ##### +EMBARGO_SITE_REDIRECT_URL = None + ############################### Pipeline ####################################### STATICFILES_STORAGE = 'pipeline.storage.PipelineCachedStorage' diff --git a/common/djangoapps/embargo/middleware.py b/common/djangoapps/embargo/middleware.py index 6eb9af373a..052c04bdbc 100644 --- a/common/djangoapps/embargo/middleware.py +++ b/common/djangoapps/embargo/middleware.py @@ -18,13 +18,13 @@ Usage: # Enable the middleware in your settings -# To enable Embargoing by courses, add: +# To enable Embargo for particular courses, set: FEATURES['EMBARGO'] = True # blocked ip will be redirected to /embargo -# To enable Embargoing site: +# To enable the Embargo feature for the whole site, set: FEATURES['SITE_EMBARGOED'] = True -# With SITE_EMBARGOED, you can define an external to redirect with: +# With SITE_EMBARGOED, you can define an external url to redirect with: EMBARGO_SITE_REDIRECT_URL = 'https://www.edx.org/' # if EMBARGO_SITE_REDIRECT_URL is missing, a HttpResponseForbidden is returned. diff --git a/common/djangoapps/embargo/models.py b/common/djangoapps/embargo/models.py index a48e39f1f2..677d174ccb 100644 --- a/common/djangoapps/embargo/models.py +++ b/common/djangoapps/embargo/models.py @@ -92,7 +92,7 @@ class IPFilter(ConfigurationModel): def __iter__(self): for network in self.networks: - yield network + yield network def __contains__(self, ip): try: diff --git a/common/djangoapps/embargo/tests/test_forms.py b/common/djangoapps/embargo/tests/test_forms.py index dc14515474..21a6c37054 100644 --- a/common/djangoapps/embargo/tests/test_forms.py +++ b/common/djangoapps/embargo/tests/test_forms.py @@ -171,16 +171,16 @@ class IPFilterFormTest(TestCase): # Network tests # ips not in whitelist network - for addr in '1.1.0.2, 1.0.1.0'.split(','): + for addr in ['1.1.0.2', '1.0.1.0']: self.assertNotIn(addr.strip(), whitelist) # ips in whitelist network - for addr in '1.1.0.1, 1.0.0.100'.split(','): + for addr in ['1.1.0.1', '1.0.0.100']: self.assertIn(addr.strip(), whitelist) # ips not in blacklist network - for addr in '2.0.0.0, 1.1.0.0'.split(','): + for addr in ['2.0.0.0', '1.1.0.0']: self.assertNotIn(addr.strip(), blacklist) # ips in blacklist network - for addr in '1.0.100.0, 1.0.0.10'.split(','): + for addr in ['1.0.100.0', '1.0.0.10']: self.assertIn(addr.strip(), blacklist) # Test clearing by adding an empty list is OK too diff --git a/common/djangoapps/embargo/tests/test_models.py b/common/djangoapps/embargo/tests/test_models.py index fb2e4c89c2..8f9dc5d930 100644 --- a/common/djangoapps/embargo/tests/test_models.py +++ b/common/djangoapps/embargo/tests/test_models.py @@ -79,7 +79,7 @@ class EmbargoModelsTest(TestCase): cblacklist = IPFilter.current().blacklist_ips self.assertTrue(blacklist in cblacklist) - # network tests + def test_ip_network_blocking(self): whitelist = '1.0.0.0/24' blacklist = '1.1.0.0/16' diff --git a/lms/envs/common.py b/lms/envs/common.py index 24eafabc66..9e6bf06714 100644 --- a/lms/envs/common.py +++ b/lms/envs/common.py @@ -225,10 +225,10 @@ FEATURES = { # Hide any Personally Identifiable Information from application logs 'SQUELCH_PII_IN_LOGS': False, - # Toggle embargo functionality + # Toggles the embargo functionality, which enable embargoing for particular courses 'EMBARGO': False, - # Toggle embargo site functionality + # Toggles the embargo site functionality, which enable embargoing for the whole site 'SITE_EMBARGOED': False, # Whether the Wiki subsystem should be accessible via the direct /wiki/ paths. Setting this to True means @@ -680,6 +680,9 @@ ZENDESK_URL = None ZENDESK_USER = None ZENDESK_API_KEY = None +##### EMBARGO ##### +EMBARGO_SITE_REDIRECT_URL = None + ##### shoppingcart Payment ##### PAYMENT_SUPPORT_EMAIL = 'payment@example.com' ##### Using cybersource by default #####