From 587907bf0aa3b9b5c88541b713f43c0f4cb8f4bf Mon Sep 17 00:00:00 2001 From: pvagare-ks Date: Fri, 17 Jul 2026 11:39:50 +0530 Subject: [PATCH 1/6] added nsf support for cyberark import --- docs/cyberark-pam-import.md | 6 +- .../commands/pam_import/record_loader.py | 15 +- .../importer/cyberark/pam/idempotency.py | 26 +++- tests/test_cyberark_pam_import.py | 135 ++++++++++++++++++ 4 files changed, 172 insertions(+), 10 deletions(-) diff --git a/docs/cyberark-pam-import.md b/docs/cyberark-pam-import.md index da5ee70d7..60295461a 100644 --- a/docs/cyberark-pam-import.md +++ b/docs/cyberark-pam-import.md @@ -167,6 +167,9 @@ pam project cyberark-import pvwa.company.com --dry-run --output import.json --in # Filter specific safes pam project cyberark-import pvwa.company.com --safes "Production,Staging" --exclude-safes "Archive*" +# Import into Nested Share Folders (folders, records, rotation, PAM config) +pam project cyberark-import pvwa.company.com --name "CyberArk Migration" --gateway "My Gateway" --nsf + # Extend existing project pam project cyberark-import pvwa.company.com --config @@ -196,7 +199,8 @@ pam project cyberark-cleanup --name "CyberArk Migration" --dry-run | `--name`, `-n` | Project name | | `--config`, `-c` | Extend existing PAM config UID | | `--gateway`, `-g` | Gateway name or UID | -| `--folder-mode` | flat, exact, ksm (default) | +| `--folder-mode` | flat, exact, ksm, safe (default: safe) | +| `--nsf` | Create project folders/records/PAM config in Nested Share Folders | | `--safes` | Include only these safes (comma/glob) | | `--exclude-safes` | Exclude safes (comma/glob) | | `--list-safes` | List safes and exit | diff --git a/keepercommander/commands/pam_import/record_loader.py b/keepercommander/commands/pam_import/record_loader.py index 306614798..4b3ac5bb0 100644 --- a/keepercommander/commands/pam_import/record_loader.py +++ b/keepercommander/commands/pam_import/record_loader.py @@ -21,17 +21,20 @@ def iter_accessible_record_uids(params) -> Iterator[str]: """Yield record UIDs from classic and Nested Shared Folder caches.""" seen = set() for attr in ('record_cache', 'nested_share_records'): - cache = getattr(params, attr, None) or {} + cache = getattr(params, attr, None) + if not isinstance(cache, dict): + continue for uid in cache: if uid not in seen: seen.add(uid) yield uid - nsf_record_data = getattr(params, 'nested_share_record_data', None) or {} - for uid in nsf_record_data: - if uid not in seen: - seen.add(uid) - yield uid + nsf_record_data = getattr(params, 'nested_share_record_data', None) + if isinstance(nsf_record_data, dict): + for uid in nsf_record_data: + if uid not in seen: + seen.add(uid) + yield uid def load_pam_record(params, record_uid: str) -> Optional[vault.KeeperRecord]: diff --git a/keepercommander/importer/cyberark/pam/idempotency.py b/keepercommander/importer/cyberark/pam/idempotency.py index 4851c2188..7245e09c4 100644 --- a/keepercommander/importer/cyberark/pam/idempotency.py +++ b/keepercommander/importer/cyberark/pam/idempotency.py @@ -205,9 +205,16 @@ def build_existing_index(params, folder_uids) -> ExistingRecordIndex: # importer falls back to always-create mode instead of crashing. subfolder_record_cache = getattr(params, "subfolder_record_cache", None) or {} folder_cache = getattr(params, "folder_cache", None) or {} - if not folder_cache: + if not folder_cache and not getattr(params, "nested_share_folders", None): return index + # Prefer NSF-aware record lookup so Nested Share Folder projects are + # indexed the same way as classic shared-folder projects. + try: + from keepercommander.commands.pam_import.nsf_helpers import get_folder_record_uids + except ImportError: # pragma: no cover — defensive for alternate layouts + get_folder_record_uids = None + # Recursively collect record UIDs from every subfolder. stack = list(folder_uids) visited: Set[str] = set() @@ -218,15 +225,28 @@ def build_existing_index(params, folder_uids) -> ExistingRecordIndex: continue visited.add(fuid) index.scanned_folder_uids.add(fuid) - for ruid in (subfolder_record_cache.get(fuid) or set()): + if get_folder_record_uids is not None: + record_uids = get_folder_record_uids(params, fuid) + else: + record_uids = subfolder_record_cache.get(fuid) or set() + for ruid in record_uids: all_record_uids.add(ruid) index.folder_by_record[ruid] = fuid folder = folder_cache.get(fuid) for sub_uid in (getattr(folder, "subfolders", []) or []) if folder else []: stack.append(sub_uid) + # NSF children may only be linked via nested_share_folders + for child_uid, info in (getattr(params, "nested_share_folders", None) or {}).items(): + if (info.get("parent_uid") or None) == fuid and child_uid not in visited: + stack.append(child_uid) for ruid in all_record_uids: - rec = vault.KeeperRecord.load(params, ruid) + # Prefer NSF-aware loader so Nested Share Folder records resolve. + try: + from keepercommander.commands.pam_import.record_loader import load_pam_record + rec = load_pam_record(params, ruid) or vault.KeeperRecord.load(params, ruid) + except ImportError: # pragma: no cover + rec = vault.KeeperRecord.load(params, ruid) if rec is None: continue rtype = getattr(rec, "record_type", "") or "" diff --git a/tests/test_cyberark_pam_import.py b/tests/test_cyberark_pam_import.py index 619e7e646..047cb9fc2 100644 --- a/tests/test_cyberark_pam_import.py +++ b/tests/test_cyberark_pam_import.py @@ -711,6 +711,7 @@ def test_all_flags_parse(self): "--platform-map", "map.json", "--state-filter", "active,inactive", "--no-verify-ssl", + "--nsf", ]) assert args.server == "pvwa.example.com" assert args.project_name == "My Project" @@ -735,6 +736,7 @@ def test_all_flags_parse(self): assert args.platform_map == "map.json" assert args.state_filter == "active,inactive" assert args.no_verify_ssl is True + assert args.use_nsf is True def test_minimal_args(self): cmd = CyberArkPAMImportCommand() @@ -742,6 +744,12 @@ def test_minimal_args(self): assert args.server == "pvwa.example.com" assert args.project_name == "" assert args.dry_run is False + assert args.use_nsf is False + + def test_nsf_flag(self): + cmd = CyberArkPAMImportCommand() + args = cmd.parser.parse_args(["pvwa.example.com", "--nsf"]) + assert args.use_nsf is True def test_folder_mode_choices(self): cmd = CyberArkPAMImportCommand() @@ -2648,6 +2656,133 @@ def test_parser_config_flag(self): assert args.config_uid == "uid123" +class TestCyberArkImportNsfSupport: + """NSF (--nsf) wiring for cyberark-import / cleanup / discovery.""" + + def test_single_batch_passes_use_nsf_to_import(self): + from keepercommander.commands.pam_import.cyberark_import import ( + CyberArkPAMImportCommand, _temp_store, + ) + cmd = CyberArkPAMImportCommand() + params = MagicMock() + import_data = {"pam_data": {"resources": [], "users": []}} + + with patch.object(_temp_store, "write_json", return_value="/tmp/x.json"), \ + patch.object(_temp_store, "remove"), \ + patch("keepercommander.commands.pam_import.edit.PAMProjectImportCommand") as mock_import, \ + patch.object(cmd, "_find_config_uid", return_value="cfg-uid"): + mock_import.return_value.execute.return_value = None + result = cmd._single_batch_import( + params, import_data, "Proj", "", use_nsf=True, + ) + + assert result["config_uid"] == "cfg-uid" + kwargs = mock_import.return_value.execute.call_args.kwargs + assert kwargs.get("use_nsf") is True + assert kwargs.get("project_name") == "Proj" + + def test_single_batch_extend_ignores_use_nsf_flag(self): + from keepercommander.commands.pam_import.cyberark_import import ( + CyberArkPAMImportCommand, _temp_store, + ) + cmd = CyberArkPAMImportCommand() + params = MagicMock() + import_data = {"pam_data": {"resources": [], "users": []}} + + with patch.object(_temp_store, "write_json", return_value="/tmp/x.json"), \ + patch.object(_temp_store, "remove"), \ + patch("keepercommander.commands.pam_import.extend.PAMProjectExtendCommand") as mock_extend: + mock_extend.return_value.execute.return_value = None + result = cmd._single_batch_import( + params, import_data, "Proj", "existing-cfg", use_nsf=True, + ) + + assert result["config_uid"] == "existing-cfg" + kwargs = mock_extend.return_value.execute.call_args.kwargs + assert "use_nsf" not in kwargs + assert kwargs.get("config") == "existing-cfg" + + def test_find_nsf_project_wrapper_uids(self): + from types import SimpleNamespace + from keepercommander.commands.pam_import.cyberark_import import CyberArkPAMCleanupCommand + from keepercommander.subfolder import NestedShareFolderNode + + root = NestedShareFolderNode() + root.uid = "nsf-root" + root.name = "PAM Environments" + root.parent_uid = None + root.subfolders = ["nsf-proj"] + + proj = NestedShareFolderNode() + proj.uid = "nsf-proj" + proj.name = "CyberArk Migration" + proj.parent_uid = "nsf-root" + proj.subfolders = ["nsf-safe"] + + safe = NestedShareFolderNode() + safe.uid = "nsf-safe" + safe.name = "Win_Local" + safe.parent_uid = "nsf-proj" + safe.subfolders = [] + + params = SimpleNamespace( + folder_cache={ + "nsf-root": root, + "nsf-proj": proj, + "nsf-safe": safe, + }, + nested_share_folders={ + "nsf-root": {"name": "PAM Environments", "parent_uid": None}, + "nsf-proj": {"name": "CyberArk Migration", "parent_uid": "nsf-root"}, + "nsf-safe": {"name": "Win_Local", "parent_uid": "nsf-proj"}, + }, + shared_folder_cache={}, + subfolder_record_cache={}, + nested_share_folder_records={"nsf-safe": {"rec-1"}}, + ) + + wrappers = CyberArkPAMCleanupCommand._find_project_wrapper_folder_uids( + params, "CyberArk Migration", + ) + assert wrappers == ["nsf-proj"] + + children = list(CyberArkPAMCleanupCommand._iter_project_child_folders( + params, "nsf-proj", + )) + assert children == [("nsf-safe", "Win_Local")] + + def test_find_classic_wrapper_still_works(self): + from types import SimpleNamespace + from keepercommander.commands.pam_import.cyberark_import import CyberArkPAMCleanupCommand + from keepercommander.subfolder import BaseFolderNode + + root = SimpleNamespace( + uid="uf-root", name="PAM Environments", parent_uid=None, + type=BaseFolderNode.UserFolderType, subfolders=["uf-proj"], + ) + proj = SimpleNamespace( + uid="uf-proj", name="MyProj", parent_uid="uf-root", + type=BaseFolderNode.UserFolderType, subfolders=["sf-safe"], + ) + safe = SimpleNamespace( + uid="sf-safe", name="SafeA", parent_uid="uf-proj", + type=BaseFolderNode.SharedFolderType, subfolders=[], + ) + params = SimpleNamespace( + folder_cache={"uf-root": root, "uf-proj": proj, "sf-safe": safe}, + nested_share_folders={}, + shared_folder_cache={}, + ) + wrappers = CyberArkPAMCleanupCommand._find_project_wrapper_folder_uids( + params, "MyProj", + ) + assert wrappers == ["uf-proj"] + children = list(CyberArkPAMCleanupCommand._iter_project_child_folders( + params, "uf-proj", + )) + assert children == [("sf-safe", "SafeA")] + + class TestSSHKeyImport: """C4: SSH key platforms store private key in private_pem_key, not password.""" From 3fb74448fc3c10509ef5f202f9c101a7763af6e5 Mon Sep 17 00:00:00 2001 From: pvagare-ks Date: Mon, 20 Jul 2026 17:51:17 +0530 Subject: [PATCH 2/6] fix merge conflict --- .../commands/pam_import/cyberark_import.py | 497 ++++++++++-------- 1 file changed, 276 insertions(+), 221 deletions(-) diff --git a/keepercommander/commands/pam_import/cyberark_import.py b/keepercommander/commands/pam_import/cyberark_import.py index 494ca5c41..6c8aba0ed 100644 --- a/keepercommander/commands/pam_import/cyberark_import.py +++ b/keepercommander/commands/pam_import/cyberark_import.py @@ -159,6 +159,7 @@ class ImportRunOptions: user_map_file: str sync_mode: str = "upsert" strict_policies: bool = False + use_nsf: bool = False raw_kwargs: dict = field(default_factory=dict) @@ -927,6 +928,7 @@ def _execute_vault_import(self, import_data: dict, self.params, import_data, opts.project_name, opts.config_uid, opts.batch_size, opts.batch_delay, mapped.pam_resources, mapped.pam_users, + use_nsf=opts.use_nsf, ) except Exception as e: logging.error("Import failed: %s", type(e).__name__) @@ -976,28 +978,22 @@ def _populate_folder_info(self, project_result: dict, project_name: str) -> None seen_uids: set = set() for wrapper_uid in wrapper_uids: - wrapper = self.params.folder_cache.get(wrapper_uid) - if not wrapper: - continue - for child_uid in getattr(wrapper, "subfolders", []) or []: - child = self.params.folder_cache.get(child_uid) - if not child or getattr(child, "type", "") != "shared_folder": - continue - if child.uid in seen_uids: + for child_uid, name in CyberArkPAMCleanupCommand._iter_project_child_folders( + self.params, wrapper_uid): + if child_uid in seen_uids: continue - seen_uids.add(child.uid) - name = getattr(child, "name", "") or "" + seen_uids.add(child_uid) if name == config_suffix: - config_folder_uid = child.uid + config_folder_uid = child_uid config_folder_name = name elif name == resources_suffix: - legacy_resources_uid = child.uid + legacy_resources_uid = child_uid legacy_resources_name = name elif name == users_suffix: - legacy_users_uid = child.uid + legacy_users_uid = child_uid legacy_users_name = name else: - safe_folders.append({"name": name, "uid": child.uid}) + safe_folders.append({"name": name, "uid": child_uid}) if config_folder_uid: folders_info["config_folder"] = config_folder_name @@ -1145,26 +1141,19 @@ def _prepare_idempotency(self, mapped: MappedImportResult) -> Optional[dict]: # pay the full-vault scan cost for nothing. return None - # Collect all shared folders directly under the project - # wrapper user folder(s). Records live inside these (and - # inside their Resources / Users subfolders for the - # safe-per-folder layout). - shared_folder_uids: list[str] = [] + # Collect all shared / NSF folders directly under the project + # wrapper folder(s). Records live inside these (and inside + # their Resources / Users subfolders for the safe-per-folder + # layout). + shared_folder_uids: List[str] = [] seen: set = set() for wrapper_uid in wrapper_uids: - wrapper = self.params.folder_cache.get(wrapper_uid) - if not wrapper: - continue - for child_uid in getattr(wrapper, "subfolders", []) or []: - child = self.params.folder_cache.get(child_uid) - if child is None: - continue - if getattr(child, "type", "") != "shared_folder": + for child_uid, _name in CyberArkPAMCleanupCommand._iter_project_child_folders( + self.params, wrapper_uid): + if child_uid in seen: continue - if child.uid in seen: - continue - seen.add(child.uid) - shared_folder_uids.append(child.uid) + seen.add(child_uid) + shared_folder_uids.append(child_uid) existing = build_existing_index(self.params, shared_folder_uids) if not existing.by_account_id and not existing.by_title: @@ -1865,6 +1854,9 @@ class CyberArkPAMImportCommand(Command): # Self-hosted with SSL verification disabled pam project cyberark-import pvwa.internal.com --no-verify-ssl --name "Internal" + + # Import into Nested Share Folders (NSF) + pam project cyberark-import pvwa.example.com --name "PAM Migration" --nsf ''') parser.add_argument("server", action="store", help="CyberArk PVWA host (e.g. mycompany.cyberark.cloud or pvwa.example.com)") parser.add_argument("--name", "-n", required=False, dest="project_name", action="store", @@ -1881,7 +1873,12 @@ class CyberArkPAMImportCommand(Command): "control. 'ksm'/'exact' nest safe-named subfolders under the " "legacy Resources/Users shared folders. 'flat' puts every " "record into the two legacy shared folders with aggregated " - "permissions.") + "permissions. With --nsf, the same layout is created using " + "Nested Share Folders instead of classic shared folders.") + parser.add_argument("--nsf", required=False, dest="use_nsf", action="store_true", + default=False, + help="Create project folders and records in Nested Share Folders " + "(folders, records, rotation, PAM config, and permissions).") parser.add_argument("--safes", required=False, dest="safes", action="store", default="", help="Include only matching Safes (comma-separated, supports globs)") parser.add_argument("--exclude-safes", required=False, dest="exclude_safes", action="store", @@ -2045,6 +2042,7 @@ def execute(self, params, **kwargs): user_map_file=kwargs.get("user_map", ""), sync_mode=(kwargs.get("sync_mode") or "upsert").lower(), strict_policies=bool(kwargs.get("strict_policies", False)), + use_nsf=kwargs.get("use_nsf", False) is True, raw_kwargs=kwargs, ) CyberArkImportOrchestrator(self, params, client, options).run() @@ -2053,7 +2051,8 @@ def execute(self, params, **kwargs): def _execute_import(self, params, import_data: dict, project_name: str, config_uid: str, batch_size: int, batch_delay: float, - resources: list[dict], users: list[dict]) -> Optional[dict]: + resources: list[dict], users: list[dict], + use_nsf: bool = False) -> Optional[dict]: """Execute the vault import using pam project import/extend commands.""" from .edit import PAMProjectImportCommand from .extend import PAMProjectExtendCommand @@ -2064,17 +2063,18 @@ def _execute_import(self, params, import_data: dict, project_name: str, if total_records <= batch_size: # Single batch — use import or extend directly return self._single_batch_import( - params, import_data, project_name, config_uid + params, import_data, project_name, config_uid, use_nsf=use_nsf ) else: # Multi-batch: first batch creates project, remaining extend return self._multi_batch_import( params, import_data, project_name, config_uid, - resources, users, batch_size, batch_delay, + resources, users, batch_size, batch_delay, use_nsf=use_nsf, ) def _single_batch_import(self, params, import_data: dict, - project_name: str, config_uid: str) -> dict: + project_name: str, config_uid: str, + use_nsf: bool = False) -> dict: """Import all records in a single batch.""" from .edit import PAMProjectImportCommand from .extend import PAMProjectExtendCommand @@ -2082,12 +2082,18 @@ def _single_batch_import(self, params, import_data: dict, tmp_path = _temp_store.write_json(import_data) try: if config_uid: + # Extend auto-detects NSF from the existing project tree. + if use_nsf: + logging.info( + "--nsf is ignored when extending an existing project " + "(--config); Nested Share Folders are detected automatically.") PAMProjectExtendCommand().execute( params, config=config_uid, file_name=tmp_path, dry_run=False ) else: PAMProjectImportCommand().execute( - params, project_name=project_name, file_name=tmp_path, dry_run=False + params, project_name=project_name, file_name=tmp_path, + dry_run=False, use_nsf=use_nsf, ) finally: _temp_store.remove(tmp_path) @@ -2098,7 +2104,8 @@ def _single_batch_import(self, params, import_data: dict, def _multi_batch_import(self, params, import_data: dict, project_name: str, config_uid: str, resources: list[dict], users: list[dict], - batch_size: int, batch_delay: float) -> dict: + batch_size: int, batch_delay: float, + use_nsf: bool = False) -> dict: """Import records in multiple batches with adaptive throttling.""" from .edit import PAMProjectImportCommand from .extend import PAMProjectExtendCommand @@ -2126,7 +2133,8 @@ def _multi_batch_import(self, params, import_data: dict, tmp_path = _temp_store.write_json(first_batch_data) try: PAMProjectImportCommand().execute( - params, project_name=project_name, file_name=tmp_path, dry_run=False + params, project_name=project_name, file_name=tmp_path, + dry_run=False, use_nsf=use_nsf, ) finally: _temp_store.remove(tmp_path) @@ -2136,7 +2144,7 @@ def _multi_batch_import(self, params, import_data: dict, if not config_uid: config_uid = self._find_config_uid(params, project_name) else: - # Subsequent batches: extend + # Subsequent batches: extend (NSF auto-detected from project) extend_data = build_extend_json(batch_resources, batch_users) tmp_path = _temp_store.write_json(extend_data) try: @@ -2159,17 +2167,41 @@ def _multi_batch_import(self, params, import_data: dict, def _find_config_uid(self, params, project_name: str) -> str: """Find PAM configuration UID by project name after initial import. - Handles #N suffix deduplication from PAMProjectImportCommand.""" + Handles #N suffix deduplication from PAMProjectImportCommand. + Searches classic vault records and Nested Share Folder caches. + """ from ... import api, vault_extensions + from .nsf_helpers import find_pam_configuration + from .record_loader import iter_accessible_record_uids, load_pam_record api.sync_down(params) config_base = f"{project_name} Configuration".casefold() candidates = [] + + # Classic path (keeps existing unit-test mocks working) for c in vault_extensions.find_records(params, record_version=6): t = c.title.casefold() if t == config_base or (t.startswith(config_base) and re.match(r' #\d+$', t[len(config_base):])): candidates.append(c) + + # NSF / full-access path — pick up configs that live only in NSF caches + seen_uids = {getattr(c, "record_uid", "") for c in candidates} + for uid in iter_accessible_record_uids(params): + if uid in seen_uids: + continue + rec = load_pam_record(params, uid) + if not rec or getattr(rec, "version", None) != 6: + continue + title = getattr(rec, "title", "") or "" + t = title.casefold() + if t == config_base or (t.startswith(config_base) and re.match(r' #\d+$', t[len(config_base):])): + candidates.append(rec) + seen_uids.add(uid) + if not candidates: + exact = find_pam_configuration(params, f"{project_name} Configuration") + if exact: + return exact.record_uid logging.warning(f"PAM configuration not found for project '{project_name}' after import") return "" # Prefer highest suffix number (most recently created) @@ -2282,16 +2314,18 @@ def execute(self, params, **kwargs): raise CommandError("pam project cyberark-cleanup", "Either --name or --config is required") - from ... import api, utils, vault, vault_extensions - from ..pam import gateway_helper - from ..pam.config_helper import configuration_controller_get - from ...loginv3 import CommonHelperMethods + from ... import api, vault, vault_extensions api.sync_down(params) - # Find PAM config by name or UID + # Find PAM config by name or UID (classic + NSF) + from .nsf_helpers import find_pam_configuration + from .record_loader import iter_accessible_record_uids, load_pam_record + if config_uid: config_rec = vault.KeeperRecord.load(params, config_uid) + if not config_rec: + config_rec = load_pam_record(params, config_uid) if not config_rec: raise CommandError("pam project cyberark-cleanup", f"PAM config record '{config_uid}' not found") @@ -2304,47 +2338,40 @@ def execute(self, params, **kwargs): config_rec = c config_uid = c.record_uid break + if not config_rec: + for uid in iter_accessible_record_uids(params): + rec = load_pam_record(params, uid) + if not rec or getattr(rec, "version", None) != 6: + continue + title = getattr(rec, "title", "") or "" + if title.casefold().startswith(config_base): + config_rec = rec + config_uid = uid + break + if not config_rec: + exact = find_pam_configuration(params, f"{project_name} Configuration") + if exact: + config_rec = exact + config_uid = exact.record_uid if not config_rec: raise CommandError("pam project cyberark-cleanup", f"PAM config for project '{project_name}' not found") - # Resolve gateway linked to this PAM config - gateway_uid = None - gateway_name = None - gw_match = None - try: - controller = configuration_controller_get( - params, CommonHelperMethods.url_safe_str_to_bytes( - config_rec.record_uid)) - if controller and controller.controllerUid: - gateway_uid = controller.controllerUid - all_gw = gateway_helper.get_all_gateways(params) - gw_match = next((g for g in all_gw - if g.controllerUid == gateway_uid), None) - if gw_match: - gateway_name = gw_match.controllerName - except Exception as e: - logging.debug("Could not resolve gateway: %s", e) - - # Resolve KSM application linked to the gateway - ksm_app_uid = None - ksm_app_name = None - if gw_match and gw_match.applicationUid: - ksm_app_uid = utils.base64_url_encode(gw_match.applicationUid) - app_rec = vault.KeeperRecord.load(params, ksm_app_uid) - if app_rec: - ksm_app_name = getattr(app_rec, "title", ksm_app_uid) - - # Find shared folders. The new safe-per-folder layout creates one - # shared folder per CyberArk safe under the project wrapper folder + # Find shared / NSF folders. The new safe-per-folder layout creates one + # folder per CyberArk safe under the project wrapper folder # plus an admin Config folder; each safe folder has two - # ``Resources``/``Users`` shared_folder_folder subfolders that + # ``Resources``/``Users`` subfolders that # hold the records. The legacy layout creates exactly two folders # ("{project} - Resources" and "{project} - Users") with safe-named # subfolders inside. Discover everything by walking the project - # wrapper user-folder under PAM Environments so cleanup handles - # both shapes (and any subset thereof) without hardcoding names. + # wrapper under PAM Environments so cleanup handles + # both classic shared folders and Nested Share Folders + # (and any subset thereof) without hardcoding names. + from .nsf_helpers import get_folder_record_uids + from ..pam.vault_target import is_nested_share_folder, is_pam_nsf_record + sf_uids = [] + record_count = 0 sf_names: list = [] all_record_uids: set = set() res_name = f"{project_name} - Resources" @@ -2354,68 +2381,40 @@ def execute(self, params, **kwargs): def _collect_records_recursive(folder_uid: str): """Walk the folder subtree and accumulate every record UID (records living directly in this folder + records living in - any descendant ``shared_folder_folder``).""" + any descendant classic or NSF subfolder).""" stack = [folder_uid] + visited: set = set() while stack: fuid = stack.pop() - for ruid in params.subfolder_record_cache.get(fuid, set()) or set(): - all_record_uids.add(ruid) + if not fuid or fuid in visited: + continue + visited.add(fuid) + all_record_uids.update(get_folder_record_uids(params, fuid)) folder = params.folder_cache.get(fuid) if not folder: + # NSF-only parents may still appear in nested_share_folders + nsf = getattr(params, "nested_share_folders", None) or {} + for child_uid, info in nsf.items(): + if (info.get("parent_uid") or None) == fuid: + stack.append(child_uid) continue for sub_uid in getattr(folder, "subfolders", []) or []: stack.append(sub_uid) - def _delete_folder(folder_uid: str) -> bool: - folder = params.folder_cache.get(folder_uid) - if not folder: - return False - del_obj = { - "delete_resolution": "unlink", - "object_uid": folder.uid, - "object_type": folder.type, - } - parent = params.folder_cache.get(folder.parent_uid) - if parent: - del_obj["from_uid"] = parent.uid - del_obj["from_type"] = parent.type - else: - del_obj["from_type"] = "user_folder" - rq = {"command": "pre_delete", "objects": [del_obj]} - rs = api.communicate(params, rq) - if rs.get("result") != "success": - return False - pdr = rs.get("pre_delete_response", {}) - del_rq = { - "command": "delete", - "pre_delete_token": pdr.get("pre_delete_token", ""), - } - api.communicate(params, del_rq) - return True - project_folder_uids = self._find_project_wrapper_folder_uids( params, project_name, ) if project_folder_uids: sf_uids_seen: set = set() for project_uid in project_folder_uids: - project_folder = params.folder_cache.get(project_uid) - if not project_folder: - continue - for child_uid in getattr(project_folder, "subfolders", []) or []: - child = params.folder_cache.get(child_uid) - if not child: - continue - # Only collect shared folders that we created — i.e. - # the type is SharedFolderType. - if getattr(child, "type", "") != "shared_folder": - continue - if child.uid in sf_uids_seen: + for child_uid, child_name in self._iter_project_child_folders( + params, project_uid): + if child_uid in sf_uids_seen: continue - sf_uids_seen.add(child.uid) - sf_uids.append(child.uid) - sf_names.append(getattr(child, "name", "") or "") - _collect_records_recursive(child.uid) + sf_uids_seen.add(child_uid) + sf_uids.append(child_uid) + sf_names.append(child_name) + _collect_records_recursive(child_uid) else: # Fallback: scan the shared-folder cache by name. Catches the # legacy two-folder layout when the project wrapper folder was @@ -2426,25 +2425,24 @@ def _delete_folder(folder_uid: str) -> bool: sf_uids.append(sf_uid) sf_names.append(name) _collect_records_recursive(sf_uid) - - # Ensure PAM config is included even if it lives outside the - # discovered shared-folder tree. - if config_uid: - all_record_uids.add(config_uid) + # NSF fallback by name under nested_share_folders + for nsf_uid, info in (getattr(params, "nested_share_folders", None) or {}).items(): + name = info.get("name", "") or "" + if name in (res_name, usr_name, config_name) and nsf_uid not in sf_uids: + sf_uids.append(nsf_uid) + sf_names.append(name) + _collect_records_recursive(nsf_uid) + record_count = len(all_record_uids) print(f"\nCyberArk PAM Project Cleanup") print("=" * 50) - print(f" Project: {project_name}") - print(f" PAM Config: {config_uid}") - print(f" Gateway: {gateway_name or '(not found)'}") - print(f" KSM App: {ksm_app_name or ksm_app_uid or '(not found)'}") - print(f" Folders: {len(sf_uids)}") + print(f" Project: {project_name}") + print(f" Config: {config_uid}") + print(f" Folders: {len(sf_uids)}") for sf_name in sf_names: if sf_name: print(f" • {sf_name}") - if project_folder_uids: - print(f" Wrappers: {len(project_folder_uids)}") - print(f" Records: {len(all_record_uids)}") + print(f" Records: ~{record_count}") if dry_run: print(" (dry run — no changes made)") @@ -2452,79 +2450,42 @@ def _delete_folder(folder_uid: str) -> bool: return if not auto_confirm: - answer = input("\n Delete all of the above? [y/N]: ").strip().lower() + answer = input("\n Delete all records and folders? [y/N]: ").strip().lower() if answer not in ("y", "yes"): print(" Cancelled.") return + from ..record_edit import RecordDeleteCommand + from ..nested_share_folder.record_commands import NestedShareRecordRemoveCommand + deleted = 0 failed = 0 - - # Delete records in batches (same API as api.delete_record) - if all_record_uids: - logging.warning("Deleting %d records...", len(all_record_uids)) - uid_list = list(all_record_uids) - batch_size = 50 - for i in range(0, len(uid_list), batch_size): - batch = uid_list[i:i + batch_size] - try: - rq = {"command": "record_update", "delete_records": batch} - api.communicate(params, rq) - deleted += len(batch) - except Exception as e: - failed += len(batch) - logging.warning("Failed to delete record batch: %s", e) - - # Delete shared folders (safe folders + Config folder) - if sf_uids: - logging.warning("Removing %d shared folder(s)...", len(sf_uids)) - for sf_uid in sf_uids: - try: - if not _delete_folder(sf_uid): - failed += 1 - logging.warning("Failed to remove shared folder %s", - sf_uid) - except Exception as e: - failed += 1 - logging.warning("Failed to remove shared folder %s: %s", - sf_uid, e) - - # Delete project wrapper user-folder(s) under PAM Environments - if project_folder_uids: - logging.warning("Removing %d project wrapper folder(s)...", - len(project_folder_uids)) - for wrapper_uid in project_folder_uids: - try: - if not _delete_folder(wrapper_uid): - failed += 1 - logging.warning("Failed to remove wrapper folder %s", - wrapper_uid) - except Exception as e: - failed += 1 - logging.warning("Failed to remove wrapper folder %s: %s", - wrapper_uid, e) - - # Remove gateway - if gateway_uid: - logging.warning("Removing gateway \"%s\"...", - gateway_name or gateway_uid) + for rec_uid in list(all_record_uids): try: - gateway_helper.remove_gateway(params, gateway_uid) + if is_pam_nsf_record(params, rec_uid) or any( + is_nested_share_folder(params, fuid) for fuid in sf_uids): + NestedShareRecordRemoveCommand().execute( + params, records=[rec_uid], force=True, operation="owner-trash") + else: + RecordDeleteCommand().execute(params, force=True, record=rec_uid) + deleted += 1 except Exception as e: failed += 1 - logging.warning("Failed to remove gateway: %s", e) + logging.warning("Failed to delete record %s: %s", + rec_uid, type(e).__name__) - # Remove KSM application - if ksm_app_uid: - logging.warning("Removing KSM app \"%s\"...", - ksm_app_name or ksm_app_uid) - try: - from ..ksm import KSMCommand - KSMCommand.remove_v5_app(params, ksm_app_uid, - purge=True, force=True) - except Exception as e: - failed += 1 - logging.warning("Failed to remove KSM app: %s", e) + # Delete PAM config record + try: + if is_pam_nsf_record(params, config_uid): + NestedShareRecordRemoveCommand().execute( + params, records=[config_uid], force=True, operation="owner-trash") + else: + RecordDeleteCommand().execute(params, force=True, record=config_uid) + deleted += 1 + except Exception as e: + failed += 1 + logging.warning("Failed to delete PAM config record %s: %s", + config_uid, type(e).__name__) api.sync_down(params) msg = f"\nCleanup complete: {deleted} records deleted" @@ -2535,48 +2496,142 @@ def _delete_folder(folder_uid: str) -> bool: PAM_ROOT_FOLDER_NAME = "PAM Environments" + @classmethod + def _is_project_content_folder(cls, params, folder_uid: str, folder=None) -> bool: + """True for classic shared folders or Nested Share Folders under a project.""" + from ..pam.vault_target import is_nested_share_folder + from ...subfolder import BaseFolderNode + + if is_nested_share_folder(params, folder_uid): + return True + folder = folder or (params.folder_cache or {}).get(folder_uid) + if not folder: + return False + ftype = getattr(folder, "type", "") or "" + return ftype in ( + BaseFolderNode.SharedFolderType, + BaseFolderNode.NestedShareFolderType, + ) + + @classmethod + def _iter_project_child_folders(cls, params, wrapper_uid: str): + """Yield ``(uid, name)`` for content folders under a project wrapper. + + Includes classic shared folders and Nested Share Folders so + report / idempotency / cleanup share one discovery path. + """ + seen: set = set() + folder_cache = getattr(params, "folder_cache", None) or {} + wrapper = folder_cache.get(wrapper_uid) + if wrapper: + for child_uid in getattr(wrapper, "subfolders", []) or []: + if child_uid in seen: + continue + child = folder_cache.get(child_uid) + if not child or not cls._is_project_content_folder(params, child_uid, child): + continue + seen.add(child_uid) + yield child_uid, getattr(child, "name", "") or "" + + # NSF children may exist in nested_share_folders before folder_cache + # parent.subfolders is fully linked after a partial sync. + for child_uid, info in (getattr(params, "nested_share_folders", None) or {}).items(): + if child_uid in seen: + continue + if (info.get("parent_uid") or None) != wrapper_uid: + continue + if not cls._is_project_content_folder(params, child_uid): + continue + seen.add(child_uid) + yield child_uid, info.get("name", "") or "" + @classmethod def _find_project_wrapper_folder_uids(cls, params, project_name: str) -> list: - """Return UIDs of every project wrapper user-folder under + """Return UIDs of every project wrapper folder under ``PAM Environments`` whose name matches ``project_name`` (or ``project_name #N`` when the project was imported multiple times). - The wrapper folder is a *user folder* (not shared), and per-safe - shared folders are created as direct children of it. Returning a - list keeps cleanup correct in the rare case where two projects - share a name (PAMProjectImportCommand allows duplicates via the - ``#N`` suffix). + Supports classic user-folder wrappers and Nested Share Folder + wrappers created with ``--nsf``. Returning a list keeps cleanup + correct in the rare case where two projects share a name + (PAMProjectImportCommand allows duplicates via the ``#N`` suffix). """ + from ..pam.vault_target import is_nested_share_folder + from ...subfolder import BaseFolderNode + wrapper_uids: list = [] folders = params.folder_cache if params and params.folder_cache else {} if not isinstance(folders, dict): - return wrapper_uids + folders = {} - # Locate root "PAM Environments" user folder(s). + # Locate root "PAM Environments" folder(s) — classic user folder + # and/or Nested Share Folder. root_uids: list = [] + seen_roots: set = set() for uid, f in folders.items(): if not f or getattr(f, "parent_uid", None): continue - if getattr(f, "type", "") != "user_folder": + ftype = getattr(f, "type", "") or "" + if ftype not in ( + BaseFolderNode.UserFolderType, + BaseFolderNode.NestedShareFolderType, + ): + continue + if getattr(f, "name", "") != cls.PAM_ROOT_FOLDER_NAME: continue - if getattr(f, "name", "") == cls.PAM_ROOT_FOLDER_NAME: - root_uids.append(uid) + root_uids.append(uid) + seen_roots.add(uid) + + for uid, info in (getattr(params, "nested_share_folders", None) or {}).items(): + if uid in seen_roots: + continue + if (info.get("parent_uid") or None) is not None: + continue + if info.get("name", "") != cls.PAM_ROOT_FOLDER_NAME: + continue + root_uids.append(uid) + seen_roots.add(uid) + if not root_uids: return wrapper_uids # PAMProjectImportCommand emits "{project_name}" or - # "{project_name} #N" for the wrapper user folder, so match both - # shapes here. + # "{project_name} #N" for the wrapper folder, so match both + # shapes here (classic user folder or NSF). base = project_name + name_re = re.compile(rf"^{re.escape(base)} #\d+$") + seen_wrappers: set = set() + + def _maybe_add_wrapper(uid: str, name: str, folder=None) -> None: + if uid in seen_wrappers: + return + if name != base and not name_re.match(name): + return + ftype = getattr(folder, "type", "") if folder else "" + if folder is not None: + if ftype not in ( + BaseFolderNode.UserFolderType, + BaseFolderNode.NestedShareFolderType, + ): + return + elif not is_nested_share_folder(params, uid): + return + wrapper_uids.append(uid) + seen_wrappers.add(uid) + for root_uid in root_uids: root_folder = folders.get(root_uid) - if not root_folder: - continue - for child_uid in getattr(root_folder, "subfolders", []) or []: - child = folders.get(child_uid) - if not child or getattr(child, "type", "") != "user_folder": + if root_folder: + for child_uid in getattr(root_folder, "subfolders", []) or []: + child = folders.get(child_uid) + if not child: + continue + name = getattr(child, "name", "") or "" + _maybe_add_wrapper(child_uid, name, child) + + for child_uid, info in (getattr(params, "nested_share_folders", None) or {}).items(): + if (info.get("parent_uid") or None) != root_uid: continue - name = getattr(child, "name", "") or "" - if name == base or re.match(rf"^{re.escape(base)} #\d+$", name): - wrapper_uids.append(child.uid) + _maybe_add_wrapper(child_uid, info.get("name", "") or "") + return wrapper_uids From 35d2312d23546be8b7f05010047b94f643562b74 Mon Sep 17 00:00:00 2001 From: pvagare-ks Date: Mon, 20 Jul 2026 18:27:32 +0530 Subject: [PATCH 3/6] updated record loader --- .../commands/pam_import/record_loader.py | 15 ++++++--------- .../importer/cyberark/pam/idempotency.py | 6 +----- 2 files changed, 7 insertions(+), 14 deletions(-) diff --git a/keepercommander/commands/pam_import/record_loader.py b/keepercommander/commands/pam_import/record_loader.py index 4b3ac5bb0..306614798 100644 --- a/keepercommander/commands/pam_import/record_loader.py +++ b/keepercommander/commands/pam_import/record_loader.py @@ -21,20 +21,17 @@ def iter_accessible_record_uids(params) -> Iterator[str]: """Yield record UIDs from classic and Nested Shared Folder caches.""" seen = set() for attr in ('record_cache', 'nested_share_records'): - cache = getattr(params, attr, None) - if not isinstance(cache, dict): - continue + cache = getattr(params, attr, None) or {} for uid in cache: if uid not in seen: seen.add(uid) yield uid - nsf_record_data = getattr(params, 'nested_share_record_data', None) - if isinstance(nsf_record_data, dict): - for uid in nsf_record_data: - if uid not in seen: - seen.add(uid) - yield uid + nsf_record_data = getattr(params, 'nested_share_record_data', None) or {} + for uid in nsf_record_data: + if uid not in seen: + seen.add(uid) + yield uid def load_pam_record(params, record_uid: str) -> Optional[vault.KeeperRecord]: diff --git a/keepercommander/importer/cyberark/pam/idempotency.py b/keepercommander/importer/cyberark/pam/idempotency.py index 7245e09c4..1c7a7f5ac 100644 --- a/keepercommander/importer/cyberark/pam/idempotency.py +++ b/keepercommander/importer/cyberark/pam/idempotency.py @@ -208,11 +208,9 @@ def build_existing_index(params, folder_uids) -> ExistingRecordIndex: if not folder_cache and not getattr(params, "nested_share_folders", None): return index - # Prefer NSF-aware record lookup so Nested Share Folder projects are - # indexed the same way as classic shared-folder projects. try: from keepercommander.commands.pam_import.nsf_helpers import get_folder_record_uids - except ImportError: # pragma: no cover — defensive for alternate layouts + except ImportError: # pragma: no cover get_folder_record_uids = None # Recursively collect record UIDs from every subfolder. @@ -235,13 +233,11 @@ def build_existing_index(params, folder_uids) -> ExistingRecordIndex: folder = folder_cache.get(fuid) for sub_uid in (getattr(folder, "subfolders", []) or []) if folder else []: stack.append(sub_uid) - # NSF children may only be linked via nested_share_folders for child_uid, info in (getattr(params, "nested_share_folders", None) or {}).items(): if (info.get("parent_uid") or None) == fuid and child_uid not in visited: stack.append(child_uid) for ruid in all_record_uids: - # Prefer NSF-aware loader so Nested Share Folder records resolve. try: from keepercommander.commands.pam_import.record_loader import load_pam_record rec = load_pam_record(params, ruid) or vault.KeeperRecord.load(params, ruid) From 2380c21b823286c8363ca1fde8b234176d060368 Mon Sep 17 00:00:00 2001 From: pvagare-ks Date: Mon, 20 Jul 2026 18:38:32 +0530 Subject: [PATCH 4/6] fix cyberark cleanup for nsf --- .../commands/pam_import/cyberark_import.py | 81 ++++++++++++------- 1 file changed, 50 insertions(+), 31 deletions(-) diff --git a/keepercommander/commands/pam_import/cyberark_import.py b/keepercommander/commands/pam_import/cyberark_import.py index 6c8aba0ed..4faaaa396 100644 --- a/keepercommander/commands/pam_import/cyberark_import.py +++ b/keepercommander/commands/pam_import/cyberark_import.py @@ -2276,7 +2276,7 @@ def _interactive_safe_picker(safes: list[dict]) -> Optional[str]: class CyberArkPAMCleanupCommand(Command): - """Remove a CyberArk-imported project: records, folders, gateway, KSM app.""" + """Remove a CyberArk-imported project: records, folders, and PAM config.""" parser = argparse.ArgumentParser( prog="pam project cyberark-cleanup", @@ -2450,47 +2450,66 @@ def _collect_records_recursive(folder_uid: str): return if not auto_confirm: - answer = input("\n Delete all records and folders? [y/N]: ").strip().lower() + answer = input("\n Delete project folders (and all records inside)? [y/N]: ").strip().lower() if answer not in ("y", "yes"): print(" Cancelled.") return - from ..record_edit import RecordDeleteCommand + from ..folder import FolderRemoveCommand + from ..record import RecordRemoveCommand + from ..nested_share_folder.folder_commands import NestedShareFolderRemoveCommand from ..nested_share_folder.record_commands import NestedShareRecordRemoveCommand - deleted = 0 - failed = 0 - for rec_uid in list(all_record_uids): + # Prefer folder deletion: records (and nested subfolders) are removed + # with the folder tree. Delete content folders first, then wrappers. + folders_deleted = 0 + folders_failed = 0 + folders_to_remove = list(sf_uids) + list(project_folder_uids) + for folder_uid in folders_to_remove: try: - if is_pam_nsf_record(params, rec_uid) or any( - is_nested_share_folder(params, fuid) for fuid in sf_uids): - NestedShareRecordRemoveCommand().execute( - params, records=[rec_uid], force=True, operation="owner-trash") + if is_nested_share_folder(params, folder_uid): + NestedShareFolderRemoveCommand().execute( + params, folders=[folder_uid], force=True, + operation="folder-trash", quiet=True) else: - RecordDeleteCommand().execute(params, force=True, record=rec_uid) - deleted += 1 + FolderRemoveCommand().execute( + params, pattern=[folder_uid], force=True, quiet=True) + folders_deleted += 1 except Exception as e: - failed += 1 - logging.warning("Failed to delete record %s: %s", - rec_uid, type(e).__name__) - - # Delete PAM config record - try: - if is_pam_nsf_record(params, config_uid): - NestedShareRecordRemoveCommand().execute( - params, records=[config_uid], force=True, operation="owner-trash") - else: - RecordDeleteCommand().execute(params, force=True, record=config_uid) - deleted += 1 - except Exception as e: - failed += 1 - logging.warning("Failed to delete PAM config record %s: %s", - config_uid, type(e).__name__) + folders_failed += 1 + logging.warning("Failed to delete folder %s: %s", + folder_uid, type(e).__name__) api.sync_down(params) - msg = f"\nCleanup complete: {deleted} records deleted" - if failed: - msg += f" ({failed} failed — see warnings above)" + + # PAM config may already be gone with its Config folder; remove only + # if it survived (e.g. legacy layout / config outside project folders). + config_deleted = False + if config_uid and ( + config_uid in (params.record_cache or {}) + or config_uid in (getattr(params, "nested_share_records", None) or {}) + or vault.KeeperRecord.load(params, config_uid) + or load_pam_record(params, config_uid)): + try: + if is_pam_nsf_record(params, config_uid): + NestedShareRecordRemoveCommand().execute( + params, records=[config_uid], force=True, + operation="owner-trash") + else: + RecordRemoveCommand().execute( + params, force=True, record=config_uid) + config_deleted = True + api.sync_down(params) + except Exception as e: + folders_failed += 1 + logging.warning("Failed to delete PAM config record %s: %s", + config_uid, type(e).__name__) + + msg = f"\nCleanup complete: {folders_deleted} folders deleted" + if config_deleted: + msg += " (PAM config removed)" + if folders_failed: + msg += f" ({folders_failed} failed — see warnings above)" print(msg) print("=" * 50) From 90c8cb7d2f105c931099666960ff51d1efb00a9e Mon Sep 17 00:00:00 2001 From: pvagare-ks Date: Mon, 27 Jul 2026 23:59:30 +0530 Subject: [PATCH 5/6] resolve review comments --- keepercommander/commands/pam/vault_target.py | 25 ++- keepercommander/commands/pam_import/base.py | 18 +- .../commands/pam_import/cyberark_import.py | 58 +++---- keepercommander/commands/pam_import/edit.py | 155 +++++++++++++++-- keepercommander/commands/pam_import/extend.py | 20 +++ .../commands/pam_import/nsf_helpers.py | 59 +++++++ keepercommander/importer/cyberark/cyberark.py | 2 +- .../importer/cyberark/pam/client.py | 2 +- .../importer/cyberark/pam/idempotency.py | 20 +-- tests/test_cyberark_pam_import.py | 67 ++++--- unit-tests/pam/test_pam_nsf_folder_batch.py | 163 ++++++++++++++++++ 11 files changed, 472 insertions(+), 117 deletions(-) create mode 100644 unit-tests/pam/test_pam_nsf_folder_batch.py diff --git a/keepercommander/commands/pam/vault_target.py b/keepercommander/commands/pam/vault_target.py index 45d28e589..ca675278b 100644 --- a/keepercommander/commands/pam/vault_target.py +++ b/keepercommander/commands/pam/vault_target.py @@ -585,8 +585,13 @@ def update_pam_record(params, record, command='pam', force_nsf=False): record_management.update_record(params, record) -def execute_record_add_in_folder(params, args, folder_uid, command='pam'): - """Add a record in *folder_uid*, using NSF-native creation when needed.""" +def execute_record_add_in_folder(params, args, folder_uid, command='pam', + sync_after=True): + """Add a record in *folder_uid*, using NSF-native creation when needed. + + When *sync_after* is False, NSF callers can defer sync_down to a batch + boundary (avoids one sync per record during large PAM imports). + """ from ..record_edit import RecordAddCommand from ..nested_share_folder.record_commands import NestedShareRecordAddCommand @@ -596,7 +601,7 @@ def execute_record_add_in_folder(params, args, folder_uid, command='pam'): nsf_args.pop('folder', None) nsf_args['folder_uid'] = folder_uid uid = NestedShareRecordAddCommand().execute(params, **nsf_args) - if uid: + if uid and sync_after: from ..pam_import.nsf_helpers import sync_down_preserving_nsf_keys sync_down_preserving_nsf_keys(params) return uid @@ -605,8 +610,13 @@ def execute_record_add_in_folder(params, args, folder_uid, command='pam'): return RecordAddCommand().execute(params, **record_args) -def execute_record_v3_add_in_folder(params, args, folder_uid, command='pam'): - """Add a v3 typed record in *folder_uid*, using NSF-native creation when needed.""" +def execute_record_v3_add_in_folder(params, args, folder_uid, command='pam', + sync_after=True): + """Add a v3 typed record in *folder_uid*, using NSF-native creation when needed. + + When *sync_after* is False, NSF callers can defer sync_down to a batch + boundary (avoids one sync per record during large PAM imports). + """ import json from ..recordv3 import RecordAddCommand @@ -629,8 +639,9 @@ def execute_record_v3_add_in_folder(params, args, folder_uid, command='pam'): if not result.get('success'): raise CommandError(command, normalize_nsf_user_message(result.get('message')) or 'Failed to create record in Nested Share Folder') - from ..pam_import.nsf_helpers import sync_down_preserving_nsf_keys - sync_down_preserving_nsf_keys(params) + if sync_after: + from ..pam_import.nsf_helpers import sync_down_preserving_nsf_keys + sync_down_preserving_nsf_keys(params) return result['record_uid'] record_args['folder'] = folder_uid diff --git a/keepercommander/commands/pam_import/base.py b/keepercommander/commands/pam_import/base.py index 412833c08..0359b4bee 100644 --- a/keepercommander/commands/pam_import/base.py +++ b/keepercommander/commands/pam_import/base.py @@ -1070,7 +1070,8 @@ def create_record(self, params, folder_uid): fields.append(f"file=@{x.file}") if fields: args["fields"] = fields - uid = execute_record_add_in_folder(params, args, folder_uid, command='pam-project-import') + uid = execute_record_add_in_folder( + params, args, folder_uid, command='pam-project-import', sync_after=False) if uid and isinstance(uid, str): self.uid = uid @@ -1165,7 +1166,8 @@ def create_record(self, params, folder_uid): fields.append(f"file=@{x.file}") if fields: args["fields"] = fields - uid = execute_record_add_in_folder(params, args, folder_uid, command='pam-project-import') + uid = execute_record_add_in_folder( + params, args, folder_uid, command='pam-project-import', sync_after=False) if uid and isinstance(uid, str): self.uid = uid return uid @@ -1435,7 +1437,8 @@ def create_record(self, params, folder_uid): # switch to f.* once RT definition(s) update w/ pamSettings field if fields: args["fields"] = fields - uid = execute_record_add_in_folder(params, args, folder_uid, command='pam-project-import') + uid = execute_record_add_in_folder( + params, args, folder_uid, command='pam-project-import', sync_after=False) if uid and isinstance(uid, str): self.uid = uid @@ -1628,7 +1631,8 @@ def create_record(self, params, folder_uid): # switch to f.* once RT definition(s) update w/ pamSettings field if fields: args["fields"] = fields - uid = execute_record_add_in_folder(params, args, folder_uid, command='pam-project-import') + uid = execute_record_add_in_folder( + params, args, folder_uid, command='pam-project-import', sync_after=False) if uid and isinstance(uid, str): self.uid = uid @@ -1776,7 +1780,8 @@ def create_record(self, params, folder_uid): # switch to f.* once RT definition(s) update w/ pamSettings field if fields: args["fields"] = fields - uid = execute_record_add_in_folder(params, args, folder_uid, command='pam-project-import') + uid = execute_record_add_in_folder( + params, args, folder_uid, command='pam-project-import', sync_after=False) if uid and isinstance(uid, str): self.uid = uid @@ -1881,7 +1886,8 @@ def create_record(self, params, folder_uid): # switch to f.* once RT definition(s) update w/ pamRemoteBrowserSettings field if fields: args["fields"] = fields - uid = execute_record_add_in_folder(params, args, folder_uid, command='pam-project-import') + uid = execute_record_add_in_folder( + params, args, folder_uid, command='pam-project-import', sync_after=False) if uid and isinstance(uid, str): self.uid = uid diff --git a/keepercommander/commands/pam_import/cyberark_import.py b/keepercommander/commands/pam_import/cyberark_import.py index 4faaaa396..cf8800b0f 100644 --- a/keepercommander/commands/pam_import/cyberark_import.py +++ b/keepercommander/commands/pam_import/cyberark_import.py @@ -30,6 +30,8 @@ from prompt_toolkit import HTML, print_formatted_text from ..base import Command +from ..pam.vault_target import is_nested_share_folder, is_pam_nsf_record +from ... import api, vault, vault_extensions from ...display import bcolors from ...error import CommandError from ...importer.cyberark.cyberark_pam import ( @@ -75,6 +77,18 @@ strip_id_marker, summarize, ) +from .nsf_helpers import find_pam_configuration, get_folder_record_uids +from .record_loader import iter_accessible_record_uids, load_pam_record + + +def is_matching_title(title: str, base: str) -> bool: + """True when *title* equals *base* or is ``{base} #N`` (case-insensitive).""" + title = title.casefold() + if title == base: + return True + if not title.startswith(base): + return False + return bool(re.fullmatch(r" #\d+", title[len(base):])) class SecureTempFileStore: @@ -1145,7 +1159,7 @@ def _prepare_idempotency(self, mapped: MappedImportResult) -> Optional[dict]: # wrapper folder(s). Records live inside these (and inside # their Resources / Users subfolders for the safe-per-folder # layout). - shared_folder_uids: List[str] = [] + shared_folder_uids: list[str] = [] seen: set = set() for wrapper_uid in wrapper_uids: for child_uid, _name in CyberArkPAMCleanupCommand._iter_project_child_folders( @@ -2170,18 +2184,13 @@ def _find_config_uid(self, params, project_name: str) -> str: Handles #N suffix deduplication from PAMProjectImportCommand. Searches classic vault records and Nested Share Folder caches. """ - from ... import api, vault_extensions - from .nsf_helpers import find_pam_configuration - from .record_loader import iter_accessible_record_uids, load_pam_record - api.sync_down(params) config_base = f"{project_name} Configuration".casefold() candidates = [] # Classic path (keeps existing unit-test mocks working) for c in vault_extensions.find_records(params, record_version=6): - t = c.title.casefold() - if t == config_base or (t.startswith(config_base) and re.match(r' #\d+$', t[len(config_base):])): + if is_matching_title(getattr(c, "title", "") or "", config_base): candidates.append(c) # NSF / full-access path — pick up configs that live only in NSF caches @@ -2190,13 +2199,10 @@ def _find_config_uid(self, params, project_name: str) -> str: if uid in seen_uids: continue rec = load_pam_record(params, uid) - if not rec or getattr(rec, "version", None) != 6: - continue - title = getattr(rec, "title", "") or "" - t = title.casefold() - if t == config_base or (t.startswith(config_base) and re.match(r' #\d+$', t[len(config_base):])): - candidates.append(rec) - seen_uids.add(uid) + if rec and getattr(rec, "version", None) == 6: + if is_matching_title(getattr(rec, "title", "") or "", config_base): + candidates.append(rec) + seen_uids.add(uid) if not candidates: exact = find_pam_configuration(params, f"{project_name} Configuration") @@ -2314,14 +2320,9 @@ def execute(self, params, **kwargs): raise CommandError("pam project cyberark-cleanup", "Either --name or --config is required") - from ... import api, vault, vault_extensions - api.sync_down(params) # Find PAM config by name or UID (classic + NSF) - from .nsf_helpers import find_pam_configuration - from .record_loader import iter_accessible_record_uids, load_pam_record - if config_uid: config_rec = vault.KeeperRecord.load(params, config_uid) if not config_rec: @@ -2334,20 +2335,18 @@ def execute(self, params, **kwargs): config_base = f"{project_name} Configuration".casefold() config_rec = None for c in vault_extensions.find_records(params, record_version=6): - if c.title.casefold().startswith(config_base): + if is_matching_title(getattr(c, "title", "") or "", config_base): config_rec = c config_uid = c.record_uid break if not config_rec: for uid in iter_accessible_record_uids(params): rec = load_pam_record(params, uid) - if not rec or getattr(rec, "version", None) != 6: - continue - title = getattr(rec, "title", "") or "" - if title.casefold().startswith(config_base): - config_rec = rec - config_uid = uid - break + if rec and getattr(rec, "version", None) == 6: + if is_matching_title(getattr(rec, "title", "") or "", config_base): + config_rec = rec + config_uid = uid + break if not config_rec: exact = find_pam_configuration(params, f"{project_name} Configuration") if exact: @@ -2367,9 +2366,6 @@ def execute(self, params, **kwargs): # wrapper under PAM Environments so cleanup handles # both classic shared folders and Nested Share Folders # (and any subset thereof) without hardcoding names. - from .nsf_helpers import get_folder_record_uids - from ..pam.vault_target import is_nested_share_folder, is_pam_nsf_record - sf_uids = [] record_count = 0 sf_names: list = [] @@ -2518,7 +2514,6 @@ def _collect_records_recursive(folder_uid: str): @classmethod def _is_project_content_folder(cls, params, folder_uid: str, folder=None) -> bool: """True for classic shared folders or Nested Share Folders under a project.""" - from ..pam.vault_target import is_nested_share_folder from ...subfolder import BaseFolderNode if is_nested_share_folder(params, folder_uid): @@ -2575,7 +2570,6 @@ def _find_project_wrapper_folder_uids(cls, params, project_name: str) -> list: correct in the rare case where two projects share a name (PAMProjectImportCommand allows duplicates via the ``#N`` suffix). """ - from ..pam.vault_target import is_nested_share_folder from ...subfolder import BaseFolderNode wrapper_uids: list = [] diff --git a/keepercommander/commands/pam_import/edit.py b/keepercommander/commands/pam_import/edit.py index 0087b1ae9..b645e0e3a 100644 --- a/keepercommander/commands/pam_import/edit.py +++ b/keepercommander/commands/pam_import/edit.py @@ -444,10 +444,22 @@ def _create_safe_folders(self, params, project: dict, project_folder_uid: str, # record. Cannot live in any safe folder, or the safe's members # would gain access to the central config record. config_folder_name = f"""{res["project_folder"]} - Config""" + + # Verify principals once (avoids one round-trip per safe). + if all_user_perms: + self.verify_users_and_teams(params, all_user_perms) + + if use_nsf: + self._create_safe_folders_nsf_batch( + params, project_folder_uid, res, safe_folder_records, + config_folder_name, safe_folder_map, + ) + return + config_uid = self.create_subfolder( params, folder_name=config_folder_name, parent_uid=project_folder_uid, permissions=dict(default_fperm), - use_nsf=use_nsf, + use_nsf=False, ) res["resources_folder"] = config_folder_name res["users_folder"] = config_folder_name @@ -456,10 +468,6 @@ def _create_safe_folders(self, params, project: dict, project_folder_uid: str, res["config_folder_uid"] = config_uid res["config_folder"] = config_folder_name - # Verify principals once (avoids one round-trip per safe). - if all_user_perms: - self.verify_users_and_teams(params, all_user_perms) - # Create one shared folder per safe under the project wrapper and # apply its specific permission set. Inside each safe folder, # create two organizational subfolders named @@ -476,7 +484,7 @@ def _create_safe_folders(self, params, project: dict, project_folder_uid: str, folder_uid = self.create_subfolder( params, folder_name=record["name"], parent_uid=project_folder_uid, permissions=record["fperm"], - use_nsf=use_nsf, + use_nsf=False, ) # Top-level lookup key (no slash) maps to the safe folder # itself for callers that still emit ``folder_path = ""`` @@ -487,11 +495,11 @@ def _create_safe_folders(self, params, project: dict, project_folder_uid: str, usr_sub_name = f"{record['name']} - Users" res_sub_uid = self.create_subfolder( params, folder_name=res_sub_name, parent_uid=folder_uid, - use_nsf=use_nsf, + use_nsf=False, ) usr_sub_uid = self.create_subfolder( params, folder_name=usr_sub_name, parent_uid=folder_uid, - use_nsf=use_nsf, + use_nsf=False, ) safe_folder_map[f"{record['name']}/{res_sub_name}"] = res_sub_uid safe_folder_map[f"{record['name']}/{usr_sub_name}"] = usr_sub_uid @@ -510,6 +518,101 @@ def _create_safe_folders(self, params, project: dict, project_folder_uid: str, res["safe_folder_map"] = safe_folder_map + def _create_safe_folders_nsf_batch(self, params, project_folder_uid: str, + res: dict, safe_folder_records: list, + config_folder_name: str, + safe_folder_map: dict) -> None: + """Create Config + per-safe NSF folders via batched folder_add_v3. + + Two layers (parents must exist before children): + 1. Config folder + each safe folder under the project + 2. ``{safe} - Resources`` / ``{safe} - Users`` under each safe + """ + from .nsf_helpers import create_nsf_folders_batch + + # Layer 1: config + safe wrappers (same parent) + layer1_specs = [ + {"name": config_folder_name, "parent_uid": project_folder_uid}, + ] + for record in safe_folder_records: + layer1_specs.append({ + "name": record["name"], + "parent_uid": project_folder_uid, + }) + + layer1 = create_nsf_folders_batch( + params, layer1_specs, sync=False, command='pam-project-import', + ) + config_uid = layer1[0]["folder_uid"] + res["resources_folder"] = config_folder_name + res["users_folder"] = config_folder_name + res["resources_folder_uid"] = config_uid + res["users_folder_uid"] = config_uid + res["config_folder_uid"] = config_uid + res["config_folder"] = config_folder_name + + safe_uids: list = [] + for i, record in enumerate(safe_folder_records): + folder_uid = layer1[i + 1]["folder_uid"] + safe_uids.append(folder_uid) + safe_folder_map[record["name"]] = folder_uid + + # Layer 2: Resources / Users children under each safe + layer2_specs: list = [] + layer2_meta: list = [] # (safe_index, kind, sub_name) + for i, record in enumerate(safe_folder_records): + res_sub_name = f"{record['name']} - Resources" + usr_sub_name = f"{record['name']} - Users" + parent_uid = safe_uids[i] + layer2_specs.append({"name": res_sub_name, "parent_uid": parent_uid}) + layer2_meta.append((i, "resources", res_sub_name)) + layer2_specs.append({"name": usr_sub_name, "parent_uid": parent_uid}) + layer2_meta.append((i, "users", usr_sub_name)) + + layer2 = [] + if layer2_specs: + layer2 = create_nsf_folders_batch( + params, layer2_specs, sync=True, command='pam-project-import', + ) + else: + # Config-only project: still need one sync after layer 1. + from .nsf_helpers import sync_down_preserving_nsf_keys + sync_down_preserving_nsf_keys(params) + + # Assemble maps / safe_folders entries + by_safe: dict = { + i: {"resources_uid": None, "users_uid": None, + "resources_name": None, "users_name": None} + for i in range(len(safe_folder_records)) + } + for result, (safe_i, kind, sub_name) in zip(layer2, layer2_meta): + uid = result["folder_uid"] + parent_name = safe_folder_records[safe_i]["name"] + safe_folder_map[f"{parent_name}/{sub_name}"] = uid + if kind == "resources": + by_safe[safe_i]["resources_uid"] = uid + by_safe[safe_i]["resources_name"] = sub_name + else: + by_safe[safe_i]["users_uid"] = uid + by_safe[safe_i]["users_name"] = sub_name + + for i, record in enumerate(safe_folder_records): + folder_uid = safe_uids[i] + info = by_safe[i] + res["safe_folders"].append({ + "name": record["name"], + "safe_name": record["safe_name"], + "uid": folder_uid, + "resources_subfolder": info["resources_name"], + "resources_subfolder_uid": info["resources_uid"], + "users_subfolder": info["users_name"], + "users_subfolder_uid": info["users_uid"], + }) + if record["uperm"]: + self.add_folder_permissions(params, folder_uid, record["uperm"]) + + res["safe_folder_map"] = safe_folder_map + def process_ksm_app(self, params, project: dict) -> dict: res = { "app_name_target": "", @@ -1019,17 +1122,14 @@ def create_subfolder(self, params, folder_name:str, parent_uid:str="", permissio name = str(folder_name or "").strip() if use_nsf or is_nested_share_folder(params, parent_uid): - from .nsf_helpers import seed_nsf_folder_cache, sync_down_preserving_nsf_keys - from ...nested_share_folder.folder_api import create_folder_v3 - result = create_folder_v3(params, name, parent_uid=parent_uid or None) - if isinstance(result, dict) and result.get('success') is False: - raise CommandError("pam", result.get('message') or 'Failed to create Nested Share Folder') - folder_uid = result.get('folder_uid') if isinstance(result, dict) else None - if not folder_uid: - raise CommandError("pam", f'Nested Share Folder creation did not return UID: {name}') - folder_key = result.get('folder_key_unencrypted') if isinstance(result, dict) else None - seed_nsf_folder_cache(params, folder_uid, name, parent_uid or None, folder_key) - sync_down_preserving_nsf_keys(params) + from .nsf_helpers import create_nsf_folders_batch + results = create_nsf_folders_batch( + params, + [{"name": name, "parent_uid": parent_uid or None}], + sync=True, + command='pam', + ) + folder_uid = results[0]["folder_uid"] params.environment_variables[LAST_FOLDER_UID] = folder_uid return folder_uid @@ -1489,6 +1589,9 @@ def _resolve_folder_uid(obj, default_uid: str) -> str: logging.warning(f"Processing external users: {len(users)}") for n, user in enumerate(users): # standalone users user.create_record(params, _resolve_folder_uid(user, shfusr)) + if user.uid and user.uid not in (getattr(params, 'record_cache', None) or {}): + from .nsf_helpers import sync_down_preserving_nsf_keys + sync_down_preserving_nsf_keys(params) if n % pdelta == 0: print(f"{n}/{len(users)}") print(f"{len(users)}/{len(users)}\n") @@ -1502,6 +1605,11 @@ def _resolve_folder_uid(obj, default_uid: str) -> str: admin_uid = get_admin_credential(mach, True) mach_folder_uid = _resolve_folder_uid(mach, shfres) mach.create_record(params, mach_folder_uid) + # NSF creates with sync_after=False; pam tunnel edit resolves from + # record_cache, so sync once when the new UID is not loaded yet. + if mach.uid and mach.uid not in (getattr(params, 'record_cache', None) or {}): + from .nsf_helpers import sync_down_preserving_nsf_keys + sync_down_preserving_nsf_keys(params) tdag.link_resource_to_config(mach.uid) if isinstance(mach, PamRemoteBrowserObject): # RBI args = parse_command_options(mach, True) @@ -1574,6 +1682,9 @@ def _resolve_folder_uid(obj, default_uid: str) -> str: # to every record originating from that CyberArk safe. user_folder_uid = _resolve_folder_uid(user, mach_folder_uid) user.create_record(params, user_folder_uid) + if user.uid and user.uid not in (getattr(params, 'record_cache', None) or {}): + from .nsf_helpers import sync_down_preserving_nsf_keys + sync_down_preserving_nsf_keys(params) if isinstance(user, PamUserObject): # rotation setup tdag.link_user_to_resource(user.uid, mach.uid, admin_uid==user.uid, True) if user.rotation_settings: @@ -1665,4 +1776,10 @@ def _resolve_folder_uid(obj, default_uid: str) -> str: else: logging.debug(f"Unable to resolve domain admin '{pce.dom_administrative_credential}' for PAM Domain configuration.") + # One sync after bulk NSF record creates (create_record uses sync_after=False). + use_nsf = project["options"].get("use_nsf", False) is True + if use_nsf or is_nested_share_folder(params, shfres) or is_nested_share_folder(params, shfusr): + from .nsf_helpers import sync_down_preserving_nsf_keys + sync_down_preserving_nsf_keys(params) + logging.debug("Done processing project data.") diff --git a/keepercommander/commands/pam_import/extend.py b/keepercommander/commands/pam_import/extend.py index b2dfa371f..acf77dcae 100644 --- a/keepercommander/commands/pam_import/extend.py +++ b/keepercommander/commands/pam_import/extend.py @@ -1396,6 +1396,9 @@ def process_data(self, params, project): for n, user in enumerate(new_users): folder_uid = getattr(user, "resolved_folder_uid", None) or shfusr extend_create_record(params, user, folder_uid) + if user.uid and user.uid not in (getattr(params, 'record_cache', None) or {}): + from .nsf_helpers import sync_down_preserving_nsf_keys + sync_down_preserving_nsf_keys(params) if n % pdelta == 0: print(f"{n}/{len(new_users)}") print(f"{len(new_users)}/{len(new_users)}\n") @@ -1411,6 +1414,11 @@ def process_data(self, params, project): folder_uid = getattr(mach, "resolved_folder_uid", None) or shfres admin_uid = get_admin_credential(mach, True) extend_create_record(params, mach, folder_uid) + # NSF creates with sync_after=False; pam tunnel edit resolves from + # record_cache / NSF caches, so sync when the new UID is not loaded yet. + if mach.uid and mach.uid not in (getattr(params, 'record_cache', None) or {}): + from .nsf_helpers import sync_down_preserving_nsf_keys + sync_down_preserving_nsf_keys(params) tdag.link_resource_to_config(mach.uid) if isinstance(mach, PamRemoteBrowserObject): args = parse_command_options(mach, True) @@ -1478,6 +1486,9 @@ def process_data(self, params, project): rs.resourceUid = mach.uid ufolder = getattr(user, "resolved_folder_uid", None) or shfusr extend_create_record(params, user, ufolder) + if user.uid and user.uid not in (getattr(params, 'record_cache', None) or {}): + from .nsf_helpers import sync_down_preserving_nsf_keys + sync_down_preserving_nsf_keys(params) if isinstance(user, PamUserObject): tdag.link_user_to_resource(user.uid, mach.uid, admin_uid == user.uid, True) if rs: @@ -1516,6 +1527,9 @@ def process_data(self, params, project): rs.resourceUid = mach.uid ufolder = getattr(user, "resolved_folder_uid", None) or shfusr extend_create_record(params, user, ufolder) + if user.uid and user.uid not in (getattr(params, 'record_cache', None) or {}): + from .nsf_helpers import sync_down_preserving_nsf_keys + sync_down_preserving_nsf_keys(params) if isinstance(user, PamUserObject): tdag.link_user_to_resource(user.uid, mach.uid, admin_uid == user.uid, True) if rs: @@ -1598,5 +1612,11 @@ def process_data(self, params, project): if refs: api.sync_down(params) add_pam_scripts(params, pam_cfg_uid, refs) + + # One sync after bulk NSF record creates (create_record uses sync_after=False). + if is_nested_share_folder(params, shfres) or is_nested_share_folder(params, shfusr): + from .nsf_helpers import sync_down_preserving_nsf_keys + sync_down_preserving_nsf_keys(params) + logging.debug("Done processing project data.") return diff --git a/keepercommander/commands/pam_import/nsf_helpers.py b/keepercommander/commands/pam_import/nsf_helpers.py index 48963cf58..dd78df7f3 100644 --- a/keepercommander/commands/pam_import/nsf_helpers.py +++ b/keepercommander/commands/pam_import/nsf_helpers.py @@ -306,6 +306,65 @@ def create_nsf_subfolder(params, folder_name: str, parent_uid: str = '', return folder_uid +_NSF_FOLDER_BATCH_LIMIT = 100 + + +def create_nsf_folders_batch(params, folder_specs: List[dict], *, + sync: bool = True, + command: str = 'pam') -> List[dict]: + """Create NSF folders via ``vault/folders/v3/add`` in chunks of 100. + + Each *folder_specs* entry is ``{'name': str, 'parent_uid': str|None}``. + Returns the batch API result list (same order as *folder_specs*). + Seeds local NSF caches after each successful create; optionally syncs once + at the end so callers avoid one round-trip per folder. + """ + from ...nested_share_folder.folder_api import create_folders_batch_v3 + + if not folder_specs: + return [] + + results: List[dict] = [] + for start in range(0, len(folder_specs), _NSF_FOLDER_BATCH_LIMIT): + chunk = folder_specs[start:start + _NSF_FOLDER_BATCH_LIMIT] + try: + chunk_results = create_folders_batch_v3(params, chunk) + except Exception as exc: + raise CommandError(command, f'NSF folder batch create failed: {exc}') from exc + + if len(chunk_results) != len(chunk): + raise CommandError( + command, + f'NSF folder batch returned {len(chunk_results)} results for {len(chunk)} folders', + ) + + for spec, result in zip(chunk, chunk_results): + if not result.get('success'): + name = spec.get('name') or '?' + raise CommandError( + command, + result.get('message') or f'Failed to create Nested Share Folder: {name}', + ) + folder_uid = result.get('folder_uid') + if not folder_uid: + raise CommandError( + command, + f"Nested Share Folder creation did not return UID: {spec.get('name')}", + ) + seed_nsf_folder_cache( + params, + folder_uid, + spec.get('name') or '', + spec.get('parent_uid') or None, + result.get('folder_key_unencrypted'), + ) + results.append(result) + + if sync: + sync_down_preserving_nsf_keys(params) + return results + + def extend_create_record(params, obj, folder_uid: str) -> Optional[str]: """Create a PAM import record in a classic or NSF folder.""" return obj.create_record(params, folder_uid) diff --git a/keepercommander/importer/cyberark/cyberark.py b/keepercommander/importer/cyberark/cyberark.py index 2620d17ce..55415d2ba 100644 --- a/keepercommander/importer/cyberark/cyberark.py +++ b/keepercommander/importer/cyberark/cyberark.py @@ -1133,7 +1133,7 @@ def _do_import_inner(self, filename, **kwargs): "Authorization": authorization_token, "Content-Type": "application/json", }, - json={"reason": "Keeper Commander Import"}, + json={"reason": "test"}, timeout=self.TIMEOUT, verify=True if pvwa_host.endswith(".cyberark.cloud") else self._verify_tls, cert=None if pvwa_host.endswith(".cyberark.cloud") else self._client_cert, diff --git a/keepercommander/importer/cyberark/pam/client.py b/keepercommander/importer/cyberark/pam/client.py index e8c982834..5c930bc1f 100644 --- a/keepercommander/importer/cyberark/pam/client.py +++ b/keepercommander/importer/cyberark/pam/client.py @@ -1206,7 +1206,7 @@ def retrieve_password(self, account_id: str, account_name: str = "", self._get_url("account_password").format(account_id=account_id), headers={"Authorization": self.auth_token, "Content-Type": "application/json"}, json={ - "reason": "Keeper Commander Import", + "reason": "test", **({"TicketingSystemName": environ["KEEPER_CYBERARK_TICKETING_SYSTEM"]} if "KEEPER_CYBERARK_TICKETING_SYSTEM" in environ else {}), **({"TicketId": environ["KEEPER_CYBERARK_TICKET_ID"]} diff --git a/keepercommander/importer/cyberark/pam/idempotency.py b/keepercommander/importer/cyberark/pam/idempotency.py index 1c7a7f5ac..5097e8232 100644 --- a/keepercommander/importer/cyberark/pam/idempotency.py +++ b/keepercommander/importer/cyberark/pam/idempotency.py @@ -44,6 +44,9 @@ from enum import Enum from typing import Any, Dict, List, Optional, Set, Tuple +from keepercommander.commands.pam_import.nsf_helpers import get_folder_record_uids +from keepercommander.commands.pam_import.record_loader import load_pam_record + # --------------------------------------------------------------------------- # Notes marker @@ -203,16 +206,10 @@ def build_existing_index(params, folder_uids) -> ExistingRecordIndex: # are populated by ``api.sync_down``, but a fresh session without a # sync will have them as ``None`` or empty dicts. Bail early so the # importer falls back to always-create mode instead of crashing. - subfolder_record_cache = getattr(params, "subfolder_record_cache", None) or {} folder_cache = getattr(params, "folder_cache", None) or {} if not folder_cache and not getattr(params, "nested_share_folders", None): return index - try: - from keepercommander.commands.pam_import.nsf_helpers import get_folder_record_uids - except ImportError: # pragma: no cover - get_folder_record_uids = None - # Recursively collect record UIDs from every subfolder. stack = list(folder_uids) visited: Set[str] = set() @@ -223,10 +220,7 @@ def build_existing_index(params, folder_uids) -> ExistingRecordIndex: continue visited.add(fuid) index.scanned_folder_uids.add(fuid) - if get_folder_record_uids is not None: - record_uids = get_folder_record_uids(params, fuid) - else: - record_uids = subfolder_record_cache.get(fuid) or set() + record_uids = get_folder_record_uids(params, fuid) for ruid in record_uids: all_record_uids.add(ruid) index.folder_by_record[ruid] = fuid @@ -238,11 +232,7 @@ def build_existing_index(params, folder_uids) -> ExistingRecordIndex: stack.append(child_uid) for ruid in all_record_uids: - try: - from keepercommander.commands.pam_import.record_loader import load_pam_record - rec = load_pam_record(params, ruid) or vault.KeeperRecord.load(params, ruid) - except ImportError: # pragma: no cover - rec = vault.KeeperRecord.load(params, ruid) + rec = load_pam_record(params, ruid) or vault.KeeperRecord.load(params, ruid) if rec is None: continue rtype = getattr(rec, "record_type", "") or "" diff --git a/tests/test_cyberark_pam_import.py b/tests/test_cyberark_pam_import.py index 047cb9fc2..0197c67aa 100644 --- a/tests/test_cyberark_pam_import.py +++ b/tests/test_cyberark_pam_import.py @@ -10,9 +10,16 @@ import json import os -import pytest +from types import SimpleNamespace from unittest.mock import MagicMock, patch +import pytest + +from keepercommander.commands.pam_import.cyberark_import import ( + CyberArkPAMCleanupCommand, + CyberArkPAMImportCommand, + _temp_store, +) from keepercommander.importer.cyberark.cyberark_pam import ( AccountMapper, AdaptiveThrottler, @@ -33,7 +40,12 @@ strip_credentials, DEFAULT_PLATFORM_MAP, ) -from keepercommander.commands.pam_import.cyberark_import import CyberArkPAMImportCommand +from keepercommander.subfolder import BaseFolderNode, NestedShareFolderNode + + +PAM_ROOT_FOLDER_NAME = "PAM Environments" +DEFAULT_PROJECT_NAME = "CyberArk Migration" +SAFE_FOLDER_NAME = "Win_Local" # ── AccountMapper Tests ────────────────────────────────────── @@ -860,8 +872,8 @@ def _make_mock_record(self, title, uid): record.record_uid = uid return record - @patch('keepercommander.vault_extensions') - @patch('keepercommander.api.sync_down') + @patch('keepercommander.commands.pam_import.cyberark_import.vault_extensions') + @patch('keepercommander.commands.pam_import.cyberark_import.api.sync_down') def test_exact_match(self, mock_sync, mock_ve): mock_ve.find_records.return_value = [ self._make_mock_record("MyProject Configuration", "uid-001"), @@ -870,8 +882,8 @@ def test_exact_match(self, mock_sync, mock_ve): result = cmd._find_config_uid(MagicMock(), "MyProject") assert result == "uid-001" - @patch('keepercommander.vault_extensions') - @patch('keepercommander.api.sync_down') + @patch('keepercommander.commands.pam_import.cyberark_import.vault_extensions') + @patch('keepercommander.commands.pam_import.cyberark_import.api.sync_down') def test_suffix_picks_highest_numerically(self, mock_sync, mock_ve): """#10 should sort after #9 (numeric, not lexicographic).""" mock_ve.find_records.return_value = [ @@ -883,8 +895,8 @@ def test_suffix_picks_highest_numerically(self, mock_sync, mock_ve): result = cmd._find_config_uid(MagicMock(), "MyProject") assert result == "uid-010" - @patch('keepercommander.vault_extensions') - @patch('keepercommander.api.sync_down') + @patch('keepercommander.commands.pam_import.cyberark_import.vault_extensions') + @patch('keepercommander.commands.pam_import.cyberark_import.api.sync_down') def test_no_match_returns_empty(self, mock_sync, mock_ve): mock_ve.find_records.return_value = [ self._make_mock_record("OtherProject Configuration", "uid-999"), @@ -893,8 +905,8 @@ def test_no_match_returns_empty(self, mock_sync, mock_ve): result = cmd._find_config_uid(MagicMock(), "MyProject") assert result == "" - @patch('keepercommander.vault_extensions') - @patch('keepercommander.api.sync_down') + @patch('keepercommander.commands.pam_import.cyberark_import.vault_extensions') + @patch('keepercommander.commands.pam_import.cyberark_import.api.sync_down') def test_rejects_partial_match(self, mock_sync, mock_ve): mock_ve.find_records.return_value = [ self._make_mock_record("MyProject Configuration Extra", "uid-bad"), @@ -2635,14 +2647,12 @@ class TestCleanupCommand: """Tests for CyberArkPAMCleanupCommand.""" def test_missing_args_raises(self): - from keepercommander.commands.pam_import.cyberark_import import CyberArkPAMCleanupCommand from keepercommander.error import CommandError cmd = CyberArkPAMCleanupCommand() with pytest.raises(CommandError): cmd.execute(MagicMock(), project_name="", config_uid="") def test_parser_has_flags(self): - from keepercommander.commands.pam_import.cyberark_import import CyberArkPAMCleanupCommand cmd = CyberArkPAMCleanupCommand() args = cmd.parser.parse_args(["--name", "Test", "--dry-run", "--yes"]) assert args.project_name == "Test" @@ -2650,7 +2660,6 @@ def test_parser_has_flags(self): assert args.auto_confirm is True def test_parser_config_flag(self): - from keepercommander.commands.pam_import.cyberark_import import CyberArkPAMCleanupCommand cmd = CyberArkPAMCleanupCommand() args = cmd.parser.parse_args(["--config", "uid123"]) assert args.config_uid == "uid123" @@ -2660,9 +2669,6 @@ class TestCyberArkImportNsfSupport: """NSF (--nsf) wiring for cyberark-import / cleanup / discovery.""" def test_single_batch_passes_use_nsf_to_import(self): - from keepercommander.commands.pam_import.cyberark_import import ( - CyberArkPAMImportCommand, _temp_store, - ) cmd = CyberArkPAMImportCommand() params = MagicMock() import_data = {"pam_data": {"resources": [], "users": []}} @@ -2682,9 +2688,6 @@ def test_single_batch_passes_use_nsf_to_import(self): assert kwargs.get("project_name") == "Proj" def test_single_batch_extend_ignores_use_nsf_flag(self): - from keepercommander.commands.pam_import.cyberark_import import ( - CyberArkPAMImportCommand, _temp_store, - ) cmd = CyberArkPAMImportCommand() params = MagicMock() import_data = {"pam_data": {"resources": [], "users": []}} @@ -2703,25 +2706,21 @@ def test_single_batch_extend_ignores_use_nsf_flag(self): assert kwargs.get("config") == "existing-cfg" def test_find_nsf_project_wrapper_uids(self): - from types import SimpleNamespace - from keepercommander.commands.pam_import.cyberark_import import CyberArkPAMCleanupCommand - from keepercommander.subfolder import NestedShareFolderNode - root = NestedShareFolderNode() root.uid = "nsf-root" - root.name = "PAM Environments" + root.name = PAM_ROOT_FOLDER_NAME root.parent_uid = None root.subfolders = ["nsf-proj"] proj = NestedShareFolderNode() proj.uid = "nsf-proj" - proj.name = "CyberArk Migration" + proj.name = DEFAULT_PROJECT_NAME proj.parent_uid = "nsf-root" proj.subfolders = ["nsf-safe"] safe = NestedShareFolderNode() safe.uid = "nsf-safe" - safe.name = "Win_Local" + safe.name = SAFE_FOLDER_NAME safe.parent_uid = "nsf-proj" safe.subfolders = [] @@ -2732,9 +2731,9 @@ def test_find_nsf_project_wrapper_uids(self): "nsf-safe": safe, }, nested_share_folders={ - "nsf-root": {"name": "PAM Environments", "parent_uid": None}, - "nsf-proj": {"name": "CyberArk Migration", "parent_uid": "nsf-root"}, - "nsf-safe": {"name": "Win_Local", "parent_uid": "nsf-proj"}, + "nsf-root": {"name": PAM_ROOT_FOLDER_NAME, "parent_uid": None}, + "nsf-proj": {"name": DEFAULT_PROJECT_NAME, "parent_uid": "nsf-root"}, + "nsf-safe": {"name": SAFE_FOLDER_NAME, "parent_uid": "nsf-proj"}, }, shared_folder_cache={}, subfolder_record_cache={}, @@ -2742,22 +2741,18 @@ def test_find_nsf_project_wrapper_uids(self): ) wrappers = CyberArkPAMCleanupCommand._find_project_wrapper_folder_uids( - params, "CyberArk Migration", + params, DEFAULT_PROJECT_NAME, ) assert wrappers == ["nsf-proj"] children = list(CyberArkPAMCleanupCommand._iter_project_child_folders( params, "nsf-proj", )) - assert children == [("nsf-safe", "Win_Local")] + assert children == [("nsf-safe", SAFE_FOLDER_NAME)] def test_find_classic_wrapper_still_works(self): - from types import SimpleNamespace - from keepercommander.commands.pam_import.cyberark_import import CyberArkPAMCleanupCommand - from keepercommander.subfolder import BaseFolderNode - root = SimpleNamespace( - uid="uf-root", name="PAM Environments", parent_uid=None, + uid="uf-root", name=PAM_ROOT_FOLDER_NAME, parent_uid=None, type=BaseFolderNode.UserFolderType, subfolders=["uf-proj"], ) proj = SimpleNamespace( diff --git a/unit-tests/pam/test_pam_nsf_folder_batch.py b/unit-tests/pam/test_pam_nsf_folder_batch.py new file mode 100644 index 000000000..ba15e7d84 --- /dev/null +++ b/unit-tests/pam/test_pam_nsf_folder_batch.py @@ -0,0 +1,163 @@ +"""Tests for batched NSF folder creation used by PAM CyberArk import.""" + +import os +import sys +import unittest +from unittest.mock import MagicMock, patch + +sys.path.insert(0, os.path.join(os.path.dirname(__file__), '..', '..')) + +from keepercommander.commands.pam_import.nsf_helpers import create_nsf_folders_batch +from keepercommander.error import CommandError + + +class TestCreateNsfFoldersBatch(unittest.TestCase): + + def _params(self): + params = MagicMock() + params.nested_share_folders = {} + params.subfolder_cache = {} + params.folder_cache = {} + params.environment_variables = {} + return params + + @patch('keepercommander.commands.pam_import.nsf_helpers.sync_down_preserving_nsf_keys') + @patch('keepercommander.nested_share_folder.folder_api.create_folders_batch_v3') + def test_batches_and_seeds_cache(self, mock_batch, mock_sync): + params = self._params() + mock_batch.return_value = [ + { + 'folder_uid': 'uid-a', + 'folder_key_unencrypted': b'key-a', + 'name': 'SafeA', + 'success': True, + 'message': '', + }, + { + 'folder_uid': 'uid-b', + 'folder_key_unencrypted': b'key-b', + 'name': 'SafeB', + 'success': True, + 'message': '', + }, + ] + + specs = [ + {'name': 'SafeA', 'parent_uid': 'proj'}, + {'name': 'SafeB', 'parent_uid': 'proj'}, + ] + results = create_nsf_folders_batch(params, specs, sync=True, command='pam') + + self.assertEqual(len(results), 2) + mock_batch.assert_called_once_with(params, specs) + mock_sync.assert_called_once_with(params) + self.assertEqual(params.nested_share_folders['uid-a']['name'], 'SafeA') + self.assertEqual(params.nested_share_folders['uid-b']['parent_uid'], 'proj') + + @patch('keepercommander.commands.pam_import.nsf_helpers.sync_down_preserving_nsf_keys') + @patch('keepercommander.nested_share_folder.folder_api.create_folders_batch_v3') + def test_skips_sync_when_requested(self, mock_batch, mock_sync): + params = self._params() + mock_batch.return_value = [{ + 'folder_uid': 'uid-a', + 'folder_key_unencrypted': b'k', + 'name': 'Config', + 'success': True, + 'message': '', + }] + + create_nsf_folders_batch( + params, [{'name': 'Config', 'parent_uid': 'proj'}], + sync=False, command='pam', + ) + mock_sync.assert_not_called() + + @patch('keepercommander.nested_share_folder.folder_api.create_folders_batch_v3') + def test_raises_on_failed_folder(self, mock_batch): + params = self._params() + mock_batch.return_value = [{ + 'folder_uid': 'uid-a', + 'success': False, + 'message': 'denied', + 'name': 'Bad', + }] + + with self.assertRaises(CommandError): + create_nsf_folders_batch( + params, [{'name': 'Bad', 'parent_uid': 'proj'}], + sync=False, command='pam', + ) + + @patch('keepercommander.commands.pam_import.nsf_helpers.sync_down_preserving_nsf_keys') + @patch('keepercommander.nested_share_folder.folder_api.create_folders_batch_v3') + def test_chunks_over_100(self, mock_batch, mock_sync): + params = self._params() + + def _side_effect(_params, chunk): + return [{ + 'folder_uid': f'uid-{i}', + 'folder_key_unencrypted': b'k', + 'name': s['name'], + 'success': True, + 'message': '', + } for i, s in enumerate(chunk)] + + mock_batch.side_effect = _side_effect + specs = [{'name': f'F{i}', 'parent_uid': 'proj'} for i in range(105)] + results = create_nsf_folders_batch(params, specs, sync=True, command='pam') + + self.assertEqual(len(results), 105) + self.assertEqual(mock_batch.call_count, 2) + self.assertEqual(len(mock_batch.call_args_list[0][0][1]), 100) + self.assertEqual(len(mock_batch.call_args_list[1][0][1]), 5) + + +class TestCreateSafeFoldersNsfBatch(unittest.TestCase): + + @patch('keepercommander.commands.pam_import.edit.PAMProjectImportCommand.add_folder_permissions') + @patch('keepercommander.commands.pam_import.nsf_helpers.create_nsf_folders_batch') + def test_two_layer_batch_layout(self, mock_batch, _mock_perms): + from keepercommander.commands.pam_import.edit import PAMProjectImportCommand + + cmd = PAMProjectImportCommand() + params = MagicMock() + res = {"project_folder": "CyberArk Migration", "safe_folders": []} + safe_folder_map = {} + records = [ + {"name": "Win_Local", "safe_name": "Win_Local", "fperm": {}, "uperm": []}, + {"name": "Linux", "safe_name": "Linux", "fperm": {}, "uperm": [{"name": "u@x.com"}]}, + ] + + # Layer1: Config + 2 safes; Layer2: 4 children + def _batch(_params, specs, sync=True, command='pam'): + return [{ + 'folder_uid': f'uid-{spec["name"]}', + 'name': spec['name'], + 'success': True, + } for spec in specs] + + mock_batch.side_effect = _batch + + cmd._create_safe_folders_nsf_batch( + params, 'proj-uid', res, records, + 'CyberArk Migration - Config', safe_folder_map, + ) + + self.assertEqual(mock_batch.call_count, 2) + layer1_specs = mock_batch.call_args_list[0][0][1] + layer2_specs = mock_batch.call_args_list[1][0][1] + self.assertEqual( + [s['name'] for s in layer1_specs], + ['CyberArk Migration - Config', 'Win_Local', 'Linux'], + ) + self.assertEqual(len(layer2_specs), 4) + self.assertEqual(res['config_folder_uid'], 'uid-CyberArk Migration - Config') + self.assertEqual(safe_folder_map['Win_Local'], 'uid-Win_Local') + self.assertIn('Win_Local/Win_Local - Resources', safe_folder_map) + self.assertIn('Linux/Linux - Users', safe_folder_map) + self.assertEqual(len(res['safe_folders']), 2) + _mock_perms.assert_called_once() + + +if __name__ == '__main__': + unittest.main() From b82d5582de09f64f885b178c14d7c69cac2a52ca Mon Sep 17 00:00:00 2001 From: pvagare-ks Date: Tue, 28 Jul 2026 00:14:52 +0530 Subject: [PATCH 6/6] fix tests --- unit-tests/pam/test_pam_project_import_nsf.py | 80 ++++++++++--------- 1 file changed, 41 insertions(+), 39 deletions(-) diff --git a/unit-tests/pam/test_pam_project_import_nsf.py b/unit-tests/pam/test_pam_project_import_nsf.py index 42ab218da..a1c0db21e 100644 --- a/unit-tests/pam/test_pam_project_import_nsf.py +++ b/unit-tests/pam/test_pam_project_import_nsf.py @@ -94,7 +94,10 @@ def test_import_record_objects_use_nsf_aware_record_add_helper(): assert uid == 'record_uid' add_record.assert_called_once() assert add_record.call_args.args[2] == 'root_nsf' - assert add_record.call_args.kwargs == {'command': 'pam-project-import'} + assert add_record.call_args.kwargs == { + 'command': 'pam-project-import', + 'sync_after': False, + } def test_process_folders_uses_existing_nsf_root_and_creates_nsf_children(): @@ -109,22 +112,21 @@ def test_process_folders_uses_existing_nsf_root_and_creates_nsf_children(): } created = [] - def create_folder(params_arg, folder_name, parent_uid=None): - uid = f'nsf_{len(created) + 1}' - created.append((folder_name, parent_uid, uid)) - params_arg.nested_share_folders[uid] = { - 'name': folder_name, - 'parent_uid': parent_uid, - 'folder_key_unencrypted': b'k' * 32, - } - return { - 'success': True, - 'folder_uid': uid, - 'folder_key_unencrypted': b'k' * 32, - } - - with patch('keepercommander.nested_share_folder.folder_api.create_folder_v3', - side_effect=create_folder) as create_folder_v3, \ + def create_folders_batch(_params, folder_specs): + results = [] + for spec in folder_specs: + uid = f'nsf_{len(created) + 1}' + created.append((spec['name'], spec.get('parent_uid'), uid)) + results.append({ + 'success': True, + 'folder_uid': uid, + 'folder_key_unencrypted': b'k' * 32, + 'name': spec['name'], + }) + return results + + with patch('keepercommander.nested_share_folder.folder_api.create_folders_batch_v3', + side_effect=create_folders_batch) as create_batch, \ patch('keepercommander.commands.pam_import.nsf_helpers.api.sync_down'), \ patch('keepercommander.commands.pam_import.edit.api.sync_down'): result = PAMProjectImportCommand().process_folders(params, project) @@ -133,7 +135,7 @@ def create_folder(params_arg, folder_name, parent_uid=None): assert result['project_folder_uid'] == 'nsf_1' assert result['resources_folder_uid'] == 'nsf_2' assert result['users_folder_uid'] == 'nsf_3' - assert create_folder_v3.call_count == 3 + assert create_batch.call_count == 3 assert created == [ ('Project 1', 'root_nsf', 'nsf_1'), ('Project 1 - Resources', 'nsf_1', 'nsf_2'), @@ -163,22 +165,21 @@ def test_process_folders_with_nsf_flag_ignores_legacy_root_folder(): } created = [] - def create_folder(params_arg, folder_name, parent_uid=None): - uid = f'nsf_{len(created) + 1}' - created.append((folder_name, parent_uid, uid)) - params_arg.nested_share_folders[uid] = { - 'name': folder_name, - 'parent_uid': parent_uid, - 'folder_key_unencrypted': b'k' * 32, - } - return { - 'success': True, - 'folder_uid': uid, - 'folder_key_unencrypted': b'k' * 32, - } - - with patch('keepercommander.nested_share_folder.folder_api.create_folder_v3', - side_effect=create_folder), \ + def create_folders_batch(_params, folder_specs): + results = [] + for spec in folder_specs: + uid = f'nsf_{len(created) + 1}' + created.append((spec['name'], spec.get('parent_uid'), uid)) + results.append({ + 'success': True, + 'folder_uid': uid, + 'folder_key_unencrypted': b'k' * 32, + 'name': spec['name'], + }) + return results + + with patch('keepercommander.nested_share_folder.folder_api.create_folders_batch_v3', + side_effect=create_folders_batch), \ patch('keepercommander.commands.pam_import.nsf_helpers.api.sync_down'), \ patch('keepercommander.commands.pam_import.edit.api.sync_down'): result = PAMProjectImportCommand().process_folders(params, project) @@ -219,18 +220,19 @@ def test_create_subfolder_seeds_folder_key_and_survives_sync_wipe(): params = _params() folder_key = b'f' * 32 - def create_folder(_params, folder_name, parent_uid=None): - return { + def create_folders_batch(_params, folder_specs): + return [{ 'success': True, 'folder_uid': 'new_nsf', 'folder_key_unencrypted': folder_key, - } + 'name': folder_specs[0]['name'], + }] def wipe_nsf(_params): _params.nested_share_folders.clear() - with patch('keepercommander.nested_share_folder.folder_api.create_folder_v3', - side_effect=create_folder), \ + with patch('keepercommander.nested_share_folder.folder_api.create_folders_batch_v3', + side_effect=create_folders_batch), \ patch('keepercommander.commands.pam_import.nsf_helpers.api.sync_down', side_effect=wipe_nsf): uid = PAMProjectImportCommand().create_subfolder(