fix(users): trace google oauth redirect mismatches
This commit is contained in:
@@ -1,10 +1,10 @@
|
|||||||
from __future__ import annotations
|
from __future__ import annotations
|
||||||
|
|
||||||
|
import logging
|
||||||
import secrets
|
import secrets
|
||||||
from dataclasses import asdict, dataclass, is_dataclass
|
from dataclasses import asdict, dataclass, is_dataclass
|
||||||
from typing import Any
|
from typing import Any
|
||||||
from urllib.parse import urlencode
|
from urllib.parse import urlencode, urlparse
|
||||||
from urllib.parse import urlparse
|
|
||||||
|
|
||||||
import requests
|
import requests
|
||||||
from django.conf import settings
|
from django.conf import settings
|
||||||
@@ -16,7 +16,6 @@ from apps.users.email_identity import mask_mobile, normalize_email_identity
|
|||||||
from apps.users.models import User, UserSocialAccount
|
from apps.users.models import User, UserSocialAccount
|
||||||
from apps.users.services.auth import generate_and_send_otp, get_tokens_for_user
|
from apps.users.services.auth import generate_and_send_otp, get_tokens_for_user
|
||||||
|
|
||||||
|
|
||||||
GOOGLE_AUTH_URL = "https://accounts.google.com/o/oauth2/v2/auth"
|
GOOGLE_AUTH_URL = "https://accounts.google.com/o/oauth2/v2/auth"
|
||||||
GOOGLE_TOKEN_URL = "https://oauth2.googleapis.com/token"
|
GOOGLE_TOKEN_URL = "https://oauth2.googleapis.com/token"
|
||||||
GOOGLE_USERINFO_URL = "https://openidconnect.googleapis.com/v1/userinfo"
|
GOOGLE_USERINFO_URL = "https://openidconnect.googleapis.com/v1/userinfo"
|
||||||
@@ -27,6 +26,8 @@ GOOGLE_FLOW_TTL_SECONDS = 900
|
|||||||
GOOGLE_STATE_CACHE_PREFIX = "google_oauth_state"
|
GOOGLE_STATE_CACHE_PREFIX = "google_oauth_state"
|
||||||
GOOGLE_FLOW_CACHE_PREFIX = "google_oauth_flow"
|
GOOGLE_FLOW_CACHE_PREFIX = "google_oauth_flow"
|
||||||
|
|
||||||
|
logger = logging.getLogger(__name__)
|
||||||
|
|
||||||
|
|
||||||
class GoogleOAuthFlowError(APIException):
|
class GoogleOAuthFlowError(APIException):
|
||||||
status_code = 409
|
status_code = 409
|
||||||
@@ -305,6 +306,16 @@ def exchange_code_for_google_profile(code: str) -> GoogleProfile:
|
|||||||
token_response.raise_for_status()
|
token_response.raise_for_status()
|
||||||
token_payload = token_response.json()
|
token_payload = token_response.json()
|
||||||
except requests.RequestException as exc:
|
except requests.RequestException as exc:
|
||||||
|
response = getattr(exc, "response", None)
|
||||||
|
logger.warning(
|
||||||
|
"Google token exchange failed",
|
||||||
|
extra={
|
||||||
|
"google_status_code": getattr(response, "status_code", None),
|
||||||
|
"google_response_text": getattr(response, "text", "")[:1000] if response is not None else "",
|
||||||
|
"google_redirect_uri": getattr(settings, "GOOGLE_OAUTH_REDIRECT_URI", ""),
|
||||||
|
},
|
||||||
|
exc_info=True,
|
||||||
|
)
|
||||||
raise ValidationError({"detail": "Google token exchange failed."}) from exc
|
raise ValidationError({"detail": "Google token exchange failed."}) from exc
|
||||||
|
|
||||||
access_token = token_payload.get("access_token")
|
access_token = token_payload.get("access_token")
|
||||||
@@ -320,6 +331,15 @@ def exchange_code_for_google_profile(code: str) -> GoogleProfile:
|
|||||||
userinfo_response.raise_for_status()
|
userinfo_response.raise_for_status()
|
||||||
userinfo = userinfo_response.json()
|
userinfo = userinfo_response.json()
|
||||||
except requests.RequestException as exc:
|
except requests.RequestException as exc:
|
||||||
|
response = getattr(exc, "response", None)
|
||||||
|
logger.warning(
|
||||||
|
"Google user profile lookup failed",
|
||||||
|
extra={
|
||||||
|
"google_status_code": getattr(response, "status_code", None),
|
||||||
|
"google_response_text": getattr(response, "text", "")[:1000] if response is not None else "",
|
||||||
|
},
|
||||||
|
exc_info=True,
|
||||||
|
)
|
||||||
raise ValidationError({"detail": "Google user profile lookup failed."}) from exc
|
raise ValidationError({"detail": "Google user profile lookup failed."}) from exc
|
||||||
|
|
||||||
provider_user_id = userinfo.get("sub", "")
|
provider_user_id = userinfo.get("sub", "")
|
||||||
@@ -431,7 +451,10 @@ def complete_google_signup(flow: str, mobile: str) -> dict[str, Any]:
|
|||||||
user=existing_mobile_user,
|
user=existing_mobile_user,
|
||||||
mobile=normalized_mobile,
|
mobile=normalized_mobile,
|
||||||
resolution="existing_mobile_claim",
|
resolution="existing_mobile_claim",
|
||||||
detail="Existing mobile account found. Verify ownership to attach Google and set the verified email address.",
|
detail=(
|
||||||
|
"Existing mobile account found. Verify ownership to attach "
|
||||||
|
"Google and set the verified email address."
|
||||||
|
),
|
||||||
)
|
)
|
||||||
update_google_flow(flow, claim_payload)
|
update_google_flow(flow, claim_payload)
|
||||||
return _build_public_google_flow_payload(claim_payload)
|
return _build_public_google_flow_payload(claim_payload)
|
||||||
|
|||||||
@@ -1,18 +1,22 @@
|
|||||||
from io import StringIO
|
from io import StringIO
|
||||||
from unittest.mock import Mock, patch
|
from unittest.mock import Mock, patch
|
||||||
|
from urllib.parse import parse_qs, urlparse
|
||||||
|
|
||||||
from django.conf import settings
|
from django.conf import settings
|
||||||
from django.core.cache import cache
|
from django.core.cache import cache
|
||||||
from django.core.management import call_command
|
from django.core.management import call_command
|
||||||
from django.db import IntegrityError
|
from django.db import IntegrityError
|
||||||
from django.test import override_settings
|
from django.test import override_settings
|
||||||
from rest_framework.test import APIRequestFactory
|
|
||||||
from rest_framework import status
|
from rest_framework import status
|
||||||
from rest_framework.test import APITestCase
|
from rest_framework.test import APIRequestFactory, APITestCase
|
||||||
|
|
||||||
from apps.users.api.views import RegisterWithPasswordView
|
from apps.users.api.views import RegisterWithPasswordView
|
||||||
from apps.users.models import User, UserSocialAccount
|
from apps.users.models import User, UserSocialAccount
|
||||||
from apps.users.services.google_oauth import GoogleProfile
|
from apps.users.services.google_oauth import (
|
||||||
|
GoogleProfile,
|
||||||
|
build_google_authorization_url,
|
||||||
|
exchange_code_for_google_profile,
|
||||||
|
)
|
||||||
|
|
||||||
|
|
||||||
class UserApiViewTests(APITestCase):
|
class UserApiViewTests(APITestCase):
|
||||||
@@ -551,6 +555,42 @@ class GoogleOAuthApiTests(APITestCase):
|
|||||||
self.assertIn("accounts.google.com", response["Location"])
|
self.assertIn("accounts.google.com", response["Location"])
|
||||||
self.assertIn("state=", response["Location"])
|
self.assertIn("state=", response["Location"])
|
||||||
|
|
||||||
|
@patch("apps.users.services.google_oauth.requests.get")
|
||||||
|
@patch("apps.users.services.google_oauth.requests.post")
|
||||||
|
def test_google_token_exchange_uses_the_same_configured_redirect_uri_as_authorization_url(
|
||||||
|
self,
|
||||||
|
requests_post,
|
||||||
|
requests_get,
|
||||||
|
):
|
||||||
|
auth_url = build_google_authorization_url()
|
||||||
|
parsed_auth_url = urlparse(auth_url)
|
||||||
|
auth_redirect_uri = parse_qs(parsed_auth_url.query)["redirect_uri"][0]
|
||||||
|
|
||||||
|
token_response = Mock()
|
||||||
|
token_response.raise_for_status.return_value = None
|
||||||
|
token_response.json.return_value = {"access_token": "google-access-token"}
|
||||||
|
requests_post.return_value = token_response
|
||||||
|
|
||||||
|
userinfo_response = Mock()
|
||||||
|
userinfo_response.raise_for_status.return_value = None
|
||||||
|
userinfo_response.json.return_value = {
|
||||||
|
"sub": "google-sub-redirect-uri",
|
||||||
|
"email": "redirect@example.com",
|
||||||
|
"email_verified": True,
|
||||||
|
"given_name": "Redirect",
|
||||||
|
"family_name": "Uri",
|
||||||
|
"picture": "https://example.com/avatar.png",
|
||||||
|
}
|
||||||
|
requests_get.return_value = userinfo_response
|
||||||
|
|
||||||
|
exchange_code_for_google_profile("google-auth-code")
|
||||||
|
|
||||||
|
self.assertEqual(
|
||||||
|
requests_post.call_args.kwargs["data"]["redirect_uri"],
|
||||||
|
auth_redirect_uri,
|
||||||
|
)
|
||||||
|
self.assertEqual(auth_redirect_uri, settings.GOOGLE_OAUTH_REDIRECT_URI)
|
||||||
|
|
||||||
@patch("apps.users.services.google_oauth.requests.get")
|
@patch("apps.users.services.google_oauth.requests.get")
|
||||||
@patch("apps.users.api.views.exchange_code_for_google_profile")
|
@patch("apps.users.api.views.exchange_code_for_google_profile")
|
||||||
def test_google_callback_redirects_with_authenticated_flow_for_linked_account(
|
def test_google_callback_redirects_with_authenticated_flow_for_linked_account(
|
||||||
@@ -995,7 +1035,7 @@ class GoogleOAuthAuditCommandTests(APITestCase):
|
|||||||
password="secret123",
|
password="secret123",
|
||||||
email="owner@example.com",
|
email="owner@example.com",
|
||||||
)
|
)
|
||||||
other_user = User.objects.create_user(
|
User.objects.create_user(
|
||||||
mobile="09126660002",
|
mobile="09126660002",
|
||||||
password="secret123",
|
password="secret123",
|
||||||
email="shared@example.com",
|
email="shared@example.com",
|
||||||
|
|||||||
Reference in New Issue
Block a user