feat(integrations): Azure DevOps models, serializer, configuration viewset (PR 2/N)#7622
feat(integrations): Azure DevOps models, serializer, configuration viewset (PR 2/N)#7622asaphko wants to merge 9 commits into
Conversation
…gration Second plan in the stacked-PRs rollout. Covers the integrations.azure_devops Django app skeleton, two models (AzureDevOpsConfiguration, AzureDevOpsServiceHook), their initial migration, the write-only-PAT serializer, the configuration CRUD viewset, and URL wiring. Defers PAT validation against ADO to PR 3 (when the REST client lands). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Add an empty app skeleton (apps.py + __init__ + migrations/__init__) and register it in INSTALLED_APPS so subsequent commits in this PR can add models, serializers, views, and migrations. No behaviour yet. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
One-per-project soft-deletable model storing the organisation URL, the PAT, and the two capability toggles (labeling_enabled / tagging_enabled). Mirrors GitLabConfiguration's shape. PAT API masking lands in the next commit; remote validation against ADO is deferred to PR 3. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The "# When / Then — split below per GWT lint rule" comment was implementer chatter and doesn't belong in committed test source. The three separate GWT markers below speak for themselves. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Persist one row per (ADO project, event type) we subscribe to on the ADO side. Unlike GitLab, ADO service hooks are one subscription per event type, so the unique constraint is on (configuration, ado_project_id, event_type) and only applies to live (non-soft-deleted) rows. The model is added now so the migration history doesn't churn when the webhook handler lands in a later PR. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
DRF ModelSerializer mirroring GitLab's pattern: PAT is writeable on input but masked with the WRITE_ONLY_PLACEHOLDER on output. Uses BaseProjectIntegrationModelSerializer so the project-scoped one-to-one soft-delete recreate logic comes for free. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…ring
CRUD viewset under /api/v1/projects/{id}/integrations/azure-devops/
following the GitLab pattern: ProjectIntegrationBaseViewSet for the
permission and one-config-per-project semantics, structured logging on
create / update / delete via the "azure_devops" structlog logger,
write-only PAT masking via the serializer. Remote validation against
ADO is deferred to PR 3 when the REST client lands.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The Azure DevOps configuration tests don't reference pytest directly (no fixtures defined here, no marks); the import was carried over from boilerplate. Ruff F401 flags it. Removing also lets ruff re-wrap one long assertion that the previous fixed-import version masked. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
… tests The AzureDevOpsServiceHook.uuid field shadowed the inherited base-class field, and the explicit models.Index(fields=["uuid"]) was redundant with unique=True (Postgres already creates a unique B-tree). Drop both and regenerate the migration. Adds two non-admin permission-denied tests on the configuration viewset that mirror the GitLab pattern. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub. 3 Skipped Deployments
|
There was a problem hiding this comment.
Code Review
This pull request introduces the integrations.azure_devops Django app, adding support for Azure DevOps configurations and service hooks. It includes models, a migration, a serializer that masks the personal access token (PAT) as "write-only", a CRUD viewset with structured logging, and comprehensive tests. The code review feedback highlights a critical bug where updating a configuration with the masked "write-only" placeholder token will overwrite and destroy the actual token in the database. To resolve this, the reviewer suggests overriding the serializer's token validation to preserve the existing token when the placeholder is received, and adding a corresponding integration test to verify this behavior.
| from typing import Any | ||
|
|
||
| from integrations.azure_devops.models import AzureDevOpsConfiguration | ||
| from integrations.common.serializers import BaseProjectIntegrationModelSerializer | ||
|
|
||
| WRITE_ONLY_PLACEHOLDER = "write-only" | ||
|
|
||
|
|
||
| class AzureDevOpsConfigurationSerializer(BaseProjectIntegrationModelSerializer): | ||
| class Meta: | ||
| model = AzureDevOpsConfiguration | ||
| fields = ( | ||
| "id", | ||
| "organisation_url", | ||
| "personal_access_token", | ||
| "labeling_enabled", | ||
| "tagging_enabled", | ||
| ) | ||
|
|
||
| def to_representation(self, instance: AzureDevOpsConfiguration) -> dict[str, Any]: | ||
| data = super().to_representation(instance) | ||
| data["personal_access_token"] = WRITE_ONLY_PLACEHOLDER | ||
| return data |
There was a problem hiding this comment.
Critical Security & Correctness Issue: Token Overwrite Bug
When a client retrieves the configuration, the personal_access_token is masked with the placeholder value "write-only" in to_representation.
However, if the client subsequently performs an update (e.g., a PUT request to toggle labeling_enabled or tagging_enabled) and sends back the masked payload, the serializer will validate and save the literal string "write-only" as the new personal access token, destroying the actual token in the database.
To prevent this, we must override validate_personal_access_token to check if the incoming value is the placeholder "write-only". If it is, and we are updating an existing instance, we should preserve the existing token.
from typing import Any
from rest_framework.exceptions import ValidationError
from integrations.azure_devops.models import AzureDevOpsConfiguration
from integrations.common.serializers import BaseProjectIntegrationModelSerializer
WRITE_ONLY_PLACEHOLDER = "write-only"
class AzureDevOpsConfigurationSerializer(BaseProjectIntegrationModelSerializer):
class Meta:
model = AzureDevOpsConfiguration
fields = (
"id",
"organisation_url",
"personal_access_token",
"labeling_enabled",
"tagging_enabled",
)
def validate_personal_access_token(self, value: str) -> str:
if value == WRITE_ONLY_PLACEHOLDER:
if self.instance:
return self.instance.personal_access_token
raise ValidationError("Personal access token cannot be 'write-only'.")
return value
def to_representation(self, instance: AzureDevOpsConfiguration) -> dict[str, Any]:
data = super().to_representation(instance)
data["personal_access_token"] = WRITE_ONLY_PLACEHOLDER
return data| def test_update_configuration__valid_data__persists_and_masks_token( | ||
| admin_client_new: APIClient, | ||
| project: Project, | ||
| azure_devops_configuration: AzureDevOpsConfiguration, | ||
| log: StructuredLogCapture, | ||
| ) -> None: | ||
| # Given | ||
| detail_url = ( | ||
| f"/api/v1/projects/{project.id}/integrations/azure-devops/" | ||
| f"{azure_devops_configuration.id}/" | ||
| ) | ||
| payload = { | ||
| "organisation_url": "https://dev.azure.com/updated", | ||
| "personal_access_token": "ado-updated-token", | ||
| "labeling_enabled": True, | ||
| "tagging_enabled": True, | ||
| } | ||
|
|
||
| # When | ||
| response = admin_client_new.put(detail_url, data=payload, format="json") | ||
|
|
||
| # Then | ||
| assert response.status_code == status.HTTP_200_OK | ||
| assert response.json()["personal_access_token"] == "write-only" | ||
|
|
||
| azure_devops_configuration.refresh_from_db() | ||
| assert ( | ||
| azure_devops_configuration.organisation_url == "https://dev.azure.com/updated" | ||
| ) | ||
| assert azure_devops_configuration.personal_access_token == "ado-updated-token" | ||
| assert azure_devops_configuration.labeling_enabled is True | ||
| assert azure_devops_configuration.tagging_enabled is True | ||
|
|
||
| assert log.events == [ | ||
| { | ||
| "event": "configuration.updated", | ||
| "level": "info", | ||
| "organisation__id": project.organisation_id, | ||
| "project__id": project.id, | ||
| "ado__organisation__url": "https://dev.azure.com/updated", | ||
| }, | ||
| ] | ||
|
|
There was a problem hiding this comment.
High Severity: Missing Integration Test for Token Preservation
We should add an integration test to verify that a PUT request to the configuration endpoint with the "write-only" placeholder preserves the existing token in the database.
def test_update_configuration__valid_data__persists_and_masks_token(
admin_client_new: APIClient,
project: Project,
azure_devops_configuration: AzureDevOpsConfiguration,
log: StructuredLogCapture,
) -> None:
# Given
detail_url = (
f"/api/v1/projects/{project.id}/integrations/azure-devops/"
f"{azure_devops_configuration.id}/"
)
payload = {
"organisation_url": "https://dev.azure.com/updated",
"personal_access_token": "ado-updated-token",
"labeling_enabled": True,
"tagging_enabled": True,
}
# When
response = admin_client_new.put(detail_url, data=payload, format="json")
# Then
assert response.status_code == status.HTTP_200_OK
assert response.json()["personal_access_token"] == "write-only"
azure_devops_configuration.refresh_from_db()
assert (
azure_devops_configuration.organisation_url == "https://dev.azure.com/updated"
)
assert azure_devops_configuration.personal_access_token == "ado-updated-token"
assert azure_devops_configuration.labeling_enabled is True
assert azure_devops_configuration.tagging_enabled is True
assert log.events == [
{
"event": "configuration.updated",
"level": "info",
"organisation__id": project.organisation_id,
"project__id": project.id,
"ado__organisation__url": "https://dev.azure.com/updated",
},
]
def test_update_configuration__placeholder_token__preserves_existing_token(
admin_client_new: APIClient,
project: Project,
azure_devops_configuration: AzureDevOpsConfiguration,
) -> None:
# Given
detail_url = (
f"/api/v1/projects/{project.id}/integrations/azure-devops/"
f"{azure_devops_configuration.id}/"
)
payload = {
"organisation_url": "https://dev.azure.com/updated",
"personal_access_token": "write-only",
"labeling_enabled": True,
"tagging_enabled": True,
}
# When
response = admin_client_new.put(detail_url, data=payload, format="json")
# Then
assert response.status_code == status.HTTP_200_OK
assert response.json()["personal_access_token"] == "write-only"
azure_devops_configuration.refresh_from_db()
assert (
azure_devops_configuration.organisation_url == "https://dev.azure.com/updated"
)
assert azure_devops_configuration.personal_access_token == "ado-test-token"
assert azure_devops_configuration.labeling_enabled is True
assert azure_devops_configuration.tagging_enabled is True
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## feat/azure-devops-01-resource-types #7622 +/- ##
=====================================================================
Coverage 98.52% 98.52%
=====================================================================
Files 1443 1454 +11
Lines 54803 55018 +215
=====================================================================
+ Hits 53993 54209 +216
+ Misses 810 809 -1 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Summary
PR 2 of the stacked Azure DevOps integration rollout. Stands up the
integrations.azure_devopsDjango app: two models, a write-only PAT serializer, and a CRUD configuration viewset under/api/v1/projects/{id}/integrations/azure-devops/. After this PR, an authorised user can create / read / update / delete an Azure DevOps configuration on a project. No business logic beyond persistence — PAT validation against ADO is deferred to PR 3 with the REST client.AzureDevOpsConfiguration—OneToOneFieldto project,organisation_url,personal_access_token,labeling_enabled,tagging_enabled.SoftDeleteExportableModelbase, mirroringGitLabConfiguration.AzureDevOpsServiceHook— placeholder for the inbound-webhook PR; FK to configuration, ADO project GUID + name, event type, subscription id, secret,created_at. Partial unique constraint on(configuration, ado_project_id, event_type)filtered to live rows. UUID inherited from the base model.0001_initial.pyfor both models.AzureDevOpsConfigurationSerializer—BaseProjectIntegrationModelSerializerparent,WRITE_ONLY_PLACEHOLDERmasking on read.AzureDevOpsConfigurationViewSet—ProjectIntegrationBaseViewSetparent,"azure_devops"logger emitsconfiguration.created/configuration.updated/configuration.deletedwithorganisation__id,project__id,ado__organisation__urlattributes per AGENTS.md.integrations/azure-devops; basenameintegrations-azure-devops.Stack
This PR targets
feat/azure-devops-01-resource-types(#7621). The stack ordering is: spec PR (#7620) → resource-types PR (#7621) → this PR → PR 3 (REST client).Plan:
docs/superpowers/plans/2026-05-28-azure-devops-02-models.md.Deviations from the spec
GitLabConfiguration.access_token", but GitLab'saccess_tokenis stored as a plainCharField. PR 2 mirrors that. If encryption-at-rest lands later, it should retrofit both integrations together.organisation_urltrailing-slash normalisation — spec calls for it but not implemented here. Deferred to PR 3 where URL parsing concentrates.Known follow-ups
configuration.deletedlog event isn't picked up by the events-catalogue docgen because of the rebound-logger pattern inperform_destroy. GitLab has the same divergence — recommend a single docgen-or-refactor follow-up that fixes both vendors together.Test plan
make lintcleanmake typecheckcleanmake test opts='-n0 tests/unit/integrations/azure_devops/'— 23 passedmake test opts='tests/unit/integrations/gitlab tests/unit/integrations/github tests/unit/features/test_unit_feature_external_resources_views.py tests/unit/features/test_migrations.py'— 235 passed (adjacent-integration regression guard)make django-make-migrations opts='--check --dry-run'—No changes detectedfor all apps🤖 Generated with Claude Code