diff --git a/common/djangoapps/student/models_api.py b/common/djangoapps/student/models_api.py index 44ab0ec6c0..997f5454db 100644 --- a/common/djangoapps/student/models_api.py +++ b/common/djangoapps/student/models_api.py @@ -111,11 +111,11 @@ def get_course_access_role(user, org, course_id, role): def get_pending_name_change(user): """ - Return a string representing the user's pending name change, or None if it does not exist. + Return the user's pending name change, or None if it does not exist. """ try: pending_name_change = _PendingNameChange.objects.get(user=user) - return pending_name_change.new_name + return pending_name_change except _PendingNameChange.DoesNotExist: return None diff --git a/lms/djangoapps/verify_student/signals.py b/lms/djangoapps/verify_student/signals.py index 686f4b07ce..d929af68dd 100644 --- a/lms/djangoapps/verify_student/signals.py +++ b/lms/djangoapps/verify_student/signals.py @@ -53,7 +53,11 @@ def send_idv_update(sender, instance, **kwargs): # pylint: disable=unused-argum import the SoftwareSecurePhotoVerification model. """ # Prioritize pending name change over current profile name, if the user has one - full_name = get_pending_name_change(instance.user) or get_name(instance.user.id) + pending_name_change = get_pending_name_change(instance.user) + if pending_name_change: + full_name = pending_name_change.new_name + else: + full_name = get_name(instance.user.id) log.info( 'IDV sending name_affirmation task (idv_id={idv_id}, user_id={user_id}) to update status={status}'.format( 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 65839d8375..eab01eb100 100644 --- a/openedx/core/djangoapps/user_api/accounts/tests/test_views.py +++ b/openedx/core/djangoapps/user_api/accounts/tests/test_views.py @@ -18,6 +18,7 @@ from rest_framework import status 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.oauth_dispatch.jwt import create_jwt_for_user from openedx.core.djangoapps.user_api.accounts import ACCOUNT_VISIBILITY_PREF_KEY @@ -1126,48 +1127,94 @@ class TestAccountAPITransactions(TransactionTestCase): class NameChangeViewTests(UserAPITestCase): """ NameChangeView tests """ - def setUp(self): - super().setUp() - self.url = reverse('name_change') + def _send_create(self, client, json_data): + """ + Helper method to send a create request to the server, defaulting to application/json + content_type and returning the response. + """ + return client.post( + reverse('request_name_change'), + data=json.dumps(json_data), + content_type='application/json' + ) - def test_request_succeeds(self): + def _send_confirm(self, client, username): + """ + Helper method to send a confirm request to the server, defaulting to application/json + content_type and returning the response. + """ + return client.post(reverse( + 'confirm_name_change', + kwargs={'username': username} + )) + + def test_create_request_succeeds(self): """ Test that a valid name change request succeeds. """ self.client.login(username=self.user.username, password=TEST_PASSWORD) - self.send_post(self.client, {'name': 'New Name'}) + response = self._send_create(self.client, {'name': 'New Name'}) + self.assertEqual(response.status_code, 201) - def test_unauthenticated(self): + def test_create_unauthenticated(self): """ Test that a name change request fails for an unauthenticated user. """ - self.send_post(self.client, {'name': 'New Name'}, expected_status=401) + response = self._send_create(self.client, {'name': 'New Name'}) + self.assertEqual(response.status_code, 401) - def test_empty_request(self): + def test_create_empty_request(self): """ Test that an empty request fails. """ self.client.login(username=self.user.username, password=TEST_PASSWORD) - self.send_post(self.client, {}, expected_status=400) + response = self._send_create(self.client, {}) + self.assertEqual(response.status_code, 400) - def test_blank_name(self): + def test_create_blank_name(self): """ Test that a blank name string fails. """ self.client.login(username=self.user.username, password=TEST_PASSWORD) - self.send_post(self.client, {'name': ''}, expected_status=400) + response = self._send_create(self.client, {'name': ''}) + self.assertEqual(response.status_code, 400) @ddt.data('invalid name', 'https://invalid.com') - def test_fails_validation(self, invalid_name): + def test_create_fails_validation(self, invalid_name): """ Test that an invalid name will return an error. """ self.client.login(username=self.user.username, password=TEST_PASSWORD) - self.send_post( - self.client, - {'name': invalid_name}, - expected_status=400 - ) + response = self._send_create(self.client, {'name': invalid_name}) + self.assertEqual(response.status_code, 400) + + def test_confirm_succeeds(self): + """ + Test that a staff user can successfully confirm a name change. + """ + self.staff_client.login(username=self.staff_user.username, password=TEST_PASSWORD) + do_name_change_request(self.user, 'New Name', 'test') + response = self._send_confirm(self.staff_client, self.user.username) + self.assertEqual(response.status_code, 204) + self.assertIsNone(get_pending_name_change(self.user)) + + def test_confirm_non_staff(self): + """ + Test that non-staff users cannot confirm name changes. + """ + self.client.login(username=self.user.username, password=TEST_PASSWORD) + do_name_change_request(self.user, 'New Name', 'test') + response = self._send_confirm(self.client, self.user.username) + self.assertEqual(response.status_code, 403) + self.assertEqual(get_pending_name_change(self.user).new_name, 'New Name') + + def test_confirm_no_pending_name_change(self): + """ + Test that attempting to confirm a non-existent name change request will result in a 404. + """ + self.staff_client.login(username=self.staff_user.username, password=TEST_PASSWORD) + response = self._send_confirm(self.staff_client, self.user.username) + self.assertEqual(response.status_code, 404) @ddt.ddt diff --git a/openedx/core/djangoapps/user_api/accounts/views.py b/openedx/core/djangoapps/user_api/accounts/views.py index 441b0ff8f4..a01f33913f 100644 --- a/openedx/core/djangoapps/user_api/accounts/views.py +++ b/openedx/core/djangoapps/user_api/accounts/views.py @@ -50,7 +50,11 @@ from common.djangoapps.student.models import ( # lint-amnesty, pylint: disable= get_retired_username_by_username, is_username_retired ) -from common.djangoapps.student.models_api import do_name_change_request +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 from openedx.core.djangoapps.api_admin.models import ApiAccessRequest from openedx.core.djangoapps.course_groups.models import UnregisteredLearnerCohortAssignments @@ -427,18 +431,20 @@ class AccountViewSet(ViewSet): return Response(account_settings) -class NameChangeView(APIView): +class NameChangeView(ViewSet): """ - Request a profile name change. This creates a PendingNameChange to be verified later, - rather than updating the user's profile name directly. + Viewset to manage profile name change requests. """ authentication_classes = (JwtAuthentication, SessionAuthentication,) permission_classes = (permissions.IsAuthenticated,) - def post(self, request): + def create(self, request): """ POST /api/user/v1/accounts/name_change/ + Request a profile name change. This creates a PendingNameChange to be verified later, + rather than updating the user's profile name directly. + Example request: { "name": "Jon Doe" @@ -462,6 +468,25 @@ class NameChangeView(APIView): return Response(status=status.HTTP_400_BAD_REQUEST, data=serializer.errors) + def confirm(self, request, username): + """ + POST /api/user/v1/account/name_change/{username}/confirm + + Confirm a name change request for the specified user, and update their profile name. + """ + if not request.user.is_staff: + return Response(status=status.HTTP_403_FORBIDDEN) + + user_model = get_user_model() + user = user_model.objects.get(username=username) + pending_name_change = get_pending_name_change(user) + + if pending_name_change: + confirm_name_change(user, pending_name_change) + return Response(status=status.HTTP_204_NO_CONTENT) + else: + return Response(status=status.HTTP_404_NOT_FOUND) + class AccountDeactivationView(APIView): """ diff --git a/openedx/core/djangoapps/user_api/urls.py b/openedx/core/djangoapps/user_api/urls.py index 2894223ed6..d0db126367 100644 --- a/openedx/core/djangoapps/user_api/urls.py +++ b/openedx/core/djangoapps/user_api/urls.py @@ -41,6 +41,14 @@ ACCOUNT_DETAIL = AccountViewSet.as_view({ 'patch': 'partial_update', }) +REQUEST_NAME_CHANGE = NameChangeView.as_view({ + 'post': 'create' +}) + +CONFIRM_NAME_CHANGE = NameChangeView.as_view({ + 'post': 'confirm' +}) + PARTNER_REPORT = AccountRetirementPartnerReportView.as_view({ 'post': 'retirement_partner_report', 'put': 'retirement_partner_status_create' @@ -120,8 +128,13 @@ urlpatterns = [ ), url( r'^v1/accounts/name_change/$', - NameChangeView.as_view(), - name='name_change' + REQUEST_NAME_CHANGE, + name='request_name_change' + ), + url( + fr'^v1/accounts/name_change/{settings.USERNAME_PATTERN}/confirm/$', + CONFIRM_NAME_CHANGE, + name='confirm_name_change' ), url( fr'^v1/accounts/{settings.USERNAME_PATTERN}/verification_status/$',