Merge pull request #23588 from edx/arch/django2-student-admin

Django2: student admin views permission fix

common/djangoapps/student/admin.py

@@ -54,6 +54,32 @@ User = get_user_model()  # pylint:disable=invalid-name
 COURSE_ENROLLMENT_ADMIN_SWITCH = WaffleSwitch(STUDENT_WAFFLE_NAMESPACE, 'courseenrollment_admin')
 
 
+class _Check(object):
+    """
+    A method decorator that pre-emptively returns false if a feature is disabled.
+    Otherwise, it returns the return value of the decorated method.
+
+    To use, add this decorator above a method and pass in a function that returns
+    a boolean indicating whether the feature is enabled.
+
+    Example:
+    @_Check.is_enabled(FEATURE_TOGGLE.is_enabled)
+    """
+    @classmethod
+    def is_enabled(cls, is_enabled_func):
+        """
+        See above docstring.
+        """
+        def inner(func):
+            @wraps(func)
+            def decorator(*args, **kwargs):
+                if not is_enabled_func():
+                    return False
+                return func(*args, **kwargs)
+            return decorator
+        return inner
+
+
 class CourseAccessRoleForm(forms.ModelForm):
     """Form for adding new Course Access Roles view the Django Admin Panel."""
 
@@ -238,37 +264,40 @@ class CourseEnrollmentAdmin(admin.ModelAdmin):
     def queryset(self, request):
         return super(CourseEnrollmentAdmin, self).queryset(request).select_related('user')
 
-    def has_permission(self, request, method):
+    @_Check.is_enabled(COURSE_ENROLLMENT_ADMIN_SWITCH.is_enabled)
+    def has_view_permission(self, request, obj=None):
         """
-        Returns True if the given admin method is allowed.
+        Returns True if CourseEnrollment objects can be viewed via the admin view.
         """
-        if COURSE_ENROLLMENT_ADMIN_SWITCH.is_enabled():
-            return getattr(super(CourseEnrollmentAdmin, self), method)(request)
-        return False
+        return super(CourseEnrollmentAdmin, self).has_view_permission(request, obj)  # pylint: disable=no-member
 
+    @_Check.is_enabled(COURSE_ENROLLMENT_ADMIN_SWITCH.is_enabled)
     def has_add_permission(self, request):
         """
         Returns True if CourseEnrollment objects can be added via the admin view.
         """
-        return self.has_permission(request, 'has_add_permission')
+        return super(CourseEnrollmentAdmin, self).has_add_permission(request)
 
+    @_Check.is_enabled(COURSE_ENROLLMENT_ADMIN_SWITCH.is_enabled)
     def has_change_permission(self, request, obj=None):
         """
         Returns True if CourseEnrollment objects can be modified via the admin view.
         """
-        return self.has_permission(request, 'has_change_permission')
+        return super(CourseEnrollmentAdmin, self).has_change_permission(request, obj)
 
+    @_Check.is_enabled(COURSE_ENROLLMENT_ADMIN_SWITCH.is_enabled)
     def has_delete_permission(self, request, obj=None):
         """
         Returns True if CourseEnrollment objects can be deleted via the admin view.
         """
-        return self.has_permission(request, 'has_delete_permission')
+        return super(CourseEnrollmentAdmin, self).has_delete_permission(request, obj)
 
+    @_Check.is_enabled(COURSE_ENROLLMENT_ADMIN_SWITCH.is_enabled)
     def has_module_permission(self, request):
         """
         Returns True if links to the CourseEnrollment admin view can be displayed.
         """
-        return self.has_permission(request, 'has_module_permission')
+        return super(CourseEnrollmentAdmin, self).has_module_permission(request)
 
 
 class UserProfileInline(admin.StackedInline):
@@ -351,45 +380,35 @@ class LoginFailuresAdmin(admin.ModelAdmin):
     actions = ['unlock_student_accounts']
     change_form_template = 'admin/student/loginfailures/change_form_template.html'
 
-    class _Feature(object):
-        """
-        Inner feature class to implement decorator.
-        """
-        @classmethod
-        def is_enabled(cls, func):
-            """
-            Check if feature is enabled.
-            """
-            @wraps(func)
-            def decorator(*args, **kwargs):
-                """Decorator class to return"""
-                if not LoginFailures.is_feature_enabled():
-                    return False
-                return func(*args, **kwargs)
-            return decorator
-
-    @_Feature.is_enabled
+    @_Check.is_enabled(LoginFailures.is_feature_enabled)
     def has_module_permission(self, request):
         """
         Only enabled if feature is enabled.
         """
         return super(LoginFailuresAdmin, self).has_module_permission(request)
 
-    @_Feature.is_enabled
+    @_Check.is_enabled(LoginFailures.is_feature_enabled)
+    def has_view_permission(self, request, obj=None):
+        """
+        Only enabled if feature is enabled.
+        """
+        return super(LoginFailuresAdmin, self).has_view_permission(request, obj)  # pylint: disable=no-member
+
+    @_Check.is_enabled(LoginFailures.is_feature_enabled)
     def has_delete_permission(self, request, obj=None):
         """
         Only enabled if feature is enabled.
         """
         return super(LoginFailuresAdmin, self).has_delete_permission(request, obj)
 
-    @_Feature.is_enabled
+    @_Check.is_enabled(LoginFailures.is_feature_enabled)
     def has_change_permission(self, request, obj=None):
         """
         Only enabled if feature is enabled.
         """
         return super(LoginFailuresAdmin, self).has_change_permission(request, obj)
 
-    @_Feature.is_enabled
+    @_Check.is_enabled(LoginFailures.is_feature_enabled)
     def has_add_permission(self, request):
         """
         Only enabled if feature is enabled.

common/djangoapps/student/tests/test_admin_views.py

@@ -347,6 +347,12 @@ class LoginFailuresAdminTest(TestCase):
         )
         self.assertEqual(str(LoginFailures.objects.get(user=self.user2)), 'Zażółć gęślą jaźń: 2 - -')
 
+    @override_settings(FEATURES={'ENABLE_MAX_FAILED_LOGIN_ATTEMPTS': True})
+    def test_feature_enabled(self):
+        url = reverse('admin:student_loginfailures_changelist')
+        response = self.client.get(url)
+        self.assertEqual(response.status_code, 200)
+
     @ddt.data(
         reverse('admin:student_loginfailures_changelist'),
         reverse('admin:student_loginfailures_add'),