diff --git a/src/magnify/apps/core/migrations/0002_alter_user_email.py b/src/magnify/apps/core/migrations/0002_alter_user_email.py new file mode 100644 index 000000000..bbb5d873a --- /dev/null +++ b/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), + ), + ] diff --git a/src/magnify/apps/core/models.py b/src/magnify/apps/core/models.py index 3b1c4c70d..56d555b92 100644 --- a/src/magnify/apps/core/models.py +++ b/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)(), diff --git a/src/magnify/apps/core/permissions.py b/src/magnify/apps/core/permissions.py index b45b01d56..ab49beb5c 100644 --- a/src/magnify/apps/core/permissions.py +++ b/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 diff --git a/tests/apps/core/test_core_api_users.py b/tests/apps/core/test_core_api_users.py index 08cf3ccfe..5440e071d 100644 --- a/tests/apps/core/test_core_api_users.py +++ b/tests/apps/core/test_core_api_users.py @@ -358,36 +358,35 @@ 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": "thomas.jeffersion@example.com", - "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": "thomas.jeffersion@example.com", + "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 @@ -395,33 +394,32 @@ def test_api_users_create_authenticated_successful(self): response = self.client.post( "/api/users/", { - "email": "thomas.jeffersion@example.com", - "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, "thomas.jeffersion@example.com") - 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": "thomas.jeffersion@example.com", - "is_device": is_device, + "id": str(new_user.id), + "email": user.email, + "is_device": False, "language": "fr", "name": "Thomas Jefferson", "username": "thomas", @@ -429,57 +427,54 @@ def test_api_users_create_authenticated_successful(self): }, ) - @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": "thomas.jeffersion@example.com", - "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": "thomas.jeffersion@example.com", + "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.jeffersion@example.com", + "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): """ diff --git a/tests/apps/core/test_core_models_users.py b/tests/apps/core/test_core_models_users.py index c1bd75e9b..3a2668b9c 100644 --- a/tests/apps/core/test_core_models_users.py +++ b/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 = "Thomas.Jefferson@Example.com" - user.save() - self.assertEqual(user.email, "Thomas.Jefferson@example.com") + 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 = "Thomas.Jefferson@Example.com" + user.save() + self.assertEqual(user.email, "Thomas.Jefferson@example.com") + def test_models_users_ordering(self): """Users should be returned ordered by their username.""" UserFactory.create_batch(3)