Browse Source

fix(admin): superuser user put (#43841)

Correctly parse snakeCase into camel_case
Cathy Teng 2 years ago
parent
commit
6e37a3fb1d

+ 19 - 10
src/sentry/api/endpoints/user_details.py

@@ -17,7 +17,7 @@ from sentry.api.bases.user import UserEndpoint
 from sentry.api.decorators import sudo_required
 from sentry.api.serializers import serialize
 from sentry.api.serializers.models.user import DetailedSelfUserSerializer
-from sentry.api.serializers.rest_framework import ListField
+from sentry.api.serializers.rest_framework import CamelSnakeModelSerializer, ListField
 from sentry.auth.superuser import is_active_superuser
 from sentry.constants import LANGUAGES
 from sentry.models import Organization, OrganizationMember, OrganizationStatus, User, UserOption
@@ -64,7 +64,7 @@ class UserOptionsSerializer(serializers.Serializer):
     )
 
 
-class BaseUserSerializer(serializers.ModelSerializer):
+class BaseUserSerializer(CamelSnakeModelSerializer):
     def validate_username(self, value):
         if User.objects.filter(username__iexact=value).exclude(id=self.instance.id).exists():
             raise serializers.ValidationError("That username is already in use.")
@@ -99,22 +99,20 @@ class UserSerializer(BaseUserSerializer):
 
 
 class SuperuserUserSerializer(BaseUserSerializer):
-    isActive = serializers.BooleanField(source="is_active")
+    is_active = serializers.BooleanField()
 
     class Meta:
         model = User
-        fields = ("name", "username", "isActive")
+        fields = ("name", "username", "is_active")
 
 
 class PrivilegedUserSerializer(SuperuserUserSerializer):
-    isStaff = serializers.BooleanField(source="is_staff")
-    isSuperuser = serializers.BooleanField(source="is_superuser")
+    is_staff = serializers.BooleanField()
+    is_superuser = serializers.BooleanField()
 
     class Meta:
         model = User
-        # no idea wtf is up with django rest framework, but we need is_active
-        # and isActive
-        fields = ("name", "username", "isActive", "isStaff", "isSuperuser")
+        fields = ("name", "username", "is_active", "is_staff", "is_superuser")
 
 
 class DeleteUserSerializer(serializers.Serializer):
@@ -151,6 +149,17 @@ class UserDetailsEndpoint(UserEndpoint):
         :param string theme: UI theme, either "light", "dark", or "system"
         :auth: required
         """
+        if not request.access.has_permission("users.admin"):
+            if not user.is_superuser and request.data.get("isSuperuser"):
+                return Response(
+                    {"detail": "Missing required permission to add superuser."},
+                    status=status.HTTP_403_FORBIDDEN,
+                )
+            elif not user.is_staff and request.data.get("isStaff"):
+                return Response(
+                    {"detail": "Missing required permission to add admin."},
+                    status=status.HTTP_403_FORBIDDEN,
+                )
 
         if request.access.has_permission("users.admin"):
             serializer_cls = PrivilegedUserSerializer
@@ -158,7 +167,7 @@ class UserDetailsEndpoint(UserEndpoint):
             serializer_cls = SuperuserUserSerializer
         else:
             serializer_cls = UserSerializer
-        serializer = serializer_cls(user, data=request.data, partial=True)
+        serializer = serializer_cls(instance=user, data=request.data, partial=True)
 
         serializer_options = UserOptionsSerializer(
             data=request.data.get("options", {}), partial=True

+ 11 - 4
tests/sentry/api/endpoints/test_user_details.py

@@ -149,6 +149,7 @@ class UserDetailsSuperuserUpdateTest(UserDetailsTest):
     method = "put"
 
     def test_superuser_can_change_is_active(self):
+        self.user.update(is_active=True)
         superuser = self.create_user(email="b@example.com", is_superuser=True)
         self.login_as(user=superuser, superuser=True)
 
@@ -162,6 +163,7 @@ class UserDetailsSuperuserUpdateTest(UserDetailsTest):
         assert not user.is_active
 
     def test_superuser_with_permission_can_change_is_active(self):
+        self.user.update(is_active=True)
         superuser = self.create_user(email="b@example.com", is_superuser=True)
         UserPermission.objects.create(user=superuser, permission="users.admin")
         self.login_as(user=superuser, superuser=True)
@@ -176,14 +178,16 @@ class UserDetailsSuperuserUpdateTest(UserDetailsTest):
         assert not user.is_active
 
     def test_superuser_cannot_add_superuser(self):
+        self.user.update(is_superuser=False)
         superuser = self.create_user(email="b@example.com", is_superuser=True)
         self.login_as(user=superuser, superuser=True)
 
-        resp = self.get_success_response(
+        resp = self.get_error_response(
             self.user.id,
             isSuperuser="true",
+            status_code=403,
         )
-        assert resp.data["id"] == str(self.user.id)
+        assert resp.data["detail"] == "Missing required permission to add superuser."
 
         user = User.objects.get(id=self.user.id)
         assert not user.is_superuser
@@ -193,16 +197,18 @@ class UserDetailsSuperuserUpdateTest(UserDetailsTest):
         superuser = self.create_user(email="b@example.com", is_superuser=True)
         self.login_as(user=superuser, superuser=True)
 
-        resp = self.get_success_response(
+        resp = self.get_error_response(
             self.user.id,
             isStaff="true",
+            status_code=403,
         )
-        assert resp.data["id"] == str(self.user.id)
+        assert resp.data["detail"] == "Missing required permission to add admin."
 
         user = User.objects.get(id=self.user.id)
         assert not user.is_staff
 
     def test_superuser_with_permission_can_add_superuser(self):
+        self.user.update(is_superuser=False)
         superuser = self.create_user(email="b@example.com", is_superuser=True)
         UserPermission.objects.create(user=superuser, permission="users.admin")
         self.login_as(user=superuser, superuser=True)
@@ -217,6 +223,7 @@ class UserDetailsSuperuserUpdateTest(UserDetailsTest):
         assert user.is_superuser
 
     def test_superuser_with_permission_can_add_staff(self):
+        self.user.update(is_staff=False)
         superuser = self.create_user(email="b@example.com", is_superuser=True)
         UserPermission.objects.create(user=superuser, permission="users.admin")
         self.login_as(user=superuser, superuser=True)