Skip to content

Commit 528024d

Browse files
authored
Fix Google SSO login via provider-agnostic user data retrieval (#597)
1 parent f01e5ec commit 528024d

File tree

2 files changed

+104
-2
lines changed

2 files changed

+104
-2
lines changed

back/users/adapter.py

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -60,7 +60,11 @@ def _get_user_role(self, raw_role_data):
6060

6161
def populate_user(self, request, sociallogin, data):
6262
user = super().populate_user(request, sociallogin, data)
63-
full_data = sociallogin.account.extra_data["id_token"]
63+
# Different providers store user data at different nesting levels in `extra_data`
64+
# (e.g., Google stores it flat, OpenID Connect nests it under "id_token").
65+
# Using get_provider_account().get_user_data() abstracts away these differences.
66+
provider_account = sociallogin.account.get_provider_account()
67+
full_data = provider_account.get_user_data()
6468

6569
if settings.OIDC_ROLE_PATH_IN_RETURN == "":
6670
# set default to OTHER

back/users/test_adapter.py

Lines changed: 99 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,40 @@
11
import pytest
22
from allauth.socialaccount.models import SocialAccount, SocialLogin
3+
from allauth.socialaccount.providers.google.provider import GoogleProvider
4+
from allauth.socialaccount.providers.openid_connect.provider import (
5+
OpenIDConnectProvider,
6+
)
37
from django.contrib.auth import get_user_model
48
from django.test import override_settings
59

610
from .adapter import SocialAccountAdapter
711

812

13+
def _ensure_provider_registered(provider_id: str, provider_class):
14+
"""
15+
Register a provider if not already registered.
16+
17+
Necessary as "django-allauth" initializes the registry before the
18+
`@override_settings` decorator is applied.
19+
"""
20+
from allauth.socialaccount.providers import registry
21+
22+
if not registry.provider_map.get(provider_id):
23+
registry.register(provider_class)
24+
25+
26+
@pytest.fixture
27+
def register_google_provider():
28+
_ensure_provider_registered("google", GoogleProvider)
29+
yield
30+
31+
32+
@pytest.fixture
33+
def register_openid_connect_provider():
34+
_ensure_provider_registered("openid_connect", OpenIDConnectProvider)
35+
yield
36+
37+
938
@pytest.mark.django_db
1039
@override_settings(OIDC_ROLE_ADMIN_PATTERN="^Administrators.*")
1140
@override_settings(OIDC_ROLE_MANAGER_PATTERN="^Manager.*")
@@ -36,7 +65,21 @@ def test_get_user_role():
3665
@override_settings(OIDC_ROLE_MANAGER_PATTERN="^Manager.*")
3766
@override_settings(OIDC_ROLE_NEW_HIRE_PATTERN="^Newhire.*")
3867
@override_settings(OIDC_ROLE_PATH_IN_RETURN="details.zoneinfo")
39-
def test_populate_user(employee_factory):
68+
@override_settings(
69+
SOCIALACCOUNT_PROVIDERS={
70+
"openid_connect": {
71+
"APPS": [
72+
{
73+
"provider_id": "test-server",
74+
"name": "Test Server",
75+
"client_id": "test-client-id",
76+
"secret": "test-secret",
77+
}
78+
]
79+
}
80+
},
81+
)
82+
def test_populate_user(employee_factory, register_openid_connect_provider):
4083
employee = employee_factory()
4184
account = SocialAccount(
4285
user=employee,
@@ -64,3 +107,58 @@ def test_populate_user(employee_factory):
64107
user.save()
65108
assert get_user_model().objects.count() == 1
66109
assert get_user_model().objects.first().role == get_user_model().Role.ADMIN
110+
111+
112+
@pytest.mark.django_db
113+
@override_settings(
114+
SOCIALACCOUNT_PROVIDERS={
115+
"google": {
116+
"APPS": [
117+
{
118+
"client_id": "test-client-id",
119+
"secret": "test-secret",
120+
"key": "dummy",
121+
}
122+
],
123+
"AUTH_PARAMS": {
124+
"access_type": "offline",
125+
},
126+
"OAUTH_PKCE_ENABLED": True,
127+
}
128+
}
129+
)
130+
def test_populate_google_user(employee_factory, register_google_provider):
131+
employee = employee_factory()
132+
account = SocialAccount(
133+
user=employee,
134+
# See: https://openid.net/specs/openid-connect-core-1_0.html#IDToken
135+
extra_data={
136+
"iss": "https://accounts.google.com",
137+
"azp": "123456789012-0e3b215c34faf2828d7bb66b5f24e992.apps.googleusercontent.com",
138+
"aud": "123456789012-0e3b215c34faf2828d7bb66b5f24e992.apps.googleusercontent.com",
139+
"sub": "123456789012345678901",
140+
"hd": "chiefonboarding.com",
141+
"email": "[email protected]",
142+
"email_verified": True,
143+
"at_hash": "-someHASH",
144+
"name": "John Do",
145+
"picture": "https://lh3.googleusercontent.com/a/...",
146+
"given_name": "John",
147+
"family_name": "Do",
148+
"iat": 1763729550,
149+
"exp": 1763733150,
150+
},
151+
)
152+
sociallogin = SocialLogin(user=employee, account=account)
153+
154+
data = {
155+
"first_name": "John",
156+
"last_name": "Do",
157+
"email": "[email protected]",
158+
}
159+
user = SocialAccountAdapter().populate_user(None, sociallogin, data)
160+
# saving happens elsewhere, so force save here
161+
user.save()
162+
assert get_user_model().objects.count() == 1
163+
# unlike `test_populate_user`, no role information is present in our data
164+
assert get_user_model().objects.first().role == get_user_model().Role.OTHER

0 commit comments

Comments
 (0)