Skip to content

Commit 12fdf4e

Browse files
committed
feat(vsts): Add API-driven integration setup
Add API-driven setup steps for the Azure DevOps (VSTS) integration so it can use the pipeline API instead of the legacy server-rendered flow. Use the shared OAuth API step with the VSTS new identity provider and add a dedicated account selection step for choosing the Azure DevOps organization. Refs [VDY-43: Azure DevOps (VSTS): API-driven integration setup](https://linear.app/getsentry/issue/VDY-43/azure-devops-vsts-api-driven-integration-setup)
1 parent 94b7028 commit 12fdf4e

File tree

3 files changed

+289
-34
lines changed

3 files changed

+289
-34
lines changed

src/sentry/integrations/vsts/integration.py

Lines changed: 117 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -11,13 +11,16 @@
1111
from django.http.request import HttpRequest
1212
from django.http.response import HttpResponseBase
1313
from django.utils.translation import gettext as _
14+
from rest_framework.fields import CharField
15+
from rest_framework.serializers import Serializer
1416

15-
from sentry import features, http
17+
from sentry import features, http, options
1618
from sentry.auth.exceptions import IdentityNotValid
1719
from sentry.constants import ObjectStatus
20+
from sentry.identity.oauth2 import OAuth2ApiStep
1821
from sentry.identity.pipeline import IdentityPipeline
1922
from sentry.identity.services.identity.model import RpcIdentity
20-
from sentry.identity.vsts.provider import get_user_info
23+
from sentry.identity.vsts.provider import VSTSNewIdentityProvider, get_user_info
2124
from sentry.integrations.base import (
2225
FeatureDescription,
2326
IntegrationData,
@@ -47,7 +50,8 @@
4750
from sentry.models.apitoken import generate_token
4851
from sentry.models.repository import Repository
4952
from sentry.organizations.services.organization.model import RpcOrganization
50-
from sentry.pipeline.views.base import PipelineView
53+
from sentry.pipeline.types import PipelineStepResult
54+
from sentry.pipeline.views.base import ApiPipelineSteps, PipelineView
5155
from sentry.pipeline.views.nested import NestedPipelineView
5256
from sentry.shared_integrations.exceptions import (
5357
ApiError,
@@ -135,6 +139,30 @@
135139
logger = logging.getLogger("sentry.integrations")
136140

137141

142+
def get_account_from_id(
143+
account_id: str, accounts: Sequence[Mapping[str, Any]]
144+
) -> Mapping[str, Any] | None:
145+
for account in accounts:
146+
if account["accountId"] == account_id:
147+
return account
148+
return None
149+
150+
151+
def get_accounts(access_token: str, user_id: str) -> Any | None:
152+
url = f"https://app.vssps.visualstudio.com/_apis/accounts?memberId={user_id}&api-version=4.1"
153+
with http.build_session() as session:
154+
response = session.get(
155+
url,
156+
headers={
157+
"Content-Type": "application/json",
158+
"Authorization": f"Bearer {access_token}",
159+
},
160+
)
161+
if response.status_code == 200:
162+
return response.json()
163+
return None
164+
165+
138166
class VstsIntegration(RepositoryIntegration[VstsApiClient], VstsIssuesSpec):
139167
logger = logger
140168
comment_key = "sync_comments"
@@ -414,6 +442,63 @@ def default_project(self) -> str | None:
414442
return None
415443

416444

445+
class VstsAccountSelectionSerializer(Serializer):
446+
account = CharField(required=True)
447+
448+
449+
class VstsAccountSelectionApiStep:
450+
step_name = "account_selection"
451+
452+
def get_serializer_cls(self) -> type:
453+
return VstsAccountSelectionSerializer
454+
455+
def get_step_data(self, pipeline: IntegrationPipeline, request: HttpRequest) -> dict[str, Any]:
456+
with IntegrationPipelineViewEvent(
457+
IntegrationPipelineViewType.ACCOUNT_CONFIG,
458+
IntegrationDomain.SOURCE_CODE_MANAGEMENT,
459+
VstsIntegrationProvider.key,
460+
).capture() as lifecycle:
461+
oauth_data = pipeline.fetch_state("oauth_data") or {}
462+
access_token = oauth_data.get("access_token")
463+
user = get_user_info(access_token)
464+
465+
accounts = get_accounts(access_token, user["uuid"])
466+
extra = {
467+
"organization_id": pipeline.organization.id if pipeline.organization else None,
468+
"user_id": request.user.id,
469+
"accounts": accounts,
470+
}
471+
472+
if not accounts or not accounts.get("value"):
473+
lifecycle.record_halt(IntegrationPipelineHaltReason.NO_ACCOUNTS, extra=extra)
474+
pipeline.bind_state("accounts", [])
475+
return {"accounts": []}
476+
477+
accounts_list = accounts["value"]
478+
pipeline.bind_state("accounts", accounts_list)
479+
return {
480+
"accounts": [
481+
{"accountId": account["accountId"], "accountName": account["accountName"]}
482+
for account in accounts_list
483+
]
484+
}
485+
486+
def handle_post(
487+
self,
488+
validated_data: dict[str, str],
489+
pipeline: IntegrationPipeline,
490+
request: HttpRequest,
491+
) -> PipelineStepResult:
492+
account_id = validated_data["account"]
493+
state_accounts: Sequence[Mapping[str, Any]] | None = pipeline.fetch_state(key="accounts")
494+
account = get_account_from_id(account_id, state_accounts or [])
495+
if account is None:
496+
return PipelineStepResult.error("Invalid Azure DevOps account")
497+
498+
pipeline.bind_state("account", account)
499+
return PipelineStepResult.advance()
500+
501+
417502
class VstsIntegrationProvider(IntegrationProvider):
418503
key = IntegrationProviderSlug.AZURE_DEVOPS.value
419504
name = "Azure DevOps"
@@ -486,8 +571,34 @@ def get_pipeline_views(self) -> Sequence[PipelineView[IntegrationPipeline]]:
486571
AccountConfigView(),
487572
]
488573

574+
def get_pipeline_api_steps(self) -> ApiPipelineSteps[IntegrationPipeline]:
575+
return [
576+
self._make_oauth_api_step(),
577+
VstsAccountSelectionApiStep(),
578+
]
579+
580+
def _make_oauth_api_step(self) -> OAuth2ApiStep:
581+
provider = VSTSNewIdentityProvider(
582+
oauth_scopes=sorted(self.get_scopes()),
583+
redirect_url=absolute_uri(self.oauth_redirect_url),
584+
)
585+
extra_authorize_params = {"response_mode": "query"}
586+
if options.get("vsts.consent-prompt"):
587+
extra_authorize_params["prompt"] = "consent"
588+
589+
return provider.make_oauth_api_step(
590+
bind_key="oauth_data",
591+
extra_authorize_params=extra_authorize_params,
592+
)
593+
489594
def build_integration(self, state: Mapping[str, Any]) -> IntegrationData:
490-
data = state["identity"]["data"]
595+
# TODO: legacy views write token data to state["identity"]["data"] via
596+
# NestedPipelineView. API steps write directly to state["oauth_data"].
597+
# Remove the legacy path once the old views are retired.
598+
if "oauth_data" in state:
599+
data = state["oauth_data"]
600+
else:
601+
data = state["identity"]["data"]
491602
oauth_data = self.get_oauth_data(data)
492603
account = state["account"]
493604
user = get_user_info(data["access_token"])
@@ -679,7 +790,7 @@ def dispatch(self, request: HttpRequest, pipeline: IntegrationPipeline) -> HttpR
679790
state_accounts: Sequence[Mapping[str, Any]] | None = pipeline.fetch_state(
680791
key="accounts"
681792
)
682-
account = self.get_account_from_id(account_id, state_accounts or [])
793+
account = get_account_from_id(account_id, state_accounts or [])
683794
if account is not None:
684795
pipeline.bind_state("account", account)
685796
return pipeline.next_step()
@@ -688,7 +799,7 @@ def dispatch(self, request: HttpRequest, pipeline: IntegrationPipeline) -> HttpR
688799
access_token = (state or {}).get("data", {}).get("access_token")
689800
user = get_user_info(access_token)
690801

691-
accounts = self.get_accounts(access_token, user["uuid"])
802+
accounts = get_accounts(access_token, user["uuid"])
692803
extra = {
693804
"organization_id": pipeline.organization.id if pipeline.organization else None,
694805
"user_id": request.user.id,
@@ -710,30 +821,6 @@ def dispatch(self, request: HttpRequest, pipeline: IntegrationPipeline) -> HttpR
710821
request=request,
711822
)
712823

713-
def get_account_from_id(
714-
self, account_id: str, accounts: Sequence[Mapping[str, Any]]
715-
) -> Mapping[str, Any] | None:
716-
for account in accounts:
717-
if account["accountId"] == account_id:
718-
return account
719-
return None
720-
721-
def get_accounts(self, access_token: str, user_id: int) -> Any | None:
722-
url = (
723-
f"https://app.vssps.visualstudio.com/_apis/accounts?memberId={user_id}&api-version=4.1"
724-
)
725-
with http.build_session() as session:
726-
response = session.get(
727-
url,
728-
headers={
729-
"Content-Type": "application/json",
730-
"Authorization": f"Bearer {access_token}",
731-
},
732-
)
733-
if response.status_code == 200:
734-
return response.json()
735-
return None
736-
737824

738825
class AccountForm(forms.Form):
739826
def __init__(self, accounts: Sequence[Mapping[str, str]], *args: Any, **kwargs: Any) -> None:

tests/sentry/integrations/vsts/test_integration.py

Lines changed: 170 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,13 +6,15 @@
66

77
import pytest
88
import responses
9+
from django.http import HttpRequest
10+
from django.urls import reverse
911

1012
from fixtures.vsts import CREATE_SUBSCRIPTION, VstsIntegrationTestCase
1113
from sentry.identity.vsts.provider import VSTSNewIdentityProvider
1214
from sentry.integrations.models.integration import Integration
1315
from sentry.integrations.models.integration_external_project import IntegrationExternalProject
1416
from sentry.integrations.models.organization_integration import OrganizationIntegration
15-
from sentry.integrations.pipeline import ensure_integration
17+
from sentry.integrations.pipeline import IntegrationPipeline, ensure_integration
1618
from sentry.integrations.vsts import VstsIntegration, VstsIntegrationProvider
1719
from sentry.models.repository import Repository
1820
from sentry.shared_integrations.exceptions import IntegrationError, IntegrationProviderError
@@ -318,6 +320,173 @@ def test_fix_subscription(self, mock_get_scopes: MagicMock) -> None:
318320
subscription = data["metadata"]["subscription"]
319321
assert subscription["id"] is not None and subscription["secret"] is not None
320322

323+
324+
@control_silo_test
325+
class VstsIntegrationApiPipelineTest(VstsIntegrationTestCase):
326+
def _get_pipeline_url(self) -> str:
327+
return reverse(
328+
"sentry-api-0-organization-pipeline",
329+
kwargs={
330+
"organization_id_or_slug": self.organization.slug,
331+
"pipeline_name": "integration_pipeline",
332+
},
333+
)
334+
335+
def _initialize_pipeline(self) -> Any:
336+
return self.client.post(
337+
self._get_pipeline_url(),
338+
data={"action": "initialize", "provider": "vsts"},
339+
format="json",
340+
)
341+
342+
def _advance_step(self, data: dict[str, Any]) -> Any:
343+
return self.client.post(self._get_pipeline_url(), data=data, format="json")
344+
345+
def _get_step_info(self) -> Any:
346+
return self.client.get(self._get_pipeline_url())
347+
348+
def _get_pipeline_signature(self, init_resp: Any) -> str:
349+
return parse_qs(urlparse(init_resp.data["data"]["oauthUrl"]).query)["state"][0]
350+
351+
def test_initialize_pipeline(self) -> None:
352+
resp = self._initialize_pipeline()
353+
assert resp.status_code == 200
354+
assert resp.data["step"] == "oauth_login"
355+
assert resp.data["stepIndex"] == 0
356+
assert resp.data["totalSteps"] == 2
357+
assert resp.data["provider"] == "vsts"
358+
assert "oauthUrl" in resp.data["data"]
359+
360+
def test_get_oauth_step_info(self) -> None:
361+
self._initialize_pipeline()
362+
resp = self._get_step_info()
363+
assert resp.status_code == 200
364+
assert resp.data["step"] == "oauth_login"
365+
oauth_url = resp.data["data"]["oauthUrl"]
366+
assert "login.microsoftonline.com/common/oauth2/v2.0/authorize" in oauth_url
367+
assert "client_id=" in oauth_url
368+
369+
def test_oauth_step_advance(self) -> None:
370+
resp = self._initialize_pipeline()
371+
pipeline_signature = self._get_pipeline_signature(resp)
372+
373+
resp = self._advance_step({"code": "oauth-code", "state": pipeline_signature})
374+
assert resp.status_code == 200
375+
assert resp.data["status"] == "advance"
376+
assert resp.data["step"] == "account_selection"
377+
assert resp.data["stepIndex"] == 1
378+
379+
def test_oauth_step_invalid_state(self) -> None:
380+
self._initialize_pipeline()
381+
resp = self._advance_step({"code": "oauth-code", "state": "invalid-state"})
382+
assert resp.status_code == 400
383+
assert resp.data["status"] == "error"
384+
assert resp.data["data"]["detail"] == "Invalid state"
385+
386+
def test_account_selection_get(self) -> None:
387+
resp = self._initialize_pipeline()
388+
pipeline_signature = self._get_pipeline_signature(resp)
389+
resp = self._advance_step({"code": "oauth-code", "state": pipeline_signature})
390+
assert resp.data["step"] == "account_selection"
391+
392+
resp = self._get_step_info()
393+
assert resp.status_code == 200
394+
assert resp.data["step"] == "account_selection"
395+
assert resp.data["data"]["accounts"] == [
396+
{"accountId": self.vsts_account_id, "accountName": self.vsts_account_name}
397+
]
398+
399+
def test_account_selection_invalid_account(self) -> None:
400+
resp = self._initialize_pipeline()
401+
pipeline_signature = self._get_pipeline_signature(resp)
402+
self._advance_step({"code": "oauth-code", "state": pipeline_signature})
403+
404+
resp = self._advance_step({"account": "invalid-account"})
405+
assert resp.status_code == 400
406+
assert resp.data["status"] == "error"
407+
assert resp.data["data"]["detail"] == "Invalid Azure DevOps account"
408+
409+
def test_full_api_pipeline_flow(self) -> None:
410+
resp = self._initialize_pipeline()
411+
pipeline_signature = self._get_pipeline_signature(resp)
412+
413+
resp = self._advance_step({"code": "oauth-code", "state": pipeline_signature})
414+
assert resp.status_code == 200
415+
assert resp.data["status"] == "advance"
416+
assert resp.data["step"] == "account_selection"
417+
418+
resp = self._advance_step({"account": self.vsts_account_id})
419+
assert resp.status_code == 200
420+
assert resp.data["status"] == "complete"
421+
assert "data" in resp.data
422+
423+
integration = Integration.objects.get(provider="vsts")
424+
assert integration.external_id == self.vsts_account_id
425+
assert integration.name == self.vsts_account_name
426+
427+
assert OrganizationIntegration.objects.filter(
428+
organization_id=self.organization.id,
429+
integration=integration,
430+
).exists()
431+
432+
@patch("sentry.integrations.utils.metrics.EventLifecycle.record_event")
433+
def test_account_selection_get_with_no_accounts(self, mock_record: MagicMock) -> None:
434+
responses.replace(
435+
responses.GET,
436+
"https://app.vssps.visualstudio.com/_apis/accounts?memberId=%s&api-version=4.1"
437+
% self.vsts_user_id,
438+
status=403,
439+
json={"$id": 1, "message": "Your account is not good"},
440+
)
441+
442+
resp = self._initialize_pipeline()
443+
pipeline_signature = self._get_pipeline_signature(resp)
444+
resp = self._advance_step({"code": "oauth-code", "state": pipeline_signature})
445+
assert resp.status_code == 200
446+
assert resp.data["step"] == "account_selection"
447+
448+
resp = self._get_step_info()
449+
assert resp.status_code == 200
450+
assert resp.data["data"]["accounts"] == []
451+
452+
assert_halt_metric(mock_record, "no_accounts")
453+
454+
def test_build_integration_uses_oauth_data_state(self) -> None:
455+
provider = VstsIntegrationProvider()
456+
pipeline = Mock()
457+
pipeline.organization = self.organization
458+
provider.set_pipeline(pipeline)
459+
460+
data = provider.build_integration(
461+
{
462+
"account": {
463+
"accountName": self.vsts_account_name,
464+
"accountId": self.vsts_account_id,
465+
},
466+
"oauth_data": {
467+
"access_token": self.access_token,
468+
"expires_in": "3600",
469+
"refresh_token": self.refresh_token,
470+
"token_type": "bearer",
471+
},
472+
}
473+
)
474+
475+
assert data["external_id"] == self.vsts_account_id
476+
assert data["name"] == self.vsts_account_name
477+
478+
def test_initialize_binds_pipeline_state(self) -> None:
479+
resp = self._initialize_pipeline()
480+
assert resp.status_code == 200
481+
482+
request = HttpRequest()
483+
request.session = self.client.session
484+
request.user = self.user
485+
486+
pipeline = IntegrationPipeline.get_for_request(request)
487+
assert pipeline is not None
488+
assert pipeline.provider.key == "vsts"
489+
321490
@responses.activate
322491
def test_source_url_matches(self) -> None:
323492
self.assert_installation()

0 commit comments

Comments
 (0)