Merge pull request #534 from edx/dcs/masters-perm

Moved the enrollment mode check to a user permission

edx_proctoring/__init__.py

@@ -5,6 +5,6 @@
 from __future__ import absolute_import
 
 # Be sure to update the version number in edx_proctoring/package.json
-__version__ = '1.5.14'
+__version__ = '1.5.15'
 
 default_app_config = 'edx_proctoring.apps.EdxProctoringConfig'  # pylint: disable=invalid-name

edx_proctoring/api.py

@@ -14,7 +14,7 @@
 
 from django.utils.translation import ugettext as _, ugettext_noop
 from django.conf import settings
-from django.contrib.auth.models import User
+from django.contrib.auth import get_user_model
 from django.template import loader
 from django.urls import reverse, NoReverseMatch
 from django.core.mail.message import EmailMessage
@@ -65,6 +65,8 @@
 
 REJECTED_GRADE_OVERRIDE_EARNED = 0.0
 
+USER_MODEL = get_user_model()
+
 
 def create_exam(course_id, content_id, exam_name, time_limit_mins, due_date=None,
                 is_proctored=True, is_practice_exam=False, external_id=None, is_active=True, hide_after_due=False,
@@ -1078,7 +1080,7 @@ def create_proctoring_attempt_status_email(user_id, exam_attempt_obj, course_nam
     if not exam_attempt_obj.taking_as_proctored or exam_attempt_obj.is_sample_attempt:
         return None
 
-    user = User.objects.get(id=user_id)
+    user = USER_MODEL.objects.get(id=user_id)
     course_info_url = ''
     email_subject = (
         _('Proctoring Results For {course_name} {exam_name}').format(
@@ -1305,19 +1307,6 @@ def get_active_exams_for_user(user_id, course_id=None):
     return result
 
 
-def _check_eligibility_of_enrollment_mode(credit_state):
-    """
-    Inspects that the enrollment mode of the user
-    is valid for proctoring
-    """
-
-    # Allow only the verified students to take the exam as a proctored exam
-    # Also make an exception for the honor students to take the "practice exam" as a proctored exam.
-    # For the rest of the enrollment modes, None is returned which shows the exam content
-    # to the student rather than the proctoring prompt.
-    return credit_state and credit_state['enrollment_mode'] == 'verified'
-
-
 def _get_ordered_prerequisites(prerequisites_statuses, filter_out_namespaces=None):
     """
     Apply filter and ordering of requirements status in our credit_state dictionary. This will
@@ -1571,13 +1560,15 @@ def get_attempt_status_summary(user_id, course_id, content_id):
     # eligibility
     if credit_service and not exam['is_practice_exam']:
         credit_state = credit_service.get_credit_state(user_id, six.text_type(course_id), return_course_info=True)
-        if not _check_eligibility_of_enrollment_mode(credit_state):
+        user = USER_MODEL.objects.get(id=user_id)
+        if not user.has_perm('edx_proctoring.can_take_proctored_exam', exam):
             return None
 
     attempt = get_exam_attempt(exam['id'], user_id)
     if attempt:
         status = attempt['status']
-    elif not exam['is_practice_exam'] and has_due_date_passed(credit_state.get('course_end_date', None)):
+    elif not exam['is_practice_exam'] \
+            and credit_state and has_due_date_passed(credit_state.get('course_end_date', None)):
         status = ProctoredExamStudentAttemptStatus.expired
     else:
         status = ProctoredExamStudentAttemptStatus.eligible
@@ -1863,20 +1854,10 @@ def _get_proctored_exam_view(exam, context, exam_id, user_id, course_id):
     """
     student_view_template = None
 
-    credit_state = context.get('credit_state')
+    user = USER_MODEL.objects.get(id=user_id)
 
-    # see if only 'verified' track students should see this *except* if it is a practice exam
-    check_mode = (
-        settings.PROCTORING_SETTINGS.get('MUST_BE_VERIFIED_TRACK', True) and
-        credit_state
-    )
-
-    if check_mode:
-        has_mode = _check_eligibility_of_enrollment_mode(credit_state)
-        if not has_mode:
-            # user does not have the required enrollment mode
-            # so do not override view this is a quick exit
-            return None
+    if not user.has_perm('edx_proctoring.can_take_proctored_exam', exam):
+        return None
 
     attempt = get_exam_attempt(exam_id, user_id)
 
@@ -1899,10 +1880,7 @@ def _get_proctored_exam_view(exam, context, exam_id, user_id, course_id):
 
         # get information about prerequisites
 
-        credit_requirement_status = (
-            credit_state.get('credit_requirement_status')
-            if credit_state else []
-        )
+        credit_requirement_status = context.get('credit_state', {}).get('credit_requirement_status', [])
 
         prerequisite_status = _are_prerequirements_satisfied(
             credit_requirement_status,
@@ -2020,11 +1998,7 @@ def get_student_view(user_id, course_id, content_id,
     if user_role != 'student':
         return None
 
-    credit_service = get_runtime_service('credit')
-
-    # call service to get course end date.
-    credit_state = credit_service.get_credit_state(user_id, course_id, return_course_info=True)
-    course_end_date = credit_state.get('course_end_date') if credit_state else None
+    course_end_date = context.get('credit_state', {}).get('course_end_date', None)
 
     exam_id = None
     try:

edx_proctoring/models.py

@@ -8,7 +8,7 @@
 from __future__ import absolute_import
 import six
 
-from django.contrib.auth.models import User
+from django.contrib.auth import get_user_model
 from django.db import models
 from django.db.models import Q
 from django.db.models.base import ObjectDoesNotExist
@@ -24,6 +24,8 @@
 )
 from edx_proctoring.statuses import ProctoredExamStudentAttemptStatus, SoftwareSecureReviewStatus
 
+USER_MODEL = get_user_model()
+
 
 @six.python_2_unicode_compatible
 class ProctoredExam(TimeStampedModel):
@@ -123,7 +125,7 @@ class ProctoredExamReviewPolicy(TimeStampedModel):
     """
 
     # who set this ProctoredExamReviewPolicy
-    set_by_user = models.ForeignKey(User)
+    set_by_user = models.ForeignKey(USER_MODEL)
 
     # for which exam?
     proctored_exam = models.ForeignKey(ProctoredExam, db_index=True)
@@ -166,7 +168,7 @@ class ProctoredExamReviewPolicyHistory(TimeStampedModel):
     original_id = models.IntegerField(db_index=True)
 
     # who set this ProctoredExamReviewPolicy
-    set_by_user = models.ForeignKey(User)
+    set_by_user = models.ForeignKey(USER_MODEL)
 
     # for which exam?
     proctored_exam = models.ForeignKey(ProctoredExam, db_index=True)
@@ -287,7 +289,7 @@ class ProctoredExamStudentAttempt(TimeStampedModel):
     """
     objects = ProctoredExamStudentAttemptManager()
 
-    user = models.ForeignKey(User, db_index=True)
+    user = models.ForeignKey(USER_MODEL, db_index=True)
 
     proctored_exam = models.ForeignKey(ProctoredExam, db_index=True)
 
@@ -374,7 +376,7 @@ class ProctoredExamStudentAttemptHistory(TimeStampedModel):
     but will record (for audit history) all entries that have been updated.
     """
 
-    user = models.ForeignKey(User, db_index=True)
+    user = models.ForeignKey(USER_MODEL, db_index=True)
 
     # this is the PK of the original table, note this is not a FK
     attempt_id = models.IntegerField(null=True)
@@ -497,7 +499,7 @@ class ProctoredExamStudentAllowance(TimeStampedModel):
 
     objects = ProctoredExamStudentAllowanceManager()
 
-    user = models.ForeignKey(User)
+    user = models.ForeignKey(USER_MODEL)
 
     proctored_exam = models.ForeignKey(ProctoredExam)
 
@@ -558,9 +560,9 @@ def add_allowance_for_user(cls, exam_id, user_info, key, value):
             user_id = user_info
         else:
             # we got a string, so try to resolve it
-            users = User.objects.filter(username=user_info)
+            users = USER_MODEL.objects.filter(username=user_info)
             if not users.exists():
-                users = User.objects.filter(email=user_info)
+                users = USER_MODEL.objects.filter(email=user_info)
 
             if not users.exists():
                 err_msg = (
@@ -626,7 +628,7 @@ class ProctoredExamStudentAllowanceHistory(TimeStampedModel):
     # what was the original id of the allowance
     allowance_id = models.IntegerField()
 
-    user = models.ForeignKey(User)
+    user = models.ForeignKey(USER_MODEL)
 
     proctored_exam = models.ForeignKey(ProctoredExam)
 
@@ -662,12 +664,12 @@ class ProctoredExamSoftwareSecureReview(TimeStampedModel):
     video_url = models.TextField()
 
     # user_id of person who did the review (can be None if submitted via server-to-server API)
-    reviewed_by = models.ForeignKey(User, null=True, related_name='+')
+    reviewed_by = models.ForeignKey(USER_MODEL, null=True, related_name='+')
 
     # student username for the exam
     # this is an optimization for the Django Admin pane (so we can search)
     # this is null because it is being added after initial production ship
-    student = models.ForeignKey(User, null=True, related_name='+')
+    student = models.ForeignKey(USER_MODEL, null=True, related_name='+')
 
     # exam_id for the review
     # this is an optimization for the Django Admin pane (so we can search)
@@ -727,12 +729,12 @@ class ProctoredExamSoftwareSecureReviewHistory(TimeStampedModel):
     video_url = models.TextField()
 
     # user_id of person who did the review (can be None if submitted via server-to-server API)
-    reviewed_by = models.ForeignKey(User, null=True, related_name='+')
+    reviewed_by = models.ForeignKey(USER_MODEL, null=True, related_name='+')
 
     # student username for the exam
     # this is an optimization for the Django Admin pane (so we can search)
     # this is null because it is being added after initial production ship
-    student = models.ForeignKey(User, null=True, related_name='+')
+    student = models.ForeignKey(USER_MODEL, null=True, related_name='+')
 
     # exam_id for the review
     # this is an optimization for the Django Admin pane (so we can search)

edx_proctoring/tests/__init__.py

@@ -32,6 +32,10 @@ def setup_test_perms():
         rules.add_perm('accounts.can_retire_user', rules.is_staff)
     except KeyError:
         pass
+    try:
+        rules.add_perm('edx_proctoring.can_take_proctored_exam', rules.is_authenticated)
+    except KeyError:
+        pass
 
 
 setup_test_backends()

edx_proctoring/tests/test_api.py

@@ -1618,23 +1618,19 @@ def test_practice_no_attempt(self):
         )
         self.assertIn(summary, [expected])
 
-    @ddt.data(
-        'honor', 'staff'
-    )
-    def test_status_summary_honor(self, enrollment_mode):
+    def test_status_summary_no_perm(self):
         """
-        Make sure status summary is None for a non-verified person
+        The summary should be None for users who don't have permission
+        (For the tests, that means non-authenticated users)
         """
-
-        set_runtime_service('credit', MockCreditService(enrollment_mode=enrollment_mode))
-
+        set_runtime_service('credit', MockCreditService(enrollment_mode='verified'))
         exam_attempt = self._create_started_exam_attempt()
-
-        summary = get_attempt_status_summary(
-            self.user.id,
-            exam_attempt.proctored_exam.course_id,
-            exam_attempt.proctored_exam.content_id
-        )
+        with patch('django.contrib.auth.models.User.is_authenticated', False):
+            summary = get_attempt_status_summary(
+                self.user.id,
+                exam_attempt.proctored_exam.course_id,
+                exam_attempt.proctored_exam.content_id
+            )
 
         self.assertIsNone(summary)
 
@@ -1776,21 +1772,6 @@ def test_declined_prerequisites(self, content_id,
         self.assertEqual(len(results['pending_prerequisites']), expected_len_pending_prerequisites)
         self.assertEqual(len(results['declined_prerequisites']), expected_len_declined_prerequisites)
 
-    def test_summary_without_credit_state(self):
-        """
-        Test that attempt status summary is None for users who are not enrolled.
-        """
-        exam_id = self._create_exam_with_due_time()
-        set_runtime_service('credit', MockCreditServiceNone())
-
-        timed_exam = get_exam_by_id(exam_id)
-        summary = get_attempt_status_summary(
-            self.user.id,
-            timed_exam['course_id'],
-            timed_exam['content_id']
-        )
-        self.assertIsNone(summary)
-
     def test_get_exam_violation_report(self):
         """
         Test to get all the exam attempts.

edx_proctoring/tests/test_student_view.py

@@ -168,18 +168,6 @@ def test_get_honor_view_with_practice_exam(self):
         })
         self.assertIsNotNone(rendered_response)
 
-    def test_get_honor_view(self):
-        """
-        Test for get_student_view prompting when the student is enrolled in non-verified
-        track, this should return None
-        """
-        rendered_response = self.render_proctored_exam({
-            'credit_state': {
-                'enrollment_mode': 'honor'
-            },
-        })
-        self.assertIsNone(rendered_response)
-
     @ddt.data(
         (None, 'Make sure you are on a computer with a webcam, and that you have valid photo identification'),
         ('pending', 'Your verification is pending'),
@@ -334,8 +322,7 @@ def test_proctored_exam_passed_end_date(self):
         Verify that we get a None back on a proctored exam
         if the course end date is passed
         """
-
-        set_runtime_service('credit', MockCreditServiceWithCourseEndDate())
+        credit_state = MockCreditServiceWithCourseEndDate().get_credit_state(self.user_id, 'foo', True)
 
         rendered_response = get_student_view(
             user_id=self.user_id,
@@ -348,6 +335,7 @@ def test_proctored_exam_passed_end_date(self):
                 'default_time_limit_mins': 90,
                 'due_date': None,
                 'hide_after_due': False,
+                'credit_state': credit_state,
             },
             user_role='student'
         )
@@ -358,8 +346,7 @@ def test_practice_exam_passed_end_date(self):
         Verify that we get a None back on a practice exam
         if the course end date is passed
         """
-
-        set_runtime_service('credit', MockCreditServiceWithCourseEndDate())
+        credit_state = MockCreditServiceWithCourseEndDate().get_credit_state(self.user_id, 'foo', True)
 
         rendered_response = get_student_view(
             user_id=self.user_id,
@@ -372,6 +359,7 @@ def test_practice_exam_passed_end_date(self):
                 'default_time_limit_mins': 90,
                 'due_date': None,
                 'hide_after_due': False,
+                'credit_state': credit_state,
             },
             user_role='student'
         )
@@ -749,6 +737,16 @@ def test_get_studentview_submitted_status_with_duedate(self):
             )
             self.assertIsNotNone(rendered_response)
 
+    def test_get_no_perm_view(self):
+        """
+        Test for get_student_view prompting when the student does not have permission
+        to view proctored exams, this should return None
+        (For edx-proctoring tests, only authenticated students have the permission)
+        """
+        with patch('django.contrib.auth.models.User.is_authenticated', False):
+            rendered_response = self.render_proctored_exam()
+        self.assertIsNone(rendered_response)
+
     def test_get_studentview_started_onboarding(self):
         """
         Test fallthrough page case for onboarding exams

edx_proctoring/views.py

@@ -11,7 +11,8 @@
 
 from crum import get_current_request
 from django.conf import settings
-from django.contrib.auth.models import User
+from django.contrib.auth import get_user_model
+from django.core.exceptions import ObjectDoesNotExist
 from django.core.paginator import Paginator, EmptyPage, PageNotAnInteger
 from django.urls import reverse, NoReverseMatch
 from django.shortcuts import redirect
@@ -858,8 +859,8 @@ def make_review(self, attempt, data, backend=None):
         review.exam_id = attempt['proctored_exam']['id']
 
         try:
-            review.reviewed_by = User.objects.get(email=data['reviewed_by'])
-        except (User.DoesNotExist, KeyError):
+            review.reviewed_by = get_user_model().objects.get(email=data['reviewed_by'])
+        except (ObjectDoesNotExist, KeyError):
             review.reviewed_by = None
 
         # If the reviewing user is a user in the system (user may be None for automated reviews) and does