From 759c40209cdf43841589d6624e26aee5fae3d18f Mon Sep 17 00:00:00 2001 From: pvagare-ks Date: Wed, 3 Jun 2026 17:12:43 +0530 Subject: [PATCH 01/14] added pam config new, pam config edit, pam split, pam project import support for NSF --- keepercommander/commands/discoveryrotation.py | 14 +- keepercommander/commands/pam/vault_target.py | 202 +++++++++++++++++ .../pam_cloud/pam_privileged_access.py | 3 +- keepercommander/commands/pam_import/base.py | 14 +- keepercommander/commands/pam_import/edit.py | 70 +++++- keepercommander/commands/pam_import/extend.py | 75 ++++++- .../commands/tunnel_and_connections.py | 8 +- tests/test_pam_privileged_cloud.py | 3 +- unit-tests/pam/test_pam_nsf_config.py | 212 ++++++++++++++++++ unit-tests/pam/test_pam_project_extend_nsf.py | 103 +++++++++ unit-tests/pam/test_pam_project_import_nsf.py | 200 +++++++++++++++++ unit-tests/pam/test_pam_split_nsf.py | 117 ++++++++++ 12 files changed, 990 insertions(+), 31 deletions(-) create mode 100644 keepercommander/commands/pam/vault_target.py create mode 100644 unit-tests/pam/test_pam_nsf_config.py create mode 100644 unit-tests/pam/test_pam_project_extend_nsf.py create mode 100644 unit-tests/pam/test_pam_project_import_nsf.py create mode 100644 unit-tests/pam/test_pam_split_nsf.py diff --git a/keepercommander/commands/discoveryrotation.py b/keepercommander/commands/discoveryrotation.py index 031665ac2..9073dd185 100644 --- a/keepercommander/commands/discoveryrotation.py +++ b/keepercommander/commands/discoveryrotation.py @@ -2512,9 +2512,9 @@ def parse_pam_configuration(self, params, record, **kwargs): shared_folder_uid = None # type: Optional[str] folder_name = kwargs.get('shared_folder_uid') # type: Optional[str] if folder_name: - if folder_name in params.shared_folder_cache: - shared_folder_uid = folder_name - else: + from .pam.vault_target import resolve_pam_folder_uid + shared_folder_uid = resolve_pam_folder_uid(params, folder_name) + if not shared_folder_uid: for sf_uid in params.shared_folder_cache: sf = api.get_shared_folder(params, sf_uid) if sf and sf.name.casefold() == folder_name.casefold(): @@ -2876,7 +2876,8 @@ def execute(self, params, **kwargs): # Moving v6 record into the folder api.sync_down(params) - FolderMoveCommand().execute(params, src=record.record_uid, dst=shared_folder_uid, force=True) + from .pam.vault_target import place_record_in_folder + place_record_in_folder(params, record.record_uid, shared_folder_uid, command='pam-config-new') params.environment_variables[LAST_RECORD_UID] = record.record_uid params.sync_data = True @@ -3005,7 +3006,8 @@ def execute(self, params, **kwargs): api.communicate_rest(params, pcc, 'pam/set_configuration_controller') shared_folder_uid = value.get('folderUid') or '' if shared_folder_uid != orig_shared_folder_uid: - FolderMoveCommand().execute(params, src=configuration.record_uid, dst=shared_folder_uid) + from .pam.vault_target import place_record_in_folder + place_record_in_folder(params, configuration.record_uid, shared_folder_uid, command='pam-config-edit') if configuration.type_name == 'pamDomainConfiguration' and not kwargs.get('force_domain_admin', False) is True: # pamUser must exist or "403 Insufficient PAM access to perform this operation" @@ -4299,4 +4301,4 @@ def execute(self, params: KeeperParams, **kwargs): gateway_name = new_name if new_name else gateway.controllerName gateway_helper.edit_gateway(params, gateway.controllerUid, gateway_name, resolved_node_id) - logging.info('Gateway %s has been edited.', gateway_uid) \ No newline at end of file + logging.info('Gateway %s has been edited.', gateway_uid) diff --git a/keepercommander/commands/pam/vault_target.py b/keepercommander/commands/pam/vault_target.py new file mode 100644 index 000000000..7b5713614 --- /dev/null +++ b/keepercommander/commands/pam/vault_target.py @@ -0,0 +1,202 @@ +from ..folder import FolderMoveCommand +from ... import api, record_management +from ...error import CommandError +from ...nested_share_folder.folder_record_api import move_record_v3 +from ...subfolder import BaseFolderNode, get_folder_path, try_resolve_path + + +def is_nested_share_folder(params, folder_uid): + if not folder_uid: + return False + + if folder_uid in getattr(params, 'nested_share_folders', {}): + return True + + subfolder = getattr(params, 'subfolder_cache', {}).get(folder_uid) + if isinstance(subfolder, dict) and subfolder.get('source') == 'nested_share_folder': + return True + + folder = getattr(params, 'folder_cache', {}).get(folder_uid) + return bool(folder and getattr(folder, 'type', None) == BaseFolderNode.NestedShareFolderType) + + +def resolve_pam_folder_uid(params, folder_name, allow_legacy_user=False): + if not folder_name: + return None + + if folder_name in getattr(params, 'shared_folder_cache', {}): + return folder_name + + if is_nested_share_folder(params, folder_name): + return folder_name + + rs = try_resolve_path(params, folder_name) + if rs: + folder, remainder = rs + if folder and not remainder and folder.uid: + if (folder.type == BaseFolderNode.SharedFolderType + or (allow_legacy_user and folder.uid in getattr(params, 'folder_cache', {})) + or is_nested_share_folder(params, folder.uid)): + return folder.uid + + nsf_matches = [ + uid for uid, folder in getattr(params, 'nested_share_folders', {}).items() + if str(folder.get('name', '')).casefold() == str(folder_name).casefold() + ] + if len(nsf_matches) == 1: + return nsf_matches[0] + + return None + + +def _find_child_folder_uid(params, parent_uid, name): + parent_uid = parent_uid or None + for uid, folder in getattr(params, 'folder_cache', {}).items(): + if folder.parent_uid == parent_uid and folder.name == name: + return uid + + for uid, folder in getattr(params, 'nested_share_folders', {}).items(): + if folder.get('parent_uid') == parent_uid and folder.get('name') == name: + return uid + + return None + + +def ensure_pam_folder_path(params, base_folder_uid, relative_path, command='pam'): + if not base_folder_uid: + raise CommandError(command, 'Base folder UID is required') + + if base_folder_uid not in getattr(params, 'folder_cache', {}) \ + and base_folder_uid not in getattr(params, 'nested_share_folders', {}): + raise CommandError(command, f'Base folder "{base_folder_uid}" not found') + + components = [x.strip() for x in str(relative_path or '').replace('\\', '/').split('/') if x.strip()] + current_uid = base_folder_uid + if not components: + return current_uid + + for component in components: + child_uid = _find_child_folder_uid(params, current_uid, component) + if child_uid: + current_uid = child_uid + continue + + if is_nested_share_folder(params, current_uid): + from ...nested_share_folder.folder_api import create_folder_v3 + result = create_folder_v3(params, component, parent_uid=current_uid) + if isinstance(result, dict) and result.get('success') is False: + raise CommandError(command, result.get('message') or 'Failed to create Nested Share Folder') + current_uid = result.get('folder_uid') if isinstance(result, dict) else None + api.sync_down(params) + else: + from ..folder import FolderMakeCommand + parent_path = get_folder_path(params, current_uid) + folder_path = f'{parent_path}{component}' if parent_path else component + FolderMakeCommand().execute(params, folder=folder_path, user_folder=True) + api.sync_down(params) + current_uid = _find_child_folder_uid(params, current_uid, component) + + if not current_uid: + raise CommandError(command, f'Folder creation succeeded but UID not found: {component}') + + return current_uid + + +def place_record_in_folder(params, record_uid, folder_uid, command='pam'): + if not folder_uid: + raise CommandError(command, 'Target folder UID is required') + + folder_cache = getattr(params, 'folder_cache', {}) + nsf_cache = getattr(params, 'nested_share_folders', {}) + if folder_uid not in folder_cache and folder_uid not in nsf_cache: + raise CommandError(command, f'Folder "{folder_uid}" not found') + + if is_nested_share_folder(params, folder_uid): + result = move_record_v3(params, record_uid, to_folder_uid=folder_uid) + if isinstance(result, dict) and result.get('success') is False: + raise CommandError(command, result.get('message') or 'Failed to place record in Nested Share Folder') + api.sync_down(params) + else: + FolderMoveCommand().execute(params, src=record_uid, dst=folder_uid, force=True) + + +def create_record_in_folder(params, record, folder_uid=None, command='pam'): + if folder_uid and is_nested_share_folder(params, folder_uid): + record_management.add_record_to_folder(params, record) + place_record_in_folder(params, record.record_uid, folder_uid, command=command) + else: + record_management.add_record_to_folder(params, record, folder_uid) + + +def execute_record_add_in_folder(params, args, folder_uid, command='pam'): + from ..record_edit import RecordAddCommand + + record_args = dict(args or {}) + if folder_uid and is_nested_share_folder(params, folder_uid): + record_args.pop('folder', None) + uid = RecordAddCommand().execute(params, **record_args) + if uid and isinstance(uid, str): + place_record_in_folder(params, uid, folder_uid, command=command) + return uid + + record_args['folder'] = folder_uid + return RecordAddCommand().execute(params, **record_args) + + +def execute_record_v3_add_in_folder(params, args, folder_uid, command='pam'): + from ..recordv3 import RecordAddCommand + + record_args = dict(args or {}) + if folder_uid and is_nested_share_folder(params, folder_uid): + record_args.pop('folder', None) + uid = RecordAddCommand().execute(params, **record_args) + if not uid: + raise CommandError(command, 'Record creation failed for Nested Share Folder target') + place_record_in_folder(params, uid, folder_uid, command=command) + return uid + + record_args['folder'] = folder_uid + return RecordAddCommand().execute(params, **record_args) + + +def grant_pam_folder_permissions(params, folder_uid, permissions, command='pam'): + if not permissions: + return + if not is_nested_share_folder(params, folder_uid): + return + + from ..nested_share_folder.helpers import classify_share_recipient + from ...nested_share_folder.folder_api import grant_folder_access_v3 + + for perm in permissions: + if not any(map(lambda x: x is not None, perm.values())): + continue + recipient = perm.get('name') or perm.get('uid') + if not recipient: + continue + + classified = classify_share_recipient(params, recipient) + if not classified: + continue + + kind, identifier = classified + manage_users = bool(perm.get('manage_users')) + manage_records = bool(perm.get('manage_records')) + if manage_users and manage_records: + role = 'full-manager' + elif manage_records: + role = 'content-manager' + elif manage_users: + role = 'share-manager' + else: + role = 'viewer' + + result = grant_folder_access_v3( + params, + folder_uid, + identifier, + role=role, + as_team=kind == 'team', + ) + if isinstance(result, dict) and result.get('success') is False: + raise CommandError(command, result.get('message') or 'Failed to grant Nested Share Folder access') diff --git a/keepercommander/commands/pam_cloud/pam_privileged_access.py b/keepercommander/commands/pam_cloud/pam_privileged_access.py index 21940efa0..ca152419e 100644 --- a/keepercommander/commands/pam_cloud/pam_privileged_access.py +++ b/keepercommander/commands/pam_cloud/pam_privileged_access.py @@ -310,6 +310,7 @@ def execute(self, params, **kwargs): if shared_folders: sf = shared_folders[0] folder_uid = sf.parent_uid if sf.parent_uid else sf.uid + record_management.add_record_to_folder(params, record, folder_uid) params.sync_data = True @@ -570,4 +571,4 @@ def execute(self, params, **kwargs): _dispatch_idp_action(params, action, kwargs.get('gateway')) - logging.info(f'User "{username}" removed from group "{group_id}".') \ No newline at end of file + logging.info(f'User "{username}" removed from group "{group_id}".') diff --git a/keepercommander/commands/pam_import/base.py b/keepercommander/commands/pam_import/base.py index 3ad394261..f3056f377 100644 --- a/keepercommander/commands/pam_import/base.py +++ b/keepercommander/commands/pam_import/base.py @@ -21,7 +21,7 @@ from pathlib import Path from typing import Any, Dict, Optional, List, Union -from ..record_edit import RecordAddCommand as RecordEditAddCommand +from ..pam.vault_target import execute_record_add_in_folder from ..workflow.helpers import RecordResolver, WorkflowFormatter from ... import api, attachment, utils, vault, vault_extensions, \ record_facades, record_management @@ -1068,7 +1068,7 @@ def create_record(self, params, folder_uid): fields.append(f"file=@{x.file}") if fields: args["fields"] = fields - uid = RecordEditAddCommand().execute(params, **args) + uid = execute_record_add_in_folder(params, args, folder_uid, command='pam-project-import') if uid and isinstance(uid, str): self.uid = uid @@ -1163,7 +1163,7 @@ def create_record(self, params, folder_uid): fields.append(f"file=@{x.file}") if fields: args["fields"] = fields - uid = RecordEditAddCommand().execute(params, **args) + uid = execute_record_add_in_folder(params, args, folder_uid, command='pam-project-import') if uid and isinstance(uid, str): self.uid = uid return uid @@ -1433,7 +1433,7 @@ def create_record(self, params, folder_uid): # switch to f.* once RT definition(s) update w/ pamSettings field if fields: args["fields"] = fields - uid = RecordEditAddCommand().execute(params, **args) + uid = execute_record_add_in_folder(params, args, folder_uid, command='pam-project-import') if uid and isinstance(uid, str): self.uid = uid @@ -1626,7 +1626,7 @@ def create_record(self, params, folder_uid): # switch to f.* once RT definition(s) update w/ pamSettings field if fields: args["fields"] = fields - uid = RecordEditAddCommand().execute(params, **args) + uid = execute_record_add_in_folder(params, args, folder_uid, command='pam-project-import') if uid and isinstance(uid, str): self.uid = uid @@ -1774,7 +1774,7 @@ def create_record(self, params, folder_uid): # switch to f.* once RT definition(s) update w/ pamSettings field if fields: args["fields"] = fields - uid = RecordEditAddCommand().execute(params, **args) + uid = execute_record_add_in_folder(params, args, folder_uid, command='pam-project-import') if uid and isinstance(uid, str): self.uid = uid @@ -1879,7 +1879,7 @@ def create_record(self, params, folder_uid): # switch to f.* once RT definition(s) update w/ pamRemoteBrowserSettings field if fields: args["fields"] = fields - uid = RecordEditAddCommand().execute(params, **args) + uid = execute_record_add_in_folder(params, args, folder_uid, command='pam-project-import') if uid and isinstance(uid, str): self.uid = uid diff --git a/keepercommander/commands/pam_import/edit.py b/keepercommander/commands/pam_import/edit.py index 149938837..9b685879d 100644 --- a/keepercommander/commands/pam_import/edit.py +++ b/keepercommander/commands/pam_import/edit.py @@ -16,6 +16,7 @@ import os.path from itertools import chain +from types import SimpleNamespace from typing import Any, Dict, Optional, List, Union from .keeper_ai_settings import set_resource_jit_settings, set_resource_keeper_ai_settings, refresh_meta_to_latest, refresh_link_to_config_to_latest @@ -50,7 +51,7 @@ from ..ksm import KSMCommand from ..pam import gateway_helper from ..pam.config_helper import pam_configurations_get_all -from ..recordv3 import RecordAddCommand +from ..pam.vault_target import execute_record_v3_add_in_folder, grant_pam_folder_permissions, is_nested_share_folder from ..record_edit import RecordUploadAttachmentCommand from ..tunnel.port_forward.TunnelGraph import TunnelDAG from ..tunnel.port_forward.tunnel_helpers import get_keeper_tokens @@ -76,6 +77,7 @@ class PAMProjectImportCommand(Command): parser.add_argument("--dry-run", "-d", required=False, dest="dry_run", action="store_true", default=False, help="Test import without modifying vault.") parser.add_argument("--sample-data", "-s", required=False, dest="sample_data", action="store_true", default=False, help="Generate sample data.") parser.add_argument("--show-template", "-t", required=False, dest="show_template", action="store_true", default=False, help="Print JSON template required for manual import.") + parser.add_argument("--nsf", required=False, dest="use_nsf", action="store_true", default=False, help="Create project folders and records in Nested Share Folders.") # parser.add_argument("--force", "-e", required=False, dest="force", action="store_true", default=False, help="Force data import (re/configure later)") parser.add_argument("--output", "-o", required=False, dest="output", action="store", choices=["token", "base64", "json", "k8s"], default="base64", help="Output format (token: one-time token, config: base64/json/k8s)") @@ -112,6 +114,7 @@ def execute(self, params, **kwargs): project["options"]["dry_run"] = kwargs.get("dry_run", False) project["options"]["sample_data"] = kwargs.get("sample_data", False) project["options"]["show_template"] = kwargs.get("show_template", False) + project["options"]["use_nsf"] = kwargs.get("use_nsf", False) project["options"]["force"] = kwargs.get("force", False) project["options"]["output"] = kwargs.get("output", "") or "" @@ -130,7 +133,7 @@ def execute(self, params, **kwargs): project["options"]["project_name"] = "Discovery Playground" project["options"]["file_name"] = "" # project["options"]["dry_run"] = False # dry-run is allowed - extra = self.get_extra_args(["sample_data", "dry_run"], **kwargs) + extra = self.get_extra_args(["sample_data", "dry_run", "use_nsf"], **kwargs) if extra: logging.warning(f"{bcolors.WARNING}Warning: --sample-data|-s overrides other options {extra}{bcolors.ENDC}") @@ -243,10 +246,15 @@ def process_folders(self, params, project: dict) -> dict: # if project["data"].get("tool_version", "") != "": # CLI generated export else: # Manually generated import file # FolderListCommand().execute(params, folders_only=True, pattern="/") + use_nsf = project["options"].get("use_nsf", False) is True folders = self.find_folders(params, "", res["root_folder_target"], False) if folders: # select first non-root non-shared sub/folder folders = [x for x in folders if x.type == x.UserFolderType] + if use_nsf: + folders = [x for x in folders if is_nested_share_folder(params, x.uid)] + else: + folders = [x for x in folders if not is_nested_share_folder(params, x.uid)] if len(folders) > 1: logging.warning(f"""Multiple user folders ({len(folders)}) match folder name "{res["root_folder_target"]}" """ f" using first match with UID: {bcolors.OKGREEN}{folders[0].uid}{bcolors.ENDC}") @@ -256,7 +264,7 @@ def process_folders(self, params, project: dict) -> dict: res["root_folder_uid"] = str(folders[0].uid) elif project["options"].get("dry_run", False) is not True: # FolderMakeCommand().execute(params, user_folder=True, folder=f"""/{res["root_folder_target"]}""") - fuid = self.create_subfolder(params, res["root_folder_target"]) + fuid = self.create_subfolder(params, res["root_folder_target"], use_nsf=use_nsf) res["root_folder_uid"] = fuid # find available project folder - incr. numeric suffix until distinct name found @@ -274,7 +282,7 @@ def process_folders(self, params, project: dict) -> dict: res["project_folder"] = folder_name if project["options"].get("dry_run", False) is not True: # FolderMakeCommand().execute(params, shared_folder=True, folder=f"""/{res["root_folder"]}/{res["project_folder"]}""") - fuid = self.create_subfolder(params, folder_name=res["project_folder"], parent_uid=res["root_folder_uid"]) + fuid = self.create_subfolder(params, folder_name=res["project_folder"], parent_uid=res["root_folder_uid"], use_nsf=use_nsf) res["project_folder_uid"] = fuid puid = res["project_folder_uid"] @@ -282,14 +290,14 @@ def process_folders(self, params, project: dict) -> dict: fname = sfn["folder_name"] if isinstance(sfn, dict) and isinstance(sfn.get("folder_name", None), str) else "" fname = fname.strip() or f"""{res["project_folder"]} - Resources""" fperm, rperm = self.get_folder_permissions(users_folder=False, data=project["data"]) - fuid = self.create_subfolder(params, folder_name=fname, parent_uid=puid, permissions=fperm) + fuid = self.create_subfolder(params, folder_name=fname, parent_uid=puid, permissions=fperm, use_nsf=use_nsf) res["resources_folder_uid"] = fuid sfn = project["data"].get("shared_folder_users", None) fname = sfn["folder_name"] if isinstance(sfn, dict) and isinstance(sfn.get("folder_name", None), str) else "" fname = fname.strip() or f"""{res["project_folder"]} - Users""" fperm, uperm = self.get_folder_permissions(users_folder=True, data=project["data"]) - fuid = self.create_subfolder(params, folder_name=fname, parent_uid=puid, permissions=fperm) + fuid = self.create_subfolder(params, folder_name=fname, parent_uid=puid, permissions=fperm, use_nsf=use_nsf) res["users_folder_uid"] = fuid # add users and teams @@ -302,8 +310,8 @@ def process_folders(self, params, project: dict) -> dict: if res["root_folder_uid"]: print(f"""Will use existing PAM root folder: {res["root_folder_uid"]} {res["root_folder"]}""") else: - print(f"""Will create new PAM root folder: {res["root_folder_target"]}""") - print(f"""Will create new Project folder: {res["project_folder"]}""") + print(f"""Will create new {"NSF " if use_nsf else ""}PAM root folder: {res["root_folder_target"]}""") + print(f"""Will create new {"NSF " if use_nsf else ""}Project folder: {res["project_folder"]}""") else: api.sync_down(params) @@ -347,6 +355,9 @@ def process_ksm_app(self, params, project: dict) -> dict: for sf_uid in [project["folders"].get("resources_folder_uid", ""), project["folders"].get("users_folder_uid", "")]: if sf_uid.strip(): + if is_nested_share_folder(params, sf_uid): + logging.info("Skipping KSM application share for Nested Share Folder %s", sf_uid) + continue KSMCommand().execute(params, command=("secret", "add"), app=res["app_uid"], @@ -609,7 +620,17 @@ def generate_discovery_playground_data(self, params, project: dict): # Fix: Rotation is disabled by the PAM configuration. tdag.set_resource_allowed(pam_config_uid, is_config=True, rotation=True, connections=True, tunneling=True, session_recording=True, typescript_recording=True, remote_browser_isolation=True) - command = RecordAddCommand() + class PamProjectRecordAddCommand: + @staticmethod + def execute(params, **kwargs): + return execute_record_v3_add_in_folder( + params, + kwargs, + kwargs.get('folder'), + command='pam-project-import' + ) + + command = PamProjectRecordAddCommand() pte = PAMTunnelEditCommand() # when NOOP rotation is added to PAMCreateRecordRotationCommand... @@ -1197,6 +1218,11 @@ def get_folder_permissions(self, users_folder: bool, data: dict): def add_folder_permissions(self, params, folder_uid: str, permissions: list): if permissions: + if is_nested_share_folder(params, folder_uid): + grant_pam_folder_permissions(params, folder_uid, permissions, command='pam-project-import') + api.sync_down(params) + return + shf = SharedFolder() shf.uid = folder_uid # type: ignore shf.path = imp_exp.get_folder_path(params, folder_uid) # type: ignore @@ -1232,7 +1258,8 @@ def add_folder_permissions(self, params, folder_uid: str, permissions: list): # args["manage_users"] = True if manage_users == False else False # ShareFolderCommand().execute(params, **args) - def create_subfolder(self, params, folder_name:str, parent_uid:str="", permissions:Optional[Dict]=None): + def create_subfolder(self, params, folder_name:str, parent_uid:str="", permissions:Optional[Dict]=None, + use_nsf: bool=False): """ Creates subfolder inside parent folder: either `user folder`, `shared folder` or `shared folder folder`. If `parent_uid == ""` then creates subfolder in root folder. @@ -1243,6 +1270,18 @@ 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 ...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}') + api.sync_down(params) + params.environment_variables[LAST_FOLDER_UID] = folder_uid + return folder_uid + base_folder = params.folder_cache.get(parent_uid, None) or params.root_folder shared_folder = True if permissions else False @@ -1314,6 +1353,17 @@ def find_folders(self, params, parent_uid:str, folder:str, is_shared_folder:bool result = [v for k, v in matches.items() if (is_shared_folder and v.type == BaseFolderNode.SharedFolderType) or (not is_shared_folder and v.type == BaseFolderNode.UserFolderType)] + if not is_shared_folder: + for uid, nsf in getattr(params, 'nested_share_folders', {}).items(): + if nsf.get('parent_uid') == puid and nsf.get('name') == folder: + result.append(SimpleNamespace( + uid=uid, + name=nsf.get('name'), + parent_uid=nsf.get('parent_uid'), + type=BaseFolderNode.UserFolderType, + UserFolderType=BaseFolderNode.UserFolderType, + SharedFolderType=BaseFolderNode.SharedFolderType, + )) return result def create_ksm_app(self, params, app_name) -> str: diff --git a/keepercommander/commands/pam_import/extend.py b/keepercommander/commands/pam_import/extend.py index c21a2a6f2..1ce6f5972 100644 --- a/keepercommander/commands/pam_import/extend.py +++ b/keepercommander/commands/pam_import/extend.py @@ -60,6 +60,7 @@ from ..ksm import KSMCommand from ..pam import gateway_helper from ..pam.config_helper import configuration_controller_get +from ..pam.vault_target import is_nested_share_folder from ..tunnel.port_forward.TunnelGraph import TunnelDAG from ..tunnel.port_forward.tunnel_helpers import get_keeper_tokens from ..tunnel_and_connections import PAMTunnelEditCommand @@ -70,7 +71,7 @@ from ...params import LAST_FOLDER_UID, LAST_SHARED_FOLDER_UID from ...proto import APIRequest_pb2, enterprise_pb2, pam_pb2, record_pb2 from ...recordv3 import RecordV3 -from ...subfolder import BaseFolderNode +from ...subfolder import BaseFolderNode, find_folders as find_record_folders def split_folder_path(path: str) -> list[str]: """Split folder path using path deilmiter / (escape: / -> //)""" @@ -478,7 +479,7 @@ def execute(self, params, **kwargs): raise CommandError("pam project extend", f"""PAM KSM Application record not found: "{ksmapp_uid}" """) # Find KSM Application shared folders - ksm_shared_folders = self.get_app_shared_folders(params, ksmapp_uid) + ksm_shared_folders = self.get_app_shared_folders(params, ksmapp_uid, configuration.record_uid) if not ksm_shared_folders: raise CommandError("pam project extend", f""" No shared folders found for KSM Application: "{ksmapp_uid}" """) @@ -559,7 +560,7 @@ def execute(self, params, **kwargs): return self.process_data(params, project) - def get_app_shared_folders(self, params, ksm_app_uid: str) -> list[dict]: + def get_app_shared_folders(self, params, ksm_app_uid: str, configuration_uid: Optional[str]=None) -> list[dict]: ksm_shared_folders = [] try: @@ -583,8 +584,52 @@ def get_app_shared_folders(self, params, ksm_app_uid: str) -> list[dict]: except Exception as e: logging.error(f"Could not retrieve KSM application shares: {e}") + if not ksm_shared_folders and configuration_uid: + ksm_shared_folders.extend(self.get_nsf_project_folders(params, configuration_uid)) + return ksm_shared_folders + @staticmethod + def get_nsf_project_folders(params, configuration_uid: str) -> list[dict]: + folder_uids = [] + for folder_uid in find_record_folders(params, configuration_uid): + if folder_uid and is_nested_share_folder(params, folder_uid): + folder_uids.append(folder_uid) + + if not folder_uids: + return [] + + project_folder_uids = [] + for folder_uid in folder_uids: + folder = params.folder_cache.get(folder_uid) + if not folder: + continue + parent_uid = folder.parent_uid + if parent_uid: + for uid, candidate in params.folder_cache.items(): + if candidate.parent_uid == parent_uid and is_nested_share_folder(params, uid): + project_folder_uids.append(uid) + else: + project_folder_uids.append(folder_uid) + + result = [] + seen = set() + for folder_uid in project_folder_uids: + if folder_uid in seen: + continue + seen.add(folder_uid) + folder = params.folder_cache.get(folder_uid) + if not folder: + continue + result.append({ + 'uid': folder_uid, + 'name': folder.name or folder_uid, + 'editable': True, + 'permissions': 'Editable', + 'source': 'nested_share_folder', + }) + return result + def process_folders(self, params, project: dict) -> dict: """Step 1: Parse folder_paths from pam_data, build tree, process paths, optionally create new folders. Fills project['folders'] with path_to_folder_uid, good_paths, bad_paths; updates project['error_count'].""" @@ -1189,6 +1234,18 @@ def create_subfolder(self, params, folder_name:str, parent_uid:str="", permissio # folder_uid: if provided, create folder with this UID (same as records with pre-generated uid). name = str(folder_name or "").strip() + if is_nested_share_folder(params, parent_uid): + from ...nested_share_folder.folder_api import create_folder_v3 + result = create_folder_v3(params, name, parent_uid=parent_uid) + if isinstance(result, dict) and result.get('success') is False: + raise CommandError("pam project extend", result.get('message') or 'Failed to create Nested Share Folder') + new_uid = result.get('folder_uid') if isinstance(result, dict) else None + if not new_uid: + raise CommandError("pam project extend", f'Nested Share Folder creation did not return UID: {name}') + api.sync_down(params) + params.environment_variables[LAST_FOLDER_UID] = new_uid + return new_uid + base_folder = params.folder_cache.get(parent_uid, None) or params.root_folder shared_folder = True if permissions else False @@ -1255,6 +1312,17 @@ def find_folders(self, params, parent_uid:str, folder:str, is_shared_folder:bool result = [v for k, v in matches.items() if (is_shared_folder and v.type == BaseFolderNode.SharedFolderType) or (not is_shared_folder and v.type == BaseFolderNode.UserFolderType)] + if not is_shared_folder: + for uid, nsf in getattr(params, 'nested_share_folders', {}).items(): + if nsf.get('parent_uid') == puid and nsf.get('name') == folder: + result.append(SimpleNamespace( + uid=uid, + name=nsf.get('name'), + parent_uid=nsf.get('parent_uid'), + type=BaseFolderNode.UserFolderType, + UserFolderType=BaseFolderNode.UserFolderType, + SharedFolderType=BaseFolderNode.SharedFolderType, + )) return result def create_ksm_app(self, params, app_name) -> str: @@ -1569,4 +1637,3 @@ def process_data(self, params, project): add_pam_scripts(params, pam_cfg_uid, refs) logging.debug("Done processing project data.") return - diff --git a/keepercommander/commands/tunnel_and_connections.py b/keepercommander/commands/tunnel_and_connections.py index 65aaecd84..b05b5a7d3 100644 --- a/keepercommander/commands/tunnel_and_connections.py +++ b/keepercommander/commands/tunnel_and_connections.py @@ -4172,7 +4172,10 @@ def execute(self, params, **kwargs): print(f"{bcolors.FAIL}Please provide a valid PAM Configuration.{bcolors.ENDC}") return - folder_uid = resolve_folder(params, folder) + folder_uid = '' + if folder: + from .pam.vault_target import resolve_pam_folder_uid + folder_uid = resolve_pam_folder_uid(params, folder, allow_legacy_user=True) or resolve_folder(params, folder) if folder and not folder_uid: print(f"{bcolors.WARNING}Unable to find destination folder '{folder}' " "(Note: folder names/paths are case sensitive) " @@ -4243,7 +4246,8 @@ def execute(self, params, **kwargs): f"and PAM User record will be created in folder '{folders[0]}'.{bcolors.ENDC}") folder_uid = folders[0] if folders else '' # '' means root folder - record_management.add_record_to_folder(params, user_rec, folder_uid) + from .pam.vault_target import create_record_in_folder + create_record_in_folder(params, user_rec, folder_uid, command='pam-split') pam_user_uid = params.environment_variables.get(LAST_RECORD_UID, '') api.sync_down(params) diff --git a/tests/test_pam_privileged_cloud.py b/tests/test_pam_privileged_cloud.py index 1c840f32f..0acecaa02 100644 --- a/tests/test_pam_privileged_cloud.py +++ b/tests/test_pam_privileged_cloud.py @@ -4,6 +4,7 @@ import unittest from unittest.mock import MagicMock, patch +import keepercommander.commands.record # noqa: F401 from keepercommander.commands.pam.pam_dto import ( GatewayActionIdpInputs, GatewayActionIdpCreateUser, @@ -270,4 +271,4 @@ def test_group_has_add_remove_list(self): if __name__ == '__main__': - unittest.main() \ No newline at end of file + unittest.main() diff --git a/unit-tests/pam/test_pam_nsf_config.py b/unit-tests/pam/test_pam_nsf_config.py new file mode 100644 index 000000000..37fbccf6a --- /dev/null +++ b/unit-tests/pam/test_pam_nsf_config.py @@ -0,0 +1,212 @@ +import unittest +from unittest import mock + +import keepercommander.commands.record # noqa: F401 +from keepercommander import vault +from keepercommander.commands.discoveryrotation import PAMConfigurationEditCommand, PAMConfigurationNewCommand +from keepercommander.commands.pam.vault_target import ( + create_record_in_folder, place_record_in_folder, resolve_pam_folder_uid) +from keepercommander.error import CommandError +from keepercommander.subfolder import NestedShareFolderNode, RootFolderNode, SharedFolderNode + + +def _make_params(): + params = mock.MagicMock() + params.folder_cache = {} + params.shared_folder_cache = {} + params.nested_share_folders = {} + params.current_folder = '' + params.root_folder = RootFolderNode() + params.environment_variables = {} + params.sync_data = False + return params + + +class TestPamVaultTarget(unittest.TestCase): + + @mock.patch('keepercommander.commands.pam.vault_target.api.sync_down') + @mock.patch('keepercommander.commands.pam.vault_target.move_record_v3') + @mock.patch('keepercommander.commands.pam.vault_target.FolderMoveCommand') + def test_place_record_uses_legacy_move_for_legacy_folder(self, mock_move_command, mock_nsf_move, mock_sync): + params = _make_params() + folder = SharedFolderNode() + folder.uid = 'legacy_folder' + params.folder_cache[folder.uid] = folder + + place_record_in_folder(params, 'record_uid', folder.uid, command='pam-config-new') + + mock_move_command.return_value.execute.assert_called_once_with( + params, src='record_uid', dst='legacy_folder', force=True) + mock_nsf_move.assert_not_called() + mock_sync.assert_not_called() + + @mock.patch('keepercommander.commands.pam.vault_target.api.sync_down') + @mock.patch('keepercommander.commands.pam.vault_target.move_record_v3') + @mock.patch('keepercommander.commands.pam.vault_target.FolderMoveCommand') + def test_place_record_uses_nsf_move_for_nsf_folder(self, mock_move_command, mock_nsf_move, mock_sync): + params = _make_params() + folder = NestedShareFolderNode() + folder.uid = 'nsf_folder' + params.folder_cache[folder.uid] = folder + + place_record_in_folder(params, 'record_uid', folder.uid, command='pam-config-new') + + mock_nsf_move.assert_called_once_with(params, 'record_uid', to_folder_uid='nsf_folder') + mock_sync.assert_called_once_with(params) + mock_move_command.assert_not_called() + + @mock.patch('keepercommander.commands.pam.vault_target.api.sync_down') + @mock.patch('keepercommander.commands.pam.vault_target.move_record_v3', + return_value={'success': False, 'message': 'denied'}) + def test_place_record_raises_when_nsf_move_fails(self, mock_nsf_move, mock_sync): + params = _make_params() + folder = NestedShareFolderNode() + folder.uid = 'nsf_folder' + params.folder_cache[folder.uid] = folder + + with self.assertRaises(CommandError): + place_record_in_folder(params, 'record_uid', folder.uid, command='pam-config-new') + mock_sync.assert_not_called() + + def test_place_record_rejects_missing_folder(self): + params = _make_params() + + with self.assertRaises(CommandError): + place_record_in_folder(params, 'record_uid', 'missing_folder', command='pam-config-new') + + @mock.patch('keepercommander.commands.pam.vault_target.record_management.add_record_to_folder') + def test_create_record_in_folder_uses_legacy_add_for_legacy_folder(self, mock_add_record): + params = _make_params() + folder = SharedFolderNode() + folder.uid = 'legacy_folder' + params.folder_cache[folder.uid] = folder + record = vault.TypedRecord() + + create_record_in_folder(params, record, folder.uid, command='pam-access-user-provision') + + mock_add_record.assert_called_once_with(params, record, 'legacy_folder') + + @mock.patch('keepercommander.commands.pam.vault_target.place_record_in_folder') + @mock.patch('keepercommander.commands.pam.vault_target.record_management.add_record_to_folder') + def test_create_record_in_folder_creates_root_then_places_for_nsf_folder(self, mock_add_record, mock_place): + params = _make_params() + folder = NestedShareFolderNode() + folder.uid = 'nsf_folder' + params.folder_cache[folder.uid] = folder + record = vault.TypedRecord() + record.record_uid = 'record_uid' + + create_record_in_folder(params, record, folder.uid, command='pam-access-user-provision') + + mock_add_record.assert_called_once_with(params, record) + mock_place.assert_called_once_with( + params, 'record_uid', 'nsf_folder', command='pam-access-user-provision') + + def test_resolve_pam_folder_uid_finds_root_nsf_folder_by_name(self): + params = _make_params() + folder = NestedShareFolderNode() + folder.uid = 'nsf_folder' + folder.name = 'testpam' + params.folder_cache[folder.uid] = folder + params.root_folder.subfolders.append(folder.uid) + + self.assertEqual(resolve_pam_folder_uid(params, 'testpam'), 'nsf_folder') + + def test_resolve_pam_folder_uid_finds_nsf_cache_folder_by_name(self): + params = _make_params() + params.nested_share_folders['nsf_folder'] = {'name': 'testpam'} + + self.assertEqual(resolve_pam_folder_uid(params, 'testpam'), 'nsf_folder') + + +class TestPamConfigNewNsfPlacement(unittest.TestCase): + + def test_parse_pam_configuration_accepts_nsf_folder_uid(self): + params = _make_params() + folder = NestedShareFolderNode() + folder.uid = 'nsf_folder' + params.folder_cache[folder.uid] = folder + + record = vault.TypedRecord(version=6) + command = PAMConfigurationNewCommand() + command.parse_pam_configuration(params, record, shared_folder_uid='nsf_folder') + + field = record.get_typed_field('pamResources') + self.assertEqual(field.value[0]['folderUid'], 'nsf_folder') + + def test_parse_pam_configuration_accepts_nsf_folder_name(self): + params = _make_params() + folder = NestedShareFolderNode() + folder.uid = 'nsf_folder' + folder.name = 'testpam' + params.folder_cache[folder.uid] = folder + params.root_folder.subfolders.append(folder.uid) + + record = vault.TypedRecord(version=6) + command = PAMConfigurationNewCommand() + command.parse_pam_configuration(params, record, shared_folder_uid='testpam') + + field = record.get_typed_field('pamResources') + self.assertEqual(field.value[0]['folderUid'], 'nsf_folder') + + @mock.patch('keepercommander.commands.pam.vault_target.place_record_in_folder') + @mock.patch('keepercommander.commands.discoveryrotation.api.sync_down') + @mock.patch('keepercommander.commands.discoveryrotation.TunnelDAG') + @mock.patch('keepercommander.commands.discoveryrotation.get_keeper_tokens', + return_value=(b'encrypted_session', b'encrypted_key', b'transmission_key')) + @mock.patch('keepercommander.commands.discoveryrotation.pam_configuration_create_record_v6') + @mock.patch('keepercommander.commands.discoveryrotation.RecordEditMixin.get_record_type_fields', + return_value=[]) + def test_execute_places_new_config_with_backend_aware_helper( + self, mock_record_fields, mock_create_config, mock_tokens, mock_dag, mock_sync, mock_place): + params = _make_params() + command = PAMConfigurationNewCommand() + + def parse_properties(_, record, **kwargs): + record.fields.append(vault.TypedField.new_field( + 'pamResources', {'folderUid': 'nsf_folder'})) + + def create_config(_, record, folder_uid): + record.record_uid = 'config_uid' + + mock_create_config.side_effect = create_config + with mock.patch.object(command, 'parse_properties', side_effect=parse_properties), \ + mock.patch.object(command, 'verify_required'): + result = command.execute(params, config_type='local', title='Config') + + self.assertEqual(result, 'config_uid') + mock_place.assert_called_once_with( + params, 'config_uid', 'nsf_folder', command='pam-config-new') + + +class TestPamConfigEditNsfPlacement(unittest.TestCase): + + @mock.patch('keepercommander.commands.pam.vault_target.place_record_in_folder') + @mock.patch('keepercommander.commands.discoveryrotation.record_management.update_record') + @mock.patch('keepercommander.commands.discoveryrotation.RecordEditMixin.get_record_type_fields', + return_value=[]) + @mock.patch('keepercommander.vault.KeeperRecord.load') + def test_execute_places_edited_config_with_backend_aware_helper( + self, mock_load, mock_record_fields, mock_update_record, mock_place): + params = _make_params() + params.record_cache = {'config_uid': {}} + command = PAMConfigurationEditCommand() + + configuration = vault.TypedRecord(version=6) + configuration.record_uid = 'config_uid' + configuration.type_name = 'pamNetworkConfiguration' + configuration.fields.append(vault.TypedField.new_field( + 'pamResources', {'folderUid': 'legacy_folder'})) + mock_load.return_value = configuration + + def parse_properties(_, record, **kwargs): + field = record.get_typed_field('pamResources') + field.value[0]['folderUid'] = 'nsf_folder' + + with mock.patch.object(command, 'parse_properties', side_effect=parse_properties), \ + mock.patch.object(command, 'verify_required'): + command.execute(params, uid='config_uid') + + mock_update_record.assert_called_once_with(params, configuration) + mock_place.assert_called_once_with( + params, 'config_uid', 'nsf_folder', command='pam-config-edit') diff --git a/unit-tests/pam/test_pam_project_extend_nsf.py b/unit-tests/pam/test_pam_project_extend_nsf.py new file mode 100644 index 000000000..4ff0e93fe --- /dev/null +++ b/unit-tests/pam/test_pam_project_extend_nsf.py @@ -0,0 +1,103 @@ +from types import SimpleNamespace +from unittest.mock import patch + +import keepercommander.commands.record # noqa: F401 + +from keepercommander.commands.pam_import.extend import PAMProjectExtendCommand +from keepercommander.subfolder import BaseFolderNode, NestedShareFolderNode, RootFolderNode + + +def _folder(uid, name, parent_uid=None): + folder = NestedShareFolderNode() + folder.uid = uid + folder.name = name + folder.parent_uid = parent_uid + folder.subfolders = [] + return folder + + +def _params(): + project = _folder('project_nsf', 'NSF Project') + users = _folder('users_nsf', 'NSF Project - Users', 'project_nsf') + resources = _folder('resources_nsf', 'NSF Project - Resources', 'project_nsf') + project.subfolders = ['users_nsf', 'resources_nsf'] + return SimpleNamespace( + data_key=b'1' * 32, + root_folder=RootFolderNode(), + folder_cache={ + 'project_nsf': project, + 'users_nsf': users, + 'resources_nsf': resources, + }, + subfolder_cache={}, + subfolder_record_cache={'users_nsf': {'config_uid'}}, + shared_folder_cache={}, + nested_share_folders={ + 'project_nsf': {'name': 'NSF Project', 'parent_uid': None}, + 'users_nsf': {'name': 'NSF Project - Users', 'parent_uid': 'project_nsf'}, + 'resources_nsf': {'name': 'NSF Project - Resources', 'parent_uid': 'project_nsf'}, + }, + environment_variables={}, + ) + + +def test_get_nsf_project_folders_from_config_folder_siblings(): + params = _params() + + folders = PAMProjectExtendCommand.get_nsf_project_folders(params, 'config_uid') + + assert {x['uid'] for x in folders} == {'users_nsf', 'resources_nsf'} + assert all(x['source'] == 'nested_share_folder' for x in folders) + + +def test_get_app_shared_folders_falls_back_to_nsf_project_folders(): + params = _params() + + with patch('keepercommander.commands.pam_import.extend.KSMCommand.get_app_info', + return_value=[]): + folders = PAMProjectExtendCommand().get_app_shared_folders(params, 'ksm_uid', 'config_uid') + + assert {x['uid'] for x in folders} == {'users_nsf', 'resources_nsf'} + + +def test_create_subfolder_uses_nsf_api_under_nsf_parent(): + params = _params() + + with patch('keepercommander.nested_share_folder.folder_api.create_folder_v3', + return_value={'success': True, 'folder_uid': 'child_nsf'}) as create_folder, \ + patch('keepercommander.commands.pam_import.extend.api.sync_down'): + uid = PAMProjectExtendCommand().create_subfolder(params, 'Child', 'users_nsf', folder_uid='pre_generated') + + assert uid == 'child_nsf' + create_folder.assert_called_once_with(params, 'Child', parent_uid='users_nsf') + + +def test_process_folders_creates_new_nsf_folder_paths(): + params = _params() + project = { + 'data': { + 'pam_data': { + 'users': [{'type': 'pamUser', 'title': 'Admin', 'login': 'admin', 'folder_path': 'NSF Project - Users/Admins'}], + 'resources': [], + } + }, + 'options': {'dry_run': False}, + 'ksm_shared_folders': [ + {'uid': 'users_nsf', 'name': 'NSF Project - Users', 'folder_tree': {}}, + {'uid': 'resources_nsf', 'name': 'NSF Project - Resources', 'folder_tree': {}}, + ], + 'folders': {}, + 'error_count': 0, + } + + def create_folder(params_arg, folder_name, parent_uid=None): + params_arg.nested_share_folders['admins_nsf'] = {'name': folder_name, 'parent_uid': parent_uid} + return {'success': True, 'folder_uid': 'admins_nsf'} + + with patch('keepercommander.nested_share_folder.folder_api.create_folder_v3', + side_effect=create_folder), \ + patch('keepercommander.commands.pam_import.extend.api.sync_down'): + folders = PAMProjectExtendCommand().process_folders(params, project) + + assert folders['path_to_folder_uid']['NSF Project - Users/Admins'] == 'admins_nsf' + assert project['error_count'] == 0 diff --git a/unit-tests/pam/test_pam_project_import_nsf.py b/unit-tests/pam/test_pam_project_import_nsf.py new file mode 100644 index 000000000..166a1391a --- /dev/null +++ b/unit-tests/pam/test_pam_project_import_nsf.py @@ -0,0 +1,200 @@ +from types import SimpleNamespace +from unittest.mock import patch + +import keepercommander.commands.record # noqa: F401 + +from keepercommander.commands.pam.vault_target import ( + execute_record_add_in_folder, execute_record_v3_add_in_folder, grant_pam_folder_permissions, is_nested_share_folder) +from keepercommander.commands.pam_import.base import PamUserObject +from keepercommander.commands.pam_import.edit import PAMProjectImportCommand +from keepercommander.subfolder import BaseFolderNode + + +def _params(): + return SimpleNamespace( + data_key=b'1' * 32, + root_folder=SimpleNamespace(type=BaseFolderNode.RootFolderType), + folder_cache={}, + shared_folder_cache={}, + subfolder_cache={}, + nested_share_folders={ + 'root_nsf': { + 'name': PAMProjectImportCommand.PAM_ROOT_FOLDER_NAME, + 'parent_uid': None, + } + }, + environment_variables={}, + enterprise={'users': []}, + available_team_cache=[], + ) + + +def test_record_add_helper_creates_at_root_then_moves_for_nsf_folder(): + params = _params() + args = { + 'force': True, + 'folder': 'root_nsf', + 'record_type': 'pamUser', + 'title': 'Admin', + } + + with patch('keepercommander.commands.record_edit.RecordAddCommand') as record_add, \ + patch('keepercommander.commands.pam.vault_target.place_record_in_folder') as place_record: + record_add.return_value.execute.return_value = 'record_uid' + + uid = execute_record_add_in_folder(params, args, 'root_nsf', command='pam-project-import') + + assert uid == 'record_uid' + record_add.return_value.execute.assert_called_once() + assert 'folder' not in record_add.return_value.execute.call_args.kwargs + place_record.assert_called_once_with(params, 'record_uid', 'root_nsf', command='pam-project-import') + + +def test_record_v3_add_helper_creates_at_root_then_moves_for_nsf_folder(): + params = _params() + args = { + 'folder': 'root_nsf', + 'data': '{"type":"pamUser","title":"Admin","fields":[]}', + } + + with patch('keepercommander.commands.recordv3.RecordAddCommand') as record_add, \ + patch('keepercommander.commands.pam.vault_target.place_record_in_folder') as place_record: + record_add.return_value.execute.return_value = 'record_uid' + + uid = execute_record_v3_add_in_folder(params, args, 'root_nsf', command='pam-project-import') + + assert uid == 'record_uid' + record_add.return_value.execute.assert_called_once() + assert 'folder' not in record_add.return_value.execute.call_args.kwargs + place_record.assert_called_once_with(params, 'record_uid', 'root_nsf', command='pam-project-import') + + +def test_is_nested_share_folder_detects_reconstructed_subfolder_cache(): + params = _params() + params.nested_share_folders = {} + params.subfolder_cache['root_nsf'] = { + 'folder_uid': 'root_nsf', + 'type': 'user_folder', + 'source': 'nested_share_folder', + } + + assert is_nested_share_folder(params, 'root_nsf') is True + + +def test_import_record_objects_use_nsf_aware_record_add_helper(): + params = _params() + user = PamUserObject.load({'type': 'pamUser', 'title': 'Admin', 'login': 'admin', 'password': 'password'}) + + with patch('keepercommander.commands.pam_import.base.execute_record_add_in_folder', + return_value='record_uid') as add_record: + uid = user.create_record(params, 'root_nsf') + + 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'} + + +def test_process_folders_uses_existing_nsf_root_and_creates_nsf_children(): + params = _params() + project = { + 'options': { + 'project_name': 'Project 1', + 'dry_run': False, + 'use_nsf': True, + }, + 'data': {}, + } + 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, + } + return {'success': True, 'folder_uid': uid} + + with patch('keepercommander.nested_share_folder.folder_api.create_folder_v3', + side_effect=create_folder) as create_folder_v3, \ + patch('keepercommander.commands.pam_import.edit.api.sync_down'): + result = PAMProjectImportCommand().process_folders(params, project) + + assert result['root_folder_uid'] == 'root_nsf' + 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 created == [ + ('Project 1', 'root_nsf', 'nsf_1'), + ('Project 1 - Resources', 'nsf_1', 'nsf_2'), + ('Project 1 - Users', 'nsf_1', 'nsf_3'), + ] + + +def test_process_folders_with_nsf_flag_ignores_legacy_root_folder(): + params = _params() + params.nested_share_folders = {} + params.folder_cache['legacy_root'] = SimpleNamespace( + uid='legacy_root', + parent_uid=None, + name=PAMProjectImportCommand.PAM_ROOT_FOLDER_NAME, + type=BaseFolderNode.UserFolderType, + UserFolderType=BaseFolderNode.UserFolderType, + SharedFolderType=BaseFolderNode.SharedFolderType, + ) + project = { + 'options': { + 'project_name': 'Project 1', + 'dry_run': False, + 'use_nsf': True, + }, + 'data': {}, + } + 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, + } + return {'success': True, 'folder_uid': uid} + + with patch('keepercommander.nested_share_folder.folder_api.create_folder_v3', + side_effect=create_folder), \ + patch('keepercommander.commands.pam_import.edit.api.sync_down'): + result = PAMProjectImportCommand().process_folders(params, project) + + assert result['root_folder_uid'] == 'nsf_1' + assert result['project_folder_uid'] == 'nsf_2' + assert result['resources_folder_uid'] == 'nsf_3' + assert result['users_folder_uid'] == 'nsf_4' + assert created[0] == (PAMProjectImportCommand.PAM_ROOT_FOLDER_NAME, None, 'nsf_1') + assert result['root_folder_uid'] != 'legacy_root' + + +def test_nsf_folder_permissions_route_to_grant_folder_access(): + params = _params() + params.nested_share_folders['child_nsf'] = {'name': 'Users', 'parent_uid': 'root_nsf'} + + with patch('keepercommander.commands.nested_share_folder.helpers.classify_share_recipient', + return_value=('user', 'user@example.com')), \ + patch('keepercommander.nested_share_folder.folder_api.grant_folder_access_v3', + return_value={'success': True}) as grant: + grant_pam_folder_permissions( + params, + 'child_nsf', + [{'name': 'user@example.com', 'manage_users': True, 'manage_records': True}], + command='pam-project-import', + ) + + grant.assert_called_once_with( + params, + 'child_nsf', + 'user@example.com', + role='full-manager', + as_team=False, + ) diff --git a/unit-tests/pam/test_pam_split_nsf.py b/unit-tests/pam/test_pam_split_nsf.py new file mode 100644 index 000000000..7ba568c90 --- /dev/null +++ b/unit-tests/pam/test_pam_split_nsf.py @@ -0,0 +1,117 @@ +import unittest +from unittest import mock + +import keepercommander.commands.record # noqa: F401 +from keepercommander import vault +from keepercommander.commands.tunnel_and_connections import PAMSplitCommand +from keepercommander.params import LAST_RECORD_UID +from keepercommander.subfolder import NestedShareFolderNode, RootFolderNode + + +def _make_params(): + params = mock.MagicMock() + params.folder_cache = {} + params.subfolder_record_cache = {} + params.shared_folder_cache = {} + params.nested_share_folders = {} + params.root_folder = RootFolderNode() + params.current_folder = '' + params.environment_variables = {} + params.sync_data = False + return params + + +def _make_pam_machine(): + record = vault.TypedRecord() + record.record_uid = 'machine_uid' + record.type_name = 'pamMachine' + record.title = 'Machine' + record.fields.append(vault.TypedField.new_field('login', 'admin')) + record.fields.append(vault.TypedField.new_field('password', 'password')) + return record + + +def _make_pam_user(): + record = vault.TypedRecord() + record.record_uid = 'user_uid' + record.type_name = 'pamUser' + record.fields.append(vault.TypedField.new_field('login', '')) + record.fields.append(vault.TypedField.new_field('password', '')) + record.fields.append(vault.TypedField.new_field('secret', '')) + return record + + +class TestPamSplitNsfPlacement(unittest.TestCase): + + @mock.patch('builtins.print') + @mock.patch('keepercommander.commands.tunnel_and_connections.TunnelDAG') + @mock.patch('keepercommander.commands.tunnel_and_connections.get_keeper_tokens', + return_value=(b'encrypted_session', b'encrypted_key', b'transmission_key')) + @mock.patch('keepercommander.commands.tunnel_and_connections.api.sync_down') + @mock.patch('keepercommander.commands.tunnel_and_connections.record_management.update_record') + @mock.patch('keepercommander.commands.pam.vault_target.create_record_in_folder') + @mock.patch('keepercommander.commands.pam.vault_target.resolve_pam_folder_uid', + return_value='nsf_folder') + @mock.patch('keepercommander.commands.tunnel_and_connections.vault.KeeperRecord.create') + @mock.patch('keepercommander.commands.tunnel_and_connections.vault.KeeperRecord.load') + @mock.patch('keepercommander.commands.tunnel_and_connections.resolve_pam_config', + return_value='config_uid') + @mock.patch('keepercommander.commands.tunnel_and_connections.resolve_record', + return_value='machine_uid') + def test_split_resolves_explicit_nsf_folder_name( + self, mock_resolve_record, mock_resolve_config, mock_load, mock_create, + mock_resolve_folder, mock_create_record, mock_update_record, + mock_sync_down, mock_tokens, mock_dag, mock_print): + params = _make_params() + params.environment_variables[LAST_RECORD_UID] = 'user_uid' + mock_load.return_value = _make_pam_machine() + mock_create.return_value = _make_pam_user() + + PAMSplitCommand().execute( + params, pam_machine_record='machine_uid', + pam_config='config_uid', pam_user_folder='testpam') + + mock_resolve_folder.assert_called_once_with( + params, 'testpam', allow_legacy_user=True) + user_record = mock_create_record.call_args.args[1] + self.assertEqual(user_record.type_name, 'pamUser') + mock_create_record.assert_called_once_with( + params, user_record, 'nsf_folder', command='pam-split') + + @mock.patch('builtins.print') + @mock.patch('keepercommander.commands.tunnel_and_connections.TunnelDAG') + @mock.patch('keepercommander.commands.tunnel_and_connections.get_keeper_tokens', + return_value=(b'encrypted_session', b'encrypted_key', b'transmission_key')) + @mock.patch('keepercommander.commands.tunnel_and_connections.api.sync_down') + @mock.patch('keepercommander.commands.tunnel_and_connections.record_management.update_record') + @mock.patch('keepercommander.commands.pam.vault_target.create_record_in_folder') + @mock.patch('keepercommander.commands.tunnel_and_connections.vault.KeeperRecord.create') + @mock.patch('keepercommander.commands.tunnel_and_connections.vault.KeeperRecord.load') + @mock.patch('keepercommander.commands.tunnel_and_connections.resolve_pam_config', + return_value='config_uid') + @mock.patch('keepercommander.commands.tunnel_and_connections.resolve_record', + return_value='machine_uid') + def test_split_defaults_to_machine_nsf_folder( + self, mock_resolve_record, mock_resolve_config, mock_load, mock_create, + mock_create_record, mock_update_record, mock_sync_down, + mock_tokens, mock_dag, mock_print): + params = _make_params() + params.environment_variables[LAST_RECORD_UID] = 'user_uid' + folder = NestedShareFolderNode() + folder.uid = 'nsf_folder' + params.folder_cache[folder.uid] = folder + params.subfolder_record_cache = {'nsf_folder': {'machine_uid'}} + mock_load.return_value = _make_pam_machine() + mock_create.return_value = _make_pam_user() + + PAMSplitCommand().execute( + params, pam_machine_record='machine_uid', + pam_config='config_uid', pam_user_folder='') + + user_record = mock_create_record.call_args.args[1] + mock_create_record.assert_called_once_with( + params, user_record, 'nsf_folder', command='pam-split') + + +if __name__ == '__main__': + unittest.main() From c1ee7a747b5553f46c1814de53a44ef5d9dd11fa Mon Sep 17 00:00:00 2001 From: amangalampalli-ks Date: Tue, 30 Jun 2026 15:41:21 +0530 Subject: [PATCH 02/14] add nsf related pam credential provision and user provision changes --- .../commands/credential_provision.py | 105 ++++------- keepercommander/commands/pam/vault_target.py | 164 +++++++++++++++++- .../pam_cloud/pam_privileged_access.py | 17 +- unit-tests/pam/test_pam_provision_nsf.py | 129 ++++++++++++++ 4 files changed, 329 insertions(+), 86 deletions(-) create mode 100644 unit-tests/pam/test_pam_provision_nsf.py diff --git a/keepercommander/commands/credential_provision.py b/keepercommander/commands/credential_provision.py index b1a9008bd..59682e90c 100644 --- a/keepercommander/commands/credential_provision.py +++ b/keepercommander/commands/credential_provision.py @@ -47,11 +47,12 @@ from .. import api, vault, vault_extensions, crypto, utils, generator from ..error import CommandError from ..params import KeeperParams -from ..subfolder import try_resolve_path, get_folder_path -from ..record_management import add_record_to_folder +from ..subfolder import try_resolve_path from ..record_facades import LoginRecordFacade from ..proto import APIRequest_pb2 from ..commands.folder import FolderMakeCommand +from ..commands.pam.vault_target import ( + create_record_in_folder, records_in_folder, resolve_provision_target_folder) # RecordLink and discoveryrotation require pydantic (Python 3.8+) try: @@ -1313,35 +1314,20 @@ def _check_duplicate_by_username_in_folder(self, config: Dict[str, Any], params: username = config['account']['username'] pam_config_uid = config['account']['pam_config_uid'] - # Get the same folder path that will be used for PAM User creation try: - gateway_folder_uid, gateway_folder_path = self._get_gateway_application_folder(pam_config_uid, params) - except Exception as e: + folder_uid = self._resolve_pam_user_folder_uid(config, params) + except Exception: return False - # Determine target folder (same logic as _create_pam_user) - user_specified_folder = config.get('vault', {}).get('folder') - - if user_specified_folder: - target_folder_path = f"{gateway_folder_path}/{user_specified_folder.strip('/')}" - else: - department = config['user'].get('department', 'Default') - target_folder_path = f"{gateway_folder_path}/PAM Users/{department}" - - # Get folder UID - folder_uid = self._get_folder_uid(target_folder_path, params) - if not folder_uid: return False - # Get all records in this folder - records_in_folder = params.subfolder_record_cache.get(folder_uid, set()) - - if not records_in_folder: + records_in_folder_set = records_in_folder(params, folder_uid) + if not records_in_folder_set: return False # Search for PAM Users in folder with matching username - for record_uid in records_in_folder: + for record_uid in records_in_folder_set: try: record = vault.KeeperRecord.load(params, record_uid) @@ -1358,7 +1344,7 @@ def _check_duplicate_by_username_in_folder(self, config: Dict[str, Any], params: if facade.login == username: logging.error(f'❌ Duplicate PAM User found (by username in folder):') logging.error(f' Username: {username}') - logging.error(f' Folder: {target_folder_path}') + logging.error(f' Folder UID: {folder_uid}') logging.error(f' Existing UID: {record_uid}') logging.error(f' Title: {record.title}') return True @@ -1481,28 +1467,7 @@ def _create_pam_user( username = config['account']['username'] pam_config_uid = config['account']['pam_config_uid'] - # Get gateway application folder from PAM Config - gateway_folder_uid, gateway_folder_path = self._get_gateway_application_folder(pam_config_uid, params) - - # Determine target folder - user_specified_folder = config.get('vault', {}).get('folder') - - if user_specified_folder: - # Check if the value is a folder UID (exists in folder cache) - if user_specified_folder in params.folder_cache: - folder_uid = user_specified_folder - elif user_specified_folder in params.shared_folder_cache: - folder_uid = user_specified_folder - else: - # Treat as a relative folder path - self._validate_folder_path(user_specified_folder) - target_folder_path = f"{gateway_folder_path}/{user_specified_folder.strip('/')}" - folder_uid = self._ensure_folder_exists(target_folder_path, params) - else: - # Auto-generate subfolder based on department - department = config['user'].get('department', 'Default') - target_folder_path = f"{gateway_folder_path}/PAM Users/{department}" - folder_uid = self._ensure_folder_exists(target_folder_path, params) + folder_uid = self._resolve_pam_user_folder_uid(config, params) # Create PAM User typed record pam_user = vault.TypedRecord() @@ -1576,7 +1541,7 @@ def _create_pam_user( try: # Create record in vault and add to folder - add_record_to_folder(params, pam_user, folder_uid) + create_record_in_folder(params, pam_user, folder_uid, command='credential-provision') # Sync to get the record UID api.sync_down(params) @@ -1619,6 +1584,20 @@ def _validate_folder_path(self, folder_path: str) -> None: f"Example: 'PAM Users/Engineering' (not '/Shared Folders/...')" ) + def _resolve_pam_user_folder_uid(self, config: Dict[str, Any], params: KeeperParams) -> str: + pam_config_uid = config['account']['pam_config_uid'] + user_specified_folder = config.get('vault', {}).get('folder') + if user_specified_folder: + self._validate_folder_path(user_specified_folder) + department = config.get('user', {}).get('department', 'Default') + return resolve_provision_target_folder( + params, + pam_config_uid, + folder_spec=user_specified_folder, + department=department, + command='credential-provision', + ) + def _get_gateway_application_folder(self, pam_config_uid: str, params: KeeperParams) -> Tuple[str, str]: """ Get gateway application folder from PAM Configuration. @@ -1636,35 +1615,11 @@ def _get_gateway_application_folder(self, pam_config_uid: str, params: KeeperPar Raises: ValueError: If PAM Config not found or folder not accessible """ - - # Load PAM Config record - pam_config_record = vault.KeeperRecord.load(params, pam_config_uid) - if not pam_config_record: - raise ValueError(f"PAM Configuration not found: {pam_config_uid}") - - # Use facade to access folder_uid - facade = PamConfigurationRecordFacade() - facade.record = pam_config_record - facade.load_typed_fields() - - folder_uid = facade.folder_uid - - if not folder_uid: - raise ValueError( - f"PAM Configuration '{pam_config_record.title}' has no application folder configured.\n" - f"Please configure the application folder in the PAM Configuration record." - ) - - # Get folder path from folder_uid using existing utility - if folder_uid not in params.folder_cache: - raise ValueError( - f"Application folder (UID: {folder_uid}) not found in vault.\n" - f"Ensure the folder is shared with you." - ) - - folder_path = get_folder_path(params, folder_uid) - - return folder_uid, folder_path + from ..commands.pam.vault_target import resolve_pam_application_folder + try: + return resolve_pam_application_folder(params, pam_config_uid, command='credential-provision') + except CommandError as exc: + raise ValueError(str(exc)) from exc def _ensure_folder_exists(self, folder_path: str, params: KeeperParams) -> Optional[str]: """ diff --git a/keepercommander/commands/pam/vault_target.py b/keepercommander/commands/pam/vault_target.py index 7b5713614..5ad835b58 100644 --- a/keepercommander/commands/pam/vault_target.py +++ b/keepercommander/commands/pam/vault_target.py @@ -1,8 +1,9 @@ from ..folder import FolderMoveCommand -from ... import api, record_management +from ... import api, record_management, vault from ...error import CommandError from ...nested_share_folder.folder_record_api import move_record_v3 from ...subfolder import BaseFolderNode, get_folder_path, try_resolve_path +from .config_facades import PamConfigurationRecordFacade def is_nested_share_folder(params, folder_uid): @@ -49,6 +50,167 @@ def resolve_pam_folder_uid(params, folder_name, allow_legacy_user=False): return None +def pam_folder_exists(params, folder_uid): + if not folder_uid: + return False + if folder_uid in getattr(params, 'folder_cache', {}): + return True + if folder_uid in getattr(params, 'nested_share_folders', {}): + return True + subfolder = getattr(params, 'subfolder_cache', {}).get(folder_uid) + return isinstance(subfolder, dict) and subfolder.get('source') == 'nested_share_folder' + + +def get_pam_folder_path(params, folder_uid, delimiter='/'): + if not folder_uid: + return '' + + parts = [] + uid = folder_uid + seen = set() + while uid and uid not in seen: + seen.add(uid) + folder = getattr(params, 'folder_cache', {}).get(uid) + if folder is not None: + name = str(folder.name or '').replace(delimiter, delimiter * 2) + parts.insert(0, name) + uid = folder.parent_uid + continue + + nsf = getattr(params, 'nested_share_folders', {}).get(uid) + if nsf: + name = str(nsf.get('name', '')).replace(delimiter, delimiter * 2) + parts.insert(0, name) + uid = nsf.get('parent_uid') + continue + break + + if parts: + return delimiter.join(parts) + return get_folder_path(params, folder_uid, delimiter=delimiter) + + +def resolve_pam_application_folder(params, pam_config_uid, command='pam'): + pam_config_record = vault.KeeperRecord.load(params, pam_config_uid) + if not pam_config_record: + raise CommandError(command, f'PAM Configuration not found: {pam_config_uid}') + + facade = PamConfigurationRecordFacade() + facade.record = pam_config_record + facade.load_typed_fields() + folder_uid = facade.folder_uid + if not folder_uid: + raise CommandError( + command, + f"PAM Configuration '{pam_config_record.title}' has no application folder configured.\n" + f'Please configure the application folder in the PAM Configuration record.') + + if not pam_folder_exists(params, folder_uid): + raise CommandError( + command, + f'Application folder (UID: {folder_uid}) not found in vault.\n' + f'Ensure the folder is shared with you.') + + return folder_uid, get_pam_folder_path(params, folder_uid) + + +def find_nsf_users_subfolder(params, parent_uid): + if not parent_uid: + return None + + for uid, folder in getattr(params, 'nested_share_folders', {}).items(): + if folder.get('parent_uid') == parent_uid and str(folder.get('name', '')).endswith(' - Users'): + return uid + + for uid, folder in getattr(params, 'folder_cache', {}).items(): + if folder.parent_uid == parent_uid and str(folder.name or '').endswith(' - Users'): + return uid + + return None + + +def records_in_folder(params, folder_uid): + records = set(getattr(params, 'subfolder_record_cache', {}).get(folder_uid, set()) or set()) + nsf_records = getattr(params, 'nested_share_folder_records', {}) + if folder_uid in nsf_records: + records.update(nsf_records[folder_uid]) + return records + + +def resolve_provision_target_folder(params, pam_config_uid, folder_spec=None, department='Default', + command='pam'): + app_uid, app_path = resolve_pam_application_folder(params, pam_config_uid, command=command) + + if folder_spec: + folder_spec = str(folder_spec).strip() + resolved = resolve_pam_folder_uid(params, folder_spec, allow_legacy_user=True) + if resolved: + return resolved + if folder_spec in getattr(params, 'shared_folder_cache', {}): + return folder_spec + relative = folder_spec.strip('/') + if is_nested_share_folder(params, app_uid): + return ensure_pam_folder_path(params, app_uid, relative, command=command) + if app_path: + return _ensure_legacy_folder_path(params, f'{app_path}/{relative}', command=command) + return _ensure_legacy_folder_path(params, relative, command=command) + + if is_nested_share_folder(params, app_uid): + users_uid = find_nsf_users_subfolder(params, app_uid) + if users_uid: + return users_uid + return ensure_pam_folder_path(params, app_uid, f'PAM Users/{department}', command=command) + + target_path = f'{app_path}/PAM Users/{department}' if app_path else f'PAM Users/{department}' + return _ensure_legacy_folder_path(params, target_path, command=command) + + +def resolve_access_user_save_folder(params, config_uid, folder_spec=None, command='pam-access-user-provision'): + if folder_spec: + folder_uid = resolve_pam_folder_uid(params, folder_spec, allow_legacy_user=True) + if not folder_uid: + raise CommandError(command, f'Folder not found: {folder_spec}') + return folder_uid + + app_uid, _ = resolve_pam_application_folder(params, config_uid, command=command) + if is_nested_share_folder(params, app_uid): + return find_nsf_users_subfolder(params, app_uid) or app_uid + return app_uid + + +def _ensure_legacy_folder_path(params, folder_path, command='pam'): + folder_uid = None + rs = try_resolve_path(params, folder_path) + if rs: + folder_node, remainder = rs + if folder_node and not remainder: + folder_uid = folder_node.uid + + if folder_uid: + return folder_uid + + from ..folder import FolderMakeCommand + components = [x.strip() for x in str(folder_path or '').replace('\\', '/').split('/') if x.strip()] + current_path = '' + for i, component in enumerate(components): + current_path = f'{current_path}/{component}' if current_path else component + rs = try_resolve_path(params, current_path) + if rs and rs[0] and not rs[1]: + continue + FolderMakeCommand().execute( + params, + folder=current_path, + shared_folder=(i == 0), + user_folder=(i != 0), + ) + api.sync_down(params) + + rs = try_resolve_path(params, folder_path) + if rs and rs[0] and not rs[1]: + return rs[0].uid + raise CommandError(command, f'Folder path creation succeeded but final UID not found: {folder_path}') + + def _find_child_folder_uid(params, parent_uid, name): parent_uid = parent_uid or None for uid, folder in getattr(params, 'folder_cache', {}).items(): diff --git a/keepercommander/commands/pam_cloud/pam_privileged_access.py b/keepercommander/commands/pam_cloud/pam_privileged_access.py index ca152419e..768302c17 100644 --- a/keepercommander/commands/pam_cloud/pam_privileged_access.py +++ b/keepercommander/commands/pam_cloud/pam_privileged_access.py @@ -14,10 +14,11 @@ GatewayActionIdpGroupList, ) from keepercommander.commands.pam.router_helper import router_send_action_to_gateway +from keepercommander.commands.pam.vault_target import ( + create_record_in_folder, resolve_access_user_save_folder) from keepercommander.error import CommandError -from keepercommander import api, crypto, record_management, vault +from keepercommander import api, crypto, vault from keepercommander.proto import pam_pb2 -from keepercommander.subfolder import find_parent_top_folder logger = logging.getLogger(__name__) @@ -202,7 +203,7 @@ class PAMAccessUserProvisionCommand(Command): parser.add_argument('--save-record', '-s', dest='save_record', action='store_true', help='Save provisioned credentials as a pamUser record') parser.add_argument('--folder', '-f', dest='folder_uid', - help='Folder UID to save the record in (used with --save-record)') + help='Folder UID, name, or path to save the record in (used with --save-record)') parser.add_argument('--gateway', '-g', dest='gateway', help='Gateway UID or name') @@ -304,14 +305,10 @@ def execute(self, params, **kwargs): user_id_label = idp_label_map.get(idp_type, 'IdP User ID') record.custom.append(vault.TypedField.new_field('text', user_id, user_id_label)) - folder_uid = kwargs.get('folder_uid') - if not folder_uid: - shared_folders = find_parent_top_folder(params, config_uid) - if shared_folders: - sf = shared_folders[0] - folder_uid = sf.parent_uid if sf.parent_uid else sf.uid + folder_uid = resolve_access_user_save_folder( + params, config_uid, folder_spec=kwargs.get('folder_uid')) - record_management.add_record_to_folder(params, record, folder_uid) + create_record_in_folder(params, record, folder_uid, command='pam-access-user-provision') params.sync_data = True print(f' Record UID: {record.record_uid}') diff --git a/unit-tests/pam/test_pam_provision_nsf.py b/unit-tests/pam/test_pam_provision_nsf.py new file mode 100644 index 000000000..6ede8dc4f --- /dev/null +++ b/unit-tests/pam/test_pam_provision_nsf.py @@ -0,0 +1,129 @@ +import unittest +from unittest import mock + +import keepercommander.commands.record # noqa: F401 +from keepercommander import vault +from keepercommander.commands.credential_provision import CredentialProvisionCommand +from keepercommander.commands.pam.vault_target import ( + create_record_in_folder, resolve_access_user_save_folder, resolve_provision_target_folder) +from keepercommander.commands.pam_cloud.pam_privileged_access import PAMAccessUserProvisionCommand +from keepercommander.error import CommandError +from keepercommander.subfolder import NestedShareFolderNode, RootFolderNode, SharedFolderNode + + +def _make_params(): + params = mock.MagicMock() + params.folder_cache = {} + params.shared_folder_cache = {} + params.nested_share_folders = {} + params.nested_share_folder_records = {} + params.subfolder_record_cache = {} + params.current_folder = '' + params.root_folder = RootFolderNode() + params.environment_variables = {} + params.sync_data = False + params.record_cache = {} + return params + + +class TestPamProvisionNsf(unittest.TestCase): + + @mock.patch('keepercommander.commands.pam.vault_target.PamConfigurationRecordFacade') + @mock.patch('keepercommander.commands.pam.vault_target.vault.KeeperRecord.load') + def test_resolve_access_user_save_folder_uses_users_subfolder(self, mock_load, mock_facade_cls): + params = _make_params() + params.nested_share_folders = { + 'users_uid': {'name': 'Project 1 - Users', 'parent_uid': 'app_uid'}, + } + params.folder_cache['app_uid'] = NestedShareFolderNode() + params.folder_cache['app_uid'].uid = 'app_uid' + + facade = mock_facade_cls.return_value + facade.folder_uid = 'app_uid' + + folder_uid = resolve_access_user_save_folder(params, 'config_uid') + self.assertEqual(folder_uid, 'users_uid') + + @mock.patch('keepercommander.commands.pam_cloud.pam_privileged_access.create_record_in_folder') + @mock.patch('keepercommander.commands.pam_cloud.pam_privileged_access.resolve_access_user_save_folder', + return_value='nsf_users_uid') + def test_access_user_provision_save_record_uses_create_record_in_folder(self, _resolve, mock_create): + params = _make_params() + cmd = PAMAccessUserProvisionCommand() + + with mock.patch('keepercommander.commands.pam_cloud.pam_privileged_access.resolve_pam_idp_config', + return_value='idp_uid'), \ + mock.patch('keepercommander.commands.pam_cloud.pam_privileged_access._get_record_key', + return_value=b'0' * 32), \ + mock.patch('keepercommander.commands.pam_cloud.pam_privileged_access._dispatch_idp_action', + return_value={'data': {'username': 'user@example.com', 'password': 'secret'}}): + cmd.execute( + params, + config_uid='config_uid', + username='user@example.com', + save_record=True, + ) + + mock_create.assert_called_once() + self.assertEqual(mock_create.call_args.args[2], 'nsf_users_uid') + self.assertEqual(mock_create.call_args.kwargs['command'], 'pam-access-user-provision') + + @mock.patch('keepercommander.commands.pam.vault_target.ensure_pam_folder_path', return_value='target_uid') + @mock.patch('keepercommander.commands.pam.vault_target.resolve_pam_application_folder', + return_value=('app_uid', 'Project 1')) + def test_resolve_provision_target_folder_uses_nsf_path_for_relative_folder(self, _app, mock_ensure): + params = _make_params() + params.nested_share_folders['app_uid'] = {'name': 'Project 1', 'parent_uid': None} + params.folder_cache['app_uid'] = NestedShareFolderNode() + params.folder_cache['app_uid'].uid = 'app_uid' + + folder_uid = resolve_provision_target_folder( + params, 'config_uid', folder_spec='Onboarding/Team A', command='credential-provision') + self.assertEqual(folder_uid, 'target_uid') + mock_ensure.assert_called_once_with( + params, 'app_uid', 'Onboarding/Team A', command='credential-provision') + + @mock.patch('keepercommander.commands.credential_provision.api.sync_down') + @mock.patch('keepercommander.commands.credential_provision.create_record_in_folder') + @mock.patch.object(CredentialProvisionCommand, '_resolve_pam_user_folder_uid', return_value='nsf_users_uid') + def test_credential_provision_create_pam_user_uses_create_record_in_folder(self, _resolve, mock_create, _sync): + params = _make_params() + cmd = CredentialProvisionCommand() + config = { + 'account': {'username': 'svc-user', 'pam_config_uid': 'config_uid'}, + 'user': {'first_name': 'Svc', 'last_name': 'User', 'department': 'Eng'}, + } + + def _assign_uid(params, record, folder_uid, command='credential-provision'): + record.record_uid = 'new_uid' + + mock_create.side_effect = _assign_uid + uid = cmd._create_pam_user(config, 'password123', params) + self.assertEqual(uid, 'new_uid') + mock_create.assert_called_once() + self.assertEqual(mock_create.call_args.args[2], 'nsf_users_uid') + self.assertEqual(mock_create.call_args.kwargs['command'], 'credential-provision') + + @mock.patch('keepercommander.commands.pam.vault_target.record_management.add_record_to_folder') + def test_create_record_in_folder_for_access_user_provision_nsf(self, mock_add): + params = _make_params() + folder = NestedShareFolderNode() + folder.uid = 'nsf_folder' + params.folder_cache[folder.uid] = folder + record = vault.TypedRecord() + record.record_uid = 'record_uid' + + with mock.patch('keepercommander.commands.pam.vault_target.place_record_in_folder') as mock_place: + create_record_in_folder(params, record, folder.uid, command='pam-access-user-provision') + mock_add.assert_called_once_with(params, record) + mock_place.assert_called_once_with( + params, 'record_uid', 'nsf_folder', command='pam-access-user-provision') + + def test_resolve_access_user_save_folder_rejects_unknown_folder(self): + params = _make_params() + with self.assertRaises(CommandError): + resolve_access_user_save_folder(params, 'config_uid', folder_spec='missing-folder') + + +if __name__ == '__main__': + unittest.main() From 9c91b0655700534047ac2a339e37c50dc8dca7b4 Mon Sep 17 00:00:00 2001 From: amangalampalli-ks Date: Tue, 30 Jun 2026 16:02:55 +0530 Subject: [PATCH 03/14] Fix credential provision test cases --- keepercommander/commands/pam/vault_target.py | 17 ++++++++++++-- unit-tests/pam/test_pam_nsf_config.py | 22 ++++++++++++------ unit-tests/pam/test_pam_provision_nsf.py | 24 +++++++++++++------- 3 files changed, 46 insertions(+), 17 deletions(-) diff --git a/keepercommander/commands/pam/vault_target.py b/keepercommander/commands/pam/vault_target.py index 5ad835b58..48c1574c0 100644 --- a/keepercommander/commands/pam/vault_target.py +++ b/keepercommander/commands/pam/vault_target.py @@ -284,8 +284,21 @@ def place_record_in_folder(params, record_uid, folder_uid, command='pam'): def create_record_in_folder(params, record, folder_uid=None, command='pam'): if folder_uid and is_nested_share_folder(params, folder_uid): - record_management.add_record_to_folder(params, record) - place_record_in_folder(params, record.record_uid, folder_uid, command=command) + from ... import vault_extensions + from ...nested_share_folder.record_api import create_record_v3 + + if not isinstance(record, vault.TypedRecord): + raise CommandError(command, 'Nested Share Folder record creation requires a typed record') + + result = create_record_v3( + params, + folder_uid=folder_uid, + record_data=vault_extensions.extract_typed_record_data(record), + ) + if not result.get('success'): + raise CommandError(command, result.get('message') or 'Failed to create record in Nested Share Folder') + record.record_uid = result['record_uid'] + api.sync_down(params) else: record_management.add_record_to_folder(params, record, folder_uid) diff --git a/unit-tests/pam/test_pam_nsf_config.py b/unit-tests/pam/test_pam_nsf_config.py index 37fbccf6a..3b3b7a989 100644 --- a/unit-tests/pam/test_pam_nsf_config.py +++ b/unit-tests/pam/test_pam_nsf_config.py @@ -86,21 +86,29 @@ def test_create_record_in_folder_uses_legacy_add_for_legacy_folder(self, mock_ad mock_add_record.assert_called_once_with(params, record, 'legacy_folder') - @mock.patch('keepercommander.commands.pam.vault_target.place_record_in_folder') - @mock.patch('keepercommander.commands.pam.vault_target.record_management.add_record_to_folder') - def test_create_record_in_folder_creates_root_then_places_for_nsf_folder(self, mock_add_record, mock_place): + @mock.patch('keepercommander.commands.pam.vault_target.api.sync_down') + @mock.patch('keepercommander.nested_share_folder.record_api.create_record_v3', + return_value={'success': True, 'record_uid': 'nsf_record_uid'}) + @mock.patch('keepercommander.vault_extensions.extract_typed_record_data', + return_value={'type': 'pamUser', 'title': 'Test', 'fields': [], 'custom': []}) + def test_create_record_in_folder_uses_v3_add_for_nsf_folder(self, _extract, mock_create, _sync): params = _make_params() + params.nested_share_folders['nsf_folder'] = { + 'name': 'Project - Users', + 'parent_uid': 'app_uid', + 'folder_key_unencrypted': b'0' * 32, + } folder = NestedShareFolderNode() folder.uid = 'nsf_folder' params.folder_cache[folder.uid] = folder record = vault.TypedRecord() - record.record_uid = 'record_uid' create_record_in_folder(params, record, folder.uid, command='pam-access-user-provision') - mock_add_record.assert_called_once_with(params, record) - mock_place.assert_called_once_with( - params, 'record_uid', 'nsf_folder', command='pam-access-user-provision') + mock_create.assert_called_once() + self.assertEqual(mock_create.call_args.kwargs['folder_uid'], 'nsf_folder') + self.assertEqual(record.record_uid, 'nsf_record_uid') + _sync.assert_called_once_with(params) def test_resolve_pam_folder_uid_finds_root_nsf_folder_by_name(self): params = _make_params() diff --git a/unit-tests/pam/test_pam_provision_nsf.py b/unit-tests/pam/test_pam_provision_nsf.py index 6ede8dc4f..7a46e414f 100644 --- a/unit-tests/pam/test_pam_provision_nsf.py +++ b/unit-tests/pam/test_pam_provision_nsf.py @@ -104,20 +104,28 @@ def _assign_uid(params, record, folder_uid, command='credential-provision'): self.assertEqual(mock_create.call_args.args[2], 'nsf_users_uid') self.assertEqual(mock_create.call_args.kwargs['command'], 'credential-provision') - @mock.patch('keepercommander.commands.pam.vault_target.record_management.add_record_to_folder') - def test_create_record_in_folder_for_access_user_provision_nsf(self, mock_add): + @mock.patch('keepercommander.commands.pam.vault_target.api.sync_down') + @mock.patch('keepercommander.nested_share_folder.record_api.create_record_v3', + return_value={'success': True, 'record_uid': 'nsf_record_uid'}) + @mock.patch('keepercommander.vault_extensions.extract_typed_record_data', + return_value={'type': 'pamUser', 'title': 'Test', 'fields': [], 'custom': []}) + def test_create_record_in_folder_for_access_user_provision_nsf(self, _extract, mock_create, _sync): params = _make_params() + params.nested_share_folders['nsf_folder'] = { + 'name': 'Project - Users', + 'parent_uid': 'app_uid', + 'folder_key_unencrypted': b'0' * 32, + } folder = NestedShareFolderNode() folder.uid = 'nsf_folder' params.folder_cache[folder.uid] = folder record = vault.TypedRecord() - record.record_uid = 'record_uid' - with mock.patch('keepercommander.commands.pam.vault_target.place_record_in_folder') as mock_place: - create_record_in_folder(params, record, folder.uid, command='pam-access-user-provision') - mock_add.assert_called_once_with(params, record) - mock_place.assert_called_once_with( - params, 'record_uid', 'nsf_folder', command='pam-access-user-provision') + create_record_in_folder(params, record, folder.uid, command='pam-access-user-provision') + + mock_create.assert_called_once() + self.assertEqual(mock_create.call_args.kwargs['folder_uid'], 'nsf_folder') + self.assertEqual(record.record_uid, 'nsf_record_uid') def test_resolve_access_user_save_folder_rejects_unknown_folder(self): params = _make_params() From 80116cc85c418801fafb1a2791d39743e6e3ed17 Mon Sep 17 00:00:00 2001 From: amangalampalli-ks Date: Thu, 2 Jul 2026 12:58:40 +0530 Subject: [PATCH 04/14] Update texts for rotation script restriction. --- keepercommander/commands/discoveryrotation.py | 7 ++++--- keepercommander/commands/discoveryrotation_v1.py | 7 ++++--- keepercommander/commands/pam_import/README.md | 3 ++- 3 files changed, 10 insertions(+), 7 deletions(-) diff --git a/keepercommander/commands/discoveryrotation.py b/keepercommander/commands/discoveryrotation.py index 9073dd185..fc925bccf 100644 --- a/keepercommander/commands/discoveryrotation.py +++ b/keepercommander/commands/discoveryrotation.py @@ -3230,8 +3230,8 @@ class PAMRouterScriptCommand(GroupCommand): def __init__(self): super().__init__() self.register_command('list', PAMScriptListCommand(), 'List script fields') - self.register_command('add', PAMScriptAddCommand(), 'List Record Rotation Schedulers') - self.register_command('edit', PAMScriptEditCommand(), 'Add, delete, or edit script field') + self.register_command('add', PAMScriptAddCommand(), 'Add script to record (record owner only)') + self.register_command('edit', PAMScriptEditCommand(), 'Edit script field') self.register_command('delete', PAMScriptDeleteCommand(), 'Delete script field') self.default_verb = 'list' @@ -3274,7 +3274,8 @@ def execute(self, params, **kwargs): class PAMScriptAddCommand(Command): - parser = argparse.ArgumentParser(prog='pam rotate script add', description='Add script to record') + parser = argparse.ArgumentParser(prog='pam rotate script add', + description='Add script to record (record owner only)') parser.add_argument('--script', required=True, dest='script', action='store', help='Script file name') parser.add_argument('--add-credential', dest='add_credential', action='append', diff --git a/keepercommander/commands/discoveryrotation_v1.py b/keepercommander/commands/discoveryrotation_v1.py index d25e0d4b6..cadd5f0c9 100644 --- a/keepercommander/commands/discoveryrotation_v1.py +++ b/keepercommander/commands/discoveryrotation_v1.py @@ -1272,8 +1272,8 @@ class PAMRouterScriptCommand(GroupCommand): def __init__(self): super().__init__() self.register_command('list', PAMScriptListCommand(), 'List script fields') - self.register_command('add', PAMScriptAddCommand(), 'List Record Rotation Schedulers') - self.register_command('edit', PAMScriptEditCommand(), 'Add, delete, or edit script field') + self.register_command('add', PAMScriptAddCommand(), 'Add script to record (record owner only)') + self.register_command('edit', PAMScriptEditCommand(), 'Edit script field') self.register_command('delete', PAMScriptDeleteCommand(), 'Delete script field') self.default_verb = 'list' @@ -1316,7 +1316,8 @@ def execute(self, params, **kwargs): class PAMScriptAddCommand(Command): - parser = argparse.ArgumentParser(prog='pam rotate script add', description='Add script to record') + parser = argparse.ArgumentParser(prog='pam rotate script add', + description='Add script to record (record owner only)') parser.add_argument('--script', required=True, dest='script', action='store', help='Script file name') parser.add_argument('--add-credential', dest='add_credential', action='append', diff --git a/keepercommander/commands/pam_import/README.md b/keepercommander/commands/pam_import/README.md index 78d62be8b..de43eec19 100644 --- a/keepercommander/commands/pam_import/README.md +++ b/keepercommander/commands/pam_import/README.md @@ -302,7 +302,8 @@ Each Machine (pamMachine, pamDatabase, pamDirectory) can specify **Administrativ > **Note 1:** `pam_settings` _(options, connection)_ are explained only in pamMachine section below (per protocol) but they are present in all machine types. > **Note 2:** `attachments` and `scripts` examples are in `pam_configuration: local` section. > **Note 3:** Post rotation scripts (a.k.a. `scripts`) are executed in following order: `pamUser` scripts after any **successful** rotation for that user, `pamMachine` scripts after any **successful** rotation on the machine and `pamConfiguration` scripts after any rotation using that configuration. - > **Note 4:** When `allow_supply_user` is false and JIT ephemeral is not used, vault may require a launch credential; import can provide it via `launch_credentials` in the resource's `connection` block. + > **Note 4:** Post-rotation scripts can only be attached by the **record owner**. Non-owners will see an upload error during import or when using `pam rotate script add`. + > **Note 5:** When `allow_supply_user` is false and JIT ephemeral is not used, vault may require a launch credential; import can provide it via `launch_credentials` in the resource's `connection` block. JIT and KeeperAI settings below are shared across all resource types (pamMachine, pamDatabase, pamDirectory) except User and RBI (pamRemoteBrowser) records. **Workflow** (approvals / checkout / temporal restrictions) is supported on all four resource types: pamMachine, pamDatabase, pamDirectory, **and** pamRemoteBrowser. From 5df8d535a212c9e9e005ea36d24dfdc6d3fa3682 Mon Sep 17 00:00:00 2001 From: adeshmukh-ks Date: Fri, 3 Jul 2026 10:54:56 +0530 Subject: [PATCH 05/14] added nsf pam support to config and rotation commands --- keepercommander/commands/discoveryrotation.py | 230 ++++++++++-------- keepercommander/commands/pam/config_helper.py | 29 ++- keepercommander/commands/pam/vault_target.py | 230 ++++++++++++++++-- 3 files changed, 367 insertions(+), 122 deletions(-) diff --git a/keepercommander/commands/discoveryrotation.py b/keepercommander/commands/discoveryrotation.py index fc925bccf..3e7349288 100644 --- a/keepercommander/commands/discoveryrotation.py +++ b/keepercommander/commands/discoveryrotation.py @@ -24,12 +24,16 @@ from keeper_secrets_manager_core.utils import url_safe_str_to_bytes from .base import (Command, GroupCommand, user_choice, dump_report_data, report_output_parser, - json_output_parser, field_to_title, - FolderMixin, RecordMixin, toggle_pam_legacy_commands) -from .folder import FolderMoveCommand + json_output_parser, field_to_title, toggle_pam_legacy_commands) from .ksm import KSMCommand from .pam import gateway_helper, router_helper from .pam.config_facades import PamConfigurationRecordFacade +from .pam.vault_target import ( + format_pam_folder_display, resolve_pam_folder_uid, + resolve_pam_record, record_exists_in_vault, collect_pam_folder_uids, + get_vault_record_title_type, find_pam_records_by_search, + resolve_pam_config_folder_info, pam_folder_json_payload, place_record_in_folder, +) from .pam.config_helper import configuration_controller_get, \ pam_configurations_get_all, pam_configuration_remove, \ pam_configuration_create_record_v6, record_rotation_get, \ @@ -57,7 +61,7 @@ from ..error import CommandError, KeeperApiError from ..params import KeeperParams, LAST_RECORD_UID from ..proto import pam_pb2, router_pb2, record_pb2, APIRequest_pb2 -from ..subfolder import find_parent_top_folder, try_resolve_path, BaseFolderNode +from ..subfolder import try_resolve_path, BaseFolderNode from ..vault import TypedField from ..discovery_common.record_link import RecordLink from .discover.job_start import PAMGatewayActionDiscoverJobStartCommand @@ -486,8 +490,8 @@ class PAMCreateRecordRotationCommand(Command): record_group.add_argument('--record', '-r', dest='record_name', action='store', help='Record UID, name, or pattern to be rotated manually or via schedule') record_group.add_argument('--folder', '-fd', dest='folder_name', action='store', - help='Used for bulk rotation setup. The folder UID or name that holds records to be ' - 'configured') + help='Used for bulk rotation setup. The folder UID or name (including Nested Share ' + 'Folders) that holds records to be configured') parser.add_argument('--force', '-f', dest='force', action='store_true', help='Do not ask for confirmation') parser.add_argument('--config', '-c', required=False, dest='config', action='store', help='UID or path of the configuration record.') @@ -556,7 +560,7 @@ def config_resource(_dag, target_record, target_config_uid, silent=None): _dag.link_resource_to_config(target_record.record_uid) admin = kwargs.get('admin') - adm_rec = RecordMixin.resolve_single_record(params, kwargs.get('admin', None)) + adm_rec = resolve_pam_record(params, kwargs.get('admin', None), rec_type='pamUser') if adm_rec and isinstance(adm_rec, vault.TypedRecord): admin = adm_rec.record_uid @@ -1378,7 +1382,10 @@ def config_user(_dag, target_record, target_resource_uid, target_config_uid=None encrypted_session_token, encrypted_transmission_key, transmission_key = get_keeper_tokens(params) if record_name: - if record_name in params.record_cache: + rec = resolve_pam_record(params, record_name) + if rec: + record_uids.add(rec.record_uid) + elif record_name in params.record_cache: record_uids.add(record_name) else: rs = try_resolve_path(params, record_name, find_all_matches=True) @@ -1397,24 +1404,11 @@ def config_user(_dag, target_record, target_resource_uid, target_config_uid=None folder_name = kwargs.get('folder_name') if folder_name: - if folder_name in params.folder_cache: - folder_uids.add(folder_name) + collected = collect_pam_folder_uids(params, folder_name) + if collected: + folder_uids.update(collected) else: - rs = try_resolve_path(params, folder_name, find_all_matches=True) - if rs is not None: - folder, record_title = rs - if not record_title: - - def add_folders(sub_folder): # type: (BaseFolderNode) -> None - folder_uids.add(sub_folder.uid or '') - - if isinstance(folder, BaseFolderNode): - folder = [folder] - if isinstance(folder, list): - for f in folder: - FolderMixin.traverse_folder_tree(params, f.uid, add_folders) - else: - logging.warning('Folder \"%s\" not found. Skipping.', folder_name) + logging.warning('Folder \"%s\" not found. Skipping.', folder_name) if record_name and folder_name: raise CommandError('', 'Cannot use both --record and --folder at the same time.') @@ -1458,7 +1452,7 @@ def add_folders(sub_folder): # type: (BaseFolderNode) -> None isinstance(x, vault.TypedRecord)} config_uid = kwargs.get('config') - cfg_rec = RecordMixin.resolve_single_record(params, kwargs.get('config', None)) + cfg_rec = resolve_pam_record(params, kwargs.get('config', None)) if cfg_rec and cfg_rec.version == 6 and cfg_rec.record_uid in pam_configurations: config_uid = cfg_rec.record_uid @@ -1502,7 +1496,7 @@ def add_folders(sub_folder): # type: (BaseFolderNode) -> None pwd_complexity_rule_list = {} resource_uid = kwargs.get('resource') - res_rec = RecordMixin.resolve_single_record(params, kwargs.get('resource', None)) + res_rec = resolve_pam_record(params, kwargs.get('resource', None)) if res_rec and isinstance(res_rec, vault.TypedRecord): resource_uid = res_rec.record_uid @@ -1674,19 +1668,11 @@ def execute(self, params, **kwargs): (poc for poc in enterprise_controllers_connected_uids_bytes if poc == controller_uid)) row_color = '' - if record_uid in params.record_cache: + if record_exists_in_vault(params, record_uid): row_color = bcolors.HIGHINTENSITYWHITE - rec = params.record_cache[record_uid] - - data_json = rec['data_unencrypted'].decode('utf-8') if isinstance(rec['data_unencrypted'], bytes) else \ - rec['data_unencrypted'] - data = json.loads(data_json) - - record_title = data.get('title') - record_type = data.get('type') or '' + record_title, record_type = get_vault_record_title_type(params, record_uid) else: row_color = bcolors.WHITE - record_title = '[record inaccessible]' record_type = '[record inaccessible]' @@ -2254,25 +2240,20 @@ def print_pam_configuration_details(params, config_uid, is_verbose=False, format facade = PamConfigurationRecordFacade() facade.record = configuration - folder_uid = facade.folder_uid - sf = None - if folder_uid in params.shared_folder_cache: - sf = api.get_shared_folder(params, folder_uid) + folder_info = resolve_pam_config_folder_info( + params, facade, configuration.record_uid) if format_type == 'json': config_data = { "uid": configuration.record_uid, "name": configuration.title, "config_type": configuration.record_type, - "shared_folder": { - "name": sf.name if sf else None, - "uid": sf.shared_folder_uid if sf else None - } if sf else None, "gateway_uid": facade.controller_uid, "gateway_name": facade.title, "resource_record_uids": facade.resource_ref, "fields": {} } + config_data.update(pam_folder_json_payload(folder_info)) for field in itertools.chain(configuration.fields, configuration.custom): if field.type in ('pamResources', 'fileRef'): @@ -2301,7 +2282,7 @@ def print_pam_configuration_details(params, config_uid, is_verbose=False, format table.append(['UID', configuration.record_uid]) table.append(['Name', configuration.title]) table.append(['Config Type', configuration.record_type]) - table.append(['Shared Folder', f'{sf.name} ({sf.shared_folder_uid})' if sf else '']) + table.append(['Folder', format_pam_folder_display(folder_info)]) table.append(['Gateway UID', facade.controller_uid]) table.append(['Resource Record UIDs', facade.resource_ref]) @@ -2327,11 +2308,11 @@ def print_root_rotation_setting(params, is_verbose=False, format_type='table'): table = [] if format_type == 'json': - headers = ['uid', 'config_name', 'config_type', 'shared_folder', 'gateway_uid', 'resource_record_uids'] + headers = ['uid', 'config_name', 'config_type', 'folder', 'gateway_uid', 'resource_record_uids'] if is_verbose: headers.append('fields') else: - headers = ['UID', 'Config Name', 'Config Type', 'Shared Folder', 'Gateway UID', 'Resource Record UIDs'] + headers = ['UID', 'Config Name', 'Config Type', 'Folder', 'Gateway UID', 'Resource Record UIDs'] if is_verbose: headers.append('Fields') @@ -2339,22 +2320,20 @@ def print_root_rotation_setting(params, is_verbose=False, format_type='table'): if c.record_type in ('pamAwsConfiguration', 'pamAzureConfiguration', 'pamGcpConfiguration', 'pamDomainConfiguration', 'pamNetworkConfiguration', 'pamOciConfiguration'): facade.record = c - shared_folder_parents = find_parent_top_folder(params, c.record_uid) - if shared_folder_parents: - sf = shared_folder_parents[0] + folder_info = resolve_pam_config_folder_info( + params, facade, c.record_uid) + if folder_info and folder_info.get('type') != 'unknown': + folder_display = format_pam_folder_display(folder_info) if format_type == 'json': config_data = { "uid": c.record_uid, "config_name": c.title, "config_type": c.record_type, - "shared_folder": { - "name": sf.name, - "uid": sf.uid - }, "gateway_uid": facade.controller_uid, "resource_record_uids": facade.resource_ref } + config_data.update(pam_folder_json_payload(folder_info)) if is_verbose: fields = {} @@ -2368,7 +2347,7 @@ def print_root_rotation_setting(params, is_verbose=False, format_type='table'): configs_data.append(config_data) else: - row = [c.record_uid, c.title, c.record_type, f'{sf.name} ({sf.uid})', + row = [c.record_uid, c.title, c.record_type, folder_display, facade.controller_uid, facade.resource_ref] if is_verbose: @@ -2383,7 +2362,7 @@ def print_root_rotation_setting(params, is_verbose=False, format_type='table'): table.append(row) else: - logging.warning('Following configuration is not in the shared folder: UID: %s, Title: %s', + logging.warning(f'Following configuration folder was not found: UID: %s, Title: %s', c.record_uid, c.title) else: logging.warning('Following configuration has unsupported type: UID: %s, Title: %s', c.record_uid, @@ -2403,8 +2382,8 @@ def print_root_rotation_setting(params, is_verbose=False, format_type='table'): common_parser.add_argument('--title', '-t', dest='title', action='store', help='Title of the PAM Configuration') common_parser.add_argument('--gateway', '-g', dest='gateway_uid', action='store', help='Gateway UID or Name') common_parser.add_argument('--shared-folder', '-sf', dest='shared_folder_uid', action='store', - help='Share Folder where this PAM Configuration is stored. Should be one of the folders to ' - 'which the gateway has access to.') + help='Shared Folder or Nested Share Folder where this PAM Configuration is stored. ' + 'Should be one of the folders to which the gateway has access.') common_parser.add_argument('--schedule', '-sc', dest='default_schedule', action='store', help='Default Schedule: Use CRON syntax') common_parser.add_argument('--port-mapping', '-pm', dest='port_mapping', action='append', help='Port Mapping') @@ -2512,7 +2491,6 @@ def parse_pam_configuration(self, params, record, **kwargs): shared_folder_uid = None # type: Optional[str] folder_name = kwargs.get('shared_folder_uid') # type: Optional[str] if folder_name: - from .pam.vault_target import resolve_pam_folder_uid shared_folder_uid = resolve_pam_folder_uid(params, folder_name) if not shared_folder_uid: for sf_uid in params.shared_folder_cache: @@ -2534,9 +2512,23 @@ def parse_pam_configuration(self, params, record, **kwargs): if rrr: pam_record_lookup = {} rti = PamConfigurationEditMixin.get_pam_record_types(params) + rti_set = set(rti) for r in vault_extensions.find_records(params, record_type=rti): pam_record_lookup[r.record_uid] = r.record_uid pam_record_lookup[r.title.lower()] = r.record_uid + nsf_data = getattr(params, 'nested_share_record_data', {}) + for uid in getattr(params, 'nested_share_records', {}): + rec = vault.KeeperRecord.load(params, uid) + if not rec or rec.record_type not in rti_set: + continue + pam_record_lookup[rec.record_uid] = rec.record_uid + rd = nsf_data.get(uid, {}) + data_json = rd.get('data_json', {}) if isinstance(rd, dict) else {} + title = data_json.get('title', '') if isinstance(data_json, dict) else '' + if title: + pam_record_lookup[title.lower()] = rec.record_uid + elif rec.title: + pam_record_lookup[rec.title.lower()] = rec.record_uid record_uids = set() if 'resourceRef' in value: @@ -2557,17 +2549,7 @@ def parse_pam_configuration(self, params, record, **kwargs): @staticmethod def resolve_single_record(params, record_name, rec_type=''): # type: (KeeperParams, str, str) -> Optional[vault.KeeperRecord] - rec = RecordMixin.resolve_single_record(params, record_name) - if not rec: - recs = [] - for rec in params.record_cache: - vrec = vault.KeeperRecord.load(params, rec) - if vrec and vrec.title == record_name and (not rec_type or rec_type == vrec.record_type): - recs.append(vrec) - if len(recs) > 1: break - if len(recs) == 1: - rec = recs[0] - return rec + return resolve_pam_record(params, record_name, rec_type=rec_type or None) @staticmethod def _domain_kwarg_supplied(kwargs, key, config_edit): @@ -2823,11 +2805,14 @@ def execute(self, params, **kwargs): # resolve folder path to UID sf_name = kwargs.get('shared_folder_uid', '') if sf_name: - fpath = try_resolve_path(params, sf_name) - # [-1] == '' -> existing folder, 'path/' -> non-existing folder - if fpath and len(fpath) >= 2 and fpath[-1] == '': - sfuid = fpath[-2].uid - if sfuid: kwargs['shared_folder_uid'] = sfuid + sfuid = resolve_pam_folder_uid(params, sf_name, allow_legacy_user=True) + if not sfuid: + fpath = try_resolve_path(params, sf_name) + # [-1] == '' -> existing folder, 'path/' -> non-existing folder + if fpath and len(fpath) >= 2 and fpath[-1] == '': + sfuid = fpath[-2].uid + if sfuid: + kwargs['shared_folder_uid'] = sfuid self.parse_properties(params, record, **kwargs) @@ -2876,7 +2861,6 @@ def execute(self, params, **kwargs): # Moving v6 record into the folder api.sync_down(params) - from .pam.vault_target import place_record_in_folder place_record_in_folder(params, record.record_uid, shared_folder_uid, command='pam-config-new') params.environment_variables[LAST_RECORD_UID] = record.record_uid @@ -3006,7 +2990,6 @@ def execute(self, params, **kwargs): api.communicate_rest(params, pcc, 'pam/set_configuration_controller') shared_folder_uid = value.get('folderUid') or '' if shared_folder_uid != orig_shared_folder_uid: - from .pam.vault_target import place_record_in_folder place_record_in_folder(params, configuration.record_uid, shared_folder_uid, command='pam-config-edit') if configuration.type_name == 'pamDomainConfiguration' and not kwargs.get('force_domain_admin', False) is True: @@ -3049,25 +3032,55 @@ class PAMConfigurationRemoveCommand(Command): help='PAM Configuration UID. To view all rotation settings with their UIDs, use command ' '`pam config list`') + _PAM_CONFIG_TYPES = frozenset({ + 'pamAwsConfiguration', 'pamAzureConfiguration', 'pamGcpConfiguration', + 'pamDomainConfiguration', 'pamNetworkConfiguration', 'pamOciConfiguration', + }) + def get_parser(self): return PAMConfigurationRemoveCommand.parser + @classmethod + def _resolve_pam_config_uid(cls, params, identifier): + # type: (KeeperParams, str) -> Optional[str] + if identifier in params.record_cache: + rec = vault.KeeperRecord.load(params, identifier) + if isinstance(rec, vault.TypedRecord) and rec.version == 6 and rec.record_type in cls._PAM_CONFIG_TYPES: + return rec.record_uid + + from ..nested_share_folder import removal_api as _nsf + resolved_uid = _nsf.resolve_nested_share_record_uid(params, identifier) + if resolved_uid: + rec = vault.KeeperRecord.load(params, resolved_uid) + if isinstance(rec, vault.TypedRecord) and rec.version == 6 and rec.record_type in cls._PAM_CONFIG_TYPES: + return resolved_uid + + l_name = identifier.casefold() + matches = [] + for config in vault_extensions.find_records(params, record_version=6): + if config.record_type not in cls._PAM_CONFIG_TYPES: + continue + if config.record_uid == identifier: + return config.record_uid + if config.title.casefold() == l_name: + matches.append(config.record_uid) + if len(matches) == 1: + return matches[0] + if len(matches) > 1: + raise CommandError('pam config remove', + f'"{identifier}" matches multiple configurations. Use a UID.') + return None + def execute(self, params, **kwargs): pam_config_name = kwargs.get('uid') if not pam_config_name: - raise CommandError('pam config edit', 'PAM Configuration UID is required') - pam_config_uid = None - for config in vault_extensions.find_records(params, record_version=6): - if config.record_uid == pam_config_name: - pam_config_uid = config.record_uid - break - if config.title.casefold() == pam_config_name.casefold(): - pass - if not pam_config_name: - raise Exception(f'Configuration "{pam_config_name}" not found') + raise CommandError('pam config remove', 'PAM Configuration UID is required') + pam_config_uid = self._resolve_pam_config_uid(params, pam_config_name) + if not pam_config_uid: + raise CommandError('pam config remove', f'Configuration "{pam_config_name}" not found') pam_config = vault.KeeperRecord.load(params, pam_config_uid) if not pam_config: - raise Exception(f'Configuration "{pam_config_uid}" not found') + raise CommandError('pam config remove', f'Configuration "{pam_config_uid}" not found') encrypted_session_token, encrypted_transmission_key, transmission_key = get_keeper_tokens(params) tmp_dag = TunnelDAG(params, encrypted_session_token, encrypted_transmission_key, pam_config.record_uid, is_config=True, transmission_key=transmission_key) @@ -3101,7 +3114,7 @@ def execute(self, params, **kwargs): gateway_uid = utils.base64_url_encode(rri.controllerUid) if rri.controllerUid else '-' def is_resource_ok(resource_id, params, configuration_uid): - if resource_id not in params.record_cache: + if not record_exists_in_vault(params, resource_id): return False configuration = vault.KeeperRecord.load(params, configuration_uid) if not isinstance(configuration, vault.TypedRecord): @@ -3125,6 +3138,8 @@ def is_resource_ok(resource_id, params, configuration_uid): if pwd_complexity_raw: try: record = params.record_cache.get(record_uid) + if not record: + record = getattr(params, 'nested_share_records', {}).get(record_uid) if record: complexity = crypto.decrypt_aes_v2(utils.base64_url_decode(pwd_complexity_raw), record['record_key_unencrypted']) @@ -3249,8 +3264,8 @@ def execute(self, params, **kwargs): table = [] header = ['record_uid', 'title', 'record_type', 'script_uid', 'script_name', 'records', 'command'] - for record in vault_extensions.find_records(params, search_str=pattern, record_version=3, - record_type=('pamUser', 'pamDirectory')): + for record in find_pam_records_by_search(params, pattern, record_version=3, + record_types=('pamUser', 'pamDirectory')): if not isinstance(record, vault.TypedRecord): continue for field in (x for x in record.fields if x.type == 'script'): @@ -3291,8 +3306,8 @@ def execute(self, params, **kwargs): record_name = kwargs.get('record') if not record_name: raise CommandError('rotate script', '"record" argument is required') - records = list(vault_extensions.find_records( - params, search_str=record_name, record_version=3, record_type=('pamUser', 'pamDirectory'))) + records = find_pam_records_by_search(params, record_name, record_version=3, + record_types=('pamUser', 'pamDirectory')) if len(records) == 0: raise CommandError('rotate script', f'Record "{record_name}" not found') if len(records) > 1: @@ -3330,7 +3345,10 @@ def execute(self, params, **kwargs): record_refs = kwargs.get('add_credential') if isinstance(record_refs, list): for ref in record_refs: - if ref in params.record_cache: + resolved_ref = resolve_pam_record(params, ref) + if resolved_ref: + script_value['recordRef'].append(resolved_ref.record_uid) + elif record_exists_in_vault(params, ref): script_value['recordRef'].append(ref) cmd = kwargs.get('script_command') if cmd: @@ -3364,8 +3382,8 @@ def execute(self, params, **kwargs): if not script_name: raise CommandError('rotate script', '"script" argument is required') - records = list(vault_extensions.find_records( - params, search_str=record_name, record_version=3, record_type=('pamUser', 'pamDirectory'))) + records = find_pam_records_by_search(params, record_name, record_version=3, + record_types=('pamUser', 'pamDirectory')) if len(records) == 0: raise CommandError('rotate script', f'Record "{record_name}" not found') if len(records) > 1: @@ -3401,11 +3419,21 @@ def execute(self, params, **kwargs): refs.update(record_refs) remove_credential = kwargs.get('remove_credential') if isinstance(remove_credential, list) and remove_credential: - refs.difference_update(remove_credential) + for ref in remove_credential: + resolved_ref = resolve_pam_record(params, ref) + if resolved_ref: + refs.discard(resolved_ref.record_uid) + else: + refs.discard(ref) modified = True add_credential = kwargs.get('add_credential') if isinstance(add_credential, list) and add_credential: - refs.update(add_credential) + for ref in add_credential: + resolved_ref = resolve_pam_record(params, ref) + if resolved_ref: + refs.add(resolved_ref.record_uid) + elif record_exists_in_vault(params, ref): + refs.add(ref) modified = True if modified: script_value['recordRef'] = list(refs) @@ -3439,8 +3467,8 @@ def execute(self, params, **kwargs): if not script_name: raise CommandError('rotate script', '"script" argument is required') - records = list(vault_extensions.find_records( - params, search_str=record_name, record_version=3, record_type=('pamUser', 'pamDirectory'))) + records = find_pam_records_by_search(params, record_name, record_version=3, + record_types=('pamUser', 'pamDirectory')) if len(records) == 0: raise CommandError('rotate script', f'Record "{record_name}" not found') if len(records) > 1: diff --git a/keepercommander/commands/pam/config_helper.py b/keepercommander/commands/pam/config_helper.py index d2aded194..d04bb87aa 100644 --- a/keepercommander/commands/pam/config_helper.py +++ b/keepercommander/commands/pam/config_helper.py @@ -1,11 +1,9 @@ -import base64 import json import logging import os from keeper_secrets_manager_core.utils import string_to_bytes, bytes_to_string -from ..folder import FolderMoveCommand from ..record import RecordRemoveCommand from ... import api, crypto, utils, vault, vault_extensions from ...params import KeeperParams @@ -162,10 +160,35 @@ def pam_configurations_get_all(params): def pam_configuration_remove(params, configuration_uid): # TODO: Check if there are record rotations associated with this config and warn user about that before removing. - RecordRemoveCommand().execute(params, record=configuration_uid, force=True) + from ..nested_share_folder.helpers import is_nested_share_record + from ... import nested_share_folder as _nsf + from ...subfolder import find_folders + from .config_facades import PamConfigurationRecordFacade + + if is_nested_share_record(params, configuration_uid): + folder_uids = list(find_folders(params, configuration_uid)) + folder_uid = folder_uids[0] if folder_uids else None + if not folder_uid: + config = vault.KeeperRecord.load(params, configuration_uid) + if config and isinstance(config, vault.TypedRecord): + facade = PamConfigurationRecordFacade() + facade.record = config + folder_uid = facade.folder_uid + result = _nsf.remove_record_v3(params, [{ + 'record_uid': configuration_uid, + 'folder_uid': folder_uid, + 'operation_type': 'owner-trash', + }], dry_run=False) + if not result.get('confirmed'): + logging.warning('PAM Configuration NSF removal was not confirmed by the server.') + else: + RecordRemoveCommand().execute(params, record=configuration_uid, force=True) if configuration_uid in params.record_cache: del params.record_cache[configuration_uid] + nested_recs = getattr(params, 'nested_share_records', {}) + if configuration_uid in nested_recs: + del nested_recs[configuration_uid] logging.info('PAM Configuration was removed successfully.') diff --git a/keepercommander/commands/pam/vault_target.py b/keepercommander/commands/pam/vault_target.py index 48c1574c0..4d8b8efae 100644 --- a/keepercommander/commands/pam/vault_target.py +++ b/keepercommander/commands/pam/vault_target.py @@ -1,5 +1,5 @@ from ..folder import FolderMoveCommand -from ... import api, record_management, vault +from ... import api, record_management, vault, vault_extensions from ...error import CommandError from ...nested_share_folder.folder_record_api import move_record_v3 from ...subfolder import BaseFolderNode, get_folder_path, try_resolve_path @@ -28,6 +28,9 @@ def resolve_pam_folder_uid(params, folder_name, allow_legacy_user=False): if folder_name in getattr(params, 'shared_folder_cache', {}): return folder_name + if allow_legacy_user and folder_name in getattr(params, 'folder_cache', {}): + return folder_name + if is_nested_share_folder(params, folder_name): return folder_name @@ -55,10 +58,7 @@ def pam_folder_exists(params, folder_uid): return False if folder_uid in getattr(params, 'folder_cache', {}): return True - if folder_uid in getattr(params, 'nested_share_folders', {}): - return True - subfolder = getattr(params, 'subfolder_cache', {}).get(folder_uid) - return isinstance(subfolder, dict) and subfolder.get('source') == 'nested_share_folder' + return is_nested_share_folder(params, folder_uid) def get_pam_folder_path(params, folder_uid, delimiter='/'): @@ -114,21 +114,36 @@ def resolve_pam_application_folder(params, pam_config_uid, command='pam'): return folder_uid, get_pam_folder_path(params, folder_uid) -def find_nsf_users_subfolder(params, parent_uid): +def _find_child_folder(params, parent_uid, name=None, name_suffix=None): if not parent_uid: return None - for uid, folder in getattr(params, 'nested_share_folders', {}).items(): - if folder.get('parent_uid') == parent_uid and str(folder.get('name', '')).endswith(' - Users'): + parent_uid = parent_uid or None + for uid, folder in getattr(params, 'folder_cache', {}).items(): + if folder.parent_uid != parent_uid: + continue + folder_name = str(folder.name or '') + if name is not None and folder_name == name: + return uid + if name_suffix is not None and folder_name.endswith(name_suffix): return uid - for uid, folder in getattr(params, 'folder_cache', {}).items(): - if folder.parent_uid == parent_uid and str(folder.name or '').endswith(' - Users'): + for uid, folder in getattr(params, 'nested_share_folders', {}).items(): + if folder.get('parent_uid') != parent_uid: + continue + folder_name = str(folder.get('name', '')) + if name is not None and folder_name == name: + return uid + if name_suffix is not None and folder_name.endswith(name_suffix): return uid return None +def find_nsf_users_subfolder(params, parent_uid): + return _find_child_folder(params, parent_uid, name_suffix=' - Users') + + def records_in_folder(params, folder_uid): records = set(getattr(params, 'subfolder_record_cache', {}).get(folder_uid, set()) or set()) nsf_records = getattr(params, 'nested_share_folder_records', {}) @@ -211,19 +226,198 @@ def _ensure_legacy_folder_path(params, folder_path, command='pam'): raise CommandError(command, f'Folder path creation succeeded but final UID not found: {folder_path}') -def _find_child_folder_uid(params, parent_uid, name): - parent_uid = parent_uid or None - for uid, folder in getattr(params, 'folder_cache', {}).items(): - if folder.parent_uid == parent_uid and folder.name == name: - return uid +def get_pam_folder_info(params, folder_uid): + # type: (...) -> Optional[dict] + """Return folder name, uid, and type for a PAM configuration target folder.""" + if not folder_uid: + return None + if folder_uid in getattr(params, 'shared_folder_cache', {}): + sf = api.get_shared_folder(params, folder_uid) + return { + 'uid': folder_uid, + 'name': sf.name if sf else folder_uid, + 'type': 'shared_folder', + } + if is_nested_share_folder(params, folder_uid): + nsf = getattr(params, 'nested_share_folders', {}).get(folder_uid, {}) + return { + 'uid': folder_uid, + 'name': nsf.get('name', folder_uid), + 'type': 'nested_share_folder', + } + folder = getattr(params, 'folder_cache', {}).get(folder_uid) + if folder: + return { + 'uid': folder_uid, + 'name': folder.name, + 'type': 'user_folder', + } + return {'uid': folder_uid, 'name': folder_uid, 'type': 'unknown'} + + +def format_pam_folder_display(folder_info): + # type: (Optional[dict]) -> str + if not folder_info: + return '' + suffix = ' [NSF]' if folder_info.get('type') == 'nested_share_folder' else '' + return f'{folder_info["name"]} ({folder_info["uid"]}){suffix}' + + +def resolve_pam_config_folder_info(params, facade, record_uid): + # type: (...) -> Optional[dict] + from ...subfolder import find_parent_top_folder + + folder_info = get_pam_folder_info(params, facade.folder_uid) + if folder_info and folder_info.get('type') != 'unknown': + return folder_info + shared_folder_parents = find_parent_top_folder(params, record_uid) + if shared_folder_parents: + sf = shared_folder_parents[0] + return {'uid': sf.uid, 'name': sf.name, 'type': 'shared_folder'} + return folder_info + + +def pam_folder_json_payload(folder_info): + # type: (Optional[dict]) -> dict + payload = {} + if not folder_info: + return payload + payload['folder'] = folder_info + if folder_info.get('type') == 'shared_folder': + payload['shared_folder'] = { + 'name': folder_info.get('name'), + 'uid': folder_info.get('uid'), + } + return payload + + +def record_exists_in_vault(params, record_uid): + # type: (...) -> bool + from ..nested_share_folder.helpers import is_nested_share_record + + if not record_uid: + return False + return (record_uid in getattr(params, 'record_cache', {}) + or is_nested_share_record(params, record_uid)) - for uid, folder in getattr(params, 'nested_share_folders', {}).items(): - if folder.get('parent_uid') == parent_uid and folder.get('name') == name: - return uid +def resolve_pam_record(params, identifier, rec_type=None): + # type: (...) -> Optional[vault.KeeperRecord] + """Resolve a record by UID, folder path, or title including Nested Share Records.""" + if not identifier: + return None + + if identifier in getattr(params, 'record_cache', {}): + rec = vault.KeeperRecord.load(params, identifier) + if rec and (not rec_type or rec.record_type == rec_type): + return rec + + from ...nested_share_folder import removal_api as _nsf + resolved_uid = _nsf.resolve_nested_share_record_uid(params, identifier) + if resolved_uid: + rec = vault.KeeperRecord.load(params, resolved_uid) + if rec and (not rec_type or rec.record_type == rec_type): + return rec + + rs = try_resolve_path(params, identifier) + if rs is not None: + folder, record_title = rs + if folder is not None and record_title is not None: + folder_uid = folder.uid or '' + if folder_uid in params.subfolder_record_cache: + for uid in params.subfolder_record_cache[folder_uid]: + record = vault.KeeperRecord.load(params, uid) + if record and record.title.casefold() == record_title.casefold(): + if not rec_type or record.record_type == rec_type: + return record + + l_name = identifier.casefold() + matches = [] + for record_uid in getattr(params, 'record_cache', {}): + record = vault.KeeperRecord.load(params, record_uid) + if record and record.title.casefold() == l_name: + if not rec_type or record.record_type == rec_type: + matches.append(record) + if len(matches) > 1: + break + if len(matches) == 1: + return matches[0] return None +def get_vault_record_title_type(params, record_uid): + # type: (...) -> tuple + rec = vault.KeeperRecord.load(params, record_uid) + if rec: + return rec.title, rec.record_type + nsf_data = getattr(params, 'nested_share_record_data', {}).get(record_uid, {}) + data_json = nsf_data.get('data_json', {}) if isinstance(nsf_data, dict) else {} + if isinstance(data_json, dict) and data_json: + return (data_json.get('title', '[record inaccessible]'), + data_json.get('type', '[record inaccessible]')) + return '[record inaccessible]', '[record inaccessible]' + + +def traverse_pam_folder_subtree(params, root_folder_uid, callback): + # type: (...) -> None + seen = set() + queue = [root_folder_uid] + while queue: + f_uid = queue.pop(0) + if not f_uid or f_uid in seen: + continue + seen.add(f_uid) + callback(f_uid) + folder = getattr(params, 'folder_cache', {}).get(f_uid) + if folder and folder.subfolders: + for child_uid in folder.subfolders: + if child_uid not in seen: + queue.append(child_uid) + for uid, nsf in getattr(params, 'nested_share_folders', {}).items(): + if nsf.get('parent_uid') == f_uid and uid not in seen: + queue.append(uid) + + +def collect_pam_folder_uids(params, folder_name): + # type: (...) -> set + folder_uids = set() + if not folder_name: + return folder_uids + + resolved = resolve_pam_folder_uid(params, folder_name, allow_legacy_user=True) + if resolved: + traverse_pam_folder_subtree(params, resolved, folder_uids.add) + return folder_uids + + rs = try_resolve_path(params, folder_name, find_all_matches=True) + if rs is not None: + folder, record_title = rs + if not record_title: + folders = folder if isinstance(folder, list) else [folder] + for f in folders: + if isinstance(f, BaseFolderNode) and f.uid: + traverse_pam_folder_subtree(params, f.uid, folder_uids.add) + + return folder_uids + + +def find_pam_records_by_search(params, search_str, record_version=3, record_types=None): + # type: (...) -> list + types = tuple(record_types) if record_types else None + records = list(vault_extensions.find_records( + params, search_str=search_str, record_version=record_version, record_type=types)) + if search_str and len(records) == 0: + rec = resolve_pam_record(params, search_str) + if isinstance(rec, vault.TypedRecord): + if rec.version == record_version and (not types or rec.record_type in types): + records = [rec] + return [r for r in records if isinstance(r, vault.TypedRecord)] + + +def _find_child_folder_uid(params, parent_uid, name): + return _find_child_folder(params, parent_uid, name=name) + + def ensure_pam_folder_path(params, base_folder_uid, relative_path, command='pam'): if not base_folder_uid: raise CommandError(command, 'Base folder UID is required') From 4b684bccca0f98c6920675780dd95adca1c22c43 Mon Sep 17 00:00:00 2001 From: adeshmukh-ks Date: Fri, 3 Jul 2026 15:03:11 +0530 Subject: [PATCH 06/14] unit test correction --- unit-tests/pam/test_pam_nsf_config.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/unit-tests/pam/test_pam_nsf_config.py b/unit-tests/pam/test_pam_nsf_config.py index 3b3b7a989..b73519da7 100644 --- a/unit-tests/pam/test_pam_nsf_config.py +++ b/unit-tests/pam/test_pam_nsf_config.py @@ -157,7 +157,7 @@ def test_parse_pam_configuration_accepts_nsf_folder_name(self): field = record.get_typed_field('pamResources') self.assertEqual(field.value[0]['folderUid'], 'nsf_folder') - @mock.patch('keepercommander.commands.pam.vault_target.place_record_in_folder') + @mock.patch('keepercommander.commands.discoveryrotation.place_record_in_folder') @mock.patch('keepercommander.commands.discoveryrotation.api.sync_down') @mock.patch('keepercommander.commands.discoveryrotation.TunnelDAG') @mock.patch('keepercommander.commands.discoveryrotation.get_keeper_tokens', @@ -189,7 +189,7 @@ def create_config(_, record, folder_uid): class TestPamConfigEditNsfPlacement(unittest.TestCase): - @mock.patch('keepercommander.commands.pam.vault_target.place_record_in_folder') + @mock.patch('keepercommander.commands.discoveryrotation.place_record_in_folder') @mock.patch('keepercommander.commands.discoveryrotation.record_management.update_record') @mock.patch('keepercommander.commands.discoveryrotation.RecordEditMixin.get_record_type_fields', return_value=[]) From c0a5890af4f4570bacb7d370b4e6313cb455b02a Mon Sep 17 00:00:00 2001 From: adeshmukh-ks Date: Fri, 3 Jul 2026 16:35:33 +0530 Subject: [PATCH 07/14] fix bugs --- keepercommander/commands/discoveryrotation.py | 55 ++-- keepercommander/commands/pam/vault_target.py | 26 +- unit-tests/pam/test_pam_nsf_config.py | 271 +++++++++++++++++- unit-tests/pam/test_pam_rotation.py | 54 ++++ 4 files changed, 372 insertions(+), 34 deletions(-) diff --git a/keepercommander/commands/discoveryrotation.py b/keepercommander/commands/discoveryrotation.py index 3e7349288..4785a7335 100644 --- a/keepercommander/commands/discoveryrotation.py +++ b/keepercommander/commands/discoveryrotation.py @@ -33,7 +33,9 @@ resolve_pam_record, record_exists_in_vault, collect_pam_folder_uids, get_vault_record_title_type, find_pam_records_by_search, resolve_pam_config_folder_info, pam_folder_json_payload, place_record_in_folder, + create_record_in_folder, update_pam_record, records_in_folder, ) +from .nested_share_folder.helpers import is_nested_share_folder from .pam.config_helper import configuration_controller_get, \ pam_configurations_get_all, pam_configuration_remove, \ pam_configuration_create_record_v6, record_rotation_get, \ @@ -1416,7 +1418,7 @@ def config_user(_dag, target_record, target_resource_uid, target_config_uid=None if folder_uids: regex = re.compile(fnmatch.translate(record_pattern), re.IGNORECASE).match if record_pattern else None for folder_uid in folder_uids: - folder_records = params.subfolder_record_cache.get(folder_uid) + folder_records = records_in_folder(params, folder_uid) if not folder_records: continue if record_pattern and record_pattern in folder_records: @@ -1632,8 +1634,10 @@ def execute(self, params, **kwargs): enterprise_all_controllers = list(gateway_helper.get_all_gateways(params)) enterprise_controllers_connected_resp = router_get_connected_gateways(params) - enterprise_controllers_connected_uids_bytes = \ - [x.controllerUid for x in enterprise_controllers_connected_resp.controllers] + enterprise_controllers_connected_uids_bytes = [] + if enterprise_controllers_connected_resp and enterprise_controllers_connected_resp.controllers: + enterprise_controllers_connected_uids_bytes = \ + [x.controllerUid for x in enterprise_controllers_connected_resp.controllers] all_pam_config_records = pam_configurations_get_all(params) table = [] @@ -1681,8 +1685,8 @@ def execute(self, params, **kwargs): continue row.append(f'{row_color}{record_uid}') - row.append(record_title) - row.append(record_type) + row.append(record_title or '[untitled]') + row.append(record_type or '[unknown]') if s.noSchedule is True: # Per Sergey A: @@ -1737,17 +1741,22 @@ def execute(self, params, **kwargs): f"{bcolors.FAIL}[No config found. Looks like configuration {configuration_uid_str} was removed but rotation schedule was not modified{bcolors.ENDC}") else: - pam_data_decrypted = pam_decrypt_configuration_data(pam_configuration) - pam_config_name = pam_data_decrypted.get('title') - pam_config_type = pam_data_decrypted.get('type') - row.append(f"{pam_config_name} ({pam_config_type})") + pam_config_name, pam_config_type = get_vault_record_title_type(params, configuration_uid_str) + if pam_config_name == '[record inaccessible]': + try: + pam_data_decrypted = pam_decrypt_configuration_data(pam_configuration) + pam_config_name = pam_data_decrypted.get('title') or pam_config_name + pam_config_type = pam_data_decrypted.get('type') or '[unknown]' + except Exception: + pass + row.append(f"{pam_config_name or '[untitled]'} ({pam_config_type or '[unknown]'})") if is_verbose: row.append(f'{utils.base64_url_encode(configuration_uid)}{bcolors.ENDC}') table.append(row) - table.sort(key=lambda x: (x[1])) + table.sort(key=lambda x: (x[1] or '')) dump_report_data(table, headers, fmt='table', filename="", row_number=False, column_width=None) @@ -2839,7 +2848,12 @@ def execute(self, params, **kwargs): self.verify_required(record) - pam_configuration_create_record_v6(params, record, shared_folder_uid) + if is_nested_share_folder(params, shared_folder_uid): + create_record_in_folder(params, record, shared_folder_uid, command='pam-config-new') + else: + pam_configuration_create_record_v6(params, record, shared_folder_uid) + api.sync_down(params) + place_record_in_folder(params, record.record_uid, shared_folder_uid, command='pam-config-new') encrypted_session_token, encrypted_transmission_key, transmission_key = get_keeper_tokens(params) # Add DAG for configuration @@ -2859,10 +2873,6 @@ def execute(self, params, **kwargs): tmp_dag.link_user_to_config_with_options(admin_cred_ref, is_admin='on') tmp_dag.print_tunneling_config(record.record_uid, None) - # Moving v6 record into the folder - api.sync_down(params) - place_record_in_folder(params, record.record_uid, shared_folder_uid, command='pam-config-new') - params.environment_variables[LAST_RECORD_UID] = record.record_uid params.sync_data = True @@ -2917,14 +2927,11 @@ def execute(self, params, **kwargs): config_name = kwargs.get('uid') if not config_name: raise CommandError('pam config edit', 'PAM Configuration UID or Title is required') - if config_name in params.record_cache: - configuration = vault.KeeperRecord.load(params, config_name) - else: - l_name = config_name.casefold() - for c in vault_extensions.find_records(params, record_version=6): - if c.title.casefold() == l_name: - configuration = c - break + + config_uid = PAMConfigurationRemoveCommand._resolve_pam_config_uid(params, config_name) + if not config_uid: + raise CommandError('pam-config-edit', f'PAM configuration "{config_name}" not found') + configuration = vault.KeeperRecord.load(params, config_uid) if not configuration: raise CommandError('pam-config-edit', f'PAM configuration "{config_name}" not found') if not isinstance(configuration, vault.TypedRecord) or configuration.version != 6: @@ -2977,7 +2984,7 @@ def execute(self, params, **kwargs): self.parse_properties(params, configuration, config_edit=True, **kwargs) self.verify_required(configuration) - record_management.update_record(params, configuration) + update_pam_record(params, configuration, command='pam-config-edit') admin_cred_ref = '' value = field.get_default_value(dict) diff --git a/keepercommander/commands/pam/vault_target.py b/keepercommander/commands/pam/vault_target.py index 4d8b8efae..faa21fe84 100644 --- a/keepercommander/commands/pam/vault_target.py +++ b/keepercommander/commands/pam/vault_target.py @@ -349,12 +349,12 @@ def get_vault_record_title_type(params, record_uid): # type: (...) -> tuple rec = vault.KeeperRecord.load(params, record_uid) if rec: - return rec.title, rec.record_type + return rec.title or '[untitled]', rec.record_type or '[unknown]' nsf_data = getattr(params, 'nested_share_record_data', {}).get(record_uid, {}) data_json = nsf_data.get('data_json', {}) if isinstance(nsf_data, dict) else {} if isinstance(data_json, dict) and data_json: - return (data_json.get('title', '[record inaccessible]'), - data_json.get('type', '[record inaccessible]')) + return (data_json.get('title') or '[record inaccessible]', + data_json.get('type') or '[record inaccessible]') return '[record inaccessible]', '[record inaccessible]' @@ -497,6 +497,26 @@ def create_record_in_folder(params, record, folder_uid=None, command='pam'): record_management.add_record_to_folder(params, record, folder_uid) +def update_pam_record(params, record, command='pam'): + from ..nested_share_folder.helpers import is_nested_share_record + from ...nested_share_folder.record_api import update_record_v3 + + if is_nested_share_record(params, record.record_uid): + if not isinstance(record, vault.TypedRecord): + raise CommandError(command, 'Nested Share Folder record update requires a typed record') + + result = update_record_v3( + params, + record.record_uid, + record_data=vault_extensions.extract_typed_record_data(record), + ) + if not result.get('success'): + raise CommandError(command, result.get('message') or 'Failed to update record in Nested Share Folder') + api.sync_down(params) + else: + record_management.update_record(params, record) + + def execute_record_add_in_folder(params, args, folder_uid, command='pam'): from ..record_edit import RecordAddCommand diff --git a/unit-tests/pam/test_pam_nsf_config.py b/unit-tests/pam/test_pam_nsf_config.py index b73519da7..5695abed5 100644 --- a/unit-tests/pam/test_pam_nsf_config.py +++ b/unit-tests/pam/test_pam_nsf_config.py @@ -3,9 +3,11 @@ import keepercommander.commands.record # noqa: F401 from keepercommander import vault -from keepercommander.commands.discoveryrotation import PAMConfigurationEditCommand, PAMConfigurationNewCommand +from keepercommander.commands.discoveryrotation import ( + PAMConfigurationEditCommand, PAMConfigurationNewCommand, PAMConfigurationRemoveCommand, + PAMCreateRecordRotationCommand) from keepercommander.commands.pam.vault_target import ( - create_record_in_folder, place_record_in_folder, resolve_pam_folder_uid) + create_record_in_folder, place_record_in_folder, resolve_pam_folder_uid, records_in_folder) from keepercommander.error import CommandError from keepercommander.subfolder import NestedShareFolderNode, RootFolderNode, SharedFolderNode @@ -126,6 +128,16 @@ def test_resolve_pam_folder_uid_finds_nsf_cache_folder_by_name(self): self.assertEqual(resolve_pam_folder_uid(params, 'testpam'), 'nsf_folder') + def test_records_in_folder_merges_legacy_and_nsf_membership(self): + params = _make_params() + params.subfolder_record_cache = {'nsf_folder': {'legacy_rec_uid'}} + params.nested_share_folder_records = {'nsf_folder': {'nsf_rec_uid'}} + + self.assertEqual( + records_in_folder(params, 'nsf_folder'), + {'legacy_rec_uid', 'nsf_rec_uid'}, + ) + class TestPamConfigNewNsfPlacement(unittest.TestCase): @@ -157,6 +169,42 @@ def test_parse_pam_configuration_accepts_nsf_folder_name(self): field = record.get_typed_field('pamResources') self.assertEqual(field.value[0]['folderUid'], 'nsf_folder') + @mock.patch('keepercommander.commands.discoveryrotation.create_record_in_folder') + @mock.patch('keepercommander.commands.discoveryrotation.api.sync_down') + @mock.patch('keepercommander.commands.discoveryrotation.TunnelDAG') + @mock.patch('keepercommander.commands.discoveryrotation.get_keeper_tokens', + return_value=(b'encrypted_session', b'encrypted_key', b'transmission_key')) + @mock.patch('keepercommander.commands.discoveryrotation.RecordEditMixin.get_record_type_fields', + return_value=[]) + def test_execute_creates_new_config_in_nsf_folder( + self, mock_record_fields, mock_tokens, mock_dag, mock_sync, mock_create_in_folder): + params = _make_params() + params.nested_share_folders['nsf_folder'] = { + 'name': 'Project - Users', + 'parent_uid': 'app_uid', + 'folder_key_unencrypted': b'0' * 32, + } + folder = NestedShareFolderNode() + folder.uid = 'nsf_folder' + params.folder_cache[folder.uid] = folder + command = PAMConfigurationNewCommand() + + def parse_properties(_, record, **kwargs): + record.fields.append(vault.TypedField.new_field( + 'pamResources', {'folderUid': 'nsf_folder'})) + + def create_in_folder(_, record, folder_uid, command='pam'): + record.record_uid = 'config_uid' + + mock_create_in_folder.side_effect = create_in_folder + with mock.patch.object(command, 'parse_properties', side_effect=parse_properties), \ + mock.patch.object(command, 'verify_required'): + result = command.execute(params, config_type='local', title='Config') + + self.assertEqual(result, 'config_uid') + mock_create_in_folder.assert_called_once_with( + params, mock.ANY, 'nsf_folder', command='pam-config-new') + @mock.patch('keepercommander.commands.discoveryrotation.place_record_in_folder') @mock.patch('keepercommander.commands.discoveryrotation.api.sync_down') @mock.patch('keepercommander.commands.discoveryrotation.TunnelDAG') @@ -168,11 +216,14 @@ def test_parse_pam_configuration_accepts_nsf_folder_name(self): def test_execute_places_new_config_with_backend_aware_helper( self, mock_record_fields, mock_create_config, mock_tokens, mock_dag, mock_sync, mock_place): params = _make_params() + folder = SharedFolderNode() + folder.uid = 'legacy_folder' + params.folder_cache[folder.uid] = folder command = PAMConfigurationNewCommand() def parse_properties(_, record, **kwargs): record.fields.append(vault.TypedField.new_field( - 'pamResources', {'folderUid': 'nsf_folder'})) + 'pamResources', {'folderUid': 'legacy_folder'})) def create_config(_, record, folder_uid): record.record_uid = 'config_uid' @@ -183,20 +234,29 @@ def create_config(_, record, folder_uid): result = command.execute(params, config_type='local', title='Config') self.assertEqual(result, 'config_uid') + mock_create_config.assert_called_once() mock_place.assert_called_once_with( - params, 'config_uid', 'nsf_folder', command='pam-config-new') + params, 'config_uid', 'legacy_folder', command='pam-config-new') class TestPamConfigEditNsfPlacement(unittest.TestCase): @mock.patch('keepercommander.commands.discoveryrotation.place_record_in_folder') - @mock.patch('keepercommander.commands.discoveryrotation.record_management.update_record') + @mock.patch('keepercommander.commands.discoveryrotation.update_pam_record') @mock.patch('keepercommander.commands.discoveryrotation.RecordEditMixin.get_record_type_fields', return_value=[]) @mock.patch('keepercommander.vault.KeeperRecord.load') - def test_execute_places_edited_config_with_backend_aware_helper( + def test_execute_updates_and_places_edited_config_in_nsf_folder( self, mock_load, mock_record_fields, mock_update_record, mock_place): params = _make_params() + params.nested_share_folders['nsf_folder'] = { + 'name': 'Project - Users', + 'parent_uid': 'app_uid', + 'folder_key_unencrypted': b'0' * 32, + } + folder = NestedShareFolderNode() + folder.uid = 'nsf_folder' + params.folder_cache[folder.uid] = folder params.record_cache = {'config_uid': {}} command = PAMConfigurationEditCommand() @@ -215,6 +275,203 @@ def parse_properties(_, record, **kwargs): mock.patch.object(command, 'verify_required'): command.execute(params, uid='config_uid') - mock_update_record.assert_called_once_with(params, configuration) + mock_update_record.assert_called_once_with(params, configuration, command='pam-config-edit') mock_place.assert_called_once_with( params, 'config_uid', 'nsf_folder', command='pam-config-edit') + + @mock.patch('keepercommander.commands.discoveryrotation.place_record_in_folder') + @mock.patch('keepercommander.commands.discoveryrotation.update_pam_record') + @mock.patch('keepercommander.commands.discoveryrotation.RecordEditMixin.get_record_type_fields', + return_value=[]) + @mock.patch('keepercommander.vault.KeeperRecord.load') + def test_execute_places_edited_config_with_backend_aware_helper( + self, mock_load, mock_record_fields, mock_update_record, mock_place): + params = _make_params() + folder = SharedFolderNode() + folder.uid = 'legacy_folder' + params.folder_cache[folder.uid] = folder + params.record_cache = {'config_uid': {}} + command = PAMConfigurationEditCommand() + + configuration = vault.TypedRecord(version=6) + configuration.record_uid = 'config_uid' + configuration.type_name = 'pamNetworkConfiguration' + configuration.fields.append(vault.TypedField.new_field( + 'pamResources', {'folderUid': 'nsf_folder'})) + mock_load.return_value = configuration + + def parse_properties(_, record, **kwargs): + field = record.get_typed_field('pamResources') + field.value[0]['folderUid'] = 'legacy_folder' + + with mock.patch.object(command, 'parse_properties', side_effect=parse_properties), \ + mock.patch.object(command, 'verify_required'): + command.execute(params, uid='config_uid') + + mock_update_record.assert_called_once_with(params, configuration, command='pam-config-edit') + mock_place.assert_called_once_with( + params, 'config_uid', 'legacy_folder', command='pam-config-edit') + + +class TestPamConfigRemoveNsf(unittest.TestCase): + + @mock.patch('keepercommander.vault.KeeperRecord.load') + def test_resolve_pam_config_uid_finds_nsf_record_by_uid(self, mock_load): + params = _make_params() + params.record_cache = {} + params.nested_share_records = { + 'config_uid': {'record_uid': 'config_uid', 'version': 6}, + } + + configuration = vault.TypedRecord(version=6) + configuration.record_uid = 'config_uid' + configuration.type_name = 'pamNetworkConfiguration' + mock_load.return_value = configuration + + uid = PAMConfigurationRemoveCommand._resolve_pam_config_uid(params, 'config_uid') + self.assertEqual(uid, 'config_uid') + + @mock.patch('keepercommander.vault.KeeperRecord.load') + def test_resolve_pam_config_uid_finds_nsf_record_by_title(self, mock_load): + params = _make_params() + params.record_cache = {} + params.nested_share_records = { + 'config_uid': {'record_uid': 'config_uid', 'version': 6}, + } + params.nested_share_record_data = { + 'config_uid': { + 'data_json': { + 'title': 'PAM NSF Test Configuration', + 'type': 'pamNetworkConfiguration', + }, + }, + } + + configuration = vault.TypedRecord(version=6) + configuration.record_uid = 'config_uid' + configuration.type_name = 'pamNetworkConfiguration' + configuration.title = 'PAM NSF Test Configuration' + mock_load.return_value = configuration + + uid = PAMConfigurationRemoveCommand._resolve_pam_config_uid( + params, 'PAM NSF Test Configuration') + self.assertEqual(uid, 'config_uid') + + @mock.patch('keepercommander.commands.pam.config_helper.RecordRemoveCommand') + @mock.patch('keepercommander.commands.nested_share_folder.helpers.is_nested_share_record', + return_value=False) + def test_pam_configuration_remove_uses_legacy_remove_for_non_nsf_record( + self, _mock_is_nsf, mock_remove_cmd): + from keepercommander.commands.pam.config_helper import pam_configuration_remove + + params = _make_params() + params.record_cache = {'config_uid': {}} + params.nested_share_records = {} + + pam_configuration_remove(params, 'config_uid') + + mock_remove_cmd.return_value.execute.assert_called_once_with( + params, record='config_uid', force=True) + self.assertNotIn('config_uid', params.record_cache) + + @mock.patch('keepercommander.nested_share_folder.remove_record_v3', + return_value={'confirmed': True}) + @mock.patch('keepercommander.subfolder.find_folders', return_value=['nsf_folder']) + @mock.patch('keepercommander.commands.nested_share_folder.helpers.is_nested_share_record', + return_value=True) + def test_pam_configuration_remove_uses_nsf_remove_for_nsf_record( + self, _mock_is_nsf, mock_find_folders, mock_remove_v3): + from keepercommander.commands.pam.config_helper import pam_configuration_remove + + params = _make_params() + params.record_cache = {'config_uid': {}} + params.nested_share_records = {'config_uid': {'record_uid': 'config_uid', 'version': 6}} + + pam_configuration_remove(params, 'config_uid') + + mock_remove_v3.assert_called_once_with(params, [{ + 'record_uid': 'config_uid', + 'folder_uid': 'nsf_folder', + 'operation_type': 'owner-trash', + }], dry_run=False) + self.assertNotIn('config_uid', params.record_cache) + self.assertNotIn('config_uid', params.nested_share_records) + + @mock.patch('keepercommander.commands.discoveryrotation.pam_configuration_remove') + @mock.patch('keepercommander.commands.discoveryrotation.TunnelDAG') + @mock.patch('keepercommander.commands.discoveryrotation.get_keeper_tokens', + return_value=(b'encrypted_session', b'encrypted_key', b'transmission_key')) + @mock.patch('keepercommander.vault.KeeperRecord.load') + def test_execute_removes_nsf_config( + self, mock_load, mock_tokens, mock_dag, mock_remove): + params = _make_params() + params.record_cache = {} + params.nested_share_records = { + 'config_uid': {'record_uid': 'config_uid', 'version': 6}, + } + command = PAMConfigurationRemoveCommand() + + configuration = vault.TypedRecord(version=6) + configuration.record_uid = 'config_uid' + configuration.type_name = 'pamNetworkConfiguration' + mock_load.return_value = configuration + mock_dag.return_value.linking_dag.has_graph = False + + command.execute(params, uid='config_uid') + + mock_remove.assert_called_once_with(params, 'config_uid') + self.assertTrue(params.sync_data) + + +class TestPamRotationNsfFolder(unittest.TestCase): + + @mock.patch('keepercommander.commands.discoveryrotation.router_set_record_rotation_information') + @mock.patch('keepercommander.commands.discoveryrotation.TunnelDAG') + @mock.patch('keepercommander.commands.discoveryrotation.get_keeper_tokens', + return_value=(b'encrypted_session', b'encrypted_key', b'transmission_key')) + @mock.patch('keepercommander.commands.discoveryrotation.collect_pam_folder_uids', + return_value={'nsf_folder'}) + @mock.patch('keepercommander.commands.discoveryrotation.records_in_folder', + return_value={'nsf_pam_user_uid'}) + @mock.patch('keepercommander.vault.KeeperRecord.load') + def test_execute_with_nsf_folder_collects_nsf_records( + self, mock_load, mock_records_in_folder, mock_collect_folders, + mock_tokens, mock_dag, _mock_router_set): + from cryptography.hazmat.primitives.asymmetric import ec + + params = _make_params() + params.rest_context.server_key_id = 8 + params.session_token = 'base64_encoded_session_token' + params.record_rotation_cache = {} + params.subfolder_record_cache = {} + params.nested_share_folder_records = {'nsf_folder': {'nsf_pam_user_uid'}} + + pam_user = vault.TypedRecord(version=3) + pam_user.record_uid = 'nsf_pam_user_uid' + pam_user.type_name = 'pamMachine' + pam_user.title = 'PAM NSF Test Machine' + pam_user.record_key = b'\x00' * 32 + mock_load.return_value = pam_user + + mock_dag_instance = mock_dag.return_value + mock_dag_instance.linking_dag.has_graph = True + mock_dag_instance.resource_belongs_to_config.return_value = True + mock_dag_instance.record.record_uid = 'config_uid' + + config_record = vault.TypedRecord(version=6) + config_record.record_uid = 'config_uid' + config_record.type_name = 'pamNetworkConfiguration' + + command = PAMCreateRecordRotationCommand() + with mock.patch('keepercommander.rest_api.SERVER_PUBLIC_KEYS', + {8: ec.generate_private_key(ec.SECP256R1()).public_key()}), \ + mock.patch('keepercommander.vault_extensions.find_records', return_value=[config_record]), \ + mock.patch('keepercommander.commands.discoveryrotation.resolve_pam_record', + side_effect=lambda _p, ident, rec_type=None: config_record if ident == 'config_uid' else pam_user), \ + mock.patch('keepercommander.commands.discoveryrotation.router_set_record_rotation_information'): + command.execute(params, folder_name='M_SR5x7Q4cu9O0-RiLDN4A', force=True, on_demand=True, + config='config_uid') + + mock_collect_folders.assert_called_once_with(params, 'M_SR5x7Q4cu9O0-RiLDN4A') + mock_records_in_folder.assert_called_once_with(params, 'nsf_folder') + mock_load.assert_any_call(params, 'nsf_pam_user_uid') diff --git a/unit-tests/pam/test_pam_rotation.py b/unit-tests/pam/test_pam_rotation.py index 73220880b..16c03c1c7 100644 --- a/unit-tests/pam/test_pam_rotation.py +++ b/unit-tests/pam/test_pam_rotation.py @@ -408,6 +408,60 @@ def test_execute_with_no_schedules(self, mock_dump_report_data, mock_pam_decrypt self.assertTrue(mock_router_get_connected_gateways.called) self.assertTrue(mock_pam_configurations_get_all.called) + @patch('keepercommander.commands.discoveryrotation.router_get_rotation_schedules') + @patch('keepercommander.commands.discoveryrotation.router_get_connected_gateways') + @patch('keepercommander.commands.discoveryrotation.pam_configurations_get_all') + @patch('keepercommander.commands.discoveryrotation.gateway_helper.get_all_gateways') + @patch('keepercommander.commands.discoveryrotation.dump_report_data') + def test_execute_reads_nsf_config_without_encrypted_data_field( + self, mock_dump_report_data, mock_get_all_gateways, mock_pam_configurations_get_all, + mock_router_get_connected_gateways, mock_router_get_rotation_schedules): + rec_uid = 'rec123456789012345678901' + cfg_uid = 'cfg123456789012345678901' + mock_params = create_mock_params() + mock_params.record_cache = { + rec_uid: { + 'record_uid': rec_uid, + 'version': 6, + 'record_key_unencrypted': b'0' * 32, + 'data_unencrypted': json.dumps({'title': 'PAM User', 'type': 'pamUser'}), + }, + cfg_uid: { + 'record_uid': cfg_uid, + 'version': 6, + 'data': None, + 'record_key_unencrypted': b'1' * 32, + 'data_unencrypted': json.dumps({ + 'title': 'PAM NSF Test Configuration', + 'type': 'pamNetworkConfiguration', + }), + }, + } + + mock_router_get_rotation_schedules.return_value.schedules = [ + MagicMock( + recordUid=utils.base64_url_decode(rec_uid), + controllerUid=utils.base64_url_decode('controller_uid'), + configurationUid=utils.base64_url_decode(cfg_uid), + noSchedule=False, + scheduleData='RotateActionJob|daily.0.12.1', + ) + ] + mock_get_all_gateways.return_value = [ + MagicMock(controllerUid=utils.base64_url_decode('controller_uid'), controllerName='Gateway') + ] + mock_router_get_connected_gateways.return_value.controllers = [] + mock_pam_configurations_get_all.return_value = [mock_params.record_cache[cfg_uid]] + + self.command.execute(mock_params, is_verbose=False) + + self.assertTrue(mock_dump_report_data.called) + table = mock_dump_report_data.call_args[0][0] + self.assertEqual( + table[0][5], + 'PAM NSF Test Configuration (pamNetworkConfiguration)', + ) + class TestPAMGatewayListCommand(unittest.TestCase): From 1e0961c577a331db89f7489b8cf7107e34c8797d Mon Sep 17 00:00:00 2001 From: sshrushanth-ks Date: Mon, 6 Jul 2026 10:09:02 +0530 Subject: [PATCH 08/14] Implemented NSF support for pam project export and pam project extend. Add record_loader and nsf_helpers for classic and NSF vault caches, wire export and extend to load/create records in nested shared folders, and add unit tests. Co-authored-by: Cursor --- keepercommander/commands/pam_import/export.py | 12 +- keepercommander/commands/pam_import/extend.py | 110 ++----- .../commands/pam_import/nsf_helpers.py | 280 ++++++++++++++++++ .../commands/pam_import/record_loader.py | 67 +++++ .../nested_share_folder/record_api.py | 4 +- unit-tests/pam/test_pam_project_export.py | 100 ++++++- unit-tests/pam/test_pam_project_extend_nsf.py | 16 +- .../test_pam_project_extend_nsf_helpers.py | 114 +++++++ 8 files changed, 604 insertions(+), 99 deletions(-) create mode 100644 keepercommander/commands/pam_import/nsf_helpers.py create mode 100644 keepercommander/commands/pam_import/record_loader.py create mode 100644 unit-tests/pam/test_pam_project_extend_nsf_helpers.py diff --git a/keepercommander/commands/pam_import/export.py b/keepercommander/commands/pam_import/export.py index 1effb4ee3..d847797ee 100644 --- a/keepercommander/commands/pam_import/export.py +++ b/keepercommander/commands/pam_import/export.py @@ -21,6 +21,7 @@ ) from ..base import Command from ..pam.config_facades import PamConfigurationRecordFacade +from .record_loader import iter_accessible_record_uids, load_pam_record from ... import vault from ...display import bcolors @@ -79,7 +80,7 @@ def execute(self, params, **kwargs): return # 1. Load PAM configuration record (v6) - config_record = vault.KeeperRecord.load(params, project_uid) + config_record = load_pam_record(params, project_uid) if not config_record: logging.warning( f"{bcolors.FAIL}PAM configuration '{project_uid}' not found in vault{bcolors.ENDC}" @@ -203,7 +204,7 @@ def _build_resources_and_users(self, params, resource_uids): title_to_uid = self._build_user_title_index(params) for res_uid in resource_uids: - res_record = vault.KeeperRecord.load(params, res_uid) + res_record = load_pam_record(params, res_uid) if not res_record or not isinstance(res_record, vault.TypedRecord): logging.debug("Export: skipping resource UID %s (not found or not TypedRecord)", res_uid) continue @@ -251,10 +252,9 @@ def _build_resources_and_users(self, params, resource_uids): def _build_user_title_index(self, params): """Index every pamUser / login record by lowercased title for title-based linking.""" index = {} - record_cache = getattr(params, "record_cache", {}) or {} - for uid in record_cache: + for uid in iter_accessible_record_uids(params): try: - rec = vault.KeeperRecord.load(params, uid) + rec = load_pam_record(params, uid) except Exception: continue if not rec or not isinstance(rec, vault.TypedRecord): @@ -302,7 +302,7 @@ def _extract_user_uids(self, pam_settings_dict, title_to_uid=None): def _load_user_obj(self, params, usr_uid): """Load a pamUser/login record and return a plain dict, or None on failure.""" - usr_record = vault.KeeperRecord.load(params, usr_uid) + usr_record = load_pam_record(params, usr_uid) if not usr_record or not isinstance(usr_record, vault.TypedRecord): logging.debug("Export: user UID %s not found or not TypedRecord", usr_uid) return None diff --git a/keepercommander/commands/pam_import/extend.py b/keepercommander/commands/pam_import/extend.py index 1ce6f5972..7cb155278 100644 --- a/keepercommander/commands/pam_import/extend.py +++ b/keepercommander/commands/pam_import/extend.py @@ -54,6 +54,16 @@ refresh_link_to_config_to_latest, ) from .workflow_apply import apply_workflow, validate_workflow_principals +from .nsf_helpers import ( + build_folder_tree, + create_nsf_subfolder, + extend_create_record, + find_pam_configuration, + get_folder_record_uids, + get_ksm_app_folders, + get_records_in_folder, +) +from .record_loader import load_pam_record from ...keeper_dag import EdgeType from ...keeper_dag.types import RefType from ..base import Command @@ -64,12 +74,12 @@ from ..tunnel.port_forward.TunnelGraph import TunnelDAG from ..tunnel.port_forward.tunnel_helpers import get_keeper_tokens from ..tunnel_and_connections import PAMTunnelEditCommand -from ... import api, crypto, utils, vault, vault_extensions, record_management +from ... import api, crypto, utils, vault, record_management from ... import enterprise as _enterprise_module from ...display import bcolors from ...error import CommandError from ...params import LAST_FOLDER_UID, LAST_SHARED_FOLDER_UID -from ...proto import APIRequest_pb2, enterprise_pb2, pam_pb2, record_pb2 +from ...proto import enterprise_pb2, pam_pb2, record_pb2 from ...recordv3 import RecordV3 from ...subfolder import BaseFolderNode, find_folders as find_record_folders @@ -192,23 +202,8 @@ def process_folder_paths(folder_paths, ksm_shared_folders): return good_paths, bad_paths def build_tree_recursive(params, folder_uid: str): - """Recursively build tree for a folder and its subfolders""" - tree = {} - folder = params.folder_cache.get(folder_uid) - if not folder: - return tree - - for subfolder_uid in folder.subfolders: - subfolder = params.folder_cache.get(subfolder_uid) - if subfolder: - folder_name = subfolder.name or '' - tree[folder_name] = { - 'uid': subfolder.uid, - 'name': folder_name, - 'subfolders': build_tree_recursive(params, subfolder.uid) - } - - return tree + """Recursively build tree for a folder and its subfolders (classic or NSF).""" + return build_folder_tree(params, folder_uid) def _collect_path_to_uid_from_tree(path_prefix: str, tree: dict, path_to_uid: dict, only_existing: bool) -> None: @@ -296,35 +291,14 @@ def _get_ksm_app_record_uids(params, ksm_shared_folders: list) -> set: """Return set of all record UIDs in any folder shared to the KSM app.""" folder_uids = _collect_all_folder_uids_under_ksm(ksm_shared_folders) record_uids = set() - subfolder_record_cache = getattr(params, "subfolder_record_cache", None) or {} for fuid in folder_uids: - if fuid in subfolder_record_cache: - record_uids.update(subfolder_record_cache[fuid]) + record_uids.update(get_folder_record_uids(params, fuid)) return record_uids def _get_records_in_folder(params, folder_uid: str): - """Return list of (record_uid, title, record_type, login) for records in folder_uid. - record_type from record for autodetect (e.g. pamUser, pamMachine, login).""" - subfolder_record_cache = getattr(params, "subfolder_record_cache", None) or {} - result = [] - for ruid in subfolder_record_cache.get(folder_uid, []): - try: - rec = vault.KeeperRecord.load(params, ruid) - title = getattr(rec, "title", "") or "" - rtype = "" - if hasattr(rec, "record_type"): - rtype = getattr(rec, "record_type", "") or "" - login = "" - fields = getattr(rec, "fields", None) - if isinstance(fields, list): - field = next((x for x in fields if getattr(x, "type", "") == "login"), None) - if field and hasattr(field, "get_default_value"): - login = (field.get_default_value() or "") or "" - result.append((ruid, title, rtype, login)) - except Exception: - pass - return result + """Return list of (record_uid, title, record_type, login) for records in folder_uid.""" + return get_records_in_folder(params, folder_uid) def _get_all_ksm_app_records(params, ksm_shared_folders: list) -> list: @@ -416,15 +390,7 @@ def execute(self, params, **kwargs): # no need to populate params.enterprise (users/teams caches). # since extend only adds new records to existing folders - configuration = None - if config_name in params.record_cache: - configuration = vault.KeeperRecord.load(params, config_name) - else: - l_name = config_name.casefold() - for c in vault_extensions.find_records(params, record_version=6): - if c.title.casefold() == l_name: - configuration = c - break + configuration = find_pam_configuration(params, config_name) if not (configuration and isinstance(configuration, vault.TypedRecord) and configuration.version == 6): raise CommandError("pam project extend", f"""PAM configuration not found: "{config_name}" """) @@ -561,32 +527,9 @@ def execute(self, params, **kwargs): self.process_data(params, project) def get_app_shared_folders(self, params, ksm_app_uid: str, configuration_uid: Optional[str]=None) -> list[dict]: - ksm_shared_folders = [] - - try: - app_info_list = KSMCommand.get_app_info(params, ksm_app_uid) - if app_info_list and len(app_info_list) > 0: - app_info = app_info_list[0] - shares = [x for x in app_info.shares if x.shareType == APIRequest_pb2.SHARE_TYPE_FOLDER] # pylint: disable=no-member - for share in shares: - folder_uid = utils.base64_url_encode(share.secretUid) - if folder_uid in params.shared_folder_cache: - cached_sf = params.shared_folder_cache[folder_uid] - folder_name = cached_sf.get('name_unencrypted', 'Unknown') - is_editable = share.editable if hasattr(share, 'editable') else False - - ksm_shared_folders.append({ - 'uid': folder_uid, - 'name': folder_name, - 'editable': is_editable, - 'permissions': "Editable" if is_editable else "Read-Only" - }) - except Exception as e: - logging.error(f"Could not retrieve KSM application shares: {e}") - + ksm_shared_folders = get_ksm_app_folders(params, ksm_app_uid) if not ksm_shared_folders and configuration_uid: ksm_shared_folders.extend(self.get_nsf_project_folders(params, configuration_uid)) - return ksm_shared_folders @staticmethod @@ -1148,7 +1091,6 @@ def autodetect_folders(self, params, project: dict) -> list: step3_errors = [] folders_out = project.get("folders") or {} ksm_shared_folders = project.get("ksm_shared_folders") or [] - subfolder_record_cache = getattr(params, "subfolder_record_cache", None) or {} new_no_path = [] for o in chain(project.get("mapped_resources", []), project.get("mapped_users", [])): @@ -1182,7 +1124,7 @@ def autodetect_folders(self, params, project: dict) -> list: non_empty = [] for shf in ksm_shared_folders: uids = _folder_uids_under_shf(shf) - if any(subfolder_record_cache.get(fuid) for fuid in uids): + if any(get_folder_record_uids(params, fuid) for fuid in uids): non_empty.append(shf) if len(non_empty) == 0: step3_errors.append("Autodetect: no folders contain records; cannot assign resources/users folders.") @@ -1235,6 +1177,8 @@ def create_subfolder(self, params, folder_name:str, parent_uid:str="", permissio name = str(folder_name or "").strip() if is_nested_share_folder(params, parent_uid): + if folder_uid: + return create_nsf_subfolder(params, name, parent_uid, folder_uid=folder_uid) from ...nested_share_folder.folder_api import create_folder_v3 result = create_folder_v3(params, name, parent_uid=parent_uid) if isinstance(result, dict) and result.get('success') is False: @@ -1450,7 +1394,7 @@ def process_data(self, params, project): logging.warning(f"Processing external users: {len(new_users)}") for n, user in enumerate(new_users): folder_uid = getattr(user, "resolved_folder_uid", None) or shfusr - user.create_record(params, folder_uid) + extend_create_record(params, user, folder_uid) if n % pdelta == 0: print(f"{n}/{len(new_users)}") print(f"{len(new_users)}/{len(new_users)}\n") @@ -1465,7 +1409,7 @@ def process_data(self, params, project): print(f"{n}/{len(new_resources)}") folder_uid = getattr(mach, "resolved_folder_uid", None) or shfres admin_uid = get_admin_credential(mach, True) - mach.create_record(params, folder_uid) + extend_create_record(params, mach, folder_uid) tdag.link_resource_to_config(mach.uid) if isinstance(mach, PamRemoteBrowserObject): args = parse_command_options(mach, True) @@ -1532,7 +1476,7 @@ def process_data(self, params, project): if isinstance(user, PamUserObject) and rs and (getattr(rs, "rotation", "") or "").lower() == "general": rs.resourceUid = mach.uid ufolder = getattr(user, "resolved_folder_uid", None) or shfusr - user.create_record(params, ufolder) + extend_create_record(params, user, ufolder) if isinstance(user, PamUserObject): tdag.link_user_to_resource(user.uid, mach.uid, admin_uid == user.uid, True) if rs: @@ -1567,7 +1511,7 @@ def process_data(self, params, project): if isinstance(user, PamUserObject) and rs and (getattr(rs, "rotation", "") or "").lower() == "general": rs.resourceUid = mach.uid ufolder = getattr(user, "resolved_folder_uid", None) or shfusr - user.create_record(params, ufolder) + extend_create_record(params, user, ufolder) if isinstance(user, PamUserObject): tdag.link_user_to_resource(user.uid, mach.uid, admin_uid == user.uid, True) if rs: @@ -1613,7 +1557,7 @@ def process_data(self, params, project): if pce and getattr(pce, "environment", "") == "domain": if getattr(pce, "admin_credential_ref", None): pcuid = (project.get("pam_config") or {}).get("pam_config_uid") - pcrec = vault.KeeperRecord.load(params, pcuid) if pcuid else None + pcrec = load_pam_record(params, pcuid) if pcuid else None if pcrec and isinstance(pcrec, vault.TypedRecord) and pcrec.version == 6: if pcrec.record_type == "pamDomainConfiguration": prf = pcrec.get_typed_field("pamResources") diff --git a/keepercommander/commands/pam_import/nsf_helpers.py b/keepercommander/commands/pam_import/nsf_helpers.py new file mode 100644 index 000000000..3b92c66a3 --- /dev/null +++ b/keepercommander/commands/pam_import/nsf_helpers.py @@ -0,0 +1,280 @@ +# _ __ +# | |/ /___ ___ _ __ ___ _ _ ® +# | ' bool: + """Return True when *folder_uid* is a Nested Shared Folder.""" + if not folder_uid: + return False + nsf_folders = getattr(params, 'nested_share_folders', None) or {} + if folder_uid in nsf_folders: + return True + subfolder = (getattr(params, 'subfolder_cache', None) or {}).get(folder_uid) or {} + return subfolder.get('source') == 'nested_share_folder' + + +def find_pam_configuration(params, config_name: str) -> Optional[vault.TypedRecord]: + """Resolve a PAM configuration record by UID or title (classic + NSF).""" + config_name = (config_name or '').strip() + if not config_name: + return None + + rec = load_pam_record(params, config_name) + if rec and isinstance(rec, vault.TypedRecord) and rec.version == 6: + return rec + + l_name = config_name.casefold() + for uid in iter_accessible_record_uids(params): + if uid == config_name: + continue + rec = load_pam_record(params, uid) + if not rec or not isinstance(rec, vault.TypedRecord) or rec.version != 6: + continue + if rec.title and rec.title.casefold() == l_name: + return rec + return None + + +def get_ksm_app_folders(params, ksm_app_uid: str) -> List[dict]: + """Return KSM-application folder roots (classic shared folders and NSF).""" + from ..ksm import KSMCommand + from ... import utils as keeper_utils + from ...proto import APIRequest_pb2 + + folders = [] + try: + app_info_list = KSMCommand.get_app_info(params, ksm_app_uid) + if not app_info_list: + return folders + app_info = app_info_list[0] + shares = [x for x in app_info.shares if x.shareType == APIRequest_pb2.SHARE_TYPE_FOLDER] # pylint: disable=no-member + for share in shares: + folder_uid = keeper_utils.base64_url_encode(share.secretUid) + is_editable = share.editable if hasattr(share, 'editable') else False + entry = _folder_entry_for_uid(params, folder_uid, is_editable) + if entry: + folders.append(entry) + except Exception as exc: + logging.error('Could not retrieve KSM application shares: %s', exc) + return folders + + +def _folder_entry_for_uid(params, folder_uid: str, is_editable: bool) -> Optional[dict]: + if folder_uid in getattr(params, 'shared_folder_cache', {}): + cached_sf = params.shared_folder_cache[folder_uid] + return { + 'uid': folder_uid, + 'name': cached_sf.get('name_unencrypted', 'Unknown'), + 'editable': is_editable, + 'permissions': 'Editable' if is_editable else 'Read-Only', + 'source': 'classic', + } + nsf_folders = getattr(params, 'nested_share_folders', None) or {} + if folder_uid in nsf_folders: + return { + 'uid': folder_uid, + 'name': nsf_folders[folder_uid].get('name', 'Unknown'), + 'editable': is_editable, + 'permissions': 'Editable' if is_editable else 'Read-Only', + 'source': 'nested', + } + return None + + +def build_folder_tree(params, folder_uid: str) -> dict: + """Build a nested folder tree under *folder_uid* (classic or NSF).""" + if is_nsf_folder_uid(params, folder_uid): + return _build_nsf_folder_tree(params, folder_uid) + return _build_classic_folder_tree(params, folder_uid) + + +def _build_classic_folder_tree(params, folder_uid: str) -> dict: + tree = {} + folder = params.folder_cache.get(folder_uid) + if not folder: + return tree + for subfolder_uid in folder.subfolders: + subfolder = params.folder_cache.get(subfolder_uid) + if not subfolder: + continue + folder_name = subfolder.name or '' + tree[folder_name] = { + 'uid': subfolder.uid, + 'name': folder_name, + 'subfolders': _build_classic_folder_tree(params, subfolder.uid), + } + return tree + + +def _build_nsf_folder_tree(params, root_uid: str) -> dict: + nsf_folders = getattr(params, 'nested_share_folders', None) or {} + + def children_of(parent_uid: str) -> dict: + kids = {} + for fuid, finfo in nsf_folders.items(): + raw_parent = finfo.get('parent_uid') or '' + if raw_parent != parent_uid: + continue + name = finfo.get('name') or fuid + kids[name] = { + 'uid': fuid, + 'name': name, + 'subfolders': children_of(fuid), + } + return kids + + return children_of(root_uid) + + +def get_folder_record_uids(params, folder_uid: str) -> set: + """Return record UIDs directly associated with a folder.""" + record_uids = set() + subfolder_record_cache = getattr(params, 'subfolder_record_cache', None) or {} + nsf_folder_records = getattr(params, 'nested_share_folder_records', None) or {} + if folder_uid in subfolder_record_cache: + record_uids.update(subfolder_record_cache[folder_uid]) + if folder_uid in nsf_folder_records: + record_uids.update(nsf_folder_records[folder_uid]) + return record_uids + + +def get_records_in_folder(params, folder_uid: str) -> list: + """Return (uid, title, record_type, login) tuples for records in *folder_uid*.""" + result = [] + for ruid in get_folder_record_uids(params, folder_uid): + try: + rec = load_pam_record(params, ruid) + if not rec: + continue + title = getattr(rec, 'title', '') or '' + rtype = getattr(rec, 'record_type', '') or '' + login = '' + fields = getattr(rec, 'fields', None) + if isinstance(fields, list): + field = next((x for x in fields if getattr(x, 'type', '') == 'login'), None) + if field and hasattr(field, 'get_default_value'): + login = (field.get_default_value() or '') or '' + result.append((ruid, title, rtype, login)) + except Exception: + continue + return result + + +def create_nsf_subfolder(params, folder_name: str, parent_uid: str = '', + folder_uid: Optional[str] = None) -> str: + """Create an NSF subfolder; returns folder UID.""" + from ... import nested_share_folder as _nsf + from ...nested_share_folder.folder_api import _prepare_folder_for_creation, folder_add_v3 + from ...nested_share_folder.helpers import command_error_handler, check_result + from ...proto import folder_pb2 + + name = str(folder_name or '').strip() + if not name: + raise CommandError('pam project extend', 'NSF subfolder name is required') + if not folder_uid: + folder_uid = api.generate_record_uid() + + parent = parent_uid or None + with command_error_handler('pam project extend'): + fd, folder_key = _prepare_folder_for_creation( + params, folder_uid, name, parent, None, True, + ) + response = folder_add_v3(params, [fd]) + if not response.folderAddResults: + raise CommandError('pam project extend', 'No results from NSF folder creation') + r = response.folderAddResults[0] + check_result({ + 'success': r.status == folder_pb2.SUCCESS, + 'message': r.message, + }, 'pam project extend') + + nsf = getattr(params, 'nested_share_folders', None) + if nsf is not None: + entry = {'name': name, 'parent_uid': parent_uid or ''} + if folder_key: + entry['folder_key_unencrypted'] = folder_key + nsf[folder_uid] = entry + subfolder_cache = getattr(params, 'subfolder_cache', None) + if subfolder_cache is not None: + subfolder_cache[folder_uid] = { + 'folder_uid': folder_uid, + 'type': 'user_folder', + 'name': name, + 'parent_uid': parent_uid or '', + 'source': 'nested_share_folder', + } + if folder_key: + subfolder_cache[folder_uid]['folder_key_unencrypted'] = folder_key + params.environment_variables['last_folder_uid'] = folder_uid + return folder_uid + + +def extend_create_record(params, obj, folder_uid: str) -> Optional[str]: + """Create a PAM import record in a classic or NSF folder.""" + if not is_nsf_folder_uid(params, folder_uid): + return obj.create_record(params, folder_uid) + + captured: Dict = {} + + class _CapturingAdd: + def execute(self, _params, **kwargs): + captured.clear() + captured.update(kwargs) + return None + + with patch('keepercommander.commands.pam_import.base.RecordEditAddCommand', return_value=_CapturingAdd()): + obj.create_record(params, folder_uid) + + from ..nested_share_folder.record_commands import NestedShareRecordAddCommand + from ...nested_share_folder.helpers import command_error_handler, check_result + from ...nested_share_folder.record_api import create_record_v3 + + cmd = NestedShareRecordAddCommand() + record_fields, add_attachments = cmd._parse_fields(captured.get('fields', [])) + if add_attachments: + logging.warning('File attachments are not yet supported for NSF pam project extend.') + + data = cmd._build_record_data( + params, + captured['record_type'], + captured.get('title', ''), + captured.get('notes'), + record_fields, + ) + + with command_error_handler('pam project extend'): + result = create_record_v3( + params, + folder_uid=folder_uid, + record_data=data, + record_uid=captured.get('record_uid'), + ) + check_result(result, 'pam project extend') + + uid = result.get('record_uid') or captured.get('record_uid') + if uid: + obj.uid = uid + scripts = getattr(obj, 'scripts', None) + if scripts and getattr(scripts, 'scripts', None): + add_pam_scripts(params, uid, scripts.scripts) + params.sync_data = True + return uid diff --git a/keepercommander/commands/pam_import/record_loader.py b/keepercommander/commands/pam_import/record_loader.py new file mode 100644 index 000000000..306614798 --- /dev/null +++ b/keepercommander/commands/pam_import/record_loader.py @@ -0,0 +1,67 @@ +# _ __ +# | |/ /___ ___ _ __ ___ _ _ ® +# | ' 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 {} + 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 + + +def load_pam_record(params, record_uid: str) -> Optional[vault.KeeperRecord]: + """Load a vault record from classic cache, NSF cache, or NSF record data.""" + record_uid = (record_uid or '').strip() + if not record_uid: + return None + + cached = get_record_from_cache(params, record_uid) + if cached and cached.get('data_unencrypted'): + rec = vault.KeeperRecord.load(params, cached) + if rec: + return rec + + rec = vault.KeeperRecord.load(params, record_uid) + if rec: + return rec + + nsf_record_data = getattr(params, 'nested_share_record_data', None) or {} + nsf_records = getattr(params, 'nested_share_records', None) or {} + rd = nsf_record_data.get(record_uid) or {} + if 'data_json' not in rd: + return None + + dj = rd['data_json'] + version = nsf_records.get(record_uid, {}).get('version', 3) + if version not in (3, 6): + return None + + keeper_record = vault.TypedRecord(version=version) + keeper_record.record_uid = record_uid + keeper_record.load_record_data(dj, None) + return keeper_record diff --git a/keepercommander/nested_share_folder/record_api.py b/keepercommander/nested_share_folder/record_api.py index f0bcc9e69..619a2078f 100644 --- a/keepercommander/nested_share_folder/record_api.py +++ b/keepercommander/nested_share_folder/record_api.py @@ -96,8 +96,8 @@ def record_update_v3(params, records, client_time=None, security_data_key_type=N def create_record_v3(params, record_type='', title='', fields=None, folder_uid=None, notes=None, custom_fields=None, - record_data=None): - uid = utils.generate_uid() + record_data=None, record_uid=None): + uid = record_uid or utils.generate_uid() rk = os.urandom(32) if record_data is not None: diff --git a/unit-tests/pam/test_pam_project_export.py b/unit-tests/pam/test_pam_project_export.py index 0f7900a5a..f12a37784 100644 --- a/unit-tests/pam/test_pam_project_export.py +++ b/unit-tests/pam/test_pam_project_export.py @@ -22,6 +22,8 @@ import unittest from unittest.mock import patch +import keepercommander.commands.record # noqa: F401 + from keepercommander import vault # ── record builders ──────────────────────────────────────────────────────── @@ -122,8 +124,9 @@ def setUp(self): self.params.record_cache = {uid: {} for uid in _RECORDS} def _execute(self, project_uid=CONFIG_UID, output=None): - """Run execute() with vault.KeeperRecord.load mocked.""" - with patch("keepercommander.vault.KeeperRecord.load", side_effect=_fake_load): + """Run execute() with load_pam_record mocked.""" + with patch("keepercommander.commands.pam_import.export.load_pam_record", + side_effect=_fake_load): with patch.object(self.cmd, "_get_allowed_settings", return_value=dict(_DEFAULT_ALLOWED)): kwargs = {"project_uid": project_uid} @@ -242,12 +245,14 @@ def test_output_flag_writes_file(self): # ── error handling ─────────────────────────────────────────── def test_missing_project_uid_returns_none(self): - with patch("keepercommander.vault.KeeperRecord.load", side_effect=_fake_load): + with patch("keepercommander.commands.pam_import.export.load_pam_record", + side_effect=_fake_load): result = self.cmd.execute(self.params, project_uid="", output=None) self.assertIsNone(result) def test_unknown_uid_returns_none(self): - with patch("keepercommander.vault.KeeperRecord.load", return_value=None): + with patch("keepercommander.commands.pam_import.export.load_pam_record", + return_value=None): result = self.cmd.execute(self.params, project_uid="unknown-uid", output=None) self.assertIsNone(result) @@ -256,7 +261,8 @@ def test_non_v6_record_returns_none(self): v3_rec.type_name = "pamMachine" v3_rec.title = "some" v3_rec.record_uid = "some-uid" - with patch("keepercommander.vault.KeeperRecord.load", return_value=v3_rec): + with patch("keepercommander.commands.pam_import.export.load_pam_record", + return_value=v3_rec): result = self.cmd.execute(self.params, project_uid="some-uid", output=None) self.assertIsNone(result) @@ -334,7 +340,8 @@ def setUp(self): def _execute(self): def _load(_p, uid): return self.records.get(uid) - with patch("keepercommander.vault.KeeperRecord.load", side_effect=_load): + with patch("keepercommander.commands.pam_import.export.load_pam_record", + side_effect=_load): with patch.object(self.cmd, "_get_allowed_settings", return_value=dict(_DEFAULT_ALLOWED)): return self.cmd.execute(self.params, project_uid=self.KCM_CFG) @@ -385,5 +392,86 @@ def test_uid_in_launch_credentials_accepted(self): self.assertEqual(users[0]["uid"], uid_22) +class TestPAMProjectExportNSF(unittest.TestCase): + """Export must resolve records stored only in NSF caches (not record_cache).""" + + NSF_CFG = "nsf-cfg-001" + NSF_MACHINE = "nsf-res-machine" + NSF_USER = "nsf-usr-admin" + + def setUp(self): + from keepercommander.commands.pam_import.export import PAMProjectExportCommand + self.cmd = PAMProjectExportCommand() + self.params = MagicMock() + self.params.record_cache = {} + self.params.nested_share_records = { + self.NSF_CFG: {"record_uid": self.NSF_CFG, "version": 6, "revision": 1}, + self.NSF_MACHINE: {"record_uid": self.NSF_MACHINE, "version": 3, "revision": 1}, + self.NSF_USER: {"record_uid": self.NSF_USER, "version": 3, "revision": 1}, + } + self.params.nested_share_record_data = { + self.NSF_CFG: { + "record_uid": self.NSF_CFG, + "data_json": { + "title": "NSF PAM Project", + "type": "pamNetworkConfiguration", + "fields": [{ + "type": "pamResources", + "value": [{ + "controllerUid": "gw-nsf", + "folderUid": "sf-nsf", + "resourceRef": [self.NSF_MACHINE], + }], + }], + }, + }, + self.NSF_MACHINE: { + "record_uid": self.NSF_MACHINE, + "data_json": { + "title": "NSF Linux Server", + "type": "pamMachine", + "fields": [{ + "type": "pamSettings", + "value": [{ + "connection": { + "userRecords": [self.NSF_USER], + "protocol": "ssh", + }, + }], + }], + }, + }, + self.NSF_USER: { + "record_uid": self.NSF_USER, + "data_json": { + "title": "NSF Admin", + "type": "pamUser", + "fields": [{"type": "login", "value": ["root"]}], + }, + }, + } + + def _execute(self): + with patch.object(self.cmd, "_get_allowed_settings", + return_value=dict(_DEFAULT_ALLOWED)): + return self.cmd.execute(self.params, project_uid=self.NSF_CFG) + + def test_nsf_config_export_succeeds(self): + parsed = json.loads(self._execute()) + self.assertEqual(parsed["project"], "NSF PAM Project") + self.assertEqual(parsed["pam_configuration"]["environment"], "local") + + def test_nsf_resources_and_users_exported(self): + parsed = json.loads(self._execute()) + self.assertEqual(len(parsed["pam_data"]["resources"]), 1) + res = parsed["pam_data"]["resources"][0] + self.assertEqual(res["uid"], self.NSF_MACHINE) + self.assertEqual(res["type"], "pamMachine") + self.assertEqual(len(res["users"]), 1) + self.assertEqual(res["users"][0]["uid"], self.NSF_USER) + self.assertEqual(len(parsed["pam_data"]["users"]), 1) + self.assertEqual(parsed["pam_data"]["users"][0]["login"], "root") + + if __name__ == "__main__": unittest.main() diff --git a/unit-tests/pam/test_pam_project_extend_nsf.py b/unit-tests/pam/test_pam_project_extend_nsf.py index 4ff0e93fe..ae372ee99 100644 --- a/unit-tests/pam/test_pam_project_extend_nsf.py +++ b/unit-tests/pam/test_pam_project_extend_nsf.py @@ -53,7 +53,7 @@ def test_get_nsf_project_folders_from_config_folder_siblings(): def test_get_app_shared_folders_falls_back_to_nsf_project_folders(): params = _params() - with patch('keepercommander.commands.pam_import.extend.KSMCommand.get_app_info', + with patch('keepercommander.commands.ksm.KSMCommand.get_app_info', return_value=[]): folders = PAMProjectExtendCommand().get_app_shared_folders(params, 'ksm_uid', 'config_uid') @@ -66,12 +66,24 @@ def test_create_subfolder_uses_nsf_api_under_nsf_parent(): with patch('keepercommander.nested_share_folder.folder_api.create_folder_v3', return_value={'success': True, 'folder_uid': 'child_nsf'}) as create_folder, \ patch('keepercommander.commands.pam_import.extend.api.sync_down'): - uid = PAMProjectExtendCommand().create_subfolder(params, 'Child', 'users_nsf', folder_uid='pre_generated') + uid = PAMProjectExtendCommand().create_subfolder(params, 'Child', 'users_nsf') assert uid == 'child_nsf' create_folder.assert_called_once_with(params, 'Child', parent_uid='users_nsf') +def test_create_subfolder_uses_pre_generated_uid_for_nsf(): + params = _params() + + with patch('keepercommander.commands.pam_import.extend.create_nsf_subfolder', + return_value='pre_generated') as create_sub: + uid = PAMProjectExtendCommand().create_subfolder( + params, 'Child', 'users_nsf', folder_uid='pre_generated') + + assert uid == 'pre_generated' + create_sub.assert_called_once_with(params, 'Child', 'users_nsf', folder_uid='pre_generated') + + def test_process_folders_creates_new_nsf_folder_paths(): params = _params() project = { diff --git a/unit-tests/pam/test_pam_project_extend_nsf_helpers.py b/unit-tests/pam/test_pam_project_extend_nsf_helpers.py new file mode 100644 index 000000000..1fd3299d9 --- /dev/null +++ b/unit-tests/pam/test_pam_project_extend_nsf_helpers.py @@ -0,0 +1,114 @@ +# _ __ +# | |/ /___ ___ _ __ ___ _ _ ® +# | ' Date: Mon, 6 Jul 2026 10:16:20 +0530 Subject: [PATCH 09/14] Fix NSF helper imports and extend folder creation test. Use commands.nested_share_folder.helpers (not nested_share_folder.helpers) and mock create_nsf_subfolder in the process_folders test. Co-authored-by: Cursor --- keepercommander/commands/pam_import/nsf_helpers.py | 5 ++--- unit-tests/pam/test_pam_project_extend_nsf.py | 9 +++------ 2 files changed, 5 insertions(+), 9 deletions(-) diff --git a/keepercommander/commands/pam_import/nsf_helpers.py b/keepercommander/commands/pam_import/nsf_helpers.py index 3b92c66a3..cadc64403 100644 --- a/keepercommander/commands/pam_import/nsf_helpers.py +++ b/keepercommander/commands/pam_import/nsf_helpers.py @@ -182,9 +182,8 @@ def get_records_in_folder(params, folder_uid: str) -> list: def create_nsf_subfolder(params, folder_name: str, parent_uid: str = '', folder_uid: Optional[str] = None) -> str: """Create an NSF subfolder; returns folder UID.""" - from ... import nested_share_folder as _nsf from ...nested_share_folder.folder_api import _prepare_folder_for_creation, folder_add_v3 - from ...nested_share_folder.helpers import command_error_handler, check_result + from ..nested_share_folder.helpers import command_error_handler, check_result from ...proto import folder_pb2 name = str(folder_name or '').strip() @@ -245,7 +244,7 @@ def execute(self, _params, **kwargs): obj.create_record(params, folder_uid) from ..nested_share_folder.record_commands import NestedShareRecordAddCommand - from ...nested_share_folder.helpers import command_error_handler, check_result + from ..nested_share_folder.helpers import command_error_handler, check_result from ...nested_share_folder.record_api import create_record_v3 cmd = NestedShareRecordAddCommand() diff --git a/unit-tests/pam/test_pam_project_extend_nsf.py b/unit-tests/pam/test_pam_project_extend_nsf.py index ae372ee99..86dfa7478 100644 --- a/unit-tests/pam/test_pam_project_extend_nsf.py +++ b/unit-tests/pam/test_pam_project_extend_nsf.py @@ -102,14 +102,11 @@ def test_process_folders_creates_new_nsf_folder_paths(): 'error_count': 0, } - def create_folder(params_arg, folder_name, parent_uid=None): - params_arg.nested_share_folders['admins_nsf'] = {'name': folder_name, 'parent_uid': parent_uid} - return {'success': True, 'folder_uid': 'admins_nsf'} - - with patch('keepercommander.nested_share_folder.folder_api.create_folder_v3', - side_effect=create_folder), \ + with patch('keepercommander.commands.pam_import.extend.create_nsf_subfolder', + return_value='admins_nsf') as create_sub, \ patch('keepercommander.commands.pam_import.extend.api.sync_down'): folders = PAMProjectExtendCommand().process_folders(params, project) + create_sub.assert_called_once() assert folders['path_to_folder_uid']['NSF Project - Users/Admins'] == 'admins_nsf' assert project['error_count'] == 0 From b1b779ca7250e4a181d103bbdecd66f827da0e52 Mon Sep 17 00:00:00 2001 From: adeshmukh-ks Date: Mon, 6 Jul 2026 16:35:12 +0530 Subject: [PATCH 10/14] added nsf support to ksm and gateway commands --- keepercommander/commands/discoveryrotation.py | 113 ++++--- keepercommander/commands/ksm.py | 301 +++++++++++++++--- keepercommander/commands/pam/config_helper.py | 68 +++- .../nested_share_folder/__init__.py | 2 +- .../nested_share_folder/record_api.py | 13 + unit-tests/pam/test_pam_nsf_config.py | 22 +- unit-tests/pam/test_pam_rotation.py | 113 ++++++- unit-tests/test_command_ksm.py | 130 ++++++++ 8 files changed, 639 insertions(+), 123 deletions(-) diff --git a/keepercommander/commands/discoveryrotation.py b/keepercommander/commands/discoveryrotation.py index 4785a7335..4c976f8b2 100644 --- a/keepercommander/commands/discoveryrotation.py +++ b/keepercommander/commands/discoveryrotation.py @@ -33,9 +33,8 @@ resolve_pam_record, record_exists_in_vault, collect_pam_folder_uids, get_vault_record_title_type, find_pam_records_by_search, resolve_pam_config_folder_info, pam_folder_json_payload, place_record_in_folder, - create_record_in_folder, update_pam_record, records_in_folder, + update_pam_record, records_in_folder, ) -from .nested_share_folder.helpers import is_nested_share_folder from .pam.config_helper import configuration_controller_get, \ pam_configurations_get_all, pam_configuration_remove, \ pam_configuration_create_record_v6, record_rotation_get, \ @@ -59,6 +58,7 @@ from .tunnel.port_forward.TunnelGraph import TunnelDAG, get_vertex_content from .tunnel.port_forward.tunnel_helpers import get_config_uid, get_keeper_tokens from .. import api, utils, vault_extensions, crypto, vault, record_management, attachment, record_facades +from ..recordv3 import RecordV3 from ..display import bcolors from ..error import CommandError, KeeperApiError from ..params import KeeperParams, LAST_RECORD_UID @@ -265,6 +265,22 @@ def uses_default_rotation_schedule(params, record_uid, configuration_uid): if not isinstance(record_schedule, list) or len(record_schedule) == 0: return False return record_schedule == default_schedule + +def _valid_record_uids(uids): + return [uid for uid in (uids or []) if RecordV3.is_valid_ref_uid(uid)] + + +def _get_pam_config_resource_uids(pam_config_record): + if not pam_config_record: + return [] + resource_field = pam_config_record.get_typed_field('pamResources') + if resource_field and isinstance(resource_field.value, list) and len(resource_field.value) > 0: + resources = resource_field.value[0] + if isinstance(resources, dict): + refs = resources.get('resourceRef') + if isinstance(refs, list): + return _valid_record_uids(refs) + return [] def register_commands(commands): @@ -547,7 +563,7 @@ def config_resource(_dag, target_record, target_config_uid, silent=None): # Add DAG for resource if target_config_uid: _dag = TunnelDAG(params, encrypted_session_token, encrypted_transmission_key, target_config_uid, - transmission_key=transmission_key) + is_config=True, transmission_key=transmission_key) _dag.edit_tunneling_config(rotation=True) else: raise CommandError('', f'{bcolors.FAIL}Resource "{target_record.record_uid}" is not associated ' @@ -1201,22 +1217,31 @@ def config_user(_dag, target_record, target_resource_uid, target_config_uid=None f"--admin-user ADMIN{bcolors.ENDC}") current_record_rotation = params.record_rotation_cache.get(target_record.record_uid) - if not _dag or not _dag.linking_dag.has_graph: - _dag = TunnelDAG(params, encrypted_session_token, encrypted_transmission_key, target_resource_uid, - transmission_key=transmission_key) + if target_config_uid and (not _dag or not _dag.linking_dag.has_graph + or _dag.record.record_uid != target_config_uid): + _dag = TunnelDAG(params, encrypted_session_token, encrypted_transmission_key, target_config_uid, + is_config=True, transmission_key=transmission_key) if not _dag.linking_dag.has_graph: - raise CommandError('', f'{bcolors.FAIL}Resource "{target_resource_uid}" is not associated ' + _dag.edit_tunneling_config(rotation=True) + elif not _dag or not _dag.linking_dag.has_graph: + if RecordV3.is_valid_ref_uid(target_resource_uid): + _dag = TunnelDAG(params, encrypted_session_token, encrypted_transmission_key, target_resource_uid, + transmission_key=transmission_key) + if not _dag or not _dag.linking_dag.has_graph: + raise CommandError('', f'{bcolors.FAIL}Record "{target_record.record_uid}" is not associated ' f'with any configuration. ' - f'{bcolors.OKBLUE}pam rotation edit -rs {target_resource_uid} ' - f'--config CONFIG{bcolors.ENDC}') + f'{bcolors.OKBLUE}pam rotation edit -r {target_record.record_uid} ' + f'--config CONFIG [--resource RESOURCE]{bcolors.ENDC}') # Noop and resource cannot be both assigned if noop_rotation: target_resource_uid = target_record.record_uid record_resource_uid = None else: - if not target_resource_uid: + if not RecordV3.is_valid_ref_uid(target_resource_uid): # Get the resource configuration from DAG - resource_uids = _dag.get_all_owners(target_record.record_uid) + resource_uids = _valid_record_uids(_dag.get_all_owners(target_record.record_uid)) + if len(resource_uids) == 0 and pam_config: + resource_uids = _get_pam_config_resource_uids(pam_config) if len(resource_uids) > 1: # When processing folders, skip records with multiple resources if folder_name: @@ -1239,17 +1264,23 @@ def config_user(_dag, target_record, target_resource_uid, target_config_uid=None f'it.{bcolors.ENDC}') target_resource_uid = resource_uids[0] - if not _dag.resource_belongs_to_config(target_resource_uid): + if (RecordV3.is_valid_ref_uid(target_resource_uid) + and not _dag.resource_belongs_to_config(target_resource_uid)): # some rotations (iam_user/noop) link straight to pamConfiguration if target_resource_uid != _dag.record.record_uid: - raise CommandError('', - f'{bcolors.FAIL}Resource "{target_resource_uid}" is not associated with the ' - f'configuration of the user "{target_record.record_uid}". To associated the resources ' - f'to this config run {bcolors.OKBLUE}"pam rotation resource {target_resource_uid} ' - f'--config {_dag.record.record_uid}"{bcolors.ENDC}') - if not _dag.user_belongs_to_resource(target_record.record_uid, target_resource_uid): + if target_config_uid: + _dag.link_resource_to_config(target_resource_uid) + else: + raise CommandError('', + f'{bcolors.FAIL}Resource "{target_resource_uid}" is not associated with the ' + f'configuration of the user "{target_record.record_uid}". To associated the resources ' + f'to this config run {bcolors.OKBLUE}"pam rotation resource {target_resource_uid} ' + f'--config {_dag.record.record_uid}"{bcolors.ENDC}') + if (RecordV3.is_valid_ref_uid(target_resource_uid) + and not _dag.user_belongs_to_resource(target_record.record_uid, target_resource_uid)): old_resource_uid = _dag.get_resource_uid(target_record.record_uid) - if old_resource_uid is not None and old_resource_uid != target_resource_uid: + if (RecordV3.is_valid_ref_uid(old_resource_uid) + and old_resource_uid != target_resource_uid): print( f'{bcolors.WARNING}User "{target_record.record_uid}" is associated with another resource: ' f'{old_resource_uid}. ' @@ -1876,19 +1907,8 @@ def execute(self, params, **kwargs): continue ksm_app_uid_str = utils.base64_url_encode(c.applicationUid) - ksm_app = KSMCommand.get_app_record(params, ksm_app_uid_str) - - if ksm_app: - ksm_app_data_unencrypted_json = ksm_app.get('data_unencrypted') - ksm_app_data_unencrypted_dict = json.loads(ksm_app_data_unencrypted_json) - ksm_app_title = ksm_app_data_unencrypted_dict.get('title') - ksm_app_info_plain = f'{ksm_app_title} ({ksm_app_uid_str})' - ksm_app_name = ksm_app_title - ksm_app_accessible = True - else: - ksm_app_info_plain = f'[APP NOT ACCESSIBLE OR DELETED] ({ksm_app_uid_str})' - ksm_app_name = None - ksm_app_accessible = False + ksm_app_name, ksm_app_accessible, ksm_app_info_plain = KSMCommand.get_ksm_app_display_info( + params, ksm_app_uid_str) # Check if this is gateway pool is_pool = len(connected_instances) > 1 @@ -2848,12 +2868,8 @@ def execute(self, params, **kwargs): self.verify_required(record) - if is_nested_share_folder(params, shared_folder_uid): - create_record_in_folder(params, record, shared_folder_uid, command='pam-config-new') - else: - pam_configuration_create_record_v6(params, record, shared_folder_uid) - api.sync_down(params) - place_record_in_folder(params, record.record_uid, shared_folder_uid, command='pam-config-new') + pam_configuration_create_record_v6(params, record, shared_folder_uid) + api.sync_down(params) encrypted_session_token, encrypted_transmission_key, transmission_key = get_keeper_tokens(params) # Add DAG for configuration @@ -4239,8 +4255,9 @@ class PAMCreateGatewayCommand(Command): help='Name of the Gateway', action='store') dr_create_controller_parser.add_argument('--application', '-a', required=True, dest='ksm_app', - help='KSM Application name or UID. Use command `sm app list` to view ' - 'available KSM Applications.', action='store') + help='KSM Application name, UID, or NSF path. Use ' + '`secrets-manager app list` to view available applications.', + action='store') dr_create_controller_parser.add_argument('--token-expires-in-min', '-e', type=int, dest='token_expire_in_min', action='store', help='Time for the one time token to expire. Maximum 1440 minutes (24 hrs). Default: 60', @@ -4268,12 +4285,24 @@ def execute(self, params, **kwargs): logging.debug(f'ksm_app =[{ksm_app}]') logging.debug(f'ott_expire_in_min =[{ott_expire_in_min}]') - one_time_token = gateway_helper.create_gateway(params, gateway_name, ksm_app, config_init, ott_expire_in_min) + ksm_app_record = KSMCommand.get_app_record(params, ksm_app) + if not ksm_app_record: + raise CommandError( + 'pam gateway new', + f'KSM Application "{ksm_app}" not found. Run sync-down and verify the application ' + f'exists in your vault or Nested Share Folder.') + + ksm_app_uid = ksm_app_record.get('record_uid', ksm_app) + ksm_app_title, _, _ = KSMCommand.get_ksm_app_display_info(params, ksm_app_uid) + + one_time_token = gateway_helper.create_gateway( + params, gateway_name, ksm_app_uid, config_init, ott_expire_in_min) if is_return_value: return one_time_token else: - print(f'The one time token has been created in application [{bcolors.OKBLUE}{ksm_app}{bcolors.ENDC}].\n\n' + print(f'The one time token has been created in application ' + f'[{bcolors.OKBLUE}{ksm_app_title or ksm_app_uid}{bcolors.ENDC}].\n\n' f'The new Gateway named {bcolors.OKBLUE}{gateway_name}{bcolors.ENDC} will show up in a list ' f'of gateways once it is initialized.\n\n') diff --git a/keepercommander/commands/ksm.py b/keepercommander/commands/ksm.py index e55463f19..d71361fdd 100644 --- a/keepercommander/commands/ksm.py +++ b/keepercommander/commands/ksm.py @@ -26,6 +26,11 @@ from .base import Command, dump_report_data, user_choice, as_boolean from . import record +from ..nested_share_folder.common import get_folder_key, get_record_key +from .nested_share_folder.helpers import ( + is_nested_share_folder, is_nested_share_record, load_record_metadata, resolve_folder_uid) +from ..nested_share_folder.removal_api import ( + resolve_nested_share_folder_uid, resolve_nested_share_record_uid) from .. import api, utils, crypto, vault from ..params import KeeperParams from ..display import bcolors @@ -90,7 +95,7 @@ --force : Do not prompt for confirmation {bcolors.BOLD}Add Secret to Application:{bcolors.ENDC} - {bcolors.OKGREEN}secrets-manager share add --app {bcolors.OKBLUE}[APP NAME OR UID] {bcolors.OKGREEN}--secret {bcolors.OKBLUE}[RECORD OR SHARED FOLDER UID]{bcolors.ENDC} + {bcolors.OKGREEN}secrets-manager share add --app {bcolors.OKBLUE}[APP NAME OR UID] {bcolors.OKGREEN}--secret {bcolors.OKBLUE}[RECORD, SHARED FOLDER, OR NSF UID/PATH]{bcolors.ENDC} Options: --editable : Allow secrets to be editable by the client @@ -130,7 +135,7 @@ help='One of: "app list", "app get", "app create", "app update", "app remove", "app share", ' + '"app unshare", "client add", "client remove", "share add", "share update", "share remove" or "token add"') ksm_parser.add_argument('--secret', '-s', type=str, action='append', required=False, - help='Record UID') + help='Record, shared folder, or Nested Share Folder (NSF) UID or path') ksm_parser.add_argument('--app', '-a', type=str, action='store', required=False, help='Application Name or UID') ksm_parser.add_argument('--client', '-i', type=str, dest='client_names_or_ids', action='append', required=False, @@ -593,11 +598,15 @@ def add_app_share(params, secret_uids, app_name_or_uid, is_editable): app_record_uid = rec_cache_val.get('record_uid') master_key = rec_cache_val.get('record_key_unencrypted') + resolved_uids = KSMCommand.resolve_secret_uids(params, secret_uids) + if not resolved_uids: + return + KSMCommand.share_secret( params=params, app_uid=app_record_uid, master_key=master_key, - secret_uids=secret_uids, + secret_uids=resolved_uids, is_editable=is_editable ) @@ -838,8 +847,6 @@ def shorten_client_id(all_clients, original_id, number_of_characters): print(bcolors.BOLD + "\nApplication Access\n" + bcolors.ENDC) if ai.shares: - recs = params.record_cache - for s in ai.shares: uid_str = utils.base64_url_encode(s.secretUid) sht = APIRequest_pb2.ApplicationShareType.Name(s.shareType) @@ -851,18 +858,17 @@ def shorten_client_id(all_clients, original_id, number_of_characters): } if sht == 'SHARE_TYPE_RECORD': - rec = recs.get(uid_str) - if rec: - record_data_dict = KSMCommand.record_data_as_dict(rec) - share_data["title"] = record_data_dict.get('title') - share_data["type"] = "RECORD" + share_data["title"] = KSMCommand.get_secret_title(params, uid_str, sht) + share_data["type"] = "RECORD" + #ToDo: check if type nsf record is valid here + if is_nested_share_record(params, uid_str): + share_data["type"] = "NSF RECORD" elif sht == 'SHARE_TYPE_FOLDER': - if uid_str in params.shared_folder_cache: - cached_sf = params.shared_folder_cache[uid_str] - share_data["title"] = cached_sf.get('name_unencrypted') - share_data["type"] = "FOLDER" - else: - continue + share_data["title"] = KSMCommand.get_secret_title(params, uid_str, sht) + share_data["type"] = "FOLDER" + #ToDo: check if type nsf folder is valid here + if is_nested_share_folder(params, uid_str): + share_data["type"] = "NSF FOLDER" else: logging.warning("Unknown Share Type %s" % sht) continue @@ -900,6 +906,96 @@ def shorten_client_id(all_clients, original_id, number_of_characters): else: return json.dumps({"applications": result_data}, indent=2) + @staticmethod + def _secret_in_cache(params, uid): + if uid in getattr(params, 'record_cache', {}): + return True + if uid in getattr(params, 'nested_share_records', {}): + return True + if api.is_shared_folder(params, uid): + return True + if is_nested_share_folder(params, uid): + return True + return False + + @staticmethod + def resolve_secret_uid(params, identifier): + if not identifier: + return None + if KSMCommand._secret_in_cache(params, identifier): + return identifier + record_uid = resolve_nested_share_record_uid(params, identifier) + if record_uid: + return record_uid + folder_uid = resolve_folder_uid(params, identifier) + if folder_uid and (api.is_shared_folder(params, folder_uid) + or is_nested_share_folder(params, folder_uid)): + return folder_uid + folder_uid = resolve_nested_share_folder_uid(params, identifier) + if folder_uid and is_nested_share_folder(params, folder_uid): + return folder_uid + return None + + @staticmethod + def resolve_secret_uids(params, identifiers): + if not identifiers: + return [] + if isinstance(identifiers, str): + identifiers = [identifiers] + resolved = [] + for ident in identifiers: + uid = KSMCommand.resolve_secret_uid(params, ident) + if uid: + resolved.append(uid) + else: + logging.warning('Could not resolve secret "%s". Run sync-down and try again.', ident) + return resolved + + @staticmethod + def classify_secret(params, uid): + share_key = None + share_type = None + + if uid in getattr(params, 'record_cache', {}) or is_nested_share_record(params, uid): + share_type = 'SHARE_TYPE_RECORD' + if uid in params.record_cache: + share_key = params.record_cache[uid].get('record_key_unencrypted') + if not share_key: + share_key = get_record_key(params, uid, raise_on_missing=False) + elif api.is_shared_folder(params, uid) or is_nested_share_folder(params, uid): + share_type = 'SHARE_TYPE_FOLDER' + cached_sf = getattr(params, 'shared_folder_cache', {}).get(uid, {}) + share_key = cached_sf.get('shared_folder_key_unencrypted') + if not share_key: + share_key = get_folder_key(params, uid, raise_on_missing=False) + + if share_type and share_key: + return {'uid': uid, 'share_type': share_type, 'share_key': share_key} + return None + + @staticmethod + def get_secret_title(params, uid, share_type): + if share_type == 'SHARE_TYPE_RECORD': + rec = getattr(params, 'record_cache', {}).get(uid) + if rec and rec.get('data_unencrypted'): + try: + return KSMCommand.record_data_as_dict(rec).get('title', uid) + except Exception: + pass + if is_nested_share_record(params, uid): + return load_record_metadata(params, uid).get('title', uid) + elif share_type == 'SHARE_TYPE_FOLDER': + cached_sf = getattr(params, 'shared_folder_cache', {}).get(uid, {}) + if cached_sf.get('name_unencrypted'): + return cached_sf.get('name_unencrypted') + nsf = getattr(params, 'nested_share_folders', {}).get(uid, {}) + if nsf.get('name'): + return nsf.get('name') + folder = getattr(params, 'folder_cache', {}).get(uid) + if folder and getattr(folder, 'name', None): + return folder.name + return uid + @staticmethod def share_secret(params, app_uid, master_key, secret_uids, is_editable=False): @@ -908,25 +1004,17 @@ def share_secret(params, app_uid, master_key, secret_uids, is_editable=False): added_secret_uids_type_pairs = [] for uid in secret_uids: - is_record = uid in params.record_cache - is_shared_folder = api.is_shared_folder(params, uid) - - if is_record: - rec = params.record_cache[uid] - share_key_decrypted = rec['record_key_unencrypted'] - share_type = 'SHARE_TYPE_RECORD' - elif is_shared_folder: - cached_sf = params.shared_folder_cache[uid] - shared_folder_key_unencrypted = cached_sf.get('shared_folder_key_unencrypted') - share_key_decrypted = shared_folder_key_unencrypted - share_type = 'SHARE_TYPE_FOLDER' - else: - print(f"""{bcolors.WARNING}\tUID="{uid}" is not a Record nor Shared Folder. Only individual records or - Shared Folders can be added to the application.{bcolors.ENDC} Make sure your local cache is up to date by + secret = KSMCommand.classify_secret(params, uid) + if not secret: + print(f"""{bcolors.WARNING}\tUID="{uid}" is not a Record, Shared Folder, or Nested Share Folder record. + Only individual records or folders can be added to the application.{bcolors.ENDC} Make sure your local cache is up to date by running 'sync-down' command and trying again.""") - continue + share_type = secret['share_type'] + share_key_decrypted = secret['share_key'] + uid = secret['uid'] + added_secret_uids_type_pairs.append((uid, share_type)) encrypted_secret_key = crypto.encrypt_aes_v2(share_key_decrypted, master_key) @@ -950,7 +1038,10 @@ def share_secret(params, app_uid, master_key, secret_uids, is_editable=False): api.communicate_rest(params, app_share_add_rq, 'vault/app_share_add') print(bcolors.OKGREEN + f'\nSuccessfully added secrets to app uid={app_uid}, ' f'editable=' + bcolors.BOLD + f'{is_editable}:' + bcolors.ENDC) - print('\n'.join(map(lambda x: ('\t' + str(x[0])) + ' ' + ('Record' if ('RECORD' in str(x[1])) else 'Shared Folder'), added_secret_uids_type_pairs))) + print('\n'.join(map(lambda x: ('\t' + str(x[0])) + ' ' + ( + 'Record' if 'RECORD' in str(x[1]) else + 'Nested Share Folder' if is_nested_share_folder(params, x[0]) else 'Shared Folder'), + added_secret_uids_type_pairs))) print('\n') return True except KeeperApiError as kae: @@ -961,21 +1052,125 @@ def share_secret(params, app_uid, master_key, secret_uids, is_editable=False): raise kae return False + @staticmethod + def _is_ksm_app_data(data_dict): + return isinstance(data_dict, dict) and data_dict.get('type') == 'app' + + @staticmethod + def _app_record_from_cache_entry(rec_cache_val): + if not rec_cache_val or rec_cache_val.get('version') != 5: + return None + data_unencrypted = rec_cache_val.get('data_unencrypted') + if not data_unencrypted: + return None + try: + data_dict = json.loads(data_unencrypted.decode('utf-8')) + except Exception: + return None + if KSMCommand._is_ksm_app_data(data_dict): + return rec_cache_val + return None + + @staticmethod + def _normalize_nsf_app_record(params, record_uid): + nsf_rec = getattr(params, 'nested_share_records', {}).get(record_uid) + if not nsf_rec: + return None + + nsf_data = getattr(params, 'nested_share_record_data', {}).get(record_uid, {}) + data_json = nsf_data.get('data_json', {}) + if not KSMCommand._is_ksm_app_data(data_json): + return None + + record_key = nsf_rec.get('record_key_unencrypted') + if not record_key: + record_key = get_record_key(params, record_uid, raise_on_missing=False) + if not record_key: + return None + + return { + 'record_uid': record_uid, + 'version': nsf_rec.get('version', 5), + 'revision': nsf_rec.get('revision', 0), + 'record_key_unencrypted': record_key, + 'data_unencrypted': json.dumps(data_json).encode('utf-8'), + 'source': 'nested_share_folder', + } + @staticmethod def get_app_record(params, app_name_or_uid): + if not app_name_or_uid: + return None + + if app_name_or_uid in getattr(params, 'record_cache', {}): + rec = KSMCommand._app_record_from_cache_entry(params.record_cache[app_name_or_uid]) + if rec: + return rec + + nsf_rec = KSMCommand._normalize_nsf_app_record(params, app_name_or_uid) + if nsf_rec: + return nsf_rec for rec_cache_val in params.record_cache.values(): + rec = KSMCommand._app_record_from_cache_entry(rec_cache_val) + if not rec: + continue + r_uid = rec.get('record_uid') + try: + r_unencr_dict = json.loads(rec.get('data_unencrypted').decode('utf-8')) + except Exception: + continue + if r_unencr_dict.get('title') == app_name_or_uid or r_uid == app_name_or_uid: + return rec + + resolved_uid = resolve_nested_share_record_uid(params, app_name_or_uid) + if resolved_uid: + if resolved_uid in getattr(params, 'record_cache', {}): + rec = KSMCommand._app_record_from_cache_entry(params.record_cache[resolved_uid]) + if rec: + return rec + nsf_rec = KSMCommand._normalize_nsf_app_record(params, resolved_uid) + if nsf_rec: + return nsf_rec - if rec_cache_val.get('version') == 5: - r_uid = rec_cache_val.get('record_uid') - r_unencr_json_data = rec_cache_val.get('data_unencrypted').decode('utf-8') - r_unencr_dict = json.loads(r_unencr_json_data) + return None - if r_unencr_dict.get('title') == app_name_or_uid or r_uid == app_name_or_uid: - return rec_cache_val + @staticmethod + def get_app_title(params, app_uid): + rec = KSMCommand.get_app_record(params, app_uid) + if rec and rec.get('data_unencrypted'): + try: + return json.loads(rec.get('data_unencrypted').decode('utf-8')).get('title', app_uid) + except Exception: + pass + if is_nested_share_record(params, app_uid): + meta = load_record_metadata(params, app_uid) + if meta.get('type') == 'app': + return meta.get('title', app_uid) + + nsf_data = getattr(params, 'nested_share_record_data', {}).get(app_uid, {}) + data_json = nsf_data.get('data_json', {}) + if isinstance(data_json, dict) and data_json.get('type') == 'app': + return data_json.get('title', app_uid) return None + @staticmethod + def get_ksm_app_display_info(params, app_uid_str): + ksm_app = KSMCommand.get_app_record(params, app_uid_str) + if ksm_app: + try: + title = json.loads(ksm_app.get('data_unencrypted').decode('utf-8')).get('title', app_uid_str) + except Exception: + title = app_uid_str + return title, True, f'{title} ({app_uid_str})' + + title = KSMCommand.get_app_title(params, app_uid_str) + if title: + return title, False, f'{title} ({app_uid_str})' + + return None, False, f'[APP NOT ACCESSIBLE OR DELETED] ({app_uid_str})' + @staticmethod def search_app_records(params, search_str): @@ -1163,6 +1358,10 @@ def update_app_share(params, secret_uids, app_name_or_uid, is_editable): app_record_uid = rec_cache_val.get('record_uid') master_key = rec_cache_val.get('record_key_unencrypted') + resolved_secret_uids = KSMCommand.resolve_secret_uids(params, secret_uids) + if not resolved_secret_uids: + return + app_info = KSMCommand.get_app_info(params, app_record_uid) existing_shares = { utils.base64_url_encode(s.secretUid): s @@ -1170,7 +1369,7 @@ def update_app_share(params, secret_uids, app_name_or_uid, is_editable): } uids_to_update = [] - for uid in secret_uids: + for uid in resolved_secret_uids: if uid not in existing_shares: logging.warning('Secret "%s" is not currently shared with this application. ' 'Use "share add" to add it first.' % uid) @@ -1198,19 +1397,15 @@ def update_app_share(params, secret_uids, app_name_or_uid, is_editable): app_shares = [] for uid in uids_to_update: - is_record = uid in params.record_cache - is_shared_folder = api.is_shared_folder(params, uid) - - if is_record: - share_key = params.record_cache[uid]['record_key_unencrypted'] - share_type = 'SHARE_TYPE_RECORD' - elif is_shared_folder: - share_key = params.shared_folder_cache[uid].get('shared_folder_key_unencrypted') - share_type = 'SHARE_TYPE_FOLDER' - else: + secret = KSMCommand.classify_secret(params, uid) + if not secret: logging.warning('UID "%s" not found in local cache. Run sync-down and try again.' % uid) continue + share_key = secret['share_key'] + share_type = secret['share_type'] + uid = secret['uid'] + encrypted_secret_key = crypto.encrypt_aes_v2(share_key, master_key) app_share = APIRequest_pb2.AppShareAdd() @@ -1248,10 +1443,14 @@ def remove_share(params, app_name_or_uid, secret_uids): app_uid = app.get('record_uid') + resolved_uids = KSMCommand.resolve_secret_uids(params, secret_uids) + if not resolved_uids: + raise Exception("No valid record or folder secrets were found for removal") + rq = APIRequest_pb2.RemoveAppSharesRequest() rq.appRecordUid = utils.base64_url_decode(app_uid) - rq.shares.extend((utils.base64_url_decode(x) for x in secret_uids)) + rq.shares.extend((utils.base64_url_decode(x) for x in resolved_uids)) api.communicate_rest(params, rq, 'vault/app_share_remove') print(bcolors.OKGREEN + "Secret share was successfully removed from the application\n" + bcolors.ENDC) diff --git a/keepercommander/commands/pam/config_helper.py b/keepercommander/commands/pam/config_helper.py index d04bb87aa..fb8d51795 100644 --- a/keepercommander/commands/pam/config_helper.py +++ b/keepercommander/commands/pam/config_helper.py @@ -5,9 +5,12 @@ from keeper_secrets_manager_core.utils import string_to_bytes, bytes_to_string from ..record import RecordRemoveCommand -from ... import api, crypto, utils, vault, vault_extensions +from ... import api, crypto, utils, vault, vault_extensions, subfolder +from ...error import KeeperApiError +from ...nested_share_folder.common import get_folder_key +from ...nested_share_folder.record_api import create_record_data_v3, record_add_pam_configuration_v3 from ...params import KeeperParams -from ...proto import pam_pb2, router_pb2 +from ...proto import folder_pb2, pam_pb2, record_pb2, router_pb2 def pam_decrypt_configuration_data(pam_config_v6_record): @@ -64,8 +67,26 @@ def pam_configuration_get_field(unencrypted_record, field_identifier): return field +def _resolve_pam_configuration_folder_key(params, folder_uid): + # type: (KeeperParams, str) -> bytes | None + folder_key = get_folder_key(params, folder_uid, raise_on_missing=False) + if folder_key: + return folder_key + + folder = params.folder_cache.get(folder_uid) + shared_folder_uid = None + if isinstance(folder, subfolder.SharedFolderFolderNode): + shared_folder_uid = folder.shared_folder_uid + elif isinstance(folder, subfolder.SharedFolderNode): + shared_folder_uid = folder.uid + if shared_folder_uid and shared_folder_uid in params.shared_folder_cache: + shared_folder = params.shared_folder_cache.get(shared_folder_uid) + return shared_folder.get('shared_folder_key_unencrypted') + return None + + def pam_configuration_create_record_v6(params, record, folder_uid): - # type: (KeeperParams, vault.TypedRecord, str, str) -> None + # type: (KeeperParams, vault.TypedRecord, str) -> None if not record.record_uid: record.record_uid = utils.generate_uid() @@ -73,14 +94,39 @@ def pam_configuration_create_record_v6(params, record, folder_uid): record.record_key = utils.generate_aes_key() record_data = vault_extensions.extract_typed_record_data(record) - json_data = api.get_record_data_json_bytes(record_data) - - car = pam_pb2.ConfigurationAddRequest() - car.configurationUid = utils.base64_url_decode(record.record_uid) - car.recordKey = crypto.encrypt_aes_v2(record.record_key, params.data_key) - car.data = crypto.encrypt_aes_v2(json_data, record.record_key) - - api.communicate_rest(params, car, 'pam/add_configuration_record') + folder_key = _resolve_pam_configuration_folder_key(params, folder_uid) if folder_uid else None + + if folder_uid and folder_key: + record_add = create_record_data_v3( + record_uid=record.record_uid, + record_key=record.record_key, + data=record_data, + folder_uid=folder_uid, + folder_key=folder_key, + client_modified_time=utils.current_milli_time(), + ) + else: + record_add = create_record_data_v3( + record_uid=record.record_uid, + record_key=record.record_key, + data=record_data, + data_key=params.data_key, + client_modified_time=utils.current_milli_time(), + ) + if folder_uid: + record_add.folderUid = utils.base64_url_decode(folder_uid) + record_add.recordKeyEncryptedBy = folder_pb2.ENCRYPTED_BY_USER_KEY + + response = record_add_pam_configuration_v3(params, [record_add]) + if not response.records: + raise KeeperApiError('no_results', 'No results from PAM configuration creation') + + record_rs = response.records[0] + if record_rs.status != record_pb2.RS_SUCCESS: + raise KeeperApiError( + record_pb2.RecordModifyResult.Name(record_rs.status), + record_rs.message or 'Failed to create PAM configuration record') + record.revision = response.revision def pam_configuration_create(params, gateway_uid_bytes, config_json_str, child_config_json_strings=None, parent_uid_bytes=None): diff --git a/keepercommander/nested_share_folder/__init__.py b/keepercommander/nested_share_folder/__init__.py index 8436b6ba1..96df53527 100644 --- a/keepercommander/nested_share_folder/__init__.py +++ b/keepercommander/nested_share_folder/__init__.py @@ -37,7 +37,7 @@ 'get_folder_access_v3', ], 'record_api': [ - 'create_record_data_v3', 'record_add_v3', 'record_update_v3', + 'create_record_data_v3', 'record_add_v3', 'record_add_pam_configuration_v3', 'record_update_v3', 'create_record_v3', 'update_record_v3', 'create_records_batch_v3', 'get_record_details_v3', 'get_record_accesses_v3', 'find_direct_user_share_access', 'is_record_share_update_noop', diff --git a/keepercommander/nested_share_folder/record_api.py b/keepercommander/nested_share_folder/record_api.py index 619a2078f..d0b1eb44a 100644 --- a/keepercommander/nested_share_folder/record_api.py +++ b/keepercommander/nested_share_folder/record_api.py @@ -77,6 +77,19 @@ def record_add_v3(params, records, client_time=None, security_data_key_type=None rs_type=record_pb2.RecordsModifyResponse) +def record_add_pam_configuration_v3(params, records, client_time=None, security_data_key_type=None): + if not records or len(records) > 1000: + raise ValueError("Provide 1..1000 records") + rq = record_endpoints_pb2.RecordsAddRequest() + rq.records.extend(records) + if client_time: + rq.clientTime = client_time + if security_data_key_type: + rq.securityDataKeyType = security_data_key_type + return api.communicate_rest(params, rq, 'vault/records/v3/add_pam_configuration', + rs_type=record_pb2.RecordsModifyResponse) + + def record_update_v3(params, records, client_time=None, security_data_key_type=None): if not records or len(records) > 1000: raise ValueError("Provide 1..1000 records") diff --git a/unit-tests/pam/test_pam_nsf_config.py b/unit-tests/pam/test_pam_nsf_config.py index 5695abed5..2a40a8623 100644 --- a/unit-tests/pam/test_pam_nsf_config.py +++ b/unit-tests/pam/test_pam_nsf_config.py @@ -169,15 +169,15 @@ def test_parse_pam_configuration_accepts_nsf_folder_name(self): field = record.get_typed_field('pamResources') self.assertEqual(field.value[0]['folderUid'], 'nsf_folder') - @mock.patch('keepercommander.commands.discoveryrotation.create_record_in_folder') @mock.patch('keepercommander.commands.discoveryrotation.api.sync_down') @mock.patch('keepercommander.commands.discoveryrotation.TunnelDAG') @mock.patch('keepercommander.commands.discoveryrotation.get_keeper_tokens', return_value=(b'encrypted_session', b'encrypted_key', b'transmission_key')) + @mock.patch('keepercommander.commands.discoveryrotation.pam_configuration_create_record_v6') @mock.patch('keepercommander.commands.discoveryrotation.RecordEditMixin.get_record_type_fields', return_value=[]) def test_execute_creates_new_config_in_nsf_folder( - self, mock_record_fields, mock_tokens, mock_dag, mock_sync, mock_create_in_folder): + self, mock_record_fields, mock_create_config, mock_tokens, mock_dag, mock_sync): params = _make_params() params.nested_share_folders['nsf_folder'] = { 'name': 'Project - Users', @@ -193,19 +193,18 @@ def parse_properties(_, record, **kwargs): record.fields.append(vault.TypedField.new_field( 'pamResources', {'folderUid': 'nsf_folder'})) - def create_in_folder(_, record, folder_uid, command='pam'): + def create_config(_, record, folder_uid): record.record_uid = 'config_uid' - mock_create_in_folder.side_effect = create_in_folder + mock_create_config.side_effect = create_config with mock.patch.object(command, 'parse_properties', side_effect=parse_properties), \ mock.patch.object(command, 'verify_required'): result = command.execute(params, config_type='local', title='Config') self.assertEqual(result, 'config_uid') - mock_create_in_folder.assert_called_once_with( - params, mock.ANY, 'nsf_folder', command='pam-config-new') + mock_create_config.assert_called_once_with(params, mock.ANY, 'nsf_folder') + mock_sync.assert_called_once_with(params) - @mock.patch('keepercommander.commands.discoveryrotation.place_record_in_folder') @mock.patch('keepercommander.commands.discoveryrotation.api.sync_down') @mock.patch('keepercommander.commands.discoveryrotation.TunnelDAG') @mock.patch('keepercommander.commands.discoveryrotation.get_keeper_tokens', @@ -213,8 +212,8 @@ def create_in_folder(_, record, folder_uid, command='pam'): @mock.patch('keepercommander.commands.discoveryrotation.pam_configuration_create_record_v6') @mock.patch('keepercommander.commands.discoveryrotation.RecordEditMixin.get_record_type_fields', return_value=[]) - def test_execute_places_new_config_with_backend_aware_helper( - self, mock_record_fields, mock_create_config, mock_tokens, mock_dag, mock_sync, mock_place): + def test_execute_creates_new_config_in_legacy_folder( + self, mock_record_fields, mock_create_config, mock_tokens, mock_dag, mock_sync): params = _make_params() folder = SharedFolderNode() folder.uid = 'legacy_folder' @@ -234,9 +233,8 @@ def create_config(_, record, folder_uid): result = command.execute(params, config_type='local', title='Config') self.assertEqual(result, 'config_uid') - mock_create_config.assert_called_once() - mock_place.assert_called_once_with( - params, 'config_uid', 'legacy_folder', command='pam-config-new') + mock_create_config.assert_called_once_with(params, mock.ANY, 'legacy_folder') + mock_sync.assert_called_once_with(params) class TestPamConfigEditNsfPlacement(unittest.TestCase): diff --git a/unit-tests/pam/test_pam_rotation.py b/unit-tests/pam/test_pam_rotation.py index 16c03c1c7..951b94b85 100644 --- a/unit-tests/pam/test_pam_rotation.py +++ b/unit-tests/pam/test_pam_rotation.py @@ -191,6 +191,109 @@ def test_execute_with_valid_record(self, mock_TunnelDAG, mock_load): self.assertTrue(mock_TunnelDAG.called) self.assertEqual(mock_typed_record.record_key, b'\x00' * 16) + @patch('keepercommander.commands.discoveryrotation.router_set_record_rotation_information') + @patch('keepercommander.vault_extensions.find_records') + @patch('keepercommander.commands.discoveryrotation.resolve_pam_record') + @patch('keepercommander.vault.KeeperRecord.load') + @patch('keepercommander.commands.discoveryrotation.get_keeper_tokens') + @patch('keepercommander.commands.discoveryrotation.TunnelDAG') + @patch('keepercommander.rest_api.SERVER_PUBLIC_KEYS', {8: ec.generate_private_key(ec.SECP256R1()).public_key()}) + def test_execute_pam_user_with_config_no_resource(self, mock_TunnelDAG, mock_get_keeper_tokens, mock_load, + mock_resolve_pam_record, mock_find_records, + mock_router_set_rotation): + record_uid = 'OYNvVgpPPJBrVfYOIRtdag' + config_uid = 'bU2LVM6LjX_hmCoSMDA7vg' + resource_uid = 'dU2LVM6LjX_hmCoSMDA7vx' + + mock_params, mock_typed_record = create_mock_params_and_record() + mock_params.record_rotation_cache = {} + mock_typed_record.record_uid = record_uid + mock_load.return_value = mock_typed_record + + mock_pam_config_record = MagicMock(spec=vault.TypedRecord) + mock_pam_config_record.record_uid = config_uid + mock_pam_config_record.record_type = 'pamAwsConfiguration' + mock_pam_config_record.version = 6 + resource_field = MagicMock() + resource_field.value = [{'resourceRef': [resource_uid]}] + mock_pam_config_record.get_typed_field.return_value = resource_field + mock_find_records.return_value = [mock_pam_config_record] + mock_resolve_pam_record.side_effect = lambda params, identifier, rec_type=None: ( + mock_typed_record if identifier == record_uid else mock_pam_config_record + ) + + mock_user_dag = MagicMock() + mock_user_dag.linking_dag.has_graph = False + mock_config_dag = MagicMock() + mock_config_dag.linking_dag.has_graph = True + mock_config_dag.get_all_owners.return_value = [] + mock_config_dag.resource_belongs_to_config.return_value = False + mock_config_dag.user_belongs_to_resource.return_value = False + mock_config_dag.get_resource_uid.return_value = None + mock_config_dag.record.record_uid = config_uid + mock_TunnelDAG.side_effect = [mock_user_dag, mock_config_dag] + mock_get_keeper_tokens.return_value = (b'token', b'encrypted_key', b'transmission_key') + + kwargs = { + 'record_name': record_uid, + 'config': config_uid, + 'force': True, + } + + self.command.execute(mock_params, **kwargs) + mock_config_dag.edit_tunneling_config.assert_not_called() + mock_config_dag.link_resource_to_config.assert_called_once_with(resource_uid) + mock_config_dag.link_user_to_resource.assert_called_once_with(record_uid, resource_uid, belongs_to=True) + self.assertTrue(mock_router_set_rotation.called) + config_call = mock_TunnelDAG.call_args_list[1] + self.assertTrue(config_call.kwargs.get('is_config')) + + @patch('keepercommander.vault_extensions.find_records') + @patch('keepercommander.commands.discoveryrotation.resolve_pam_record') + @patch('keepercommander.vault.KeeperRecord.load') + @patch('keepercommander.commands.discoveryrotation.get_keeper_tokens') + @patch('keepercommander.commands.discoveryrotation.TunnelDAG') + @patch('keepercommander.rest_api.SERVER_PUBLIC_KEYS', {8: ec.generate_private_key(ec.SECP256R1()).public_key()}) + def test_execute_pam_user_with_config_empty_resource_ref(self, mock_TunnelDAG, mock_get_keeper_tokens, mock_load, + mock_resolve_pam_record, mock_find_records): + record_uid = 'OYNvVgpPPJBrVfYOIRtdag' + config_uid = 'bU2LVM6LjX_hmCoSMDA7vg' + + mock_params, mock_typed_record = create_mock_params_and_record() + mock_params.record_rotation_cache = {} + mock_typed_record.record_uid = record_uid + mock_load.return_value = mock_typed_record + + mock_pam_config_record = MagicMock(spec=vault.TypedRecord) + mock_pam_config_record.record_uid = config_uid + mock_pam_config_record.record_type = 'pamAwsConfiguration' + mock_pam_config_record.version = 6 + resource_field = MagicMock() + resource_field.value = [{'resourceRef': ['']}] + mock_pam_config_record.get_typed_field.return_value = resource_field + mock_find_records.return_value = [mock_pam_config_record] + mock_resolve_pam_record.side_effect = lambda params, identifier, rec_type=None: ( + mock_typed_record if identifier == record_uid else mock_pam_config_record + ) + + mock_user_dag = MagicMock() + mock_user_dag.linking_dag.has_graph = False + mock_config_dag = MagicMock() + mock_config_dag.linking_dag.has_graph = True + mock_config_dag.get_all_owners.return_value = [] + mock_config_dag.record.record_uid = config_uid + mock_TunnelDAG.side_effect = [mock_user_dag, mock_config_dag] + mock_get_keeper_tokens.return_value = (b'token', b'encrypted_key', b'transmission_key') + + kwargs = { + 'record_name': record_uid, + 'config': config_uid, + 'force': True, + } + + with self.assertRaises(CommandError): + self.command.execute(mock_params, **kwargs) + class TestPAMResourceRotateCommand(unittest.TestCase): @@ -484,9 +587,9 @@ def test_parser_online(self): @patch('keepercommander.commands.discoveryrotation.router_get_connected_gateways') @patch('keepercommander.commands.discoveryrotation.router_helper.get_router_url') @patch('keepercommander.commands.discoveryrotation.gateway_helper.get_all_gateways') - @patch('keepercommander.commands.discoveryrotation.KSMCommand.get_app_record') + @patch('keepercommander.commands.discoveryrotation.KSMCommand.get_ksm_app_display_info') @patch('keepercommander.commands.discoveryrotation.dump_report_data') - def test_execute(self, mock_dump_report_data, mock_get_app_record, mock_get_all_gateways, + def test_execute(self, mock_dump_report_data, mock_get_ksm_app_display_info, mock_get_all_gateways, mock_get_router_url, mock_router_get_connected_gateways): mock_params = create_mock_params() @@ -508,9 +611,7 @@ def test_execute(self, mock_dump_report_data, mock_get_app_record, mock_get_all_ ) ] - mock_get_app_record.return_value = { - 'data_unencrypted': json.dumps({'title': 'App Title'}) - } + mock_get_ksm_app_display_info.return_value = ('App Title', True, 'App Title (app_uid)') kwargs = {'is_force': True, 'is_verbose': True} self.command.execute(mock_params, **kwargs) @@ -519,7 +620,7 @@ def test_execute(self, mock_dump_report_data, mock_get_app_record, mock_get_all_ self.assertTrue(mock_router_get_connected_gateways.called) self.assertTrue(mock_get_all_gateways.called) self.assertTrue(mock_get_router_url.called) - self.assertTrue(mock_get_app_record.called) + self.assertTrue(mock_get_ksm_app_display_info.called) @patch('keepercommander.commands.discoveryrotation.router_get_connected_gateways') @patch('keepercommander.commands.discoveryrotation.router_helper.get_router_url') diff --git a/unit-tests/test_command_ksm.py b/unit-tests/test_command_ksm.py index 0cf618225..031868a0d 100644 --- a/unit-tests/test_command_ksm.py +++ b/unit-tests/test_command_ksm.py @@ -5,6 +5,136 @@ from keepercommander.commands.ksm import KSMCommand +class TestKSMSecretResolution(unittest.TestCase): + + def _make_params(self): + params = MagicMock() + params.record_cache = {} + params.shared_folder_cache = {} + params.nested_share_records = {} + params.nested_share_folders = {} + params.nested_share_record_data = {} + params.folder_cache = {} + return params + + def test_resolve_secret_uid_from_nsf_record_cache(self): + params = self._make_params() + record_uid = 'OYNvVgpPPJBrVfYOIRtdag' + params.nested_share_records = {record_uid: {'record_key_unencrypted': b'key'}} + self.assertEqual(KSMCommand.resolve_secret_uid(params, record_uid), record_uid) + + def test_resolve_secret_uid_from_nsf_folder_cache(self): + params = self._make_params() + folder_uid = 'bU2LVM6LjX_hmCoSMDA7vg' + params.nested_share_folders = {folder_uid: {'name': 'Project Folder'}} + with patch('keepercommander.commands.ksm.is_nested_share_folder', return_value=True): + self.assertEqual(KSMCommand.resolve_secret_uid(params, folder_uid), folder_uid) + + @patch('keepercommander.commands.ksm.resolve_nested_share_record_uid') + def test_resolve_secret_uid_by_record_title(self, mock_resolve_record): + params = self._make_params() + mock_resolve_record.return_value = 'resolved_record_uid' + self.assertEqual(KSMCommand.resolve_secret_uid(params, 'My Record'), 'resolved_record_uid') + + @patch('keepercommander.commands.ksm.resolve_folder_uid') + @patch('keepercommander.commands.ksm.resolve_nested_share_record_uid', return_value=None) + def test_resolve_secret_uid_by_folder_path(self, _mock_resolve_record, mock_resolve_folder): + params = self._make_params() + mock_resolve_folder.return_value = 'resolved_folder_uid' + with patch('keepercommander.commands.ksm.is_nested_share_folder', return_value=True): + with patch('keepercommander.commands.ksm.api.is_shared_folder', return_value=False): + self.assertEqual(KSMCommand.resolve_secret_uid(params, 'NSF/Folder'), 'resolved_folder_uid') + + def test_classify_secret_nsf_record(self): + params = self._make_params() + record_uid = 'OYNvVgpPPJBrVfYOIRtdag' + params.nested_share_records = {record_uid: {'record_key_unencrypted': b'record_key'}} + with patch('keepercommander.commands.ksm.is_nested_share_record', return_value=True): + secret = KSMCommand.classify_secret(params, record_uid) + self.assertIsNotNone(secret) + self.assertEqual(secret['share_type'], 'SHARE_TYPE_RECORD') + self.assertEqual(secret['share_key'], b'record_key') + + def test_classify_secret_nsf_folder(self): + params = self._make_params() + folder_uid = 'bU2LVM6LjX_hmCoSMDA7vg' + with patch('keepercommander.commands.ksm.is_nested_share_folder', return_value=True): + with patch('keepercommander.commands.ksm.api.is_shared_folder', return_value=False): + with patch('keepercommander.commands.ksm.get_folder_key', return_value=b'folder_key'): + secret = KSMCommand.classify_secret(params, folder_uid) + self.assertIsNotNone(secret) + self.assertEqual(secret['share_type'], 'SHARE_TYPE_FOLDER') + self.assertEqual(secret['share_key'], b'folder_key') + + @patch('keepercommander.commands.ksm.KSMCommand.update_secrets_user_permissions') + @patch('keepercommander.commands.ksm.api.sync_down') + @patch('keepercommander.commands.ksm.api.communicate_rest') + @patch('keepercommander.commands.ksm.KSMCommand.get_app_record') + def test_share_secret_adds_nsf_record(self, mock_get_app_record, mock_communicate_rest, + _mock_sync_down, _mock_update_perms): + params = self._make_params() + record_uid = 'OYNvVgpPPJBrVfYOIRtdag' + params.nested_share_records = {record_uid: {'record_key_unencrypted': b'record_key'}} + mock_get_app_record.return_value = { + 'record_uid': 'app_uid___________', + 'record_key_unencrypted': b'app_key' * 2, + } + with patch('keepercommander.commands.ksm.is_nested_share_record', return_value=True): + KSMCommand.add_app_share(params, [record_uid], 'MyApp', False) + mock_communicate_rest.assert_called_once() + self.assertEqual(mock_communicate_rest.call_args[0][2], 'vault/app_share_add') + + +class TestKSMAppRecordResolution(unittest.TestCase): + + def _make_params(self): + params = MagicMock() + params.record_cache = {} + params.nested_share_records = {} + params.nested_share_record_data = {} + return params + + def test_get_app_record_from_nsf_cache(self): + params = self._make_params() + app_uid = 'appUid00000000000001' + params.nested_share_records = { + app_uid: {'version': 5, 'record_key_unencrypted': b'app_key', 'revision': 1} + } + params.nested_share_record_data = { + app_uid: {'data_json': {'title': 'NSF App', 'type': 'app'}} + } + with patch('keepercommander.commands.ksm.is_nested_share_record', return_value=True): + rec = KSMCommand.get_app_record(params, app_uid) + self.assertIsNotNone(rec) + self.assertEqual(rec['record_uid'], app_uid) + self.assertEqual(rec['record_key_unencrypted'], b'app_key') + + @patch('keepercommander.commands.ksm.resolve_nested_share_record_uid') + def test_get_app_record_by_nsf_path(self, mock_resolve): + params = self._make_params() + app_uid = 'appUid00000000000001' + mock_resolve.return_value = app_uid + params.nested_share_records = { + app_uid: {'version': 5, 'record_key_unencrypted': b'app_key', 'revision': 1} + } + params.nested_share_record_data = { + app_uid: {'data_json': {'title': 'NSF App', 'type': 'app'}} + } + rec = KSMCommand.get_app_record(params, 'NSF/NSF App') + self.assertIsNotNone(rec) + self.assertEqual(rec['record_uid'], app_uid) + + def test_get_ksm_app_display_info_from_nsf_metadata(self): + params = self._make_params() + app_uid = 'appUid00000000000001' + with patch('keepercommander.commands.ksm.KSMCommand.get_app_record', return_value=None): + with patch('keepercommander.commands.ksm.KSMCommand.get_app_title', return_value='NSF App'): + title, accessible, info = KSMCommand.get_ksm_app_display_info(params, app_uid) + self.assertEqual(title, 'NSF App') + self.assertFalse(accessible) + self.assertIn('NSF App', info) + + class TestKSMTokenAdd(unittest.TestCase): """secrets-manager token add → calls add_client.""" From 7f24eeb272e95d403ac229b2aee501535dbab0d3 Mon Sep 17 00:00:00 2001 From: adeshmukh-ks Date: Mon, 6 Jul 2026 16:55:15 +0530 Subject: [PATCH 11/14] unit test fix --- unit-tests/pam/test_pam_rotation.py | 79 ++++++++++++++++------------- unit-tests/test_command_ksm.py | 5 +- 2 files changed, 48 insertions(+), 36 deletions(-) diff --git a/unit-tests/pam/test_pam_rotation.py b/unit-tests/pam/test_pam_rotation.py index 951b94b85..60adc8a29 100644 --- a/unit-tests/pam/test_pam_rotation.py +++ b/unit-tests/pam/test_pam_rotation.py @@ -6,19 +6,26 @@ from keepercommander.error import CommandError import keepercommander.vault as vault +TEST_RECORD_UID = 'OYNvVgpPPJBrVfYOIRtdag' +TEST_CONFIG_UID = 'bU2LVM6LjX_hmCoSMDA7vg' +TEST_RESOURCE_UID = 'dU2LVM6LjX_hmCoSMDA7vx' +TEST_FOLDER_UID = 'M_SR5x7Q4cu9O0-RiLDN4A' +TEST_ADMIN_UID = 'aU2LVM6LjX_hmCoSMDA7va' + + def create_mock_params_and_record(record_type='pamUser'): mock_params = MagicMock() mock_params.rest_context.server_key_id = 8 mock_params.session_token = 'base64_encoded_session_token' # Mock a base64 encoded session token - mock_params.record_cache = {'record_uid': MagicMock(record_type='pamUser')} - mock_params.subfolder_record_cache = {'folder_uid': ['record_uid']} - mock_params.folder_cache = {'folder_uid': MagicMock()} + mock_params.record_cache = {TEST_RECORD_UID: MagicMock(record_type='pamUser')} + mock_params.subfolder_record_cache = {TEST_FOLDER_UID: [TEST_RECORD_UID]} + mock_params.folder_cache = {TEST_FOLDER_UID: MagicMock()} mock_params.record_rotation_cache = { - 'record_uid': { + TEST_RECORD_UID: { 'pwd_complexity': 'eyJ0eXBlIjogInBhc3N3b3JkX2NvbXBsZXhpdHkiLCAidmFsdWUiOiAiY29tcGxleGl0eV92YWx1ZSJ9', - 'configuration_uid': 'config_uid', + 'configuration_uid': TEST_CONFIG_UID, 'schedule': '[]', - 'resourceUid': 'resource_uid', + 'resourceUid': TEST_RESOURCE_UID, 'revision': 1 # Ensure revision is set } } @@ -26,7 +33,7 @@ def create_mock_params_and_record(record_type='pamUser'): mock_typed_record = MagicMock(spec=vault.TypedRecord) mock_typed_record.record_type = record_type - mock_typed_record.record_uid = 'record_uid' + mock_typed_record.record_uid = TEST_RECORD_UID mock_typed_record.title = 'Mock Title' # Add the title attribute mock_typed_record.record_key = b'\x00' * 16 # Add the record_key attribute @@ -38,12 +45,12 @@ def create_mock_params(): mock_params.rest_context.server_key_id = 8 mock_params.session_token = 'base64_encoded_session_token' mock_params.record_cache = { - 'record_uid': { + TEST_RECORD_UID: { 'data_unencrypted': json.dumps({'title': 'Mock Title', 'type': 'pamMachine'}) } } - mock_params.subfolder_record_cache = {'folder_uid': ['record_uid']} - mock_params.folder_cache = {'folder_uid': MagicMock()} + mock_params.subfolder_record_cache = {TEST_FOLDER_UID: [TEST_RECORD_UID]} + mock_params.folder_cache = {TEST_FOLDER_UID: MagicMock()} mock_params.rest_context.server_base = 'https://fake.keepersecurity.com' return mock_params @@ -95,13 +102,13 @@ def test_execute_with_folder(self, mock_TunnelDAG, mock_load): mock_dag_instance = mock_TunnelDAG.return_value mock_dag_instance.linking_dag.has_graph = True mock_dag_instance.check_if_resource_has_admin.return_value = True - mock_dag_instance.get_all_owners.return_value = ['resource_uid'] + mock_dag_instance.get_all_owners.return_value = [TEST_RESOURCE_UID] mock_dag_instance.resource_belongs_to_config.return_value = True mock_dag_instance.user_belongs_to_resource.return_value = True - mock_dag_instance.record.record_uid = 'config_uid' # Ensure it returns a string + mock_dag_instance.record.record_uid = TEST_CONFIG_UID # Ensure it returns a string kwargs = { - 'folder_name': 'folder_uid', + 'folder_name': TEST_FOLDER_UID, 'force': True # Add force to the kwargs } @@ -131,7 +138,7 @@ def test_execute_with_invalid_password_complexity(self, mock_TunnelDAG, mock_loa mock_params, _ = create_mock_params_and_record() kwargs = { - 'record_name': 'record_uid', + 'record_name': TEST_RECORD_UID, 'pwd_complexity': 'invalid_complexity', 'force': True # Add force to the kwargs } @@ -150,13 +157,13 @@ def test_execute_with_valid_password_complexity(self, mock_TunnelDAG, mock_load) mock_dag_instance = mock_TunnelDAG.return_value mock_dag_instance.linking_dag.has_graph = True mock_dag_instance.check_if_resource_has_admin.return_value = True - mock_dag_instance.get_all_owners.return_value = ['resource_uid'] + mock_dag_instance.get_all_owners.return_value = [TEST_RESOURCE_UID] mock_dag_instance.resource_belongs_to_config.return_value = True mock_dag_instance.user_belongs_to_resource.return_value = True - mock_dag_instance.record.record_uid = 'config_uid' # Ensure it returns a string + mock_dag_instance.record.record_uid = TEST_CONFIG_UID # Ensure it returns a string kwargs = { - 'record_name': 'record_uid', + 'record_name': TEST_RECORD_UID, 'pwd_complexity': '32,5,5,5,5', 'force': True # Add force to the kwargs } @@ -176,13 +183,13 @@ def test_execute_with_valid_record(self, mock_TunnelDAG, mock_load): mock_dag_instance = mock_TunnelDAG.return_value mock_dag_instance.linking_dag.has_graph = True mock_dag_instance.check_if_resource_has_admin.return_value = True - mock_dag_instance.get_all_owners.return_value = ['resource_uid'] + mock_dag_instance.get_all_owners.return_value = [TEST_RESOURCE_UID] mock_dag_instance.resource_belongs_to_config.return_value = True mock_dag_instance.user_belongs_to_resource.return_value = True - mock_dag_instance.record.record_uid = 'config_uid' # Ensure it returns a string + mock_dag_instance.record.record_uid = TEST_CONFIG_UID # Ensure it returns a string kwargs = { - 'record_name': 'record_uid', + 'record_name': TEST_RECORD_UID, 'force': True # Add force to the kwargs } @@ -219,7 +226,9 @@ def test_execute_pam_user_with_config_no_resource(self, mock_TunnelDAG, mock_get mock_pam_config_record.get_typed_field.return_value = resource_field mock_find_records.return_value = [mock_pam_config_record] mock_resolve_pam_record.side_effect = lambda params, identifier, rec_type=None: ( - mock_typed_record if identifier == record_uid else mock_pam_config_record + mock_typed_record if identifier == record_uid + else mock_pam_config_record if identifier == config_uid + else None ) mock_user_dag = MagicMock() @@ -273,7 +282,9 @@ def test_execute_pam_user_with_config_empty_resource_ref(self, mock_TunnelDAG, m mock_pam_config_record.get_typed_field.return_value = resource_field mock_find_records.return_value = [mock_pam_config_record] mock_resolve_pam_record.side_effect = lambda params, identifier, rec_type=None: ( - mock_typed_record if identifier == record_uid else mock_pam_config_record + mock_typed_record if identifier == record_uid + else mock_pam_config_record if identifier == config_uid + else None ) mock_user_dag = MagicMock() @@ -321,14 +332,14 @@ def test_execute_with_enable(self, mock_tunneldag, mock_get_keeper_tokens, mock_ mock_load.return_value = mock_typed_record mock_pam_config_record = MagicMock(spec=vault.TypedRecord) - mock_pam_config_record.record_uid = 'config_uid' + mock_pam_config_record.record_uid = TEST_CONFIG_UID mock_pam_config_record.record_type = 'pamConfiguration' # Use a valid PAM configuration record type mock_find_records.return_value = [mock_pam_config_record] kwargs = { - 'record_name': 'record_uid', + 'record_name': TEST_RECORD_UID, 'enable': True, - 'config_uid': 'config_uid' + 'config_uid': TEST_CONFIG_UID } self.command.execute(mock_params, **kwargs) @@ -354,7 +365,7 @@ def test_execute_with_invalid_record_type(self, mock_load): mock_load.return_value = mock_typed_record kwargs = { - 'record_name': 'record_uid', + 'record_name': TEST_RECORD_UID, 'enable': True } @@ -376,14 +387,14 @@ def test_execute_with_disable(self, mock_tunneldag, mock_get_keeper_tokens, mock mock_load.return_value = mock_typed_record mock_pam_config_record = MagicMock(spec=vault.TypedRecord) - mock_pam_config_record.record_uid = 'config_uid' + mock_pam_config_record.record_uid = TEST_CONFIG_UID mock_pam_config_record.record_type = 'pamConfiguration' # Use a valid PAM configuration record type mock_find_records.return_value = [mock_pam_config_record] kwargs = { - 'record_name': 'record_uid', + 'record_name': TEST_RECORD_UID, 'disable': True, - 'config_uid': 'config_uid' + 'config_uid': TEST_CONFIG_UID } self.command.execute(mock_params, **kwargs) @@ -406,22 +417,22 @@ def test_execute_with_enable_and_admin(self, mock_tunneldag, mock_get_keeper_tok mock_load.return_value = mock_typed_record mock_pam_config_record = MagicMock(spec=vault.TypedRecord) - mock_pam_config_record.record_uid = 'config_uid' + mock_pam_config_record.record_uid = TEST_CONFIG_UID mock_pam_config_record.record_type = 'pamConfiguration' # Use a valid PAM configuration record type mock_find_records.return_value = [mock_pam_config_record] kwargs = { - 'record_name': 'record_uid', + 'record_name': TEST_RECORD_UID, 'enable': True, - 'config_uid': 'config_uid', - 'admin': 'admin_uid' + 'config_uid': TEST_CONFIG_UID, + 'admin': TEST_ADMIN_UID } self.command.execute(mock_params, **kwargs) self.assertTrue(mock_load.called) self.assertTrue(mock_tunneldag.called) self.assertTrue(mock_get_keeper_tokens.called) - mock_dag_instance.link_user_to_resource.assert_called_with('admin_uid', 'record_uid', is_admin=True) + mock_dag_instance.link_user_to_resource.assert_called_with(TEST_ADMIN_UID, TEST_RECORD_UID, is_admin=True) class TestPAMListRecordRotationCommand(unittest.TestCase): diff --git a/unit-tests/test_command_ksm.py b/unit-tests/test_command_ksm.py index 031868a0d..d6dbb9ef7 100644 --- a/unit-tests/test_command_ksm.py +++ b/unit-tests/test_command_ksm.py @@ -41,7 +41,8 @@ def test_resolve_secret_uid_by_record_title(self, mock_resolve_record): def test_resolve_secret_uid_by_folder_path(self, _mock_resolve_record, mock_resolve_folder): params = self._make_params() mock_resolve_folder.return_value = 'resolved_folder_uid' - with patch('keepercommander.commands.ksm.is_nested_share_folder', return_value=True): + with patch('keepercommander.commands.ksm.is_nested_share_folder', + side_effect=lambda _params, uid: uid == 'resolved_folder_uid'): with patch('keepercommander.commands.ksm.api.is_shared_folder', return_value=False): self.assertEqual(KSMCommand.resolve_secret_uid(params, 'NSF/Folder'), 'resolved_folder_uid') @@ -77,7 +78,7 @@ def test_share_secret_adds_nsf_record(self, mock_get_app_record, mock_communicat params.nested_share_records = {record_uid: {'record_key_unencrypted': b'record_key'}} mock_get_app_record.return_value = { 'record_uid': 'app_uid___________', - 'record_key_unencrypted': b'app_key' * 2, + 'record_key_unencrypted': b'a' * 32, } with patch('keepercommander.commands.ksm.is_nested_share_record', return_value=True): KSMCommand.add_app_share(params, [record_uid], 'MyApp', False) From 781fe4ad8341f0a0b411fa296fccdbb0c9858f57 Mon Sep 17 00:00:00 2001 From: adeshmukh-ks Date: Mon, 13 Jul 2026 15:18:11 +0530 Subject: [PATCH 12/14] review comments addressed --- .../commands/credential_provision.py | 6 ++- keepercommander/commands/discoveryrotation.py | 44 +++++++++++++++---- keepercommander/commands/pam/vault_target.py | 27 ++++++++++-- keepercommander/commands/pam_import/edit.py | 16 +++---- .../commands/pam_import/nsf_helpers.py | 6 +++ 5 files changed, 75 insertions(+), 24 deletions(-) diff --git a/keepercommander/commands/credential_provision.py b/keepercommander/commands/credential_provision.py index 59682e90c..5029b2b6d 100644 --- a/keepercommander/commands/credential_provision.py +++ b/keepercommander/commands/credential_provision.py @@ -1316,7 +1316,11 @@ def _check_duplicate_by_username_in_folder(self, config: Dict[str, Any], params: try: folder_uid = self._resolve_pam_user_folder_uid(config, params) - except Exception: + except Exception as e: + logging.warning( + f'Could not resolve target folder for duplicate username check ' + f'(username={username}, PAM config={pam_config_uid}): {e}' + ) return False if not folder_uid: diff --git a/keepercommander/commands/discoveryrotation.py b/keepercommander/commands/discoveryrotation.py index 4c976f8b2..2f3ae988e 100644 --- a/keepercommander/commands/discoveryrotation.py +++ b/keepercommander/commands/discoveryrotation.py @@ -1710,6 +1710,10 @@ def execute(self, params, **kwargs): row_color = bcolors.WHITE record_title = '[record inaccessible]' record_type = '[record inaccessible]' + logging.warning( + 'Rotation list: PAM User record %s is not accessible in the local vault cache', + record_uid, + ) if record_type != "pamUser": # only pamUser records are supported for rotation @@ -1778,8 +1782,18 @@ def execute(self, params, **kwargs): pam_data_decrypted = pam_decrypt_configuration_data(pam_configuration) pam_config_name = pam_data_decrypted.get('title') or pam_config_name pam_config_type = pam_data_decrypted.get('type') or '[unknown]' - except Exception: - pass + except Exception as exc: + logging.warning( + 'Rotation list: PAM configuration %s is not accessible in the vault ' + 'and could not be decrypted from router data: %s', + configuration_uid_str, + exc, + ) + if pam_config_name == '[record inaccessible]': + logging.warning( + 'Rotation list: PAM configuration %s title/type remain inaccessible', + configuration_uid_str, + ) row.append(f"{pam_config_name or '[untitled]'} ({pam_config_type or '[unknown]'})") if is_verbose: @@ -3068,15 +3082,27 @@ def _resolve_pam_config_uid(cls, params, identifier): # type: (KeeperParams, str) -> Optional[str] if identifier in params.record_cache: rec = vault.KeeperRecord.load(params, identifier) - if isinstance(rec, vault.TypedRecord) and rec.version == 6 and rec.record_type in cls._PAM_CONFIG_TYPES: - return rec.record_uid + if isinstance(rec, vault.TypedRecord) and rec.version == 6: + if rec.record_type in cls._PAM_CONFIG_TYPES: + return rec.record_uid + logging.warning( + 'Record "%s" is v6 but is not a PAM configuration type (found %s)', + identifier, + rec.record_type, + ) from ..nested_share_folder import removal_api as _nsf resolved_uid = _nsf.resolve_nested_share_record_uid(params, identifier) if resolved_uid: rec = vault.KeeperRecord.load(params, resolved_uid) - if isinstance(rec, vault.TypedRecord) and rec.version == 6 and rec.record_type in cls._PAM_CONFIG_TYPES: - return resolved_uid + if isinstance(rec, vault.TypedRecord) and rec.version == 6: + if rec.record_type in cls._PAM_CONFIG_TYPES: + return resolved_uid + logging.warning( + 'Nested Share Record "%s" is v6 but is not a PAM configuration type (found %s)', + resolved_uid, + rec.record_type, + ) l_name = identifier.casefold() matches = [] @@ -3368,7 +3394,7 @@ def execute(self, params, **kwargs): record_refs = kwargs.get('add_credential') if isinstance(record_refs, list): for ref in record_refs: - resolved_ref = resolve_pam_record(params, ref) + resolved_ref = resolve_pam_record(params, ref, rec_type='pamUser') if resolved_ref: script_value['recordRef'].append(resolved_ref.record_uid) elif record_exists_in_vault(params, ref): @@ -3443,7 +3469,7 @@ def execute(self, params, **kwargs): remove_credential = kwargs.get('remove_credential') if isinstance(remove_credential, list) and remove_credential: for ref in remove_credential: - resolved_ref = resolve_pam_record(params, ref) + resolved_ref = resolve_pam_record(params, ref, rec_type='pamUser') if resolved_ref: refs.discard(resolved_ref.record_uid) else: @@ -3452,7 +3478,7 @@ def execute(self, params, **kwargs): add_credential = kwargs.get('add_credential') if isinstance(add_credential, list) and add_credential: for ref in add_credential: - resolved_ref = resolve_pam_record(params, ref) + resolved_ref = resolve_pam_record(params, ref, rec_type='pamUser') if resolved_ref: refs.add(resolved_ref.record_uid) elif record_exists_in_vault(params, ref): diff --git a/keepercommander/commands/pam/vault_target.py b/keepercommander/commands/pam/vault_target.py index faa21fe84..4e25c539c 100644 --- a/keepercommander/commands/pam/vault_target.py +++ b/keepercommander/commands/pam/vault_target.py @@ -145,10 +145,10 @@ def find_nsf_users_subfolder(params, parent_uid): def records_in_folder(params, folder_uid): - records = set(getattr(params, 'subfolder_record_cache', {}).get(folder_uid, set()) or set()) - nsf_records = getattr(params, 'nested_share_folder_records', {}) + records = set((getattr(params, 'subfolder_record_cache', None) or {}).get(folder_uid, set()) or set()) + nsf_records = getattr(params, 'nested_share_folder_records', None) or {} if folder_uid in nsf_records: - records.update(nsf_records[folder_uid]) + records.update(nsf_records[folder_uid] or set()) return records @@ -470,7 +470,14 @@ def place_record_in_folder(params, record_uid, folder_uid, command='pam'): if is_nested_share_folder(params, folder_uid): result = move_record_v3(params, record_uid, to_folder_uid=folder_uid) if isinstance(result, dict) and result.get('success') is False: - raise CommandError(command, result.get('message') or 'Failed to place record in Nested Share Folder') + message = result.get('message') or 'Failed to place record in Nested Share Folder' + lowered = message.casefold() + if any(token in lowered for token in ('denied', 'permission', 'forbidden', 'read-only')): + message = ( + f'{message}. Verify you have record-management permission ' + f'on the Nested Share Folder.' + ) + raise CommandError(command, message) api.sync_down(params) else: FolderMoveCommand().execute(params, src=record_uid, dst=folder_uid, force=True) @@ -518,6 +525,12 @@ def update_pam_record(params, record, command='pam'): def execute_record_add_in_folder(params, args, folder_uid, command='pam'): + """Add a record, placing it in an NSF folder when needed. + + For Nested Share Folders, ``folder`` is removed from *args* before calling + RecordAddCommand because the record is created at the vault root first and + then moved into the NSF via ``place_record_in_folder``. + """ from ..record_edit import RecordAddCommand record_args = dict(args or {}) @@ -533,6 +546,12 @@ def execute_record_add_in_folder(params, args, folder_uid, command='pam'): def execute_record_v3_add_in_folder(params, args, folder_uid, command='pam'): + """Add a v3 typed record, placing it in an NSF folder when needed. + + For Nested Share Folders, ``folder`` is removed from *args* before calling + RecordAddCommand because the record is created without a legacy folder target + and then placed into the NSF via ``place_record_in_folder``. + """ from ..recordv3 import RecordAddCommand record_args = dict(args or {}) diff --git a/keepercommander/commands/pam_import/edit.py b/keepercommander/commands/pam_import/edit.py index 9b685879d..5ec244d0c 100644 --- a/keepercommander/commands/pam_import/edit.py +++ b/keepercommander/commands/pam_import/edit.py @@ -16,7 +16,6 @@ import os.path from itertools import chain -from types import SimpleNamespace from typing import Any, Dict, Optional, List, Union from .keeper_ai_settings import set_resource_jit_settings, set_resource_keeper_ai_settings, refresh_meta_to_latest, refresh_link_to_config_to_latest @@ -67,7 +66,7 @@ from ...params import LAST_FOLDER_UID, LAST_SHARED_FOLDER_UID from ...proto import record_pb2, APIRequest_pb2, enterprise_pb2 from ...recordv3 import RecordV3 -from ...subfolder import BaseFolderNode +from ...subfolder import BaseFolderNode, NestedShareFolderNode class PAMProjectImportCommand(Command): @@ -1356,14 +1355,11 @@ def find_folders(self, params, parent_uid:str, folder:str, is_shared_folder:bool if not is_shared_folder: for uid, nsf in getattr(params, 'nested_share_folders', {}).items(): if nsf.get('parent_uid') == puid and nsf.get('name') == folder: - result.append(SimpleNamespace( - uid=uid, - name=nsf.get('name'), - parent_uid=nsf.get('parent_uid'), - type=BaseFolderNode.UserFolderType, - UserFolderType=BaseFolderNode.UserFolderType, - SharedFolderType=BaseFolderNode.SharedFolderType, - )) + nsf_folder = NestedShareFolderNode() + nsf_folder.uid = uid + nsf_folder.name = nsf.get('name') + nsf_folder.parent_uid = nsf.get('parent_uid') + result.append(nsf_folder) return result def create_ksm_app(self, params, app_name) -> str: diff --git a/keepercommander/commands/pam_import/nsf_helpers.py b/keepercommander/commands/pam_import/nsf_helpers.py index cadc64403..7458f8384 100644 --- a/keepercommander/commands/pam_import/nsf_helpers.py +++ b/keepercommander/commands/pam_import/nsf_helpers.py @@ -193,6 +193,12 @@ def create_nsf_subfolder(params, folder_name: str, parent_uid: str = '', folder_uid = api.generate_record_uid() parent = parent_uid or None + if parent: + nsf_folders = getattr(params, 'nested_share_folders', None) or {} + folder_cache = getattr(params, 'folder_cache', None) or {} + if parent not in nsf_folders and parent not in folder_cache: + raise CommandError('pam project extend', f'Parent folder "{parent_uid}" not found') + with command_error_handler('pam project extend'): fd, folder_key = _prepare_folder_for_creation( params, folder_uid, name, parent, None, True, From 1edfe98906ff90f660b19ca53a580db1af85a9d6 Mon Sep 17 00:00:00 2001 From: sshrushanth-ks Date: Mon, 13 Jul 2026 17:08:22 +0530 Subject: [PATCH 13/14] Fix NSF record create/update for pam project extend Create PAM records natively in Nested Share Folders, sync after add, resolve NSF records in tunnel edit, and normalize legacy error wording. --- keepercommander/commands/base.py | 5 ++ keepercommander/commands/folder.py | 10 +-- .../commands/nested_share_folder/helpers.py | 12 +++- .../nested_share_folder/record_commands.py | 7 +- keepercommander/commands/pam/vault_target.py | 65 +++++++++++-------- .../commands/pam_import/nsf_helpers.py | 53 +-------------- .../commands/tunnel_and_connections.py | 10 ++- .../nested_share_folder/folder_record_api.py | 3 +- .../nested_share_folder/record_api.py | 13 ++-- unit-tests/pam/test_pam_project_import_nsf.py | 32 ++++----- 10 files changed, 97 insertions(+), 113 deletions(-) diff --git a/keepercommander/commands/base.py b/keepercommander/commands/base.py index cba03f2dc..572e0f8c0 100644 --- a/keepercommander/commands/base.py +++ b/keepercommander/commands/base.py @@ -915,6 +915,11 @@ def resolve_single_record(params, record_name): # type: (KeeperParams, str) -> if record_name in params.record_cache: return vault.KeeperRecord.load(params, record_name) + from .pam_import.record_loader import load_pam_record + rec = load_pam_record(params, record_name) + if rec: + return rec + rs = try_resolve_path(params, record_name) if rs is None: return None diff --git a/keepercommander/commands/folder.py b/keepercommander/commands/folder.py index f473eb754..84a1ef7d5 100644 --- a/keepercommander/commands/folder.py +++ b/keepercommander/commands/folder.py @@ -899,13 +899,13 @@ def execute(self, params, **kwargs): if src_folder.type == BaseFolderNode.NestedShareFolderType: if dst_folder.type in {BaseFolderNode.SharedFolderType, BaseFolderNode.SharedFolderFolderType}: - raise CommandError('mv', 'Drive folders cannot be moved inside a Shared folder.') - raise CommandError('mv', 'Moving drive folders is currently not supported.') + raise CommandError('mv', 'Nested Share Folders cannot be moved inside a Shared folder.') + raise CommandError('mv', 'Moving Nested Share Folders is currently not supported.') if dst_folder.type == BaseFolderNode.NestedShareFolderType: if src_folder.type in {BaseFolderNode.SharedFolderType, BaseFolderNode.SharedFolderFolderType}: - raise CommandError('mv', 'Shared folders cannot be moved inside a drive folder.') - raise CommandError('mv', 'Folders cannot be moved inside a drive folder.') + raise CommandError('mv', 'Shared folders cannot be moved inside a Nested Share Folder.') + raise CommandError('mv', 'Folders cannot be moved inside a Nested Share Folder.') dp = set() f = dst_folder @@ -949,7 +949,7 @@ def execute(self, params, **kwargs): else: if src_folder.type == BaseFolderNode.NestedShareFolderType: - raise CommandError('mv', 'Moving drive records is currently not supported.') + raise CommandError('mv', 'Moving Nested Share Folder records is currently not supported.') if record_uid in getattr(params, 'nested_share_records', {}) \ and dst_folder.type != BaseFolderNode.NestedShareFolderType: diff --git a/keepercommander/commands/nested_share_folder/helpers.py b/keepercommander/commands/nested_share_folder/helpers.py index 5c990d04a..1f213d803 100644 --- a/keepercommander/commands/nested_share_folder/helpers.py +++ b/keepercommander/commands/nested_share_folder/helpers.py @@ -103,6 +103,15 @@ def command_error_handler(cmd_name): raise CommandError(cmd_name, str(exc)) +def normalize_nsf_user_message(message): + """Replace legacy server terminology in user-facing error text.""" + if not message: + return message + return (message + .replace('Keeper Drive', 'Nested Share Folder') + .replace('keeper drive', 'nested share folder')) + + def check_result(result, cmd_name): """Raise ``CommandError`` when *result['success']* is falsy. @@ -110,7 +119,8 @@ def check_result(result, cmd_name): appears in almost every command. """ if not result.get('success'): - raise CommandError(cmd_name, result.get('message', 'Unknown error')) + raise CommandError(cmd_name, normalize_nsf_user_message( + result.get('message', 'Unknown error'))) # ═══════════════════════════════════════════════════════════════════════════ diff --git a/keepercommander/commands/nested_share_folder/record_commands.py b/keepercommander/commands/nested_share_folder/record_commands.py index 6fde8bc25..6a0480646 100644 --- a/keepercommander/commands/nested_share_folder/record_commands.py +++ b/keepercommander/commands/nested_share_folder/record_commands.py @@ -97,7 +97,12 @@ def execute(self, params, **kwargs): return with command_error_handler('nsf-record-add'): - result = _nsf.create_record_v3(params=params, folder_uid=folder_uid, record_data=data) + result = _nsf.create_record_v3( + params=params, + folder_uid=folder_uid, + record_data=data, + record_uid=kwargs.get('record_uid'), + ) check_result(result, 'nsf-record-add') params.sync_data = True return result['record_uid'] diff --git a/keepercommander/commands/pam/vault_target.py b/keepercommander/commands/pam/vault_target.py index 4e25c539c..94c3a272c 100644 --- a/keepercommander/commands/pam/vault_target.py +++ b/keepercommander/commands/pam/vault_target.py @@ -459,6 +459,8 @@ def ensure_pam_folder_path(params, base_folder_uid, relative_path, command='pam' def place_record_in_folder(params, record_uid, folder_uid, command='pam'): + from ..nested_share_folder.helpers import normalize_nsf_user_message + if not folder_uid: raise CommandError(command, 'Target folder UID is required') @@ -470,7 +472,8 @@ def place_record_in_folder(params, record_uid, folder_uid, command='pam'): if is_nested_share_folder(params, folder_uid): result = move_record_v3(params, record_uid, to_folder_uid=folder_uid) if isinstance(result, dict) and result.get('success') is False: - message = result.get('message') or 'Failed to place record in Nested Share Folder' + message = normalize_nsf_user_message(result.get('message')) or \ + 'Failed to place record in Nested Share Folder' lowered = message.casefold() if any(token in lowered for token in ('denied', 'permission', 'forbidden', 'read-only')): message = ( @@ -487,6 +490,7 @@ def create_record_in_folder(params, record, folder_uid=None, command='pam'): if folder_uid and is_nested_share_folder(params, folder_uid): from ... import vault_extensions from ...nested_share_folder.record_api import create_record_v3 + from ..nested_share_folder.helpers import normalize_nsf_user_message if not isinstance(record, vault.TypedRecord): raise CommandError(command, 'Nested Share Folder record creation requires a typed record') @@ -497,7 +501,8 @@ def create_record_in_folder(params, record, folder_uid=None, command='pam'): record_data=vault_extensions.extract_typed_record_data(record), ) if not result.get('success'): - raise CommandError(command, result.get('message') or 'Failed to create record in Nested Share Folder') + raise CommandError(command, normalize_nsf_user_message(result.get('message')) or + 'Failed to create record in Nested Share Folder') record.record_uid = result['record_uid'] api.sync_down(params) else: @@ -505,7 +510,7 @@ def create_record_in_folder(params, record, folder_uid=None, command='pam'): def update_pam_record(params, record, command='pam'): - from ..nested_share_folder.helpers import is_nested_share_record + from ..nested_share_folder.helpers import is_nested_share_record, normalize_nsf_user_message from ...nested_share_folder.record_api import update_record_v3 if is_nested_share_record(params, record.record_uid): @@ -515,30 +520,29 @@ def update_pam_record(params, record, command='pam'): result = update_record_v3( params, record.record_uid, - record_data=vault_extensions.extract_typed_record_data(record), + data=vault_extensions.extract_typed_record_data(record), ) if not result.get('success'): - raise CommandError(command, result.get('message') or 'Failed to update record in Nested Share Folder') + raise CommandError(command, normalize_nsf_user_message(result.get('message')) or + 'Failed to update record in Nested Share Folder') api.sync_down(params) else: record_management.update_record(params, record) def execute_record_add_in_folder(params, args, folder_uid, command='pam'): - """Add a record, placing it in an NSF folder when needed. - - For Nested Share Folders, ``folder`` is removed from *args* before calling - RecordAddCommand because the record is created at the vault root first and - then moved into the NSF via ``place_record_in_folder``. - """ + """Add a record in *folder_uid*, using NSF-native creation when needed.""" from ..record_edit import RecordAddCommand + from ..nested_share_folder.record_commands import NestedShareRecordAddCommand record_args = dict(args or {}) if folder_uid and is_nested_share_folder(params, folder_uid): - record_args.pop('folder', None) - uid = RecordAddCommand().execute(params, **record_args) - if uid and isinstance(uid, str): - place_record_in_folder(params, uid, folder_uid, command=command) + nsf_args = dict(record_args) + nsf_args.pop('folder', None) + nsf_args['folder_uid'] = folder_uid + uid = NestedShareRecordAddCommand().execute(params, **nsf_args) + if uid: + api.sync_down(params) return uid record_args['folder'] = folder_uid @@ -546,22 +550,31 @@ def execute_record_add_in_folder(params, args, folder_uid, command='pam'): def execute_record_v3_add_in_folder(params, args, folder_uid, command='pam'): - """Add a v3 typed record, placing it in an NSF folder when needed. + """Add a v3 typed record in *folder_uid*, using NSF-native creation when needed.""" + import json - For Nested Share Folders, ``folder`` is removed from *args* before calling - RecordAddCommand because the record is created without a legacy folder target - and then placed into the NSF via ``place_record_in_folder``. - """ from ..recordv3 import RecordAddCommand + from ..nested_share_folder.helpers import normalize_nsf_user_message + from ...nested_share_folder.record_api import create_record_v3 record_args = dict(args or {}) if folder_uid and is_nested_share_folder(params, folder_uid): - record_args.pop('folder', None) - uid = RecordAddCommand().execute(params, **record_args) - if not uid: - raise CommandError(command, 'Record creation failed for Nested Share Folder target') - place_record_in_folder(params, uid, folder_uid, command=command) - return uid + data = record_args.get('data') + if not data: + raise CommandError(command, 'Record data is required for Nested Share Folder creation') + if isinstance(data, str): + data = json.loads(data) + result = create_record_v3( + params, + folder_uid=folder_uid, + record_data=data, + record_uid=record_args.get('record_uid') or data.get('uid'), + ) + if not result.get('success'): + raise CommandError(command, normalize_nsf_user_message(result.get('message')) or + 'Failed to create record in Nested Share Folder') + api.sync_down(params) + return result['record_uid'] record_args['folder'] = folder_uid return RecordAddCommand().execute(params, **record_args) diff --git a/keepercommander/commands/pam_import/nsf_helpers.py b/keepercommander/commands/pam_import/nsf_helpers.py index 7458f8384..71af4681d 100644 --- a/keepercommander/commands/pam_import/nsf_helpers.py +++ b/keepercommander/commands/pam_import/nsf_helpers.py @@ -12,11 +12,9 @@ from __future__ import annotations import logging -from typing import Dict, List, Optional -from unittest.mock import patch +from typing import List, Optional from .record_loader import iter_accessible_record_uids, load_pam_record -from .base import add_pam_scripts from ... import api, vault from ...error import CommandError @@ -235,51 +233,4 @@ def create_nsf_subfolder(params, folder_name: str, parent_uid: str = '', def extend_create_record(params, obj, folder_uid: str) -> Optional[str]: """Create a PAM import record in a classic or NSF folder.""" - if not is_nsf_folder_uid(params, folder_uid): - return obj.create_record(params, folder_uid) - - captured: Dict = {} - - class _CapturingAdd: - def execute(self, _params, **kwargs): - captured.clear() - captured.update(kwargs) - return None - - with patch('keepercommander.commands.pam_import.base.RecordEditAddCommand', return_value=_CapturingAdd()): - obj.create_record(params, folder_uid) - - from ..nested_share_folder.record_commands import NestedShareRecordAddCommand - from ..nested_share_folder.helpers import command_error_handler, check_result - from ...nested_share_folder.record_api import create_record_v3 - - cmd = NestedShareRecordAddCommand() - record_fields, add_attachments = cmd._parse_fields(captured.get('fields', [])) - if add_attachments: - logging.warning('File attachments are not yet supported for NSF pam project extend.') - - data = cmd._build_record_data( - params, - captured['record_type'], - captured.get('title', ''), - captured.get('notes'), - record_fields, - ) - - with command_error_handler('pam project extend'): - result = create_record_v3( - params, - folder_uid=folder_uid, - record_data=data, - record_uid=captured.get('record_uid'), - ) - check_result(result, 'pam project extend') - - uid = result.get('record_uid') or captured.get('record_uid') - if uid: - obj.uid = uid - scripts = getattr(obj, 'scripts', None) - if scripts and getattr(scripts, 'scripts', None): - add_pam_scripts(params, uid, scripts.scripts) - params.sync_data = True - return uid + return obj.create_record(params, folder_uid) diff --git a/keepercommander/commands/tunnel_and_connections.py b/keepercommander/commands/tunnel_and_connections.py index b05b5a7d3..2b478e83e 100644 --- a/keepercommander/commands/tunnel_and_connections.py +++ b/keepercommander/commands/tunnel_and_connections.py @@ -36,6 +36,7 @@ wait_for_tunnel_connection, create_rust_webrtc_settings, \ print_above_keeper_prompt from .pam.router_helper import get_dag_leafs +from .pam.vault_target import update_pam_record from .tunnel_registry import ( PARENT_GRACE_SECONDS, is_pid_alive, @@ -431,8 +432,7 @@ def execute(self, params, **kwargs): record.custom.append(record_seed) dirty = True if dirty: - record_management.update_record(params, record) - api.sync_down(params) + update_pam_record(params, record, command='pam tunnel edit') traffic_encryption_key = record.get_typed_field('trafficEncryptionSeed') if not traffic_encryption_key: @@ -504,8 +504,7 @@ def execute(self, params, **kwargs): dirty = True # Persist the record changes (new pamSettings field or port modifications) if dirty: - record_management.update_record(params, record) - api.sync_down(params) + update_pam_record(params, record, command='pam tunnel edit') dirty = False if not tmp_dag.is_tunneling_config_set_up(record_uid): print(f"{bcolors.FAIL}No PAM Configuration UID set. This must be set for tunneling to work. " @@ -555,8 +554,7 @@ def execute(self, params, **kwargs): if dirty: tmp_dag.set_resource_allowed(resource_uid=record_uid, tunneling=_tunneling, allowed_settings_name=allowed_settings_name) - record_management.update_record(params, record) - api.sync_down(params) + update_pam_record(params, record, command='pam tunnel edit') # Print out the tunnel settings if not kwargs.get('silent'): diff --git a/keepercommander/nested_share_folder/folder_record_api.py b/keepercommander/nested_share_folder/folder_record_api.py index 1e64141a5..9070c78fc 100644 --- a/keepercommander/nested_share_folder/folder_record_api.py +++ b/keepercommander/nested_share_folder/folder_record_api.py @@ -13,6 +13,7 @@ encrypt_record_key_for_folder, ) from .folder_api import resolve_folder_identifier +from ..commands.nested_share_folder.helpers import normalize_nsf_user_message logger = logging.getLogger(__name__) @@ -175,7 +176,7 @@ def move_record_v3(params, record_uid, from_folder_uid=None, to_folder_uid=None) r = rs.folderRecordUpdateResult[0] if r.status != folder_pb2.SUCCESS: return _move_failure(record_uid, from_folder_uid, to_folder_uid, - f"Add failed: {r.message}") + f"Add failed: {normalize_nsf_user_message(r.message)}") except Exception as e: return _move_failure(record_uid, from_folder_uid, to_folder_uid, f"Add error: {e}") diff --git a/keepercommander/nested_share_folder/record_api.py b/keepercommander/nested_share_folder/record_api.py index d0b1eb44a..721ac5a6e 100644 --- a/keepercommander/nested_share_folder/record_api.py +++ b/keepercommander/nested_share_folder/record_api.py @@ -148,16 +148,15 @@ def create_record_v3(params, record_type='', title='', fields=None, def update_record_v3(params, record_uid, data=None, title=None, record_type=None, fields=None, notes=None, non_shared_data=None, revision=None): - if record_uid not in params.record_cache: + rec = get_record_from_cache(params, record_uid) + if not rec: from .. import sync_down sync_down.sync_down(params) - if record_uid not in params.record_cache: - raise ValueError(f"Record {record_uid} not found") + rec = get_record_from_cache(params, record_uid) + if not rec: + raise ValueError(f"Record {record_uid} not found") - rec = params.record_cache[record_uid] - rk = rec.get('record_key_unencrypted') - if not rk: - raise ValueError(f"Record key not available for {record_uid}") + rk = rec.get('record_key_unencrypted') or get_record_key(params, record_uid) if data is None: existing = None diff --git a/unit-tests/pam/test_pam_project_import_nsf.py b/unit-tests/pam/test_pam_project_import_nsf.py index 166a1391a..170c88971 100644 --- a/unit-tests/pam/test_pam_project_import_nsf.py +++ b/unit-tests/pam/test_pam_project_import_nsf.py @@ -29,7 +29,7 @@ def _params(): ) -def test_record_add_helper_creates_at_root_then_moves_for_nsf_folder(): +def test_record_add_helper_creates_natively_in_nsf_folder(): params = _params() args = { 'force': True, @@ -38,35 +38,37 @@ def test_record_add_helper_creates_at_root_then_moves_for_nsf_folder(): 'title': 'Admin', } - with patch('keepercommander.commands.record_edit.RecordAddCommand') as record_add, \ - patch('keepercommander.commands.pam.vault_target.place_record_in_folder') as place_record: - record_add.return_value.execute.return_value = 'record_uid' + with patch('keepercommander.commands.nested_share_folder.record_commands.NestedShareRecordAddCommand') as nsf_add, \ + patch('keepercommander.commands.pam.vault_target.api.sync_down') as sync_down: + nsf_add.return_value.execute.return_value = 'record_uid' uid = execute_record_add_in_folder(params, args, 'root_nsf', command='pam-project-import') assert uid == 'record_uid' - record_add.return_value.execute.assert_called_once() - assert 'folder' not in record_add.return_value.execute.call_args.kwargs - place_record.assert_called_once_with(params, 'record_uid', 'root_nsf', command='pam-project-import') + nsf_add.return_value.execute.assert_called_once() + sync_down.assert_called_once_with(params) + call_kwargs = nsf_add.return_value.execute.call_args.kwargs + assert call_kwargs['folder_uid'] == 'root_nsf' + assert call_kwargs['record_type'] == 'pamUser' + assert 'folder' not in call_kwargs -def test_record_v3_add_helper_creates_at_root_then_moves_for_nsf_folder(): +def test_record_v3_add_helper_creates_natively_in_nsf_folder(): params = _params() args = { 'folder': 'root_nsf', 'data': '{"type":"pamUser","title":"Admin","fields":[]}', } - with patch('keepercommander.commands.recordv3.RecordAddCommand') as record_add, \ - patch('keepercommander.commands.pam.vault_target.place_record_in_folder') as place_record: - record_add.return_value.execute.return_value = 'record_uid' - + with patch('keepercommander.nested_share_folder.record_api.create_record_v3', + return_value={'success': True, 'record_uid': 'record_uid'}) as create_record, \ + patch('keepercommander.commands.pam.vault_target.api.sync_down'): uid = execute_record_v3_add_in_folder(params, args, 'root_nsf', command='pam-project-import') assert uid == 'record_uid' - record_add.return_value.execute.assert_called_once() - assert 'folder' not in record_add.return_value.execute.call_args.kwargs - place_record.assert_called_once_with(params, 'record_uid', 'root_nsf', command='pam-project-import') + create_record.assert_called_once() + assert create_record.call_args.kwargs['folder_uid'] == 'root_nsf' + assert create_record.call_args.kwargs['record_data']['type'] == 'pamUser' def test_is_nested_share_folder_detects_reconstructed_subfolder_cache(): From 56ec8c60052e77e9bb616fde7a8ec51b8fe786c1 Mon Sep 17 00:00:00 2001 From: adeshmukh-ks Date: Mon, 13 Jul 2026 18:31:33 +0530 Subject: [PATCH 14/14] unit test fix --- keepercommander/commands/discoveryrotation.py | 42 +++++++++----- keepercommander/commands/pam/vault_target.py | 44 +++++++++++--- keepercommander/commands/pam_import/edit.py | 12 +++- unit-tests/pam/test_pam_nsf_config.py | 58 ++++++++++++++----- .../test_command_enterprise_api_keys.py | 6 +- 5 files changed, 123 insertions(+), 39 deletions(-) diff --git a/keepercommander/commands/discoveryrotation.py b/keepercommander/commands/discoveryrotation.py index 2f3ae988e..4a4ce94b2 100644 --- a/keepercommander/commands/discoveryrotation.py +++ b/keepercommander/commands/discoveryrotation.py @@ -33,11 +33,11 @@ resolve_pam_record, record_exists_in_vault, collect_pam_folder_uids, get_vault_record_title_type, find_pam_records_by_search, resolve_pam_config_folder_info, pam_folder_json_payload, place_record_in_folder, - update_pam_record, records_in_folder, + create_pam_configuration_in_folder, update_pam_record, records_in_folder, ) from .pam.config_helper import configuration_controller_get, \ pam_configurations_get_all, pam_configuration_remove, \ - pam_configuration_create_record_v6, record_rotation_get, \ + record_rotation_get, \ pam_decrypt_configuration_data from .pam.pam_dto import ( @@ -1415,7 +1415,8 @@ def config_user(_dag, target_record, target_resource_uid, target_config_uid=None encrypted_session_token, encrypted_transmission_key, transmission_key = get_keeper_tokens(params) if record_name: - rec = resolve_pam_record(params, record_name) + rec = resolve_pam_record( + params, record_name, rec_type=PamConfigurationEditMixin.ROTATION_TARGET_RECORD_TYPES) if rec: record_uids.add(rec.record_uid) elif record_name in params.record_cache: @@ -1485,16 +1486,24 @@ def config_user(_dag, target_record, target_resource_uid, target_config_uid=None isinstance(x, vault.TypedRecord)} config_uid = kwargs.get('config') - cfg_rec = resolve_pam_record(params, kwargs.get('config', None)) - if cfg_rec and cfg_rec.version == 6 and cfg_rec.record_uid in pam_configurations: + cfg_rec = resolve_pam_record( + params, kwargs.get('config', None), rec_type=PamConfigurationEditMixin.PAM_CONFIG_RECORD_TYPES) + if cfg_rec and cfg_rec.version == 6 and cfg_rec.record_type in PamConfigurationEditMixin.PAM_CONFIG_RECORD_TYPES: config_uid = cfg_rec.record_uid pam_config = None # type: Optional[vault.TypedRecord] if config_uid: if config_uid in pam_configurations: pam_config = pam_configurations[config_uid] + elif cfg_rec and cfg_rec.record_uid == config_uid: + pam_config = cfg_rec else: - raise CommandError('', f'Record uid {config_uid} is not a PAM Configuration record.') + loaded = vault.KeeperRecord.load(params, config_uid) + if (isinstance(loaded, vault.TypedRecord) and loaded.version == 6 + and loaded.record_type in PamConfigurationEditMixin.PAM_CONFIG_RECORD_TYPES): + pam_config = loaded + else: + raise CommandError('', f'Record uid {config_uid} is not a PAM Configuration record.') schedule_data = parse_schedule_data(kwargs) @@ -1529,7 +1538,8 @@ def config_user(_dag, target_record, target_resource_uid, target_config_uid=None pwd_complexity_rule_list = {} resource_uid = kwargs.get('resource') - res_rec = resolve_pam_record(params, kwargs.get('resource', None)) + res_rec = resolve_pam_record( + params, kwargs.get('resource', None), rec_type=PamConfigurationEditMixin.PAM_RESOURCE_RECORD_TYPES) if res_rec and isinstance(res_rec, vault.TypedRecord): resource_uid = res_rec.record_uid @@ -2483,6 +2493,14 @@ def print_root_rotation_setting(params, is_verbose=False, format_type='table'): class PamConfigurationEditMixin(RecordEditMixin): pam_record_types = None + PAM_CONFIG_RECORD_TYPES = frozenset({ + 'pamAwsConfiguration', 'pamAzureConfiguration', 'pamGcpConfiguration', + 'pamDomainConfiguration', 'pamNetworkConfiguration', 'pamOciConfiguration', + }) + PAM_RESOURCE_RECORD_TYPES = frozenset({ + 'pamDatabase', 'pamDirectory', 'pamMachine', 'pamRemoteBrowser', + }) + ROTATION_TARGET_RECORD_TYPES = PAM_RESOURCE_RECORD_TYPES | frozenset({'pamUser'}) def __init__(self): super().__init__() @@ -2882,8 +2900,7 @@ def execute(self, params, **kwargs): self.verify_required(record) - pam_configuration_create_record_v6(params, record, shared_folder_uid) - api.sync_down(params) + create_pam_configuration_in_folder(params, record, shared_folder_uid, command='pam-config-new') encrypted_session_token, encrypted_transmission_key, transmission_key = get_keeper_tokens(params) # Add DAG for configuration @@ -3069,10 +3086,7 @@ class PAMConfigurationRemoveCommand(Command): help='PAM Configuration UID. To view all rotation settings with their UIDs, use command ' '`pam config list`') - _PAM_CONFIG_TYPES = frozenset({ - 'pamAwsConfiguration', 'pamAzureConfiguration', 'pamGcpConfiguration', - 'pamDomainConfiguration', 'pamNetworkConfiguration', 'pamOciConfiguration', - }) + _PAM_CONFIG_TYPES = PamConfigurationEditMixin.PAM_CONFIG_RECORD_TYPES def get_parser(self): return PAMConfigurationRemoveCommand.parser @@ -4281,7 +4295,7 @@ class PAMCreateGatewayCommand(Command): help='Name of the Gateway', action='store') dr_create_controller_parser.add_argument('--application', '-a', required=True, dest='ksm_app', - help='KSM Application name, UID, or NSF path. Use ' + help='KSM Application name or UID. Use ' '`secrets-manager app list` to view available applications.', action='store') dr_create_controller_parser.add_argument('--token-expires-in-min', '-e', type=int, dest='token_expire_in_min', diff --git a/keepercommander/commands/pam/vault_target.py b/keepercommander/commands/pam/vault_target.py index 94c3a272c..7dba2e324 100644 --- a/keepercommander/commands/pam/vault_target.py +++ b/keepercommander/commands/pam/vault_target.py @@ -301,22 +301,38 @@ def record_exists_in_vault(params, record_uid): or is_nested_share_record(params, record_uid)) +def _record_matches_type(record, rec_type): + # type: (...) -> bool + if not rec_type: + return True + record_type = getattr(record, 'record_type', None) + if isinstance(rec_type, str): + return record_type == rec_type + try: + return record_type in rec_type + except TypeError: + return record_type == rec_type + + def resolve_pam_record(params, identifier, rec_type=None): # type: (...) -> Optional[vault.KeeperRecord] - """Resolve a record by UID, folder path, or title including Nested Share Records.""" + """Resolve a record by UID, folder path, or title including Nested Share Records. + + *rec_type* may be a single record type string or a collection of allowed types. + """ if not identifier: return None if identifier in getattr(params, 'record_cache', {}): rec = vault.KeeperRecord.load(params, identifier) - if rec and (not rec_type or rec.record_type == rec_type): + if rec and _record_matches_type(rec, rec_type): return rec from ...nested_share_folder import removal_api as _nsf resolved_uid = _nsf.resolve_nested_share_record_uid(params, identifier) if resolved_uid: rec = vault.KeeperRecord.load(params, resolved_uid) - if rec and (not rec_type or rec.record_type == rec_type): + if rec and _record_matches_type(rec, rec_type): return rec rs = try_resolve_path(params, identifier) @@ -324,11 +340,12 @@ def resolve_pam_record(params, identifier, rec_type=None): folder, record_title = rs if folder is not None and record_title is not None: folder_uid = folder.uid or '' - if folder_uid in params.subfolder_record_cache: - for uid in params.subfolder_record_cache[folder_uid]: + subfolder_cache = getattr(params, 'subfolder_record_cache', None) or {} + if folder_uid in subfolder_cache: + for uid in subfolder_cache[folder_uid]: record = vault.KeeperRecord.load(params, uid) if record and record.title.casefold() == record_title.casefold(): - if not rec_type or record.record_type == rec_type: + if _record_matches_type(record, rec_type): return record l_name = identifier.casefold() @@ -336,7 +353,7 @@ def resolve_pam_record(params, identifier, rec_type=None): for record_uid in getattr(params, 'record_cache', {}): record = vault.KeeperRecord.load(params, record_uid) if record and record.title.casefold() == l_name: - if not rec_type or record.record_type == rec_type: + if _record_matches_type(record, rec_type): matches.append(record) if len(matches) > 1: break @@ -509,6 +526,19 @@ def create_record_in_folder(params, record, folder_uid=None, command='pam'): record_management.add_record_to_folder(params, record, folder_uid) +def create_pam_configuration_in_folder(params, record, folder_uid, command='pam-config-new'): + """Create a v6 PAM configuration in *folder_uid* using NSF or legacy placement.""" + from .config_helper import pam_configuration_create_record_v6 + + if is_nested_share_folder(params, folder_uid): + create_record_in_folder(params, record, folder_uid, command=command) + return + + pam_configuration_create_record_v6(params, record, folder_uid) + api.sync_down(params) + place_record_in_folder(params, record.record_uid, folder_uid, command=command) + + def update_pam_record(params, record, command='pam'): from ..nested_share_folder.helpers import is_nested_share_record, normalize_nsf_user_message from ...nested_share_folder.record_api import update_record_v3 diff --git a/keepercommander/commands/pam_import/edit.py b/keepercommander/commands/pam_import/edit.py index 5ec244d0c..f96d226d7 100644 --- a/keepercommander/commands/pam_import/edit.py +++ b/keepercommander/commands/pam_import/edit.py @@ -224,6 +224,13 @@ def _has_perms(section_key): PAM_ROOT_FOLDER_NAME = "PAM Environments" + @staticmethod + def _pam_folder_types(use_nsf=False): + types = {BaseFolderNode.UserFolderType} + if use_nsf: + types.add(BaseFolderNode.NestedShareFolderType) + return types + def process_folders(self, params, project: dict) -> dict: res = { "root_folder_target": self.PAM_ROOT_FOLDER_NAME, @@ -246,10 +253,11 @@ def process_folders(self, params, project: dict) -> dict: # FolderListCommand().execute(params, folders_only=True, pattern="/") use_nsf = project["options"].get("use_nsf", False) is True + allowed_types = self._pam_folder_types(use_nsf) folders = self.find_folders(params, "", res["root_folder_target"], False) if folders: # select first non-root non-shared sub/folder - folders = [x for x in folders if x.type == x.UserFolderType] + folders = [x for x in folders if x.type in allowed_types] if use_nsf: folders = [x for x in folders if is_nested_share_folder(params, x.uid)] else: @@ -274,7 +282,7 @@ def process_folders(self, params, project: dict) -> dict: while True: folder_name = res["project_folder_target"] if n <= START_INDEX else f"""{res["project_folder_target"]} #{n}""" folders = self.find_folders(params, res["root_folder_uid"], folder_name, False) - folders = [x for x in folders if x.type == x.UserFolderType] + folders = [x for x in folders if x.type in allowed_types] n += 1 if len(folders) > 0: continue diff --git a/unit-tests/pam/test_pam_nsf_config.py b/unit-tests/pam/test_pam_nsf_config.py index 2a40a8623..bf4b103c6 100644 --- a/unit-tests/pam/test_pam_nsf_config.py +++ b/unit-tests/pam/test_pam_nsf_config.py @@ -7,7 +7,8 @@ PAMConfigurationEditCommand, PAMConfigurationNewCommand, PAMConfigurationRemoveCommand, PAMCreateRecordRotationCommand) from keepercommander.commands.pam.vault_target import ( - create_record_in_folder, place_record_in_folder, resolve_pam_folder_uid, records_in_folder) + create_pam_configuration_in_folder, create_record_in_folder, place_record_in_folder, + resolve_pam_folder_uid, records_in_folder) from keepercommander.error import CommandError from keepercommander.subfolder import NestedShareFolderNode, RootFolderNode, SharedFolderNode @@ -138,6 +139,39 @@ def test_records_in_folder_merges_legacy_and_nsf_membership(self): {'legacy_rec_uid', 'nsf_rec_uid'}, ) + @mock.patch('keepercommander.commands.pam.vault_target.create_record_in_folder') + def test_create_pam_configuration_in_folder_uses_nsf_create(self, mock_create_in_folder): + params = _make_params() + params.nested_share_folders['nsf_folder'] = {'name': 'Project - Users'} + folder = NestedShareFolderNode() + folder.uid = 'nsf_folder' + params.folder_cache[folder.uid] = folder + record = vault.TypedRecord(version=6) + + create_pam_configuration_in_folder(params, record, 'nsf_folder', command='pam-config-new') + + mock_create_in_folder.assert_called_once_with( + params, record, 'nsf_folder', command='pam-config-new') + + @mock.patch('keepercommander.commands.pam.vault_target.place_record_in_folder') + @mock.patch('keepercommander.commands.pam.vault_target.api.sync_down') + @mock.patch('keepercommander.commands.pam.config_helper.pam_configuration_create_record_v6') + def test_create_pam_configuration_in_folder_uses_legacy_create_and_place( + self, mock_create_v6, mock_sync, mock_place): + params = _make_params() + folder = SharedFolderNode() + folder.uid = 'legacy_folder' + params.folder_cache[folder.uid] = folder + record = vault.TypedRecord(version=6) + record.record_uid = 'config_uid' + + create_pam_configuration_in_folder(params, record, 'legacy_folder', command='pam-config-new') + + mock_create_v6.assert_called_once_with(params, record, 'legacy_folder') + mock_sync.assert_called_once_with(params) + mock_place.assert_called_once_with( + params, 'config_uid', 'legacy_folder', command='pam-config-new') + class TestPamConfigNewNsfPlacement(unittest.TestCase): @@ -169,15 +203,14 @@ def test_parse_pam_configuration_accepts_nsf_folder_name(self): field = record.get_typed_field('pamResources') self.assertEqual(field.value[0]['folderUid'], 'nsf_folder') - @mock.patch('keepercommander.commands.discoveryrotation.api.sync_down') @mock.patch('keepercommander.commands.discoveryrotation.TunnelDAG') @mock.patch('keepercommander.commands.discoveryrotation.get_keeper_tokens', return_value=(b'encrypted_session', b'encrypted_key', b'transmission_key')) - @mock.patch('keepercommander.commands.discoveryrotation.pam_configuration_create_record_v6') + @mock.patch('keepercommander.commands.discoveryrotation.create_pam_configuration_in_folder') @mock.patch('keepercommander.commands.discoveryrotation.RecordEditMixin.get_record_type_fields', return_value=[]) def test_execute_creates_new_config_in_nsf_folder( - self, mock_record_fields, mock_create_config, mock_tokens, mock_dag, mock_sync): + self, mock_record_fields, mock_create_config, mock_tokens, mock_dag): params = _make_params() params.nested_share_folders['nsf_folder'] = { 'name': 'Project - Users', @@ -193,7 +226,7 @@ def parse_properties(_, record, **kwargs): record.fields.append(vault.TypedField.new_field( 'pamResources', {'folderUid': 'nsf_folder'})) - def create_config(_, record, folder_uid): + def create_config(_, record, folder_uid, command='pam-config-new'): record.record_uid = 'config_uid' mock_create_config.side_effect = create_config @@ -202,18 +235,17 @@ def create_config(_, record, folder_uid): result = command.execute(params, config_type='local', title='Config') self.assertEqual(result, 'config_uid') - mock_create_config.assert_called_once_with(params, mock.ANY, 'nsf_folder') - mock_sync.assert_called_once_with(params) + mock_create_config.assert_called_once_with( + params, mock.ANY, 'nsf_folder', command='pam-config-new') - @mock.patch('keepercommander.commands.discoveryrotation.api.sync_down') @mock.patch('keepercommander.commands.discoveryrotation.TunnelDAG') @mock.patch('keepercommander.commands.discoveryrotation.get_keeper_tokens', return_value=(b'encrypted_session', b'encrypted_key', b'transmission_key')) - @mock.patch('keepercommander.commands.discoveryrotation.pam_configuration_create_record_v6') + @mock.patch('keepercommander.commands.discoveryrotation.create_pam_configuration_in_folder') @mock.patch('keepercommander.commands.discoveryrotation.RecordEditMixin.get_record_type_fields', return_value=[]) def test_execute_creates_new_config_in_legacy_folder( - self, mock_record_fields, mock_create_config, mock_tokens, mock_dag, mock_sync): + self, mock_record_fields, mock_create_config, mock_tokens, mock_dag): params = _make_params() folder = SharedFolderNode() folder.uid = 'legacy_folder' @@ -224,7 +256,7 @@ def parse_properties(_, record, **kwargs): record.fields.append(vault.TypedField.new_field( 'pamResources', {'folderUid': 'legacy_folder'})) - def create_config(_, record, folder_uid): + def create_config(_, record, folder_uid, command='pam-config-new'): record.record_uid = 'config_uid' mock_create_config.side_effect = create_config @@ -233,8 +265,8 @@ def create_config(_, record, folder_uid): result = command.execute(params, config_type='local', title='Config') self.assertEqual(result, 'config_uid') - mock_create_config.assert_called_once_with(params, mock.ANY, 'legacy_folder') - mock_sync.assert_called_once_with(params) + mock_create_config.assert_called_once_with( + params, mock.ANY, 'legacy_folder', command='pam-config-new') class TestPamConfigEditNsfPlacement(unittest.TestCase): diff --git a/unit-tests/test_command_enterprise_api_keys.py b/unit-tests/test_command_enterprise_api_keys.py index e90108927..f3de54eff 100644 --- a/unit-tests/test_command_enterprise_api_keys.py +++ b/unit-tests/test_command_enterprise_api_keys.py @@ -100,7 +100,7 @@ def test_api_key_list_json_format(self): "name": "SIEM Tool", "status": "Active", "issued_date": "2025-07-08 14:16:07", - "expiration_date": "2026-07-08 14:16:07", + "expiration_date": "2099-07-08 14:16:07", "integration": "SIEM:2" }, { @@ -661,13 +661,13 @@ def communicate_rest_success(params, request, path, rs_type=None): integration5.apiIntegrationTypeName = "SIEM" integration5.actionType = 2 - # Token 53 - Active + # Token 53 - Active (expiration far in the future so status stays Active) token4 = rs.tokens.add() token4.token = "active_token_53" token4.name = "SIEM Tool" token4.enterprise_id = 8560 token4.issuedDate = int(datetime.datetime(2025, 7, 8, 14, 16, 7).timestamp() * 1000) - token4.expirationDate = int(datetime.datetime(2026, 7, 8, 14, 16, 7).timestamp() * 1000) + token4.expirationDate = int(datetime.datetime(2099, 7, 8, 14, 16, 7).timestamp() * 1000) integration7 = token4.integrations.add() integration7.roleName = "SIEM" integration7.apiIntegrationTypeName = "SIEM"