From a9c7a7a40cc6410a87f8eafafd0566a9318e0f6e Mon Sep 17 00:00:00 2001 From: Albrecht Degering Date: Wed, 19 Aug 2026 23:25:43 +0200 Subject: [PATCH] feat(files): add safe connector space removal --- docs/FILES_HANDBOOK.md | 12 ++++ src/govoplan_files/backend/manifest.py | 4 +- tests/test_connector_space_deletion.py | 68 +++++++++++++++++++ tests/test_router_contract.py | 2 + webui/package.json | 1 + ...test-connector-space-removal-structure.mjs | 12 ++++ webui/src/features/files/FilesPage.tsx | 51 ++++++++++++++ 7 files changed, 149 insertions(+), 1 deletion(-) create mode 100644 tests/test_connector_space_deletion.py create mode 100644 webui/scripts/test-connector-space-removal-structure.mjs diff --git a/docs/FILES_HANDBOOK.md b/docs/FILES_HANDBOOK.md index 606194b..a7bf282 100644 --- a/docs/FILES_HANDBOOK.md +++ b/docs/FILES_HANDBOOK.md @@ -724,6 +724,18 @@ All routes below are under `/api/v1/files`. | Incremental connector settings | `GET /connectors/settings/delta` | | Form evidence | `POST /form-evidence/upload` with a short-lived `X-Form-Evidence-Token` issued by Forms Runtime | +The Files workspace exposes **Remove space** only for read-only connector +spaces and only to actors with file-organization authority over the owning user +or group space. Confirmation explains the exact boundary: removal soft-deletes +the local connector-space definition and makes that virtual view disappear. It +does not mutate or delete remote provider content, previously imported managed +files, their metadata or shares, the connector profile, credentials, or remote +object references. Those retained objects therefore do not block removal and +the connector location can be linked again later. User and group managed spaces +are intrinsic ownership scopes rather than removable records, so they never +offer this action. A missing, already removed, cross-tenant, or inaccessible +connector space fails through the backend lookup and owner-access checks. + Consumers should use cursor/watermark contracts instead of assuming an unbounded complete list. The default full-list page size is 500 and public page sizes are capped at 1,000. diff --git a/src/govoplan_files/backend/manifest.py b/src/govoplan_files/backend/manifest.py index 713b674..a0dbca7 100644 --- a/src/govoplan_files/backend/manifest.py +++ b/src/govoplan_files/backend/manifest.py @@ -1096,7 +1096,8 @@ manifest = ModuleManifest( summary="Keep endpoint profiles, reusable credentials, and inherited connector policy separate, and understand what DELETE removes immediately.", body=( "System, tenant, and one user/group/campaign leaf form the effective policy chain: deny rules win and every configured allow rule must match. " - "Responses redact secret values and deployment references. Deleting a database-managed credential or profile immediately scrubs Files-owned encrypted material and private metadata in the same transaction as a non-secret audit event; dependent profiles are disabled, while legacy non-owned references are only detached and audited." + "Responses redact secret values and deployment references. Deleting a database-managed credential or profile immediately scrubs Files-owned encrypted material and private metadata in the same transaction as a non-secret audit event; dependent profiles are disabled, while legacy non-owned references are only detached and audited. " + "Removing a connector space is a separate owner-authorized operation: it retires only the local virtual-space link and leaves provider content, imported managed files and shares, profiles, credentials, and remote references untouched. Intrinsic user and group managed spaces cannot be removed." ), layer="configured", documentation_types=("admin",), @@ -1166,6 +1167,7 @@ manifest = ModuleManifest( "New API-managed external secret references fail closed until Files can prove ownership and provider-side deletion.", "Deletion and destructive retirement scrub Files-owned encrypted connector material before completion and emit non-secret audit evidence.", "Legacy non-owned external references are detached and audited, never sent to an arbitrary provider delete operation.", + "Connector-space removal is local and soft; it never claims to delete remote or previously imported managed content.", ], "related_topic_ids": [ "files.workflow.import-managed-snapshot", diff --git a/tests/test_connector_space_deletion.py b/tests/test_connector_space_deletion.py new file mode 100644 index 0000000..7925443 --- /dev/null +++ b/tests/test_connector_space_deletion.py @@ -0,0 +1,68 @@ +from __future__ import annotations + +import unittest +from types import SimpleNamespace +from unittest.mock import Mock, patch + +from govoplan_files.backend.storage.common import FileStorageError +from govoplan_files.backend.storage.connector_spaces import soft_delete_connector_space + + +class ConnectorSpaceDeletionTests(unittest.TestCase): + def setUp(self) -> None: + self.session = Mock() + self.space = SimpleNamespace( + tenant_id="tenant-1", + owner_type="user", + owner_user_id="owner-1", + owner_group_id=None, + deleted_at=None, + is_active=True, + ) + + @patch("govoplan_files.backend.storage.connector_spaces.utcnow") + @patch("govoplan_files.backend.storage.connector_spaces.ensure_owner_access") + def test_owner_can_soft_delete_local_space_definition( + self, ensure_owner_access: Mock, utcnow: Mock + ) -> None: + deleted_at = object() + utcnow.return_value = deleted_at + + result = soft_delete_connector_space( + self.session, self.space, user_id="owner-1" + ) + + self.assertIs(result, self.space) + self.assertIs(deleted_at, self.space.deleted_at) + self.assertFalse(self.space.is_active) + ensure_owner_access.assert_called_once_with( + self.session, + tenant_id="tenant-1", + owner_type="user", + owner_id="owner-1", + user_id="owner-1", + is_admin=False, + ) + self.session.add.assert_called_once_with(self.space) + self.session.flush.assert_called_once_with() + + @patch( + "govoplan_files.backend.storage.connector_spaces.ensure_owner_access", + side_effect=FileStorageError("File space access denied"), + ) + def test_inaccessible_space_is_blocked_without_mutation( + self, _ensure_owner_access: Mock + ) -> None: + with self.assertRaisesRegex(FileStorageError, "access denied"): + soft_delete_connector_space( + self.session, self.space, user_id="other-user" + ) + + self.assertIsNone(self.space.deleted_at) + self.assertTrue(self.space.is_active) + self.session.add.assert_not_called() + self.session.flush.assert_not_called() + + +if __name__ == "__main__": + unittest.main() diff --git a/tests/test_router_contract.py b/tests/test_router_contract.py index dee8f3d..fa09180 100644 --- a/tests/test_router_contract.py +++ b/tests/test_router_contract.py @@ -85,6 +85,8 @@ class FilesRouterContractTests(unittest.TestCase): (("POST",), "/files/connectors/credentials"), (("GET",), "/files/connector-spaces"), (("POST",), "/files/connector-spaces"), + (("PATCH",), "/files/connector-spaces/{space_id}"), + (("DELETE",), "/files/connector-spaces/{space_id}"), } self.assertTrue(expected.issubset(routes)) diff --git a/webui/package.json b/webui/package.json index 3b00528..da083ea 100644 --- a/webui/package.json +++ b/webui/package.json @@ -16,6 +16,7 @@ "scripts": { "test:file-drop-target": "node scripts/test-file-drop-target-structure.mjs", "test:file-property-filters": "node scripts/test-file-property-filters-structure.mjs", + "test:connector-space-removal": "node scripts/test-connector-space-removal-structure.mjs", "test:interface-pattern-language": "node scripts/test-interface-pattern-language.mjs" }, "peerDependencies": { diff --git a/webui/scripts/test-connector-space-removal-structure.mjs b/webui/scripts/test-connector-space-removal-structure.mjs new file mode 100644 index 0000000..4e92595 --- /dev/null +++ b/webui/scripts/test-connector-space-removal-structure.mjs @@ -0,0 +1,12 @@ +import assert from "node:assert/strict"; +import { readFileSync } from "node:fs"; + +const source = readFileSync(new URL("../src/features/files/FilesPage.tsx", import.meta.url), "utf8"); + +assert.match(source, /deleteFileConnectorSpace\(settings, target\.connector_space_id\)/); +assert.match(source, /activeSpaceIsConnector &&[\s\S]*setConnectorSpaceRemovalTarget\(activeSpace\)/); +assert.match(source, /Remote files and folders remain at the provider/); +assert.match(source, /Imported GovOPlaN files, metadata and shares remain managed/); +assert.match(source, /connector profile, credentials and remote references are not deleted/); + +console.log("Connector-space removal structure checks passed."); diff --git a/webui/src/features/files/FilesPage.tsx b/webui/src/features/files/FilesPage.tsx index 574177d..0fb6b40 100644 --- a/webui/src/features/files/FilesPage.tsx +++ b/webui/src/features/files/FilesPage.tsx @@ -25,6 +25,7 @@ import { createFolder, confirmArchiveUpload, deleteFolder, + deleteFileConnectorSpace, downloadFile, downloadFilesAsZip, fetchResourceAccessExplanation, @@ -168,6 +169,7 @@ export default function FilesPage({ settings, auth }: {settings: ApiSettings;aut const [connectorSpaceItemsBySpace, setConnectorSpaceItemsBySpace] = useState>({}); const [connectorSpaceLibraryBySpace, setConnectorSpaceLibraryBySpace] = useState>({}); const [connectorSpaceSelectedItem, setConnectorSpaceSelectedItem] = useState(null); + const [connectorSpaceRemovalTarget, setConnectorSpaceRemovalTarget] = useState(null); const [connectorSpaceLoading, setConnectorSpaceLoading] = useState(false); const [connectorSpaceError, setConnectorSpaceError] = useState(""); const [message, setMessage] = useState(""); @@ -1119,6 +1121,35 @@ export default function FilesPage({ settings, auth }: {settings: ApiSettings;aut } } + async function removeConnectorSpace() { + const target = connectorSpaceRemovalTarget; + if (!canOrganize || !target?.connector_space_id || !isConnectorSpace(target)) return; + setBusy(true); + setError(""); + setMessage(""); + try { + await deleteFileConnectorSpace(settings, target.connector_space_id); + setConnectorSpaceRemovalTarget(null); + setConnectorSpaceItemsBySpace((current) => { + const next = { ...current }; + delete next[target.id]; + return next; + }); + setConnectorSpaceLibraryBySpace((current) => { + const next = { ...current }; + delete next[target.id]; + return next; + }); + setConnectorSpaceSelectedItem(null); + setMessage(`Removed connector space “${target.label}”. Remote content and managed GovOPlaN files were not changed.`); + await loadSpaces(); + } catch (err) { + setError(err instanceof Error ? err.message : String(err)); + } finally { + setBusy(false); + } + } + function connectorMetadataString(item: FileConnectorBrowseItem, key: string): string | null { const value = item.metadata[key]; if (typeof value === "string" && value.trim()) return value; @@ -2150,6 +2181,15 @@ export default function FilesPage({ settings, auth }: {settings: ApiSettings;aut {activeSpaceIsConnector && + + } + {activeSpaceIsConnector && @@ -2520,6 +2560,17 @@ export default function FilesPage({ settings, auth }: {settings: ApiSettings;aut busy={busy} onConfirm={() => void performConfirmedDelete()} onCancel={() => setDeleteDialog(null)} /> + + void removeConnectorSpace()} + onCancel={() => setConnectorSpaceRemovalTarget(null)} /> {contextMenu &&