diff --git a/Makefile b/Makefile index c440dea..0903578 100644 --- a/Makefile +++ b/Makefile @@ -183,7 +183,7 @@ migrate: ## run django migrations for the people project. superuser: ## Create an admin superuser with password "admin" @echo "$(BOLD)Creating a Django superuser$(RESET)" - @$(MANAGE) createsuperuser --email admin@example.com --password admin + @$(MANAGE) createsuperuser --admin_email admin@example.com --password admin .PHONY: superuser back-i18n-compile: ## compile the gettext files diff --git a/src/backend/core/admin.py b/src/backend/core/admin.py index e4c7fb0..cfb386c 100644 --- a/src/backend/core/admin.py +++ b/src/backend/core/admin.py @@ -67,7 +67,7 @@ class UserAdmin(auth_admin.UserAdmin): ) }, ), - (_("Personal info"), {"fields": ("email", "language", "timezone")}), + (_("Personal info"), {"fields": ("admin_email", "language", "timezone")}), ( _("Permissions"), { @@ -88,13 +88,13 @@ class UserAdmin(auth_admin.UserAdmin): None, { "classes": ("wide",), - "fields": ("email", "password1", "password2"), + "fields": ("admin_email", "password1", "password2"), }, ), ) inlines = (IdentityInline, TeamAccessInline) list_display = ( - "email", + "admin_email", "created_at", "updated_at", "is_active", @@ -105,7 +105,7 @@ class UserAdmin(auth_admin.UserAdmin): list_filter = ("is_staff", "is_superuser", "is_device", "is_active") ordering = ("is_active", "-is_superuser", "-is_staff", "-is_device", "-updated_at") readonly_fields = ("id", "created_at", "updated_at") - search_fields = ("id", "email", "identities__sub", "identities__email") + search_fields = ("id", "admin_email", "identities__sub", "identities__email") @admin.register(models.Team) diff --git a/src/backend/core/api/serializers.py b/src/backend/core/api/serializers.py index 1b2bab0..5a49344 100644 --- a/src/backend/core/api/serializers.py +++ b/src/backend/core/api/serializers.py @@ -52,42 +52,22 @@ class UserSerializer(DynamicFieldsModelSerializer): """Serialize users.""" timezone = TimeZoneSerializerField(use_pytz=False, required=True) - name = serializers.SerializerMethodField(read_only=True) - email = serializers.SerializerMethodField(read_only=True) + email = serializers.ReadOnlyField() + name = serializers.ReadOnlyField() class Meta: model = models.User fields = [ "id", - "name", "email", "language", + "name", "timezone", "is_device", "is_staff", ] read_only_fields = ["id", "name", "email", "is_device", "is_staff"] - def _get_main_identity_attr(self, obj, attribute_name): - """Return the specified attribute of the main identity.""" - try: - return getattr(obj.main_identity[0], attribute_name) - except TypeError: - return getattr(obj.main_identity, attribute_name) - except IndexError: - main_identity = obj.identities.filter(is_main=True).first() - return getattr(obj.main_identity, attribute_name) if main_identity else None - except AttributeError: - return None - - def get_name(self, obj): - """Return main identity's name.""" - return self._get_main_identity_attr(obj, "name") - - def get_email(self, obj): - """Return main identity's email.""" - return self._get_main_identity_attr(obj, "email") - class TeamAccessSerializer(serializers.ModelSerializer): """Serialize team accesses.""" diff --git a/src/backend/core/api/viewsets.py b/src/backend/core/api/viewsets.py index be4dd4c..8a609c6 100644 --- a/src/backend/core/api/viewsets.py +++ b/src/backend/core/api/viewsets.py @@ -202,7 +202,7 @@ class UserViewSet( Prefetch( "identities", queryset=models.Identity.objects.filter(is_main=True), - to_attr="main_identity", + to_attr="_identities_main", ) ) @@ -245,9 +245,6 @@ class UserViewSet( Return information on currently logged user """ user = request.user - user.main_identity = models.Identity.objects.filter( - user=user, is_main=True - ).first() return response.Response( self.serializer_class(user, context={"request": request}).data ) @@ -378,7 +375,7 @@ class TeamAccessViewSet( Prefetch( "user__identities", queryset=models.Identity.objects.filter(is_main=True), - to_attr="main_identity", + to_attr="_identities_main", ) ) # Abilities are computed based on logged-in user's role and diff --git a/src/backend/core/factories.py b/src/backend/core/factories.py index 80aca09..4aaf28b 100644 --- a/src/backend/core/factories.py +++ b/src/backend/core/factories.py @@ -120,13 +120,13 @@ class ContactFactory(BaseContactFactory): class UserFactory(factory.django.DjangoModelFactory): - """A factory to random users for testing purposes.""" + """A factory to create random users for testing purposes.""" class Meta: model = models.User - django_get_or_create = ("email",) + django_get_or_create = ("admin_email",) - email = factory.Faker("email") + admin_email = factory.Faker("email") language = factory.fuzzy.FuzzyChoice([lang[0] for lang in settings.LANGUAGES]) password = make_password("password") diff --git a/src/backend/core/models.py b/src/backend/core/models.py index 11bac69..89a09a8 100644 --- a/src/backend/core/models.py +++ b/src/backend/core/models.py @@ -74,7 +74,7 @@ class BaseModel(models.Model): def save(self, *args, **kwargs): """Call `full_clean` before saving.""" self.full_clean() - super().save(*args, **kwargs) + return super().save(*args, **kwargs) class Contact(BaseModel): @@ -157,7 +157,9 @@ class Contact(BaseModel): class User(AbstractBaseUser, BaseModel, auth_models.PermissionsMixin): """User model to work with OIDC only authentication.""" - email = models.EmailField(_("email address"), unique=True, null=True, blank=True) + admin_email = models.EmailField( + _("admin email address"), unique=True, null=True, blank=True + ) profile_contact = models.OneToOneField( Contact, on_delete=models.SET_NULL, @@ -199,7 +201,7 @@ class User(AbstractBaseUser, BaseModel, auth_models.PermissionsMixin): objects = auth_models.UserManager() - USERNAME_FIELD = "email" + USERNAME_FIELD = "admin_email" REQUIRED_FIELDS = [] class Meta: @@ -211,9 +213,32 @@ class User(AbstractBaseUser, BaseModel, auth_models.PermissionsMixin): return ( str(self.profile_contact) if self.profile_contact - else self.email or str(self.id) + else self.admin_email or str(self.id) ) + def _get_identities_main(self): + """Return a list with the main identity or an empty list.""" + try: + return self._identities_main + except AttributeError: + return self.identities.filter(is_main=True) + + @property + def name(self): + """Return main identity's name.""" + try: + return self._get_identities_main()[0].name + except IndexError: + return None + + @property + def email(self): + """Return main identity's email.""" + try: + return self._get_identities_main()[0].email + except IndexError: + return None + def clean(self): """Validate fields.""" super().clean() @@ -225,8 +250,10 @@ class User(AbstractBaseUser, BaseModel, auth_models.PermissionsMixin): def email_user(self, subject, message, from_email=None, **kwargs): """Email this user.""" - main_identity = self.identities.get(is_main=True) - mail.send_mail(subject, message, from_email, [main_identity.email], **kwargs) + email = self.email or self.admin_email + if not email: + raise ValueError("You must first set an email for the user.") + mail.send_mail(subject, message, from_email, [email], **kwargs) @classmethod def get_email_field_name(cls): @@ -368,7 +395,7 @@ class Team(BaseModel): return self.name def save(self, *args, **kwargs): - """Overriding save function to compute the slug.""" + """Override save function to compute the slug.""" self.slug = self.get_slug() return super().save(*args, **kwargs) diff --git a/src/backend/core/tests/test_api_users.py b/src/backend/core/tests/test_api_users.py index 9623649..8607f34 100644 --- a/src/backend/core/tests/test_api_users.py +++ b/src/backend/core/tests/test_api_users.py @@ -53,8 +53,8 @@ def test_api_users_authenticated_list_by_email(): Authenticated users should be able to search users with a case-insensitive and partial query on the email. """ - user = factories.UserFactory(email="tester@ministry.fr") - factories.IdentityFactory(user=user, email=user.email, name="john doe") + user = factories.UserFactory(admin_email="tester@ministry.fr") + factories.IdentityFactory(user=user, email=user.admin_email, name="john doe") client = APIClient() client.force_login(user) @@ -125,8 +125,8 @@ def test_api_users_authenticated_list_by_name(): Authenticated users should be able to search users with a case-insensitive and partial query on the name. """ - user = factories.UserFactory(email="tester@ministry.fr") - factories.IdentityFactory(user=user, email=user.email, name="john doe") + user = factories.UserFactory(admin_email="tester@ministry.fr") + factories.IdentityFactory(user=user, email=user.admin_email, name="john doe") client = APIClient() client.force_login(user) @@ -192,8 +192,8 @@ def test_api_users_authenticated_list_by_name_and_email(): partial query on the name and email. """ - user = factories.UserFactory(email="tester@ministry.fr") - factories.IdentityFactory(user=user, email=user.email, name="john doe") + user = factories.UserFactory(admin_email="tester@ministry.fr") + factories.IdentityFactory(user=user, email=user.admin_email, name="john doe") client = APIClient() client.force_login(user) @@ -225,7 +225,7 @@ def test_api_users_authenticated_list_exclude_users_already_in_team( Authenticated users should be able to search users but the result should exclude all users already in the given team. """ - user = factories.UserFactory(email="tester@ministry.fr") + user = factories.UserFactory(admin_email="tester@ministry.fr") factories.IdentityFactory(user=user, email=user.email, name="john doe") client = APIClient() client.force_login(user) @@ -280,8 +280,8 @@ def test_api_users_authenticated_list_multiple_identities_single_user(): """ User with multiple identities should appear only once in results. """ - user = factories.UserFactory(email="tester@ministry.fr") - factories.IdentityFactory(user=user, email=user.email, name="eva karl") + user = factories.UserFactory(admin_email="tester@ministry.fr") + factories.IdentityFactory(user=user, email=user.admin_email, name="eva karl") client = APIClient() client.force_login(user) @@ -308,8 +308,8 @@ def test_api_users_authenticated_list_multiple_identities_multiple_users(): User with multiple identities should be ranked on their best matching identity. """ - user = factories.UserFactory(email="tester@ministry.fr") - factories.IdentityFactory(user=user, email=user.email, name="john doe") + user = factories.UserFactory(admin_email="tester@ministry.fr") + factories.IdentityFactory(user=user, email=user.admin_email, name="john doe") client = APIClient() client.force_login(user) @@ -368,8 +368,8 @@ def test_api_users_authenticated_list_multiple_identities_multiple_users(): def test_api_users_authenticated_list_uppercase_content(): """Upper case content should be found by lower case query.""" - user = factories.UserFactory(email="tester@ministry.fr") - factories.IdentityFactory(user=user, email=user.email, name="eva karl") + user = factories.UserFactory(admin_email="tester@ministry.fr") + factories.IdentityFactory(user=user, email=user.admin_email, name="eva karl") client = APIClient() client.force_login(user) @@ -399,8 +399,8 @@ def test_api_users_authenticated_list_uppercase_content(): def test_api_users_list_authenticated_capital_query(): """Upper case query should find lower case content.""" - user = factories.UserFactory(email="tester@ministry.fr") - factories.IdentityFactory(user=user, email=user.email, name="eva karl") + user = factories.UserFactory(admin_email="tester@ministry.fr") + factories.IdentityFactory(user=user, email=user.admin_email, name="eva karl") client = APIClient() client.force_login(user) @@ -428,8 +428,8 @@ def test_api_users_list_authenticated_capital_query(): def test_api_contacts_list_authenticated_accented_query(): """Accented content should be found by unaccented query.""" - user = factories.UserFactory(email="tester@ministry.fr") - factories.IdentityFactory(user=user, email=user.email, name="john doe") + user = factories.UserFactory(admin_email="tester@ministry.fr") + factories.IdentityFactory(user=user, email=user.admin_email, name="john doe") client = APIClient() client.force_login(user) @@ -510,7 +510,7 @@ def test_api_users_list_pagination_page_size( client.force_login(user) for i in range(page_size): - factories.UserFactory.create(email=f"user-{i}@people.com") + factories.UserFactory.create(admin_email=f"user-{i}@people.com") response = client.get( f"/api/v1.0/users/?page_size={page_size}", @@ -535,7 +535,7 @@ def test_api_users_list_pagination_wrong_page_size( client.force_login(user) for i in range(page_size): - factories.UserFactory.create(email=f"user-{i}@people.com") + factories.UserFactory.create(admin_email=f"user-{i}@people.com") response = client.get( f"/api/v1.0/users/?page_size={page_size}", diff --git a/src/backend/core/tests/test_authentication_get_or_create_user.py b/src/backend/core/tests/test_authentication_get_or_create_user.py index 2c771c5..6bb4974 100644 --- a/src/backend/core/tests/test_authentication_get_or_create_user.py +++ b/src/backend/core/tests/test_authentication_get_or_create_user.py @@ -95,7 +95,7 @@ def test_authentication_getter_existing_user_change_fields( klass = OIDCAuthenticationBackend() identity = IdentityFactory(name="John Doe", email="john.doe@example.com") - user_email = identity.user.email + user_email = identity.user.admin_email # Create multiple identities for a user for _ in range(5): @@ -125,7 +125,7 @@ def test_authentication_getter_existing_user_change_fields( assert models.User.objects.count() == 1 assert user == identity.user - assert user.email == user_email + assert user.admin_email == user_email def test_authentication_getter_new_user_no_email(monkeypatch): @@ -148,7 +148,7 @@ def test_authentication_getter_new_user_no_email(monkeypatch): assert identity.sub == "123" assert identity.email is None - assert user.email is None + assert user.admin_email is None assert user.password == "!" assert models.User.objects.count() == 1 @@ -177,7 +177,7 @@ def test_authentication_getter_new_user_with_email(monkeypatch): assert identity.email == email assert identity.name == "John Doe" - assert user.email is None + assert user.admin_email is None assert models.User.objects.count() == 1 diff --git a/src/backend/core/tests/test_models_users.py b/src/backend/core/tests/test_models_users.py index d522f74..1894663 100644 --- a/src/backend/core/tests/test_models_users.py +++ b/src/backend/core/tests/test_models_users.py @@ -31,20 +31,20 @@ def test_models_users_id_unique(): def test_models_users_email_unique(): - """The "email" field should be unique except for the null value.""" + """The "admin_email" field should be unique except for the null value.""" user = factories.UserFactory() with pytest.raises( - ValidationError, match="User with this Email address already exists." + ValidationError, match="User with this Admin email address already exists." ): - models.User.objects.create(email=user.email) + models.User.objects.create(admin_email=user.admin_email, password="password") def test_models_users_email_several_null(): """Several users with a null value for the "email" field can co-exist.""" - factories.UserFactory(email=None) - models.User.objects.create(email=None, password="foo.") + factories.UserFactory(admin_email=None) + models.User.objects.create(admin_email=None, password="foo.") - assert models.User.objects.filter(email__isnull=True).count() == 2 + assert models.User.objects.filter(admin_email__isnull=True).count() == 2 def test_models_users_profile_not_owned(): @@ -91,11 +91,26 @@ def test_models_users_send_mail_main_existing(): ) -def test_models_users_send_mail_main_missing(): - """The 'email_user' method should fail if the user has no email address.""" +def test_models_users_send_mail_main_admin(): + """ + The 'email_user' method should send mail to the user's admin email address if the + user has no related identities. + """ user = factories.UserFactory() - with pytest.raises(models.Identity.DoesNotExist) as excinfo: + with mock.patch("django.core.mail.send_mail") as mock_send: user.email_user("my subject", "my message") - assert str(excinfo.value) == "Identity matching query does not exist." + mock_send.assert_called_once_with( + "my subject", "my message", None, [user.admin_email] + ) + + +def test_models_users_send_mail_main_missing(): + """The 'email_user' method should fail if the user has no email address.""" + user = factories.UserFactory(admin_email=None) + + with pytest.raises(ValueError) as excinfo: + user.email_user("my subject", "my message") + + assert str(excinfo.value) == "You must first set an email for the user." diff --git a/src/backend/demo/management/commands/create_demo.py b/src/backend/demo/management/commands/create_demo.py index a60e3f3..52c3198 100755 --- a/src/backend/demo/management/commands/create_demo.py +++ b/src/backend/demo/management/commands/create_demo.py @@ -115,7 +115,7 @@ def create_demo(stdout): for i in range(defaults.NB_OBJECTS["users"]): queue.push( models.User( - email=f"user{i:d}@example.com", + admin_email=f"user{i:d}@example.com", password="!", is_superuser=False, is_active=True, @@ -126,12 +126,12 @@ def create_demo(stdout): queue.flush() with Timeit(stdout, "Creating identities"): - users_values = list(models.User.objects.values("id", "email")) + users_values = list(models.User.objects.values("id", "admin_email")) for user_dict in users_values: for i in range( random.choices(range(5), weights=[5, 50, 30, 10, 5], k=1)[0] ): - user_email = user_dict["email"] + user_email = user_dict["admin_email"] queue.push( models.Identity( user_id=user_dict["id"], diff --git a/src/backend/demo/management/commands/createsuperuser.py b/src/backend/demo/management/commands/createsuperuser.py index ad73aa7..baa4e83 100644 --- a/src/backend/demo/management/commands/createsuperuser.py +++ b/src/backend/demo/management/commands/createsuperuser.py @@ -17,7 +17,7 @@ class Command(BaseCommand): def add_arguments(self, parser): """Define required arguments "email" and "password".""" parser.add_argument( - "--email", + "--admin_email", help=("Email for the user."), ) parser.add_argument( @@ -30,11 +30,11 @@ class Command(BaseCommand): Given an email and a password, create a superuser or upgrade the existing user to superuser status. """ - email = options.get("email") + email = options.get("admin_email") try: - user = User.objects.get(email=email) + user = User.objects.get(admin_email=email) except User.DoesNotExist: - user = User(email=email) + user = User(admin_email=email) message = "Superuser created successfully." else: if user.is_superuser and user.is_staff: diff --git a/src/backend/demo/tests/test_commands_create_demo.py b/src/backend/demo/tests/test_commands_create_demo.py index 0c37f6f..cb2e67d 100644 --- a/src/backend/demo/tests/test_commands_create_demo.py +++ b/src/backend/demo/tests/test_commands_create_demo.py @@ -31,3 +31,14 @@ def test_commands_create_demo(): assert models.Identity.objects.exists() assert models.Team.objects.count() == 3 assert models.TeamAccess.objects.count() >= 3 + + +def test_commands_createsuperuser(): + """ + The createsuperuser management command should create an use + with superuser permissions. + """ + + call_command("createsuperuser") + + assert models.User.objects.count() == 1