Merge pull request #8592 from edx/nickersoft/improved-embargo-restriction

XCOM-416: Improved basket API's handling of embargo restrictions

common/djangoapps/embargo/api.py

@@ -10,6 +10,9 @@ import pygeoip
 
 from django.core.cache import cache
 from django.conf import settings
+from rest_framework.response import Response
+from rest_framework import status
+from ipware.ip import get_ip
 
 from embargo.models import CountryAccessRule, RestrictedCourse
 
@@ -166,3 +169,30 @@ def _country_code_from_ip(ip_addr):
         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)
+
+
+def get_embargo_response(request, course_id, user):
+    """
+    Check whether any country access rules block the user from enrollment.
+
+    Args:
+        request (HttpRequest): The request object
+        course_id (str): The requested course ID
+        user (str): The current user object
+
+    Returns:
+        HttpResponse: Response of the embargo page if embargoed, None if not
+
+    """
+    redirect_url = redirect_if_blocked(
+        course_id, user=user, ip_address=get_ip(request), url=request.path)
+    if redirect_url:
+        return Response(
+            status=status.HTTP_403_FORBIDDEN,
+            data={
+                "message": (
+                    u"Users from this location cannot access the course '{course_id}'."
+                ).format(course_id=course_id),
+                "user_message_url": request.build_absolute_uri(redirect_url)
+            }
+        )

common/djangoapps/enrollment/tests/test_views.py

@@ -26,6 +26,7 @@ from util.models import RateLimitConfiguration
 from util.testing import UrlResetMixin
 from enrollment import api
 from enrollment.errors import CourseEnrollmentError
+from openedx.core.lib.django_test_client_utils import get_absolute_url
 from openedx.core.djangoapps.user_api.models import UserOrgTag
 from student.tests.factories import UserFactory, CourseModeFactory
 from student.models import CourseEnrollment
@@ -725,10 +726,6 @@ class EnrollmentEmbargoTest(EnrollmentTestMixin, UrlResetMixin, ModuleStoreTestC
             'user': self.user.username
         })
 
-    def _get_absolute_url(self, path):
-        """ Generate an absolute URL for a resource on the test server. """
-        return u'http://testserver/{}'.format(path.lstrip('/'))
-
     def assert_access_denied(self, user_message_path):
         """
         Verify that the view returns HTTP status 403 and includes a URL in the response, and no enrollment is created.
@@ -741,7 +738,7 @@ class EnrollmentEmbargoTest(EnrollmentTestMixin, UrlResetMixin, ModuleStoreTestC
 
         # Expect that the redirect URL is included in the response
         resp_data = json.loads(response.content)
-        user_message_url = self._get_absolute_url(user_message_path)
+        user_message_url = get_absolute_url(user_message_path)
         self.assertEqual(resp_data['user_message_url'], user_message_url)
 
         # Verify that we were not enrolled

common/djangoapps/enrollment/views.py

@@ -32,7 +32,6 @@ from enrollment.errors import (
 )
 from student.models import User
 
-
 log = logging.getLogger(__name__)
 
 
@@ -406,21 +405,10 @@ class EnrollmentListView(APIView, ApiKeyPermissionMixIn):
                 }
             )
 
-        # Check whether any country access rules block the user from enrollment
-        # We do this at the view level (rather than the Python API level)
-        # because this check requires information about the HTTP request.
-        redirect_url = embargo_api.redirect_if_blocked(
-            course_id, user=user, ip_address=get_ip(request), url=request.path)
-        if redirect_url:
-            return Response(
-                status=status.HTTP_403_FORBIDDEN,
-                data={
-                    "message": (
-                        u"Users from this location cannot access the course '{course_id}'."
-                    ).format(course_id=course_id),
-                    "user_message_url": request.build_absolute_uri(redirect_url)
-                }
-            )
+        embargo_response = embargo_api.get_embargo_response(request, course_id, user)
+
+        if embargo_response:
+            return embargo_response
 
         try:
             is_active = request.DATA.get('is_active')

lms/djangoapps/commerce/tests/test_views.py

@@ -5,6 +5,7 @@ from uuid import uuid4
 from nose.plugins.attrib import attr
 
 import ddt
+from django.conf import settings
 from django.core.urlresolvers import reverse
 from django.test import TestCase
 from django.test.utils import override_settings
@@ -17,6 +18,8 @@ from commerce.constants import Messages
 from commerce.tests import TEST_BASKET_ID, TEST_ORDER_NUMBER, TEST_PAYMENT_DATA, TEST_API_URL, TEST_API_SIGNING_KEY
 from commerce.tests.mocks import mock_basket_order, mock_create_basket
 from course_modes.models import CourseMode
+from embargo.test_utils import restrict_course
+from openedx.core.lib.django_test_client_utils import get_absolute_url
 from enrollment.api import get_enrollment
 from student.models import CourseEnrollment
 from student.tests.factories import UserFactory, CourseModeFactory
@@ -42,7 +45,6 @@ class BasketsViewTests(EnrollmentEventTestMixin, UserMixin, ModuleStoreTestCase)
     """
     Tests for the commerce orders view.
     """
-
     def _post_to_view(self, course_id=None):
         """
         POST to the view being tested.
@@ -96,6 +98,17 @@ class BasketsViewTests(EnrollmentEventTestMixin, UserMixin, ModuleStoreTestCase)
         # Ignore events fired from UserFactory creation
         self.reset_tracker()
 
+    @mock.patch.dict(settings.FEATURES, {'EMBARGO': True})
+    def test_embargo_restriction(self):
+        """
+        The view should return HTTP 403 status if the course is embargoed.
+        """
+        with restrict_course(self.course.id) as redirect_url:
+            response = self._post_to_view()
+            self.assertEqual(403, response.status_code)
+            body = json.loads(response.content)
+            self.assertEqual(get_absolute_url(redirect_url), body['user_message_url'])
+
     def test_login_required(self):
         """
         The view should return HTTP 403 status if the user is not logged in.

lms/djangoapps/commerce/views.py

@@ -20,6 +20,7 @@ from course_modes.models import CourseMode
 from courseware import courses
 from edxmako.shortcuts import render_to_response
 from enrollment.api import add_enrollment
+from embargo import api as embargo_api
 from microsite_configuration import microsite
 from student.models import CourseEnrollment
 from openedx.core.lib.api.authentication import SessionAuthenticationAllowInactiveUser
@@ -76,6 +77,11 @@ class BasketsView(APIView):
         if not valid:
             return DetailResponse(error, status=HTTP_406_NOT_ACCEPTABLE)
 
+        embargo_response = embargo_api.get_embargo_response(request, course_key, user)
+
+        if embargo_response:
+            return embargo_response
+
         # Don't do anything if an enrollment already exists
         course_id = unicode(course_key)
         enrollment = CourseEnrollment.get_enrollment(user, course_key)

openedx/core/lib/django_test_client_utils.py

@@ -52,3 +52,8 @@ if not hasattr(RequestFactory, 'patch'):
 
 if not hasattr(Client, 'patch'):
     setattr(Client, 'patch', client_patch)
+
+
+def get_absolute_url(path):
+    """ Generate an absolute URL for a resource on the test server. """
+    return u'http://testserver/{}'.format(path.lstrip('/'))