From 5f7503598c99aaf277f5f278c0494ff480d402ae Mon Sep 17 00:00:00 2001 From: Albrecht Degering Date: Thu, 30 Jul 2026 17:42:10 +0200 Subject: [PATCH] feat: govern attachment exceptions and ownership transfers --- docs/ACCESS_EXPLANATION_COVERAGE.md | 40 ++ .../backend/attachments/resolver.py | 131 +++-- .../backend/campaign/mail_profile_boundary.py | 116 +++++ .../backend/campaign/models.py | 4 +- src/govoplan_campaign/backend/capabilities.py | 453 +++++++++++++++++- src/govoplan_campaign/backend/manifest.py | 13 + .../backend/messages/builder.py | 29 +- .../backend/messages/models.py | 2 + .../backend/persistence/versions.py | 146 +++++- .../backend/reports/campaigns.py | 142 +++++- .../backend/routes/sharing.py | 62 +-- .../backend/routes/versions.py | 5 + .../backend/schema/campaign.schema.json | 4 +- src/govoplan_campaign/backend/schemas.py | 12 + tests/test_access_provider.py | 321 ++++++++++++- tests/test_attachment_building.py | 69 ++- tests/test_campaign_reports.py | 50 +- tests/test_review_decisions.py | 103 ++++ webui/src/api/campaigns.ts | 5 + .../features/campaigns/GlobalSettingsPage.tsx | 4 +- .../src/features/campaigns/ReviewSendPage.tsx | 81 +++- .../components/CampaignAccessCard.tsx | 450 +++++++++++++++-- .../campaigns/review/BuiltMessagePreview.tsx | 69 ++- .../campaigns/review/builtMessageQuery.ts | 21 +- .../features/campaigns/utils/attachments.ts | 2 +- .../features/campaigns/utils/draftEditor.ts | 2 +- .../wizard/steps/CreateWizardSteps.tsx | 2 +- webui/src/styles/campaign-workspace.css | 71 +++ 28 files changed, 2229 insertions(+), 180 deletions(-) create mode 100644 docs/ACCESS_EXPLANATION_COVERAGE.md create mode 100644 tests/test_review_decisions.py diff --git a/docs/ACCESS_EXPLANATION_COVERAGE.md b/docs/ACCESS_EXPLANATION_COVERAGE.md new file mode 100644 index 0000000..8dc7f37 --- /dev/null +++ b/docs/ACCESS_EXPLANATION_COVERAGE.md @@ -0,0 +1,40 @@ +# Campaign Access Explanation Coverage + +Campaign access explanations are resource-specific evidence. They inherit the +parent Campaign decision only where the child has no independent grant model, +and they must identify that inheritance explicitly. + +## Implemented + +- Campaign +- Campaign version +- Campaign delivery job / built message +- Computed Campaign report, identified by Campaign, version, and report kind + +## Planned Slices + +1. Recipient rows and imported recipient-source snapshots +2. Attachment bindings and frozen attachment resolutions +3. Validation issues, review decisions, and attachment-policy overrides +4. Delivery attempts, IMAP append attempts, Postbox attempts, and + reconciliation decisions +5. Campaign shares and ownership-transfer records +6. Import mapping profiles and import executions +7. Reusable Campaign templates and template revisions when the template + library becomes persistent +8. Export packages and protocol/report artifacts + +Each child explanation must include: + +- the child resource identity and current state; +- the parent Campaign and version where applicable; +- whether access is inherited, independently granted, or further restricted; +- effective owner/share/policy provenance; +- missing-module or unavailable-evidence reasons without leaking the hidden + object; +- a stable resource identifier suitable for audit and support links. + +Delivery attempts, review decisions, reports, and exports can contain more +sensitive evidence than the Campaign summary. Their read and diagnostic/export +permissions therefore remain independently enforceable even when the parent +Campaign is readable. diff --git a/src/govoplan_campaign/backend/attachments/resolver.py b/src/govoplan_campaign/backend/attachments/resolver.py index 11cb5fa..b5fd8fc 100644 --- a/src/govoplan_campaign/backend/attachments/resolver.py +++ b/src/govoplan_campaign/backend/attachments/resolver.py @@ -53,6 +53,17 @@ class AttachmentIssue(BaseModel): code: str message: str behavior: Behavior | None = None + details: dict[str, Any] = Field(default_factory=dict) + + +class AttachmentPolicyDecision(BaseModel): + model_config = ConfigDict(extra="forbid") + + requirement_policy: Behavior + campaign_policy: Behavior + rule_policy: Behavior | None = None + effective_behavior: Behavior + legacy_drop_normalized: bool = False class ResolvedAttachment(BaseModel): @@ -82,6 +93,7 @@ class ResolvedAttachment(BaseModel): zip_entry_names: list[str] = Field(default_factory=list) status: AttachmentMatchStatus behavior: Behavior | None = None + missing_policy: AttachmentPolicyDecision | None = None matches: list[str] = Field(default_factory=list) issues: list[AttachmentIssue] = Field(default_factory=list) @@ -193,12 +205,48 @@ def _rule_allows_multiple(config: AttachmentConfig, rendered_file_filter: str) - return config.allow_multiple or any(char in rendered_file_filter for char in "*?[") -def _missing_behavior(campaign_config: CampaignConfig, config: AttachmentConfig) -> Behavior: +_MISSING_BEHAVIOR_STRENGTH = { + Behavior.CONTINUE: 0, + Behavior.WARN: 1, + Behavior.ASK: 2, + Behavior.DROP: 2, + Behavior.BLOCK: 3, +} + + +def _missing_policy_decision( + campaign_config: CampaignConfig, + config: AttachmentConfig, +) -> AttachmentPolicyDecision: + requirement_policy = ( + campaign_config.validation_policy.missing_required_attachment + if config.required + else campaign_config.validation_policy.missing_optional_attachment + ) + candidates = [ + requirement_policy, + campaign_config.attachments.missing_behavior, + ] if config.missing_behavior is not None: - return config.missing_behavior - if config.required: - return campaign_config.validation_policy.missing_required_attachment - return campaign_config.validation_policy.missing_optional_attachment + candidates.append(config.missing_behavior) + configured = max( + candidates, + key=lambda behavior: _MISSING_BEHAVIOR_STRENGTH[behavior], + ) + legacy_drop_normalized = configured == Behavior.DROP + if legacy_drop_normalized: + configured = Behavior.BLOCK if config.required else Behavior.ASK + return AttachmentPolicyDecision( + requirement_policy=requirement_policy, + campaign_policy=campaign_config.attachments.missing_behavior, + rule_policy=config.missing_behavior, + effective_behavior=configured, + legacy_drop_normalized=legacy_drop_normalized, + ) + + +def _missing_behavior(campaign_config: CampaignConfig, config: AttachmentConfig) -> Behavior: + return _missing_policy_decision(campaign_config, config).effective_behavior def _ambiguous_behavior(campaign_config: CampaignConfig, config: AttachmentConfig) -> Behavior: @@ -356,14 +404,19 @@ def _confine_managed_matches(directory: Path, matches: list[Path]) -> tuple[list return confined, rejected -def _issue_for_missing(config: AttachmentConfig, behavior: Behavior) -> AttachmentIssue: +def _issue_for_missing( + config: AttachmentConfig, + policy: AttachmentPolicyDecision, +) -> AttachmentIssue: code = "missing_required_attachment" if config.required else "missing_optional_attachment" - severity = ResolutionSeverity.ERROR if config.required and behavior == Behavior.BLOCK else ResolutionSeverity.WARNING + behavior = policy.effective_behavior + severity = ResolutionSeverity.ERROR if behavior == Behavior.BLOCK else ResolutionSeverity.WARNING return AttachmentIssue( severity=severity, code=code, message=f"No file matched attachment filter {config.file_filter!r}", behavior=behavior, + details={"effective_policy": policy.model_dump(mode="json")}, ) @@ -377,10 +430,13 @@ def _issue_for_ambiguous(config: AttachmentConfig, behavior: Behavior, match_cou ) -def _send_without_attachments_behavior(config: CampaignConfig) -> Behavior: - return config.attachments.send_without_attachments_behavior or ( +def effective_send_without_attachments_behavior(config: CampaignConfig) -> Behavior: + configured = config.attachments.send_without_attachments_behavior or ( Behavior.CONTINUE if config.attachments.send_without_attachments else Behavior.BLOCK ) + # Recipient exclusion must be an explicit reviewed action, not an implicit + # consequence of a legacy attachment policy value. + return Behavior.ASK if configured == Behavior.DROP else configured def _issue_for_missing_attachment_coverage(behavior: Behavior) -> AttachmentIssue: @@ -395,6 +451,7 @@ def _issue_for_missing_attachment_coverage(behavior: Behavior) -> AttachmentIssu code="missing_attachment_coverage", message=messages.get(behavior, "No attachment file was resolved for this message."), behavior=behavior, + details={"effective_behavior": behavior.value}, ) @@ -422,24 +479,13 @@ def _resolve_one_config( behavior: Behavior | None = None managed_source = selected_base_path is not None and is_managed_source(selected_base_path.source) unsafe_managed_path = False - if managed_source: - try: + resolution_failed = False + try: + if managed_source: assert_logical_relative_path( rendered_file_filter, field="rendered managed attachment file_filter", ) - except CampaignPathSecurityError as exc: - matches = [] - unsafe_managed_path = True - issues.append( - AttachmentIssue( - severity=ResolutionSeverity.ERROR, - code="unsafe_managed_attachment_path", - message=str(exc), - behavior=Behavior.BLOCK, - ) - ) - else: matches = _match_files(directory, rendered_file_filter, config.include_subdirs, match_index) matches, rejected = _confine_managed_matches(directory, matches) if rejected: @@ -453,16 +499,44 @@ def _resolve_one_config( behavior=Behavior.BLOCK, ) ) - else: - matches = _match_files(directory, rendered_file_filter, config.include_subdirs, match_index) + else: + matches = _match_files(directory, rendered_file_filter, config.include_subdirs, match_index) + except CampaignPathSecurityError as exc: + matches = [] + unsafe_managed_path = True + issues.append( + AttachmentIssue( + severity=ResolutionSeverity.ERROR, + code="unsafe_managed_attachment_path", + message=str(exc), + behavior=Behavior.BLOCK, + ) + ) + except (OSError, RuntimeError) as exc: + matches = [] + resolution_failed = True + issues.append( + AttachmentIssue( + severity=ResolutionSeverity.ERROR, + code="attachment_resolution_failed", + message=f"Attachment source could not be read while resolving filter {config.file_filter!r}.", + behavior=Behavior.BLOCK, + details={"error_type": type(exc).__name__}, + ) + ) + missing_policy: AttachmentPolicyDecision | None = None if unsafe_managed_path: status = AttachmentMatchStatus.MISSING behavior = Behavior.BLOCK + elif resolution_failed: + status = AttachmentMatchStatus.MISSING + behavior = Behavior.BLOCK elif not matches: status = AttachmentMatchStatus.MISSING - behavior = _missing_behavior(campaign_config, config) - issues.append(_issue_for_missing(config, behavior)) + missing_policy = _missing_policy_decision(campaign_config, config) + behavior = missing_policy.effective_behavior + issues.append(_issue_for_missing(config, missing_policy)) elif len(matches) > 1 and not allow_multiple: status = AttachmentMatchStatus.AMBIGUOUS behavior = _ambiguous_behavior(campaign_config, config) @@ -494,6 +568,7 @@ def _resolve_one_config( zip_entry_name_template=config.zip_entry_name_template, status=status, behavior=behavior, + missing_policy=missing_policy, matches=[str(path) for path in matches], issues=issues, ) @@ -540,7 +615,7 @@ def resolve_entry_attachments( ) issues = [issue for item in resolved for issue in item.issues] - missing_coverage_behavior = _send_without_attachments_behavior(config) + missing_coverage_behavior = effective_send_without_attachments_behavior(config) if ( entry.active and resolved diff --git a/src/govoplan_campaign/backend/campaign/mail_profile_boundary.py b/src/govoplan_campaign/backend/campaign/mail_profile_boundary.py index 2067b49..5547b32 100644 --- a/src/govoplan_campaign/backend/campaign/mail_profile_boundary.py +++ b/src/govoplan_campaign/backend/campaign/mail_profile_boundary.py @@ -22,6 +22,7 @@ CAMPAIGN_REVIEW_STATE_KEYS = frozenset( "build_token", "inspection_complete", "reviewed_message_keys", + "issue_decisions", "updated_at", "updated_by_user_id", } @@ -142,6 +143,22 @@ def public_campaign_editor_state( review_state = _validated_server_review_state(value["review_send"]) if not include_diagnostics: review_state.pop("build_token", None) + review_state["issue_decisions"] = [ + { + key: item[key] + for key in ( + "job_id", + "review_key", + "decision", + "reason", + "actor_user_id", + "decided_at", + "issue_codes", + ) + if key in item + } + for item in review_state.get("issue_decisions", []) + ] result["review_send"] = review_state except CampaignMailProfileBoundaryError: pass @@ -178,6 +195,102 @@ def _validated_review_actor(value: Any) -> str | None: return value +def _validated_issue_decisions(value: Any) -> list[dict[str, Any]]: + if not isinstance(value, list) or len(value) > 100_000: + raise CampaignMailProfileBoundaryError( + "Campaign review issue decisions are invalid" + ) + result: list[dict[str, Any]] = [] + allowed_keys = { + "job_id", + "review_key", + "decision", + "reason", + "actor_user_id", + "decided_at", + "build_token", + "message_sha256", + "issue_fingerprint", + "issue_codes", + } + for item in value: + if not isinstance(item, dict) or any( + key not in allowed_keys for key in item + ): + raise CampaignMailProfileBoundaryError( + "Campaign review issue decision is invalid" + ) + normalized = { + "job_id": _validated_required_string( + item.get("job_id"), + max_length=36, + error="Campaign review decision job is invalid", + ), + "review_key": _validated_required_string( + item.get("review_key"), + max_length=512, + error="Campaign review decision key is invalid", + ), + "decision": _validated_required_string( + item.get("decision"), + max_length=30, + error="Campaign review decision is invalid", + ), + "reason": item.get("reason"), + "actor_user_id": _validated_review_actor( + item.get("actor_user_id") + ), + "decided_at": _validated_required_string( + item.get("decided_at"), + max_length=128, + error="Campaign review decision timestamp is invalid", + ), + "build_token": _validated_required_string( + item.get("build_token"), + max_length=256, + error="Campaign review decision build token is invalid", + ), + "message_sha256": item.get("message_sha256"), + "issue_fingerprint": _validated_required_string( + item.get("issue_fingerprint"), + max_length=64, + error="Campaign review issue fingerprint is invalid", + ), + "issue_codes": item.get("issue_codes"), + } + if normalized["decision"] != "accept": + raise CampaignMailProfileBoundaryError( + "Campaign review decision outcome is invalid" + ) + if normalized["reason"] is not None and ( + not isinstance(normalized["reason"], str) + or len(normalized["reason"]) > 4_000 + ): + raise CampaignMailProfileBoundaryError( + "Campaign review decision reason is invalid" + ) + if normalized["message_sha256"] is not None and ( + not isinstance(normalized["message_sha256"], str) + or len(normalized["message_sha256"]) > 64 + ): + raise CampaignMailProfileBoundaryError( + "Campaign review decision message hash is invalid" + ) + if ( + not isinstance(normalized["issue_codes"], list) + or len(normalized["issue_codes"]) > 100 + or any( + not isinstance(code, str) or not code or len(code) > 100 + for code in normalized["issue_codes"] + ) + ): + raise CampaignMailProfileBoundaryError( + "Campaign review decision issue codes are invalid" + ) + result.append(normalized) + return result + + def _validated_server_review_state(value: Any) -> dict[str, Any]: if not isinstance(value, dict) or any( key not in CAMPAIGN_REVIEW_STATE_KEYS for key in value @@ -188,6 +301,7 @@ def _validated_server_review_state(value: Any) -> dict[str, Any]: build_token = value.get("build_token") inspected = value.get("inspection_complete") keys = value.get("reviewed_message_keys", []) + issue_decisions = value.get("issue_decisions", []) updated_at = value.get("updated_at") updated_by = value.get("updated_by_user_id") validated_build_token = _validated_required_string( @@ -200,6 +314,7 @@ def _validated_server_review_state(value: Any) -> dict[str, Any]: "Campaign review completion state is invalid" ) validated_keys = _validated_reviewed_message_keys(keys) + validated_decisions = _validated_issue_decisions(issue_decisions) validated_updated_at = _validated_required_string( updated_at, max_length=128, @@ -210,6 +325,7 @@ def _validated_server_review_state(value: Any) -> dict[str, Any]: "build_token": validated_build_token, "inspection_complete": inspected, "reviewed_message_keys": validated_keys, + "issue_decisions": validated_decisions, "updated_at": validated_updated_at, "updated_by_user_id": validated_updated_by, } diff --git a/src/govoplan_campaign/backend/campaign/models.py b/src/govoplan_campaign/backend/campaign/models.py index 9668c0a..cb7d1f2 100644 --- a/src/govoplan_campaign/backend/campaign/models.py +++ b/src/govoplan_campaign/backend/campaign/models.py @@ -480,7 +480,7 @@ class AttachmentsConfig(StrictModel): send_without_attachments_behavior: Behavior | None = None zip: ZipCollectionConfig = Field(default_factory=ZipCollectionConfig) global_: list[AttachmentConfig] = Field(default_factory=list, alias="global") - missing_behavior: Behavior = Behavior.ASK + missing_behavior: Behavior = Behavior.WARN ambiguous_behavior: Behavior = Behavior.ASK @model_validator(mode="after") @@ -638,7 +638,7 @@ class EntriesConfig(StrictModel): class ValidationPolicy(StrictModel): - missing_required_attachment: Behavior = Behavior.ASK + missing_required_attachment: Behavior = Behavior.BLOCK missing_optional_attachment: Behavior = Behavior.WARN ambiguous_attachment_match: Behavior = Behavior.ASK ignore_empty_fields: bool = False diff --git a/src/govoplan_campaign/backend/capabilities.py b/src/govoplan_campaign/backend/capabilities.py index de9d2e4..5b32bef 100644 --- a/src/govoplan_campaign/backend/capabilities.py +++ b/src/govoplan_campaign/backend/capabilities.py @@ -2,6 +2,7 @@ from __future__ import annotations from collections.abc import Iterable, Mapping +from fastapi import HTTPException from sqlalchemy import and_, or_ from govoplan_core.core.access import AccessDecisionProvenance, PrincipalRef @@ -14,12 +15,61 @@ from govoplan_core.core.campaigns import ( CampaignPolicyContextProvider, CampaignRetentionProvider, ) +from govoplan_core.core.ownership import ( + OwnershipActionDecision, + OwnershipSubjectRef, + OwnershipTransferError, +) from govoplan_core.security.module_permissions import scopes_grant_compatible -from govoplan_campaign.backend.db.models import Campaign, CampaignShare +from govoplan_campaign.backend.db.models import ( + Campaign, + CampaignJob, + CampaignShare, + CampaignVersion, +) READ_ACTIONS = {"campaigns:campaign:read", "campaign:read"} +CAMPAIGN_RESOURCE_TYPES = { + "campaign", + "campaign_object", + "campaigns:campaign", +} +CAMPAIGN_VERSION_RESOURCE_TYPES = { + "campaign_version", + "campaigns:version", +} +CAMPAIGN_DELIVERY_JOB_RESOURCE_TYPES = { + "campaign_delivery_job", + "campaign_job", + "campaigns:delivery_job", +} +CAMPAIGN_REPORT_RESOURCE_TYPES = { + "campaign_report", + "campaigns:report", +} + + +def campaign_report_resource_id( + *, + campaign_id: str, + version_id: str, + report_kind: str, +) -> str: + kind = report_kind.strip().lower() + if not campaign_id or not version_id or not kind or ":" in kind: + raise ValueError("Campaign report references require campaign, version, and a bounded kind") + return f"{campaign_id}:{version_id}:{kind}" + + +def _campaign_report_reference( + resource_id: str, +) -> tuple[str, str, str] | None: + parts = resource_id.split(":", 2) + if len(parts) != 3 or not all(part.strip() for part in parts): + return None + return parts[0], parts[1], parts[2].strip().lower() class CampaignMailPolicyContextService(CampaignMailPolicyContextProvider): @@ -120,20 +170,112 @@ class CampaignAccessService(CampaignAccessProvider): action: str, ) -> tuple[AccessDecisionProvenance, ...]: normalized_type = resource_type.lower().strip() - if normalized_type not in {"campaign", "campaign_object", "campaigns:campaign"}: - return () - campaign = session.get(Campaign, resource_id) # type: ignore[attr-defined] - if campaign is None or (principal.tenant_id and campaign.tenant_id != principal.tenant_id): - return ( - AccessDecisionProvenance( - kind="resource", - id=resource_id, - tenant_id=principal.tenant_id, - source="campaigns.not_found", - details={"resource_type": "campaign", "found": False}, - ), + child_item: AccessDecisionProvenance | None = None + campaign: Campaign | None + if normalized_type in CAMPAIGN_RESOURCE_TYPES: + campaign = session.get(Campaign, resource_id) # type: ignore[attr-defined] + elif normalized_type in CAMPAIGN_VERSION_RESOURCE_TYPES: + version = session.get(CampaignVersion, resource_id) # type: ignore[attr-defined] + if version is None: + return _missing_resource_provenance( + principal, + resource_type="campaign_version", + resource_id=resource_id, + ) + campaign = session.get(Campaign, version.campaign_id) # type: ignore[attr-defined] + child_item = AccessDecisionProvenance( + kind="resource", + id=version.id, + label=f"Version {version.version_number}", + tenant_id=campaign.tenant_id if campaign else principal.tenant_id, + source="campaigns.version", + details={ + "resource_type": "campaign_version", + "campaign_id": version.campaign_id, + "version_number": version.version_number, + "workflow_state": version.workflow_state, + "authorization_inherited_from": { + "resource_type": "campaign", + "resource_id": version.campaign_id, + }, + }, ) - items = [ + elif normalized_type in CAMPAIGN_DELIVERY_JOB_RESOURCE_TYPES: + job = session.get(CampaignJob, resource_id) # type: ignore[attr-defined] + if job is None: + return _missing_resource_provenance( + principal, + resource_type="campaign_delivery_job", + resource_id=resource_id, + ) + campaign = session.get(Campaign, job.campaign_id) # type: ignore[attr-defined] + child_item = AccessDecisionProvenance( + kind="resource", + id=job.id, + label=job.recipient_email or f"Recipient {job.entry_index}", + tenant_id=job.tenant_id, + source="campaigns.delivery_job", + details={ + "resource_type": "campaign_delivery_job", + "campaign_id": job.campaign_id, + "campaign_version_id": job.campaign_version_id, + "queue_status": job.queue_status, + "send_status": job.send_status, + "authorization_inherited_from": { + "resource_type": "campaign", + "resource_id": job.campaign_id, + }, + }, + ) + elif normalized_type in CAMPAIGN_REPORT_RESOURCE_TYPES: + reference = _campaign_report_reference(resource_id) + if reference is None: + return _missing_resource_provenance( + principal, + resource_type="campaign_report", + resource_id=resource_id, + reason="invalid_reference", + ) + campaign_id, version_id, report_kind = reference + campaign = session.get(Campaign, campaign_id) # type: ignore[attr-defined] + version = session.get(CampaignVersion, version_id) # type: ignore[attr-defined] + if ( + campaign is None + or version is None + or version.campaign_id != campaign.id + ): + return _missing_resource_provenance( + principal, + resource_type="campaign_report", + resource_id=resource_id, + ) + child_item = AccessDecisionProvenance( + kind="resource", + id=resource_id, + label=report_kind, + tenant_id=campaign.tenant_id, + source="campaigns.report", + details={ + "resource_type": "campaign_report", + "campaign_id": campaign.id, + "campaign_version_id": version.id, + "report_kind": report_kind, + "persisted": False, + "authorization_inherited_from": { + "resource_type": "campaign", + "resource_id": campaign.id, + }, + }, + ) + else: + return () + if campaign is None or (principal.tenant_id and campaign.tenant_id != principal.tenant_id): + return _missing_resource_provenance( + principal, + resource_type=normalized_type, + resource_id=resource_id, + ) + items = ([child_item] if child_item is not None else []) + [ AccessDecisionProvenance( kind="resource", id=campaign.id, @@ -149,7 +291,11 @@ class CampaignAccessService(CampaignAccessProvider): ] items.extend(_owner_provenance(campaign, principal)) items.extend(_tenant_admin_provenance(principal)) - permission_values = {"read", "write"} if action in READ_ACTIONS else {"write"} + permission_values = ( + {"read", "write"} + if action in READ_ACTIONS or action.strip().lower().endswith(":read") + else {"write"} + ) shares = ( session.query(CampaignShare) # type: ignore[attr-defined] .filter( @@ -187,6 +333,283 @@ def access_capability(context: object) -> CampaignAccessService: return CampaignAccessService() +class CampaignOwnershipService: + def current_owner( + self, + session: object, + *, + tenant_id: str, + resource_id: str, + ) -> OwnershipSubjectRef | None: + campaign = session.get(Campaign, resource_id) # type: ignore[attr-defined] + if campaign is None or campaign.tenant_id != tenant_id: + return None + if campaign.owner_user_id: + return OwnershipSubjectRef(type="user", id=campaign.owner_user_id) + if campaign.owner_group_id: + return OwnershipSubjectRef(type="group", id=campaign.owner_group_id) + return None + + def authorize_ownership_action( + self, + session: object, + *, + tenant_id: str, + resource_id: str, + action: str, + actor: OwnershipSubjectRef, + current_owner: OwnershipSubjectRef, + target_owner: OwnershipSubjectRef, + ) -> OwnershipActionDecision: + campaign = session.get(Campaign, resource_id) # type: ignore[attr-defined] + if campaign is None or campaign.tenant_id != tenant_id: + return OwnershipActionDecision(False, "Campaign was not found") + if target_owner.type not in {"user", "group"}: + return OwnershipActionDecision( + False, + "Campaign ownership can be assigned only to a user or group", + ) + + actor_is_current_owner = _actor_controls_owner(actor, current_owner) + actor_is_target = _actor_controls_owner(actor, target_owner) + actor_can_share = scopes_grant_compatible( + actor.scopes, + "campaigns:campaign:share", + ) + actor_can_accept_for_group = scopes_grant_compatible( + actor.scopes, + "campaigns:ownership:accept_group", + ) + actor_can_recover = scopes_grant_compatible( + actor.scopes, + "campaigns:ownership:recover", + ) + actor_can_read = ( + scopes_grant_compatible( + actor.scopes, + "campaigns:campaign:read", + ) + and CampaignAccessService().can_read_campaign( + session, + tenant_id=tenant_id, + campaign_id=resource_id, + user_id=actor.id, + group_ids=actor.group_ids, + tenant_admin=scopes_grant_compatible( + actor.scopes, + "tenant:*", + ), + ) + ) + + if action in { + "propose_transfer", + "request_ownership", + "request_recovery", + }: + target_decision = _valid_campaign_owner_target( + tenant_id=tenant_id, + target_owner=target_owner, + ) + if target_decision is not None: + return target_decision + + if action == "view_transfer": + allowed = ( + _actor_is_owner_member(actor, current_owner) + or _actor_is_owner_member(actor, target_owner) + or actor_can_recover + ) + elif action in { + "propose_transfer", + "approve_requested_transfer", + }: + allowed = actor_is_current_owner and actor_can_share + elif action == "cancel_transfer": + allowed = ( + (actor_is_current_owner and actor_can_share) + or actor_is_target + ) + elif action == "request_ownership": + allowed = actor_can_read and actor_is_target and ( + target_owner.type != "group" or actor_can_accept_for_group + ) + elif action == "accept_transfer": + allowed = target_owner.type == "user" and actor_is_target + elif action == "accept_group_transfer": + allowed = ( + target_owner.type == "group" + and actor_is_target + and actor_can_accept_for_group + ) + elif action == "decline_transfer": + allowed = ( + actor_is_target + and ( + target_owner.type != "group" + or actor_can_accept_for_group + ) + ) or ( + actor_is_current_owner + and actor_can_share + ) + elif action in { + "request_recovery", + "approve_recovery", + "execute_recovery", + }: + allowed = actor_can_recover + else: + allowed = False + + return OwnershipActionDecision( + allowed=allowed, + reason=None if allowed else f"Campaign ownership action is not allowed: {action}", + ) + + def apply_owner( + self, + session: object, + *, + tenant_id: str, + resource_id: str, + expected_owner: OwnershipSubjectRef, + target_owner: OwnershipSubjectRef, + actor: OwnershipSubjectRef, + reason: str | None, + ) -> None: + del actor, reason + campaign = session.get(Campaign, resource_id) # type: ignore[attr-defined] + if campaign is None or campaign.tenant_id != tenant_id: + raise OwnershipTransferError("Campaign was not found") + if target_owner.type not in {"user", "group"}: + raise OwnershipTransferError( + "Campaign ownership can be assigned only to a user or group" + ) + from govoplan_campaign.backend.route_support import ( + _clear_current_version_mail_profile_for_owner_transfer, + ) + + try: + _clear_current_version_mail_profile_for_owner_transfer( + session, # type: ignore[arg-type] + campaign, + ) + except HTTPException as exc: + raise OwnershipTransferError( + str( + exc.detail + or "Campaign owner cannot be changed in its current state" + ) + ) from exc + expected_filter = ( + ( + Campaign.owner_user_id == expected_owner.id, + Campaign.owner_group_id.is_(None), + ) + if expected_owner.type == "user" + else ( + Campaign.owner_user_id.is_(None), + Campaign.owner_group_id == expected_owner.id, + ) + ) + updated = ( + session.query(Campaign) # type: ignore[attr-defined] + .filter( + Campaign.id == resource_id, + Campaign.tenant_id == tenant_id, + *expected_filter, + ) + .update( + { + Campaign.owner_user_id: ( + target_owner.id + if target_owner.type == "user" + else None + ), + Campaign.owner_group_id: ( + target_owner.id + if target_owner.type == "group" + else None + ), + }, + synchronize_session=False, + ) + ) + if updated != 1: + raise OwnershipTransferError( + "Campaign owner changed while the transfer was pending" + ) + session.expire(campaign) # type: ignore[attr-defined] + + +def _valid_campaign_owner_target( + *, + tenant_id: str, + target_owner: OwnershipSubjectRef, +) -> OwnershipActionDecision | None: + from govoplan_campaign.backend.route_support import _access_directory + + directory = _access_directory() + if target_owner.type == "user": + item = directory.get_user(target_owner.id) + elif target_owner.type == "group": + item = directory.get_group(target_owner.id) + else: + return OwnershipActionDecision( + False, + "Campaign ownership can be assigned only to a user or group", + ) + if ( + item is None + or item.tenant_id != tenant_id + or item.status != "active" + ): + return OwnershipActionDecision( + False, + f"Target {target_owner.type} is not active in this tenant", + ) + return None + + +def _actor_is_owner_member( + actor: OwnershipSubjectRef, + owner: OwnershipSubjectRef, +) -> bool: + if owner.type == "group": + return owner.id in actor.group_ids + return actor.type == owner.type and actor.id == owner.id + + +def _actor_controls_owner( + actor: OwnershipSubjectRef, + owner: OwnershipSubjectRef, +) -> bool: + return _actor_is_owner_member(actor, owner) + + +def _missing_resource_provenance( + principal: PrincipalRef, + *, + resource_type: str, + resource_id: str, + reason: str = "not_found", +) -> tuple[AccessDecisionProvenance, ...]: + return ( + AccessDecisionProvenance( + kind="resource", + id=resource_id, + tenant_id=principal.tenant_id, + source="campaigns.not_found", + details={ + "resource_type": resource_type, + "found": False, + "reason": reason, + }, + ), + ) + + def _owner_provenance(campaign: Campaign, principal: PrincipalRef) -> tuple[AccessDecisionProvenance, ...]: if campaign.owner_user_id and campaign.owner_user_id == principal.membership_id: return ( diff --git a/src/govoplan_campaign/backend/manifest.py b/src/govoplan_campaign/backend/manifest.py index b6ddfd8..15efa97 100644 --- a/src/govoplan_campaign/backend/manifest.py +++ b/src/govoplan_campaign/backend/manifest.py @@ -11,6 +11,7 @@ from govoplan_core.core.campaigns import ( CAPABILITY_CAMPAIGNS_RETENTION, ) from govoplan_core.core.module_guards import drop_table_retirement_provider, persistent_table_uninstall_guard +from govoplan_core.core.ownership import OwnershipProviderRegistration from govoplan_core.core.modules import ( DocumentationCondition, DocumentationLink, @@ -63,6 +64,8 @@ PERMISSIONS = ( _permission("campaigns:campaign:archive", "Archive campaigns", "Archive campaigns without destroying audit evidence.", "Campaigns"), _permission("campaigns:campaign:delete", "Delete campaigns", "Delete draft-only campaigns where retention policy allows it.", "Campaigns"), _permission("campaigns:campaign:share", "Share campaigns", "Grant or revoke explicit campaign access.", "Campaigns"), + _permission("campaigns:ownership:accept_group", "Accept group campaign ownership", "Accept or decline campaign ownership on behalf of a group the actor belongs to.", "Campaign governance"), + _permission("campaigns:ownership:recover", "Recover campaign ownership", "Participate in delayed, quorum-backed administrative campaign ownership recovery.", "Campaign governance"), _permission("campaigns:campaign:validate", "Validate campaigns", "Run technical validation and manage validation locks.", "Campaigns"), _permission("campaigns:campaign:build", "Build campaigns", "Build exact messages and attachment evidence.", "Campaigns"), _permission("campaigns:campaign:review", "Complete campaign review", "Record review completion and the exact built messages inspected.", "Campaigns"), @@ -118,6 +121,7 @@ ROLE_TEMPLATES = ( "campaigns:campaign:copy", "campaigns:campaign:validate", "campaigns:campaign:build", + "campaigns:ownership:accept_group", "campaigns:recipient:read", "campaigns:recipient:write", "campaigns:recipient:import", @@ -715,6 +719,15 @@ manifest = ModuleManifest( ), ), documentation_providers=(documentation_topics,), + ownership_providers=( + OwnershipProviderRegistration( + resource_type="campaign", + provider=__import__( + "govoplan_campaign.backend.capabilities", + fromlist=["CampaignOwnershipService"], + ).CampaignOwnershipService(), + ), + ), capability_factories={ CAPABILITY_CAMPAIGNS_ACCESS: lambda context: __import__( "govoplan_campaign.backend.capabilities", diff --git a/src/govoplan_campaign/backend/messages/builder.py b/src/govoplan_campaign/backend/messages/builder.py index 572fc31..abcb700 100644 --- a/src/govoplan_campaign/backend/messages/builder.py +++ b/src/govoplan_campaign/backend/messages/builder.py @@ -16,6 +16,7 @@ from govoplan_campaign.backend.attachments.resolver import ( EntryAttachmentResolution, MessageAttachmentStatus, ResolvedAttachment, + effective_send_without_attachments_behavior, resolve_entry_attachments, ) from govoplan_campaign.backend.campaign.addressing import effective_address_lists, formatted_recipient @@ -172,6 +173,11 @@ def _attachment_summaries(resolution: EntryAttachmentResolution) -> list[Message label=attachment.label, status=attachment.status.value, behavior=attachment.behavior.value if attachment.behavior else None, + missing_policy=( + attachment.missing_policy.model_dump(mode="json") + if attachment.missing_policy + else None + ), required=attachment.required, allow_multiple=attachment.allow_multiple, zip_enabled=attachment.zip_enabled, @@ -200,6 +206,7 @@ def _message_issues_from_attachment_resolution(resolution: EntryAttachmentResolu message=issue.message, behavior=issue.behavior.value if issue.behavior else None, source="attachments", + details=issue.details, ) for issue in resolution.issues ] @@ -219,12 +226,6 @@ def _append_no_attachment_coverage_issue(issues: list[MessageIssue]) -> None: ) -def _send_without_attachments_behavior(config: CampaignConfig) -> Behavior: - return config.attachments.send_without_attachments_behavior or ( - Behavior.CONTINUE if config.attachments.send_without_attachments else Behavior.BLOCK - ) - - def _safe_filename(value: str | None, fallback: str) -> str: raw = value or fallback safe = re.sub(r"[^A-Za-z0-9_.-]+", "_", raw).strip("._") @@ -761,7 +762,11 @@ def _build_mime_message( values=rendered.values, work_dir=work_dir, ) - if attachment_count == 0 and context.resolution.attachments and _send_without_attachments_behavior(config) == Behavior.BLOCK: + if ( + attachment_count == 0 + and context.resolution.attachments + and effective_send_without_attachments_behavior(config) == Behavior.BLOCK + ): _append_no_attachment_coverage_issue(context.issues) return _MimeBuildResult( message=None, @@ -785,6 +790,16 @@ def _build_mime_message( source="attachments", ) ) + except OSError as exc: + context.issues.append( + MessageIssue( + severity="error", + code="attachment_unreadable", + message="A resolved attachment could not be read while building the message.", + behavior="block", + source=f"attachments:{type(exc).__name__}", + ) + ) except Exception as exc: context.issues.append( MessageIssue( diff --git a/src/govoplan_campaign/backend/messages/models.py b/src/govoplan_campaign/backend/messages/models.py index a2ca046..cd5be4d 100644 --- a/src/govoplan_campaign/backend/messages/models.py +++ b/src/govoplan_campaign/backend/messages/models.py @@ -33,6 +33,7 @@ class MessageIssue(BaseModel): message: str behavior: str | None = None source: str | None = None + details: dict[str, object] = Field(default_factory=dict) class MessageAddress(BaseModel): @@ -49,6 +50,7 @@ class MessageAttachmentSummary(BaseModel): label: str | None = None status: str behavior: str | None = None + missing_policy: dict[str, object] | None = None required: bool allow_multiple: bool zip_enabled: bool diff --git a/src/govoplan_campaign/backend/persistence/versions.py b/src/govoplan_campaign/backend/persistence/versions.py index 79a8fcb..a7aed2c 100644 --- a/src/govoplan_campaign/backend/persistence/versions.py +++ b/src/govoplan_campaign/backend/persistence/versions.py @@ -1,6 +1,8 @@ from __future__ import annotations import copy +import hashlib +import json from dataclasses import dataclass, field from datetime import UTC, datetime from typing import Any @@ -157,7 +159,7 @@ def minimal_campaign_json(*, external_id: str, name: str, description: str | Non "send_without_attachments_behavior": "block", "global": [], "zip": {"enabled": False, "archives": []}, - "missing_behavior": "ask", + "missing_behavior": "warn", "ambiguous_behavior": "ask", }, "entries": { @@ -175,7 +177,7 @@ def minimal_campaign_json(*, external_id: str, name: str, description: str | Non }, }, "validation_policy": { - "missing_required_attachment": "ask", + "missing_required_attachment": "block", "missing_optional_attachment": "warn", "ambiguous_attachment_match": "ask", "unsent_attachment_files": "warn", @@ -832,6 +834,7 @@ def update_campaign_review_state( version_id: str, inspection_complete: bool, reviewed_message_keys: list[str], + issue_decisions: list[dict[str, Any]] | None = None, user_id: str | None, commit: bool = True, ) -> CampaignVersion: @@ -854,13 +857,22 @@ def update_campaign_review_state( raise LockedCampaignVersionError("Delivery has started; message review state can no longer be changed.") build_token = _campaign_review_build_token(version) normalized_reviewed = list(dict.fromkeys(str(value) for value in reviewed_message_keys if str(value).strip())) + normalized_decisions: list[dict[str, Any]] = [] if inspection_complete: - normalized_reviewed = _complete_campaign_review_keys(session, version, normalized_reviewed) + normalized_reviewed, normalized_decisions = _complete_campaign_review( + session, + version, + normalized_reviewed, + issue_decisions or [], + user_id=user_id, + build_token=build_token, + ) _write_campaign_review_state( version, build_token=build_token, inspection_complete=inspection_complete, reviewed_message_keys=normalized_reviewed, + issue_decisions=normalized_decisions, user_id=user_id, ) session.add(version) @@ -885,11 +897,15 @@ def _campaign_review_build_token(version: CampaignVersion) -> str: return build_token -def _complete_campaign_review_keys( +def _complete_campaign_review( session: Session, version: CampaignVersion, reviewed_message_keys: list[str], -) -> list[str]: + issue_decisions: list[dict[str, Any]], + *, + user_id: str | None, + build_token: str, +) -> tuple[list[str], list[dict[str, Any]]]: jobs = ( session.query(CampaignJob) .filter(CampaignJob.campaign_version_id == version.id) @@ -904,7 +920,123 @@ def _complete_campaign_review_keys( raise CampaignPersistenceError( "Messages requiring an explicit decision must be opened before review can be completed: " + ", ".join(missing) ) - return list(dict.fromkeys([*reviewed_message_keys, *_bulk_acceptable_review_keys(jobs)])) + decisions = _normalize_review_issue_decisions( + jobs, + issue_decisions, + user_id=user_id, + build_token=build_token, + ) + return ( + list( + dict.fromkeys( + [*reviewed_message_keys, *_bulk_acceptable_review_keys(jobs)] + ) + ), + decisions, + ) + + +def _normalize_review_issue_decisions( + jobs: list[CampaignJob], + requested: list[dict[str, Any]], + *, + user_id: str | None, + build_token: str, + decided_at: datetime | None = None, +) -> list[dict[str, Any]]: + jobs_by_id = {job.id: job for job in jobs} + requested_by_job: dict[str, dict[str, Any]] = {} + for item in requested: + job_id = str(item.get("job_id") or "").strip() + if not job_id or job_id not in jobs_by_id: + raise CampaignPersistenceError( + "A review decision references a message outside the current build." + ) + if jobs_by_id[job_id].validation_status != "needs_review": + raise CampaignPersistenceError( + "Review decisions are accepted only for messages requiring review." + ) + if job_id in requested_by_job: + raise CampaignPersistenceError( + "Only one review decision may be recorded per built message." + ) + if str(item.get("decision") or "accept") != "accept": + raise CampaignPersistenceError( + "Unsupported campaign review decision." + ) + requested_by_job[job_id] = item + + timestamp = (decided_at or datetime.now(UTC)).isoformat() + normalized: list[dict[str, Any]] = [] + for job in jobs: + reviewable_issues = [ + issue + for issue in (job.issues_snapshot or []) + if isinstance(issue, dict) + and str(issue.get("behavior") or "").lower() == "ask" + ] + attachment_overrides = [ + issue + for issue in reviewable_issues + if str(issue.get("source") or "").startswith("attachments") + ] + decision = requested_by_job.get(job.id) + if attachment_overrides and decision is None: + raise CampaignPersistenceError( + "Attachment exceptions require an explicit reason before review can be completed " + f"for message {job.entry_id or job.entry_index}." + ) + if decision is None: + continue + reason = str(decision.get("reason") or "").strip() + if attachment_overrides and not reason: + raise CampaignPersistenceError( + "Attachment exception decisions require a reason." + ) + evidence_issues = reviewable_issues or [ + issue + for issue in (job.issues_snapshot or []) + if isinstance(issue, dict) + ] + issue_payload = [ + { + "code": str(issue.get("code") or ""), + "behavior": str(issue.get("behavior") or ""), + "source": str(issue.get("source") or ""), + "details": issue.get("details") + if isinstance(issue.get("details"), dict) + else {}, + } + for issue in evidence_issues + ] + fingerprint = hashlib.sha256( + json.dumps( + issue_payload, + sort_keys=True, + separators=(",", ":"), + ).encode("utf-8") + ).hexdigest() + normalized.append( + { + "job_id": job.id, + "review_key": str(job.entry_id or job.entry_index), + "decision": "accept", + "reason": reason or None, + "actor_user_id": user_id, + "decided_at": timestamp, + "build_token": build_token, + "message_sha256": job.eml_sha256, + "issue_fingerprint": fingerprint, + "issue_codes": sorted( + { + str(issue.get("code") or "") + for issue in evidence_issues + if issue.get("code") + } + ), + } + ) + return normalized def _required_review_keys(jobs: list[CampaignJob]) -> set[str]: @@ -929,6 +1061,7 @@ def _write_campaign_review_state( build_token: str, inspection_complete: bool, reviewed_message_keys: list[str], + issue_decisions: list[dict[str, Any]], user_id: str | None, ) -> None: editor_state = copy.deepcopy(version.editor_state or {}) @@ -936,6 +1069,7 @@ def _write_campaign_review_state( "build_token": build_token, "inspection_complete": bool(inspection_complete), "reviewed_message_keys": reviewed_message_keys, + "issue_decisions": issue_decisions, "updated_at": datetime.now(UTC).isoformat(), "updated_by_user_id": user_id, } diff --git a/src/govoplan_campaign/backend/reports/campaigns.py b/src/govoplan_campaign/backend/reports/campaigns.py index 0680c36..72ec876 100644 --- a/src/govoplan_campaign/backend/reports/campaigns.py +++ b/src/govoplan_campaign/backend/reports/campaigns.py @@ -478,7 +478,91 @@ def _recent_failures(jobs: list[CampaignJob], *, limit: int = 20) -> list[dict[s ] -def _job_row(job: CampaignJob, *, include_diagnostics: bool = False) -> dict[str, Any]: +def _review_decisions_by_job( + version: CampaignVersion | None, +) -> dict[str, dict[str, Any]]: + if version is None: + return {} + editor_state = ( + getattr(version, "editor_state", None) + if isinstance(getattr(version, "editor_state", None), dict) + else {} + ) + review_state = editor_state.get("review_send") + build_summary = ( + getattr(version, "build_summary", None) + if isinstance(getattr(version, "build_summary", None), dict) + else {} + ) + build_token = str( + build_summary.get("build_token") + or build_summary.get("built_at") + or "" + ) + if ( + not isinstance(review_state, dict) + or review_state.get("inspection_complete") is not True + or not build_token + or str(review_state.get("build_token") or "") != build_token + ): + return {} + decisions: dict[str, dict[str, Any]] = {} + for item in review_state.get("issue_decisions") or []: + if not isinstance(item, dict): + continue + job_id = str(item.get("job_id") or "").strip() + if job_id and job_id not in decisions: + decisions[job_id] = item + return decisions + + +def _public_review_decision( + decision: dict[str, Any] | None, + *, + include_diagnostics: bool, +) -> dict[str, Any] | None: + if not decision: + return None + result = { + "decision": decision.get("decision"), + "reason": decision.get("reason"), + "actor_user_id": decision.get("actor_user_id"), + "decided_at": decision.get("decided_at"), + "issue_codes": list(decision.get("issue_codes") or []), + } + if include_diagnostics: + result.update( + { + "review_key": decision.get("review_key"), + "message_sha256": decision.get("message_sha256"), + "issue_fingerprint": decision.get("issue_fingerprint"), + } + ) + return result + + +def _review_decision_summary( + decisions: dict[str, dict[str, Any]], +) -> dict[str, Any]: + issue_codes: Counter[str] = Counter() + for decision in decisions.values(): + issue_codes.update( + str(code) + for code in decision.get("issue_codes") or [] + if code + ) + return { + "exception_decision_count": len(decisions), + "by_issue_code": dict(issue_codes), + } + + +def _job_row( + job: CampaignJob, + *, + include_diagnostics: bool = False, + review_decision: dict[str, Any] | None = None, +) -> dict[str, Any]: row = { "job_id": job.id, "entry_index": job.entry_index, @@ -519,6 +603,10 @@ def _job_row(job: CampaignJob, *, include_diagnostics: bool = False) -> dict[str "issues_count": len(job.issues_snapshot or []), "attachment_config_count": len(job.resolved_attachments or []), "matched_file_count": sum(len(item.get("matches") or []) for item in (job.resolved_attachments or []) if isinstance(item, dict)), + "review_decision": _public_review_decision( + review_decision, + include_diagnostics=include_diagnostics, + ), } if include_diagnostics: row.update( @@ -592,8 +680,14 @@ def _job_evidence_row( latest_imap: ImapAppendAttempt | None = None, latest_message_action: CampaignMessageAction | None = None, include_diagnostics: bool = False, + review_decision: dict[str, Any] | None = None, ) -> dict[str, Any]: - row = _job_row(job, include_diagnostics=include_diagnostics) + row = _job_row( + job, + include_diagnostics=include_diagnostics, + review_decision=review_decision, + ) + public_decision = row.pop("review_decision") or {} recipients = job.resolved_recipients or {} row.update({ "campaign_id": job.campaign_id, @@ -617,7 +711,27 @@ def _job_evidence_row( if latest_message_action else None ), + "review_decision": public_decision.get("decision"), + "review_reason": public_decision.get("reason"), + "review_actor_user_id": public_decision.get("actor_user_id"), + "review_decided_at": public_decision.get("decided_at"), + "review_issue_codes": "; ".join( + str(code) + for code in public_decision.get("issue_codes") or [] + ), }) + if include_diagnostics: + row.update( + { + "review_key": public_decision.get("review_key"), + "review_message_sha256": public_decision.get( + "message_sha256" + ), + "review_issue_fingerprint": public_decision.get( + "issue_fingerprint" + ), + } + ) return row @@ -684,6 +798,7 @@ def generate_campaign_report( ) aggregate = _JobReportAggregate() job_rows: list[dict[str, Any]] = [] + review_decisions = _review_decisions_by_job(version) retry_max_attempts = _retry_max_attempts(version) iterator = jobs.yield_per(500) if hasattr(jobs, "yield_per") else jobs for job in iterator: @@ -700,7 +815,11 @@ def generate_campaign_report( "or the paginated recipient table." ) job_rows.append( - _job_row(job, include_diagnostics=include_diagnostics) + _job_row( + job, + include_diagnostics=include_diagnostics, + review_decision=review_decisions.get(job.id), + ) ) report = _campaign_report_payload( session, @@ -710,6 +829,7 @@ def generate_campaign_report( tenant_id=tenant_id, include_recent_failures=include_recent_failures, include_diagnostics=include_diagnostics, + review_decisions=review_decisions, ) if include_jobs: report["jobs"] = job_rows @@ -743,6 +863,7 @@ def _campaign_report_payload( tenant_id: str, include_recent_failures: bool, include_diagnostics: bool, + review_decisions: dict[str, dict[str, Any]] | None = None, ) -> dict[str, Any]: report = { "generated_at": _utcnow_iso(), @@ -775,6 +896,7 @@ def _campaign_report_payload( "by_status": dict(aggregate.attachment_status), "by_behavior": dict(aggregate.attachment_behavior), }, + "review": _review_decision_summary(review_decisions or {}), "attempts": _campaign_report_attempt_counts( session, tenant_id=tenant_id, @@ -1199,6 +1321,7 @@ def generate_jobs_csv( session, job_ids, ) + review_decisions = _review_decisions_by_job(version) rows = [ _job_evidence_row( job, @@ -1206,6 +1329,7 @@ def generate_jobs_csv( latest_imap=latest_imap.get(job.id), latest_message_action=latest_message_actions.get(job.id), include_diagnostics=include_diagnostics, + review_decision=review_decisions.get(job.id), ) for job in jobs ] @@ -1258,10 +1382,22 @@ def generate_jobs_csv( "latest_message_action_status", "latest_message_action_id", "latest_message_action_at", + "review_decision", + "review_reason", + "review_actor_user_id", + "review_decided_at", + "review_issue_codes", ] if include_diagnostics: sent_at_index = fieldnames.index("outcome_unknown_at") fieldnames[sent_at_index:sent_at_index] = ["claimed_at", "smtp_started_at"] + fieldnames.extend( + [ + "review_key", + "review_message_sha256", + "review_issue_fingerprint", + ] + ) buffer = io.StringIO() writer = csv.DictWriter(buffer, fieldnames=fieldnames) writer.writeheader() diff --git a/src/govoplan_campaign/backend/routes/sharing.py b/src/govoplan_campaign/backend/routes/sharing.py index 7c696df..03639cd 100644 --- a/src/govoplan_campaign/backend/routes/sharing.py +++ b/src/govoplan_campaign/backend/routes/sharing.py @@ -35,7 +35,6 @@ from govoplan_core.security.time import utc_now from govoplan_campaign.backend.route_support import ( _access_directory, - _clear_current_version_mail_profile_for_owner_transfer, _get_campaign_for_principal, ) @@ -170,59 +169,20 @@ def update_campaign_owner( session: Session = Depends(get_session), principal: ApiPrincipal = Depends(require_scope("campaigns:campaign:share")), ): - campaign = _get_campaign_for_principal(session, campaign_id, principal, write=True) - if payload.owner_user_id and payload.owner_group_id: - raise HTTPException( - status_code=status.HTTP_422_UNPROCESSABLE_CONTENT, - detail="Choose either a user owner or a group owner, not both", - ) - directory = _access_directory() - if payload.owner_user_id: - owner = directory.get_user(payload.owner_user_id) - if owner is not None and ( - owner.tenant_id != principal.tenant_id or owner.status != "active" - ): - owner = None - if owner is None: - raise HTTPException( - status_code=status.HTTP_404_NOT_FOUND, detail="Owner user not found" - ) - if payload.owner_group_id: - group = directory.get_group(payload.owner_group_id) - if group is not None and ( - group.tenant_id != principal.tenant_id or group.status != "active" - ): - group = None - if group is None: - raise HTTPException( - status_code=status.HTTP_404_NOT_FOUND, detail="Owner group not found" - ) - owner_changed = ( - campaign.owner_user_id != payload.owner_user_id - or campaign.owner_group_id != payload.owner_group_id - ) - mail_profile_reselection_required = False - if owner_changed: - mail_profile_reselection_required = ( - _clear_current_version_mail_profile_for_owner_transfer(session, campaign) - ) - - campaign.owner_user_id = payload.owner_user_id - campaign.owner_group_id = payload.owner_group_id - session.add(campaign) - audit_from_principal( + del payload + _get_campaign_for_principal( session, + campaign_id, principal, - action="campaign.owner_updated", - object_type="campaign", - object_id=campaign.id, - details={ - **payload.model_dump(), - "mail_profile_reselection_required": mail_profile_reselection_required, - }, - commit=True, + write=True, + ) + raise HTTPException( + status_code=status.HTTP_409_CONFLICT, + detail=( + "Direct campaign owner mutation has been retired. Use the " + "ownership transfer workflow so the target can accept." + ), ) - return CampaignResponse.model_validate(campaign) @router.post( diff --git a/src/govoplan_campaign/backend/routes/versions.py b/src/govoplan_campaign/backend/routes/versions.py index 6a9c283..b45106e 100644 --- a/src/govoplan_campaign/backend/routes/versions.py +++ b/src/govoplan_campaign/backend/routes/versions.py @@ -439,6 +439,10 @@ def set_version_review_state( version_id=version_id, inspection_complete=payload.inspection_complete, reviewed_message_keys=payload.reviewed_message_keys, + issue_decisions=[ + item.model_dump() + for item in payload.issue_decisions + ], user_id=principal.user.id, commit=False, ) @@ -452,6 +456,7 @@ def set_version_review_state( "campaign_id": campaign_id, "inspection_complete": payload.inspection_complete, "reviewed_message_count": len(payload.reviewed_message_keys), + "issue_decision_count": len(payload.issue_decisions), }, commit=True, ) diff --git a/src/govoplan_campaign/backend/schema/campaign.schema.json b/src/govoplan_campaign/backend/schema/campaign.schema.json index 50a8eb4..0b15296 100644 --- a/src/govoplan_campaign/backend/schema/campaign.schema.json +++ b/src/govoplan_campaign/backend/schema/campaign.schema.json @@ -359,7 +359,7 @@ "continue", "warn" ], - "default": "ask" + "default": "warn" }, "ambiguous_behavior": { "type": "string", @@ -455,7 +455,7 @@ "continue", "warn" ], - "default": "ask" + "default": "block" }, "missing_optional_attachment": { "type": "string", diff --git a/src/govoplan_campaign/backend/schemas.py b/src/govoplan_campaign/backend/schemas.py index ecec616..fffd8a0 100644 --- a/src/govoplan_campaign/backend/schemas.py +++ b/src/govoplan_campaign/backend/schemas.py @@ -75,11 +75,23 @@ class CampaignVersionSetStepRequest(BaseModel): current_step: str +class CampaignReviewDecisionRequest(BaseModel): + model_config = ConfigDict(extra="forbid") + + job_id: str = Field(min_length=1, max_length=36) + decision: Literal["accept"] = "accept" + reason: str | None = Field(default=None, max_length=4_000) + + class CampaignReviewStateRequest(BaseModel): model_config = ConfigDict(extra="forbid") inspection_complete: bool = False reviewed_message_keys: list[str] = Field(default_factory=list) + issue_decisions: list[CampaignReviewDecisionRequest] = Field( + default_factory=list, + max_length=100_000, + ) class CampaignPartialValidationRequest(BaseModel): diff --git a/tests/test_access_provider.py b/tests/test_access_provider.py index 41f08a9..37e9a9d 100644 --- a/tests/test_access_provider.py +++ b/tests/test_access_provider.py @@ -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__, ], ) diff --git a/tests/test_attachment_building.py b/tests/test_attachment_building.py index f6b7fa6..4a20e38 100644 --- a/tests/test_attachment_building.py +++ b/tests/test_attachment_building.py @@ -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) diff --git a/tests/test_campaign_reports.py b/tests/test_campaign_reports.py index 2956f89..dab5433 100644 --- a/tests/test_campaign_reports.py +++ b/tests/test_campaign_reports.py @@ -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: diff --git a/tests/test_review_decisions.py b/tests/test_review_decisions.py new file mode 100644 index 0000000..c158711 --- /dev/null +++ b/tests/test_review_decisions.py @@ -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, + ) diff --git a/webui/src/api/campaigns.ts b/webui/src/api/campaigns.ts index ba305df..97d5b63 100644 --- a/webui/src/api/campaigns.ts +++ b/webui/src/api/campaigns.ts @@ -471,6 +471,11 @@ export type CampaignMockSendPayload = { export type CampaignReviewStatePayload = { inspection_complete: boolean; reviewed_message_keys: string[]; + issue_decisions: Array<{ + job_id: string; + decision: "accept"; + reason?: string | null; + }>; }; export type CampaignSendJobPayload = { diff --git a/webui/src/features/campaigns/GlobalSettingsPage.tsx b/webui/src/features/campaigns/GlobalSettingsPage.tsx index 210d8d5..067cd98 100644 --- a/webui/src/features/campaigns/GlobalSettingsPage.tsx +++ b/webui/src/features/campaigns/GlobalSettingsPage.tsx @@ -251,8 +251,8 @@ export default function GlobalSettingsPage({ settings, auth, campaignId, view = effectiveClassName="campaign-policy-effective-note" label="i18n:govoplan-campaign.default_missing_behavior.ffbd9cd7" note="i18n:govoplan-campaign.used_by_attachment_rules_that_do_not_override_mi.ab8bc0d5" - control={ patch(["attachments", "missing_behavior"], value)} />} - effective={behaviorSummary(getText(attachments, "missing_behavior", "ask"))} /> + control={ patch(["attachments", "missing_behavior"], value)} />} + effective={behaviorSummary(getText(attachments, "missing_behavior", "warn"))} /> >(() => new Set()); + const [reviewIssueDecisions, setReviewIssueDecisions] = useState< + Record + >({}); const [newlyReviewedRequiredKeys, setNewlyReviewedRequiredKeys] = useState>(() => new Set()); const [selectedBuiltIndex, setSelectedBuiltIndex] = useState(null); const [singleSendConfirmIndex, setSingleSendConfirmIndex] = useState(null); @@ -182,7 +185,7 @@ export default function ReviewSendPage({ settings, auth, campaignId }: {settings const imapDiagnosticsRef = useRef(emptyCampaignJobsResponse()); const [selectedDeliveryJobDetail, setSelectedDeliveryJobDetail] = useState | null>(null); const persistedReview = storedMessageReviewState(version); - const persistedReviewKey = `${persistedReview.buildToken}|${persistedReview.inspectionComplete ? "1" : "0"}|${persistedReview.reviewedMessageKeys.join(",")}`; + const persistedReviewKey = `${persistedReview.buildToken}|${persistedReview.inspectionComplete ? "1" : "0"}|${persistedReview.reviewedMessageKeys.join(",")}|${JSON.stringify(persistedReview.issueDecisions)}`; useEffect(() => { setBuiltReviewRows([]); @@ -192,6 +195,7 @@ export default function ReviewSendPage({ settings, auth, campaignId }: {settings setReviewQuery({ sort: null, filters: {} }); setJobsLoadedKey(""); setNewlyReviewedRequiredKeys(new Set()); + setReviewIssueDecisions({}); setSelectedBuiltIndex(null); setMockResult(null); setSelectedMockMessage(null); @@ -215,6 +219,12 @@ export default function ReviewSendPage({ settings, auth, campaignId }: {settings useEffect(() => { setMessageReviewComplete(persistedReview.inspectionComplete); setReviewedMessageKeys(new Set(persistedReview.reviewedMessageKeys)); + setReviewIssueDecisions(Object.fromEntries( + persistedReview.issueDecisions.flatMap((item) => { + const jobId = String(item.job_id ?? "").trim(); + return jobId ? [[jobId, String(item.reason ?? "")]] : []; + }) + )); }, [version?.id, persistedReviewKey]); useEffect(() => { @@ -1010,7 +1020,14 @@ export default function ReviewSendPage({ settings, auth, campaignId }: {settings const reviewed = new Set(reviewedMessageKeys); await updateCampaignReviewState(settings, campaignId, version.id, { inspection_complete: true, - reviewed_message_keys: [...reviewed] + reviewed_message_keys: [...reviewed], + issue_decisions: Object.entries(reviewIssueDecisions).map( + ([jobId, reason]) => ({ + job_id: jobId, + decision: "accept" as const, + reason: reason.trim() || null + }) + ) }); setReviewedMessageKeys(reviewed); setNewlyReviewedRequiredKeys(new Set()); @@ -1063,15 +1080,6 @@ export default function ReviewSendPage({ settings, auth, campaignId }: {settings if (index < 0) return; const reviewRow = filteredBuiltReviewRows[index] ?? row; const jobId = String(reviewRow.id ?? ""); - const reviewKey = String(reviewRow.review_key ?? builtMessageKey(reviewRow, index)); - setReviewedMessageKeys((current) => { - const next = new Set(current); - next.add(reviewKey); - return next; - }); - if (messageNeedsExplicitReview(reviewRow) && reviewRow.reviewed !== true) { - setNewlyReviewedRequiredKeys((current) => new Set(current).add(reviewKey)); - } if (!jobId || reviewRow.resolved_recipients || reviewRow.attachments || reviewRow.issues) { setSelectedBuiltIndex(index); return; @@ -1091,6 +1099,35 @@ export default function ReviewSendPage({ settings, auth, campaignId }: {settings } } + function acceptBuiltMessageReview( + row: Record, + index: number, + reasonRequired: boolean + ) { + const jobId = String(row.id ?? "").trim(); + const reviewKey = String( + row.review_key ?? builtMessageKey(row, index) + ); + const reason = reviewIssueDecisions[jobId] ?? ""; + if (!jobId) { + setError("The built message has no delivery job id."); + return; + } + if (reasonRequired && !reason.trim()) { + setError("Enter a reason for accepting the attachment exception."); + return; + } + setReviewedMessageKeys((current) => new Set(current).add(reviewKey)); + setNewlyReviewedRequiredKeys((current) => + new Set(current).add(reviewKey) + ); + setReviewIssueDecisions((current) => ({ + ...current, + [jobId]: reason + })); + setError(""); + } + async function sendSingleBuiltMessage() { if (!version || busy || singleSendConfirmIndex === null) return; const row = filteredBuiltReviewRows[singleSendConfirmIndex]; @@ -1667,6 +1704,28 @@ export default function ReviewSendPage({ settings, auth, campaignId }: {settings index={selectedBuiltIndex} canStartSingleMessageSend={canStartSingleMessageSend} singleMessageSendBusy={busy === "send"} + reviewed={reviewedMessageKeys.has(String( + selectedBuiltMessage.review_key + ?? builtMessageKey(selectedBuiltMessage, selectedBuiltIndex) + ))} + reviewReason={reviewIssueDecisions[ + String(selectedBuiltMessage.id ?? "") + ] ?? ""} + onReviewReasonChange={(value) => { + const jobId = String(selectedBuiltMessage.id ?? "").trim(); + if (!jobId) return; + setReviewIssueDecisions((current) => ({ + ...current, + [jobId]: value + })); + }} + onAcceptReview={(reasonRequired) => + acceptBuiltMessageReview( + selectedBuiltMessage, + selectedBuiltIndex, + reasonRequired + ) + } onSelect={openBuiltMessageAtIndex} onSendSingle={(targetIndex) => { const target = filteredBuiltReviewRows[targetIndex]; diff --git a/webui/src/features/campaigns/components/CampaignAccessCard.tsx b/webui/src/features/campaigns/components/CampaignAccessCard.tsx index 0db7a55..f5c15ca 100644 --- a/webui/src/features/campaigns/components/CampaignAccessCard.tsx +++ b/webui/src/features/campaigns/components/CampaignAccessCard.tsx @@ -1,20 +1,30 @@ import { useEffect, useMemo, useState } from "react"; import type { ApiSettings, AuthInfo, CampaignListItem } from "../../../types"; -import { KeyRound, Trash2 } from "lucide-react"; +import { + ArrowRightLeft, + Check, + KeyRound, + Trash2, + XCircle +} from "lucide-react"; import { fetchResourceAccessExplanation, getCampaignShares, campaignShareTargetProvider, revokeCampaignShare, - updateCampaignOwner, upsertCampaignShare, type CampaignShare, type ResourceAccessExplanationResponse } from "../../../api/campaigns"; import { Button, + decideOwnershipTransfer, + listOwnershipTransfers, ReferenceSelect, ResourceAccessExplanation, + requestResourceOwnership, + startOwnershipTransfer, + type OwnershipTransfer, type ReferenceOption, type ReferenceOptionProvider } from "@govoplan/core-webui"; @@ -45,20 +55,56 @@ export default function CampaignAccessCard({ }>({ users: [], groups: [] }); const [shares, setShares] = useState([]); const [ownerOpen, setOwnerOpen] = useState(false); + const [requestOpen, setRequestOpen] = useState(false); const [shareOpen, setShareOpen] = useState(false); const [revokeTarget, setRevokeTarget] = useState(null); const [ownerType, setOwnerType] = useState(campaign.owner_group_id ? "group" : "user"); const [ownerId, setOwnerId] = useState(campaign.owner_group_id || campaign.owner_user_id || ""); + const [ownerReason, setOwnerReason] = useState(""); + const [ownerExpiryDays, setOwnerExpiryDays] = useState(7); + const [ownerRequestKey, setOwnerRequestKey] = useState(""); + const [requestTargetType, setRequestTargetType] = useState("user"); + const [requestGroupId, setRequestGroupId] = useState(""); + const [requestReason, setRequestReason] = useState(""); + const [requestKey, setRequestKey] = useState(""); const [shareType, setShareType] = useState("user"); const [shareTargetId, setShareTargetId] = useState(""); const [sharePermission, setSharePermission] = useState<"read" | "write">("read"); const [accessExplanationOpen, setAccessExplanationOpen] = useState(false); const [accessExplanation, setAccessExplanation] = useState(null); const [accessExplanationLoading, setAccessExplanationLoading] = useState(false); + const [ownershipTransfers, setOwnershipTransfers] = useState([]); const [busy, setBusy] = useState(false); const currentOwnerType: TargetType = campaign.owner_group_id ? "group" : "user"; const currentOwnerId = campaign.owner_group_id || campaign.owner_user_id || ""; - const ownerDirty = ownerOpen && (ownerType !== currentOwnerType || ownerId !== currentOwnerId); + const actorId = auth.principal?.membership_id || auth.user.id; + const actorGroupIds = new Set( + auth.principal?.group_ids ?? auth.groups.map((group) => group.id) + ); + const actorIsCurrentOwner = ( + currentOwnerType === "user" + ? currentOwnerId === actorId + : actorGroupIds.has(currentOwnerId) + ); + const canProposeOwnership = actorIsCurrentOwner && hasScope( + auth, + "campaigns:campaign:share" + ); + const canRequestOwnership = !actorIsCurrentOwner && hasScope( + auth, + "campaigns:campaign:read" + ); + const ownerDirty = ownerOpen && ( + ownerType !== currentOwnerType + || ownerId !== currentOwnerId + || Boolean(ownerReason) + || ownerExpiryDays !== 7 + ); + const requestDirty = requestOpen && ( + requestTargetType !== "user" + || Boolean(requestGroupId) + || Boolean(requestReason) + ); const shareDirty = shareOpen && (shareType !== "user" || Boolean(shareTargetId) || sharePermission !== "read"); const canExplainResourceAccess = hasScope(auth, "admin:users:read") || @@ -75,40 +121,62 @@ export default function CampaignAccessCard({ ); useUnsavedDraftGuard({ - dirty: ownerDirty || shareDirty, + dirty: ownerDirty || requestDirty || shareDirty, onSave: saveActiveDialog, onDiscard: () => { closeOwnerDialog(); + closeRequestDialog(); closeShareDialog(); } }); async function load() { + let nextShares: CampaignShare[] = []; + let nextTransfers: OwnershipTransfer[] = []; try { - const nextShares = (await getCampaignShares(settings, campaign.id)) + nextTransfers = await listOwnershipTransfers(settings, { + resourceType: "campaign", + resourceId: campaign.id, + limit: 100 + }); + setOwnershipTransfers(nextTransfers); + } catch (err) { + onError(err instanceof Error ? err.message : String(err)); + } + try { + nextShares = (await getCampaignShares(settings, campaign.id)) .filter((item) => !item.revoked_at); - const userIds = uniqueTargetIds( - nextShares, - "user", - campaign.owner_user_id - ); - const groupIds = uniqueTargetIds( - nextShares, - "group", - campaign.owner_group_id - ); - const controller = new AbortController(); + setShares(nextShares); + setAvailable(true); + } catch (err) { + const message = err instanceof Error ? err.message : String(err); + if (message.startsWith("403 ")) { + setAvailable(false); + } else { + onError(message); + } + } + const userIds = uniqueTargetIds( + nextShares, + nextTransfers, + "user", + campaign.owner_user_id + ); + const groupIds = uniqueTargetIds( + nextShares, + nextTransfers, + "group", + campaign.owner_group_id + ); + const controller = new AbortController(); + try { const [users, groups] = await Promise.all([ resolveReferenceOptions(userTargetProvider, userIds, controller.signal), resolveReferenceOptions(groupTargetProvider, groupIds, controller.signal) ]); setTargets({ users, groups }); - setShares(nextShares); - setAvailable(true); } catch (err) { - const message = err instanceof Error ? err.message : String(err); - if (message.startsWith("403 ")) setAvailable(false);else - onError(message); + onError(err instanceof Error ? err.message : String(err)); } } @@ -142,6 +210,34 @@ export default function CampaignAccessCard({ setOwnerOpen(false); setOwnerType(currentOwnerType); setOwnerId(currentOwnerId); + setOwnerReason(""); + setOwnerExpiryDays(7); + setOwnerRequestKey(""); + } + + function openOwnerDialog() { + setOwnerType(currentOwnerType); + setOwnerId(""); + setOwnerReason(""); + setOwnerExpiryDays(7); + setOwnerRequestKey(crypto.randomUUID()); + setOwnerOpen(true); + } + + function closeRequestDialog() { + setRequestOpen(false); + setRequestTargetType("user"); + setRequestGroupId(""); + setRequestReason(""); + setRequestKey(""); + } + + function openRequestDialog() { + setRequestTargetType("user"); + setRequestGroupId(""); + setRequestReason(""); + setRequestKey(crypto.randomUUID()); + setRequestOpen(true); } function closeShareDialog() { @@ -153,25 +249,78 @@ export default function CampaignAccessCard({ async function saveActiveDialog(): Promise { if (ownerDirty) return saveOwner(); + if (requestDirty) return requestOwnership(); if (shareDirty) return saveShare(); return true; } async function saveOwner(): Promise { - if (!ownerId) return false; + if (!ownerId || ownerId === currentOwnerId && ownerType === currentOwnerType) { + return false; + } setBusy(true); try { - await updateCampaignOwner(settings, campaign.id, ownerType === "user" ? - { owner_user_id: ownerId, owner_group_id: null } : - { owner_user_id: null, owner_group_id: ownerId }); - setOwnerOpen(false); - await onChanged(); + await startOwnershipTransfer(settings, { + resource: { + module_id: "campaigns", + resource_type: "campaign", + resource_id: campaign.id + }, + target_owner: { type: ownerType, id: ownerId }, + idempotency_key: ownerRequestKey || crypto.randomUUID(), + reason: ownerReason.trim() || null, + expiry_days: ownerExpiryDays + }); + closeOwnerDialog(); await load(); return true; } catch (err) {onError(err instanceof Error ? err.message : String(err));return false;} finally {setBusy(false);} } + async function requestOwnership(): Promise { + const targetId = requestTargetType === "user" ? actorId : requestGroupId; + if (!targetId) return false; + setBusy(true); + try { + await requestResourceOwnership(settings, { + resource: { + module_id: "campaigns", + resource_type: "campaign", + resource_id: campaign.id + }, + target_owner: { type: requestTargetType, id: targetId }, + idempotency_key: requestKey || crypto.randomUUID(), + reason: requestReason.trim() || null, + expiry_days: 7 + }); + closeRequestDialog(); + await load(); + return true; + } catch (err) { + onError(err instanceof Error ? err.message : String(err)); + return false; + } finally { + setBusy(false); + } + } + + async function performOwnershipAction( + transfer: OwnershipTransfer, + action: "owner-approval" | "acceptance" | "decline" | "cancel" + ) { + setBusy(true); + try { + await decideOwnershipTransfer(settings, transfer.id, action); + await onChanged(); + await load(); + } catch (err) { + onError(err instanceof Error ? err.message : String(err)); + } finally { + setBusy(false); + } + } + async function saveShare(): Promise { if (!shareTargetId) return false; setBusy(true); @@ -237,17 +386,74 @@ export default function CampaignAccessCard({ { id: "actions", header: "i18n:govoplan-campaign.actions.c3cd636a", width: 72, resizable: false, sticky: "end", align: "right", render: (row) =>