diff --git a/src/sentry/integrations/github/issues.py b/src/sentry/integrations/github/issues.py index 1564dcf71b21..dfbbf87cd5c9 100644 --- a/src/sentry/integrations/github/issues.py +++ b/src/sentry/integrations/github/issues.py @@ -13,6 +13,7 @@ from sentry.integrations.models.external_issue import ExternalIssue from sentry.integrations.source_code_management.issues import SourceCodeIssueIntegration from sentry.integrations.types import IntegrationIssueConfigField +from sentry.integrations.utils.issue_url import get_issue_url_path from sentry.issues.grouptype import GroupCategory from sentry.issues.issue_occurrence import IssueOccurrence from sentry.models.group import Group @@ -344,6 +345,29 @@ def get_default_comment(group: Group) -> str: }, ] + def get_issue_link_data(self, url: str) -> dict[str, str]: + domain, account = self.model.metadata["domain_name"].split("/", 1) + path = get_issue_url_path(url, f"https://{domain}") + match = re.fullmatch( + r"/([^/]+/[^/]+)/(issues|pull)/(\d+)(?:/(files|changes|commits|checks))?", path + ) + if not match or (match[2] == "issues" and match[4]): + raise IntegrationFormError({"externalIssue": "Invalid GitHub issue URL"}) + if match[1].split("/")[0].casefold() != account.casefold(): + raise IntegrationFormError( + {"externalIssue": "Issue URL does not belong to this installation"} + ) + repositories = Repository.objects.filter( + name__iexact=match[1], + integration_id=self.model.id, + organization_id=self.organization_id, + status=ObjectStatus.ACTIVE, + ) + repo = repositories.first() + if repo is None: + raise IntegrationFormError({"repo": "Repository does not belong to this installation"}) + return {"repo": repo.name, "externalIssue": match[3]} + def get_issue(self, issue_id: str, **kwargs: Any) -> Mapping[str, Any]: data = kwargs["data"] repo = data.get("repo") diff --git a/tests/sentry/integrations/github/test_issues.py b/tests/sentry/integrations/github/test_issues.py index b189b9ca5d10..5df762c11969 100644 --- a/tests/sentry/integrations/github/test_issues.py +++ b/tests/sentry/integrations/github/test_issues.py @@ -500,6 +500,38 @@ def test_get_issue_with_repo_not_belonging_to_integration(self) -> None: "repo": "Given repository, different-org/different-repo does not belong to this installation" } + def test_issue_url_uses_installed_repository(self) -> None: + self.install.model.metadata["domain_name"] = "github.com/getsentry" + self.create_repo( + name="getsentry/sentry", project=self.project, integration_id=self.integration.id + ) + for path in ( + "issues/321", + "pull/321", + "pull/321/files", + "pull/321/changes", + "pull/321/commits", + "pull/321/checks", + ): + assert self.install.get_issue_link_data( + f"https://github.com/GETSENTRY/Sentry/{path}?view=1#comment" + ) == {"repo": "getsentry/sentry", "externalIssue": "321"} + + def test_issue_url_rejects_invalid_targets(self) -> None: + self.install.model.metadata["domain_name"] = "github.com/getsentry" + for url in ( + "https://github.example.org/getsentry/sentry/issues/321", + "https://github.com/another-org/sentry/issues/321", + "https://github.com/getsentry/unregistered/issues/321", + ): + with pytest.raises(IntegrationFormError): + self.install.get_issue_link_data(url) + + for path in ("pull/321/unknown", "issues/321/files", "issues/321/changes"): + with pytest.raises(IntegrationFormError) as exc: + self.install.get_issue_link_data(f"https://github.com/getsentry/sentry/{path}") + assert exc.value.field_errors == {"externalIssue": "Invalid GitHub issue URL"} + @responses.activate def test_get_issue_with_valid_repo_ownership(self) -> None: with assume_test_silo_mode(SiloMode.CELL): diff --git a/tests/sentry/integrations/github_enterprise/test_issues.py b/tests/sentry/integrations/github_enterprise/test_issues.py index f49b0782ddf9..db3358716d56 100644 --- a/tests/sentry/integrations/github_enterprise/test_issues.py +++ b/tests/sentry/integrations/github_enterprise/test_issues.py @@ -3,12 +3,14 @@ from unittest.mock import MagicMock, patch import orjson +import pytest import responses from django.test import RequestFactory from sentry.integrations.github_enterprise.integration import GitHubEnterpriseIntegration from sentry.integrations.models.external_issue import ExternalIssue from sentry.models.repository import Repository +from sentry.shared_integrations.exceptions import IntegrationFormError from sentry.silo.base import SiloMode from sentry.silo.util import PROXY_BASE_URL_HEADER, PROXY_OI_HEADER, PROXY_SIGNATURE_HEADER from sentry.testutils.cases import IntegratedApiTestCase, TestCase @@ -48,6 +50,16 @@ def _check_proxying(self) -> None: assert request.headers[PROXY_BASE_URL_HEADER] == f"https://{self._IP_ADDRESS}" assert PROXY_SIGNATURE_HEADER in request.headers + def test_issue_url_uses_enterprise_host(self) -> None: + self.create_repo( + name="getsentry/sentry", project=self.project, integration_id=self.model.id + ) + assert self.install.get_issue_link_data( + f"https://{self._IP_ADDRESS}/getsentry/sentry/pull/321/files" + ) == {"repo": "getsentry/sentry", "externalIssue": "321"} + with pytest.raises(IntegrationFormError): + self.install.get_issue_link_data("https://github.com/getsentry/sentry/pull/321") + @responses.activate @patch("sentry.integrations.github_enterprise.client.get_jwt", return_value="jwt_token_1") def test_get_repo_labels(self, mock_get_jwt: MagicMock) -> None: diff --git a/tests/sentry/issues/endpoints/test_group_integration_details.py b/tests/sentry/issues/endpoints/test_group_integration_details.py index 6c4f07ea7c55..d045a42f5636 100644 --- a/tests/sentry/issues/endpoints/test_group_integration_details.py +++ b/tests/sentry/issues/endpoints/test_group_integration_details.py @@ -7,6 +7,7 @@ from sentry.integrations.example.integration import ExampleIntegration from sentry.integrations.models import Integration from sentry.integrations.models.external_issue import ExternalIssue +from sentry.integrations.services.integration import integration_service from sentry.integrations.types import EventLifecycleOutcome from sentry.models.activity import Activity from sentry.models.group import Group @@ -260,6 +261,67 @@ def test_simple_put(self, mock_record_event: mock.MagicMock) -> None: mock_record_event.assert_called_with(EventLifecycleOutcome.SUCCESS, None, False, None) + @responses.activate + def test_put_github_issue_url(self) -> None: + self.login_as(self.user) + for provider, domain, api_url, issue_path in ( + ("github", "github.com", "https://api.github.com", "issues/321"), + ("github", "github.com", "https://api.github.com", "pull/321/changes"), + ( + "github_enterprise", + "github.example.com", + "https://github.example.com/api/v3", + "pull/321", + ), + ): + metadata: dict[str, Any] = { + "domain_name": f"{domain}/example", + "access_token": "access-token", + "expires_at": "3000-01-01T00:00:00Z", + "installation": {"id": 2, "private_key": "private-key", "verify_ssl": True}, + } + integration = self.create_integration( + organization=self.organization, + provider=provider, + external_id=f"{provider}:{issue_path}", + metadata=metadata, + ) + self.create_repo( + name="example/repo", project=self.project, integration_id=integration.id + ) + responses.get( + f"{api_url}/repos/example/repo/issues/321", + json={ + "number": 321, + "title": "Existing issue", + "body": "Description", + "html_url": f"https://{domain}/example/repo/{issue_path.removesuffix('/changes')}", + }, + ) + group = self.create_group(project=self.project) + path = f"/api/0/organizations/{self.organization.slug}/issues/{group.id}/integrations/{integration.id}/" + with self.feature("organizations:integrations-issue-basic"): + response = self.client.put( + path, + data={"externalIssue": f"https://{domain}/EXAMPLE/Repo/{issue_path}"}, + ) + assert response.status_code == 201, response.content + assert response.data["key"] == "example/repo#321" + assert GroupLink.objects.filter( + group_id=group.id, + linked_id=response.data["id"], + linked_type=GroupLink.LinkedType.issue, + relationship=GroupLink.Relationship.references, + ).exists() + org_integration = integration_service.get_organization_integration( + integration_id=integration.id, organization_id=self.organization.id + ) + assert org_integration is not None + assert org_integration.config["project_issue_defaults"][str(self.project.id)] == { + "repo": "example/repo" + } + assert len(responses.calls) == 3 + @responses.activate def test_put_jira_issue_url(self) -> None: self.login_as(self.user)