(backend) persist OIDC first name and last name while authenticating

Inspired by @sampaccoud's eee2003 commit on impress, adapt the code to be more
Pythonic. Add basic test coverage for user name synchronization on login. User
name fields now update automatically at each login when new data is available.

Note: current logic doesn't handle the case where a user with existing names
logs in with missing first/last names - should we clear the names then?

Removing a field that was present in the initial form is not a valid update
operation.
This commit is contained in:
lebaudantoine
2024-11-13 12:18:41 +01:00
parent 0fd06ef6c0
commit 82bb5f0f8b
3 changed files with 173 additions and 1 deletions

View File

@@ -75,11 +75,16 @@ class OIDCAuthenticationBackend(MozillaOIDCAuthenticationBackend):
email = user_info.get("email")
user = self.get_existing_user(sub, email)
claims = {
"email": email,
"full_name": self.compute_full_name(user_info),
"short_name": user_info.get(settings.OIDC_USERINFO_SHORTNAME_FIELD),
}
if not user and self.get_settings("OIDC_CREATE_USER", True):
user = User.objects.create(
sub=sub,
email=email,
password="!", # noqa: S106
**claims,
)
elif not user:
return None
@@ -87,6 +92,8 @@ class OIDCAuthenticationBackend(MozillaOIDCAuthenticationBackend):
if not user.is_active:
raise SuspiciousOperation(_("User account is disabled"))
self.update_user_if_needed(user, claims)
return user
def get_existing_user(self, sub, email):
@@ -104,3 +111,32 @@ class OIDCAuthenticationBackend(MozillaOIDCAuthenticationBackend):
_("Multiple user accounts share a common email.")
) from e
return None
@staticmethod
def compute_full_name(user_info):
"""Compute user's full name based on OIDC fields in settings."""
full_name = " ".join(
filter(
None,
(
user_info.get(field)
for field in settings.OIDC_USERINFO_FULLNAME_FIELDS
),
)
)
return full_name or None
@staticmethod
def update_user_if_needed(user, claims):
"""Update user claims if they have changed."""
user_fields = vars(user.__class__) # Get available model fields
updated_claims = {
key: value
for key, value in claims.items()
if value and key in user_fields and value != getattr(user, key)
}
if not updated_claims:
return
User.objects.filter(sub=user.sub).update(**updated_claims)

View File

@@ -85,6 +85,32 @@ def test_authentication_getter_new_user_with_email(monkeypatch):
assert models.User.objects.count() == 1
@pytest.mark.parametrize("email", [None, "johndoe@foo.com"])
def test_authentication_getter_new_user_with_names(monkeypatch, email):
"""
If no user matches, a user should be created.
User's info contains name-related field, created user's full and short names should be filled,
whether the email is filled
"""
klass = OIDCAuthenticationBackend()
def get_userinfo_mocked(*args):
return {"sub": "123", "first_name": "John", "last_name": "Doe", "email": email}
monkeypatch.setattr(OIDCAuthenticationBackend, "get_userinfo", get_userinfo_mocked)
user = klass.get_or_create_user(
access_token="test-token", id_token=None, payload=None
)
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
def test_models_oidc_user_getter_invalid_token(django_assert_num_queries, monkeypatch):
"""The user's info doesn't contain a sub."""
klass = OIDCAuthenticationBackend()
@@ -286,3 +312,103 @@ def test_authentication_getter_existing_user_email_tricky(email, monkeypatch, se
)
assert user != db_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
@pytest.mark.parametrize(
"user_info, expected_name",
[
({"given_name": "John", "family_name": "Doe"}, "John Doe"),
(
{"given_name": "John", "middle_name": "M", "family_name": "Doe"},
"John M Doe",
),
({"family_name": "Doe"}, "Doe"),
({"given_name": "", "family_name": ""}, None),
({}, None),
],
)
def test_compute_full_name(user_info, expected_name, settings):
"""Test full name computation from OIDC user info fields."""
settings.OIDC_USERINFO_FULLNAME_FIELDS = [
"given_name",
"middle_name",
"family_name",
]
klass = OIDCAuthenticationBackend()
assert klass.compute_full_name(user_info) == expected_name
def test_compute_full_name_no_fields(settings):
"""Test full name computation with empty field configuration."""
settings.OIDC_USERINFO_FULLNAME_FIELDS = []
klass = OIDCAuthenticationBackend()
assert klass.compute_full_name({"given_name": "John"}) is None
@pytest.mark.parametrize(
"claims",
[
{"email": "john.doe@example.com"}, # Same data - no change needed
{"email": ""}, # Empty strings should not override
{"non_related_field": "foo"}, # Unrelated fields should be ignored
{}, # Empty claims should not affect user
{"email": None}, # None values should be ignored
],
)
def test_update_user_when_no_update_needed(django_assert_num_queries, claims):
"""Test that user attributes remain unchanged when claims don't require updates."""
user = UserFactory(
full_name="John Doe", short_name="John", email="john.doe@example.com"
)
klass = OIDCAuthenticationBackend()
with django_assert_num_queries(0):
klass.update_user_if_needed(user, claims)
user.refresh_from_db()
assert user.email == "john.doe@example.com"

View File

@@ -390,6 +390,16 @@ class Base(Configuration):
OIDC_REDIRECT_FIELD_NAME = values.Value(
"returnTo", environ_name="OIDC_REDIRECT_FIELD_NAME", environ_prefix=None
)
OIDC_USERINFO_FULLNAME_FIELDS = values.ListValue(
default=["first_name", "last_name"],
environ_name="OIDC_USERINFO_FULLNAME_FIELDS",
environ_prefix=None,
)
OIDC_USERINFO_SHORTNAME_FIELD = values.Value(
default="first_name",
environ_name="OIDC_USERINFO_SHORTNAME_FIELD",
environ_prefix=None,
)
# Video conference configuration
LIVEKIT_CONFIGURATION = {