From c79afa5ed15c3f7ddd0021ca18d94802575fe239 Mon Sep 17 00:00:00 2001 From: bmedx Date: Fri, 11 May 2018 11:13:50 -0400 Subject: [PATCH 1/4] Fix PLAT-2123- Bug with retiring user's original_email being hashed --- openedx/core/djangoapps/user_api/accounts/views.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/openedx/core/djangoapps/user_api/accounts/views.py b/openedx/core/djangoapps/user_api/accounts/views.py index eb26d21f26..1bc3485b60 100644 --- a/openedx/core/djangoapps/user_api/accounts/views.py +++ b/openedx/core/djangoapps/user_api/accounts/views.py @@ -406,6 +406,7 @@ class DeactivateLogoutView(APIView): if verify_user_password_response.status_code != status.HTTP_204_NO_CONTENT: return verify_user_password_response with transaction.atomic(): + UserRetirementStatus.create_retirement(request.user) # Unlink LMS social auth accounts UserSocialAuth.objects.filter(user_id=request.user.id).delete() # Change LMS password & email @@ -416,7 +417,6 @@ class DeactivateLogoutView(APIView): # Remove the activation keys sent by email to the user for account activation. Registration.objects.filter(user=request.user).delete() # Add user to retirement queue. - UserRetirementStatus.create_retirement(request.user) # Log the user out. logout(request) return Response(status=status.HTTP_204_NO_CONTENT) From 06e087b99dc62692fc22287d1eb1255955457c55 Mon Sep 17 00:00:00 2001 From: Simon Chen Date: Fri, 11 May 2018 11:05:42 -0400 Subject: [PATCH 2/4] Create the retire_user method on ApiAccessRequest model --- openedx/core/djangoapps/api_admin/models.py | 24 +++++++++++++++++++ .../djangoapps/api_admin/tests/test_models.py | 13 ++++++++++ 2 files changed, 37 insertions(+) diff --git a/openedx/core/djangoapps/api_admin/models.py b/openedx/core/djangoapps/api_admin/models.py index 3977884ea8..94666e345d 100644 --- a/openedx/core/djangoapps/api_admin/models.py +++ b/openedx/core/djangoapps/api_admin/models.py @@ -80,6 +80,30 @@ class ApiAccessRequest(TimeStampedModel): except cls.DoesNotExist: return None + @classmethod + def retire_user(cls, user): + """ + Retires the user's API acccess request table for GDPR + + Arguments: + user (User): The user linked to the data to retire in the model. + + Returns: + True: If the user has a linked data in the model and retirement is successful + False: user has no linked data in the model. + """ + try: + retire_target = cls.objects.get(user=user) + except cls.DoesNotExist: + return False + else: + retire_target.website = '' + retire_target.company_address = '' + retire_target.company_name = '' + retire_target.reason = '' + retire_target.save() + return True + def approve(self): """Approve this request.""" log.info('Approving API request from user [%s].', self.user.id) diff --git a/openedx/core/djangoapps/api_admin/tests/test_models.py b/openedx/core/djangoapps/api_admin/tests/test_models.py index 8db4abf204..8c39620f00 100644 --- a/openedx/core/djangoapps/api_admin/tests/test_models.py +++ b/openedx/core/djangoapps/api_admin/tests/test_models.py @@ -64,6 +64,19 @@ class ApiAccessRequestTests(TestCase): self.assertIn(self.request.website, request_unicode) # pylint: disable=no-member self.assertIn(self.request.status, request_unicode) + def test_retire_user_success(self): + retire_result = self.request.retire_user(self.user) + self.assertTrue(retire_result) + self.assertEqual(self.request.company_address, '') + self.assertEqual(self.request.company_name, '') + self.assertEqual(self.request.website, '') + self.assertEqual(self.request.reason, '') + + def test_retire_user_do_not_exist(self): + user2 = UserFactory() + retire_result = self.request.retire_user(user2) + self.assertFalse(retire_result) + class ApiAccessConfigTests(TestCase): From 43d761496e0e482c9becfacbcb72c1c70454351e Mon Sep 17 00:00:00 2001 From: Sanford Student Date: Fri, 11 May 2018 11:29:17 -0400 Subject: [PATCH 3/4] EDUCAOTR-2870: move oauth token retirement to dea deactivation endpoint --- .../core/djangoapps/user_api/accounts/tests/test_views.py | 6 +++++- openedx/core/djangoapps/user_api/accounts/views.py | 4 ++++ 2 files changed, 9 insertions(+), 1 deletion(-) diff --git a/openedx/core/djangoapps/user_api/accounts/tests/test_views.py b/openedx/core/djangoapps/user_api/accounts/tests/test_views.py index 6054eca9fd..fca7eaf31f 100644 --- a/openedx/core/djangoapps/user_api/accounts/tests/test_views.py +++ b/openedx/core/djangoapps/user_api/accounts/tests/test_views.py @@ -1112,7 +1112,8 @@ class TestDeactivateLogout(RetirementTestCase): def build_post(self, password): return {'password': password} - def test_user_can_deactivate_self(self): + @mock.patch('openedx.core.djangolib.oauth2_retirement_utils') + def test_user_can_deactivate_self(self, retirement_utils_mock): """ Verify a user calling the deactivation endpoint logs out the user, deletes all their SSO tokens, and creates a user retirement row. @@ -1128,6 +1129,9 @@ class TestDeactivateLogout(RetirementTestCase): self.assertEqual(list(UserSocialAuth.objects.filter(user=self.test_user)), []) self.assertEqual(list(Registration.objects.filter(user=self.test_user)), []) self.assertEqual(len(UserRetirementStatus.objects.filter(user_id=self.test_user.id)), 1) + # these retirement utils are tested elsewhere; just make sure we called them + retirement_utils_mock.retire_dop_oauth2_models.assertCalledWith(self.test_user) + retirement_utils_mock.retire_dot_oauth2_models.assertCalledWith(self.test_user) # make sure the user cannot log in self.assertFalse(self.client.login(username=self.test_user.username, password=self.test_password)) diff --git a/openedx/core/djangoapps/user_api/accounts/views.py b/openedx/core/djangoapps/user_api/accounts/views.py index 1bc3485b60..b87bf27527 100644 --- a/openedx/core/djangoapps/user_api/accounts/views.py +++ b/openedx/core/djangoapps/user_api/accounts/views.py @@ -32,6 +32,7 @@ from openedx.core.djangoapps.course_groups.models import UnregisteredLearnerCoho from openedx.core.djangoapps.profile_images.images import remove_profile_images from openedx.core.djangoapps.user_api.accounts.image_helpers import get_profile_image_names, set_has_profile_image from openedx.core.djangoapps.user_api.preferences.api import update_email_opt_in +from openedx.core.djangolib.oauth2_retirement_utils import retire_dop_oauth2_models, retire_dot_oauth2_models from openedx.core.lib.api.authentication import ( OAuth2AuthenticationAllowInactiveUser, SessionAuthenticationAllowInactiveUser @@ -417,6 +418,9 @@ class DeactivateLogoutView(APIView): # Remove the activation keys sent by email to the user for account activation. Registration.objects.filter(user=request.user).delete() # Add user to retirement queue. + # Delete OAuth tokens associated with the user. + retire_dop_oauth2_models(request.user) + retire_dot_oauth2_models(request.user) # Log the user out. logout(request) return Response(status=status.HTTP_204_NO_CONTENT) From eb23d32dcc19cab0994824768b3a214d808c29bd Mon Sep 17 00:00:00 2001 From: Alex Dusenbery Date: Fri, 11 May 2018 14:44:44 -0400 Subject: [PATCH 4/4] Call count() when you want a count. --- lms/djangoapps/grades/tasks.py | 2 +- lms/djangoapps/grades/tests/test_tasks.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/lms/djangoapps/grades/tasks.py b/lms/djangoapps/grades/tasks.py index 3c18d66e9f..f3bd35fe5e 100644 --- a/lms/djangoapps/grades/tasks.py +++ b/lms/djangoapps/grades/tasks.py @@ -140,7 +140,7 @@ def recalculate_course_and_subsection_grades_for_user(self, **kwargs): # pylint course_key = CourseKey.from_string(course_key_str) # Hotfix to address LEARNER-5123, to be removed later - visible_blocks_count = VisibleBlocks.objects.filter(course_id=course_key) + visible_blocks_count = VisibleBlocks.objects.filter(course_id=course_key).count() if visible_blocks_count > MAX_VISIBLE_BLOCKS_ALLOWED: message = '{} has too many VisibleBlocks to recalculate grades for {}' raise Exception(message.format(course_key_str, user_id)) diff --git a/lms/djangoapps/grades/tests/test_tasks.py b/lms/djangoapps/grades/tests/test_tasks.py index b859b14a40..109a119a60 100644 --- a/lms/djangoapps/grades/tests/test_tasks.py +++ b/lms/djangoapps/grades/tests/test_tasks.py @@ -486,7 +486,7 @@ class RecalculateGradesForUserTest(HasCourseWithProblemsMixin, ModuleStoreTestCa self.set_up_course() CourseEnrollment.enroll(self.user, self.course.id) self.original_max_visible_blocks_allowed = tasks.MAX_VISIBLE_BLOCKS_ALLOWED - tasks.MAX_VISIBLE_BLOCKS_ALLOWED = 1 + tasks.MAX_VISIBLE_BLOCKS_ALLOWED = -1 def tearDown(self): super(RecalculateGradesForUserTest, self).tearDown()