Merge pull request #9018 from edx/hotfix/2015-07-23

Hotfix 2015-07-23: Allow course staff to bypass embargo rules.

common/djangoapps/embargo/api.py

@@ -14,6 +14,7 @@ from rest_framework.response import Response
 from rest_framework import status
 from ipware.ip import get_ip
 
+from student.auth import has_course_author_access
 from embargo.models import CountryAccessRule, RestrictedCourse
 
 
@@ -70,6 +71,10 @@ def check_course_access(course_key, user=None, ip_address=None, url=None):
     if not course_is_restricted:
         return True
 
+    # Always give global and course staff access, regardless of embargo settings.
+    if user is not None and has_course_author_access(user, course_key):
+        return True
+
     if ip_address is not None:
         # Retrieve the country code from the IP address
         # and check it against the allowed countries list for a course

common/djangoapps/embargo/tests/test_api.py

@@ -18,6 +18,11 @@ from xmodule.modulestore.tests.factories import CourseFactory
 from xmodule.modulestore.tests.django_utils import (
     ModuleStoreTestCase, mixed_store_config
 )
+from student.roles import (
+    GlobalStaff, CourseRole, OrgRole,
+    CourseStaffRole, CourseInstructorRole,
+    OrgStaffRole, OrgInstructorRole
+)
 
 from embargo.models import (
     RestrictedCourse, Country, CountryAccessRule,
@@ -166,7 +171,7 @@ class EmbargoCheckAccessApiTests(ModuleStoreTestCase):
             # (restricted course, but pass all the checks)
             # This is the worst case, so it will hit all of the
             # caching code.
-            with self.assertNumQueries(3):
+            with self.assertNumQueries(4):
                 embargo_api.check_course_access(self.course.id, user=self.user, ip_address='0.0.0.0')
 
             with self.assertNumQueries(0):
@@ -182,6 +187,45 @@ class EmbargoCheckAccessApiTests(ModuleStoreTestCase):
         with self.assertNumQueries(0):
             embargo_api.check_course_access(self.course.id, user=self.user, ip_address='0.0.0.0')
 
+    @ddt.data(
+        GlobalStaff,
+        CourseStaffRole,
+        CourseInstructorRole,
+        OrgStaffRole,
+        OrgInstructorRole,
+    )
+    def test_staff_access_country_block(self, staff_role_cls):
+        # Add a country to the blacklist
+        CountryAccessRule.objects.create(
+            rule_type=CountryAccessRule.BLACKLIST_RULE,
+            restricted_course=self.restricted_course,
+            country=Country.objects.get(country='US')
+        )
+
+        # Appear to make a request from an IP in the blocked country
+        with self._mock_geoip('US'):
+            result = embargo_api.check_course_access(self.course.id, user=self.user, ip_address='0.0.0.0')
+
+        # Expect that the user is blocked, because the user isn't staff
+        self.assertFalse(result, msg="User should not have access because the user isn't staff.")
+
+        # Instantiate the role, configuring it for this course or org
+        if issubclass(staff_role_cls, CourseRole):
+            staff_role = staff_role_cls(self.course.id)
+        elif issubclass(staff_role_cls, OrgRole):
+            staff_role = staff_role_cls(self.course.id.org)
+        else:
+            staff_role = staff_role_cls()
+
+        # Add the user to the role
+        staff_role.add_users(self.user)
+
+        # Now the user should have access
+        with self._mock_geoip('US'):
+            result = embargo_api.check_course_access(self.course.id, user=self.user, ip_address='0.0.0.0')
+
+        self.assertTrue(result, msg="User should have access because the user is staff.")
+
     @contextmanager
     def _mock_geoip(self, country_code):
         with mock.patch.object(pygeoip.GeoIP, 'country_code_by_addr') as mock_ip: