Skip to content

Commit

Permalink
Add GitHub App OAuth provider (#12020)
Browse files Browse the repository at this point in the history
Extracted from readthedocs/common#259. The
provider is not exposed to users yet, and we allow only staff users to
use it (manually going to /accounts/githubapp/login).

There are steps for ops team and Eric or Anthony to do:

- Create a new GH app from
https://github.com/organizations/readthedocs/settings/apps/new (the name
will be used when we do actions as the installation, like when creating
a comment).
- Callback URL should be
https://app.readthedocs.org/accounts/githubapp/login/callback/
- Keep marked "Expire user authorization tokens"
- Don't active the webhook, since we aren't going to use it yet.
- Permissions (can be updated later if required):
- Repository permissions: Commit statuses (read and write, so we can
create commit statuses), Contents (read only, so we can clone repos with
a token), Metadata (read only, so we read the repo collaborators), Pull
requests (read and write, so we can post a comment on PRs in the
future).
- Organization permissions: Members (read only so we can read the
organization members)
- Account permissions: Email addresses (read only, so allauth can fetch
all verified emails)
- Subscribe to events (can be updated later if required): Installation
target, Member, Organization, Membership, Pull request, Push,
Repository.
- Where can this GitHub App be installed?: any account
- Copy the client ID and client secret into ops repo for the githubapp
provider, we can skip setting a webhook secret and private key, as they
won't be used for now.

Same process for the app for .com.
  • Loading branch information
stsewd authored Feb 26, 2025
1 parent 092e2e5 commit d02bfd3
Show file tree
Hide file tree
Showing 10 changed files with 295 additions and 3 deletions.
2 changes: 1 addition & 1 deletion common
4 changes: 3 additions & 1 deletion docs/dev/settings.rst
Original file line number Diff line number Diff line change
Expand Up @@ -158,6 +158,8 @@ providers using the following environment variables:

.. envvar:: RTD_SOCIALACCOUNT_PROVIDERS_GITHUB_CLIENT_ID
.. envvar:: RTD_SOCIALACCOUNT_PROVIDERS_GITHUB_SECRET
.. envvar:: RTD_SOCIALACCOUNT_PROVIDERS_GITHUBAPP_CLIENT_ID
.. envvar:: RTD_SOCIALACCOUNT_PROVIDERS_GITHUBAPP_SECRET
.. envvar:: RTD_SOCIALACCOUNT_PROVIDERS_GITLAB_CLIENT_ID
.. envvar:: RTD_SOCIALACCOUNT_PROVIDERS_GITLAB_SECRET
.. envvar:: RTD_SOCIALACCOUNT_PROVIDERS_BITBUCKET_OAUTH2_CLIENT_ID
Expand All @@ -179,4 +181,4 @@ Ethical Ads variables

The following variables are required to use ``ethicalads`` in dev:

.. envvar:: RTD_USE_PROMOS
.. envvar:: RTD_USE_PROMOS
Empty file.
17 changes: 17 additions & 0 deletions readthedocs/allauth/providers/githubapp/provider.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
from allauth.socialaccount.providers.github.provider import GitHubProvider

from readthedocs.allauth.providers.githubapp.views import GitHubAppOAuth2Adapter


class GitHubAppProvider(GitHubProvider):
"""
Provider for GitHub App.
We subclass the GitHubProvider to have two separate providers for the GitHub OAuth App and the GitHub App.
"""

id = "githubapp"
oauth2_adapter_class = GitHubAppOAuth2Adapter


provider_classes = [GitHubAppProvider]
7 changes: 7 additions & 0 deletions readthedocs/allauth/providers/githubapp/urls.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
"""Copied from allauth.socialaccount.providers.github.urls."""

from allauth.socialaccount.providers.oauth2.urls import default_urlpatterns

from readthedocs.allauth.providers.githubapp.provider import GitHubAppProvider

urlpatterns = default_urlpatterns(GitHubAppProvider)
15 changes: 15 additions & 0 deletions readthedocs/allauth/providers/githubapp/views.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
"""Copied from allauth.socialaccount.providers.github.views."""

from allauth.socialaccount.providers.github.views import GitHubOAuth2Adapter
from allauth.socialaccount.providers.oauth2.views import (
OAuth2CallbackView,
OAuth2LoginView,
)


class GitHubAppOAuth2Adapter(GitHubOAuth2Adapter):
provider_id = "githubapp"


oauth2_login = OAuth2LoginView.adapter_view(GitHubAppOAuth2Adapter)
oauth2_callback = OAuth2CallbackView.adapter_view(GitHubAppOAuth2Adapter)
68 changes: 68 additions & 0 deletions readthedocs/core/adapters.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,17 @@

