🐛(users) allow several users with empty email

We allowed empty emails but it breaks the unique constraint if we
don't also allow several users with empty email. This commit also
takes the opportunity to block user creation from a logged-in user
as it does not seem legitimate.

src/magnify/apps/core/migrations/0002_alter_user_email.py

@@ -0,0 +1,18 @@
+# Generated by Django 4.1.6 on 2023-02-04 17:15
+
+from django.db import migrations, models
+
+
+class Migration(migrations.Migration):
+
+    dependencies = [
+        ("core", "0001_initial"),
+    ]
+
+    operations = [
+        migrations.AlterField(
+            model_name="user",
+            name="email",
+            field=models.EmailField(blank=True, max_length=255, null=True),
+        ),
+    ]

src/magnify/apps/core/models.py

@@ -92,7 +92,7 @@ class User(BaseModel, AbstractBaseUser, PermissionsMixin):
         blank=True,
         null=True,
     )
-    email = models.EmailField(max_length=255, unique=True, blank=True, null=True)
+    email = models.EmailField(max_length=255, blank=True, null=True)
     language = models.CharField(
         max_length=10,
         choices=lazy(lambda: settings.LANGUAGES, tuple)(),

src/magnify/apps/core/permissions.py

@@ -40,7 +40,10 @@ def has_permission(self, request, view):
         of its administrator.
         """
         if view.action == "create":
-            return getattr(settings, "ALLOW_API_USER_CREATE", False)
+            return (
+                getattr(settings, "ALLOW_API_USER_CREATE", False)
+                and not request.user.is_authenticated
+            )
 
         if view.action in ["list", "retrieve"]:
             return request.user.is_authenticated

tests/apps/core/test_core_api_users.py

@@ -358,128 +358,123 @@ def test_api_users_create_anonymous_successful(self):
             },
         )
 
-    def test_api_users_create_authenticated_forbidden(self):
+    @override_settings(ALLOW_API_USER_CREATE=True)
+    def test_api_users_create_anonymous_existing_username(self):
         """
-        Authenticated users should not be able to create users via the API if not allowed.
+        Trying to create a user with a username that already exists should receive a 400 error.
         """
         user = UserFactory()
-        jwt_token = AccessToken.for_user(user)
 
-        with mock.patch(
-            "magnify.apps.core.utils.get_tokens_for_user", return_value=MOCK_TOKENS
-        ):
-            response = self.client.post(
-                "/api/users/",
-                {
-                    "email": "[email protected]",
-                    "language": "fr",
-                    "name": "Thomas Jefferson",
-                    "username": "thomas",
-                    "password": "mypassword",
-                },
-                HTTP_AUTHORIZATION=f"Bearer {jwt_token}",
-            )
-        self.assertEqual(response.status_code, 403)
-        self.assertEqual(User.objects.count(), 1)
+        response = self.client.post(
+            "/api/users/",
+            {
+                "email": "[email protected]",
+                "language": "fr",
+                "name": "Thomas Jefferson",
+                "username": user.username,
+                "password": "mypassword",
+            },
+        )
+
+        self.assertEqual(response.status_code, 400)
+        self.assertEqual(
+            response.json(), {"username": ["A user with that username already exists."]}
+        )
 
     @override_settings(ALLOW_API_USER_CREATE=True)
-    def test_api_users_create_authenticated_successful(self):
-        """Authenticated users should be able to create users."""
+    def test_api_users_create_anonymous_existing_email(self):
+        """
+        It should be possible to create a user with an email that already exists.
+        """
         user = UserFactory()
-        jwt_token = AccessToken.for_user(user)
-        is_device = random.choice([True, False])
 
         with mock.patch(
             "magnify.apps.core.utils.get_tokens_for_user", return_value=MOCK_TOKENS
         ):
             response = self.client.post(
                 "/api/users/",
                 {
-                    "email": "[email protected]",
-                    "is_device": is_device,
+                    "email": user.email,
                     "language": "fr",
                     "name": "Thomas Jefferson",
                     "username": "thomas",
                     "password": "mypassword",
                 },
-                HTTP_AUTHORIZATION=f"Bearer {jwt_token}",
             )
+
         self.assertEqual(response.status_code, 201)
-        self.assertEqual(User.objects.count(), 2)
 
-        user = User.objects.get(username="thomas")
-        self.assertEqual(user.email, "[email protected]")
-        self.assertEqual(user.name, "Thomas Jefferson")
-        self.assertEqual(user.language, "fr")
-        self.assertEqual(user.is_device, is_device)
+        self.assertEqual(User.objects.filter(email=user.email).count(), 2)
+        new_user = User.objects.get(username="thomas")
+        self.assertEqual(new_user.email, user.email)
+        self.assertEqual(new_user.name, "Thomas Jefferson")
+        self.assertEqual(new_user.language, "fr")
+        self.assertFalse(new_user.is_device)
 
-        self.assertIn("pbkdf2_sha256", user.password)
-        self.assertTrue(check_password("mypassword", user.password))
+        self.assertIn("pbkdf2_sha256", new_user.password)
+        self.assertTrue(check_password("mypassword", new_user.password))
 
         self.assertEqual(
             response.json(),
             {
-                "id": str(user.id),
-                "email": "[email protected]",
-                "is_device": is_device,
+                "id": str(new_user.id),
+                "email": user.email,
+                "is_device": False,
                 "language": "fr",
                 "name": "Thomas Jefferson",
                 "username": "thomas",
                 "auth": MOCK_TOKENS,
             },
         )
 
-    @override_settings(ALLOW_API_USER_CREATE=True)
-    def test_api_users_create_authenticated_existing_username(self):
+    def test_api_users_create_authenticated_settings_forbidden(self):
         """
-        A user trying to create a user with a username that already exists
-        should receive a 400 error.
+        Authenticated users should not be able to create users via the API if settings allow.
         """
         user = UserFactory()
         jwt_token = AccessToken.for_user(user)
 
-        response = self.client.post(
-            "/api/users/",
-            {
-                "email": "[email protected]",
-                "language": "fr",
-                "name": "Thomas Jefferson",
-                "username": user.username,
-                "password": "mypassword",
-            },
-            HTTP_AUTHORIZATION=f"Bearer {jwt_token}",
-        )
-
-        self.assertEqual(response.status_code, 400)
-        self.assertEqual(
-            response.json(), {"username": ["A user with that username already exists."]}
-        )
+        with mock.patch(
+            "magnify.apps.core.utils.get_tokens_for_user", return_value=MOCK_TOKENS
+        ):
+            response = self.client.post(
+                "/api/users/",
+                {
+                    "email": "[email protected]",
+                    "language": "fr",
+                    "name": "Thomas Jefferson",
+                    "username": "thomas",
+                    "password": "mypassword",
+                },
+                HTTP_AUTHORIZATION=f"Bearer {jwt_token}",
+            )
+        self.assertEqual(response.status_code, 403)
+        self.assertEqual(User.objects.count(), 1)
 
     @override_settings(ALLOW_API_USER_CREATE=True)
-    def test_api_users_create_authenticated_existing_email(self):
+    def test_api_users_create_authenticated_settings_authorized(self):
         """
-        A user trying to create a user with an email that already exists
-        should receive a 400 error.
+        Authenticated users should not be able to create users via the API even settings allow.
         """
         user = UserFactory()
         jwt_token = AccessToken.for_user(user)
 
-        response = self.client.post(
-            "/api/users/",
-            {
-                "email": user.email,
-                "language": "fr",
-                "name": "Thomas Jefferson",
-                "username": "thomas",
-                "password": "mypassword",
-            },
-            HTTP_AUTHORIZATION=f"Bearer {jwt_token}",
-        )
-
-        self.assertEqual(response.status_code, 400)
-        self.assertEqual(
-            response.json(), {"email": ["User with this email already exists."]}
-        )
+        with mock.patch(
+            "magnify.apps.core.utils.get_tokens_for_user", return_value=MOCK_TOKENS
+        ):
+            response = self.client.post(
+                "/api/users/",
+                {
+                    "email": "thomas[email protected]",
+                    "language": "fr",
+                    "name": "Thomas Jefferson",
+                    "username": "thomas",
+                    "password": "mypassword",
+                },
+                HTTP_AUTHORIZATION=f"Bearer {jwt_token}",
+            )
+        self.assertEqual(response.status_code, 403)
+        self.assertEqual(User.objects.count(), 1)
 
     def test_api_users_update_authenticated_self(self):
         """

tests/apps/core/test_core_models_users.py

@@ -18,12 +18,36 @@ def test_models_users_str(self):
         user = UserFactory()
         self.assertEqual(str(user), user.username)
 
-    def test_models_users_email_normalization(self):
-        """The email field should be automatically normalized upon saving."""
+    def test_models_users_username_null(self):
+        """The "username" field should not be null."""
+        with self.assertRaises(ValidationError) as context:
+            UserFactory(username=None)
+
+        self.assertEqual(
+            context.exception.messages,
+            ["This field cannot be null."],
+        )
+
+    def test_models_users_username_empty(self):
+        """The "username" field should not be empty."""
+        with self.assertRaises(ValidationError) as context:
+            UserFactory(username="")
+
+        self.assertEqual(
+            context.exception.messages,
+            ["This field cannot be blank."],
+        )
+
+    def test_models_users_username_unique(self):
+        """The "username" field should be unique."""
         user = UserFactory()
-        user.email = "[email protected]"
-        user.save()
-        self.assertEqual(user.email, "[email protected]")
+        with self.assertRaises(ValidationError) as context:
+            UserFactory(username=user.username)
+
+        self.assertEqual(
+            context.exception.messages,
+            ["A user with that username already exists."],
+        )
 
     def test_models_users_username_max_length(self):
         """The username field should be 30 characters maximum."""
@@ -72,6 +96,22 @@ def test_models_users_username_ascii(self):
             ],
         )
 
+    def test_models_users_email_empty(self):
+        """The "email" field can be empty."""
+        UserFactory(email="")
+
+    def test_models_users_email_unique(self):
+        """The "email" field is not unique."""
+        user = UserFactory()
+        UserFactory(email=user.email)
+
+    def test_models_users_email_normalization(self):
+        """The email field should be automatically normalized upon saving."""
+        user = UserFactory()
+        user.email = "[email protected]"
+        user.save()
+        self.assertEqual(user.email, "[email protected]")
+
     def test_models_users_ordering(self):
         """Users should be returned ordered by their username."""
         UserFactory.create_batch(3)