Skip to content

Commit 041107b

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 1aa6667 commit 041107b

File tree

2 files changed

+282
-31
lines changed

2 files changed

+282
-31
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: int) -> 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: 165 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,13 +6,14 @@
66

77
import pytest
88
import responses
9+
from django.urls import reverse
910

1011
from fixtures.vsts import CREATE_SUBSCRIPTION, VstsIntegrationTestCase
1112
from sentry.identity.vsts.provider import VSTSNewIdentityProvider
1213
from sentry.integrations.models.integration import Integration
1314
from sentry.integrations.models.integration_external_project import IntegrationExternalProject
1415
from sentry.integrations.models.organization_integration import OrganizationIntegration
15-
from sentry.integrations.pipeline import ensure_integration
16+
from sentry.integrations.pipeline import IntegrationPipeline, ensure_integration
1617
from sentry.integrations.vsts import VstsIntegration, VstsIntegrationProvider
1718
from sentry.models.repository import Repository
1819
from sentry.shared_integrations.exceptions import IntegrationError, IntegrationProviderError
@@ -318,6 +319,169 @@ def test_fix_subscription(self, mock_get_scopes: MagicMock) -> None:
318319
subscription = data["metadata"]["subscription"]
319320
assert subscription["id"] is not None and subscription["secret"] is not None
320321

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

0 commit comments

Comments
 (0)