diff --git a/.gitignore b/.gitignore index 856a77917fb..6edb30eae87 100644 --- a/.gitignore +++ b/.gitignore @@ -37,6 +37,9 @@ tramp *.swp *.swo +# VS Code +.vscode + # Mac .DS_Store ._* diff --git a/README.rst b/README.rst index dec9287fdd5..1e88e25eb14 100644 --- a/README.rst +++ b/README.rst @@ -98,6 +98,14 @@ In your lms.auth.json file, please add the following *secure* information:: You will need to restart services after these configuration changes for them to take effect. +Debugging +------------ + +To debug with PDB, run ``pytest`` with the ``-n0`` flag. This restricts the number +of processes in a way that is compatible with ``pytest`` + + pytest -n0 [file-path] + License ------- diff --git a/edx_proctoring/__init__.py b/edx_proctoring/__init__.py index b97c5c91201..4c3e7b34076 100644 --- a/edx_proctoring/__init__.py +++ b/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__ = '2.3.0' +__version__ = '2.3.1' default_app_config = 'edx_proctoring.apps.EdxProctoringConfig' # pylint: disable=invalid-name diff --git a/edx_proctoring/models.py b/edx_proctoring/models.py index 116d15bd176..bc28de622a9 100644 --- a/edx_proctoring/models.py +++ b/edx_proctoring/models.py @@ -126,10 +126,7 @@ class ProctoredExamReviewPolicy(TimeStampedModel): """ This is how an instructor can set review policies for a proctored exam - .. pii: records who set a review policy in set_by_user - retirement to be implemented in https://openedx.atlassian.net/browse/EDUCATOR-4776 - .. pii_types: id - .. pii_retirement: to_be_implemented + .. no_pii: """ # who set this ProctoredExamReviewPolicy @@ -301,9 +298,8 @@ class ProctoredExamStudentAttempt(TimeStampedModel): Proctored Exam. .. pii: new attempts log the student's name and IP - retirement to be implemented in https://openedx.atlassian.net/browse/EDUCATOR-4776 .. pii_types: name, ip - .. pii_retirement: to_be_implemented + .. pii_retirement: local_api """ objects = ProctoredExamStudentAttemptManager() @@ -343,6 +339,7 @@ class ProctoredExamStudentAttempt(TimeStampedModel): # the proctoring software is_sample_attempt = models.BooleanField(default=False, verbose_name=ugettext_noop("Is Sample Attempt")) + # Note - this is currently unset student_name = models.CharField(max_length=255) # what review policy was this exam submitted under @@ -394,9 +391,8 @@ class ProctoredExamStudentAttemptHistory(TimeStampedModel): but will record (for audit history) all entries that have been updated. .. pii: new attempts log the student's name and IP - retirement to be implemented in https://openedx.atlassian.net/browse/EDUCATOR-4776 .. pii_types: name, ip - .. pii_retirement: to_be_implemented + .. pii_retirement: local_api """ user = models.ForeignKey(USER_MODEL, db_index=True, on_delete=models.CASCADE) @@ -431,6 +427,7 @@ class ProctoredExamStudentAttemptHistory(TimeStampedModel): # the proctoring software is_sample_attempt = models.BooleanField(default=False) + # Note - this is currently unset student_name = models.CharField(max_length=255) # what review policy was this exam submitted under @@ -512,9 +509,8 @@ class ProctoredExamStudentAllowance(TimeStampedModel): Information about allowing a student additional time on exam. .. pii: allowances have a free-form text field which may be identifiable - retirement to be implemented in https://openedx.atlassian.net/browse/EDUCATOR-4776 .. pii_types: other - .. pii_retirement: to_be_implemented + .. pii_retirement: local_api """ # DONT EDIT THE KEYS - THE FIRST VALUE OF THE TUPLE - AS ARE THEY ARE STORED IN THE DATABASE @@ -654,9 +650,8 @@ class ProctoredExamStudentAllowanceHistory(TimeStampedModel): but will record (for audit history) all entries that have been updated. .. pii: allowances have a free-form text field which may be identifiable - retirement to be implemented in https://openedx.atlassian.net/browse/EDUCATOR-4776 .. pii_types: other - .. pii_retirement: to_be_implemented + .. pii_retirement: local_api """ # what was the original id of the allowance diff --git a/edx_proctoring/tests/test_views.py b/edx_proctoring/tests/test_views.py index de8f65367ee..4673a37755d 100644 --- a/edx_proctoring/tests/test_views.py +++ b/edx_proctoring/tests/test_views.py @@ -21,6 +21,7 @@ from edx_proctoring.api import ( _calculate_allowed_mins, + add_allowance_for_user, create_exam, create_exam_attempt, get_backend_provider, @@ -34,7 +35,13 @@ ProctoredExamPermissionDenied, StudentExamAttemptDoesNotExistsException ) -from edx_proctoring.models import ProctoredExam, ProctoredExamStudentAllowance, ProctoredExamStudentAttempt +from edx_proctoring.models import ( + ProctoredExam, + ProctoredExamStudentAllowance, + ProctoredExamStudentAllowanceHistory, + ProctoredExamStudentAttempt, + ProctoredExamStudentAttemptHistory +) from edx_proctoring.runtime import get_runtime_service, set_runtime_service from edx_proctoring.serializers import ProctoredExamSerializer from edx_proctoring.statuses import ProctoredExamStudentAttemptStatus @@ -2847,3 +2854,114 @@ def test_no_access(self): response = self.client.post(deletion_url) assert response.status_code == 403 + + +class TestUserRetirement(LoggedInTestCase): + """ + Tests for deleting user PII for proctoring + """ + def setUp(self): + super(TestUserRetirement, self).setUp() + self.user.is_staff = True + self.user.save() + self.user_to_retire = User(username='tester2', email='tester2@test.com') + self.user_to_retire.save() + self.client.login_user(self.user) + self.deletion_url = reverse('edx_proctoring:user_retirement_api', kwargs={'user_id': self.user_to_retire.id}) + + def _create_proctored_exam(self): + """ Create a mock proctored exam with common values """ + return ProctoredExam.objects.create( + course_id='a/b/c', + content_id='test_content', + exam_name='Test Exam', + external_id='123aXqe3', + is_proctored=True, + is_active=True, + time_limit_mins=90, + backend='test' + ) + + def test_retire_no_access(self): + """ A user without retirement permissions should not be able to retire other users """ + self.client.login_user(self.user_to_retire) + deletion_url = reverse('edx_proctoring:user_retirement_api', kwargs={'user_id': self.user.id}) + + response = self.client.post(deletion_url) + assert response.status_code == 403 + + def test_retire_user_no_data(self): + """ + Attempting to retire an unknown user or user with no proctored attempts + returns 204 but does not carry out a retirment + """ + response = self.client.post(self.deletion_url) + + assert response.status_code == 204 + + def test_retire_user_exam_attempt(self): + """ Retiring a user should obfuscate PII for exam attempts and return a 204 status """ + # Create an exam attempt + proctored_exam = self._create_proctored_exam() + ProctoredExamStudentAttempt.objects.create( + proctored_exam=proctored_exam, + user=self.user_to_retire, + student_name='me', + last_poll_ipaddr='127.0.0.1' + ) + + # Run the retirement command + deletion_url = reverse('edx_proctoring:user_retirement_api', kwargs={'user_id': self.user_to_retire.id}) + response = self.client.post(deletion_url) + assert response.status_code == 204 + + retired_attempt = ProctoredExamStudentAttempt.objects.filter(user_id=self.user_to_retire.id).first() + assert retired_attempt.student_name == '' + assert retired_attempt.last_poll_ipaddr is None + + def test_retire_user_exam_attempt_history(self): + """ Retiring a user should obfuscate PII for exam attempt history and return a 204 status """ + # Create and archive an exam attempt so it appears in the history table + proctored_exam = self._create_proctored_exam() + ProctoredExamStudentAttemptHistory.objects.create( + proctored_exam=proctored_exam, + user=self.user_to_retire, + student_name='me', + last_poll_ipaddr='127.0.0.1' + ) + + # Run the retirement command + response = self.client.post(self.deletion_url) + assert response.status_code == 204 + + retired_attempt_history = ProctoredExamStudentAttemptHistory \ + .objects.filter(user_id=self.user_to_retire.id).first() + assert retired_attempt_history.student_name == '' + assert retired_attempt_history.last_poll_ipaddr is None + + def test_retire_user_allowances(self): + """ Retiring a user should delete their allowances and return a 204 """ + proctored_exam = self._create_proctored_exam() + add_allowance_for_user(proctored_exam.id, self.user_to_retire.id, 'a_key', 30) + + # Run the retirement command + response = self.client.post(self.deletion_url) + assert response.status_code == 204 + + retired_allowance = ProctoredExamStudentAllowance \ + .objects.filter(user=self.user_to_retire.id).first() + assert retired_allowance.value == '' + + def test_retire_user_allowances_history(self): + """ Retiring a user should delete their allowances and return a 204 """ + proctored_exam = self._create_proctored_exam() + add_allowance_for_user(proctored_exam.id, self.user_to_retire.id, 'a_key', 30) + add_allowance_for_user(proctored_exam.id, self.user_to_retire.id, 'a_key', 60) + + # Run the retirement command + response = self.client.post(self.deletion_url) + assert response.status_code == 204 + + retired_allowance_history = ProctoredExamStudentAllowanceHistory \ + .objects.filter(user=self.user_to_retire.id).first() + assert retired_allowance_history.value == '' diff --git a/edx_proctoring/urls.py b/edx_proctoring/urls.py index 6c0d6ce3723..14d88565c3a 100644 --- a/edx_proctoring/urls.py +++ b/edx_proctoring/urls.py @@ -95,6 +95,11 @@ views.BackendUserManagementAPI.as_view(), name='backend_user_deletion_api' ), + url( + r'edx_proctoring/v1/retire_user/(?P[\d]+)/$', + views.UserRetirement.as_view(), + name='user_retirement_api' + ), # Unauthenticated callbacks from SoftwareSecure. Note we use other # security token measures to protect data diff --git a/edx_proctoring/views.py b/edx_proctoring/views.py index bed2b3f23f7..ce0bfdcc237 100644 --- a/edx_proctoring/views.py +++ b/edx_proctoring/views.py @@ -58,7 +58,10 @@ ProctoredExam, ProctoredExamSoftwareSecureComment, ProctoredExamSoftwareSecureReview, - ProctoredExamStudentAttempt + ProctoredExamStudentAllowance, + ProctoredExamStudentAllowanceHistory, + ProctoredExamStudentAttempt, + ProctoredExamStudentAttemptHistory ) from edx_proctoring.runtime import get_runtime_service from edx_proctoring.serializers import ProctoredExamSerializer, ProctoredExamStudentAttemptSerializer @@ -1104,3 +1107,47 @@ def post(self, request, user_id): # pylint: disable=unused-argument code = 500 seen.add(backend_name) return Response(data=results, status=code) + + +class UserRetirement(AuthenticatedAPIView): + """ + Retire user personally-identifiable information (PII) for a user + """ + def _retire_exam_attempts_user_info(self, user_id): + """ Remove PII for exam attempts and exam history """ + attempts = ProctoredExamStudentAttempt.objects.filter(user_id=user_id) + if attempts: + for attempt in attempts: + attempt.student_name = '' + attempt.last_poll_ipaddr = None + attempt.save() + + attempts_history = ProctoredExamStudentAttemptHistory.objects.filter(user_id=user_id) + if attempts_history: + for attempt_history in attempts_history: + attempt_history.student_name = '' + attempt_history.last_poll_ipaddr = None + attempt_history.save() + + def _retire_user_allowances(self, user_id): + """ Clear user allowance values """ + allowances = ProctoredExamStudentAllowance.objects.filter(user=user_id) + for allowance in allowances: + allowance.value = '' + allowance.save() + + allowances_history = ProctoredExamStudentAllowanceHistory.objects.filter(user=user_id) + for allowance_history in allowances_history: + allowance_history.value = '' + allowance_history.save() + + def post(self, request, user_id): # pylint: disable=unused-argument + """ Obfuscates all PII for a given user_id """ + if not request.user.has_perm('accounts.can_retire_user'): + return Response(status=403) + code = 204 + + self._retire_exam_attempts_user_info(user_id) + self._retire_user_allowances(user_id) + + return Response(status=code) diff --git a/package.json b/package.json index a6c4e701b60..3afd4245e05 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": "2.3.0", + "version": "2.3.1", "main": "edx_proctoring/static/index.js", "repository": { "type": "git",