Merge pull request #251 from edx/cdodge/dont-store-video-url

redact video url in the review postback

edx_proctoring/admin.py

@@ -75,18 +75,9 @@ class ProctoredExamSoftwareSecureReviewForm(forms.ModelForm):
     ]
 
     review_status = forms.ChoiceField(choices=REVIEW_STATUS_CHOICES)
-    video_url = forms.URLField()
     raw_data = forms.CharField(widget=forms.Textarea, label='Reviewer Notes')
 
 
-def video_url_for_review(obj):
-    """Return hyperlink to review video url"""
-    return (
-        '<a href="{video_url}" target="_blank">{video_url}</a>'.format(video_url=obj.video_url)
-    )
-video_url_for_review.allow_tags = True
-
-
 class ReviewListFilter(admin.SimpleListFilter):
     """
     Quick filter to allow admins to see which reviews have not been reviewed internally
@@ -238,7 +229,7 @@ class ProctoredExamSoftwareSecureReviewAdmin(admin.ModelAdmin):
     The admin panel for SoftwareSecure Review records
     """
 
-    readonly_fields = [video_url_for_review, 'attempt_code', 'exam', 'student', 'reviewed_by', 'modified']
+    readonly_fields = ['attempt_code', 'exam', 'student', 'reviewed_by', 'modified']
     list_filter = [
         ReviewListFilter,
         ProctoredExamListFilter,
@@ -309,7 +300,8 @@ class ProctoredExamSoftwareSecureReviewAdmin(admin.ModelAdmin):
 
     def get_form(self, request, obj=None, **kwargs):
         form = super(ProctoredExamSoftwareSecureReviewAdmin, self).get_form(request, obj, **kwargs)
-        del form.base_fields['video_url']
+        if 'video_url' in form.base_fields:
+            del form.base_fields['video_url']
         return form
 
     def lookup_allowed(self, key, value):
@@ -324,7 +316,6 @@ class ProctoredExamSoftwareSecureReviewHistoryAdmin(ProctoredExamSoftwareSecureR
     """
 
     readonly_fields = [
-        video_url_for_review,
         'review_status',
         'raw_data',
         'attempt_code',

edx_proctoring/backends/software_secure.py

@@ -129,6 +129,10 @@ class SoftwareSecureBackendProvider(ProctoringBackendProvider):
         documentation named "Reviewer Data Transfer"
         """
 
+        # redact the videoReviewLink from the payload
+        if 'videoReviewLink' in payload:
+            del payload['videoReviewLink']
+
         log_msg = (
             'Received callback from SoftwareSecure with review data: {payload}'.format(
                 payload=payload
@@ -189,10 +193,6 @@ class SoftwareSecureBackendProvider(ProctoringBackendProvider):
 
         # do some limited parsing of the JSON payload
         review_status = payload['reviewStatus']
-        video_review_link = payload['videoReviewLink']
-
-        # be sure to change over any http: to https: on the video_review_link
-        video_review_link = video_review_link.replace('http:', 'https:')
 
         # do we already have a review for this attempt?!? We may not allow updates
         review = ProctoredExamSoftwareSecureReview.get_review_by_attempt_code(attempt_code)
@@ -224,7 +224,6 @@ class SoftwareSecureBackendProvider(ProctoringBackendProvider):
         review.attempt_code = attempt_code
         review.raw_data = json.dumps(payload)
         review.review_status = review_status
-        review.video_url = video_review_link
         review.student = attempt_obj.user
         review.exam = attempt_obj.proctored_exam
         # set reviewed_by to None because it was reviewed by our 3rd party
@@ -256,7 +255,6 @@ class SoftwareSecureBackendProvider(ProctoringBackendProvider):
             'review_attempt_code': review.attempt_code,
             'review_raw_data': review.raw_data,
             'review_status': review.review_status,
-            'review_video_url': review.video_url
         }
 
         serialized_attempt_obj = ProctoredExamStudentAttemptSerializer(attempt_obj)

edx_proctoring/backends/tests/test_software_secure.py

@@ -486,10 +486,8 @@ class SoftwareSecureTests(TestCase):
 
         self.assertIsNotNone(review)
         self.assertEqual(review.review_status, review_status)
-        self.assertEqual(
-            review.video_url,
-            'https://www.remoteproctor.com/AdminSite/Account/Reviewer/DirectLink-Generic.aspx?ID=foo'
-        )
+        self.assertFalse(review.video_url)
+
         self.assertIsNotNone(review.raw_data)
         self.assertIsNone(review.reviewed_by)
 
@@ -660,10 +658,8 @@ class SoftwareSecureTests(TestCase):
 
         self.assertIsNotNone(review)
         self.assertEqual(review.review_status, 'Clean')
-        self.assertEqual(
-            review.video_url,
-            'https://www.remoteproctor.com/AdminSite/Account/Reviewer/DirectLink-Generic.aspx?ID=foo'
-        )
+        self.assertFalse(review.video_url)
+
         self.assertIsNotNone(review.raw_data)
 
         # now check the comments that were stored
@@ -758,10 +754,8 @@ class SoftwareSecureTests(TestCase):
 
         self.assertIsNotNone(review)
         self.assertEqual(review.review_status, 'Suspicious')
-        self.assertEqual(
-            review.video_url,
-            'https://www.remoteproctor.com/AdminSite/Account/Reviewer/DirectLink-Generic.aspx?ID=foo'
-        )
+        self.assertFalse(review.video_url)
+
         self.assertIsNotNone(review.raw_data)
 
         # make sure history table is no longer empty

edx_proctoring/models.py

@@ -871,6 +871,8 @@ class ProctoredExamSoftwareSecureReview(TimeStampedModel):
     raw_data = models.TextField()
 
     # URL for the exam video that had been reviewed
+    # NOTE: To be deleted in future release, once the code that depends on it
+    # has been removed
     video_url = models.TextField()
 
     # user_id of person who did the review (can be None if submitted via server-to-server API)
@@ -972,7 +974,6 @@ def _make_review_archive_copy(instance):
         attempt_code=instance.attempt_code,
         review_status=instance.review_status,
         raw_data=instance.raw_data,
-        video_url=instance.video_url,
         reviewed_by=instance.reviewed_by,
         student=instance.student,
         exam=instance.exam,