Skip to content

feat(vsts): Add API-driven integration setup#113073

Open
evanpurkhiser wants to merge 1 commit intomasterfrom
evanpurkhiser/feat-vsts-add-api-driven-integration-setup
Open

feat(vsts): Add API-driven integration setup#113073
evanpurkhiser wants to merge 1 commit intomasterfrom
evanpurkhiser/feat-vsts-add-api-driven-integration-setup

Conversation

@evanpurkhiser
Copy link
Copy Markdown
Member

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

@evanpurkhiser evanpurkhiser requested review from a team as code owners April 15, 2026 17:14
@linear-code
Copy link
Copy Markdown

linear-code bot commented Apr 15, 2026

@evanpurkhiser evanpurkhiser requested review from a team and removed request for a team April 15, 2026 17:14
@github-actions github-actions bot added the Scope: Backend Automatically applied to PRs that change backend components label Apr 15, 2026
Comment on lines +462 to +463
access_token = oauth_data.get("access_token")
user = get_user_info(access_token)
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bug: The code does not validate that access_token exists before using it, leading to an unhandled HTTPError if the token is missing during the VSTS integration setup.
Severity: MEDIUM

Suggested Fix

Before calling get_user_info(), check if access_token is None. If it is, return a graceful error to the user. Alternatively, wrap the get_user_info() call in a try...except HTTPError block to catch the potential exception and handle it gracefully.

Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent.
Verify if this is a real issue. If it is, propose a fix; if not, explain why it's not
valid.

Location: src/sentry/integrations/vsts/integration.py#L462-L463

Potential issue: In the VSTS integration setup flow, the code defensively handles a
potentially missing `oauth_data` from the pipeline state by defaulting to an empty
dictionary. However, it does not validate the `access_token` retrieved from this data.
If the `access_token` is missing, `None` is passed to `get_user_info()`. This function
then attempts an API call to Azure DevOps with an invalid authorization header (`"bearer
None"`), which results in a 401 response. The subsequent call to
`resp.raise_for_status()` raises an `HTTPError` that is not caught, causing an unhandled
exception and returning a 500 server error to the user, breaking the integration setup
process.

Did we get this right? 👍 / 👎 to inform future reviews.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm, serializer should ensure this. will follow up

Comment thread src/sentry/integrations/vsts/integration.py
@github-actions
Copy link
Copy Markdown
Contributor

Backend Test Failures

Failures on a5ff8fa in this run:

tests/sentry/integrations/vsts/test_integration.py::VstsIntegrationApiPipelineTest::test_account_selection_get_with_no_accountslog
[gw1] linux -- Python 3.13.1 /home/runner/work/sentry/sentry/.venv/bin/python3
tests/sentry/integrations/vsts/test_integration.py:451: in test_account_selection_get_with_no_accounts
    assert_halt_metric(mock_record, "no_accounts")
