✨(backend) add full_name and short_name to user model and API

The full_name and short_name field are synchronized with the OIDC
token upon each login.

CHANGELOG.md

@@ -11,6 +11,7 @@ and this project adheres to
 
 ## Added
 
+- ✨(backend) add name fields to the user synchronized with OIDC #301
 - ✨(ci) add security scan #291
 - ✨(frontend) Activate versions feature #240
 - ✨(frontend) one-click document creation #275

src/backend/.pylintrc

@@ -447,10 +447,10 @@ max-bool-expr=5
 max-branches=12
 
 # Maximum number of locals for function / method body
-max-locals=15
+max-locals=20
 
 # Maximum number of parents for a class (see R0901).
-max-parents=7
+max-parents=10
 
 # Maximum number of public methods for a class (see R0904).
 max-public-methods=20

src/backend/core/admin.py

@@ -29,7 +29,19 @@ class UserAdmin(auth_admin.UserAdmin):
                 )
             },
         ),
-        (_("Personal info"), {"fields": ("sub", "email", "language", "timezone")}),
+        (
+            _("Personal info"),
+            {
+                "fields": (
+                    "sub",
+                    "email",
+                    "full_name",
+                    "short_name",
+                    "language",
+                    "timezone",
+                )
+            },
+        ),
         (
             _("Permissions"),
             {
@@ -58,6 +70,7 @@ class UserAdmin(auth_admin.UserAdmin):
     list_display = (
         "id",
         "sub",
+        "full_name",
         "admin_email",
         "email",
         "is_active",
@@ -68,9 +81,24 @@ class UserAdmin(auth_admin.UserAdmin):
         "updated_at",
     )
     list_filter = ("is_staff", "is_superuser", "is_device", "is_active")
-    ordering = ("is_active", "-is_superuser", "-is_staff", "-is_device", "-updated_at")
+    ordering = (
-    readonly_fields = ("id", "sub", "email", "created_at", "updated_at")
+        "is_active",
-    search_fields = ("id", "sub", "admin_email", "email")
+        "-is_superuser",
+        "-is_staff",
+        "-is_device",
+        "-updated_at",
+        "full_name",
+    )
+    readonly_fields = (
+        "id",
+        "sub",
+        "email",
+        "full_name",
+        "short_name",
+        "created_at",
+        "updated_at",
+    )
+    search_fields = ("id", "sub", "admin_email", "email", "full_name")
 
 
 @admin.register(models.Template)

src/backend/core/api/serializers.py

@@ -16,8 +16,8 @@ class UserSerializer(serializers.ModelSerializer):
 
     class Meta:
         model = models.User
-        fields = ["id", "email"]
+        fields = ["id", "email", "full_name", "short_name"]
-        read_only_fields = ["id", "email"]
+        read_only_fields = ["id", "email", "full_name", "short_name"]
 
 
 class BaseAccessSerializer(serializers.ModelSerializer):

src/backend/core/authentication/backends.py

@@ -1,5 +1,6 @@
 """Authentication Backends for the Impress core app."""
 
+from django.conf import settings
 from django.core.exceptions import SuspiciousOperation
 from django.utils.translation import gettext_lazy as _
 
@@ -74,37 +75,42 @@ class OIDCAuthenticationBackend(MozillaOIDCAuthenticationBackend):
         """
 
         user_info = self.get_userinfo(access_token, id_token, payload)
-        sub = user_info.get("sub")
 
+        # Compute user full name from OIDC name fields as defined in settings
+        names_list = [
+            user_info[field]
+            for field in settings.USER_OIDC_FIELDS_TO_FULLNAME
+            if user_info.get(field)
+        ]
+        claims = {
+            "email": user_info.get("email"),
+            "full_name": " ".join(names_list) or None,
+            "short_name": user_info.get(settings.USER_OIDC_FIELD_TO_SHORTNAME),
+        }
+
+        sub = user_info.get("sub")
         if sub is None:
             raise SuspiciousOperation(
                 _("User info contained no recognizable user identification")
             )
 
         try:
-            user = User.objects.get(sub=sub)
+            user = User.objects.get(sub=sub, is_active=True)
         except User.DoesNotExist:
             if self.get_settings("OIDC_CREATE_USER", True):
-                user = self.create_user(user_info)
+                user = User.objects.create(
+                    sub=sub,
+                    password="!",  # noqa: S106
+                    **claims,
+                )
             else:
                 user = None
-
+        else:
-        return user
+            has_changed = any(
-
+                value and value != getattr(user, key) for key, value in claims.items()
-    def create_user(self, claims):
-        """Return a newly created User instance."""
-
-        sub = claims.get("sub")
-
-        if sub is None:
-            raise SuspiciousOperation(
-                _("Claims contained no recognizable user identification")
             )
-
+            if has_changed:
-        user = User.objects.create(
+                updated_claims = {key: value for key, value in claims.items() if value}
-            sub=sub,
+                self.UserModel.objects.filter(sub=sub).update(**updated_claims)
-            email=claims.get("email"),
-            password="!",  # noqa: S106
-        )
 
         return user

src/backend/core/factories.py

@@ -22,6 +22,8 @@ class UserFactory(factory.django.DjangoModelFactory):
 
     sub = factory.Sequence(lambda n: f"user{n!s}")
     email = factory.Faker("email")
+    full_name = factory.Faker("name")
+    short_name = factory.Faker("first_name")
     language = factory.fuzzy.FuzzyChoice([lang[0] for lang in settings.LANGUAGES])
     password = make_password("password")
 

src/backend/core/migrations/0006_add_user_full_name_and_short_name.py

@@ -0,0 +1,28 @@
+# Generated by Django 5.1.1 on 2024-09-29 03:47
+
+from django.db import migrations, models
+
+
+class Migration(migrations.Migration):
+
+    dependencies = [
+        ('core', '0005_remove_document_is_public_alter_document_link_reach_and_more'),
+    ]
+
+    operations = [
+        migrations.AddField(
+            model_name='user',
+            name='full_name',
+            field=models.CharField(blank=True, max_length=100, null=True, verbose_name='full name'),
+        ),
+        migrations.AddField(
+            model_name='user',
+            name='short_name',
+            field=models.CharField(blank=True, max_length=20, null=True, verbose_name='short name'),
+        ),
+        migrations.AlterField(
+            model_name='user',
+            name='language',
+            field=models.CharField(choices="(('en-us', 'English'), ('fr-fr', 'French'))", default='en-us', help_text='The language in which the user wants to see the interface.', max_length=10, verbose_name='language'),
+        ),
+    ]

src/backend/core/models.py

@@ -145,6 +145,10 @@ class User(AbstractBaseUser, BaseModel, auth_models.PermissionsMixin):
         blank=True,
         null=True,
     )
+
+    full_name = models.CharField(_("full name"), max_length=100, null=True, blank=True)
+    short_name = models.CharField(_("short name"), max_length=20, null=True, blank=True)
+
     email = models.EmailField(_("identity email address"), blank=True, null=True)
 
     # Unlike the "email" field which stores the email coming from the OIDC token, this field

src/backend/core/tests/authentication/test_backends.py

@@ -38,6 +38,77 @@ def test_authentication_getter_existing_user_no_email(
     assert user == db_user
 
 
+def test_authentication_getter_existing_user_with_email(
+    django_assert_num_queries, monkeypatch
+):
+    """
+    When the user's info contains an email and targets an existing user,
+    """
+    klass = OIDCAuthenticationBackend()
+    user = UserFactory(full_name="John Doe", short_name="John")
+
+    def get_userinfo_mocked(*args):
+        return {
+            "sub": user.sub,
+            "email": user.email,
+            "first_name": "John",
+            "last_name": "Doe",
+        }
+
+    monkeypatch.setattr(OIDCAuthenticationBackend, "get_userinfo", get_userinfo_mocked)
+
+    # Only 1 query because email and names have not changed
+    with django_assert_num_queries(1):
+        authenticated_user = klass.get_or_create_user(
+            access_token="test-token", id_token=None, payload=None
+        )
+
+    assert user == authenticated_user
+
+
+@pytest.mark.parametrize(
+    "first_name, last_name, email",
+    [
+        ("Jack", "Doe", "john.doe@example.com"),
+        ("John", "Duy", "john.doe@example.com"),
+        ("John", "Doe", "jack.duy@example.com"),
+        ("Jack", "Duy", "jack.duy@example.com"),
+    ],
+)
+def test_authentication_getter_existing_user_change_fields(
+    first_name, last_name, email, django_assert_num_queries, monkeypatch
+):
+    """
+    It should update the email or name fields on the user when they change.
+    """
+    klass = OIDCAuthenticationBackend()
+    user = UserFactory(
+        full_name="John Doe", short_name="John", email="john.doe@example.com"
+    )
+
+    def get_userinfo_mocked(*args):
+        return {
+            "sub": user.sub,
+            "email": email,
+            "first_name": first_name,
+            "last_name": last_name,
+        }
+
+    monkeypatch.setattr(OIDCAuthenticationBackend, "get_userinfo", get_userinfo_mocked)
+
+    # One and only one additional update query when a field has changed
+    with django_assert_num_queries(2):
+        authenticated_user = klass.get_or_create_user(
+            access_token="test-token", id_token=None, payload=None
+        )
+
+    assert user == authenticated_user
+    user.refresh_from_db()
+    assert user.email == email
+    assert user.full_name == f"{first_name:s} {last_name:s}"
+    assert user.short_name == first_name
+
+
 def test_authentication_getter_new_user_no_email(monkeypatch):
     """
     If no user matches the user's info sub, a user should be created.
@@ -56,6 +127,8 @@ def test_authentication_getter_new_user_no_email(monkeypatch):
 
     assert user.sub == "123"
     assert user.email is None
+    assert user.full_name is None
+    assert user.short_name is None
     assert user.password == "!"
     assert models.User.objects.count() == 1
 
@@ -81,6 +154,8 @@ def test_authentication_getter_new_user_with_email(monkeypatch):
 
     assert user.sub == "123"
     assert user.email == email
+    assert user.full_name == "John Doe"
+    assert user.short_name == "John"
     assert user.password == "!"
     assert models.User.objects.count() == 1
 
@@ -116,14 +191,19 @@ def test_authentication_get_userinfo_json_response():
     responses.add(
         responses.GET,
         re.compile(r".*/userinfo"),
-        json={"name": "John Doe", "email": "john.doe@example.com"},
+        json={
+            "first_name": "John",
+            "last_name": "Doe",
+            "email": "john.doe@example.com",
+        },
         status=200,
     )
 
     oidc_backend = OIDCAuthenticationBackend()
     result = oidc_backend.get_userinfo("fake_access_token", None, None)
 
-    assert result["name"] == "John Doe"
+    assert result["first_name"] == "John"
+    assert result["last_name"] == "Doe"
     assert result["email"] == "john.doe@example.com"
 
 
@@ -137,14 +217,19 @@ def test_authentication_get_userinfo_token_response(monkeypatch):
     )
 
     def mock_verify_token(self, token):  # pylint: disable=unused-argument
-        return {"name": "Jane Doe", "email": "jane.doe@example.com"}
+        return {
+            "first_name": "Jane",
+            "last_name": "Doe",
+            "email": "jane.doe@example.com",
+        }
 
     monkeypatch.setattr(OIDCAuthenticationBackend, "verify_token", mock_verify_token)
 
     oidc_backend = OIDCAuthenticationBackend()
     result = oidc_backend.get_userinfo("fake_access_token", None, None)
 
-    assert result["name"] == "Jane Doe"
+    assert result["first_name"] == "Jane"
+    assert result["last_name"] == "Doe"
     assert result["email"] == "jane.doe@example.com"
 
 

src/backend/core/tests/test_api_users.py

@@ -120,6 +120,8 @@ def test_api_users_retrieve_me_authenticated():
     assert response.json() == {
         "id": str(user.id),
         "email": user.email,
+        "full_name": user.full_name,
+        "short_name": user.short_name,
     }
 
 

src/backend/demo/management/commands/create_demo.py

@@ -2,6 +2,7 @@
 """create_demo management command"""
 
 import logging
+import math
 import random
 import time
 from collections import defaultdict
@@ -111,7 +112,11 @@ def create_demo(stdout):
     queue = BulkQueue(stdout)
 
     with Timeit(stdout, "Creating users"):
+        name_size = int(math.sqrt(defaults.NB_OBJECTS["users"]))
+        first_names = [fake.first_name() for _ in range(name_size)]
+        last_names = [fake.last_name() for _ in range(name_size)]
         for i in range(defaults.NB_OBJECTS["users"]):
+            first_name = random.choice(first_names)
             queue.push(
                 models.User(
                     admin_email=f"user{i:d}@example.com",
@@ -120,6 +125,8 @@ def create_demo(stdout):
                     is_superuser=False,
                     is_active=True,
                     is_staff=False,
+                    short_name=first_name,
+                    full_name=f"{first_name:s} {random.choice(last_names):s}",
                     language=random.choice(settings.LANGUAGES)[0],
                 )
             )

src/backend/impress/settings.py

@@ -388,6 +388,17 @@ class Base(Configuration):
         default=True, environ_name="ALLOW_LOGOUT_GET_METHOD", environ_prefix=None
     )
 
+    USER_OIDC_FIELDS_TO_FULLNAME = values.ListValue(
+        default=["first_name", "last_name"],
+        environ_name="USER_OIDC_FIELDS_TO_FULLNAME",
+        environ_prefix=None,
+    )
+    USER_OIDC_FIELD_TO_SHORTNAME = values.Value(
+        default="first_name",
+        environ_name="USER_OIDC_FIELD_TO_SHORTNAME",
+        environ_prefix=None,
+    )
+
     # pylint: disable=invalid-name
     @property
     def ENVIRONMENT(self):