studio/people
Archived
2
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

🗃️(backend) add name field to identity

Browse Source 
We need a name for the user when we display the members in the
frontend. This commit adds the name column to the identity model.
We sync the Keycloak user with the identity model when the user
logs in to fill and udpate the name automatically.
This commit is contained in:
Anthony LC
2024-02-15 12:55:00 +01:00
committed by Samuel Paccoud
parent 8cbfb38cc4
commit 8d7c545d1a
9 changed files with 107 additions and 36 deletions
Download Patch File Download Diff File Expand all files Collapse all files

2
docker-compose.yml
View File

@@ -92,8 +92,8 @@ services:
image: nginx:1.25
ports:
- "8082:8082"
- "8088:8088"
- "8083:8083"
- "8088:8088"
volumes:
- ./docker/files/etc/nginx/conf.d:/etc/nginx/conf.d:ro
- ./src/frontend/apps/desk/out:/home/desk

11
docker/auth/realm.json
View File

@@ -46,6 +46,9 @@
"users": [
{
"username": "people",
"email": "people@people.world",
"firstName": "John",
"lastName": "Doe",
"enabled": true,
"credentials": [
{
@@ -1098,7 +1101,7 @@
},
{
"id": "79712bcf-b7f7-4ca3-b97c-418f48fded9b",
"name": "given name",
"name": "first name",
"protocol": "openid-connect",
"protocolMapper": "oidc-usermodel-property-mapper",
"consentRequired": false,
@@ -1107,7 +1110,7 @@
"user.attribute": "firstName",
"id.token.claim": "true",
"access.token.claim": "true",
"claim.name": "given_name",
"claim.name": "first_name",
"jsonType.label": "String"
}
},
@@ -1128,7 +1131,7 @@
},
{
"id": "7f741e96-41fe-4021-bbfd-506e7eb94e69",
"name": "family name",
"name": "last name",
"protocol": "openid-connect",
"protocolMapper": "oidc-usermodel-property-mapper",
"consentRequired": false,
@@ -1137,7 +1140,7 @@
"user.attribute": "lastName",
"id.token.claim": "true",
"access.token.claim": "true",
"claim.name": "family_name",
"claim.name": "last_name",
"jsonType.label": "String"
}
},

38
src/backend/core/authentication.py
View File