src/sentry/testutils/asserts.py:91: in assert_halt_metric
    (event_halts,) = (
E   ValueError: too many values to unpack (expected 1)
tests/sentry/integrations/vsts/test_integration.py::VstsIntegrationApiPipelineTest::test_oauth_step_invalid_statelog
[gw1] linux -- Python 3.13.1 /home/runner/work/sentry/sentry/.venv/bin/python3
tests/sentry/integrations/vsts/test_integration.py:383: in test_oauth_step_invalid_state
    assert resp.data["data"]["detail"] == "Invalid state"
E   AssertionError: assert 'An error occ...your request.' == 'Invalid state'
E     
E     - Invalid state
E     + An error occurred while validating your request.
tests/sentry/integrations/vsts/test_provider.py::TestAccountConfigView::test_get_accountslog
[gw0] linux -- Python 3.13.1 /home/runner/work/sentry/sentry/.venv/bin/python3
tests/sentry/integrations/vsts/test_provider.py:245: in test_get_accounts
    accounts = view.get_accounts("access-token", 123)
E   AttributeError: 'AccountConfigView' object has no attribute 'get_accounts'

@evanpurkhiser evanpurkhiser force-pushed the evanpurkhiser/feat-vsts-add-api-driven-integration-setup branch from 041107b to 12fdf4e Compare April 15, 2026 19:32
@github-actions
Copy link
Copy Markdown
Contributor

Backend Test Failures

Failures on 21817eb in this run:

tests/sentry/integrations/vsts/test_integration.py::VstsIntegrationApiPipelineTest::test_account_selection_get_with_no_accountslog
[gw1] linux -- Python 3.13.1 /home/runner/work/sentry/sentry/.venv/bin/python3
tests/sentry/integrations/vsts/test_integration.py:452: in test_account_selection_get_with_no_accounts
    assert_halt_metric(mock_record, "no_accounts")
src/sentry/testutils/asserts.py:91: in assert_halt_metric
    (event_halts,) = (
E   ValueError: too many values to unpack (expected 1)
tests/sentry/integrations/vsts/test_integration.py::VstsIntegrationApiPipelineTest::test_oauth_step_invalid_statelog
[gw1] linux -- Python 3.13.1 /home/runner/work/sentry/sentry/.venv/bin/python3
tests/sentry/integrations/vsts/test_integration.py:384: in test_oauth_step_invalid_state
    assert resp.data["data"]["detail"] == "Invalid state"
E   AssertionError: assert 'An error occ...your request.' == 'Invalid state'
E     
E     - Invalid state
E     + An error occurred while validating your request.

Copy link
Copy Markdown
Contributor

@cursor cursor bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cursor Bugbot has reviewed your changes and found 1 potential issue.

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit 12fdf4e. Configure here.

).capture() as lifecycle:
oauth_data = pipeline.fetch_state("oauth_data") or {}
access_token = oauth_data.get("access_token")
user = get_user_info(access_token)
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unhandled None access_token crashes get_step_data

Medium Severity

In get_step_data, oauth_data.get("access_token") can return None, which is then passed to get_user_info. That function builds an auth header "bearer None" and calls raise_for_status() on the resulting 401 response, raising an unhandled HTTPError that surfaces as a 500 to the caller. Unlike the legacy AccountConfigView.dispatch (which shares this weakness), this new API-driven path has no intermediary error handling, so the raw exception propagates directly to the API endpoint. A guard or early error return before calling get_user_info would prevent the crash.

Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit 12fdf4e. Configure here.

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)
@evanpurkhiser evanpurkhiser force-pushed the evanpurkhiser/feat-vsts-add-api-driven-integration-setup branch from 12fdf4e to 8322207 Compare April 15, 2026 19:49
},
)
if response.status_code == 200:
return response.json()
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Missing JSONDecodeError handling in get_accounts API call

The get_accounts function calls response.json() without catching JSONDecodeError. While the code checks for status_code == 200, Azure DevOps can return 200 with malformed JSON (truncated responses, HTML error pages from CDNs/proxies, or network corruption). This is a documented VSTS bug pattern (SENTRY-5CKF) causing 30,593 events. An unhandled exception during integration setup would surface as a 500 error to users.

Verification

Read the get_accounts function at lines 151-163. Confirmed response.json() is called without try/except. Cross-referenced with references/data-validation.md Example 5 (SENTRY-5CKF) which documents this exact VSTS JSON parsing bug pattern. Also checked references/integration-errors.md which lists 'HTML instead of JSON' and 'Truncated JSON payload' as common VSTS failure modes.

Identified by Warden sentry-backend-bugs · 9JZ-GEY

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

probably fine

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Need to look deeper into this to understand what's actually wrong.



def get_accounts(access_token: str, user_id: str) -> Any | None:
url = f"https://app.vssps.visualstudio.com/_apis/accounts?memberId={user_id}&api-version=4.1"
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It feels a little weird that we hardcode this string in this way, rather than having a base string somewhere that we add specific urls onto

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

eah this is how this already was lol

def get_serializer_cls(self) -> type:
return VstsAccountSelectionSerializer

def get_step_data(self, pipeline: IntegrationPipeline, request: HttpRequest) -> dict[str, Any]:
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It could be nice to return a typed dict here since we know exactly what fields we're returning

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah yeah this should be typed better

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Scope: Backend Automatically applied to PRs that change backend components

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants