diff --git a/eox_core/api/support/v1/serializers.py b/eox_core/api/support/v1/serializers.py index 52f6ebc1f..0fbbddd9d 100644 --- a/eox_core/api/support/v1/serializers.py +++ b/eox_core/api/support/v1/serializers.py @@ -28,24 +28,33 @@ class WrittableEdxappRemoveUserSerializer(serializers.Serializer): is_support_user = serializers.BooleanField(default=True) -class WrittableEdxappUsernameSerializer(serializers.Serializer): +class WrittableEdxappUserSerializer(serializers.Serializer): """ - Handles the serialization of the data required to update the username of an edxapp user. + Base serializer for updating username or email of an edxapp user. + + When a username/email update is being made the following validations are performed: + - The new username/email is not already taken by another user. + - The user is not staff or superuser. + - The user has just one signup source. """ - new_username = serializers.CharField(max_length=USERNAME_MAX_LENGTH, write_only=True) - def validate(self, attrs): + def validate_conflicts(self, attrs): """ - When a username update is being made, then it checks that: - - The new username is not already taken by other user. - - The user is not staff or superuser. - - The user has just one signup source. + Validates that no conflicts exist for the provided username or email. """ username = attrs.get("new_username") - conflicts = check_edxapp_account_conflicts(None, username) + email = attrs.get("new_email") + + conflicts = check_edxapp_account_conflicts(email, username) if conflicts: - raise serializers.ValidationError({"detail": "An account already exists with the provided username."}) + raise serializers.ValidationError({"detail": "An account already exists with the provided username or email."}) + return attrs + + def validate_role_restrictions(self, attrs): + """ + Validates that the user is not staff or superuser and has just one signup source. + """ if self.instance.is_staff or self.instance.is_superuser: raise serializers.ValidationError({"detail": "You can't update users with roles like staff or superuser."}) @@ -54,15 +63,54 @@ def validate(self, attrs): return attrs + def validate(self, attrs): + """ + Base validate method to be used by child serializers to validate common restrictions. + """ + self.validate_conflicts(attrs) + self.validate_role_restrictions(attrs) + + return attrs + + +class WrittableEdxappUsernameSerializer(WrittableEdxappUserSerializer): + """ + Handles the serialization of the data required to update the username of an edxapp user. + """ + + new_username = serializers.CharField( + max_length=USERNAME_MAX_LENGTH, + required=True, + allow_blank=False, + allow_null=False, + ) + def update(self, instance, validated_data): """ - Method to update username of edxapp User. + Updates the username of the edxapp User. """ - key = 'username' - if validated_data: - setattr(instance, key, validated_data['new_username']) - instance.save() + instance.username = validated_data["new_username"] + instance.save() + return instance + +class WrittableEdxappEmailSerializer(WrittableEdxappUserSerializer): + """ + Handles the serialization of the data required to update the email of an edxapp user. + """ + + new_email = serializers.EmailField( + required=True, + allow_blank=False, + allow_null=False, + ) + + def update(self, instance, validated_data): + """ + Updates the email of the edxapp User. + """ + instance.email = validated_data["new_email"] + instance.save() return instance diff --git a/eox_core/api/support/v1/tests/integration/test_views.py b/eox_core/api/support/v1/tests/integration/test_views.py index 14484cc25..8a96e957b 100644 --- a/eox_core/api/support/v1/tests/integration/test_views.py +++ b/eox_core/api/support/v1/tests/integration/test_views.py @@ -22,6 +22,9 @@ class SupportAPIRequestMixin: UPDATE_USERNAME_URL = ( f"{settings['EOX_CORE_API_BASE']}{reverse('eox-support-api:eox-support-api:edxapp-replace-username')}" ) + UPDATE_EMAIL_URL = ( + f"{settings['EOX_CORE_API_BASE']}{reverse('eox-support-api:eox-support-api:edxapp-replace-email')}" + ) OAUTH_APP_URL = ( f"{settings['EOX_CORE_API_BASE']}{reverse('eox-support-api:eox-support-api:edxapp-oauth-application')}" ) @@ -53,6 +56,20 @@ def update_username(self, tenant: dict, params: dict | None = None, data: dict | """ return make_request(tenant, "PATCH", url=self.UPDATE_USERNAME_URL, params=params, data=data) + def update_email(self, tenant: dict, params: dict | None = None, data: dict | None = None) -> requests.Response: + """ + Update an edxapp user's email in a tenant. + + Args: + tenant (dict): The tenant data. + params (dict, optional): The query parameters for the request. + data (dict, optional): The body data for the request. + + Returns: + requests.Response: The response object. + """ + return make_request(tenant, "PATCH", url=self.UPDATE_EMAIL_URL, params=params, data=data) + def create_oauth_application(self, tenant: dict, data: dict | None = None) -> requests.Response: """ Create an oauth application in a tenant. @@ -284,6 +301,153 @@ def test_update_username_in_tenant_missing_body(self) -> None: self.assertEqual(response.status_code, status.HTTP_400_BAD_REQUEST) self.assertEqual(response_data, {"new_username": ["This field is required."]}) + @ddt.data("username", "email") + def test_update_email_in_tenant_success(self, query_param: str) -> None: + """ + Test update an edxapp user's email in a tenant. + + Open edX definitions tested: + - `get_edxapp_user` + - `check_edxapp_account_conflicts` + - `get_user_read_only_serializer` + + Expected result: + - The status code is 200. + - The response indicates the email was updated successfully. + - The user is found in the tenant with the new email. + - The user cannot be found with the old email. + """ + data = next(FAKE_USER_DATA) + self.create_user(self.tenant_x, data) + old_email = data["email"] + new_email = f"new-email-{query_param}@example.com" + + response = self.update_email(self.tenant_x, {query_param: data[query_param]}, {"new_email": new_email}) + response_data = response.json() + get_response = self.get_user(self.tenant_x, {"email": new_email}) + get_response_data = get_response.json() + old_email_response = self.get_user(self.tenant_x, {"email": old_email}) + + self.assertEqual(response.status_code, status.HTTP_200_OK) + self.assertEqual(response_data["email"], new_email) + self.assertEqual(get_response.status_code, status.HTTP_200_OK) + self.assertEqual(get_response_data["email"], new_email) + self.assertEqual(old_email_response.status_code, status.HTTP_404_NOT_FOUND) + + def test_update_email_in_tenant_conflict(self) -> None: + """ + Test update an edxapp user's email to an email that already exists. + + Open edX definitions tested: + - `get_edxapp_user` + - `check_edxapp_account_conflicts` + + Expected result: + - The status code is 400. + - The response indicates the email already exists. + """ + data1 = next(FAKE_USER_DATA) + data2 = next(FAKE_USER_DATA) + self.create_user(self.tenant_x, data1) + self.create_user(self.tenant_x, data2) + + response = self.update_email( + self.tenant_x, + {"username": data1["username"]}, + {"new_email": data2["email"]}, + ) + response_data = response.json() + + self.assertEqual(response.status_code, status.HTTP_400_BAD_REQUEST) + self.assertEqual( + response_data["detail"], + ["An account already exists with the provided username or email."], + ) + + def test_update_email_in_tenant_not_found(self) -> None: + """ + Test update an edxapp user's email in a tenant that does not exist. + + Open edX definitions tested: + - `get_edxapp_user` + + Expected result: + - The status code is 404. + - The response indicates the user was not found in the tenant. + """ + response = self.update_email( + self.tenant_x, + {"username": "user-not-found"}, + {"new_email": "new-email@example.com"}, + ) + response_data = response.json() + + self.assertEqual(response.status_code, status.HTTP_404_NOT_FOUND) + self.assertEqual( + response_data["detail"], + f"No user found by {{'username': 'user-not-found'}} on site {self.tenant_x['domain']}.", + ) + + @ddt.data("username", "email") + def test_update_user_email_of_another_tenant(self, query_param: str) -> None: + """ + Test update an edxapp user's email of another tenant. + + Open edX definitions tested: + - `get_edxapp_user` + + Expected result: + - The status code is 404. + - The response indicates the user was not found in the tenant. + """ + data = next(FAKE_USER_DATA) + self.create_user(self.tenant_x, data) + new_email = f"new-email-{query_param}@example.com" + + response = self.update_email( + self.tenant_y, + {query_param: data[query_param]}, + {"new_email": new_email}, + ) + response_data = response.json() + + self.assertEqual(response.status_code, status.HTTP_404_NOT_FOUND) + self.assertEqual( + response_data["detail"], + f"No user found by {{'{query_param}': '{data[query_param]}'}} on site {self.tenant_y['domain']}.", + ) + + def test_update_email_in_tenant_missing_params(self) -> None: + """ + Test update an edxapp user's email in a tenant without providing the username or email. + + Expected result: + - The status code is 400. + - The response indicates the username or email is required. + """ + response = self.update_email(self.tenant_x) + response_data = response.json() + + self.assertEqual(response.status_code, status.HTTP_400_BAD_REQUEST) + self.assertEqual(response_data, ["Email or username needed"]) + + def test_update_email_in_tenant_missing_body(self) -> None: + """ + Test update an edxapp user's email in a tenant without providing the new email. + + Expected result: + - The status code is 400. + - The response indicates the new email is required. + """ + data = next(FAKE_USER_DATA) + self.create_user(self.tenant_x, data) + + response = self.update_email(self.tenant_x, params={"username": data["username"]}) + response_data = response.json() + + self.assertEqual(response.status_code, status.HTTP_400_BAD_REQUEST) + self.assertEqual(response_data, {"new_email": ["This field is required."]}) + @ddt.ddt class TestOauthApplicationAPIIntegration(SupportAPIRequestMixin, BaseIntegrationTest, UsersAPIRequestMixin): diff --git a/eox_core/api/support/v1/urls.py b/eox_core/api/support/v1/urls.py index cfb7d6b29..90015ce53 100644 --- a/eox_core/api/support/v1/urls.py +++ b/eox_core/api/support/v1/urls.py @@ -9,5 +9,6 @@ urlpatterns = [ # pylint: disable=invalid-name re_path(r'^user/$', views.EdxappUser.as_view(), name='edxapp-user'), re_path(r'^user/replace-username/$', views.EdxappReplaceUsername.as_view(), name='edxapp-replace-username'), + re_path(r'^user/replace-email/$', views.EdxappReplaceEmail.as_view(), name='edxapp-replace-email'), re_path(r'^oauth-application/$', views.OauthApplicationAPIView.as_view(), name='edxapp-oauth-application'), ] diff --git a/eox_core/api/support/v1/views.py b/eox_core/api/support/v1/views.py index 3a88e6520..ba75da3ab 100644 --- a/eox_core/api/support/v1/views.py +++ b/eox_core/api/support/v1/views.py @@ -24,6 +24,7 @@ from eox_core.api.support.v1.permissions import EoxCoreSupportAPIPermission from eox_core.api.support.v1.serializers import ( OauthApplicationSerializer, + WrittableEdxappEmailSerializer, WrittableEdxappRemoveUserSerializer, WrittableEdxappUsernameSerializer, ) @@ -95,27 +96,71 @@ def delete(self, request, *args, **kwargs): # pylint: disable=too-many-locals return Response(message, status=status) -class EdxappReplaceUsername(UserQueryMixin, APIView): +class EdxappUserUpdateBase(UserQueryMixin, APIView): """ - Handles the replacement of the username. + Base view for updating edxapp user attributes (username, email, etc.). + Provides common functionality for user updates with forum synchronization. """ authentication_classes = (BearerAuthentication, SessionAuthentication, JwtAuthentication) permission_classes = (EoxCoreSupportAPIPermission,) renderer_classes = (JSONRenderer, BrowsableAPIRenderer) - @audit_drf_api(action="Update an Edxapp user's Username.", method_name='eox_core_api_method') + def get_serializer_class(self): + """ + Returns the serializer class to use. + Must be overridden by subclasses. + """ + raise NotImplementedError("Subclasses must implement get_serializer_class()") + + def patch(self, request, *args, **kwargs): + """ + Allows to safely update an Edxapp user's attribute. + """ + query = self.get_user_query(request) + user = get_edxapp_user(**query) + data = request.data + + with transaction.atomic(): + serializer = self.get_serializer_class()(user, data=data) + serializer.is_valid(raise_exception=True) + serializer.save() + + data = serializer.validated_data + data["user"] = user + + admin_fields = getattr(settings, "ACCOUNT_VISIBILITY_CONFIGURATION", {}).get( + "admin_fields", {} + ) + serialized_user = EdxappUserReadOnlySerializer( + user, custom_fields=admin_fields, context={"request": request} + ) + return Response(serialized_user.data) + + +class EdxappReplaceUsername(EdxappUserUpdateBase): + """ + Handles the replacement of the username. + """ + + def get_serializer_class(self): + """ + Returns the serializer class to use. + """ + return WrittableEdxappUsernameSerializer + + @audit_drf_api(action="Update an Edxapp user's Username.", method_name="eox_core_api_method") def patch(self, request, *args, **kwargs): """ Allows to safely update an Edxapp user's Username along with the forum associated User. - For now users that have different signup sources cannot be updated. + This method is overridden to include explicit forum synchronization. For example: **Requests**: - PATCH /eox-core/support-api/v1/replace-username/ + PATCH /eox-core/support-api/v1/replace-username/?username=old_username **Request body** { @@ -130,7 +175,7 @@ def patch(self, request, *args, **kwargs): data = request.data with transaction.atomic(): - serializer = WrittableEdxappUsernameSerializer(user, data=data) + serializer = self.get_serializer_class()(user, data=data) serializer.is_valid(raise_exception=True) serializer.save() @@ -149,6 +194,38 @@ def patch(self, request, *args, **kwargs): return Response(serialized_user.data) +class EdxappReplaceEmail(EdxappUserUpdateBase): + """ + Handles the replacement of the email. + """ + + def get_serializer_class(self): + """Returns the serializer class to use.""" + return WrittableEdxappEmailSerializer + + @audit_drf_api(action="Update an Edxapp user's Email.", method_name="eox_core_api_method") + def patch(self, request, *args, **kwargs): + """ + Allows to safely update an Edxapp user's Email address. + + For now users that have different signup sources cannot be updated. + + For example: + + **Requests**: + PATCH /eox-core/support-api/v1/replace-email/?username=username + + **Request body** + { + "new_email": "new email" + } + + **Response values** + User serialized. + """ + return super().patch(request, *args, **kwargs) + + class OauthApplicationAPIView(UserQueryMixin, APIView): """ Handles requests related to the diff --git a/eox_core/tests/integration/data/fake_users.py b/eox_core/tests/integration/data/fake_users.py index 80f57eef1..6c83a2aca 100644 --- a/eox_core/tests/integration/data/fake_users.py +++ b/eox_core/tests/integration/data/fake_users.py @@ -1316,5 +1316,96 @@ "city": "San Francisco", "goals": "In ultrices sem nec turpis pretium, non consequat quam venenatis.", }, + { + "username": "twalker50", + "email": "twalker50@example.com", + "fullname": "Tracy Walker", + "password": "wT8@3Kp!vN#", + "activate_user": True, + "mailing_address": "912 Redwood Lane", + "year_of_birth": 1994, + "gender": "f", + "level_of_education": "b", + "city": "Portland", + "goals": "Curabitur pretium tincidunt lacus.", + }, + { + "username": "acooper51", + "email": "acooper51@example.com", + "fullname": "Anna Cooper", + "password": "aC9@2Tm!pK#", + "activate_user": True, + "mailing_address": "123 Oak Street", + "year_of_birth": 1991, + "gender": "f", + "level_of_education": "m", + "city": "Seattle", + "goals": "Vestibulum ante ipsum primis.", + }, + { + "username": "rbailey52", + "email": "rbailey52@example.com", + "fullname": "Robert Bailey", + "password": "rB4@7Xq!nL%", + "activate_user": True, + "mailing_address": "456 Pine Avenue", + "year_of_birth": 1988, + "gender": "m", + "level_of_education": "b", + "city": "Austin", + "goals": "Donec vitae sapien ut libero.", + }, + { + "username": "mrivera53", + "email": "mrivera53@example.com", + "fullname": "Maria Rivera", + "password": "mR6@3Dp!vT#", + "activate_user": True, + "mailing_address": "789 Maple Drive", + "year_of_birth": 1995, + "gender": "f", + "level_of_education": "p", + "city": "Miami", + "goals": "Pellentesque habitant morbi tristique.", + }, + { + "username": "jcoleman54", + "email": "jcoleman54@example.com", + "fullname": "James Coleman", + "password": "jC2@8Hn!kQ%", + "activate_user": True, + "mailing_address": "321 Cedar Lane", + "year_of_birth": 1992, + "gender": "m", + "level_of_education": "hs", + "city": "Denver", + "goals": "Etiam rhoncus. Maecenas tempus.", + }, + { + "username": "sbell55", + "email": "sbell55@example.com", + "fullname": "Sarah Bell", + "password": "sB5@1Wm!gR#", + "activate_user": True, + "mailing_address": "654 Birch Court", + "year_of_birth": 1989, + "gender": "f", + "level_of_education": "b", + "city": "Boston", + "goals": "Cras ultricies mi eu turpis.", + }, + { + "username": "tparker56", + "email": "tparker56@example.com", + "fullname": "Thomas Parker", + "password": "tP3@6Zn!mH#", + "activate_user": True, + "mailing_address": "987 Elm Street", + "year_of_birth": 1993, + "gender": "m", + "level_of_education": "b", + "city": "Chicago", + "goals": "Vivamus vestibulum sagittis sapien.", + }, ] )