diff --git a/CHANGELOG.md b/CHANGELOG.md index 79bc5a87..ec2f664a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -15,6 +15,8 @@ and this project adheres to - ♿️(frontend) Focus main container after navigation #1854 - 🚸(backend) sort user search results by proximity with the active user #1802 +- 🚸(oidc) ignore case when fallback on email #1880 + ### Fixed diff --git a/src/backend/core/models.py b/src/backend/core/models.py index 69fc24f2..7b3a2dc2 100644 --- a/src/backend/core/models.py +++ b/src/backend/core/models.py @@ -118,11 +118,11 @@ class UserManager(auth_models.UserManager): if settings.OIDC_FALLBACK_TO_EMAIL_FOR_IDENTIFICATION: try: - return self.get(email=email) + return self.get(email__iexact=email) except self.model.DoesNotExist: pass elif ( - self.filter(email=email).exists() + self.filter(email__iexact=email).exists() and not settings.OIDC_ALLOW_DUPLICATE_EMAILS ): raise DuplicateEmailError( @@ -1905,7 +1905,7 @@ class Invitation(BaseModel): # Check if an identity already exists for the provided email if ( - User.objects.filter(email=self.email).exists() + User.objects.filter(email__iexact=self.email).exists() and not settings.OIDC_ALLOW_DUPLICATE_EMAILS ): raise ValidationError( diff --git a/src/backend/core/tests/authentication/test_backends.py b/src/backend/core/tests/authentication/test_backends.py index 6d2d1ad1..0ff0a78a 100644 --- a/src/backend/core/tests/authentication/test_backends.py +++ b/src/backend/core/tests/authentication/test_backends.py @@ -68,6 +68,30 @@ def test_authentication_getter_existing_user_via_email( assert user == db_user +def test_authentication_getter_existing_user_via_email_case_insensitive( + django_assert_num_queries, monkeypatch +): + """ + If an existing user doesn't match the sub but matches the email with different case, + the user should be returned (case-insensitive email matching). + """ + + klass = OIDCAuthenticationBackend() + db_user = UserFactory(email="john.doe@example.com") + + def get_userinfo_mocked(*args): + return {"sub": "123", "email": "JOHN.DOE@EXAMPLE.COM"} + + monkeypatch.setattr(OIDCAuthenticationBackend, "get_userinfo", get_userinfo_mocked) + + with django_assert_num_queries(4): # user by sub, user by mail, update sub + user = klass.get_or_create_user( + access_token="test-token", id_token=None, payload=None + ) + + assert user == db_user + + def test_authentication_getter_email_none(monkeypatch): """ If no user is found with the sub and no email is provided, a new user should be created. @@ -157,6 +181,39 @@ def test_authentication_getter_existing_user_no_fallback_to_email_no_duplicate( assert models.User.objects.count() == 1 +def test_authentication_getter_existing_user_no_fallback_to_email_no_duplicate_case_insensitive( + settings, monkeypatch +): + """ + When the "OIDC_FALLBACK_TO_EMAIL_FOR_IDENTIFICATION" setting is set to False, + the system should detect duplicate emails even with different case. + """ + + klass = OIDCAuthenticationBackend() + _db_user = UserFactory(email="john.doe@example.com") + + # Set the setting to False + settings.OIDC_FALLBACK_TO_EMAIL_FOR_IDENTIFICATION = False + settings.OIDC_ALLOW_DUPLICATE_EMAILS = False + + def get_userinfo_mocked(*args): + return {"sub": "123", "email": "JOHN.DOE@EXAMPLE.COM"} + + monkeypatch.setattr(OIDCAuthenticationBackend, "get_userinfo", get_userinfo_mocked) + + with pytest.raises( + SuspiciousOperation, + match=( + "We couldn't find a user with this sub but the email is already associated " + "with a registered user." + ), + ): + klass.get_or_create_user(access_token="test-token", id_token=None, payload=None) + + # Since the sub doesn't match, it should not create a new user + assert models.User.objects.count() == 1 + + def test_authentication_getter_existing_user_with_email( django_assert_num_queries, monkeypatch ): diff --git a/src/backend/core/tests/documents/test_api_document_invitations.py b/src/backend/core/tests/documents/test_api_document_invitations.py index 46428c6f..94eda072 100644 --- a/src/backend/core/tests/documents/test_api_document_invitations.py +++ b/src/backend/core/tests/documents/test_api_document_invitations.py @@ -596,6 +596,38 @@ def test_api_document_invitations_create_cannot_invite_existing_users(): } +def test_api_item_invitations_create_cannot_invite_existing_users_case_insensitive(): + """ + It should not be possible to invite already existing users, even with different email case. + """ + user = factories.UserFactory() + document = factories.DocumentFactory(users=[(user, "owner")]) + existing_user = factories.UserFactory() + + # Build an invitation to the email of an existing identity with different case + invitation_values = { + "email": existing_user.email.upper(), + "role": random.choice(models.RoleChoices.values), + } + + client = APIClient() + client.force_login(user) + + client = APIClient() + client.force_login(user) + + response = client.post( + f"/api/v1.0/documents/{document.id!s}/invitations/", + invitation_values, + format="json", + ) + + assert response.status_code == 400 + assert response.json() == { + "email": ["This email is already associated to a registered user."] + } + + def test_api_document_invitations_create_lower_email(): """ No matter the case, the email should be converted to lowercase.