Skip to content

Commit 8d6a42c

Browse files
authored
GitHub App: block GH app users to re-connect to old OAuth app (#12175)
This prevents users already using the new GH app from creating another account by mistake with the old app.
1 parent 4e5e0f4 commit 8d6a42c

File tree

2 files changed

+97
-0
lines changed

2 files changed

+97
-0
lines changed

readthedocs/core/adapters.py

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -64,6 +64,7 @@ def save_user(self, request, user, form, commit=True):
6464
class SocialAccountAdapter(DefaultSocialAccountAdapter):
6565
def pre_social_login(self, request, sociallogin):
6666
self._filter_email_addresses(sociallogin)
67+
self._block_use_of_old_github_oauth_app(request, sociallogin)
6768
self._connect_github_app_to_existing_github_account(request, sociallogin)
6869

6970
def _filter_email_addresses(self, sociallogin):
@@ -134,3 +135,42 @@ def _can_use_github_app(self, user):
134135
Only staff users can use the GitHub App for now.
135136
"""
136137
return user.is_staff
138+
139+
def _block_use_of_old_github_oauth_app(self, request, sociallogin):
140+
"""
141+
Block the use of the old GitHub OAuth app if the user is already using the new GitHub App.
142+
143+
This is a temporary measure to block the use of the old GitHub OAuth app
144+
until we switch our login to always use the new GitHub App.
145+
146+
If the user has its account still connected to the old GitHub OAuth app,
147+
we allow them to use it, since there is no difference between using the two apps
148+
for logging in.
149+
"""
150+
provider = sociallogin.account.get_provider()
151+
152+
# If the provider is not GitHub, nothing to do.
153+
if provider.id != GitHubProvider.id:
154+
return
155+
156+
# If the user is still using the old GitHub OAuth app, nothing to do.
157+
if sociallogin.is_existing:
158+
return
159+
160+
has_gh_app_social_account = SocialAccount.objects.filter(
161+
provider=GitHubAppProvider.id,
162+
uid=sociallogin.account.uid,
163+
).exists()
164+
165+
# If there is no existing GitHub App account, nothing to do.
166+
if not has_gh_app_social_account:
167+
return
168+
169+
# Show a warning to the user and redirect them to the GitHub App login page.
170+
messages.warning(
171+
request,
172+
"You already migrated from our old GitHub OAuth app. "
173+
"Click below to sign in with the new GitHub App.",
174+
)
175+
url = reverse("githubapp_login")
176+
raise ImmediateHttpResponse(HttpResponseRedirect(url))

readthedocs/core/tests/test_adapters.py

Lines changed: 57 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -172,3 +172,60 @@ def test_allow_existing_githubapp_accounts_to_login(self):
172172
self.user.save()
173173
assert self.user.is_staff
174174
self.adapter.pre_social_login(request, sociallogin)
175+
176+
def test_dont_allow_using_old_github_app(self):
177+
github_app_account = get(
178+
SocialAccount,
179+
provider=GitHubAppProvider.id,
180+
uid="1234",
181+
user=self.user,
182+
)
183+
184+
request = mock.MagicMock(user=AnonymousUser())
185+
sociallogin = SocialLogin(
186+
user=User(email="[email protected]"),
187+
account=SocialAccount(
188+
provider=GitHubProvider.id, uid=github_app_account.uid
189+
),
190+
)
191+
with self.assertRaises(ImmediateHttpResponse) as exc:
192+
self.adapter.pre_social_login(request, sociallogin)
193+
194+
assert self.user.socialaccount_set.filter(
195+
provider=GitHubAppProvider.id
196+
).exists()
197+
assert not self.user.socialaccount_set.filter(
198+
provider=GitHubProvider.id
199+
).exists()
200+
201+
response = exc.exception.response
202+
assert response.status_code == 302
203+
assert response.url == "/accounts/githubapp/login/"
204+
205+
def test_allow_using_old_github_app(self):
206+
get(
207+
SocialAccount,
208+
provider=GitHubAppProvider.id,
209+
uid="1234",
210+
user=self.user,
211+
)
212+
github_account = get(
213+
SocialAccount,
214+
provider=GitHubProvider.id,
215+
uid="1234",
216+
user=self.user,
217+
)
218+
219+
request = mock.MagicMock(user=AnonymousUser())
220+
sociallogin = SocialLogin(
221+
user=self.user,
222+
account=github_account,
223+
)
224+
self.adapter.pre_social_login(request, sociallogin)
225+
226+
assert self.user.socialaccount_set.filter(
227+
provider=GitHubAppProvider.id
228+
).exists()
229+
assert self.user.socialaccount_set.filter(
230+
provider=GitHubProvider.id
231+
).exists()

0 commit comments

Comments
 (0)