From 6ef85e5922d908601b67b5ce942b623989cfd55e Mon Sep 17 00:00:00 2001 From: Alie Langston Date: Fri, 4 Jun 2021 16:31:46 -0400 Subject: [PATCH 1/2] fix: exam content will not be viewable if due date has passed Previously, if a learner had submitted their proctored exam and if the exam due date had passed, they were able to acknowledge their status and view the exam content. This is no longer wanted and poses an integrity threat. Students will not be able to acknowledge their status after this change, and exam content will only be viewable if the django setting PROCTORED_EXAM_VIEWABLE_PAST_DUE is set to True. --- CHANGELOG.rst | 5 ++ edx_proctoring/__init__.py | 2 +- edx_proctoring/api.py | 19 +++--- edx_proctoring/constants.py | 2 + .../proctored_exam/visit_exam_content.html | 2 +- edx_proctoring/tests/test_student_view.py | 59 +++++++++++++++++++ edx_proctoring/tests/test_views.py | 22 +++++++ edx_proctoring/views.py | 7 +++ package.json | 2 +- test_settings.py | 2 + 10 files changed, 110 insertions(+), 12 deletions(-) diff --git a/CHANGELOG.rst b/CHANGELOG.rst index d4a48c02710..d3ec30da47f 100644 --- a/CHANGELOG.rst +++ b/CHANGELOG.rst @@ -14,6 +14,11 @@ Change Log Unreleased ~~~~~~~~~~ +[3.13.0] - 2021-06-07 +~~~~~~~~~~~~~~~~~~~~~ +* If the Django setting `PROCTORED_EXAM_VIEWABLE_PAST_DUE` is false, exam content will not be viewable past + an exam's due date, even if a learner has acknowledged their status. + [3.12.0] - 2021-06-04 ~~~~~~~~~~~~~~~~~~~~~ * If the `is_integrity_signature_enabled` waffle flag is turned on, do not render the ID verification diff --git a/edx_proctoring/__init__.py b/edx_proctoring/__init__.py index 6f0091677b7..c65f7cfe7e3 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__ = '3.12.0' +__version__ = '3.13.0' default_app_config = 'edx_proctoring.apps.EdxProctoringConfig' # pylint: disable=invalid-name diff --git a/edx_proctoring/api.py b/edx_proctoring/api.py index 7f806c4df91..89ff0268a54 100644 --- a/edx_proctoring/api.py +++ b/edx_proctoring/api.py @@ -2281,7 +2281,8 @@ def _get_proctored_exam_context(exam, attempt, user_id, course_id, is_practice_e 'integration_specific_email': get_integration_specific_email(provider), 'exam_display_name': exam['exam_name'], 'reset_link': password_url, - 'ping_interval': provider.ping_interval + 'ping_interval': provider.ping_interval, + 'can_view_content_past_due': constants.CONTENT_VIEWABLE_PAST_DUE_DATE, } if attempt: context['exam_code'] = attempt['attempt_code'] @@ -2507,32 +2508,32 @@ def _get_proctored_exam_view(exam, context, exam_id, user_id, course_id): elif attempt_status == ProctoredExamStudentAttemptStatus.timed_out: raise NotImplementedError('There is no defined rendering for ProctoredExamStudentAttemptStatus.timed_out!') elif attempt_status == ProctoredExamStudentAttemptStatus.submitted: - student_view_template = None if _was_review_status_acknowledged( + student_view_template = None if (_was_review_status_acknowledged( attempt['is_status_acknowledged'], exam - ) else 'proctored_exam/submitted.html' + ) and constants.CONTENT_VIEWABLE_PAST_DUE_DATE) else 'proctored_exam/submitted.html' elif attempt_status == ProctoredExamStudentAttemptStatus.second_review_required: # the student should still see a 'submitted' # rendering even if the review needs a 2nd review - student_view_template = None if _was_review_status_acknowledged( + student_view_template = None if (_was_review_status_acknowledged( attempt['is_status_acknowledged'], exam - ) else 'proctored_exam/submitted.html' + ) and constants.CONTENT_VIEWABLE_PAST_DUE_DATE) else 'proctored_exam/submitted.html' elif attempt_status == ProctoredExamStudentAttemptStatus.verified: - student_view_template = None if _was_review_status_acknowledged( + student_view_template = None if (_was_review_status_acknowledged( attempt['is_status_acknowledged'], exam - ) else 'proctored_exam/verified.html' + ) and constants.CONTENT_VIEWABLE_PAST_DUE_DATE) else 'proctored_exam/verified.html' has_context_updated = verify_and_add_wait_deadline(context, exam, user_id) # The edge case where student has already acknowledged the result # but the course team changed the grace period if has_context_updated and not student_view_template: student_view_template = 'proctored_exam/verified.html' elif attempt_status == ProctoredExamStudentAttemptStatus.rejected: - student_view_template = None if _was_review_status_acknowledged( + student_view_template = None if (_was_review_status_acknowledged( attempt['is_status_acknowledged'], exam - ) else 'proctored_exam/rejected.html' + ) and constants.CONTENT_VIEWABLE_PAST_DUE_DATE) else 'proctored_exam/rejected.html' elif attempt_status == ProctoredExamStudentAttemptStatus.ready_to_submit: student_view_template = 'proctored_exam/ready_to_submit.html' elif attempt_status in ProctoredExamStudentAttemptStatus.onboarding_errors: diff --git a/edx_proctoring/constants.py b/edx_proctoring/constants.py index 685f2840511..3834afe4fe6 100644 --- a/edx_proctoring/constants.py +++ b/edx_proctoring/constants.py @@ -69,3 +69,5 @@ PING_FAILURE_PASSTHROUGH_TEMPLATE = 'edx_proctoring.{}_ping_failure_passthrough' ONBOARDING_PROFILE_API = 'edx_proctoring.onboarding_profile_api' + +CONTENT_VIEWABLE_PAST_DUE_DATE = getattr(settings, 'PROCTORED_EXAM_VIEWABLE_PAST_DUE', False) diff --git a/edx_proctoring/templates/proctored_exam/visit_exam_content.html b/edx_proctoring/templates/proctored_exam/visit_exam_content.html index 50615f2773a..e03c0183155 100644 --- a/edx_proctoring/templates/proctored_exam/visit_exam_content.html +++ b/edx_proctoring/templates/proctored_exam/visit_exam_content.html @@ -1,5 +1,5 @@ {% load i18n %} -{% if has_due_date_passed %} +{% if has_due_date_passed and can_view_content_past_due %}

