fix(security): validate GitHub user during integration installation (#67876)

We're adding one more step in the GitHub integration installation
pipeline, namely GitHub OAuth2 authorize. This is transparent from the
UX perspective as the data exchange happens without user interaction.

The pipeline will now fail in these cases:
- If there is a mismatch between currently authenticated GitHub user
(derived from OAuth2 authorize step) and the user who installed the
GitHub app (https://github.com/apps/sentry-io)
- If there is a mismatch between `state` parameter supplied by user and
pipeline signature
- If GitHub could not generate correct `access_token` from the `code`
(wrong or attempt of re-use of `code`).

In all those cases, this error is shown:

![image](https://github.com/getsentry/sentry/assets/1127549/18923861-2ead-4cf5-adda-7738aef801d7)

src/sentry/integrations/github/integration.py

@@ -4,8 +4,10 @@ import logging
 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
@@ -13,6 +15,8 @@ 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 import migrate_repo
 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 @@ API_ERRORS = {
 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 @@ class GitHubIntegrationProvider(IntegrationProvider):
         )
 
     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,72 @@ class GitHubIntegrationProvider(IntegrationProvider):
         )
 
 
+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()
+
+
 class GitHubInstallation(PipelineView):
     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 +465,43 @@ class GitHubInstallation(PipelineView):
             ).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 @@ from sentry.web.frontend.base import BaseView
 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 @@ class PipelineAdvancerView(BaseView):
             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
         ):

tests/sentry/integrations/github/test_integration.py

@@ -12,6 +12,7 @@ import responses
 from django.urls import reverse
 
 import sentry
+from fixtures.github import INSTALLATION_EVENT_EXAMPLE
 from sentry.api.utils import generate_organization_url
 from sentry.constants import ObjectStatus
 from sentry.integrations.github import (
@@ -32,6 +33,7 @@ from sentry.shared_integrations.exceptions import ApiError
 from sentry.silo.base import SiloMode
 from sentry.testutils.cases import IntegrationTestCase
 from sentry.testutils.silo import assume_test_silo_mode, control_silo_test
+from sentry.utils import json
 from sentry.utils.cache import cache
 
 TREE_RESPONSES = {
@@ -111,6 +113,15 @@ class GitHubIntegrationTest(IntegrationTestCase):
         self.gh_org = "Test-Organization"
         pp = 1
 
+        access_token = "xxxxx-xxxxxxxxx-xxxxxxxxxx-xxxxxxxxxxxx"
+        responses.add(
+            responses.POST,
+            "https://github.com/login/oauth/access_token",
+            body=f"access_token={access_token}",
+        )
+
+        responses.add(responses.GET, self.base_url + "/user", json={"login": "octocat"})
+
         responses.add(
             responses.POST,
             self.base_url + f"/app/installations/{self.installation_id}/access_tokens",
@@ -218,6 +229,24 @@ class GitHubIntegrationTest(IntegrationTestCase):
         redirect = urlparse(resp["Location"])
         assert redirect.scheme == "https"
         assert redirect.netloc == "github.com"
+        assert redirect.path == "/login/oauth/authorize"
+        assert (
+            redirect.query
+            == "client_id=github-client-id&state=9cae5e88803f35ed7970fc131e6e65d3&redirect_uri=http://testserver/extensions/github/setup/"
+        )
+
+        resp = self.client.get(
+            "{}?{}".format(
+                self.setup_path,
+                urlencode(
+                    {"code": "12345678901234567890", "state": "9cae5e88803f35ed7970fc131e6e65d3"}
+                ),
+            )
+        )
+        assert resp.status_code == 302
+        redirect = urlparse(resp["Location"])
+        assert redirect.scheme == "https"
+        assert redirect.netloc == "github.com"
         assert redirect.path == "/apps/sentry-test-app"
 
         # App installation ID is provided
@@ -225,7 +254,7 @@ class GitHubIntegrationTest(IntegrationTestCase):
             "{}?{}".format(self.setup_path, urlencode({"installation_id": self.installation_id}))
         )
 
-        auth_header = responses.calls[0].request.headers["Authorization"]
+        auth_header = responses.calls[2].request.headers["Authorization"]
         assert auth_header == "Bearer jwt_token_1"
 
         self.assertDialogSuccess(resp)
@@ -303,8 +332,15 @@ class GitHubIntegrationTest(IntegrationTestCase):
             ),
             urlencode({"installation_id": self.installation_id}),
         )
+        self.setup_path_2 = "{}?{}".format(
+            self.setup_path,
+            urlencode(
+                {"code": "12345678901234567890", "state": "9cae5e88803f35ed7970fc131e6e65d3"}
+            ),
+        )
         with self.feature({"organizations:customer-domains": [self.organization_2.slug]}):
             resp = self.client.get(self.init_path_2)
+            resp = self.client.get(self.setup_path_2)
             self.assertTemplateUsed(resp, "sentry/integrations/github-integration-failed.html")
             assert (
                 b'{"success":false,"data":{"error":"Github installed on another Sentry organization."}}'
@@ -330,6 +366,8 @@ class GitHubIntegrationTest(IntegrationTestCase):
 
         # Try again and should be successful
         resp = self.client.get(self.init_path_2)
+        resp = self.client.get(self.setup_path_2)
+
         self.assertDialogSuccess(resp)
         integration = Integration.objects.get(external_id=self.installation_id)
         assert integration.provider == "github"
@@ -347,7 +385,73 @@ class GitHubIntegrationTest(IntegrationTestCase):
         resp = self.client.get(
             "{}?{}".format(self.setup_path, urlencode({"installation_id": self.installation_id}))
         )
-        assert b"The GitHub installation could not be found." in resp.content
+        resp = self.client.get(
+            "{}?{}".format(
+                self.setup_path,
+                urlencode(
+                    {"code": "12345678901234567890", "state": "ddd023d87a913d5226e2a882c4c4cc05"}
+                ),
+            )
+        )
+        assert b"Invalid installation request." in resp.content
+
+    @responses.activate
+    def test_github_user_mismatch(self):
+        self._stub_github()
+
+        # Emulate GitHub installation
+        init_path_1 = "{}?{}".format(
+            reverse(
+                "sentry-organization-integrations-setup",
+                kwargs={
+                    "organization_slug": self.organization.slug,
+                    "provider_id": self.provider.key,
+                },
+            ),
+            urlencode({"installation_id": self.installation_id}),
+        )
+        self.client.get(init_path_1)
+
+        webhook_event = json.loads(INSTALLATION_EVENT_EXAMPLE)
+        webhook_event["installation"]["id"] = self.installation_id
+        webhook_event["sender"]["login"] = "attacker"
+        resp = self.client.post(
+            path="/extensions/github/webhook/",
+            data=json.dumps(webhook_event),
+            content_type="application/json",
+            HTTP_X_GITHUB_EVENT="installation",
+            HTTP_X_HUB_SIGNATURE="sha1=d184e6717f8bfbcc291ebc8c0756ee446c6c9486",
+            HTTP_X_GITHUB_DELIVERY="00000000-0000-4000-8000-1234567890ab",
+        )
+        assert resp.status_code == 204
+
+        # Validate the installation user
+        user_2 = self.create_user("foo@example.com")
+        org_2 = self.create_organization(name="Rowdy Tiger", owner=user_2)
+        self.login_as(user_2)
+        init_path_2 = "{}?{}".format(
+            reverse(
+                "sentry-organization-integrations-setup",
+                kwargs={
+                    "organization_slug": org_2.slug,
+                    "provider_id": self.provider.key,
+                },
+            ),
+            urlencode({"installation_id": self.installation_id}),
+        )
+        setup_path_2 = "{}?{}".format(
+            self.setup_path,
+            urlencode(
+                {"code": "12345678901234567890", "state": "9cae5e88803f35ed7970fc131e6e65d3"}
+            ),
+        )
+        with self.feature({"organizations:customer-domains": [org_2.slug]}):
+            resp = self.client.get(init_path_2)
+            resp = self.client.get(setup_path_2)
+            self.assertTemplateUsed(resp, "sentry/integrations/github-integration-failed.html")
+            assert resp.status_code == 200
+            assert b'window.opener.postMessage({"success":false' in resp.content
+            assert b"Invalid installation request." in resp.content
 
     @responses.activate
     def test_disable_plugin_when_fully_migrated(self):
@@ -594,6 +698,17 @@ class GitHubIntegrationTest(IntegrationTestCase):
             resp = self.client.get(
                 "{}?{}".format(self.init_path, urlencode({"installation_id": self.installation_id}))
             )
+            resp = self.client.get(
+                "{}?{}".format(
+                    self.setup_path,
+                    urlencode(
+                        {
+                            "code": "12345678901234567890",
+                            "state": "9cae5e88803f35ed7970fc131e6e65d3",
+                        }
+                    ),
+                )
+            )
 
         assert resp.status_code == 200
         self.assertTemplateUsed(resp, "sentry/integrations/github-integration-failed.html")
@@ -615,6 +730,14 @@ class GitHubIntegrationTest(IntegrationTestCase):
         resp = self.client.get(
             "{}?{}".format(self.init_path, urlencode({"installation_id": self.installation_id}))
         )
+        resp = self.client.get(
+            "{}?{}".format(
+                self.setup_path,
+                urlencode(
+                    {"code": "12345678901234567890", "state": "9cae5e88803f35ed7970fc131e6e65d3"}
+                ),
+            )
+        )
         self.assertDialogSuccess(resp)
         integration = Integration.objects.get(external_id=self.installation_id)
         assert integration.provider == "github"