From 781b9f7b810b9003e745b96ce9f608937bf43aa8 Mon Sep 17 00:00:00 2001 From: Rachid Mrad Date: Mon, 29 Apr 2024 18:39:00 -0400 Subject: [PATCH] if phone number is already filled in on user, do not overwrite --- src/djangooidc/backends.py | 30 +++++++++++++-- src/djangooidc/tests/test_backends.py | 53 +++++++++++++++++++++++++-- 2 files changed, 76 insertions(+), 7 deletions(-) diff --git a/src/djangooidc/backends.py b/src/djangooidc/backends.py index b0e6417db..69b34d775 100644 --- a/src/djangooidc/backends.py +++ b/src/djangooidc/backends.py @@ -65,13 +65,35 @@ class OpenIdConnectBackend(ModelBackend): return user def update_existing_user(self, user, kwargs): - """Update other fields without overwriting first_name and last_name. - Overwrite first_name and last_name if not empty string""" + """ + Update user fields without overwriting certain fields. + Args: + user: User object to be updated. + kwargs: Dictionary containing fields to update and their new values. + + Note: + This method updates user fields while preserving the values of 'first_name', + 'last_name', and 'phone' fields, unless specific conditions are met. + + - 'phone' field will be updated if it's None or an empty string. + - 'first_name' and 'last_name' will be updated if the provided value is not empty. + """ + + # Iterate over fields to update for key, value in kwargs.items(): - # Check if the key is not first_name or last_name or value is not empty string - if key not in ["first_name", "last_name"] or value: + # Check if the field is not 'first_name', 'last_name', or 'phone', + # or if it's the 'phone' field and 'user.phone' is None or empty, + # or if it's 'first_name' or 'last_name' and the provided value is not empty + if ( + key not in ["first_name", "last_name", "phone"] + or (key == "phone" and (user.phone is None or user.phone == "")) + or (key in ["first_name", "last_name"] and value) + ): + # Update the corresponding attribute of the user object setattr(user, key, value) + + # Save the user object with the updated fields user.save() def clean_username(self, username): diff --git a/src/djangooidc/tests/test_backends.py b/src/djangooidc/tests/test_backends.py index ac7f74903..ffe26e2cb 100644 --- a/src/djangooidc/tests/test_backends.py +++ b/src/djangooidc/tests/test_backends.py @@ -50,11 +50,16 @@ class OpenIdConnectBackendTestCase(TestCase): self.assertEqual(user.email, "john.doe@example.com") self.assertEqual(user.phone, "123456789") - def test_authenticate_with_existing_user_no_name(self): + def test_authenticate_with_existing_user_with_existing_first_last_phone(self): """Test that authenticate updates an existing user if it finds one. - For this test, given_name and family_name are not supplied""" + For this test, given_name and family_name are not supplied. + + The existing user's first and last name are not overwritten. + The existing user's phone number is not overwritten""" # Create an existing user with the same username and with first and last names - existing_user = User.objects.create_user(username="test_user", first_name="John", last_name="Doe") + existing_user = User.objects.create_user( + username="test_user", first_name="WillNotBe", last_name="Replaced", phone="9999999999" + ) # Remove given_name and family_name from the input, self.kwargs self.kwargs.pop("given_name", None) @@ -67,6 +72,48 @@ class OpenIdConnectBackendTestCase(TestCase): self.assertIsInstance(user, User) self.assertEqual(user, existing_user) # The same user instance should be returned + # Verify that user fields are correctly updated + self.assertEqual(user.first_name, "WillNotBe") + self.assertEqual(user.last_name, "Replaced") + self.assertEqual(user.email, "john.doe@example.com") + self.assertEqual(user.phone, "9999999999") + + def test_authenticate_with_existing_user_with_no_phone_will_update_phone(self): + """Test that authenticate updates an existing user if it finds one. + For this test, the existing user has an empty string for phone + + The existing user's phone number is overwritten""" + # Create an existing user with the same username and with first and last names + existing_user = User.objects.create_user(username="test_user", phone="") + + # Ensure that the authenticate method updates the existing user + # and preserves existing first and last names + user = self.backend.authenticate(request=None, **self.kwargs) + self.assertIsNotNone(user) + self.assertIsInstance(user, User) + self.assertEqual(user, existing_user) # The same user instance should be returned + + # Verify that user fields are correctly updated + self.assertEqual(user.first_name, "John") + self.assertEqual(user.last_name, "Doe") + self.assertEqual(user.email, "john.doe@example.com") + self.assertEqual(user.phone, "123456789") + + def test_authenticate_with_existing_user_with_none_phone_will_update_phone(self): + """Test that authenticate updates an existing user if it finds one. + For this test, the existing user has None for phone + + The existing user's phone number is overwritten""" + # Create an existing user with the same username and with first and last names + existing_user = User.objects.create_user(username="test_user") + + # Ensure that the authenticate method updates the existing user + # and preserves existing first and last names + user = self.backend.authenticate(request=None, **self.kwargs) + self.assertIsNotNone(user) + self.assertIsInstance(user, User) + self.assertEqual(user, existing_user) # The same user instance should be returned + # Verify that user fields are correctly updated self.assertEqual(user.first_name, "John") self.assertEqual(user.last_name, "Doe")