GitHub OAuth Security Enhancement

src/sentry/integrations/github/integration.py

@@ -4,15 +4,19 @@
 import re
 from collections.abc import Collection, Mapping, Sequence
 from typing import Any
+from urllib.parse import parse_qsl
 
 from django.http import HttpResponse
+from django.urls import reverse
 from django.utils.text import slugify
 from django.utils.translation import gettext_lazy as _
 from rest_framework.request import Request
 
 from sentry import features, options
 from sentry.api.utils import generate_organization_url
 from sentry.constants import ObjectStatus
+from sentry.http import safe_urlopen, safe_urlread
+from sentry.identity.github import GitHubIdentityProvider, get_user_info
 from sentry.integrations import (
     FeatureDescription,
     IntegrationFeatures,
@@ -35,6 +39,7 @@
 from sentry.tasks.integrations.github.constants import RATE_LIMITED_MESSAGE
 from sentry.tasks.integrations.link_all_repos import link_all_repos
 from sentry.utils import metrics
+from sentry.utils.http import absolute_uri
 from sentry.web.helpers import render_to_response
 
 from .client import GitHubAppsClient, GitHubClientMixin
@@ -108,6 +113,9 @@
 ERR_INTEGRATION_EXISTS_ON_ANOTHER_ORG = _(
     "It seems that your GitHub account has been installed on another Sentry organization. Please uninstall and try again."
 )
+ERR_INTEGRATION_INVALID_INSTALLATION_REQUEST = _(
+    "We could not verify the authenticity of the installation request. We recommend restarting the installation process."
+)
 ERR_INTEGRATION_PENDING_DELETION = _(
     "It seems that your Sentry organization has an installation pending deletion. Please wait ~15min for the uninstall to complete and try again."
 )
@@ -118,6 +126,32 @@ def build_repository_query(metadata: Mapping[str, Any], name: str, query: str) -
     return f"{account_type}:{name} {query}".encode()
 
 
+def error(
+    request,
+    org,
+    error_short="Invalid installation request.",
+    error_long=ERR_INTEGRATION_INVALID_INSTALLATION_REQUEST,
+):
+    return render_to_response(
+        "sentry/integrations/github-integration-failed.html",
+        context={
+            "error": error_long,
+            "payload": {
+                "success": False,
+                "data": {"error": _(error_short)},
+            },
+            "document_origin": get_document_origin(org),
+        },
+        request=request,
+    )
+
+
+def get_document_origin(org) -> str:
+    if org and features.has("organizations:customer-domains", org.organization):
+        return f'"{generate_organization_url(org.organization.slug)}"'
+    return "document.origin"
+
+
 # Github App docs and list of available endpoints
 # https://docs.github.com/en/rest/apps/installations
 # https://docs.github.com/en/rest/overview/endpoints-available-for-github-apps
@@ -307,7 +341,7 @@ def post_install(
         )
 
     def get_pipeline_views(self) -> Sequence[PipelineView]:
-        return [GitHubInstallation()]
+        return [OAuthLoginView(), GitHubInstallation()]
 
     def get_installation_info(self, installation_id: str) -> Mapping[str, Any]:
         client = self.get_client()
@@ -352,15 +386,77 @@ def setup(self) -> None:
         )
 
 
-class GitHubInstallation(PipelineView):
+class OAuthLoginView(PipelineView):
+    def dispatch(self, request: Request, pipeline) -> HttpResponse:
+        self.determine_active_organization(request)
+
+        ghip = GitHubIdentityProvider()
+        github_client_id = ghip.get_oauth_client_id()
+        github_client_secret = ghip.get_oauth_client_secret()
+
+        installation_id = request.GET.get("installation_id")
+        if installation_id:
+            pipeline.bind_state("installation_id", installation_id)
+
+        if not request.GET.get("state"):
+            state = pipeline.signature
+
+            redirect_uri = absolute_uri(
+                reverse("sentry-extension-setup", kwargs={"provider_id": "github"})
+            )
+            return self.redirect(
+                f"{ghip.get_oauth_authorize_url()}?client_id={github_client_id}&state={state}&redirect_uri={redirect_uri}"
+            )
+
+        # At this point, we are past the GitHub "authorize" step
+        if request.GET.get("state") != pipeline.signature:
+            return error(request, self.active_organization)
+
+        # similar to OAuth2CallbackView.get_token_params
+        data = {
+            "code": request.GET.get("code"),
+            "client_id": github_client_id,
+            "client_secret": github_client_secret,
+        }
+
+        # similar to OAuth2CallbackView.exchange_token
+        req = safe_urlopen(url=ghip.get_oauth_access_token_url(), data=data)
+
+        try:
+            body = safe_urlread(req).decode("utf-8")
+            payload = dict(parse_qsl(body))
+        except Exception:
+            payload = {}
+
+        if "access_token" not in payload:
+            return error(request, self.active_organization)
+
+        authenticated_user_info = get_user_info(payload["access_token"])
+        if "login" not in authenticated_user_info:
+            return error(request, self.active_organization)
+
+        pipeline.bind_state("github_authenticated_user", authenticated_user_info["login"])
+        return pipeline.next_step()
+
+
+try:
+    body = safe_urlread(req).decode("utf-8")
+    payload = dict(parse_qsl(body))
+except (UnicodeDecodeError, ValueError, OSError) as e:
+    logger.warning("GitHub OAuth token exchange failed: %s", e)
+    payload = {}
     def get_app_url(self) -> str:
         name = options.get("github-app.name")
         return f"https://github.com/apps/{slugify(name)}"
 
     def dispatch(self, request: Request, pipeline: Pipeline) -> HttpResponse:
-        if "installation_id" not in request.GET:
+        installation_id = request.GET.get(
+            "installation_id", pipeline.fetch_state("installation_id")
+        )
+        if installation_id is None:
             return self.redirect(self.get_app_url())
 
+        pipeline.bind_state("installation_id", installation_id)
         self.determine_active_organization(request)
 
         integration_pending_deletion_exists = False
@@ -374,57 +470,43 @@ def dispatch(self, request: Request, pipeline: Pipeline) -> HttpResponse:
             ).exists()
 
         if integration_pending_deletion_exists:
-            document_origin = "document.origin"
-            if self.active_organization and features.has(
-                "organizations:customer-domains", self.active_organization.organization
-            ):
-                document_origin = (
-                    f'"{generate_organization_url(self.active_organization.organization.slug)}"'
-                )
-            return render_to_response(
-                "sentry/integrations/github-integration-failed.html",
-                context={
-                    "error": ERR_INTEGRATION_PENDING_DELETION,
-                    "payload": {
-                        "success": False,
-                        "data": {"error": _("GitHub installation pending deletion.")},
-                    },
-                    "document_origin": document_origin,
-                },
-                request=request,
+            return error(
+                request,
+                self.active_organization,
+                error_short="GitHub installation pending deletion.",
+                error_long=ERR_INTEGRATION_PENDING_DELETION,
             )
 
         try:
             # We want to limit GitHub integrations to 1 organization
             installations_exist = OrganizationIntegration.objects.filter(
-                integration=Integration.objects.get(external_id=request.GET["installation_id"])
+                integration=Integration.objects.get(external_id=installation_id)
             ).exists()
 
         except Integration.DoesNotExist:
-            pipeline.bind_state("installation_id", request.GET["installation_id"])
             return pipeline.next_step()
 
         if installations_exist:
-            document_origin = "document.origin"
-            if self.active_organization and features.has(
-                "organizations:customer-domains", self.active_organization.organization
-            ):
-                document_origin = (
-                    f'"{generate_organization_url(self.active_organization.organization.slug)}"'
-                )
-            return render_to_response(
-                "sentry/integrations/github-integration-failed.html",
-                context={
-                    "error": ERR_INTEGRATION_EXISTS_ON_ANOTHER_ORG,
-                    "payload": {
-                        "success": False,
-                        "data": {"error": _("Github installed on another Sentry organization.")},
-                    },
-                    "document_origin": document_origin,
-                },
-                request=request,
+            return error(
+                request,
+                self.active_organization,
+                error_short="Github installed on another Sentry organization.",
+                error_long=ERR_INTEGRATION_EXISTS_ON_ANOTHER_ORG,
             )
 
         # OrganizationIntegration does not exist, but Integration does exist.
-        pipeline.bind_state("installation_id", request.GET["installation_id"])
+        try:
+            integration = Integration.objects.get(
+                external_id=installation_id, status=ObjectStatus.ACTIVE
+            )
+        except Integration.DoesNotExist:
+            return error(request, self.active_organization)
+
+        # Check that the authenticated GitHub user is the same as who installed the app.
+        if (
+            pipeline.fetch_state("github_authenticated_user")
+            != integration.metadata["sender"]["login"]
+        ):
+            return error(request, self.active_organization)
+
         return pipeline.next_step()

src/sentry/web/frontend/pipeline_advancer.py

@@ -16,12 +16,6 @@
 PIPELINE_CLASSES = [IntegrationPipeline, IdentityProviderPipeline]
 
 
-# GitHub apps may be installed directly from GitHub, in which case
-# they will redirect here *without* being in the pipeline. If that happens
-# redirect to the integration install org picker.
-FORWARD_INSTALL_FOR = ["github"]
-
-
 from rest_framework.request import Request
 
 
@@ -40,8 +34,11 @@ def handle(self, request: Request, provider_id: str) -> HttpResponseBase:
             if pipeline:
                 break
 
+        # GitHub apps may be installed directly from GitHub, in which case
+        # they will redirect here *without* being in the pipeline. If that happens
+        # redirect to the integration install org picker.
         if (
-            provider_id in FORWARD_INSTALL_FOR
+            provider_id == "github"
             and request.GET.get("setup_action") == "install"
             and pipeline is None
         ):