Skip to content

Commit 843d7c7

Browse files
authored
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)
1 parent ccd14e4 commit 843d7c7

File tree

3 files changed

+247
-50
lines changed

3 files changed

+247
-50
lines changed

src/sentry/integrations/github/integration.py

Lines changed: 118 additions & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -4,15 +4,19 @@
44
import re
55
from collections.abc import Collection, Mapping, Sequence
66
from typing import Any
7+
from urllib.parse import parse_qsl
78

89
from django.http import HttpResponse
10+
from django.urls import reverse
911
from django.utils.text import slugify
1012
from django.utils.translation import gettext_lazy as _
1113
from rest_framework.request import Request
1214

1315
from sentry import features, options
1416
from sentry.api.utils import generate_organization_url
1517
from sentry.constants import ObjectStatus
18+
from sentry.http import safe_urlopen, safe_urlread
19+
from sentry.identity.github import GitHubIdentityProvider, get_user_info
1620
from sentry.integrations import (
1721
FeatureDescription,
1822
IntegrationFeatures,
@@ -35,6 +39,7 @@
3539
from sentry.tasks.integrations.github.constants import RATE_LIMITED_MESSAGE
3640
from sentry.tasks.integrations.link_all_repos import link_all_repos
3741
from sentry.utils import metrics
42+
from sentry.utils.http import absolute_uri
3843
from sentry.web.helpers import render_to_response
3944

4045
from .client import GitHubAppsClient, GitHubClientMixin
@@ -108,6 +113,9 @@
108113
ERR_INTEGRATION_EXISTS_ON_ANOTHER_ORG = _(
109114
"It seems that your GitHub account has been installed on another Sentry organization. Please uninstall and try again."
110115
)
116+
ERR_INTEGRATION_INVALID_INSTALLATION_REQUEST = _(
117+
"We could not verify the authenticity of the installation request. We recommend restarting the installation process."
118+
)
111119
ERR_INTEGRATION_PENDING_DELETION = _(
112120
"It seems that your Sentry organization has an installation pending deletion. Please wait ~15min for the uninstall to complete and try again."
113121
)
@@ -118,6 +126,32 @@ def build_repository_query(metadata: Mapping[str, Any], name: str, query: str) -
118126
return f"{account_type}:{name} {query}".encode()
119127

120128

129+
def error(
130+
request,
131+
org,
132+
error_short="Invalid installation request.",
133+
error_long=ERR_INTEGRATION_INVALID_INSTALLATION_REQUEST,
134+
):
135+
return render_to_response(
136+
"sentry/integrations/github-integration-failed.html",
137+
context={
138+
"error": error_long,
139+
"payload": {
140+
"success": False,
141+
"data": {"error": _(error_short)},
142+
},
143+
"document_origin": get_document_origin(org),
144+
},
145+
request=request,
146+
)
147+
148+
149+
def get_document_origin(org) -> str:
150+
if org and features.has("organizations:customer-domains", org.organization):
151+
return f'"{generate_organization_url(org.organization.slug)}"'
152+
return "document.origin"
153+
154+
121155
# Github App docs and list of available endpoints
122156
# https://docs.github.com/en/rest/apps/installations
123157
# https://docs.github.com/en/rest/overview/endpoints-available-for-github-apps
@@ -307,7 +341,7 @@ def post_install(
307341
)
308342

309343
def get_pipeline_views(self) -> Sequence[PipelineView]:
310-
return [GitHubInstallation()]
344+
return [OAuthLoginView(), GitHubInstallation()]
311345

312346
def get_installation_info(self, installation_id: str) -> Mapping[str, Any]:
313347
client = self.get_client()
@@ -352,15 +386,72 @@ def setup(self) -> None:
352386
)
353387

354388

389+
class OAuthLoginView(PipelineView):
390+
def dispatch(self, request: Request, pipeline) -> HttpResponse:
391+
self.determine_active_organization(request)
392+
393+
ghip = GitHubIdentityProvider()
394+
github_client_id = ghip.get_oauth_client_id()
395+
github_client_secret = ghip.get_oauth_client_secret()
396+
397+
installation_id = request.GET.get("installation_id")
398+
if installation_id:
399+
pipeline.bind_state("installation_id", installation_id)
400+
401+
if not request.GET.get("state"):
402+
state = pipeline.signature
403+
404+
redirect_uri = absolute_uri(
405+
reverse("sentry-extension-setup", kwargs={"provider_id": "github"})
406+
)
407+
return self.redirect(
408+
f"{ghip.get_oauth_authorize_url()}?client_id={github_client_id}&state={state}&redirect_uri={redirect_uri}"
409+
)
410+
411+
# At this point, we are past the GitHub "authorize" step
412+
if request.GET.get("state") != pipeline.signature:
413+
return error(request, self.active_organization)
414+
415+
# similar to OAuth2CallbackView.get_token_params
416+
data = {
417+
"code": request.GET.get("code"),
418+
"client_id": github_client_id,
419+
"client_secret": github_client_secret,
420+
}
421+
422+
# similar to OAuth2CallbackView.exchange_token
423+
req = safe_urlopen(url=ghip.get_oauth_access_token_url(), data=data)
424+
425+
try:
426+
body = safe_urlread(req).decode("utf-8")
427+
payload = dict(parse_qsl(body))
428+
except Exception:
429+
payload = {}
430+
431+
if "access_token" not in payload:
432+
return error(request, self.active_organization)
433+
434+
authenticated_user_info = get_user_info(payload["access_token"])
435+
if "login" not in authenticated_user_info:
436+
return error(request, self.active_organization)
437+
438+
pipeline.bind_state("github_authenticated_user", authenticated_user_info["login"])
439+
return pipeline.next_step()
440+
441+
355442
class GitHubInstallation(PipelineView):
356443
def get_app_url(self) -> str:
357444
name = options.get("github-app.name")
358445
return f"https://github.com/apps/{slugify(name)}"
359446