@@ -1,5 +1,5 @@
"""Authentication for the People core app."""
from django.conf import settings
from django.core.exceptions import SuspiciousOperation
from django.db import models
from django.utils.translation import gettext_lazy as _
@@ -66,9 +66,15 @@ class OIDCAuthenticationBackend(MozillaOIDCAuthenticationBackend):
user_info = self.get_userinfo(access_token, id_token, payload)
email = user_info.get("email")
sub = user_info.get("sub")
# Compute user name from OIDC name fields as defined in settings
names_list = [
user_info[field]
for field in settings.USER_OIDC_FIELDS_TO_NAME
if user_info.get(field)
]
user_info["name"] = " ".join(names_list) or None
sub = user_info.get("sub")
if sub is None:
raise SuspiciousOperation(
_("User info contained no recognizable user identification")
@@ -76,14 +82,23 @@ class OIDCAuthenticationBackend(MozillaOIDCAuthenticationBackend):
user = (
self.UserModel.objects.filter(identities__sub=sub)
.annotate(identity_email=models.F("identities__email"))
.annotate(
identity_email=models.F("identities__email"),
identity_name=models.F("identities__name"),
)
.distinct()
.first()
)
if user:
if email and email != user.identity_email:
Identity.objects.filter(sub=sub).update(email=email)
email = user_info.get("email")
name = user_info.get("name")
if (
email
and email != user.identity_email
or name
and name != user.identity_name
):
Identity.objects.filter(sub=sub).update(email=email, name=name)
elif self.get_settings("OIDC_CREATE_USER", True):
user = self.create_user(user_info)
@@ -92,16 +107,15 @@ class OIDCAuthenticationBackend(MozillaOIDCAuthenticationBackend):
def create_user(self, claims):
"""Return a newly created User instance."""
email = claims.get("email")
sub = claims.get("sub")
if sub is None:
raise SuspiciousOperation(
_("Claims contained no recognizable user identification")
)
user = self.UserModel.objects.create(password="!", email=email) # noqa: S106
Identity.objects.create(user=user, sub=sub, email=email)
user = self.UserModel.objects.create(password="!") # noqa: S106
Identity.objects.create(
user=user, sub=sub, email=claims.get("email"), name=claims.get("name")
)
return user

1
src/backend/core/factories.py
View File

@@ -139,6 +139,7 @@ class IdentityFactory(factory.django.DjangoModelFactory):
user = factory.SubFactory(UserFactory)
sub = factory.Sequence(lambda n: f"user{n!s}")
email = factory.Faker("email")
name = factory.Faker("name")
class TeamFactory(factory.django.DjangoModelFactory):

8
src/backend/core/migrations/0001_initial.py
View File

@@ -1,4 +1,4 @@
# Generated by Django 5.0.1 on 2024-02-21 12:34
# Generated by Django 5.0.2 on 2024-02-23 06:46
import django.contrib.auth.models
import django.core.validators
@@ -27,7 +27,7 @@ class Migration(migrations.Migration):
('created_at', models.DateTimeField(auto_now_add=True, help_text='date and time at which a record was created', verbose_name='created at')),
('updated_at', models.DateTimeField(auto_now=True, help_text='date and time at which a record was last updated', verbose_name='updated at')),
('name', models.CharField(max_length=100)),
('slug', models.SlugField(max_length=100, unique=True)),
('slug', models.SlugField(editable=False, max_length=100, unique=True)),
],
options={
'verbose_name': 'Team',
@@ -59,6 +59,9 @@ class Migration(migrations.Migration):
'verbose_name_plural': 'users',
'db_table': 'people_user',
},
managers=[
('objects', django.contrib.auth.models.UserManager()),
],
),
migrations.CreateModel(
name='Contact',
@@ -92,6 +95,7 @@ class Migration(migrations.Migration):
('updated_at', models.DateTimeField(auto_now=True, help_text='date and time at which a record was last updated', verbose_name='updated at')),
('sub', models.CharField(help_text='Required. 255 characters or fewer. Letters, numbers, and @/./+/-/_ characters only.', max_length=255, unique=True, validators=[django.core.validators.RegexValidator(message='Enter a valid sub. This value may contain only letters, numbers, and @/./+/-/_ characters.', regex='^[\\w.@+-]+\\Z')], verbose_name='sub')),
('email', models.EmailField(blank=True, max_length=254, null=True, verbose_name='email address')),
('name', models.CharField(blank=True, max_length=100, null=True, verbose_name='name')),
('is_main', models.BooleanField(default=False, help_text='Designates whether the email is the main one.', verbose_name='main')),
('user', models.ForeignKey(on_delete=django.db.models.deletion.CASCADE, related_name='identities', to=settings.AUTH_USER_MODEL)),
],

1
src/backend/core/models.py
View File

