From db161543d8a793010f327a517afe41c23e4e2337 Mon Sep 17 00:00:00 2001 From: Alan Boudreault Date: Mon, 19 May 2014 15:53:45 -0400 Subject: [PATCH] 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