diff --git a/cms/envs/common.py b/cms/envs/common.py index 5ce8ce98e8..60550c83f8 100644 --- a/cms/envs/common.py +++ b/cms/envs/common.py @@ -94,12 +94,10 @@ FEATURES = { # Hide any Personally Identifiable Information from application logs 'SQUELCH_PII_IN_LOGS': False, - # Toggles the embargo functionality, which enable embargoing for particular courses + # Toggles the embargo functionality, which blocks users + # based on their location. 'EMBARGO': False, - # Toggles the embargo site functionality, which enable embargoing for the whole site - 'SITE_EMBARGOED': False, - # Turn on/off Microsites feature 'USE_MICROSITES': False, diff --git a/cms/urls.py b/cms/urls.py index 89ef86040c..5407560127 100644 --- a/cms/urls.py +++ b/cms/urls.py @@ -60,7 +60,6 @@ urlpatterns += patterns( # ajax view that actually does the work url(r'^login_post$', 'student.views.login_user', name='login_post'), url(r'^logout$', 'student.views.logout_user', name='logout'), - url(r'^embargo$', 'student.views.embargo', name="embargo"), ) # restful api diff --git a/common/djangoapps/course_modes/tests/test_views.py b/common/djangoapps/course_modes/tests/test_views.py index a779f1a24b..b3fb841e6e 100644 --- a/common/djangoapps/course_modes/tests/test_views.py +++ b/common/djangoapps/course_modes/tests/test_views.py @@ -289,7 +289,7 @@ class CourseModeViewTest(UrlResetMixin, ModuleStoreTestCase): class TrackSelectionEmbargoTest(UrlResetMixin, ModuleStoreTestCase): """Test embargo restrictions on the track selection page. """ - @patch.dict(settings.FEATURES, {'ENABLE_COUNTRY_ACCESS': True}) + @patch.dict(settings.FEATURES, {'EMBARGO': True}) def setUp(self): super(TrackSelectionEmbargoTest, self).setUp('embargo') @@ -305,7 +305,7 @@ class TrackSelectionEmbargoTest(UrlResetMixin, ModuleStoreTestCase): # Construct the URL for the track selection page self.url = reverse('course_modes_choose', args=[unicode(self.course.id)]) - @patch.dict(settings.FEATURES, {'ENABLE_COUNTRY_ACCESS': True}) + @patch.dict(settings.FEATURES, {'EMBARGO': True}) def test_embargo_restrict(self): with restrict_course(self.course.id) as redirect_url: response = self.client.get(self.url) diff --git a/common/djangoapps/embargo/admin.py b/common/djangoapps/embargo/admin.py index 173b48f8d5..bc61f2ca31 100644 --- a/common/djangoapps/embargo/admin.py +++ b/common/djangoapps/embargo/admin.py @@ -5,51 +5,8 @@ from django.contrib import admin import textwrap from config_models.admin import ConfigurationModelAdmin -from embargo.models import ( - EmbargoedCourse, EmbargoedState, IPFilter, - CountryAccessRule, RestrictedCourse -) -from embargo.forms import ( - EmbargoedCourseForm, EmbargoedStateForm, IPFilterForm, - RestrictedCourseForm -) - - -class EmbargoedCourseAdmin(admin.ModelAdmin): - """Admin for embargoed course ids""" - form = EmbargoedCourseForm - fieldsets = ( - (None, { - 'fields': ('course_id', 'embargoed'), - 'description': textwrap.dedent("""\ - Enter a course id in the following box. - Do not enter leading or trailing slashes. There is no need to surround the - course ID with quotes. - Validation will be performed on the course name, and if it is invalid, an - error message will display. - - To enable embargos against this course (restrict course access from embargoed - states), check the "Embargoed" box, then click "Save". - """) - }), - ) - - -class EmbargoedStateAdmin(ConfigurationModelAdmin): - """Admin for embargoed countries""" - form = EmbargoedStateForm - fieldsets = ( - (None, { - 'fields': ('enabled', 'embargoed_countries',), - 'description': textwrap.dedent("""Enter the two-letter ISO-3166-1 Alpha-2 - code of the country or countries to embargo in the following box. For help, - see - this list of ISO-3166-1 country codes. - - Enter the embargoed country codes separated by a comma. Do not surround with quotes. - """) - }), - ) +from embargo.models import IPFilter, CountryAccessRule, RestrictedCourse +from embargo.forms import IPFilterForm, RestrictedCourseForm class IPFilterAdmin(ConfigurationModelAdmin): @@ -81,7 +38,5 @@ class RestrictedCourseAdmin(admin.ModelAdmin): form = RestrictedCourseForm -admin.site.register(EmbargoedCourse, EmbargoedCourseAdmin) -admin.site.register(EmbargoedState, EmbargoedStateAdmin) admin.site.register(IPFilter, IPFilterAdmin) admin.site.register(RestrictedCourse, RestrictedCourseAdmin) diff --git a/common/djangoapps/embargo/api.py b/common/djangoapps/embargo/api.py index 5c4e216f89..25e85a8050 100644 --- a/common/djangoapps/embargo/api.py +++ b/common/djangoapps/embargo/api.py @@ -27,7 +27,7 @@ def redirect_if_blocked(course_key, access_point='enrollment', **kwargs): Same as `check_course_access` and `message_url_path` """ - if settings.FEATURES.get('ENABLE_COUNTRY_ACCESS'): + if settings.FEATURES.get('EMBARGO'): is_blocked = not check_course_access(course_key, **kwargs) if is_blocked: return message_url_path(course_key, access_point) @@ -52,7 +52,7 @@ def check_course_access(course_key, user=None, ip_address=None, url=None): """ # No-op if the country access feature is not enabled - if not settings.FEATURES.get('ENABLE_COUNTRY_ACCESS'): + if not settings.FEATURES.get('EMBARGO'): return True # First, check whether there are any restrictions on the course. diff --git a/common/djangoapps/embargo/forms.py b/common/djangoapps/embargo/forms.py index 3fae3f1471..27e49eb8db 100644 --- a/common/djangoapps/embargo/forms.py +++ b/common/djangoapps/embargo/forms.py @@ -11,15 +11,11 @@ from xmodule.modulestore.django import modulestore from opaque_keys import InvalidKeyError from opaque_keys.edx.keys import CourseKey -from embargo.models import ( - EmbargoedCourse, EmbargoedState, IPFilter, - RestrictedCourse -) -from embargo.fixtures.country_codes import COUNTRY_CODES +from embargo.models import IPFilter, RestrictedCourse -class CourseKeyValidationForm(forms.ModelForm): - """Base class for validating the "course_key" (or "course_id") field. +class RestrictedCourseForm(forms.ModelForm): + """Validate course keys for the RestrictedCourse model. The default behavior in Django admin is to: * Save course keys for courses that do not exist. @@ -29,16 +25,10 @@ class CourseKeyValidationForm(forms.ModelForm): error message instead. """ - - def clean_course_id(self): - """Clean the 'course_id' field in the form. """ - return self._clean_course_key("course_id") + class Meta: # pylint: disable=missing-docstring + model = RestrictedCourse def clean_course_key(self): - """Clean the 'course_key' field in the form. """ - return self._clean_course_key("course_key") - - def _clean_course_key(self, field_name): """Validate the course key. Checks that the key format is valid and that @@ -51,7 +41,7 @@ class CourseKeyValidationForm(forms.ModelForm): CourseKey """ - cleaned_id = self.cleaned_data[field_name] + cleaned_id = self.cleaned_data['course_key'] error_msg = _('COURSE NOT FOUND. Please check that the course ID is valid.') try: @@ -65,51 +55,6 @@ class CourseKeyValidationForm(forms.ModelForm): return course_key -class EmbargoedCourseForm(CourseKeyValidationForm): - """Validate course keys for the EmbargoedCourse model. """ - - class Meta: # pylint: disable=missing-docstring - model = EmbargoedCourse - - -class RestrictedCourseForm(CourseKeyValidationForm): - """Validate course keys for the RestirctedCourse model. """ - - class Meta: # pylint: disable=missing-docstring - model = RestrictedCourse - - -class EmbargoedStateForm(forms.ModelForm): # pylint: disable=incomplete-protocol - """Form validating entry of states to embargo""" - - class Meta: # pylint: disable=missing-docstring - model = EmbargoedState - - def _is_valid_code(self, code): - """Whether or not code is a valid country code""" - return code in COUNTRY_CODES - - def clean_embargoed_countries(self): - """Validate the country list""" - embargoed_countries = self.cleaned_data["embargoed_countries"] - if not embargoed_countries: - return '' - - error_countries = [] - - for country in embargoed_countries.split(','): - country = country.strip().upper() - if not self._is_valid_code(country): - error_countries.append(country) - - if error_countries: - msg = 'COULD NOT PARSE COUNTRY CODE(S) FOR: {0}'.format(error_countries) - msg += ' Please check the list of country codes and verify your entries.' - raise forms.ValidationError(msg) - - return embargoed_countries - - class IPFilterForm(forms.ModelForm): # pylint: disable=incomplete-protocol """Form validating entry of IP addresses""" diff --git a/common/djangoapps/embargo/middleware.py b/common/djangoapps/embargo/middleware.py index 556c3b37f7..8f560377a6 100644 --- a/common/djangoapps/embargo/middleware.py +++ b/common/djangoapps/embargo/middleware.py @@ -16,49 +16,34 @@ Embargo can restrict by states and whitelist/blacklist (IP Addresses Usage: -# Enable the middleware in your settings +1) Enable embargo by setting `settings.FEATURES['EMBARGO']` to True. -# To enable Embargo for particular courses, set: -FEATURES['EMBARGO'] = True # blocked ip will be redirected to /embargo +2) In Django admin, create a new `IPFilter` model to block or whitelist + an IP address from accessing the site. -# To enable the Embargo feature for the whole site, set: -FEATURES['SITE_EMBARGOED'] = True - -# 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. +3) In Django admin, create a new `RestrictedCourse` model and + configure a whitelist or blacklist of countries for that course. """ -from functools import partial import logging import re -import pygeoip -from lazy import lazy from django.core.exceptions import MiddlewareNotUsed -from django.core.cache import cache from django.core.urlresolvers import reverse 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 -from student.models import unique_id_for_user -from embargo.models import EmbargoedCourse, EmbargoedState, IPFilter +from embargo.models import IPFilter from embargo import api as embargo_api + log = logging.getLogger(__name__) class EmbargoMiddleware(object): - """ - 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. - """ + """Middleware for embargoing site and courses. """ ALLOW_URL_PATTERNS = [ # Don't block the embargo message pages; otherwise we'd @@ -71,35 +56,15 @@ class EmbargoMiddleware(object): re.compile(r'^/admin/'), ] - # Reasons a user might be blocked. - # These are used to generate info messages in the logs. - REASONS = { - "ip_blacklist": u"Restricting IP address {ip_addr} {from_course} because IP is blacklisted.", - "ip_country": u"Restricting IP address {ip_addr} {from_course} because IP is from country {ip_country}.", - "profile_country": ( - u"Restricting user {user_id} {from_course} because " - u"the user set the profile country to {profile_country}." - ) - } - def __init__(self): - self.site_enabled = settings.FEATURES.get('SITE_EMBARGOED', False) - self.enable_country_access = settings.FEATURES.get('ENABLE_COUNTRY_ACCESS', False) - # If embargoing is turned off, make this middleware do nothing - disable_middleware = not ( - settings.FEATURES.get('EMBARGO') or - self.site_enabled or - self.enable_country_access - ) - if disable_middleware: + if not settings.FEATURES.get('EMBARGO'): raise MiddlewareNotUsed() def process_request(self, request): """Block requests based on embargo rules. - In the new ENABLE_COUNTRY_ACCESS implmentation, - this will perform the following checks: + This will perform the following checks: 1) If the user's IP address is blacklisted, block. 2) If the user's IP address is whitelisted, allow. @@ -110,275 +75,50 @@ class EmbargoMiddleware(object): 5) Allow access. """ - # If the feature flag is set, use the new "country access" implementation. - # This is a more flexible implementation of the embargo feature that allows - # per-course country access rules. - if self.enable_country_access: - - # Never block certain patterns by IP address - for pattern in self.ALLOW_URL_PATTERNS: - if pattern.match(request.path) is not None: - return None - - ip_address = get_ip(request) - ip_filter = IPFilter.current() - - if ip_filter.enabled and ip_address in ip_filter.blacklist_ips: - log.info( - ( - u"User %s was blocked from accessing %s " - u"because IP address %s is blacklisted." - ), request.user.id, request.path, ip_address - ) - - # If the IP is blacklisted, reject. - # This applies to any request, not just courseware URLs. - ip_blacklist_url = reverse( - 'embargo_blocked_message', - kwargs={ - 'access_point': 'courseware', - 'message_key': 'embargo' - } - ) - return redirect(ip_blacklist_url) - - elif ip_filter.enabled and ip_address in ip_filter.whitelist_ips: - log.info( - ( - u"User %s was allowed access to %s because " - u"IP address %s is whitelisted." - ), - request.user.id, request.path, ip_address - ) - - # If the IP is whitelisted, then allow access, - # skipping later checks. + # Never block certain patterns by IP address + for pattern in self.ALLOW_URL_PATTERNS: + if pattern.match(request.path) is not None: return None - else: - # Otherwise, perform the country access checks. - # This applies only to courseware URLs. - return self.country_access_rules(request.user, ip_address, request.path) + ip_address = get_ip(request) + ip_filter = IPFilter.current() - 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 course_is_embargoed: - - # Construct the list of functions that check whether the user is embargoed. - # We wrap each of these functions in a decorator that logs the reason the user - # was blocked. - # Each function should return `True` iff the user is blocked by an embargo. - check_functions = [ - self._log_embargo_reason(check_func, course_id, course_is_embargoed) - for check_func in [ - partial(self._is_embargoed_by_ip, get_ip(request)), - partial(self._is_embargoed_by_profile_country, request.user) - ] - ] - - # Perform each of the checks - # If the user fails any of the checks, immediately redirect them - # and skip later checks. - for check_func in check_functions: - if check_func(): - return self._embargo_redirect_response - - # If all the check functions pass, implicitly return None - # so that the middleware processor can continue processing - # the response. - - def _is_embargoed_by_ip(self, ip_addr, course_id=u"", course_is_embargoed=False): - """ - Check whether the user is embargoed based on the IP address. - - Args: - ip_addr (str): The IP address the request originated from. - - Keyword Args: - course_id (unicode): The course the user is trying to access. - course_is_embargoed (boolean): Whether the course the user is accessing has been embargoed. - - Returns: - A unicode message if the user is embargoed, otherwise `None` - - """ - # If blacklisted, immediately fail - if ip_addr in IPFilter.current().blacklist_ips: - return self.REASONS['ip_blacklist'].format( - ip_addr=ip_addr, - from_course=self._from_course_msg(course_id, course_is_embargoed) + if ip_filter.enabled and ip_address in ip_filter.blacklist_ips: + log.info( + ( + u"User %s was blocked from accessing %s " + u"because IP address %s is blacklisted." + ), request.user.id, request.path, ip_address ) - # If we're white-listed, then allow access - if ip_addr in IPFilter.current().whitelist_ips: + # If the IP is blacklisted, reject. + # This applies to any request, not just courseware URLs. + ip_blacklist_url = reverse( + 'embargo_blocked_message', + kwargs={ + 'access_point': 'courseware', + 'message_key': 'embargo' + } + ) + return redirect(ip_blacklist_url) + + elif ip_filter.enabled and ip_address in ip_filter.whitelist_ips: + log.info( + ( + u"User %s was allowed access to %s because " + u"IP address %s is whitelisted." + ), + request.user.id, request.path, ip_address + ) + + # If the IP is whitelisted, then allow access, + # skipping later checks. return None - # Retrieve the country code from the IP address - # and check it against the list of embargoed countries - ip_country = self._country_code_from_ip(ip_addr) - if ip_country in self._embargoed_countries: - return self.REASONS['ip_country'].format( - ip_addr=ip_addr, - ip_country=ip_country, - from_course=self._from_course_msg(course_id, course_is_embargoed) - ) - - # If none of the other checks caught anything, - # implicitly return None to indicate that the user can access the course - - def _is_embargoed_by_profile_country(self, user, course_id="", course_is_embargoed=False): - """ - Check whether the user is embargoed based on the country code in the user's profile. - - Args: - user (User): The user attempting to access courseware. - - Keyword Args: - course_id (unicode): The course the user is trying to access. - course_is_embargoed (boolean): Whether the course the user is accessing has been embargoed. - - Returns: - A unicode message if the user is embargoed, otherwise `None` - - """ - cache_key = u'user.{user_id}.profile.country'.format(user_id=user.id) - profile_country = cache.get(cache_key) - if profile_country is None: - profile = getattr(user, 'profile', None) - if profile is not None and profile.country.code is not None: - profile_country = profile.country.code.upper() - else: - profile_country = "" - cache.set(cache_key, profile_country) - - if profile_country in self._embargoed_countries: - return self.REASONS['profile_country'].format( - user_id=unique_id_for_user(user), - profile_country=profile_country, - from_course=self._from_course_msg(course_id, course_is_embargoed) - ) else: - return None - - def _country_code_from_ip(self, ip_addr): - """ - Return the country code associated with an IP address. - Handles both IPv4 and IPv6 addresses. - - Args: - ip_addr (str): The IP address to look up. - - Returns: - 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) - - @property - def _embargo_redirect_response(self): - """ - The HTTP response to send when the user is blocked from a course. - This will either be a redirect to a URL configured in Django settings - or a forbidden response. - - Returns: - HTTPResponse - - """ - 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') - ) - - return response - - @lazy - def _embargoed_countries(self): - """ - Return the list of 2-letter country codes for embargoed countries. - The result is cached within the scope of the response. - - Returns: - list - - """ - return EmbargoedState.current().embargoed_countries_list - - def _from_course_msg(self, course_id, course_is_embargoed): - """ - Format a message indicating whether the user was blocked from a specific course. - This can be used in info messages, but should not be used in user-facing messages. - - Args: - course_id (unicode): The ID of the course being accessed. - course_is_embarged (boolean): Whether the course being accessed is embargoed. - - Returns: - unicode - - """ - return ( - u"from course {course_id}".format(course_id=course_id) - if course_is_embargoed - else u"" - ) - - def _log_embargo_reason(self, check_func, course_id, course_is_embargoed): - """ - Decorator for embargo check functions that will: - * execute the check function - * check whether the user is blocked by an embargo, and if so, log the reason - * return a boolean indicating whether the user was blocked. - - Args: - check_func (partial): A function that should return unicode reason if the user - was blocked, otherwise should return None. This function will be passed - `course_id` and `course_is_embarged` kwargs so it can format a detailed - reason message. - - course_id (unicode): The ID of the course the user is trying to access. - - course_is_embargoed (boolean): Whether the course the user is trying - to access is under an embargo. - - Returns: - boolean: True iff the user was blocked by an embargo - - """ - def _inner(): - # Perform the check and retrieve the reason string. - # The reason will be `None` if the user passes the check and can access the course. - # We pass in the course ID and whether the course is embargoed - # so that the check function can fill in the "reason" message with more specific details. - reason = check_func( - course_id=course_id, - course_is_embargoed=course_is_embargoed - ) - - # If the reason was `None`, indicate that the user was not blocked. - if reason is None: - return False - - # Otherwise, log the reason the user was blocked - # and return True. - else: - msg = u"Embargo: {reason}".format(reason=reason) - log.info(msg) - return True - - return _inner + # Otherwise, perform the country access checks. + # This applies only to courseware URLs. + return self.country_access_rules(request.user, ip_address, request.path) def country_access_rules(self, user, ip_address, url_path): """ diff --git a/common/djangoapps/embargo/models.py b/common/djangoapps/embargo/models.py index 447c023fd6..8b6d2c7eaa 100644 --- a/common/djangoapps/embargo/models.py +++ b/common/djangoapps/embargo/models.py @@ -37,6 +37,8 @@ log = logging.getLogger(__name__) class EmbargoedCourse(models.Model): """ Enable course embargo on a course-by-course basis. + + Deprecated by `RestrictedCourse` """ objects = NoneToEmptyManager() @@ -70,6 +72,8 @@ class EmbargoedCourse(models.Model): class EmbargoedState(ConfigurationModel): """ Register countries to be embargoed. + + Deprecated by `Country`. """ # The countries to embargo embargoed_countries = models.TextField( diff --git a/common/djangoapps/embargo/tests/test_api.py b/common/djangoapps/embargo/tests/test_api.py index 299e59defe..bf1b349f78 100644 --- a/common/djangoapps/embargo/tests/test_api.py +++ b/common/djangoapps/embargo/tests/test_api.py @@ -36,7 +36,7 @@ MODULESTORE_CONFIG = mixed_store_config(settings.COMMON_TEST_DATA_ROOT, {}, incl @ddt.ddt @override_settings(MODULESTORE=MODULESTORE_CONFIG) @unittest.skipUnless(settings.ROOT_URLCONF == 'lms.urls', 'Test only valid in lms') -@mock.patch.dict(settings.FEATURES, {'ENABLE_COUNTRY_ACCESS': True}) +@mock.patch.dict(settings.FEATURES, {'EMBARGO': True}) class EmbargoCheckAccessApiTests(ModuleStoreTestCase): """Test the embargo API calls to determine whether a user has access. """ @@ -137,7 +137,7 @@ class EmbargoCheckAccessApiTests(ModuleStoreTestCase): result = embargo_api.check_course_access(self.course.id, user=self.user, ip_address='FE80::0202:B3FF:FE1E:8329') self.assertTrue(result) - @mock.patch.dict(settings.FEATURES, {'ENABLE_COUNTRY_ACCESS': True}) + @mock.patch.dict(settings.FEATURES, {'EMBARGO': True}) def test_profile_country_db_null(self): # Django country fields treat NULL values inconsistently. # When saving a profile with country set to None, Django saves an empty string to the database. @@ -183,7 +183,7 @@ class EmbargoCheckAccessApiTests(ModuleStoreTestCase): class EmbargoMessageUrlApiTests(UrlResetMixin, ModuleStoreTestCase): """Test the embargo API calls for retrieving the blocking message URLs. """ - @patch.dict(settings.FEATURES, {'ENABLE_COUNTRY_ACCESS': True}) + @patch.dict(settings.FEATURES, {'EMBARGO': True}) def setUp(self): super(EmbargoMessageUrlApiTests, self).setUp('embargo') self.course = CourseFactory.create() diff --git a/common/djangoapps/embargo/tests/test_forms.py b/common/djangoapps/embargo/tests/test_forms.py index e613b253d2..fa5f38b6a8 100644 --- a/common/djangoapps/embargo/tests/test_forms.py +++ b/common/djangoapps/embargo/tests/test_forms.py @@ -4,140 +4,51 @@ Unit tests for embargo app admin forms. """ from django.test import TestCase -from django.test.utils import override_settings + +from opaque_keys.edx.locator import CourseLocator # Explicitly import the cache from ConfigurationModel so we can reset it after each test from config_models.models import cache -from embargo.forms import EmbargoedCourseForm, EmbargoedStateForm, IPFilterForm -from embargo.models import EmbargoedCourse, EmbargoedState, IPFilter +from embargo.models import IPFilter +from embargo.forms import RestrictedCourseForm, IPFilterForm from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase from xmodule.modulestore.tests.factories import CourseFactory -from xmodule.modulestore.tests.django_utils import TEST_DATA_MOCK_MODULESTORE -class EmbargoCourseFormTest(ModuleStoreTestCase): +class RestrictedCourseFormTest(ModuleStoreTestCase): """Test the course form properly validates course IDs""" - def setUp(self): - super(EmbargoCourseFormTest, self).setUp() - self.course = CourseFactory.create() - self.true_form_data = {'course_id': self.course.id.to_deprecated_string(), 'embargoed': True} - self.false_form_data = {'course_id': self.course.id.to_deprecated_string(), 'embargoed': False} - - def test_embargo_course(self): - self.assertFalse(EmbargoedCourse.is_embargoed(self.course.id)) - # Test adding embargo to this course - form = EmbargoedCourseForm(data=self.true_form_data) - # Validation should work + def test_save_valid_data(self): + course = CourseFactory.create() + data = { + 'course_key': unicode(course.id), + 'enroll_msg_key': 'default', + 'access_msg_key': 'default' + } + form = RestrictedCourseForm(data=data) self.assertTrue(form.is_valid()) - form.save() - # Check that this course is embargoed - self.assertTrue(EmbargoedCourse.is_embargoed(self.course.id)) - def test_repeat_course(self): - # Initially course shouldn't be authorized - self.assertFalse(EmbargoedCourse.is_embargoed(self.course.id)) - # Test authorizing the course, which should totally work - form = EmbargoedCourseForm(data=self.true_form_data) - # Validation should work - self.assertTrue(form.is_valid()) - form.save() - # Check that this course is authorized - self.assertTrue(EmbargoedCourse.is_embargoed(self.course.id)) + def test_invalid_course_key(self): + # Invalid format for the course key + form = RestrictedCourseForm(data={'course_key': 'not/valid'}) + self._assert_course_field_error(form) - # Now make a new course authorization with the same course id that tries to turn email off - form = EmbargoedCourseForm(data=self.false_form_data) - # Validation should not work because course_id field is unique - self.assertFalse(form.is_valid()) - self.assertEquals( - "Embargoed course with this Course id already exists.", - form._errors['course_id'][0] # pylint: disable=protected-access - ) - with self.assertRaisesRegexp(ValueError, "The EmbargoedCourse could not be created because the data didn't validate."): - form.save() + def test_course_not_found(self): + course_key = CourseLocator(org='test', course='test', run='test') + form = RestrictedCourseForm(data={'course_key': course_key}) + self._assert_course_field_error(form) - # Course should still be authorized (invalid attempt had no effect) - self.assertTrue(EmbargoedCourse.is_embargoed(self.course.id)) - - def test_form_typo(self): - # Munge course id - bad_id = self.course.id.to_deprecated_string() + '_typo' - - form_data = {'course_id': bad_id, 'embargoed': True} - form = EmbargoedCourseForm(data=form_data) + def _assert_course_field_error(self, form): # Validation shouldn't work self.assertFalse(form.is_valid()) msg = 'COURSE NOT FOUND' - self.assertIn(msg, form._errors['course_id'][0]) # pylint: disable=protected-access + self.assertIn(msg, form._errors['course_key'][0]) # pylint: disable=protected-access - with self.assertRaisesRegexp(ValueError, "The EmbargoedCourse could not be created because the data didn't validate."): + with self.assertRaisesRegexp(ValueError, "The RestrictedCourse could not be created because the data didn't validate."): form.save() - def test_invalid_location(self): - # Munge course id - bad_id = self.course.id.to_deprecated_string().split('/')[-1] - - form_data = {'course_id': bad_id, 'embargoed': True} - form = EmbargoedCourseForm(data=form_data) - # Validation shouldn't work - self.assertFalse(form.is_valid()) - - msg = 'COURSE NOT FOUND' - self.assertIn(msg, form._errors['course_id'][0]) # pylint: disable=protected-access - - with self.assertRaisesRegexp(ValueError, "The EmbargoedCourse could not be created because the data didn't validate."): - form.save() - - -class EmbargoedStateFormTest(TestCase): - """Test form for adding new states""" - - def setUp(self): - # Explicitly clear the cache, since ConfigurationModel relies on the cache - cache.clear() - - def tearDown(self): - # Explicitly clear ConfigurationModel's cache so tests have a clear cache - # and don't interfere with each other - cache.clear() - - def test_add_valid_states(self): - # test adding valid two letter states - # case and spacing should not matter - form_data = {'embargoed_countries': 'cu, Sy , US'} - form = EmbargoedStateForm(data=form_data) - self.assertTrue(form.is_valid()) - form.save() - current_embargoes = EmbargoedState.current().embargoed_countries_list - for country in ["CU", "SY", "US"]: - self.assertIn(country, current_embargoes) - # Test clearing by adding an empty list is OK too - form_data = {'embargoed_countries': ''} - form = EmbargoedStateForm(data=form_data) - self.assertTrue(form.is_valid()) - form.save() - self.assertTrue(len(EmbargoedState.current().embargoed_countries_list) == 0) - - def test_add_invalid_states(self): - # test adding invalid codes - # xx is not valid - # usa is not valid - form_data = {'embargoed_countries': 'usa, xx'} - form = EmbargoedStateForm(data=form_data) - self.assertFalse(form.is_valid()) - - msg = 'COULD NOT PARSE COUNTRY CODE(S) FOR: {0}'.format([u'USA', u'XX']) - msg += ' Please check the list of country codes and verify your entries.' - self.assertEquals(msg, form._errors['embargoed_countries'][0]) # pylint: disable=protected-access - - with self.assertRaisesRegexp(ValueError, "The EmbargoedState could not be created because the data didn't validate."): - form.save() - - self.assertFalse('USA' in EmbargoedState.current().embargoed_countries_list) - self.assertFalse('XX' in EmbargoedState.current().embargoed_countries_list) - class IPFilterFormTest(TestCase): """Test form for adding [black|white]list IP addresses""" diff --git a/common/djangoapps/embargo/tests/test_middleware.py b/common/djangoapps/embargo/tests/test_middleware.py index 6c0ef7cd08..3e26b9bf4c 100644 --- a/common/djangoapps/embargo/tests/test_middleware.py +++ b/common/djangoapps/embargo/tests/test_middleware.py @@ -1,332 +1,168 @@ """ -Tests for EmbargoMiddleware +Tests for EmbargoMiddleware with CountryAccessRules """ -import mock -import pygeoip import unittest +from mock import patch +import ddt from django.core.urlresolvers import reverse from django.conf import settings -from django.db import connection, transaction -from django.test.utils import override_settings -import ddt +from django.core.cache import cache as django_cache -from student.models import CourseEnrollment +from util.testing import UrlResetMixin from student.tests.factories import UserFactory from xmodule.modulestore.tests.factories import CourseFactory -from xmodule.modulestore.tests.django_utils import ( - ModuleStoreTestCase, mixed_store_config -) +from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase +from config_models.models import cache as config_cache -# Explicitly import the cache from ConfigurationModel so we can reset it after each test -from config_models.models import cache -from embargo.models import EmbargoedCourse, EmbargoedState, IPFilter +from embargo.models import RestrictedCourse, IPFilter +from embargo.test_utils import restrict_course @ddt.ddt @unittest.skipUnless(settings.ROOT_URLCONF == 'lms.urls', 'Test only valid in lms') -class EmbargoMiddlewareTests(ModuleStoreTestCase): - """ - Tests of EmbargoMiddleware +class EmbargoMiddlewareAccessTests(UrlResetMixin, ModuleStoreTestCase): + """Tests of embargo middleware country access rules. + + There are detailed unit tests for the rule logic in + `test_api.py`; here, we're mainly testing the integration + with middleware + """ + USERNAME = 'fred' + PASSWORD = 'secret' + + @patch.dict(settings.FEATURES, {'EMBARGO': True}) def setUp(self): - super(EmbargoMiddlewareTests, self).setUp() + super(EmbargoMiddlewareAccessTests, self).setUp('embargo') + self.user = UserFactory(username=self.USERNAME, password=self.PASSWORD) + self.course = CourseFactory.create() + self.client.login(username=self.USERNAME, password=self.PASSWORD) - self.user = UserFactory(username='fred', password='secret') - self.client.login(username='fred', password='secret') - self.embargo_course = CourseFactory.create() - self.embargo_course.save() - self.regular_course = CourseFactory.create(org="Regular") - self.regular_course.save() - self.embargoed_page = '/courses/' + self.embargo_course.id.to_deprecated_string() + '/info' - self.regular_page = '/courses/' + self.regular_course.id.to_deprecated_string() + '/info' - EmbargoedCourse(course_id=self.embargo_course.id, embargoed=True).save() - EmbargoedState( - embargoed_countries="cu, ir, Sy, SD", - changed_by=self.user, - enabled=True - ).save() - CourseEnrollment.enroll(self.user, self.regular_course.id) - CourseEnrollment.enroll(self.user, self.embargo_course.id) - # Text from lms/templates/static_templates/embargo.html - self.embargo_text = "Unfortunately, at this time edX must comply with export controls, and we cannot allow you to access this course." - - self.patcher = mock.patch.object(pygeoip.GeoIP, 'country_code_by_addr', self.mock_country_code_by_addr) - self.patcher.start() - - def tearDown(self): - # Explicitly clear ConfigurationModel's cache so tests have a clear cache - # and don't interfere with each other - cache.clear() - self.patcher.stop() - - def mock_country_code_by_addr(self, ip_addr): - """ - Gives us a fake set of IPs - """ - ip_dict = { - '1.0.0.0': 'CU', - '2.0.0.0': 'IR', - '3.0.0.0': 'SY', - '4.0.0.0': 'SD', - '5.0.0.0': 'AQ', # Antartica - '2001:250::': 'CN', - '2001:1340::': 'CU', - } - return ip_dict.get(ip_addr, 'US') - - def test_countries(self): - # Accessing an embargoed page from a blocked IP should cause a redirect - 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, 302) - # Following the redirect should give us the embargo page - response = self.client.get( - self.embargoed_page, - HTTP_X_FORWARDED_FOR='1.0.0.0', - REMOTE_ADDR='1.0.0.0', - follow=True + self.courseware_url = reverse( + 'course_root', + kwargs={'course_id': unicode(self.course.id)} ) - self.assertIn(self.embargo_text, response.content) + self.non_courseware_url = reverse('dashboard') - # Accessing a regular page from a blocked IP should succeed - response = self.client.get(self.regular_page, HTTP_X_FORWARDED_FOR='1.0.0.0', REMOTE_ADDR='1.0.0.0') + # Clear the cache to avoid interference between tests + django_cache.clear() + config_cache.clear() + + @patch.dict(settings.FEATURES, {'EMBARGO': True}) + def test_blocked(self): + with restrict_course(self.course.id, access_point='courseware') as redirect_url: + response = self.client.get(self.courseware_url) + self.assertRedirects(response, redirect_url) + + @patch.dict(settings.FEATURES, {'EMBARGO': True}) + def test_allowed(self): + # Add the course to the list of restricted courses + # but don't create any access rules + RestrictedCourse.objects.create(course_key=self.course.id) + + # Expect that we can access courseware + response = self.client.get(self.courseware_url) self.assertEqual(response.status_code, 200) - # Accessing an embargoed page from a non-embargoed IP should succeed - response = self.client.get(self.embargoed_page, HTTP_X_FORWARDED_FOR='5.0.0.0', REMOTE_ADDR='5.0.0.0') - self.assertEqual(response.status_code, 200) - - # Accessing a regular page from a non-embargoed 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) - - def test_countries_ipv6(self): - # Accessing an embargoed page from a blocked IP should cause a redirect - response = self.client.get(self.embargoed_page, HTTP_X_FORWARDED_FOR='2001:1340::', REMOTE_ADDR='2001:1340::') - 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='2001:1340::', - REMOTE_ADDR='2001:1340::', - follow=True - ) - self.assertIn(self.embargo_text, response.content) - - # Accessing a regular page from a blocked IP should succeed - response = self.client.get(self.regular_page, HTTP_X_FORWARDED_FOR='2001:1340::', REMOTE_ADDR='2001:1340::') - self.assertEqual(response.status_code, 200) - - # Accessing an embargoed page from a non-embargoed IP should succeed - response = self.client.get(self.embargoed_page, HTTP_X_FORWARDED_FOR='2001:250::', REMOTE_ADDR='2001:250::') - self.assertEqual(response.status_code, 200) - - # Accessing a regular page from a non-embargoed IP should succeed - response = self.client.get(self.regular_page, HTTP_X_FORWARDED_FOR='2001:250::', REMOTE_ADDR='2001:250::') - self.assertEqual(response.status_code, 200) - - def test_ip_exceptions(self): - # Explicitly whitelist/blacklist some IPs - IPFilter( - whitelist='1.0.0.0', - blacklist='5.0.0.0', - changed_by=self.user, - enabled=True - ).save() - - # Accessing an embargoed page from a blocked IP that's been whitelisted - # 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 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 - # should cause a redirect - response = self.client.get(self.embargoed_page, HTTP_X_FORWARDED_FOR='5.0.0.0', REMOTE_ADDR='5.0.0.0') - 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 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) - - 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.100', - REMOTE_ADDR='5.0.0.100', - 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) - - @ddt.data( - (None, False), - ("", False), - ("us", False), - ("CU", True), - ("Ir", True), - ("sy", True), - ("sd", True) - ) - @ddt.unpack - def test_embargo_profile_country(self, profile_country, is_embargoed): - # Set the country in the user's profile - profile = self.user.profile - profile.country = profile_country - profile.save() - - # Attempt to access an embargoed course - response = self.client.get(self.embargoed_page) - - # If the user is from an embargoed country, verify that - # they are redirected to the embargo page. - if is_embargoed: - embargo_url = reverse('embargo') - self.assertRedirects(response, embargo_url) - - # Otherwise, verify that the student can access the page - else: + @patch.dict(settings.FEATURES, {'EMBARGO': True}) + def test_non_courseware_url(self): + with restrict_course(self.course.id): + response = self.client.get(self.non_courseware_url) self.assertEqual(response.status_code, 200) - # For non-embargoed courses, the student should be able to access - # the page, even if he/she is from an embargoed country. - response = self.client.get(self.regular_page) - self.assertEqual(response.status_code, 200) + @patch.dict(settings.FEATURES, {'EMBARGO': True}) + @ddt.data( + # request_ip, blacklist, whitelist, is_enabled, allow_access + ('173.194.123.35', ['173.194.123.35'], [], True, False), + ('173.194.123.35', ['173.194.0.0/16'], [], True, False), + ('173.194.123.35', ['127.0.0.0/32', '173.194.0.0/16'], [], True, False), + ('173.195.10.20', ['173.194.0.0/16'], [], True, True), + ('173.194.123.35', ['173.194.0.0/16'], ['173.194.0.0/16'], True, False), + ('173.194.123.35', [], ['173.194.0.0/16'], True, True), + ('192.178.2.3', [], ['173.194.0.0/16'], True, True), + ('173.194.123.35', ['173.194.123.35'], [], False, True), + ) + @ddt.unpack + def test_ip_access_rules(self, request_ip, blacklist, whitelist, is_enabled, allow_access): + # Ensure that IP blocking works for anonymous users + self.client.logout() - def test_embargo_profile_country_cache(self): - # Set the country in the user's profile - profile = self.user.profile - profile.country = "us" - profile.save() + # Set up the IP rules + IPFilter.objects.create( + blacklist=", ".join(blacklist), + whitelist=", ".join(whitelist), + enabled=is_enabled + ) - # Warm the cache - with self.assertNumQueries(16): - self.client.get(self.embargoed_page) + # Check that access is enforced + response = self.client.get( + "/", + HTTP_X_FORWARDED_FOR=request_ip, + REMOTE_ADDR=request_ip + ) - # Access the page multiple times, but expect that we hit - # the database to check the user's profile only once - with self.assertNumQueries(10): - self.client.get(self.embargoed_page) + if allow_access: + self.assertEqual(response.status_code, 200) + else: + redirect_url = reverse( + 'embargo_blocked_message', + kwargs={ + 'access_point': 'courseware', + 'message_key': 'embargo' + } + ) + self.assertRedirects(response, redirect_url) - def test_embargo_profile_country_db_null(self): - # Django country fields treat NULL values inconsistently. - # When saving a profile with country set to None, Django saves an empty string to the database. - # However, when the country field loads a NULL value from the database, it sets - # `country.code` to `None`. This caused a bug in which country values created by - # the original South schema migration -- which defaulted to NULL -- caused a runtime - # exception when the embargo middleware treated the value as a string. - # In order to simulate this behavior, we can't simply set `profile.country = None`. - # (because when we save it, it will set the database field to an empty string instead of NULL) - query = "UPDATE auth_userprofile SET country = NULL WHERE id = %s" - connection.cursor().execute(query, [str(self.user.profile.id)]) - transaction.commit_unless_managed() - - # Attempt to access an embargoed course - # Verify that the student can access the page without an error - response = self.client.get(self.embargoed_page) - self.assertEqual(response.status_code, 200) - - @mock.patch.dict(settings.FEATURES, {'EMBARGO': False}) - def test_countries_embargo_off(self): - # When the middleware is turned off, all requests should go through - # Accessing an embargoed page from a blocked IP OK - 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 page from a blocked IP 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) - - # Explicitly whitelist/blacklist some IPs - IPFilter( - whitelist='1.0.0.0', - blacklist='5.0.0.0', - changed_by=self.user, + @patch.dict(settings.FEATURES, {'EMBARGO': True}) + @ddt.data( + ('courseware', 'default'), + ('courseware', 'embargo'), + ('enrollment', 'default'), + ('enrollment', 'embargo') + ) + @ddt.unpack + def test_always_allow_access_to_embargo_messages(self, access_point, msg_key): + # Blacklist an IP address + IPFilter.objects.create( + blacklist="192.168.10.20", enabled=True - ).save() + ) - # Accessing an embargoed course from non-embargoed IP that's been blacklisted - # should be OK - response = self.client.get(self.embargoed_page, HTTP_X_FORWARDED_FOR='5.0.0.0', REMOTE_ADDR='5.0.0.0') + url = reverse( + 'embargo_blocked_message', + kwargs={ + 'access_point': access_point, + 'message_key': msg_key + } + ) + response = self.client.get( + url, + HTTP_X_FORWARDED_FOR="192.168.10.20", + REMOTE_ADDR="192.168.10.20" + ) 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') + @patch.dict(settings.FEATURES, {'EMBARGO': True}) + def test_whitelist_ip_skips_country_access_checks(self): + # Whitelist an IP address + IPFilter.objects.create( + whitelist="192.168.10.20", + enabled=True + ) + + # Set up country access rules so the user would + # be restricted from the course. + with restrict_course(self.course.id): + # Make a request from the whitelisted IP address + response = self.client.get( + self.courseware_url, + HTTP_X_FORWARDED_FOR="192.168.10.20", + REMOTE_ADDR="192.168.10.20" + ) + + # Expect that we were still able to access the page, + # even though we would have been blocked by country + # access rules. self.assertEqual(response.status_code, 200) - - @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. - 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) - - @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 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') - self.assertEqual(response.status_code, 302) diff --git a/common/djangoapps/embargo/tests/test_middleware_access_rules.py b/common/djangoapps/embargo/tests/test_middleware_access_rules.py deleted file mode 100644 index e4a4e2d1e6..0000000000 --- a/common/djangoapps/embargo/tests/test_middleware_access_rules.py +++ /dev/null @@ -1,175 +0,0 @@ -""" -Tests for EmbargoMiddleware with CountryAccessRules -""" - -import unittest -from mock import patch -import ddt - -from django.core.urlresolvers import reverse -from django.conf import settings -from django.core.cache import cache as django_cache - -from util.testing import UrlResetMixin -from student.tests.factories import UserFactory -from xmodule.modulestore.tests.factories import CourseFactory -from xmodule.modulestore.tests.django_utils import ( - ModuleStoreTestCase, mixed_store_config -) -from config_models.models import cache as config_cache - -from embargo.models import RestrictedCourse, IPFilter -from embargo.test_utils import restrict_course - - -# Since we don't need any XML course fixtures, use a modulestore configuration -# that disables the XML modulestore. -MODULESTORE_CONFIG = mixed_store_config(settings.COMMON_TEST_DATA_ROOT, {}, include_xml=False) - - -@ddt.ddt -@unittest.skipUnless(settings.ROOT_URLCONF == 'lms.urls', 'Test only valid in lms') -class EmbargoMiddlewareAccessTests(UrlResetMixin, ModuleStoreTestCase): - """Tests of embargo middleware country access rules. - - There are detailed unit tests for the rule logic in - `test_api.py`; here, we're mainly testing the integration - with middleware - - """ - USERNAME = 'fred' - PASSWORD = 'secret' - - @patch.dict(settings.FEATURES, {'ENABLE_COUNTRY_ACCESS': True}) - def setUp(self): - super(EmbargoMiddlewareAccessTests, self).setUp('embargo') - self.user = UserFactory(username=self.USERNAME, password=self.PASSWORD) - self.course = CourseFactory.create() - self.client.login(username=self.USERNAME, password=self.PASSWORD) - - self.courseware_url = reverse( - 'course_root', - kwargs={'course_id': unicode(self.course.id)} - ) - self.non_courseware_url = reverse('dashboard') - - # Clear the cache to avoid interference between tests - django_cache.clear() - config_cache.clear() - - @patch.dict(settings.FEATURES, {'ENABLE_COUNTRY_ACCESS': True}) - def test_blocked(self): - with restrict_course(self.course.id, access_point='courseware') as redirect_url: - response = self.client.get(self.courseware_url) - self.assertRedirects(response, redirect_url) - - @patch.dict(settings.FEATURES, {'ENABLE_COUNTRY_ACCESS': True}) - def test_allowed(self): - # Add the course to the list of restricted courses - # but don't create any access rules - RestrictedCourse.objects.create(course_key=self.course.id) - - # Expect that we can access courseware - response = self.client.get(self.courseware_url) - self.assertEqual(response.status_code, 200) - - @patch.dict(settings.FEATURES, {'ENABLE_COUNTRY_ACCESS': True}) - def test_non_courseware_url(self): - with restrict_course(self.course.id): - response = self.client.get(self.non_courseware_url) - self.assertEqual(response.status_code, 200) - - @patch.dict(settings.FEATURES, {'ENABLE_COUNTRY_ACCESS': True}) - @ddt.data( - # request_ip, blacklist, whitelist, is_enabled, allow_access - ('173.194.123.35', ['173.194.123.35'], [], True, False), - ('173.194.123.35', ['173.194.0.0/16'], [], True, False), - ('173.194.123.35', ['127.0.0.0/32', '173.194.0.0/16'], [], True, False), - ('173.195.10.20', ['173.194.0.0/16'], [], True, True), - ('173.194.123.35', ['173.194.0.0/16'], ['173.194.0.0/16'], True, False), - ('173.194.123.35', [], ['173.194.0.0/16'], True, True), - ('192.178.2.3', [], ['173.194.0.0/16'], True, True), - ('173.194.123.35', ['173.194.123.35'], [], False, True), - ) - @ddt.unpack - def test_ip_access_rules(self, request_ip, blacklist, whitelist, is_enabled, allow_access): - # Ensure that IP blocking works for anonymous users - self.client.logout() - - # Set up the IP rules - IPFilter.objects.create( - blacklist=", ".join(blacklist), - whitelist=", ".join(whitelist), - enabled=is_enabled - ) - - # Check that access is enforced - response = self.client.get( - "/", - HTTP_X_FORWARDED_FOR=request_ip, - REMOTE_ADDR=request_ip - ) - - if allow_access: - self.assertEqual(response.status_code, 200) - else: - redirect_url = reverse( - 'embargo_blocked_message', - kwargs={ - 'access_point': 'courseware', - 'message_key': 'embargo' - } - ) - self.assertRedirects(response, redirect_url) - - @patch.dict(settings.FEATURES, {'ENABLE_COUNTRY_ACCESS': True}) - @ddt.data( - ('courseware', 'default'), - ('courseware', 'embargo'), - ('enrollment', 'default'), - ('enrollment', 'embargo') - ) - @ddt.unpack - def test_always_allow_access_to_embargo_messages(self, access_point, msg_key): - # Blacklist an IP address - IPFilter.objects.create( - blacklist="192.168.10.20", - enabled=True - ) - - url = reverse( - 'embargo_blocked_message', - kwargs={ - 'access_point': access_point, - 'message_key': msg_key - } - ) - response = self.client.get( - url, - HTTP_X_FORWARDED_FOR="192.168.10.20", - REMOTE_ADDR="192.168.10.20" - ) - self.assertEqual(response.status_code, 200) - - @patch.dict(settings.FEATURES, {'ENABLE_COUNTRY_ACCESS': True}) - def test_whitelist_ip_skips_country_access_checks(self): - # Whitelist an IP address - IPFilter.objects.create( - whitelist="192.168.10.20", - enabled=True - ) - - # Set up country access rules so the user would - # be restricted from the course. - with restrict_course(self.course.id): - # Make a request from the whitelisted IP address - response = self.client.get( - self.courseware_url, - HTTP_X_FORWARDED_FOR="192.168.10.20", - REMOTE_ADDR="192.168.10.20" - ) - - # Expect that we were still able to access the page, - # even though we would have been blocked by country - # access rules. - self.assertEqual(response.status_code, 200) diff --git a/common/djangoapps/embargo/tests/test_views.py b/common/djangoapps/embargo/tests/test_views.py index 74c1caa5ef..48a8c94653 100644 --- a/common/djangoapps/embargo/tests/test_views.py +++ b/common/djangoapps/embargo/tests/test_views.py @@ -32,7 +32,7 @@ class CourseAccessMessageViewTest(UrlResetMixin, TestCase): """ - @patch.dict(settings.FEATURES, {'ENABLE_COUNTRY_ACCESS': True}) + @patch.dict(settings.FEATURES, {'EMBARGO': True}) def setUp(self): super(CourseAccessMessageViewTest, self).setUp('embargo') diff --git a/common/djangoapps/enrollment/tests/test_views.py b/common/djangoapps/enrollment/tests/test_views.py index a21c8940ef..e51bc9a646 100644 --- a/common/djangoapps/enrollment/tests/test_views.py +++ b/common/djangoapps/enrollment/tests/test_views.py @@ -283,7 +283,7 @@ class EnrollmentEmbargoTest(UrlResetMixin, ModuleStoreTestCase): EMAIL = "bob@example.com" PASSWORD = "edx" - @patch.dict(settings.FEATURES, {'ENABLE_COUNTRY_ACCESS': True}) + @patch.dict(settings.FEATURES, {'EMBARGO': True}) def setUp(self): """ Create a course and user, then log in. """ super(EnrollmentEmbargoTest, self).setUp('embargo') @@ -291,7 +291,7 @@ class EnrollmentEmbargoTest(UrlResetMixin, ModuleStoreTestCase): self.user = UserFactory.create(username=self.USERNAME, email=self.EMAIL, password=self.PASSWORD) self.client.login(username=self.USERNAME, password=self.PASSWORD) - @patch.dict(settings.FEATURES, {'ENABLE_COUNTRY_ACCESS': True}) + @patch.dict(settings.FEATURES, {'EMBARGO': True}) def test_embargo_change_enrollment_restrict(self): url = reverse('courseenrollments') data = json.dumps({ @@ -315,7 +315,7 @@ class EnrollmentEmbargoTest(UrlResetMixin, ModuleStoreTestCase): # Verify that we were not enrolled self.assertEqual(self._get_enrollments(), []) - @patch.dict(settings.FEATURES, {'ENABLE_COUNTRY_ACCESS': True}) + @patch.dict(settings.FEATURES, {'EMBARGO': True}) def test_embargo_change_enrollment_allow(self): url = reverse('courseenrollments') data = json.dumps({ diff --git a/common/djangoapps/student/tests/test_enrollment.py b/common/djangoapps/student/tests/test_enrollment.py index a256eb5aae..ecfab72331 100644 --- a/common/djangoapps/student/tests/test_enrollment.py +++ b/common/djangoapps/student/tests/test_enrollment.py @@ -26,7 +26,7 @@ class EnrollmentTest(UrlResetMixin, ModuleStoreTestCase): EMAIL = "bob@example.com" PASSWORD = "edx" - @patch.dict(settings.FEATURES, {'ENABLE_COUNTRY_ACCESS': True}) + @patch.dict(settings.FEATURES, {'EMBARGO': True}) def setUp(self): """ Create a course and user, then log in. """ super(EnrollmentTest, self).setUp('embargo') @@ -134,7 +134,7 @@ class EnrollmentTest(UrlResetMixin, ModuleStoreTestCase): else: self.assertFalse(mock_update_email_opt_in.called) - @patch.dict(settings.FEATURES, {'ENABLE_COUNTRY_ACCESS': True}) + @patch.dict(settings.FEATURES, {'EMBARGO': True}) def test_embargo_restrict(self): # When accessing the course from an embargoed country, # we should be blocked. @@ -147,7 +147,7 @@ class EnrollmentTest(UrlResetMixin, ModuleStoreTestCase): is_enrolled = CourseEnrollment.is_enrolled(self.user, self.course.id) self.assertFalse(is_enrolled) - @patch.dict(settings.FEATURES, {'ENABLE_COUNTRY_ACCESS': True}) + @patch.dict(settings.FEATURES, {'EMBARGO': True}) def test_embargo_allow(self): response = self._change_enrollment('enroll') self.assertEqual(response.status_code, 200) diff --git a/common/djangoapps/student/views.py b/common/djangoapps/student/views.py index bbb3482a9b..3771d8f730 100644 --- a/common/djangoapps/student/views.py +++ b/common/djangoapps/student/views.py @@ -168,21 +168,6 @@ def index(request, extra_context=None, user=AnonymousUser()): return render_to_response('index.html', context) -def embargo(_request): - """ - Render the embargo page. - - Explains to the user why they are not able to access a particular embargoed course. - Tries to use the themed version, but fall back to the default if not found. - """ - try: - if settings.FEATURES["USE_CUSTOM_THEME"]: - return render_to_response("static_templates/theme-embargo.html") - except TopLevelLookupException: - pass - return render_to_response("static_templates/embargo.html") - - def process_survey_link(survey_link, user): """ If {UNIQUE_ID} appears in the link, replace it with a unique id for the user. diff --git a/common/djangoapps/third_party_auth/tests/test_change_enrollment.py b/common/djangoapps/third_party_auth/tests/test_change_enrollment.py index 461211ed91..48f08b48e3 100644 --- a/common/djangoapps/third_party_auth/tests/test_change_enrollment.py +++ b/common/djangoapps/third_party_auth/tests/test_change_enrollment.py @@ -29,14 +29,14 @@ THIRD_PARTY_AUTH_CONFIGURED = ( @unittest.skipUnless(THIRD_PARTY_AUTH_CONFIGURED, "Third party auth must be configured") -@patch.dict(settings.FEATURES, {'ENABLE_COUNTRY_ACCESS': True}) +@patch.dict(settings.FEATURES, {'EMBARGO': True}) @ddt.ddt class PipelineEnrollmentTest(UrlResetMixin, ModuleStoreTestCase): """Test that the pipeline auto-enrolls students upon successful authentication. """ BACKEND_NAME = "google-oauth2" - @patch.dict(settings.FEATURES, {'ENABLE_COUNTRY_ACCESS': True}) + @patch.dict(settings.FEATURES, {'EMBARGO': True}) def setUp(self): """Create a test course and user. """ super(PipelineEnrollmentTest, self).setUp('embargo') @@ -129,7 +129,7 @@ class PipelineEnrollmentTest(UrlResetMixin, ModuleStoreTestCase): self.assertEqual(result, {}) self.assertFalse(CourseEnrollment.is_enrolled(self.user, self.course.id)) - @patch.dict(settings.FEATURES, {'ENABLE_COUNTRY_ACCESS': True}) + @patch.dict(settings.FEATURES, {'EMBARGO': True}) def test_blocked_by_embargo(self): strategy = self._fake_strategy() strategy.session_set('enroll_course_id', unicode(self.course.id)) diff --git a/lms/djangoapps/shoppingcart/tests/test_views.py b/lms/djangoapps/shoppingcart/tests/test_views.py index 337742933e..4cb9fc02e9 100644 --- a/lms/djangoapps/shoppingcart/tests/test_views.py +++ b/lms/djangoapps/shoppingcart/tests/test_views.py @@ -1585,7 +1585,7 @@ class RedeemCodeEmbargoTests(UrlResetMixin, ModuleStoreTestCase): USERNAME = 'bob' PASSWORD = 'test' - @patch.dict(settings.FEATURES, {'ENABLE_COUNTRY_ACCESS': True}) + @patch.dict(settings.FEATURES, {'EMBARGO': True}) def setUp(self): super(RedeemCodeEmbargoTests, self).setUp('embargo') self.course = CourseFactory.create() @@ -1594,7 +1594,7 @@ class RedeemCodeEmbargoTests(UrlResetMixin, ModuleStoreTestCase): self.assertTrue(result, msg="Could not log in") @ddt.data('get', 'post') - @patch.dict(settings.FEATURES, {'ENABLE_COUNTRY_ACCESS': True}) + @patch.dict(settings.FEATURES, {'EMBARGO': True}) def test_registration_code_redemption_embargo(self, method): # Create a valid registration code reg_code = CourseRegistrationCode.objects.create( diff --git a/lms/djangoapps/student_account/test/test_views.py b/lms/djangoapps/student_account/test/test_views.py index 035a8a7856..0d8d3bedf0 100644 --- a/lms/djangoapps/student_account/test/test_views.py +++ b/lms/djangoapps/student_account/test/test_views.py @@ -382,7 +382,7 @@ class StudentAccountLoginAndRegistrationTest(UrlResetMixin, ModuleStoreTestCase) EMAIL = "bob@example.com" PASSWORD = "password" - @mock.patch.dict(settings.FEATURES, {'ENABLE_COUNTRY_ACCESS': True}) + @mock.patch.dict(settings.FEATURES, {'EMBARGO': True}) def setUp(self): super(StudentAccountLoginAndRegistrationTest, self).setUp('embargo') @@ -551,7 +551,7 @@ class StudentAccountLoginAndRegistrationTest(UrlResetMixin, ModuleStoreTestCase) response = self.client.get(reverse("account_login"), {"course_id": unicode(course.id)}) self._assert_third_party_auth_data(response, None, expected_providers) - @mock.patch.dict(settings.FEATURES, {'ENABLE_COUNTRY_ACCESS': True}) + @mock.patch.dict(settings.FEATURES, {'EMBARGO': True}) def test_third_party_auth_enrollment_embargo(self): course = CourseFactory.create() diff --git a/lms/djangoapps/verify_student/tests/test_views.py b/lms/djangoapps/verify_student/tests/test_views.py index 4bd7108c89..f41d45596c 100644 --- a/lms/djangoapps/verify_student/tests/test_views.py +++ b/lms/djangoapps/verify_student/tests/test_views.py @@ -74,7 +74,7 @@ class TestPayAndVerifyView(UrlResetMixin, ModuleStoreTestCase): YESTERDAY = NOW - timedelta(days=1) TOMORROW = NOW + timedelta(days=1) - @mock.patch.dict(settings.FEATURES, {'ENABLE_COUNTRY_ACCESS': True}) + @mock.patch.dict(settings.FEATURES, {'EMBARGO': True}) def setUp(self): super(TestPayAndVerifyView, self).setUp('embargo') self.user = UserFactory.create(username=self.USERNAME, password=self.PASSWORD) @@ -625,7 +625,7 @@ class TestPayAndVerifyView(UrlResetMixin, ModuleStoreTestCase): self.assertContains(response, "verification deadline") self.assertContains(response, "Jan 02, 1999 at 00:00 UTC") - @mock.patch.dict(settings.FEATURES, {'ENABLE_COUNTRY_ACCESS': True}) + @mock.patch.dict(settings.FEATURES, {'EMBARGO': True}) def test_embargo_restrict(self): course = self._create_course("verified") with restrict_course(course.id) as redirect_url: @@ -634,7 +634,7 @@ class TestPayAndVerifyView(UrlResetMixin, ModuleStoreTestCase): response = self._get_page('verify_student_start_flow', course.id, expected_status_code=302) self.assertRedirects(response, redirect_url) - @mock.patch.dict(settings.FEATURES, {'ENABLE_COUNTRY_ACCESS': True}) + @mock.patch.dict(settings.FEATURES, {'EMBARGO': True}) def test_embargo_allow(self): course = self._create_course("verified") self._get_page('verify_student_start_flow', course.id) diff --git a/lms/envs/common.py b/lms/envs/common.py index d0365412f7..df01993d06 100644 --- a/lms/envs/common.py +++ b/lms/envs/common.py @@ -268,16 +268,10 @@ FEATURES = { # Hide any Personally Identifiable Information from application logs 'SQUELCH_PII_IN_LOGS': True, - # Toggles the embargo functionality, which enable embargoing for particular courses + # Toggles the embargo functionality, which blocks users from + # the site or courses based on their location. 'EMBARGO': False, - # Toggles the embargo site functionality, which enable embargoing for the whole site - 'SITE_EMBARGOED': False, - - # Toggle whether to replace the current embargo implementation with - # the more flexible "country access" feature. - 'ENABLE_COUNTRY_ACCESS': 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/lms/urls.py b/lms/urls.py index 3ae637d130..340c397f53 100644 --- a/lms/urls.py +++ b/lms/urls.py @@ -67,8 +67,6 @@ urlpatterns = ('', # nopep8 url(r'^i18n/', include('django.conf.urls.i18n')), - url(r'^embargo$', 'student.views.embargo', name="embargo"), - # Feedback Form endpoint url(r'^submit_feedback$', 'util.views.submit_feedback'), @@ -494,8 +492,8 @@ urlpatterns += ( url(r'^shoppingcart/', include('shoppingcart.urls')), ) -# Country access (embargo) -if settings.FEATURES.get('ENABLE_COUNTRY_ACCESS'): +# Embargo +if settings.FEATURES.get('EMBARGO'): urlpatterns += ( url(r'^embargo/', include('embargo.urls')), )