@@ -254,6 +254,7 @@ class Identity(BaseModel):
validators=[sub_validator],
)
email = models.EmailField(_("email address"), null=True, blank=True)
name = models.CharField(_("name"), max_length=100, null=True, blank=True)
is_main = models.BooleanField(
_("main"),
default=False,

71
src/backend/core/tests/test_authentication_get_or_create_user.py
View File

@@ -21,7 +21,7 @@ def test_authentication_getter_existing_user_no_email(
klass = OIDCAuthenticationBackend()
# Create a user and its identity
identity = IdentityFactory()
identity = IdentityFactory(name=None)
# Create multiple identities for a user
for _ in range(5):
@@ -46,47 +46,82 @@ def test_authentication_getter_existing_user_with_email(
):
"""
When the user's info contains an email and targets an existing user,
it should update the email on the identity but not on the user.
"""
klass = OIDCAuthenticationBackend()
identity = IdentityFactory()
identity = IdentityFactory(name="John Doe")
# Create multiple identities for a user
for _ in range(5):
IdentityFactory(user=identity.user)
user_email = identity.user.email
assert models.User.objects.count() == 1
def get_userinfo_mocked(*args):
return {"sub": identity.sub, "email": identity.email}
return {
"sub": identity.sub,
"email": identity.email,
"first_name": "John",
"last_name": "Doe",
}
monkeypatch.setattr(OIDCAuthenticationBackend, "get_userinfo", get_userinfo_mocked)
# Only 1 query if the email has not changed
# Only 1 query because email and names have not changed
with django_assert_num_queries(1):
user = klass.get_or_create_user(
access_token="test-token", id_token=None, payload=None
)
new_email = "test@fooo.com"
assert models.User.objects.get() == user
def get_userinfo_mocked_new_email(*args):
return {"sub": identity.sub, "email": new_email}
monkeypatch.setattr(
OIDCAuthenticationBackend, "get_userinfo", get_userinfo_mocked_new_email
)
@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 identity when they change.
The email on the user should not be changed.
"""
klass = OIDCAuthenticationBackend()
# Additional update query if the email has changed
identity = IdentityFactory(name="John Doe", email="john.doe@example.com")
user_email = identity.user.email
# Create multiple identities for a user
for _ in range(5):
IdentityFactory(user=identity.user)
assert models.User.objects.count() == 1
def get_userinfo_mocked(*args):
return {
"sub": identity.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):
user = klass.get_or_create_user(
access_token="test-token", id_token=None, payload=None
)
identity.refresh_from_db()
assert identity.email == new_email
assert identity.email == email
assert identity.name == f"{first_name:s} {last_name:s}"
assert models.User.objects.count() == 1
assert user == identity.user
@@ -121,14 +156,15 @@ def test_authentication_getter_new_user_no_email(monkeypatch):
def test_authentication_getter_new_user_with_email(monkeypatch):
"""
If no user matches the user's info sub, a user should be created.
User's info contains an email, created user's email should be set.
User's email and name should be set on the identity.
The "email" field on the User model should not be set as it is reserved for staff users.
"""
klass = OIDCAuthenticationBackend()
email = "people@example.com"
def get_userinfo_mocked(*args):
return {"sub": "123", "email": email}
return {"sub": "123", "email": email, "first_name": "John", "last_name": "Doe"}
monkeypatch.setattr(OIDCAuthenticationBackend, "get_userinfo", get_userinfo_mocked)
@@ -139,8 +175,9 @@ def test_authentication_getter_new_user_with_email(monkeypatch):
identity = user.identities.get()
assert identity.sub == "123"
assert identity.email == email
assert identity.name == "John Doe"
assert user.email == email
assert user.email is None
assert models.User.objects.count() == 1

5
src/backend/demo/management/commands/create_demo.py
View File

@@ -11,10 +11,14 @@ from django import db
from django.conf import settings
from django.core.management.base import BaseCommand, CommandError
from faker import Faker
from core import models
from demo import defaults
fake = Faker()
logger = logging.getLogger("people.commands.demo.create_demo")
@@ -134,6 +138,7 @@ def create_demo(stdout):
sub=uuid4(),
email=f"identity{i:d}{user_email:s}",
is_main=(i == 0),
name=fake.name(),
)
)
queue.flush()

6
src/backend/people/settings.py
View File

@@ -334,6 +334,12 @@ class Base(Configuration):
default=[], environ_name="OIDC_REDIRECT_ALLOWED_HOSTS", environ_prefix=None
)
USER_OIDC_FIELDS_TO_NAME = values.ListValue(
default=["first_name", "last_name"],
environ_name="USER_OIDC_FIELDS_TO_NAME",
environ_prefix=None,
)
# pylint: disable=invalid-name
@property
def ENVIRONMENT(self):