feat: govern attachment exceptions and ownership transfers
This commit is contained in:
@@ -1,15 +1,26 @@
|
||||
from __future__ import annotations
|
||||
|
||||
import unittest
|
||||
from unittest.mock import patch
|
||||
|
||||
from sqlalchemy import create_engine
|
||||
from sqlalchemy.orm import sessionmaker
|
||||
|
||||
from govoplan_access.backend.db.models import Account, Group, User
|
||||
from govoplan_campaign.backend.capabilities import CampaignAccessService
|
||||
from govoplan_campaign.backend.db.models import Campaign, CampaignShare
|
||||
from govoplan_campaign.backend.capabilities import (
|
||||
CampaignAccessService,
|
||||
CampaignOwnershipService,
|
||||
campaign_report_resource_id,
|
||||
)
|
||||
from govoplan_campaign.backend.db.models import (
|
||||
Campaign,
|
||||
CampaignJob,
|
||||
CampaignShare,
|
||||
CampaignVersion,
|
||||
)
|
||||
from govoplan_core.core.access import PrincipalRef
|
||||
from govoplan_core.core.change_sequence import ChangeSequenceEntry
|
||||
from govoplan_core.core.ownership import OwnershipSubjectRef, OwnershipTransferError
|
||||
from govoplan_core.db.base import Base
|
||||
|
||||
|
||||
@@ -70,6 +81,310 @@ class CampaignAccessProviderTests(unittest.TestCase):
|
||||
self.assertTrue(any(item.kind == "share" and item.id == "share-write" for item in items))
|
||||
self.assertFalse(any(item.kind == "share" and item.id == "share-read" for item in items))
|
||||
|
||||
def test_campaign_children_explain_their_parent_campaign_access(self) -> None:
|
||||
session = _session()
|
||||
self.addCleanup(_close_session, session)
|
||||
_seed_access_subjects(session)
|
||||
campaign = Campaign(
|
||||
id="campaign-child",
|
||||
tenant_id=TENANT_ID,
|
||||
owner_user_id=OTHER_USER_ID,
|
||||
external_id="child",
|
||||
name="Child resources",
|
||||
)
|
||||
version = CampaignVersion(
|
||||
id="version-child",
|
||||
campaign_id=campaign.id,
|
||||
version_number=1,
|
||||
raw_json={},
|
||||
)
|
||||
job = CampaignJob(
|
||||
id="job-child",
|
||||
tenant_id=TENANT_ID,
|
||||
campaign_id=campaign.id,
|
||||
campaign_version_id=version.id,
|
||||
entry_index=1,
|
||||
recipient_email="recipient@example.test",
|
||||
)
|
||||
session.add_all(
|
||||
[
|
||||
campaign,
|
||||
version,
|
||||
job,
|
||||
CampaignShare(
|
||||
id="share-child",
|
||||
tenant_id=TENANT_ID,
|
||||
campaign_id=campaign.id,
|
||||
target_type="group",
|
||||
target_id=GROUP_ID,
|
||||
permission="read",
|
||||
),
|
||||
]
|
||||
)
|
||||
session.commit()
|
||||
|
||||
service = CampaignAccessService()
|
||||
principal = _principal(group_ids={GROUP_ID})
|
||||
version_items = service.explain_resource_provenance(
|
||||
session,
|
||||
principal,
|
||||
resource_type="campaign_version",
|
||||
resource_id=version.id,
|
||||
action="campaigns:version:read",
|
||||
)
|
||||
job_items = service.explain_resource_provenance(
|
||||
session,
|
||||
principal,
|
||||
resource_type="campaign_delivery_job",
|
||||
resource_id=job.id,
|
||||
action="campaigns:delivery_job:read",
|
||||
)
|
||||
report_id = campaign_report_resource_id(
|
||||
campaign_id=campaign.id,
|
||||
version_id=version.id,
|
||||
report_kind="delivery",
|
||||
)
|
||||
report_items = service.explain_resource_provenance(
|
||||
session,
|
||||
principal,
|
||||
resource_type="campaign_report",
|
||||
resource_id=report_id,
|
||||
action="campaigns:report:read",
|
||||
)
|
||||
|
||||
for items, source in (
|
||||
(version_items, "campaigns.version"),
|
||||
(job_items, "campaigns.delivery_job"),
|
||||
(report_items, "campaigns.report"),
|
||||
):
|
||||
child = next(item for item in items if item.source == source)
|
||||
self.assertEqual(
|
||||
child.details["authorization_inherited_from"],
|
||||
{
|
||||
"resource_type": "campaign",
|
||||
"resource_id": campaign.id,
|
||||
},
|
||||
)
|
||||
self.assertTrue(
|
||||
any(
|
||||
item.source == "campaigns.campaign"
|
||||
and item.id == campaign.id
|
||||
for item in items
|
||||
)
|
||||
)
|
||||
self.assertTrue(
|
||||
any(item.kind == "share" and item.id == "share-child" for item in items)
|
||||
)
|
||||
|
||||
report = next(item for item in report_items if item.source == "campaigns.report")
|
||||
self.assertEqual(report.details["campaign_version_id"], version.id)
|
||||
self.assertEqual(report.details["report_kind"], "delivery")
|
||||
self.assertFalse(report.details["persisted"])
|
||||
|
||||
def test_campaign_ownership_provider_requires_group_acceptance_authority(self) -> None:
|
||||
session = _session()
|
||||
self.addCleanup(_close_session, session)
|
||||
_seed_access_subjects(session)
|
||||
campaign = Campaign(
|
||||
id="campaign-ownership",
|
||||
tenant_id=TENANT_ID,
|
||||
owner_user_id=USER_ID,
|
||||
external_id="ownership",
|
||||
name="Ownership",
|
||||
)
|
||||
session.add(campaign)
|
||||
session.commit()
|
||||
|
||||
service = CampaignOwnershipService()
|
||||
current_owner = OwnershipSubjectRef(type="user", id=USER_ID)
|
||||
target_group = OwnershipSubjectRef(type="group", id=GROUP_ID)
|
||||
ordinary_member = OwnershipSubjectRef(
|
||||
type="user",
|
||||
id=OTHER_USER_ID,
|
||||
group_ids=frozenset({GROUP_ID}),
|
||||
)
|
||||
group_manager = OwnershipSubjectRef(
|
||||
type="user",
|
||||
id=OTHER_USER_ID,
|
||||
group_ids=frozenset({GROUP_ID}),
|
||||
scopes=frozenset({"campaigns:ownership:accept_group"}),
|
||||
)
|
||||
|
||||
denied = service.authorize_ownership_action(
|
||||
session,
|
||||
tenant_id=TENANT_ID,
|
||||
resource_id=campaign.id,
|
||||
action="accept_group_transfer",
|
||||
actor=ordinary_member,
|
||||
current_owner=current_owner,
|
||||
target_owner=target_group,
|
||||
)
|
||||
allowed = service.authorize_ownership_action(
|
||||
session,
|
||||
tenant_id=TENANT_ID,
|
||||
resource_id=campaign.id,
|
||||
action="accept_group_transfer",
|
||||
actor=group_manager,
|
||||
current_owner=current_owner,
|
||||
target_owner=target_group,
|
||||
)
|
||||
|
||||
self.assertFalse(denied.allowed)
|
||||
self.assertTrue(allowed.allowed)
|
||||
|
||||
@patch(
|
||||
"govoplan_campaign.backend.capabilities._valid_campaign_owner_target",
|
||||
return_value=None,
|
||||
)
|
||||
def test_campaign_ownership_request_requires_existing_read_access(
|
||||
self,
|
||||
_target_validation,
|
||||
) -> None:
|
||||
session = _session()
|
||||
self.addCleanup(_close_session, session)
|
||||
_seed_access_subjects(session)
|
||||
campaign = Campaign(
|
||||
id="campaign-request",
|
||||
tenant_id=TENANT_ID,
|
||||
owner_user_id=USER_ID,
|
||||
external_id="request",
|
||||
name="Request",
|
||||
)
|
||||
session.add(campaign)
|
||||
session.commit()
|
||||
service = CampaignOwnershipService()
|
||||
requester = OwnershipSubjectRef(
|
||||
type="user",
|
||||
id=OTHER_USER_ID,
|
||||
scopes=frozenset({"campaigns:campaign:read"}),
|
||||
)
|
||||
current_owner = OwnershipSubjectRef(type="user", id=USER_ID)
|
||||
|
||||
denied = service.authorize_ownership_action(
|
||||
session,
|
||||
tenant_id=TENANT_ID,
|
||||
resource_id=campaign.id,
|
||||
action="request_ownership",
|
||||
actor=requester,
|
||||
current_owner=current_owner,
|
||||
target_owner=requester,
|
||||
)
|
||||
session.add(
|
||||
CampaignShare(
|
||||
id="share-requester",
|
||||
tenant_id=TENANT_ID,
|
||||
campaign_id=campaign.id,
|
||||
target_type="user",
|
||||
target_id=OTHER_USER_ID,
|
||||
permission="read",
|
||||
)
|
||||
)
|
||||
session.commit()
|
||||
allowed = service.authorize_ownership_action(
|
||||
session,
|
||||
tenant_id=TENANT_ID,
|
||||
resource_id=campaign.id,
|
||||
action="request_ownership",
|
||||
actor=requester,
|
||||
current_owner=current_owner,
|
||||
target_owner=requester,
|
||||
)
|
||||
|
||||
self.assertFalse(denied.allowed)
|
||||
self.assertTrue(allowed.allowed)
|
||||
|
||||
@patch(
|
||||
"govoplan_campaign.backend.capabilities._valid_campaign_owner_target",
|
||||
return_value=None,
|
||||
)
|
||||
def test_campaign_owner_proposal_requires_share_authority(
|
||||
self,
|
||||
_target_validation,
|
||||
) -> None:
|
||||
session = _session()
|
||||
self.addCleanup(_close_session, session)
|
||||
_seed_access_subjects(session)
|
||||
campaign = Campaign(
|
||||
id="campaign-proposal",
|
||||
tenant_id=TENANT_ID,
|
||||
owner_user_id=USER_ID,
|
||||
external_id="proposal",
|
||||
name="Proposal",
|
||||
)
|
||||
session.add(campaign)
|
||||
session.commit()
|
||||
service = CampaignOwnershipService()
|
||||
current_owner = OwnershipSubjectRef(type="user", id=USER_ID)
|
||||
target_owner = OwnershipSubjectRef(type="user", id=OTHER_USER_ID)
|
||||
|
||||
denied = service.authorize_ownership_action(
|
||||
session,
|
||||
tenant_id=TENANT_ID,
|
||||
resource_id=campaign.id,
|
||||
action="propose_transfer",
|
||||
actor=current_owner,
|
||||
current_owner=current_owner,
|
||||
target_owner=target_owner,
|
||||
)
|
||||
allowed = service.authorize_ownership_action(
|
||||
session,
|
||||
tenant_id=TENANT_ID,
|
||||
resource_id=campaign.id,
|
||||
action="propose_transfer",
|
||||
actor=OwnershipSubjectRef(
|
||||
type="user",
|
||||
id=USER_ID,
|
||||
scopes=frozenset({"campaigns:campaign:share"}),
|
||||
),
|
||||
current_owner=current_owner,
|
||||
target_owner=target_owner,
|
||||
)
|
||||
|
||||
self.assertFalse(denied.allowed)
|
||||
self.assertTrue(allowed.allowed)
|
||||
|
||||
def test_campaign_ownership_provider_applies_only_against_expected_owner(self) -> None:
|
||||
session = _session()
|
||||
self.addCleanup(_close_session, session)
|
||||
_seed_access_subjects(session)
|
||||
campaign = Campaign(
|
||||
id="campaign-owner-apply",
|
||||
tenant_id=TENANT_ID,
|
||||
owner_user_id=USER_ID,
|
||||
external_id="owner-apply",
|
||||
name="Owner apply",
|
||||
)
|
||||
session.add(campaign)
|
||||
session.commit()
|
||||
|
||||
service = CampaignOwnershipService()
|
||||
service.apply_owner(
|
||||
session,
|
||||
tenant_id=TENANT_ID,
|
||||
resource_id=campaign.id,
|
||||
expected_owner=OwnershipSubjectRef(type="user", id=USER_ID),
|
||||
target_owner=OwnershipSubjectRef(type="group", id=GROUP_ID),
|
||||
actor=OwnershipSubjectRef(type="user", id=OTHER_USER_ID),
|
||||
reason=None,
|
||||
)
|
||||
session.flush()
|
||||
self.assertIsNone(campaign.owner_user_id)
|
||||
self.assertEqual(campaign.owner_group_id, GROUP_ID)
|
||||
|
||||
with self.assertRaisesRegex(
|
||||
OwnershipTransferError,
|
||||
"changed while the transfer was pending",
|
||||
):
|
||||
service.apply_owner(
|
||||
session,
|
||||
tenant_id=TENANT_ID,
|
||||
resource_id=campaign.id,
|
||||
expected_owner=OwnershipSubjectRef(type="user", id=USER_ID),
|
||||
target_owner=OwnershipSubjectRef(type="user", id=OTHER_USER_ID),
|
||||
actor=OwnershipSubjectRef(type="user", id=USER_ID),
|
||||
reason=None,
|
||||
)
|
||||
|
||||
|
||||
def _session():
|
||||
engine = create_engine("sqlite:///:memory:", future=True)
|
||||
@@ -81,6 +396,8 @@ def _session():
|
||||
Group.__table__,
|
||||
Campaign.__table__,
|
||||
CampaignShare.__table__,
|
||||
CampaignVersion.__table__,
|
||||
CampaignJob.__table__,
|
||||
ChangeSequenceEntry.__table__,
|
||||
],
|
||||
)
|
||||
|
||||
@@ -100,9 +100,9 @@ class CampaignAttachmentBuildTests(unittest.TestCase):
|
||||
cases = {
|
||||
"block": ("build_failed", "blocked", 0, "block", False),
|
||||
"ask": ("built", "needs_review", 0, "ask", True),
|
||||
"drop": ("built", "excluded", 0, "drop", True),
|
||||
"drop": ("built", "needs_review", 0, "ask", True),
|
||||
"warn": ("built", "warning", 1, "warn", True),
|
||||
"continue": ("built", "ready", 1, None, True),
|
||||
"continue": ("built", "warning", 1, None, True),
|
||||
}
|
||||
for behavior, (build_status, validation_status, queueable_count, issue_behavior, has_mime) in cases.items():
|
||||
with self.subTest(behavior=behavior):
|
||||
@@ -133,6 +133,71 @@ class CampaignAttachmentBuildTests(unittest.TestCase):
|
||||
self.assertEqual(coverage_issues[0].behavior, issue_behavior)
|
||||
self.assertEqual(result.built_messages[0].mime is not None, has_mime)
|
||||
|
||||
def test_required_missing_policy_cannot_be_loosened_by_rule(self) -> None:
|
||||
with tempfile.TemporaryDirectory() as tmp:
|
||||
root = Path(tmp)
|
||||
campaign_file = root / "campaign.json"
|
||||
campaign_file.write_text("{}", encoding="utf-8")
|
||||
config = self._no_attachment_config(
|
||||
behavior="continue",
|
||||
configure_missing_rule=True,
|
||||
)
|
||||
rule = config.attachments.global_[0]
|
||||
rule.required = True
|
||||
rule.missing_behavior = "continue"
|
||||
config.attachments.missing_behavior = "continue"
|
||||
|
||||
result = build_campaign_messages(
|
||||
config,
|
||||
campaign_file=campaign_file,
|
||||
output_dir=root / "out",
|
||||
write_eml=True,
|
||||
)
|
||||
|
||||
message = result.report.messages[0]
|
||||
self.assertEqual(message.validation_status.value, "blocked")
|
||||
issue = next(
|
||||
item
|
||||
for item in message.issues
|
||||
if item.code == "missing_required_attachment"
|
||||
)
|
||||
self.assertEqual(issue.behavior, "block")
|
||||
self.assertEqual(
|
||||
issue.details["effective_policy"]["requirement_policy"],
|
||||
"block",
|
||||
)
|
||||
self.assertEqual(
|
||||
message.attachments[0].missing_policy["effective_behavior"],
|
||||
"block",
|
||||
)
|
||||
|
||||
def test_optional_missing_policy_warns_by_default(self) -> None:
|
||||
with tempfile.TemporaryDirectory() as tmp:
|
||||
root = Path(tmp)
|
||||
campaign_file = root / "campaign.json"
|
||||
campaign_file.write_text("{}", encoding="utf-8")
|
||||
config = self._no_attachment_config(
|
||||
behavior="continue",
|
||||
configure_missing_rule=True,
|
||||
)
|
||||
config.attachments.global_[0].missing_behavior = None
|
||||
|
||||
result = build_campaign_messages(
|
||||
config,
|
||||
campaign_file=campaign_file,
|
||||
output_dir=root / "out",
|
||||
write_eml=True,
|
||||
)
|
||||
|
||||
message = result.report.messages[0]
|
||||
self.assertEqual(message.validation_status.value, "warning")
|
||||
issue = next(
|
||||
item
|
||||
for item in message.issues
|
||||
if item.code == "missing_optional_attachment"
|
||||
)
|
||||
self.assertEqual(issue.behavior, "warn")
|
||||
|
||||
def test_missing_pattern_does_not_create_zip_member_or_count_as_attachment(self) -> None:
|
||||
with tempfile.TemporaryDirectory() as tmp:
|
||||
root = Path(tmp)
|
||||
|
||||
@@ -11,6 +11,8 @@ from govoplan_campaign.backend.reports.campaigns import (
|
||||
_job_evidence_row,
|
||||
_latest_by_job_id,
|
||||
_load_delivery_info,
|
||||
_review_decision_summary,
|
||||
_review_decisions_by_job,
|
||||
generate_campaign_report,
|
||||
)
|
||||
|
||||
@@ -84,7 +86,21 @@ def test_job_evidence_row_contains_transport_and_message_evidence() -> None:
|
||||
updated_at=_dt(),
|
||||
)
|
||||
|
||||
row = _job_evidence_row(job, latest_smtp=smtp, latest_imap=imap)
|
||||
row = _job_evidence_row(
|
||||
job,
|
||||
latest_smtp=smtp,
|
||||
latest_imap=imap,
|
||||
review_decision={
|
||||
"decision": "accept",
|
||||
"reason": "Recipient confirmed no attachment was expected.",
|
||||
"actor_user_id": "reviewer-1",
|
||||
"decided_at": "2026-07-08T12:30:00+00:00",
|
||||
"review_key": "recipient-1",
|
||||
"message_sha256": "abc123",
|
||||
"issue_fingerprint": "def456",
|
||||
"issue_codes": ["missing_optional_attachment"],
|
||||
},
|
||||
)
|
||||
|
||||
assert row["campaign_id"] == "campaign-1"
|
||||
assert row["campaign_version_id"] == "version-1"
|
||||
@@ -103,6 +119,38 @@ def test_job_evidence_row_contains_transport_and_message_evidence() -> None:
|
||||
assert "latest_imap_error_message" not in row
|
||||
assert "eml_storage_key" not in row
|
||||
assert "eml_local_path" not in row
|
||||
assert row["review_decision"] == "accept"
|
||||
assert row["review_actor_user_id"] == "reviewer-1"
|
||||
assert row["review_issue_codes"] == "missing_optional_attachment"
|
||||
assert "review_message_sha256" not in row
|
||||
|
||||
|
||||
def test_report_uses_only_review_decisions_for_the_current_build() -> None:
|
||||
decision = {
|
||||
"job_id": "job-1",
|
||||
"decision": "accept",
|
||||
"issue_codes": ["missing_optional_attachment"],
|
||||
}
|
||||
version = SimpleNamespace(
|
||||
build_summary={"build_token": "build-2"},
|
||||
editor_state={
|
||||
"review_send": {
|
||||
"build_token": "build-2",
|
||||
"inspection_complete": True,
|
||||
"issue_decisions": [decision],
|
||||
}
|
||||
},
|
||||
)
|
||||
|
||||
decisions = _review_decisions_by_job(version)
|
||||
|
||||
assert decisions == {"job-1": decision}
|
||||
assert _review_decision_summary(decisions) == {
|
||||
"exception_decision_count": 1,
|
||||
"by_issue_code": {"missing_optional_attachment": 1},
|
||||
}
|
||||
version.editor_state["review_send"]["build_token"] = "stale-build"
|
||||
assert _review_decisions_by_job(version) == {}
|
||||
|
||||
|
||||
def test_latest_by_job_id_keeps_highest_attempt_number() -> None:
|
||||
|
||||
103
tests/test_review_decisions.py
Normal file
103
tests/test_review_decisions.py
Normal file
@@ -0,0 +1,103 @@
|
||||
from __future__ import annotations
|
||||
|
||||
from datetime import UTC, datetime
|
||||
from types import SimpleNamespace
|
||||
|
||||
import pytest
|
||||
|
||||
from govoplan_campaign.backend.persistence.campaigns import (
|
||||
CampaignPersistenceError,
|
||||
)
|
||||
from govoplan_campaign.backend.persistence.versions import (
|
||||
_normalize_review_issue_decisions,
|
||||
)
|
||||
|
||||
|
||||
def test_attachment_ask_requires_reason_and_freezes_evidence() -> None:
|
||||
job = _job(
|
||||
issues=[
|
||||
{
|
||||
"code": "missing_optional_attachment",
|
||||
"behavior": "ask",
|
||||
"source": "attachments",
|
||||
"details": {
|
||||
"effective_policy": {
|
||||
"effective_behavior": "ask",
|
||||
}
|
||||
},
|
||||
}
|
||||
]
|
||||
)
|
||||
|
||||
with pytest.raises(
|
||||
CampaignPersistenceError,
|
||||
match="require an explicit reason",
|
||||
):
|
||||
_normalize_review_issue_decisions(
|
||||
[job],
|
||||
[],
|
||||
user_id="reviewer-1",
|
||||
build_token="build-1",
|
||||
)
|
||||
|
||||
decisions = _normalize_review_issue_decisions(
|
||||
[job],
|
||||
[
|
||||
{
|
||||
"job_id": job.id,
|
||||
"decision": "accept",
|
||||
"reason": "Recipient confirmed that no attachment is expected.",
|
||||
}
|
||||
],
|
||||
user_id="reviewer-1",
|
||||
build_token="build-1",
|
||||
decided_at=datetime(2026, 7, 30, 12, 0, tzinfo=UTC),
|
||||
)
|
||||
|
||||
assert decisions == [
|
||||
{
|
||||
"job_id": "job-1",
|
||||
"review_key": "entry-1",
|
||||
"decision": "accept",
|
||||
"reason": "Recipient confirmed that no attachment is expected.",
|
||||
"actor_user_id": "reviewer-1",
|
||||
"decided_at": "2026-07-30T12:00:00+00:00",
|
||||
"build_token": "build-1",
|
||||
"message_sha256": "a" * 64,
|
||||
"issue_fingerprint": decisions[0]["issue_fingerprint"],
|
||||
"issue_codes": ["missing_optional_attachment"],
|
||||
}
|
||||
]
|
||||
assert len(decisions[0]["issue_fingerprint"]) == 64
|
||||
|
||||
|
||||
def test_attachment_block_cannot_be_overridden_by_review_decision() -> None:
|
||||
job = _job(
|
||||
issues=[
|
||||
{
|
||||
"code": "missing_required_attachment",
|
||||
"behavior": "block",
|
||||
"source": "attachments",
|
||||
}
|
||||
]
|
||||
)
|
||||
|
||||
decisions = _normalize_review_issue_decisions(
|
||||
[job],
|
||||
[],
|
||||
user_id="reviewer-1",
|
||||
build_token="build-1",
|
||||
)
|
||||
|
||||
assert decisions == []
|
||||
|
||||
|
||||
def _job(*, issues: list[dict[str, object]]) -> SimpleNamespace:
|
||||
return SimpleNamespace(
|
||||
id="job-1",
|
||||
entry_id="entry-1",
|
||||
entry_index=1,
|
||||
validation_status="needs_review",
|
||||
issues_snapshot=issues,
|
||||
eml_sha256="a" * 64,
|
||||
)
|
||||
Reference in New Issue
Block a user