Compare commits
3 Commits
5dc9392290
...
75c9ece709
| Author | SHA1 | Date | |
|---|---|---|---|
| 75c9ece709 | |||
| d005040a8e | |||
| 613fb15a80 |
@@ -43,6 +43,13 @@ inspection UI are implemented. The conflict review UI compares stored local and
|
|||||||
remote field payloads, can apply a stored remote vCard payload, and supports
|
remote field payloads, can apply a stored remote vCard payload, and supports
|
||||||
manual per-field local/remote merge choices.
|
manual per-field local/remote merge choices.
|
||||||
|
|
||||||
|
API-managed CardDAV credentials are encrypted inside the source record. Source
|
||||||
|
deletion physically removes that credential material and records a non-secret
|
||||||
|
audit event in the same database transaction; destructive module retirement
|
||||||
|
audits every remaining credential before the owning tables are dropped. Legacy
|
||||||
|
external references are detached but are never sent to a secret provider for
|
||||||
|
deletion because Addresses cannot prove that it owns them.
|
||||||
|
|
||||||
## Boundary
|
## Boundary
|
||||||
|
|
||||||
`govoplan-addresses` owns:
|
`govoplan-addresses` owns:
|
||||||
|
|||||||
@@ -148,7 +148,12 @@ a conflict instead of silently overwriting remote data. The first conflict
|
|||||||
review UI compares stored local and remote field payloads and can apply a
|
review UI compares stored local and remote field payloads and can apply a
|
||||||
stored remote vCard payload or a manual per-field local/remote merge payload.
|
stored remote vCard payload or a manual per-field local/remote merge payload.
|
||||||
Source disconnect/delete removes the source binding and related sync records
|
Source disconnect/delete removes the source binding and related sync records
|
||||||
while keeping local contacts.
|
while keeping local contacts. Because API-managed CardDAV credentials are
|
||||||
|
encrypted in the source row, the same transaction physically removes their
|
||||||
|
ciphertext and emits non-secret credential-deletion audit evidence. Destructive
|
||||||
|
module retirement audits all remaining owned credential material before table
|
||||||
|
removal. An unowned legacy reference is detached rather than passed to an
|
||||||
|
external secret provider.
|
||||||
|
|
||||||
## Connector Direction
|
## Connector Direction
|
||||||
|
|
||||||
|
|||||||
@@ -1,7 +1,10 @@
|
|||||||
from __future__ import annotations
|
from __future__ import annotations
|
||||||
|
|
||||||
|
from dataclasses import replace
|
||||||
from pathlib import Path
|
from pathlib import Path
|
||||||
|
|
||||||
|
from sqlalchemy import inspect
|
||||||
|
|
||||||
from govoplan_addresses.backend.capabilities import (
|
from govoplan_addresses.backend.capabilities import (
|
||||||
CAPABILITY_ADDRESSES_CONTACT_WRITER,
|
CAPABILITY_ADDRESSES_CONTACT_WRITER,
|
||||||
CAPABILITY_ADDRESSES_LOOKUP,
|
CAPABILITY_ADDRESSES_LOOKUP,
|
||||||
@@ -25,6 +28,47 @@ from govoplan_core.core.modules import (
|
|||||||
from govoplan_core.db.base import Base
|
from govoplan_core.db.base import Base
|
||||||
|
|
||||||
|
|
||||||
|
_addresses_table_retirement_provider = drop_table_retirement_provider(
|
||||||
|
addresses_models.AddressSyncDiagnostic,
|
||||||
|
addresses_models.AddressSyncConflict,
|
||||||
|
addresses_models.AddressSyncTombstone,
|
||||||
|
addresses_models.AddressSyncSource,
|
||||||
|
addresses_models.AddressListEntry,
|
||||||
|
addresses_models.AddressList,
|
||||||
|
addresses_models.ContactPostalAddress,
|
||||||
|
addresses_models.ContactPhone,
|
||||||
|
addresses_models.ContactEmail,
|
||||||
|
addresses_models.Contact,
|
||||||
|
addresses_models.AddressBook,
|
||||||
|
label="Addresses",
|
||||||
|
)
|
||||||
|
|
||||||
|
|
||||||
|
def _addresses_retirement_provider(session: object | None, module_id: str):
|
||||||
|
plan = _addresses_table_retirement_provider(session, module_id)
|
||||||
|
base_executor = plan.destroy_data_executor
|
||||||
|
if base_executor is None:
|
||||||
|
return plan
|
||||||
|
|
||||||
|
def executor(execute_session: object, execute_module_id: str) -> None:
|
||||||
|
if not hasattr(execute_session, "get_bind") or not hasattr(execute_session, "query"):
|
||||||
|
raise RuntimeError("No database session is available for Addresses credential retirement.")
|
||||||
|
if inspect(execute_session.get_bind()).has_table(addresses_models.AddressSyncSource.__tablename__):
|
||||||
|
from govoplan_addresses.backend.service import audit_address_credentials_for_retirement
|
||||||
|
|
||||||
|
audit_address_credentials_for_retirement(execute_session)
|
||||||
|
base_executor(execute_session, execute_module_id)
|
||||||
|
|
||||||
|
return replace(
|
||||||
|
plan,
|
||||||
|
destroy_data_warnings=(
|
||||||
|
*plan.destroy_data_warnings,
|
||||||
|
"Addresses-owned encrypted connector credentials are audited and deleted with the sync-source table.",
|
||||||
|
),
|
||||||
|
destroy_data_executor=executor,
|
||||||
|
)
|
||||||
|
|
||||||
|
|
||||||
def _permission(scope: str, label: str, description: str) -> PermissionDefinition:
|
def _permission(scope: str, label: str, description: str) -> PermissionDefinition:
|
||||||
module_id, resource, action = scope.split(":", 2)
|
module_id, resource, action = scope.split(":", 2)
|
||||||
return PermissionDefinition(
|
return PermissionDefinition(
|
||||||
@@ -128,20 +172,7 @@ manifest = ModuleManifest(
|
|||||||
metadata=Base.metadata,
|
metadata=Base.metadata,
|
||||||
script_location=str(Path(__file__).with_name("migrations") / "versions"),
|
script_location=str(Path(__file__).with_name("migrations") / "versions"),
|
||||||
retirement_supported=True,
|
retirement_supported=True,
|
||||||
retirement_provider=drop_table_retirement_provider(
|
retirement_provider=_addresses_retirement_provider,
|
||||||
addresses_models.AddressSyncDiagnostic,
|
|
||||||
addresses_models.AddressSyncConflict,
|
|
||||||
addresses_models.AddressSyncTombstone,
|
|
||||||
addresses_models.AddressSyncSource,
|
|
||||||
addresses_models.AddressListEntry,
|
|
||||||
addresses_models.AddressList,
|
|
||||||
addresses_models.ContactPostalAddress,
|
|
||||||
addresses_models.ContactPhone,
|
|
||||||
addresses_models.ContactEmail,
|
|
||||||
addresses_models.Contact,
|
|
||||||
addresses_models.AddressBook,
|
|
||||||
label="Addresses",
|
|
||||||
),
|
|
||||||
retirement_notes="Destructive retirement drops address-owned database tables after the installer captures a database snapshot.",
|
retirement_notes="Destructive retirement drops address-owned database tables after the installer captures a database snapshot.",
|
||||||
),
|
),
|
||||||
capability_factories={
|
capability_factories={
|
||||||
|
|||||||
@@ -12,6 +12,7 @@ from sqlalchemy import and_, false, func, or_
|
|||||||
from sqlalchemy.orm import Session
|
from sqlalchemy.orm import Session
|
||||||
|
|
||||||
from govoplan_core.auth import ApiPrincipal
|
from govoplan_core.auth import ApiPrincipal
|
||||||
|
from govoplan_core.audit.logging import audit_event
|
||||||
from govoplan_core.core.change_sequence import record_change
|
from govoplan_core.core.change_sequence import record_change
|
||||||
from govoplan_core.db.base import utcnow
|
from govoplan_core.db.base import utcnow
|
||||||
from govoplan_core.security.secrets import decrypt_secret, encrypt_secret
|
from govoplan_core.security.secrets import decrypt_secret, encrypt_secret
|
||||||
@@ -352,9 +353,89 @@ def delete_sync_source(session: Session, principal: ApiPrincipal, sync_source_id
|
|||||||
book.sync_status = None
|
book.sync_status = None
|
||||||
book.sync_error = None
|
book.sync_error = None
|
||||||
book.updated_by_account_id = _account_id(principal)
|
book.updated_by_account_id = _account_id(principal)
|
||||||
|
_audit_sync_credential_deletion(session, principal, sync_source)
|
||||||
session.delete(sync_source)
|
session.delete(sync_source)
|
||||||
|
|
||||||
|
|
||||||
|
def _audit_sync_credential_deletion(
|
||||||
|
session: Session,
|
||||||
|
principal: ApiPrincipal,
|
||||||
|
sync_source: AddressSyncSource,
|
||||||
|
) -> None:
|
||||||
|
"""Audit removal of source-owned credential material in the DB transaction.
|
||||||
|
|
||||||
|
CardDAV credentials are encrypted inside the sync-source row. Deleting that
|
||||||
|
row therefore deletes the credential atomically with the connector. Legacy
|
||||||
|
credential references are detached from the source, but are never resolved
|
||||||
|
or sent to an external provider because ownership cannot be proven.
|
||||||
|
"""
|
||||||
|
|
||||||
|
tenant_id = _tenant_id_or_none(principal)
|
||||||
|
user = getattr(principal, "user", None)
|
||||||
|
_record_sync_credential_deletion_audit(
|
||||||
|
session,
|
||||||
|
sync_source=sync_source,
|
||||||
|
tenant_id=tenant_id,
|
||||||
|
user_id=getattr(user, "id", None),
|
||||||
|
api_key_id=getattr(principal, "api_key_id", None),
|
||||||
|
deletion_reason="sync_source_deleted",
|
||||||
|
)
|
||||||
|
|
||||||
|
|
||||||
|
def _record_sync_credential_deletion_audit(
|
||||||
|
session: Session,
|
||||||
|
*,
|
||||||
|
sync_source: AddressSyncSource,
|
||||||
|
tenant_id: str | None,
|
||||||
|
user_id: str | None,
|
||||||
|
api_key_id: str | None,
|
||||||
|
deletion_reason: str,
|
||||||
|
) -> bool:
|
||||||
|
auth = _carddav_auth_metadata(sync_source.metadata_)
|
||||||
|
if auth.get("secret_encrypted"):
|
||||||
|
storage_backend = "encrypted_database"
|
||||||
|
elif auth.get("credential_ref"):
|
||||||
|
storage_backend = "legacy_reference"
|
||||||
|
else:
|
||||||
|
return False
|
||||||
|
audit_event(
|
||||||
|
session,
|
||||||
|
tenant_id=tenant_id,
|
||||||
|
user_id=user_id,
|
||||||
|
api_key_id=api_key_id,
|
||||||
|
action="addresses.sync_credential_deleted",
|
||||||
|
scope="tenant" if tenant_id is not None else "system",
|
||||||
|
object_type="address_sync_credential",
|
||||||
|
object_id=sync_source.id,
|
||||||
|
details={
|
||||||
|
"sync_source_id": sync_source.id,
|
||||||
|
"connector_type": sync_source.connector_type,
|
||||||
|
"storage_backend": storage_backend,
|
||||||
|
"deletion_reason": deletion_reason,
|
||||||
|
},
|
||||||
|
)
|
||||||
|
return True
|
||||||
|
|
||||||
|
|
||||||
|
def audit_address_credentials_for_retirement(session: Session) -> int:
|
||||||
|
"""Audit credentials that the ensuing destructive table drop deletes."""
|
||||||
|
|
||||||
|
sources = session.query(AddressSyncSource).order_by(AddressSyncSource.id.asc()).all()
|
||||||
|
audited = 0
|
||||||
|
for source in sources:
|
||||||
|
if _record_sync_credential_deletion_audit(
|
||||||
|
session,
|
||||||
|
sync_source=source,
|
||||||
|
tenant_id=source.tenant_id,
|
||||||
|
user_id=None,
|
||||||
|
api_key_id=None,
|
||||||
|
deletion_reason="module_data_retired",
|
||||||
|
):
|
||||||
|
audited += 1
|
||||||
|
session.flush()
|
||||||
|
return audited
|
||||||
|
|
||||||
|
|
||||||
def start_sync_attempt(session: Session, principal: ApiPrincipal, sync_source_id: str) -> AddressSyncSource:
|
def start_sync_attempt(session: Session, principal: ApiPrincipal, sync_source_id: str) -> AddressSyncSource:
|
||||||
sync_source = get_visible_sync_source(session, principal, sync_source_id)
|
sync_source = get_visible_sync_source(session, principal, sync_source_id)
|
||||||
if not sync_source.enabled:
|
if not sync_source.enabled:
|
||||||
|
|||||||
@@ -3,7 +3,7 @@ from __future__ import annotations
|
|||||||
import unittest
|
import unittest
|
||||||
from unittest.mock import patch
|
from unittest.mock import patch
|
||||||
|
|
||||||
from sqlalchemy import create_engine
|
from sqlalchemy import create_engine, inspect
|
||||||
from sqlalchemy.orm import sessionmaker
|
from sqlalchemy.orm import sessionmaker
|
||||||
|
|
||||||
from govoplan_core.core.change_sequence import ChangeSequenceEntry
|
from govoplan_core.core.change_sequence import ChangeSequenceEntry
|
||||||
@@ -1051,6 +1051,107 @@ END:VCARD
|
|||||||
self.assertFalse(book.read_only)
|
self.assertFalse(book.read_only)
|
||||||
self.assertEqual([item.id for item in list_contacts(self.session, self.principal, address_book_id=book.id)], [contact.id])
|
self.assertEqual([item.id for item in list_contacts(self.session, self.principal, address_book_id=book.id)], [contact.id])
|
||||||
|
|
||||||
|
def test_delete_sync_source_deletes_and_audits_encrypted_credential_atomically(self) -> None:
|
||||||
|
book = create_address_book(
|
||||||
|
self.session,
|
||||||
|
self.principal,
|
||||||
|
AddressBookCreateRequest(scope_type="user", name="Audited remote"),
|
||||||
|
)
|
||||||
|
self.session.commit()
|
||||||
|
self.session.refresh(book)
|
||||||
|
source = create_carddav_sync_source(
|
||||||
|
self.session,
|
||||||
|
self.principal,
|
||||||
|
book.id,
|
||||||
|
AddressCardDavSourceCreateRequest(
|
||||||
|
collection_url="https://carddav.example.local/addressbooks/audited/",
|
||||||
|
auth_type="basic",
|
||||||
|
username="ada",
|
||||||
|
password="do-not-audit-this",
|
||||||
|
),
|
||||||
|
)
|
||||||
|
self.session.commit()
|
||||||
|
source_id = source.id
|
||||||
|
|
||||||
|
with patch("govoplan_addresses.backend.service.audit_event") as audit:
|
||||||
|
delete_sync_source(self.session, self.principal, source_id)
|
||||||
|
self.session.commit()
|
||||||
|
|
||||||
|
self.assertIsNone(self.session.get(AddressSyncSource, source_id))
|
||||||
|
audit.assert_called_once()
|
||||||
|
audit_call = audit.call_args.kwargs
|
||||||
|
self.assertEqual(audit_call["action"], "addresses.sync_credential_deleted")
|
||||||
|
self.assertEqual(audit_call["object_id"], source_id)
|
||||||
|
self.assertEqual(audit_call["details"]["storage_backend"], "encrypted_database")
|
||||||
|
self.assertNotIn("credential_ref", audit_call["details"])
|
||||||
|
self.assertNotIn("secret_encrypted", audit_call["details"])
|
||||||
|
self.assertNotIn("do-not-audit-this", repr(audit_call))
|
||||||
|
|
||||||
|
def test_delete_sync_source_is_not_staged_when_credential_audit_fails(self) -> None:
|
||||||
|
book = create_address_book(
|
||||||
|
self.session,
|
||||||
|
self.principal,
|
||||||
|
AddressBookCreateRequest(scope_type="user", name="Audit failure"),
|
||||||
|
)
|
||||||
|
self.session.commit()
|
||||||
|
self.session.refresh(book)
|
||||||
|
source = create_carddav_sync_source(
|
||||||
|
self.session,
|
||||||
|
self.principal,
|
||||||
|
book.id,
|
||||||
|
AddressCardDavSourceCreateRequest(
|
||||||
|
collection_url="https://carddav.example.local/addressbooks/audit-failure/",
|
||||||
|
auth_type="bearer",
|
||||||
|
bearer_token="do-not-audit-this",
|
||||||
|
),
|
||||||
|
)
|
||||||
|
self.session.commit()
|
||||||
|
|
||||||
|
with patch(
|
||||||
|
"govoplan_addresses.backend.service.audit_event",
|
||||||
|
side_effect=RuntimeError("audit unavailable"),
|
||||||
|
), self.assertRaisesRegex(RuntimeError, "audit unavailable"):
|
||||||
|
delete_sync_source(self.session, self.principal, source.id)
|
||||||
|
|
||||||
|
self.session.rollback()
|
||||||
|
persisted = self.session.get(AddressSyncSource, source.id)
|
||||||
|
self.assertIsNotNone(persisted)
|
||||||
|
self.assertIn("secret_encrypted", (persisted.metadata_ or {})["carddav"])
|
||||||
|
|
||||||
|
def test_destructive_module_retirement_audits_credentials_before_tables_are_dropped(self) -> None:
|
||||||
|
book = create_address_book(
|
||||||
|
self.session,
|
||||||
|
self.principal,
|
||||||
|
AddressBookCreateRequest(scope_type="user", name="Retired module"),
|
||||||
|
)
|
||||||
|
self.session.commit()
|
||||||
|
self.session.refresh(book)
|
||||||
|
source = create_carddav_sync_source(
|
||||||
|
self.session,
|
||||||
|
self.principal,
|
||||||
|
book.id,
|
||||||
|
AddressCardDavSourceCreateRequest(
|
||||||
|
collection_url="https://carddav.example.local/addressbooks/module-retirement/",
|
||||||
|
auth_type="bearer",
|
||||||
|
bearer_token="do-not-audit-this",
|
||||||
|
),
|
||||||
|
)
|
||||||
|
self.session.commit()
|
||||||
|
source_id = source.id
|
||||||
|
retirement_provider = manifest.migration_spec.retirement_provider
|
||||||
|
assert retirement_provider is not None
|
||||||
|
plan = retirement_provider(self.session, "addresses")
|
||||||
|
assert plan.destroy_data_executor is not None
|
||||||
|
|
||||||
|
with patch("govoplan_addresses.backend.service.audit_event") as audit:
|
||||||
|
plan.destroy_data_executor(self.session, "addresses")
|
||||||
|
|
||||||
|
audit.assert_called_once()
|
||||||
|
audit_call = audit.call_args.kwargs
|
||||||
|
self.assertEqual(audit_call["object_id"], source_id)
|
||||||
|
self.assertEqual(audit_call["details"]["deletion_reason"], "module_data_retired")
|
||||||
|
self.assertNotIn("do-not-audit-this", repr(audit_call))
|
||||||
|
self.assertFalse(inspect(self.engine).has_table("addresses_sync_sources"))
|
||||||
|
|
||||||
if __name__ == "__main__":
|
if __name__ == "__main__":
|
||||||
unittest.main()
|
unittest.main()
|
||||||
|
|||||||
Reference in New Issue
Block a user