mirror of
https://github.com/cisagov/manage.get.gov.git
synced 2025-05-15 09:07:02 +02:00
revise logic a bit to ensure user and contact do not overwrite each other
This commit is contained in:
parent
8b36fef5c1
commit
c6b6c17540
3 changed files with 26 additions and 67 deletions
|
@ -8,8 +8,6 @@ from django.contrib.auth import get_user_model
|
||||||
from django.contrib.auth.backends import ModelBackend
|
from django.contrib.auth.backends import ModelBackend
|
||||||
from django.utils import timezone
|
from django.utils import timezone
|
||||||
|
|
||||||
from registrar.models.contact import Contact
|
|
||||||
|
|
||||||
logger = logging.getLogger(__name__)
|
logger = logging.getLogger(__name__)
|
||||||
|
|
||||||
|
|
||||||
|
@ -79,26 +77,15 @@ class OpenIdConnectBackend(ModelBackend):
|
||||||
'last_name', and 'phone' fields, unless specific conditions are met.
|
'last_name', and 'phone' fields, unless specific conditions are met.
|
||||||
|
|
||||||
- 'phone' field will be updated if it's None or an empty string.
|
- '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.
|
- 'first_name', 'last_name' or 'phone' will be updated if the provided value is not empty.
|
||||||
"""
|
"""
|
||||||
|
|
||||||
contacts = Contact.objects.filter(user=user)
|
|
||||||
|
|
||||||
if len(contacts) == 0: # no matching contact
|
|
||||||
logger.warning("Could not find a contact when one should've existed.")
|
|
||||||
|
|
||||||
if len(contacts) > 1: # multiple matches
|
|
||||||
logger.warning("There are multiple Contacts with the same email address.")
|
|
||||||
|
|
||||||
# Iterate over fields to update
|
# Iterate over fields to update
|
||||||
for key, value in kwargs.items():
|
for key, value in kwargs.items():
|
||||||
# Check if the field is not 'first_name', 'last_name', or 'phone',
|
# 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' or 'phone' and the provided value is not 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 (
|
||||||
if (
|
key in ["first_name", "last_name", "phone"] and value
|
||||||
key not in ["first_name", "last_name", "phone"]
|
|
||||||
or (key == "phone" and not contacts[0].phone)
|
|
||||||
or (key in ["first_name", "last_name"] and value)
|
|
||||||
):
|
):
|
||||||
# Update the corresponding attribute of the user object
|
# Update the corresponding attribute of the user object
|
||||||
setattr(user, key, value)
|
setattr(user, key, value)
|
||||||
|
|
|
@ -64,6 +64,7 @@ class OpenIdConnectBackendTestCase(TestCase):
|
||||||
# Remove given_name and family_name from the input, self.kwargs
|
# Remove given_name and family_name from the input, self.kwargs
|
||||||
self.kwargs.pop("given_name", None)
|
self.kwargs.pop("given_name", None)
|
||||||
self.kwargs.pop("family_name", None)
|
self.kwargs.pop("family_name", None)
|
||||||
|
self.kwargs.pop("phone", None)
|
||||||
|
|
||||||
# Ensure that the authenticate method updates the existing user
|
# Ensure that the authenticate method updates the existing user
|
||||||
# and preserves existing first and last names
|
# and preserves existing first and last names
|
||||||
|
@ -78,53 +79,13 @@ class OpenIdConnectBackendTestCase(TestCase):
|
||||||
self.assertEqual(user.email, "john.doe@example.com")
|
self.assertEqual(user.email, "john.doe@example.com")
|
||||||
self.assertEqual(user.phone, "9999999999")
|
self.assertEqual(user.phone, "9999999999")
|
||||||
|
|
||||||
def test_authenticate_with_existing_user_with_no_phone_will_update_phone(self):
|
def test_authenticate_with_existing_user_different_name_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")
|
|
||||||
self.assertEqual(user.email, "john.doe@example.com")
|
|
||||||
self.assertEqual(user.phone, "123456789")
|
|
||||||
|
|
||||||
def test_authenticate_with_existing_user_different_name(self):
|
|
||||||
"""Test that authenticate updates an existing user if it finds one.
|
"""Test that authenticate updates an existing user if it finds one.
|
||||||
For this test, given_name and family_name are supplied and overwrite"""
|
For this test, given_name and family_name are supplied and overwrite"""
|
||||||
# Create an existing user with the same username and with first and last names
|
# 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="WillBe", last_name="Replaced")
|
existing_user = User.objects.create_user(
|
||||||
|
username="test_user", first_name="WillBe", last_name="Replaced", phone="987654321"
|
||||||
|
)
|
||||||
|
|
||||||
# Ensure that the authenticate method updates the existing user
|
# Ensure that the authenticate method updates the existing user
|
||||||
# and preserves existing first and last names
|
# and preserves existing first and last names
|
||||||
|
|
|
@ -101,11 +101,22 @@ class Contact(TimeStampedModel):
|
||||||
# Call the parent class's save method to perform the actual save
|
# Call the parent class's save method to perform the actual save
|
||||||
super().save(*args, **kwargs)
|
super().save(*args, **kwargs)
|
||||||
|
|
||||||
# Update the related User object's first_name and last_name
|
if self.user:
|
||||||
if self.user and (not self.user.first_name or not self.user.last_name or not self.user.phone):
|
updated = False
|
||||||
|
|
||||||
|
# Update first name and last name if necessary
|
||||||
|
if not self.user.first_name or not self.user.last_name:
|
||||||
self.user.first_name = self.first_name
|
self.user.first_name = self.first_name
|
||||||
self.user.last_name = self.last_name
|
self.user.last_name = self.last_name
|
||||||
|
updated = True
|
||||||
|
|
||||||
|
# Update phone if necessary
|
||||||
|
if not self.user.phone:
|
||||||
self.user.phone = self.phone
|
self.user.phone = self.phone
|
||||||
|
updated = True
|
||||||
|
|
||||||
|
# Save user if any updates were made
|
||||||
|
if updated:
|
||||||
self.user.save()
|
self.user.save()
|
||||||
|
|
||||||
def __str__(self):
|
def __str__(self):
|
||||||
|
|
Loading…
Add table
Add a link
Reference in a new issue