{% blocktrans %} diff --git a/edx_proctoring/tests/test_student_view.py b/edx_proctoring/tests/test_student_view.py index c267ea8ae15..c98eecce0ae 100644 --- a/edx_proctoring/tests/test_student_view.py +++ b/edx_proctoring/tests/test_student_view.py @@ -14,6 +14,7 @@ from freezegun import freeze_time from mock import MagicMock, patch +from django.test.utils import override_settings from django.urls import reverse from edx_proctoring.api import ( @@ -81,6 +82,7 @@ def setUp(self): self.timed_footer_msg = 'Can I request additional time to complete my exam?' self.wait_deadline_msg = "The result will be visible after" self.inactive_account_msg = "You have not activated your account" + self.review_exam_msg = "To view your exam questions and responses" def _render_exam(self, content_id, context_overrides=None): """ @@ -682,6 +684,7 @@ def test_get_studentview_submitted_timed_exam_with_grace_period(self, is_proctor else: self.assertNotIn(self.wait_deadline_msg, rendered_response) + @patch("edx_proctoring.api.constants.CONTENT_VIEWABLE_PAST_DUE_DATE", True) def test_get_studentview_acknowledged_proctored_exam_with_grace_period(self): """ Verify the student view for an acknowledge proctored exam with an active @@ -861,6 +864,58 @@ def test_get_studentview_submitted_status(self): rendered_response = self.render_proctored_exam() self.assertIn(self.proctored_exam_submitted_msg, rendered_response) + @override_settings(PROCTORED_EXAM_VIEWABLE_PAST_DUE=False) + @ddt.data( + (ProctoredExamStudentAttemptStatus.submitted, True), + (ProctoredExamStudentAttemptStatus.submitted, False), + (ProctoredExamStudentAttemptStatus.second_review_required, True), + (ProctoredExamStudentAttemptStatus.second_review_required, False), + (ProctoredExamStudentAttemptStatus.rejected, True), + (ProctoredExamStudentAttemptStatus.rejected, False), + (ProctoredExamStudentAttemptStatus.verified, True), + (ProctoredExamStudentAttemptStatus.verified, False) + ) + @ddt.unpack + def test_get_studentview_without_viewable_content(self, status, status_acknowledged): + """ + Test for get_student_view proctored exam which has been submitted + but exam content is not viewable if the due date has passed + """ + due_date = datetime.now(pytz.UTC) + timedelta(minutes=40) + exam_id = self._create_exam_with_due_time( + is_proctored=True, due_date=due_date + ) + + exam_attempt = ProctoredExamStudentAttempt.objects.create( + proctored_exam_id=exam_id, + user=self.user, + allowed_time_limit_mins=30, + taking_as_proctored=True, + external_id='fdage332', + status=status, + ) + + exam_attempt.is_status_acknowledged = status_acknowledged + exam_attempt.save() + + # due date is after 10 minutes + reset_time = datetime.now(pytz.UTC) + timedelta(minutes=60) + with freeze_time(reset_time): + rendered_response = get_student_view( + user_id=self.user.id, + course_id=self.course_id, + content_id=self.content_id_for_exam_with_due_date, + context={ + 'is_proctored': True, + 'display_name': 'Test Exam', + 'default_time_limit_mins': 30, + 'due_date': due_date + } + ) + self.assertIsNotNone(rendered_response) + self.assertNotIn(self.review_exam_msg, rendered_response) + + @patch("edx_proctoring.api.constants.CONTENT_VIEWABLE_PAST_DUE_DATE", True) @ddt.data( 60, 20, @@ -906,6 +961,9 @@ def test_get_studentview_submitted_status_with_duedate_status_acknowledged(self, } ) self.assertIn(self.proctored_exam_submitted_msg, rendered_response) + if due_date_passed: + self.assertIn(self.review_exam_msg, rendered_response) + exam_attempt.is_status_acknowledged = True exam_attempt.save() @@ -925,6 +983,7 @@ def test_get_studentview_submitted_status_with_duedate_status_acknowledged(self, else: self.assertIsNotNone(rendered_response) + @patch("edx_proctoring.api.constants.CONTENT_VIEWABLE_PAST_DUE_DATE", True) @patch('edx_when.api.get_date_for_block') def test_get_studentview_submitted_personalize_scheduled_duedate_status_acknowledged(self, get_date_for_block_mock): """ diff --git a/edx_proctoring/tests/test_views.py b/edx_proctoring/tests/test_views.py index 8479309ef28..582950f78ae 100644 --- a/edx_proctoring/tests/test_views.py +++ b/edx_proctoring/tests/test_views.py @@ -16,6 +16,7 @@ from django.contrib.auth import get_user_model from django.test.client import Client +from django.test.utils import override_settings from django.urls import NoReverseMatch, reverse from django.utils import timezone @@ -3541,6 +3542,25 @@ def _create_proctored_exam_attempt_with_duedate(self, due_date=datetime.now(pytz status=ProctoredExamStudentAttemptStatus.started ) + def test_attempt_review_status_callback_non_reviewable(self): + """ + Test the ProctoredExamAttemptReviewStatus view + """ + attempt = self._create_proctored_exam_attempt_with_duedate( + due_date=datetime.now(pytz.UTC) + timedelta(minutes=40) + ) + + response = self.client.put( + reverse( + 'edx_proctoring:proctored_exam.attempt.review_status', + args=[attempt.id] + ), + {}, + content_type='application/json' + ) + self.assertEqual(response.status_code, 404) + + @override_settings(PROCTORED_EXAM_VIEWABLE_PAST_DUE=True) def test_attempt_review_status_callback(self): """ Test the ProctoredExamAttemptReviewStatus view @@ -3559,6 +3579,7 @@ def test_attempt_review_status_callback(self): ) self.assertEqual(response.status_code, 200) + @override_settings(PROCTORED_EXAM_VIEWABLE_PAST_DUE=True) def test_attempt_review_status_callback_with_doesnotexit_exception(self): """ Test the ProctoredExamAttemptReviewStatus view with does not exit exception @@ -3578,6 +3599,7 @@ def test_attempt_review_status_callback_with_doesnotexit_exception(self): self.assertEqual(response.status_code, 400) self.assertRaises(StudentExamAttemptDoesNotExistsException) + @override_settings(PROCTORED_EXAM_VIEWABLE_PAST_DUE=True) def test_attempt_review_status_callback_with_permission_exception(self): """ Test the ProctoredExamAttemptReviewStatus view with permission exception diff --git a/edx_proctoring/views.py b/edx_proctoring/views.py index 38d69f0876b..8e84837a41d 100644 --- a/edx_proctoring/views.py +++ b/edx_proctoring/views.py @@ -17,6 +17,7 @@ from rest_framework.response import Response from rest_framework.views import APIView +from django.conf import settings from django.contrib.auth import get_user_model from django.core.exceptions import ObjectDoesNotExist from django.core.paginator import Paginator @@ -1360,6 +1361,12 @@ def put(self, request, attempt_id): # pylint: disable=unused-argument """ Update the is_status_acknowledged flag for the specific attempt """ + if not getattr(settings, 'PROCTORED_EXAM_VIEWABLE_PAST_DUE', False): + return Response( + status=404, + data={'detail': _('Cannot update attempt review status')} + ) + attempt = get_exam_attempt_by_id(attempt_id) # make sure the the attempt belongs to the calling user_id diff --git a/package.json b/package.json index eb7222fdf7c..c7976bdf215 100644 --- a/package.json +++ b/package.json @@ -2,7 +2,7 @@ "name": "@edx/edx-proctoring", "//": "Be sure to update the version number in edx_proctoring/__init__.py", "//": "Note that the version format is slightly different than that of the Python version when using prereleases.", - "version": "3.12.0", + "version": "3.13.0", "main": "edx_proctoring/static/index.js", "scripts": { "test": "gulp test" diff --git a/test_settings.py b/test_settings.py index b7d7084ed0b..41d32040a45 100644 --- a/test_settings.py +++ b/test_settings.py @@ -125,6 +125,8 @@ CONTACT_EMAIL = 'info@edx.org' TECH_SUPPORT_EMAIL = 'technical@example.com' +PROCTORED_EXAM_VIEWABLE_PAST_DUE = False + ########## TEMPLATE CONFIGURATION TEMPLATES = [{ 'BACKEND': 'django.template.backends.django.DjangoTemplates', From e74f2594a21a2d3c17731eeb75c64ae03813305e Mon Sep 17 00:00:00 2001 From: Alie Langston Date: Mon, 7 Jun 2021 10:22:42 -0400 Subject: [PATCH 2/2] temporarily disable codecov fail --- .github/workflows/ci.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 37bd2c98249..ca046870c94 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -61,4 +61,4 @@ jobs: uses: codecov/codecov-action@v1 with: flags: unittests - fail_ci_if_error: true + fail_ci_if_error: false