Merge pull request #7076 from edx/will/deprecate-embargo

Deprecate old embargo implementation.

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,
 

cms/urls.py

@@ -63,7 +63,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

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)

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 <a href="http://en.wikipedia.org/wiki/ISO_3166-1#Officially_assigned_code_elements">
-            this list of ISO-3166-1 country codes</a>.
-
-            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)

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.

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"""
 

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)
-
-        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`
+        ip_address = get_ip(request)
+        ip_filter = IPFilter.current()
 
-        """
-        # 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:
-            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 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'
+                }
             )
-
-        # 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)
+            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
             )
-        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 the IP is whitelisted, then allow access,
+            # skipping later checks.
+            return None
 
-        """
-        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):
         """

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(

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()

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
-        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
+    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 authorized
-        self.assertTrue(EmbargoedCourse.is_embargoed(self.course.id))
-
-        # 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()
-
-        # 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'
+    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)
 
-        form_data = {'course_id': bad_id, 'embargoed': True}
-        form = EmbargoedCourseForm(data=form_data)
-        # Validation shouldn't work
-        self.assertFalse(form.is_valid())
+    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)
 
-        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()
-
-    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)
+    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
-
-        with self.assertRaisesRegexp(ValueError, "The EmbargoedCourse could not be created because the data didn't validate."):
-            form.save()
+        self.assertIn(msg, form._errors['course_key'][0])  # pylint: disable=protected-access
 
-
-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."):
+        with self.assertRaisesRegexp(ValueError, "The RestrictedCourse 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"""

common/djangoapps/embargo/tests/test_middleware.py

 """
-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
-    """
-    def setUp(self):
-        super(EmbargoMiddlewareTests, self).setUp()
+class EmbargoMiddlewareAccessTests(UrlResetMixin, ModuleStoreTestCase):
+    """Tests of embargo middleware country access rules.
 
-        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()
+    There are detailed unit tests for the rule logic in
+    `test_api.py`; here, we're mainly testing the integration
+    with middleware
 
-    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.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='1.0.0.0', REMOTE_ADDR='1.0.0.0')
-        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)
+    """
+    USERNAME = 'fred'
+    PASSWORD = 'secret'
 
-    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
+    @patch.dict(settings.FEATURES, {'EMBARGO': 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.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()
+        self.non_courseware_url = reverse('dashboard')
 
-        # 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)
+        # Clear the cache to avoid interference between tests
+        django_cache.clear()
+        config_cache.clear()
 
-        # 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)
+    @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)
 
-        # 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)
+    @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)
 
-        # 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')
+        # Expect that we can access courseware
+        response = self.client.get(self.courseware_url)
         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)
+    @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)
 
-        # 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
+    @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()
+
+        # Set up the IP rules
+        IPFilter.objects.create(
+            blacklist=", ".join(blacklist),
+            whitelist=", ".join(whitelist),
+            enabled=is_enabled
         )
-        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
+        # Check that access is enforced
         response = self.client.get(
-            self.embargoed_page,
-            HTTP_X_FORWARDED_FOR='1.1.0.0',
-            REMOTE_ADDR='1.1.0.0',
-            follow=True
+            "/",
+            HTTP_X_FORWARDED_FOR=request_ip,
+            REMOTE_ADDR=request_ip
         )
-        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)
 
+        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, {'EMBARGO': True})
     @ddt.data(
-        (None, False),
-        ("", False),
-        ("us", False),
-        ("CU", True),
-        ("Ir", True),
-        ("sy", True),
-        ("sd", True)
+        ('courseware', 'default'),
+        ('courseware', 'embargo'),
+        ('enrollment', 'default'),
+        ('enrollment', 'embargo')
     )
     @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:
-            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)
-
-    def test_embargo_profile_country_cache(self):
-        # Set the country in the user's profile
-        profile = self.user.profile
-        profile.country = "us"
-        profile.save()
-
-        # Warm the cache
-        with self.assertNumQueries(16):
-            self.client.get(self.embargoed_page)
-
-        # 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)
-
-    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,
+    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')
-        self.assertEqual(response.status_code, 200)
+    @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
+        )
 
-    @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)
+        # 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"
+            )
 
-        # 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')
+        # 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})
-    @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)

common/djangoapps/embargo/tests/test_middleware_access_rules.py

-"""
-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)

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')
 

common/djangoapps/enrollment/tests/test_views.py

@@ -312,7 +312,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')
@@ -320,7 +320,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({
@@ -344,7 +344,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({

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)

common/djangoapps/student/views.py

@@ -166,21 +166,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.

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))

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(

lms/djangoapps/student_account/test/test_views.py

@@ -209,7 +209,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')
 
@@ -398,7 +398,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()
 

lms/djangoapps/verify_student/tests/test_views.py

@@ -73,7 +73,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)
@@ -624,7 +624,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:
@@ -633,7 +633,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)

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

lms/urls.py

@@ -72,8 +72,6 @@ urlpatterns = (
 
     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'),
 
@@ -499,8 +497,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')),
     )