diff --git a/apps/users/api/views.py b/apps/users/api/views.py index d9a98c2..6840309 100644 --- a/apps/users/api/views.py +++ b/apps/users/api/views.py @@ -65,6 +65,7 @@ from apps.users.services.google_oauth import ( find_social_account_for_profile, get_google_flow, send_google_claim_otp, + sync_user_from_google_profile, verify_google_claim, ) @@ -190,6 +191,7 @@ class GoogleOAuthCallbackView(APIView): social_account = find_social_account_for_profile(profile) if social_account: + sync_user_from_google_profile(social_account.user, profile) flow_payload = build_authenticated_flow_payload(social_account.user) else: flow_payload = build_pending_google_flow_payload(profile) diff --git a/apps/users/services/google_oauth.py b/apps/users/services/google_oauth.py index eb407d7..76c0bd9 100644 --- a/apps/users/services/google_oauth.py +++ b/apps/users/services/google_oauth.py @@ -4,10 +4,12 @@ import secrets from dataclasses import asdict, dataclass, is_dataclass from typing import Any from urllib.parse import urlencode +from urllib.parse import urlparse import requests from django.conf import settings from django.core.cache import cache +from django.core.files.base import ContentFile from rest_framework.exceptions import APIException, ValidationError from apps.users.email_identity import mask_mobile, normalize_email_identity @@ -111,6 +113,46 @@ def _normalize_mobile(mobile: str) -> str: return normalized +def _avatar_file_extension(profile: GoogleProfile) -> str: + path = urlparse(profile.avatar_url or "").path + if "." in path: + suffix = path.rsplit(".", 1)[-1].lower() + if suffix in {"jpg", "jpeg", "png", "webp", "gif"}: + return suffix + return "jpg" + + +def sync_user_from_google_profile(user: User, profile: GoogleProfile) -> None: + update_fields: list[str] = [] + + if not user.first_name and profile.first_name: + user.first_name = profile.first_name + update_fields.append("first_name") + + if not user.last_name and profile.last_name: + user.last_name = profile.last_name + update_fields.append("last_name") + + if normalize_email_identity(user.email) is None and profile.email: + user.email = profile.email + update_fields.append("email") + + if not user.profile_picture and profile.avatar_url: + try: + avatar_response = requests.get(profile.avatar_url, timeout=10) + avatar_response.raise_for_status() + except requests.RequestException: + avatar_response = None + + if avatar_response and avatar_response.content: + filename = f"google-{profile.provider_user_id}.{_avatar_file_extension(profile)}" + user.profile_picture.save(filename, ContentFile(avatar_response.content), save=False) + update_fields.append("profile_picture") + + if update_fields: + user.save(update_fields=update_fields) + + def _build_claim_required_payload( *, profile: GoogleProfile, @@ -379,6 +421,7 @@ def complete_google_signup(flow: str, mobile: str) -> dict[str, Any]: ) user.set_unusable_password() user.save(update_fields=["password"]) + sync_user_from_google_profile(user, profile) UserSocialAccount.objects.create( user=user, @@ -457,6 +500,8 @@ def verify_google_claim(flow: str, code: str) -> dict[str, Any]: user.email = profile.email user.save(update_fields=["email"]) + sync_user_from_google_profile(user, profile) + if existing_link: existing_link.email = profile.email existing_link.email_verified = profile.email_verified diff --git a/apps/users/tests/test_api_views.py b/apps/users/tests/test_api_views.py index 40a002f..ed2150a 100644 --- a/apps/users/tests/test_api_views.py +++ b/apps/users/tests/test_api_views.py @@ -1,5 +1,5 @@ from io import StringIO -from unittest.mock import patch +from unittest.mock import Mock, patch from django.conf import settings from django.core.cache import cache @@ -512,8 +512,9 @@ class GoogleOAuthApiTests(APITestCase): cls.linked_user = User.objects.create_user( mobile="09125550001", password="secret123", - first_name="Google", - last_name="Linked", + first_name="", + last_name="", + email=None, ) cls.email_matched_user = User.objects.create_user( mobile="09125550002", @@ -525,8 +526,8 @@ class GoogleOAuthApiTests(APITestCase): cls.blank_email_user = User.objects.create_user( mobile="09125550003", password="secret123", - first_name="Blank", - last_name="Email", + first_name="", + last_name="", email=None, ) cls.other_email_user = User.objects.create_user( @@ -550,11 +551,18 @@ class GoogleOAuthApiTests(APITestCase): self.assertIn("accounts.google.com", response["Location"]) self.assertIn("state=", response["Location"]) + @patch("apps.users.services.google_oauth.requests.get") @patch("apps.users.api.views.exchange_code_for_google_profile") def test_google_callback_redirects_with_authenticated_flow_for_linked_account( self, exchange_code_for_google_profile, + requests_get, ): + avatar_response = Mock() + avatar_response.content = b"avatar-bytes" + avatar_response.headers = {"Content-Type": "image/png"} + avatar_response.raise_for_status.return_value = None + requests_get.return_value = avatar_response exchange_code_for_google_profile.return_value = GoogleProfile( provider_user_id="google-sub-1", email="linked@example.com", @@ -589,6 +597,11 @@ class GoogleOAuthApiTests(APITestCase): self.assertEqual(flow_response.data["status"], "authenticated") self.assertIn("access", flow_response.data) self.assertIn("refresh", flow_response.data) + self.linked_user.refresh_from_db() + self.assertEqual(self.linked_user.email, "linked@example.com") + self.assertEqual(self.linked_user.first_name, "Google") + self.assertEqual(self.linked_user.last_name, "Linked") + self.assertTrue(bool(self.linked_user.profile_picture)) @patch("apps.users.api.views.exchange_code_for_google_profile") def test_google_callback_redirects_with_mobile_collection_flow_for_new_account( @@ -720,7 +733,13 @@ class GoogleOAuthApiTests(APITestCase): self.assertEqual(response.data["code"], "google_email_mobile_conflict") self.assertEqual(response.data["mobile_hint"], "09*****0002") - def test_google_complete_new_mobile_creates_user_and_link(self): + @patch("apps.users.services.google_oauth.requests.get") + def test_google_complete_new_mobile_creates_user_and_link(self, requests_get): + avatar_response = Mock() + avatar_response.content = b"avatar-bytes" + avatar_response.headers = {"Content-Type": "image/png"} + avatar_response.raise_for_status.return_value = None + requests_get.return_value = avatar_response cache.set( "google_oauth_flow:new-flow", { @@ -751,6 +770,10 @@ class GoogleOAuthApiTests(APITestCase): self.assertEqual(response.data["status"], "authenticated") created_user = User.objects.get(mobile="09125550009") self.assertFalse(created_user.has_usable_password()) + self.assertEqual(created_user.email, "created@example.com") + self.assertEqual(created_user.first_name, "Created") + self.assertEqual(created_user.last_name, "User") + self.assertTrue(bool(created_user.profile_picture)) self.assertTrue( UserSocialAccount.objects.filter( user=created_user, @@ -858,9 +881,15 @@ class GoogleOAuthApiTests(APITestCase): self.assertEqual(response.data["access"], "a") verify_google_claim.assert_called_once_with(flow="claim-flow", code="12345") + @patch("apps.users.services.google_oauth.requests.get") @patch("apps.users.services.google_oauth.get_tokens_for_user") - def test_google_claim_verify_sets_blank_user_email_from_google(self, get_tokens_for_user): + def test_google_claim_verify_sets_blank_user_email_from_google(self, get_tokens_for_user, requests_get): get_tokens_for_user.return_value = {"access": "a", "refresh": "r"} + avatar_response = Mock() + avatar_response.content = b"avatar-bytes" + avatar_response.headers = {"Content-Type": "image/png"} + avatar_response.raise_for_status.return_value = None + requests_get.return_value = avatar_response cache.set( "google_oauth_flow:claim-verify-flow", { @@ -897,6 +926,9 @@ class GoogleOAuthApiTests(APITestCase): self.assertEqual(response.data["status"], "authenticated") self.blank_email_user.refresh_from_db() self.assertEqual(self.blank_email_user.email, "claimed@example.com") + self.assertEqual(self.blank_email_user.first_name, "Claimed") + self.assertEqual(self.blank_email_user.last_name, "User") + self.assertTrue(bool(self.blank_email_user.profile_picture)) self.assertTrue( UserSocialAccount.objects.filter( user=self.blank_email_user,