[Bug 1136586] Fix validation for users api.

kitsune/users/api.py

@@ -145,8 +145,8 @@ class ProfileSerializer(serializers.ModelSerializer):
     display_name = serializers.WritableField(source='name', required=False)
     date_joined = DateTimeUTCField(source='user.date_joined', read_only=True)
     avatar = serializers.SerializerMethodField('get_avatar_url')
-    email = (PermissionMod(serializers.WritableField, permissions=[OnlySelf])
-             (source='user.email', required=False))
+    email = (PermissionMod(serializers.EmailField, permissions=[OnlySelf])
+             (source='user.email', required=True))
     settings = (PermissionMod(UserSettingSerializer, permissions=[OnlySelf])
                 (many=True, read_only=True))
     helpfulness = serializers.Field(source='answer_helpfulness')
@@ -222,22 +222,14 @@ def restore_object(self, attrs, instance=None):
         instance = (super(ProfileSerializer, self)
                     .restore_object(attrs, instance))
         if instance.user_id is None:
-            # The Profile doesn't have a user, so create one. If an email is
-            # specified, the user will be inactive until the email is
-            # confirmed. Otherwise the user can be created immediately.
-            if 'user.email' in attrs:
-                # This is a bit of cheat. The user shouldn't be saved yet, but
-                # ``create_inactive_user`` saves it, so their isn't much of a choice.
-                u = RegistrationProfile.objects.create_inactive_user(
-                    attrs['user.username'],
-                    attrs['user.password'],
-                    attrs['user.email'])
-                instance.user_id = u.id
-                instance.save()
-            else:
-                u = User(username=attrs['user.username'])
-                u.set_password(attrs['user.password'])
-                instance._nested_forward_relations['user'] = u
+            # This is a bit of cheat. The user shouldn't be saved yet, but
+            # ``create_inactive_user`` saves it, so their isn't much of a choice.
+            u = RegistrationProfile.objects.create_inactive_user(
+                attrs['user.username'],
+                attrs['user.password'],
+                attrs['user.email'])
+            instance.user_id = u.id
+            instance.save()
         return instance
 
     def validate_username(self, attrs, source):

kitsune/users/tests/test_api.py

@@ -98,16 +98,9 @@ def test_no_duplicate_emails(self):
         })
         assert not serializer.is_valid()
 
-    def test_users_without_emails_are_active(self):
-        del self.data['email']
-        serializer = api.ProfileSerializer(data=self.data)
-        serializer.is_valid()
-        assert serializer.is_valid()
-        serializer.save()
-        eq_(serializer.object.user.is_active, True)
-
     def test_users_with_emails_are_inactive(self):
         serializer = api.ProfileSerializer(data=self.data)
+        serializer.is_valid()
         assert serializer.is_valid()
         serializer.save()
         eq_(serializer.object.user.is_active, False)
@@ -347,23 +340,7 @@ def test_avatar_size(self):
         res = self.client.get(url, {'avatar_size': 128})
         assert '?s=128' in res.data['avatar']
 
-    def test_create_user_no_email(self):
-        # There is at least one user in existence due to migrations
-        number_users = User.objects.count()
-
-        url = reverse('user-list')
-        res = self.client.post(url, {
-            'username': 'kris',
-            'password': 'testpass'
-        })
-
-        eq_(res.status_code, 201)
-        eq_(User.objects.count(), number_users + 1)
-        u = User.objects.order_by('-id')[0]
-        eq_(u.username, 'kris')
-        eq_(u.is_active, True)
-
-    def test_create_user_with_email(self):
+    def test_create_use(self):
         # There is at least one user in existence due to migrations
         number_users = User.objects.count()
 
@@ -381,3 +358,44 @@ def test_create_user_with_email(self):
         eq_(u.username, username)
         eq_(u.email, '[email protected]')
         eq_(u.is_active, False)
+
+    def test_invalid_email(self):
+        username = 'sarah-{}'.format(random())
+        url = reverse('user-list')
+        res = self.client.post(url, {
+            'username': username,
+            'password': 'testpass',
+            'email': 'sarah',  # invalid
+        })
+        eq_(res.status_code, 400)
+        eq_(res.data, {'email': [u'Enter a valid email address.']})
+
+    def test_invalid_username(self):
+        url = reverse('user-list')
+        res = self.client.post(url, {
+            'username': '&',  # invalid
+            'password': 'testpass',
+            'email': '[email protected]',
+        })
+        eq_(res.status_code, 400)
+        eq_(res.data, {'username': [u'Usernames may only be letters, numbers, "." and "-".']})
+
+    def test_too_short_username(self):
+        url = reverse('user-list')
+        res = self.client.post(url, {
+            'username': 'a',  # too short
+            'password': 'testpass',
+            'email': '[email protected]',
+        })
+        eq_(res.status_code, 400)
+        eq_(res.data, {'username': [u'Usernames may only be letters, numbers, "." and "-".']})
+
+    def test_too_long_username(self):
+        url = reverse('user-list')
+        res = self.client.post(url, {
+            'username': 'a' * 100,  # too long
+            'password': 'testpass',
+            'email': '[email protected]',
+        })
+        eq_(res.status_code, 400)
+        eq_(res.data, {'username': [u'Usernames may only be letters, numbers, "." and "-".']})