feat: harden file sharing and integrity
This commit is contained in:
@@ -1,6 +1,7 @@
|
||||
from __future__ import annotations
|
||||
|
||||
import unittest
|
||||
from datetime import datetime, timedelta, timezone
|
||||
|
||||
from sqlalchemy import create_engine, text
|
||||
from sqlalchemy.orm import sessionmaker
|
||||
@@ -102,6 +103,15 @@ class FilesAccessProviderTests(unittest.TestCase):
|
||||
target_id="campaign-1",
|
||||
permission="read",
|
||||
),
|
||||
FileShare(
|
||||
id="share-campaign-expired",
|
||||
tenant_id=TENANT_ID,
|
||||
file_asset_id=assets[2].id,
|
||||
target_type="campaign",
|
||||
target_id="campaign-1",
|
||||
permission="read",
|
||||
expires_at=datetime.now(timezone.utc) - timedelta(seconds=1),
|
||||
),
|
||||
])
|
||||
session.commit()
|
||||
session.execute(
|
||||
|
||||
196
tests/test_integrity_reconciliation.py
Normal file
196
tests/test_integrity_reconciliation.py
Normal file
@@ -0,0 +1,196 @@
|
||||
from __future__ import annotations
|
||||
|
||||
import hashlib
|
||||
import tempfile
|
||||
import unittest
|
||||
from pathlib import Path
|
||||
|
||||
from sqlalchemy import create_engine
|
||||
from sqlalchemy.orm import sessionmaker
|
||||
|
||||
from govoplan_access.backend.db.models import Account, Group, User
|
||||
from govoplan_core.db.base import Base
|
||||
from govoplan_files.backend.db.models import (
|
||||
FileBlob,
|
||||
FileIntegrityFinding,
|
||||
FileIntegrityScan,
|
||||
)
|
||||
from govoplan_files.backend.storage.backends import LocalFilesystemStorageBackend
|
||||
from govoplan_files.backend.storage.common import FileStorageError
|
||||
from govoplan_files.backend.storage.integrity import (
|
||||
cleanup_orphan_finding,
|
||||
create_integrity_scan,
|
||||
read_verified_blob_bytes,
|
||||
recheck_integrity_finding,
|
||||
run_integrity_scan_batch,
|
||||
)
|
||||
|
||||
|
||||
TENANT_ID = "tenant-1"
|
||||
USER_ID = "user-1"
|
||||
|
||||
|
||||
class IntegrityReconciliationTests(unittest.TestCase):
|
||||
def setUp(self) -> None:
|
||||
self.temporary_directory = tempfile.TemporaryDirectory()
|
||||
self.addCleanup(self.temporary_directory.cleanup)
|
||||
self.backend = LocalFilesystemStorageBackend(
|
||||
Path(self.temporary_directory.name)
|
||||
)
|
||||
self.engine = create_engine("sqlite:///:memory:", future=True)
|
||||
Base.metadata.create_all(
|
||||
bind=self.engine,
|
||||
tables=[
|
||||
Account.__table__,
|
||||
User.__table__,
|
||||
Group.__table__,
|
||||
FileBlob.__table__,
|
||||
FileIntegrityScan.__table__,
|
||||
FileIntegrityFinding.__table__,
|
||||
],
|
||||
)
|
||||
self.session = sessionmaker(bind=self.engine, future=True)()
|
||||
self.addCleanup(self._close)
|
||||
|
||||
def _close(self) -> None:
|
||||
self.session.close()
|
||||
self.engine.dispose()
|
||||
|
||||
def test_scan_is_bounded_resumable_and_reconciles_both_orphan_directions(
|
||||
self,
|
||||
) -> None:
|
||||
valid_data = b"valid"
|
||||
restored_data = b"restore-me"
|
||||
expected_corrupt_data = b"expected"
|
||||
stored_corrupt_data = b"corrupt!"
|
||||
valid = _blob("blob-1", "valid.bin", valid_data)
|
||||
missing = _blob("blob-2", "missing.bin", restored_data)
|
||||
corrupt = _blob("blob-3", "corrupt.bin", expected_corrupt_data)
|
||||
self.session.add_all([valid, missing, corrupt])
|
||||
self.session.commit()
|
||||
self.backend.put_bytes(valid.storage_key, valid_data)
|
||||
self.backend.put_bytes(corrupt.storage_key, stored_corrupt_data)
|
||||
orphan_key = f"tenants/{TENANT_ID}/files/orphan.bin"
|
||||
self.backend.put_bytes(orphan_key, b"orphan")
|
||||
|
||||
scan = create_integrity_scan(
|
||||
self.session,
|
||||
tenant_id=TENANT_ID,
|
||||
user_id=USER_ID,
|
||||
batch_size=1,
|
||||
backend=self.backend,
|
||||
)
|
||||
self.session.commit()
|
||||
|
||||
invocations = 0
|
||||
while scan.status != "completed":
|
||||
run_integrity_scan_batch(
|
||||
self.session,
|
||||
scan,
|
||||
backend=self.backend,
|
||||
)
|
||||
self.session.commit()
|
||||
invocations += 1
|
||||
scan = self.session.get(FileIntegrityScan, scan.id)
|
||||
self.assertIsNotNone(scan)
|
||||
self.session.expire_all()
|
||||
if invocations > 12:
|
||||
self.fail("Integrity scan did not complete")
|
||||
|
||||
self.assertGreater(invocations, 3)
|
||||
self.assertEqual(3, scan.scanned_blob_count)
|
||||
self.assertEqual(1, scan.verified_blob_count)
|
||||
self.assertEqual(2, scan.quarantined_blob_count)
|
||||
self.assertEqual(3, scan.scanned_object_count)
|
||||
self.assertEqual(1, scan.orphan_object_count)
|
||||
findings = (
|
||||
self.session.query(FileIntegrityFinding)
|
||||
.filter(FileIntegrityFinding.scan_id == scan.id)
|
||||
.all()
|
||||
)
|
||||
self.assertEqual(
|
||||
{"missing", "checksum_mismatch", "orphan_object"},
|
||||
{finding.kind for finding in findings},
|
||||
)
|
||||
|
||||
missing = self.session.get(FileBlob, missing.id)
|
||||
corrupt = self.session.get(FileBlob, corrupt.id)
|
||||
self.assertEqual("missing", missing.integrity_status)
|
||||
self.assertEqual("checksum_mismatch", corrupt.integrity_status)
|
||||
with self.assertRaisesRegex(FileStorageError, "quarantined"):
|
||||
read_verified_blob_bytes(missing, backend=self.backend)
|
||||
|
||||
missing_finding = next(
|
||||
finding for finding in findings if finding.kind == "missing"
|
||||
)
|
||||
self.backend.put_bytes(missing.storage_key, restored_data)
|
||||
preview = recheck_integrity_finding(
|
||||
self.session,
|
||||
missing_finding,
|
||||
user_id=USER_ID,
|
||||
dry_run=True,
|
||||
backend=self.backend,
|
||||
)
|
||||
self.assertTrue(preview.inspection.valid)
|
||||
self.assertEqual("open", missing_finding.state)
|
||||
repaired = recheck_integrity_finding(
|
||||
self.session,
|
||||
missing_finding,
|
||||
user_id=USER_ID,
|
||||
dry_run=False,
|
||||
backend=self.backend,
|
||||
)
|
||||
self.session.commit()
|
||||
self.assertTrue(repaired.changed)
|
||||
self.assertEqual("resolved", missing_finding.state)
|
||||
self.assertEqual(
|
||||
restored_data,
|
||||
read_verified_blob_bytes(missing, backend=self.backend),
|
||||
)
|
||||
|
||||
orphan_finding = next(
|
||||
finding for finding in findings if finding.kind == "orphan_object"
|
||||
)
|
||||
preview_cleanup = cleanup_orphan_finding(
|
||||
self.session,
|
||||
orphan_finding,
|
||||
user_id=USER_ID,
|
||||
dry_run=True,
|
||||
backend=self.backend,
|
||||
)
|
||||
self.assertEqual("would_delete", preview_cleanup.action)
|
||||
self.assertTrue(self.backend.exists(orphan_key))
|
||||
cleanup = cleanup_orphan_finding(
|
||||
self.session,
|
||||
orphan_finding,
|
||||
user_id=USER_ID,
|
||||
dry_run=False,
|
||||
backend=self.backend,
|
||||
)
|
||||
repeated = cleanup_orphan_finding(
|
||||
self.session,
|
||||
orphan_finding,
|
||||
user_id=USER_ID,
|
||||
dry_run=False,
|
||||
backend=self.backend,
|
||||
)
|
||||
self.assertEqual("deleted", cleanup.action)
|
||||
self.assertFalse(self.backend.exists(orphan_key))
|
||||
self.assertEqual("already_deleted", repeated.action)
|
||||
self.assertFalse(repeated.changed)
|
||||
|
||||
|
||||
def _blob(blob_id: str, filename: str, expected_data: bytes) -> FileBlob:
|
||||
return FileBlob(
|
||||
id=blob_id,
|
||||
tenant_id=TENANT_ID,
|
||||
storage_backend="local",
|
||||
storage_key=f"tenants/{TENANT_ID}/files/{filename}",
|
||||
checksum_sha256=hashlib.sha256(expected_data).hexdigest(),
|
||||
size_bytes=len(expected_data),
|
||||
ref_count=1,
|
||||
)
|
||||
|
||||
|
||||
if __name__ == "__main__":
|
||||
unittest.main()
|
||||
@@ -103,8 +103,8 @@ class FilesManifestDocumentationTests(unittest.TestCase):
|
||||
def test_share_and_delete_tasks_state_current_boundaries(self) -> None:
|
||||
share = self.topic("files.workflow.share-managed-files")
|
||||
self.assertEqual("available", share.layer)
|
||||
self.assertIn("does not yet provide a general share editor", share.body)
|
||||
self.assertIn("no share-revocation route", share.body)
|
||||
self.assertIn("Expired and revoked grants stop authorizing", share.body)
|
||||
self.assertIn("revocation is idempotent", share.body)
|
||||
self.assertIn(
|
||||
"/api/v1/files/{file_id}/shares", {link.href for link in share.links}
|
||||
)
|
||||
@@ -154,8 +154,14 @@ class FilesManifestDocumentationTests(unittest.TestCase):
|
||||
self.assertIn("fail closed", topic.body)
|
||||
self.assertIn("DFS referrals", topic.body)
|
||||
self.assertIn("does not remove backend blob objects", topic.body)
|
||||
self.assertIn("bounded resumable integrity scan", topic.body)
|
||||
self.assertIn("quarantined", topic.body)
|
||||
self.assertIn("MASTER_KEY_B64", topic.metadata["recovery_unit"])
|
||||
self.assertTrue(topic.metadata["verification"])
|
||||
self.assertIn(
|
||||
"/api/v1/files/integrity/scans",
|
||||
{link.href for link in topic.links},
|
||||
)
|
||||
self.assertIn(
|
||||
"GOVOPLAN_CONNECTOR_ALLOW_PRIVATE_NETWORKS", topic.configuration_keys
|
||||
)
|
||||
@@ -184,7 +190,7 @@ class FilesManifestDocumentationTests(unittest.TestCase):
|
||||
self.assertIn("process_owner", topic.audience)
|
||||
self.assertIn("release_manager", topic.audience)
|
||||
self.assertIn("versions align", topic.body)
|
||||
self.assertIn("no general share-management UI or share revocation", topic.body)
|
||||
self.assertIn("Share grant, change, expiry, and revocation", topic.body)
|
||||
self.assertIn("no enforced retention or legal hold", topic.body)
|
||||
for key in ("prerequisites", "steps", "outcome", "verification"):
|
||||
self.assertTrue(topic.metadata[key])
|
||||
|
||||
@@ -10,6 +10,7 @@ from govoplan_files.backend.routes.connector_io import router as connector_io_ro
|
||||
from govoplan_files.backend.routes.connector_profiles import router as connector_profiles_router
|
||||
from govoplan_files.backend.routes.connector_settings import router as connector_settings_router
|
||||
from govoplan_files.backend.routes.folders import router as folders_router
|
||||
from govoplan_files.backend.routes.integrity import router as integrity_router
|
||||
from govoplan_files.backend.routes.listing import router as listing_router
|
||||
from govoplan_files.backend.routes.shares import router as shares_router
|
||||
from govoplan_files.backend.routes.spaces import router as spaces_router
|
||||
@@ -30,6 +31,7 @@ class FilesRouterContractTests(unittest.TestCase):
|
||||
workflow_routers = (
|
||||
spaces_router,
|
||||
folders_router,
|
||||
integrity_router,
|
||||
listing_router,
|
||||
uploads_router,
|
||||
connector_settings_router,
|
||||
@@ -47,7 +49,7 @@ class FilesRouterContractTests(unittest.TestCase):
|
||||
actual = self._operation_keys(router)
|
||||
|
||||
self.assertEqual(expected, actual)
|
||||
self.assertEqual(41, len(actual))
|
||||
self.assertEqual(50, len(actual))
|
||||
self.assertFalse(
|
||||
[operation for operation, count in Counter(actual).items() if count > 1]
|
||||
)
|
||||
@@ -77,6 +79,14 @@ class FilesRouterContractTests(unittest.TestCase):
|
||||
self.assertIn((("POST",), "/files/bulk-rename"), routes)
|
||||
self.assertIn((("POST",), "/files/transfer"), routes)
|
||||
|
||||
def test_share_lifecycle_routes_are_exposed(self) -> None:
|
||||
routes = {(tuple(sorted(route.methods or ())), route.path) for route in router.routes}
|
||||
|
||||
self.assertIn((("GET",), "/files/{file_id}/shares"), routes)
|
||||
self.assertIn((("POST",), "/files/{file_id}/shares"), routes)
|
||||
self.assertIn((("DELETE",), "/files/{file_id}/shares/{share_id}"), routes)
|
||||
self.assertIn((("GET",), "/files/{file_id}/share-target-options"), routes)
|
||||
|
||||
def test_file_listing_exposes_structured_property_filters(self) -> None:
|
||||
route = next(
|
||||
route
|
||||
|
||||
219
tests/test_share_lifecycle.py
Normal file
219
tests/test_share_lifecycle.py
Normal file
@@ -0,0 +1,219 @@
|
||||
from __future__ import annotations
|
||||
|
||||
import unittest
|
||||
from datetime import timedelta
|
||||
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_core.core.change_sequence import ChangeSequenceEntry
|
||||
from govoplan_core.db.base import Base
|
||||
from govoplan_files.backend.db.models import FileAsset, FileShare
|
||||
from govoplan_files.backend.storage.common import FileStorageError, utcnow
|
||||
from govoplan_files.backend.storage.files import (
|
||||
get_asset_for_user,
|
||||
list_file_shares,
|
||||
revoke_file_share,
|
||||
share_file,
|
||||
)
|
||||
|
||||
|
||||
TENANT_ID = "tenant-1"
|
||||
OWNER_ID = "owner-1"
|
||||
RECIPIENT_ID = "recipient-1"
|
||||
GROUP_ID = "group-1"
|
||||
|
||||
|
||||
class FileShareLifecycleTests(unittest.TestCase):
|
||||
def setUp(self) -> None:
|
||||
self.engine = create_engine("sqlite:///:memory:", future=True)
|
||||
Base.metadata.create_all(
|
||||
bind=self.engine,
|
||||
tables=[
|
||||
Account.__table__,
|
||||
User.__table__,
|
||||
Group.__table__,
|
||||
ChangeSequenceEntry.__table__,
|
||||
FileAsset.__table__,
|
||||
FileShare.__table__,
|
||||
],
|
||||
)
|
||||
self.session = sessionmaker(bind=self.engine, future=True)()
|
||||
self.asset = FileAsset(
|
||||
id="file-1",
|
||||
tenant_id=TENANT_ID,
|
||||
owner_type="user",
|
||||
owner_user_id=OWNER_ID,
|
||||
display_path="shared.pdf",
|
||||
filename="shared.pdf",
|
||||
)
|
||||
self.session.add(self.asset)
|
||||
self.session.commit()
|
||||
|
||||
def tearDown(self) -> None:
|
||||
self.session.close()
|
||||
self.engine.dispose()
|
||||
|
||||
@patch(
|
||||
"govoplan_files.backend.storage.files.user_group_ids",
|
||||
return_value=[],
|
||||
)
|
||||
def test_expired_share_stops_access_immediately(self, _groups) -> None:
|
||||
self.session.add(
|
||||
FileShare(
|
||||
id="expired-share",
|
||||
tenant_id=TENANT_ID,
|
||||
file_asset_id=self.asset.id,
|
||||
target_type="user",
|
||||
target_id=RECIPIENT_ID,
|
||||
permission="read",
|
||||
expires_at=utcnow() - timedelta(seconds=1),
|
||||
)
|
||||
)
|
||||
self.session.commit()
|
||||
|
||||
with self.assertRaisesRegex(FileStorageError, "No access"):
|
||||
get_asset_for_user(
|
||||
self.session,
|
||||
tenant_id=TENANT_ID,
|
||||
user_id=RECIPIENT_ID,
|
||||
asset_id=self.asset.id,
|
||||
)
|
||||
self.assertEqual(
|
||||
[],
|
||||
list_file_shares(
|
||||
self.session,
|
||||
tenant_id=TENANT_ID,
|
||||
asset_id=self.asset.id,
|
||||
),
|
||||
)
|
||||
|
||||
@patch(
|
||||
"govoplan_files.backend.storage.files.user_group_ids",
|
||||
return_value=[GROUP_ID],
|
||||
)
|
||||
def test_independent_active_grant_survives_other_expiry(self, _groups) -> None:
|
||||
self.session.add_all(
|
||||
[
|
||||
FileShare(
|
||||
id="expired-user-share",
|
||||
tenant_id=TENANT_ID,
|
||||
file_asset_id=self.asset.id,
|
||||
target_type="user",
|
||||
target_id=RECIPIENT_ID,
|
||||
permission="read",
|
||||
expires_at=utcnow() - timedelta(seconds=1),
|
||||
),
|
||||
FileShare(
|
||||
id="active-group-share",
|
||||
tenant_id=TENANT_ID,
|
||||
file_asset_id=self.asset.id,
|
||||
target_type="group",
|
||||
target_id=GROUP_ID,
|
||||
permission="read",
|
||||
expires_at=utcnow() + timedelta(hours=1),
|
||||
),
|
||||
]
|
||||
)
|
||||
self.session.commit()
|
||||
|
||||
result = get_asset_for_user(
|
||||
self.session,
|
||||
tenant_id=TENANT_ID,
|
||||
user_id=RECIPIENT_ID,
|
||||
asset_id=self.asset.id,
|
||||
)
|
||||
|
||||
self.assertEqual(self.asset.id, result.id)
|
||||
self.assertEqual(
|
||||
["active-group-share"],
|
||||
[
|
||||
share.id
|
||||
for share in list_file_shares(
|
||||
self.session,
|
||||
tenant_id=TENANT_ID,
|
||||
asset_id=self.asset.id,
|
||||
)
|
||||
],
|
||||
)
|
||||
|
||||
@patch(
|
||||
"govoplan_files.backend.storage.files.ensure_share_target_exists",
|
||||
return_value=None,
|
||||
)
|
||||
def test_revoke_is_idempotent_and_future_expiry_is_persisted(
|
||||
self, _target_exists
|
||||
) -> None:
|
||||
expiry = utcnow() + timedelta(days=1)
|
||||
share = share_file(
|
||||
self.session,
|
||||
tenant_id=TENANT_ID,
|
||||
asset=self.asset,
|
||||
target_type="user",
|
||||
target_id=RECIPIENT_ID,
|
||||
permission="read",
|
||||
user_id=OWNER_ID,
|
||||
expires_at=expiry,
|
||||
)
|
||||
self.session.commit()
|
||||
|
||||
revoked, first_changed = revoke_file_share(
|
||||
self.session,
|
||||
tenant_id=TENANT_ID,
|
||||
asset_id=self.asset.id,
|
||||
share_id=share.id,
|
||||
user_id=OWNER_ID,
|
||||
)
|
||||
self.session.commit()
|
||||
repeated, second_changed = revoke_file_share(
|
||||
self.session,
|
||||
tenant_id=TENANT_ID,
|
||||
asset_id=self.asset.id,
|
||||
share_id=share.id,
|
||||
user_id=OWNER_ID,
|
||||
)
|
||||
|
||||
self.assertTrue(first_changed)
|
||||
self.assertFalse(second_changed)
|
||||
self.assertEqual(OWNER_ID, revoked.revoked_by_user_id)
|
||||
self.assertEqual(revoked.revoked_at, repeated.revoked_at)
|
||||
self.assertEqual([], list_file_shares(
|
||||
self.session,
|
||||
tenant_id=TENANT_ID,
|
||||
asset_id=self.asset.id,
|
||||
))
|
||||
self.assertEqual(
|
||||
[share.id],
|
||||
[
|
||||
item.id
|
||||
for item in list_file_shares(
|
||||
self.session,
|
||||
tenant_id=TENANT_ID,
|
||||
asset_id=self.asset.id,
|
||||
include_inactive=True,
|
||||
)
|
||||
],
|
||||
)
|
||||
|
||||
@patch(
|
||||
"govoplan_files.backend.storage.files.ensure_share_target_exists",
|
||||
return_value=None,
|
||||
)
|
||||
def test_past_expiry_is_rejected(self, _target_exists) -> None:
|
||||
with self.assertRaisesRegex(FileStorageError, "future"):
|
||||
share_file(
|
||||
self.session,
|
||||
tenant_id=TENANT_ID,
|
||||
asset=self.asset,
|
||||
target_type="user",
|
||||
target_id=RECIPIENT_ID,
|
||||
permission="read",
|
||||
user_id=OWNER_ID,
|
||||
expires_at=utcnow() - timedelta(seconds=1),
|
||||
)
|
||||
|
||||
|
||||
if __name__ == "__main__":
|
||||
unittest.main()
|
||||
Reference in New Issue
Block a user