🚑️(backend) fix claim contains non user field
When we use the feature to get Organization registration number, the claim contains this value and it does not match with any user field. I switched to a whitelist instead of a blacklist (and two loops, with an if condition on each)
This commit is contained in:
@@ -11,6 +11,7 @@ and this project adheres to
|
||||
### Fixed
|
||||
|
||||
- 🩹(mailbox) fix status of current mailboxes
|
||||
- 🚑️(backend) fix claim contains non-user field #548
|
||||
|
||||
## [1.6.0] - 2024-11-20
|
||||
|
||||
|
||||
@@ -220,13 +220,11 @@ class OIDCAuthenticationBackend(MozillaOIDCAuthenticationBackend):
|
||||
|
||||
def update_user_if_needed(self, user, claims):
|
||||
"""Update user claims if they have changed."""
|
||||
has_changed = any(
|
||||
value and value != getattr(user, key)
|
||||
for key, value in claims.items()
|
||||
if key != "sub"
|
||||
)
|
||||
if has_changed:
|
||||
updated_claims = {
|
||||
key: value for key, value in claims.items() if value and key != "sub"
|
||||
}
|
||||
updated_claims = {}
|
||||
for key in ["email", "name"]:
|
||||
claim_value = claims.get(key)
|
||||
if claim_value and claim_value != getattr(user, key):
|
||||
updated_claims[key] = claim_value
|
||||
|
||||
if updated_claims:
|
||||
self.UserModel.objects.filter(sub=user.sub).update(**updated_claims)
|
||||
|
||||
@@ -106,6 +106,39 @@ def test_authentication_getter_existing_user_change_fields(
|
||||
assert user.name == f"{first_name:s} {last_name:s}"
|
||||
|
||||
|
||||
def test_authentication_getter_existing_user_keep_fields(
|
||||
django_assert_num_queries, monkeypatch
|
||||
):
|
||||
"""
|
||||
Falsy values in claim should not update the user's fields.
|
||||
"""
|
||||
klass = OIDCAuthenticationBackend()
|
||||
user = factories.UserFactory(
|
||||
name="John Doe", email="john.doe@example.com", with_organization=True
|
||||
)
|
||||
|
||||
def get_userinfo_mocked(*args):
|
||||
return {
|
||||
"sub": user.sub,
|
||||
"email": None,
|
||||
"first_name": "",
|
||||
"last_name": "",
|
||||
}
|
||||
|
||||
monkeypatch.setattr(OIDCAuthenticationBackend, "get_userinfo", get_userinfo_mocked)
|
||||
|
||||
# No field changed no more query
|
||||
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
|
||||
user.refresh_from_db()
|
||||
assert user.email == "john.doe@example.com"
|
||||
assert user.name == "John Doe"
|
||||
|
||||
|
||||
def test_authentication_getter_existing_user_via_email(
|
||||
django_assert_num_queries, monkeypatch
|
||||
):
|
||||
@@ -381,3 +414,42 @@ def test_authentication_getter_existing_user_via_email_update_organization(
|
||||
assert user == db_user
|
||||
assert user.organization is not None
|
||||
assert user.organization.domain_list == ["my-domain.com"]
|
||||
|
||||
|
||||
def test_authentication_getter_existing_user_with_registration_id(
|
||||
monkeypatch,
|
||||
settings,
|
||||
):
|
||||
"""
|
||||
Authenticate a user who already exists with an organization registration ID.
|
||||
|
||||
This asserts the "update_user_if_needed" does not fail when the claim
|
||||
contains the organization registration ID (or any value not present in the
|
||||
User model).
|
||||
"""
|
||||
settings.OIDC_ORGANIZATION_REGISTRATION_ID_FIELD = "registration_number"
|
||||
|
||||
klass = OIDCAuthenticationBackend()
|
||||
_existing_user = factories.UserFactory(
|
||||
sub="123",
|
||||
name="John Doe",
|
||||
email="people@example.com",
|
||||
)
|
||||
|
||||
def get_userinfo_mocked(*args):
|
||||
return {
|
||||
"sub": "123",
|
||||
"email": "people@example.com",
|
||||
"first_name": "John",
|
||||
"last_name": "Doe",
|
||||
"registration_number": "12345678901234",
|
||||
}
|
||||
|
||||
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.organization is not None
|
||||
assert user.organization.registration_id_list == ["12345678901234"]
|
||||
|
||||
Reference in New Issue
Block a user