fix update_user method to work without a request (#5375)

* fix update_user method to work without a request

* adjust based on feedback
This commit is contained in:
Ryan Johnson 2023-01-24 11:36:53 -08:00 коммит произвёл GitHub
Родитель 6c1400d101
Коммит 51c09db063
Не найден ключ, соответствующий данной подписи
Идентификатор ключа GPG: 4AEE18F83AFDEB23
2 изменённых файлов: 44 добавлений и 32 удалений

Просмотреть файл

@ -79,9 +79,11 @@ class FXAAuthBackend(OIDCAuthenticationBackend):
def update_contributor_status(self, profile): def update_contributor_status(self, profile):
"""Register user as contributor.""" """Register user as contributor."""
if contribution_area := self.request.session.get("contributor"): # The request attribute might not be set.
request = getattr(self, "request", None)
if request and (contribution_area := request.session.get("contributor")):
add_to_contributors(profile.user, profile.locale, contribution_area) add_to_contributors(profile.user, profile.locale, contribution_area)
del self.request.session["contributor"] del request.session["contributor"]
def create_user(self, claims): def create_user(self, claims):
"""Override create user method to mark the profile as migrated.""" """Override create user method to mark the profile as migrated."""
@ -195,11 +197,11 @@ class FXAAuthBackend(OIDCAuthenticationBackend):
# Check if the user has active subscriptions # Check if the user has active subscriptions
subscriptions = claims.get("subscriptions", []) subscriptions = claims.get("subscriptions", [])
if not profile.is_fxa_migrated: if (request := getattr(self, "request", None)) and not profile.is_fxa_migrated:
# Check if there is already a Firefox Account with this ID # Check if there is already a Firefox Account with this ID
if Profile.objects.filter(fxa_uid=fxa_uid).exists(): if Profile.objects.filter(fxa_uid=fxa_uid).exists():
msg = _("This Firefox Account is already used in another profile.") msg = _("This Firefox Account is already used in another profile.")
messages.error(self.request, msg) messages.error(request, msg)
return None return None
# If it's not migrated, we can assume that there isn't an FxA id too # If it's not migrated, we can assume that there isn't an FxA id too
@ -207,18 +209,19 @@ class FXAAuthBackend(OIDCAuthenticationBackend):
profile.fxa_uid = fxa_uid profile.fxa_uid = fxa_uid
# This is the first time an existing user is using FxA. Redirect to profile edit # This is the first time an existing user is using FxA. Redirect to profile edit
# in case the user wants to update any settings. # in case the user wants to update any settings.
self.request.session["oidc_login_next"] = reverse("users.edit_my_profile") request.session["oidc_login_next"] = reverse("users.edit_my_profile")
messages.info(self.request, "fxa_notification_updated") messages.info(request, "fxa_notification_updated")
# There is a change in the email in Firefox Accounts. Let's update user's email # There is a change in the email in Firefox Accounts. Let's update user's email
# unless we have a superuser # unless we have a superuser
if user.email != email and not user.is_staff: if user.email != email and not user.is_staff:
if User.objects.exclude(id=user.id).filter(email=email).exists(): if User.objects.exclude(id=user.id).filter(email=email).exists():
if request:
msg = _( msg = _(
"The e-mail address used with this Firefox Account is already " "The e-mail address used with this Firefox Account is already "
"linked in another profile." "linked in another profile."
) )
messages.error(self.request, msg) messages.error(request, msg)
return None return None
user.email = email user.email = email
user_attr_changed = True user_attr_changed = True

Просмотреть файл

@ -130,9 +130,16 @@ class FXAAuthBackendTests(TestCase):
claims = { claims = {
"uid": "my_unique_fxa_id", "uid": "my_unique_fxa_id",
} }
# Test without a request (for example, when called from a Celery task).
with self.subTest("without a request"):
self.backend.update_user(user, claims)
assert not message_mock.error.called
assert not User.objects.get(id=user.id).profile.is_fxa_migrated
assert not User.objects.get(id=user.id).profile.fxa_uid
# Test with a request.
request_mock = Mock(spec=HttpRequest) request_mock = Mock(spec=HttpRequest)
request_mock.session = {} request_mock.session = {}
self.backend.claims = claims with self.subTest("with a request"):
self.backend.request = request_mock self.backend.request = request_mock
self.backend.update_user(user, claims) self.backend.update_user(user, claims)
message_mock.error.assert_called_with( message_mock.error.assert_called_with(
@ -166,10 +173,6 @@ class FXAAuthBackendTests(TestCase):
profile__is_fxa_migrated=True, profile__is_fxa_migrated=True,
) )
claims = {"uid": "my_unique_fxa_id", "email": "bar@example.com", "subscriptions": "[]"} claims = {"uid": "my_unique_fxa_id", "email": "bar@example.com", "subscriptions": "[]"}
request_mock = Mock(spec=HttpRequest)
request_mock.session = {}
self.backend.claims = claims
self.backend.request = request_mock
self.backend.update_user(user, claims) self.backend.update_user(user, claims)
user = User.objects.get(id=user.id) user = User.objects.get(id=user.id)
self.assertEqual(user.email, "bar@example.com") self.assertEqual(user.email, "bar@example.com")
@ -224,9 +227,15 @@ class FXAAuthBackendTests(TestCase):
UserFactory.create(email="foo@example.com") UserFactory.create(email="foo@example.com")
user = UserFactory.create(email="bar@example.com") user = UserFactory.create(email="bar@example.com")
claims = {"uid": "my_unique_fxa_id", "email": "foo@example.com"} claims = {"uid": "my_unique_fxa_id", "email": "foo@example.com"}
# Test without a request (for example, when called from a Celery task).
with self.subTest("without a request"):
self.backend.update_user(user, claims)
assert not message_mock.error.called
self.assertEqual(User.objects.get(id=user.id).email, "bar@example.com")
# Test with a request.
request_mock = Mock(spec=HttpRequest) request_mock = Mock(spec=HttpRequest)
request_mock.session = {} request_mock.session = {}
self.backend.claims = claims with self.subTest("with a request"):
self.backend.request = request_mock self.backend.request = request_mock
self.backend.update_user(user, claims) self.backend.update_user(user, claims)
message_mock.error.assert_called_with( message_mock.error.assert_called_with(