From aeac231c72ddbddffba9a3bbcfe611ad5c54ad6e Mon Sep 17 00:00:00 2001 From: Alie Langston Date: Tue, 5 Oct 2021 14:00:55 -0400 Subject: [PATCH] feat: use video review url provided by PSI for instructor dashboard url The instructor dashboard url is passed to zendesk tickets sent to support when a review for an attempt is received. This link has never worked for PSI attempts, as PSI's backend class is not set up to support the instructor dashboard view. With this PR, PSI's backend class is built out to work with the instructor dashboard view (which requires a backend class to provide a link to redirect to). PSI's backend class provides the attempt video review url as the link to redirect to. --- CHANGELOG.rst | 4 + edx_proctoring/__init__.py | 2 +- edx_proctoring/backends/backend.py | 5 +- edx_proctoring/backends/rest.py | 5 +- edx_proctoring/backends/software_secure.py | 28 +++ edx_proctoring/backends/tests/test_backend.py | 5 +- edx_proctoring/constants.py | 2 + edx_proctoring/tests/test_views.py | 177 ++++++++++++++++++ edx_proctoring/views.py | 13 +- package.json | 2 +- 10 files changed, 236 insertions(+), 7 deletions(-) diff --git a/CHANGELOG.rst b/CHANGELOG.rst index 4462af8db97..602e80ddded 100644 --- a/CHANGELOG.rst +++ b/CHANGELOG.rst @@ -14,6 +14,10 @@ Change Log Unreleased ~~~~~~~~~~ +[4.1.2] - 2021-10-07 +~~~~~~~~~~~~~~~~~~~~ +* Instructor dashboard view should redirect to review url for PSI exam attempts + [4.1.1] - 2021-10-05 ~~~~~~~~~~~~~~~~~~~~ * Bug fix to redact video url from raw data in exam review diff --git a/edx_proctoring/__init__.py b/edx_proctoring/__init__.py index dec1b67b49c..aff079fa43c 100644 --- a/edx_proctoring/__init__.py +++ b/edx_proctoring/__init__.py @@ -3,6 +3,6 @@ """ # Be sure to update the version number in edx_proctoring/package.json -__version__ = '4.1.1' +__version__ = '4.1.2' default_app_config = 'edx_proctoring.apps.EdxProctoringConfig' # pylint: disable=invalid-name diff --git a/edx_proctoring/backends/backend.py b/edx_proctoring/backends/backend.py index 82519d773dc..2b4ac8c0fd2 100644 --- a/edx_proctoring/backends/backend.py +++ b/edx_proctoring/backends/backend.py @@ -113,7 +113,10 @@ def get_attempt(self, attempt): return attempt # pylint: disable=unused-argument - def get_instructor_url(self, course_id, user, exam_id=None, attempt_id=None, show_configuration_dashboard=False): + def get_instructor_url( + self, course_id, user, exam_id=None, attempt_id=None, + show_configuration_dashboard=False, encrypted_video_review_url=None + ): """ Returns the instructor dashboard url for reviews """ diff --git a/edx_proctoring/backends/rest.py b/edx_proctoring/backends/rest.py index 7fc93bdd217..884a40cfd2f 100644 --- a/edx_proctoring/backends/rest.py +++ b/edx_proctoring/backends/rest.py @@ -287,7 +287,10 @@ def on_exam_saved(self, exam): data = {} return data.get('id') - def get_instructor_url(self, course_id, user, exam_id=None, attempt_id=None, show_configuration_dashboard=False): + def get_instructor_url( + self, course_id, user, exam_id=None, attempt_id=None, + show_configuration_dashboard=False, encrypted_video_review_url=None + ): """ Return a URL to the instructor dashboard course_id: str diff --git a/edx_proctoring/backends/software_secure.py b/edx_proctoring/backends/software_secure.py index 8d13be7a5a2..d9dff1d4aef 100644 --- a/edx_proctoring/backends/software_secure.py +++ b/edx_proctoring/backends/software_secure.py @@ -4,6 +4,7 @@ import base64 import binascii +import codecs import datetime import hmac import json @@ -22,6 +23,7 @@ from edx_proctoring.backends.backend import ProctoringBackendProvider from edx_proctoring.exceptions import BackendProviderCannotRegisterAttempt, ProctoredExamSuspiciousLookup from edx_proctoring.statuses import SoftwareSecureReviewStatus +from edx_proctoring.utils import decode_and_decrypt log = logging.getLogger(__name__) @@ -390,3 +392,29 @@ def get_video_review_aes_key(self): Returns the aes key used to encrypt the video review url """ return self.video_review_aes_key + + def get_instructor_url( + self, course_id, user, exam_id=None, attempt_id=None, + show_configuration_dashboard=False, encrypted_video_review_url=None + ): + """ + Returns the url for video reviews + """ + # video_review_url is required for PSI backend + if not encrypted_video_review_url: + return None + + try: + aes_key = codecs.decode(self.video_review_aes_key, "hex") + decrypted_video_url = decode_and_decrypt(encrypted_video_review_url, aes_key).decode("utf-8") + + # reformat video url as per MST-871 findings + reformatted_url = decrypted_video_url.replace('DirectLink-Generic', 'DirectLink-HTML5') + return reformatted_url + except Exception as err: # pylint: disable=broad-except + log.exception( + 'Could not decrypt video url for attempt_id=%(attempt_id)s ' + 'due to the following error: %(error_string)s', + {'attempt_id': attempt_id, 'error_string': str(err)} + ) + return None diff --git a/edx_proctoring/backends/tests/test_backend.py b/edx_proctoring/backends/tests/test_backend.py index 0193975afbd..03ecab39271 100644 --- a/edx_proctoring/backends/tests/test_backend.py +++ b/edx_proctoring/backends/tests/test_backend.py @@ -89,7 +89,10 @@ def on_exam_saved(self, exam): return exam.get('external_id', None) or 'externalid' # pylint: disable=unused-argument - def get_instructor_url(self, course_id, user, exam_id=None, attempt_id=None, show_configuration_dashboard=False): + def get_instructor_url( + self, course_id, user, exam_id=None, attempt_id=None, + show_configuration_dashboard=False, encrypted_video_review_url=None + ): "Return a fake instructor url" url = f'/instructor/{course_id}/' if exam_id: diff --git a/edx_proctoring/constants.py b/edx_proctoring/constants.py index cec9c49d042..44706a2ba69 100644 --- a/edx_proctoring/constants.py +++ b/edx_proctoring/constants.py @@ -72,6 +72,8 @@ ONBOARDING_PROFILE_INSTRUCTOR_DASHBOARD_API = 'edx_proctoring.onboarding_profile_instructor_dashboard_api' +REDS_API_REDIRECT = 'edx_proctoring.reds_api_redirect' + CONTENT_VIEWABLE_PAST_DUE_DATE = getattr(settings, 'PROCTORED_EXAM_VIEWABLE_PAST_DUE', False) TIME_MULTIPLIER = 'time_multiplier' diff --git a/edx_proctoring/tests/test_views.py b/edx_proctoring/tests/test_views.py index eb5db358b09..c8989ef5359 100644 --- a/edx_proctoring/tests/test_views.py +++ b/edx_proctoring/tests/test_views.py @@ -5479,6 +5479,7 @@ def setUp(self): self.client.login_user(self.user) self.course_id = 'a/b/c' + set_runtime_service('credit', MockCreditService()) set_runtime_service('instructor', MockInstructorService(is_user_course_staff=True)) def test_launch_for_course(self): @@ -5654,6 +5655,182 @@ def test_multiple_exams_returns_correct_dashboard(self, exam_1_is_proctored, exa else: self.assertRedirects(response, expected_url, fetch_redirect_response=False) + @patch('edx_proctoring.views.waffle.switch_is_active') + @ddt.data( + (True, False), + (True, True), + (False, False), + (False, True), + ) + @ddt.unpack + def test_psi_instructor_dashboard_url(self, include_attempt_id, is_switch_active, mock_switch): + """ + Test that instructor dashboard redirects correctly for psi software secure backend + """ + mock_switch.return_value = is_switch_active + # set up exam + exam = ProctoredExam.objects.create( + course_id=self.course_id, + content_id='test_content', + exam_name='Test Exam', + external_id='123aXqe3', + time_limit_mins=90, + is_active=True, + is_proctored=True, + backend='software_secure', + ) + + # create exam attempt + with HTTMock(mock_response_content): + attempt_id = create_exam_attempt(exam.id, self.second_user.id, True) + + attempt = get_exam_attempt_by_id(attempt_id) + self.assertIsNotNone(attempt['external_id']) + + # create review for attempt + test_payload = create_test_review_payload( + attempt_code=attempt['attempt_code'], + external_id=attempt['external_id'].upper(), + review_status='Clean' + ) + response = self.client.post( + reverse('edx_proctoring:anonymous.proctoring_review_callback'), + data=test_payload, + content_type='foo' + ) + self.assertEqual(response.status_code, 200) + + # call dashboard exam url with attempt id + dashboard_url = reverse('edx_proctoring:instructor_dashboard_exam', + kwargs={'course_id': self.course_id, 'exam_id': exam.id}) + query_params = {'attempt': attempt['external_id']} if include_attempt_id else {} + response = self.client.get(dashboard_url, query_params) + if not (include_attempt_id and is_switch_active): + self.assertEqual(response.status_code, 404) + else: + video_url = json.loads(test_payload)['videoReviewLink'] + expected_url = video_url.replace('DirectLink-Generic', 'DirectLink-HTML5') + self.assertRedirects(response, expected_url, fetch_redirect_response=False) + + @patch('edx_proctoring.views.waffle.switch_is_active') + def test_psi_instructor_dashboard_url_deleted_attempt(self, mock_switch): + """ + Test that instructor dashboard redirects correctly for psi software secure backend when an attempt is deleted + """ + mock_switch.return_value = True + # set up exam + exam = ProctoredExam.objects.create( + course_id=self.course_id, + content_id='test_content', + exam_name='Test Exam', + external_id='123aXqe3', + time_limit_mins=90, + is_active=True, + is_proctored=True, + backend='software_secure', + ) + + # create exam attempt + with HTTMock(mock_response_content): + attempt_id = create_exam_attempt(exam.id, self.second_user.id, True) + + attempt = get_exam_attempt_by_id(attempt_id) + external_id = attempt['external_id'] + + # remove the attempt + response = self.client.delete( + reverse('edx_proctoring:proctored_exam.attempt', args=[attempt_id]) + ) + self.assertEqual(response.status_code, 200) + + # call dashboard exam url with attempt id + dashboard_url = reverse('edx_proctoring:instructor_dashboard_exam', + kwargs={'course_id': self.course_id, 'exam_id': exam.id}) + response = self.client.get(dashboard_url, {'attempt': external_id}) + + self.assertEqual(response.status_code, 404) + + @patch('edx_proctoring.views.waffle.switch_is_active') + def test_psi_instructor_dashboard_url_no_review(self, mock_switch): + """ + Test that instructor dashboard redirects correctly for psi software secure backend when there + is no review for an attempt + """ + mock_switch.return_value = True + # set up exam + exam = ProctoredExam.objects.create( + course_id=self.course_id, + content_id='test_content', + exam_name='Test Exam', + external_id='123aXqe3', + time_limit_mins=90, + is_active=True, + is_proctored=True, + backend='software_secure', + ) + + # create exam attempt + with HTTMock(mock_response_content): + attempt_id = create_exam_attempt(exam.id, self.second_user.id, True) + + attempt = get_exam_attempt_by_id(attempt_id) + external_id = attempt['external_id'] + + # call dashboard exam url with attempt id + dashboard_url = reverse('edx_proctoring:instructor_dashboard_exam', + kwargs={'course_id': self.course_id, 'exam_id': exam.id}) + response = self.client.get(dashboard_url, {'attempt': external_id}) + + self.assertEqual(response.status_code, 404) + + @patch('edx_proctoring.backends.software_secure.decode_and_decrypt') + @patch('edx_proctoring.views.waffle.switch_is_active') + def test_psi_instructor_dashboard_url_decoding_error(self, mock_switch, mock_decode): + """ + Test that the instructor dashboard returns a 404 if there was an error decoding the video url + """ + mock_switch.return_value = True + mock_decode.side_effect = Exception() + # set up exam + exam = ProctoredExam.objects.create( + course_id=self.course_id, + content_id='test_content', + exam_name='Test Exam', + external_id='123aXqe3', + time_limit_mins=90, + is_active=True, + is_proctored=True, + backend='software_secure', + ) + + # create exam attempt + with HTTMock(mock_response_content): + attempt_id = create_exam_attempt(exam.id, self.second_user.id, True) + + attempt = get_exam_attempt_by_id(attempt_id) + self.assertIsNotNone(attempt['external_id']) + + # create review for attempt + test_payload = create_test_review_payload( + attempt_code=attempt['attempt_code'], + external_id=attempt['external_id'].upper(), + review_status='Clean' + ) + response = self.client.post( + reverse('edx_proctoring:anonymous.proctoring_review_callback'), + data=test_payload, + content_type='foo' + ) + self.assertEqual(response.status_code, 200) + + # call dashboard exam url with attempt id + dashboard_url = reverse('edx_proctoring:instructor_dashboard_exam', + kwargs={'course_id': self.course_id, 'exam_id': exam.id}) + query_params = {'attempt': attempt['external_id']} + response = self.client.get(dashboard_url, query_params) + + self.assertEqual(response.status_code, 404) + class TestBackendUserDeletion(LoggedInTestCase): """ diff --git a/edx_proctoring/views.py b/edx_proctoring/views.py index 4c53bafa859..5f2ba0daea8 100644 --- a/edx_proctoring/views.py +++ b/edx_proctoring/views.py @@ -64,7 +64,8 @@ from edx_proctoring.constants import ( ONBOARDING_PROFILE_API, ONBOARDING_PROFILE_INSTRUCTOR_DASHBOARD_API, - PING_FAILURE_PASSTHROUGH_TEMPLATE + PING_FAILURE_PASSTHROUGH_TEMPLATE, + REDS_API_REDIRECT ) from edx_proctoring.exceptions import ( AllowanceValueNotAllowedException, @@ -1998,12 +1999,20 @@ def get(self, request, course_id, exam_id=None): 'email': request.user.email } + encrypted_video_review_url = None + if waffle.switch_is_active(REDS_API_REDIRECT) and attempt_id: + attempt = get_exam_attempt_by_external_id(attempt_id) + if attempt: + reviews = ProctoredExamSoftwareSecureReview.objects.filter(attempt_code=attempt['attempt_code']) + encrypted_video_review_url = reviews[0].encrypted_video_url if reviews else None + url = backend.get_instructor_url( exam['course_id'], user, exam_id=ext_exam_id, attempt_id=attempt_id, - show_configuration_dashboard=show_configuration_dashboard + show_configuration_dashboard=show_configuration_dashboard, + encrypted_video_review_url=encrypted_video_review_url ) if not url: return Response( diff --git a/package.json b/package.json index 2e96a2e000a..4ed8acee3f0 100644 --- a/package.json +++ b/package.json @@ -1,7 +1,7 @@ { "name": "@edx/edx-proctoring", "//": "Note that the version format is slightly different than that of the Python version when using prereleases.", - "version": "4.1.1", + "version": "4.1.2", "main": "edx_proctoring/static/index.js", "scripts": { "test": "gulp test"