import structlog
from allauth.account.adapter import DefaultAccountAdapter
from allauth.account.adapter import get_adapter as get_account_adapter
from allauth.exceptions import ImmediateHttpResponse
from allauth.socialaccount.adapter import DefaultSocialAccountAdapter
from allauth.socialaccount.models import SocialAccount
from allauth.socialaccount.providers.github.provider import GitHubProvider
from django.contrib import messages
from django.http import HttpResponseRedirect
from django.urls import reverse
from django.utils.encoding import force_str

from readthedocs.allauth.providers.githubapp.provider import GitHubAppProvider
from readthedocs.core.utils import send_email_from_object
from readthedocs.invitations.models import Invitation

Expand Down Expand Up @@ -54,6 +62,10 @@ def save_user(self, request, user, form, commit=True):

class SocialAccountAdapter(DefaultSocialAccountAdapter):
def pre_social_login(self, request, sociallogin):
self._filter_email_addresses(sociallogin)
self._connect_github_app_to_existing_github_account(request, sociallogin)

def _filter_email_addresses(self, sociallogin):
"""
Remove all email addresses except the primary one.
Expand All @@ -65,3 +77,59 @@ def pre_social_login(self, request, sociallogin):
sociallogin.email_addresses = [
email for email in sociallogin.email_addresses if email.primary
]

def _connect_github_app_to_existing_github_account(self, request, sociallogin):
"""
Connect a GitHub App (new integration) account to an existing GitHub account (old integration).
When a user signs up with the GitHub App we check if there is an existing GitHub account,
and if it belongs to the same user, we connect the accounts instead of creating a new one.
"""
provider = sociallogin.account.get_provider()

# If the provider is not GitHub App, nothing to do.
if provider.id != GitHubAppProvider.id:
return

# If the user already signed up with the GitHub App, nothing to do.
if sociallogin.is_existing:
return

social_account = SocialAccount.objects.filter(
provider=GitHubProvider.id,
uid=sociallogin.account.uid,
).first()

# If there is an existing GH account, we check if that user can use the GH App,
# otherwise we check for the current user.
user_to_check = social_account.user if social_account else request.user
if not self._can_use_github_app(user_to_check):
raise ImmediateHttpResponse(HttpResponseRedirect(reverse("account_login")))

# If there isn't an existing GH account, nothing to do,
# just let allauth create the new account.
if not social_account:
return

# If the user is logged in, and the GH OAuth account belongs to
# a different user, we should not connect the accounts,
# this is the same as trying to connect an existing GH account to another user.
if request.user.is_authenticated and request.user != social_account.user:
message_template = "socialaccount/messages/account_connected_other.txt"
get_account_adapter(request).add_message(
request=request,
level=messages.ERROR,
message_template=message_template,
)
url = reverse("socialaccount_connections")
raise ImmediateHttpResponse(HttpResponseRedirect(url))

sociallogin.connect(request, social_account.user)

def _can_use_github_app(self, user):
"""
Check if the user can use the GitHub App.
Only staff users can use the GitHub App for now.
"""
return user.is_staff
174 changes: 174 additions & 0 deletions readthedocs/core/tests/test_adapters.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,174 @@
from unittest import mock

from allauth.exceptions import ImmediateHttpResponse
from allauth.socialaccount.adapter import get_adapter as get_social_account_adapter
from allauth.socialaccount.models import SocialAccount, SocialLogin
from allauth.socialaccount.providers.github.provider import GitHubProvider
from django.contrib.auth.models import AnonymousUser, User
from django.test import TestCase
from django_dynamic_fixture import get

from readthedocs.allauth.providers.githubapp.provider import GitHubAppProvider


class SocialAdapterTest(TestCase):
def setUp(self):
self.user = get(User, username="test")
self.adapter = get_social_account_adapter()

def test_dont_allow_using_githubapp_for_non_staff_users(self):
assert not SocialAccount.objects.filter(provider=GitHubAppProvider.id).exists()

# Anonymous user
request = mock.MagicMock(user=AnonymousUser())
sociallogin = SocialLogin(
user=User(email="me@example.com"),
account=SocialAccount(provider=GitHubAppProvider.id),
)
with self.assertRaises(ImmediateHttpResponse) as exc:
self.adapter.pre_social_login(request, sociallogin)
self.assertEqual(exc.exception.response.status_code, 302)

assert not SocialAccount.objects.filter(provider=GitHubAppProvider.id).exists()

# Existing non-staff user
assert not self.user.is_staff
request = mock.MagicMock(user=self.user)
sociallogin = SocialLogin(
user=User(email="me@example.com"),
account=SocialAccount(provider=GitHubAppProvider.id),
)
with self.assertRaises(ImmediateHttpResponse) as exc:
self.adapter.pre_social_login(request, sociallogin)
self.assertEqual(exc.exception.response.status_code, 302)
assert not self.user.socialaccount_set.filter(
provider=GitHubAppProvider.id
).exists()

def test_allow_using_githubapp_for_staff_users(self):
self.user.is_staff = True
self.user.save()
assert self.user.is_staff

request = mock.MagicMock(user=self.user)
sociallogin = SocialLogin(
user=User(email="me@example.com"),
account=SocialAccount(provider=GitHubAppProvider.id),
)
self.adapter.pre_social_login(request, sociallogin)
# No exception raised, but the account is not created, as that is done in another step by allauth.
assert not self.user.socialaccount_set.filter(
provider=GitHubAppProvider.id
).exists()

def test_connect_to_existing_github_account_from_staff_user(self):
self.user.is_staff = True
self.user.save()
assert self.user.is_staff
assert not self.user.socialaccount_set.filter(
provider=GitHubAppProvider.id
).exists()

github_account = get(
SocialAccount,
provider=GitHubProvider.id,
uid="1234",
user=self.user,
)

request = mock.MagicMock(user=AnonymousUser())
sociallogin = SocialLogin(
user=User(email="me@example.com"),
account=SocialAccount(
provider=GitHubAppProvider.id, uid=github_account.uid
),
)
self.adapter.pre_social_login(request, sociallogin)
# A new user is not created, but the existing user is connected to the GitHub App.
assert self.user.socialaccount_set.filter(
provider=GitHubAppProvider.id
).exists()

def test_connect_to_existing_github_account_from_staff_user_logged_in(self):
self.user.is_staff = True
self.user.save()
assert self.user.is_staff
assert not self.user.socialaccount_set.filter(
provider=GitHubAppProvider.id
).exists()

github_account = get(
SocialAccount,
provider=GitHubProvider.id,
uid="1234",
user=self.user,
)

request = mock.MagicMock(user=self.user)
sociallogin = SocialLogin(
user=User(email="me@example.com"),
account=SocialAccount(
provider=GitHubAppProvider.id, uid=github_account.uid
),
)
self.adapter.pre_social_login(request, sociallogin)
# A new user is not created, but the existing user is connected to the GitHub App.
assert self.user.socialaccount_set.filter(
provider=GitHubAppProvider.id
).exists()

def test_dont_connect_to_existing_github_account_if_user_is_logged_in_with_different_account(
self,
):
self.user.is_staff = True
self.user.save()
assert self.user.is_staff
assert not self.user.socialaccount_set.filter(
provider=GitHubAppProvider.id
).exists()

github_account = get(
SocialAccount,
provider=GitHubProvider.id,
uid="1234",
user=self.user,
)

another_user = get(User, username="another")
request = mock.MagicMock(user=another_user)
sociallogin = SocialLogin(
user=User(email="me@example.com"),
account=SocialAccount(
provider=GitHubAppProvider.id, uid=github_account.uid
),
)
with self.assertRaises(ImmediateHttpResponse) as exc:
self.adapter.pre_social_login(request, sociallogin)
self.assertEqual(exc.exception.response.status_code, 302)
assert not self.user.socialaccount_set.filter(
provider=GitHubAppProvider.id
).exists()
assert not another_user.socialaccount_set.filter(
provider=GitHubAppProvider.id
).exists()

def test_allow_existing_githubapp_accounts_to_login(self):
assert not self.user.is_staff
githubapp_account = get(
SocialAccount,
provider=GitHubAppProvider.id,
uid="1234",
user=self.user,
)

request = mock.MagicMock(user=AnonymousUser())
sociallogin = SocialLogin(
user=self.user,
account=githubapp_account,
)
self.adapter.pre_social_login(request, sociallogin)

self.user.is_staff = True
self.user.save()
assert self.user.is_staff
self.adapter.pre_social_login(request, sociallogin)
8 changes: 8 additions & 0 deletions readthedocs/settings/base.py
Original file line number Diff line number Diff line change
Expand Up @@ -297,6 +297,7 @@ def INSTALLED_APPS(self): # noqa
"allauth.account",
"allauth.socialaccount",
"allauth.socialaccount.providers.github",
"readthedocs.allauth.providers.githubapp",
"allauth.socialaccount.providers.gitlab",
"allauth.socialaccount.providers.bitbucket_oauth2",
"allauth.mfa",
Expand Down Expand Up @@ -713,6 +714,13 @@ def DOCKER_LIMITS(self):
"repo:status",
],
},
"githubapp": {
"APPS": [
{"client_id": "123", "secret": "456", "key": ""},
],
# Scope is determined by the GitHub App permissions.
"SCOPE": [],
},
"gitlab": {
"APPS": [
{"client_id": "123", "secret": "456", "key": ""},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,9 @@
{% comment %}
- OpenID is not implemented.
- SAML is handled in another view, we don't want to list all SAML integrations here.
- GitHub App is not exposed to users yet.
{% endcomment %}
{% if provider.id != 'saml' %}
{% if provider.id != 'saml' and provider.id != 'githubapp' %}
{% if allowed_providers and provider.id in allowed_providers or not allowed_providers %}
<li>
{# Bitbucket doesn't allow more than one callback URL for their OAuth app, so we are redirecting users to the new dashboard for now. #}
Expand Down

0 comments on commit d02bfd3

Please sign in to comment.