diff --git a/lms/envs/common.py b/lms/envs/common.py index f8347a0403..9cd25455c2 100644 --- a/lms/envs/common.py +++ b/lms/envs/common.py @@ -5241,3 +5241,6 @@ MFE_CONFIG_API_CACHE_TIMEOUT = 60 * 5 ######################## Settings for Outcome Surveys plugin ######################## OUTCOME_SURVEYS_EVENTS_ENABLED = True + +######################## Settings for cancel retirement in Support Tools ######################## +COOL_OFF_DAYS = 14 diff --git a/openedx/core/djangoapps/user_api/accounts/forms.py b/openedx/core/djangoapps/user_api/accounts/forms.py index 7583c1d8ee..771b6a7ccf 100644 --- a/openedx/core/djangoapps/user_api/accounts/forms.py +++ b/openedx/core/djangoapps/user_api/accounts/forms.py @@ -6,7 +6,7 @@ Django forms for accounts from django import forms from django.core.exceptions import ValidationError -from edx_django_utils.user import generate_password +from openedx.core.djangoapps.user_api.accounts.utils import handle_retirement_cancellation class RetirementQueueDeletionForm(forms.Form): @@ -34,12 +34,4 @@ class RetirementQueueDeletionForm(forms.Form): ) raise ValidationError('Retirement is in the wrong state!') - # Load the user record using the retired email address -and- change the email address back. - retirement.user.email = retirement.original_email - # Reset users password so they can request a password reset and log in again. - retirement.user.set_password(generate_password(length=25)) - retirement.user.save() - - # Delete the user retirement status record. - # No need to delete the accompanying "permanent" retirement request record - it gets done via Django signal. - retirement.delete() + handle_retirement_cancellation(retirement) diff --git a/openedx/core/djangoapps/user_api/accounts/permissions.py b/openedx/core/djangoapps/user_api/accounts/permissions.py index 8c99911f32..c3169c76b7 100644 --- a/openedx/core/djangoapps/user_api/accounts/permissions.py +++ b/openedx/core/djangoapps/user_api/accounts/permissions.py @@ -16,6 +16,18 @@ class CanDeactivateUser(permissions.BasePermission): return request.user.has_perm('student.can_deactivate_users') +class CanCancelUserRetirement(permissions.BasePermission): + """ + Grants access to cancel retirement if the requesting user is a superuser, + or has the explicit permission to cancel retirement of a User account. + """ + + def has_permission(self, request, view): + return request.user.is_superuser or ( + request.user.is_staff and request.user.has_perm('user_api.change_userretirementstatus') + ) + + class CanRetireUser(permissions.BasePermission): """ Grants access to the various retirement API endpoints if the requesting user is diff --git a/openedx/core/djangoapps/user_api/accounts/tests/factories.py b/openedx/core/djangoapps/user_api/accounts/tests/factories.py new file mode 100644 index 0000000000..a3a3bad92b --- /dev/null +++ b/openedx/core/djangoapps/user_api/accounts/tests/factories.py @@ -0,0 +1,29 @@ +""" +Model Factories for testing purposes of User Accounts +""" +from factory import SubFactory +from factory.django import DjangoModelFactory +from openedx.core.djangoapps.user_api.models import UserRetirementStatus, RetirementState +from common.djangoapps.student.tests.factories import UserFactory + + +class RetirementStateFactory(DjangoModelFactory): + """ + Simple factory class for storing retirement state. + """ + + class Meta: + model = RetirementState + + +class UserRetirementStatusFactory(DjangoModelFactory): + """ + Simple factory class for storing user retirement status. + """ + + class Meta: + model = UserRetirementStatus + + user = SubFactory(UserFactory) + current_state = SubFactory(RetirementStateFactory) + last_state = SubFactory(RetirementStateFactory) diff --git a/openedx/core/djangoapps/user_api/accounts/tests/test_permissions.py b/openedx/core/djangoapps/user_api/accounts/tests/test_permissions.py index 9697aa50be..d02f1f4459 100644 --- a/openedx/core/djangoapps/user_api/accounts/tests/test_permissions.py +++ b/openedx/core/djangoapps/user_api/accounts/tests/test_permissions.py @@ -5,8 +5,18 @@ Tests for User deactivation API permissions from django.test import RequestFactory, TestCase -from openedx.core.djangoapps.user_api.accounts.permissions import CanDeactivateUser, CanRetireUser -from common.djangoapps.student.tests.factories import ContentTypeFactory, PermissionFactory, SuperuserFactory, UserFactory # lint-amnesty, pylint: disable=line-too-long +from common.djangoapps.student.tests.factories import ( # lint-amnesty, pylint: disable=line-too-long + AdminFactory, + ContentTypeFactory, + PermissionFactory, + SuperuserFactory, + UserFactory +) +from openedx.core.djangoapps.user_api.accounts.permissions import ( + CanCancelUserRetirement, + CanDeactivateUser, + CanRetireUser +) class CanDeactivateUserTest(TestCase): @@ -67,3 +77,36 @@ class CanRetireUserTest(TestCase): self.request.user = UserFactory() result = CanRetireUser().has_permission(self.request, None) assert not result + + +class CanCancelUserRetirementTest(TestCase): + """ Tests for cancel user retirement API permissions """ + + def setUp(self): + super().setUp() + self.request = RequestFactory().get('/test/url') + + def test_permission_superuser(self): + self.request.user = SuperuserFactory() + + can_cancel_retirement = CanCancelUserRetirement().has_permission(self.request, None) + assert can_cancel_retirement is True + + def test_permission_user_granted_permission(self): + user = AdminFactory() + permission = PermissionFactory( + codename='change_userretirementstatus', + content_type=ContentTypeFactory( + app_label='user_api' + ) + ) + user.user_permissions.add(permission) + self.request.user = user + + can_cancel_retirement = CanCancelUserRetirement().has_permission(self.request, None) + assert can_cancel_retirement is True + + def test_api_permission_user_without_permission(self): + self.request.user = UserFactory() + can_cancel_retirement = CanCancelUserRetirement().has_permission(self.request, None) + assert can_cancel_retirement is False 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 ff078119bd..63cb59cace 100644 --- a/openedx/core/djangoapps/user_api/accounts/tests/test_views.py +++ b/openedx/core/djangoapps/user_api/accounts/tests/test_views.py @@ -20,11 +20,20 @@ from rest_framework.test import APIClient, APITestCase from common.djangoapps.student.models import PendingEmailChange, UserProfile from common.djangoapps.student.models_api import do_name_change_request, get_pending_name_change -from common.djangoapps.student.tests.factories import TEST_PASSWORD, RegistrationFactory, UserFactory -from openedx.core.djangoapps.user_api.accounts import RETIRED_EMAIL_MSG +from common.djangoapps.student.tests.factories import ( + TEST_PASSWORD, + ContentTypeFactory, + PermissionFactory, + RegistrationFactory, + UserFactory +) from openedx.core.djangoapps.oauth_dispatch.jwt import create_jwt_for_user from openedx.core.djangoapps.user_api.accounts import ACCOUNT_VISIBILITY_PREF_KEY -from openedx.core.djangoapps.user_api.models import UserPreference +from openedx.core.djangoapps.user_api.accounts.tests.factories import ( + RetirementStateFactory, + UserRetirementStatusFactory +) +from openedx.core.djangoapps.user_api.models import UserPreference, UserRetirementStatus from openedx.core.djangoapps.user_api.preferences.api import set_user_preference from openedx.core.djangoapps.waffle_utils.testutils import WAFFLE_TABLES from openedx.core.djangolib.testing.utils import CacheIsolationTestCase, FilteredQueryCountMixin, skip_unless_lms @@ -245,6 +254,96 @@ class TestOwnUsernameAPI(FilteredQueryCountMixin, CacheIsolationTestCase, UserAP self._verify_get_own_username(12, expected_status=401) +@skip_unless_lms +class TestCancelAccountRetirementStatusView(UserAPITestCase): + """ + Unit tests for CancelAccountRetirementStatusView + """ + def setUp(self): + super().setUp() + permission = PermissionFactory( + codename='change_userretirementstatus', + content_type=ContentTypeFactory( + app_label='user_api' + ) + ) + self.staff_user.user_permissions.add(permission) + self.client = self.login_client('staff_client', 'staff_user') + + def test_cancel_retirement_bad_request(self): + """ + Test that cancel_retirement throws 400 if no retirement_id is given. + """ + client = self.login_client('staff_client', 'staff_user') + url = reverse("cancel_account_retirement") + response = client.post(url) + assert response.status_code == status.HTTP_400_BAD_REQUEST + assert response.data == {'message': 'retirement_id must be specified.'} + + def test_cancel_retirement_does_not_exist(self): + """ + Test that cancel_retirement throws 400 if no retirement status exists. + """ + client = self.login_client('staff_client', 'staff_user') + url = reverse("cancel_account_retirement") + response = client.post(url, data={'retirement_id': 1}) + assert response.status_code == status.HTTP_400_BAD_REQUEST + assert response.data == {"message": 'Retirement does not exist!'} + + def test_cancel_retirement_not_pending(self): + """ + Test that cancel_retirement throws 400 if retirement state is not PENDING. + """ + client = self.login_client('staff_client', 'staff_user') + retirement_state = RetirementStateFactory.create(state_name='NOT_PENDING', state_execution_order=1) + user_retirement_status = UserRetirementStatusFactory.create( + user=self.user, + current_state=retirement_state, + last_state=retirement_state, + original_email=self.user.email, + created=datetime.datetime.now(pytz.UTC) + ) + url = reverse("cancel_account_retirement") + response = client.post(url, data={'retirement_id': user_retirement_status.id}) + assert response.status_code == status.HTTP_400_BAD_REQUEST + assert response.data == { + "message": f"Retirement requests can only be cancelled for users in the PENDING state. " + f"Current request state for '{user_retirement_status.original_username}': " + f"{user_retirement_status.current_state.state_name}" + } + + def test_cancel_retirement_successful(self): + """ + Test that cancel_retirement does the following things properly: + 1. Restore user's email + 2. Reset user's password + 3. Delete Retirement Status entry + """ + client = self.login_client('staff_client', 'staff_user') + retirement_state = RetirementStateFactory.create(state_name='PENDING', state_execution_order=1) + user_retirement_status = UserRetirementStatusFactory.create( + user=self.user, + current_state=retirement_state, + last_state=retirement_state, + original_email=self.user.email, + created=datetime.datetime.now(pytz.UTC) + ) + user_retirement_status.user.set_unusable_password() + assert UserRetirementStatus.objects.count() == 1 + assert user_retirement_status.user.has_usable_password() is False + + url = reverse("cancel_account_retirement") + response = client.post(url, data={'retirement_id': user_retirement_status.id}) + self.user.refresh_from_db() + + assert response.status_code == status.HTTP_200_OK + assert response.data == {"success": True} + assert user_retirement_status.user.email == user_retirement_status.original_email + assert self.user.has_usable_password() is True + + assert UserRetirementStatus.objects.count() == 0 + + @ddt.ddt @skip_unless_lms @mock.patch('openedx.core.djangoapps.user_api.accounts.image_helpers._PROFILE_IMAGE_SIZES', [50, 10]) @@ -483,15 +582,32 @@ class TestAccountsAPI(FilteredQueryCountMixin, CacheIsolationTestCase, UserAPITe assert response.status_code == status.HTTP_400_BAD_REQUEST @mock.patch('openedx.core.djangoapps.user_api.accounts.views.is_email_retired') - def test_get_retired_user_from_email(self, mock_is_email_retired): + @ddt.data( + (datetime.datetime.now(pytz.UTC), True), + (datetime.datetime.now(pytz.UTC) - datetime.timedelta(days=15), False) + ) + @ddt.unpack + def test_search_emails_retired_before_cooloff_period(self, created_date, can_cancel, mock_is_email_retired): """ - Tests that the retired user from email cannot be accessed and shows an error message. + Tests either of the two possibilities i.e. either the retirement is created before the cool off time + or after the cool off time. """ mock_is_email_retired.return_value = True client = self.login_client('staff_client', 'staff_user') + retirement_state = RetirementStateFactory.create(state_name='PENDING', state_execution_order=1) + user_retirement_status = UserRetirementStatusFactory.create( + user=self.user, + current_state=retirement_state, + last_state=retirement_state, + original_email=self.user.email, + created=created_date + ) url = reverse("accounts_detail_api") response = client.get(url + f'?email={quote(self.user.email)}') - assert response.data == {"error_msg": RETIRED_EMAIL_MSG} + assert response.data == { + "error_msg": "This email is associated to a retired account.", "can_cancel_retirement": can_cancel, + "retirement_id": user_retirement_status.id if can_cancel else None + } def test_search_emails(self): client = self.login_client('staff_client', 'staff_user') diff --git a/openedx/core/djangoapps/user_api/accounts/utils.py b/openedx/core/djangoapps/user_api/accounts/utils.py index 2b348b4e13..c9c8cf085b 100644 --- a/openedx/core/djangoapps/user_api/accounts/utils.py +++ b/openedx/core/djangoapps/user_api/accounts/utils.py @@ -9,16 +9,17 @@ import string from urllib.parse import urlparse # pylint: disable=import-error import waffle # lint-amnesty, pylint: disable=invalid-django-waffle-import -from completion.waffle import ENABLE_COMPLETION_TRACKING_SWITCH from completion.models import BlockCompletion +from completion.waffle import ENABLE_COMPLETION_TRACKING_SWITCH from django.conf import settings from django.utils.translation import gettext as _ +from edx_django_utils.user import generate_password from social_django.models import UserSocialAuth from common.djangoapps.student.models import AccountRecovery, Registration, get_retired_email_by_email -from openedx.core.djangolib.oauth2_retirement_utils import retire_dot_oauth2_models from openedx.core.djangoapps.site_configuration.models import SiteConfiguration from openedx.core.djangoapps.theming.helpers import get_config_value_from_site_or_settings, get_current_site +from openedx.core.djangolib.oauth2_retirement_utils import retire_dot_oauth2_models from xmodule.modulestore.django import modulestore # lint-amnesty, pylint: disable=wrong-import-order from ..models import UserRetirementStatus @@ -236,3 +237,19 @@ def username_suffix_generator(suffix_length=4): else: output += random.choice(string.digits) return output + + +def handle_retirement_cancellation(retirement, email_address=None): + """ + Do the following in order to cancel retirement for a given user: + + 1. Load the user record using the retired email address -and- change the email address back. + 2. Reset users password so they can request a password reset and log in again. + 3. No need to delete the accompanying "permanent" retirement request record - it gets done via Django signal. + """ + retirement.user.email = email_address if email_address else retirement.original_email + + retirement.user.set_password(generate_password(length=25)) + retirement.user.save() + + retirement.delete() diff --git a/openedx/core/djangoapps/user_api/accounts/views.py b/openedx/core/djangoapps/user_api/accounts/views.py index 23fdac04e4..a558227b24 100644 --- a/openedx/core/djangoapps/user_api/accounts/views.py +++ b/openedx/core/djangoapps/user_api/accounts/views.py @@ -48,8 +48,8 @@ from common.djangoapps.student.models import ( # lint-amnesty, pylint: disable= get_potentially_retired_user_by_username, get_retired_email_by_email, get_retired_username_by_username, - is_username_retired, - is_email_retired + is_email_retired, + is_username_retired ) from common.djangoapps.student.models_api import confirm_name_change, do_name_change_request, get_pending_name_change from openedx.core.djangoapps.ace_common.template_context import get_base_template_context @@ -59,8 +59,9 @@ from openedx.core.djangoapps.credit.models import CreditRequest, CreditRequireme from openedx.core.djangoapps.external_user_ids.models import ExternalId, ExternalIdType from openedx.core.djangoapps.lang_pref import LANGUAGE_KEY from openedx.core.djangoapps.profile_images.images import remove_profile_images -from openedx.core.djangoapps.user_api.accounts import RETIRED_EMAIL_MSG +from openedx.core.djangoapps.user_api import accounts from openedx.core.djangoapps.user_api.accounts.image_helpers import get_profile_image_names, set_has_profile_image +from openedx.core.djangoapps.user_api.accounts.utils import handle_retirement_cancellation from openedx.core.djangoapps.user_authn.exceptions import AuthFailedError from openedx.core.lib.api.authentication import BearerAuthenticationAllowInactiveUser from openedx.core.lib.api.parsers import MergePatchParser @@ -72,15 +73,21 @@ from ..models import ( RetirementStateError, UserOrgTag, UserRetirementPartnerReportingStatus, - UserRetirementStatus, + UserRetirementStatus ) from .api import get_account_settings, update_account_settings -from .permissions import CanDeactivateUser, CanGetAccountInfo, CanReplaceUsername, CanRetireUser +from .permissions import ( + CanCancelUserRetirement, + CanDeactivateUser, + CanGetAccountInfo, + CanReplaceUsername, + CanRetireUser +) from .serializers import ( PendingNameChangeSerializer, UserRetirementPartnerReportSerializer, UserRetirementStatusSerializer, - UserSearchEmailSerializer, + UserSearchEmailSerializer ) from .signals import USER_RETIRE_LMS_CRITICAL, USER_RETIRE_LMS_MISC, USER_RETIRE_MAILINGS from .utils import create_retirement_request_and_deactivate_account, username_suffix_generator @@ -317,7 +324,28 @@ class AccountViewSet(ViewSet): search_usernames = usernames.strip(',').split(',') elif user_email: if is_email_retired(user_email): - return Response({'error_msg': RETIRED_EMAIL_MSG}, status=status.HTTP_404_NOT_FOUND) + can_cancel_retirement = True + retirement_id = None + earliest_datetime = datetime.datetime.now(pytz.UTC) - datetime.timedelta(days=settings.COOL_OFF_DAYS) + try: + retirement_status = UserRetirementStatus.objects.get( + created__gt=earliest_datetime, + created__lt=datetime.datetime.now(pytz.UTC), + original_email=user_email + ) + retirement_id = retirement_status.id + except UserRetirementStatus.DoesNotExist: + can_cancel_retirement = False + + context = { + 'error_msg': accounts.RETIRED_EMAIL_MSG, + 'can_cancel_retirement': can_cancel_retirement, + 'retirement_id': retirement_id + } + + return Response( + context, status=status.HTTP_404_NOT_FOUND + ) user_email = user_email.strip('') try: user = User.objects.get(email=user_email) @@ -835,6 +863,48 @@ class AccountRetirementPartnerReportView(ViewSet): return Response(status=status.HTTP_204_NO_CONTENT) +class CancelAccountRetirementStatusView(ViewSet): + """ + Provides API endpoints for canceling retirement process for a user's account. + """ + authentication_classes = (JwtAuthentication, SessionAuthentication) + permission_classes = (permissions.IsAuthenticated, CanCancelUserRetirement,) + + def cancel_retirement(self, request): + """ + POST /api/user/v1/accounts/cancel_retirement/ + + Cancels the retirement for a user's account. + This also handles the top level error handling, and permissions. + """ + try: + retirement_id = request.data['retirement_id'] + except KeyError: + return Response( + status=status.HTTP_400_BAD_REQUEST, + data={'message': 'retirement_id must be specified.'} + ) + + try: + retirement = UserRetirementStatus.objects.get(id=retirement_id) + except UserRetirementStatus.DoesNotExist: + return Response(data={"message": 'Retirement does not exist!'}, status=status.HTTP_400_BAD_REQUEST) + + if retirement.current_state.state_name != 'PENDING': + return Response( + status=status.HTTP_400_BAD_REQUEST, + data={ + "message": f"Retirement requests can only be cancelled for users in the PENDING state. Current " + f"request state for '{retirement.original_username}': " + f"{retirement.current_state.state_name}" + } + ) + + handle_retirement_cancellation(retirement) + + return Response(data={"success": True}, status=status.HTTP_200_OK) + + class AccountRetirementStatusView(ViewSet): """ Provides API endpoints for managing the user retirement process. diff --git a/openedx/core/djangoapps/user_api/management/commands/cancel_user_retirement_request.py b/openedx/core/djangoapps/user_api/management/commands/cancel_user_retirement_request.py index 400df360a8..adb29a4fd3 100644 --- a/openedx/core/djangoapps/user_api/management/commands/cancel_user_retirement_request.py +++ b/openedx/core/djangoapps/user_api/management/commands/cancel_user_retirement_request.py @@ -9,10 +9,9 @@ import logging from django.core.management.base import BaseCommand, CommandError +from openedx.core.djangoapps.user_api.accounts.utils import handle_retirement_cancellation from openedx.core.djangoapps.user_api.models import UserRetirementStatus -from edx_django_utils.user import generate_password # lint-amnesty, pylint: disable=wrong-import-order - LOGGER = logging.getLogger(__name__) @@ -50,13 +49,6 @@ class Command(BaseCommand): ) ) - # Load the user record using the retired email address -and- change the email address back. - retirement_status.user.email = email_address - retirement_status.user.set_password(generate_password(length=25)) - retirement_status.user.save() - - # Delete the user retirement status record. - # No need to delete the accompanying "permanent" retirement request record - it gets done via Django signal. - retirement_status.delete() + handle_retirement_cancellation(retirement_status, email_address) print(f"Successfully cancelled retirement request for user with email address '{email_address}'.") diff --git a/openedx/core/djangoapps/user_api/urls.py b/openedx/core/djangoapps/user_api/urls.py index 078ac69e0f..396ffcb74b 100644 --- a/openedx/core/djangoapps/user_api/urls.py +++ b/openedx/core/djangoapps/user_api/urls.py @@ -17,7 +17,7 @@ from .accounts.views import ( DeactivateLogoutView, LMSAccountRetirementView, NameChangeView, - UsernameReplacementView + UsernameReplacementView, CancelAccountRetirementStatusView ) from . import views as user_api_views from .models import UserPreference @@ -62,6 +62,10 @@ PARTNER_REPORT_CLEANUP = AccountRetirementPartnerReportView.as_view({ 'post': 'retirement_partner_cleanup' }) +CANCEL_RETIREMENT = CancelAccountRetirementStatusView.as_view({ + 'post': 'cancel_retirement' +}) + RETIREMENT_QUEUE = AccountRetirementStatusView.as_view({ 'get': 'retirement_queue' }) @@ -158,6 +162,9 @@ urlpatterns = [ path('v1/accounts/retirement_partner_report_cleanup/', PARTNER_REPORT_CLEANUP, name='accounts_retirement_partner_report_cleanup' ), + path('v1/accounts/cancel_retirement/', CANCEL_RETIREMENT, + name='cancel_account_retirement' + ), path('v1/accounts/retirement_queue/', RETIREMENT_QUEUE, name='accounts_retirement_queue' ),