Throw 400 in patch if read-only fields specified.

openedx/core/djangoapps/user_api/accounts/tests/test_views.py

@@ -93,13 +93,34 @@ class TestAccountAPI(APITestCase):
     @ddt.unpack
     def test_patch_account(self, api_client, user):
         client = self.login_client(api_client, user)
-        response = client.patch(self.accounts_base_uri, data={"usernamae": "willbeignored", "gender": "f"})
+        response = client.patch(self.accounts_base_uri, data={"gender": "f"})
         self.assert_status_code(200, response)
         data = response.data
-        # Note that username is read-only, so passing it in patch is ignored. We want to change this behavior so it throws an exception.
-        self.assertEqual(self.user.username, data["username"])
         self.assertEqual("f", data["gender"])
 
+    @ddt.data(
+        ("client", "user"),
+        ("staff_client", "staff_user"),
+    )
+    @ddt.unpack
+    def test_patch_account_noneditable(self, api_client, user):
+        client = self.login_client(api_client, user)
+
+        for field_name in ["username", "email", "date_joined", "name"]:
+            response = client.patch(self.accounts_base_uri, data={field_name: "willbeignored", "gender": "f"})
+            self.assert_status_code(400, response)
+            data = response.data
+            self.assertEqual("The following fields are not editable: " + field_name, data["message"])
+
+        # Make sure that gender did not change.
+        response = client.get(self.accounts_base_uri)
+        self.assertEqual("m", response.data["gender"])
+
+        # Test error message with multiple read-only items
+        response = client.patch(self.accounts_base_uri, data={"username": "willbeignored", "email": "xx"})
+        self.assert_status_code(400, response)
+        self.assertEqual("The following fields are not editable: username, email", response.data["message"])
+
     def assert_status_code(self, expected_status_code, response):
         """Assert that the given response has the expected status code"""
         self.assertEqual(expected_status_code, response.status_code)

openedx/core/djangoapps/user_api/accounts/views.py

@@ -10,6 +10,7 @@ from openedx.core.lib.api.permissions import IsUserInUrlOrStaff
 
 from rest_framework.authentication import OAuth2Authentication, SessionAuthentication
 from rest_framework import permissions
+from rest_framework import status
 
 
 class AccountView(APIView):
@@ -78,11 +79,21 @@ class AccountView(APIView):
         """
         existing_user, existing_user_profile = self._get_user_and_profile(username)
 
-        user_serializer = AccountUserSerializer(existing_user, data=request.DATA)
+        # Check for fields that are not editable. Marking them read-only causes them to be ignored, but we wish to 400.
+        update = request.DATA
+        read_only_fields = set(update.keys()).intersection(
+            AccountUserSerializer.Meta.read_only_fields + AccountLegacyProfileSerializer.Meta.read_only_fields
+        )
+        if read_only_fields:
+            response_data = {}
+            response_data['message'] = "The following fields are not editable: " + ", ".join(str(e) for e in read_only_fields)
+            return Response(response_data, status=status.HTTP_400_BAD_REQUEST)
+
+        user_serializer = AccountUserSerializer(existing_user, data=update)
         user_serializer.is_valid()
         user_serializer.save()
 
-        legacy_profile_serializer = AccountLegacyProfileSerializer(existing_user_profile, data=request.DATA)
+        legacy_profile_serializer = AccountLegacyProfileSerializer(existing_user_profile, data=update)
         legacy_profile_serializer.is_valid()
         legacy_profile_serializer.save()