From 2f9b5269de56e9af6e6e8946d86208536678501a Mon Sep 17 00:00:00 2001 From: Josema Camacho Date: Mon, 31 Aug 2026 16:12:04 +0200 Subject: [PATCH] feat(api): harden Jira issue delivery ledger --- .../api/migrations/0098_jira_issues.py | 238 +++++++++++++++++- .../migrations/0099_jira_issues_indexes.py | 36 ++- .../api/migrations/0100_jira_site_identity.py | 85 +++++++ api/src/backend/api/models.py | 210 ++++++++++++++-- api/src/backend/api/tests/test_models.py | 152 +++++++++-- api/src/backend/api/tests/test_serializers.py | 3 +- api/src/backend/api/tests/test_views.py | 141 ++++++++++- api/src/backend/api/v1/serializers.py | 82 +++--- api/src/backend/conftest.py | 18 +- 9 files changed, 861 insertions(+), 104 deletions(-) create mode 100644 api/src/backend/api/migrations/0100_jira_site_identity.py diff --git a/api/src/backend/api/migrations/0098_jira_issues.py b/api/src/backend/api/migrations/0098_jira_issues.py index 99f8478596..c77bf073ed 100644 --- a/api/src/backend/api/migrations/0098_jira_issues.py +++ b/api/src/backend/api/migrations/0098_jira_issues.py @@ -28,21 +28,28 @@ class Migration(migrations.Migration): ("finding_uid", models.CharField(max_length=300)), ("finding_id", models.UUIDField()), ( - "issue_key", - models.CharField(blank=True, default="", max_length=64), + "issue_id", + models.CharField(blank=True, max_length=64, null=True), ), ( - "issue_id", - models.CharField(blank=True, default="", max_length=64), + "issue_key", + models.CharField(blank=True, max_length=64, null=True), ), ( "issue_url", - models.URLField(blank=True, default="", max_length=2048), + models.URLField(blank=True, max_length=2048, null=True), + ), + ( + "project_key", + models.CharField(blank=True, max_length=64, null=True), + ), + ( + "issue_type", + models.CharField(blank=True, max_length=64, null=True), ), - ("project_key", models.CharField(max_length=64)), ( "issue_status", - models.CharField(blank=True, default="", max_length=64), + models.CharField(blank=True, max_length=64, null=True), ), ( "issue_status_category", @@ -53,11 +60,62 @@ class Migration(migrations.Migration): ("indeterminate", "In progress"), ("done", "Done"), ], - default="", max_length=16, + null=True, ), ), ("status_synced_at", models.DateTimeField(blank=True, null=True)), + ( + "attempt_state", + models.CharField( + choices=[ + ("idle", "Idle"), + ("creating", "Creating"), + ("uncertain", "Uncertain"), + ("retryable_failure", "Retryable failure"), + ("terminal_failure", "Terminal failure"), + ], + default="idle", + max_length=32, + ), + ), + ( + "claim_token", + models.CharField(blank=True, max_length=255, null=True), + ), + ("claim_expires_at", models.DateTimeField(blank=True, null=True)), + ( + "delivery_attempt_token", + models.UUIDField(blank=True, null=True), + ), + ( + "attempt_operation", + models.CharField( + blank=True, + choices=[ + ("initial", "Initial"), + ("replacement", "Replacement"), + ], + max_length=16, + null=True, + ), + ), + ( + "attempt_project_key", + models.CharField(blank=True, max_length=64, null=True), + ), + ( + "attempt_issue_type", + models.CharField(blank=True, max_length=64, null=True), + ), + ("attempt_count", models.PositiveIntegerField(default=0)), + ("last_attempt_at", models.DateTimeField(blank=True, null=True)), + ( + "last_error_code", + models.CharField(blank=True, max_length=128, null=True), + ), + ("last_error_message", models.TextField(blank=True, null=True)), + ("next_reconcile_at", models.DateTimeField(blank=True, null=True)), ( "integration", models.ForeignKey( @@ -77,7 +135,8 @@ class Migration(migrations.Migration): ( "tenant", models.ForeignKey( - on_delete=django.db.models.deletion.CASCADE, to="api.tenant" + on_delete=django.db.models.deletion.CASCADE, + to="api.tenant", ), ), ], @@ -90,9 +149,170 @@ class Migration(migrations.Migration): model_name="jiraissue", constraint=models.UniqueConstraint( fields=("tenant_id", "integration_id", "provider_id", "finding_uid"), + include=( + "id", + "finding_id", + "issue_id", + "issue_key", + "issue_status_category", + "attempt_state", + "claim_expires_at", + ), name="unique_jira_issue_per_finding", ), ), + migrations.AddConstraint( + model_name="jiraissue", + constraint=models.UniqueConstraint( + condition=models.Q(("delivery_attempt_token__isnull", False)), + fields=("delivery_attempt_token",), + name="unique_jira_delivery_attempt", + ), + ), + migrations.AddConstraint( + model_name="jiraissue", + constraint=models.UniqueConstraint( + condition=models.Q(("issue_id__isnull", False)), + fields=("tenant_id", "integration_id", "issue_id"), + name="unique_jira_issue_identity", + ), + ), + migrations.AddConstraint( + model_name="jiraissue", + constraint=models.CheckConstraint( + condition=models.Q( + models.Q( + ("issue_id__isnull", True), + ("issue_key__isnull", True), + ("issue_url__isnull", True), + ), + models.Q( + models.Q( + ("issue_id__isnull", False), + ("issue_key__isnull", False), + ("issue_url__isnull", False), + ), + models.Q(("issue_id", ""), _negated=True), + models.Q(("issue_key", ""), _negated=True), + models.Q(("issue_url", ""), _negated=True), + ), + _connector="OR", + ), + name="jira_issue_link_all_or_none", + ), + ), + migrations.AddConstraint( + model_name="jiraissue", + constraint=models.CheckConstraint( + condition=models.Q( + models.Q( + ("claim_token__isnull", True), + ("claim_expires_at__isnull", True), + ), + models.Q( + ("claim_token__isnull", False), + ("claim_expires_at__isnull", False), + models.Q(("claim_token", ""), _negated=True), + ), + _connector="OR", + ), + name="jira_issue_claim_all_or_none", + ), + ), + migrations.AddConstraint( + model_name="jiraissue", + constraint=models.CheckConstraint( + condition=models.Q( + ( + "attempt_state__in", + ( + "idle", + "creating", + "uncertain", + "retryable_failure", + "terminal_failure", + ), + ) + ), + name="jira_issue_valid_attempt_state", + ), + ), + migrations.AddConstraint( + model_name="jiraissue", + constraint=models.CheckConstraint( + condition=models.Q( + ("attempt_operation__isnull", True), + ("attempt_operation__in", ("initial", "replacement")), + _connector="OR", + ), + name="jira_issue_valid_operation", + ), + ), + migrations.AddConstraint( + model_name="jiraissue", + constraint=models.CheckConstraint( + condition=models.Q( + ("attempt_state", "idle"), + models.Q( + ("delivery_attempt_token__isnull", False), + ("attempt_operation__isnull", False), + ("attempt_project_key__isnull", False), + ("attempt_issue_type__isnull", False), + models.Q(("attempt_project_key", ""), _negated=True), + models.Q(("attempt_issue_type", ""), _negated=True), + ), + _connector="OR", + ), + name="jira_issue_attempt_fields", + ), + ), + migrations.AddConstraint( + model_name="jiraissue", + constraint=models.CheckConstraint( + condition=models.Q( + models.Q(("attempt_state", "creating"), _negated=True), + models.Q( + ("claim_token__isnull", False), + ("claim_expires_at__isnull", False), + ), + _connector="OR", + ), + name="jira_issue_creating_has_claim", + ), + ), + migrations.AddConstraint( + model_name="jiraissue", + constraint=models.CheckConstraint( + condition=models.Q( + ("claim_token__isnull", True), + ("attempt_state", "creating"), + _connector="OR", + ), + name="jira_issue_claim_only_creating", + ), + ), + migrations.AddConstraint( + model_name="jiraissue", + constraint=models.CheckConstraint( + condition=models.Q( + models.Q(("attempt_operation", "replacement"), _negated=True), + ("issue_id__isnull", False), + _connector="OR", + ), + name="jira_issue_replacement_has_link", + ), + ), + migrations.AddConstraint( + model_name="jiraissue", + constraint=models.CheckConstraint( + condition=models.Q( + ("next_reconcile_at__isnull", True), + ("attempt_state", "uncertain"), + _connector="OR", + ), + name="jira_issue_reconcile_uncertain", + ), + ), migrations.AddConstraint( model_name="jiraissue", constraint=api.rls.RowLevelSecurityConstraint( diff --git a/api/src/backend/api/migrations/0099_jira_issues_indexes.py b/api/src/backend/api/migrations/0099_jira_issues_indexes.py index be8b7ae971..5da0b206b4 100644 --- a/api/src/backend/api/migrations/0099_jira_issues_indexes.py +++ b/api/src/backend/api/migrations/0099_jira_issues_indexes.py @@ -10,8 +10,40 @@ class Migration(migrations.Migration): migrations.AddIndex( model_name="jiraissue", index=models.Index( - fields=["tenant_id", "provider_id", "finding_uid"], - name="ji_tenant_prov_uid_idx", + fields=["tenant_id", "provider_id", "finding_uid", "integration_id"], + include=[ + "id", + "finding_id", + "issue_id", + "issue_key", + "issue_status_category", + "attempt_state", + ], + name="ji_ui_lookup_idx", + ), + ), + migrations.AddIndex( + model_name="jiraissue", + index=models.Index( + condition=models.Q( + ("attempt_state", "creating"), + ("claim_expires_at__isnull", False), + ), + fields=["tenant_id", "claim_expires_at"], + include=["id"], + name="ji_stale_claim_idx", + ), + ), + migrations.AddIndex( + model_name="jiraissue", + index=models.Index( + condition=models.Q( + ("attempt_state", "uncertain"), + ("next_reconcile_at__isnull", False), + ), + fields=["tenant_id", "next_reconcile_at"], + include=["id"], + name="ji_reconcile_due_idx", ), ), ] diff --git a/api/src/backend/api/migrations/0100_jira_site_identity.py b/api/src/backend/api/migrations/0100_jira_site_identity.py new file mode 100644 index 0000000000..582e134455 --- /dev/null +++ b/api/src/backend/api/migrations/0100_jira_site_identity.py @@ -0,0 +1,85 @@ +import json + +from api.db_router import MainRouter +from cryptography.fernet import Fernet, InvalidToken +from django.conf import settings +from django.db import migrations, models +from django.db.models.fields.json import KeyTextTransform +from django.db.models.functions import Lower + + +def normalize_jira_domains(apps, schema_editor): + Integration = apps.get_model("api", "Integration") + cipher = Fernet(settings.SECRETS_ENCRYPTION_KEY.encode()) + integrations = list( + Integration.objects.using(MainRouter.admin_db) + .filter(integration_type="jira") + .only("id", "tenant_id", "configuration", "_credentials") + ) + + seen_sites = {} + for integration in integrations: + encrypted_credentials = integration._credentials + if isinstance(encrypted_credentials, memoryview): + encrypted_credentials = encrypted_credentials.tobytes() + elif isinstance(encrypted_credentials, str): + encrypted_credentials = encrypted_credentials.encode() + + try: + credentials = json.loads(cipher.decrypt(encrypted_credentials).decode()) + except (InvalidToken, TypeError, ValueError, json.JSONDecodeError) as error: + raise RuntimeError( + f"Cannot read Jira credentials for integration {integration.id}." + ) from error + + configuration = dict(integration.configuration or {}) + domain = credentials.get("domain") or configuration.get("domain") + if not isinstance(domain, str) or not domain.strip(): + raise RuntimeError( + f"Jira integration {integration.id} does not have a valid site domain." + ) + + canonical_domain = domain.strip().lower() + site_identity = (str(integration.tenant_id), canonical_domain) + existing_id = seen_sites.get(site_identity) + if existing_id is not None: + raise RuntimeError( + "Duplicate Jira integrations use site " + f"{canonical_domain!r} for tenant {integration.tenant_id}: " + f"{existing_id} and {integration.id}." + ) + seen_sites[site_identity] = integration.id + + configuration["domain"] = canonical_domain + credentials["domain"] = canonical_domain + integration.configuration = configuration + integration._credentials = cipher.encrypt(json.dumps(credentials).encode()) + + if integrations: + Integration.objects.using(MainRouter.admin_db).bulk_update( + integrations, + ["configuration", "_credentials"], + batch_size=500, + ) + + +class Migration(migrations.Migration): + dependencies = [ + ("api", "0099_jira_issues_indexes"), + ] + + operations = [ + migrations.RunPython( + normalize_jira_domains, + reverse_code=migrations.RunPython.noop, + ), + migrations.AddConstraint( + model_name="integration", + constraint=models.UniqueConstraint( + models.F("tenant_id"), + Lower(KeyTextTransform("domain", "configuration")), + condition=models.Q(("integration_type", "jira")), + name="unique_jira_site_per_tenant", + ), + ), + ] diff --git a/api/src/backend/api/models.py b/api/src/backend/api/models.py index ce28b8f916..1077bfe761 100644 --- a/api/src/backend/api/models.py +++ b/api/src/backend/api/models.py @@ -46,7 +46,8 @@ from django.core.exceptions import ValidationError from django.core.validators import MinLengthValidator from django.db import models from django.db.models import Q -from django.db.models.functions import Upper +from django.db.models.fields.json import KeyTextTransform +from django.db.models.functions import Lower, Upper from django.utils import timezone as django_timezone from django.utils.translation import gettext_lazy as _ from django_celery_beat.models import PeriodicTask @@ -1966,6 +1967,12 @@ class Integration(RowLevelSecurityProtectedModel): db_table = "integrations" constraints = [ + models.UniqueConstraint( + models.F("tenant_id"), + Lower(KeyTextTransform("domain", "configuration")), + condition=Q(integration_type="jira"), + name="unique_jira_site_per_tenant", + ), RowLevelSecurityConstraint( field="tenant_id", name="rls_on_%(class)s", @@ -3116,13 +3123,11 @@ class TenantComplianceSummary(RowLevelSecurityProtectedModel): class JiraIssue(RowLevelSecurityProtectedModel): - """Jira issue created from a finding through a Jira integration. + """Current Jira delivery state for one finding and Jira integration. One row per (integration, provider, finding uid). Keyed on the finding ``uid`` - rather than the per-scan finding id so the link survives rescans, which is - what lets a repeated send be recognised as already ticketed. Only the latest - ticket is kept: when a linked issue is closed or deleted in Jira and the - finding is sent again, the row is updated to point at the new issue. + rather than the per-scan finding id so the link survives rescans. The current + link stays populated while a replacement attempt is in progress. """ class StatusCategoryChoices(models.TextChoices): @@ -3130,6 +3135,17 @@ class JiraIssue(RowLevelSecurityProtectedModel): INDETERMINATE = "indeterminate", _("In progress") DONE = "done", _("Done") + class AttemptStateChoices(models.TextChoices): + IDLE = "idle", _("Idle") + CREATING = "creating", _("Creating") + UNCERTAIN = "uncertain", _("Uncertain") + RETRYABLE_FAILURE = "retryable_failure", _("Retryable failure") + TERMINAL_FAILURE = "terminal_failure", _("Terminal failure") + + class AttemptOperationChoices(models.TextChoices): + INITIAL = "initial", _("Initial") + REPLACEMENT = "replacement", _("Replacement") + id = models.UUIDField(primary_key=True, default=uuid4, editable=False) inserted_at = models.DateTimeField(auto_now_add=True, editable=False) updated_at = models.DateTimeField(auto_now=True, editable=False) @@ -3140,20 +3156,42 @@ class JiraIssue(RowLevelSecurityProtectedModel): Provider, on_delete=models.CASCADE, related_name="jira_issues" ) finding_uid = models.CharField(max_length=300) - # Last finding record that was sent; informational, findings are partitioned - # and rotate per scan so this is not a foreign key + # Findings are partitioned and rotate per scan, so this is not a foreign key. finding_id = models.UUIDField() - # Empty while the issue is being created (reservation), filled after Jira - # confirms the creation - issue_key = models.CharField(max_length=64, blank=True, default="") - issue_id = models.CharField(max_length=64, blank=True, default="") - issue_url = models.URLField(max_length=2048, blank=True, default="") - project_key = models.CharField(max_length=64) - issue_status = models.CharField(max_length=64, blank=True, default="") + issue_id = models.CharField(max_length=64, null=True, blank=True) + issue_key = models.CharField(max_length=64, null=True, blank=True) + issue_url = models.URLField(max_length=2048, null=True, blank=True) + project_key = models.CharField(max_length=64, null=True, blank=True) + issue_type = models.CharField(max_length=64, null=True, blank=True) + issue_status = models.CharField(max_length=64, null=True, blank=True) issue_status_category = models.CharField( - max_length=16, choices=StatusCategoryChoices.choices, blank=True, default="" + max_length=16, + choices=StatusCategoryChoices.choices, + null=True, + blank=True, ) status_synced_at = models.DateTimeField(null=True, blank=True) + attempt_state = models.CharField( + max_length=32, + choices=AttemptStateChoices.choices, + default=AttemptStateChoices.IDLE, + ) + claim_token = models.CharField(max_length=255, null=True, blank=True) + claim_expires_at = models.DateTimeField(null=True, blank=True) + delivery_attempt_token = models.UUIDField(null=True, blank=True) + attempt_operation = models.CharField( + max_length=16, + choices=AttemptOperationChoices.choices, + null=True, + blank=True, + ) + attempt_project_key = models.CharField(max_length=64, null=True, blank=True) + attempt_issue_type = models.CharField(max_length=64, null=True, blank=True) + attempt_count = models.PositiveIntegerField(default=0) + last_attempt_at = models.DateTimeField(null=True, blank=True) + last_error_code = models.CharField(max_length=128, null=True, blank=True) + last_error_message = models.TextField(null=True, blank=True) + next_reconcile_at = models.DateTimeField(null=True, blank=True) class Meta(RowLevelSecurityProtectedModel.Meta): db_table = "jira_issues" @@ -3161,8 +3199,115 @@ class JiraIssue(RowLevelSecurityProtectedModel): constraints = [ models.UniqueConstraint( fields=("tenant_id", "integration_id", "provider_id", "finding_uid"), + include=( + "id", + "finding_id", + "issue_id", + "issue_key", + "issue_status_category", + "attempt_state", + "claim_expires_at", + ), name="unique_jira_issue_per_finding", ), + models.UniqueConstraint( + fields=("delivery_attempt_token",), + condition=Q(delivery_attempt_token__isnull=False), + name="unique_jira_delivery_attempt", + ), + models.UniqueConstraint( + fields=("tenant_id", "integration_id", "issue_id"), + condition=Q(issue_id__isnull=False), + name="unique_jira_issue_identity", + ), + models.CheckConstraint( + condition=( + Q( + issue_id__isnull=True, + issue_key__isnull=True, + issue_url__isnull=True, + ) + | ( + Q( + issue_id__isnull=False, + issue_key__isnull=False, + issue_url__isnull=False, + ) + & ~Q(issue_id="") + & ~Q(issue_key="") + & ~Q(issue_url="") + ) + ), + name="jira_issue_link_all_or_none", + ), + models.CheckConstraint( + condition=( + Q(claim_token__isnull=True, claim_expires_at__isnull=True) + | ( + Q(claim_token__isnull=False, claim_expires_at__isnull=False) + & ~Q(claim_token="") + ) + ), + name="jira_issue_claim_all_or_none", + ), + models.CheckConstraint( + condition=Q( + attempt_state__in=( + "idle", + "creating", + "uncertain", + "retryable_failure", + "terminal_failure", + ) + ), + name="jira_issue_valid_attempt_state", + ), + models.CheckConstraint( + condition=( + Q(attempt_operation__isnull=True) + | Q(attempt_operation__in=("initial", "replacement")) + ), + name="jira_issue_valid_operation", + ), + models.CheckConstraint( + condition=( + Q(attempt_state="idle") + | ( + Q( + delivery_attempt_token__isnull=False, + attempt_operation__isnull=False, + attempt_project_key__isnull=False, + attempt_issue_type__isnull=False, + ) + & ~Q(attempt_project_key="") + & ~Q(attempt_issue_type="") + ) + ), + name="jira_issue_attempt_fields", + ), + models.CheckConstraint( + condition=( + ~Q(attempt_state="creating") + | Q(claim_token__isnull=False, claim_expires_at__isnull=False) + ), + name="jira_issue_creating_has_claim", + ), + models.CheckConstraint( + condition=(Q(claim_token__isnull=True) | Q(attempt_state="creating")), + name="jira_issue_claim_only_creating", + ), + models.CheckConstraint( + condition=( + ~Q(attempt_operation="replacement") | Q(issue_id__isnull=False) + ), + name="jira_issue_replacement_has_link", + ), + models.CheckConstraint( + condition=( + Q(next_reconcile_at__isnull=True) | Q(attempt_state="uncertain") + ), + name="jira_issue_reconcile_uncertain", + ), RowLevelSecurityConstraint( field="tenant_id", name="rls_on_%(class)s", @@ -3171,8 +3316,34 @@ class JiraIssue(RowLevelSecurityProtectedModel): ] indexes = [ models.Index( - fields=["tenant_id", "provider_id", "finding_uid"], - name="ji_tenant_prov_uid_idx", + fields=["tenant_id", "provider_id", "finding_uid", "integration_id"], + include=[ + "id", + "finding_id", + "issue_id", + "issue_key", + "issue_status_category", + "attempt_state", + ], + name="ji_ui_lookup_idx", + ), + models.Index( + fields=["tenant_id", "claim_expires_at"], + condition=Q( + attempt_state="creating", + claim_expires_at__isnull=False, + ), + include=["id"], + name="ji_stale_claim_idx", + ), + models.Index( + fields=["tenant_id", "next_reconcile_at"], + condition=Q( + attempt_state="uncertain", + next_reconcile_at__isnull=False, + ), + include=["id"], + name="ji_reconcile_due_idx", ), ] @@ -3181,8 +3352,7 @@ class JiraIssue(RowLevelSecurityProtectedModel): @property def is_linked(self) -> bool: - """Whether the row points at a confirmed Jira issue (not a reservation).""" - return bool(self.issue_key) + return self.issue_id is not None @property def is_done(self) -> bool: diff --git a/api/src/backend/api/tests/test_models.py b/api/src/backend/api/tests/test_models.py index 0d7db6918c..2355d35495 100644 --- a/api/src/backend/api/tests/test_models.py +++ b/api/src/backend/api/tests/test_models.py @@ -1,4 +1,5 @@ -from datetime import UTC, datetime +from datetime import UTC, datetime, timedelta +from uuid import uuid4 import pytest from allauth.socialaccount.models import SocialApp @@ -15,7 +16,8 @@ from api.models import ( TenantComplianceSummary, ) from django.core.exceptions import ValidationError -from django.db import IntegrityError +from django.db import IntegrityError, transaction +from django.utils import timezone @pytest.mark.django_db @@ -529,36 +531,50 @@ class TestTenantComplianceSummaryModel: @pytest.mark.django_db class TestJiraIssueModel: + @staticmethod + def _common(jira_integration, provider, finding): + return { + "tenant_id": jira_integration.tenant_id, + "integration": jira_integration, + "provider": provider, + "finding_uid": finding.uid, + "finding_id": finding.id, + } + + @staticmethod + def _link(issue_number): + return { + "issue_id": str(10000 + issue_number), + "issue_key": f"TEST-{issue_number}", + "issue_url": f"https://test.atlassian.net/browse/TEST-{issue_number}", + "project_key": "TEST", + "issue_type": "Task", + } + def test_create_jira_issue( self, jira_integration_fixture, aws_provider, findings_fixture ): finding = findings_fixture[0] issue = JiraIssue.objects.create( - tenant_id=jira_integration_fixture.tenant_id, - integration=jira_integration_fixture, - provider=aws_provider, - finding_uid=finding.uid, - finding_id=finding.id, - issue_key="TEST-1", - project_key="TEST", + **self._common(jira_integration_fixture, aws_provider, finding), + **self._link(1), ) assert issue.is_linked assert not issue.is_done - assert issue.issue_status_category == "" + assert issue.issue_status_category is None + assert issue.attempt_state == JiraIssue.AttemptStateChoices.IDLE - def test_reservation_is_not_linked( + def test_idle_row_is_not_linked( self, jira_integration_fixture, aws_provider, findings_fixture ): finding = findings_fixture[0] issue = JiraIssue.objects.create( - tenant_id=jira_integration_fixture.tenant_id, - integration=jira_integration_fixture, - provider=aws_provider, - finding_uid=finding.uid, - finding_id=finding.id, - project_key="TEST", + **self._common(jira_integration_fixture, aws_provider, finding), ) assert not issue.is_linked + assert issue.issue_id is None + assert issue.issue_key is None + assert issue.issue_url is None def test_unique_per_integration_provider_and_finding_uid( self, jira_integration_fixture, aws_provider_pair, findings_fixture @@ -570,10 +586,102 @@ class TestJiraIssueModel: "integration": jira_integration_fixture, "finding_uid": finding.uid, "finding_id": finding.id, - "project_key": "TEST", } - JiraIssue.objects.create(provider=provider, issue_key="TEST-1", **common) + JiraIssue.objects.create(provider=provider, **common) # Same finding uid on another provider is a different finding - JiraIssue.objects.create(provider=provider2, issue_key="TEST-2", **common) - with pytest.raises(IntegrityError): - JiraIssue.objects.create(provider=provider, issue_key="TEST-3", **common) + JiraIssue.objects.create(provider=provider2, **common) + with pytest.raises(IntegrityError), transaction.atomic(): + JiraIssue.objects.create(provider=provider, **common) + + def test_delivery_attempt_token_is_unique( + self, jira_integration_fixture, aws_provider_pair, findings_fixture + ): + provider, provider2 = aws_provider_pair + finding1, finding2 = findings_fixture + token = uuid4() + attempt = { + "attempt_state": JiraIssue.AttemptStateChoices.RETRYABLE_FAILURE, + "delivery_attempt_token": token, + "attempt_operation": JiraIssue.AttemptOperationChoices.INITIAL, + "attempt_project_key": "TEST", + "attempt_issue_type": "Task", + } + JiraIssue.objects.create( + **self._common(jira_integration_fixture, provider, finding1), + **attempt, + ) + with pytest.raises(IntegrityError), transaction.atomic(): + JiraIssue.objects.create( + **self._common(jira_integration_fixture, provider2, finding2), + **attempt, + ) + + def test_issue_identity_is_unique_per_integration( + self, jira_integration_fixture, aws_provider_pair, findings_fixture + ): + provider, provider2 = aws_provider_pair + finding1, finding2 = findings_fixture + JiraIssue.objects.create( + **self._common(jira_integration_fixture, provider, finding1), + **self._link(1), + ) + duplicate_link = self._link(2) | {"issue_id": "10001"} + with pytest.raises(IntegrityError), transaction.atomic(): + JiraIssue.objects.create( + **self._common(jira_integration_fixture, provider2, finding2), + **duplicate_link, + ) + + def test_link_fields_are_all_populated_or_all_null( + self, jira_integration_fixture, aws_provider, findings_fixture + ): + with pytest.raises(IntegrityError), transaction.atomic(): + JiraIssue.objects.create( + **self._common( + jira_integration_fixture, aws_provider, findings_fixture[0] + ), + issue_id="10001", + ) + + def test_creating_attempt_requires_complete_claim_and_destination( + self, jira_integration_fixture, aws_provider, findings_fixture + ): + common = self._common( + jira_integration_fixture, aws_provider, findings_fixture[0] + ) + with pytest.raises(IntegrityError), transaction.atomic(): + JiraIssue.objects.create( + **common, + attempt_state=JiraIssue.AttemptStateChoices.CREATING, + delivery_attempt_token=uuid4(), + attempt_operation=JiraIssue.AttemptOperationChoices.INITIAL, + attempt_project_key="TEST", + attempt_issue_type="Task", + ) + + issue = JiraIssue.objects.create( + **common, + attempt_state=JiraIssue.AttemptStateChoices.CREATING, + claim_token="task-id", + claim_expires_at=timezone.now() + timedelta(minutes=15), + delivery_attempt_token=uuid4(), + attempt_operation=JiraIssue.AttemptOperationChoices.INITIAL, + attempt_project_key="TEST", + attempt_issue_type="Task", + ) + assert issue.claim_token == "task-id" + + def test_replacement_attempt_requires_current_link( + self, jira_integration_fixture, aws_provider, findings_fixture + ): + with pytest.raises(IntegrityError), transaction.atomic(): + JiraIssue.objects.create( + **self._common( + jira_integration_fixture, aws_provider, findings_fixture[0] + ), + attempt_state=JiraIssue.AttemptStateChoices.RETRYABLE_FAILURE, + delivery_attempt_token=uuid4(), + attempt_operation=JiraIssue.AttemptOperationChoices.REPLACEMENT, + attempt_project_key="TEST", + attempt_issue_type="Task", + ) diff --git a/api/src/backend/api/tests/test_serializers.py b/api/src/backend/api/tests/test_serializers.py index 78a3e14c4f..542afd0117 100644 --- a/api/src/backend/api/tests/test_serializers.py +++ b/api/src/backend/api/tests/test_serializers.py @@ -375,5 +375,6 @@ class TestIntegrationSerializerJiraDomain: assert representation["configuration"]["domain"] == "test" assert jira_integration_fixture.configuration == { - "projects": {"TEST": "Test project"} + "projects": {"TEST": "Test project"}, + "domain": "test", } diff --git a/api/src/backend/api/tests/test_views.py b/api/src/backend/api/tests/test_views.py index 8529084427..f8a57d5bdc 100644 --- a/api/src/backend/api/tests/test_views.py +++ b/api/src/backend/api/tests/test_views.py @@ -79,7 +79,7 @@ from conftest import ( today_after_n_days, ) from django.conf import settings -from django.db import close_old_connections, connection, connections +from django.db import IntegrityError, close_old_connections, connection, connections from django.db.models import Count from django.db.models.signals import pre_delete from django.http import JsonResponse @@ -13901,7 +13901,7 @@ class TestIntegrationViewSet: "integration_type": Integration.IntegrationChoices.JIRA, "configuration": {}, "credentials": { - "domain": "prowlerdomain", + "domain": " ProwlerDomain ", "api_token": "this-is-an-api-token-for-jira-that-works-for-sure", "user_mail": "testing@prowler.com", }, @@ -13922,7 +13922,9 @@ class TestIntegrationViewSet: ] assert "projects" in integration_configuration assert "issue_types" in integration_configuration - assert "domain" in integration_configuration + assert integration_configuration["domain"] == "prowlerdomain" + assert integration.configuration["domain"] == "prowlerdomain" + assert integration.credentials["domain"] == "prowlerdomain" assert integration.enabled == data["data"]["attributes"]["enabled"] assert ( integration.integration_type @@ -14350,6 +14352,40 @@ class TestIntegrationViewSet: == "/data/attributes/configuration" ) + @patch("api.v1.serializers.Integration.objects.create") + def test_integrations_create_jira_database_race_returns_conflict( + self, mock_create, authenticated_client + ): + error = IntegrityError() + cause = Exception() + cause.diag = SimpleNamespace(constraint_name="unique_jira_site_per_tenant") + error.__cause__ = cause + mock_create.side_effect = error + data = { + "data": { + "type": "integrations", + "attributes": { + "integration_type": Integration.IntegrationChoices.JIRA, + "configuration": {}, + "credentials": { + "user_mail": "test@example.com", + "api_token": "fake-api-token", + "domain": "prowlerdomain", + }, + "enabled": True, + }, + } + } + + response = authenticated_client.post( + reverse("integration-list"), + data=json.dumps(data), + content_type="application/vnd.api+json", + ) + + assert response.status_code == status.HTTP_409_CONFLICT + mock_create.assert_called_once() + def test_integrations_create_duplicate_jira(self, authenticated_client): # Create first JIRA integration data = { @@ -14376,7 +14412,8 @@ class TestIntegrationViewSet: ) assert response.status_code == status.HTTP_201_CREATED - # Attempt to create duplicate should return 409 + # Site identity is case-insensitive. + data["data"]["attributes"]["credentials"]["domain"] = "PROWLERDOMAIN" response = authenticated_client.post( reverse("integration-list"), data=json.dumps(data), @@ -14391,6 +14428,39 @@ class TestIntegrationViewSet: == "/data/attributes/configuration" ) + def test_integrations_create_different_jira_sites(self, authenticated_client): + data = { + "data": { + "type": "integrations", + "attributes": { + "integration_type": Integration.IntegrationChoices.JIRA, + "configuration": {}, + "credentials": { + "user_mail": "test@example.com", + "api_token": "fake-api-token", + "domain": "first-site", + }, + "enabled": True, + }, + } + } + + first_response = authenticated_client.post( + reverse("integration-list"), + data=json.dumps(data), + content_type="application/vnd.api+json", + ) + data["data"]["attributes"]["credentials"]["domain"] = "second-site" + second_response = authenticated_client.post( + reverse("integration-list"), + data=json.dumps(data), + content_type="application/vnd.api+json", + ) + + assert first_response.status_code == status.HTTP_201_CREATED + assert second_response.status_code == status.HTTP_201_CREATED + assert Integration.objects.count() == 2 + def test_integrations_update_jira_configuration_readonly( self, authenticated_client ): @@ -14442,7 +14512,7 @@ class TestIntegrationViewSet: ) assert response.status_code == status.HTTP_400_BAD_REQUEST - def test_integrations_update_jira_credentials_domain_reflects_in_configuration( + def test_integrations_update_jira_credentials_for_same_site( self, authenticated_client ): # Create JIRA integration first @@ -14478,7 +14548,7 @@ class TestIntegrationViewSet: == "original-domain" ) - # Update credentials with new domain + # Rotate the email and token while preserving the canonical site. update_data = { "data": { "type": "integrations", @@ -14487,7 +14557,7 @@ class TestIntegrationViewSet: "credentials": { "user_mail": "updated@example.com", "api_token": "updated-api-token", - "domain": "updated-domain", + "domain": "ORIGINAL-DOMAIN", } }, } @@ -14500,14 +14570,67 @@ class TestIntegrationViewSet: ) assert response.status_code == status.HTTP_200_OK - # Verify the new domain is reflected in configuration updated_integration = response.json()["data"] configuration = updated_integration["attributes"]["configuration"] - assert configuration["domain"] == "updated-domain" + assert configuration["domain"] == "original-domain" # Verify other configuration fields are preserved assert "projects" in configuration assert "issue_types" in configuration + integration = Integration.objects.get(id=integration_id) + assert integration.credentials == { + "user_mail": "updated@example.com", + "api_token": "updated-api-token", + "domain": "original-domain", + } + + def test_integrations_update_jira_rejects_site_change(self, authenticated_client): + create_data = { + "data": { + "type": "integrations", + "attributes": { + "integration_type": Integration.IntegrationChoices.JIRA, + "configuration": {}, + "credentials": { + "user_mail": "test@example.com", + "api_token": "fake-api-token", + "domain": "original-domain", + }, + "enabled": True, + }, + } + } + create_response = authenticated_client.post( + reverse("integration-list"), + data=json.dumps(create_data), + content_type="application/vnd.api+json", + ) + assert create_response.status_code == status.HTTP_201_CREATED + integration_id = create_response.json()["data"]["id"] + + update_data = { + "data": { + "type": "integrations", + "id": integration_id, + "attributes": { + "credentials": { + "user_mail": "updated@example.com", + "api_token": "updated-api-token", + "domain": "different-domain", + } + }, + } + } + response = authenticated_client.patch( + reverse("integration-detail", kwargs={"pk": integration_id}), + data=json.dumps(update_data), + content_type="application/vnd.api+json", + ) + + assert response.status_code == status.HTTP_400_BAD_REQUEST + integration = Integration.objects.get(id=integration_id) + assert integration.configuration["domain"] == "original-domain" + assert integration.credentials["domain"] == "original-domain" def test_integrations_update_jira_rejects_invalid_domain( self, authenticated_client diff --git a/api/src/backend/api/v1/serializers.py b/api/src/backend/api/v1/serializers.py index ece4b37cae..b14af9f507 100644 --- a/api/src/backend/api/v1/serializers.py +++ b/api/src/backend/api/v1/serializers.py @@ -2895,12 +2895,12 @@ class BaseWriteIntegrationSerializer(BaseWriteSerializer): pointer="/data/attributes/configuration", ) - if ( - integration_type == Integration.IntegrationChoices.JIRA - and Integration.objects.filter( - configuration__contains={ - "domain": attrs.get("configuration").get("domain") - } + configuration = attrs.get("configuration") or {} + if integration_type == Integration.IntegrationChoices.JIRA and ( + Integration.objects.filter( + tenant_id=self.context.get("tenant_id"), + integration_type=Integration.IntegrationChoices.JIRA, + configuration__domain__iexact=configuration.get("domain"), ).exists() ): raise ConflictException( @@ -2977,6 +2977,9 @@ class BaseWriteIntegrationSerializer(BaseWriteSerializer): } ) config_serializer = JiraConfigSerializer + domain = credentials.get("domain") + if isinstance(domain, str): + credentials["domain"] = domain.strip().lower() # Create non-editable configuration for JIRA integration # issue_types will be populated per project when connection is tested configuration.update( @@ -3041,19 +3044,7 @@ class IntegrationSerializer(IntegrationProviderVisibilityMixin, RLSSerializer): } def to_representation(self, instance): - representation = self.hide_restricted_providers( - super().to_representation(instance) - ) - # `configuration` is missing when the request asks for a subset of the fields - if ( - instance.integration_type == Integration.IntegrationChoices.JIRA - and "configuration" in representation - ): - representation["configuration"] = { - **representation["configuration"], - "domain": instance.credentials.get("domain"), - } - return representation + return self.hide_restricted_providers(super().to_representation(instance)) class IntegrationCreateSerializer( @@ -3109,11 +3100,28 @@ class IntegrationCreateSerializer( tenant_id = self.context.get("tenant_id") providers = validated_data.pop("providers", []) - with transaction.atomic(): - integration = Integration.objects.create( - tenant_id=tenant_id, **validated_data + try: + with transaction.atomic(): + integration = Integration.objects.create( + tenant_id=tenant_id, **validated_data + ) + replace_integration_providers(integration, providers, tenant_id) + except IntegrityError as error: + constraint_name = getattr( + getattr(getattr(error, "__cause__", None), "diag", None), + "constraint_name", + None, ) - replace_integration_providers(integration, providers, tenant_id) + if ( + validated_data.get("integration_type") + == Integration.IntegrationChoices.JIRA + and constraint_name == "unique_jira_site_per_tenant" + ): + raise ConflictException( + detail="This integration already exists.", + pointer="/data/attributes/configuration", + ) from error + raise return integration @@ -3156,6 +3164,19 @@ class IntegrationUpdateSerializer( else: configuration = attrs.get("configuration", {}) credentials = attrs.get("credentials") or self.instance.credentials + if integration_type == Integration.IntegrationChoices.JIRA: + current_domain = self.instance.configuration.get( + "domain" + ) or self.instance.credentials.get("domain") + current_domain = current_domain.strip().lower() + requested_domain = credentials.get("domain") + if isinstance(requested_domain, str): + requested_domain = requested_domain.strip().lower() + if requested_domain != current_domain: + raise serializers.ValidationError( + {"credentials": {"domain": "The Jira site cannot be changed."}} + ) + credentials["domain"] = current_domain self.validate_integration_data( integration_type, providers, configuration, credentials @@ -3183,20 +3204,7 @@ class IntegrationUpdateSerializer( return super().update(instance, validated_data) def to_representation(self, instance): - representation = self.hide_restricted_providers( - super().to_representation(instance) - ) - # Ensure JIRA integrations show updated domain in configuration from credentials. - # `configuration` is missing when the request asks for a subset of the fields - if ( - instance.integration_type == Integration.IntegrationChoices.JIRA - and "configuration" in representation - ): - representation["configuration"] = { - **representation["configuration"], - "domain": instance.credentials.get("domain"), - } - return representation + return self.hide_restricted_providers(super().to_representation(instance)) class IntegrationJiraIssueTypesSerializer(BaseSerializerV1): diff --git a/api/src/backend/conftest.py b/api/src/backend/conftest.py index 6e7c793dea..f7f6332464 100644 --- a/api/src/backend/conftest.py +++ b/api/src/backend/conftest.py @@ -1453,8 +1453,7 @@ def integrations_fixture(aws_provider_pair): @pytest.fixture def jira_integration_fixture(tenants_fixture): - # Jira is a tenant-wide integration: it is not attached to any provider, and its - # `domain` is read from the credentials when the integration is serialized + # Jira is a tenant-wide integration and is not attached to any provider. tenant_id = tenants_fixture[0].id with rls_transaction(str(tenant_id)): return Integration.objects.create( @@ -1462,7 +1461,10 @@ def jira_integration_fixture(tenants_fixture): enabled=True, connected=True, integration_type=Integration.IntegrationChoices.JIRA, - configuration={"projects": {"TEST": "Test project"}}, + configuration={ + "projects": {"TEST": "Test project"}, + "domain": "test", + }, credentials={ "domain": "test", "user_mail": "a@b.com", @@ -1488,6 +1490,7 @@ def jira_issues_fixture(jira_integration_fixture, aws_provider_pair, findings_fi issue_id="10001", issue_url="https://test.atlassian.net/browse/TEST-1", project_key="TEST", + issue_type="Task", issue_status="To Do", issue_status_category=JiraIssue.StatusCategoryChoices.NEW, status_synced_at=datetime.now(UTC), @@ -1502,6 +1505,7 @@ def jira_issues_fixture(jira_integration_fixture, aws_provider_pair, findings_fi issue_id="10002", issue_url="https://test.atlassian.net/browse/TEST-2", project_key="TEST", + issue_type="Task", issue_status="Done", issue_status_category=JiraIssue.StatusCategoryChoices.DONE, status_synced_at=datetime.now(UTC), @@ -1512,7 +1516,13 @@ def jira_issues_fixture(jira_integration_fixture, aws_provider_pair, findings_fi provider=provider, finding_uid=finding2.uid, finding_id=finding2.id, - project_key="TEST", + attempt_state=JiraIssue.AttemptStateChoices.CREATING, + claim_token="fixture-task-id", + claim_expires_at=datetime.now(UTC) + timedelta(minutes=15), + delivery_attempt_token=uuid4(), + attempt_operation=JiraIssue.AttemptOperationChoices.INITIAL, + attempt_project_key="TEST", + attempt_issue_type="Task", ) return linked, hidden_provider_issue, reservation