360447
def dispatch(self, request: Request, pipeline: Pipeline) -> HttpResponse:
361-
if "installation_id" not in request.GET:
448+
installation_id = request.GET.get(
449+
"installation_id", pipeline.fetch_state("installation_id")
450+
)
451+
if installation_id is None:
362452
return self.redirect(self.get_app_url())
363453

454+
pipeline.bind_state("installation_id", installation_id)
364455
self.determine_active_organization(request)
365456

366457
integration_pending_deletion_exists = False
@@ -374,57 +465,43 @@ def dispatch(self, request: Request, pipeline: Pipeline) -> HttpResponse:
374465
).exists()
375466

376467
if integration_pending_deletion_exists:
377-
document_origin = "document.origin"
378-
if self.active_organization and features.has(
379-
"organizations:customer-domains", self.active_organization.organization
380-
):
381-
document_origin = (
382-
f'"{generate_organization_url(self.active_organization.organization.slug)}"'
383-
)
384-
return render_to_response(
385-
"sentry/integrations/github-integration-failed.html",
386-
context={
387-
"error": ERR_INTEGRATION_PENDING_DELETION,
388-
"payload": {
389-
"success": False,
390-
"data": {"error": _("GitHub installation pending deletion.")},
391-
},
392-
"document_origin": document_origin,
393-
},
394-
request=request,
468+
return error(
469+
request,
470+
self.active_organization,
471+
error_short="GitHub installation pending deletion.",
472+
error_long=ERR_INTEGRATION_PENDING_DELETION,
395473
)
396474

397475
try:
398476
# We want to limit GitHub integrations to 1 organization
399477
installations_exist = OrganizationIntegration.objects.filter(
400-
integration=Integration.objects.get(external_id=request.GET["installation_id"])
478+
integration=Integration.objects.get(external_id=installation_id)
401479
).exists()
402480

403481
except Integration.DoesNotExist:
404-
pipeline.bind_state("installation_id", request.GET["installation_id"])
405482
return pipeline.next_step()
406483

407484
if installations_exist:
408-
document_origin = "document.origin"
409-
if self.active_organization and features.has(
410-
"organizations:customer-domains", self.active_organization.organization
411-
):
412-
document_origin = (
413-
f'"{generate_organization_url(self.active_organization.organization.slug)}"'
414-
)
415-
return render_to_response(
416-
"sentry/integrations/github-integration-failed.html",
417-
context={
418-
"error": ERR_INTEGRATION_EXISTS_ON_ANOTHER_ORG,
419-
"payload": {
420-
"success": False,
421-
"data": {"error": _("Github installed on another Sentry organization.")},
422-
},
423-
"document_origin": document_origin,
424-
},
425-
request=request,
485+
return error(
486+
request,
487+
self.active_organization,
488+
error_short="Github installed on another Sentry organization.",
489+
error_long=ERR_INTEGRATION_EXISTS_ON_ANOTHER_ORG,
426490
)
427491

428492
# OrganizationIntegration does not exist, but Integration does exist.
429-
pipeline.bind_state("installation_id", request.GET["installation_id"])
493+
try:
494+
integration = Integration.objects.get(
495+
external_id=installation_id, status=ObjectStatus.ACTIVE
496+
)
497+
except Integration.DoesNotExist:
498+
return error(request, self.active_organization)
499+
500+
# Check that the authenticated GitHub user is the same as who installed the app.
501+
if (
502+
pipeline.fetch_state("github_authenticated_user")
503+
!= integration.metadata["sender"]["login"]
504+
):
505+
return error(request, self.active_organization)
506+
430507
return pipeline.next_step()

src/sentry/web/frontend/pipeline_advancer.py

Lines changed: 4 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -16,12 +16,6 @@
1616
PIPELINE_CLASSES = [IntegrationPipeline, IdentityProviderPipeline]
1717

1818

19-
# GitHub apps may be installed directly from GitHub, in which case
20-
# they will redirect here *without* being in the pipeline. If that happens
21-
# redirect to the integration install org picker.
22-
FORWARD_INSTALL_FOR = ["github"]
23-
24-
2519
from rest_framework.request import Request
2620

2721

@@ -40,8 +34,11 @@ def handle(self, request: Request, provider_id: str) -> HttpResponseBase:
4034
if pipeline:
4135
break
4236

37+
# GitHub apps may be installed directly from GitHub, in which case
38+
# they will redirect here *without* being in the pipeline. If that happens
39+
# redirect to the integration install org picker.
4340
if (
44-
provider_id in FORWARD_INSTALL_FOR
41+
provider_id == "github"
4542
and request.GET.get("setup_action") == "install"
4643
and pipeline is None
4744
):

0 commit comments

Comments
 (0)